Merge pull request #265 from Karthic-Hackintosh/master

browse: Better test coverage and fix #264

middleware/browse/browse.go

@@ -242,24 +242,31 @@ func (b Browse) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) {
 		listing.Sort, listing.Order = r.URL.Query().Get("sort"), r.URL.Query().Get("order")
 
 		// If the query 'sort' or 'order' is empty, check the cookies
-		if listing.Sort == "" || listing.Order == "" {
+		if listing.Sort == "" {
 			sortCookie, sortErr := r.Cookie("sort")
-			orderCookie, orderErr := r.Cookie("order")
-
 			// if there's no sorting values in the cookies, default to "name" and "asc"
-			if sortErr != nil || orderErr != nil {
+			if sortErr != nil {
 				listing.Sort = "name"
-				listing.Order = "asc"
 			} else { // if we have values in the cookies, use them
 				listing.Sort = sortCookie.Value
-				listing.Order = orderCookie.Value
 			}
-
 		} else { // save the query value of 'sort' and 'order' as cookies
 			http.SetCookie(w, &http.Cookie{Name: "sort", Value: listing.Sort, Path: "/"})
 			http.SetCookie(w, &http.Cookie{Name: "order", Value: listing.Order, Path: "/"})
 		}
 
+		if listing.Order == "" {
+			orderCookie, orderErr := r.Cookie("order")
+			// if there's no sorting values in the cookies, default to "name" and "asc"
+			if orderErr != nil {
+				listing.Order = "asc"
+			} else { // if we have values in the cookies, use them
+				listing.Order = orderCookie.Value
+			}
+		} else { // save the query value of 'sort' and 'order' as cookies
+			http.SetCookie(w, &http.Cookie{Name: "order", Value: listing.Order, Path: "/"})
+		}
+
 		// Apply the sorting
 		listing.applySort()
 

middleware/browse/browse_test.go

@@ -129,9 +129,9 @@ func TestBrowseTemplate(t *testing.T) {
 
 	rec := httptest.NewRecorder()
 
-	b.ServeHTTP(rec, req)
+	code, err := b.ServeHTTP(rec, req)
-	if rec.Code != http.StatusOK {
+	if code != http.StatusOK {
-		t.Fatalf("Wrong status, expected %d, got %d", http.StatusOK, rec.Code)
+		t.Fatalf("Wrong status, expected %d, got %d", http.StatusOK, code)
 	}
 
 	respBody := rec.Body.String()
@@ -149,6 +149,8 @@ func TestBrowseTemplate(t *testing.T) {
 
 <a href="test2.html">test2.html</a><br>
 
+<a href="test3.html">test3.html</a><br>
+
 </body>
 </html>
 `
@@ -169,30 +171,13 @@ func TestBrowseJson(t *testing.T) {
 		Root: "./testdata",
 		Configs: []Config{
 			Config{
-				PathScope: "/photos",
+				PathScope: "/photos/",
 			},
 		},
 	}
 
-	req, err := http.NewRequest("GET", "/photos/", nil)
+	//Getting the listing from the ./testdata/photos, the listing returned will be used to validate test results
-	if err != nil {
+	file, err := os.Open(b.Root + "/photos")
-		t.Fatalf("Test: Could not create HTTP request: %v", err)
-	}
-	req.Header.Set("Accept", "application/json")
-	rec := httptest.NewRecorder()
-
-	b.ServeHTTP(rec, req)
-	if rec.Code != http.StatusOK {
-		t.Fatalf("Wrong status, expected %d, got %d", http.StatusOK, rec.Code)
-	}
-	if rec.HeaderMap.Get("Content-Type") != "application/json; charset=utf-8" {
-		t.Fatalf("Expected Content type to be application/json; charset=utf-8, but got %s ", rec.HeaderMap.Get("Content-Type"))
-	}
-
-	actualJsonResponseString := rec.Body.String()
-
-	//generating the listing to compare it with the response body
-	file, err := os.Open(b.Root + req.URL.Path)
 	if err != nil {
 		if os.IsPermission(err) {
 			t.Fatalf("Os Permission Error")
@@ -207,6 +192,7 @@ func TestBrowseJson(t *testing.T) {
 		t.Fatalf("Unable to Read Contents of the directory")
 	}
 	var fileinfos []FileInfo
+
 	for _, f := range files {
 		name := f.Name()
 
@@ -228,16 +214,99 @@ func TestBrowseJson(t *testing.T) {
 	listing := Listing{
 		Items: fileinfos,
 	}
-	listing.Sort = "name"
+	//listing obtained above and will be used for validation inside the tests
-	listing.Order = "asc"
+
-	listing.applySort()
+	tests := []struct {
+		QueryUrl       string
+		SortBy         string
+		OrderBy        string
+		Limit          int
+		shouldErr      bool
+		expectedResult []FileInfo
+	}{
+		//test case 1: testing for default sort and  order and without the limit parameter, default sort is by name and the default order is ascending
+		//without the limit query entire listing will be produced
+		{"/", "", "", -1, false, listing.Items},
+		//test case 2: limit is set to 1, orderBy and sortBy is default
+		{"/?limit=1", "", "", 1, false, listing.Items[:1]},
+		//test case 3 : if the listing request is bigger than total size of listing then it should return everything
+		{"/?limit=100000000", "", "", 100000000, false, listing.Items},
+		//testing for negative limit
+		{"/?limit=-1", "", "", -1, false, listing.Items},
+		//testing with limit set to -1 and order set to descending
+		{"/?limit=-1&order=desc", "", "desc", -1, false, listing.Items},
+		//testing with limit set to 2 and order set to descending
+		{"/?limit=2&order=desc", "", "desc", 2, false, listing.Items},
+		//testing with limit set to 3 and order set to descending
+		{"/?limit=3&order=desc", "", "desc", 3, false, listing.Items},
+		//testing with limit set to 3 and order set to ascending
+		{"/?limit=3&order=asc", "", "asc", 3, false, listing.Items},
+		//testing with limit set to 1111111 and order set to ascending
+		{"/?limit=1111111&order=asc", "", "asc", 1111111, false, listing.Items},
+		//testing with limit set to default and order set to ascending and sorting by size
+		{"/?order=asc&sort=size", "size", "asc", -1, false, listing.Items},
+		//testing with limit set to default and order set to ascending and sorting by last modified
+		{"/?order=asc&sort=time", "time", "asc", -1, false, listing.Items},
+		//testing with limit set to 1 and order set to ascending and sorting by last modified
+		{"/?order=asc&sort=time&limit=1", "time", "asc", 1, false, listing.Items},
+		//testing with limit set to -100 and order set to ascending and sorting by last modified
+		{"/?order=asc&sort=time&limit=-100", "time", "asc", -100, false, listing.Items},
+		//testing with limit set to -100 and order set to ascending and sorting by size
+		{"/?order=asc&sort=size&limit=-100", "size", "asc", -100, false, listing.Items},
+	}
+
+	for i, test := range tests {
+		var marsh []byte
+		req, err := http.NewRequest("GET", "/photos"+test.QueryUrl, nil)
+
+		if err == nil && test.shouldErr {
+			t.Errorf("Test %d didn't error, but it should have", i)
+		} else if err != nil && !test.shouldErr {
+			t.Errorf("Test %d errored, but it shouldn't have; got '%v'", i, err)
+		}
+
+		req.Header.Set("Accept", "application/json")
+		rec := httptest.NewRecorder()
+
+		code, err := b.ServeHTTP(rec, req)
+
+		if code != http.StatusOK {
+			t.Fatalf("Wrong status, expected %d, got %d", http.StatusOK, code)
+		}
+		if rec.HeaderMap.Get("Content-Type") != "application/json; charset=utf-8" {
+			t.Fatalf("Expected Content type to be application/json; charset=utf-8, but got %s ", rec.HeaderMap.Get("Content-Type"))
+		}
+
+		actualJsonResponseString := rec.Body.String()
+		copyOflisting := listing
+		if test.SortBy == "" {
+			copyOflisting.Sort = "name"
+		} else {
+			copyOflisting.Sort = test.SortBy
+		}
+		if test.OrderBy == "" {
+			copyOflisting.Order = "asc"
+		} else {
+			copyOflisting.Order = test.OrderBy
+		}
+
+		copyOflisting.applySort()
+
+		limit := test.Limit
+		if limit <= len(copyOflisting.Items) && limit > 0 {
+			marsh, err = json.Marshal(copyOflisting.Items[:limit])
+		} else { // if the 'limit' query is empty, or has the wrong value, list everything
+			marsh, err = json.Marshal(copyOflisting.Items)
+		}
 
-	marsh, err := json.Marshal(listing.Items)
 		if err != nil {
 			t.Fatalf("Unable to Marshal the listing ")
 		}
 		expectedJsonString := string(marsh)
+
 		if actualJsonResponseString != expectedJsonString {
-		t.Errorf("Json response string doesnt match the expected Json response ")
+			t.Errorf("Json response string doesnt match the expected Json response for test number %d with sort = %s , order = %s,\nExpected response %s\nActual response = %s\n ", i+1, test.SortBy, test.OrderBy, expectedJsonString, actualJsonResponseString)
+		}
+
 	}
 }

middleware/browse/testdata/photos/test3.html

@@ -0,0 +1,3 @@
+<!DOCTYPE html>
+<html>
+</html>