You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jmeter.apache.org by pm...@apache.org on 2016/01/25 21:44:56 UTC

svn commit: r1726684 - in /jmeter/trunk: src/core/org/apache/jmeter/util/JSR223TestElement.java src/core/org/apache/jmeter/util/ScriptingBeanInfoSupport.java xdocs/changes.xml

Author: pmouawad
Date: Mon Jan 25 20:44:55 2016
New Revision: 1726684

URL: http://svn.apache.org/viewvc?rev=1726684&view=rev
Log:
Bug 56554 : the script cache key is now automatically generated
#resolve #83
Bugzilla Id: 56554

Modified:
    jmeter/trunk/src/core/org/apache/jmeter/util/JSR223TestElement.java
    jmeter/trunk/src/core/org/apache/jmeter/util/ScriptingBeanInfoSupport.java
    jmeter/trunk/xdocs/changes.xml

Modified: jmeter/trunk/src/core/org/apache/jmeter/util/JSR223TestElement.java
URL: http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/util/JSR223TestElement.java?rev=1726684&r1=1726683&r2=1726684&view=diff
==============================================================================
--- jmeter/trunk/src/core/org/apache/jmeter/util/JSR223TestElement.java (original)
+++ jmeter/trunk/src/core/org/apache/jmeter/util/JSR223TestElement.java Mon Jan 25 20:44:55 2016
@@ -34,6 +34,7 @@ import javax.script.ScriptEngine;
 import javax.script.ScriptEngineManager;
 import javax.script.ScriptException;
 
+import org.apache.commons.codec.digest.DigestUtils;
 import org.apache.commons.collections.map.LRUMap;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.StringUtils;
@@ -66,8 +67,12 @@ public abstract class JSR223TestElement
     
     private static final long serialVersionUID = 233L;
 
-    private String cacheKey = ""; // If not empty then script in ScriptText will be compiled and cached
-
+    /** If not empty then script in ScriptText will be compiled and cached */
+    private String cacheKey = "";
+    
+    /** md5 of the script, used as an unique key for the cache */
+    private String scriptMd5 = null;
+    
     /**
      * Cache of compiled scripts
      */
@@ -182,21 +187,20 @@ public abstract class JSR223TestElement
             }  else {
                 throw new ScriptException("Script file '"+scriptFile.getAbsolutePath()+"' does not exist or is unreadable for element:"+getName());
             }
-        } else if (!StringUtils.isEmpty(getScript())){
+        } else if (!StringUtils.isEmpty(getScript())) {
             if (supportsCompilable && !StringUtils.isEmpty(cacheKey)) {
-                CompiledScript compiledScript = 
-                        compiledScriptsCache.get(cacheKey);
-                if (compiledScript==null) {
+                computeScriptMD5();
+                CompiledScript compiledScript = compiledScriptsCache.get(this.scriptMd5);
+                if (compiledScript == null) {
                     synchronized (compiledScriptsCache) {
-                        compiledScript = 
-                                compiledScriptsCache.get(cacheKey);
-                        if (compiledScript==null) {
-                            compiledScript = 
-                                    ((Compilable) scriptEngine).compile(getScript());
-                            compiledScriptsCache.put(cacheKey, compiledScript);
+                        compiledScript = compiledScriptsCache.get(this.scriptMd5);
+                        if (compiledScript == null) {
+                            compiledScript = ((Compilable) scriptEngine).compile(getScript());
+                            compiledScriptsCache.put(this.scriptMd5, compiledScript);
                         }
                     }
                 }
+                
                 return compiledScript.eval(bindings);
             } else {
                 return scriptEngine.eval(getScript(), bindings);
@@ -206,6 +210,15 @@ public abstract class JSR223TestElement
         }
     }
 
+    /**
+     * compute MD5 if it is null
+     */
+    private void computeScriptMD5() {
+        // compute the md5 of the script if needed
+        if(scriptMd5 == null) {
+            scriptMd5 = DigestUtils.md5Hex(getScript());
+        }
+    }
 
     /**
      * @return the cacheKey
@@ -251,7 +264,9 @@ public abstract class JSR223TestElement
     @Override
     public void testEnded(String host) {
         compiledScriptsCache.clear();
+        this.scriptMd5 = null;
     }
+    
     public String getScriptLanguage() {
         return scriptLanguage;
     }

Modified: jmeter/trunk/src/core/org/apache/jmeter/util/ScriptingBeanInfoSupport.java
URL: http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/util/ScriptingBeanInfoSupport.java?rev=1726684&r1=1726683&r2=1726684&view=diff
==============================================================================
--- jmeter/trunk/src/core/org/apache/jmeter/util/ScriptingBeanInfoSupport.java (original)
+++ jmeter/trunk/src/core/org/apache/jmeter/util/ScriptingBeanInfoSupport.java Mon Jan 25 20:44:55 2016
@@ -18,9 +18,17 @@
 
 package org.apache.jmeter.util;
 
+import java.awt.Component;
+import java.awt.event.ActionEvent;
+import java.awt.event.ActionListener;
 import java.beans.PropertyDescriptor;
+import java.beans.PropertyEditorSupport;
 import java.util.ResourceBundle;
+import java.util.UUID;
 
+import javax.swing.JCheckBox;
+
+import org.apache.commons.lang3.StringUtils;
 import org.apache.jmeter.testbeans.BeanInfoSupport;
 import org.apache.jmeter.testbeans.TestBean;
 import org.apache.jmeter.testbeans.gui.FileEditor;
@@ -35,7 +43,7 @@ public abstract class ScriptingBeanInfoS
         this(beanClass, languageTags, null);
     }
 
-    protected ScriptingBeanInfoSupport(Class<? extends TestBean> beanClass, String[] LANGUAGE_TAGS, ResourceBundle rb) {
+    protected ScriptingBeanInfoSupport(Class<? extends TestBean> beanClass, String[] languageTags, ResourceBundle rb) {
         super(beanClass);
         PropertyDescriptor p;
 
@@ -45,7 +53,7 @@ public abstract class ScriptingBeanInfoS
         if (rb != null) {
             p.setValue(RESOURCE_BUNDLE, rb);
         }
-        p.setValue(TAGS, LANGUAGE_TAGS);
+        p.setValue(TAGS, languageTags);
 
         createPropertyGroup("scriptingLanguage", // $NON-NLS-1$
                 new String[] { "scriptLanguage" }); // $NON-NLS-1$
@@ -92,7 +100,8 @@ public abstract class ScriptingBeanInfoS
             p = property("cacheKey"); // $NON-NLS-1$
             p.setValue(NOT_UNDEFINED, Boolean.TRUE);
             p.setValue(DEFAULT, ""); // $NON-NLS-1$
-    
+            p.setPropertyEditorClass(JSR223ScriptCacheCheckboxEditor.class);
+            
             createPropertyGroup("cacheKey_group", // $NON-NLS-1$
                 new String[] { "cacheKey" }); // $NON-NLS-1$
         }
@@ -104,6 +113,90 @@ public abstract class ScriptingBeanInfoS
 
         createPropertyGroup("scripting", // $NON-NLS-1$
                 new String[] { "script" }); // $NON-NLS-1$
+        
     }
+    
+    public static class JSR223ScriptCacheCheckboxEditor extends PropertyEditorSupport implements ActionListener {
+
+        private final JCheckBox checkbox;
+
+        /**
+         * Value on which we started the editing.
+         */
+        private String initialValue = null;
+
+        public JSR223ScriptCacheCheckboxEditor() {
+            super();
+
+            checkbox = new JCheckBox();
+            checkbox.addActionListener(this);
+        }
+
+        @Override
+        public String getAsText() {
+            String value = null;
+            if(checkbox.isSelected()) {
+                if(initialValue != null) {
+                    value = initialValue;
+                }
+                else {
+                    // the value is unique -> if the script is opened with a previous version of jmeter
+                    // where the cache key is used as the key for the cache
+                    // in the current version the key is automatically generated from the script content
+                    value = UUID.randomUUID().toString();
+                }
+            }
+            
+            return value;
+        }
+
+        @Override
+        public void setAsText(String value) {
+            if(StringUtils.isNotBlank(value)) {
+                initialValue = value;                
+            }
+            checkbox.setSelected(initialValue!= null);
+        }
 
+        @Override
+        public Object getValue() {
+            return getAsText();
+        }
+
+        @Override
+        public void setValue(Object value) {
+            if (value instanceof String) {
+                setAsText((String) value);
+            } else {
+                throw new IllegalArgumentException();
+            }
+        }
+
+        @Override
+        public Component getCustomEditor() {
+            return checkbox;
+        }
+
+        @Override
+        public void firePropertyChange() {
+            String newValue = getAsText();
+
+            if (initialValue != null && initialValue.equals(newValue)) {
+                return;
+            }
+            initialValue = newValue;
+
+            super.firePropertyChange();
+        }
+
+        @Override
+        public void actionPerformed(ActionEvent e) {
+            firePropertyChange();
+        }
+
+        @Override
+        public boolean supportsCustomEditor() {
+            return true;
+        }
+    }
 }

Modified: jmeter/trunk/xdocs/changes.xml
URL: http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1726684&r1=1726683&r2=1726684&view=diff
==============================================================================
--- jmeter/trunk/xdocs/changes.xml (original)
+++ jmeter/trunk/xdocs/changes.xml Mon Jan 25 20:44:55 2016
@@ -183,6 +183,7 @@ Summary
 <li><bug>58790</bug>Issue in CheckDirty and its relation to ActionRouter</li>
 <li><bug>58814</bug>JVM don't recognize option MaxLiveObjectEvacuationRatio; remove from comments</li>
 <li><bug>58810</bug>Config Element Counter (and others): Check Boxes Toggle Area Too Big</li>
+<li><bug>56554</bug>JSR223 Test Element : Generate compilation cache key automatically. Contributed by Benoit Wiart (benoit dot wiart at gmail.com)</li>
 </ul>
 <ch_section>Non-functional changes</ch_section>
 <ul>



Re: svn commit: r1726684 - in /jmeter/trunk: src/core/org/apache/jmeter/util/JSR223TestElement.java src/core/org/apache/jmeter/util/ScriptingBeanInfoSupport.java xdocs/changes.xml

Posted by Philippe Mouawad <ph...@gmail.com>.
On Tuesday, January 26, 2016, sebb <se...@gmail.com> wrote:

> On 25 January 2016 at 20:44,  <pmouawad@apache.org <javascript:;>> wrote:
> > Author: pmouawad
> > Date: Mon Jan 25 20:44:55 2016
> > New Revision: 1726684
> >
> > URL: http://svn.apache.org/viewvc?rev=1726684&view=rev
> > Log:
> > Bug 56554 : the script cache key is now automatically generated
> > #resolve #83
> > Bugzilla Id: 56554
> >
> > Modified:
> >     jmeter/trunk/src/core/org/apache/jmeter/util/JSR223TestElement.java
> >
>  jmeter/trunk/src/core/org/apache/jmeter/util/ScriptingBeanInfoSupport.java
> >     jmeter/trunk/xdocs/changes.xml
> >
> > Modified:
> jmeter/trunk/src/core/org/apache/jmeter/util/JSR223TestElement.java
> > URL:
> http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/util/JSR223TestElement.java?rev=1726684&r1=1726683&r2=1726684&view=diff
> >
> ==============================================================================
> > --- jmeter/trunk/src/core/org/apache/jmeter/util/JSR223TestElement.java
> (original)
> > +++ jmeter/trunk/src/core/org/apache/jmeter/util/JSR223TestElement.java
> Mon Jan 25 20:44:55 2016
> > @@ -34,6 +34,7 @@ import javax.script.ScriptEngine;
> >  import javax.script.ScriptEngineManager;
> >  import javax.script.ScriptException;
> >
> > +import org.apache.commons.codec.digest.DigestUtils;
> >  import org.apache.commons.collections.map.LRUMap;
> >  import org.apache.commons.io.IOUtils;
> >  import org.apache.commons.lang3.StringUtils;
> > @@ -66,8 +67,12 @@ public abstract class JSR223TestElement
> >
> >      private static final long serialVersionUID = 233L;
> >
> > -    private String cacheKey = ""; // If not empty then script in
> ScriptText will be compiled and cached
> > -
> > +    /** If not empty then script in ScriptText will be compiled and
> cached */
> > +    private String cacheKey = "";
> > +
> > +    /** md5 of the script, used as an unique key for the cache */
> > +    private String scriptMd5 = null;
> > +
>
> -1
>
> I already mentioned this on the Bugzilla - the md5 of the script may
> change, so needs to be calculated each time, and NOT cached.


before this commit, the compiled script used the code after fiest variable
replacement, no recompilation would occur even if variables changed.

So behaviour is not changed.

Computing md5 each time is overkill knowing that variables are better used
by passing them as parameters.



>
> >      /**
> >       * Cache of compiled scripts
> >       */
> > @@ -182,21 +187,20 @@ public abstract class JSR223TestElement
> >              }  else {
> >                  throw new ScriptException("Script file
> '"+scriptFile.getAbsolutePath()+"' does not exist or is unreadable for
> element:"+getName());
> >              }
> > -        } else if (!StringUtils.isEmpty(getScript())){
> > +        } else if (!StringUtils.isEmpty(getScript())) {
> >              if (supportsCompilable && !StringUtils.isEmpty(cacheKey)) {
> > -                CompiledScript compiledScript =
> > -                        compiledScriptsCache.get(cacheKey);
> > -                if (compiledScript==null) {
> > +                computeScriptMD5();
> > +                CompiledScript compiledScript =
> compiledScriptsCache.get(this.scriptMd5);
> > +                if (compiledScript == null) {
> >                      synchronized (compiledScriptsCache) {
> > -                        compiledScript =
> > -                                compiledScriptsCache.get(cacheKey);
> > -                        if (compiledScript==null) {
> > -                            compiledScript =
> > -                                    ((Compilable)
> scriptEngine).compile(getScript());
> > -                            compiledScriptsCache.put(cacheKey,
> compiledScript);
> > +                        compiledScript =
> compiledScriptsCache.get(this.scriptMd5);
> > +                        if (compiledScript == null) {
> > +                            compiledScript = ((Compilable)
> scriptEngine).compile(getScript());
> > +                            compiledScriptsCache.put(this.scriptMd5,
> compiledScript);
> >                          }
> >                      }
> >                  }
> > +
> >                  return compiledScript.eval(bindings);
> >              } else {
> >                  return scriptEngine.eval(getScript(), bindings);
> > @@ -206,6 +210,15 @@ public abstract class JSR223TestElement
> >          }
> >      }
> >
> > +    /**
> > +     * compute MD5 if it is null
> > +     */
> > +    private void computeScriptMD5() {
> > +        // compute the md5 of the script if needed
> > +        if(scriptMd5 == null) {
>
> -1
>
> The hash MUST be calculated each time.


As per my above note, -1 for this idea.
There is no regression introduced.
User who wants variables can uncheck the cache checkbox.




>
> > +            scriptMd5 = DigestUtils.md5Hex(getScript());
> > +        }
> > +    }
> >
> >      /**
> >       * @return the cacheKey
> > @@ -251,7 +264,9 @@ public abstract class JSR223TestElement
> >      @Override
> >      public void testEnded(String host) {
> >          compiledScriptsCache.clear();
> > +        this.scriptMd5 = null;
> >      }
> > +
> >      public String getScriptLanguage() {
> >          return scriptLanguage;
> >      }
> >
>


-- 
Cordialement.
Philippe Mouawad.

Re: svn commit: r1726684 - in /jmeter/trunk: src/core/org/apache/jmeter/util/JSR223TestElement.java src/core/org/apache/jmeter/util/ScriptingBeanInfoSupport.java xdocs/changes.xml

Posted by sebb <se...@gmail.com>.
On 25 January 2016 at 20:44,  <pm...@apache.org> wrote:
> Author: pmouawad
> Date: Mon Jan 25 20:44:55 2016
> New Revision: 1726684
>
> URL: http://svn.apache.org/viewvc?rev=1726684&view=rev
> Log:
> Bug 56554 : the script cache key is now automatically generated
> #resolve #83
> Bugzilla Id: 56554
>
> Modified:
>     jmeter/trunk/src/core/org/apache/jmeter/util/JSR223TestElement.java
>     jmeter/trunk/src/core/org/apache/jmeter/util/ScriptingBeanInfoSupport.java
>     jmeter/trunk/xdocs/changes.xml
>
> Modified: jmeter/trunk/src/core/org/apache/jmeter/util/JSR223TestElement.java
> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/util/JSR223TestElement.java?rev=1726684&r1=1726683&r2=1726684&view=diff
> ==============================================================================
> --- jmeter/trunk/src/core/org/apache/jmeter/util/JSR223TestElement.java (original)
> +++ jmeter/trunk/src/core/org/apache/jmeter/util/JSR223TestElement.java Mon Jan 25 20:44:55 2016
> @@ -34,6 +34,7 @@ import javax.script.ScriptEngine;
>  import javax.script.ScriptEngineManager;
>  import javax.script.ScriptException;
>
> +import org.apache.commons.codec.digest.DigestUtils;
>  import org.apache.commons.collections.map.LRUMap;
>  import org.apache.commons.io.IOUtils;
>  import org.apache.commons.lang3.StringUtils;
> @@ -66,8 +67,12 @@ public abstract class JSR223TestElement
>
>      private static final long serialVersionUID = 233L;
>
> -    private String cacheKey = ""; // If not empty then script in ScriptText will be compiled and cached
> -
> +    /** If not empty then script in ScriptText will be compiled and cached */
> +    private String cacheKey = "";
> +
> +    /** md5 of the script, used as an unique key for the cache */
> +    private String scriptMd5 = null;
> +

-1

I already mentioned this on the Bugzilla - the md5 of the script may
change, so needs to be calculated each time, and NOT cached.

>      /**
>       * Cache of compiled scripts
>       */
> @@ -182,21 +187,20 @@ public abstract class JSR223TestElement
>              }  else {
>                  throw new ScriptException("Script file '"+scriptFile.getAbsolutePath()+"' does not exist or is unreadable for element:"+getName());
>              }
> -        } else if (!StringUtils.isEmpty(getScript())){
> +        } else if (!StringUtils.isEmpty(getScript())) {
>              if (supportsCompilable && !StringUtils.isEmpty(cacheKey)) {
> -                CompiledScript compiledScript =
> -                        compiledScriptsCache.get(cacheKey);
> -                if (compiledScript==null) {
> +                computeScriptMD5();
> +                CompiledScript compiledScript = compiledScriptsCache.get(this.scriptMd5);
> +                if (compiledScript == null) {
>                      synchronized (compiledScriptsCache) {
> -                        compiledScript =
> -                                compiledScriptsCache.get(cacheKey);
> -                        if (compiledScript==null) {
> -                            compiledScript =
> -                                    ((Compilable) scriptEngine).compile(getScript());
> -                            compiledScriptsCache.put(cacheKey, compiledScript);
> +                        compiledScript = compiledScriptsCache.get(this.scriptMd5);
> +                        if (compiledScript == null) {
> +                            compiledScript = ((Compilable) scriptEngine).compile(getScript());
> +                            compiledScriptsCache.put(this.scriptMd5, compiledScript);
>                          }
>                      }
>                  }
> +
>                  return compiledScript.eval(bindings);
>              } else {
>                  return scriptEngine.eval(getScript(), bindings);
> @@ -206,6 +210,15 @@ public abstract class JSR223TestElement
>          }
>      }
>
> +    /**
> +     * compute MD5 if it is null
> +     */
> +    private void computeScriptMD5() {
> +        // compute the md5 of the script if needed
> +        if(scriptMd5 == null) {

-1

The hash MUST be calculated each time.

> +            scriptMd5 = DigestUtils.md5Hex(getScript());
> +        }
> +    }
>
>      /**
>       * @return the cacheKey
> @@ -251,7 +264,9 @@ public abstract class JSR223TestElement
>      @Override
>      public void testEnded(String host) {
>          compiledScriptsCache.clear();
> +        this.scriptMd5 = null;
>      }
> +
>      public String getScriptLanguage() {
>          return scriptLanguage;
>      }
>