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'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. 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. >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >> >> > >
