Ok, so I found a problem with my solution once I tried to put it into
real use... it doesn't play nice with service overrides, because they
are implemented in an object provider which uses the master object
provider...
There is a problem here. I'm using @Match("*") and since the object
providers are services they match. This creates a loop, I tried using
@Local, but that doesn't change the fact that it's trying to go to the
object providers to do the injection so still fails. Specifying
@InjectService (or @Named) seems to be the only way to short circuit
the issue because my dependencies don't need to use the
MasterObjectProvider.
Although I made them services so that the functionality could be
overridden, but that doesn't seem possible since service override is
an object provider and using @InjectService by passes service
override.
catch 22.
Josh
On Thu, Feb 3, 2011 at 7:57 AM, Josh Canfield <[email protected]> wrote:
> I'm not in front of my computer to test yet, but I believe that
> @InjectService would have the same problem because its exactly the same code
> except it gets the id from the annotation.
>
> I believe I'm using the registry via the locator which is what implements
> the exception you are referring to...
>
> I'm heading into the office in an hour and I'll re-verify.
>
> On Feb 3, 2011 7:21 AM, "Igor Drobiazko" <[email protected]> wrote:
>>
>> Hi Josh,
>>
>>
>> On Thu, Feb 3, 2011 at 3:50 PM, Josh Canfield
>> <[email protected]>wrote:
>>
>> > On Feb 3, 2011 12:12 AM, "Igor Drobiazko" <[email protected]>
>> > wrote:
>> > >
>> > > I have strong objections against your suggestion as this change would
>> > > completely change the IoC experience.
>> > >
>> > I'm not sure I agree. There is no change in experience as far as I can
>> > tell.
>> >
>> > > Guessing service id can be very dangerous.
>> > There is no guessing, this is the default. I thought this was
>> > documented,
>> > perhaps its just a comment in the code.
>> >
>>
>> This is not quite correct. When binding or building a service, you may
>> provide an id. If not provided, then Tapestry will take the default one.
>> This is documented. But resolving a service implementation is a completely
>> another story. In this case you are guessing as user didn't tell you which
>> implementation to inject. As of now you will get an exception because
>> Tapestry is not able to disambiguate and IMO it should stay this way.Now
>> if
>> you change the code as you suggest, you might inject an implementation
>> which
>> you shouldn't. The user would not see a message "Please disambiguate your
>> service" because you take the default id. This will definitely change the
>> current behaviour and break any existing app.
>>
>> >
>> > > Just imagine a Tapestry beginner
>> > > who found a service interface in the javadoc. He injects the service
>> > > into
>> > > his service/page/component and, if there are several implementations
>> > > of
>> > the
>> > > interface, he will get a meaningful exception, saying that he must
>> > > disambiguate the implementation to be injeted. This is the point where
>> > the
>> >
>> > This is still true after my change.
>> >
>> It's not. Just take a look at the example which I provided in the last
>> mail.
>> In this scenarion, the following code will cause an exception. After your
>> change it will not as, MyServiceRed will be injected.
>>
>> public class Foo {
>> @Inject
>> private MyService myService;
>> }
>>
>> >
>> > > user begns dig and read the documentation deeper. Now if you try to
>> > inject
>> > > by "default name", you could occasionally inject a wrong
>> > > implementation.
>> > The
>> > > user will never notice that and will wonder why his app behaves
>> > > strange.
>> > > Just as example.
>> > >
>> > > public static void bind(ServiceBinder binder) {
>> > > binder.bind(MyService.class, MyServiceRed.class);
>> > > binder.bind(MyService.class, MyServiceBlue.class).withId("Green");
>> > > }
>> > >
>> >
>> > This still works. As I mentioned this code passes all of the unit tests
>> > unchanged. It only shortcuts entrance into the master object provider.
>> >
>>
>> Well, if you find the test which explicitely checks that disambiguation
>> exception is still thrown after your change, than you can be sure. If no
>> test fails, it might be a sign of missing test cases.
>>
>> >
>> > > Having said that: -1 for this change.
>> > >
>> > > BTW I'm woundering why you need @InjectService annotation to inject
>> > > your
>> > > MonitorAdvisor into the advise method. Do you have several
>> > implementations
>> > > of MonitorAdvisor? If not, just remove the annotations and it will
>> > > work.
>> > :) if this were true then I wouldn't have added the annotations in the
>> > first
>> > place. I spent a long time playing with preventdecoration and scoured
>> > the
>> > codebase for examples.
>> >
>>
>> It is true. Here is a code from a production application:
>>
>> @Advise(serviceInterface = AssetSource.class)
>> public static void adviseAssetSource(MethodAdviceReceiver receiver,
>> TemplateClassifier classifier) {
>> receiver.adviseAllMethods(new
>> DeviceAwareAssetSourceAdvisor(classifier));
>> }
>>
>>
>> >
>> > > Also the constructor of MonitorAdvisorImpl looks strange to me. You
>> > > don't
>> > > need @InjectService annotation at all.
>> >
>> > I agree, I shouldn't need @InjectService, but I do. If you look at the
>> > error
>> > provided by the recursion exception (when I don't use @InjectService)
>> > you
>> > see why I have to use the annotations. The annotations are evaluated
>> > before
>> > the masterobjectprovider when calculating injection parameters. I've
>> > just
>> > moved the default service id logic up a level to get the same behavior.
>> >
>> > Howard suggested using @Local, but I don't think I should need anything.
>> > I
>> > did notice that the only similar code was the LoggingAdvisor which is
>> > created in one module but only used in a @Match(*) in a demo module.
>> >
>> > Josh
>> >
>> > >
>> > > On Thu, Feb 3, 2011 at 2:48 AM, Josh Canfield <[email protected]
>> > >wrote:
>> > >
>> > > > I started writing a long winded description of my problem, but let's
>> > > > start with the gist and then get into the details and finally a
>> > > > solution.
>> > > >
>> > > > The gist:
>> > > > I struggled for too long to figure out how to build a service
>> > > > advisor
>> > > > that uses services, and is a service. Not due to recursion into my
>> > > > own
>> > > > advisor, but due to recursion generated by the MasterObjectProvider
>> > > > when it attempts to construct it's various ObjectProvider instances.
>> > > >
>> > > > The details:
>> > > >
>> > > > One form of the error:
>> > > > [ERROR] Registry [ 1] Resolving object of type
>> > > > org.apache.tapestry5.services.ApplicationGlobals using
>> > > > MasterObjectProvider
>> > > > [ERROR] Registry [ 2] Realizing service AssetObjectProvider
>> > > > [ERROR] Registry [ 3] Invoking
>> > > >
>> > > >
>> >
>> >
>> > org.apache.tapestry5.monitor.MonitorModule.adviseForMonitoredServices(MethodAdviceReceiver,
>> > > > MonitorAdvisor) (at MonitorModule.java:52)
>> > > > [ERROR] Registry [ 4] Reloading class
>> > > > org.apache.tapestry5.internal.monitor.MonitorAdvisorImpl.
>> > > > [ERROR] Registry [ 5] Determining injection value for parameter #2
>> > > > (org.apache.tapestry5.monitor.MonitorNamingStrategy)
>> > > > [ERROR] Registry [ 6] Resolving object of type
>> > > > org.apache.tapestry5.monitor.MonitorNamingStrategy using
>> > > > MasterObjectProvider
>> > > > [ERROR] Registry [ 7] Realizing service AssetObjectProvider <---
>> > > > AHHH!!!! I don't need an AssetObjectProvider
>> > > >
>> > > > I can work around the problem by using 'InjectService("MyService")
>> > > > MyService' because by default the service id maps to the simple name
>> > > > of the service interface and the MasterObjectProvider isn't
>> > > > consulted
>> > > > about getting my Service. But I'm hopefully not alone in thinking
>> > > > this
>> > > > is silly.
>> > > >
>> > > > I am building service advice based on an annotation, I haven't seen
>> > > > anything in the system that puts all these together using the
>> > > > current
>> > > > advice (as opposed to decoration), but if you have I'd love to see
>> > > > it.
>> > > >
>> > > > This is what I have that works with the existing system.
>> > > >
>> > > > @Match("*")
>> > > > public static void adviceForMonitoredServices(
>> > > > MethodAdviceReceiver receiver,
>> > > > @InjectService("MonitorAdvisor") MonitorAdvisor
>> > monitorAdvisor)
>> > > > {
>> > > > monitorAdvisor.monitor(receiver);
>> > > > }
>> > > >
>> > > > public MonitorAdvisorImpl(
>> > > > Logger logger,
>> > > > @InjectService("MonitorNamingStrategy")
>> > > > MonitorNamingStrategy monitorNamingStrategy,
>> > > > @InjectService("MonitorJmxObjectNamingStrategy")
>> > > > MonitorJmxObjectNamingStrategy jmxObjectNamingStrategy,
>> > > > @InjectService("MBeanSupport") MBeanSupport mBeanSupport)
>> > > > {
>> > > >
>> > > > The solution:
>> > > >
>> > > > If I modify the InternalUtils.calculateInjection method so that it
>> > > > checks for the simple name of the service before delegating to the
>> > > > master object provider then I don't have to use the @InjectService
>> > > > annotation anymore.
>> > > >
>> > > > // by default services are registered by their simple name,
>> > > > see if we can get to it that way
>> > > > // although, if there is a marker annotation then you can't
>> > > > load the service this way.
>> > > > if ( provider.getAnnotation(Marker.class) != null) {
>> > > > try
>> > > > {
>> > > > return
>> > > > locator.getService(injectionType.getSimpleName(), injectionType);
>> > > > }
>> > > > catch (RuntimeException e)
>> > > > {
>> > > > // ignored, it will fall through to the master object
>> > > > provider
>> > > > }
>> > > > }
>> > > >
>> > > > I've run the ioc,core, spring and hibernate unit tests and they
>> > > > pass.
>> > > > The fact that the simple name is used as the default id is pervasive
>> > > > so I don't imagine using it here breaks any encapsulation.
>> > > >
>> > > > What use cases have I missed that will cause this code to break
>> > > > something not covered in the unit tests?
>> > > >
>> > > > Thanks,
>> > > > Josh
>> > > >
>> > > >
>> > > > ---------------------------------------------------------------------
>> > > > To unsubscribe, e-mail: [email protected]
>> > > > For additional commands, e-mail: [email protected]
>> > > >
>> > > >
>> > >
>> > >
>> > > --
>> > > Best regards,
>> > >
>> > > Igor Drobiazko
>> > > http://tapestry5.de
>> >
>>
>>
>>
>> --
>> Best regards,
>>
>> Igor Drobiazko
>> http://tapestry5.de
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
