Uploaded image for project: 'OFBiz'
  1. OFBiz
  2. OFBIZ-8153

LabelManager doesn't search labels in .groovy files

    Details

      Description

      There are over 60 labels used in groovy files. These don't get reflected in the counts of the label manager and in the search for labels not used.

      1. OFBIZ-8153.patch
        2 kB
        Suraj Khurana

        Issue Links

          Activity

          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          HI Pierre, I assigned myself because I have already decided to work on OFBIZ-8154 . But let me know if you expect to work on this issue, and then please assign yourself. Thanks.

          Show
          jacques.le.roux Jacques Le Roux added a comment - HI Pierre, I assigned myself because I have already decided to work on OFBIZ-8154 . But let me know if you expect to work on this issue, and then please assign yourself. Thanks.
          Hide
          pfm.smits Pierre Smits added a comment -

          Hi Jacques Le Roux,

          No worries and thanks. If I was planning on working on this I would have assigned myself.

          Show
          pfm.smits Pierre Smits added a comment - Hi Jacques Le Roux , No worries and thanks. If I was planning on working on this I would have assigned myself.
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          HI Pierre,

          Could you please try the following patch. I did not put much brain it it, it's just a copy of the Java one, I did not test...

          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 1759573)
          +++ framework/webtools/src/main/java/org/apache/ofbiz/webtools/labelmanager/LabelReferences.java	(working copy)
          @@ -109,8 +109,10 @@
                   }
                   // get labels from FTL files
                   getLabelsFromFtlFiles();
          -        // get labels from java files
          +        // get labels from Java files
                   getLabelsFromJavaFiles();
          +        // get labels from Groovy files
          +        getLabelsFromGroovyFiles();
                   // get labels from simple method files
                   getLabelsFromSimpleMethodFiles();
                   // get labels from widgets files
          @@ -209,6 +211,33 @@
                   }
               }
           
          +    private void getLabelsFromGroovyFiles() throws IOException {
          +        for (String rootFolder : this.rootFolders) {
          +            List<File> groovyFiles = FileUtil.findFiles("groovy", rootFolder + "groovyScripts", null, null);
          +            for (File groovyFile : groovyFiles) {
          +                String inFile = FileUtil.readString("UTF-8", groovyFile);
          +                inFile = inFile.replaceAll(getResourceRegex, getResource);
          +                int pos = inFile.indexOf(getMessage);
          +                while (pos >= 0) {
          +                    int endLabel = inFile.indexOf(")", pos);
          +                    if (endLabel >= 0) {
          +                        String[] args = inFile.substring(pos + getMessage.length(), endLabel).split(",");
          +                        for (String labelKey : this.labelSet) {
          +                            String searchString = "\"" + labelKey + "\"";
          +                            if (searchString.equals(args[1].trim())) {
          +                                setLabelReference(labelKey, groovyFile.getPath());
          +                            }
          +                        }
          +                        pos = endLabel;
          +                    } else {
          +                        pos = pos + getMessage.length();
          +                    }
          +                    pos = inFile.indexOf(getMessage, pos);
          +                }
          +            }
          +        }
          +    }
          +
               protected void findUiLabelMapInFile(String inFile, String filePath) {
                   int pos = inFile.indexOf(uiLabelMap);
                   while (pos >= 0) {
          
          Show
          jacques.le.roux Jacques Le Roux added a comment - HI Pierre, Could you please try the following patch. I did not put much brain it it, it's just a copy of the Java one, I did not test... 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 1759573) +++ framework/webtools/src/main/java/org/apache/ofbiz/webtools/labelmanager/LabelReferences.java (working copy) @@ -109,8 +109,10 @@ } // get labels from FTL files getLabelsFromFtlFiles(); - // get labels from java files + // get labels from Java files getLabelsFromJavaFiles(); + // get labels from Groovy files + getLabelsFromGroovyFiles(); // get labels from simple method files getLabelsFromSimpleMethodFiles(); // get labels from widgets files @@ -209,6 +211,33 @@ } } + private void getLabelsFromGroovyFiles() throws IOException { + for ( String rootFolder : this .rootFolders) { + List<File> groovyFiles = FileUtil.findFiles( "groovy" , rootFolder + "groovyScripts" , null , null ); + for (File groovyFile : groovyFiles) { + String inFile = FileUtil.readString( "UTF-8" , groovyFile); + inFile = inFile.replaceAll(getResourceRegex, getResource); + int pos = inFile.indexOf(getMessage); + while (pos >= 0) { + int endLabel = inFile.indexOf( ")" , pos); + if (endLabel >= 0) { + String [] args = inFile.substring(pos + getMessage.length(), endLabel).split( "," ); + for ( String labelKey : this .labelSet) { + String searchString = "\" " + labelKey + " \""; + if (searchString.equals(args[1].trim())) { + setLabelReference(labelKey, groovyFile.getPath()); + } + } + pos = endLabel; + } else { + pos = pos + getMessage.length(); + } + pos = inFile.indexOf(getMessage, pos); + } + } + } + } + protected void findUiLabelMapInFile( String inFile, String filePath) { int pos = inFile.indexOf(uiLabelMap); while (pos >= 0) {
          Hide
          pfm.smits Pierre Smits added a comment -

          Hi Jacques,

          It still seem to forgo on the issues in groovy files. I tested it against various labels in BalanceSheet.groovy in the accounting component. It didn't recognise:

          • uiLabelMap.AccountingTotalAssets
          • uiLabelMap.AccountingTotalAccumulatedDepreciation

          etc.

          Show
          pfm.smits Pierre Smits added a comment - Hi Jacques, It still seem to forgo on the issues in groovy files. I tested it against various labels in BalanceSheet.groovy in the accounting component. It didn't recognise: uiLabelMap.AccountingTotalAssets uiLabelMap.AccountingTotalAccumulatedDepreciation etc.
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Thanks Pierre, I'll double check that

          Show
          jacques.le.roux Jacques Le Roux added a comment - Thanks Pierre, I'll double check that
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Thanks for report Pierre,

          I committed a fix in
          trunk r1759941
          R15.12, R14.12, R13.07 r1759942

          My previous patch was non sense, this commit uses the already existing findUiLabelMapInFile so should do the work rightly.

          Show
          jacques.le.roux Jacques Le Roux added a comment - Thanks for report Pierre, I committed a fix in trunk r1759941 R15.12, R14.12, R13.07 r1759942 My previous patch was non sense, this commit uses the already existing findUiLabelMapInFile so should do the work rightly.
          Hide
          suraj.khurana Suraj Khurana added a comment -

          I am still unable to find reference of all uiLabels used in groovy files hence re-opening the issue.

          Tried for uiLabelMap.AccountingTotalAssets in BalanceSheet.groovy in the accounting component, didn't get any luck.

          IMO, the logic written needs more use cases to be covered for groovy files, existing findUiLabelMapInFile is not fit for groovy files.
          Following patterns are currently used in groovy files:

          1. uiLabelMap.xxxxx
          2. uiLabelMap.get("xxxxx")
          3. Should not be like uiLabelMap.xxxxx( (As it might be calling a method of ResourceBundleMapWrapper class)

          Show
          suraj.khurana Suraj Khurana added a comment - I am still unable to find reference of all uiLabels used in groovy files hence re-opening the issue. Tried for uiLabelMap.AccountingTotalAssets in BalanceSheet.groovy in the accounting component, didn't get any luck. IMO, the logic written needs more use cases to be covered for groovy files, existing findUiLabelMapInFile is not fit for groovy files. Following patterns are currently used in groovy files: 1. uiLabelMap.xxxxx 2. uiLabelMap.get("xxxxx") 3. Should not be like uiLabelMap.xxxxx( (As it might be calling a method of ResourceBundleMapWrapper class)
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Hi Suraj,

          Seems that you are already advanced on this issue, will you provide a patch?

          Show
          jacques.le.roux Jacques Le Roux added a comment - Hi Suraj, Seems that you are already advanced on this issue, will you provide a patch?
          Hide
          suraj.khurana Suraj Khurana added a comment -

          Sure Jacques Le Roux,

          I will look into it and soon provide a patch for the same. Assigning this task to me.

          Show
          suraj.khurana Suraj Khurana added a comment - Sure Jacques Le Roux , I will look into it and soon provide a patch for the same. Assigning this task to me.
          Hide
          suraj.khurana Suraj Khurana added a comment -

          Hi Jacques Le Roux,

          Here is the proposed patch which covers every case as mentioned, please have a look into it.

          Show
          suraj.khurana Suraj Khurana added a comment - Hi Jacques Le Roux , Here is the proposed patch which covers every case as mentioned, please have a look into it.
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Thanks Suraj,

          Your pach looks good to me. I tested you get indeed less unused labels (2184 vs 2201). I wondered why in 3 places we use uiLabelMap.get("xxxxx") instead of uiLabelMap.xxxxx which does essentially the same when there is no default parameter passed to get(). So I changed them at r1805548. But anyway we can hardly prevent committers to use uiLabelMap.get("xxxxx") so I kept it in your patch.

          BTW I'm not quite sure I understood these sentences

          Tried for uiLabelMap.AccountingTotalAssets in BalanceSheet.groovy in the accounting component, didn't get any luck.

          Should not be like uiLabelMap.xxxxx( (As it might be calling a method of ResourceBundleMapWrapper class)

          For the second I guess it was about existing code in LabelReferences.java

          Anyway your patch is in
          trunk r1805558
          R16.11 r1805559
          R15.12, R14.12, R13.07 r1805560

          Show
          jacques.le.roux Jacques Le Roux added a comment - Thanks Suraj, Your pach looks good to me. I tested you get indeed less unused labels (2184 vs 2201). I wondered why in 3 places we use uiLabelMap.get("xxxxx") instead of uiLabelMap.xxxxx which does essentially the same when there is no default parameter passed to get() . So I changed them at r1805548. But anyway we can hardly prevent committers to use uiLabelMap.get("xxxxx") so I kept it in your patch. BTW I'm not quite sure I understood these sentences Tried for uiLabelMap.AccountingTotalAssets in BalanceSheet.groovy in the accounting component, didn't get any luck. Should not be like uiLabelMap.xxxxx( (As it might be calling a method of ResourceBundleMapWrapper class) For the second I guess it was about existing code in LabelReferences.java Anyway your patch is in trunk r1805558 R16.11 r1805559 R15.12, R14.12, R13.07 r1805560
          Hide
          suraj.khurana Suraj Khurana added a comment -

          Thanks Jacques Le Roux,

          Tried for uiLabelMap.AccountingTotalAssets in BalanceSheet.groovy in the accounting component, didn't get any luck.

          I tried to test this considering uiLabelMap.AccountingTotalAssets in BalanceSheet.groovy but it was not working earlier.

          Should not be like uiLabelMap.xxxxx( (As it might be calling a method of ResourceBundleMapWrapper class)

          There are some occurrences such as uiLabelMap.addBottomResourceBundle("CommonUiLabels") as well, which are not in our scope of consideration.

          I am also thinking some way by which existing code in LabelReferences.java can be handled/ignored to improve performance.

          Show
          suraj.khurana Suraj Khurana added a comment - Thanks Jacques Le Roux , Tried for uiLabelMap.AccountingTotalAssets in BalanceSheet.groovy in the accounting component, didn't get any luck. I tried to test this considering uiLabelMap.AccountingTotalAssets in BalanceSheet.groovy but it was not working earlier. Should not be like uiLabelMap.xxxxx( (As it might be calling a method of ResourceBundleMapWrapper class) There are some occurrences such as uiLabelMap.addBottomResourceBundle("CommonUiLabels") as well, which are not in our scope of consideration. I am also thinking some way by which existing code in LabelReferences.java can be handled/ignored to improve performance.
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Thanks Suraj,

          Indeed improving performance is certainly possible. But also not a big deal, we don't use the label manager much and it's usable as is. Anyway all improvements are welcome.

          Show
          jacques.le.roux Jacques Le Roux added a comment - Thanks Suraj, Indeed improving performance is certainly possible. But also not a big deal, we don't use the label manager much and it's usable as is. Anyway all improvements are welcome.

            People

            • Assignee:
              jacques.le.roux Jacques Le Roux
              Reporter:
              pfm.smits Pierre Smits
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development