[GitHub] [jmeter] vlsi commented on a diff in pull request #5788: feat: add property to disable FunctionProperty caching

["vlsi (via GitHub)" <gi...@apache.org>]

vlsi commented on code in PR #5788:
URL: https://github.com/apache/jmeter/pull/5788#discussion_r1186450333


##########
src/protocol/java/src/main/java/org/apache/jmeter/protocol/java/sampler/BeanShellSampler.java:
##########
@@ -102,14 +102,6 @@ public SampleResult sample(Entry e)// Entry tends to be ignored ...
             return res;
         }
         try {
-            String request = getScript();

Review Comment:
   @FSchumacher , it turns out this caused two executions of `getScript` for `BeanShellSampler`.
   One was to pass the script for `res.setSamplerData`, and the second `getScript` was in `processFileOrScript` for actual BSH execution.
   
   I guess that is unexpected. However, the fix does not seem to be trivial: if I change `processFileOrScript` signature, it breaks backward compatibility because others might have overridden the method.
   On the other hand, `processFileOrScript` does not expose `script` and `fileName`, so it is not clear how to get that information out.
   
   For now, I added a new parameter to `processFileOrScript` so it calls `setSamplerData` as needed. I think it is unlikely that people override `processFileOrScript`. I checked GitHub search, and I found no cases for overriding `processFileOrScript`.
   
   If you are ok with that, I think we need to make the same changes for JSR223 samplers. They have exactly the same issue, and it can call `getScript()` several times during execution as well.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org