Currently, the service engine classes are a bit muddled.
GenericDispatcher (the default LocalDispatcher implementation) and DispatchContext have feature envy. Some functionality in DispatchContext belongs in ServiceDispatcher. I would like to clean this up a bit and provide better separation of concerns. The service engine API will not change - the work will be done "under the hood." My hope is to get the code to a point where DispatchContext becomes less integral to its neighbors, and it becomes more of a single-use lightweight container. Again, this will not change the API at all. However, it will open up the possibility to improve the API. For example, instead of this service implementation: public static Map<String, Object> myService(DispatchContext dctx, Map<String, Object> context) { Locale locale = (Locale) context.get("locale"); ... we could have: public static Map<String, Object> myService(DispatchContext dctx) { Locale locale = dctx.getParameter("locale"); ... What do you think? -- Adrian Crum Sandglass Software www.sandglass-software.com |
Thank you Adrian,
please see inline: On Aug 31, 2014, at 10:50 AM, Adrian Crum <[hidden email]> wrote: > Currently, the service engine classes are a bit muddled. GenericDispatcher (the default LocalDispatcher implementation) and DispatchContext have feature envy. Some functionality in DispatchContext belongs in ServiceDispatcher. > > I would like to clean this up a bit and provide better separation of concerns. The service engine API will not change - the work will be done "under the hood." > > My hope is to get the code to a point where DispatchContext becomes less integral to its neighbors, and it becomes more of a single-use lightweight container. Again, this will not change the API at all. I think it would help the other committers to provide feedback if you could (in just a few sentences) summarize the following: a) what is the original concern of DispatchContext and of GenericDispatcher b) where do you see that this concerns are not well implemented in the code c) if you would like to modify #a, quickly describe the new purposes of the classes in your vision d) what are the methods/fields that you would like to move from DispatchContext Thanks Jacopo PS: in general I also see a lot of code that should be cleaned in DispatchContext; for example, the initialization of the static map in the constructor, the method getAllServiceNames() that should be static, bad thread-safe implementation, probably the modelServiceMapByModel should not even be there... > > However, it will open up the possibility to improve the API. For example, instead of this service implementation: > > public static Map<String, Object> myService(DispatchContext dctx, Map<String, Object> context) { > Locale locale = (Locale) context.get("locale"); > ... > > we could have: > > public static Map<String, Object> myService(DispatchContext dctx) { > Locale locale = dctx.getParameter("locale"); > ... > > > What do you think? > > > -- > Adrian Crum > Sandglass Software > www.sandglass-software.com |
In reply to this post by Adrian Crum-3
On Aug 31, 2014, at 10:50 AM, Adrian Crum <[hidden email]> wrote: > However, it will open up the possibility to improve the API. For example, instead of this service implementation: > > public static Map<String, Object> myService(DispatchContext dctx, Map<String, Object> context) { > Locale locale = (Locale) context.get("locale"); > ... > > we could have: > > public static Map<String, Object> myService(DispatchContext dctx) { > Locale locale = dctx.getParameter("locale"); > ... DispatchContext and the Map with input parameters have a completely different purpose and I don't think this would be an "improvement" per se. We will have to revisit this conversation at some point to be sure we are in the same page. Jacopo |
In reply to this post by Jacopo Cappellato-4
On Aug 31, 2014, at 11:36 AM, Jacopo Cappellato <[hidden email]> wrote: > a) what is the original concern of DispatchContext and of GenericDispatcher This comes from this old but still interesting document: http://ofbiz.apache.org/docs/services.html ======================================== *Service Dispatcher* The Service Dispatcher handles dispatching services to the appropriate Service Engine where it is then invoked. There is exactly one ServiceDispatcher for each Entity Delegator. If there are multiple delegators in an application there will also be multiple dispatchers. The ServiceDispatcher is accessed via a LocalDispatcher. There can be many LocalDispatchers associated with a ServiceDispatcher. Each LocalDispatcher is uniquely named and contains its own list of service definitions. When creating an instance of a LocalDispatcher, a DispatchContext is also created and passed to the ServiceEngine. A LocalDispatcher is associated with an application. Applications never talk directly to the ServiceDispatcher. The LocalDispatcher contains an API for invoking services, which are routed through the ServiceDispather. However, applications may be running in different threads than the actual ServiceDispatcher, so it is left to the LocalDispatcher to keep a DispatchContext which among other things keeps a reference to the applications classloader. *Dispatch Context* The DispatchContext is created by the LocalDispatcher upon instantiation. This is the runtime dispatcher context. It contains necessary information to process services for each dispatcher. This context contains the reference to each of the service definition files, the classloader which should be used for invocation, a reference to the delegator and its dispatcher along with a 'bag' of user defined attributes. This context is passed on to each service when invoked and is used by the dispatcher to determine the service's model. ======================================== Jacopo |
That last paragraph describes the cleanup I want to do. If a
LocalDispatcher contains things specific to an application, then why are some of those things kept in GenericDispatcher and others are kept in DispatchContext? This is the feature-envy part - they are both trying to be the same thing. It would simpler and cleaner if we keep application-specific bits in GenericDispatcher, and just have DispatchContext reference it. Adrian Crum Sandglass Software www.sandglass-software.com On 8/31/2014 10:56 AM, Jacopo Cappellato wrote: > > On Aug 31, 2014, at 11:36 AM, Jacopo Cappellato <[hidden email]> wrote: > >> a) what is the original concern of DispatchContext and of GenericDispatcher > > This comes from this old but still interesting document: http://ofbiz.apache.org/docs/services.html > > ======================================== > *Service Dispatcher* > > The Service Dispatcher handles dispatching services to the appropriate Service Engine where it is then invoked. There is exactly one ServiceDispatcher for each Entity Delegator. If there are multiple delegators in an application there will also be multiple dispatchers. The ServiceDispatcher is accessed via a LocalDispatcher. There can be many LocalDispatchers associated with a ServiceDispatcher. Each LocalDispatcher is uniquely named and contains its own list of service definitions. When creating an instance of a LocalDispatcher, a DispatchContext is also created and passed to the ServiceEngine. > > A LocalDispatcher is associated with an application. Applications never talk directly to the ServiceDispatcher. The LocalDispatcher contains an API for invoking services, which are routed through the ServiceDispather. However, applications may be running in different threads than the actual ServiceDispatcher, so it is left to the LocalDispatcher to keep a DispatchContext which among other things keeps a reference to the applications classloader. > > *Dispatch Context* > > The DispatchContext is created by the LocalDispatcher upon instantiation. This is the runtime dispatcher context. It contains necessary information to process services for each dispatcher. This context contains the reference to each of the service definition files, the classloader which should be used for invocation, a reference to the delegator and its dispatcher along with a 'bag' of user defined attributes. This context is passed on to each service when invoked and is used by the dispatcher to determine the service's model. > > ======================================== > > Jacopo > |
In reply to this post by Jacopo Cappellato-4
Inline...
Adrian Crum Sandglass Software www.sandglass-software.com On 8/31/2014 10:36 AM, Jacopo Cappellato wrote: > Thank you Adrian, > > please see inline: > > On Aug 31, 2014, at 10:50 AM, Adrian Crum <[hidden email]> wrote: > >> Currently, the service engine classes are a bit muddled. GenericDispatcher (the default LocalDispatcher implementation) and DispatchContext have feature envy. Some functionality in DispatchContext belongs in ServiceDispatcher. >> >> I would like to clean this up a bit and provide better separation of concerns. The service engine API will not change - the work will be done "under the hood." >> >> My hope is to get the code to a point where DispatchContext becomes less integral to its neighbors, and it becomes more of a single-use lightweight container. Again, this will not change the API at all. > > I think it would help the other committers to provide feedback if you could (in just a few sentences) summarize the following: > a) what is the original concern of DispatchContext and of GenericDispatcher I replied to this in another post. > b) where do you see that this concerns are not well implemented in the code > c) if you would like to modify #a, quickly describe the new purposes of the classes in your vision DispatchContext becomes a lightweight container for objects that a service implementation needs. Instead of relatively-constant instances being kept inside a GenericDispatcher, a new instance is created on-the-fly whenever a service is invoked. > d) what are the methods/fields that you would like to move from DispatchContext It's the other way around. I want to remove the DispatchContext reference from GenericDispatcher. > > Thanks > > Jacopo > > PS: in general I also see a lot of code that should be cleaned in DispatchContext; for example, the initialization of the static map in the constructor, the method getAllServiceNames() that should be static, bad thread-safe implementation, probably the modelServiceMapByModel should not even be there... Exactly. From my perspective, that code should be in ServiceDispatcher. > >> >> However, it will open up the possibility to improve the API. For example, instead of this service implementation: >> >> public static Map<String, Object> myService(DispatchContext dctx, Map<String, Object> context) { >> Locale locale = (Locale) context.get("locale"); >> ... >> >> we could have: >> >> public static Map<String, Object> myService(DispatchContext dctx) { >> Locale locale = dctx.getParameter("locale"); >> ... >> >> >> What do you think? >> >> >> -- >> Adrian Crum >> Sandglass Software >> www.sandglass-software.com > |
In reply to this post by Jacopo Cappellato-4
That is a nice piece of explanation. Should be part of the documentation, if it is not already.
Regards, Pierre Sent from my iPhone > On 31 aug. 2014, at 11:56, Jacopo Cappellato <[hidden email]> wrote: > > >> On Aug 31, 2014, at 11:36 AM, Jacopo Cappellato <[hidden email]> wrote: >> >> a) what is the original concern of DispatchContext and of GenericDispatcher > > This comes from this old but still interesting document: http://ofbiz.apache.org/docs/services.html > > ======================================== > *Service Dispatcher* > > The Service Dispatcher handles dispatching services to the appropriate Service Engine where it is then invoked. There is exactly one ServiceDispatcher for each Entity Delegator. If there are multiple delegators in an application there will also be multiple dispatchers. The ServiceDispatcher is accessed via a LocalDispatcher. There can be many LocalDispatchers associated with a ServiceDispatcher. Each LocalDispatcher is uniquely named and contains its own list of service definitions. When creating an instance of a LocalDispatcher, a DispatchContext is also created and passed to the ServiceEngine. > > A LocalDispatcher is associated with an application. Applications never talk directly to the ServiceDispatcher. The LocalDispatcher contains an API for invoking services, which are routed through the ServiceDispather. However, applications may be running in different threads than the actual ServiceDispatcher, so it is left to the LocalDispatcher to keep a DispatchContext which among other things keeps a reference to the applications classloader. > > *Dispatch Context* > > The DispatchContext is created by the LocalDispatcher upon instantiation. This is the runtime dispatcher context. It contains necessary information to process services for each dispatcher. This context contains the reference to each of the service definition files, the classloader which should be used for invocation, a reference to the delegator and its dispatcher along with a 'bag' of user defined attributes. This context is passed on to each service when invoked and is used by the dispatcher to determine the service's model. > > ======================================== > > Jacopo |
In reply to this post by Jacopo Cappellato-4
That's fine. The opportunity will be there if anyone wants to explore
it. It is not something I will push for. Adrian Crum Sandglass Software www.sandglass-software.com On 8/31/2014 10:54 AM, Jacopo Cappellato wrote: > > On Aug 31, 2014, at 10:50 AM, Adrian Crum <[hidden email]> wrote: > >> However, it will open up the possibility to improve the API. For example, instead of this service implementation: >> >> public static Map<String, Object> myService(DispatchContext dctx, Map<String, Object> context) { >> Locale locale = (Locale) context.get("locale"); >> ... >> >> we could have: >> >> public static Map<String, Object> myService(DispatchContext dctx) { >> Locale locale = dctx.getParameter("locale"); >> ... > > DispatchContext and the Map with input parameters have a completely different purpose and I don't think this would be an "improvement" per se. We will have to revisit this conversation at some point to be sure we are in the same page. > > Jacopo > |
Administrator
|
In reply to this post by Pierre Smits
For now there is only a link to services.html from the tutorial.
The best place to add another link seems https://cwiki.apache.org/confluence/display/OFBTECH/Service+Engine+Guide Jacques Le 31/08/2014 12:14, Pierre @GMail a écrit : > That is a nice piece of explanation. Should be part of the documentation, if it is not already. > > Regards, > > Pierre > > Sent from my iPhone > >> On 31 aug. 2014, at 11:56, Jacopo Cappellato <[hidden email]> wrote: >> >> >>> On Aug 31, 2014, at 11:36 AM, Jacopo Cappellato <[hidden email]> wrote: >>> >>> a) what is the original concern of DispatchContext and of GenericDispatcher >> This comes from this old but still interesting document: http://ofbiz.apache.org/docs/services.html >> >> ======================================== >> *Service Dispatcher* >> >> The Service Dispatcher handles dispatching services to the appropriate Service Engine where it is then invoked. There is exactly one ServiceDispatcher for each Entity Delegator. If there are multiple delegators in an application there will also be multiple dispatchers. The ServiceDispatcher is accessed via a LocalDispatcher. There can be many LocalDispatchers associated with a ServiceDispatcher. Each LocalDispatcher is uniquely named and contains its own list of service definitions. When creating an instance of a LocalDispatcher, a DispatchContext is also created and passed to the ServiceEngine. >> >> A LocalDispatcher is associated with an application. Applications never talk directly to the ServiceDispatcher. The LocalDispatcher contains an API for invoking services, which are routed through the ServiceDispather. However, applications may be running in different threads than the actual ServiceDispatcher, so it is left to the LocalDispatcher to keep a DispatchContext which among other things keeps a reference to the applications classloader. >> >> *Dispatch Context* >> >> The DispatchContext is created by the LocalDispatcher upon instantiation. This is the runtime dispatcher context. It contains necessary information to process services for each dispatcher. This context contains the reference to each of the service definition files, the classloader which should be used for invocation, a reference to the delegator and its dispatcher along with a 'bag' of user defined attributes. This context is passed on to each service when invoked and is used by the dispatcher to determine the service's model. >> >> ======================================== >> >> Jacopo > |
In reply to this post by Jacopo Cappellato-4
Here is some ASCII art that might help:
DispatchContext (a container for objects needed by service implementations) | |__ (references) LocalDispatcher (an application-specific service dispatcher) | |__ (delegates to) ServiceDispatcher | |__ (based on) Delegator Adrian Crum Sandglass Software www.sandglass-software.com On 8/31/2014 10:36 AM, Jacopo Cappellato wrote: > Thank you Adrian, > > please see inline: > > On Aug 31, 2014, at 10:50 AM, Adrian Crum <[hidden email]> wrote: > >> Currently, the service engine classes are a bit muddled. GenericDispatcher (the default LocalDispatcher implementation) and DispatchContext have feature envy. Some functionality in DispatchContext belongs in ServiceDispatcher. >> >> I would like to clean this up a bit and provide better separation of concerns. The service engine API will not change - the work will be done "under the hood." >> >> My hope is to get the code to a point where DispatchContext becomes less integral to its neighbors, and it becomes more of a single-use lightweight container. Again, this will not change the API at all. > > I think it would help the other committers to provide feedback if you could (in just a few sentences) summarize the following: > a) what is the original concern of DispatchContext and of GenericDispatcher > b) where do you see that this concerns are not well implemented in the code > c) if you would like to modify #a, quickly describe the new purposes of the classes in your vision > d) what are the methods/fields that you would like to move from DispatchContext > > Thanks > > Jacopo > > PS: in general I also see a lot of code that should be cleaned in DispatchContext; for example, the initialization of the static map in the constructor, the method getAllServiceNames() that should be static, bad thread-safe implementation, probably the modelServiceMapByModel should not even be there... > >> >> However, it will open up the possibility to improve the API. For example, instead of this service implementation: >> >> public static Map<String, Object> myService(DispatchContext dctx, Map<String, Object> context) { >> Locale locale = (Locale) context.get("locale"); >> ... >> >> we could have: >> >> public static Map<String, Object> myService(DispatchContext dctx) { >> Locale locale = dctx.getParameter("locale"); >> ... >> >> >> What do you think? >> >> >> -- >> Adrian Crum >> Sandglass Software >> www.sandglass-software.com > |
In reply to this post by Adrian Crum-3
I have this done. The change is quite subtle, and it will pave the way
for removing GenericDelegator references from the service engine. I will wait a few days before committing so others have time to respond. Adrian Crum Sandglass Software www.sandglass-software.com On 8/31/2014 9:50 AM, Adrian Crum wrote: > Currently, the service engine classes are a bit muddled. > GenericDispatcher (the default LocalDispatcher implementation) and > DispatchContext have feature envy. Some functionality in DispatchContext > belongs in ServiceDispatcher. > > I would like to clean this up a bit and provide better separation of > concerns. The service engine API will not change - the work will be done > "under the hood." > > My hope is to get the code to a point where DispatchContext becomes less > integral to its neighbors, and it becomes more of a single-use > lightweight container. Again, this will not change the API at all. > > However, it will open up the possibility to improve the API. For > example, instead of this service implementation: > > public static Map<String, Object> myService(DispatchContext dctx, > Map<String, Object> context) { > Locale locale = (Locale) context.get("locale"); > ... > > we could have: > > public static Map<String, Object> myService(DispatchContext dctx) { > Locale locale = dctx.getParameter("locale"); > ... > > > What do you think? > > |
I would like to review your work; could you please commit the changes to an experimental branch or at least share a patch in Jira?
Thanks, Jacopo On Aug 31, 2014, at 8:35 PM, Adrian Crum <[hidden email]> wrote: > I have this done. The change is quite subtle, and it will pave the way for removing GenericDelegator references from the service engine. > > I will wait a few days before committing so others have time to respond. > > Adrian Crum > Sandglass Software > www.sandglass-software.com > > On 8/31/2014 9:50 AM, Adrian Crum wrote: >> Currently, the service engine classes are a bit muddled. >> GenericDispatcher (the default LocalDispatcher implementation) and >> DispatchContext have feature envy. Some functionality in DispatchContext >> belongs in ServiceDispatcher. >> >> I would like to clean this up a bit and provide better separation of >> concerns. The service engine API will not change - the work will be done >> "under the hood." >> >> My hope is to get the code to a point where DispatchContext becomes less >> integral to its neighbors, and it becomes more of a single-use >> lightweight container. Again, this will not change the API at all. >> >> However, it will open up the possibility to improve the API. For >> example, instead of this service implementation: >> >> public static Map<String, Object> myService(DispatchContext dctx, >> Map<String, Object> context) { >> Locale locale = (Locale) context.get("locale"); >> ... >> >> we could have: >> >> public static Map<String, Object> myService(DispatchContext dctx) { >> Locale locale = dctx.getParameter("locale"); >> ... >> >> >> What do you think? >> >> |
https://issues.apache.org/jira/browse/OFBIZ-5743
Adrian Crum Sandglass Software www.sandglass-software.com On 9/1/2014 7:26 AM, Jacopo Cappellato wrote: > I would like to review your work; could you please commit the changes to an experimental branch or at least share a patch in Jira? > > Thanks, > > Jacopo > > On Aug 31, 2014, at 8:35 PM, Adrian Crum <[hidden email]> wrote: > >> I have this done. The change is quite subtle, and it will pave the way for removing GenericDelegator references from the service engine. >> >> I will wait a few days before committing so others have time to respond. >> >> Adrian Crum >> Sandglass Software >> www.sandglass-software.com >> >> On 8/31/2014 9:50 AM, Adrian Crum wrote: >>> Currently, the service engine classes are a bit muddled. >>> GenericDispatcher (the default LocalDispatcher implementation) and >>> DispatchContext have feature envy. Some functionality in DispatchContext >>> belongs in ServiceDispatcher. >>> >>> I would like to clean this up a bit and provide better separation of >>> concerns. The service engine API will not change - the work will be done >>> "under the hood." >>> >>> My hope is to get the code to a point where DispatchContext becomes less >>> integral to its neighbors, and it becomes more of a single-use >>> lightweight container. Again, this will not change the API at all. >>> >>> However, it will open up the possibility to improve the API. For >>> example, instead of this service implementation: >>> >>> public static Map<String, Object> myService(DispatchContext dctx, >>> Map<String, Object> context) { >>> Locale locale = (Locale) context.get("locale"); >>> ... >>> >>> we could have: >>> >>> public static Map<String, Object> myService(DispatchContext dctx) { >>> Locale locale = dctx.getParameter("locale"); >>> ... >>> >>> >>> What do you think? >>> >>> > |
Free forum by Nabble | Edit this page |