Re: svn commit: r1776243 - /ofbiz/trunk/framework/service
Locked
Re: svn commit: r1776243 - /ofbiz/trunk/framework/service
 
|
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, <[hidden email]> 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("[ModelService.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. > > > |
Re: svn commit: r1776243 - /ofbiz/trunk/framework/service
Administrator
|
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, <[hidden email]> 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("[ModelService.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. >> >> >> |
|
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" <[hidden email]> 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, <[hidden email]> 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. >>> >>> >>> >>> > |
Re: svn commit: r1776243 - /ofbiz/trunk/framework/service
|
Administrator
|
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" <[hidden email]> > 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, <[hidden email]> 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. >>>> >>>> >>>> >>>> |
Re: svn commit: r1776243 - /ofbiz/trunk/framework/service
|
Administrator
|
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" <[hidden email]> >> 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, <[hidden email]> 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. >>>>> >>>>> >>>>> >>>>> > > |
Re: svn commit: r1776243 - /ofbiz/trunk/framework/service
|
Administrator
|
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" <[hidden email]> >>> 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, <[hidden email]> 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. >>>>>> >>>>>> >>>>>> >>>>>> >> >> > > |
Re: svn commit: r1776243 - /ofbiz/trunk/framework/service
|
|
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" <[hidden email]> 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" < >>> [hidden email]> >>> 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, <[hidden email]> 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. >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >> >> > > |
Re: svn commit: r1776243 - /ofbiz/trunk/framework/service
|
Administrator
|
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. > 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. Jacques > > Regards > Scott > > On 30/12/2016 05:10, "Jacques Le Roux" <[hidden email]> 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" < >>>> [hidden email]> >>>> 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, <[hidden email]> 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. >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>> >> |
|
|
inline
On Fri, Dec 30, 2016 at 11:50 AM, Jacques Le Roux < [hidden email]> 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" <[hidden email]> >> 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" < >>>>> [hidden email]> >>>>> 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, <[hidden email]> 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. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>> >>> > |
Re: svn commit: r1776243 - /ofbiz/trunk/framework/service
|
Administrator
|
Le 30/12/2016 à 10:04, Taher Alkhateeb a écrit :
> inline > > On Fri, Dec 30, 2016 at 11:50 AM, Jacques Le Roux < > [hidden email]> 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 call this method from a Java event. With a SECA or service group or any other method I'd lose the control I want to keep in the event. > > >> 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. This method does not frighten me :) Jacques > >> Jacques >> >> >> >>> Regards >>> Scott >>> >>> On 30/12/2016 05:10, "Jacques Le Roux" <[hidden email]> >>> 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" < >>>>>> [hidden email]> >>>>>> 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, <[hidden email]> 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. >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> |
«
Return to OFBiz - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |