server: unexport some image stuff, add docs, TODOs, clean up

Change-Id: I4914de33406da08a4bbf806d9d0386a285e4a8d7

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

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
 		}
 	}

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)

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
 }

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)
 		}