Skip to content

Commit

Permalink
address PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
mikemountain committed Dec 18, 2023
1 parent f19aa92 commit 424afbf
Show file tree
Hide file tree
Showing 16 changed files with 436 additions and 488 deletions.
2 changes: 1 addition & 1 deletion internal/auth/auth_method.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ type AuthMethodListQueryResult struct {
}

func (am *AuthMethodListQueryResult) toAuthMethod(ctx context.Context) (AuthMethod, error) {
const op = "credential.(*AuthMethodListQueryResult).toAuthMethod"
const op = "auth.(*AuthMethodListQueryResult).toAuthMethod"

newFn, ok := subtypeRegistry.newFunc(globals.Subtype(am.Subtype))
if !ok {
Expand Down
2 changes: 1 addition & 1 deletion internal/auth/oidc/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func init() {

type authMethodHooks struct{}

// NewAuthMethod creates a new static auth method from the result
// NewAuthMethod creates a new oidc auth method from the result
func (authMethodHooks) NewAuthMethod(ctx context.Context, result *auth.AuthMethodListQueryResult) (auth.AuthMethod, error) {
delimiter := "|"

Expand Down
30 changes: 5 additions & 25 deletions internal/auth/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,21 @@ package auth

import (
"context"
"errors"

"github.com/hashicorp/boundary/internal/db"
"github.com/hashicorp/boundary/internal/pagination"
"github.com/hashicorp/boundary/internal/util"
)

// The option type is how options are passed as arugments
type Option func(*Options) error

// Options is a struct that contains all the possible auth options
type Options struct {
WithLimit int
WithStartPageAfterItem pagination.Item
WithReader db.Reader
WithWriter db.Writer
WithUnauthenticatedUser bool
}

// getDefaultOptions returns a new Options struct
func getDefaultOptions() *Options {
return &Options{}
}
Expand All @@ -38,9 +36,8 @@ func GetOpts(opts ...Option) (*Options, error) {
return newOpts, nil
}

// WithLimit provides an option to provide a limit. Intentionally allowing
// negative integers. If WithLimit < 0, then unlimited results are returned.
// If WithLimit == 0, then default limits are used for results.
// WithLimit provides an option to provide a limit.
// If WithLimit <= 0, then default limits are used for results.
func WithLimit(_ context.Context, l int) Option {
return func(o *Options) error {
o.WithLimit = l
Expand All @@ -57,23 +54,6 @@ func WithStartPageAfterItem(_ context.Context, item pagination.Item) Option {
}
}

// WithReaderWriter allows the caller to pass an inflight transaction to be used
// for all database operations. If WithReaderWriter(...) is used, then the
// caller is responsible for managing the transaction.
func WithReaderWriter(_ context.Context, r db.Reader, w db.Writer) Option {
return func(o *Options) error {
switch {
case util.IsNil(r):
return errors.New("nil reader")
case util.IsNil(w):
return errors.New("nil writer")
}
o.WithReader = r
o.WithWriter = w
return nil
}
}

// WithUnauthenticatedUser provides an option for filtering results for
// an unauthenticated users.
func WithUnauthenticatedUser(_ context.Context, enabled bool) Option {
Expand Down
44 changes: 2 additions & 42 deletions internal/auth/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,12 @@ import (
"testing"
"time"

"github.com/hashicorp/boundary/internal/db"
"github.com/hashicorp/boundary/internal/db/timestamp"
"github.com/hashicorp/boundary/internal/pagination"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

type fakeWriter struct {
db.Writer
}

type fakeReader struct {
db.Reader
}

type fakeItem struct {
pagination.Item
publicId string
Expand Down Expand Up @@ -50,7 +41,7 @@ func Test_GetOpts(t *testing.T) {
opts, err := GetOpts()
require.NoError(t, err)
testOpts := getDefaultOptions()
testOpts.WithLimit = -1
testOpts.WithLimit = 0
assert.Equal(t, opts, testOpts)

opts, err = GetOpts(WithLimit(ctx, -1))
Expand All @@ -67,41 +58,10 @@ func Test_GetOpts(t *testing.T) {
})
t.Run("WithReaderWriter", func(t *testing.T) {
t.Parallel()
ctx := context.Background()
t.Run("success", func(t *testing.T) {
t.Parallel()
opts := getDefaultOptions()
assert.Empty(t, opts.WithReader)
assert.Empty(t, opts.WithWriter)
r, w := &fakeReader{}, &fakeWriter{}
opts, err := GetOpts(WithReaderWriter(ctx, r, w))
_, err := GetOpts()
require.NoError(t, err)
assert.Equal(t, r, opts.WithReader)
assert.Equal(t, w, opts.WithWriter)
})
t.Run("nil reader", func(t *testing.T) {
t.Parallel()
w := &fakeWriter{}
_, err := GetOpts(WithReaderWriter(ctx, nil, w))
require.Error(t, err)
})
t.Run("nil interface reader", func(t *testing.T) {
t.Parallel()
w := &fakeWriter{}
_, err := GetOpts(WithReaderWriter(ctx, (*fakeReader)(nil), w))
require.Error(t, err)
})
t.Run("nil writer", func(t *testing.T) {
t.Parallel()
r := &fakeReader{}
_, err := GetOpts(WithReaderWriter(ctx, r, nil))
require.Error(t, err)
})
t.Run("nil interface writer", func(t *testing.T) {
t.Parallel()
r := &fakeReader{}
_, err := GetOpts(WithReaderWriter(ctx, r, (*fakeWriter)(nil)))
require.Error(t, err)
})
})
t.Run("WithStartPageAfterItem", func(t *testing.T) {
Expand Down
Loading

0 comments on commit 424afbf

Please sign in to comment.