gethandler: rewrite to not use SeekerFromStreamingFetcher; add tests

Change-Id: I785dd56f7bc7c1f113575700c74f910b4f4ea4a0

pkg/blobserver/gethandler/get.go

@@ -18,7 +18,6 @@ limitations under the License.
 package gethandler
 
 import (
-	"bytes"
 	"fmt"
 	"io"
 	"net/http"
@@ -26,10 +25,10 @@ import (
 	"regexp"
 	"strings"
 	"time"
-	"unicode/utf8"
 
 	"camlistore.org/pkg/blob"
 	"camlistore.org/pkg/httputil"
+	"camlistore.org/pkg/types"
 )
 
 var kGetPattern = regexp.MustCompile(`/camli/` + blob.Pattern + `$`)
@@ -62,9 +61,7 @@ func (h *Handler) ServeHTTP(conn http.ResponseWriter, req *http.Request) {
 
 // ServeBlobRef serves a blob.
 func ServeBlobRef(rw http.ResponseWriter, req *http.Request, blobRef blob.Ref, fetcher blob.StreamingFetcher) {
-	seekFetcher := blob.SeekerFromStreamingFetcher(fetcher)
-
-	file, size, err := seekFetcher.Fetch(blobRef)
+	rc, size, err := fetcher.FetchStreaming(blobRef)
 	switch err {
 	case nil:
 		break
@@ -76,29 +73,32 @@ func ServeBlobRef(rw http.ResponseWriter, req *http.Request, blobRef blob.Ref, f
 		httputil.ServeError(rw, req, err)
 		return
 	}
-	defer file.Close()
-	var content io.ReadSeeker = file
-
+	defer rc.Close()
 	rw.Header().Set("Content-Type", "application/octet-stream")
-	if req.Header.Get("Range") == "" {
+
+	var content io.ReadSeeker = types.NewFakeSeeker(rc, int64(size))
+	rangeHeader := req.Header.Get("Range") != ""
+	const small = 32 << 10
+	var b *blob.Blob
+	if rangeHeader || size < small {
+		// Slurp to memory, so we can actually seek on it (for Range support),
+		// or if we're going to be showing it in the browser (below).
+		b, err = blob.FromReader(blobRef, rc, size)
+		if err != nil {
+			httputil.ServeError(rw, req, err)
+			return
+		}
+		content = b.Open()
+	}
+	if !rangeHeader && size < small {
 		// If it's small and all UTF-8, assume it's text and
 		// just render it in the browser.  This is more for
 		// demos/debuggability than anything else.  It isn't
 		// part of the spec.
-		if size <= 32<<10 {
-			var buf bytes.Buffer
-			_, err := io.Copy(&buf, file)
-			if err != nil {
-				httputil.ServeError(rw, req, err)
-				return
-			}
-			if utf8.Valid(buf.Bytes()) {
-				rw.Header().Set("Content-Type", "text/plain; charset=utf-8")
-			}
-			content = bytes.NewReader(buf.Bytes())
+		if b.IsUTF8() {
+			rw.Header().Set("Content-Type", "text/plain; charset=utf-8")
 		}
 	}
-
 	http.ServeContent(rw, req, "", dummyModTime, content)
 }
 

pkg/blobserver/gethandler/get_test.go

@@ -17,6 +17,14 @@ limitations under the License.
 package gethandler
 
 import (
+	"bytes"
+	"errors"
+	"io"
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"strings"
 	"testing"
 
 	"camlistore.org/pkg/blob"
@@ -32,3 +40,107 @@ func TestBlobFromURLPath(t *testing.T) {
 		t.Fatalf("got = %v; want %v", br, want)
 	}
 }
+
+func TestServeBlobRef_UTF8(t *testing.T) {
+	testServeBlobContents(t, "foo", "text/plain; charset=utf-8")
+}
+
+func TestServeBlobRef_Binary(t *testing.T) {
+	testServeBlobContents(t, "foo\xff\x00\x80", "application/octet-stream")
+}
+
+func TestServeBlobRef_Missing(t *testing.T) {
+	rr := testServeBlobRef(nil, fetcher{})
+	if rr.Code != 404 {
+		t.Errorf("Response code = %d; want 404", rr.Code)
+	}
+}
+
+func TestServeBlobRef_Error(t *testing.T) {
+	rr := testServeBlobRef(nil, fetcher{size: -1})
+	if rr.Code != 500 {
+		t.Errorf("Response code = %d; want 500", rr.Code)
+	}
+}
+
+func TestServeBlobRef_Range(t *testing.T) {
+	req, _ := http.NewRequest("GET", "/path/isn't/used", nil)
+	req.Header.Set("Range", "bytes=0-2")
+	br := blob.MustParse("foo-000")
+	rr := httptest.NewRecorder()
+	rr.Body = new(bytes.Buffer)
+	ServeBlobRef(rr, req, br, fetcher{strings.NewReader("foobar"), 6})
+	if rr.Body.String() != "foo" {
+		t.Errorf("Got %q; want foo", rr.Body)
+	}
+}
+
+func TestServeBlobRef_Streams(t *testing.T) {
+	var whatWasRead bytes.Buffer
+	const size = 1 << 20
+	testServeBlobRef(failWriter{}, fetcher{
+		io.TeeReader(
+			strings.NewReader(strings.Repeat("x", size)),
+			&whatWasRead),
+		size,
+	})
+	if whatWasRead.Len() == size {
+		t.Errorf("handler slurped instead of streamed")
+	}
+}
+
+func testServeBlobContents(t *testing.T, contents, wantType string) {
+	rr := testServeBlobRef(nil, fetcher{strings.NewReader(contents), int64(len(contents))})
+	if rr.Code != 200 {
+		t.Errorf("Response code = %d; want 200", rr.Code)
+	}
+	if g, w := rr.HeaderMap.Get("Content-Type"), wantType; g != w {
+		t.Errorf("Content-Type = %q; want %q", g, w)
+	}
+	if rr.Body.String() != contents {
+		t.Errorf("Wrote %q; want %q", rr.Body.String(), contents)
+	}
+}
+
+func testServeBlobRef(w io.Writer, fetcher blob.Fetcher) *httptest.ResponseRecorder {
+	req, _ := http.NewRequest("GET", "/path/isn't/used", nil)
+	br := blob.MustParse("foo-123")
+
+	rr := httptest.NewRecorder()
+	rr.Body = new(bytes.Buffer)
+	var rw http.ResponseWriter = rr
+	if w != nil {
+		rw = &altWriterRecorder{io.MultiWriter(w, rr.Body), rr}
+	}
+	ServeBlobRef(rw, req, br, fetcher)
+	return rr
+}
+
+type fetcher struct {
+	r    io.Reader
+	size int64
+}
+
+func (f fetcher) FetchStreaming(br blob.Ref) (rc io.ReadCloser, size uint32, err error) {
+	if f.r == nil {
+		if f.size < 0 {
+			return nil, 0, errors.New("some other error type")
+		}
+		return nil, 0, os.ErrNotExist
+	}
+	if rc, ok := f.r.(io.ReadCloser); ok {
+		return rc, uint32(f.size), nil
+	}
+	return ioutil.NopCloser(f.r), uint32(f.size), nil
+}
+
+type altWriterRecorder struct {
+	w io.Writer
+	*httptest.ResponseRecorder
+}
+
+func (a *altWriterRecorder) Write(p []byte) (int, error) { return a.w.Write(p) }
+
+type failWriter struct{}
+
+func (failWriter) Write([]byte) (int, error) { return 0, errors.New("failed to write") }