Re: svn commit: r1759457 - in /ofbiz/trunk/framework/webtools: config/webtools.properties src/main/java/org/apache/ofbiz/webtools/labelmanager/LabelReferences.java widget/LabelManagerScreens.xml

[Jacopo Cappellato-5]

On Tue, Sep 6, 2016 at 6:38 PM, <[hidden email]> wrote:

> Author: jleroux
> Date: Tue Sep  6 16:38:14 2016
> New Revision: 1759457
>
> URL: http://svn.apache.org/viewvc?rev=1759457&view=rev
> Log:
>
...

> * Moreover the string can't be in the LabelReferences.java file, to avoid
> side effects
>

Hi Jacques,

what are the side effects?

Thanks,

Jacopo

[Jacques Le Roux]

Le 06/09/2016 à 19:03, Jacopo Cappellato a écrit :

> On Tue, Sep 6, 2016 at 6:38 PM, <[hidden email]> wrote:
>
>> Author: jleroux
>> Date: Tue Sep  6 16:38:14 2016
>> New Revision: 1759457
>>
>> URL: http://svn.apache.org/viewvc?rev=1759457&view=rev
>> Log:
>>
> ...
>
>> * Moreover the string can't be in the LabelReferences.java file, to avoid
>> side effects
>>
> Hi Jacques,
>
> what are the side effects?
>
> Thanks,
>
> Jacopo
>

Hi Jacopo,

Good question, it has tortured me a moment, please try yourself you will see (kidding, see below ;))

I initially thought about 2 places to put the replacing regexp string:

 1. I began directly in code to test: inFile = inFile.replaceAll("ServiceUtil\\.getResource\\(\\)", getResource);
    This worked, and I decided to follow the pattern already used: a private member
 2. So I created a private static final String: private static final String getResourceRegex = "ServiceUtil\\.getResource\\(\\)"
    But this did not work. I got

    2016-09-07 08:27:32,489 |ttp-nio-8443-exec-10 |ScriptUtil                    |W| Error running script at location
    [component://webtools/groovyScripts/labelmanager/LabelManager.groovy]: java.lang.ArrayIndexOutOfBoundsException: 1
    java.lang.ArrayIndexOutOfBoundsException: 1  at
    org.apache.ofbiz.webtools.labelmanager.LabelReferences.getLabelsFromJavaFiles(LabelReferences.java:196) ~[ofbiz.jar:?]

    I wondered about doubling backslashes (which made not sense but with Java regexp and backslashes ...), when doing got the same error. And by
    investigating I found that using a member had a side effect during the parsing.
    If you use

                                 if (args.length > 1) {
                                     if (searchString.equals(args[1].trim())) {
                                         setLabelReference(labelKey, javaFile.getPath());
                                     }
                                 } else {
                                     Debug.log("searchString = " + searchString);
                                     Debug.log("args[0] = " + args[0]);
                                 }
    You will see the culprit:

    2016-09-07 09:17:32,873 |http-nio-8443-exec-7 |LabelManagerFactory           |I| Current file : AccountingEntityLabels.xml
    2016-09-07 09:17:33,903 |http-nio-8443-exec-7 |NoModule                      |F| searchString = "InvoiceItemType.description.PITM_DISCOUNT_ADJ"
    2016-09-07 09:17:33,903 |http-nio-8443-exec-7 |NoModule                      |F| args[0] = ";
         private static final String getResourceRegex = "ServiceUtil\\.getResource\\(\\

    That's also how I discovered we are considering entity description labels as not used when they are automatically by widget and even overriding
    some in data files
    BTW there are sometimes better English labels descriptions in data files. They should be used in labels and translations, 2 Jiras to create...

HTH

Jacques

[Jacopo Cappellato-5]

On Wed, Sep 7, 2016 at 9:23 AM, Jacques Le Roux <
[hidden email]> wrote:

> ...
>
2. So I created a private static final String: private static final String

> getResourceRegex = "ServiceUtil\\.getResource\\(\\)"
>    But this did not work. I got
>
>    2016-09-07 08:27:32,489 |ttp-nio-8443-exec-10 |ScriptUtil
>       |W| Error running script at location
>    [component://webtools/groovyScripts/labelmanager/LabelManager.groovy]:
> java.lang.ArrayIndexOutOfBoundsException: 1
>    java.lang.ArrayIndexOutOfBoundsException: 1  at
>    org.apache.ofbiz.webtools.labelmanager.LabelReferences.getL
> abelsFromJavaFiles(LabelReferences.java:196) ~[ofbiz.jar:?]
>
>

Without further digging into this issue, I would prefer the following tweak:

Index:
framework/webtools/src/main/java/org/apache/ofbiz/webtools/labelmanager/LabelReferences.java
===================================================================
---
framework/webtools/src/main/java/org/apache/ofbiz/webtools/labelmanager/LabelReferences.java
(revision
1759552)
+++
framework/webtools/src/main/java/org/apache/ofbiz/webtools/labelmanager/LabelReferences.java
(working
copy)
@@ -60,7 +60,7 @@
     private static final String uiLabelMap = "uiLabelMap.";
     private static final String formFieldTitle = "FormFieldTitle_";
     private static final String getMessage = "UtilProperties.getMessage(";
-    private static final String getResourceRegex =
UtilProperties.getPropertyValue("webtools", "getResourceRegex");
+    private static final String getResourceRegex =
"ServiceUtil\\.getResource\\(\\)";
     private static final String getResource = "ServiceUtil.getResource  ";

     protected Map<String, Map<String, Integer>> references = new
TreeMap<String, Map<String, Integer>>();
@@ -185,6 +185,8 @@
         for (String rootFolder : this.rootFolders) {
             List<File> javaFiles = FileUtil.findFiles("java", rootFolder +
"src", null, null);
             for (File javaFile : javaFiles) {
+                // do not parse this file
+                if ("LabelReferences.java".equals(javaFile.getName()))
continue;
                 String inFile = FileUtil.readString("UTF-8", javaFile);
                 inFile = inFile.replaceAll(getResourceRegex, getResource);
                 int pos = inFile.indexOf(getMessage);


In this way, we will not need an additional configuration file and the
ugliness will be self contained in the LabelReference class.

Jacopo

[Jacques Le Roux]

OK no problems with me, I'll change that. I must say I did no like the property either.

Jacques


Le 07/09/2016 à 09:56, Jacopo Cappellato a écrit :

> On Wed, Sep 7, 2016 at 9:23 AM, Jacques Le Roux <
> [hidden email]> wrote:
>
>> ...
>>
> 2. So I created a private static final String: private static final String
>> getResourceRegex = "ServiceUtil\\.getResource\\(\\)"
>>     But this did not work. I got
>>
>>     2016-09-07 08:27:32,489 |ttp-nio-8443-exec-10 |ScriptUtil
>>        |W| Error running script at location
>>     [component://webtools/groovyScripts/labelmanager/LabelManager.groovy]:
>> java.lang.ArrayIndexOutOfBoundsException: 1
>>     java.lang.ArrayIndexOutOfBoundsException: 1  at
>>     org.apache.ofbiz.webtools.labelmanager.LabelReferences.getL
>> abelsFromJavaFiles(LabelReferences.java:196) ~[ofbiz.jar:?]
>>
>>
> Without further digging into this issue, I would prefer the following tweak:
>
> Index:
> framework/webtools/src/main/java/org/apache/ofbiz/webtools/labelmanager/LabelReferences.java
> ===================================================================
> ---
> framework/webtools/src/main/java/org/apache/ofbiz/webtools/labelmanager/LabelReferences.java
> (revision
> 1759552)
> +++
> framework/webtools/src/main/java/org/apache/ofbiz/webtools/labelmanager/LabelReferences.java
> (working
> copy)
> @@ -60,7 +60,7 @@
>       private static final String uiLabelMap = "uiLabelMap.";
>       private static final String formFieldTitle = "FormFieldTitle_";
>       private static final String getMessage = "UtilProperties.getMessage(";
> -    private static final String getResourceRegex =
> UtilProperties.getPropertyValue("webtools", "getResourceRegex");
> +    private static final String getResourceRegex =
> "ServiceUtil\\.getResource\\(\\)";
>       private static final String getResource = "ServiceUtil.getResource  ";
>
>       protected Map<String, Map<String, Integer>> references = new
> TreeMap<String, Map<String, Integer>>();
> @@ -185,6 +185,8 @@
>           for (String rootFolder : this.rootFolders) {
>               List<File> javaFiles = FileUtil.findFiles("java", rootFolder +
> "src", null, null);
>               for (File javaFile : javaFiles) {
> +                // do not parse this file
> +                if ("LabelReferences.java".equals(javaFile.getName()))
> continue;
>                   String inFile = FileUtil.readString("UTF-8", javaFile);
>                   inFile = inFile.replaceAll(getResourceRegex, getResource);
>                   int pos = inFile.indexOf(getMessage);
>
>
> In this way, we will not need an additional configuration file and the
> ugliness will be self contained in the LabelReference class.
>
> Jacopo
>