[GitHub] [trafficcontrol] srijeet0406 commented on a change in pull request #4721: Queries with invalid type parameter should return 400s, not 500s

[GitBox <gi...@apache.org>]

srijeet0406 commented on a change in pull request #4721:
URL: https://github.com/apache/trafficcontrol/pull/4721#discussion_r432602092



##########
File path: traffic_ops/traffic_ops_golang/cachegroup/cachegroups_test.go
##########
@@ -272,3 +273,73 @@ func TestValidate(t *testing.T) {
 		t.Errorf("expected nil, got %s", err)
 	}
 }
+
+func TestBadTypeParamCacheGroups(t *testing.T) {
+	detail := `cachegroup read: converting cachegroup type to integer strconv.Atoi: parsing "wrong": invalid syntax`
+	mockDB, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatalf("an error '%s' was not expected when opening a stub database connection", err)
+	}
+	defer mockDB.Close()
+
+	db := sqlx.NewDb(mockDB, "sqlmock")
+	defer db.Close()
+
+	testCGs := getTestCacheGroups()
+	rows := sqlmock.NewRows([]string{
+		"id",
+		"name",
+		"short_name",
+		"latitude",
+		"longitude",
+		"localization_methods",
+		"parent_cachegroup_id",
+		"parent_cachegroup_name",
+		"secondary_parent_cachegroup_id",
+		"secondary_parent_cachegroup_name",
+		"type_name",
+		"type_id",
+		"last_updated",
+		"fallbacks",
+		"fallbackToClosest",
+	})
+
+	for _, ts := range testCGs {
+		rows = rows.AddRow(
+			ts.ID,
+			ts.Name,
+			ts.ShortName,
+			ts.Latitude,
+			ts.Longitude,
+			[]byte("{DEEP_CZ,CZ,GEO}"),
+			ts.ParentCachegroupID,
+			ts.ParentName,
+			ts.SecondaryParentCachegroupID,
+			ts.SecondaryParentName,
+			ts.Type,
+			ts.TypeID,
+			ts.LastUpdated,
+			[]byte("{cachegroup2,cachegroup3}"),
+			ts.FallbackToClosest,
+		)
+	}
+	mock.ExpectBegin()
+	mock.ExpectQuery("SELECT").WillReturnRows(rows)
+	mock.ExpectCommit()
+
+	reqInfo := api.APIInfo{Tx: db.MustBegin(), Params: map[string]string{"type": "wrong"}}
+	obj := TOCacheGroup{
+		api.APIInfoImpl{&reqInfo},
+		tc.CacheGroupNullable{},
+	}
+	_, userErr, _, sc := obj.Read()
+	if sc != http.StatusBadRequest {
+		t.Errorf("Read expected status code: Bad Request, actual: %v ", sc)
+	}
+	if userErr == nil {
+		t.Errorf("Read expected: user error with details %v, actual: nil", detail)
+	}
+	if userErr.Error() != detail {
+		t.Errorf("Read expected: user error with details %v, actual: %v", detail, userErr.Error())

Review comment:
       Yeah, I meant to do that before creating the PR, but slipped off my mind. Its done now.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org