tapestry-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Josh Canfield <joshcanfi...@gmail.com>
Subject Re: Injection calculation is difficult when building a service advisor
Date Thu, 03 Feb 2011 14:50:08 GMT
On Feb 3, 2011 12:12 AM, "Igor Drobiazko" <igor.drobiazko@gmail.com> 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.

> 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.

> 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.

> 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.

> 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 <joshcanfield@gmail.com
>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: dev-unsubscribe@tapestry.apache.org
> > For additional commands, e-mail: dev-help@tapestry.apache.org
> >
> >
>
>
> --
> Best regards,
>
> Igor Drobiazko
> http://tapestry5.de

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message