Skip to content

Using comma-ok pattern for rule evaluation #10

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
serroba merged 1 commit into from
Jan 2, 2026
Merged

Using comma-ok pattern for rule evaluation #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .testcoverage.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ exclude:
# Main entry points and DI wiring
paths:
- ^cmd/server/main\.go$
- ^internal/handler/routes\.go

Copilot AI Jan 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exclude pattern is missing the dollar sign anchor at the end. This pattern should use the same format as the existing pattern for consistency and to avoid accidentally matching other files that start with this path. The pattern should be ^internal/handler/routes\.go$ to match only this specific file.

Suggested change
- ^internal/handler/routes\.go
- ^internal/handler/routes\.go$

Copilot uses AI. Check for mistakes.
1 change: 1 addition & 0 deletions codecov.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ coverage:

ignore:
- "cmd/**/*"
- "internal/handler/routes.go"
12 changes: 6 additions & 6 deletions internal/flags/evaluator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@ package flags

import "strings"

type RuleMatcher func(rules []Rule, evalCtx EvalContext) *Rule
type RuleMatcher func(rules []Rule, evalCtx EvalContext) (Rule, bool)

func DefaultRuleMatcher() RuleMatcher {
return func(rules []Rule, evalCtx EvalContext) *Rule {
for i := range rules {
if matchesRule(rules[i], evalCtx) {
return &rules[i]
return func(rules []Rule, evalCtx EvalContext) (Rule, bool) {
for _, rule := range rules {
if matchesRule(rule, evalCtx) {
return rule, true
}
}

return nil
return Rule{}, false
}
}

Expand Down
96 changes: 48 additions & 48 deletions internal/flags/evaluator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ func TestRuleMatcher_ReturnsFirstMatch(t *testing.T) {
},
}

result := matcher(rules, flags.EvalContext{
result, ok := matcher(rules, flags.EvalContext{
Attrs: map[string]any{"plan": "premium"},
})

require.NotNil(t, result)
require.True(t, ok)
assert.Equal(t, "rule-2", result.ID)
}

Expand All @@ -46,21 +46,21 @@ func TestRuleMatcher_NoMatch(t *testing.T) {
},
}

result := matcher(rules, flags.EvalContext{
_, ok := matcher(rules, flags.EvalContext{
Attrs: map[string]any{"plan": "free"},
})

assert.Nil(t, result)
assert.False(t, ok)
}

func TestRuleMatcher_EmptyRules(t *testing.T) {
t.Parallel()

matcher := flags.DefaultRuleMatcher()

result := matcher([]flags.Rule{}, flags.EvalContext{})
_, ok := matcher([]flags.Rule{}, flags.EvalContext{})

assert.Nil(t, result)
assert.False(t, ok)
}

func TestRuleMatcher_OpEquals(t *testing.T) {
Expand All @@ -75,15 +75,15 @@ func TestRuleMatcher_OpEquals(t *testing.T) {
},
}

result := matcher(rules, flags.EvalContext{
_, ok := matcher(rules, flags.EvalContext{
Attrs: map[string]any{"country": "US"},
})
require.NotNil(t, result)
require.True(t, ok)

result = matcher(rules, flags.EvalContext{
_, ok = matcher(rules, flags.EvalContext{
Attrs: map[string]any{"country": "UK"},
})
assert.Nil(t, result)
assert.False(t, ok)
}

func TestRuleMatcher_OpNotEquals(t *testing.T) {
Expand All @@ -98,15 +98,15 @@ func TestRuleMatcher_OpNotEquals(t *testing.T) {
},
}

result := matcher(rules, flags.EvalContext{
_, ok := matcher(rules, flags.EvalContext{
Attrs: map[string]any{"env": "staging"},
})
require.NotNil(t, result)
require.True(t, ok)

result = matcher(rules, flags.EvalContext{
_, ok = matcher(rules, flags.EvalContext{
Attrs: map[string]any{"env": "production"},
})
assert.Nil(t, result)
assert.False(t, ok)
}

func TestRuleMatcher_OpIn(t *testing.T) {
Expand All @@ -121,15 +121,15 @@ func TestRuleMatcher_OpIn(t *testing.T) {
},
}

result := matcher(rules, flags.EvalContext{
_, ok := matcher(rules, flags.EvalContext{
Attrs: map[string]any{"region": "us-east"},
})
require.NotNil(t, result)
require.True(t, ok)

result = matcher(rules, flags.EvalContext{
_, ok = matcher(rules, flags.EvalContext{
Attrs: map[string]any{"region": "eu-west"},
})
assert.Nil(t, result)
assert.False(t, ok)
}

func TestRuleMatcher_OpNotIn(t *testing.T) {
Expand All @@ -144,15 +144,15 @@ func TestRuleMatcher_OpNotIn(t *testing.T) {
},
}

result := matcher(rules, flags.EvalContext{
_, ok := matcher(rules, flags.EvalContext{
Attrs: map[string]any{"country": "US"},
})
require.NotNil(t, result)
require.True(t, ok)

result = matcher(rules, flags.EvalContext{
_, ok = matcher(rules, flags.EvalContext{
Attrs: map[string]any{"country": "CN"},
})
assert.Nil(t, result)
assert.False(t, ok)
}

func TestRuleMatcher_OpExists(t *testing.T) {
Expand All @@ -167,15 +167,15 @@ func TestRuleMatcher_OpExists(t *testing.T) {
},
}

result := matcher(rules, flags.EvalContext{
_, ok := matcher(rules, flags.EvalContext{
Attrs: map[string]any{"beta_enabled": true},
})
require.NotNil(t, result)
require.True(t, ok)

result = matcher(rules, flags.EvalContext{
_, ok = matcher(rules, flags.EvalContext{
Attrs: map[string]any{},
})
assert.Nil(t, result)
assert.False(t, ok)
}

func TestRuleMatcher_OpStartsWith(t *testing.T) {
Expand All @@ -190,15 +190,15 @@ func TestRuleMatcher_OpStartsWith(t *testing.T) {
},
}

result := matcher(rules, flags.EvalContext{
_, ok := matcher(rules, flags.EvalContext{
Attrs: map[string]any{"email": "@internal.company.com"},
})
require.NotNil(t, result)
require.True(t, ok)

result = matcher(rules, flags.EvalContext{
_, ok = matcher(rules, flags.EvalContext{
Attrs: map[string]any{"email": "user@external.com"},
})
assert.Nil(t, result)
assert.False(t, ok)
}

func TestRuleMatcher_MultipleConditions_AllMustMatch(t *testing.T) {
Expand All @@ -217,20 +217,20 @@ func TestRuleMatcher_MultipleConditions_AllMustMatch(t *testing.T) {
},
}

result := matcher(rules, flags.EvalContext{
_, ok := matcher(rules, flags.EvalContext{
Attrs: map[string]any{"plan": "premium", "country": "US", "verified": true},
})
require.NotNil(t, result)
require.True(t, ok)

result = matcher(rules, flags.EvalContext{
_, ok = matcher(rules, flags.EvalContext{
Attrs: map[string]any{"plan": "premium", "country": "US"},
})
assert.Nil(t, result)
assert.False(t, ok)

result = matcher(rules, flags.EvalContext{
_, ok = matcher(rules, flags.EvalContext{
Attrs: map[string]any{"plan": "free", "country": "US", "verified": true},
})
assert.Nil(t, result)
assert.False(t, ok)
}

func TestRuleMatcher_BuiltInAttrs(t *testing.T) {
Expand All @@ -249,16 +249,16 @@ func TestRuleMatcher_BuiltInAttrs(t *testing.T) {
},
}

result := matcher(rules, flags.EvalContext{
result, ok := matcher(rules, flags.EvalContext{
UserID: "user-123",
})
require.NotNil(t, result)
require.True(t, ok)
assert.Equal(t, "user-rule", result.ID)

result = matcher(rules, flags.EvalContext{
result, ok = matcher(rules, flags.EvalContext{
TenantID: "tenant-456",
})
require.NotNil(t, result)
require.True(t, ok)
assert.Equal(t, "tenant-rule", result.ID)
}

Expand All @@ -274,10 +274,10 @@ func TestRuleMatcher_UnknownOp(t *testing.T) {
},
}

result := matcher(rules, flags.EvalContext{
_, ok := matcher(rules, flags.EvalContext{
Attrs: map[string]any{"x": "y"},
})
assert.Nil(t, result)
assert.False(t, ok)
}

func TestRuleMatcher_NilAttrs(t *testing.T) {
Expand All @@ -292,10 +292,10 @@ func TestRuleMatcher_NilAttrs(t *testing.T) {
},
}

result := matcher(rules, flags.EvalContext{
_, ok := matcher(rules, flags.EvalContext{
Attrs: nil,
})
assert.Nil(t, result)
assert.False(t, ok)
}

func TestRuleMatcher_OpIn_InvalidListType(t *testing.T) {
Expand All @@ -310,10 +310,10 @@ func TestRuleMatcher_OpIn_InvalidListType(t *testing.T) {
},
}

result := matcher(rules, flags.EvalContext{
_, ok := matcher(rules, flags.EvalContext{
Attrs: map[string]any{"plan": "premium"},
})
assert.Nil(t, result)
assert.False(t, ok)
}

func TestRuleMatcher_OpNotIn_InvalidListType(t *testing.T) {
Expand All @@ -328,9 +328,9 @@ func TestRuleMatcher_OpNotIn_InvalidListType(t *testing.T) {
},
}

result := matcher(rules, flags.EvalContext{
result, ok := matcher(rules, flags.EvalContext{
Attrs: map[string]any{"plan": "premium"},
})
require.NotNil(t, result)
require.True(t, ok)
assert.Equal(t, "invalid-not-in", result.ID)
}
7 changes: 2 additions & 5 deletions internal/flags/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,7 @@ type Service struct {
}

func NewService(repo Repository) *Service {
return &Service{
repo: repo,
ruleMatcher: DefaultRuleMatcher(),
}
return NewServiceWithMatcher(repo, DefaultRuleMatcher())
}

func NewServiceWithMatcher(repo Repository, matcher RuleMatcher) *Service {
Expand Down Expand Up @@ -48,7 +45,7 @@ func (s *Service) Evaluate(ctx context.Context, key string, evalCtx EvalContext)
return result, nil
}

if rule := s.ruleMatcher(flag.Rules, evalCtx); rule != nil {
if rule, ok := s.ruleMatcher(flag.Rules, evalCtx); ok {
result.Value = rule.Value
result.Reason = ReasonRuleMatch
result.RuleID = rule.ID
Expand Down
6 changes: 3 additions & 3 deletions internal/flags/service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,11 +365,11 @@ func TestService_WithCustomMatcher(t *testing.T) {
t.Parallel()

repo := flags.NewMemoryRepository()
customMatcher := func(_ []flags.Rule, _ flags.EvalContext) *flags.Rule {
return &flags.Rule{
customMatcher := func(_ []flags.Rule, _ flags.EvalContext) (flags.Rule, bool) {
return flags.Rule{
ID: "custom-rule",
Value: flags.StringValue("custom-value"),
}
}, true
}
svc := flags.NewServiceWithMatcher(repo, customMatcher)
ctx := context.Background()
Expand Down
19 changes: 0 additions & 19 deletions internal/handler/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package handler
import (
"context"
"errors"
"net/http"

"github.com/danielgtaylor/huma/v2"
"github.com/serroba/features/internal/flags"
Expand All @@ -17,24 +16,6 @@ func New(service *flags.Service) *Handler {
return &Handler{service: service}
}

func (h *Handler) Register(api huma.API) {
huma.Register(api, huma.Operation{
OperationID: "create-flag",
Method: http.MethodPost,
Path: "/flags",
Summary: "Create a new feature flag",
Tags: []string{"Flags"},
}, h.CreateFlag)

huma.Register(api, huma.Operation{
OperationID: "evaluate-flag",
Method: http.MethodPost,
Path: "/flags/{key}/evaluate",
Summary: "Evaluate a feature flag",
Tags: []string{"Flags"},
}, h.EvaluateFlag)
}

func (h *Handler) CreateFlag(ctx context.Context, req *CreateFlagRequest) (*CreateFlagResponse, error) {
flag := ToFlag(req.Body)

Expand Down
25 changes: 25 additions & 0 deletions internal/handler/routes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package handler

import (
"net/http"

"github.com/danielgtaylor/huma/v2"
)

func (h *Handler) Register(api huma.API) {
huma.Register(api, huma.Operation{
OperationID: "create-flag",
Method: http.MethodPost,
Path: "/flags",
Summary: "Create a new feature flag",
Tags: []string{"Flags"},
}, h.CreateFlag)

huma.Register(api, huma.Operation{
OperationID: "evaluate-flag",
Method: http.MethodPost,
Path: "/flags/{key}/evaluate",
Summary: "Evaluate a feature flag",
Tags: []string{"Flags"},
}, h.EvaluateFlag)
}