Finer-grained control over service methods (Pecl Request #11944)

On 8 Sep, 09:30, Graham Charters <gchart...googlemail.com> wrote:
> Just a quick status update...
>
> I've done the code to optionally allow an interface to
be specified
> and tested this independent of a protocol binding and
all works fine.
> Unfortunately, when called from a remote invocation,
the classexists
> tests for the service implementation fails. 
Get_declared_classes()
> confirms that it does not have the definition and
> get_declared_interfaces() confirms that it knows about
the
> interfaces.  Removing the interfaces makes everything
work as before,
> so it appears to be a PHP nit.
>
> I'm going to try to create a little test case to
reproduce this
> outside SCA.  If anyone has heard of anything which
might be relevant,
> please let me know.
>
> Graham.
>
> On 3 Sep, 13:18, Graham Charters <gchart...googlemail.com> wrote:
>
> > It sounds like we may have consensus.  To
summarise:
>
> > 1.  We should add the ability to specify an
optional interface for the
> > service on the service annotation (e.g.
service
> > MyServiceInterface).  This would only be used to
limit the methods
> > exposed by the service, so we would not look for
any other metadata in
> > the interface (we could support this later).
> > 2.  We should not include methods in service
descriptions which have
> > not been appropriately documented (this is to
avoid generating duff
> > services descriptions, and is not meant to be used
as a means of
> > controlling the service interface).
>
> > Hopefully I'll be able to take a look at 1
sometime towards the end of
> > this week, unless someone else is itching to do it
:-D
>
> > Graham.
>
> > On 31 Aug, 15:03, Matthew Schultz <matt...gmail.com> wrote:> Actually after a second
glance, I see all annotations are still set in
> > > the class.  It probably wouldn't make any
sense to have SCA parse
> > > annotations in the interface.
>
> > > Matt
>
> > > On 31 Aug, 06:20, Graham Charters
<gchart...googlemail.com> wrote:> Comments
inlined below...
>
> > > > On 31 Aug, 11:35, simonsl...googlemail.com wrote:
>
> > > > > Hi Graham
>
> > > > > Some more comments in line...
>
> > > > > On 31 Aug, 11:26, Graham Charters
<gchart...googlemail.com> wrote:
>
> > > > > > Hi Simon, thanks for the rapid
comments.  Here's my thoughts on the
> > > > > > two issues you identified:
>
> > > > > > > 1/ What should SCA do if
it finds a method without annotations, i.e.
> > > > > > > no type information
>
> > > > > > This probably depends on the
type of service.  Service types which
> > > > > > don't have a service
description (e.g. local, REST and Atom), don't
> > > > > > require this information to be
specified.  Service types which do have
> > > > > > service descriptions (soap,
json-rpc, xml-rpc), do (to varying
> > > > > > degrees).
>
> > > > > > I think if the binding type
requires documentation then we should warn
> > > > > > and not generate (rather than
generating something which is of no
> > > > > > use).  If the binding type
doesn't require documentation then it's
> > > > > > included.
>
> > > > > > > 2/ How can methods be
omitted from the service interface, i.e. we just
> > > > > > > don't want to expose the
method
>
> > > > > > I don't think the absence of
comments should be used as a control
> > > > > > mechanism for the reason you
and Matt have already highlighted (might
> > > > > > want to document something but
still not have it on the service
> > > > > > interface).
>
> > > > > > I quite like the idea of using
interfaces to add this level of
> > > > > > control.  It would also be
consistent with SCA (SCA Java does this and
> > > > > > also supports the same class
implementation approach we use (i.e.
> > > > > > where there is no interface)).
 How about something like:
>
> > > > > > /**
> > > > > >  * Scaffold implementation for
a remote StockQuote Web service.
> > > > > >  *
> > > > > >  * service
StockQuoteInterface
> > > > > >  * binding.soap
> > > > > >  *
> > > > > >  */
> > > > > > StockQuoteImpl implements
StockQuoteInterface, ANOtherInterface {
> > > > > >   ...
>
> > > > > > }
>
> > > > > So I like service
StokeQuoteInterface
>
> > > > > If you are suggesting that we can
parse the StockQuoteInterface to
> > > > > pull out the method names which
should be provided as service
> > > > > interfaces (without the need for
further annotation on the interface
> > > > > itself) then this could work. My
approach was a contraction of this by
> > > > > just providing the method names
after the interface name in the
> > > > > annotation but your approach is
more forward thinking.
>
> > > > We should be able to update this in
SCA_AnnotationReader.php.  I
> > > > believe all the information is available
through Reflection.  You can
> > > > call getInterfaces() on the
ReflectionClass, which returns an array of
> > > > ReflectionClass objects, one for each
interface.  I don't think we
> > > > would require further annotations in the
interface, but we will need
> > > > to decided whether to use
annotations/documentation in an interface if
> > > > it is provided.
>
> > > > > Presumably the default would be to
provide all methods if no interface
> > > > > is described.
>
> > > > Yes, so this would be backwards
compatible.
>
> > > > > > This would be taken to mean
that only those methods defined on
> > > > > > StockQuoteInterface are part
of the soap service.  Those in
> > > > > > ANOtherInterface or just in
StockQuoteImpl would be excluded.  I'd
> > > > > > prefer to pursue this approach
rather than creating a new annotations
> > > > > > which may go away in the near
future.
>
> > > > > > Does this make sense and seem
sensible?
>
> > > > > > On 31 Aug, 09:30,
simonsl...googlemail.com wrote:
>
> > > > > > > On 31 Aug, 08:42, Graham
Charters <gchart...googlemail.com> wrote:
>
> > > > > > > > Pecl Request #11944
(http://pecl
.php.net/bugs/bug.php?id=11944) is
> > > > > > > > asking for
finer-grained control over the methods which are surfaced
> > > > > > > > on a service
interface.  We currently just use class visibility (i.e.
> > > > > > > > all public methods)
to control this.
>
> > > > > > > > There's a small
amount of discussion in the request, but I thought it
> > > > > > > > would be good to
raise it here to get greater visibility and gather
> > > > > > > > more
options/opinions.
>
> > > > > > > > Graham.
>
> > > > > > > Following up on the
comments made in the feature request...
>
> > > > > > > It is true that in the
Java SCA implementation the Service
> > > > > > > information is associated
with interfaces so a class will implementat
> > > > > > > one, or more, interfaces
and this tells the runtime which methods of
> > > > > > > the class should be
treated as service methods.
>
> > > > > > > This is not currently how
the PHP SCA implementation works. All
> > > > > > > annotations are placed on
the class itself. This leads to a level of
> > > > > > > simplicity in service
construction but we pay for this in lack of
> > > > > > > flexibility, for example,
in excluding some methods from the service
> > > > > > > interface. At least given
the set of annotations we have at the
> > > > > > > moment.
>
> > > > > > > Having said this I'm not
convinced that the use of unannotated (is
> > > > > > > that a word?) methods as
part of the service interface makes a lot of
> > > > > > > sense give the way that
the implementation works at the moment. If you
> > > > > > > look at what is actually
generated in the WSDL in the case of the
> > > > > > > method "function
foo($bar)" in the feature request it doesn't seem to
> > > > > > > be that useful. I.e. it
defines null input and output types. I assume
> > > > > > > this is because there are
no annotations for SCA to use for typing the
> > > > > > > interface. Fine for PHP
but not so great for a service interface.
>
> > > > > > > So there are two issues
here
>
> > > > > > > 1/ What should SCA do if
it finds a method without annotations, i.e.
> > > > > > > no type information
> > > > > > > 2/ How can methods be
omitted from the service interface, i.e. we just
> > > > > > > don't want to expose the
method
>
> > > > > > > As first suggested we
could omit methods that don't have comments at
> > > > > > > all.. However this is
problematic for issue 2 as annotations may have
> > > > > > > been included for
producing the documentation that the annotations
> > > > > > > were originally designed
for. However I think we should consider
> > > > > > > omitting methods without
annotations due to issue 1 so this could be a
> > > > > > > short term solution 2.
>
> > > > > > > Following the
conversation on for issue 2. Maybe, as an alternative to
> > > > > > > scaExcludeMethod we could
defined some new annotation for the header
> > > > > > > block that works as an
alternative to defining an interface (we should
> > > > > > > look whether interfaces
could be made to work), e.g.
>
> > > > > > > /**
> > > > > > >  * Scaffold
implementation for a remote StockQuote Web service.
> > > > > > >  *
> > > > > > >  * service
> > > > > > >  * serviceinterface GetQuote
getQuote
> > > > > > >  * binding.soap
> > > > > > >  *
> > > > > > >  */
>
> > > > > > >  If these don't appear
then the default would be to operate as now
> > > > > > > with all of the
(annotated) methods being parsed. The intention here
> > > > > > > being to replace/augment
this with annotations on interfaces if/'when
> > > > > > > they work.
>
> > > > > > > Regards
>
> > > > > > > Simon

Doesn't ring any bells with me. In this situation does it
think the
class has been declared once the class has been used?

Simon


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the
Google Groups "phpsoa" group.
To post to this group, send email to phpsoagooglegroups.com
To unsubscribe from this group, send email to
phpsoa-unsubscribegooglegroups.com
For more options, visit this group at http://
groups.google.co.uk/group/phpsoa?hl=en
-~----------~----~----~----~------~----~------~--~---

I've just created the test case and reproduced the behaviour
(I won't
say "problem" because I think perhaps what we
recommend today might be
the real problem).  I tried something which I thought I'd
already
attempted and it worked.  So here's the explanation.

It seems having interfaces changes when the class gets
defined.  We
currently suggest the following code layout for a service
implementation:

/* includes for anything the service impl requires */
include 'stuff.php';

// this must be the last include
include 'SCA/SCA.php';

class MyService {}

This all works fine.  SCA::initComponent(...) runs in the
include of
SCA.php and it tests for classexists('MyService'); and finds
that it
does exist.

The problem comes when we add an interface, as shown below:

/* includes for anything the service impl requires */
include 'stuff.php';

// this must be the last include
include 'SCA/SCA.php';

interface MyServiceInterface {}

class MyService implements MyServiceInterface {}

In the above example, the test for classexists('MyService')
fails.
Get_declared_classes() confirms it doesn't exist and
get_declared_interfaces() shows that the interface does
exist.  The
simple/obvious fix, which I thought I'd tried last week, but
clearly
hadn't is to move the include after the class declaration,
as follows:

/* includes for anything the service impl requires */
include 'stuff.php';

interface MyServiceInterface {}

class MyService implements MyServiceInterface {}

// this must be the last include
include 'SCA/SCA.php';

I need to turn my tests into phpunit tests and merge with
the latest
in pecl.  I'm hoping I'll be able to do that tonight.

Graham.

On 10 Sep, 16:14, Caroline Maynard <c...php.net> wrote:
> Graham Charters wrote:
> > Just a quick status update...
>
> > I've done the code to optionally allow an
interface to be specified
> > and tested this independent of a protocol binding
and all works fine.
> > Unfortunately, when called from a remote
invocation, the classexists
> > tests for the service implementation fails. 
Get_declared_classes()
> > confirms that it does not have the definition and
> > get_declared_interfaces() confirms that it knows
about the
> > interfaces.  Removing the interfaces makes
everything work as before,
> > so it appears to be a PHP nit.
>
> There are some peculiarities in this area, and people
familiar with
> inheritance from other languages are likely to have
expectations which
> are not satisfied by PHP. For example, there are
restrictions on how you
> can use inherited interfaces in type hints, which
result in fatal errors
> for code you might reasonably expect to work. It
doesn't sound as if
> that's the problem here, but it may be something
similar.
>
> > I'm going to try to create a little test case to
reproduce this
> > outside SCA.
>
> That would help.


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the
Google Groups "phpsoa" group.
To post to this group, send email to phpsoagooglegroups.com
To unsubscribe from this group, send email to
phpsoa-unsubscribegooglegroups.com
For more options, visit this group at http://
groups.google.co.uk/group/phpsoa?hl=en
-~----------~----~----~----~------~----~------~--~---

Graham Charters wrote:
> I've just created the test case and reproduced the
behaviour (I won't
> say "problem" because I think perhaps what we
recommend today might be
> the real problem).  I tried something which I thought
I'd already
> attempted and it worked.  So here's the explanation.
> 
> It seems having interfaces changes when the class gets
defined.  We
> currently suggest the following code layout for a
service
> implementation:
> 
> /* includes for anything the service impl requires */
> include 'stuff.php';
> 
> // this must be the last include
> include 'SCA/SCA.php';
> 
> class MyService {}
> 
> This all works fine.  SCA::initComponent(...) runs in
the include of
> SCA.php and it tests for class_exists('MyService'); and
finds that it
> does exist.
> 
> The problem comes when we add an interface, as shown
below:
> 
> /* includes for anything the service impl requires */
> include 'stuff.php';
> 
> // this must be the last include
> include 'SCA/SCA.php';
> 
> interface MyServiceInterface {}
> 
> class MyService implements MyServiceInterface {}
> 
> In the above example, the test for
class_exists('MyService') fails.
> get_declared_classes() confirms it doesn't exist and
> get_declared_interfaces() shows that the interface does
exist.  The
> simple/obvious fix, which I thought I'd tried last
week, but clearly
> hadn't is to move the include after the class
declaration, as follows:
> 
> /* includes for anything the service impl requires */
> include 'stuff.php';
> 
> interface MyServiceInterface {}
> 
> class MyService implements MyServiceInterface {}
> 
> // this must be the last include
> include 'SCA/SCA.php';

It's good that you've got this to work now.

Are you concluding that this is a "documentation
error", and we should 
just modify the instructions and examples for how to include
SCA?

I can't see an alternative. This is a consequence of the php
internals 
design, in that a derived class (whether by extending a base
class or 
implementing an interface) is not processed at compile-time.
So

$my_foobar = new FooBar();
class FooBar {}

is fine. But

$my_bar = new Bar();
interface Foo {}
class Bar implements Foo {}

fails. You have to reorder it thus:

interface Foo {}
class Bar implements Foo {}
$my_bar = new Bar();

I can't see a way round this (other than to submit a patch
for the 
engine  ) -
there's no way to force Bar to processed at compile time.

You could go the other way though, and exploit this
behaviour to force 
SCA to be deferred.


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the
Google Groups "phpsoa" group.
To post to this group, send email to phpsoagooglegroups.com
To unsubscribe from this group, send email to
phpsoa-unsubscribegooglegroups.com
For more options, visit this group at http://
groups.google.co.uk/group/phpsoa?hl=en
-~----------~----~----~----~------~----~------~--~---

>
> You could go the other way though, and exploit this
behaviour to force
> SCA to be deferred.

Interesting idea, so I thought I'd try it.  I had my SCA
class
implement an empty interface, and sadly, it made no
difference.  The
call to the static SCA::initComponent method is after the
definition
of the SCA class and I think that is sufficient for the SCA
class to
be defined irrespective of whether or not there is an
interface.  If I
move the SCA::initComponent to before the SCA class
definition then I
get a class not found fatal error for the SCA class.

Is this what you were suggesting, or have I missed the
point?

Graham.

On 11 Sep, 11:28, Caroline Maynard <c...php.net> wrote:
> Graham Charters wrote:
> > I've just created the test case and reproduced the
behaviour (I won't
> > say "problem" because I think perhaps
what we recommend today might be
> > the real problem).  I tried something which I
thought I'd already
> > attempted and it worked.  So here's the
explanation.
>
> > It seems having interfaces changes when the class
gets defined.  We
> > currently suggest the following code layout for a
service
> > implementation:
>
> > /* includes for anything the service impl requires
*/
> > include 'stuff.php';
>
> > // this must be the last include
> > include 'SCA/SCA.php';
>
> > class MyService {}
>
> > This all works fine.  SCA::initComponent(...) runs
in the include of
> > SCA.php and it tests for
class_exists('MyService'); and finds that it
> > does exist.
>
> > The problem comes when we add an interface, as
shown below:
>
> > /* includes for anything the service impl requires
*/
> > include 'stuff.php';
>
> > // this must be the last include
> > include 'SCA/SCA.php';
>
> > interface MyServiceInterface {}
>
> > class MyService implements MyServiceInterface {}
>
> > In the above example, the test for
class_exists('MyService') fails.
> > get_declared_classes() confirms it doesn't exist
and
> > get_declared_interfaces() shows that the interface
does exist.  The
> > simple/obvious fix, which I thought I'd tried last
week, but clearly
> > hadn't is to move the include after the class
declaration, as follows:
>
> > /* includes for anything the service impl requires
*/
> > include 'stuff.php';
>
> > interface MyServiceInterface {}
>
> > class MyService implements MyServiceInterface {}
>
> > // this must be the last include
> > include 'SCA/SCA.php';
>
> It's good that you've got this to work now.
>
> Are you concluding that this is a "documentation
error", and we should
> just modify the instructions and examples for how to
include SCA?
>
> I can't see an alternative. This is a consequence of
the php internals
> design, in that a derived class (whether by extending a
base class or
> implementing an interface) is not processed at
compile-time. So
>
> $my_foobar = new FooBar();
> class FooBar {}
>
> is fine. But
>
> $my_bar = new Bar();
> interface Foo {}
> class Bar implements Foo {}
>
> fails. You have to reorder it thus:
>
> interface Foo {}
> class Bar implements Foo {}
> $my_bar = new Bar();
>
> I can't see a way round this (other than to submit a
patch for the
> engine  ) -
there's no way to force Bar to processed at compile time.
>
> You could go the other way though, and exploit this
behaviour to force
> SCA to be deferred.


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the
Google Groups "phpsoa" group.
To post to this group, send email to phpsoagooglegroups.com
To unsubscribe from this group, send email to
phpsoa-unsubscribegooglegroups.com
For more options, visit this group at http://
groups.google.co.uk/group/phpsoa?hl=en
-~----------~----~----~----~------~----~------~--~---

I've checked the code and unit tests into FULMAR.  Couldn't
get round
the PHP difference in behaviour for defining classes with
interfaces,
so services using this feature will have to put the SCA.php
include
after the definition of the class.

I'll update the bug.

Graham.

On 11 Sep, 16:29, Graham Charters <gchart...googlemail.com> wrote:
> > You could go the other way though, and exploit
this behaviour to force
> > SCA to be deferred.
>
> Interesting idea, so I thought I'd try it.  I had my
SCA class
> implement an empty interface, and sadly, it made no
difference.  The
> call to the static SCA::initComponent method is after
the definition
> of the SCA class and I think that is sufficient for the
SCA class to
> be defined irrespective of whether or not there is an
interface.  If I
> move the SCA::initComponent to before the SCA class
definition then I
> get a class not found fatal error for the SCA class.
>
> Is this what you were suggesting, or have I missed the
point?
>
> Graham.
>
> On 11 Sep, 11:28, Caroline Maynard <c...php.net> wrote:> Graham Charters wrote:
> > > I've just created the test case and
reproduced the behaviour (I won't
> > > say "problem" because I think
perhaps what we recommend today might be
> > > the real problem).  I tried something which I
thought I'd already
> > > attempted and it worked.  So here's the
explanation.
>
> > > It seems having interfaces changes when the
class gets defined.  We
> > > currently suggest the following code layout
for a service
> > > implementation:
>
> > > /* includes for anything the service impl
requires */
> > > include 'stuff.php';
>
> > > // this must be the last include
> > > include 'SCA/SCA.php';
>
> > > class MyService {}
>
> > > This all works fine.  SCA::initComponent(...)
runs in the include of
> > > SCA.php and it tests for
class_exists('MyService'); and finds that it
> > > does exist.
>
> > > The problem comes when we add an interface,
as shown below:
>
> > > /* includes for anything the service impl
requires */
> > > include 'stuff.php';
>
> > > // this must be the last include
> > > include 'SCA/SCA.php';
>
> > > interface MyServiceInterface {}
>
> > > class MyService implements MyServiceInterface
{}
>
> > > In the above example, the test for
class_exists('MyService') fails.
> > > get_declared_classes() confirms it doesn't
exist and
> > > get_declared_interfaces() shows that the
interface does exist.  The
> > > simple/obvious fix, which I thought I'd tried
last week, but clearly
> > > hadn't is to move the include after the class
declaration, as follows:
>
> > > /* includes for anything the service impl
requires */
> > > include 'stuff.php';
>
> > > interface MyServiceInterface {}
>
> > > class MyService implements MyServiceInterface
{}
>
> > > // this must be the last include
> > > include 'SCA/SCA.php';
>
> > It's good that you've got this to work now.
>
> > Are you concluding that this is a
"documentation error", and we should
> > just modify the instructions and examples for how
to include SCA?
>
> > I can't see an alternative. This is a consequence
of the php internals
> > design, in that a derived class (whether by
extending a base class or
> > implementing an interface) is not processed at
compile-time. So
>
> > $my_foobar = new FooBar();
> > class FooBar {}
>
> > is fine. But
>
> > $my_bar = new Bar();
> > interface Foo {}
> > class Bar implements Foo {}
>
> > fails. You have to reorder it thus:
>
> > interface Foo {}
> > class Bar implements Foo {}
> > $my_bar = new Bar();
>
> > I can't see a way round this (other than to submit
a patch for the
> > engine  ) -
there's no way to force Bar to processed at compile time.
>
> > You could go the other way though, and exploit
this behaviour to force
> > SCA to be deferred.


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the
Google Groups "phpsoa" group.
To post to this group, send email to phpsoagooglegroups.com
To unsubscribe from this group, send email to
phpsoa-unsubscribegooglegroups.com
For more options, visit this group at http://
groups.google.co.uk/group/phpsoa?hl=en
-~----------~----~----~----~------~----~------~--~---