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: 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
Date Sun, 08 Jul 2012 13:43:52 GMT
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 <sebbaz@gmail.com> wrote:
>
>> On 8 July 2012 14:24, Philippe Mouawad <philippe.mouawad@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 <sebbaz@gmail.com> wrote:
>> >
>> >> On 8 July 2012 14:07, Philippe Mouawad <philippe.mouawad@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 <sebbaz@gmail.com> wrote:
>> >> >
>> >> >> On 8 July 2012 10:40,  <pmouawad@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.

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