Merge pull request #19 from VoycerAG/use-stdlib-caching

Refactor caching to use stdlib

.travis.yml

@@ -1,6 +1,7 @@
 language: go
 go:
   - 1.5
+  - 1.6
   - tip
 
 before_install:

server/config.go

@@ -53,6 +53,10 @@ func (config *Config) validateConfig() error {
 			return fmt.Errorf("The width and height of the configuration element with name \"%s\" are invalid.", element.Name)
 		}
 
+		if element.Name == "" {
+			return fmt.Errorf("Name must be set")
+		}
+
 		if element.Type == "" {
 			return fmt.Errorf("Type must be set")
 		}

server/server.go

@@ -7,7 +7,6 @@ import (
 	"io"
 	"log"
 	"net/http"
-	"time"
 
 	"github.com/VoycerAG/gridfs-image-server/server/paint"
 	"github.com/gorilla/context"
@@ -59,7 +58,7 @@ func NewImageServerWithNewRelic(config *Config, storage Storage, licenseKey stri
 				return
 			}
 
-			imageHandler(w, r, requestConfig, storage, z)
+			imageHandler(w, r, *requestConfig, storage, *z)
 		}
 	}(storage, config))
 	http.Handle("/", r)
@@ -84,121 +83,49 @@ func (i imageServer) Handler() http.Handler {
 	return i.handlerMux
 }
 
-// isModified returns true if the file must be delivered, false otherwise.
-func isModified(c Cacheable, header *http.Header) bool {
-	md5 := c.CacheIdentifier()
-	modifiedHeader := header.Get("If-Modified-Since")
-	modifiedTime := time.Now()
-
-	if modifiedHeader != "" {
-		modifiedTime, _ = time.Parse(time.RFC1123, modifiedHeader)
-	}
-
-	// normalize upload date to use the same format as the browser
-	uploadDate, _ := time.Parse(time.RFC1123, c.LastModified().Format(time.RFC1123))
-
-	if header.Get("Cache-Control") == "no-cache" {
-		log.Printf("Is modified, because caching not enabled.")
-		return true
-	}
-
-	if uploadDate.After(modifiedTime) {
-		log.Printf("Is modified, because upload date after modified date.\n")
-		return true
-	}
-
-	if md5 != header.Get("If-None-Match") {
-		log.Printf("Is modified, because md5 mismatch. %s != %s", md5, header.Get("If-None-Match"))
-		return true
-	}
-
-	log.Println("not modified")
-
-	return false
-}
-
-// setCacheHeaders sets the cache headers into the http.ResponseWriter
-func setCacheHeaders(c Cacheable, w http.ResponseWriter) {
-	w.Header().Set("Etag", c.CacheIdentifier())
-	w.Header().Set("Cache-Control", fmt.Sprintf("max-age=%d", ImageCacheDuration))
-	d, _ := time.ParseDuration(fmt.Sprintf("%ds", ImageCacheDuration))
-
-	expires := c.LastModified().Add(d)
-
-	w.Header().Set("Last-Modified", c.LastModified().Format(time.RFC1123))
-	w.Header().Set("Expires", expires.Format(time.RFC1123))
-	w.Header().Set("Date", c.LastModified().Format(time.RFC1123))
-}
-
-// imageHandler the main handler
 func imageHandler(
 	w http.ResponseWriter,
 	r *http.Request,
-	requestConfig *Configuration,
+	requestConfig Configuration,
 	storage Storage,
-	imageConfig *Config) {
+	imageConfig Config,
+) {
 	log.Printf("Request on %s", r.URL)
 
-	if imageConfig == nil {
-		log.Printf("imageConfiguration object is not set.")
-		w.WriteHeader(http.StatusInternalServerError)
-		return
-	}
-
-	resizeEntry, _ := imageConfig.GetEntryByName(requestConfig.FormatName)
-
-	var foundImage Cacheable
-	var notFoundErr error
-
-	if storage.IsValidID(requestConfig.Filename) {
-		foundImage, notFoundErr = storage.FindImageByParentID(requestConfig.Database, requestConfig.Filename, resizeEntry)
-	} else {
-		foundImage, notFoundErr = storage.FindImageByParentFilename(requestConfig.Database, requestConfig.Filename, resizeEntry)
+	respondWithImage := func(w http.ResponseWriter, r *http.Request, img Cacheable, data io.ReadSeeker) {
+		w.Header().Set("Etag", img.CacheIdentifier())
+		http.ServeContent(w, r, "", img.LastModified(), data)
+		log.Printf("%d Responding with image.\n", http.StatusOK)
 	}
 
-	found := notFoundErr == nil
+	resizeEntry, err := imageConfig.GetEntryByName(requestConfig.FormatName)
+	if err != nil { // no valid resize configuration in request
+		img, notFoundErr := getOriginalImage(requestConfig.Filename, requestConfig.Database, storage)
 
-	// case that we do not want resizing and did not find any image
-	if !found && resizeEntry == nil {
-		log.Printf("%d file not found.\n", http.StatusNotFound)
-		w.WriteHeader(http.StatusNotFound)
-		return
-	}
-
-	// we found an image but did not want resizing
-	if found {
-		if !isModified(foundImage, &r.Header) {
-			w.WriteHeader(http.StatusNotModified)
-			log.Printf("%d Returning cached image.\n", http.StatusNotModified)
+		if notFoundErr != nil {
+			log.Printf("%d file not found.\n", http.StatusNotFound)
+			w.WriteHeader(http.StatusNotFound)
 			return
 		}
 
-		setCacheHeaders(foundImage, w)
-
-		io.Copy(w, foundImage.Data())
-		foundImage.Data().Close()
+		respondWithImage(w, r, img, img.Data())
 		log.Printf("%d Image found, no resizing.\n", http.StatusOK)
 		return
 	}
 
-	if !found && resizeEntry != nil {
-		var notFoundErr error
-		if storage.IsValidID(requestConfig.Filename) {
-			foundImage, notFoundErr = storage.FindImageByParentID(requestConfig.Database, requestConfig.Filename, nil)
-		} else {
-			foundImage, notFoundErr = storage.FindImageByParentFilename(requestConfig.Database, requestConfig.Filename, nil)
-		}
+	img, notFoundErr := getResizeImage(*resizeEntry, requestConfig.Filename, requestConfig.Database, storage)
 
-		found = notFoundErr == nil
+	if notFoundErr != nil {
+		img, err := getOriginalImage(requestConfig.Filename, requestConfig.Database, storage)
 
-		if !found {
-			log.Printf("%d Could not find original image.\n", http.StatusNotFound)
+		if err != nil {
+			log.Printf("%d original file not found.\n", http.StatusNotFound)
 			w.WriteHeader(http.StatusNotFound)
 			return
 		}
 
 		customResizers := paint.GetCustomResizers()
-		controller, err := paint.NewController(foundImage.Data(), customResizers)
+		controller, err := paint.NewController(img.Data(), customResizers)
 
 		if err != nil {
 			log.Printf("%d image could not be decoded. Reason: [%s].\n", http.StatusNotFound, err.Error())
@@ -211,7 +138,6 @@ func imageHandler(
 			log.Printf("%d image could not be resized.\n", http.StatusNotFound)
 			w.WriteHeader(http.StatusNotFound)
 			return
-
 		}
 
 		var b bytes.Buffer
@@ -226,21 +152,47 @@ func imageHandler(
 			bytes.NewReader(data),
 			controller.Image().Bounds().Dx(),
 			controller.Image().Bounds().Dy(),
-			foundImage,
+			img,
 			resizeEntry,
 		)
 
 		if err != nil {
-			log.Fatal(err)
+			log.Printf("%d error %s\n", http.StatusInternalServerError, err.Error())
 			w.WriteHeader(http.StatusInternalServerError)
 			return
 		}
 
-		setCacheHeaders(targetfile, w)
-		w.Write(data)
-
+		respondWithImage(w, r, targetfile, bytes.NewReader(data))
 		log.Printf("%d image succesfully resized and returned.\n", http.StatusOK)
+
+		return
+	}
+
+	respondWithImage(w, r, img, img.Data())
+}
+
+func getResizeImage(entry Entry, filename, database string, storage Storage) (Cacheable, error) {
+	var foundImage Cacheable
+	var err error
+	if storage.IsValidID(filename) {
+		foundImage, err = storage.FindImageByParentID(database, filename, &entry)
+	} else {
+		foundImage, err = storage.FindImageByParentFilename(database, filename, &entry)
+	}
+
+	return foundImage, err
+}
+
+func getOriginalImage(filename, database string, storage Storage) (Cacheable, error) {
+	var foundImage Cacheable
+	var err error
+	if storage.IsValidID(filename) {
+		foundImage, err = storage.FindImageByParentID(database, filename, nil)
+	} else {
+		foundImage, err = storage.FindImageByParentFilename(database, filename, nil)
 	}
+
+	return foundImage, err
 }
 
 // just a static welcome handler

server/server_test.go

@@ -1,18 +1,22 @@
 package server_test
 
 import (
+	"image"
 	"io"
 	"net/http"
 	"net/http/httptest"
 	"os"
 
+	"image/jpeg"
+
 	"gopkg.in/mgo.v2"
 	"gopkg.in/mgo.v2/bson"
 
 	. "github.com/VoycerAG/gridfs-image-server/server"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+	. "github.com/sharpner/matcher"
 )
 
 const (
@@ -74,6 +78,25 @@ var _ = Describe("Server", func() {
 		return err
 	}
 
+	loadFileImage := func(filename string) (image.Image, error) {
+		fp, err := os.Open(filename)
+		if err != nil {
+			return nil, err
+		}
+
+		return jpeg.Decode(fp)
+
+	}
+
+	loadMongoImage := func(filename string, gridfs *mgo.GridFS) (image.Image, error) {
+		fp, err := gridfs.Open(filename)
+		if err != nil {
+			return nil, err
+		}
+
+		return jpeg.Decode(fp)
+	}
+
 	Context("Test basic responses", func() {
 		var (
 			rec          *httptest.ResponseRecorder
@@ -136,6 +159,23 @@ var _ = Describe("Server", func() {
 			Expect(rec.Header().Get("Etag")).ToNot(Equal(""))
 		})
 
+		It("will deliver the original image with an invalid filter", func() {
+			err := loadFixtureFile("./testdata/image.jpg", "test.jpg", gridfs, map[string]string{})
+			Expect(err).ToNot(HaveOccurred())
+			req, err := http.NewRequest("GET", "/"+databaseName+"/test.jpg?size=ruski", nil)
+			Expect(err).ToNot(HaveOccurred())
+			handler := imageServer.Handler()
+			handler.ServeHTTP(rec, req)
+			Expect(rec.Code).To(Equal(http.StatusOK))
+			Expect(len(rec.Body.Bytes())).To(BeNumerically(">", 0))
+			Expect(rec.Header().Get("Etag")).ToNot(Equal(""))
+			file, err := loadFileImage("./testdata/image.jpg")
+			Expect(err).ToNot(HaveOccurred())
+			mongoFile, err := loadMongoImage("test.jpg", gridfs)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(file).To(EqualImage(mongoFile))
+		})
+
 		It("will deliver the resized image with filter", func() {
 			err := loadFixtureFile("./testdata/image.jpg", "test.jpg", gridfs, map[string]string{})
 			Expect(err).ToNot(HaveOccurred())