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 <seb...@gmail.com> wrote: > On 27 February 2016 at 12:09, Philippe Mouawad > <philippe.moua...@gmail.com> wrote: > > On Sat, Feb 27, 2016 at 2:01 AM, <s...@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.
