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: r1732590 - /jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CookieManager.java
Date Sat, 27 Feb 2016 13:06:11 GMT
Hi,
I changed code a bit in r1732634 because there was a bug anyway.

My answers inline.

Regards
Philippe

On Sat, Feb 27, 2016 at 1:54 PM, sebb <sebbaz@gmail.com> wrote:

> On 27 February 2016 at 12:09, Philippe Mouawad
> <philippe.mouawad@gmail.com> wrote:
> > On Sat, Feb 27, 2016 at 2:01 AM, <sebb@apache.org> wrote:
> >
> >> Author: sebb
> >> Date: Sat Feb 27 01:01:07 2016
> >> New Revision: 1732590
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1732590&view=rev
> >> Log:
> >> Revert change that broke JMX compatibility
> >> TODO fix GUI to support change in default
> >>
> >> Modified:
> >>
> >>
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CookieManager.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=1732590&r1=1732589&r2=1732590&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 01:01:07 2016
> >> @@ -101,8 +101,13 @@ public class CookieManager extends Confi
> >>      private transient CookieHandler cookieHandler;
> >>
> >>      private transient CollectionProperty initialCookies;
> >> +
> >> +    // MUST NOT BE CHANGED
> >>
> > A more explicit note should be here to explain why it must not be
> changed:
> > - Reference the bug or the case
>
> I can expand the text but the reason has little to do with the bug or
> the test case
> The reason it cannot be changed is that it relates to how JMX files
> are read/written.
> Upwards compatibility requires immutable default values
>
> >> +    @SuppressWarnings("deprecation") // cannot be changed
> >> +    public static final String DEFAULT_POLICY =
> >> CookieSpecs.BROWSER_COMPATIBILITY;
> >>
> > Thinking more about it, keeping "Wrong Default" while the good one are in
> > GUI does not seem to me great:
> > - Code readability, HC4CookieHandler is the real default, developers or
> > users might think it is still HC3CookieHandler
>
> That is not the whole story, see below
>
> > - These Constants will disappear in next HttpClient 5 version , they are
> > already deprecated
>
> No, they cannot just disappear.
>
I am speaking about the HC constants, they are already gone.
I understand our constants cannot disappear.

>
> There are two defaults in operation here.
>
> * The defaults used in connection with reading/writing JMX files.
> These must remain immutable otherwise JMX files are not upwards compatible.
> For the same reason we cannot change the names of other attributes in JMX
> files.
> The actual values of defaults are largely arbitrary, but are chosen to
> be the commonest values.
>
I am aware of that

>
> * The defaults used when adding a new test element to a plan.
> These can be changed at will.
>

They can if we stop adding the 3rd parameter to setProperty

>
> Now if the default JMX value ceases to have a meaning, then we have a
> different problem.
>

The old defaults for CookieManager have the following status:
- HC3CookieHandler
- default for HC4 and HC3 have different behaviour. HC3 default is closer
to HC4 standard , but HC4 supports up to date RFC, not HC3.



> In the case of class names, this is generally handled by fixing the
> JMX file on input.
> The code could be extended for field values, or the GUI code could
> convert invalid values.
>
> >
> >>
> >> -    public static final String DEFAULT_IMPLEMENTATION =
> >> HC4CookieHandler.class.getName();
> >> +    // MUST NOT BE CHANGED
> >> +    public static final String DEFAULT_IMPLEMENTATION =
> >> HC3CookieHandler.class.getName();
> >>
> >>      public CookieManager() {
> >>          clearCookies(); // Ensure that there is always a collection
> >> available
> >> @@ -119,11 +124,11 @@ public class CookieManager extends Confi
> >>      }
> >>
> >>      public String getPolicy() {
> >> -        return getPropertyAsString(POLICY,
> >> HC4CookieHandler.DEFAULT_POLICY_NAME);
> >> +        return getPropertyAsString(POLICY, DEFAULT_POLICY);
> >>      }
> >>
> >>      public void setCookiePolicy(String policy){
> >> -        setProperty(POLICY, policy,
> HC4CookieHandler.DEFAULT_POLICY_NAME);
> >> +        setProperty(POLICY, policy, DEFAULT_POLICY);
> >>      }
> >>
> >>      public CollectionProperty getCookies() {
> >>
> >>
> >>
> >
> >
> > --
> > Cordialement.
> > Philippe Mouawad.
>



-- 
Cordialement.
Philippe Mouawad.

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