You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Jacopo Cappellato <ja...@hotwaxmedia.com> on 2012/07/20 08:33:07 UTC

General question about classes in the service package

Hi all,

I am a bit confused about the meaning/purpose/responsibilities of the following classes:

ServiceDispatcher/GenericDispatcher/LocalDispatcher/DispatchContext

Would any of you help me understand the role of each of these classes? They all see rather intermingled in terms of responsibilities.
For example, what is the purpose of creating two instances with different names of a dispatcher associated to the same delegator:

LocalDispatcher dispatcher = GenericDispatcher.getLocalDispatcher("entity-" + delegator.getDelegatorName(), delegator);

and

LocalDispatcher thisDispatcher = GenericDispatcher.getLocalDispatcher(delegator.getDelegatorName(), delegator);

?

Thanks,

Jacopo





Re: General question about classes in the service package

Posted by Adrian Crum <ad...@sandglass-software.com>.
On 7/22/2012 8:36 AM, Jacopo Cappellato wrote:
> On Jul 20, 2012, at 9:33 AM, Adrian Crum wrote:
>
>> I agree there is a lot of feature envy between classes. The API could be cleaned up a lot.
>>
>>  From my perspective, the GenericDispatcher.getLocalDispatcher method should not exist - since it forces you to reference an implementation. Instead, there should be a separate service dispatcher factory.
> I agree this is the right direction to go.
>
> In rev. 1364222, I did a first pass cleanup of the code in the DispatchContext and in the methods related to the creation/retrieval of ServiceDispatcher/GenericDispatcher objects; this should simplify the refactoring of the API and now the code is a bit cleaner and more readable.
> Since this first pass is quite relevant in terms of code changes (unfortunately I couldn't find a better way to split these in more commits) I would really appreciate your reviews and bug reports (if any) and also your patience if you will find issues caused by this change: I will do my best to fix them asap.

Some other things that could be included in a refactoring:

1. Move Temporal Expression code to the base component. Keep the 
TemporalExpressionWorker class in the service component since it handles 
expression persistence.
2. Move the model classes to a org.ofbiz.service.model package.
3. Make the model classes final and thread-safe.

-Adrian


Re: General question about classes in the service package

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
Of course, if you have the setup to test it and if see any issues, I will be happy to fix them.

Jacopo

On Jul 23, 2012, at 10:07 AM, Jacques Le Roux wrote:

> Agreed, most of case would have worked OOTB, but better to have a specific name indeed. So I expect it will work now, OK thanks.
> BTW, I began to study the Executor framework, interesting.
> 
> Jacques
> 
> From: "Jacopo Cappellato" <ja...@hotwaxmedia.com>
>> No, I didn't. But the NPE that was initially reported was actually caused by an issue in the construction mechanism of the
>> ServiceDispatcher/GenericDispatcher classes and this is fixed by my recent refactoring; I believe that the error should not happen
>> again (and changing the name from JMSDispatcher to "entity-default" was really a wrong way to fix it, because it was simply
>> working if the "entity-default" dispatcher exists... and this may depend on how the system is configured).
>> 
>> Jacopo
>> 
>> 
>> 
>> On Jul 23, 2012, at 8:48 AM, Jacques Le Roux wrote:
>> 
>>> Hi Jacopo,
>>> 
>>> In r1364222 I saw you changed back the name of the AbstractJmsListener to JMSDispatcher. Did you check if it works when you set a
>>> jms-service in serviceengine.xml, like with DCC?
>>> 
>>> Jacques
>>> 
>>> From: "Jacopo Cappellato" <ja...@hotwaxmedia.com>
>>>> On Jul 20, 2012, at 9:33 AM, Adrian Crum wrote:
>>>> 
>>>>> I agree there is a lot of feature envy between classes. The API could be cleaned up a lot.
>>>>> 
>>>>> From my perspective, the GenericDispatcher.getLocalDispatcher method should not exist - since it forces you to reference an
>>>>> implementation. Instead, there should be a separate service dispatcher factory.
>>>> 
>>>> I agree this is the right direction to go.
>>>> 
>>>> In rev. 1364222, I did a first pass cleanup of the code in the DispatchContext and in the methods related to the
>>>> creation/retrieval of ServiceDispatcher/GenericDispatcher objects; this should simplify the refactoring of the API and now the
>>>> code is a bit cleaner and more readable.
>>>> Since this first pass is quite relevant in terms of code changes (unfortunately I couldn't find a better way to split these in
>>>> more commits) I would really appreciate your reviews and bug reports (if any) and also your patience if you will find issues
>>>> caused by this change: I will do my best to fix them asap.
>>>> 
>>>> Jacopo
>> 
>> 


Re: General question about classes in the service package

Posted by Jacques Le Roux <ja...@les7arts.com>.
Agreed, most of case would have worked OOTB, but better to have a specific name indeed. So I expect it will work now, OK thanks.
BTW, I began to study the Executor framework, interesting.

Jacques

From: "Jacopo Cappellato" <ja...@hotwaxmedia.com>
> No, I didn't. But the NPE that was initially reported was actually caused by an issue in the construction mechanism of the
> ServiceDispatcher/GenericDispatcher classes and this is fixed by my recent refactoring; I believe that the error should not happen
> again (and changing the name from JMSDispatcher to "entity-default" was really a wrong way to fix it, because it was simply
> working if the "entity-default" dispatcher exists... and this may depend on how the system is configured).
>
> Jacopo
>
>
>
> On Jul 23, 2012, at 8:48 AM, Jacques Le Roux wrote:
>
>> Hi Jacopo,
>>
>> In r1364222 I saw you changed back the name of the AbstractJmsListener to JMSDispatcher. Did you check if it works when you set a
>> jms-service in serviceengine.xml, like with DCC?
>>
>> Jacques
>>
>> From: "Jacopo Cappellato" <ja...@hotwaxmedia.com>
>>> On Jul 20, 2012, at 9:33 AM, Adrian Crum wrote:
>>>
>>>> I agree there is a lot of feature envy between classes. The API could be cleaned up a lot.
>>>>
>>>> From my perspective, the GenericDispatcher.getLocalDispatcher method should not exist - since it forces you to reference an
>>>> implementation. Instead, there should be a separate service dispatcher factory.
>>>
>>> I agree this is the right direction to go.
>>>
>>> In rev. 1364222, I did a first pass cleanup of the code in the DispatchContext and in the methods related to the
>>> creation/retrieval of ServiceDispatcher/GenericDispatcher objects; this should simplify the refactoring of the API and now the
>>> code is a bit cleaner and more readable.
>>> Since this first pass is quite relevant in terms of code changes (unfortunately I couldn't find a better way to split these in
>>> more commits) I would really appreciate your reviews and bug reports (if any) and also your patience if you will find issues
>>> caused by this change: I will do my best to fix them asap.
>>>
>>> Jacopo
>
>

Re: General question about classes in the service package

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
No, I didn't. But the NPE that was initially reported was actually caused by an issue in the construction mechanism of the ServiceDispatcher/GenericDispatcher classes and this is fixed by my recent refactoring; I believe that the error should not happen again (and changing the name from JMSDispatcher to "entity-default" was really a wrong way to fix it, because it was simply working if the "entity-default" dispatcher exists... and this may depend on how the system is configured).

Jacopo



On Jul 23, 2012, at 8:48 AM, Jacques Le Roux wrote:

> Hi Jacopo,
> 
> In r1364222 I saw you changed back the name of the AbstractJmsListener to JMSDispatcher. Did you check if it works when you set a jms-service in serviceengine.xml, like with DCC?
> 
> Jacques
> 
> From: "Jacopo Cappellato" <ja...@hotwaxmedia.com>
>> On Jul 20, 2012, at 9:33 AM, Adrian Crum wrote:
>> 
>>> I agree there is a lot of feature envy between classes. The API could be cleaned up a lot.
>>> 
>>> From my perspective, the GenericDispatcher.getLocalDispatcher method should not exist - since it forces you to reference an
>>> implementation. Instead, there should be a separate service dispatcher factory.
>> 
>> I agree this is the right direction to go.
>> 
>> In rev. 1364222, I did a first pass cleanup of the code in the DispatchContext and in the methods related to the
>> creation/retrieval of ServiceDispatcher/GenericDispatcher objects; this should simplify the refactoring of the API and now the
>> code is a bit cleaner and more readable.
>> Since this first pass is quite relevant in terms of code changes (unfortunately I couldn't find a better way to split these in
>> more commits) I would really appreciate your reviews and bug reports (if any) and also your patience if you will find issues
>> caused by this change: I will do my best to fix them asap.
>> 
>> Jacopo


Re: General question about classes in the service package

Posted by Jacques Le Roux <ja...@les7arts.com>.
Hi Jacopo,

In r1364222 I saw you changed back the name of the AbstractJmsListener to JMSDispatcher. Did you check if it works when you set a 
jms-service in serviceengine.xml, like with DCC?

Jacques

From: "Jacopo Cappellato" <ja...@hotwaxmedia.com>
> On Jul 20, 2012, at 9:33 AM, Adrian Crum wrote:
>
>> I agree there is a lot of feature envy between classes. The API could be cleaned up a lot.
>>
>> From my perspective, the GenericDispatcher.getLocalDispatcher method should not exist - since it forces you to reference an
>> implementation. Instead, there should be a separate service dispatcher factory.
>
> I agree this is the right direction to go.
>
> In rev. 1364222, I did a first pass cleanup of the code in the DispatchContext and in the methods related to the
> creation/retrieval of ServiceDispatcher/GenericDispatcher objects; this should simplify the refactoring of the API and now the
> code is a bit cleaner and more readable.
> Since this first pass is quite relevant in terms of code changes (unfortunately I couldn't find a better way to split these in
> more commits) I would really appreciate your reviews and bug reports (if any) and also your patience if you will find issues
> caused by this change: I will do my best to fix them asap.
>
> Jacopo

Re: General question about classes in the service package

Posted by Adrian Crum <ad...@sandglass-software.com>.
Done.

Thank you for working on this. It is truly needed.

-Adrian

On 7/24/2012 12:22 PM, Jacopo Cappellato wrote:
> Adrian,
>
> thanks for the enhancements; I don't have local changes at the moment so feel free to proceed.
>
> Jacopo
>
> On Jul 24, 2012, at 1:19 PM, Adrian Crum wrote:
>
>> I committed some small changes in rev 1364972. I apologize for stepping on your work - I thought it would be quicker to make them myself than to describe them in an email.
>>
>> I also have GenericDispatcher moved to a GenericDispatcherFactory inner class on my local copy. I can commit that too if it doesn't interfere with your work.
>>
>> -Adrian
>>
>> On 7/24/2012 10:18 AM, Jacopo Cappellato wrote:
>>> On Jul 22, 2012, at 9:36 AM, Jacopo Cappellato wrote:
>>>
>>>>>  From my perspective, the GenericDispatcher.getLocalDispatcher method should not exist - since it forces you to reference an implementation. Instead, there should be a separate service dispatcher factory.
>>> This is now implemented in rev.  1364952


Re: General question about classes in the service package

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
Adrian,

thanks for the enhancements; I don't have local changes at the moment so feel free to proceed.

Jacopo

On Jul 24, 2012, at 1:19 PM, Adrian Crum wrote:

> I committed some small changes in rev 1364972. I apologize for stepping on your work - I thought it would be quicker to make them myself than to describe them in an email.
> 
> I also have GenericDispatcher moved to a GenericDispatcherFactory inner class on my local copy. I can commit that too if it doesn't interfere with your work.
> 
> -Adrian
> 
> On 7/24/2012 10:18 AM, Jacopo Cappellato wrote:
>> On Jul 22, 2012, at 9:36 AM, Jacopo Cappellato wrote:
>> 
>>>> From my perspective, the GenericDispatcher.getLocalDispatcher method should not exist - since it forces you to reference an implementation. Instead, there should be a separate service dispatcher factory.
>> This is now implemented in rev.  1364952
> 


Re: General question about classes in the service package

Posted by Adrian Crum <ad...@sandglass-software.com>.
I committed some small changes in rev 1364972. I apologize for stepping 
on your work - I thought it would be quicker to make them myself than to 
describe them in an email.

I also have GenericDispatcher moved to a GenericDispatcherFactory inner 
class on my local copy. I can commit that too if it doesn't interfere 
with your work.

-Adrian

On 7/24/2012 10:18 AM, Jacopo Cappellato wrote:
> On Jul 22, 2012, at 9:36 AM, Jacopo Cappellato wrote:
>
>>>  From my perspective, the GenericDispatcher.getLocalDispatcher method should not exist - since it forces you to reference an implementation. Instead, there should be a separate service dispatcher factory.
> This is now implemented in rev.  1364952


Re: General question about classes in the service package

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
On Jul 22, 2012, at 9:36 AM, Jacopo Cappellato wrote:

>> From my perspective, the GenericDispatcher.getLocalDispatcher method should not exist - since it forces you to reference an implementation. Instead, there should be a separate service dispatcher factory.

This is now implemented in rev.  1364952

Jacopo


Re: General question about classes in the service package

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
On Jul 20, 2012, at 9:33 AM, Adrian Crum wrote:

> I agree there is a lot of feature envy between classes. The API could be cleaned up a lot.
> 
> From my perspective, the GenericDispatcher.getLocalDispatcher method should not exist - since it forces you to reference an implementation. Instead, there should be a separate service dispatcher factory.

I agree this is the right direction to go.

In rev. 1364222, I did a first pass cleanup of the code in the DispatchContext and in the methods related to the creation/retrieval of ServiceDispatcher/GenericDispatcher objects; this should simplify the refactoring of the API and now the code is a bit cleaner and more readable.
Since this first pass is quite relevant in terms of code changes (unfortunately I couldn't find a better way to split these in more commits) I would really appreciate your reviews and bug reports (if any) and also your patience if you will find issues caused by this change: I will do my best to fix them asap.

Jacopo

Re: General question about classes in the service package

Posted by Jacques Le Roux <ja...@les7arts.com>.
From: "Adrian Crum" <ad...@sandglass-software.com>
> On 7/20/2012 10:13 AM, Jacopo Cappellato wrote:
>> On Jul 20, 2012, at 10:51 AM, Adrian Crum wrote:
>>
>>> On 7/20/2012 9:43 AM, Jacopo Cappellato wrote:
>>>> Thank you Adrian, it is of great help.
>>>>
>>>> On Jul 20, 2012, at 10:07 AM, Adrian Crum wrote:
>>>>
>>>>> If you have a LocalDispatcher in your hand, then you can use it to get a DispatchContext. If you have a DispatchContext in 
>>>>> your hand, you can use it to get a LocalDispatcher.
>>>>> Keep in mind the DispatchContext contains the objects related to a service invocation: The service dispatcher, delegator, and 
>>>>> security. You need all three to write a service.
>>>> Well, actually the getDelegator and getSecurity methods of DispatchContext simply returns the objects from the LocalDispatcher.
>>>> It seems that the only real purpose of DispatchContext is as an helper to read (and cache) service definitions; there is also a 
>>>> useful makeValidContext method but that can be made static (I will probably commit this change later) and may in theory be 
>>>> moved in some other class.
>>>> If my understanding is correct, then the name of this class is misleading.
>>>> Do you agree?
>>> No, I don't agree. If you look at a typical Java service method signature, you have something like:
>>>
>>> public static Map<String, Object> myService(DispatchContext.dctx, Map<String, Object> context)...
>>>
>>> The DispatchContext contains the objects common to all invocations of myService, and the context Map contains the objects 
>>> specific to each invocation. In order to write a service, you will need the objects the DispatchContext contains. From my 
>>> perspective, the class name accurately describes its purpose.
>> What I meant to say is that DispatchContext actually delegates the retrieval of the delegator and security to the 
>> LocalDispatcher; it is not the glue that keeps together a Delegator, a Security and a LocalDispatcher but it is instead an helper 
>> class to retrieve the Delegator and Security from the LocalDispatcher.
>
> Correct. I was speaking from an API perspective, or from the client code's perspective: DispatchContext is a collection of objects 
> needed for service execution.
>
>> I agree that it is used as a "context" in all service implementations.
>> The real added value of the DispatchContext object is that it compose a LocalDispatcher with a ClassLoader and a collection of 
>> localReaders (i.e. paths to service definitions).
>
> I think of it as a convenience class that makes it easier to write services.

+1, it's very convenient...

Jacques

> -Adrian
> 

Re: General question about classes in the service package

Posted by Adrian Crum <ad...@sandglass-software.com>.
On 7/20/2012 10:13 AM, Jacopo Cappellato wrote:
> On Jul 20, 2012, at 10:51 AM, Adrian Crum wrote:
>
>> On 7/20/2012 9:43 AM, Jacopo Cappellato wrote:
>>> Thank you Adrian, it is of great help.
>>>
>>> On Jul 20, 2012, at 10:07 AM, Adrian Crum wrote:
>>>
>>>> If you have a LocalDispatcher in your hand, then you can use it to get a DispatchContext. If you have a DispatchContext in your hand, you can use it to get a LocalDispatcher.
>>>> Keep in mind the DispatchContext contains the objects related to a service invocation: The service dispatcher, delegator, and security. You need all three to write a service.
>>> Well, actually the getDelegator and getSecurity methods of DispatchContext simply returns the objects from the LocalDispatcher.
>>> It seems that the only real purpose of DispatchContext is as an helper to read (and cache) service definitions; there is also a useful makeValidContext method but that can be made static (I will probably commit this change later) and may in theory be moved in some other class.
>>> If my understanding is correct, then the name of this class is misleading.
>>> Do you agree?
>> No, I don't agree. If you look at a typical Java service method signature, you have something like:
>>
>> public static Map<String, Object> myService(DispatchContext.dctx, Map<String, Object> context)...
>>
>> The DispatchContext contains the objects common to all invocations of myService, and the context Map contains the objects specific to each invocation. In order to write a service, you will need the objects the DispatchContext contains. From my perspective, the class name accurately describes its purpose.
> What I meant to say is that DispatchContext actually delegates the retrieval of the delegator and security to the LocalDispatcher; it is not the glue that keeps together a Delegator, a Security and a LocalDispatcher but it is instead an helper class to retrieve the Delegator and Security from the LocalDispatcher.

Correct. I was speaking from an API perspective, or from the client 
code's perspective: DispatchContext is a collection of objects needed 
for service execution.

> I agree that it is used as a "context" in all service implementations.
> The real added value of the DispatchContext object is that it compose a LocalDispatcher with a ClassLoader and a collection of localReaders (i.e. paths to service definitions).

I think of it as a convenience class that makes it easier to write services.

-Adrian


Re: General question about classes in the service package

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
On Jul 20, 2012, at 10:51 AM, Adrian Crum wrote:

> 
> On 7/20/2012 9:43 AM, Jacopo Cappellato wrote:
>> Thank you Adrian, it is of great help.
>> 
>> On Jul 20, 2012, at 10:07 AM, Adrian Crum wrote:
>> 
>>> If you have a LocalDispatcher in your hand, then you can use it to get a DispatchContext. If you have a DispatchContext in your hand, you can use it to get a LocalDispatcher.
>>> Keep in mind the DispatchContext contains the objects related to a service invocation: The service dispatcher, delegator, and security. You need all three to write a service.
>> Well, actually the getDelegator and getSecurity methods of DispatchContext simply returns the objects from the LocalDispatcher.
>> It seems that the only real purpose of DispatchContext is as an helper to read (and cache) service definitions; there is also a useful makeValidContext method but that can be made static (I will probably commit this change later) and may in theory be moved in some other class.
>> If my understanding is correct, then the name of this class is misleading.
>> Do you agree?
> 
> No, I don't agree. If you look at a typical Java service method signature, you have something like:
> 
> public static Map<String, Object> myService(DispatchContext.dctx, Map<String, Object> context)...
> 
> The DispatchContext contains the objects common to all invocations of myService, and the context Map contains the objects specific to each invocation. In order to write a service, you will need the objects the DispatchContext contains. From my perspective, the class name accurately describes its purpose.

What I meant to say is that DispatchContext actually delegates the retrieval of the delegator and security to the LocalDispatcher; it is not the glue that keeps together a Delegator, a Security and a LocalDispatcher but it is instead an helper class to retrieve the Delegator and Security from the LocalDispatcher.
I agree that it is used as a "context" in all service implementations.
The real added value of the DispatchContext object is that it compose a LocalDispatcher with a ClassLoader and a collection of localReaders (i.e. paths to service definitions).

Jacopo

> 
> -Adrian
> 
> 


Re: General question about classes in the service package

Posted by Adrian Crum <ad...@sandglass-software.com>.
On 7/20/2012 9:43 AM, Jacopo Cappellato wrote:
> Thank you Adrian, it is of great help.
>
> On Jul 20, 2012, at 10:07 AM, Adrian Crum wrote:
>
>> If you have a LocalDispatcher in your hand, then you can use it to get a DispatchContext. If you have a DispatchContext in your hand, you can use it to get a LocalDispatcher.
>> Keep in mind the DispatchContext contains the objects related to a service invocation: The service dispatcher, delegator, and security. You need all three to write a service.
> Well, actually the getDelegator and getSecurity methods of DispatchContext simply returns the objects from the LocalDispatcher.
> It seems that the only real purpose of DispatchContext is as an helper to read (and cache) service definitions; there is also a useful makeValidContext method but that can be made static (I will probably commit this change later) and may in theory be moved in some other class.
> If my understanding is correct, then the name of this class is misleading.
> Do you agree?

No, I don't agree. If you look at a typical Java service method 
signature, you have something like:

public static Map<String, Object> myService(DispatchContext.dctx, 
Map<String, Object> context)...

The DispatchContext contains the objects common to all invocations of 
myService, and the context Map contains the objects specific to each 
invocation. In order to write a service, you will need the objects the 
DispatchContext contains. From my perspective, the class name accurately 
describes its purpose.

-Adrian



Re: General question about classes in the service package

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
Thank you Adrian, it is of great help.

On Jul 20, 2012, at 10:07 AM, Adrian Crum wrote:

> If you have a LocalDispatcher in your hand, then you can use it to get a DispatchContext. If you have a DispatchContext in your hand, you can use it to get a LocalDispatcher.
> Keep in mind the DispatchContext contains the objects related to a service invocation: The service dispatcher, delegator, and security. You need all three to write a service.

Well, actually the getDelegator and getSecurity methods of DispatchContext simply returns the objects from the LocalDispatcher.
It seems that the only real purpose of DispatchContext is as an helper to read (and cache) service definitions; there is also a useful makeValidContext method but that can be made static (I will probably commit this change later) and may in theory be moved in some other class.
If my understanding is correct, then the name of this class is misleading.
Do you agree?

Jacopo


Re: General question about classes in the service package

Posted by Adrian Crum <ad...@sandglass-software.com>.
On 7/20/2012 8:53 AM, Jacopo Cappellato wrote:
> Thank you Adrian, please see inline:
>
> On Jul 20, 2012, at 9:33 AM, Adrian Crum wrote:
>
>> This will be very helpful - thanks for starting it Jacopo!
>>
>> I will start off with the obvious answers...
>>
>> LocalDispatcher: The service dispatcher interface.
>> GenericAbstractDispatcher/GenericDispatcher: LocalDispatcher implementations.
>> ServiceDispatcher: A service dispatcher "engine". Basically, the LocalDispatcher implementations delegate to it.
> I guess that with "engine" you mean an "helper" class (or similar) and not a service engine (i.e. implementation of GenericEngine).

Correct.

> Apart from this, I see that the ServiceDispatcher is used in a lot of code directly... shouldn't we use instead the LocalDispatcher? According to your notes, if I understand them, the only class that should reference the ServiceDispatcher should be GenericDispatcher that is the only class that implement LocalDispatcher; but GenericDispatcher doesn't use at all the ServiceDispatcher.

Correct. Ideally, client code should look something like this:

LocalDispatcher dispatcher = DispatcherFactory.getInstance(...);

GenericDispatcher uses ServiceDispatcher: 
GenericAbstractDispatcher.java, line 45.

>
> My next doubt/question is about the GenericEngine interface: how it relates with LocalDispatcher and ServiceDispatcher?

GenericEngine is the abstraction that allows us to write services in 
Java, scripts, etc... So, a service dispatcher uses the GenericEngine to 
invoke the service in a language-independent way.

>
>> DispatchContext: A bag of objects needed for service execution, plus some utility methods.
> So, is the DispatchContext associated to the LocalDispatcher or instead it is an implementation detail that makes sense only for the ServiceDispatcher?

It is associated with LocalDispatcher. If you have a LocalDispatcher in 
your hand, then you can use it to get a DispatchContext. If you have a 
DispatchContext in your hand, you can use it to get a LocalDispatcher.
Keep in mind the DispatchContext contains the objects related to a 
service invocation: The service dispatcher, delegator, and security. You 
need all three to write a service.

>
> Thanks,
>
> Jacopo
>
>> I agree there is a lot of feature envy between classes. The API could be cleaned up a lot.
>>
>>  From my perspective, the GenericDispatcher.getLocalDispatcher method should not exist - since it forces you to reference an implementation. Instead, there should be a separate service dispatcher factory.
>>
>> -Adrian
>>
>> On 7/20/2012 7:33 AM, Jacopo Cappellato wrote:
>>> Hi all,
>>>
>>> I am a bit confused about the meaning/purpose/responsibilities of the following classes:
>>>
>>> ServiceDispatcher/GenericDispatcher/LocalDispatcher/DispatchContext
>>>
>>> Would any of you help me understand the role of each of these classes? They all see rather intermingled in terms of responsibilities.
>>> For example, what is the purpose of creating two instances with different names of a dispatcher associated to the same delegator:
>>>
>>> LocalDispatcher dispatcher = GenericDispatcher.getLocalDispatcher("entity-" + delegator.getDelegatorName(), delegator);
>>>
>>> and
>>>
>>> LocalDispatcher thisDispatcher = GenericDispatcher.getLocalDispatcher(delegator.getDelegatorName(), delegator);
>>>
>>> ?
>>>
>>> Thanks,
>>>
>>> Jacopo
>>>
>>>
>>>
>>>


Re: General question about classes in the service package

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
On Jul 20, 2012, at 9:53 AM, Jacopo Cappellato wrote:

> but GenericDispatcher doesn't use at all the ServiceDispatcher.

please ignore this part, as it is clearly not correct.

Jacopo

Re: General question about classes in the service package

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
Thank you Adrian, please see inline:

On Jul 20, 2012, at 9:33 AM, Adrian Crum wrote:

> This will be very helpful - thanks for starting it Jacopo!
> 
> I will start off with the obvious answers...
> 
> LocalDispatcher: The service dispatcher interface.
> GenericAbstractDispatcher/GenericDispatcher: LocalDispatcher implementations.
> ServiceDispatcher: A service dispatcher "engine". Basically, the LocalDispatcher implementations delegate to it.

I guess that with "engine" you mean an "helper" class (or similar) and not a service engine (i.e. implementation of GenericEngine).
Apart from this, I see that the ServiceDispatcher is used in a lot of code directly... shouldn't we use instead the LocalDispatcher? According to your notes, if I understand them, the only class that should reference the ServiceDispatcher should be GenericDispatcher that is the only class that implement LocalDispatcher; but GenericDispatcher doesn't use at all the ServiceDispatcher.

My next doubt/question is about the GenericEngine interface: how it relates with LocalDispatcher and ServiceDispatcher?

> DispatchContext: A bag of objects needed for service execution, plus some utility methods.

So, is the DispatchContext associated to the LocalDispatcher or instead it is an implementation detail that makes sense only for the ServiceDispatcher?

Thanks,

Jacopo

> 
> I agree there is a lot of feature envy between classes. The API could be cleaned up a lot.
> 
> From my perspective, the GenericDispatcher.getLocalDispatcher method should not exist - since it forces you to reference an implementation. Instead, there should be a separate service dispatcher factory.
> 
> -Adrian
> 
> On 7/20/2012 7:33 AM, Jacopo Cappellato wrote:
>> Hi all,
>> 
>> I am a bit confused about the meaning/purpose/responsibilities of the following classes:
>> 
>> ServiceDispatcher/GenericDispatcher/LocalDispatcher/DispatchContext
>> 
>> Would any of you help me understand the role of each of these classes? They all see rather intermingled in terms of responsibilities.
>> For example, what is the purpose of creating two instances with different names of a dispatcher associated to the same delegator:
>> 
>> LocalDispatcher dispatcher = GenericDispatcher.getLocalDispatcher("entity-" + delegator.getDelegatorName(), delegator);
>> 
>> and
>> 
>> LocalDispatcher thisDispatcher = GenericDispatcher.getLocalDispatcher(delegator.getDelegatorName(), delegator);
>> 
>> ?
>> 
>> Thanks,
>> 
>> Jacopo
>> 
>> 
>> 
>> 
> 


Re: General question about classes in the service package

Posted by Adrian Crum <ad...@sandglass-software.com>.
This will be very helpful - thanks for starting it Jacopo!

I will start off with the obvious answers...

LocalDispatcher: The service dispatcher interface.
GenericAbstractDispatcher/GenericDispatcher: LocalDispatcher 
implementations.
ServiceDispatcher: A service dispatcher "engine". Basically, the 
LocalDispatcher implementations delegate to it.
DispatchContext: A bag of objects needed for service execution, plus 
some utility methods.

I agree there is a lot of feature envy between classes. The API could be 
cleaned up a lot.

 From my perspective, the GenericDispatcher.getLocalDispatcher method 
should not exist - since it forces you to reference an implementation. 
Instead, there should be a separate service dispatcher factory.

-Adrian

On 7/20/2012 7:33 AM, Jacopo Cappellato wrote:
> Hi all,
>
> I am a bit confused about the meaning/purpose/responsibilities of the following classes:
>
> ServiceDispatcher/GenericDispatcher/LocalDispatcher/DispatchContext
>
> Would any of you help me understand the role of each of these classes? They all see rather intermingled in terms of responsibilities.
> For example, what is the purpose of creating two instances with different names of a dispatcher associated to the same delegator:
>
> LocalDispatcher dispatcher = GenericDispatcher.getLocalDispatcher("entity-" + delegator.getDelegatorName(), delegator);
>
> and
>
> LocalDispatcher thisDispatcher = GenericDispatcher.getLocalDispatcher(delegator.getDelegatorName(), delegator);
>
> ?
>
> Thanks,
>
> Jacopo
>
>
>
>


Re: General question about classes in the service package

Posted by Jacques Le Roux <ja...@les7arts.com>.
Is that not related to Multitenant introduction (just a guess)?

Jacques

From: "Jacopo Cappellato" <ja...@hotwaxmedia.com>
> Hi all,
>
> I am a bit confused about the meaning/purpose/responsibilities of the following classes:
>
> ServiceDispatcher/GenericDispatcher/LocalDispatcher/DispatchContext
>
> Would any of you help me understand the role of each of these classes? They all see rather intermingled in terms of 
> responsibilities.
> For example, what is the purpose of creating two instances with different names of a dispatcher associated to the same delegator:
>
> LocalDispatcher dispatcher = GenericDispatcher.getLocalDispatcher("entity-" + delegator.getDelegatorName(), delegator);
>
> and
>
> LocalDispatcher thisDispatcher = GenericDispatcher.getLocalDispatcher(delegator.getDelegatorName(), delegator);
>
> ?
>
> Thanks,
>
> Jacopo
>
>
>
>
>