Skip to content

Commit ed30ebe

Browse files
committed
refactor: standardize evaluator initialization with store getters
- Updated various evaluators to utilize the new `NewEvaluatorFromStore` method, enhancing clarity in dependency management. - Introduced a `Getters` interface to streamline data retrieval for evaluators, improving maintainability and consistency across the codebase. - Refactored related tests to ensure comprehensive coverage and functionality in line with these changes.
1 parent 7be9133 commit ed30ebe

6 files changed

Lines changed: 65 additions & 30 deletions

File tree

apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/approval/approval.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,16 @@ func NewEvaluatorFromStore(store *store.Store, approvalRule *oapi.PolicyRule) ev
6666
return nil
6767
}
6868

69+
return NewEvaluator(&storeGetters{store: store}, approvalRule)
70+
}
71+
72+
func NewEvaluator(getters Getters, approvalRule *oapi.PolicyRule) evaluator.Evaluator {
73+
if approvalRule == nil || approvalRule.AnyApproval == nil || getters == nil {
74+
return nil
75+
}
76+
6977
return evaluator.WithMemoization(&AnyApprovalEvaluator{
70-
getters: &storeGetters{store: store},
78+
getters: getters,
7179
ruleId: approvalRule.Id,
7280
rule: approvalRule.AnyApproval,
7381
ruleCreatedAt: approvalRule.CreatedAt,

apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/deployableversions/deployableversions.go

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,22 @@ import (
1111
var _ evaluator.Evaluator = &DeployableVersionStatusEvaluator{}
1212

1313
type DeployableVersionStatusEvaluator struct {
14-
store *store.Store
14+
getters Getters
1515
}
1616

17-
func NewEvaluator(store *store.Store) evaluator.Evaluator {
17+
func NewEvaluatorFromStore(store *store.Store) evaluator.Evaluator {
1818
if store == nil {
1919
return nil
2020
}
21+
return NewEvaluator(&storeGetters{store: store})
22+
}
23+
24+
func NewEvaluator(getters Getters) evaluator.Evaluator {
25+
if getters == nil {
26+
return nil
27+
}
2128
return evaluator.WithMemoization(&DeployableVersionStatusEvaluator{
22-
store: store,
29+
getters: getters,
2330
})
2431
}
2532

@@ -61,7 +68,7 @@ func (e *DeployableVersionStatusEvaluator) Evaluate(
6168
// Paused versions are "grandfathered in" - they can continue on targets
6269
// where they're already deployed, but cannot deploy to new targets.
6370
// Check if this paused version has an existing release for this target.
64-
releases := e.store.Releases.Items()
71+
releases := e.getters.GetReleases()
6572
for _, release := range releases {
6673
if release == nil {
6774
continue

apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/deployableversions/deployableversions_test.go

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ func setupStore() *store.Store {
2020

2121
func TestDeployableVersionStatusEvaluator_ReadyVersion(t *testing.T) {
2222
st := setupStore()
23-
eval := NewEvaluator(st)
23+
eval := NewEvaluatorFromStore(st)
2424
require.NotNil(t, eval, "evaluator should not be nil")
2525

2626
version := &oapi.DeploymentVersion{
@@ -51,7 +51,7 @@ func TestDeployableVersionStatusEvaluator_ReadyVersion(t *testing.T) {
5151

5252
func TestDeployableVersionStatusEvaluator_PausedVersionWithoutRelease(t *testing.T) {
5353
st := setupStore()
54-
eval := NewEvaluator(st)
54+
eval := NewEvaluatorFromStore(st)
5555

5656
version := &oapi.DeploymentVersion{
5757
Id: "version-2",
@@ -101,7 +101,7 @@ func TestDeployableVersionStatusEvaluator_PausedVersionWithRelease(t *testing.T)
101101
}
102102
_ = st.Releases.Upsert(ctx, release)
103103

104-
eval := NewEvaluator(st)
104+
eval := NewEvaluatorFromStore(st)
105105

106106
scope := evaluator.EvaluatorScope{
107107
Version: version,
@@ -120,7 +120,7 @@ func TestDeployableVersionStatusEvaluator_PausedVersionWithRelease(t *testing.T)
120120

121121
func TestDeployableVersionStatusEvaluator_BuildingVersion(t *testing.T) {
122122
st := setupStore()
123-
eval := NewEvaluator(st)
123+
eval := NewEvaluatorFromStore(st)
124124

125125
version := &oapi.DeploymentVersion{
126126
Id: "version-4",
@@ -150,7 +150,7 @@ func TestDeployableVersionStatusEvaluator_BuildingVersion(t *testing.T) {
150150

151151
func TestDeployableVersionStatusEvaluator_FailedVersion(t *testing.T) {
152152
st := setupStore()
153-
eval := NewEvaluator(st)
153+
eval := NewEvaluatorFromStore(st)
154154

155155
version := &oapi.DeploymentVersion{
156156
Id: "version-5",
@@ -180,7 +180,7 @@ func TestDeployableVersionStatusEvaluator_FailedVersion(t *testing.T) {
180180

181181
func TestDeployableVersionStatusEvaluator_RejectedVersion(t *testing.T) {
182182
st := setupStore()
183-
eval := NewEvaluator(st)
183+
eval := NewEvaluatorFromStore(st)
184184

185185
version := &oapi.DeploymentVersion{
186186
Id: "version-6",
@@ -210,7 +210,7 @@ func TestDeployableVersionStatusEvaluator_RejectedVersion(t *testing.T) {
210210

211211
func TestDeployableVersionStatusEvaluator_UnspecifiedVersion(t *testing.T) {
212212
st := setupStore()
213-
eval := NewEvaluator(st)
213+
eval := NewEvaluatorFromStore(st)
214214

215215
version := &oapi.DeploymentVersion{
216216
Id: "version-7",
@@ -240,7 +240,7 @@ func TestDeployableVersionStatusEvaluator_UnspecifiedVersion(t *testing.T) {
240240

241241
func TestDeployableVersionStatusEvaluator_Caching(t *testing.T) {
242242
st := setupStore()
243-
eval := NewEvaluator(st)
243+
eval := NewEvaluatorFromStore(st)
244244

245245
version := &oapi.DeploymentVersion{
246246
Id: "version-1",
@@ -278,7 +278,7 @@ func TestDeployableVersionStatusEvaluator_Caching(t *testing.T) {
278278

279279
func TestDeployableVersionStatusEvaluator_DifferentVersionsNotCached(t *testing.T) {
280280
st := setupStore()
281-
eval := NewEvaluator(st)
281+
eval := NewEvaluatorFromStore(st)
282282

283283
readyVersion := &oapi.DeploymentVersion{
284284
Id: "version-1",
@@ -319,7 +319,7 @@ func TestDeployableVersionStatusEvaluator_DifferentVersionsNotCached(t *testing.
319319

320320
func TestDeployableVersionStatusEvaluator_MissingFields(t *testing.T) {
321321
st := setupStore()
322-
eval := NewEvaluator(st)
322+
eval := NewEvaluatorFromStore(st)
323323

324324
// Scope without version - should be denied by memoization wrapper
325325
rt := &oapi.ReleaseTarget{
@@ -341,7 +341,7 @@ func TestDeployableVersionStatusEvaluator_MissingFields(t *testing.T) {
341341

342342
func TestDeployableVersionStatusEvaluator_ScopeFields(t *testing.T) {
343343
st := setupStore()
344-
eval := NewEvaluator(st)
344+
eval := NewEvaluatorFromStore(st)
345345

346346
// Verify that the evaluator declares it needs Version and ReleaseTarget
347347
scopeFields := eval.ScopeFields()
@@ -351,7 +351,7 @@ func TestDeployableVersionStatusEvaluator_ScopeFields(t *testing.T) {
351351

352352
func TestDeployableVersionStatusEvaluator_ResultStructure(t *testing.T) {
353353
st := setupStore()
354-
eval := NewEvaluator(st)
354+
eval := NewEvaluatorFromStore(st)
355355

356356
version := &oapi.DeploymentVersion{
357357
Id: "version-1",
@@ -409,7 +409,7 @@ func TestDeployableVersionStatusEvaluator_PausedVersionMultipleTargets(t *testin
409409
}
410410
_ = st.Releases.Upsert(ctx, release)
411411

412-
eval := NewEvaluator(st)
412+
eval := NewEvaluatorFromStore(st)
413413

414414
// Test target1 (has release) - should be allowed
415415
scope1 := evaluator.EvaluatorScope{
@@ -449,7 +449,7 @@ func TestDeployableVersionStatusEvaluator_StatusTransitions(t *testing.T) {
449449
Status: oapi.DeploymentVersionStatusReady,
450450
}
451451

452-
eval := NewEvaluator(st)
452+
eval := NewEvaluatorFromStore(st)
453453

454454
scope1 := evaluator.EvaluatorScope{
455455
Version: version1,
@@ -482,7 +482,7 @@ func TestDeployableVersionStatusEvaluator_StatusTransitions(t *testing.T) {
482482
_ = st.Releases.Upsert(ctx, release)
483483

484484
// Need fresh evaluator due to memoization
485-
eval = NewEvaluator(st)
485+
eval = NewEvaluatorFromStore(st)
486486
result = eval.Evaluate(ctx, scope2)
487487
assert.True(t, result.Allowed, "paused version with release should be allowed")
488488

@@ -541,7 +541,7 @@ func TestDeployableVersionStatusEvaluator_PausedWithMultipleReleases(t *testing.
541541
_ = st.Releases.Upsert(ctx, release1)
542542
_ = st.Releases.Upsert(ctx, release2)
543543

544-
eval := NewEvaluator(st)
544+
eval := NewEvaluatorFromStore(st)
545545

546546
// Paused version on target1 (has release) - allowed
547547
scope1 := evaluator.EvaluatorScope{
@@ -604,7 +604,7 @@ func TestDeployableVersionStatusEvaluator_PausedVersionDifferentEnvironments(t *
604604
}
605605
_ = st.Releases.Upsert(ctx, devRelease)
606606

607-
eval := NewEvaluator(st)
607+
eval := NewEvaluatorFromStore(st)
608608

609609
// Dev environment (has release) - allowed
610610
devScope := evaluator.EvaluatorScope{
@@ -645,7 +645,7 @@ func TestDeployableVersionStatusEvaluator_EmptyStoreHandling(t *testing.T) {
645645
}
646646

647647
// Store is empty (no releases)
648-
eval := NewEvaluator(st)
648+
eval := NewEvaluatorFromStore(st)
649649

650650
scope := evaluator.EvaluatorScope{
651651
Version: version,
@@ -689,7 +689,7 @@ func TestDeployableVersionStatusEvaluator_WrongVersionRelease(t *testing.T) {
689689
}
690690
_ = st.Releases.Upsert(ctx, release)
691691

692-
eval := NewEvaluator(st)
692+
eval := NewEvaluatorFromStore(st)
693693

694694
scope := evaluator.EvaluatorScope{
695695
Version: pausedVersion,
@@ -734,7 +734,7 @@ func TestDeployableVersionStatusEvaluator_PausedVersionCaching(t *testing.T) {
734734
}
735735
_ = st.Releases.Upsert(ctx, release)
736736

737-
eval := NewEvaluator(st)
737+
eval := NewEvaluatorFromStore(st)
738738

739739
// Evaluate for target1 twice - should be cached
740740
scope1a := evaluator.EvaluatorScope{
@@ -795,7 +795,7 @@ func TestDeployableVersionStatusEvaluator_AllStatusesComprehensive(t *testing.T)
795795
{oapi.DeploymentVersionStatusPaused, false, "paused without release should be denied"},
796796
}
797797

798-
eval := NewEvaluator(st)
798+
eval := NewEvaluatorFromStore(st)
799799

800800
for i, tc := range statuses {
801801
t.Run(string(tc.status), func(t *testing.T) {
@@ -823,13 +823,13 @@ func TestDeployableVersionStatusEvaluator_AllStatusesComprehensive(t *testing.T)
823823
_ = st.Releases.Upsert(ctx, release)
824824

825825
// Need fresh evaluator due to memoization
826-
evalWithRelease := NewEvaluator(st)
826+
evalWithRelease := NewEvaluatorFromStore(st)
827827
resultWithRelease := evalWithRelease.Evaluate(ctx, scope)
828828
assert.True(t, resultWithRelease.Allowed, "paused with release should be allowed")
829829

830830
// Clean up for next iteration
831831
st = setupStore()
832-
eval = NewEvaluator(st)
832+
eval = NewEvaluatorFromStore(st)
833833
}
834834

835835
// Verify all results have proper details
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
package deployableversions
2+
3+
import (
4+
"workspace-engine/pkg/oapi"
5+
"workspace-engine/pkg/workspace/store"
6+
)
7+
8+
type Getters interface {
9+
GetReleases() map[string]*oapi.Release
10+
}
11+
12+
var _ Getters = (*storeGetters)(nil)
13+
14+
type storeGetters struct {
15+
store *store.Store
16+
}
17+
18+
func (s *storeGetters) GetReleases() map[string]*oapi.Release {
19+
return s.store.Releases.Items()
20+
}

apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluators.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616

1717
func EvaluatorsForPolicy(store *store.Store, rule *oapi.PolicyRule) []evaluator.Evaluator {
1818
return evaluator.CollectEvaluators(
19-
deployableversions.NewEvaluator(store),
19+
deployableversions.NewEvaluatorFromStore(store),
2020
approval.NewEvaluatorFromStore(store, rule),
2121
environmentprogression.NewEvaluatorFromStore(store, rule),
2222
gradualrollout.NewEvaluatorFromStore(store, rule),

apps/workspace-engine/pkg/workspace/releasemanager/policy/policymanager.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ func (m *Manager) PlannerPolicyEvaluators(rule *oapi.PolicyRule) []evaluator.Eva
5454

5555
func (m *Manager) PlannerGlobalEvaluators() []evaluator.Evaluator {
5656
return evaluator.CollectEvaluators(
57-
deployableversions.NewEvaluator(m.store),
57+
deployableversions.NewEvaluatorFromStore(m.store),
5858
)
5959
}
6060

0 commit comments

Comments
 (0)