You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Taher Alkhateeb <sl...@gmail.com> on 2017/09/02 18:06:25 UTC
Re: svn commit: r1807045 - /ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/CommonEvents.java
Do we have any derived classes that require exposing these interfaces?
I think it's not a good design to break encapsulation just because of
things we "might" need.
On Sat, Sep 2, 2017 at 3:25 PM, <jl...@apache.org> wrote:
> Author: jleroux
> Date: Sat Sep 2 12:25:13 2017
> New Revision: 1807045
>
> URL: http://svn.apache.org/viewvc?rev=1807045&view=rev
> Log:
> Improved: CommonEvents improvements
> (OFBIZ-9673)
>
> Replaces private to protected so that CommonEvents.java is more usable to
> derived classes.
>
> Thanks: Wai
>
> Modified:
> ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/CommonEvents.java
>
> Modified: ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/CommonEvents.java
> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/CommonEvents.java?rev=1807045&r1=1807044&r2=1807045&view=diff
> ==============================================================================
> --- ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/CommonEvents.java (original)
> +++ ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/CommonEvents.java Sat Sep 2 12:25:13 2017
> @@ -68,7 +68,7 @@ public class CommonEvents {
>
> public static final String module = CommonEvents.class.getName();
>
> - private static final String[] ignoreAttrs = new String[] { // Attributes removed for security reason; _ERROR_MESSAGE_ is kept
> + static final String[] ignoreAttrs = new String[] { // Attributes removed for security reason; _ERROR_MESSAGE_ is kept
> "javax.servlet.request.key_size",
> "_CONTEXT_ROOT_",
> "_FORWARDED_FROM_SERVLET_",
> @@ -82,7 +82,7 @@ public class CommonEvents {
> "thisRequestUri"
> };
>
> - private static final UtilCache<String, Map<String, String>> appletSessions = UtilCache.createUtilCache("AppletSessions", 0, 600000, true);
> + static final UtilCache<String, Map<String, String>> appletSessions = UtilCache.createUtilCache("AppletSessions", 0, 600000, true);
>
> public static String checkAppletRequest(HttpServletRequest request, HttpServletResponse response) {
> Delegator delegator = (Delegator) request.getAttribute("delegator");
> @@ -309,7 +309,7 @@ public class CommonEvents {
> return "success";
> }
>
> - private static void writeJSONtoResponse(JSON json, HttpServletRequest request, HttpServletResponse response) throws UnsupportedEncodingException {
> + static void writeJSONtoResponse(JSON json, HttpServletRequest request, HttpServletResponse response) throws UnsupportedEncodingException {
> String jsonStr = json.toString();
> if (jsonStr == null) {
> Debug.logError("JSON Object was empty; fatal error!", module);
>
>
Re: svn commit: r1807045 -
/ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbi
z/common/CommonEvents.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
Le 03/09/2017 à 06:58, Paul Foxworthy a écrit :
> Hi Jacques,
>
> These fields and methods will have package scope, but your description
> suggests you intended protected. I'm really uncomfortable about fields with
> package scope. Is it really necessary? Could they be protected?
Paul,
On this point, using "no modifier" is more protected than using the "protected modifier".
https://docs.oracle.com/javase/tutorial/java/javaOO/accesscontrol.html
Jacques
Re: svn commit: r1807045 -
/ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbi
z/common/CommonEvents.java
Posted by Wai <bz...@gmail.com>.
I'll contribute a patch. Thanks.
--
Sent from: http://ofbiz.135035.n4.nabble.com/OFBiz-Dev-f165671.html
Re: svn commit: r1807045 -
/ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbi
z/common/CommonEvents.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
Hi Wai (I suppose :))
Do you want to contribute your changes? Else you can do that on your custom project w/o needing to change in OOTB.
Jacques
Le 04/09/2017 à 15:01, bzb.ofbiz@gmail.com a écrit :
> I'm deriving from CommonEvents.java and creating a new method that calls CommonEvent.writeJSONtoResponse(). Unfortunately,
> CommonEvent.writeJSONtoResponse() is private. Hence, I could not compile. I believe that methods that should not be accessible from other classes
> should at least allow derived classes to extend it. Hence, changing it to "protected" rather than "private" is more appropriate.
>
Re: svn commit: r1807045 -
/ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbi
z/common/CommonEvents.java
Posted by "bzb.ofbiz@gmail.com" <bz...@gmail.com>.
I'm deriving from CommonEvents.java and creating a new method that calls
CommonEvent.writeJSONtoResponse(). Unfortunately,
CommonEvent.writeJSONtoResponse() is private. Hence, I could not
compile. I believe that methods that should not be accessible from other
classes should at least allow derived classes to extend it. Hence,
changing it to "protected" rather than "private" is more appropriate.
Re: svn commit: r1807045 -
/ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbi
z/common/CommonEvents.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
Wai has asked about this discussion, let's wait to see what he has to say
Jacques
Le 04/09/2017 à 08:22, Taher Alkhateeb a écrit :
> It's not a matter of impact, it's a matter of principle. You don't leave
> code in a worse state for no valid reason. People are working very hard to
> refactor the code to decrease the entropy, not increase it.
>
> On Sep 3, 2017 3:45 PM, "Jacques Le Roux" <ja...@les7arts.com>
> wrote:
>
> Reverted at revision: 1807142
>
> I did not thought that changing from private to "no modifier" there would
> have a such impact on dev ML
>
> It's right that Wai did not give any justifications. Let's see if he will...
>
> Jacques
>
>
>
> Le 03/09/2017 à 11:20, Michael Brohl a écrit :
>
>> +1
>>
>> Additionally, I think such modifications should not be made in a hurry.
>> They should wait for committer/contributor reactions or be (briefly)
>> discussed before being committed. In this case, the the time between issue
>> creation and the commit was just a few hours, no chance for objections.
>>
>> We had similar discussions in the past - PLEASE be more careful about such
>> changes.
>>
>> Thanks and regards,
>>
>> Michael
>>
>>
>> Am 03.09.17 um 10:50 schrieb Taher Alkhateeb:
>>
>>> I see two things which happened here:
>>> 1- a modification on the code base to accommodate one person working on
>>> their own project
>>> 2- The modification exposes internals which makes the system more fragile
>>> for no added value.
>>>
>>> I recommend a revert, and I believe the one who committed is the one who
>>> should revert, not others. Inviting people to "feel free" to revert your
>>> work does not make much sense and takes away from their time which they
>>> already spent reviewing your code.
>>>
>>> On Sep 3, 2017 10:18 AM, "Jacques Le Roux" <ja...@les7arts.com>
>>> wrote:
>>>
>>> Hi Paul,
>>>> That's a good idea, please feel free to enhance or revert
>>>>
>>>> Thanks
>>>>
>>>> Jacques
>>>>
>>>>
>>>> Le 03/09/2017 à 06:58, Paul Foxworthy a écrit :
>>>>
>>>> Hi Jacques,
>>>>> These fields and methods will have package scope, but your description
>>>>> suggests you intended protected. I'm really uncomfortable about fields
>>>>> with
>>>>> package scope. Is it really necessary? Could they be protected?
>>>>>
>>>>> In the case of ignoreAttrs, the array should remain private. The loop
>>>>> in jsonResponseFromRequestAttributes that uses the array should be
>>>>> made a
>>>>> method in its own right, with a name such as removeSensitiveAttributes.
>>>>> That method could have higher visibility such as protected. The general
>>>>> principle is "don't ask for data, ask for help" (
>>>>> https://martinfowler.com/bliki/TellDontAsk.html).
>>>>>
>>>>> Cheers
>>>>>
>>>>> Paul Foxworthy
>>>>>
>>>>>
>>>>> On 3 September 2017 at 04:45, Jacques Le Roux <
>>>>> jacques.le.roux@les7arts.com>
>>>>> wrote:
>>>>>
>>>>> I think Wai asked for that because he needs it for a custom project.
>>>>> It's
>>>>>
>>>>>> sill reasonably encapsulated.
>>>>>>
>>>>>> You can revert if you want
>>>>>>
>>>>>> Jacques
>>>>>>
>>>>>>
>>>>>>
>>>>>> Le 02/09/2017 à 20:06, Taher Alkhateeb a écrit :
>>>>>>
>>>>>> Do we have any derived classes that require exposing these interfaces?
>>>>>>
>>>>>>> I think it's not a good design to break encapsulation just because of
>>>>>>> things we "might" need.
>>>>>>>
>>>>>>> On Sat, Sep 2, 2017 at 3:25 PM, <jl...@apache.org> wrote:
>>>>>>>
>>>>>>> Author: jleroux
>>>>>>>
>>>>>>>> Date: Sat Sep 2 12:25:13 2017
>>>>>>>> New Revision: 1807045
>>>>>>>>
>>>>>>>> URL: http://svn.apache.org/viewvc?rev=1807045&view=rev
>>>>>>>> Log:
>>>>>>>> Improved: CommonEvents improvements
>>>>>>>> (OFBIZ-9673)
>>>>>>>>
>>>>>>>> Replaces private to protected so that CommonEvents.java is more
>>>>>>>> usable
>>>>>>>> to
>>>>>>>> derived classes.
>>>>>>>>
>>>>>>>> Thanks: Wai
>>>>>>>>
>>>>>>>> Modified:
>>>>>>>> ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>>>>>>> org/apache/ofbiz/common/CommonEvents.java
>>>>>>>>
>>>>>>>> Modified: ofbiz/ofbiz-framework/trunk/fr
>>>>>>>> amework/common/src/main/java/
>>>>>>>> org/apache/ofbiz/common/CommonEvents.java
>>>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/fra
>>>>>>>> mework/common/src/main/java/org/apache/ofbiz/common/Common
>>>>>>>> Events.java?rev=1807045&r1=1807044&r2=1807045&view=diff
>>>>>>>> ============================================================
>>>>>>>> ==================
>>>>>>>> --- ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>>>>>>> org/apache/ofbiz/common/CommonEvents.java (original)
>>>>>>>> +++ ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>>>>>>> org/apache/ofbiz/common/CommonEvents.java Sat Sep 2 12:25:13 2017
>>>>>>>> @@ -68,7 +68,7 @@ public class CommonEvents {
>>>>>>>>
>>>>>>>> public static final String module =
>>>>>>>> CommonEvents.class.getName();
>>>>>>>>
>>>>>>>> - private static final String[] ignoreAttrs = new String[] { //
>>>>>>>> Attributes removed for security reason; _ERROR_MESSAGE_ is kept
>>>>>>>> + static final String[] ignoreAttrs = new String[] { // Attributes
>>>>>>>> removed for security reason; _ERROR_MESSAGE_ is kept
>>>>>>>> "javax.servlet.request.key_size",
>>>>>>>> "_CONTEXT_ROOT_",
>>>>>>>> "_FORWARDED_FROM_SERVLET_",
>>>>>>>> @@ -82,7 +82,7 @@ public class CommonEvents {
>>>>>>>> "thisRequestUri"
>>>>>>>> };
>>>>>>>>
>>>>>>>> - private static final UtilCache<String, Map<String, String>>
>>>>>>>> appletSessions = UtilCache.createUtilCache("AppletSessions", 0,
>>>>>>>> 600000,
>>>>>>>> true);
>>>>>>>> + static final UtilCache<String, Map<String, String>>
>>>>>>>> appletSessions
>>>>>>>> = UtilCache.createUtilCache("AppletSessions", 0, 600000, true);
>>>>>>>>
>>>>>>>> public static String checkAppletRequest(HttpServletRequest
>>>>>>>> request, HttpServletResponse response) {
>>>>>>>> Delegator delegator = (Delegator)
>>>>>>>> request.getAttribute("delegator");
>>>>>>>> @@ -309,7 +309,7 @@ public class CommonEvents {
>>>>>>>> return "success";
>>>>>>>> }
>>>>>>>>
>>>>>>>> - private static void writeJSONtoResponse(JSON json,
>>>>>>>> HttpServletRequest request, HttpServletResponse response) throws
>>>>>>>> UnsupportedEncodingException {
>>>>>>>> + static void writeJSONtoResponse(JSON json, HttpServletRequest
>>>>>>>> request, HttpServletResponse response) throws
>>>>>>>> UnsupportedEncodingException {
>>>>>>>> String jsonStr = json.toString();
>>>>>>>> if (jsonStr == null) {
>>>>>>>> Debug.logError("JSON Object was empty; fatal error!",
>>>>>>>> module);
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>
Re: svn commit: r1807045 - /ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbi
z/common/CommonEvents.java
Posted by Taher Alkhateeb <sl...@gmail.com>.
It's not a matter of impact, it's a matter of principle. You don't leave
code in a worse state for no valid reason. People are working very hard to
refactor the code to decrease the entropy, not increase it.
On Sep 3, 2017 3:45 PM, "Jacques Le Roux" <ja...@les7arts.com>
wrote:
Reverted at revision: 1807142
I did not thought that changing from private to "no modifier" there would
have a such impact on dev ML
It's right that Wai did not give any justifications. Let's see if he will...
Jacques
Le 03/09/2017 à 11:20, Michael Brohl a écrit :
> +1
>
> Additionally, I think such modifications should not be made in a hurry.
> They should wait for committer/contributor reactions or be (briefly)
> discussed before being committed. In this case, the the time between issue
> creation and the commit was just a few hours, no chance for objections.
>
> We had similar discussions in the past - PLEASE be more careful about such
> changes.
>
> Thanks and regards,
>
> Michael
>
>
> Am 03.09.17 um 10:50 schrieb Taher Alkhateeb:
>
>> I see two things which happened here:
>> 1- a modification on the code base to accommodate one person working on
>> their own project
>> 2- The modification exposes internals which makes the system more fragile
>> for no added value.
>>
>> I recommend a revert, and I believe the one who committed is the one who
>> should revert, not others. Inviting people to "feel free" to revert your
>> work does not make much sense and takes away from their time which they
>> already spent reviewing your code.
>>
>> On Sep 3, 2017 10:18 AM, "Jacques Le Roux" <ja...@les7arts.com>
>> wrote:
>>
>> Hi Paul,
>>>
>>> That's a good idea, please feel free to enhance or revert
>>>
>>> Thanks
>>>
>>> Jacques
>>>
>>>
>>> Le 03/09/2017 à 06:58, Paul Foxworthy a écrit :
>>>
>>> Hi Jacques,
>>>>
>>>> These fields and methods will have package scope, but your description
>>>> suggests you intended protected. I'm really uncomfortable about fields
>>>> with
>>>> package scope. Is it really necessary? Could they be protected?
>>>>
>>>> In the case of ignoreAttrs, the array should remain private. The loop
>>>> in jsonResponseFromRequestAttributes that uses the array should be
>>>> made a
>>>> method in its own right, with a name such as removeSensitiveAttributes.
>>>> That method could have higher visibility such as protected. The general
>>>> principle is "don't ask for data, ask for help" (
>>>> https://martinfowler.com/bliki/TellDontAsk.html).
>>>>
>>>> Cheers
>>>>
>>>> Paul Foxworthy
>>>>
>>>>
>>>> On 3 September 2017 at 04:45, Jacques Le Roux <
>>>> jacques.le.roux@les7arts.com>
>>>> wrote:
>>>>
>>>> I think Wai asked for that because he needs it for a custom project.
>>>> It's
>>>>
>>>>> sill reasonably encapsulated.
>>>>>
>>>>> You can revert if you want
>>>>>
>>>>> Jacques
>>>>>
>>>>>
>>>>>
>>>>> Le 02/09/2017 à 20:06, Taher Alkhateeb a écrit :
>>>>>
>>>>> Do we have any derived classes that require exposing these interfaces?
>>>>>
>>>>>> I think it's not a good design to break encapsulation just because of
>>>>>> things we "might" need.
>>>>>>
>>>>>> On Sat, Sep 2, 2017 at 3:25 PM, <jl...@apache.org> wrote:
>>>>>>
>>>>>> Author: jleroux
>>>>>>
>>>>>>> Date: Sat Sep 2 12:25:13 2017
>>>>>>> New Revision: 1807045
>>>>>>>
>>>>>>> URL: http://svn.apache.org/viewvc?rev=1807045&view=rev
>>>>>>> Log:
>>>>>>> Improved: CommonEvents improvements
>>>>>>> (OFBIZ-9673)
>>>>>>>
>>>>>>> Replaces private to protected so that CommonEvents.java is more
>>>>>>> usable
>>>>>>> to
>>>>>>> derived classes.
>>>>>>>
>>>>>>> Thanks: Wai
>>>>>>>
>>>>>>> Modified:
>>>>>>> ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>>>>>> org/apache/ofbiz/common/CommonEvents.java
>>>>>>>
>>>>>>> Modified: ofbiz/ofbiz-framework/trunk/fr
>>>>>>> amework/common/src/main/java/
>>>>>>> org/apache/ofbiz/common/CommonEvents.java
>>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/fra
>>>>>>> mework/common/src/main/java/org/apache/ofbiz/common/Common
>>>>>>> Events.java?rev=1807045&r1=1807044&r2=1807045&view=diff
>>>>>>> ============================================================
>>>>>>> ==================
>>>>>>> --- ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>>>>>> org/apache/ofbiz/common/CommonEvents.java (original)
>>>>>>> +++ ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>>>>>> org/apache/ofbiz/common/CommonEvents.java Sat Sep 2 12:25:13 2017
>>>>>>> @@ -68,7 +68,7 @@ public class CommonEvents {
>>>>>>>
>>>>>>> public static final String module =
>>>>>>> CommonEvents.class.getName();
>>>>>>>
>>>>>>> - private static final String[] ignoreAttrs = new String[] { //
>>>>>>> Attributes removed for security reason; _ERROR_MESSAGE_ is kept
>>>>>>> + static final String[] ignoreAttrs = new String[] { // Attributes
>>>>>>> removed for security reason; _ERROR_MESSAGE_ is kept
>>>>>>> "javax.servlet.request.key_size",
>>>>>>> "_CONTEXT_ROOT_",
>>>>>>> "_FORWARDED_FROM_SERVLET_",
>>>>>>> @@ -82,7 +82,7 @@ public class CommonEvents {
>>>>>>> "thisRequestUri"
>>>>>>> };
>>>>>>>
>>>>>>> - private static final UtilCache<String, Map<String, String>>
>>>>>>> appletSessions = UtilCache.createUtilCache("AppletSessions", 0,
>>>>>>> 600000,
>>>>>>> true);
>>>>>>> + static final UtilCache<String, Map<String, String>>
>>>>>>> appletSessions
>>>>>>> = UtilCache.createUtilCache("AppletSessions", 0, 600000, true);
>>>>>>>
>>>>>>> public static String checkAppletRequest(HttpServletRequest
>>>>>>> request, HttpServletResponse response) {
>>>>>>> Delegator delegator = (Delegator)
>>>>>>> request.getAttribute("delegator");
>>>>>>> @@ -309,7 +309,7 @@ public class CommonEvents {
>>>>>>> return "success";
>>>>>>> }
>>>>>>>
>>>>>>> - private static void writeJSONtoResponse(JSON json,
>>>>>>> HttpServletRequest request, HttpServletResponse response) throws
>>>>>>> UnsupportedEncodingException {
>>>>>>> + static void writeJSONtoResponse(JSON json, HttpServletRequest
>>>>>>> request, HttpServletResponse response) throws
>>>>>>> UnsupportedEncodingException {
>>>>>>> String jsonStr = json.toString();
>>>>>>> if (jsonStr == null) {
>>>>>>> Debug.logError("JSON Object was empty; fatal error!",
>>>>>>> module);
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>
>
Re: svn commit: r1807045 -
/ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbi
z/common/CommonEvents.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
Reverted at revision: 1807142
I did not thought that changing from private to "no modifier" there would have a such impact on dev ML
It's right that Wai did not give any justifications. Let's see if he will...
Jacques
Le 03/09/2017 à 11:20, Michael Brohl a écrit :
> +1
>
> Additionally, I think such modifications should not be made in a hurry. They should wait for committer/contributor reactions or be (briefly)
> discussed before being committed. In this case, the the time between issue creation and the commit was just a few hours, no chance for objections.
>
> We had similar discussions in the past - PLEASE be more careful about such changes.
>
> Thanks and regards,
>
> Michael
>
>
> Am 03.09.17 um 10:50 schrieb Taher Alkhateeb:
>> I see two things which happened here:
>> 1- a modification on the code base to accommodate one person working on
>> their own project
>> 2- The modification exposes internals which makes the system more fragile
>> for no added value.
>>
>> I recommend a revert, and I believe the one who committed is the one who
>> should revert, not others. Inviting people to "feel free" to revert your
>> work does not make much sense and takes away from their time which they
>> already spent reviewing your code.
>>
>> On Sep 3, 2017 10:18 AM, "Jacques Le Roux" <ja...@les7arts.com>
>> wrote:
>>
>>> Hi Paul,
>>>
>>> That's a good idea, please feel free to enhance or revert
>>>
>>> Thanks
>>>
>>> Jacques
>>>
>>>
>>> Le 03/09/2017 à 06:58, Paul Foxworthy a écrit :
>>>
>>>> Hi Jacques,
>>>>
>>>> These fields and methods will have package scope, but your description
>>>> suggests you intended protected. I'm really uncomfortable about fields
>>>> with
>>>> package scope. Is it really necessary? Could they be protected?
>>>>
>>>> In the case of ignoreAttrs, the array should remain private. The loop
>>>> in jsonResponseFromRequestAttributes that uses the array should be made a
>>>> method in its own right, with a name such as removeSensitiveAttributes.
>>>> That method could have higher visibility such as protected. The general
>>>> principle is "don't ask for data, ask for help" (
>>>> https://martinfowler.com/bliki/TellDontAsk.html).
>>>>
>>>> Cheers
>>>>
>>>> Paul Foxworthy
>>>>
>>>>
>>>> On 3 September 2017 at 04:45, Jacques Le Roux <
>>>> jacques.le.roux@les7arts.com>
>>>> wrote:
>>>>
>>>> I think Wai asked for that because he needs it for a custom project. It's
>>>>> sill reasonably encapsulated.
>>>>>
>>>>> You can revert if you want
>>>>>
>>>>> Jacques
>>>>>
>>>>>
>>>>>
>>>>> Le 02/09/2017 à 20:06, Taher Alkhateeb a écrit :
>>>>>
>>>>> Do we have any derived classes that require exposing these interfaces?
>>>>>> I think it's not a good design to break encapsulation just because of
>>>>>> things we "might" need.
>>>>>>
>>>>>> On Sat, Sep 2, 2017 at 3:25 PM, <jl...@apache.org> wrote:
>>>>>>
>>>>>> Author: jleroux
>>>>>>> Date: Sat Sep 2 12:25:13 2017
>>>>>>> New Revision: 1807045
>>>>>>>
>>>>>>> URL: http://svn.apache.org/viewvc?rev=1807045&view=rev
>>>>>>> Log:
>>>>>>> Improved: CommonEvents improvements
>>>>>>> (OFBIZ-9673)
>>>>>>>
>>>>>>> Replaces private to protected so that CommonEvents.java is more usable
>>>>>>> to
>>>>>>> derived classes.
>>>>>>>
>>>>>>> Thanks: Wai
>>>>>>>
>>>>>>> Modified:
>>>>>>> ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>>>>>> org/apache/ofbiz/common/CommonEvents.java
>>>>>>>
>>>>>>> Modified: ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>>>>>> org/apache/ofbiz/common/CommonEvents.java
>>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/fra
>>>>>>> mework/common/src/main/java/org/apache/ofbiz/common/Common
>>>>>>> Events.java?rev=1807045&r1=1807044&r2=1807045&view=diff
>>>>>>> ============================================================
>>>>>>> ==================
>>>>>>> --- ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>>>>>> org/apache/ofbiz/common/CommonEvents.java (original)
>>>>>>> +++ ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>>>>>> org/apache/ofbiz/common/CommonEvents.java Sat Sep 2 12:25:13 2017
>>>>>>> @@ -68,7 +68,7 @@ public class CommonEvents {
>>>>>>>
>>>>>>> public static final String module =
>>>>>>> CommonEvents.class.getName();
>>>>>>>
>>>>>>> - private static final String[] ignoreAttrs = new String[] { //
>>>>>>> Attributes removed for security reason; _ERROR_MESSAGE_ is kept
>>>>>>> + static final String[] ignoreAttrs = new String[] { // Attributes
>>>>>>> removed for security reason; _ERROR_MESSAGE_ is kept
>>>>>>> "javax.servlet.request.key_size",
>>>>>>> "_CONTEXT_ROOT_",
>>>>>>> "_FORWARDED_FROM_SERVLET_",
>>>>>>> @@ -82,7 +82,7 @@ public class CommonEvents {
>>>>>>> "thisRequestUri"
>>>>>>> };
>>>>>>>
>>>>>>> - private static final UtilCache<String, Map<String, String>>
>>>>>>> appletSessions = UtilCache.createUtilCache("AppletSessions", 0,
>>>>>>> 600000,
>>>>>>> true);
>>>>>>> + static final UtilCache<String, Map<String, String>> appletSessions
>>>>>>> = UtilCache.createUtilCache("AppletSessions", 0, 600000, true);
>>>>>>>
>>>>>>> public static String checkAppletRequest(HttpServletRequest
>>>>>>> request, HttpServletResponse response) {
>>>>>>> Delegator delegator = (Delegator)
>>>>>>> request.getAttribute("delegator");
>>>>>>> @@ -309,7 +309,7 @@ public class CommonEvents {
>>>>>>> return "success";
>>>>>>> }
>>>>>>>
>>>>>>> - private static void writeJSONtoResponse(JSON json,
>>>>>>> HttpServletRequest request, HttpServletResponse response) throws
>>>>>>> UnsupportedEncodingException {
>>>>>>> + static void writeJSONtoResponse(JSON json, HttpServletRequest
>>>>>>> request, HttpServletResponse response) throws
>>>>>>> UnsupportedEncodingException {
>>>>>>> String jsonStr = json.toString();
>>>>>>> if (jsonStr == null) {
>>>>>>> Debug.logError("JSON Object was empty; fatal error!",
>>>>>>> module);
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>
>
Re: svn commit: r1807045 -
/ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbi
z/common/CommonEvents.java
Posted by Michael Brohl <mi...@ecomify.de>.
+1
Additionally, I think such modifications should not be made in a hurry.
They should wait for committer/contributor reactions or be (briefly)
discussed before being committed. In this case, the the time between
issue creation and the commit was just a few hours, no chance for
objections.
We had similar discussions in the past - PLEASE be more careful about
such changes.
Thanks and regards,
Michael
Am 03.09.17 um 10:50 schrieb Taher Alkhateeb:
> I see two things which happened here:
> 1- a modification on the code base to accommodate one person working on
> their own project
> 2- The modification exposes internals which makes the system more fragile
> for no added value.
>
> I recommend a revert, and I believe the one who committed is the one who
> should revert, not others. Inviting people to "feel free" to revert your
> work does not make much sense and takes away from their time which they
> already spent reviewing your code.
>
> On Sep 3, 2017 10:18 AM, "Jacques Le Roux" <ja...@les7arts.com>
> wrote:
>
>> Hi Paul,
>>
>> That's a good idea, please feel free to enhance or revert
>>
>> Thanks
>>
>> Jacques
>>
>>
>> Le 03/09/2017 à 06:58, Paul Foxworthy a écrit :
>>
>>> Hi Jacques,
>>>
>>> These fields and methods will have package scope, but your description
>>> suggests you intended protected. I'm really uncomfortable about fields
>>> with
>>> package scope. Is it really necessary? Could they be protected?
>>>
>>> In the case of ignoreAttrs, the array should remain private. The loop
>>> in jsonResponseFromRequestAttributes that uses the array should be made a
>>> method in its own right, with a name such as removeSensitiveAttributes.
>>> That method could have higher visibility such as protected. The general
>>> principle is "don't ask for data, ask for help" (
>>> https://martinfowler.com/bliki/TellDontAsk.html).
>>>
>>> Cheers
>>>
>>> Paul Foxworthy
>>>
>>>
>>> On 3 September 2017 at 04:45, Jacques Le Roux <
>>> jacques.le.roux@les7arts.com>
>>> wrote:
>>>
>>> I think Wai asked for that because he needs it for a custom project. It's
>>>> sill reasonably encapsulated.
>>>>
>>>> You can revert if you want
>>>>
>>>> Jacques
>>>>
>>>>
>>>>
>>>> Le 02/09/2017 à 20:06, Taher Alkhateeb a écrit :
>>>>
>>>> Do we have any derived classes that require exposing these interfaces?
>>>>> I think it's not a good design to break encapsulation just because of
>>>>> things we "might" need.
>>>>>
>>>>> On Sat, Sep 2, 2017 at 3:25 PM, <jl...@apache.org> wrote:
>>>>>
>>>>> Author: jleroux
>>>>>> Date: Sat Sep 2 12:25:13 2017
>>>>>> New Revision: 1807045
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?rev=1807045&view=rev
>>>>>> Log:
>>>>>> Improved: CommonEvents improvements
>>>>>> (OFBIZ-9673)
>>>>>>
>>>>>> Replaces private to protected so that CommonEvents.java is more usable
>>>>>> to
>>>>>> derived classes.
>>>>>>
>>>>>> Thanks: Wai
>>>>>>
>>>>>> Modified:
>>>>>> ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>>>>> org/apache/ofbiz/common/CommonEvents.java
>>>>>>
>>>>>> Modified: ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>>>>> org/apache/ofbiz/common/CommonEvents.java
>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/fra
>>>>>> mework/common/src/main/java/org/apache/ofbiz/common/Common
>>>>>> Events.java?rev=1807045&r1=1807044&r2=1807045&view=diff
>>>>>> ============================================================
>>>>>> ==================
>>>>>> --- ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>>>>> org/apache/ofbiz/common/CommonEvents.java (original)
>>>>>> +++ ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>>>>> org/apache/ofbiz/common/CommonEvents.java Sat Sep 2 12:25:13 2017
>>>>>> @@ -68,7 +68,7 @@ public class CommonEvents {
>>>>>>
>>>>>> public static final String module =
>>>>>> CommonEvents.class.getName();
>>>>>>
>>>>>> - private static final String[] ignoreAttrs = new String[] { //
>>>>>> Attributes removed for security reason; _ERROR_MESSAGE_ is kept
>>>>>> + static final String[] ignoreAttrs = new String[] { // Attributes
>>>>>> removed for security reason; _ERROR_MESSAGE_ is kept
>>>>>> "javax.servlet.request.key_size",
>>>>>> "_CONTEXT_ROOT_",
>>>>>> "_FORWARDED_FROM_SERVLET_",
>>>>>> @@ -82,7 +82,7 @@ public class CommonEvents {
>>>>>> "thisRequestUri"
>>>>>> };
>>>>>>
>>>>>> - private static final UtilCache<String, Map<String, String>>
>>>>>> appletSessions = UtilCache.createUtilCache("AppletSessions", 0,
>>>>>> 600000,
>>>>>> true);
>>>>>> + static final UtilCache<String, Map<String, String>> appletSessions
>>>>>> = UtilCache.createUtilCache("AppletSessions", 0, 600000, true);
>>>>>>
>>>>>> public static String checkAppletRequest(HttpServletRequest
>>>>>> request, HttpServletResponse response) {
>>>>>> Delegator delegator = (Delegator)
>>>>>> request.getAttribute("delegator");
>>>>>> @@ -309,7 +309,7 @@ public class CommonEvents {
>>>>>> return "success";
>>>>>> }
>>>>>>
>>>>>> - private static void writeJSONtoResponse(JSON json,
>>>>>> HttpServletRequest request, HttpServletResponse response) throws
>>>>>> UnsupportedEncodingException {
>>>>>> + static void writeJSONtoResponse(JSON json, HttpServletRequest
>>>>>> request, HttpServletResponse response) throws
>>>>>> UnsupportedEncodingException {
>>>>>> String jsonStr = json.toString();
>>>>>> if (jsonStr == null) {
>>>>>> Debug.logError("JSON Object was empty; fatal error!",
>>>>>> module);
>>>>>>
>>>>>>
>>>>>>
>>>>>>
Re: svn commit: r1807045 - /ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbi
z/common/CommonEvents.java
Posted by Taher Alkhateeb <sl...@gmail.com>.
I see two things which happened here:
1- a modification on the code base to accommodate one person working on
their own project
2- The modification exposes internals which makes the system more fragile
for no added value.
I recommend a revert, and I believe the one who committed is the one who
should revert, not others. Inviting people to "feel free" to revert your
work does not make much sense and takes away from their time which they
already spent reviewing your code.
On Sep 3, 2017 10:18 AM, "Jacques Le Roux" <ja...@les7arts.com>
wrote:
> Hi Paul,
>
> That's a good idea, please feel free to enhance or revert
>
> Thanks
>
> Jacques
>
>
> Le 03/09/2017 à 06:58, Paul Foxworthy a écrit :
>
>> Hi Jacques,
>>
>> These fields and methods will have package scope, but your description
>> suggests you intended protected. I'm really uncomfortable about fields
>> with
>> package scope. Is it really necessary? Could they be protected?
>>
>> In the case of ignoreAttrs, the array should remain private. The loop
>> in jsonResponseFromRequestAttributes that uses the array should be made a
>> method in its own right, with a name such as removeSensitiveAttributes.
>> That method could have higher visibility such as protected. The general
>> principle is "don't ask for data, ask for help" (
>> https://martinfowler.com/bliki/TellDontAsk.html).
>>
>> Cheers
>>
>> Paul Foxworthy
>>
>>
>> On 3 September 2017 at 04:45, Jacques Le Roux <
>> jacques.le.roux@les7arts.com>
>> wrote:
>>
>> I think Wai asked for that because he needs it for a custom project. It's
>>> sill reasonably encapsulated.
>>>
>>> You can revert if you want
>>>
>>> Jacques
>>>
>>>
>>>
>>> Le 02/09/2017 à 20:06, Taher Alkhateeb a écrit :
>>>
>>> Do we have any derived classes that require exposing these interfaces?
>>>>
>>>> I think it's not a good design to break encapsulation just because of
>>>> things we "might" need.
>>>>
>>>> On Sat, Sep 2, 2017 at 3:25 PM, <jl...@apache.org> wrote:
>>>>
>>>> Author: jleroux
>>>>> Date: Sat Sep 2 12:25:13 2017
>>>>> New Revision: 1807045
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=1807045&view=rev
>>>>> Log:
>>>>> Improved: CommonEvents improvements
>>>>> (OFBIZ-9673)
>>>>>
>>>>> Replaces private to protected so that CommonEvents.java is more usable
>>>>> to
>>>>> derived classes.
>>>>>
>>>>> Thanks: Wai
>>>>>
>>>>> Modified:
>>>>> ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>>>> org/apache/ofbiz/common/CommonEvents.java
>>>>>
>>>>> Modified: ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>>>> org/apache/ofbiz/common/CommonEvents.java
>>>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/fra
>>>>> mework/common/src/main/java/org/apache/ofbiz/common/Common
>>>>> Events.java?rev=1807045&r1=1807044&r2=1807045&view=diff
>>>>> ============================================================
>>>>> ==================
>>>>> --- ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>>>> org/apache/ofbiz/common/CommonEvents.java (original)
>>>>> +++ ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>>>> org/apache/ofbiz/common/CommonEvents.java Sat Sep 2 12:25:13 2017
>>>>> @@ -68,7 +68,7 @@ public class CommonEvents {
>>>>>
>>>>> public static final String module =
>>>>> CommonEvents.class.getName();
>>>>>
>>>>> - private static final String[] ignoreAttrs = new String[] { //
>>>>> Attributes removed for security reason; _ERROR_MESSAGE_ is kept
>>>>> + static final String[] ignoreAttrs = new String[] { // Attributes
>>>>> removed for security reason; _ERROR_MESSAGE_ is kept
>>>>> "javax.servlet.request.key_size",
>>>>> "_CONTEXT_ROOT_",
>>>>> "_FORWARDED_FROM_SERVLET_",
>>>>> @@ -82,7 +82,7 @@ public class CommonEvents {
>>>>> "thisRequestUri"
>>>>> };
>>>>>
>>>>> - private static final UtilCache<String, Map<String, String>>
>>>>> appletSessions = UtilCache.createUtilCache("AppletSessions", 0,
>>>>> 600000,
>>>>> true);
>>>>> + static final UtilCache<String, Map<String, String>> appletSessions
>>>>> = UtilCache.createUtilCache("AppletSessions", 0, 600000, true);
>>>>>
>>>>> public static String checkAppletRequest(HttpServletRequest
>>>>> request, HttpServletResponse response) {
>>>>> Delegator delegator = (Delegator)
>>>>> request.getAttribute("delegator");
>>>>> @@ -309,7 +309,7 @@ public class CommonEvents {
>>>>> return "success";
>>>>> }
>>>>>
>>>>> - private static void writeJSONtoResponse(JSON json,
>>>>> HttpServletRequest request, HttpServletResponse response) throws
>>>>> UnsupportedEncodingException {
>>>>> + static void writeJSONtoResponse(JSON json, HttpServletRequest
>>>>> request, HttpServletResponse response) throws
>>>>> UnsupportedEncodingException {
>>>>> String jsonStr = json.toString();
>>>>> if (jsonStr == null) {
>>>>> Debug.logError("JSON Object was empty; fatal error!",
>>>>> module);
>>>>>
>>>>>
>>>>>
>>>>>
>>
>
Re: svn commit: r1807045 -
/ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbi
z/common/CommonEvents.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
Hi Paul,
That's a good idea, please feel free to enhance or revert
Thanks
Jacques
Le 03/09/2017 à 06:58, Paul Foxworthy a écrit :
> Hi Jacques,
>
> These fields and methods will have package scope, but your description
> suggests you intended protected. I'm really uncomfortable about fields with
> package scope. Is it really necessary? Could they be protected?
>
> In the case of ignoreAttrs, the array should remain private. The loop
> in jsonResponseFromRequestAttributes that uses the array should be made a
> method in its own right, with a name such as removeSensitiveAttributes.
> That method could have higher visibility such as protected. The general
> principle is "don't ask for data, ask for help" (
> https://martinfowler.com/bliki/TellDontAsk.html).
>
> Cheers
>
> Paul Foxworthy
>
>
> On 3 September 2017 at 04:45, Jacques Le Roux <ja...@les7arts.com>
> wrote:
>
>> I think Wai asked for that because he needs it for a custom project. It's
>> sill reasonably encapsulated.
>>
>> You can revert if you want
>>
>> Jacques
>>
>>
>>
>> Le 02/09/2017 à 20:06, Taher Alkhateeb a écrit :
>>
>>> Do we have any derived classes that require exposing these interfaces?
>>>
>>> I think it's not a good design to break encapsulation just because of
>>> things we "might" need.
>>>
>>> On Sat, Sep 2, 2017 at 3:25 PM, <jl...@apache.org> wrote:
>>>
>>>> Author: jleroux
>>>> Date: Sat Sep 2 12:25:13 2017
>>>> New Revision: 1807045
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1807045&view=rev
>>>> Log:
>>>> Improved: CommonEvents improvements
>>>> (OFBIZ-9673)
>>>>
>>>> Replaces private to protected so that CommonEvents.java is more usable to
>>>> derived classes.
>>>>
>>>> Thanks: Wai
>>>>
>>>> Modified:
>>>> ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>>> org/apache/ofbiz/common/CommonEvents.java
>>>>
>>>> Modified: ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>>> org/apache/ofbiz/common/CommonEvents.java
>>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/fra
>>>> mework/common/src/main/java/org/apache/ofbiz/common/Common
>>>> Events.java?rev=1807045&r1=1807044&r2=1807045&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>>> org/apache/ofbiz/common/CommonEvents.java (original)
>>>> +++ ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>>> org/apache/ofbiz/common/CommonEvents.java Sat Sep 2 12:25:13 2017
>>>> @@ -68,7 +68,7 @@ public class CommonEvents {
>>>>
>>>> public static final String module = CommonEvents.class.getName();
>>>>
>>>> - private static final String[] ignoreAttrs = new String[] { //
>>>> Attributes removed for security reason; _ERROR_MESSAGE_ is kept
>>>> + static final String[] ignoreAttrs = new String[] { // Attributes
>>>> removed for security reason; _ERROR_MESSAGE_ is kept
>>>> "javax.servlet.request.key_size",
>>>> "_CONTEXT_ROOT_",
>>>> "_FORWARDED_FROM_SERVLET_",
>>>> @@ -82,7 +82,7 @@ public class CommonEvents {
>>>> "thisRequestUri"
>>>> };
>>>>
>>>> - private static final UtilCache<String, Map<String, String>>
>>>> appletSessions = UtilCache.createUtilCache("AppletSessions", 0, 600000,
>>>> true);
>>>> + static final UtilCache<String, Map<String, String>> appletSessions
>>>> = UtilCache.createUtilCache("AppletSessions", 0, 600000, true);
>>>>
>>>> public static String checkAppletRequest(HttpServletRequest
>>>> request, HttpServletResponse response) {
>>>> Delegator delegator = (Delegator)
>>>> request.getAttribute("delegator");
>>>> @@ -309,7 +309,7 @@ public class CommonEvents {
>>>> return "success";
>>>> }
>>>>
>>>> - private static void writeJSONtoResponse(JSON json,
>>>> HttpServletRequest request, HttpServletResponse response) throws
>>>> UnsupportedEncodingException {
>>>> + static void writeJSONtoResponse(JSON json, HttpServletRequest
>>>> request, HttpServletResponse response) throws UnsupportedEncodingException {
>>>> String jsonStr = json.toString();
>>>> if (jsonStr == null) {
>>>> Debug.logError("JSON Object was empty; fatal error!",
>>>> module);
>>>>
>>>>
>>>>
>
Re: svn commit: r1807045 - /ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbi
z/common/CommonEvents.java
Posted by Paul Foxworthy <pa...@cohsoft.com.au>.
Hi Jacques,
These fields and methods will have package scope, but your description
suggests you intended protected. I'm really uncomfortable about fields with
package scope. Is it really necessary? Could they be protected?
In the case of ignoreAttrs, the array should remain private. The loop
in jsonResponseFromRequestAttributes that uses the array should be made a
method in its own right, with a name such as removeSensitiveAttributes.
That method could have higher visibility such as protected. The general
principle is "don't ask for data, ask for help" (
https://martinfowler.com/bliki/TellDontAsk.html).
Cheers
Paul Foxworthy
On 3 September 2017 at 04:45, Jacques Le Roux <ja...@les7arts.com>
wrote:
> I think Wai asked for that because he needs it for a custom project. It's
> sill reasonably encapsulated.
>
> You can revert if you want
>
> Jacques
>
>
>
> Le 02/09/2017 à 20:06, Taher Alkhateeb a écrit :
>
>> Do we have any derived classes that require exposing these interfaces?
>>
>> I think it's not a good design to break encapsulation just because of
>> things we "might" need.
>>
>> On Sat, Sep 2, 2017 at 3:25 PM, <jl...@apache.org> wrote:
>>
>>> Author: jleroux
>>> Date: Sat Sep 2 12:25:13 2017
>>> New Revision: 1807045
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1807045&view=rev
>>> Log:
>>> Improved: CommonEvents improvements
>>> (OFBIZ-9673)
>>>
>>> Replaces private to protected so that CommonEvents.java is more usable to
>>> derived classes.
>>>
>>> Thanks: Wai
>>>
>>> Modified:
>>> ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>> org/apache/ofbiz/common/CommonEvents.java
>>>
>>> Modified: ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>> org/apache/ofbiz/common/CommonEvents.java
>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/fra
>>> mework/common/src/main/java/org/apache/ofbiz/common/Common
>>> Events.java?rev=1807045&r1=1807044&r2=1807045&view=diff
>>> ============================================================
>>> ==================
>>> --- ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>> org/apache/ofbiz/common/CommonEvents.java (original)
>>> +++ ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/
>>> org/apache/ofbiz/common/CommonEvents.java Sat Sep 2 12:25:13 2017
>>> @@ -68,7 +68,7 @@ public class CommonEvents {
>>>
>>> public static final String module = CommonEvents.class.getName();
>>>
>>> - private static final String[] ignoreAttrs = new String[] { //
>>> Attributes removed for security reason; _ERROR_MESSAGE_ is kept
>>> + static final String[] ignoreAttrs = new String[] { // Attributes
>>> removed for security reason; _ERROR_MESSAGE_ is kept
>>> "javax.servlet.request.key_size",
>>> "_CONTEXT_ROOT_",
>>> "_FORWARDED_FROM_SERVLET_",
>>> @@ -82,7 +82,7 @@ public class CommonEvents {
>>> "thisRequestUri"
>>> };
>>>
>>> - private static final UtilCache<String, Map<String, String>>
>>> appletSessions = UtilCache.createUtilCache("AppletSessions", 0, 600000,
>>> true);
>>> + static final UtilCache<String, Map<String, String>> appletSessions
>>> = UtilCache.createUtilCache("AppletSessions", 0, 600000, true);
>>>
>>> public static String checkAppletRequest(HttpServletRequest
>>> request, HttpServletResponse response) {
>>> Delegator delegator = (Delegator)
>>> request.getAttribute("delegator");
>>> @@ -309,7 +309,7 @@ public class CommonEvents {
>>> return "success";
>>> }
>>>
>>> - private static void writeJSONtoResponse(JSON json,
>>> HttpServletRequest request, HttpServletResponse response) throws
>>> UnsupportedEncodingException {
>>> + static void writeJSONtoResponse(JSON json, HttpServletRequest
>>> request, HttpServletResponse response) throws UnsupportedEncodingException {
>>> String jsonStr = json.toString();
>>> if (jsonStr == null) {
>>> Debug.logError("JSON Object was empty; fatal error!",
>>> module);
>>>
>>>
>>>
>
--
Coherent Software Australia Pty Ltd
PO Box 2773
Cheltenham Vic 3192
Australia
Phone: +61 3 9585 6788
Web: http://www.coherentsoftware.com.au/
Email: info@coherentsoftware.com.au
Re: svn commit: r1807045 -
/ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbi
z/common/CommonEvents.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
I think Wai asked for that because he needs it for a custom project. It's sill reasonably encapsulated.
You can revert if you want
Jacques
Le 02/09/2017 à 20:06, Taher Alkhateeb a écrit :
> Do we have any derived classes that require exposing these interfaces?
>
> I think it's not a good design to break encapsulation just because of
> things we "might" need.
>
> On Sat, Sep 2, 2017 at 3:25 PM, <jl...@apache.org> wrote:
>> Author: jleroux
>> Date: Sat Sep 2 12:25:13 2017
>> New Revision: 1807045
>>
>> URL: http://svn.apache.org/viewvc?rev=1807045&view=rev
>> Log:
>> Improved: CommonEvents improvements
>> (OFBIZ-9673)
>>
>> Replaces private to protected so that CommonEvents.java is more usable to
>> derived classes.
>>
>> Thanks: Wai
>>
>> Modified:
>> ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/CommonEvents.java
>>
>> Modified: ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/CommonEvents.java
>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/CommonEvents.java?rev=1807045&r1=1807044&r2=1807045&view=diff
>> ==============================================================================
>> --- ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/CommonEvents.java (original)
>> +++ ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/CommonEvents.java Sat Sep 2 12:25:13 2017
>> @@ -68,7 +68,7 @@ public class CommonEvents {
>>
>> public static final String module = CommonEvents.class.getName();
>>
>> - private static final String[] ignoreAttrs = new String[] { // Attributes removed for security reason; _ERROR_MESSAGE_ is kept
>> + static final String[] ignoreAttrs = new String[] { // Attributes removed for security reason; _ERROR_MESSAGE_ is kept
>> "javax.servlet.request.key_size",
>> "_CONTEXT_ROOT_",
>> "_FORWARDED_FROM_SERVLET_",
>> @@ -82,7 +82,7 @@ public class CommonEvents {
>> "thisRequestUri"
>> };
>>
>> - private static final UtilCache<String, Map<String, String>> appletSessions = UtilCache.createUtilCache("AppletSessions", 0, 600000, true);
>> + static final UtilCache<String, Map<String, String>> appletSessions = UtilCache.createUtilCache("AppletSessions", 0, 600000, true);
>>
>> public static String checkAppletRequest(HttpServletRequest request, HttpServletResponse response) {
>> Delegator delegator = (Delegator) request.getAttribute("delegator");
>> @@ -309,7 +309,7 @@ public class CommonEvents {
>> return "success";
>> }
>>
>> - private static void writeJSONtoResponse(JSON json, HttpServletRequest request, HttpServletResponse response) throws UnsupportedEncodingException {
>> + static void writeJSONtoResponse(JSON json, HttpServletRequest request, HttpServletResponse response) throws UnsupportedEncodingException {
>> String jsonStr = json.toString();
>> if (jsonStr == null) {
>> Debug.logError("JSON Object was empty; fatal error!", module);
>>
>>