Skip to content

Commit

Permalink
Merge pull request #49 from cultureamp/fix-nil-dereference
Browse files Browse the repository at this point in the history
fix: panic on repo URL mismatch
  • Loading branch information
jamestelfer authored May 27, 2024
2 parents a6b4d66 + 669f86e commit 0d766e0
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 5 deletions.
6 changes: 5 additions & 1 deletion internal/vendor/cached.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,11 @@ func Cached(ttl time.Duration) (func(PipelineTokenVendor) PipelineTokenVendor, e
return nil, err
}

cache.Set(key, *token)
// token can be nil if the vendor wishes to indicate that there's neither
// a token nor an error
if token != nil {
cache.Set(key, *token)
}

return token, nil
}
Expand Down
33 changes: 29 additions & 4 deletions internal/vendor/cached_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,29 @@ func TestCacheMissOnFirstRequest(t *testing.T) {
assert.Equal(t, "first-call", token.Token)
}

func TestCacheMissWithNilResponse(t *testing.T) {
wrapped := sequenceVendor("first-call", nil)

c, err := vendor.Cached(defaultTTL)
require.NoError(t, err)

v := c(wrapped)

// first call misses cache
token, err := v(context.Background(), jwt.BuildkiteClaims{PipelineID: "pipeline-id"}, "any-repo")
require.NoError(t, err)
assert.Equal(t, &vendor.PipelineRepositoryToken{
Token: "first-call",
RepositoryURL: "any-repo",
PipelineSlug: "pipeline-id",
}, token)

// second call misses and returns nil
token, err = v(context.Background(), jwt.BuildkiteClaims{PipelineID: "pipeline-id-not-recognized"}, "any-repo")
require.NoError(t, err)
assert.Nil(t, token)
}

func TestCacheHitOnSecondRequest(t *testing.T) {
wrapped := sequenceVendor("first-call", "second-call")

Expand Down Expand Up @@ -191,9 +214,7 @@ func (e E) Error() string {
}

// sequenceVendor returns each of the calls in sequence, either a token or an error
func sequenceVendor[T interface {
string | E
}](calls ...T) vendor.PipelineTokenVendor {
func sequenceVendor(calls ...any) vendor.PipelineTokenVendor {
callIndex := 0

return vendor.PipelineTokenVendor(func(ctx context.Context, claims jwt.BuildkiteClaims, repo string) (*vendor.PipelineRepositoryToken, error) {
Expand All @@ -204,7 +225,11 @@ func sequenceVendor[T interface {
var token *vendor.PipelineRepositoryToken
var err error

switch v := any(calls[callIndex]).(type) {
c := calls[callIndex]

switch v := any(c).(type) {
case nil:
// all nil return
case string:
token = &vendor.PipelineRepositoryToken{
Token: v,
Expand Down

0 comments on commit 0d766e0

Please sign in to comment.