You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tapestry.apache.org by Josh Canfield <jo...@gmail.com> on 2011/02/03 02:48:17 UTC

Injection calculation is difficult when building a service advisor

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


Re: Injection calculation is difficult when building a service advisor

Posted by Igor Drobiazko <ig...@gmail.com>.
I have strong objections against your suggestion as this change would
completely change the IoC experience.

Guessing service id can be very dangerous. 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
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");
}

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.
Also the constructor of MonitorAdvisorImpl looks strange to me. You don't
need @InjectService annotation at all.

On Thu, Feb 3, 2011 at 2:48 AM, Josh Canfield <jo...@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