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 6 years ago commit b6cb26e2c15cfcd53b0cd4b269f5e38d1d3ca5f2
Collapse all
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