Repository: gomods/athens
Pull request #1637 (2020-06-26 02:54:58)

actions: add index handler tests
Following up on the previous /index addition to Athens, this PR adds unit tests to the /index http handler. 

It also adds better logging and error handling to the handler itself. 

marwan-at-work: Going to merge this PR since it's just tests. But please feel free to review and happy to make follow up PRs :pray: 

Patch:
diff --git a/cmd/proxy/actions/index.go b/cmd/proxy/actions/index.go
index 3be942c0..19fce99e 100644
--- a/cmd/proxy/actions/index.go
+++ b/cmd/proxy/actions/index.go
@@ -2,51 +2,60 @@ package actions
 
 import (
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"strconv"
 	"time"
 
 	"github.com/gomods/athens/pkg/errors"
 	"github.com/gomods/athens/pkg/index"
+	"github.com/gomods/athens/pkg/log"
+	"github.com/sirupsen/logrus"
 )
 
 // indexHandler implements GET baseURL/index
 func indexHandler(index index.Indexer) http.HandlerFunc {
-	const op errors.Op = "actions.IndexHandler"
 	return func(w http.ResponseWriter, r *http.Request) {
-		var (
-			err   error
-			limit int
-			since time.Time
-		)
-		if limitStr := r.FormValue("limit"); limitStr != "" {
-			limit, err = strconv.Atoi(limitStr)
-			if err != nil {
-				http.Error(w, err.Error(), http.StatusBadRequest)
-				return
-			}
-		}
-		if sinceStr := r.FormValue("since"); sinceStr != "" {
-			since, err = time.Parse(time.RFC3339, sinceStr)
-			if err != nil {
-				http.Error(w, err.Error(), http.StatusBadRequest)
-				return
-			}
-		}
-		if limit <= 0 {
-			limit = 2000
-		}
-		list, err := index.Lines(r.Context(), since, limit)
+		ctx := r.Context()
+		list, err := getIndexLines(r, index)
 		if err != nil {
-			http.Error(w, err.Error(), 500)
+			log.EntryFromContext(ctx).SystemErr(err)
+			http.Error(w, err.Error(), errors.Kind(err))
 			return
 		}
 		enc := json.NewEncoder(w)
 		for _, meta := range list {
 			if err = enc.Encode(meta); err != nil {
-				http.Error(w, err.Error(), 500)
+				log.EntryFromContext(ctx).SystemErr(err)
+				fmt.Fprintln(w, err)
 				return
 			}
 		}
 	}
 }
+
+func getIndexLines(r *http.Request, index index.Indexer) ([]*index.Line, error) {
+	const op errors.Op = "actions.IndexHandler"
+	var (
+		err   error
+		limit = 2000
+		since time.Time
+	)
+	if limitStr := r.FormValue("limit"); limitStr != "" {
+		limit, err = strconv.Atoi(limitStr)
+		if err != nil || limit <= 0 {
+			return nil, errors.E(op, err, errors.KindBadRequest, logrus.InfoLevel)
+		}
+	}
+	if sinceStr := r.FormValue("since"); sinceStr != "" {
+		since, err = time.Parse(time.RFC3339, sinceStr)
+		if err != nil {
+			return nil, errors.E(op, err, errors.KindBadRequest, logrus.InfoLevel)
+		}
+	}
+	list, err := index.Lines(r.Context(), since, limit)
+	if err != nil {
+		return nil, errors.E(op, err)
+	}
+	return list, nil
+}


Test patch:
diff --git a/cmd/proxy/actions/index_test.go b/cmd/proxy/actions/index_test.go
new file mode 100644
index 00000000..fdb1bf0d
--- /dev/null
+++ b/cmd/proxy/actions/index_test.go
@@ -0,0 +1,124 @@
+package actions
+
+import (
+	"context"
+	"fmt"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+	"time"
+
+	"github.com/gomods/athens/pkg/index"
+)
+
+var indexHandlerTests = []struct {
+	name  string
+	desc  string
+	lines []*index.Line
+	err   error
+	limit string
+	since string
+	code  int
+}{
+	{
+		name: "happy path",
+		desc: "given no params and 1 line, the handler should return 200 along with the index line",
+		lines: []*index.Line{
+			{
+				Path:      "github.com/pkg/errors",
+				Version:   "v0.9.1",
+				Timestamp: time.Now(),
+			},
+		},
+		code: 200,
+	},
+	{
+		name: "valid limit",
+		desc: "given a valid limit number, the handler should return 200",
+		lines: []*index.Line{
+			{
+				Path:      "github.com/pkg/errors",
+				Version:   "v0.9.1",
+				Timestamp: time.Now(),
+			},
+		},
+		limit: "1",
+		code:  200,
+	},
+	{
+		name: "valid since",
+		desc: "given a valid since string, the handler should return 200",
+		lines: []*index.Line{
+			{
+				Path:      "github.com/pkg/errors",
+				Version:   "v0.9.1",
+				Timestamp: time.Now(),
+			},
+		},
+		limit: "1",
+		since: time.Now().Add(-time.Hour).Format(time.RFC3339),
+		code:  200,
+	},
+	{
+		name:  "invalid limit",
+		desc:  "a limit query param must be a valid integer",
+		limit: "im not an integer",
+		code:  400,
+	},
+	{
+		name:  "limit too low",
+		desc:  "a limit query cannot be a negative number",
+		limit: "-1",
+		code:  400,
+	},
+	{
+		name:  "invalid zero limit",
+		desc:  "a limit cannot be 0",
+		limit: "0",
+		code:  400,
+	},
+	{
+		name:  "invalid since",
+		desc:  "since must be a valid RFC3339 format",
+		since: time.Now().Format(time.RFC822),
+		code:  400,
+	},
+	{
+		name: "index error",
+		desc: "given an underlying index error, the handler must return 500",
+		err:  fmt.Errorf("internal error"),
+		code: 500,
+	},
+}
+
+func TestIndexHandler(t *testing.T) {
+	for _, tc := range indexHandlerTests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Log(tc.desc)
+			req := httptest.NewRequest("GET", "/index", nil)
+			q := url.Values{}
+			q.Set("limit", tc.limit)
+			q.Set("since", tc.since)
+			req.URL.RawQuery = q.Encode()
+			w := httptest.NewRecorder()
+			mi := &mockIndexer{lines: tc.lines, err: tc.err}
+			handler := indexHandler(mi)
+			handler(w, req)
+			if w.Code != tc.code {
+				t.Fatalf("expected response code to be %d but got %d", tc.code, w.Code)
+			}
+
+		})
+	}
+}
+
+type mockIndexer struct {
+	index.Indexer
+
+	lines []*index.Line
+	err   error
+}
+
+func (mi *mockIndexer) Lines(ctx context.Context, since time.Time, limit int) ([]*index.Line, error) {
+	return mi.lines, mi.err
+}