You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@abdera.apache.org by ro...@apache.org on 2006/08/08 01:45:07 UTC

svn commit: r429514 - /incubator/abdera/java/trunk/client/src/main/java/org/apache/abdera/protocol/cache/CachedResponseBase.java

Author: rooneg
Date: Mon Aug  7 16:45:07 2006
New Revision: 429514

URL: http://svn.apache.org/viewvc?rev=429514&view=rev
Log:
Don't crash if the server doesn't send back a Date header.

[ in client/src/main/java/org/apache/abdera/protocol/cache ]

* CachedResponseBase.java
  (getCachedTime): Don't dereference a null pointer if there's no Date
   header in the response.

Modified:
    incubator/abdera/java/trunk/client/src/main/java/org/apache/abdera/protocol/cache/CachedResponseBase.java

Modified: incubator/abdera/java/trunk/client/src/main/java/org/apache/abdera/protocol/cache/CachedResponseBase.java
URL: http://svn.apache.org/viewvc/incubator/abdera/java/trunk/client/src/main/java/org/apache/abdera/protocol/cache/CachedResponseBase.java?rev=429514&r1=429513&r2=429514&view=diff
==============================================================================
--- incubator/abdera/java/trunk/client/src/main/java/org/apache/abdera/protocol/cache/CachedResponseBase.java (original)
+++ incubator/abdera/java/trunk/client/src/main/java/org/apache/abdera/protocol/cache/CachedResponseBase.java Mon Aug  7 16:45:07 2006
@@ -68,7 +68,11 @@
   }
 
   public long getCachedTime() {
-    return getServerDate().getTime();
+    Date serverDate = getServerDate();
+    if (serverDate != null)
+      return serverDate.getTime();
+    else
+      return 0;
   }
   
   public long getResidentAge() {



Re: svn commit: r429514 - /incubator/abdera/java/trunk/client/src/main/java/org/apache/abdera/protocol/cache/CachedResponseBase.java

Posted by James M Snell <ja...@gmail.com>.
Yep, saw that.  Cool stuff.

I'm continuing to make some tweaks in the implementation, largely to
clean it up and make it a bit more manageable.

- James

Garrett Rooney wrote:
> On 8/7/06, James M Snell <ja...@gmail.com> wrote:
>> Yeah, I went back and checked :-) ... If the server Date header does not
>> exist, getServerDate should default to the moment that the
>> InMemoryCachedResponse instance was created.
>>
>> A quick fix is to add the following to ResponseBase
>>
>>   protected Date now = new Date();
>>
>> And change... (also in ResponseBase)
>>
>>   public Date getServerDate() {
>>     if (response_date == null) {
>>       Date date = getDateHeader("Date");
>>       response_date = (date != null) ? date : now;
>>     }
>>     return response_date;
>>   }
> 
> Works great, committed in r429521.
> 
> -garrett
> 

Re: svn commit: r429514 - /incubator/abdera/java/trunk/client/src/main/java/org/apache/abdera/protocol/cache/CachedResponseBase.java

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 8/7/06, James M Snell <ja...@gmail.com> wrote:
> Yeah, I went back and checked :-) ... If the server Date header does not
> exist, getServerDate should default to the moment that the
> InMemoryCachedResponse instance was created.
>
> A quick fix is to add the following to ResponseBase
>
>   protected Date now = new Date();
>
> And change... (also in ResponseBase)
>
>   public Date getServerDate() {
>     if (response_date == null) {
>       Date date = getDateHeader("Date");
>       response_date = (date != null) ? date : now;
>     }
>     return response_date;
>   }

Works great, committed in r429521.

-garrett

Re: svn commit: r429514 - /incubator/abdera/java/trunk/client/src/main/java/org/apache/abdera/protocol/cache/CachedResponseBase.java

Posted by James M Snell <ja...@gmail.com>.
Yeah, I went back and checked :-) ... If the server Date header does not
exist, getServerDate should default to the moment that the
InMemoryCachedResponse instance was created.

A quick fix is to add the following to ResponseBase

  protected Date now = new Date();

And change... (also in ResponseBase)

  public Date getServerDate() {
    if (response_date == null) {
      Date date = getDateHeader("Date");
      response_date = (date != null) ? date : now;
    }
    return response_date;
  }

- James

Garrett Rooney wrote:
> On 8/7/06, James M Snell <ja...@gmail.com> wrote:
>> The cached time is the instant that the resource entered the cached.
>> From what I can tell from RFC2616, that is supposed to be the value of
>> the Date header, if it exists, or the instant the response was received.
>>  I believe I modified getServerDate() so that it will never return a
>> null, so the else return 0 below should actually never execute.
> 
> Trust me, it does actually execute if there's literally no Date header
> returned from the server ;-)
> 
> Without that patch, my embedded Jetty Servlet based tests were getting
> NullPointer exceptions.
> 
>> However, I'll couch all of this is a blanket statement that I am not a
>> caching expert.  For some of this, I'm learning as I go.
> 
> Ditto ;-)
> 
> -garrett
> 

Re: svn commit: r429514 - /incubator/abdera/java/trunk/client/src/main/java/org/apache/abdera/protocol/cache/CachedResponseBase.java

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 8/7/06, James M Snell <ja...@gmail.com> wrote:
> The cached time is the instant that the resource entered the cached.
> From what I can tell from RFC2616, that is supposed to be the value of
> the Date header, if it exists, or the instant the response was received.
>  I believe I modified getServerDate() so that it will never return a
> null, so the else return 0 below should actually never execute.

Trust me, it does actually execute if there's literally no Date header
returned from the server ;-)

Without that patch, my embedded Jetty Servlet based tests were getting
NullPointer exceptions.

> However, I'll couch all of this is a blanket statement that I am not a
> caching expert.  For some of this, I'm learning as I go.

Ditto ;-)

-garrett

Re: svn commit: r429514 - /incubator/abdera/java/trunk/client/src/main/java/org/apache/abdera/protocol/cache/CachedResponseBase.java

Posted by James M Snell <ja...@gmail.com>.
The cached time is the instant that the resource entered the cached.
>From what I can tell from RFC2616, that is supposed to be the value of
the Date header, if it exists, or the instant the response was received.
 I believe I modified getServerDate() so that it will never return a
null, so the else return 0 below should actually never execute.

However, I'll couch all of this is a blanket statement that I am not a
caching expert.  For some of this, I'm learning as I go.

- James

Garrett Rooney wrote:
> On 8/7/06, rooneg@apache.org <ro...@apache.org> wrote:
> 
>>    public long getCachedTime() {
>> -    return getServerDate().getTime();
>> +    Date serverDate = getServerDate();
>> +    if (serverDate != null)
>> +      return serverDate.getTime();
>> +    else
>> +      return 0;
>>    }
> 
> James, if you could verify that this is the right thing to do in this
> case I'd appreciate it.  I ran across this while trying to rig up a
> Jetty based server for the cache tests...
> 
> -garrett
> 

Re: svn commit: r429514 - /incubator/abdera/java/trunk/client/src/main/java/org/apache/abdera/protocol/cache/CachedResponseBase.java

Posted by James M Snell <ja...@gmail.com>.
Actually, scratch that, if the Date response header is null,
getServerDate will be null, which means we'll have a NPE. We'll need to
fix that.

- James

Garrett Rooney wrote:
> On 8/7/06, rooneg@apache.org <ro...@apache.org> wrote:
> 
>>    public long getCachedTime() {
>> -    return getServerDate().getTime();
>> +    Date serverDate = getServerDate();
>> +    if (serverDate != null)
>> +      return serverDate.getTime();
>> +    else
>> +      return 0;
>>    }
> 
> James, if you could verify that this is the right thing to do in this
> case I'd appreciate it.  I ran across this while trying to rig up a
> Jetty based server for the cache tests...
> 
> -garrett
> 

Re: svn commit: r429514 - /incubator/abdera/java/trunk/client/src/main/java/org/apache/abdera/protocol/cache/CachedResponseBase.java

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 8/7/06, rooneg@apache.org <ro...@apache.org> wrote:

>    public long getCachedTime() {
> -    return getServerDate().getTime();
> +    Date serverDate = getServerDate();
> +    if (serverDate != null)
> +      return serverDate.getTime();
> +    else
> +      return 0;
>    }

James, if you could verify that this is the right thing to do in this
case I'd appreciate it.  I ran across this while trying to rig up a
Jetty based server for the cache tests...

-garrett