inline On Fri, Dec 30, 2016 at 11:50 AM, Jacques Le Roux < [email protected]> wrote:
> Le 30/12/2016 à 07:09, Scott Gray a écrit : > >> Jacques is correct, I was questioning the operation of the method rather >> than the method itself. >> >> But honestly I also struggle to understand why anyone would need to >> perform >> validation before sending a call to the service engine which is going to >> do >> the same thing anyway. >> > > I needed that in a case where I need to create, or not, related Entities > before calling an async service where I have no ideas if the coming context > is correct. > Wouldn't SECAs be a good option in here? or just a simple service group, or calling a service from another service. I can think of many ways to achieve this without the need for new code. > I'm also hesitant to say it's a good idea to add to the framework API >> without a use case for the OOTB applications. If our apps aren't using it >> then maybe it just belongs in a custom component. >> > > I believe, it's generic enough simple and easy to use code, so I see no > reasons to not put it there. > There are many things that are "generic" out there. If we put anything in the framework because it is generic then we would indeed have a very big code base. > Jacques > > > >> Regards >> Scott >> >> On 30/12/2016 05:10, "Jacques Le Roux" <[email protected]> >> wrote: >> >> Done at r1776447 >> >> Jacques >> >> >> >> Le 29/12/2016 à 16:56, Jacques Le Roux a écrit : >> >> BTW the current Implementation of isValid() is not only ambiguous as >>> spotted Scott but is also wrong because I should have returned true in >>> the >>> 1st block of code. Actually I did not care much about that because, as I >>> already explained it's not supposed to be called afer the service has >>> been >>> called but before, hence no RESPONSE_MESSAGE is expected. It was more a >>> negligence on my side. >>> >>> I'll code as I newly proposed below >>> >>> Jacques >>> >>> >>> Le 29/12/2016 à 15:59, Jacques Le Roux a écrit : >>> >>> Taher, >>>> >>>> I think you misunderstood what Scott said. What I understood is he was >>>> speaking about the 1st block of code in ModelService.validate() which >>>> ends >>>> with a return. >>>> >>>> I agreed that this clock of code is not needed in isValid() and removed >>>> it in my proposition. >>>> Because isValid() does not makes sense to be called with a context with >>>> a >>>> RESPONSE_MESSAGE which could be RESPOND_ERROR or RESPOND_FAIL. >>>> You should simply call isValid() with IN parameters to validate the >>>> service and its context before calling the service with this context. >>>> BTW maybe I should make this more clear in the Javadoc? >>>> >>>> So it's not about returning a response message and a boolean perfectly >>>> fits there. Scott please confirm. >>>> >>>> Thanks >>>> >>>> Jacques >>>> >>>> >>>> Le 29/12/2016 à 07:45, Taher Alkhateeb a écrit : >>>> >>>> I tend to also prefer a response message. Adding something new like a >>>>> boolean flag puts more information for people to process needlessly. >>>>> Less >>>>> is more I think. >>>>> >>>>> On Dec 29, 2016 12:55 AM, "Jacques Le Roux" < >>>>> [email protected]> >>>>> wrote: >>>>> >>>>> Yes, I wondered about that. >>>>> >>>>>> In my case the following simplified code would work, because it's used >>>>>> before running the service. >>>>>> I guess it's how it should be used (why running isValid() before >>>>>> running?). So I guess we can simply use that >>>>>> >>>>>> public boolean isValid(Map<String, Object> context, String mode, >>>>>> Locale locale) { >>>>>> try { >>>>>> validate(context, mode, locale); >>>>>> } catch (ServiceValidationException e) { >>>>>> return false; >>>>>> } >>>>>> return true; >>>>>> } >>>>>> >>>>>> Other opinions? >>>>>> >>>>>> Jacques >>>>>> >>>>>> >>>>>> Le 28/12/2016 à 21:48, Scott Gray a écrit : >>>>>> >>>>>> I guess it depends on how this will be used but I think there is a big >>>>>> >>>>>>> difference between " // do not validate results with errors" and >>>>>>> returning >>>>>>> false from an isValid method. IMO an error output response is >>>>>>> perfectly >>>>>>> valid. >>>>>>> >>>>>>> Regards >>>>>>> Scott >>>>>>> >>>>>>> On 28 December 2016 at 21:47, <[email protected]> wrote: >>>>>>> >>>>>>> Author: jleroux >>>>>>> >>>>>>> Date: Wed Dec 28 08:47:25 2016 >>>>>>>> New Revision: 1776243 >>>>>>>> >>>>>>>> URL: http://svn.apache.org/viewvc?rev=1776243&view=rev >>>>>>>> Log: >>>>>>>> Implemented: Add a isValid() method to the ModelService class >>>>>>>> (OFBIZ-9158) >>>>>>>> >>>>>>>> The idea is to use validate() to render a boolean result. I needed >>>>>>>> that >>>>>>>> in >>>>>>>> a >>>>>>>> custom project, I think it's worth contributing. >>>>>>>> >>>>>>>> Modified: >>>>>>>> ofbiz/trunk/framework/service/src/main/java/org/apache/ >>>>>>>> ofbiz/service/ModelService.java >>>>>>>> >>>>>>>> Modified: ofbiz/trunk/framework/service/src/main/java/org/apache/ >>>>>>>> ofbiz/service/ModelService.java >>>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/ >>>>>>>> src/main/java/org/apache/ofbiz/service/ModelService. >>>>>>>> java?rev=1776243&r1=1776242&r2=1776243&view=diff >>>>>>>> ============================================================ >>>>>>>> ================== >>>>>>>> --- ofbiz/trunk/framework/service/src/main/java/org/apache/ >>>>>>>> ofbiz/service/ModelService.java (original) >>>>>>>> +++ ofbiz/trunk/framework/service/src/main/java/org/apache/ >>>>>>>> ofbiz/service/ModelService.java Wed Dec 28 08:47:25 2016 >>>>>>>> @@ -600,6 +600,32 @@ public class ModelService extends Abstra >>>>>>>> } >>>>>>>> >>>>>>>> /** >>>>>>>> + * Validates a Map against the IN or OUT parameter information >>>>>>>> + * Same than validate() with same signature but returns a >>>>>>>> boolean >>>>>>>> instead of exceptions >>>>>>>> + * @param context the context >>>>>>>> + * @param mode Test either mode IN or mode OUT >>>>>>>> + * @param locale the actual locale to use >>>>>>>> + */ >>>>>>>> + public boolean isValid(Map<String, Object> context, String >>>>>>>> mode, >>>>>>>> Locale locale) { >>>>>>>> + boolean verboseOn = Debug.verboseOn(); >>>>>>>> + if (verboseOn) Debug.logVerbose("[ModelService.validate] : >>>>>>>> {" + >>>>>>>> this.name + "} : Validating context - " + context, module); >>>>>>>> + >>>>>>>> + // do not validate results with errors >>>>>>>> + if (mode.equals(OUT_PARAM) && context != null && >>>>>>>> context.containsKey(RESPONSE_MESSAGE)) { >>>>>>>> + if (RESPOND_ERROR.equals(context. >>>>>>>> get(RESPONSE_MESSAGE)) >>>>>>>> || >>>>>>>> RESPOND_FAIL.equals(context.get(RESPONSE_MESSAGE))) { >>>>>>>> + if (verboseOn) Debug.logVerbose("[ModelServic >>>>>>>> e.validate] >>>>>>>> : {" + this.name + "} : response was an error, not validating.", >>>>>>>> module); >>>>>>>> + return false; >>>>>>>> + } >>>>>>>> + } >>>>>>>> + try { >>>>>>>> + validate(context, mode, locale); >>>>>>>> + } catch (ServiceValidationException e) { >>>>>>>> + return false; >>>>>>>> + } >>>>>>>> + return true; >>>>>>>> + } >>>>>>>> + >>>>>>>> + /** >>>>>>>> * Validates a map of name, object types to a map of name, >>>>>>>> objects >>>>>>>> * @param info The map of name, object types >>>>>>>> * @param test The map to test its value types. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>> >>> >
