Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ETag and If-None-Match #831

Closed
arturdryomov opened this issue May 17, 2014 · 50 comments
Closed

ETag and If-None-Match #831

arturdryomov opened this issue May 17, 2014 · 50 comments

Comments

@arturdryomov
Copy link

I have troubles with the ETag-based caching. I am using OkHttp 1.5.4 and Retrofit 1.5.1 with HttpResponseCache set. There are logs below for better explanation.

The request with the Session header set to a token A. OkHttp hits the network, which is great.

---> HTTP GET https://localhost/shares
Session: TOKEN_A
Accept: application/json
User-Agent: Application/1.0.0 (Android 4.4.2; Nexus 4) Size/4.7 Resolution/1280x768
---> END HTTP (0-byte body)
<--- HTTP 200 https://localhost/shares (402ms)
: HTTP/1.1 200 OK
Content-Length: 136
Content-Type: application/json
Date: Sat, 17 May 2014 11:07:20 GMT
Etag: "677210f4c22a13203551e2b92a20684528a23c87"
Last-Modified: Sat, 17 May 2014 04:07:20 GMT
OkHttp-Received-Millis: 1400324839589
OkHttp-Response-Source: NETWORK 200
OkHttp-Sent-Millis: 1400324839194
<--- END HTTP (136-byte body)

The second request with the Session header set to a token A. OkHttp hits the cache, which is great as well.

---> HTTP GET https://localhost/shares
Session: TOKEN_A
Accept: application/json
User-Agent: Application/1.0.0 (Android 4.4.2; Nexus 4) Size/4.7 Resolution/1280x768
---> END HTTP (0-byte body)
GC_FOR_ALLOC freed 415K, 5% free 9588K/10036K, paused 19ms, total 19ms
<--- HTTP 200 https://localhost/shares (46ms)
: HTTP/1.1 200 OK
Content-Length: 136
Content-Type: application/json
Date: Sat, 17 May 2014 11:07:20 GMT
Etag: "677210f4c22a13203551e2b92a20684528a23c87"
Last-Modified: Sat, 17 May 2014 04:07:20 GMT
OkHttp-Received-Millis: 1400324839589
OkHttp-Response-Source: CACHE 200
OkHttp-Sent-Millis: 1400324839194
<--- END HTTP (136-byte body)

The third request with the Session header set to token B. OkHttp hits the cache again, which is not so great, because in this case the server returns a different response with a different ETag actually.

---> HTTP GET https://localhost/shares
Session: TOKEN_B
Accept: application/json
User-Agent: Application/1.0.0 (Android 4.4.2; Nexus 4) Size/4.7 Resolution/1280x768
---> END HTTP (0-byte body)
<--- HTTP 200 https://localhost/shares (11ms)
: HTTP/1.1 200 OK
Content-Length: 136
Content-Type: application/json
Date: Sat, 17 May 2014 11:07:20 GMT
Etag: "677210f4c22a13203551e2b92a20684528a23c87"
Last-Modified: Sat, 17 May 2014 04:07:20 GMT
OkHttp-Received-Millis: 1400324839589
OkHttp-Response-Source: CACHE 200
OkHttp-Sent-Millis: 1400324839194
<--- END HTTP (136-byte body)

Personally I expect to see the If-None-Match at the second and the third requests. Is the issue related to the lack of Cache-Control header at servers’s responses? Maybe I just don’t understand the caching strategy properly.

@arturdryomov arturdryomov changed the title ETag and If-Note-Match ETag and If-None-Match May 17, 2014
@swankjesse
Copy link
Member

What happens if you add this header to your server's response:

Vary: Session

My guess is that OkHttp will do the right thing if it knows the response content changes with the Session request header.

@AAverin
Copy link

AAverin commented Oct 4, 2014

Got a similar issue.
Basically my take is that I don't see 'If-None-Match' header set for request at all.

If I do a manual ETag implementation, saving eTag to preferences cache and adding it as a header to every appropriate request - I get 304 from the server (and Retrofit throws a RetrofitError, not thinking it as a successful response, btw). Looks like a somewhat correct behavior.

With Cache set as in example, I see that Retrofit is hitting CONDITIONAL CACHE 200, but I don't see "If-None-Match" set to my request in logs, and with the time it takes for request, I assume that even though Retorift hits the cache, it still did a full request to the server, and server had responded not with 304, but with full set of data - it takes too long.
With Cache enabled, I expect request to return right away with no delays, yielding cached results.

How can I be sure that 'If-None-Match' header is correctly set? Shouldn't it be displayed in logs when setLogLevel(RestAdapter.LogLevel.FULL) is set?

@arturdryomov
Copy link
Author

Actually Vary header helped for me—exactly like Jesse said.

@AAverin
Copy link

AAverin commented Oct 4, 2014

I don't use Session header at all and doubt our server uses it too.
I know for sure that server supports "If-None-Match" technique.
And I don't really have much control over the server too...

@crossle
Copy link

crossle commented Dec 31, 2014

How to add If-None-Match on okhttp ?

@swankjesse
Copy link
Member

@crossle sounds like a question for stack overflow.

@crossle
Copy link

crossle commented Jan 3, 2015

@AAverin
Copy link

AAverin commented Jan 3, 2015

I'll reference my question here
#1278

@crossle
Copy link

crossle commented Jan 5, 2015

The server return a ETag, but retrofit full log not show ETag header, use curl -I print ETag, why?
wait retrofit new release version.

@crossle
Copy link

crossle commented Jan 5, 2015

@swankjesse if server return cache-control: max-age=0, private, must-revalidate, okhttp will cache or not?

@JakeWharton
Copy link
Member

It will cache, but it has to make a request back to the server when you
access to determine whether the resource is stale or not. If it is not
stale the server will return a 304 Not Modified and the cached value will
be used.
On Jan 5, 2015 1:18 AM, "Crossle Song" notifications@github.com wrote:

@swankjesse https://github.com/swankjesse if server return cache-control:
max-age=0, private, must-revalidate, okhttp will cache or not?


Reply to this email directly or view it on GitHub
#831 (comment).

@AAverin
Copy link

AAverin commented Jan 5, 2015

@JakeWharton, could you please give us some more information about tying together OkHttp Inteceptors with Retrofit? @swankjesse have mentioned downloading a pre-release snapshot and said a release that supports that is planned to be released soon. When can we expect it?

@JakeWharton
Copy link
Member

We don't give ETAs. It could be anywhere from tomorrow up to 4-6 weeks.
When I find the time to completely test that it's working.
On Jan 5, 2015 1:34 AM, "AAverin" notifications@github.com wrote:

@JakeWharton https://github.com/JakeWharton, could you please give us
some more information about tying together OkHttp Inteceptors with
Retrofit? @swankjesse https://github.com/swankjesse have mentioned
downloading a pre-release snapshot and said a release that supports that is
planned to be released soon. When can we expect it?


Reply to this email directly or view it on GitHub
#831 (comment).

@AAverin
Copy link

AAverin commented Jan 5, 2015

I see, thanks.
That pre-release snapshot, where can I get one? Should I just grab the sources and build them myself?

@swankjesse
Copy link
Member

Yup, building it yourself works.

@crossle
Copy link

crossle commented Jan 6, 2015

I test use curl, add -H 'If-None-Match: "17b6637079897a93673bc4badd3e6f84"' return 304, but okhttp:2.1.0 retrofit:1.8.0, not return 304, retrofit log still 200.

@JakeWharton
Copy link
Member

Because Retrofit only sees the 200. OkHttp sees the 304.
On Jan 5, 2015 6:24 PM, "Crossle Song" notifications@github.com wrote:

I test use curl, return add -H 'If-None-Match:
"17b6637079897a93673bc4badd3e6f84"' return 304, but okhttp:2.1.0
retrofit:1.8.0, not return 304, retrofit log still 200.


Reply to this email directly or view it on GitHub
#831 (comment).

@crossle
Copy link

crossle commented Jan 6, 2015

@JakeWharton Thank you!

@crossle
Copy link

crossle commented Jan 8, 2015

Bad news, I used retrofit 1.9, okhttp 2.2, use intercepter log, no ETag log, but curl have ETag

@JakeWharton
Copy link
Member

Did you use interceptors() or networkInterceptors()?

@crossle
Copy link

crossle commented Jan 8, 2015

use interceptors()

@JakeWharton
Copy link
Member

That is above the cache, you'll always get 200s.

@JakeWharton
Copy link
Member

@AAverin
Copy link

AAverin commented Jan 8, 2015

Hey @JakeWharton.
I've been using networkInteceptors for the same purpose, to check if everything is correct in headers.
Still no If-None-Match present.
You can find log examples in this SO question: http://stackoverflow.com/questions/27561992/retrofit-etag-and-caching/27758584

@crossle
Copy link

crossle commented Jan 8, 2015

Try networkInteceptors, same problem.

@crossle
Copy link

crossle commented Jan 12, 2015

@JakeWharton How about this? same problem use networkInteceptors, no ETag.

@AAverin
Copy link

AAverin commented Jan 16, 2015

Guys, any update on this matter?
My projects are kinda stuck because if Etag not working properly.
Should I just go and do a manual implementation by manually passing Etag/If-None-Match headers and building a cache?

@swankjesse
Copy link
Member

Can you paste the network interceptors log for two calls to the same URL, with a cache installed? I wanna read the headers myself!

@AAverin
Copy link

AAverin commented Jan 20, 2015

====== clean install, initial request =======

Sending request http://demo.***.com/api/v1/download/map-manifest on Connection{demo.***.com:80, proxy=DIRECT@ hostAddress=*** cipherSuite=none protocol=http/1.1}
Accept: application/json;
Host: demo.***.com
Connection: Keep-Alive
Accept-Encoding: gzip
User-Agent: okhttp/2.2.0

Received response for http://demo.***.com/api/v1/download/map-manifest in 935.1ms
Date: Sun, 18 Jan 2015 09:38:29 GMT
Server: Apache
X-Powered-By: PHP/5.4.31
Access-Control-Allow-Credentials: true
Pragma:
Cache-Control: public, max-age=3600
X-Frame-Options: SAMEORIGIN
Etag: "hLxLRYztkinJAB453nRV7ncBSuU=-gzip"
Last-Modified: Wed, 24 Dec 2014 13:09:04 GMT
Vary: Accept-Encoding
Content-Encoding: gzip
Keep-Alive: timeout=2, max=100
Connection: Keep-Alive
Transfer-Encoding: chunked
Content-Type: application/json; charset=UTF-8
OkHttp-Selected-Protocol: http/1.1
OkHttp-Sent-Millis: 1421573831462
OkHttp-Received-Millis: 1421573832397

====== same app instance, second request =======

Sending request http://demo.***.com/api/v1/download/map-manifest on Connection{demo.***.com:80, proxy=DIRECT@ hostAddress=*** cipherSuite=none protocol=http/1.1}
Accept: application/json;
Host: demo.***.com
Connection: Keep-Alive
Accept-Encoding: gzip
User-Agent: okhttp/2.2.0

Received response for http://demo.***.com/api/v1/download/map-manifest in 438.8ms
Date: Sun, 18 Jan 2015 09:39:54 GMT
Server: Apache
X-Powered-By: PHP/5.4.31
Access-Control-Allow-Credentials: true
Pragma:
Cache-Control: public, max-age=3600
X-Frame-Options: SAMEORIGIN
Etag: "hLxLRYztkinJAB453nRV7ncBSuU=-gzip"
Last-Modified: Wed, 24 Dec 2014 13:09:04 GMT
Vary: Accept-Encoding
Content-Encoding: gzip
Keep-Alive: timeout=2, max=100
Connection: Keep-Alive
Transfer-Encoding: chunked
Content-Type: application/json; charset=UTF-8
OkHttp-Selected-Protocol: http/1.1
OkHttp-Sent-Millis: 1421573916400
OkHttp-Received-Millis: 1421573916839

====== application restart, first request =======

Sending request http://demo.***.com/api/v1/download/map-manifest on Connection{demo.***.com:80, proxy=DIRECT@ hostAddress=*** cipherSuite=none protocol=http/1.1}
Accept: application/json;
Host: demo.***.com
Connection: Keep-Alive
Accept-Encoding: gzip
User-Agent: okhttp/2.2.0

Received response for http://demo.***.com/api/v1/download/map-manifest in 439.9ms
Date: Sun, 18 Jan 2015 09:40:58 GMT
Server: Apache
X-Powered-By: PHP/5.4.31
Access-Control-Allow-Credentials: true
Pragma:
Cache-Control: public, max-age=3600
X-Frame-Options: SAMEORIGIN
Etag: "hLxLRYztkinJAB453nRV7ncBSuU=-gzip"
Last-Modified: Wed, 24 Dec 2014 13:09:04 GMT
Vary: Accept-Encoding
Content-Encoding: gzip
Keep-Alive: timeout=2, max=100
Connection: Keep-Alive
Transfer-Encoding: chunked
Content-Type: application/json; charset=UTF-8
OkHttp-Selected-Protocol: http/1.1
OkHttp-Sent-Millis: 1421573980104
OkHttp-Received-Millis: 1421573980543

@AAverin
Copy link

AAverin commented Jan 20, 2015

I will give example from a different server that doesn't gzip once I make sure it really works manually. Asked a server dev to check everything.

The example provided works manually and sends 304 with
Etag: "hLxLRYztkinJAB453nRV7ncBSuU="

@crossle
Copy link

crossle commented Jan 20, 2015

Same problem.

@swankjesse
Copy link
Member

When you read the response, are you calling close() when you're done?

@AAverin
Copy link

AAverin commented Jan 20, 2015

Where should I call it? Isn't retrofit responsible on this stuff?
In the retrofit Callback I get a Json-based model that is parsed by Jackson.
JacksonConverter implements Converter and overrides methods fromBody and toBody.
So I don't interact and parse response manually.

@AAverin
Copy link

AAverin commented Jan 26, 2015

Any other ideas, guys? I need this Etag working finally.

@swankjesse
Copy link
Member

Put a breakpoint in the close() method inside of HttpEngine.cacheWritingResponse(). If that gets called, you're committing the response to the cache. If it doesn't, you need to make sure that it gets called. Possibly by fixing your JacksonConverter to call close().

@AAverin
Copy link

AAverin commented Jan 28, 2015

You keep mentioning this 'close()' method, but I don't get which class it this method from and where should I call it.

HttpEngine.cacheWritingResponse() - that's probably in okHttp source, I don't yet have it added to the project as a source, so if nothing will work - I'll try this out.

Here's my JacksonConverter implementation:
https://github.com/AAverin/android-skeleton-project/blob/master/app/src/main/java/pro/anton/averin/android/skeleton/data/net/json/JacksonConverter.java

I'll try closing TypedInput.in(), just in case this is the correct thing to do, but basically implementation was grabbed from here: http://kdubblabs.com/java/retrofit-by-square/retrofit-using-jackson-json-conversion/
And there is a very similar implementation here too, that also doesn't call any close() methods:
https://github.com/square/retrofit/blob/master/retrofit-converters/jackson/src/main/java/retrofit/converter/JacksonConverter.java

@JakeWharton
Copy link
Member

The Jackson converter was contributed. I'll note the Gson converter does
the right thing!

On Wed Jan 28 2015 at 1:41:31 PM AAverin notifications@github.com wrote:

You keep mentioning this 'close()' method, but I don't get which class it
this method from and where should I call it.

HttpEngine.cacheWritingResponse() - that's probably in okHttp source, I
don't yet have it added to the project as a source, so if nothing will work

  • I'll try this out.

Here's my JacksonConverter implementation:

https://github.com/AAverin/android-skeleton-project/blob/master/app/src/main/java/pro/anton/averin/android/skeleton/data/net/json/JacksonConverter.java

I'll try closing TypedInput.in(), just in case this is the correct thing
to do, but basically implementation was grabbed from here:
http://kdubblabs.com/java/retrofit-by-square/retrofit-using-jackson-json-conversion/
And there is a very similar implementation here too, that also doesn't
call any close() methods:

https://github.com/square/retrofit/blob/master/retrofit-converters/jackson/src/main/java/retrofit/converter/JacksonConverter.java


Reply to this email directly or view it on GitHub
#831 (comment).

@AAverin
Copy link

AAverin commented Jan 28, 2015

Thanks, Jake.
GsonConverter uses InputStreamReader and it calls isr.close() in finally. That probably results in close() called on the inputstream itself.
I'll try this in my JacksonConverter to see if it resolves the issue.

@JakeWharton
Copy link
Member

Yes. Per its documentation it always calls close on the underlying reader.

On Wed Jan 28 2015 at 1:47:27 PM AAverin notifications@github.com wrote:

Thanks, Jake.
GsonConverter uses InputStreamReader and it calls isr.close() in finally.
That probably results in close() called on the inputstream itself.
I'll try this in my JacksonConverter to see if it resolves the issue.


Reply to this email directly or view it on GitHub
#831 (comment).

@AAverin
Copy link

AAverin commented Jan 28, 2015

Looks like closing TypedInput.in() doesn't resolve the issue. At least I still don't see 'If-None-Match' header being sent with a request by OkHttp.
Here's the updated JacksonConverter code:

public class JacksonConverter implements Converter {

    private final ObjectMapper objectMapper;

    public JacksonConverter(ObjectMapper mapper) {
        this.objectMapper = mapper;
    }

    @Override
    public Object fromBody(TypedInput typedInput, Type type) throws ConversionException {
        JavaType javaType = objectMapper.getTypeFactory().constructType(type);

        try {
            InputStream in = typedInput.in();
            try {
                return objectMapper.readValue(in, javaType);
            } finally {
                if (in != null) {
                    in.close();
                }
            }
        } catch (IOException e) {
            throw new ConversionException(e);
        }
    }

    @Override
    public TypedOutput toBody(Object o) {
        try {
            return new JsonTypedOutput(objectMapper.writeValueAsString(o).getBytes("UTF-8"));
        } catch (IOException e) {
            throw new AssertionError(e);
        }
    }
}

@swankjesse
Copy link
Member

@AAverin can you share the URL that's impacted? I'd like to figure out if this is OkHttp misbehaving, your server, or something else. If you don't want to share it here, I'm jesse at swank.ca.

@AAverin
Copy link

AAverin commented Jan 29, 2015

When passing If-None-Match manually to my server - everything works correctly.
You can use this link for testing:
http://widget-test.myrentacar.me/api/cities/133/places?key=antonaverin&signature=7d07b01b6c09e676cc59a4f2f0ae499e

@swankjesse
Copy link
Member

Looks like the bug is we're sending both If-Modified-Since and If-None-Match, and that's causing a problem. I'm going to fix this in OkHttp to send only If-None-Match if both conditions are possible.

swankjesse added a commit that referenced this issue Jan 30, 2015
@AAverin
Copy link

AAverin commented Jan 30, 2015

Great, thanks.
Although I didn't see these headers in logs when set up as networkInterceptor()
Anyways, if there is any more information I can provide with my setup - feel free to ask.

swankjesse added a commit that referenced this issue Jan 30, 2015
@crossle
Copy link

crossle commented Jan 31, 2015

When release include this issue fix?

@swankjesse
Copy link
Member

February.

@crossle
Copy link

crossle commented Mar 2, 2015

@swankjesse When release include this issue fix?

@swankjesse
Copy link
Member

The plan is to do 2.3 in March, but no promises.

@crossle
Copy link

crossle commented Mar 2, 2015

Maybe publish 2.2.1?

@JakeWharton
Copy link
Member

You can use 2.3.0-SNAPSHOT for now.
On Mar 2, 2015 5:09 AM, "Crossle Song" notifications@github.com wrote:

Maybe publish 2.2.1?


Reply to this email directly or view it on GitHub
#831 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants