Skip to content

Commit

Permalink
Addressing comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
beautifulentropy committed Oct 3, 2023
1 parent f2e93de commit 88bd93a
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 19 deletions.
2 changes: 1 addition & 1 deletion ratelimits/limiter.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ func (l *Limiter) initialize(ctx context.Context, rl limit, name Name, id string
// the limit specified by name is returned. If no default limit exists for the
// specified name, ErrLimitDisabled is returned.
func (l *Limiter) getLimit(name Name, id string) (limit, error) {
if !isNameValid(name) {
if !name.isValid() {
// This should never happen. Callers should only be specifying the limit
// Name enums defined in this package.
return limit{}, fmt.Errorf("specified name enum %q, is invalid", name)
Expand Down
13 changes: 6 additions & 7 deletions ratelimits/names.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,15 @@ const (
CertificatesPerFQDNSetPerAccount
)

// isValid returns true if the Name is a valid rate limit name.
func (n Name) isValid() bool {
return n > Unknown && n < Name(len(nameToString))
}

// String returns the string representation of the Name. It allows Name to
// satisfy the fmt.Stringer interface.
func (n Name) String() string {
if !isNameValid(n) {
if !n.isValid() {
return nameToString[Unknown]
}
return nameToString[n]
Expand Down Expand Up @@ -215,9 +220,3 @@ func nameToEnumString(s Name) string {
func bucketKey(name Name, id string) string {
return nameToEnumString(name) + ":" + id
}

// isLimitNameValid returns true if the provided Name is a valid rate limit
// name.
func isNameValid(name Name) bool {
return name > Unknown && name < Name(len(nameToString))
}
4 changes: 2 additions & 2 deletions ratelimits/names_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"github.com/letsencrypt/boulder/test"
)

func TestIsNameValid(t *testing.T) {
func TestNameIsValid(t *testing.T) {
t.Parallel()
type args struct {
name Name
Expand All @@ -22,7 +22,7 @@ func TestIsNameValid(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := isNameValid(tt.args.name)
got := tt.args.name.isValid()
test.AssertEquals(t, tt.want, got)
})
}
Expand Down
26 changes: 17 additions & 9 deletions ratelimits/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,27 @@ var ErrBucketNotFound = fmt.Errorf("bucket not found")

// source is an interface for creating and modifying TATs.
type source interface {
// Set stores the TAT at the specified bucketKey ('name:id'). Contexts
// passed to this method should have a timeout or deadline set to prevent
// the operation from blocking indefinitely.
// Set stores the TAT at the specified bucketKey (formatted as 'name:id').
// Implementations MUST ensure non-blocking operations by either:
// a) applying a deadline or timeout to the context WITHIN the method, or
// b) guaranteeing the operation will not block indefinitely (e.g. via
// the underlying storage client implementation).
Set(ctx context.Context, bucketKey string, tat time.Time) error

// Get retrieves the TAT at the specified bucketKey ('name:id'). Contexts
// passed to this method should have a timeout or deadline set to prevent
// the operation from blocking indefinitely.
// Get retrieves the TAT associated with the specified bucketKey (formatted
// as 'name:id'). Implementations MUST ensure non-blocking operations by
// either:
// a) applying a deadline or timeout to the context WITHIN the method, or
// b) guaranteeing the operation will not block indefinitely (e.g. via
// the underlying storage client implementation).
Get(ctx context.Context, bucketKey string) (time.Time, error)

// Delete deletes the TAT at the specified bucketKey ('name:id'). Contexts
// passed to this method should have a timeout or deadline set to prevent
// the operation from blocking indefinitely.
// Delete removes the TAT associated with the specified bucketKey (formatted
// as 'name:id'). Implementations MUST ensure non-blocking operations by
// either:
// a) applying a deadline or timeout to the context WITHIN the method, or
// b) guaranteeing the operation will not block indefinitely (e.g. via
// the underlying storage client implementation).
Delete(ctx context.Context, bucketKey string) error
}

Expand Down

0 comments on commit 88bd93a

Please sign in to comment.