You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by sebb <se...@gmail.com> on 2012/07/08 15:01:05 UTC

Re: svn commit: r1358710 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java xdocs/changes.xml

On 8 July 2012 10:40,  <pm...@apache.org> wrote:
> Author: pmouawad
> Date: Sun Jul  8 09:40:51 2012
> New Revision: 1358710
>
> URL: http://svn.apache.org/viewvc?rev=1358710&view=rev
> Log:
> Bug 53521 - Cache Manager should cache content with Cache-control=private
> Bugzilla Id: 53521
>
> Modified:
>     jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
>     jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
>     jmeter/trunk/xdocs/changes.xml
>
> Modified: jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java?rev=1358710&r1=1358709&r2=1358710&view=diff
> ==============================================================================
> --- jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java (original)
> +++ jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java Sun Jul  8 09:40:51 2012
> @@ -161,7 +161,7 @@ public class CacheManager extends Config
>          if (useExpires) {// Check that we are processing Expires/CacheControl
>              final String MAX_AGE = "max-age=";
>              // TODO - check for other CacheControl attributes?
> -            if (cacheControl != null && cacheControl.contains("public") && cacheControl.contains(MAX_AGE)) {
> +            if (cacheControl != null && (cacheControl.contains("public") || cacheControl.contains("private")) && cacheControl.contains(MAX_AGE)) {

Not sure this is the correct fix.
Do we really care if the string "public" or "private" is present so
long as there is a MAX_AGE entry?
We could just drop the check for "public" instead.

>                  long maxAgeInSecs = Long.parseLong(
>                          cacheControl.substring(cacheControl.indexOf(MAX_AGE)+MAX_AGE.length())
>                              .split("[, ]")[0] // Bug 51932 - allow for optional trailing attributes
>
> Modified: jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
> URL: http://svn.apache.org/viewvc/jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java?rev=1358710&r1=1358709&r2=1358710&view=diff
> ==============================================================================
> --- jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java (original)
> +++ jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java Sun Jul  8 09:40:51 2012
> @@ -238,7 +238,30 @@ public class TestCacheManager extends JM
>          assertNotNull("Should find entry",getThreadCacheEntry(LOCAL_HOST));
>          assertTrue("Should find valid entry",this.cacheManager.inCache(url));
>      }
> +
> +    public void testPrivateCacheHttpClient() throws Exception{
> +        this.cacheManager.setUseExpires(true);
> +        this.cacheManager.testIterationStart(null);
> +        assertNull("Should not find entry",getThreadCacheEntry(LOCAL_HOST));
> +        assertFalse("Should not find valid entry",this.cacheManager.inCache(url));
> +        ((HttpMethodStub)httpMethod).expires=makeDate(new Date(System.currentTimeMillis()));
> +        ((HttpMethodStub)httpMethod).cacheControl="private, max-age=10";
> +        this.cacheManager.saveDetails(httpMethod, sampleResultOK);
> +        assertNotNull("Should find entry",getThreadCacheEntry(LOCAL_HOST));
> +        assertTrue("Should find valid entry",this.cacheManager.inCache(url));
> +    }
>
> +    public void testNoCacheHttpClient() throws Exception{
> +        this.cacheManager.setUseExpires(true);
> +        this.cacheManager.testIterationStart(null);
> +        assertNull("Should not find entry",getThreadCacheEntry(LOCAL_HOST));
> +        assertFalse("Should not find valid entry",this.cacheManager.inCache(url));
> +        ((HttpMethodStub)httpMethod).cacheControl="no-cache";
> +        this.cacheManager.saveDetails(httpMethod, sampleResultOK);
> +        assertNotNull("Should find entry",getThreadCacheEntry(LOCAL_HOST));
> +        assertFalse("Should not find valid entry",this.cacheManager.inCache(url));
> +    }
> +
>      public void testCacheHttpClientBug51932() throws Exception{
>          this.cacheManager.setUseExpires(true);
>          this.cacheManager.testIterationStart(null);
>
> Modified: jmeter/trunk/xdocs/changes.xml
> URL: http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1358710&r1=1358709&r2=1358710&view=diff
> ==============================================================================
> --- jmeter/trunk/xdocs/changes.xml (original)
> +++ jmeter/trunk/xdocs/changes.xml Sun Jul  8 09:40:51 2012
> @@ -59,6 +59,7 @@ or a Debug Sampler with all fields set t
>
>  <h3>HTTP Samplers and Proxy</h3>
>  <ul>
> +<li><bugzilla>53521</bugzilla> - Cache Manager should cache content with Cache-control=private</li>
>  </ul>
>
>  <h3>Other Samplers</h3>
>
>

Re: svn commit: r1358710 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java xdocs/changes.xml

Posted by sebb <se...@gmail.com>.
On 8 July 2012 14:57, sebb <se...@gmail.com> wrote:
> On 8 July 2012 14:43, Philippe Mouawad <ph...@gmail.com> wrote:
>> We would have this:
>>
>>     private void setCache(String lastModified, String cacheControl, String
>> expires, String etag, String url) {
>>         if (log.isDebugEnabled()){
>>             log.debug("SET(both) "+url + " " + cacheControl + " " +
>> lastModified + " " + " " + expires + " " + etag);
>>         }
>>         Date expiresDate = null; // i.e. not using Expires
>>         if (useExpires) {// Check that we are processing
>> Expires/CacheControl
>>             final String MAX_AGE = "max-age=";
>>             if(cacheControl != null) {
>>                 if(cacheControl.contains("no-cache")) {
>>                     return;
>>                 }
>
> Surely that should be done before checking useExpires?
>
>>                 if(cacheControl.contains(MAX_AGE)) {
>
> cacheControl could be null here.

Sorry, misread conditional nesting - ignore that.

> Could fix this by changing
>
>     if(useExpires)    to     if(useExpires && cacheControl != null)
>
> We also need to change component_reference now that public is not
> required for max-age.
>
>>                     long maxAgeInSecs = Long.parseLong(
>>
>> cacheControl.substring(cacheControl.indexOf(MAX_AGE)+MAX_AGE.length())
>>                                 .split("[, ]")[0] // Bug 51932 - allow for
>> optional trailing attributes
>>                             );
>>                     expiresDate=new
>> Date(System.currentTimeMillis()+maxAgeInSecs*1000);
>>                 }
>>             } else if (expires != null) {
>>                 try {
>>                     expiresDate = DateUtil.parseDate(expires);
>>                 } catch (DateParseException e) {
>>                     if (log.isDebugEnabled()){
>>                         log.debug("Unable to parse Expires: '"+expires+"'
>> "+e);
>>                     }
>>                     expiresDate = new Date(0L); // invalid dates must be
>> treated as expired
>>                 }
>>             }
>>         }
>>         getCache().put(url, new CacheEntry(lastModified, expiresDate,
>> etag));
>>     }
>>
>> On Sun, Jul 8, 2012 at 3:36 PM, Philippe Mouawad <philippe.mouawad@gmail.com
>>> wrote:
>>
>>> Looking at existing code, I noticed that with no-cache we store an entry
>>> in Cache for which CacheManager#inCache will return false .
>>>
>>> I don't understand why we just skip the entry, currently we use on entry
>>> in map for nothing.
>>>
>>> So reading what you suggest  + this I propose we change to the following:
>>>
>>>    - We test no-cache or no-store and if true we just return
>>>    - we remove check on (cacheControl.contains("public") ||
>>>    cacheControl.contains("private"))
>>>
>>>
>>> Agree ?
>>>
>>> On Sun, Jul 8, 2012 at 3:29 PM, sebb <se...@gmail.com> wrote:
>>>
>>>> On 8 July 2012 14:24, Philippe Mouawad <ph...@gmail.com>
>>>> wrote:
>>>> > but if we have this:
>>>> > Cache-Control=no-cache, max-age=10.
>>>> >
>>>> > If we don't check we will cache which is wrong right ?
>>>> > This header is wrong but I have already seen this on tests I did.
>>>> >
>>>> > Or I am misunderstanding ?
>>>>
>>>> I don't have the source to hand at present, but we should not cache at
>>>> all if Cache-Control=no-cache; the max-age is then not relevant.
>>>>
>>>> > Regards
>>>> > Philippe
>>>> >
>>>> > On Sun, Jul 8, 2012 at 3:19 PM, sebb <se...@gmail.com> wrote:
>>>> >
>>>> >> On 8 July 2012 14:07, Philippe Mouawad <ph...@gmail.com>
>>>> wrote:
>>>> >> > Can't it also be no-cache ? no-store ?
>>>> >> > If we don't check it , we could store in cache if server returns
>>>> invalid
>>>> >> > headers no ?
>>>> >>
>>>> >> What do we do if we don't check MaxAge?
>>>> >>
>>>> >> I don't think we should be more restrictive just because we are also
>>>> >> checking the age.
>>>> >>
>>>> >> >
>>>> >> > Thansk for looking at 53365.
>>>> >> > Regards
>>>> >> > Philippe
>>>> >> >
>>>> >> > On Sun, Jul 8, 2012 at 3:01 PM, sebb <se...@gmail.com> wrote:
>>>> >> >
>>>> >> >> On 8 July 2012 10:40,  <pm...@apache.org> wrote:
>>>> >> >> > Author: pmouawad
>>>> >> >> > Date: Sun Jul  8 09:40:51 2012
>>>> >> >> > New Revision: 1358710
>>>> >> >> >
>>>> >> >> > URL: http://svn.apache.org/viewvc?rev=1358710&view=rev
>>>> >> >> > Log:
>>>> >> >> > Bug 53521 - Cache Manager should cache content with
>>>> >> Cache-control=private
>>>> >> >> > Bugzilla Id: 53521
>>>> >> >> >
>>>> >> >> > Modified:
>>>> >> >> >
>>>> >> >>
>>>> >>
>>>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
>>>> >> >> >
>>>> >> >>
>>>> >>
>>>> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
>>>> >> >> >     jmeter/trunk/xdocs/changes.xml
>>>> >> >> >
>>>> >> >> > Modified:
>>>> >> >>
>>>> >>
>>>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
>>>> >> >> > URL:
>>>> >> >>
>>>> >>
>>>> http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java?rev=1358710&r1=1358709&r2=1358710&view=diff
>>>> >> >> >
>>>> >> >>
>>>> >>
>>>> ==============================================================================
>>>> >> >> > ---
>>>> >> >>
>>>> >>
>>>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
>>>> >> >> (original)
>>>> >> >> > +++
>>>> >> >>
>>>> >>
>>>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
>>>> >> >> Sun Jul  8 09:40:51 2012
>>>> >> >> > @@ -161,7 +161,7 @@ public class CacheManager extends Config
>>>> >> >> >          if (useExpires) {// Check that we are processing
>>>> >> >> Expires/CacheControl
>>>> >> >> >              final String MAX_AGE = "max-age=";
>>>> >> >> >              // TODO - check for other CacheControl attributes?
>>>> >> >> > -            if (cacheControl != null &&
>>>> >> cacheControl.contains("public")
>>>> >> >> && cacheControl.contains(MAX_AGE)) {
>>>> >> >> > +            if (cacheControl != null &&
>>>> >> >> (cacheControl.contains("public") ||
>>>> cacheControl.contains("private")) &&
>>>> >> >> cacheControl.contains(MAX_AGE)) {
>>>> >> >>
>>>> >> >> Not sure this is the correct fix.
>>>> >> >> Do we really care if the string "public" or "private" is present so
>>>> >> >> long as there is a MAX_AGE entry?
>>>> >> >> We could just drop the check for "public" instead.
>>>> >> >>
>>>> >> >> >                  long maxAgeInSecs = Long.parseLong(
>>>> >> >> >
>>>> >> >>
>>>>  cacheControl.substring(cacheControl.indexOf(MAX_AGE)+MAX_AGE.length())
>>>> >> >> >                              .split("[, ]")[0] // Bug 51932 -
>>>> allow
>>>> >> for
>>>> >> >> optional trailing attributes
>>>> >> >> >
>>>> >> >> > Modified:
>>>> >> >>
>>>> >>
>>>> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
>>>> >> >> > URL:
>>>> >> >>
>>>> >>
>>>> http://svn.apache.org/viewvc/jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java?rev=1358710&r1=1358709&r2=1358710&view=diff
>>>> >> >> >
>>>> >> >>
>>>> >>
>>>> ==============================================================================
>>>> >> >> > ---
>>>> >> >>
>>>> >>
>>>> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
>>>> >> >> (original)
>>>> >> >> > +++
>>>> >> >>
>>>> >>
>>>> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
>>>> >> >> Sun Jul  8 09:40:51 2012
>>>> >> >> > @@ -238,7 +238,30 @@ public class TestCacheManager extends JM
>>>> >> >> >          assertNotNull("Should find
>>>> >> >> entry",getThreadCacheEntry(LOCAL_HOST));
>>>> >> >> >          assertTrue("Should find valid
>>>> >> >> entry",this.cacheManager.inCache(url));
>>>> >> >> >      }
>>>> >> >> > +
>>>> >> >> > +    public void testPrivateCacheHttpClient() throws Exception{
>>>> >> >> > +        this.cacheManager.setUseExpires(true);
>>>> >> >> > +        this.cacheManager.testIterationStart(null);
>>>> >> >> > +        assertNull("Should not find
>>>> >> >> entry",getThreadCacheEntry(LOCAL_HOST));
>>>> >> >> > +        assertFalse("Should not find valid
>>>> >> >> entry",this.cacheManager.inCache(url));
>>>> >> >> > +        ((HttpMethodStub)httpMethod).expires=makeDate(new
>>>> >> >> Date(System.currentTimeMillis()));
>>>> >> >> > +        ((HttpMethodStub)httpMethod).cacheControl="private,
>>>> >> max-age=10";
>>>> >> >> > +        this.cacheManager.saveDetails(httpMethod,
>>>> sampleResultOK);
>>>> >> >> > +        assertNotNull("Should find
>>>> >> >> entry",getThreadCacheEntry(LOCAL_HOST));
>>>> >> >> > +        assertTrue("Should find valid
>>>> >> >> entry",this.cacheManager.inCache(url));
>>>> >> >> > +    }
>>>> >> >> >
>>>> >> >> > +    public void testNoCacheHttpClient() throws Exception{
>>>> >> >> > +        this.cacheManager.setUseExpires(true);
>>>> >> >> > +        this.cacheManager.testIterationStart(null);
>>>> >> >> > +        assertNull("Should not find
>>>> >> >> entry",getThreadCacheEntry(LOCAL_HOST));
>>>> >> >> > +        assertFalse("Should not find valid
>>>> >> >> entry",this.cacheManager.inCache(url));
>>>> >> >> > +        ((HttpMethodStub)httpMethod).cacheControl="no-cache";
>>>> >> >> > +        this.cacheManager.saveDetails(httpMethod,
>>>> sampleResultOK);
>>>> >> >> > +        assertNotNull("Should find
>>>> >> >> entry",getThreadCacheEntry(LOCAL_HOST));
>>>> >> >> > +        assertFalse("Should not find valid
>>>> >> >> entry",this.cacheManager.inCache(url));
>>>> >> >> > +    }
>>>> >> >> > +
>>>> >> >> >      public void testCacheHttpClientBug51932() throws Exception{
>>>> >> >> >          this.cacheManager.setUseExpires(true);
>>>> >> >> >          this.cacheManager.testIterationStart(null);
>>>> >> >> >
>>>> >> >> > Modified: jmeter/trunk/xdocs/changes.xml
>>>> >> >> > URL:
>>>> >> >>
>>>> >>
>>>> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1358710&r1=1358709&r2=1358710&view=diff
>>>> >> >> >
>>>> >> >>
>>>> >>
>>>> ==============================================================================
>>>> >> >> > --- jmeter/trunk/xdocs/changes.xml (original)
>>>> >> >> > +++ jmeter/trunk/xdocs/changes.xml Sun Jul  8 09:40:51 2012
>>>> >> >> > @@ -59,6 +59,7 @@ or a Debug Sampler with all fields set t
>>>> >> >> >
>>>> >> >> >  <h3>HTTP Samplers and Proxy</h3>
>>>> >> >> >  <ul>
>>>> >> >> > +<li><bugzilla>53521</bugzilla> - Cache Manager should cache
>>>> content
>>>> >> >> with Cache-control=private</li>
>>>> >> >> >  </ul>
>>>> >> >> >
>>>> >> >> >  <h3>Other Samplers</h3>
>>>> >> >> >
>>>> >> >> >
>>>> >> >>
>>>> >> >
>>>> >> >
>>>> >> >
>>>> >> > --
>>>> >> > Cordialement.
>>>> >> > Philippe Mouawad.
>>>> >>
>>>> >
>>>> >
>>>> >
>>>> > --
>>>> > Cordialement.
>>>> > Philippe Mouawad.
>>>>
>>>
>>>
>>>
>>> --
>>> Cordialement.
>>> Philippe Mouawad.
>>>
>>>
>>>
>>>
>>
>>
>> --
>> Cordialement.
>> Philippe Mouawad.

Re: svn commit: r1358710 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java xdocs/changes.xml

Posted by sebb <se...@gmail.com>.
On 8 July 2012 14:43, Philippe Mouawad <ph...@gmail.com> wrote:
> We would have this:
>
>     private void setCache(String lastModified, String cacheControl, String
> expires, String etag, String url) {
>         if (log.isDebugEnabled()){
>             log.debug("SET(both) "+url + " " + cacheControl + " " +
> lastModified + " " + " " + expires + " " + etag);
>         }
>         Date expiresDate = null; // i.e. not using Expires
>         if (useExpires) {// Check that we are processing
> Expires/CacheControl
>             final String MAX_AGE = "max-age=";
>             if(cacheControl != null) {
>                 if(cacheControl.contains("no-cache")) {
>                     return;
>                 }

Surely that should be done before checking useExpires?

>                 if(cacheControl.contains(MAX_AGE)) {

cacheControl could be null here.

Could fix this by changing

    if(useExpires)    to     if(useExpires && cacheControl != null)

We also need to change component_reference now that public is not
required for max-age.

>                     long maxAgeInSecs = Long.parseLong(
>
> cacheControl.substring(cacheControl.indexOf(MAX_AGE)+MAX_AGE.length())
>                                 .split("[, ]")[0] // Bug 51932 - allow for
> optional trailing attributes
>                             );
>                     expiresDate=new
> Date(System.currentTimeMillis()+maxAgeInSecs*1000);
>                 }
>             } else if (expires != null) {
>                 try {
>                     expiresDate = DateUtil.parseDate(expires);
>                 } catch (DateParseException e) {
>                     if (log.isDebugEnabled()){
>                         log.debug("Unable to parse Expires: '"+expires+"'
> "+e);
>                     }
>                     expiresDate = new Date(0L); // invalid dates must be
> treated as expired
>                 }
>             }
>         }
>         getCache().put(url, new CacheEntry(lastModified, expiresDate,
> etag));
>     }
>
> On Sun, Jul 8, 2012 at 3:36 PM, Philippe Mouawad <philippe.mouawad@gmail.com
>> wrote:
>
>> Looking at existing code, I noticed that with no-cache we store an entry
>> in Cache for which CacheManager#inCache will return false .
>>
>> I don't understand why we just skip the entry, currently we use on entry
>> in map for nothing.
>>
>> So reading what you suggest  + this I propose we change to the following:
>>
>>    - We test no-cache or no-store and if true we just return
>>    - we remove check on (cacheControl.contains("public") ||
>>    cacheControl.contains("private"))
>>
>>
>> Agree ?
>>
>> On Sun, Jul 8, 2012 at 3:29 PM, sebb <se...@gmail.com> wrote:
>>
>>> On 8 July 2012 14:24, Philippe Mouawad <ph...@gmail.com>
>>> wrote:
>>> > but if we have this:
>>> > Cache-Control=no-cache, max-age=10.
>>> >
>>> > If we don't check we will cache which is wrong right ?
>>> > This header is wrong but I have already seen this on tests I did.
>>> >
>>> > Or I am misunderstanding ?
>>>
>>> I don't have the source to hand at present, but we should not cache at
>>> all if Cache-Control=no-cache; the max-age is then not relevant.
>>>
>>> > Regards
>>> > Philippe
>>> >
>>> > On Sun, Jul 8, 2012 at 3:19 PM, sebb <se...@gmail.com> wrote:
>>> >
>>> >> On 8 July 2012 14:07, Philippe Mouawad <ph...@gmail.com>
>>> wrote:
>>> >> > Can't it also be no-cache ? no-store ?
>>> >> > If we don't check it , we could store in cache if server returns
>>> invalid
>>> >> > headers no ?
>>> >>
>>> >> What do we do if we don't check MaxAge?
>>> >>
>>> >> I don't think we should be more restrictive just because we are also
>>> >> checking the age.
>>> >>
>>> >> >
>>> >> > Thansk for looking at 53365.
>>> >> > Regards
>>> >> > Philippe
>>> >> >
>>> >> > On Sun, Jul 8, 2012 at 3:01 PM, sebb <se...@gmail.com> wrote:
>>> >> >
>>> >> >> On 8 July 2012 10:40,  <pm...@apache.org> wrote:
>>> >> >> > Author: pmouawad
>>> >> >> > Date: Sun Jul  8 09:40:51 2012
>>> >> >> > New Revision: 1358710
>>> >> >> >
>>> >> >> > URL: http://svn.apache.org/viewvc?rev=1358710&view=rev
>>> >> >> > Log:
>>> >> >> > Bug 53521 - Cache Manager should cache content with
>>> >> Cache-control=private
>>> >> >> > Bugzilla Id: 53521
>>> >> >> >
>>> >> >> > Modified:
>>> >> >> >
>>> >> >>
>>> >>
>>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
>>> >> >> >
>>> >> >>
>>> >>
>>> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
>>> >> >> >     jmeter/trunk/xdocs/changes.xml
>>> >> >> >
>>> >> >> > Modified:
>>> >> >>
>>> >>
>>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
>>> >> >> > URL:
>>> >> >>
>>> >>
>>> http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java?rev=1358710&r1=1358709&r2=1358710&view=diff
>>> >> >> >
>>> >> >>
>>> >>
>>> ==============================================================================
>>> >> >> > ---
>>> >> >>
>>> >>
>>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
>>> >> >> (original)
>>> >> >> > +++
>>> >> >>
>>> >>
>>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
>>> >> >> Sun Jul  8 09:40:51 2012
>>> >> >> > @@ -161,7 +161,7 @@ public class CacheManager extends Config
>>> >> >> >          if (useExpires) {// Check that we are processing
>>> >> >> Expires/CacheControl
>>> >> >> >              final String MAX_AGE = "max-age=";
>>> >> >> >              // TODO - check for other CacheControl attributes?
>>> >> >> > -            if (cacheControl != null &&
>>> >> cacheControl.contains("public")
>>> >> >> && cacheControl.contains(MAX_AGE)) {
>>> >> >> > +            if (cacheControl != null &&
>>> >> >> (cacheControl.contains("public") ||
>>> cacheControl.contains("private")) &&
>>> >> >> cacheControl.contains(MAX_AGE)) {
>>> >> >>
>>> >> >> Not sure this is the correct fix.
>>> >> >> Do we really care if the string "public" or "private" is present so
>>> >> >> long as there is a MAX_AGE entry?
>>> >> >> We could just drop the check for "public" instead.
>>> >> >>
>>> >> >> >                  long maxAgeInSecs = Long.parseLong(
>>> >> >> >
>>> >> >>
>>>  cacheControl.substring(cacheControl.indexOf(MAX_AGE)+MAX_AGE.length())
>>> >> >> >                              .split("[, ]")[0] // Bug 51932 -
>>> allow
>>> >> for
>>> >> >> optional trailing attributes
>>> >> >> >
>>> >> >> > Modified:
>>> >> >>
>>> >>
>>> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
>>> >> >> > URL:
>>> >> >>
>>> >>
>>> http://svn.apache.org/viewvc/jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java?rev=1358710&r1=1358709&r2=1358710&view=diff
>>> >> >> >
>>> >> >>
>>> >>
>>> ==============================================================================
>>> >> >> > ---
>>> >> >>
>>> >>
>>> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
>>> >> >> (original)
>>> >> >> > +++
>>> >> >>
>>> >>
>>> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
>>> >> >> Sun Jul  8 09:40:51 2012
>>> >> >> > @@ -238,7 +238,30 @@ public class TestCacheManager extends JM
>>> >> >> >          assertNotNull("Should find
>>> >> >> entry",getThreadCacheEntry(LOCAL_HOST));
>>> >> >> >          assertTrue("Should find valid
>>> >> >> entry",this.cacheManager.inCache(url));
>>> >> >> >      }
>>> >> >> > +
>>> >> >> > +    public void testPrivateCacheHttpClient() throws Exception{
>>> >> >> > +        this.cacheManager.setUseExpires(true);
>>> >> >> > +        this.cacheManager.testIterationStart(null);
>>> >> >> > +        assertNull("Should not find
>>> >> >> entry",getThreadCacheEntry(LOCAL_HOST));
>>> >> >> > +        assertFalse("Should not find valid
>>> >> >> entry",this.cacheManager.inCache(url));
>>> >> >> > +        ((HttpMethodStub)httpMethod).expires=makeDate(new
>>> >> >> Date(System.currentTimeMillis()));
>>> >> >> > +        ((HttpMethodStub)httpMethod).cacheControl="private,
>>> >> max-age=10";
>>> >> >> > +        this.cacheManager.saveDetails(httpMethod,
>>> sampleResultOK);
>>> >> >> > +        assertNotNull("Should find
>>> >> >> entry",getThreadCacheEntry(LOCAL_HOST));
>>> >> >> > +        assertTrue("Should find valid
>>> >> >> entry",this.cacheManager.inCache(url));
>>> >> >> > +    }
>>> >> >> >
>>> >> >> > +    public void testNoCacheHttpClient() throws Exception{
>>> >> >> > +        this.cacheManager.setUseExpires(true);
>>> >> >> > +        this.cacheManager.testIterationStart(null);
>>> >> >> > +        assertNull("Should not find
>>> >> >> entry",getThreadCacheEntry(LOCAL_HOST));
>>> >> >> > +        assertFalse("Should not find valid
>>> >> >> entry",this.cacheManager.inCache(url));
>>> >> >> > +        ((HttpMethodStub)httpMethod).cacheControl="no-cache";
>>> >> >> > +        this.cacheManager.saveDetails(httpMethod,
>>> sampleResultOK);
>>> >> >> > +        assertNotNull("Should find
>>> >> >> entry",getThreadCacheEntry(LOCAL_HOST));
>>> >> >> > +        assertFalse("Should not find valid
>>> >> >> entry",this.cacheManager.inCache(url));
>>> >> >> > +    }
>>> >> >> > +
>>> >> >> >      public void testCacheHttpClientBug51932() throws Exception{
>>> >> >> >          this.cacheManager.setUseExpires(true);
>>> >> >> >          this.cacheManager.testIterationStart(null);
>>> >> >> >
>>> >> >> > Modified: jmeter/trunk/xdocs/changes.xml
>>> >> >> > URL:
>>> >> >>
>>> >>
>>> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1358710&r1=1358709&r2=1358710&view=diff
>>> >> >> >
>>> >> >>
>>> >>
>>> ==============================================================================
>>> >> >> > --- jmeter/trunk/xdocs/changes.xml (original)
>>> >> >> > +++ jmeter/trunk/xdocs/changes.xml Sun Jul  8 09:40:51 2012
>>> >> >> > @@ -59,6 +59,7 @@ or a Debug Sampler with all fields set t
>>> >> >> >
>>> >> >> >  <h3>HTTP Samplers and Proxy</h3>
>>> >> >> >  <ul>
>>> >> >> > +<li><bugzilla>53521</bugzilla> - Cache Manager should cache
>>> content
>>> >> >> with Cache-control=private</li>
>>> >> >> >  </ul>
>>> >> >> >
>>> >> >> >  <h3>Other Samplers</h3>
>>> >> >> >
>>> >> >> >
>>> >> >>
>>> >> >
>>> >> >
>>> >> >
>>> >> > --
>>> >> > Cordialement.
>>> >> > Philippe Mouawad.
>>> >>
>>> >
>>> >
>>> >
>>> > --
>>> > Cordialement.
>>> > Philippe Mouawad.
>>>
>>
>>
>>
>> --
>> Cordialement.
>> Philippe Mouawad.
>>
>>
>>
>>
>
>
> --
> Cordialement.
> Philippe Mouawad.

Re: svn commit: r1358710 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java xdocs/changes.xml

Posted by Philippe Mouawad <ph...@gmail.com>.
We would have this:

    private void setCache(String lastModified, String cacheControl, String
expires, String etag, String url) {
        if (log.isDebugEnabled()){
            log.debug("SET(both) "+url + " " + cacheControl + " " +
lastModified + " " + " " + expires + " " + etag);
        }
        Date expiresDate = null; // i.e. not using Expires
        if (useExpires) {// Check that we are processing
Expires/CacheControl
            final String MAX_AGE = "max-age=";
            if(cacheControl != null) {
                if(cacheControl.contains("no-cache")) {
                    return;
                }
                if(cacheControl.contains(MAX_AGE)) {
                    long maxAgeInSecs = Long.parseLong(

cacheControl.substring(cacheControl.indexOf(MAX_AGE)+MAX_AGE.length())
                                .split("[, ]")[0] // Bug 51932 - allow for
optional trailing attributes
                            );
                    expiresDate=new
Date(System.currentTimeMillis()+maxAgeInSecs*1000);
                }
            } else if (expires != null) {
                try {
                    expiresDate = DateUtil.parseDate(expires);
                } catch (DateParseException e) {
                    if (log.isDebugEnabled()){
                        log.debug("Unable to parse Expires: '"+expires+"'
"+e);
                    }
                    expiresDate = new Date(0L); // invalid dates must be
treated as expired
                }
            }
        }
        getCache().put(url, new CacheEntry(lastModified, expiresDate,
etag));
    }

On Sun, Jul 8, 2012 at 3:36 PM, Philippe Mouawad <philippe.mouawad@gmail.com
> wrote:

> Looking at existing code, I noticed that with no-cache we store an entry
> in Cache for which CacheManager#inCache will return false .
>
> I don't understand why we just skip the entry, currently we use on entry
> in map for nothing.
>
> So reading what you suggest  + this I propose we change to the following:
>
>    - We test no-cache or no-store and if true we just return
>    - we remove check on (cacheControl.contains("public") ||
>    cacheControl.contains("private"))
>
>
> Agree ?
>
> On Sun, Jul 8, 2012 at 3:29 PM, sebb <se...@gmail.com> wrote:
>
>> On 8 July 2012 14:24, Philippe Mouawad <ph...@gmail.com>
>> wrote:
>> > but if we have this:
>> > Cache-Control=no-cache, max-age=10.
>> >
>> > If we don't check we will cache which is wrong right ?
>> > This header is wrong but I have already seen this on tests I did.
>> >
>> > Or I am misunderstanding ?
>>
>> I don't have the source to hand at present, but we should not cache at
>> all if Cache-Control=no-cache; the max-age is then not relevant.
>>
>> > Regards
>> > Philippe
>> >
>> > On Sun, Jul 8, 2012 at 3:19 PM, sebb <se...@gmail.com> wrote:
>> >
>> >> On 8 July 2012 14:07, Philippe Mouawad <ph...@gmail.com>
>> wrote:
>> >> > Can't it also be no-cache ? no-store ?
>> >> > If we don't check it , we could store in cache if server returns
>> invalid
>> >> > headers no ?
>> >>
>> >> What do we do if we don't check MaxAge?
>> >>
>> >> I don't think we should be more restrictive just because we are also
>> >> checking the age.
>> >>
>> >> >
>> >> > Thansk for looking at 53365.
>> >> > Regards
>> >> > Philippe
>> >> >
>> >> > On Sun, Jul 8, 2012 at 3:01 PM, sebb <se...@gmail.com> wrote:
>> >> >
>> >> >> On 8 July 2012 10:40,  <pm...@apache.org> wrote:
>> >> >> > Author: pmouawad
>> >> >> > Date: Sun Jul  8 09:40:51 2012
>> >> >> > New Revision: 1358710
>> >> >> >
>> >> >> > URL: http://svn.apache.org/viewvc?rev=1358710&view=rev
>> >> >> > Log:
>> >> >> > Bug 53521 - Cache Manager should cache content with
>> >> Cache-control=private
>> >> >> > Bugzilla Id: 53521
>> >> >> >
>> >> >> > Modified:
>> >> >> >
>> >> >>
>> >>
>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
>> >> >> >
>> >> >>
>> >>
>> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
>> >> >> >     jmeter/trunk/xdocs/changes.xml
>> >> >> >
>> >> >> > Modified:
>> >> >>
>> >>
>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
>> >> >> > URL:
>> >> >>
>> >>
>> http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java?rev=1358710&r1=1358709&r2=1358710&view=diff
>> >> >> >
>> >> >>
>> >>
>> ==============================================================================
>> >> >> > ---
>> >> >>
>> >>
>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
>> >> >> (original)
>> >> >> > +++
>> >> >>
>> >>
>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
>> >> >> Sun Jul  8 09:40:51 2012
>> >> >> > @@ -161,7 +161,7 @@ public class CacheManager extends Config
>> >> >> >          if (useExpires) {// Check that we are processing
>> >> >> Expires/CacheControl
>> >> >> >              final String MAX_AGE = "max-age=";
>> >> >> >              // TODO - check for other CacheControl attributes?
>> >> >> > -            if (cacheControl != null &&
>> >> cacheControl.contains("public")
>> >> >> && cacheControl.contains(MAX_AGE)) {
>> >> >> > +            if (cacheControl != null &&
>> >> >> (cacheControl.contains("public") ||
>> cacheControl.contains("private")) &&
>> >> >> cacheControl.contains(MAX_AGE)) {
>> >> >>
>> >> >> Not sure this is the correct fix.
>> >> >> Do we really care if the string "public" or "private" is present so
>> >> >> long as there is a MAX_AGE entry?
>> >> >> We could just drop the check for "public" instead.
>> >> >>
>> >> >> >                  long maxAgeInSecs = Long.parseLong(
>> >> >> >
>> >> >>
>>  cacheControl.substring(cacheControl.indexOf(MAX_AGE)+MAX_AGE.length())
>> >> >> >                              .split("[, ]")[0] // Bug 51932 -
>> allow
>> >> for
>> >> >> optional trailing attributes
>> >> >> >
>> >> >> > Modified:
>> >> >>
>> >>
>> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
>> >> >> > URL:
>> >> >>
>> >>
>> http://svn.apache.org/viewvc/jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java?rev=1358710&r1=1358709&r2=1358710&view=diff
>> >> >> >
>> >> >>
>> >>
>> ==============================================================================
>> >> >> > ---
>> >> >>
>> >>
>> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
>> >> >> (original)
>> >> >> > +++
>> >> >>
>> >>
>> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
>> >> >> Sun Jul  8 09:40:51 2012
>> >> >> > @@ -238,7 +238,30 @@ public class TestCacheManager extends JM
>> >> >> >          assertNotNull("Should find
>> >> >> entry",getThreadCacheEntry(LOCAL_HOST));
>> >> >> >          assertTrue("Should find valid
>> >> >> entry",this.cacheManager.inCache(url));
>> >> >> >      }
>> >> >> > +
>> >> >> > +    public void testPrivateCacheHttpClient() throws Exception{
>> >> >> > +        this.cacheManager.setUseExpires(true);
>> >> >> > +        this.cacheManager.testIterationStart(null);
>> >> >> > +        assertNull("Should not find
>> >> >> entry",getThreadCacheEntry(LOCAL_HOST));
>> >> >> > +        assertFalse("Should not find valid
>> >> >> entry",this.cacheManager.inCache(url));
>> >> >> > +        ((HttpMethodStub)httpMethod).expires=makeDate(new
>> >> >> Date(System.currentTimeMillis()));
>> >> >> > +        ((HttpMethodStub)httpMethod).cacheControl="private,
>> >> max-age=10";
>> >> >> > +        this.cacheManager.saveDetails(httpMethod,
>> sampleResultOK);
>> >> >> > +        assertNotNull("Should find
>> >> >> entry",getThreadCacheEntry(LOCAL_HOST));
>> >> >> > +        assertTrue("Should find valid
>> >> >> entry",this.cacheManager.inCache(url));
>> >> >> > +    }
>> >> >> >
>> >> >> > +    public void testNoCacheHttpClient() throws Exception{
>> >> >> > +        this.cacheManager.setUseExpires(true);
>> >> >> > +        this.cacheManager.testIterationStart(null);
>> >> >> > +        assertNull("Should not find
>> >> >> entry",getThreadCacheEntry(LOCAL_HOST));
>> >> >> > +        assertFalse("Should not find valid
>> >> >> entry",this.cacheManager.inCache(url));
>> >> >> > +        ((HttpMethodStub)httpMethod).cacheControl="no-cache";
>> >> >> > +        this.cacheManager.saveDetails(httpMethod,
>> sampleResultOK);
>> >> >> > +        assertNotNull("Should find
>> >> >> entry",getThreadCacheEntry(LOCAL_HOST));
>> >> >> > +        assertFalse("Should not find valid
>> >> >> entry",this.cacheManager.inCache(url));
>> >> >> > +    }
>> >> >> > +
>> >> >> >      public void testCacheHttpClientBug51932() throws Exception{
>> >> >> >          this.cacheManager.setUseExpires(true);
>> >> >> >          this.cacheManager.testIterationStart(null);
>> >> >> >
>> >> >> > Modified: jmeter/trunk/xdocs/changes.xml
>> >> >> > URL:
>> >> >>
>> >>
>> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1358710&r1=1358709&r2=1358710&view=diff
>> >> >> >
>> >> >>
>> >>
>> ==============================================================================
>> >> >> > --- jmeter/trunk/xdocs/changes.xml (original)
>> >> >> > +++ jmeter/trunk/xdocs/changes.xml Sun Jul  8 09:40:51 2012
>> >> >> > @@ -59,6 +59,7 @@ or a Debug Sampler with all fields set t
>> >> >> >
>> >> >> >  <h3>HTTP Samplers and Proxy</h3>
>> >> >> >  <ul>
>> >> >> > +<li><bugzilla>53521</bugzilla> - Cache Manager should cache
>> content
>> >> >> with Cache-control=private</li>
>> >> >> >  </ul>
>> >> >> >
>> >> >> >  <h3>Other Samplers</h3>
>> >> >> >
>> >> >> >
>> >> >>
>> >> >
>> >> >
>> >> >
>> >> > --
>> >> > Cordialement.
>> >> > Philippe Mouawad.
>> >>
>> >
>> >
>> >
>> > --
>> > Cordialement.
>> > Philippe Mouawad.
>>
>
>
>
> --
> Cordialement.
> Philippe Mouawad.
>
>
>
>


-- 
Cordialement.
Philippe Mouawad.

Re: svn commit: r1358710 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java xdocs/changes.xml

Posted by Philippe Mouawad <ph...@gmail.com>.
Looking at existing code, I noticed that with no-cache we store an entry in
Cache for which CacheManager#inCache will return false .

I don't understand why we just skip the entry, currently we use on entry in
map for nothing.

So reading what you suggest  + this I propose we change to the following:

   - We test no-cache or no-store and if true we just return
   - we remove check on (cacheControl.contains("public") ||
   cacheControl.contains("private"))


Agree ?

On Sun, Jul 8, 2012 at 3:29 PM, sebb <se...@gmail.com> wrote:

> On 8 July 2012 14:24, Philippe Mouawad <ph...@gmail.com> wrote:
> > but if we have this:
> > Cache-Control=no-cache, max-age=10.
> >
> > If we don't check we will cache which is wrong right ?
> > This header is wrong but I have already seen this on tests I did.
> >
> > Or I am misunderstanding ?
>
> I don't have the source to hand at present, but we should not cache at
> all if Cache-Control=no-cache; the max-age is then not relevant.
>
> > Regards
> > Philippe
> >
> > On Sun, Jul 8, 2012 at 3:19 PM, sebb <se...@gmail.com> wrote:
> >
> >> On 8 July 2012 14:07, Philippe Mouawad <ph...@gmail.com>
> wrote:
> >> > Can't it also be no-cache ? no-store ?
> >> > If we don't check it , we could store in cache if server returns
> invalid
> >> > headers no ?
> >>
> >> What do we do if we don't check MaxAge?
> >>
> >> I don't think we should be more restrictive just because we are also
> >> checking the age.
> >>
> >> >
> >> > Thansk for looking at 53365.
> >> > Regards
> >> > Philippe
> >> >
> >> > On Sun, Jul 8, 2012 at 3:01 PM, sebb <se...@gmail.com> wrote:
> >> >
> >> >> On 8 July 2012 10:40,  <pm...@apache.org> wrote:
> >> >> > Author: pmouawad
> >> >> > Date: Sun Jul  8 09:40:51 2012
> >> >> > New Revision: 1358710
> >> >> >
> >> >> > URL: http://svn.apache.org/viewvc?rev=1358710&view=rev
> >> >> > Log:
> >> >> > Bug 53521 - Cache Manager should cache content with
> >> Cache-control=private
> >> >> > Bugzilla Id: 53521
> >> >> >
> >> >> > Modified:
> >> >> >
> >> >>
> >>
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
> >> >> >
> >> >>
> >>
> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
> >> >> >     jmeter/trunk/xdocs/changes.xml
> >> >> >
> >> >> > Modified:
> >> >>
> >>
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
> >> >> > URL:
> >> >>
> >>
> http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java?rev=1358710&r1=1358709&r2=1358710&view=diff
> >> >> >
> >> >>
> >>
> ==============================================================================
> >> >> > ---
> >> >>
> >>
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
> >> >> (original)
> >> >> > +++
> >> >>
> >>
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
> >> >> Sun Jul  8 09:40:51 2012
> >> >> > @@ -161,7 +161,7 @@ public class CacheManager extends Config
> >> >> >          if (useExpires) {// Check that we are processing
> >> >> Expires/CacheControl
> >> >> >              final String MAX_AGE = "max-age=";
> >> >> >              // TODO - check for other CacheControl attributes?
> >> >> > -            if (cacheControl != null &&
> >> cacheControl.contains("public")
> >> >> && cacheControl.contains(MAX_AGE)) {
> >> >> > +            if (cacheControl != null &&
> >> >> (cacheControl.contains("public") ||
> cacheControl.contains("private")) &&
> >> >> cacheControl.contains(MAX_AGE)) {
> >> >>
> >> >> Not sure this is the correct fix.
> >> >> Do we really care if the string "public" or "private" is present so
> >> >> long as there is a MAX_AGE entry?
> >> >> We could just drop the check for "public" instead.
> >> >>
> >> >> >                  long maxAgeInSecs = Long.parseLong(
> >> >> >
> >> >>
>  cacheControl.substring(cacheControl.indexOf(MAX_AGE)+MAX_AGE.length())
> >> >> >                              .split("[, ]")[0] // Bug 51932 - allow
> >> for
> >> >> optional trailing attributes
> >> >> >
> >> >> > Modified:
> >> >>
> >>
> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
> >> >> > URL:
> >> >>
> >>
> http://svn.apache.org/viewvc/jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java?rev=1358710&r1=1358709&r2=1358710&view=diff
> >> >> >
> >> >>
> >>
> ==============================================================================
> >> >> > ---
> >> >>
> >>
> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
> >> >> (original)
> >> >> > +++
> >> >>
> >>
> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
> >> >> Sun Jul  8 09:40:51 2012
> >> >> > @@ -238,7 +238,30 @@ public class TestCacheManager extends JM
> >> >> >          assertNotNull("Should find
> >> >> entry",getThreadCacheEntry(LOCAL_HOST));
> >> >> >          assertTrue("Should find valid
> >> >> entry",this.cacheManager.inCache(url));
> >> >> >      }
> >> >> > +
> >> >> > +    public void testPrivateCacheHttpClient() throws Exception{
> >> >> > +        this.cacheManager.setUseExpires(true);
> >> >> > +        this.cacheManager.testIterationStart(null);
> >> >> > +        assertNull("Should not find
> >> >> entry",getThreadCacheEntry(LOCAL_HOST));
> >> >> > +        assertFalse("Should not find valid
> >> >> entry",this.cacheManager.inCache(url));
> >> >> > +        ((HttpMethodStub)httpMethod).expires=makeDate(new
> >> >> Date(System.currentTimeMillis()));
> >> >> > +        ((HttpMethodStub)httpMethod).cacheControl="private,
> >> max-age=10";
> >> >> > +        this.cacheManager.saveDetails(httpMethod, sampleResultOK);
> >> >> > +        assertNotNull("Should find
> >> >> entry",getThreadCacheEntry(LOCAL_HOST));
> >> >> > +        assertTrue("Should find valid
> >> >> entry",this.cacheManager.inCache(url));
> >> >> > +    }
> >> >> >
> >> >> > +    public void testNoCacheHttpClient() throws Exception{
> >> >> > +        this.cacheManager.setUseExpires(true);
> >> >> > +        this.cacheManager.testIterationStart(null);
> >> >> > +        assertNull("Should not find
> >> >> entry",getThreadCacheEntry(LOCAL_HOST));
> >> >> > +        assertFalse("Should not find valid
> >> >> entry",this.cacheManager.inCache(url));
> >> >> > +        ((HttpMethodStub)httpMethod).cacheControl="no-cache";
> >> >> > +        this.cacheManager.saveDetails(httpMethod, sampleResultOK);
> >> >> > +        assertNotNull("Should find
> >> >> entry",getThreadCacheEntry(LOCAL_HOST));
> >> >> > +        assertFalse("Should not find valid
> >> >> entry",this.cacheManager.inCache(url));
> >> >> > +    }
> >> >> > +
> >> >> >      public void testCacheHttpClientBug51932() throws Exception{
> >> >> >          this.cacheManager.setUseExpires(true);
> >> >> >          this.cacheManager.testIterationStart(null);
> >> >> >
> >> >> > Modified: jmeter/trunk/xdocs/changes.xml
> >> >> > URL:
> >> >>
> >>
> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1358710&r1=1358709&r2=1358710&view=diff
> >> >> >
> >> >>
> >>
> ==============================================================================
> >> >> > --- jmeter/trunk/xdocs/changes.xml (original)
> >> >> > +++ jmeter/trunk/xdocs/changes.xml Sun Jul  8 09:40:51 2012
> >> >> > @@ -59,6 +59,7 @@ or a Debug Sampler with all fields set t
> >> >> >
> >> >> >  <h3>HTTP Samplers and Proxy</h3>
> >> >> >  <ul>
> >> >> > +<li><bugzilla>53521</bugzilla> - Cache Manager should cache
> content
> >> >> with Cache-control=private</li>
> >> >> >  </ul>
> >> >> >
> >> >> >  <h3>Other Samplers</h3>
> >> >> >
> >> >> >
> >> >>
> >> >
> >> >
> >> >
> >> > --
> >> > Cordialement.
> >> > Philippe Mouawad.
> >>
> >
> >
> >
> > --
> > Cordialement.
> > Philippe Mouawad.
>



-- 
Cordialement.
Philippe Mouawad.

Re: svn commit: r1358710 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java xdocs/changes.xml

Posted by sebb <se...@gmail.com>.
On 8 July 2012 14:24, Philippe Mouawad <ph...@gmail.com> wrote:
> but if we have this:
> Cache-Control=no-cache, max-age=10.
>
> If we don't check we will cache which is wrong right ?
> This header is wrong but I have already seen this on tests I did.
>
> Or I am misunderstanding ?

I don't have the source to hand at present, but we should not cache at
all if Cache-Control=no-cache; the max-age is then not relevant.

> Regards
> Philippe
>
> On Sun, Jul 8, 2012 at 3:19 PM, sebb <se...@gmail.com> wrote:
>
>> On 8 July 2012 14:07, Philippe Mouawad <ph...@gmail.com> wrote:
>> > Can't it also be no-cache ? no-store ?
>> > If we don't check it , we could store in cache if server returns invalid
>> > headers no ?
>>
>> What do we do if we don't check MaxAge?
>>
>> I don't think we should be more restrictive just because we are also
>> checking the age.
>>
>> >
>> > Thansk for looking at 53365.
>> > Regards
>> > Philippe
>> >
>> > On Sun, Jul 8, 2012 at 3:01 PM, sebb <se...@gmail.com> wrote:
>> >
>> >> On 8 July 2012 10:40,  <pm...@apache.org> wrote:
>> >> > Author: pmouawad
>> >> > Date: Sun Jul  8 09:40:51 2012
>> >> > New Revision: 1358710
>> >> >
>> >> > URL: http://svn.apache.org/viewvc?rev=1358710&view=rev
>> >> > Log:
>> >> > Bug 53521 - Cache Manager should cache content with
>> Cache-control=private
>> >> > Bugzilla Id: 53521
>> >> >
>> >> > Modified:
>> >> >
>> >>
>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
>> >> >
>> >>
>> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
>> >> >     jmeter/trunk/xdocs/changes.xml
>> >> >
>> >> > Modified:
>> >>
>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
>> >> > URL:
>> >>
>> http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java?rev=1358710&r1=1358709&r2=1358710&view=diff
>> >> >
>> >>
>> ==============================================================================
>> >> > ---
>> >>
>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
>> >> (original)
>> >> > +++
>> >>
>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
>> >> Sun Jul  8 09:40:51 2012
>> >> > @@ -161,7 +161,7 @@ public class CacheManager extends Config
>> >> >          if (useExpires) {// Check that we are processing
>> >> Expires/CacheControl
>> >> >              final String MAX_AGE = "max-age=";
>> >> >              // TODO - check for other CacheControl attributes?
>> >> > -            if (cacheControl != null &&
>> cacheControl.contains("public")
>> >> && cacheControl.contains(MAX_AGE)) {
>> >> > +            if (cacheControl != null &&
>> >> (cacheControl.contains("public") || cacheControl.contains("private")) &&
>> >> cacheControl.contains(MAX_AGE)) {
>> >>
>> >> Not sure this is the correct fix.
>> >> Do we really care if the string "public" or "private" is present so
>> >> long as there is a MAX_AGE entry?
>> >> We could just drop the check for "public" instead.
>> >>
>> >> >                  long maxAgeInSecs = Long.parseLong(
>> >> >
>> >>  cacheControl.substring(cacheControl.indexOf(MAX_AGE)+MAX_AGE.length())
>> >> >                              .split("[, ]")[0] // Bug 51932 - allow
>> for
>> >> optional trailing attributes
>> >> >
>> >> > Modified:
>> >>
>> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
>> >> > URL:
>> >>
>> http://svn.apache.org/viewvc/jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java?rev=1358710&r1=1358709&r2=1358710&view=diff
>> >> >
>> >>
>> ==============================================================================
>> >> > ---
>> >>
>> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
>> >> (original)
>> >> > +++
>> >>
>> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
>> >> Sun Jul  8 09:40:51 2012
>> >> > @@ -238,7 +238,30 @@ public class TestCacheManager extends JM
>> >> >          assertNotNull("Should find
>> >> entry",getThreadCacheEntry(LOCAL_HOST));
>> >> >          assertTrue("Should find valid
>> >> entry",this.cacheManager.inCache(url));
>> >> >      }
>> >> > +
>> >> > +    public void testPrivateCacheHttpClient() throws Exception{
>> >> > +        this.cacheManager.setUseExpires(true);
>> >> > +        this.cacheManager.testIterationStart(null);
>> >> > +        assertNull("Should not find
>> >> entry",getThreadCacheEntry(LOCAL_HOST));
>> >> > +        assertFalse("Should not find valid
>> >> entry",this.cacheManager.inCache(url));
>> >> > +        ((HttpMethodStub)httpMethod).expires=makeDate(new
>> >> Date(System.currentTimeMillis()));
>> >> > +        ((HttpMethodStub)httpMethod).cacheControl="private,
>> max-age=10";
>> >> > +        this.cacheManager.saveDetails(httpMethod, sampleResultOK);
>> >> > +        assertNotNull("Should find
>> >> entry",getThreadCacheEntry(LOCAL_HOST));
>> >> > +        assertTrue("Should find valid
>> >> entry",this.cacheManager.inCache(url));
>> >> > +    }
>> >> >
>> >> > +    public void testNoCacheHttpClient() throws Exception{
>> >> > +        this.cacheManager.setUseExpires(true);
>> >> > +        this.cacheManager.testIterationStart(null);
>> >> > +        assertNull("Should not find
>> >> entry",getThreadCacheEntry(LOCAL_HOST));
>> >> > +        assertFalse("Should not find valid
>> >> entry",this.cacheManager.inCache(url));
>> >> > +        ((HttpMethodStub)httpMethod).cacheControl="no-cache";
>> >> > +        this.cacheManager.saveDetails(httpMethod, sampleResultOK);
>> >> > +        assertNotNull("Should find
>> >> entry",getThreadCacheEntry(LOCAL_HOST));
>> >> > +        assertFalse("Should not find valid
>> >> entry",this.cacheManager.inCache(url));
>> >> > +    }
>> >> > +
>> >> >      public void testCacheHttpClientBug51932() throws Exception{
>> >> >          this.cacheManager.setUseExpires(true);
>> >> >          this.cacheManager.testIterationStart(null);
>> >> >
>> >> > Modified: jmeter/trunk/xdocs/changes.xml
>> >> > URL:
>> >>
>> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1358710&r1=1358709&r2=1358710&view=diff
>> >> >
>> >>
>> ==============================================================================
>> >> > --- jmeter/trunk/xdocs/changes.xml (original)
>> >> > +++ jmeter/trunk/xdocs/changes.xml Sun Jul  8 09:40:51 2012
>> >> > @@ -59,6 +59,7 @@ or a Debug Sampler with all fields set t
>> >> >
>> >> >  <h3>HTTP Samplers and Proxy</h3>
>> >> >  <ul>
>> >> > +<li><bugzilla>53521</bugzilla> - Cache Manager should cache content
>> >> with Cache-control=private</li>
>> >> >  </ul>
>> >> >
>> >> >  <h3>Other Samplers</h3>
>> >> >
>> >> >
>> >>
>> >
>> >
>> >
>> > --
>> > Cordialement.
>> > Philippe Mouawad.
>>
>
>
>
> --
> Cordialement.
> Philippe Mouawad.

Re: svn commit: r1358710 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java xdocs/changes.xml

Posted by Philippe Mouawad <ph...@gmail.com>.
but if we have this:
Cache-Control=no-cache, max-age=10.

If we don't check we will cache which is wrong right ?
This header is wrong but I have already seen this on tests I did.

Or I am misunderstanding ?

Regards
Philippe

On Sun, Jul 8, 2012 at 3:19 PM, sebb <se...@gmail.com> wrote:

> On 8 July 2012 14:07, Philippe Mouawad <ph...@gmail.com> wrote:
> > Can't it also be no-cache ? no-store ?
> > If we don't check it , we could store in cache if server returns invalid
> > headers no ?
>
> What do we do if we don't check MaxAge?
>
> I don't think we should be more restrictive just because we are also
> checking the age.
>
> >
> > Thansk for looking at 53365.
> > Regards
> > Philippe
> >
> > On Sun, Jul 8, 2012 at 3:01 PM, sebb <se...@gmail.com> wrote:
> >
> >> On 8 July 2012 10:40,  <pm...@apache.org> wrote:
> >> > Author: pmouawad
> >> > Date: Sun Jul  8 09:40:51 2012
> >> > New Revision: 1358710
> >> >
> >> > URL: http://svn.apache.org/viewvc?rev=1358710&view=rev
> >> > Log:
> >> > Bug 53521 - Cache Manager should cache content with
> Cache-control=private
> >> > Bugzilla Id: 53521
> >> >
> >> > Modified:
> >> >
> >>
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
> >> >
> >>
> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
> >> >     jmeter/trunk/xdocs/changes.xml
> >> >
> >> > Modified:
> >>
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
> >> > URL:
> >>
> http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java?rev=1358710&r1=1358709&r2=1358710&view=diff
> >> >
> >>
> ==============================================================================
> >> > ---
> >>
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
> >> (original)
> >> > +++
> >>
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
> >> Sun Jul  8 09:40:51 2012
> >> > @@ -161,7 +161,7 @@ public class CacheManager extends Config
> >> >          if (useExpires) {// Check that we are processing
> >> Expires/CacheControl
> >> >              final String MAX_AGE = "max-age=";
> >> >              // TODO - check for other CacheControl attributes?
> >> > -            if (cacheControl != null &&
> cacheControl.contains("public")
> >> && cacheControl.contains(MAX_AGE)) {
> >> > +            if (cacheControl != null &&
> >> (cacheControl.contains("public") || cacheControl.contains("private")) &&
> >> cacheControl.contains(MAX_AGE)) {
> >>
> >> Not sure this is the correct fix.
> >> Do we really care if the string "public" or "private" is present so
> >> long as there is a MAX_AGE entry?
> >> We could just drop the check for "public" instead.
> >>
> >> >                  long maxAgeInSecs = Long.parseLong(
> >> >
> >>  cacheControl.substring(cacheControl.indexOf(MAX_AGE)+MAX_AGE.length())
> >> >                              .split("[, ]")[0] // Bug 51932 - allow
> for
> >> optional trailing attributes
> >> >
> >> > Modified:
> >>
> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
> >> > URL:
> >>
> http://svn.apache.org/viewvc/jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java?rev=1358710&r1=1358709&r2=1358710&view=diff
> >> >
> >>
> ==============================================================================
> >> > ---
> >>
> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
> >> (original)
> >> > +++
> >>
> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
> >> Sun Jul  8 09:40:51 2012
> >> > @@ -238,7 +238,30 @@ public class TestCacheManager extends JM
> >> >          assertNotNull("Should find
> >> entry",getThreadCacheEntry(LOCAL_HOST));
> >> >          assertTrue("Should find valid
> >> entry",this.cacheManager.inCache(url));
> >> >      }
> >> > +
> >> > +    public void testPrivateCacheHttpClient() throws Exception{
> >> > +        this.cacheManager.setUseExpires(true);
> >> > +        this.cacheManager.testIterationStart(null);
> >> > +        assertNull("Should not find
> >> entry",getThreadCacheEntry(LOCAL_HOST));
> >> > +        assertFalse("Should not find valid
> >> entry",this.cacheManager.inCache(url));
> >> > +        ((HttpMethodStub)httpMethod).expires=makeDate(new
> >> Date(System.currentTimeMillis()));
> >> > +        ((HttpMethodStub)httpMethod).cacheControl="private,
> max-age=10";
> >> > +        this.cacheManager.saveDetails(httpMethod, sampleResultOK);
> >> > +        assertNotNull("Should find
> >> entry",getThreadCacheEntry(LOCAL_HOST));
> >> > +        assertTrue("Should find valid
> >> entry",this.cacheManager.inCache(url));
> >> > +    }
> >> >
> >> > +    public void testNoCacheHttpClient() throws Exception{
> >> > +        this.cacheManager.setUseExpires(true);
> >> > +        this.cacheManager.testIterationStart(null);
> >> > +        assertNull("Should not find
> >> entry",getThreadCacheEntry(LOCAL_HOST));
> >> > +        assertFalse("Should not find valid
> >> entry",this.cacheManager.inCache(url));
> >> > +        ((HttpMethodStub)httpMethod).cacheControl="no-cache";
> >> > +        this.cacheManager.saveDetails(httpMethod, sampleResultOK);
> >> > +        assertNotNull("Should find
> >> entry",getThreadCacheEntry(LOCAL_HOST));
> >> > +        assertFalse("Should not find valid
> >> entry",this.cacheManager.inCache(url));
> >> > +    }
> >> > +
> >> >      public void testCacheHttpClientBug51932() throws Exception{
> >> >          this.cacheManager.setUseExpires(true);
> >> >          this.cacheManager.testIterationStart(null);
> >> >
> >> > Modified: jmeter/trunk/xdocs/changes.xml
> >> > URL:
> >>
> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1358710&r1=1358709&r2=1358710&view=diff
> >> >
> >>
> ==============================================================================
> >> > --- jmeter/trunk/xdocs/changes.xml (original)
> >> > +++ jmeter/trunk/xdocs/changes.xml Sun Jul  8 09:40:51 2012
> >> > @@ -59,6 +59,7 @@ or a Debug Sampler with all fields set t
> >> >
> >> >  <h3>HTTP Samplers and Proxy</h3>
> >> >  <ul>
> >> > +<li><bugzilla>53521</bugzilla> - Cache Manager should cache content
> >> with Cache-control=private</li>
> >> >  </ul>
> >> >
> >> >  <h3>Other Samplers</h3>
> >> >
> >> >
> >>
> >
> >
> >
> > --
> > Cordialement.
> > Philippe Mouawad.
>



-- 
Cordialement.
Philippe Mouawad.

Re: svn commit: r1358710 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java xdocs/changes.xml

Posted by sebb <se...@gmail.com>.
On 8 July 2012 14:07, Philippe Mouawad <ph...@gmail.com> wrote:
> Can't it also be no-cache ? no-store ?
> If we don't check it , we could store in cache if server returns invalid
> headers no ?

What do we do if we don't check MaxAge?

I don't think we should be more restrictive just because we are also
checking the age.

>
> Thansk for looking at 53365.
> Regards
> Philippe
>
> On Sun, Jul 8, 2012 at 3:01 PM, sebb <se...@gmail.com> wrote:
>
>> On 8 July 2012 10:40,  <pm...@apache.org> wrote:
>> > Author: pmouawad
>> > Date: Sun Jul  8 09:40:51 2012
>> > New Revision: 1358710
>> >
>> > URL: http://svn.apache.org/viewvc?rev=1358710&view=rev
>> > Log:
>> > Bug 53521 - Cache Manager should cache content with Cache-control=private
>> > Bugzilla Id: 53521
>> >
>> > Modified:
>> >
>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
>> >
>> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
>> >     jmeter/trunk/xdocs/changes.xml
>> >
>> > Modified:
>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
>> > URL:
>> http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java?rev=1358710&r1=1358709&r2=1358710&view=diff
>> >
>> ==============================================================================
>> > ---
>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
>> (original)
>> > +++
>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
>> Sun Jul  8 09:40:51 2012
>> > @@ -161,7 +161,7 @@ public class CacheManager extends Config
>> >          if (useExpires) {// Check that we are processing
>> Expires/CacheControl
>> >              final String MAX_AGE = "max-age=";
>> >              // TODO - check for other CacheControl attributes?
>> > -            if (cacheControl != null && cacheControl.contains("public")
>> && cacheControl.contains(MAX_AGE)) {
>> > +            if (cacheControl != null &&
>> (cacheControl.contains("public") || cacheControl.contains("private")) &&
>> cacheControl.contains(MAX_AGE)) {
>>
>> Not sure this is the correct fix.
>> Do we really care if the string "public" or "private" is present so
>> long as there is a MAX_AGE entry?
>> We could just drop the check for "public" instead.
>>
>> >                  long maxAgeInSecs = Long.parseLong(
>> >
>>  cacheControl.substring(cacheControl.indexOf(MAX_AGE)+MAX_AGE.length())
>> >                              .split("[, ]")[0] // Bug 51932 - allow for
>> optional trailing attributes
>> >
>> > Modified:
>> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
>> > URL:
>> http://svn.apache.org/viewvc/jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java?rev=1358710&r1=1358709&r2=1358710&view=diff
>> >
>> ==============================================================================
>> > ---
>> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
>> (original)
>> > +++
>> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
>> Sun Jul  8 09:40:51 2012
>> > @@ -238,7 +238,30 @@ public class TestCacheManager extends JM
>> >          assertNotNull("Should find
>> entry",getThreadCacheEntry(LOCAL_HOST));
>> >          assertTrue("Should find valid
>> entry",this.cacheManager.inCache(url));
>> >      }
>> > +
>> > +    public void testPrivateCacheHttpClient() throws Exception{
>> > +        this.cacheManager.setUseExpires(true);
>> > +        this.cacheManager.testIterationStart(null);
>> > +        assertNull("Should not find
>> entry",getThreadCacheEntry(LOCAL_HOST));
>> > +        assertFalse("Should not find valid
>> entry",this.cacheManager.inCache(url));
>> > +        ((HttpMethodStub)httpMethod).expires=makeDate(new
>> Date(System.currentTimeMillis()));
>> > +        ((HttpMethodStub)httpMethod).cacheControl="private, max-age=10";
>> > +        this.cacheManager.saveDetails(httpMethod, sampleResultOK);
>> > +        assertNotNull("Should find
>> entry",getThreadCacheEntry(LOCAL_HOST));
>> > +        assertTrue("Should find valid
>> entry",this.cacheManager.inCache(url));
>> > +    }
>> >
>> > +    public void testNoCacheHttpClient() throws Exception{
>> > +        this.cacheManager.setUseExpires(true);
>> > +        this.cacheManager.testIterationStart(null);
>> > +        assertNull("Should not find
>> entry",getThreadCacheEntry(LOCAL_HOST));
>> > +        assertFalse("Should not find valid
>> entry",this.cacheManager.inCache(url));
>> > +        ((HttpMethodStub)httpMethod).cacheControl="no-cache";
>> > +        this.cacheManager.saveDetails(httpMethod, sampleResultOK);
>> > +        assertNotNull("Should find
>> entry",getThreadCacheEntry(LOCAL_HOST));
>> > +        assertFalse("Should not find valid
>> entry",this.cacheManager.inCache(url));
>> > +    }
>> > +
>> >      public void testCacheHttpClientBug51932() throws Exception{
>> >          this.cacheManager.setUseExpires(true);
>> >          this.cacheManager.testIterationStart(null);
>> >
>> > Modified: jmeter/trunk/xdocs/changes.xml
>> > URL:
>> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1358710&r1=1358709&r2=1358710&view=diff
>> >
>> ==============================================================================
>> > --- jmeter/trunk/xdocs/changes.xml (original)
>> > +++ jmeter/trunk/xdocs/changes.xml Sun Jul  8 09:40:51 2012
>> > @@ -59,6 +59,7 @@ or a Debug Sampler with all fields set t
>> >
>> >  <h3>HTTP Samplers and Proxy</h3>
>> >  <ul>
>> > +<li><bugzilla>53521</bugzilla> - Cache Manager should cache content
>> with Cache-control=private</li>
>> >  </ul>
>> >
>> >  <h3>Other Samplers</h3>
>> >
>> >
>>
>
>
>
> --
> Cordialement.
> Philippe Mouawad.

Re: svn commit: r1358710 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java xdocs/changes.xml

Posted by Philippe Mouawad <ph...@gmail.com>.
Can't it also be no-cache ? no-store ?
If we don't check it , we could store in cache if server returns invalid
headers no ?


Thansk for looking at 53365.
Regards
Philippe

On Sun, Jul 8, 2012 at 3:01 PM, sebb <se...@gmail.com> wrote:

> On 8 July 2012 10:40,  <pm...@apache.org> wrote:
> > Author: pmouawad
> > Date: Sun Jul  8 09:40:51 2012
> > New Revision: 1358710
> >
> > URL: http://svn.apache.org/viewvc?rev=1358710&view=rev
> > Log:
> > Bug 53521 - Cache Manager should cache content with Cache-control=private
> > Bugzilla Id: 53521
> >
> > Modified:
> >
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
> >
> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
> >     jmeter/trunk/xdocs/changes.xml
> >
> > Modified:
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
> > URL:
> http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java?rev=1358710&r1=1358709&r2=1358710&view=diff
> >
> ==============================================================================
> > ---
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
> (original)
> > +++
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
> Sun Jul  8 09:40:51 2012
> > @@ -161,7 +161,7 @@ public class CacheManager extends Config
> >          if (useExpires) {// Check that we are processing
> Expires/CacheControl
> >              final String MAX_AGE = "max-age=";
> >              // TODO - check for other CacheControl attributes?
> > -            if (cacheControl != null && cacheControl.contains("public")
> && cacheControl.contains(MAX_AGE)) {
> > +            if (cacheControl != null &&
> (cacheControl.contains("public") || cacheControl.contains("private")) &&
> cacheControl.contains(MAX_AGE)) {
>
> Not sure this is the correct fix.
> Do we really care if the string "public" or "private" is present so
> long as there is a MAX_AGE entry?
> We could just drop the check for "public" instead.
>
> >                  long maxAgeInSecs = Long.parseLong(
> >
>  cacheControl.substring(cacheControl.indexOf(MAX_AGE)+MAX_AGE.length())
> >                              .split("[, ]")[0] // Bug 51932 - allow for
> optional trailing attributes
> >
> > Modified:
> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
> > URL:
> http://svn.apache.org/viewvc/jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java?rev=1358710&r1=1358709&r2=1358710&view=diff
> >
> ==============================================================================
> > ---
> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
> (original)
> > +++
> jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManager.java
> Sun Jul  8 09:40:51 2012
> > @@ -238,7 +238,30 @@ public class TestCacheManager extends JM
> >          assertNotNull("Should find
> entry",getThreadCacheEntry(LOCAL_HOST));
> >          assertTrue("Should find valid
> entry",this.cacheManager.inCache(url));
> >      }
> > +
> > +    public void testPrivateCacheHttpClient() throws Exception{
> > +        this.cacheManager.setUseExpires(true);
> > +        this.cacheManager.testIterationStart(null);
> > +        assertNull("Should not find
> entry",getThreadCacheEntry(LOCAL_HOST));
> > +        assertFalse("Should not find valid
> entry",this.cacheManager.inCache(url));
> > +        ((HttpMethodStub)httpMethod).expires=makeDate(new
> Date(System.currentTimeMillis()));
> > +        ((HttpMethodStub)httpMethod).cacheControl="private, max-age=10";
> > +        this.cacheManager.saveDetails(httpMethod, sampleResultOK);
> > +        assertNotNull("Should find
> entry",getThreadCacheEntry(LOCAL_HOST));
> > +        assertTrue("Should find valid
> entry",this.cacheManager.inCache(url));
> > +    }
> >
> > +    public void testNoCacheHttpClient() throws Exception{
> > +        this.cacheManager.setUseExpires(true);
> > +        this.cacheManager.testIterationStart(null);
> > +        assertNull("Should not find
> entry",getThreadCacheEntry(LOCAL_HOST));
> > +        assertFalse("Should not find valid
> entry",this.cacheManager.inCache(url));
> > +        ((HttpMethodStub)httpMethod).cacheControl="no-cache";
> > +        this.cacheManager.saveDetails(httpMethod, sampleResultOK);
> > +        assertNotNull("Should find
> entry",getThreadCacheEntry(LOCAL_HOST));
> > +        assertFalse("Should not find valid
> entry",this.cacheManager.inCache(url));
> > +    }
> > +
> >      public void testCacheHttpClientBug51932() throws Exception{
> >          this.cacheManager.setUseExpires(true);
> >          this.cacheManager.testIterationStart(null);
> >
> > Modified: jmeter/trunk/xdocs/changes.xml
> > URL:
> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1358710&r1=1358709&r2=1358710&view=diff
> >
> ==============================================================================
> > --- jmeter/trunk/xdocs/changes.xml (original)
> > +++ jmeter/trunk/xdocs/changes.xml Sun Jul  8 09:40:51 2012
> > @@ -59,6 +59,7 @@ or a Debug Sampler with all fields set t
> >
> >  <h3>HTTP Samplers and Proxy</h3>
> >  <ul>
> > +<li><bugzilla>53521</bugzilla> - Cache Manager should cache content
> with Cache-control=private</li>
> >  </ul>
> >
> >  <h3>Other Samplers</h3>
> >
> >
>



-- 
Cordialement.
Philippe Mouawad.