dmitri.shuralyov.com/service/change/...

fs: Validate change.ListOptions.Filter first.

This way, we detect a bad request and return an appropriate error
before starting anything else.
dmitshur committed 2 years ago commit b6cb26e2c15cfcd53b0cd4b269f5e38d1d3ca5f2
fs/fs.go
@@ -141,24 +141,24 @@ spec as is. Addressing this will be a part of future changes.`,
 	},
 }
 
 // List changes.
 func (*Service) List(ctx context.Context, repo string, opt change.ListOptions) ([]change.Change, error) {
-	if repo != "dmitri.shuralyov.com/font/woff2" {
-		return nil, nil
-	}
 	var counts func(s change.State) bool
 	switch opt.Filter {
 	case change.FilterOpen:
 		counts = func(s change.State) bool { return s == change.OpenState }
 	case change.FilterClosedMerged:
 		counts = func(s change.State) bool { return s == change.ClosedState || s == change.MergedState }
 	case change.FilterAll:
 		counts = func(s change.State) bool { return true }
 	default:
 		// TODO: Map to 400 Bad Request HTTP error.
-		return nil, fmt.Errorf("opt.State has unsupported value %q", opt.Filter)
+		return nil, fmt.Errorf("invalid change.ListOptions.Filter value: %q", opt.Filter)
+	}
+	if repo != "dmitri.shuralyov.com/font/woff2" {
+		return nil, nil
 	}
 	var cs []change.Change
 	for _, c := range s.changes {
 		if !counts(c.State) {
 			continue
@@ -168,24 +168,24 @@ func (*Service) List(ctx context.Context, repo string, opt change.ListOptions) (
 	return cs, nil
 }
 
 // Count changes.
 func (*Service) Count(ctx context.Context, repo string, opt change.ListOptions) (uint64, error) {
-	if repo != "dmitri.shuralyov.com/font/woff2" {
-		return 0, nil
-	}
 	var counts func(s change.State) bool
 	switch opt.Filter {
 	case change.FilterOpen:
 		counts = func(s change.State) bool { return s == change.OpenState }
 	case change.FilterClosedMerged:
 		counts = func(s change.State) bool { return s == change.ClosedState || s == change.MergedState }
 	case change.FilterAll:
 		counts = func(s change.State) bool { return true }
 	default:
 		// TODO: Map to 400 Bad Request HTTP error.
-		return 0, fmt.Errorf("opt.State has unsupported value %q", opt.Filter)
+		return 0, fmt.Errorf("invalid change.ListOptions.Filter value: %q", opt.Filter)
+	}
+	if repo != "dmitri.shuralyov.com/font/woff2" {
+		return 0, nil
 	}
 	var count uint64
 	for _, c := range s.changes {
 		if !counts(c.State) {
 			continue
githubapi/githubapi.go
@@ -64,11 +64,11 @@ func (s service) List(ctx context.Context, rs string, opt change.ListOptions) ([
 		states = []githubql.PullRequestState{githubql.PullRequestStateClosed, githubql.PullRequestStateMerged}
 	case change.FilterAll:
 		states = nil // No states to filter the PRs by.
 	default:
 		// TODO: Map to 400 Bad Request HTTP error.
-		return nil, fmt.Errorf("opt.State has unsupported value %q", opt.Filter)
+		return nil, fmt.Errorf("invalid change.ListOptions.Filter value: %q", opt.Filter)
 	}
 	var q struct {
 		Repository struct {
 			PullRequests struct {
 				Nodes []struct {
@@ -135,11 +135,11 @@ func (s service) Count(ctx context.Context, rs string, opt change.ListOptions) (
 		states = []githubql.PullRequestState{githubql.PullRequestStateClosed, githubql.PullRequestStateMerged}
 	case change.FilterAll:
 		states = nil // No states to filter the PRs by.
 	default:
 		// TODO: Map to 400 Bad Request HTTP error.
-		return 0, fmt.Errorf("opt.State has unsupported value %q", opt.Filter)
+		return 0, fmt.Errorf("invalid change.ListOptions.Filter value: %q", opt.Filter)
 	}
 	var q struct {
 		Repository struct {
 			PullRequests struct {
 				TotalCount uint64