Skip to content
This repository was archived by the owner on Aug 8, 2023. It is now read-only.

have a more explicit Response representation for "304 Not Modified" - #3516

Merged
jfirebaugh merged 5 commits into
masterfrom
simplify-cache
Jan 14, 2016
Merged

jfirebaugh merged 5 commits into
masterfrom
simplify-cache

Conversation

@jfirebaugh

Copy link
Copy Markdown
Contributor

Currently, the way that a 304 Not Modified response gets encoded into Response is that the std::shared_ptr<const std::string> data from the previous response is copied into the new one. This requires a subtle invariant to be preserved by OnlineFileSource, the resulting pointer equality comparisons can be mistaken for a bug, and it means that the raw response body must remain in memory in addition to any parsed representation.

Instead, we should have a way for a 304 Not Modified response to be more explicitly encoded into Response, and consumers should only have to maintain the etag and expires values to make a revalidating request and check if things changed.

cc @kkaefer @tmpsantos

@kkaefer

kkaefer commented Jan 13, 2016

Copy link
Copy Markdown
Member

This gets unlocked when #2826 is landed (for coalescing, we need to keep the response around anyway).

@jfirebaugh

Copy link
Copy Markdown
Contributor Author

Converted to PR. CC @kkaefer, this should simplify #2721 a bit.

@tmpsantos

Copy link
Copy Markdown
Contributor

LGTM

@jfirebaugh
jfirebaugh merged commit 0066637 into master Jan 14, 2016
@jfirebaugh
jfirebaugh deleted the simplify-cache branch January 14, 2016 22:33
jfirebaugh referenced this pull request Jan 14, 2016
We're now supporting using stale TileJSON and GeoJSON data. When we receive a new answer with an updated TileJSON file, we're replacing the Source's metadata with the new one and trigger updates to make sure we're loading the correct tiles. Similarly, GeoJSON data will be reparsed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants