refactor: eliminate report.ReadBinary() in favour of MakeFromBinary()

The signature of MakeFromFile changed to return a pointer for
consistency.

app/benchmark_internal_test.go

@@ -34,7 +34,7 @@ func readReportFiles(b *testing.B, path string) []report.Report {
 			if err != nil {
 				return err
 			}
-			reports = append(reports, rpt)
+			reports = append(reports, *rpt)
 			return nil
 		}); err != nil {
 		b.Fatal(err)

app/collector.go

@@ -282,7 +282,7 @@ func NewFileCollector(path string, window time.Duration) (Collector, error) {
 			if err != nil {
 				return err
 			}
-			reports = append(reports, rpt)
+			reports = append(reports, *rpt)
 			return nil
 		}); err != nil {
 		return nil, err

app/multitenant/memcache_client.go

@@ -184,14 +184,13 @@ func (c *MemcacheClient) FetchReports(ctx context.Context, keys []string) (map[s
 			continue
 		}
 		go func(key string) {
-			rep := report.MakeReport()
-			err := rep.ReadBinary(ctx, bytes.NewBuffer(item.Value), true, true)
+			rep, err := report.MakeFromBinary(ctx, bytes.NewBuffer(item.Value), true, true)
 			if err != nil {
 				log.Warningf("Corrupt report in memcache %v: %v", key, err)
 				ch <- result{key: key}
 				return
 			}
-			ch <- result{key: key, report: &rep}
+			ch <- result{key: key, report: rep}
 		}(key)
 	}
 

app/multitenant/s3_client.go

@@ -87,7 +87,7 @@ func (store *S3Store) fetchReport(ctx context.Context, key string) (*report.Repo
 		return nil, err
 	}
 	defer resp.Body.Close()
-	return report.MakeFromBinary(ctx, resp.Body)
+	return report.MakeFromBinary(ctx, resp.Body, true, true)
 }
 
 // StoreReportBytes stores a report.

app/router.go

@@ -116,7 +116,6 @@ func RegisterReportPostHandler(a Adder, router *mux.Router) {
 	post := router.Methods("POST").Subrouter()
 	post.HandleFunc("/api/report", requestContextDecorator(func(ctx context.Context, w http.ResponseWriter, r *http.Request) {
 		var (
-			rpt    report.Report
 			buf    = &bytes.Buffer{}
 			reader = io.TeeReader(r.Body, buf)
 		)
@@ -138,7 +137,8 @@ func RegisterReportPostHandler(a Adder, router *mux.Router) {
 			return
 		}
 
-		if err := rpt.ReadBinary(ctx, reader, gzipped, isMsgpack); err != nil {
+		rpt, err := report.MakeFromBinary(ctx, reader, gzipped, isMsgpack)
+		if err != nil {
 			respondWith(w, http.StatusBadRequest, err)
 			return
 		}
@@ -148,7 +148,7 @@ func RegisterReportPostHandler(a Adder, router *mux.Router) {
 			buf, _ = rpt.WriteBinary()
 		}
 
-		if err := a.Add(ctx, rpt, buf.Bytes()); err != nil {
+		if err := a.Add(ctx, *rpt, buf.Bytes()); err != nil {
 			log.Errorf("Error Adding report: %v", err)
 			respondWith(w, http.StatusInternalServerError, err)
 			return

report/marshal.go

@@ -72,11 +72,11 @@ var gzipWriterPool = &sync.Pool{
 	New: func() interface{} { w, _ := gzip.NewWriterLevel(nil, gzip.DefaultCompression); return w },
 }
 
-// ReadBinary reads bytes into a Report.
+// MakeFromBinary constructs a Report from binary data.
 //
 // Will decompress the binary if gzipped is true, and decode as
 // msgpack if true, otherwise JSON
-func (rep *Report) ReadBinary(ctx context.Context, r io.Reader, gzipped bool, msgpack bool) error {
+func MakeFromBinary(ctx context.Context, r io.Reader, gzipped bool, msgpack bool) (*Report, error) {
 	span, ctx := opentracing.StartSpanFromContext(ctx, "report.ReadBinary")
 	defer span.Finish()
 	var err error
@@ -91,7 +91,7 @@ func (rep *Report) ReadBinary(ctx context.Context, r io.Reader, gzipped bool, ms
 	if gzipped {
 		r, err = gzip.NewReader(r)
 		if err != nil {
-			return err
+			return nil, err
 		}
 	}
 	// Read everything into memory before decoding: it's faster
@@ -100,10 +100,11 @@ func (rep *Report) ReadBinary(ctx context.Context, r io.Reader, gzipped bool, ms
 	defer bufferPool.Put(buf)
 	uncompressedSize, err := buf.ReadFrom(r)
 	if err != nil {
-		return err
+		return nil, err
 	}
+	rep := MakeReport()
 	if err := codec.NewDecoderBytes(buf.Bytes(), codecHandle(msgpack)).Decode(&rep); err != nil {
-		return err
+		return nil, err
 	}
 	log.Debugf(
 		"Received report sizes: compressed %d bytes, uncompressed %d bytes (%.2f%%)",
@@ -112,22 +113,13 @@ func (rep *Report) ReadBinary(ctx context.Context, r io.Reader, gzipped bool, ms
 		float32(compressedSize)/float32(uncompressedSize)*100,
 	)
 	span.LogFields(otlog.Uint64("compressedSize", compressedSize), otlog.Int64("uncompressedSize", uncompressedSize))
-	return nil
-}
-
-// MakeFromBinary constructs a Report from a gzipped msgpack.
-func MakeFromBinary(ctx context.Context, r io.Reader) (*Report, error) {
-	rep := MakeReport()
-	if err := rep.ReadBinary(ctx, r, true, true); err != nil {
-		return nil, err
-	}
 	return &rep, nil
 }
 
 // MakeFromFile construct a Report from a file, with the encoding
 // determined by the extension (".msgpack" or ".json", with an
 // optional ".gz").
-func MakeFromFile(ctx context.Context, path string) (rpt Report, _ error) {
+func MakeFromFile(ctx context.Context, path string) (rpt *Report, _ error) {
 	f, err := os.Open(path)
 	if err != nil {
 		return rpt, err
@@ -139,8 +131,7 @@ func MakeFromFile(ctx context.Context, path string) (rpt Report, _ error) {
 		return rpt, err
 	}
 
-	err = rpt.ReadBinary(ctx, f, gzipped, msgpack)
-	return rpt, err
+	return MakeFromBinary(ctx, f, gzipped, msgpack)
 }
 
 // WriteToFile writes a Report to a file. The encoding is determined

report/marshal_test.go

@@ -1,7 +1,6 @@
 package report_test
 
 import (
-	"bytes"
 	"context"
 	"reflect"
 	"testing"
@@ -15,22 +14,13 @@ import (
 func TestRoundtrip(t *testing.T) {
 	r1 := report.MakeReport()
 	buf, _ := r1.WriteBinary()
-	original := append([]byte{}, buf.Bytes()...) // copy the contents for later
-	r2, err := report.MakeFromBinary(context.Background(), buf)
+	r2, err := report.MakeFromBinary(context.Background(), buf, true, true)
 	if err != nil {
 		t.Error(err)
 	}
 	if !reflect.DeepEqual(r1, *r2) {
 		t.Errorf("%v != %v", r1, *r2)
 	}
-	r3 := report.MakeReport()
-	err = r3.ReadBinary(context.Background(), bytes.NewBuffer(original), true, true)
-	if err != nil {
-		t.Error(err)
-	}
-	if !reflect.DeepEqual(r1, r3) {
-		t.Errorf("%v != %v", r1, r3)
-	}
 }
 
 // Create a Report for test purposes that contains about one of
@@ -74,7 +64,7 @@ func makeTestReport() report.Report {
 func TestBiggerRoundtrip(t *testing.T) {
 	r1 := makeTestReport()
 	buf, _ := r1.WriteBinary()
-	r2, err := report.MakeFromBinary(context.Background(), buf)
+	r2, err := report.MakeFromBinary(context.Background(), buf, true, true)
 	if err != nil {
 		t.Error(err)
 	}