From 30c7d8859aca6a0a419daa2c8b8e865e96a524a1 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 16 Dec 2013 20:27:49 -0800 Subject: [PATCH] thumbnails: add a cache-busting URL component, support ETag And quiet noisy logging on normal write failures. We can now stress test the thumbnail generation by setting CAMLI_DISABLE_THUMB_CACHE=1 which will make all the thumbnails in the browsers be unique, and not write them to cache on the server. Then, when we're happy with the thumbnails, we just increment the thumbnailVersion string and that busts all the browser- and server-side caches. Change-Id: I3cda8e85ab8b1b0b2c9113f6dff613dfbf736028 --- pkg/images/images.go | 19 +++++++++++++++++++ pkg/search/handler.go | 4 ++-- pkg/server/image.go | 28 ++++++++++++++++++++++------ pkg/server/root.go | 12 +++++++----- server/camlistored/ui/blob_item.js | 7 ++++++- server/camlistored/ui/blobinfo.js | 4 +++- 6 files changed, 59 insertions(+), 15 deletions(-) diff --git a/pkg/images/images.go b/pkg/images/images.go index 690d21202..ed931d8e1 100644 --- a/pkg/images/images.go +++ b/pkg/images/images.go @@ -25,6 +25,7 @@ import ( "log" "os" "strconv" + "time" _ "image/gif" _ "image/png" @@ -33,6 +34,24 @@ import ( "camlistore.org/third_party/github.com/camlistore/goexif/exif" ) +var disableThumbCache, _ = strconv.ParseBool(os.Getenv("CAMLI_DISABLE_THUMB_CACHE")) + +// thumbnailVersion should be incremented whenever we want to +// invalidate the cache of previous thumbnails on the server's +// cache and in browsers. +const thumbnailVersion = "2" + +// ThumbnailVersion returns a string safe for URL query components +// which is a generation number. Whenever the thumbnailing code is +// updated, so will this string. It should be placed in some URL +// component (typically "tv"). +func ThumbnailVersion() string { + if disableThumbCache { + return fmt.Sprintf("nocache%d", time.Now().UnixNano()) + } + return thumbnailVersion +} + // Exif Orientation Tag values // http://sylvana.net/jpegcrop/exif_orientation.html const ( diff --git a/pkg/search/handler.go b/pkg/search/handler.go index 1710a1595..b6f03c253 100644 --- a/pkg/search/handler.go +++ b/pkg/search/handler.go @@ -919,8 +919,8 @@ func (b *DescribedBlob) thumbnail(thumbSize int) (path string, width, height int peer := b.peerBlob(content) if peer.File != nil { if peer.File.IsImage() { - image := fmt.Sprintf("thumbnail/%s/%s?mh=%d", peer.BlobRef, - url.QueryEscape(peer.File.FileName), thumbSize) + image := fmt.Sprintf("thumbnail/%s/%s?mh=%d&tv=%s", peer.BlobRef, + url.QueryEscape(peer.File.FileName), thumbSize, images.ThumbnailVersion()) if peer.Image != nil { mw, mh := images.ScaledDimensions( int(peer.Image.Width), int(peer.Image.Height), diff --git a/pkg/server/image.go b/pkg/server/image.go index a1448dad5..c40e9734d 100644 --- a/pkg/server/image.go +++ b/pkg/server/image.go @@ -27,6 +27,7 @@ import ( "io" "log" "net/http" + "strconv" "strings" "time" @@ -125,7 +126,7 @@ func (ih *ImageHandler) cached(fileRef blob.Ref) (*schema.FileReader, error) { // Key format: "scaled:" + bref + ":" + width "x" + height // where bref is the blobref of the unscaled image. func cacheKey(bref string, width int, height int) string { - return fmt.Sprintf("scaled:%v:%dx%d", bref, width, height) + return fmt.Sprintf("scaled:%v:%dx%d:tv%d", bref, width, height, images.ThumbnailVersion()) } // ScaledCached reads the scaled version of the image in file, @@ -254,10 +255,20 @@ func (ih *ImageHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request, fil http.Error(rw, "bogus dimensions", 400) return } - if req.Header.Get("If-Modified-Since") != "" && !disableThumbCache { - // Immutable, so any copy's a good copy. - rw.WriteHeader(http.StatusNotModified) - return + + key := cacheKey(file.String(), mw, mh) + etag := blob.SHA1FromString(key).String()[5:] + inm := req.Header.Get("If-None-Match") + if inm != "" { + if strings.Trim(inm, `"`) == etag { + rw.WriteHeader(http.StatusNotModified) + return + } + } else { + if !disableThumbCache && req.Header.Get("If-Modified-Since") != "" { + rw.WriteHeader(http.StatusNotModified) + return + } } var imageData []byte @@ -273,7 +284,6 @@ func (ih *ImageHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request, fil } if !cacheHit { - key := cacheKey(file.String(), mw, mh) imi, err := singleResize.Do(key, func() (interface{}, error) { return ih.scaleImage(file) }) @@ -295,6 +305,7 @@ func (ih *ImageHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request, fil if !disableThumbCache { h.Set("Expires", time.Now().Add(oneYear).Format(http.TimeFormat)) h.Set("Last-Modified", time.Now().Format(http.TimeFormat)) + h.Set("Etag", strconv.Quote(etag)) } h.Set("Content-Type", imageContentTypeOfFormat(format)) size := len(imageData) @@ -304,6 +315,11 @@ func (ih *ImageHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request, fil if req.Method == "GET" { n, err := rw.Write(imageData) if err != nil { + if strings.Contains(err.Error(), "broken pipe") { + // boring. + return + } + // TODO: vlog this: log.Printf("error serving thumbnail of file schema %s: %v", file, err) return } diff --git a/pkg/server/root.go b/pkg/server/root.go index d2cc7608c..6d699427e 100644 --- a/pkg/server/root.go +++ b/pkg/server/root.go @@ -28,6 +28,7 @@ import ( "camlistore.org/pkg/auth" "camlistore.org/pkg/blobserver" + "camlistore.org/pkg/images" "camlistore.org/pkg/jsonconfig" "camlistore.org/pkg/search" ) @@ -168,11 +169,12 @@ func (b byFromTo) Less(i, j int) bool { func (rh *RootHandler) serveDiscovery(rw http.ResponseWriter, req *http.Request) { m := map[string]interface{}{ - "blobRoot": rh.BlobRoot, - "searchRoot": rh.SearchRoot, - "ownerName": rh.OwnerName, - "statusRoot": rh.statusRoot, - "wsAuthToken": auth.ProcessRandom(), + "blobRoot": rh.BlobRoot, + "searchRoot": rh.SearchRoot, + "ownerName": rh.OwnerName, + "statusRoot": rh.statusRoot, + "wsAuthToken": auth.ProcessRandom(), + "thumbVersion": images.ThumbnailVersion(), } if gener, ok := rh.Storage.(blobserver.Generationer); ok { initTime, gen, err := gener.StorageGeneration() diff --git a/server/camlistored/ui/blob_item.js b/server/camlistored/ui/blob_item.js index ddc05f898..ffe57f385 100644 --- a/server/camlistored/ui/blob_item.js +++ b/server/camlistored/ui/blob_item.js @@ -253,7 +253,12 @@ camlistore.BlobItem.prototype.setThumbSize = function(w, h) { // TODO(aa): This is kind of a hack, it would be better if the server just // returned the base URL and the aspect ratio, rather than specific // dimensions. - this.thumb_.src = this.getThumbSrc_().split('?')[0] + '?mh=' + rh; + var tv = ''; + if (!!CAMLISTORE_CONFIG) { + tv = CAMLISTORE_CONFIG.thumbVersion || ''; + } + this.thumb_.src = this.getThumbSrc_().split('?')[0] + '?mh=' + rh + + '&tv=' + tv; } }; diff --git a/server/camlistored/ui/blobinfo.js b/server/camlistored/ui/blobinfo.js index 88bb32361..1fd39cf1c 100644 --- a/server/camlistored/ui/blobinfo.js +++ b/server/camlistored/ui/blobinfo.js @@ -115,6 +115,7 @@ function(bmap) { } blobmeta.innerHTML = htmlEscape(JSON.stringify(binfo, null, 2)); if (binfo.camliType || (binfo.type && binfo.type.indexOf("text/") == 0)) { + var conf = this.config_; this.connection_.getBlobContents(blobref, goog.bind(function(data) { goog.dom.getElement("blobdata").innerHTML = linkifyBlobRefs(data); @@ -139,7 +140,8 @@ function(bmap) { binfo.file.mimeType.indexOf("image/") == 0) { var thumbURL = ""; + "&mh=" + this.thumbnailSize_ + + "&tv=" + (conf.thumbVersion || '') + "'>"; goog.dom.getElement("thumbnail").innerHTML = thumbURL; } else { goog.dom.getElement("thumbnail").innerHTML = "";