Skip to content

chore: refactor entitlements to be a safe object to use #14406

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
merged 7 commits into from
Aug 23, 2024
+247 −124
Merged

chore: refactor entitlements to be a safe object to use #14406

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
33b8199
chore: refactor entitlements to be a safe object to use
Emyrk Aug 22, 2024
66b363c
chore: refactor entitlements to be passable as an argument
Emyrk Aug 22, 2024
65fd791
linting
Emyrk Aug 22, 2024
8d90083
linting and fix
Emyrk Aug 22, 2024
3e44716
update test
Emyrk Aug 22, 2024
04d9b03
update to rlocks
Emyrk Aug 23, 2024
aaea397
some extra tests
Emyrk Aug 23, 2024
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
Prev Previous commit
Next Next commit
update to rlocks
  • Loading branch information
@Emyrk
Emyrk committed Aug 23, 2024
commit 04d9b03ed96fb83e097ac0ae6655515ca6f7c22a
12 changes: 6 additions & 6 deletions coderd/entitlements/entitlements.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ func (l *Set) Feature(name codersdk.FeatureName) (codersdk.Feature, bool) {
}

func (l *Set) Enabled(feature codersdk.FeatureName) bool {
Copy link
Member

Choose a reason for hiding this comment

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

Potential follow-up: we could replace this with f, ok := Features(name); ok && f.Enabled?

Copy link
Member Author

Choose a reason for hiding this comment

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

True. Because before we had access to the whole struct, our usage of it seemed a bit arbitrary at times. Sometimes we grab it and check entitled, most times just enabled.

I'm not trying to fix all our usage right now, but it would be good to audit at some times.

l.entitlementsMu.Lock()
defer l.entitlementsMu.Unlock()
l.entitlementsMu.RLock()
defer l.entitlementsMu.RUnlock()

f, ok := l.entitlements.Features[feature]
if !ok {
Expand All @@ -67,8 +67,8 @@ func (l *Set) Enabled(feature codersdk.FeatureName) bool {
// AsJSON is used to return this to the api without exposing the entitlements for
// mutation.
func (l *Set) AsJSON() json.RawMessage {
l.entitlementsMu.Lock()
defer l.entitlementsMu.Unlock()
l.entitlementsMu.RLock()
defer l.entitlementsMu.RUnlock()

b, _ := json.Marshal(l.entitlements)
return b
Expand All @@ -89,8 +89,8 @@ func (l *Set) Update(do func(entitlements *codersdk.Entitlements)) {
}

func (l *Set) FeatureChanged(featureName codersdk.FeatureName, newFeature codersdk.Feature) (initial, changed, enabled bool) {
l.entitlementsMu.Lock()
defer l.entitlementsMu.Unlock()
l.entitlementsMu.RLock()
defer l.entitlementsMu.RUnlock()

oldFeature := l.entitlements.Features[featureName]
if oldFeature.Enabled != newFeature.Enabled {
Expand Down
Loading