jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Philippe Mouawad <philippe.moua...@gmail.com>
Subject Re: svn commit: r1732634 - in /jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http: control/CookieManager.java gui/CookiePanel.java
Date Sat, 27 Feb 2016 13:58:33 GMT
On Sat, Feb 27, 2016 at 2:52 PM, sebb <sebbaz@gmail.com> wrote:

> Also the commit fixed a bug and made some unrelated changes.
>
> Don't mix separate fixes.
>
> It's much harder to review and to follow the history later.
>
> Please revert and re-apply the change to CookiePanel separately.
>
It is part of the fix.

>
> I don't agree with the change to CookieManager; that needs further
> discussion.
>

Based  on my last commit few minutes ago, can you open a new discussion (as
subject is not clear) to mention what you disagree with ?
Thanks

>
>
> On 27 February 2016 at 13:45, sebb <sebbaz@gmail.com> wrote:
> > -1
> >
> > Must not change public constant names.
> >
> > If the name is not ideal, fix this by adding Javadoc, not a rename.
> >
> > On 27 February 2016 at 12:53,  <pmouawad@apache.org> wrote:
> >> Author: pmouawad
> >> Date: Sat Feb 27 12:53:18 2016
> >> New Revision: 1732634
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1732634&view=rev
> >> Log:
> >> Bug 58756 - CookieManager : Cookie Policy select box content must
> depend on Cookie implementation
> >> A) Fix bug introduced by commit r1732632:
> >> 1/ Save with 2.13 a Plan containing CookieManager that uses defaults
> >> 2/ Open it with trunk
> >> 3/ It is ok
> >> 4/ Close it
> >> 5/ Add a new CookieManager => KO the policy is default instead of
> standard
> >>
> >>
> >> B) Also made constants explicit and removed dependency on deprecated
> class.
> >>
> >> C) Forced the save of policy and implementation even if there values
> are set at defaults to avoid this headache again
> >>
> >>
> >> Bugzilla Id: 58756
> >>
> >> Modified:
> >>
>  jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CookieManager.java
> >>
>  jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/gui/CookiePanel.java
> >>
> >> Modified:
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CookieManager.java
> >> URL:
> http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CookieManager.java?rev=1732634&r1=1732633&r2=1732634&view=diff
> >>
> ==============================================================================
> >> ---
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CookieManager.java
> (original)
> >> +++
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CookieManager.java
> Sat Feb 27 12:53:18 2016
> >> @@ -30,7 +30,6 @@ import java.io.Serializable;
> >>  import java.net.URL;
> >>  import java.util.ArrayList;
> >>
> >> -import org.apache.http.client.config.CookieSpecs;
> >>  import org.apache.jmeter.config.ConfigTestElement;
> >>  import org.apache.jmeter.engine.event.LoopIterationEvent;
> >>  import org.apache.jmeter.testelement.TestIterationListener;
> >> @@ -102,12 +101,19 @@ public class CookieManager extends Confi
> >>
> >>      private transient CollectionProperty initialCookies;
> >>
> >> -    // MUST NOT BE CHANGED
> >> -    @SuppressWarnings("deprecation") // cannot be changed
> >> -    public static final String DEFAULT_POLICY =
> CookieSpecs.BROWSER_COMPATIBILITY;
> >> +    // MUST NOT BE CHANGED because as defaults are not saved,
> >> +    // when a Test Plan was loaded from an N-1 version and if defaults
> changed,
> >> +    // you end up changing the previously set policy
> >> +    // see issues with Bug 58756
> >> +    public static final String POLICY_FOR_BACKWARD_COMPATIBILITY =
> "compatibility";
> >>
> >> -    // MUST NOT BE CHANGED
> >> -    public static final String DEFAULT_IMPLEMENTATION =
> HC3CookieHandler.class.getName();
> >> +    // MUST NOT BE CHANGED because as defaults are not saved,
> >> +    // when a Test Plan was loaded from an N-1 version and if defaults
> changed,
> >> +    // you end up changing the previously set policy
> >> +    // see issues with Bug 58756
> >> +    public static final String
> IMPLEMENTATION_FOR_BACKWARD_COMPATIBILITY =
> HC3CookieHandler.class.getName();
> >> +
> >> +    public static final String DEFAULT_IMPLEMENTATION =
> HC4CookieHandler.class.getName();
> >>
> >>      public CookieManager() {
> >>          clearCookies(); // Ensure that there is always a collection
> available
> >> @@ -124,11 +130,13 @@ public class CookieManager extends Confi
> >>      }
> >>
> >>      public String getPolicy() {
> >> -        return getPropertyAsString(POLICY, DEFAULT_POLICY);
> >> +        return getPropertyAsString(POLICY,
> POLICY_FOR_BACKWARD_COMPATIBILITY);
> >>      }
> >>
> >>      public void setCookiePolicy(String policy){
> >> -        setProperty(POLICY, policy, DEFAULT_POLICY);
> >> +        // we must explicitely save the policy
> >> +        // not use a default implementation
> >> +        setProperty(POLICY, policy);
> >>      }
> >>
> >>      public CollectionProperty getCookies() {
> >> @@ -148,11 +156,13 @@ public class CookieManager extends Confi
> >>      }
> >>
> >>      public String getImplementation() {
> >> -        return getPropertyAsString(IMPLEMENTATION,
> DEFAULT_IMPLEMENTATION);
> >> +        return getPropertyAsString(IMPLEMENTATION,
> IMPLEMENTATION_FOR_BACKWARD_COMPATIBILITY);
> >>      }
> >>
> >>      public void setImplementation(String implementation){
> >> -        setProperty(IMPLEMENTATION, implementation,
> DEFAULT_IMPLEMENTATION);
> >> +        // we must explicitely save the policy
> >> +        // not use a default implementation
> >> +        setProperty(IMPLEMENTATION, implementation);
> >>      }
> >>
> >>      /**
> >>
> >> Modified:
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/gui/CookiePanel.java
> >> URL:
> http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/gui/CookiePanel.java?rev=1732634&r1=1732633&r2=1732634&view=diff
> >>
> ==============================================================================
> >> ---
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/gui/CookiePanel.java
> (original)
> >> +++
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/gui/CookiePanel.java
> Sat Feb 27 12:53:18 2016
> >> @@ -280,9 +280,9 @@ public class CookiePanel extends Abstrac
> >>
> >>          tableModel.clearData();
> >>          clearEachIteration.setSelected(false);
> >> -        policy.setText(HC4CookieHandler.DEFAULT_POLICY_NAME);
> >>          selectHandlerPanel.setSelectedItem(DEFAULT_IMPLEMENTATION
> >>                  .substring(DEFAULT_IMPLEMENTATION.lastIndexOf('.') +
> 1));
> >> +        policy.setText(HC4CookieHandler.DEFAULT_POLICY_NAME);
> >>          deleteButton.setEnabled(false);
> >>          saveButton.setEnabled(false);
> >>      }
> >>
> >>
>



-- 
Cordialement.
Philippe Mouawad.

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message