Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove ProxyControl dependency on GuiPackage #3491

Closed
asfimport opened this issue Dec 3, 2014 · 26 comments
Closed

remove ProxyControl dependency on GuiPackage #3491

asfimport opened this issue Dec 3, 2014 · 26 comments

Comments

@asfimport
Copy link
Collaborator

Jaroslaw Fuks (Bug 57305):
I'm running proxy without UI.

Main problem is that ProxyControl class uses
GuiPackage.getInstance().getTreeModel() as source of tree model
to fix this i've added static member to ProxyControl,
witch can be overridden by user.

Created attachment jmeter.patch: patch implementing this feature

jmeter.patch
--- F:/Users/Jarek/workspace/apache-jmeter-2.12/src/protocol/http/org/apache/jmeter/protocol/http/proxy/ProxyControl.java	Wed Nov 05 20:17:58 2014
+++ F:/Users/Jarek/workspace/LoadTestAutomationFX/src/proxy/ProxyControl.java	Tue Dec 02 19:55:59 2014
@@ -202,6 +202,8 @@
     // If this is defined, it is assumed to be the alias of a user-supplied certificate; overrides dynamic mode
     static final String CERT_ALIAS = JMeterUtils.getProperty("proxy.cert.alias"); // $NON-NLS-1$
 
+    private static JMeterTreeModel treeModel = null;
+    
     public static enum KeystoreMode {
         USER_KEYSTORE,   // user-provided keystore
         JMETER_KEYSTORE, // keystore generated by JMeter; single entry
@@ -226,6 +228,10 @@
                 log.warn("HTTP(S) Test Script Recorder SSL Proxy will use keys that may not work for embedded resources in file " + CERT_PATH_ABS);
             }
         }
+        
+        if (GuiPackage.getInstance() != null) {
+        	treeModel = GuiPackage.getInstance().getTreeModel();
+        }
     }
 
     // Whether to use the redirect disabling feature (can be switched off if it does not work)
@@ -280,6 +286,14 @@
         setCaptureHttpHeaders(true); // maintain original behaviour
     }
 
+    /**
+     * When working without GUI required for operation
+     * @param treeModel JMeterTreeModel used for recording
+     */
+    public static void setGlobalTreeRoot(JMeterTreeModel treeModel) {
+    	ProxyControl.treeModel = treeModel;
+    }
+
     public void setPort(int port) {
         this.setProperty(new IntegerProperty(PORT, port));
     }
@@ -457,6 +471,7 @@
         try {
             server = new Daemon(getPort(), this);
             server.start();
+            if (GuiPackage.getInstance() != null)
             GuiPackage.getInstance().register(server);
         } catch (IOException e) {
             log.error("Could not create Proxy daemon", e);
@@ -571,6 +586,7 @@
     public void stopProxy() {
         if (server != null) {
             server.stopServer();
+            if (GuiPackage.getInstance() != null)
             GuiPackage.getInstance().unregister(server);
             try {
                 server.join(1000); // wait for server to stop
@@ -844,7 +860,6 @@
      *         <code>null</code> if none was found.
      */
     private JMeterTreeNode findFirstNodeOfType(Class<?> type) {
-        JMeterTreeModel treeModel = GuiPackage.getInstance().getTreeModel();
         List<JMeterTreeNode> nodes = treeModel.getNodesOfType(type);
         for (JMeterTreeNode node : nodes) {
             if (node.isEnabled()) {
@@ -910,7 +925,6 @@
      */
     // TODO - could be converted to generic class?
     private Collection<?> findApplicableElements(JMeterTreeNode myTarget, Class<? extends TestElement> myClass, boolean ascending) {
-        JMeterTreeModel treeModel = GuiPackage.getInstance().getTreeModel();
         LinkedList<TestElement> elements = new LinkedList<TestElement>();
 
         // Look for elements directly within the HTTP proxy:
@@ -968,8 +982,6 @@
     private void placeSampler(final HTTPSamplerBase sampler, final TestElement[] subConfigs,
             JMeterTreeNode myTarget) {
         try {
-            final JMeterTreeModel treeModel = GuiPackage.getInstance().getTreeModel();
-
             boolean firstInBatch = false;
             long now = System.currentTimeMillis();
             long deltaT = now - lastTime;
@@ -1163,7 +1175,6 @@
      *            sampling event to be delivered
      */
     private void notifySampleListeners(SampleEvent event) {
-        JMeterTreeModel treeModel = GuiPackage.getInstance().getTreeModel();
         JMeterTreeNode myNode = treeModel.getNodeOf(this);
         Enumeration<JMeterTreeNode> kids = myNode.children();
         while (kids.hasMoreElements()) {
@@ -1182,7 +1193,6 @@
      * (here meaning the proxy recording) has started.
      */
     private void notifyTestListenersOfStart() {
-        JMeterTreeModel treeModel = GuiPackage.getInstance().getTreeModel();
         JMeterTreeNode myNode = treeModel.getNodeOf(this);
         Enumeration<JMeterTreeNode> kids = myNode.children();
         while (kids.hasMoreElements()) {
@@ -1201,7 +1211,6 @@
      * (here meaning the proxy recording) has ended.
      */
     private void notifyTestListenersOfEnd() {
-        JMeterTreeModel treeModel = GuiPackage.getInstance().getTreeModel();
         JMeterTreeNode myNode = treeModel.getNodeOf(this);
         Enumeration<JMeterTreeNode> kids = myNode.children();
         while (kids.hasMoreElements()) {

Votes in Bugzilla: 2
OS: All

@asfimport
Copy link
Collaborator Author

Jaroslaw Fuks (migrated from Bugzilla):
Usage:

    treeModel = new JMeterTreeModel(new Object());
    
    ProxyControl.setGlobalTreeRoot(treeModel);
    
    JMeterTreeNode root = (JMeterTreeNode) treeModel.getRoot();
    treeModel.addSubTree(testPlanTree, root);
    
    proxy = new ProxyControl();
    proxy.setTarget(findTargetNode(root));
    proxy.setPort(8282);
    
    treeModel.addComponent(proxy, (JMeterTreeNode) root.getChildAt(1));
    
    proxy.startProxy();

@asfimport
Copy link
Collaborator Author

@pmouawad (migrated from Bugzilla):
Hi,
Thanks for contribution.
To be complete it would be interesting to have an option to start proxy from command-line .
I see you start it through API manipulation.

Regards

@asfimport
Copy link
Collaborator Author

Wyatt Epp (migrated from Bugzilla):
Why is this NEEDINFO? This is an important first step, and it would nice to get this merged.

@asfimport
Copy link
Collaborator Author

Wyatt Epp (migrated from Bugzilla):
Turns out this patch is actually important for being able to build against JMeter 3.0 and still make a valid JMeterTreeModel.

The method I was using for 2.13 was a shortcut that worked fine, but I had to rewrite it to create JMeterTreeNode.parent relations after updating. But that caused this to become an issue at runtime.

The posted patch still applies mostly clean; needs two small changes. I'll attach an updated version.

@asfimport
Copy link
Collaborator Author

Wyatt Epp (migrated from Bugzilla):
Created attachment newpatch.diff: Updated patch should apply clean to 3.0 source tree

newpatch.diff
diff --git a/src/protocol/http/org/apache/jmeter/protocol/http/proxy/ProxyControl.java b/src/protocol/http/org/apache/jmeter/protocol/http/proxy/ProxyControl.java
index f0e8cb8..fc79fdc 100644
--- a/src/protocol/http/org/apache/jmeter/protocol/http/proxy/ProxyControl.java
+++ b/src/protocol/http/org/apache/jmeter/protocol/http/proxy/ProxyControl.java
@@ -222,6 +222,8 @@ public class ProxyControl extends GenericController {
     // If this is defined, it is assumed to be the alias of a user-supplied certificate; overrides dynamic mode
     static final String CERT_ALIAS = JMeterUtils.getProperty("proxy.cert.alias"); // $NON-NLS-1$
 
+    private static JMeterTreeModel treeModel = null;
+
     public enum KeystoreMode {
         USER_KEYSTORE,   // user-provided keystore
         JMETER_KEYSTORE, // keystore generated by JMeter; single entry
@@ -246,6 +248,10 @@ public class ProxyControl extends GenericController {
                 log.warn("HTTP(S) Test Script Recorder SSL Proxy will use keys that may not work for embedded resources in file " + CERT_PATH_ABS);
             }
         }
+
+        if (GuiPackage.getInstance() != null) {
+		treeModel = GuiPackage.getInstance().getTreeModel();
+        }
     }
 
     // Whether to use the redirect disabling feature (can be switched off if it does not work)
@@ -300,6 +306,14 @@ public class ProxyControl extends GenericController {
         setCaptureHttpHeaders(true); // maintain original behaviour
     }
 
+    /**
+     * When working without GUI required for operation
+     * @param treeModel JMeterTreeModel used for recording
+     */
+    public static void setGlobalTreeRoot(JMeterTreeModel treeModel) {
+	ProxyControl.treeModel = treeModel;
+    }
+
     public void setPort(int port) {
         this.setProperty(new IntegerProperty(PORT, port));
     }
@@ -485,6 +499,7 @@ public class ProxyControl extends GenericController {
         try {
             server = new Daemon(getPort(), this);
             server.start();
+            if (GuiPackage.getInstance() != null)
             GuiPackage.getInstance().register(server);
         } catch (IOException e) {
             log.error("Could not create Proxy daemon", e);
@@ -674,6 +689,7 @@ public class ProxyControl extends GenericController {
     public void stopProxy() {
         if (server != null) {
             server.stopServer();
+            if (GuiPackage.getInstance() != null)
             GuiPackage.getInstance().unregister(server);
             try {
                 server.join(1000); // wait for server to stop
@@ -972,7 +988,6 @@ public class ProxyControl extends GenericController {
      *         <code>null</code> if none was found.
      */
     private JMeterTreeNode findFirstNodeOfType(Class<?> type) {
-        JMeterTreeModel treeModel = GuiPackage.getInstance().getTreeModel();
         List<JMeterTreeNode> nodes = treeModel.getNodesOfType(type);
         for (JMeterTreeNode node : nodes) {
             if (node.isEnabled()) {
@@ -1038,7 +1053,6 @@ public class ProxyControl extends GenericController {
      */
     // TODO - could be converted to generic class?
     private Collection<?> findApplicableElements(JMeterTreeNode myTarget, Class<? extends TestElement> myClass, boolean ascending) {
-        JMeterTreeModel treeModel = GuiPackage.getInstance().getTreeModel();
         LinkedList<TestElement> elements = new LinkedList<>();
 
         // Look for elements directly within the HTTP proxy:
@@ -1096,8 +1110,6 @@ public class ProxyControl extends GenericController {
     private void placeSampler(final HTTPSamplerBase sampler, final TestElement[] subConfigs,
             JMeterTreeNode myTarget) {
         try {
-            final JMeterTreeModel treeModel = GuiPackage.getInstance().getTreeModel();
-
             boolean firstInBatch = false;
             long now = System.currentTimeMillis();
             long deltaT = now - lastTime;
@@ -1288,7 +1300,6 @@ public class ProxyControl extends GenericController {
      *            sampling event to be delivered
      */
     private void notifySampleListeners(SampleEvent event) {
-        JMeterTreeModel treeModel = GuiPackage.getInstance().getTreeModel();
         JMeterTreeNode myNode = treeModel.getNodeOf(this);
         Enumeration<JMeterTreeNode> kids = myNode.children();
         while (kids.hasMoreElements()) {
@@ -1307,7 +1318,6 @@ public class ProxyControl extends GenericController {
      * (here meaning the proxy recording) has started.
      */
     private void notifyTestListenersOfStart() {
-        JMeterTreeModel treeModel = GuiPackage.getInstance().getTreeModel();
         JMeterTreeNode myNode = treeModel.getNodeOf(this);
         Enumeration<JMeterTreeNode> kids = myNode.children();
         while (kids.hasMoreElements()) {
@@ -1326,7 +1336,6 @@ public class ProxyControl extends GenericController {
      * (here meaning the proxy recording) has ended.
      */
     private void notifyTestListenersOfEnd() {
-        JMeterTreeModel treeModel = GuiPackage.getInstance().getTreeModel();
         JMeterTreeNode myNode = treeModel.getNodeOf(this);
         Enumeration<JMeterTreeNode> kids = myNode.children();
         while (kids.hasMoreElements()) {

@asfimport
Copy link
Collaborator Author

@pmouawad (migrated from Bugzilla):
(In reply to Wyatt Epp from comment 5)

Created attachment 34374 [details]
Updated patch should apply clean to 3.0 source tree

Thanks for patch.
Could you give an example of how you use it ?
Thank you

@asfimport
Copy link
Collaborator Author

Wyatt Epp (migrated from Bugzilla):
(In reply to Philippe Mouawad from comment 6)

(In reply to Wyatt Epp from comment 5)
> Created attachment 34374 [details]
> Updated patch should apply clean to 3.0 source tree

Thanks for patch.
Could you give an example of how you use it ?

It's just an update of Jarek's patch, so usage is unchanged from Comment 2 (only necessary when you're running headless).

@asfimport
Copy link
Collaborator Author

@pmouawad (migrated from Bugzilla):
My question was more, do you have a complete example of using it ?
For me it means you're using it through API.
Regards

@asfimport
Copy link
Collaborator Author

Wyatt Epp (migrated from Bugzilla):
(In reply to Philippe Mouawad from comment 8)

My question was more, do you have a complete example of using it ?

Complete example of using... what, exactly? It's not clear what you're looking for.

For me it means you're using it through API.

Well... yes? I mean, that's the only way to record headless, right now.

BTW, would it be asking too much to get this patch added to the 3.1 branch?

@asfimport
Copy link
Collaborator Author

@pmouawad (migrated from Bugzilla):
(In reply to Wyatt Epp from comment 9)

(In reply to Philippe Mouawad from comment 8)
> My question was more, do you have a complete example of using it ?

Complete example of using... what, exactly? It's not clear what you're
looking for.

Of using JMeter in headless mode to record.

> For me it means you're using it through API.

Well... yes? I mean, that's the only way to record headless, right now.

So my request is to have an example of using the API.

BTW, would it be asking too much to get this patch added to the 3.1 branch?

Yes if you are able to provide this example :-)
Thanks

@asfimport
Copy link
Collaborator Author

Wyatt Epp (migrated from Bugzilla):
(In reply to Philippe Mouawad from comment 10)

(In reply to Wyatt Epp from comment 9)
> Complete example of using... what, exactly? It's not clear what you're
> looking for.

Of using JMeter in headless mode to record.

Please clarify your definition of "complete", then?

Anyway, the sample in Comment 2 should be plenty for anyone deep enough into the code that they're legitimately affected by this bug (especially prior to 3.0, where you didn't have to properly establish JMeterTreeNode.parent relations on your RecordingController et al to capture properly).

@asfimport
Copy link
Collaborator Author

@pmouawad (migrated from Bugzilla):
(In reply to Wyatt Epp from comment 11)

(In reply to Philippe Mouawad from comment 10)
> (In reply to Wyatt Epp from comment 9)
> > Complete example of using... what, exactly? It's not clear what you're
> > looking for.
>
> Of using JMeter in headless mode to record.

Please clarify your definition of "complete", then?

Anyway, the sample in Comment 2 should be plenty for anyone deep enough
into the code that they're legitimately affected by this bug (especially
prior to 3.0, where you didn't have to properly establish
JMeterTreeNode.parent relations on your RecordingController et al to capture
properly).

If snipped above is fine, then that's what I mean by complete :-)

@asfimport
Copy link
Collaborator Author

Wyatt Epp (migrated from Bugzilla):
(In reply to Philippe Mouawad from comment 12)

If snipped above is fine, then that's what I mean by complete :-)

Well, it was more than enough for me, so great! It'll be very nice to have this; I had to reopen a bug in our internal tracker when I rolled back to 2.13. Thanks!

@asfimport
Copy link
Collaborator Author

@pmouawad (migrated from Bugzilla):
(In reply to Wyatt Epp from comment 13)

(In reply to Philippe Mouawad from comment 12)
> If snipped above is fine, then that's what I mean by complete :-)

Well, it was more than enough for me, so great! It'll be very nice to have
this; I had to reopen a bug in our internal tracker when I rolled back to
2.13. Thanks!

Hi,
I used the sample above and it is not complete.
1)testPlanTree variable is not initialized
2) findTargetNode does not exist

If we add this patch to core, we need to provide a complete and working example and in this case see which API should be made public. That's why I am asking for those information.

Thanks
Regards

@asfimport
Copy link
Collaborator Author

Wyatt Epp (migrated from Bugzilla):
(In reply to Philippe Mouawad from comment 14)

I used the sample above and it is not complete.

What, you didn't expect that to run as-is, did you?

If we add this patch to core, we need to provide a complete and working
example

Oh, moving the goalposts on me? Rude.

I'll play along, but only because we actually need this patch merged so we don't have to maintain our own version of JMeter internally.

and in this case see which API should be made public.

What? No. Stop. The method this patch adds must be public; anything else would be completely useless.

Created attachment MinimalProxy(1).java: "complete and working" example

MinimalProxy(1).java
import org.apache.jmeter.exceptions.IllegalUserActionException;
import org.apache.jmeter.gui.tree.JMeterTreeModel;
import org.apache.jmeter.gui.tree.JMeterTreeNode;
import org.apache.jmeter.protocol.http.proxy.ProxyControl;
import org.apache.jmeter.testelement.TestPlan;
import org.apache.jmeter.threads.ThreadGroup;
import org.apache.jmeter.util.JMeterUtils;
import org.apache.jorphan.collections.ListedHashTree;

import java.io.IOException;

public class MinimalProxy {
    public static void main(String args[]) throws IOException, IllegalUserActionException {
        // Without these, no ResourceBundle is made, and you get an NPE
        // at org.apache.jmeter.util.JMeterUtils.getResStringDefault(JMeterUtils.java:505)
        JMeterUtils.setJMeterHome("/opt/jmeter"); // Or wherever you put it.
        JMeterUtils.loadJMeterProperties(JMeterUtils.getJMeterBinDir() + "/jmeter.properties");
        JMeterUtils.initLocale();

        TestPlan testPlan = new TestPlan();
        ThreadGroup threadGroup = new ThreadGroup();
        ListedHashTree testPlanTree = new ListedHashTree();
        testPlanTree.add(testPlan);
        testPlanTree.add(threadGroup, testPlan);

        JMeterTreeModel treeModel = new JMeterTreeModel(new Object());
        ProxyControl.setGlobalTreeRoot(treeModel);
        JMeterTreeNode root = (JMeterTreeNode) treeModel.getRoot();
        treeModel.addSubTree(testPlanTree, root);

        ProxyControl proxy = new ProxyControl();
        proxy.setTarget(treeModel.getNodeOf(threadGroup));

        treeModel.addComponent(proxy, (JMeterTreeNode) root.getChildAt(1));

        proxy.startProxy();
    }
}

@asfimport
Copy link
Collaborator Author

@pmouawad (migrated from Bugzilla):
(In reply to Wyatt Epp from comment 15)

Created attachment 34386 [details]
"complete and working" example

(In reply to Philippe Mouawad from comment 14)
> I used the sample above and it is not complete.

What, you didn't expect that to run as-is, did you?

I did :-)

> If we add this patch to core, we need to provide a complete and working
> example

Oh, moving the goalposts on me? Rude.

:-)

I'll play along, but only because we actually need this patch merged so we
don't have to maintain our own version of JMeter internally.

> and in this case see which API should be made public.

What? No. Stop. The method this patch adds must be public; anything else
would be completely useless.

Of course for this one.

Thanks for patch and example

@asfimport
Copy link
Collaborator Author

@pmouawad (migrated from Bugzilla):
Hello again @wyatt,

Few questions:

  • How is user supposed to use the Proxy. He starts it in non gui mode. But how does he saves the Test plan recorded ?

I suppose you have some custom development to do that ? Any chance you contribute more ?

  • Would it make sense in the future to have a command line option to start proxy in NON GUI mode

Regards

@asfimport
Copy link
Collaborator Author

Wyatt Epp (migrated from Bugzilla):
(In reply to Philippe Mouawad from comment 17)

  • How is user supposed to use the Proxy. He starts it in non gui mode. But
    how does he saves the Test plan recorded ?

That's far beyond the scope of this bug. Think of it as an exercise for the reader. ;)

I suppose you have some custom development to do that ? Any chance you
contribute more ?

I'd like to, but we've been over this before: I can't actually release/upstream our full headless recording daemon until the release has been cleared with legal. I'm still making noise about it, but it's not up to me and I don't have an ETA. I'm in a bit of a grey area already for cleaning up that patch and sample.

  • Would it make sense in the future to have a command line option to start
    proxy in NON GUI mode

Maybe. In practice, we've found it works best to have the recorder be a daemon that accepts commands from a TCP client and gives feedback. (Similar to how jcmd or jfr work, I think.)

BTW, can you be update the milestone? I don't have permissions for that.

@asfimport
Copy link
Collaborator Author

@FSchumacher (migrated from Bugzilla):
Use instance variable instead of static one.

@wyatt
Was there any reason to use a static variable? Can you check, if this patch helps you?

@Team
I think it is safe to include this in 3.1, what do you think?

Created attachment 0001-Allow-to-override-the-JMeterTreeModel-for-non-GUI-mo.patch: Allow to use ProxyControl in non GUI mode

0001-Allow-to-override-the-JMeterTreeModel-for-non-GUI-mo.patch
From f49e763a418f230c9755056a7e935b008e6893d2 Mon Sep 17 00:00:00 2001
From: Felix Schumacher <felix.schumacher@internetallee.de>
Date: Thu, 20 Oct 2016 22:11:36 +0200
Subject: [PATCH] Allow to override the JMeterTreeModel for non-GUI mode

---
 .../jmeter/protocol/http/proxy/ProxyControl.java   | 51 ++++++++++++++++++----
 1 file changed, 42 insertions(+), 9 deletions(-)

diff --git a/src/protocol/http/org/apache/jmeter/protocol/http/proxy/ProxyControl.java b/src/protocol/http/org/apache/jmeter/protocol/http/proxy/ProxyControl.java
index c9a2e10..989cba8 100644
--- a/src/protocol/http/org/apache/jmeter/protocol/http/proxy/ProxyControl.java
+++ b/src/protocol/http/org/apache/jmeter/protocol/http/proxy/ProxyControl.java
@@ -301,6 +301,8 @@ public class ProxyControl extends GenericController {
 
     private String keyPassword;
 
+    private JMeterTreeModel nonGuiTreeModel;
+
     public ProxyControl() {
         setPort(DEFAULT_PORT);
         setExcludeList(new HashSet<String>());
@@ -308,6 +310,19 @@ public class ProxyControl extends GenericController {
         setCaptureHttpHeaders(true); // maintain original behaviour
     }
 
+    /**
+     * Set a {@link JMeterTreeModel} to be used by the ProxyControl, when used
+     * in a non-GUI environment, where the {@link JMeterTreeModel} can't be
+     * acquired through {@link GuiPackage#getTreeModel()}
+     *
+     * @param treeModel
+     *            the {@link JMeterTreeModel} to be used, or {@code null} when
+     *            the GUI model should be used
+     */
+    public void setNonGuiTreeModel(JMeterTreeModel treeModel) {
+        this.nonGuiTreeModel = treeModel;
+    }
+
     public void setPort(int port) {
         this.setProperty(new IntegerProperty(PORT, port));
     }
@@ -475,6 +490,13 @@ public class ProxyControl extends GenericController {
         return getPropertyAsString(CONTENT_TYPE_INCLUDE);
     }
 
+    /**
+     * @return the {@link JMeterTreeModel} used when run in non-GUI mode, or {@code null} when run in GUI mode
+     */
+    public JMeterTreeModel getNonGuiTreeModel() {
+        return nonGuiTreeModel;
+    }
+
     public void addConfigElement(ConfigElement config) {
         // NOOP
     }
@@ -493,7 +515,9 @@ public class ProxyControl extends GenericController {
         try {
             server = new Daemon(getPort(), this);
             server.start();
-            GuiPackage.getInstance().register(server);
+            if (GuiPackage.getInstance() != null) {
+                GuiPackage.getInstance().register(server);
+            }
         } catch (IOException e) {
             log.error("Could not create Proxy daemon", e);
             throw e;
@@ -682,7 +706,9 @@ public class ProxyControl extends GenericController {
     public void stopProxy() {
         if (server != null) {
             server.stopServer();
-            GuiPackage.getInstance().unregister(server);
+            if (GuiPackage.getInstance() != null) {
+                GuiPackage.getInstance().unregister(server);
+            }
             try {
                 server.join(1000); // wait for server to stop
             } catch (InterruptedException e) {
@@ -819,7 +845,7 @@ public class ProxyControl extends GenericController {
      * @param target {@link JMeterTreeNode}
      */
     private void setAuthorization(Authorization authorization, JMeterTreeNode target) {
-        JMeterTreeModel jmeterTreeModel = GuiPackage.getInstance().getTreeModel();
+        JMeterTreeModel jmeterTreeModel = getJmeterTreeModel();
         List<JMeterTreeNode> authManagerNodes = jmeterTreeModel.getNodesOfType(AuthManager.class);
         if (authManagerNodes.size() == 0) {
             try {
@@ -835,6 +861,13 @@ public class ProxyControl extends GenericController {
         }
     }
 
+    private JMeterTreeModel getJmeterTreeModel() {
+        if (this.nonGuiTreeModel == null) {
+            return GuiPackage.getInstance().getTreeModel();
+        }
+        return this.nonGuiTreeModel;
+    }
+
     /**
      * Helper method to add a Response Assertion
      * Called from AWT Event thread
@@ -980,7 +1013,7 @@ public class ProxyControl extends GenericController {
      *         <code>null</code> if none was found.
      */
     private JMeterTreeNode findFirstNodeOfType(Class<?> type) {
-        JMeterTreeModel treeModel = GuiPackage.getInstance().getTreeModel();
+        JMeterTreeModel treeModel = getJmeterTreeModel();
         List<JMeterTreeNode> nodes = treeModel.getNodesOfType(type);
         for (JMeterTreeNode node : nodes) {
             if (node.isEnabled()) {
@@ -1046,7 +1079,7 @@ public class ProxyControl extends GenericController {
      */
     // TODO - could be converted to generic class?
     private Collection<?> findApplicableElements(JMeterTreeNode myTarget, Class<? extends TestElement> myClass, boolean ascending) {
-        JMeterTreeModel treeModel = GuiPackage.getInstance().getTreeModel();
+        JMeterTreeModel treeModel = getJmeterTreeModel();
         LinkedList<TestElement> elements = new LinkedList<>();
 
         // Look for elements directly within the HTTP proxy:
@@ -1104,7 +1137,7 @@ public class ProxyControl extends GenericController {
     private void placeSampler(final HTTPSamplerBase sampler, final TestElement[] testElements,
             JMeterTreeNode myTarget) {
         try {
-            final JMeterTreeModel treeModel = GuiPackage.getInstance().getTreeModel();
+            final JMeterTreeModel treeModel = getJmeterTreeModel();
 
             boolean firstInBatch = false;
             long now = System.currentTimeMillis();
@@ -1326,7 +1359,7 @@ public class ProxyControl extends GenericController {
      *            sampling event to be delivered
      */
     private void notifySampleListeners(SampleEvent event) {
-        JMeterTreeModel treeModel = GuiPackage.getInstance().getTreeModel();
+        JMeterTreeModel treeModel = getJmeterTreeModel();
         JMeterTreeNode myNode = treeModel.getNodeOf(this);
         Enumeration<JMeterTreeNode> kids = myNode.children();
         while (kids.hasMoreElements()) {
@@ -1345,7 +1378,7 @@ public class ProxyControl extends GenericController {
      * (here meaning the proxy recording) has started.
      */
     private void notifyTestListenersOfStart() {
-        JMeterTreeModel treeModel = GuiPackage.getInstance().getTreeModel();
+        JMeterTreeModel treeModel = getJmeterTreeModel();
         JMeterTreeNode myNode = treeModel.getNodeOf(this);
         Enumeration<JMeterTreeNode> kids = myNode.children();
         while (kids.hasMoreElements()) {
@@ -1364,7 +1397,7 @@ public class ProxyControl extends GenericController {
      * (here meaning the proxy recording) has ended.
      */
     private void notifyTestListenersOfEnd() {
-        JMeterTreeModel treeModel = GuiPackage.getInstance().getTreeModel();
+        JMeterTreeModel treeModel = getJmeterTreeModel();
         JMeterTreeNode myNode = treeModel.getNodeOf(this);
         Enumeration<JMeterTreeNode> kids = myNode.children();
         while (kids.hasMoreElements()) {
-- 
2.7.4

@asfimport
Copy link
Collaborator Author

Wyatt Epp (migrated from Bugzilla):
(In reply to Felix Schumacher from comment 19)

@wyatt
Was there any reason to use a static variable? Can you check, if this patch
helps you?

That's just how the original patch posted by the reporter did things-- all I did was fix up his patch to apply clean on the 3.0 source tree.

If I had to guess it's just because he thought it was more natural to set things up by calling a static method. (I happen to disagree.) I'm pretty sure it'll be fine if it's not static.

@Team
I think it is safe to include this in 3.1, what do you think?

We would appreciate it greatly.

@asfimport
Copy link
Collaborator Author

@pmouawad (migrated from Bugzilla):
(In reply to Felix Schumacher from comment 19)

Created attachment 34394 [details]
Allow to use ProxyControl in non GUI mode

Use instance variable instead of static one.

@wyatt
Was there any reason to use a static variable? Can you check, if this patch
helps you?

@Team
I think it is safe to include this in 3.1, what do you think?

Hi Felix,
Ok for inclusion provided we provide the code sample to use it somewhere in docs.
Note this means we make classes mentionned kind of public API that should then be backward compatible.
Finally I don't see currently how users using code can easily save the recorded test plan.

Thanks for review and patch
Regards

@asfimport
Copy link
Collaborator Author

Wyatt Epp (migrated from Bugzilla):
(In reply to Philippe Mouawad from comment 21)

Note this means we make classes mentionned kind of public API that should
then be backward compatible.

Eh? The whole API is already public (if largely undocumented)-- I wouldn't have been able to write any of our tooling otherwise.

Finally I don't see currently how users using code can easily save the
recorded test plan.

That's because there isn't one. For all that it only ends up being about twenty lines of code, it was really annoying to write that part, in fact.

(It would be quite grand if there were a simple way to dump a JMeterTreeNode or JMeterTreeModel to JMX via SaveService. But there isn't. Has anyone filed a bug about that...?)

@asfimport
Copy link
Collaborator Author

@FSchumacher (migrated from Bugzilla):
@philippe I put a sample class inside the test src dirextory. It is not complete in the sense, that it outputs correct code, but it will hopefully give the right direction for those who are interested in this "feature".

Date: Fri Oct 28 17:56:25 2016
New Revision: 1767048

URL: http://svn.apache.org/viewvc?rev=1767048&view=rev
Log:
Remove dependency of ProxyControl on GuiPackage.
Based on patches by jarek102 at gmail.com and Wyatt Epp (wyatt.epp at gmail.com)

#3491

Added:
jmeter/trunk/test/src/org/apache/jmeter/protocol/http/proxy/NonGuiProxySample.java (with props)
Modified:
jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/proxy/ProxyControl.java
jmeter/trunk/xdocs/changes.xml

@asfimport
Copy link
Collaborator Author

Lars H (migrated from Bugzilla):
This is really cool stuff!

It would be very useful for me (and others too, I think) if this could be exposed as a script, useable out of the box (similar to bin/mirror-server).

Perhaps it could trigger writing the test plan on receiving SIGINT/CTRL-C from the user.

@asfimport
Copy link
Collaborator Author

Wyatt Epp (migrated from Bugzilla):
(In reply to Lars H from comment 24)

It would be very useful for me (and others too, I think) if this could be
exposed as a script, useable out of the box (similar to bin/mirror-server).

I'd recommend filing a bug about that separately.

@asfimport
Copy link
Collaborator Author

Lars H (migrated from Bugzilla):
(In reply to Wyatt Epp from comment 25)

(In reply to Lars H from comment 24)
>
> It would be very useful for me (and others too, I think) if this could be
> exposed as a script, useable out of the box (similar to bin/mirror-server).

I'd recommend filing a bug about that separately.

Yes that makes sense. Added separate issue for the script: #4164

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant