Re: [OFBIZ-12801] "Error at CommunicationEventServices.groovy:489"
+1
A much better explanation of my thoughts than I could have put into words.
Thanks,
Michael
Am 02.05.23 um 11:43 schrieb Daniel Watford:
Hi Nicholas,
I can see value in the approach I think you are proposing (please correct
me if I am wrong) which I would summarise as:
- All scripts are considered service implementations and will handle
parameters and return types accordingly, and
- GroovyEventHandler should invoke an event handler as if it was a service
implementation and convert the result (i.e. a Map) in a way
similar to that in the GroovyEventHandler code that you quoted.
GroovyEventHandler effectively becomes an adapter between service calling
conventions and event-handler calling conventions.
The problem I see with the above approach is that an event handler may need
to access the Request object, something that service implementations cannot
do. If we force all groovy event handlers to be implemented as service
implementations which are later 'adapted' to the event handler calling
conventions, then we are making it difficult for the developer to
understand what interface (calling conventions) they are working with.
Taking my previous suggestion further, we could implement logic around the
following test in GroovyEventHandler and GroovyEngine respectively:
script
.getClass().getDeclaredMethod(event.getInvoke()).getReturnType().isAssignableFrom(EventResponse.
class)
script
.getClass().getDeclaredMethod(modelService.getInvoke()).getReturnType().isAssignableFrom(ServiceResponse.
class)
If the methods to be called by GroovyEventHandler and GroovyEngine do not
declare the expected return type, then a warning can be logged.
For the simple 'adapter' cases, we could implement helper methods in
GroovyBaseScript to convert between EventResponse and ServiceResponse
objects, but for more complicated cases, the method author is free to
implement an adapter appropriate to their business rules.
A principle that drives the above, in my opinion at least, is that a method
author MUST know whether they are implementing a service or an event
handler since there may be differences in context that they depend on. By
incorporating the EventReponse and ServiceReponse types the author can make
it clear to the reader (and to OFBiz) which context the code was written
for.
Thanks,
Dan.
On Fri, 28 Apr 2023 at 16:53, Nicolas Malin
wrote:
Hi,
My preference would be to the simplest for developer and keep the three
word (error, failure and success). For that, we can force the return of
each function as Map.
After delegate the problematic to each handler. By the way, the
GroovyEventHandler that call GroovyBaseScript already support an output
as Map or as String :
// GroovyEventHandler.java:117 [1]
// check the result
if (result instanceof Map) {
Map resultMap = UtilGenerics.cast(result);
String successMessage = (String)
resultMap.get("_event_message_");
if (successMessage != null) {
request.setAttribute("_EVENT_MESSAGE_",
successMessage);
}
String errorMessage = (String)
resultMap.get("_error_message_");
if (errorMessage != null) {
request.setAttribute("_ERROR_MESSAGE_", errorMessage);
}
return (String) resultMap.get("_response_code_");
}
if (result != null && !(result instanceof String)) {
throw new EventHandlerException("Event did not return a
String result, it returned a " + result.getClass().getName());
}
return (String) result;
If I understand well the problematic, move the return to Map, and
improve GroovyEventHandler to support the groovy script return (better
than what it did currently) would be cover all case ?
Cheers,
Nicolas
[1]
https://github.com/apache/ofbiz-framework/blob/64d012d2c20d76200cedd3e1861b720d55a61398/framework/webapp/src/main/java/org/apache/ofbiz/webapp/event/GroovyEventHandler.java#L117
On 28/04/2023 16:07, Daniel Watford wrote:
Hi Gil,
I don't think we need to go as far as creating a new GroovyBaseClass.
Deprecating GroovyBaseScript:success is still preferred in my opinion,
replacing it with serviceSuccess and scriptSuccess. These two methods
could
return separate types which extend from Map and help make it clear
whether
the method in the groovyScript is intended by the author to implement a
service or an event handler.
In the rare cases of a current groovy method being used to implement
both a
service and an event handler, I think we could do something similar to
this
contrived example:
/
// GroovyBaseScript.groovy
interface ServiceResponse extends Map
interface EventResponse extends Map
ServiceResponse serviceSuccess() { ... }
ServiceResponse serviceFail(...) {...}
ServiceResponse serviceError(...) {...}
EventResponse eventSuccess(...)
Re: [OFBIZ-12801] "Error at CommunicationEventServices.groovy:489"
Hi Nicholas,
I can see value in the approach I think you are proposing (please correct
me if I am wrong) which I would summarise as:
- All scripts are considered service implementations and will handle
parameters and return types accordingly, and
- GroovyEventHandler should invoke an event handler as if it was a service
implementation and convert the result (i.e. a Map) in a way
similar to that in the GroovyEventHandler code that you quoted.
GroovyEventHandler effectively becomes an adapter between service calling
conventions and event-handler calling conventions.
The problem I see with the above approach is that an event handler may need
to access the Request object, something that service implementations cannot
do. If we force all groovy event handlers to be implemented as service
implementations which are later 'adapted' to the event handler calling
conventions, then we are making it difficult for the developer to
understand what interface (calling conventions) they are working with.
Taking my previous suggestion further, we could implement logic around the
following test in GroovyEventHandler and GroovyEngine respectively:
script
.getClass().getDeclaredMethod(event.getInvoke()).getReturnType().isAssignableFrom(EventResponse.
class)
script
.getClass().getDeclaredMethod(modelService.getInvoke()).getReturnType().isAssignableFrom(ServiceResponse.
class)
If the methods to be called by GroovyEventHandler and GroovyEngine do not
declare the expected return type, then a warning can be logged.
For the simple 'adapter' cases, we could implement helper methods in
GroovyBaseScript to convert between EventResponse and ServiceResponse
objects, but for more complicated cases, the method author is free to
implement an adapter appropriate to their business rules.
A principle that drives the above, in my opinion at least, is that a method
author MUST know whether they are implementing a service or an event
handler since there may be differences in context that they depend on. By
incorporating the EventReponse and ServiceReponse types the author can make
it clear to the reader (and to OFBiz) which context the code was written
for.
Thanks,
Dan.
On Fri, 28 Apr 2023 at 16:53, Nicolas Malin
wrote:
> Hi,
>
> My preference would be to the simplest for developer and keep the three
> word (error, failure and success). For that, we can force the return of
> each function as Map.
>
> After delegate the problematic to each handler. By the way, the
> GroovyEventHandler that call GroovyBaseScript already support an output
> as Map or as String :
>
> // GroovyEventHandler.java:117 [1]
>
> // check the result
> if (result instanceof Map) {
> Map resultMap = UtilGenerics.cast(result);
> String successMessage = (String)
> resultMap.get("_event_message_");
> if (successMessage != null) {
> request.setAttribute("_EVENT_MESSAGE_",
> successMessage);
> }
> String errorMessage = (String)
> resultMap.get("_error_message_");
> if (errorMessage != null) {
> request.setAttribute("_ERROR_MESSAGE_", errorMessage);
> }
> return (String) resultMap.get("_response_code_");
> }
> if (result != null && !(result instanceof String)) {
> throw new EventHandlerException("Event did not return a
> String result, it returned a " + result.getClass().getName());
> }
> return (String) result;
>
>
> If I understand well the problematic, move the return to Map, and
> improve GroovyEventHandler to support the groovy script return (better
> than what it did currently) would be cover all case ?
>
> Cheers,
> Nicolas
>
> [1]
>
> https://github.com/apache/ofbiz-framework/blob/64d012d2c20d76200cedd3e1861b720d55a61398/framework/webapp/src/main/java/org/apache/ofbiz/webapp/event/GroovyEventHandler.java#L117
>
> On 28/04/2023 16:07, Daniel Watford wrote:
> > Hi Gil,
> >
> > I don't think we need to go as far as creating a new GroovyBaseClass.
> >
> > Deprecating GroovyBaseScript:success is still preferred in my opinion,
> > replacing it with serviceSuccess and scriptSuccess. These two methods
> could
> > return separate types which extend from Map and help make it clear
> whether
> > the method in the groovyScript is intended by the author to implement a
> > service or an event handler.
> >
> > In the rare cases of a current groovy method being used to implement
> both a
> > service and an event handler, I think we could do something similar to
> this
> > contrived example:
> >
> > /
> > // GroovyBaseScript.groovy
> >
> > interface ServiceResponse extends Map
> >
> > interface EventResponse extends Map
> >
> > ServiceResponse serviceSuccess() { ... }
> > ServiceResponse serviceFail(...) {...}
> > ServiceResponse serviceError(...) {...}
> >
> > EventResponse
Re: [OFBIZ-12801] "Error at CommunicationEventServices.groovy:489"
Hi,
My preference would be to the simplest for developer and keep the three
word (error, failure and success). For that, we can force the return of
each function as Map.
After delegate the problematic to each handler. By the way, the
GroovyEventHandler that call GroovyBaseScript already support an output
as Map or as String :
// GroovyEventHandler.java:117 [1]
// check the result
if (result instanceof Map) {
Map resultMap = UtilGenerics.cast(result);
String successMessage = (String)
resultMap.get("_event_message_");
if (successMessage != null) {
request.setAttribute("_EVENT_MESSAGE_",
successMessage);
}
String errorMessage = (String)
resultMap.get("_error_message_");
if (errorMessage != null) {
request.setAttribute("_ERROR_MESSAGE_", errorMessage);
}
return (String) resultMap.get("_response_code_");
}
if (result != null && !(result instanceof String)) {
throw new EventHandlerException("Event did not return a
String result, it returned a " + result.getClass().getName());
}
return (String) result;
If I understand well the problematic, move the return to Map, and
improve GroovyEventHandler to support the groovy script return (better
than what it did currently) would be cover all case ?
Cheers,
Nicolas
[1]
https://github.com/apache/ofbiz-framework/blob/64d012d2c20d76200cedd3e1861b720d55a61398/framework/webapp/src/main/java/org/apache/ofbiz/webapp/event/GroovyEventHandler.java#L117
On 28/04/2023 16:07, Daniel Watford wrote:
Hi Gil,
I don't think we need to go as far as creating a new GroovyBaseClass.
Deprecating GroovyBaseScript:success is still preferred in my opinion,
replacing it with serviceSuccess and scriptSuccess. These two methods could
return separate types which extend from Map and help make it clear whether
the method in the groovyScript is intended by the author to implement a
service or an event handler.
In the rare cases of a current groovy method being used to implement both a
service and an event handler, I think we could do something similar to this
contrived example:
/
// GroovyBaseScript.groovy
interface ServiceResponse extends Map
interface EventResponse extends Map
ServiceResponse serviceSuccess() { ... }
ServiceResponse serviceFail(...) {...}
ServiceResponse serviceError(...) {...}
EventResponse eventSuccess(...) {...}
EventResponse eventFail(...) {...}
EventResponse eventError(...) {...}
//
// ExampleServiceAndEventImpl.groovy
// Here is the main implementation, initially implemented as a service
handler
ServiceResponse countProducts () {
...
...
return serviceSuccess([stock: 42])
// Here is the event handler implementation, leveraging the service
implementation as an adapter.
EventResponse countProductsEventHandler () {
ServiceResponse sr = countProducts();
return eventSuccess("Found ${sr.stock} products");
}
The return type of the above methods help identify whether we are dealing
with a service or event handler. If conversion is needed from one type to
the other, an adapter specific to the business logic can decide how to map
between EventResponses and ServiceResponses.
Thanks,
Dan.
On Fri, 28 Apr 2023 at 14:27, Gil Portenseigne
wrote:
Hello I got a quick look about it, having two separate class means
having two distinct groovy DSL and need lot of modification that IMO are
not worth the complexity for this subject.
I now only wonder, is it that bad too keep that one exception (about
untyped return) for GroovyBaseScript::success ?
Gil
Le 26/04/2023 à 09:49, Gil Portenseigne a écrit :
Hello,
I like the idea that the developer do not have to sync about which
method to use.
If I understand well what Michael envision, i.e. to use for event a
new GroovyBaseEvent class, and for services/scripts a GroovyBaseScript
class, that both extends a common class for the common code, should be
one way to allow this usage.
But I don't know about IDE integration behavior of such a solution...
Do you think that is worth a look ?
I will just add that there is a chance that project implementation are
using groovy script as the event target (I know some ;) )
Thanks,
Gil
Le 20/04/2023 à 17:13, Michael Brohl a écrit :
To have it even more clear, I would separate logic for events and
services.
The GroovyBaseScript in the service engine package should only be
used for services and there should be another one for events, if
really needed. Mixing both together is bad practice IMO. There seem
to be only 7 controller entries using a groovy script as the event
target.
Best regards,
Michael Brohl
ecomify GmbH -www.ecomify.de
Am 20.04.23 um 16:49 schrieb Jacques Le Roux:
Hi Daniel,
I dont think there is a knowledge about methods being both services
Re: [OFBIZ-12801] "Error at CommunicationEventServices.groovy:489"
Hi Gil,
I don't think we need to go as far as creating a new GroovyBaseClass.
Deprecating GroovyBaseScript:success is still preferred in my opinion,
replacing it with serviceSuccess and scriptSuccess. These two methods could
return separate types which extend from Map and help make it clear whether
the method in the groovyScript is intended by the author to implement a
service or an event handler.
In the rare cases of a current groovy method being used to implement both a
service and an event handler, I think we could do something similar to this
contrived example:
/
// GroovyBaseScript.groovy
interface ServiceResponse extends Map
interface EventResponse extends Map
ServiceResponse serviceSuccess() { ... }
ServiceResponse serviceFail(...) {...}
ServiceResponse serviceError(...) {...}
EventResponse eventSuccess(...) {...}
EventResponse eventFail(...) {...}
EventResponse eventError(...) {...}
//
// ExampleServiceAndEventImpl.groovy
// Here is the main implementation, initially implemented as a service
handler
ServiceResponse countProducts () {
...
...
return serviceSuccess([stock: 42])
// Here is the event handler implementation, leveraging the service
implementation as an adapter.
EventResponse countProductsEventHandler () {
ServiceResponse sr = countProducts();
return eventSuccess("Found ${sr.stock} products");
}
The return type of the above methods help identify whether we are dealing
with a service or event handler. If conversion is needed from one type to
the other, an adapter specific to the business logic can decide how to map
between EventResponses and ServiceResponses.
Thanks,
Dan.
On Fri, 28 Apr 2023 at 14:27, Gil Portenseigne
wrote:
> Hello I got a quick look about it, having two separate class means
> having two distinct groovy DSL and need lot of modification that IMO are
> not worth the complexity for this subject.
>
> I now only wonder, is it that bad too keep that one exception (about
> untyped return) for GroovyBaseScript::success ?
>
> Gil
>
> Le 26/04/2023 à 09:49, Gil Portenseigne a écrit :
> > Hello,
> >
> > I like the idea that the developer do not have to sync about which
> > method to use.
> >
> > If I understand well what Michael envision, i.e. to use for event a
> > new GroovyBaseEvent class, and for services/scripts a GroovyBaseScript
> > class, that both extends a common class for the common code, should be
> > one way to allow this usage.
> >
> > But I don't know about IDE integration behavior of such a solution...
> >
> > Do you think that is worth a look ?
> >
> > I will just add that there is a chance that project implementation are
> > using groovy script as the event target (I know some ;) )
> >
> > Thanks,
> >
> > Gil
> >
> > Le 20/04/2023 à 17:13, Michael Brohl a écrit :
> >> To have it even more clear, I would separate logic for events and
> >> services.
> >>
> >> The GroovyBaseScript in the service engine package should only be
> >> used for services and there should be another one for events, if
> >> really needed. Mixing both together is bad practice IMO. There seem
> >> to be only 7 controller entries using a groovy script as the event
> >> target.
> >>
> >> Best regards,
> >>
> >> Michael Brohl
> >>
> >> ecomify GmbH - www.ecomify.de
> >>
> >>
> >> Am 20.04.23 um 16:49 schrieb Jacques Le Roux:
> >>> Hi Daniel,
> >>>
> >>> I dont think there is a knowledge about methods being both services
> >>> and events. I think there are not (much?) such cases.
> >>> Being acquainted to OFBiz logs I did not check the trunk demo log
> >>> content (now in Docker);
> >>> so I wonder if there are such other cases than
> >>> CommunicationEventServices::sendEmail (colon notation is available
> >>> in Groovy 3)
> >>> that bots and demo uses could have generated.
> >>>
> >>> I tend to agree about having GroovyBaseScript::success deprecated
> >>> and replaced with methods GroovyBaseScript::scriptSuccess
> >>> GroovyBaseScript::serviceSuccess and GroovyCaseScript::eventSuccess
> >>>
> >>> I'm not yet acquainted with Codernarc rules, but the changes in
> >>> GroovyBaseScript seem straightforward.
> >>> And (hopefully) this should not be a big deal to change accordingly
> >>> in scripts methods with the help of Codenarc, right ?
> >>>
> >>> My 2 cts
> >>>
> >>> Jacques
> >>>
> >>> Le 19/04/2023 à 18:37, Daniel Watford a écrit :
> Hello,
>
> In my opinion, the semantics of calling an event handler vs a service
> implementation are different, albeit similar enough that most
> handler/implementation authors wouldn't necessarily care how the
> code was
> called.
>
> The untyped nature of Groovy had allowed a certain degree of
> flexibility in
> code that GroovyBaseScript#success could be relied upon to prepare a
> response appropriate to the calling conventions of an event handler or
> service implementation. However over the last decade, possibly
> driven
Re: [OFBIZ-12801] "Error at CommunicationEventServices.groovy:489"
Hello I got a quick look about it, having two separate class means having two distinct groovy DSL and need lot of modification that IMO are not worth the complexity for this subject. I now only wonder, is it that bad too keep that one exception (about untyped return) for GroovyBaseScript::success ? Gil Le 26/04/2023 à 09:49, Gil Portenseigne a écrit : Hello, I like the idea that the developer do not have to sync about which method to use. If I understand well what Michael envision, i.e. to use for event a new GroovyBaseEvent class, and for services/scripts a GroovyBaseScript class, that both extends a common class for the common code, should be one way to allow this usage. But I don't know about IDE integration behavior of such a solution... Do you think that is worth a look ? I will just add that there is a chance that project implementation are using groovy script as the event target (I know some ;) ) Thanks, Gil Le 20/04/2023 à 17:13, Michael Brohl a écrit : To have it even more clear, I would separate logic for events and services. The GroovyBaseScript in the service engine package should only be used for services and there should be another one for events, if really needed. Mixing both together is bad practice IMO. There seem to be only 7 controller entries using a groovy script as the event target. Best regards, Michael Brohl ecomify GmbH - www.ecomify.de Am 20.04.23 um 16:49 schrieb Jacques Le Roux: Hi Daniel, I dont think there is a knowledge about methods being both services and events. I think there are not (much?) such cases. Being acquainted to OFBiz logs I did not check the trunk demo log content (now in Docker); so I wonder if there are such other cases than CommunicationEventServices::sendEmail (colon notation is available in Groovy 3) that bots and demo uses could have generated. I tend to agree about having GroovyBaseScript::success deprecated and replaced with methods GroovyBaseScript::scriptSuccess GroovyBaseScript::serviceSuccess and GroovyCaseScript::eventSuccess I'm not yet acquainted with Codernarc rules, but the changes in GroovyBaseScript seem straightforward. And (hopefully) this should not be a big deal to change accordingly in scripts methods with the help of Codenarc, right ? My 2 cts Jacques Le 19/04/2023 à 18:37, Daniel Watford a écrit : Hello, In my opinion, the semantics of calling an event handler vs a service implementation are different, albeit similar enough that most handler/implementation authors wouldn't necessarily care how the code was called. The untyped nature of Groovy had allowed a certain degree of flexibility in code that GroovyBaseScript#success could be relied upon to prepare a response appropriate to the calling conventions of an event handler or service implementation. However over the last decade, possibly driven by increased use of linters/static analysers, we have seen a push back towards explicit typing, particularly on public methods. If we continue to adopt the guidance from static analysers and apply explicit typing to public methods in our groovy code, then we need to avoid the black box approach of GroovyBaseScript#success figuring out what calling conventions (i.e. event or service) are in play and, instead, a groovy method should be intentionally written as either a service or event handler. If we have cases where a groovy method is used to provide implementations for both a service and an event handler, then we can employ a thin adapter layer to convert the result type between the two calling conventions. Do we know if many such cases currently exist in OFBiz? My preference would be to see GroovyBaseScript#success deprecated and replaced with methods along the lines of GroovyBaseScript#scriptSuccess and GroovyCaseScript#eventSuccess that return a Map and String respectively. Thanks, Dan. On Wed, 19 Apr 2023 at 16:44, Jacques Le Roux wrote: Hi All, At OFBIZ-12801, we had a discussion with Daniel and Gil about the described issue (last comments there) We are unsure of the best solution, so this thread to discuss and decide. As Gil reported, Jacopo's comment of the related commit* contains <> What would be your opinion about a best solution? TIA Jacques *http://svn.apache.org/viewvc?view=revision=1298908
Re: [OFBIZ-12801] "Error at CommunicationEventServices.groovy:489"
Hello, I like the idea that the developer do not have to sync about which method to use. If I understand well what Michael envision, i.e. to use for event a new GroovyBaseEvent class, and for services/scripts a GroovyBaseScript class, that both extends a common class for the common code, should be one way to allow this usage. But I don't know about IDE integration behavior of such a solution... Do you think that is worth a look ? I will just add that there is a chance that project implementation are using groovy script as the event target (I know some ;) ) Thanks, Gil Le 20/04/2023 à 17:13, Michael Brohl a écrit : To have it even more clear, I would separate logic for events and services. The GroovyBaseScript in the service engine package should only be used for services and there should be another one for events, if really needed. Mixing both together is bad practice IMO. There seem to be only 7 controller entries using a groovy script as the event target. Best regards, Michael Brohl ecomify GmbH - www.ecomify.de Am 20.04.23 um 16:49 schrieb Jacques Le Roux: Hi Daniel, I dont think there is a knowledge about methods being both services and events. I think there are not (much?) such cases. Being acquainted to OFBiz logs I did not check the trunk demo log content (now in Docker); so I wonder if there are such other cases than CommunicationEventServices::sendEmail (colon notation is available in Groovy 3) that bots and demo uses could have generated. I tend to agree about having GroovyBaseScript::success deprecated and replaced with methods GroovyBaseScript::scriptSuccess GroovyBaseScript::serviceSuccess and GroovyCaseScript::eventSuccess I'm not yet acquainted with Codernarc rules, but the changes in GroovyBaseScript seem straightforward. And (hopefully) this should not be a big deal to change accordingly in scripts methods with the help of Codenarc, right ? My 2 cts Jacques Le 19/04/2023 à 18:37, Daniel Watford a écrit : Hello, In my opinion, the semantics of calling an event handler vs a service implementation are different, albeit similar enough that most handler/implementation authors wouldn't necessarily care how the code was called. The untyped nature of Groovy had allowed a certain degree of flexibility in code that GroovyBaseScript#success could be relied upon to prepare a response appropriate to the calling conventions of an event handler or service implementation. However over the last decade, possibly driven by increased use of linters/static analysers, we have seen a push back towards explicit typing, particularly on public methods. If we continue to adopt the guidance from static analysers and apply explicit typing to public methods in our groovy code, then we need to avoid the black box approach of GroovyBaseScript#success figuring out what calling conventions (i.e. event or service) are in play and, instead, a groovy method should be intentionally written as either a service or event handler. If we have cases where a groovy method is used to provide implementations for both a service and an event handler, then we can employ a thin adapter layer to convert the result type between the two calling conventions. Do we know if many such cases currently exist in OFBiz? My preference would be to see GroovyBaseScript#success deprecated and replaced with methods along the lines of GroovyBaseScript#scriptSuccess and GroovyCaseScript#eventSuccess that return a Map and String respectively. Thanks, Dan. On Wed, 19 Apr 2023 at 16:44, Jacques Le Roux wrote: Hi All, At OFBIZ-12801, we had a discussion with Daniel and Gil about the described issue (last comments there) We are unsure of the best solution, so this thread to discuss and decide. As Gil reported, Jacopo's comment of the related commit* contains <> What would be your opinion about a best solution? TIA Jacques *http://svn.apache.org/viewvc?view=revision=1298908
Re: [OFBIZ-12801] "Error at CommunicationEventServices.groovy:489"
To have it even more clear, I would separate logic for events and services. The GroovyBaseScript in the service engine package should only be used for services and there should be another one for events, if really needed. Mixing both together is bad practice IMO. There seem to be only 7 controller entries using a groovy script as the event target. Best regards, Michael Brohl ecomify GmbH - www.ecomify.de Am 20.04.23 um 16:49 schrieb Jacques Le Roux: Hi Daniel, I dont think there is a knowledge about methods being both services and events. I think there are not (much?) such cases. Being acquainted to OFBiz logs I did not check the trunk demo log content (now in Docker); so I wonder if there are such other cases than CommunicationEventServices::sendEmail (colon notation is available in Groovy 3) that bots and demo uses could have generated. I tend to agree about having GroovyBaseScript::success deprecated and replaced with methods GroovyBaseScript::scriptSuccess GroovyBaseScript::serviceSuccess and GroovyCaseScript::eventSuccess I'm not yet acquainted with Codernarc rules, but the changes in GroovyBaseScript seem straightforward. And (hopefully) this should not be a big deal to change accordingly in scripts methods with the help of Codenarc, right ? My 2 cts Jacques Le 19/04/2023 à 18:37, Daniel Watford a écrit : Hello, In my opinion, the semantics of calling an event handler vs a service implementation are different, albeit similar enough that most handler/implementation authors wouldn't necessarily care how the code was called. The untyped nature of Groovy had allowed a certain degree of flexibility in code that GroovyBaseScript#success could be relied upon to prepare a response appropriate to the calling conventions of an event handler or service implementation. However over the last decade, possibly driven by increased use of linters/static analysers, we have seen a push back towards explicit typing, particularly on public methods. If we continue to adopt the guidance from static analysers and apply explicit typing to public methods in our groovy code, then we need to avoid the black box approach of GroovyBaseScript#success figuring out what calling conventions (i.e. event or service) are in play and, instead, a groovy method should be intentionally written as either a service or event handler. If we have cases where a groovy method is used to provide implementations for both a service and an event handler, then we can employ a thin adapter layer to convert the result type between the two calling conventions. Do we know if many such cases currently exist in OFBiz? My preference would be to see GroovyBaseScript#success deprecated and replaced with methods along the lines of GroovyBaseScript#scriptSuccess and GroovyCaseScript#eventSuccess that return a Map and String respectively. Thanks, Dan. On Wed, 19 Apr 2023 at 16:44, Jacques Le Roux wrote: Hi All, At OFBIZ-12801, we had a discussion with Daniel and Gil about the described issue (last comments there) We are unsure of the best solution, so this thread to discuss and decide. As Gil reported, Jacopo's comment of the related commit* contains <> What would be your opinion about a best solution? TIA Jacques *http://svn.apache.org/viewvc?view=revision=1298908
Re: [OFBIZ-12801] "Error at CommunicationEventServices.groovy:489"
Hi Daniel, I dont think there is a knowledge about methods being both services and events. I think there are not (much?) such cases. Being acquainted to OFBiz logs I did not check the trunk demo log content (now in Docker); so I wonder if there are such other cases than CommunicationEventServices::sendEmail (colon notation is available in Groovy 3) that bots and demo uses could have generated. I tend to agree about having GroovyBaseScript::success deprecated and replaced with methods GroovyBaseScript::scriptSuccess GroovyBaseScript::serviceSuccess and GroovyCaseScript::eventSuccess I'm not yet acquainted with Codernarc rules, but the changes in GroovyBaseScript seem straightforward. And (hopefully) this should not be a big deal to change accordingly in scripts methods with the help of Codenarc, right ? My 2 cts Jacques Le 19/04/2023 à 18:37, Daniel Watford a écrit : Hello, In my opinion, the semantics of calling an event handler vs a service implementation are different, albeit similar enough that most handler/implementation authors wouldn't necessarily care how the code was called. The untyped nature of Groovy had allowed a certain degree of flexibility in code that GroovyBaseScript#success could be relied upon to prepare a response appropriate to the calling conventions of an event handler or service implementation. However over the last decade, possibly driven by increased use of linters/static analysers, we have seen a push back towards explicit typing, particularly on public methods. If we continue to adopt the guidance from static analysers and apply explicit typing to public methods in our groovy code, then we need to avoid the black box approach of GroovyBaseScript#success figuring out what calling conventions (i.e. event or service) are in play and, instead, a groovy method should be intentionally written as either a service or event handler. If we have cases where a groovy method is used to provide implementations for both a service and an event handler, then we can employ a thin adapter layer to convert the result type between the two calling conventions. Do we know if many such cases currently exist in OFBiz? My preference would be to see GroovyBaseScript#success deprecated and replaced with methods along the lines of GroovyBaseScript#scriptSuccess and GroovyCaseScript#eventSuccess that return a Map and String respectively. Thanks, Dan. On Wed, 19 Apr 2023 at 16:44, Jacques Le Roux wrote: Hi All, At OFBIZ-12801, we had a discussion with Daniel and Gil about the described issue (last comments there) We are unsure of the best solution, so this thread to discuss and decide. As Gil reported, Jacopo's comment of the related commit* contains <> What would be your opinion about a best solution? TIA Jacques *http://svn.apache.org/viewvc?view=revision=1298908
Re: [OFBIZ-12801] "Error at CommunicationEventServices.groovy:489"
Hello, In my opinion, the semantics of calling an event handler vs a service implementation are different, albeit similar enough that most handler/implementation authors wouldn't necessarily care how the code was called. The untyped nature of Groovy had allowed a certain degree of flexibility in code that GroovyBaseScript#success could be relied upon to prepare a response appropriate to the calling conventions of an event handler or service implementation. However over the last decade, possibly driven by increased use of linters/static analysers, we have seen a push back towards explicit typing, particularly on public methods. If we continue to adopt the guidance from static analysers and apply explicit typing to public methods in our groovy code, then we need to avoid the black box approach of GroovyBaseScript#success figuring out what calling conventions (i.e. event or service) are in play and, instead, a groovy method should be intentionally written as either a service or event handler. If we have cases where a groovy method is used to provide implementations for both a service and an event handler, then we can employ a thin adapter layer to convert the result type between the two calling conventions. Do we know if many such cases currently exist in OFBiz? My preference would be to see GroovyBaseScript#success deprecated and replaced with methods along the lines of GroovyBaseScript#scriptSuccess and GroovyCaseScript#eventSuccess that return a Map and String respectively. Thanks, Dan. On Wed, 19 Apr 2023 at 16:44, Jacques Le Roux wrote: > Hi All, > > At OFBIZ-12801, we had a discussion with Daniel and Gil about the > described issue (last comments there) > We are unsure of the best solution, so this thread to discuss and decide. > > As Gil reported, Jacopo's comment of the related commit* contains > > < groovy method executed as services or events in a transparent way.>> > > What would be your opinion about a best solution? > > TIA > > Jacques > > * http://svn.apache.org/viewvc?view=revision=1298908 > -- Daniel Watford
[OFBIZ-12801] "Error at CommunicationEventServices.groovy:489"
Hi All, At OFBIZ-12801, we had a discussion with Daniel and Gil about the described issue (last comments there) We are unsure of the best solution, so this thread to discuss and decide. As Gil reported, Jacopo's comment of the related commit* contains <> What would be your opinion about a best solution? TIA Jacques * http://svn.apache.org/viewvc?view=revision=1298908
