From a159d20b4193e5c2a8041aa3d64776859dbc17bb Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sat, 14 Dec 2013 15:16:21 +0100 Subject: [PATCH] server: unexport some image stuff, add docs, TODOs, clean up Change-Id: I4914de33406da08a4bbf806d9d0386a285e4a8d7 --- TODO | 12 +++++++++++ pkg/server/image.go | 45 ++++++++++++++++++++-------------------- pkg/server/publish.go | 4 ++-- pkg/server/thumbcache.go | 21 +++++++++---------- pkg/server/ui.go | 7 +++++-- 5 files changed, 51 insertions(+), 38 deletions(-) diff --git a/TODO b/TODO index 88cec29c4..fd160f051 100644 --- a/TODO +++ b/TODO @@ -4,6 +4,18 @@ There are two TODO lists. This file (good for airplanes) and the online bug trac Offline list: +-- unexport more stuff from pkg/server. Cache, PublishRoots, etc. + +-- make UI handler's scaledImage cache take a jsonconfig constructor + of a sorted.KeyValue-registered type. Always slap a memory LRU in front + of it. + +-- In ImageHandler.cache, write the thumbnail out as one large blob + instead of using schema.WriteFileFromReader if the thumbnail + is smaller than blobserver.MaxBlobSize (16MB). + +-- gate in front of image resizes. limit a few at a time. + -- blobhub.go NotifyBlobReceived should do the sync hooks before firing off goroutines to notify that things were good. they might not be good yet if the hook fails. also, no need for the slice. can just fire off goroutines diff --git a/pkg/server/image.go b/pkg/server/image.go index 2157ab7eb..d3e7412b8 100644 --- a/pkg/server/image.go +++ b/pkg/server/image.go @@ -47,7 +47,7 @@ type ImageHandler struct { Cache blobserver.Storage // optional MaxWidth, MaxHeight int Square bool - sc ScaledImage // optional cache for scaled images + sc scaledImage // optional cache for scaled images } func (ih *ImageHandler) storageSeekFetcher() blob.SeekFetcher { @@ -89,7 +89,7 @@ func (ih *ImageHandler) cache(tr io.Reader, name string) (blob.Ref, error) { return br, nil } -// CacheScaled saves in the image handler's cache the scaled image read +// cacheScaled saves in the image handler's cache the scaled image read // from tr, and puts its blobref in the scaledImage under the key name. func (ih *ImageHandler) cacheScaled(tr io.Reader, name string) error { br, err := ih.cache(tr, name) @@ -121,34 +121,35 @@ func cacheKey(bref string, width int, height int) string { } // ScaledCached reads the scaled version of the image in file, -// if it is in cache. On success, the image format is returned. -func (ih *ImageHandler) scaledCached(buf *bytes.Buffer, file blob.Ref) (format string, err error) { - name := cacheKey(file.String(), ih.MaxWidth, ih.MaxHeight) - br, err := ih.sc.Get(name) +// if it is in cache and writes it to buf. +// +// On successful read and population of buf, the returned format is non-empty. +// Almost all errors are not interesting. Real errors will be logged. +func (ih *ImageHandler) scaledCached(buf *bytes.Buffer, file blob.Ref) (format string) { + key := cacheKey(file.String(), ih.MaxWidth, ih.MaxHeight) + br, err := ih.sc.Get(key) + if err == errCacheMiss { + return + } if err != nil { - return format, fmt.Errorf("%v: %v", name, err) + log.Printf("Warning: thumbnail cachekey(%q)->meta lookup error: %v", key, err) + return } fr, err := ih.cached(br) if err != nil { - return format, fmt.Errorf("No cache hit for %v: %v", br, err) + return } defer fr.Close() _, err = io.Copy(buf, fr) if err != nil { - return format, fmt.Errorf("error reading cached thumbnail %v: %v", name, err) + return } mime := magic.MIMEType(buf.Bytes()) - if mime == "" { - return format, fmt.Errorf("error with cached thumbnail %v: unknown mime type", name) + if format = strings.TrimPrefix(mime, "image/"); format == mime { + log.Printf("Warning: unescaped MIME type %q of %v file for thumbnail %q", mime, br, key) + return } - pieces := strings.Split(mime, "/") - if len(pieces) < 2 { - return format, fmt.Errorf("error with cached thumbnail %v: bogus mime type", name) - } - if pieces[0] != "image" { - return format, fmt.Errorf("error with cached thumbnail %v: not an image", name) - } - return pieces[1], nil + return format } func (ih *ImageHandler) scaleImage(buf *bytes.Buffer, file blob.Ref) (format string, err error) { @@ -221,10 +222,8 @@ func (ih *ImageHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request, fil format := "" cacheHit := false if ih.sc != nil { - format, err = ih.scaledCached(&buf, file) - if err != nil { - log.Printf("image resize: %v", err) - } else { + format = ih.scaledCached(&buf, file) + if format != "" { cacheHit = true } } diff --git a/pkg/server/publish.go b/pkg/server/publish.go index 2f7677402..2f66eabb0 100644 --- a/pkg/server/publish.go +++ b/pkg/server/publish.go @@ -56,7 +56,7 @@ type PublishHandler struct { Search *search.Handler Storage blobserver.Storage // of blobRoot Cache blobserver.Storage // or nil - sc ScaledImage // cache of scaled images, optional + sc scaledImage // cache of scaled images, optional CSSFiles []string // goTemplate is the go html template used for publishing. @@ -159,7 +159,7 @@ func newPublishFromConfig(ld blobserver.Loader, conf jsonconfig.Obj) (h http.Han ph.Cache = bs switch scType { case "lrucache": - ph.sc = NewScaledImageLRU() + ph.sc = newScaledImageLRU() case "": default: return nil, fmt.Errorf("unsupported publish handler's scType: %q ", scType) diff --git a/pkg/server/thumbcache.go b/pkg/server/thumbcache.go index 3ce5b51d9..1c4eddd3b 100644 --- a/pkg/server/thumbcache.go +++ b/pkg/server/thumbcache.go @@ -25,40 +25,39 @@ import ( const cacheSize = 1024 -// ScaledImage is a mapping between the blobref of an image and +// scaledImage is a mapping between the blobref of an image and // its scaling parameters, and the blobref of such a rescaled // version of that image. // Key will be some string containing the original full-sized image's blobref, // its target dimensions, and any possible transformations on it (e.g. cropping // it to square). This string packing should not be parsed by a ScaledImage // implementation and is not guaranteed to be stable over time. -type ScaledImage interface { - Get(key string) (blob.Ref, error) // returns ErrCacheMiss when item not in cache +type scaledImage interface { + Get(key string) (blob.Ref, error) // returns errCacheMiss when item not in cache Put(key string, br blob.Ref) error } -var ErrCacheMiss = errors.New("not in cache") +var errCacheMiss = errors.New("not in cache") -type ScaledImageLRU struct { +type scaledImageLRU struct { nameToBlob *lru.Cache // string (see key format) -> blob.Ref } -func NewScaledImageLRU() *ScaledImageLRU { - sc := &ScaledImageLRU{ +func newScaledImageLRU() scaledImage { + return &scaledImageLRU{ nameToBlob: lru.New(cacheSize), } - return sc } -func (sc *ScaledImageLRU) Get(key string) (blob.Ref, error) { +func (sc *scaledImageLRU) Get(key string) (blob.Ref, error) { br, ok := sc.nameToBlob.Get(key) if !ok { - return blob.Ref{}, ErrCacheMiss + return blob.Ref{}, errCacheMiss } return br.(blob.Ref), nil } -func (sc *ScaledImageLRU) Put(key string, br blob.Ref) error { +func (sc *scaledImageLRU) Put(key string, br blob.Ref) error { sc.nameToBlob.Add(key, br) return nil } diff --git a/pkg/server/ui.go b/pkg/server/ui.go index 5e18031ba..32b426816 100644 --- a/pkg/server/ui.go +++ b/pkg/server/ui.go @@ -72,8 +72,11 @@ type UIHandler struct { root *RootHandler sigh *signhandler.Handler // or nil + // Cache optionally specifies a cache blob server, used for + // caching image thumbnails and other emphemeral data. Cache blobserver.Storage // or nil - sc ScaledImage // cache for scaled images, optional + + sc scaledImage // optional thumbnail key->blob.Ref cache // sourceRoot optionally specifies the path to root of Camlistore's // source. If empty, the UI files must be compiled in to the @@ -150,7 +153,7 @@ func uiFromConfig(ld blobserver.Loader, conf jsonconfig.Obj) (h http.Handler, er ui.Cache = bs switch scType { case "lrucache": - ui.sc = NewScaledImageLRU() + ui.sc = newScaledImageLRU() default: return nil, fmt.Errorf("unsupported ui handler's scType: %q ", scType) }