You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Scott Gray <sc...@hotwaxsystems.com> on 2016/12/28 20:48:36 UTC

Re: svn commit: r1776243 - /ofbiz/trunk/framework/service/src/main/java/org/apache/ofbiz/service/ModelService.java

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, <jl...@apache.org> 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/src/main/java/org/apache/ofbiz/service/Model Service.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
Le 30/12/2016 � 10:04, Taher Alkhateeb a �crit :
> inline
>
> On Fri, Dec 30, 2016 at 11:50 AM, Jacques Le Roux <
> jacques.le.roux@les7arts.com> 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" <ja...@les7arts.com>
>>> 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" <
>>>>>> jacques.le.roux@les7arts.com>
>>>>>> 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, <jl...@apache.org> 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/src/main/java/org/apache/ofbiz/service/Model Service.java

Posted by Taher Alkhateeb <sl...@gmail.com>.
inline

On Fri, Dec 30, 2016 at 11:50 AM, Jacques Le Roux <
jacques.le.roux@les7arts.com> 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" <ja...@les7arts.com>
>> 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" <
>>>>> jacques.le.roux@les7arts.com>
>>>>> 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, <jl...@apache.org> 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/src/main/java/org/apache/ofbiz/service/Model Service.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
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" <ja...@les7arts.com> 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" <
>>>> jacques.le.roux@les7arts.com>
>>>> 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, <jl...@apache.org> 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/src/main/java/org/apache/ofbiz/service/Model Service.java

Posted by Scott Gray <sc...@hotwaxsystems.com>.
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" <ja...@les7arts.com> 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" <
>>> jacques.le.roux@les7arts.com>
>>> 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, <jl...@apache.org> 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/src/main/java/org/apache/ofbiz/service/Model Service.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
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" <ja...@les7arts.com>
>>> 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, <jl...@apache.org> 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/src/main/java/org/apache/ofbiz/service/Model Service.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
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" <ja...@les7arts.com>
>> 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, <jl...@apache.org> 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/src/main/java/org/apache/ofbiz/service/Model Service.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
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" <ja...@les7arts.com>
> 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, <jl...@apache.org> 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/src/main/java/org/apache/ofbiz/service/Model Service.java

Posted by Taher Alkhateeb <sl...@gmail.com>.
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" <ja...@les7arts.com>
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, <jl...@apache.org> 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/src/main/java/org/apache/ofbiz/service/Model Service.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
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, <jl...@apache.org> 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.
>>
>>
>>