From cdeea36705889ba6d9c14bad22a4240f1385aa4b Mon Sep 17 00:00:00 2001 From: Aditya Choudhari Date: Thu, 14 Aug 2025 13:14:38 -0700 Subject: [PATCH 1/8] chore: implement version any approval rule --- .../pkg/engine/policy/rules/rule.go | 13 + .../version_any_approval_record_repository.go | 278 ++++++++++++++ ...ion_any_approval_record_repository_test.go | 344 ++++++++++++++++++ .../version_any_approval_rule.go | 66 ++++ .../version_any_approval_rule_test.go | 123 +++++++ 5 files changed, 824 insertions(+) create mode 100644 apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go create mode 100644 apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository_test.go create mode 100644 apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_rule.go create mode 100644 apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_rule_test.go diff --git a/apps/workspace-engine/pkg/engine/policy/rules/rule.go b/apps/workspace-engine/pkg/engine/policy/rules/rule.go index 74b92cf7e..04fe8022a 100644 --- a/apps/workspace-engine/pkg/engine/policy/rules/rule.go +++ b/apps/workspace-engine/pkg/engine/policy/rules/rule.go @@ -104,3 +104,16 @@ type Rule interface { // request-scoped values needed during evaluation. Evaluate(ctx context.Context, target rt.ReleaseTarget, version deployment.DeploymentVersion) (*RuleEvaluationResult, error) } + +type BaseRule struct { + ID string + PolicyID string +} + +func (r *BaseRule) GetID() string { + return r.ID +} + +func (r *BaseRule) GetPolicyID() string { + return r.PolicyID +} diff --git a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go new file mode 100644 index 000000000..ae4135d49 --- /dev/null +++ b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go @@ -0,0 +1,278 @@ +package versionanyapproval + +import ( + "context" + "fmt" + "sort" + "sync" + "time" + "workspace-engine/pkg/model" +) + +type VersionAnyApprovalRecordStatus string + +const ( + VersionAnyApprovalRecordStatusApproved VersionAnyApprovalRecordStatus = "approved" + VersionAnyApprovalRecordStatusRejected VersionAnyApprovalRecordStatus = "rejected" +) + +// VersionAnyApprovalRecord represents a record for the version-any-approval rule. +// It is used to track the approval status of a version for a given environment, for use by the version-any-approval rule. +// There can only be one record per version and environment per user. +type VersionAnyApprovalRecord struct { + ID string + VersionID string + EnvironmentID string + UserID string + Status VersionAnyApprovalRecordStatus + ApprovedAt *time.Time + Reason *string + CreatedAt time.Time + UpdatedAt time.Time +} + +func (v VersionAnyApprovalRecord) GetID() string { + return v.ID +} + +var _ model.Repository[VersionAnyApprovalRecord] = (*VersionAnyApprovalRecordRepository)(nil) + +// VersionAnyApprovalRecordRepository is a repository for all version-any-approval records. +type VersionAnyApprovalRecordRepository struct { + records map[string]map[string][]*VersionAnyApprovalRecord // versionID -> environmentID -> array of records + mu sync.RWMutex +} + +func NewVersionAnyApprovalRecordRepository() *VersionAnyApprovalRecordRepository { + return &VersionAnyApprovalRecordRepository{ + records: make(map[string]map[string][]*VersionAnyApprovalRecord), + } +} + +func (v *VersionAnyApprovalRecordRepository) Create(ctx context.Context, record *VersionAnyApprovalRecord) error { + v.mu.Lock() + defer v.mu.Unlock() + + if record == nil { + return fmt.Errorf("record is nil") + } + + environmentID := record.EnvironmentID + versionID := record.VersionID + + if _, ok := v.records[versionID]; !ok { + v.records[versionID] = make(map[string][]*VersionAnyApprovalRecord) + } + + if _, ok := v.records[versionID][environmentID]; !ok { + v.records[versionID][environmentID] = make([]*VersionAnyApprovalRecord, 0) + } + + currentRecords := v.records[versionID][environmentID] + for _, r := range currentRecords { + if r == nil { + continue + } + + if r.ID == record.ID { + return fmt.Errorf("record already exists") + } + + if r.UserID == record.UserID { + return fmt.Errorf("record already exists for user") + } + } + + if record.CreatedAt.IsZero() { + record.CreatedAt = time.Now().UTC() + } + if record.UpdatedAt.IsZero() { + record.UpdatedAt = time.Now().UTC() + } + + v.records[versionID][environmentID] = append(currentRecords, record) + return nil +} + +func (v *VersionAnyApprovalRecordRepository) Delete(ctx context.Context, recordID string) error { + v.mu.Lock() + defer v.mu.Unlock() + + for versionID, environmentRecords := range v.records { + for environmentID, records := range environmentRecords { + for _, r := range records { + if r == nil { + continue + } + + if r.ID != recordID { + continue + } + + newRecords := make([]*VersionAnyApprovalRecord, 0, len(records)-1) + for _, r := range records { + if r.ID != recordID { + newRecords = append(newRecords, r) + } + } + v.records[versionID][environmentID] = newRecords + } + } + } + + return nil +} + +func (v *VersionAnyApprovalRecordRepository) Update(ctx context.Context, record *VersionAnyApprovalRecord) error { + v.mu.Lock() + defer v.mu.Unlock() + + if record == nil { + return fmt.Errorf("record is nil") + } + + environmentID := record.EnvironmentID + versionID := record.VersionID + + if _, ok := v.records[versionID]; !ok { + return fmt.Errorf("version not found") + } + + if _, ok := v.records[versionID][environmentID]; !ok { + return fmt.Errorf("environment not found") + } + + currentRecords := v.records[versionID][environmentID] + for _, r := range currentRecords { + if r == nil { + continue + } + + if r.ID != record.ID { + continue + } + + r.Status = record.Status + r.ApprovedAt = record.ApprovedAt + r.Reason = record.Reason + r.UpdatedAt = record.UpdatedAt + if r.UpdatedAt.Equal(record.UpdatedAt) { + r.UpdatedAt = time.Now().UTC() + } + + return nil + } + + return fmt.Errorf("record not found") +} + +func (v *VersionAnyApprovalRecordRepository) Upsert(ctx context.Context, record *VersionAnyApprovalRecord) error { + v.mu.Lock() + defer v.mu.Unlock() + + if record == nil { + return fmt.Errorf("record is nil") + } + + environmentID := record.EnvironmentID + versionID := record.VersionID + userID := record.UserID + + if _, ok := v.records[versionID]; !ok { + v.records[versionID] = make(map[string][]*VersionAnyApprovalRecord) + } + + if _, ok := v.records[versionID][environmentID]; !ok { + v.records[versionID][environmentID] = make([]*VersionAnyApprovalRecord, 0) + } + + for _, r := range v.records[versionID][environmentID] { + if r.UserID == userID { + r.Status = record.Status + r.ApprovedAt = record.ApprovedAt + r.Reason = record.Reason + r.UpdatedAt = record.UpdatedAt + if r.UpdatedAt.Equal(record.UpdatedAt) { + r.UpdatedAt = time.Now().UTC() + } + + return nil + } + } + + if record.CreatedAt.IsZero() { + record.CreatedAt = time.Now().UTC() + } + if record.UpdatedAt.IsZero() { + record.UpdatedAt = time.Now().UTC() + } + + v.records[versionID][environmentID] = append(v.records[versionID][environmentID], record) + + return nil +} + +func (v *VersionAnyApprovalRecordRepository) Exists(ctx context.Context, recordID string) bool { + v.mu.RLock() + defer v.mu.RUnlock() + + for _, environmentRecords := range v.records { + for _, records := range environmentRecords { + for _, r := range records { + if r.ID == recordID { + return true + } + } + } + } + return false +} + +func (v *VersionAnyApprovalRecordRepository) Get(ctx context.Context, recordID string) *VersionAnyApprovalRecord { + v.mu.RLock() + defer v.mu.RUnlock() + + for _, environmentRecords := range v.records { + for _, records := range environmentRecords { + for _, r := range records { + if r.ID == recordID { + return r + } + } + } + } + return nil +} + +// GetAll returns all records, sorted by updatedAt in descending order (newest first) +func (v *VersionAnyApprovalRecordRepository) GetAll(ctx context.Context) []*VersionAnyApprovalRecord { + v.mu.RLock() + defer v.mu.RUnlock() + + allRecords := make([]*VersionAnyApprovalRecord, 0) + for _, environmentRecords := range v.records { + for _, records := range environmentRecords { + allRecords = append(allRecords, records...) + } + } + sort.Slice(allRecords, func(i, j int) bool { + return allRecords[i].UpdatedAt.After(allRecords[j].UpdatedAt) + }) + return allRecords +} + +// GetAllForVersionAndEnvironment returns all records for a given version and environment, sorted by updatedAt in descending order (newest first) +func (v *VersionAnyApprovalRecordRepository) GetAllForVersionAndEnvironment(ctx context.Context, versionID string, environmentID string) []*VersionAnyApprovalRecord { + v.mu.RLock() + defer v.mu.RUnlock() + + var records []*VersionAnyApprovalRecord + if _, ok := v.records[versionID][environmentID]; ok { + records = make([]*VersionAnyApprovalRecord, len(v.records[versionID][environmentID])) + copy(records, v.records[versionID][environmentID]) + } + sort.Slice(records, func(i, j int) bool { + return records[i].UpdatedAt.After(records[j].UpdatedAt) + }) + return records +} diff --git a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository_test.go b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository_test.go new file mode 100644 index 000000000..239c65811 --- /dev/null +++ b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository_test.go @@ -0,0 +1,344 @@ +package versionanyapproval_test + +import ( + "context" + "testing" + versionanyapproval "workspace-engine/pkg/engine/policy/rules/version-any-approval" + + "gotest.tools/assert" +) + +type VersionAnyApprovalRecordRepositoryTestStep struct { + createRecord *versionanyapproval.VersionAnyApprovalRecord + updateRecord *versionanyapproval.VersionAnyApprovalRecord + deleteRecord *versionanyapproval.VersionAnyApprovalRecord + upsertRecord *versionanyapproval.VersionAnyApprovalRecord + + expectedRecords map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord +} + +type VersionAnyApprovalRecordRepositoryTest struct { + name string + steps []VersionAnyApprovalRecordRepositoryTestStep +} + +type TestStepBundle struct { + t *testing.T + ctx context.Context + repo *versionanyapproval.VersionAnyApprovalRecordRepository + step VersionAnyApprovalRecordRepositoryTestStep + message string +} + +func (b *TestStepBundle) executeStep() { + if b.step.createRecord != nil { + err := b.repo.Create(b.ctx, b.step.createRecord) + if err != nil { + b.t.Fatalf("failed to create record: %v", err) + } + } + + if b.step.updateRecord != nil { + err := b.repo.Update(b.ctx, b.step.updateRecord) + if err != nil { + b.t.Fatalf("failed to update record: %v", err) + } + } + + if b.step.deleteRecord != nil { + err := b.repo.Delete(b.ctx, b.step.deleteRecord.ID) + if err != nil { + b.t.Fatalf("failed to delete record: %v", err) + } + } + + if b.step.upsertRecord != nil { + err := b.repo.Upsert(b.ctx, b.step.upsertRecord) + if err != nil { + b.t.Fatalf("failed to upsert record: %v", err) + } + } +} + +func (b *TestStepBundle) validateExpectedState() { + for versionID, environmentRecords := range b.step.expectedRecords { + for environmentID, records := range environmentRecords { + actualRecords := b.repo.GetAllForVersionAndEnvironment(b.ctx, versionID, environmentID) + assert.Equal(b.t, len(records), len(actualRecords)) + + for i, expectedRecord := range records { + actualRecord := actualRecords[i] + assert.Equal(b.t, expectedRecord.ID, actualRecord.ID) + assert.Equal(b.t, expectedRecord.VersionID, actualRecord.VersionID) + assert.Equal(b.t, expectedRecord.EnvironmentID, actualRecord.EnvironmentID) + assert.Equal(b.t, expectedRecord.UserID, actualRecord.UserID) + assert.Equal(b.t, expectedRecord.Status, actualRecord.Status) + } + } + } +} + +func TestBasicCRUD(t *testing.T) { + createRecord := VersionAnyApprovalRecordRepositoryTest{ + name: "creates a record", + steps: []VersionAnyApprovalRecordRepositoryTestStep{ + { + createRecord: &versionanyapproval.VersionAnyApprovalRecord{ + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + }, + expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ + "version-1": { + "environment-1": { + { + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + }, + }, + }, + }, + }, + }, + } + + updateRecord := VersionAnyApprovalRecordRepositoryTest{ + name: "updates a record", + steps: []VersionAnyApprovalRecordRepositoryTestStep{ + { + createRecord: &versionanyapproval.VersionAnyApprovalRecord{ + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusRejected, + }, + expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ + "version-1": { + "environment-1": { + { + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusRejected, + }, + }, + }, + }, + }, + { + updateRecord: &versionanyapproval.VersionAnyApprovalRecord{ + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + }, + expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ + "version-1": { + "environment-1": { + { + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + }, + }, + }, + }, + }, + }, + } + + deleteRecord := VersionAnyApprovalRecordRepositoryTest{ + name: "deletes a record", + steps: []VersionAnyApprovalRecordRepositoryTestStep{ + { + createRecord: &versionanyapproval.VersionAnyApprovalRecord{ + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + }, + expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ + "version-1": { + "environment-1": { + { + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + }, + }, + }, + }, + }, + { + deleteRecord: &versionanyapproval.VersionAnyApprovalRecord{ + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + }, + expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ + "version-1": {"environment-1": {}}, + }, + }, + }, + } + + upsertRecord := VersionAnyApprovalRecordRepositoryTest{ + name: "upserts a record", + steps: []VersionAnyApprovalRecordRepositoryTestStep{ + { + upsertRecord: &versionanyapproval.VersionAnyApprovalRecord{ + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusRejected, + }, + expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ + "version-1": { + "environment-1": { + { + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusRejected, + }, + }, + }, + }, + }, + { + upsertRecord: &versionanyapproval.VersionAnyApprovalRecord{ + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + }, + expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ + "version-1": { + "environment-1": { + { + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + }, + }, + }, + }, + }, + }, + } + + tests := []VersionAnyApprovalRecordRepositoryTest{ + createRecord, + updateRecord, + deleteRecord, + upsertRecord, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + repo := versionanyapproval.NewVersionAnyApprovalRecordRepository() + + for _, step := range test.steps { + bundle := TestStepBundle{ + t: t, + ctx: context.Background(), + repo: repo, + step: step, + message: "", + } + bundle.executeStep() + bundle.validateExpectedState() + } + }) + } +} + +func TestDuplicateRecords(t *testing.T) { + repo := versionanyapproval.NewVersionAnyApprovalRecordRepository() + ctx := context.Background() + + record := &versionanyapproval.VersionAnyApprovalRecord{ + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + } + + err := repo.Create(ctx, record) + assert.NilError(t, err) + + err = repo.Create(ctx, record) + assert.ErrorContains(t, err, "record already exists") + + err = repo.Upsert(ctx, record) + assert.NilError(t, err) + + record.Status = versionanyapproval.VersionAnyApprovalRecordStatusRejected + err = repo.Update(ctx, record) + assert.NilError(t, err) +} + +func TestUpdateNonExistingRecord(t *testing.T) { + repo := versionanyapproval.NewVersionAnyApprovalRecordRepository() + ctx := context.Background() + + record := &versionanyapproval.VersionAnyApprovalRecord{ + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + } + + err := repo.Create(ctx, record) + assert.NilError(t, err) + + record2 := &versionanyapproval.VersionAnyApprovalRecord{ + ID: "record-2", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusRejected, + } + + err = repo.Update(ctx, record2) + assert.ErrorContains(t, err, "record not found") +} + +func TestNilHandling(t *testing.T) { + repo := versionanyapproval.NewVersionAnyApprovalRecordRepository() + ctx := context.Background() + + err := repo.Create(ctx, nil) + assert.ErrorContains(t, err, "record is nil") + + err = repo.Update(ctx, nil) + assert.ErrorContains(t, err, "record is nil") + + err = repo.Delete(ctx, "") + assert.NilError(t, err) + + err = repo.Upsert(ctx, nil) + assert.ErrorContains(t, err, "record is nil") +} diff --git a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_rule.go b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_rule.go new file mode 100644 index 000000000..0a3786e1e --- /dev/null +++ b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_rule.go @@ -0,0 +1,66 @@ +package versionanyapproval + +import ( + "context" + "time" + rt "workspace-engine/pkg/engine/policy/releasetargets" + "workspace-engine/pkg/engine/policy/rules" + "workspace-engine/pkg/model/deployment" +) + +var _ rules.Rule = (*VersionAnyApprovalRule)(nil) + +func NewVersionAnyApprovalRule( + versionAnyApprovalRecordRepository *VersionAnyApprovalRecordRepository, + requiredApprovalsCount int, + policyID string, + ruleID string, +) *VersionAnyApprovalRule { + return &VersionAnyApprovalRule{ + BaseRule: rules.BaseRule{ + ID: ruleID, + PolicyID: policyID, + }, + versionAnyApprovalRecordRepository: versionAnyApprovalRecordRepository, + RequiredApprovalsCount: requiredApprovalsCount, + } +} + +type VersionAnyApprovalRule struct { + rules.BaseRule + versionAnyApprovalRecordRepository *VersionAnyApprovalRecordRepository + RequiredApprovalsCount int +} + +func (r *VersionAnyApprovalRule) Evaluate(ctx context.Context, target rt.ReleaseTarget, version deployment.DeploymentVersion) (*rules.RuleEvaluationResult, error) { + environmentID := target.Environment.GetID() + versionID := version.GetID() + + result := &rules.RuleEvaluationResult{ + RuleID: r.GetID(), + EvaluatedAt: time.Now().UTC(), + } + + records := r.versionAnyApprovalRecordRepository.GetAllForVersionAndEnvironment( + ctx, + versionID, + environmentID, + ) + + approvedCount := 0 + for _, record := range records { + if record.Status == VersionAnyApprovalRecordStatusApproved { + approvedCount++ + } + } + + if approvedCount >= r.RequiredApprovalsCount { + result.Decision = rules.PolicyDecisionAllow + result.Message = "Minimum number of approvals reached" + return result, nil + } + + result.Decision = rules.PolicyDecisionDeny + result.Message = "Minimum number of approvals not reached" + return result, nil +} diff --git a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_rule_test.go b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_rule_test.go new file mode 100644 index 000000000..a7d06ed6a --- /dev/null +++ b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_rule_test.go @@ -0,0 +1,123 @@ +package versionanyapproval_test + +import ( + "context" + "testing" + rt "workspace-engine/pkg/engine/policy/releasetargets" + "workspace-engine/pkg/engine/policy/rules" + versionanyapproval "workspace-engine/pkg/engine/policy/rules/version-any-approval" + "workspace-engine/pkg/model/deployment" + "workspace-engine/pkg/model/environment" + + "github.com/google/uuid" + "gotest.tools/assert" +) + +type VersionAnyApprovalRuleTest struct { + name string + numApprovalsToCreate int + numRejectionsToCreate int + minimumApprovalsCount int + releaseTarget rt.ReleaseTarget + version deployment.DeploymentVersion + expectedDecision rules.PolicyDecision +} + +func randomID() string { + return uuid.New().String() +} + +func TestVersionAnyApprovalRule(t *testing.T) { + deniesIfInsufficientApprovals := VersionAnyApprovalRuleTest{ + name: "denies if insufficient approvals", + numApprovalsToCreate: 1, + numRejectionsToCreate: 1, + minimumApprovalsCount: 2, + releaseTarget: rt.ReleaseTarget{Environment: environment.Environment{ID: randomID()}}, + version: deployment.DeploymentVersion{ID: randomID()}, + expectedDecision: rules.PolicyDecisionDeny, + } + + allowsIfEqualToMinimumApprovalsCount := VersionAnyApprovalRuleTest{ + name: "allows if equal to minimum approvals count", + numApprovalsToCreate: 2, + numRejectionsToCreate: 0, + minimumApprovalsCount: 2, + releaseTarget: rt.ReleaseTarget{Environment: environment.Environment{ID: randomID()}}, + version: deployment.DeploymentVersion{ID: randomID()}, + expectedDecision: rules.PolicyDecisionAllow, + } + + allowsIfGreaterThanMinimumApprovalsCount := VersionAnyApprovalRuleTest{ + name: "allows if greater than minimum approvals count", + numApprovalsToCreate: 3, + numRejectionsToCreate: 0, + minimumApprovalsCount: 2, + releaseTarget: rt.ReleaseTarget{Environment: environment.Environment{ID: randomID()}}, + version: deployment.DeploymentVersion{ID: randomID()}, + expectedDecision: rules.PolicyDecisionAllow, + } + + allowsIfEqualToMinimumApprovalsCountDespiteRejections := VersionAnyApprovalRuleTest{ + name: "allows if equal to minimum approvals count despite rejections", + numApprovalsToCreate: 2, + numRejectionsToCreate: 5, + minimumApprovalsCount: 2, + releaseTarget: rt.ReleaseTarget{Environment: environment.Environment{ID: randomID()}}, + version: deployment.DeploymentVersion{ID: randomID()}, + expectedDecision: rules.PolicyDecisionAllow, + } + + tests := []VersionAnyApprovalRuleTest{ + deniesIfInsufficientApprovals, + allowsIfEqualToMinimumApprovalsCount, + allowsIfGreaterThanMinimumApprovalsCount, + allowsIfEqualToMinimumApprovalsCountDespiteRejections, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + ctx := context.Background() + repo := versionanyapproval.NewVersionAnyApprovalRecordRepository() + rule := versionanyapproval.NewVersionAnyApprovalRule( + repo, + test.minimumApprovalsCount, + randomID(), + randomID(), + ) + + environmentID := test.releaseTarget.Environment.GetID() + versionID := test.version.GetID() + + for i := 0; i < test.numApprovalsToCreate; i++ { + record := &versionanyapproval.VersionAnyApprovalRecord{ + ID: randomID(), + VersionID: versionID, + EnvironmentID: environmentID, + UserID: randomID(), + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + } + + err := repo.Create(ctx, record) + assert.NilError(t, err) + } + + for i := 0; i < test.numRejectionsToCreate; i++ { + record := &versionanyapproval.VersionAnyApprovalRecord{ + ID: randomID(), + VersionID: versionID, + EnvironmentID: environmentID, + UserID: randomID(), + Status: versionanyapproval.VersionAnyApprovalRecordStatusRejected, + } + + err := repo.Create(ctx, record) + assert.NilError(t, err) + } + + result, err := rule.Evaluate(ctx, test.releaseTarget, test.version) + assert.NilError(t, err) + assert.Equal(t, test.expectedDecision, result.Decision) + }) + } +} From 19f205a8537565bf0254c72c56cc88d5604d4b1a Mon Sep 17 00:00:00 2001 From: Aditya Choudhari Date: Thu, 14 Aug 2025 14:06:47 -0700 Subject: [PATCH 2/8] cleanup --- .../version_any_approval_record_repository.go | 28 +++- ...ion_any_approval_record_repository_test.go | 151 ++++++++++++++++++ 2 files changed, 173 insertions(+), 6 deletions(-) diff --git a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go index ae4135d49..5ef7b1d85 100644 --- a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go +++ b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go @@ -57,6 +57,22 @@ func (v *VersionAnyApprovalRecordRepository) Create(ctx context.Context, record return fmt.Errorf("record is nil") } + if record.ID == "" { + return fmt.Errorf("record ID is empty") + } + + if record.EnvironmentID == "" { + return fmt.Errorf("environment ID is empty") + } + + if record.VersionID == "" { + return fmt.Errorf("version ID is empty") + } + + if record.UserID == "" { + return fmt.Errorf("user ID is empty") + } + environmentID := record.EnvironmentID versionID := record.VersionID @@ -155,10 +171,10 @@ func (v *VersionAnyApprovalRecordRepository) Update(ctx context.Context, record r.Status = record.Status r.ApprovedAt = record.ApprovedAt r.Reason = record.Reason - r.UpdatedAt = record.UpdatedAt - if r.UpdatedAt.Equal(record.UpdatedAt) { - r.UpdatedAt = time.Now().UTC() + if record.UpdatedAt.Equal(r.UpdatedAt) || record.UpdatedAt.Before(r.UpdatedAt) { + record.UpdatedAt = time.Now().UTC() } + r.UpdatedAt = record.UpdatedAt return nil } @@ -191,10 +207,10 @@ func (v *VersionAnyApprovalRecordRepository) Upsert(ctx context.Context, record r.Status = record.Status r.ApprovedAt = record.ApprovedAt r.Reason = record.Reason - r.UpdatedAt = record.UpdatedAt - if r.UpdatedAt.Equal(record.UpdatedAt) { - r.UpdatedAt = time.Now().UTC() + if record.UpdatedAt.Equal(r.UpdatedAt) || record.UpdatedAt.Before(r.UpdatedAt) { + record.UpdatedAt = time.Now().UTC() } + r.UpdatedAt = record.UpdatedAt return nil } diff --git a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository_test.go b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository_test.go index 239c65811..d8f5ec65d 100644 --- a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository_test.go +++ b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository_test.go @@ -3,6 +3,7 @@ package versionanyapproval_test import ( "context" "testing" + "time" versionanyapproval "workspace-engine/pkg/engine/policy/rules/version-any-approval" "gotest.tools/assert" @@ -247,11 +248,161 @@ func TestBasicCRUD(t *testing.T) { }, } + maintainsRecordOrderByUpdatedAt := VersionAnyApprovalRecordRepositoryTest{ + name: "maintains record order by updated at", + steps: []VersionAnyApprovalRecordRepositoryTestStep{ + { + createRecord: &versionanyapproval.VersionAnyApprovalRecord{ + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + UpdatedAt: time.Now().Add(-time.Second * 2), + }, + expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ + "version-1": { + "environment-1": { + { + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + UpdatedAt: time.Now().Add(-time.Second * 2), + }, + }, + }, + }, + }, + { + createRecord: &versionanyapproval.VersionAnyApprovalRecord{ + ID: "record-2", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-2", + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + UpdatedAt: time.Now().Add(-time.Second), + }, + expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ + "version-1": { + "environment-1": { + { + ID: "record-2", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-2", + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + UpdatedAt: time.Now().Add(-time.Second), + }, + { + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + UpdatedAt: time.Now().Add(-time.Second * 2), + }, + }, + }, + }, + }, + }, + } + + maintainsOrderWhenRecordsAreUpdated := VersionAnyApprovalRecordRepositoryTest{ + name: "maintains order when records are updated", + steps: []VersionAnyApprovalRecordRepositoryTestStep{ + { + createRecord: &versionanyapproval.VersionAnyApprovalRecord{ + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusRejected, + }, + expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ + "version-1": { + "environment-1": { + { + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusRejected, + }, + }, + }, + }, + }, + { + createRecord: &versionanyapproval.VersionAnyApprovalRecord{ + ID: "record-2", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-2", + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + }, + expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ + "version-1": { + "environment-1": { + { + ID: "record-2", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-2", + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + }, + { + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusRejected, + }, + }, + }, + }, + }, + { + updateRecord: &versionanyapproval.VersionAnyApprovalRecord{ + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + }, + expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ + "version-1": { + "environment-1": { + { + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + }, + { + ID: "record-2", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-2", + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + }, + }, + }, + }, + }, + }, + } + tests := []VersionAnyApprovalRecordRepositoryTest{ createRecord, updateRecord, deleteRecord, upsertRecord, + maintainsRecordOrderByUpdatedAt, + maintainsOrderWhenRecordsAreUpdated, } for _, test := range tests { From 19f5414e2aa181cb6a3c30e775ea49bf2d39ae76 Mon Sep 17 00:00:00 2001 From: Aditya Choudhari Date: Mon, 18 Aug 2025 11:26:58 -0700 Subject: [PATCH 3/8] more robust testing --- .../version_any_approval_record_repository.go | 127 +++++++++++------- ...ion_any_approval_record_repository_test.go | 123 +++++++++++++++++ 2 files changed, 201 insertions(+), 49 deletions(-) diff --git a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go index 5ef7b1d85..47dc3908c 100644 --- a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go +++ b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go @@ -49,10 +49,20 @@ func NewVersionAnyApprovalRecordRepository() *VersionAnyApprovalRecordRepository } } -func (v *VersionAnyApprovalRecordRepository) Create(ctx context.Context, record *VersionAnyApprovalRecord) error { - v.mu.Lock() - defer v.mu.Unlock() +func (v *VersionAnyApprovalRecordRepository) getRecordByID(id string) *VersionAnyApprovalRecord { + for _, environmentRecords := range v.records { + for _, records := range environmentRecords { + for _, r := range records { + if r.ID == id { + return r + } + } + } + } + return nil +} +func (v *VersionAnyApprovalRecordRepository) validateRecord(record *VersionAnyApprovalRecord) error { if record == nil { return fmt.Errorf("record is nil") } @@ -73,6 +83,17 @@ func (v *VersionAnyApprovalRecordRepository) Create(ctx context.Context, record return fmt.Errorf("user ID is empty") } + return nil +} + +func (v *VersionAnyApprovalRecordRepository) Create(ctx context.Context, record *VersionAnyApprovalRecord) error { + v.mu.Lock() + defer v.mu.Unlock() + + if err := v.validateRecord(record); err != nil { + return err + } + environmentID := record.EnvironmentID versionID := record.VersionID @@ -84,16 +105,16 @@ func (v *VersionAnyApprovalRecordRepository) Create(ctx context.Context, record v.records[versionID][environmentID] = make([]*VersionAnyApprovalRecord, 0) } + if v.getRecordByID(record.ID) != nil { + return fmt.Errorf("record already exists") + } + currentRecords := v.records[versionID][environmentID] for _, r := range currentRecords { if r == nil { continue } - if r.ID == record.ID { - return fmt.Errorf("record already exists") - } - if r.UserID == record.UserID { return fmt.Errorf("record already exists for user") } @@ -143,43 +164,36 @@ func (v *VersionAnyApprovalRecordRepository) Update(ctx context.Context, record v.mu.Lock() defer v.mu.Unlock() - if record == nil { - return fmt.Errorf("record is nil") + if err := v.validateRecord(record); err != nil { + return err } - environmentID := record.EnvironmentID - versionID := record.VersionID - - if _, ok := v.records[versionID]; !ok { - return fmt.Errorf("version not found") + existingRecord := v.getRecordByID(record.ID) + if existingRecord == nil { + return fmt.Errorf("record not found") } - if _, ok := v.records[versionID][environmentID]; !ok { - return fmt.Errorf("environment not found") + if existingRecord.EnvironmentID != record.EnvironmentID { + return fmt.Errorf("environment ID mismatch") } - currentRecords := v.records[versionID][environmentID] - for _, r := range currentRecords { - if r == nil { - continue - } - - if r.ID != record.ID { - continue - } + if existingRecord.VersionID != record.VersionID { + return fmt.Errorf("version ID mismatch") + } - r.Status = record.Status - r.ApprovedAt = record.ApprovedAt - r.Reason = record.Reason - if record.UpdatedAt.Equal(r.UpdatedAt) || record.UpdatedAt.Before(r.UpdatedAt) { - record.UpdatedAt = time.Now().UTC() - } - r.UpdatedAt = record.UpdatedAt + if existingRecord.UserID != record.UserID { + return fmt.Errorf("user ID mismatch") + } - return nil + existingRecord.Status = record.Status + existingRecord.ApprovedAt = record.ApprovedAt + existingRecord.Reason = record.Reason + if record.UpdatedAt.Equal(existingRecord.UpdatedAt) || record.UpdatedAt.Before(existingRecord.UpdatedAt) { + record.UpdatedAt = time.Now().UTC() } + existingRecord.UpdatedAt = record.UpdatedAt - return fmt.Errorf("record not found") + return nil } func (v *VersionAnyApprovalRecordRepository) Upsert(ctx context.Context, record *VersionAnyApprovalRecord) error { @@ -192,7 +206,30 @@ func (v *VersionAnyApprovalRecordRepository) Upsert(ctx context.Context, record environmentID := record.EnvironmentID versionID := record.VersionID - userID := record.UserID + + existingRecord := v.getRecordByID(record.ID) + if existingRecord != nil { + if existingRecord.EnvironmentID != record.EnvironmentID { + return fmt.Errorf("environment ID mismatch") + } + + if existingRecord.VersionID != record.VersionID { + return fmt.Errorf("version ID mismatch") + } + + if existingRecord.UserID != record.UserID { + return fmt.Errorf("user ID mismatch") + } + + existingRecord.Status = record.Status + existingRecord.ApprovedAt = record.ApprovedAt + existingRecord.Reason = record.Reason + if record.UpdatedAt.Equal(existingRecord.UpdatedAt) || record.UpdatedAt.Before(existingRecord.UpdatedAt) { + record.UpdatedAt = time.Now().UTC() + } + existingRecord.UpdatedAt = record.UpdatedAt + return nil + } if _, ok := v.records[versionID]; !ok { v.records[versionID] = make(map[string][]*VersionAnyApprovalRecord) @@ -202,20 +239,6 @@ func (v *VersionAnyApprovalRecordRepository) Upsert(ctx context.Context, record v.records[versionID][environmentID] = make([]*VersionAnyApprovalRecord, 0) } - for _, r := range v.records[versionID][environmentID] { - if r.UserID == userID { - r.Status = record.Status - r.ApprovedAt = record.ApprovedAt - r.Reason = record.Reason - if record.UpdatedAt.Equal(r.UpdatedAt) || record.UpdatedAt.Before(r.UpdatedAt) { - record.UpdatedAt = time.Now().UTC() - } - r.UpdatedAt = record.UpdatedAt - - return nil - } - } - if record.CreatedAt.IsZero() { record.CreatedAt = time.Now().UTC() } @@ -223,6 +246,12 @@ func (v *VersionAnyApprovalRecordRepository) Upsert(ctx context.Context, record record.UpdatedAt = time.Now().UTC() } + for _, r := range v.records[versionID][environmentID] { + if r.UserID == record.UserID { + return fmt.Errorf("record already exists for user") + } + } + v.records[versionID][environmentID] = append(v.records[versionID][environmentID], record) return nil diff --git a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository_test.go b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository_test.go index d8f5ec65d..d5d9a20f3 100644 --- a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository_test.go +++ b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository_test.go @@ -477,6 +477,53 @@ func TestUpdateNonExistingRecord(t *testing.T) { assert.ErrorContains(t, err, "record not found") } +func TestUpdateRecordWithReadonlyIds(t *testing.T) { + repo := versionanyapproval.NewVersionAnyApprovalRecordRepository() + ctx := context.Background() + + record := &versionanyapproval.VersionAnyApprovalRecord{ + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + } + + err := repo.Create(ctx, record) + assert.NilError(t, err) + + updateRecord := &versionanyapproval.VersionAnyApprovalRecord{ + ID: "record-1", + VersionID: "version-2", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + } + + err = repo.Update(ctx, updateRecord) + assert.ErrorContains(t, err, "version ID mismatch") + err = repo.Upsert(ctx, updateRecord) + assert.ErrorContains(t, err, "version ID mismatch") + + updateRecord.VersionID = "version-1" + updateRecord.EnvironmentID = "environment-2" + err = repo.Update(ctx, updateRecord) + assert.ErrorContains(t, err, "environment ID mismatch") + err = repo.Upsert(ctx, updateRecord) + assert.ErrorContains(t, err, "environment ID mismatch") + + updateRecord.EnvironmentID = "environment-1" + updateRecord.UserID = "user-2" + err = repo.Update(ctx, updateRecord) + assert.ErrorContains(t, err, "user ID mismatch") + err = repo.Upsert(ctx, updateRecord) + assert.ErrorContains(t, err, "user ID mismatch") + + updateRecord.UserID = "user-1" + err = repo.Update(ctx, updateRecord) + assert.NilError(t, err) +} + func TestNilHandling(t *testing.T) { repo := versionanyapproval.NewVersionAnyApprovalRecordRepository() ctx := context.Background() @@ -493,3 +540,79 @@ func TestNilHandling(t *testing.T) { err = repo.Upsert(ctx, nil) assert.ErrorContains(t, err, "record is nil") } + +func TestGlobalIDUniqueness(t *testing.T) { + repo := versionanyapproval.NewVersionAnyApprovalRecordRepository() + ctx := context.Background() + + // Create first record in version-1/environment-1 + record1 := &versionanyapproval.VersionAnyApprovalRecord{ + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + } + + err := repo.Create(ctx, record1) + assert.NilError(t, err) + + // Attempt to create a record with the same ID in a different version/environment bucket + record2 := &versionanyapproval.VersionAnyApprovalRecord{ + ID: "record-1", // Same ID as record1 + VersionID: "version-2", // Different version + EnvironmentID: "environment-2", // Different environment + UserID: "user-2", + Status: versionanyapproval.VersionAnyApprovalRecordStatusRejected, + } + + err = repo.Create(ctx, record2) + assert.ErrorContains(t, err, "record already exists") + + // Verify the first record still exists and is unchanged + existingRecord := repo.Get(ctx, "record-1") + assert.Assert(t, existingRecord != nil) + assert.Equal(t, existingRecord.ID, "record-1") + assert.Equal(t, existingRecord.VersionID, "version-1") + assert.Equal(t, existingRecord.EnvironmentID, "environment-1") + assert.Equal(t, existingRecord.UserID, "user-1") + assert.Equal(t, existingRecord.Status, versionanyapproval.VersionAnyApprovalRecordStatusApproved) + + // Verify the second record was not created + nonExistentRecord := repo.Get(ctx, "record-2") + assert.Assert(t, nonExistentRecord == nil) +} + +func TestUserUniquenessForEnvAndVersion(t *testing.T) { + repo := versionanyapproval.NewVersionAnyApprovalRecordRepository() + ctx := context.Background() + + record1 := &versionanyapproval.VersionAnyApprovalRecord{ + ID: "record-1", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + } + + err := repo.Create(ctx, record1) + assert.NilError(t, err) + + record2 := &versionanyapproval.VersionAnyApprovalRecord{ + ID: "record-2", + VersionID: "version-1", + EnvironmentID: "environment-1", + UserID: "user-1", + Status: versionanyapproval.VersionAnyApprovalRecordStatusRejected, + } + + err = repo.Create(ctx, record2) + assert.ErrorContains(t, err, "record already exists for user") + + err = repo.Upsert(ctx, record2) + assert.ErrorContains(t, err, "record already exists for user") + + record2.UserID = "user-2" + err = repo.Upsert(ctx, record2) + assert.NilError(t, err) +} From 03c3175915ef579e5bafde381a5d1e52a68052b3 Mon Sep 17 00:00:00 2001 From: Aditya Choudhari Date: Mon, 18 Aug 2025 11:36:23 -0700 Subject: [PATCH 4/8] nit --- .../version_any_approval_record_repository.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go index 47dc3908c..7f7d67ff6 100644 --- a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go +++ b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go @@ -200,8 +200,8 @@ func (v *VersionAnyApprovalRecordRepository) Upsert(ctx context.Context, record v.mu.Lock() defer v.mu.Unlock() - if record == nil { - return fmt.Errorf("record is nil") + if err := v.validateRecord(record); err != nil { + return err } environmentID := record.EnvironmentID From 77843671da90f16226758c33c27656245a5e4493 Mon Sep 17 00:00:00 2001 From: Aditya Choudhari Date: Mon, 18 Aug 2025 11:46:11 -0700 Subject: [PATCH 5/8] cleanup --- .../version_any_approval_record_repository.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go index 7f7d67ff6..2577f29a8 100644 --- a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go +++ b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go @@ -188,10 +188,11 @@ func (v *VersionAnyApprovalRecordRepository) Update(ctx context.Context, record existingRecord.Status = record.Status existingRecord.ApprovedAt = record.ApprovedAt existingRecord.Reason = record.Reason - if record.UpdatedAt.Equal(existingRecord.UpdatedAt) || record.UpdatedAt.Before(existingRecord.UpdatedAt) { - record.UpdatedAt = time.Now().UTC() + ts := record.UpdatedAt + if ts.Equal(existingRecord.UpdatedAt) || ts.Before(existingRecord.UpdatedAt) { + ts = time.Now().UTC() } - existingRecord.UpdatedAt = record.UpdatedAt + existingRecord.UpdatedAt = ts return nil } @@ -224,10 +225,11 @@ func (v *VersionAnyApprovalRecordRepository) Upsert(ctx context.Context, record existingRecord.Status = record.Status existingRecord.ApprovedAt = record.ApprovedAt existingRecord.Reason = record.Reason - if record.UpdatedAt.Equal(existingRecord.UpdatedAt) || record.UpdatedAt.Before(existingRecord.UpdatedAt) { - record.UpdatedAt = time.Now().UTC() + ts := record.UpdatedAt + if ts.Equal(existingRecord.UpdatedAt) || ts.Before(existingRecord.UpdatedAt) { + ts = time.Now().UTC() } - existingRecord.UpdatedAt = record.UpdatedAt + existingRecord.UpdatedAt = ts return nil } From 627d621baae277b3c3c5877445fb0af164dc63d1 Mon Sep 17 00:00:00 2001 From: Aditya Choudhari Date: Mon, 18 Aug 2025 13:47:16 -0700 Subject: [PATCH 6/8] feat: add ApprovalRecord interface and VersionAnyApprovalRecord implementation - Introduced ApprovalRecord interface with methods for managing approval records. - Implemented VersionAnyApprovalRecord struct with a builder pattern for easier instantiation. - Updated tests to utilize the new builder pattern for creating approval records. - Refactored existing code to replace direct field access with getter methods from the ApprovalRecord interface. --- .../version-any-approval/approval_record.go | 22 + .../version_any_approval_record_repository.go | 210 +++++-- ...ion_any_approval_record_repository_test.go | 587 ++++++++++-------- .../version_any_approval_rule.go | 2 +- .../version_any_approval_rule_test.go | 28 +- 5 files changed, 497 insertions(+), 352 deletions(-) create mode 100644 apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/approval_record.go diff --git a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/approval_record.go b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/approval_record.go new file mode 100644 index 000000000..e8970d7e4 --- /dev/null +++ b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/approval_record.go @@ -0,0 +1,22 @@ +package versionanyapproval + +import "time" + +type ApprovalRecordStatus string + +const ( + ApprovalRecordStatusApproved ApprovalRecordStatus = "approved" + ApprovalRecordStatusRejected ApprovalRecordStatus = "rejected" +) + +type ApprovalRecord interface { + GetID() string + GetVersionID() string + GetEnvironmentID() string + GetUserID() string + GetStatus() ApprovalRecordStatus + GetApprovedAt() *time.Time + GetReason() *string + + IsApproved() bool +} diff --git a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go index 2577f29a8..21be6a686 100644 --- a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go +++ b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go @@ -9,30 +9,116 @@ import ( "workspace-engine/pkg/model" ) -type VersionAnyApprovalRecordStatus string - -const ( - VersionAnyApprovalRecordStatusApproved VersionAnyApprovalRecordStatus = "approved" - VersionAnyApprovalRecordStatusRejected VersionAnyApprovalRecordStatus = "rejected" -) +var _ ApprovalRecord = (*VersionAnyApprovalRecord)(nil) // VersionAnyApprovalRecord represents a record for the version-any-approval rule. // It is used to track the approval status of a version for a given environment, for use by the version-any-approval rule. // There can only be one record per version and environment per user. type VersionAnyApprovalRecord struct { - ID string - VersionID string - EnvironmentID string - UserID string - Status VersionAnyApprovalRecordStatus - ApprovedAt *time.Time - Reason *string - CreatedAt time.Time - UpdatedAt time.Time + id string + versionID string + environmentID string + userID string + status ApprovalRecordStatus + approvedAt *time.Time + reason *string + createdAt time.Time + updatedAt time.Time +} + +type versionAnyApprovalRecordBuilder struct { + id string + versionID string + environmentID string + userID string + status ApprovalRecordStatus + reason *string +} + +func NewVersionAnyApprovalRecordBuilder() *versionAnyApprovalRecordBuilder { + return &versionAnyApprovalRecordBuilder{ + id: "", + versionID: "", + environmentID: "", + userID: "", + status: "", + reason: nil, + } +} + +func (b *versionAnyApprovalRecordBuilder) WithID(id string) *versionAnyApprovalRecordBuilder { + b.id = id + return b +} + +func (b *versionAnyApprovalRecordBuilder) WithVersionID(versionID string) *versionAnyApprovalRecordBuilder { + b.versionID = versionID + return b +} + +func (b *versionAnyApprovalRecordBuilder) WithEnvironmentID(environmentID string) *versionAnyApprovalRecordBuilder { + b.environmentID = environmentID + return b +} + +func (b *versionAnyApprovalRecordBuilder) WithUserID(userID string) *versionAnyApprovalRecordBuilder { + b.userID = userID + return b +} + +func (b *versionAnyApprovalRecordBuilder) WithStatus(status ApprovalRecordStatus) *versionAnyApprovalRecordBuilder { + b.status = status + return b +} + +func (b *versionAnyApprovalRecordBuilder) WithReason(reason *string) *versionAnyApprovalRecordBuilder { + b.reason = reason + return b +} + +func (b *versionAnyApprovalRecordBuilder) Build() *VersionAnyApprovalRecord { + return &VersionAnyApprovalRecord{ + id: b.id, + versionID: b.versionID, + environmentID: b.environmentID, + userID: b.userID, + status: b.status, + reason: b.reason, + createdAt: time.Now().UTC(), + updatedAt: time.Now().UTC(), + } } func (v VersionAnyApprovalRecord) GetID() string { - return v.ID + return v.id +} + +func (v VersionAnyApprovalRecord) GetVersionID() string { + return v.versionID +} + +func (v VersionAnyApprovalRecord) GetEnvironmentID() string { + return v.environmentID +} + +func (v VersionAnyApprovalRecord) GetUserID() string { + return v.userID +} + +func (v VersionAnyApprovalRecord) GetStatus() ApprovalRecordStatus { + return v.status +} + +func (v VersionAnyApprovalRecord) IsApproved() bool { + return v.status == ApprovalRecordStatusApproved +} + +func (v VersionAnyApprovalRecord) GetApprovedAt() *time.Time { + return v.approvedAt +} + +func (v VersionAnyApprovalRecord) GetReason() *string { + return v.reason } var _ model.Repository[VersionAnyApprovalRecord] = (*VersionAnyApprovalRecordRepository)(nil) @@ -53,7 +139,7 @@ func (v *VersionAnyApprovalRecordRepository) getRecordByID(id string) *VersionAn for _, environmentRecords := range v.records { for _, records := range environmentRecords { for _, r := range records { - if r.ID == id { + if r.id == id { return r } } @@ -67,19 +153,19 @@ func (v *VersionAnyApprovalRecordRepository) validateRecord(record *VersionAnyAp return fmt.Errorf("record is nil") } - if record.ID == "" { + if record.id == "" { return fmt.Errorf("record ID is empty") } - if record.EnvironmentID == "" { + if record.environmentID == "" { return fmt.Errorf("environment ID is empty") } - if record.VersionID == "" { + if record.versionID == "" { return fmt.Errorf("version ID is empty") } - if record.UserID == "" { + if record.userID == "" { return fmt.Errorf("user ID is empty") } @@ -94,8 +180,8 @@ func (v *VersionAnyApprovalRecordRepository) Create(ctx context.Context, record return err } - environmentID := record.EnvironmentID - versionID := record.VersionID + environmentID := record.environmentID + versionID := record.versionID if _, ok := v.records[versionID]; !ok { v.records[versionID] = make(map[string][]*VersionAnyApprovalRecord) @@ -105,7 +191,7 @@ func (v *VersionAnyApprovalRecordRepository) Create(ctx context.Context, record v.records[versionID][environmentID] = make([]*VersionAnyApprovalRecord, 0) } - if v.getRecordByID(record.ID) != nil { + if v.getRecordByID(record.id) != nil { return fmt.Errorf("record already exists") } @@ -115,16 +201,16 @@ func (v *VersionAnyApprovalRecordRepository) Create(ctx context.Context, record continue } - if r.UserID == record.UserID { + if r.userID == record.userID { return fmt.Errorf("record already exists for user") } } - if record.CreatedAt.IsZero() { - record.CreatedAt = time.Now().UTC() + if record.createdAt.IsZero() { + record.createdAt = time.Now().UTC() } - if record.UpdatedAt.IsZero() { - record.UpdatedAt = time.Now().UTC() + if record.updatedAt.IsZero() { + record.updatedAt = time.Now().UTC() } v.records[versionID][environmentID] = append(currentRecords, record) @@ -142,13 +228,13 @@ func (v *VersionAnyApprovalRecordRepository) Delete(ctx context.Context, recordI continue } - if r.ID != recordID { + if r.id != recordID { continue } newRecords := make([]*VersionAnyApprovalRecord, 0, len(records)-1) for _, r := range records { - if r.ID != recordID { + if r.id != recordID { newRecords = append(newRecords, r) } } @@ -168,31 +254,31 @@ func (v *VersionAnyApprovalRecordRepository) Update(ctx context.Context, record return err } - existingRecord := v.getRecordByID(record.ID) + existingRecord := v.getRecordByID(record.id) if existingRecord == nil { return fmt.Errorf("record not found") } - if existingRecord.EnvironmentID != record.EnvironmentID { + if existingRecord.environmentID != record.environmentID { return fmt.Errorf("environment ID mismatch") } - if existingRecord.VersionID != record.VersionID { + if existingRecord.versionID != record.versionID { return fmt.Errorf("version ID mismatch") } - if existingRecord.UserID != record.UserID { + if existingRecord.userID != record.userID { return fmt.Errorf("user ID mismatch") } - existingRecord.Status = record.Status - existingRecord.ApprovedAt = record.ApprovedAt - existingRecord.Reason = record.Reason - ts := record.UpdatedAt - if ts.Equal(existingRecord.UpdatedAt) || ts.Before(existingRecord.UpdatedAt) { + existingRecord.status = record.status + existingRecord.approvedAt = record.approvedAt + existingRecord.reason = record.reason + ts := record.updatedAt + if ts.Equal(existingRecord.updatedAt) || ts.Before(existingRecord.updatedAt) { ts = time.Now().UTC() } - existingRecord.UpdatedAt = ts + existingRecord.updatedAt = ts return nil } @@ -205,31 +291,31 @@ func (v *VersionAnyApprovalRecordRepository) Upsert(ctx context.Context, record return err } - environmentID := record.EnvironmentID - versionID := record.VersionID + environmentID := record.environmentID + versionID := record.versionID - existingRecord := v.getRecordByID(record.ID) + existingRecord := v.getRecordByID(record.id) if existingRecord != nil { - if existingRecord.EnvironmentID != record.EnvironmentID { + if existingRecord.environmentID != record.environmentID { return fmt.Errorf("environment ID mismatch") } - if existingRecord.VersionID != record.VersionID { + if existingRecord.versionID != record.versionID { return fmt.Errorf("version ID mismatch") } - if existingRecord.UserID != record.UserID { + if existingRecord.userID != record.userID { return fmt.Errorf("user ID mismatch") } - existingRecord.Status = record.Status - existingRecord.ApprovedAt = record.ApprovedAt - existingRecord.Reason = record.Reason - ts := record.UpdatedAt - if ts.Equal(existingRecord.UpdatedAt) || ts.Before(existingRecord.UpdatedAt) { + existingRecord.status = record.status + existingRecord.approvedAt = record.approvedAt + existingRecord.reason = record.reason + ts := record.updatedAt + if ts.Equal(existingRecord.updatedAt) || ts.Before(existingRecord.updatedAt) { ts = time.Now().UTC() } - existingRecord.UpdatedAt = ts + existingRecord.updatedAt = ts return nil } @@ -241,15 +327,15 @@ func (v *VersionAnyApprovalRecordRepository) Upsert(ctx context.Context, record v.records[versionID][environmentID] = make([]*VersionAnyApprovalRecord, 0) } - if record.CreatedAt.IsZero() { - record.CreatedAt = time.Now().UTC() + if record.createdAt.IsZero() { + record.createdAt = time.Now().UTC() } - if record.UpdatedAt.IsZero() { - record.UpdatedAt = time.Now().UTC() + if record.updatedAt.IsZero() { + record.updatedAt = time.Now().UTC() } for _, r := range v.records[versionID][environmentID] { - if r.UserID == record.UserID { + if r.userID == record.userID { return fmt.Errorf("record already exists for user") } } @@ -266,7 +352,7 @@ func (v *VersionAnyApprovalRecordRepository) Exists(ctx context.Context, recordI for _, environmentRecords := range v.records { for _, records := range environmentRecords { for _, r := range records { - if r.ID == recordID { + if r.id == recordID { return true } } @@ -282,7 +368,7 @@ func (v *VersionAnyApprovalRecordRepository) Get(ctx context.Context, recordID s for _, environmentRecords := range v.records { for _, records := range environmentRecords { for _, r := range records { - if r.ID == recordID { + if r.id == recordID { return r } } @@ -303,7 +389,7 @@ func (v *VersionAnyApprovalRecordRepository) GetAll(ctx context.Context) []*Vers } } sort.Slice(allRecords, func(i, j int) bool { - return allRecords[i].UpdatedAt.After(allRecords[j].UpdatedAt) + return allRecords[i].updatedAt.After(allRecords[j].updatedAt) }) return allRecords } @@ -319,7 +405,7 @@ func (v *VersionAnyApprovalRecordRepository) GetAllForVersionAndEnvironment(ctx copy(records, v.records[versionID][environmentID]) } sort.Slice(records, func(i, j int) bool { - return records[i].UpdatedAt.After(records[j].UpdatedAt) + return records[i].updatedAt.After(records[j].updatedAt) }) return records } diff --git a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository_test.go b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository_test.go index d5d9a20f3..75b3f3ba6 100644 --- a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository_test.go +++ b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository_test.go @@ -47,7 +47,7 @@ func (b *TestStepBundle) executeStep() { } if b.step.deleteRecord != nil { - err := b.repo.Delete(b.ctx, b.step.deleteRecord.ID) + err := b.repo.Delete(b.ctx, b.step.deleteRecord.GetID()) if err != nil { b.t.Fatalf("failed to delete record: %v", err) } @@ -59,6 +59,8 @@ func (b *TestStepBundle) executeStep() { b.t.Fatalf("failed to upsert record: %v", err) } } + + time.Sleep(1 * time.Millisecond) } func (b *TestStepBundle) validateExpectedState() { @@ -69,11 +71,11 @@ func (b *TestStepBundle) validateExpectedState() { for i, expectedRecord := range records { actualRecord := actualRecords[i] - assert.Equal(b.t, expectedRecord.ID, actualRecord.ID) - assert.Equal(b.t, expectedRecord.VersionID, actualRecord.VersionID) - assert.Equal(b.t, expectedRecord.EnvironmentID, actualRecord.EnvironmentID) - assert.Equal(b.t, expectedRecord.UserID, actualRecord.UserID) - assert.Equal(b.t, expectedRecord.Status, actualRecord.Status) + assert.Equal(b.t, expectedRecord.GetID(), actualRecord.GetID()) + assert.Equal(b.t, expectedRecord.GetVersionID(), actualRecord.GetVersionID()) + assert.Equal(b.t, expectedRecord.GetEnvironmentID(), actualRecord.GetEnvironmentID()) + assert.Equal(b.t, expectedRecord.GetUserID(), actualRecord.GetUserID()) + assert.Equal(b.t, expectedRecord.GetStatus(), actualRecord.GetStatus()) } } } @@ -84,23 +86,23 @@ func TestBasicCRUD(t *testing.T) { name: "creates a record", steps: []VersionAnyApprovalRecordRepositoryTestStep{ { - createRecord: &versionanyapproval.VersionAnyApprovalRecord{ - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - }, + createRecord: versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build(), expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ "version-1": { "environment-1": { - { - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - }, + versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build(), }, }, }, @@ -112,45 +114,45 @@ func TestBasicCRUD(t *testing.T) { name: "updates a record", steps: []VersionAnyApprovalRecordRepositoryTestStep{ { - createRecord: &versionanyapproval.VersionAnyApprovalRecord{ - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusRejected, - }, + createRecord: versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusRejected). + Build(), expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ "version-1": { "environment-1": { - { - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusRejected, - }, + versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusRejected). + Build(), }, }, }, }, { - updateRecord: &versionanyapproval.VersionAnyApprovalRecord{ - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - }, + updateRecord: versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build(), expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ "version-1": { "environment-1": { - { - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - }, + versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build(), }, }, }, @@ -162,35 +164,35 @@ func TestBasicCRUD(t *testing.T) { name: "deletes a record", steps: []VersionAnyApprovalRecordRepositoryTestStep{ { - createRecord: &versionanyapproval.VersionAnyApprovalRecord{ - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - }, + createRecord: versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build(), expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ "version-1": { "environment-1": { - { - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - }, + versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build(), }, }, }, }, { - deleteRecord: &versionanyapproval.VersionAnyApprovalRecord{ - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - }, + deleteRecord: versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build(), expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ "version-1": {"environment-1": {}}, }, @@ -202,45 +204,45 @@ func TestBasicCRUD(t *testing.T) { name: "upserts a record", steps: []VersionAnyApprovalRecordRepositoryTestStep{ { - upsertRecord: &versionanyapproval.VersionAnyApprovalRecord{ - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusRejected, - }, + upsertRecord: versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusRejected). + Build(), expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ "version-1": { "environment-1": { - { - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusRejected, - }, + versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusRejected). + Build(), }, }, }, }, { - upsertRecord: &versionanyapproval.VersionAnyApprovalRecord{ - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - }, + upsertRecord: versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build(), expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ "version-1": { "environment-1": { - { - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - }, + versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build(), }, }, }, @@ -252,57 +254,52 @@ func TestBasicCRUD(t *testing.T) { name: "maintains record order by updated at", steps: []VersionAnyApprovalRecordRepositoryTestStep{ { - createRecord: &versionanyapproval.VersionAnyApprovalRecord{ - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - UpdatedAt: time.Now().Add(-time.Second * 2), - }, + createRecord: versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build(), expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ "version-1": { "environment-1": { - { - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - UpdatedAt: time.Now().Add(-time.Second * 2), - }, + versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build(), }, }, }, }, { - createRecord: &versionanyapproval.VersionAnyApprovalRecord{ - ID: "record-2", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-2", - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - UpdatedAt: time.Now().Add(-time.Second), - }, + createRecord: versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-2"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-2"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build(), expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ "version-1": { "environment-1": { - { - ID: "record-2", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-2", - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - UpdatedAt: time.Now().Add(-time.Second), - }, - { - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - UpdatedAt: time.Now().Add(-time.Second * 2), - }, + versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-2"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-2"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build(), + versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build(), }, }, }, @@ -314,81 +311,81 @@ func TestBasicCRUD(t *testing.T) { name: "maintains order when records are updated", steps: []VersionAnyApprovalRecordRepositoryTestStep{ { - createRecord: &versionanyapproval.VersionAnyApprovalRecord{ - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusRejected, - }, + createRecord: versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusRejected). + Build(), expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ "version-1": { "environment-1": { - { - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusRejected, - }, + versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusRejected). + Build(), }, }, }, }, { - createRecord: &versionanyapproval.VersionAnyApprovalRecord{ - ID: "record-2", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-2", - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - }, + createRecord: versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-2"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-2"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build(), expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ "version-1": { "environment-1": { - { - ID: "record-2", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-2", - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - }, - { - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusRejected, - }, + versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-2"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-2"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build(), + versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusRejected). + Build(), }, }, }, }, { - updateRecord: &versionanyapproval.VersionAnyApprovalRecord{ - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - }, + updateRecord: versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build(), expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ "version-1": { "environment-1": { - { - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - }, - { - ID: "record-2", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-2", - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - }, + versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build(), + versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-2"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-2"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build(), }, }, }, @@ -428,13 +425,13 @@ func TestDuplicateRecords(t *testing.T) { repo := versionanyapproval.NewVersionAnyApprovalRecordRepository() ctx := context.Background() - record := &versionanyapproval.VersionAnyApprovalRecord{ - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - } + record := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build() err := repo.Create(ctx, record) assert.NilError(t, err) @@ -445,8 +442,15 @@ func TestDuplicateRecords(t *testing.T) { err = repo.Upsert(ctx, record) assert.NilError(t, err) - record.Status = versionanyapproval.VersionAnyApprovalRecordStatusRejected - err = repo.Update(ctx, record) + record2 := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusRejected). + Build() + + err = repo.Update(ctx, record2) assert.NilError(t, err) } @@ -454,24 +458,32 @@ func TestUpdateNonExistingRecord(t *testing.T) { repo := versionanyapproval.NewVersionAnyApprovalRecordRepository() ctx := context.Background() - record := &versionanyapproval.VersionAnyApprovalRecord{ - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - } + // record := &versionanyapproval.VersionAnyApprovalRecord{ + // ID: "record-1", + // VersionID: "version-1", + // EnvironmentID: "environment-1", + // UserID: "user-1", + // Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, + // } + + record := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build() err := repo.Create(ctx, record) assert.NilError(t, err) - record2 := &versionanyapproval.VersionAnyApprovalRecord{ - ID: "record-2", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusRejected, - } + record2 := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-2"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusRejected). + Build() err = repo.Update(ctx, record2) assert.ErrorContains(t, err, "record not found") @@ -481,46 +493,64 @@ func TestUpdateRecordWithReadonlyIds(t *testing.T) { repo := versionanyapproval.NewVersionAnyApprovalRecordRepository() ctx := context.Background() - record := &versionanyapproval.VersionAnyApprovalRecord{ - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - } + record := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build() err := repo.Create(ctx, record) assert.NilError(t, err) - updateRecord := &versionanyapproval.VersionAnyApprovalRecord{ - ID: "record-1", - VersionID: "version-2", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - } + updateRecord := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-2"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build() err = repo.Update(ctx, updateRecord) assert.ErrorContains(t, err, "version ID mismatch") err = repo.Upsert(ctx, updateRecord) assert.ErrorContains(t, err, "version ID mismatch") - updateRecord.VersionID = "version-1" - updateRecord.EnvironmentID = "environment-2" - err = repo.Update(ctx, updateRecord) + updateRecord2 := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-2"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build() + + err = repo.Update(ctx, updateRecord2) assert.ErrorContains(t, err, "environment ID mismatch") - err = repo.Upsert(ctx, updateRecord) + err = repo.Upsert(ctx, updateRecord2) assert.ErrorContains(t, err, "environment ID mismatch") - updateRecord.EnvironmentID = "environment-1" - updateRecord.UserID = "user-2" - err = repo.Update(ctx, updateRecord) + updateRecord3 := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-2"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build() + + err = repo.Update(ctx, updateRecord3) assert.ErrorContains(t, err, "user ID mismatch") - err = repo.Upsert(ctx, updateRecord) + err = repo.Upsert(ctx, updateRecord3) assert.ErrorContains(t, err, "user ID mismatch") - updateRecord.UserID = "user-1" - err = repo.Update(ctx, updateRecord) + updateRecord4 := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build() + err = repo.Update(ctx, updateRecord4) assert.NilError(t, err) } @@ -546,25 +576,25 @@ func TestGlobalIDUniqueness(t *testing.T) { ctx := context.Background() // Create first record in version-1/environment-1 - record1 := &versionanyapproval.VersionAnyApprovalRecord{ - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - } + record1 := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build() err := repo.Create(ctx, record1) assert.NilError(t, err) // Attempt to create a record with the same ID in a different version/environment bucket - record2 := &versionanyapproval.VersionAnyApprovalRecord{ - ID: "record-1", // Same ID as record1 - VersionID: "version-2", // Different version - EnvironmentID: "environment-2", // Different environment - UserID: "user-2", - Status: versionanyapproval.VersionAnyApprovalRecordStatusRejected, - } + record2 := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-2"). + WithEnvironmentID("environment-2"). + WithUserID("user-2"). + WithStatus(versionanyapproval.ApprovalRecordStatusRejected). + Build() err = repo.Create(ctx, record2) assert.ErrorContains(t, err, "record already exists") @@ -572,11 +602,11 @@ func TestGlobalIDUniqueness(t *testing.T) { // Verify the first record still exists and is unchanged existingRecord := repo.Get(ctx, "record-1") assert.Assert(t, existingRecord != nil) - assert.Equal(t, existingRecord.ID, "record-1") - assert.Equal(t, existingRecord.VersionID, "version-1") - assert.Equal(t, existingRecord.EnvironmentID, "environment-1") - assert.Equal(t, existingRecord.UserID, "user-1") - assert.Equal(t, existingRecord.Status, versionanyapproval.VersionAnyApprovalRecordStatusApproved) + assert.Equal(t, existingRecord.GetID(), "record-1") + assert.Equal(t, existingRecord.GetVersionID(), "version-1") + assert.Equal(t, existingRecord.GetEnvironmentID(), "environment-1") + assert.Equal(t, existingRecord.GetUserID(), "user-1") + assert.Equal(t, existingRecord.GetStatus(), versionanyapproval.ApprovalRecordStatusApproved) // Verify the second record was not created nonExistentRecord := repo.Get(ctx, "record-2") @@ -587,24 +617,24 @@ func TestUserUniquenessForEnvAndVersion(t *testing.T) { repo := versionanyapproval.NewVersionAnyApprovalRecordRepository() ctx := context.Background() - record1 := &versionanyapproval.VersionAnyApprovalRecord{ - ID: "record-1", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - } + record1 := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build() err := repo.Create(ctx, record1) assert.NilError(t, err) - record2 := &versionanyapproval.VersionAnyApprovalRecord{ - ID: "record-2", - VersionID: "version-1", - EnvironmentID: "environment-1", - UserID: "user-1", - Status: versionanyapproval.VersionAnyApprovalRecordStatusRejected, - } + record2 := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-2"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusRejected). + Build() err = repo.Create(ctx, record2) assert.ErrorContains(t, err, "record already exists for user") @@ -612,7 +642,14 @@ func TestUserUniquenessForEnvAndVersion(t *testing.T) { err = repo.Upsert(ctx, record2) assert.ErrorContains(t, err, "record already exists for user") - record2.UserID = "user-2" - err = repo.Upsert(ctx, record2) + record3 := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-2"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-2"). + WithStatus(versionanyapproval.ApprovalRecordStatusRejected). + Build() + + err = repo.Upsert(ctx, record3) assert.NilError(t, err) } diff --git a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_rule.go b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_rule.go index 0a3786e1e..0dab1e529 100644 --- a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_rule.go +++ b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_rule.go @@ -49,7 +49,7 @@ func (r *VersionAnyApprovalRule) Evaluate(ctx context.Context, target rt.Release approvedCount := 0 for _, record := range records { - if record.Status == VersionAnyApprovalRecordStatusApproved { + if record.IsApproved() { approvedCount++ } } diff --git a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_rule_test.go b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_rule_test.go index a7d06ed6a..f0f75e7b3 100644 --- a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_rule_test.go +++ b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_rule_test.go @@ -90,26 +90,26 @@ func TestVersionAnyApprovalRule(t *testing.T) { versionID := test.version.GetID() for i := 0; i < test.numApprovalsToCreate; i++ { - record := &versionanyapproval.VersionAnyApprovalRecord{ - ID: randomID(), - VersionID: versionID, - EnvironmentID: environmentID, - UserID: randomID(), - Status: versionanyapproval.VersionAnyApprovalRecordStatusApproved, - } + record := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID(randomID()). + WithVersionID(versionID). + WithEnvironmentID(environmentID). + WithUserID(randomID()). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build() err := repo.Create(ctx, record) assert.NilError(t, err) } for i := 0; i < test.numRejectionsToCreate; i++ { - record := &versionanyapproval.VersionAnyApprovalRecord{ - ID: randomID(), - VersionID: versionID, - EnvironmentID: environmentID, - UserID: randomID(), - Status: versionanyapproval.VersionAnyApprovalRecordStatusRejected, - } + record := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID(randomID()). + WithVersionID(versionID). + WithEnvironmentID(environmentID). + WithUserID(randomID()). + WithStatus(versionanyapproval.ApprovalRecordStatusRejected). + Build() err := repo.Create(ctx, record) assert.NilError(t, err) From 8878a2fa03e9be7cbb547dead425713ca748fe38 Mon Sep 17 00:00:00 2001 From: Aditya Choudhari Date: Mon, 18 Aug 2025 14:33:53 -0700 Subject: [PATCH 7/8] more robust testing --- .../version-any-approval/approval_record.go | 2 + .../version_any_approval_record_repository.go | 59 ++- ...ion_any_approval_record_repository_test.go | 352 +++++++++++++++++- 3 files changed, 407 insertions(+), 6 deletions(-) diff --git a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/approval_record.go b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/approval_record.go index e8970d7e4..a7bdfbbd5 100644 --- a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/approval_record.go +++ b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/approval_record.go @@ -17,6 +17,8 @@ type ApprovalRecord interface { GetStatus() ApprovalRecordStatus GetApprovedAt() *time.Time GetReason() *string + GetCreatedAt() time.Time + GetUpdatedAt() time.Time IsApproved() bool } diff --git a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go index 21be6a686..a6af4a539 100644 --- a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go +++ b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository.go @@ -33,6 +33,8 @@ type versionAnyApprovalRecordBuilder struct { userID string status ApprovalRecordStatus reason *string + createdAt *time.Time + updatedAt *time.Time } func NewVersionAnyApprovalRecordBuilder() *versionAnyApprovalRecordBuilder { @@ -76,17 +78,32 @@ func (b *versionAnyApprovalRecordBuilder) WithReason(reason *string) *versionAny return b } +func (b *versionAnyApprovalRecordBuilder) WithCreatedAt(createdAt time.Time) *versionAnyApprovalRecordBuilder { + b.createdAt = &createdAt + return b +} + +func (b *versionAnyApprovalRecordBuilder) WithUpdatedAt(updatedAt time.Time) *versionAnyApprovalRecordBuilder { + b.updatedAt = &updatedAt + return b +} + func (b *versionAnyApprovalRecordBuilder) Build() *VersionAnyApprovalRecord { - return &VersionAnyApprovalRecord{ + r := &VersionAnyApprovalRecord{ id: b.id, versionID: b.versionID, environmentID: b.environmentID, userID: b.userID, status: b.status, reason: b.reason, - createdAt: time.Now().UTC(), - updatedAt: time.Now().UTC(), } + if b.createdAt != nil { + r.createdAt = *b.createdAt + } + if b.updatedAt != nil { + r.updatedAt = *b.updatedAt + } + return r } func (v VersionAnyApprovalRecord) GetID() string { @@ -121,6 +138,14 @@ func (v VersionAnyApprovalRecord) GetReason() *string { return v.reason } +func (v VersionAnyApprovalRecord) GetCreatedAt() time.Time { + return v.createdAt +} + +func (v VersionAnyApprovalRecord) GetUpdatedAt() time.Time { + return v.updatedAt +} + var _ model.Repository[VersionAnyApprovalRecord] = (*VersionAnyApprovalRecordRepository)(nil) // VersionAnyApprovalRecordRepository is a repository for all version-any-approval records. @@ -169,6 +194,10 @@ func (v *VersionAnyApprovalRecordRepository) validateRecord(record *VersionAnyAp return fmt.Errorf("user ID is empty") } + if record.status == "" { + return fmt.Errorf("status is empty") + } + return nil } @@ -176,10 +205,16 @@ func (v *VersionAnyApprovalRecordRepository) Create(ctx context.Context, record v.mu.Lock() defer v.mu.Unlock() + now := time.Now().UTC() + if err := v.validateRecord(record); err != nil { return err } + if record.status == ApprovalRecordStatusApproved && record.approvedAt == nil { + record.approvedAt = &now + } + environmentID := record.environmentID versionID := record.versionID @@ -214,6 +249,7 @@ func (v *VersionAnyApprovalRecordRepository) Create(ctx context.Context, record } v.records[versionID][environmentID] = append(currentRecords, record) + fmt.Println("creatd record", record.id, record.updatedAt) return nil } @@ -250,6 +286,8 @@ func (v *VersionAnyApprovalRecordRepository) Update(ctx context.Context, record v.mu.Lock() defer v.mu.Unlock() + now := time.Now().UTC() + if err := v.validateRecord(record); err != nil { return err } @@ -276,10 +314,14 @@ func (v *VersionAnyApprovalRecordRepository) Update(ctx context.Context, record existingRecord.reason = record.reason ts := record.updatedAt if ts.Equal(existingRecord.updatedAt) || ts.Before(existingRecord.updatedAt) { - ts = time.Now().UTC() + ts = now } existingRecord.updatedAt = ts + if existingRecord.status == ApprovalRecordStatusApproved && existingRecord.approvedAt == nil { + existingRecord.approvedAt = &now + } + return nil } @@ -287,10 +329,16 @@ func (v *VersionAnyApprovalRecordRepository) Upsert(ctx context.Context, record v.mu.Lock() defer v.mu.Unlock() + now := time.Now().UTC() + if err := v.validateRecord(record); err != nil { return err } + if record.status == ApprovalRecordStatusApproved && record.approvedAt == nil { + record.approvedAt = &now + } + environmentID := record.environmentID versionID := record.versionID @@ -313,9 +361,10 @@ func (v *VersionAnyApprovalRecordRepository) Upsert(ctx context.Context, record existingRecord.reason = record.reason ts := record.updatedAt if ts.Equal(existingRecord.updatedAt) || ts.Before(existingRecord.updatedAt) { - ts = time.Now().UTC() + ts = now } existingRecord.updatedAt = ts + return nil } diff --git a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository_test.go b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository_test.go index 75b3f3ba6..1e68023eb 100644 --- a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository_test.go +++ b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository_test.go @@ -2,6 +2,7 @@ package versionanyapproval_test import ( "context" + "fmt" "testing" "time" versionanyapproval "workspace-engine/pkg/engine/policy/rules/version-any-approval" @@ -69,6 +70,10 @@ func (b *TestStepBundle) validateExpectedState() { actualRecords := b.repo.GetAllForVersionAndEnvironment(b.ctx, versionID, environmentID) assert.Equal(b.t, len(records), len(actualRecords)) + for _, actualRecord := range actualRecords { + fmt.Println(actualRecord.GetID(), actualRecord.GetUpdatedAt()) + } + for i, expectedRecord := range records { actualRecord := actualRecords[i] assert.Equal(b.t, expectedRecord.GetID(), actualRecord.GetID()) @@ -185,6 +190,35 @@ func TestBasicCRUD(t *testing.T) { }, }, }, + { + createRecord: versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-2"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-2"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build(), + expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ + "version-1": { + "environment-1": { + versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-2"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-2"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build(), + versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build(), + }, + }, + }, + }, { deleteRecord: versionanyapproval.NewVersionAnyApprovalRecordBuilder(). WithID("record-1"). @@ -194,7 +228,17 @@ func TestBasicCRUD(t *testing.T) { WithStatus(versionanyapproval.ApprovalRecordStatusApproved). Build(), expectedRecords: map[string]map[string][]*versionanyapproval.VersionAnyApprovalRecord{ - "version-1": {"environment-1": {}}, + "version-1": { + "environment-1": { + versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-2"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-2"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build(), + }, + }, }, }, }, @@ -653,3 +697,309 @@ func TestUserUniquenessForEnvAndVersion(t *testing.T) { err = repo.Upsert(ctx, record3) assert.NilError(t, err) } + +func TestExists(t *testing.T) { + repo := versionanyapproval.NewVersionAnyApprovalRecordRepository() + ctx := context.Background() + + record := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build() + + err := repo.Create(ctx, record) + assert.NilError(t, err) + + assert.Equal(t, repo.Exists(ctx, "record-1"), true) + assert.Equal(t, repo.Exists(ctx, "record-2"), false) +} + +func TestGetAll(t *testing.T) { + repo := versionanyapproval.NewVersionAnyApprovalRecordRepository() + ctx := context.Background() + + record := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build() + + err := repo.Create(ctx, record) + assert.NilError(t, err) + + record2 := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-2"). + WithVersionID("version-2"). + WithEnvironmentID("environment-2"). + WithUserID("user-2"). + WithStatus(versionanyapproval.ApprovalRecordStatusRejected). + Build() + + err = repo.Create(ctx, record2) + assert.NilError(t, err) + + records := repo.GetAll(ctx) + assert.Equal(t, len(records), 2) + assert.Equal(t, records[0].GetID(), "record-2") + assert.Equal(t, records[1].GetID(), "record-1") +} + +func TestCreateWithReason(t *testing.T) { + repo := versionanyapproval.NewVersionAnyApprovalRecordRepository() + ctx := context.Background() + + reason := "reason-1" + record := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + WithReason(&reason). + Build() + + err := repo.Create(ctx, record) + assert.NilError(t, err) + + actualRecord := repo.Get(ctx, "record-1") + assert.Equal(t, *actualRecord.GetReason(), reason) + + recordWithNilReason := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-2"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-2"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build() + + err = repo.Create(ctx, recordWithNilReason) + assert.NilError(t, err) + + actualRecord = repo.Get(ctx, "record-2") + assert.Equal(t, actualRecord.GetReason(), (*string)(nil)) +} + +func TestCreateInvalidRecord(t *testing.T) { + repo := versionanyapproval.NewVersionAnyApprovalRecordRepository() + ctx := context.Background() + + recordWithEmptyVersionID := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build() + + err := repo.Create(ctx, recordWithEmptyVersionID) + assert.ErrorContains(t, err, "version ID is empty") + + recordWithEmptyEnvironmentID := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build() + + err = repo.Create(ctx, recordWithEmptyEnvironmentID) + assert.ErrorContains(t, err, "environment ID is empty") + + recordWithEmptyUserID := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build() + + err = repo.Create(ctx, recordWithEmptyUserID) + assert.ErrorContains(t, err, "user ID is empty") + + recordWithEmptyStatus := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + Build() + + recordWithEmptyID := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build() + + err = repo.Create(ctx, recordWithEmptyID) + assert.ErrorContains(t, err, "record ID is empty") + + err = repo.Create(ctx, recordWithEmptyStatus) + assert.ErrorContains(t, err, "status is empty") +} + +func TestCreateWithTimestamps(t *testing.T) { + repo := versionanyapproval.NewVersionAnyApprovalRecordRepository() + ctx := context.Background() + + now := time.Now().UTC() + + record := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + WithCreatedAt(now). + WithUpdatedAt(now). + Build() + + err := repo.Create(ctx, record) + assert.NilError(t, err) + + actualRecord := repo.Get(ctx, "record-1") + assert.Equal(t, actualRecord.GetCreatedAt(), now) + assert.Equal(t, actualRecord.GetUpdatedAt(), now) + + record2 := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-2"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-2"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + WithCreatedAt(now). + WithUpdatedAt(now). + Build() + + err = repo.Upsert(ctx, record2) + assert.NilError(t, err) + + actualRecord = repo.Get(ctx, "record-2") + assert.Equal(t, actualRecord.GetCreatedAt(), now) + assert.Equal(t, actualRecord.GetUpdatedAt(), now) +} + +func TestInsertsTimestampsIfNotProvided(t *testing.T) { + repo := versionanyapproval.NewVersionAnyApprovalRecordRepository() + ctx := context.Background() + + record := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build() + + err := repo.Create(ctx, record) + assert.NilError(t, err) + + actualRecord := repo.Get(ctx, "record-1") + assert.Assert(t, actualRecord.GetCreatedAt() != time.Time{}) + assert.Assert(t, actualRecord.GetUpdatedAt() != time.Time{}) + + record2 := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-2"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-2"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build() + + err = repo.Upsert(ctx, record2) + assert.NilError(t, err) + + actualRecord = repo.Get(ctx, "record-2") + assert.Assert(t, actualRecord.GetCreatedAt() != time.Time{}) + assert.Assert(t, actualRecord.GetUpdatedAt() != time.Time{}) +} + +func TestAutopopulatesApprovedAtIfBuildWithApprovedStatus(t *testing.T) { + repo := versionanyapproval.NewVersionAnyApprovalRecordRepository() + ctx := context.Background() + + record := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-1"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-1"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build() + + err := repo.Create(ctx, record) + assert.NilError(t, err) + + actualRecord := repo.Get(ctx, "record-1") + assert.Assert(t, actualRecord.GetApprovedAt() != nil) + + record2 := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-2"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-2"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build() + + err = repo.Upsert(ctx, record2) + assert.NilError(t, err) + + actualRecord = repo.Get(ctx, "record-2") + assert.Assert(t, actualRecord.GetApprovedAt() != nil) + + record3 := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-3"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-3"). + WithStatus(versionanyapproval.ApprovalRecordStatusRejected). + Build() + + err = repo.Upsert(ctx, record3) + assert.NilError(t, err) + + actualRecord = repo.Get(ctx, "record-3") + assert.Assert(t, actualRecord.GetApprovedAt() == nil) + + record3Updated := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-3"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-3"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build() + + err = repo.Upsert(ctx, record3Updated) + assert.NilError(t, err) + + actualRecord = repo.Get(ctx, "record-3") + assert.Assert(t, actualRecord.GetApprovedAt() != nil) + + record4 := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-4"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-4"). + WithStatus(versionanyapproval.ApprovalRecordStatusRejected). + Build() + + err = repo.Upsert(ctx, record4) + assert.NilError(t, err) + + actualRecord = repo.Get(ctx, "record-4") + assert.Assert(t, actualRecord.GetApprovedAt() == nil) + + record4Upserted := versionanyapproval.NewVersionAnyApprovalRecordBuilder(). + WithID("record-4"). + WithVersionID("version-1"). + WithEnvironmentID("environment-1"). + WithUserID("user-4"). + WithStatus(versionanyapproval.ApprovalRecordStatusApproved). + Build() + + err = repo.Upsert(ctx, record4Upserted) + assert.NilError(t, err) + + actualRecord = repo.Get(ctx, "record-4") + assert.Assert(t, actualRecord.GetApprovedAt() != nil) +} From b4b3e986db1d5ec84b2a451fd94f0955cac59cbd Mon Sep 17 00:00:00 2001 From: Aditya Choudhari Date: Mon, 18 Aug 2025 14:57:16 -0700 Subject: [PATCH 8/8] rabbit comment --- .../version_any_approval_record_repository_test.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository_test.go b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository_test.go index 1e68023eb..1c9bd3ed2 100644 --- a/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository_test.go +++ b/apps/workspace-engine/pkg/engine/policy/rules/version-any-approval/version_any_approval_record_repository_test.go @@ -652,9 +652,14 @@ func TestGlobalIDUniqueness(t *testing.T) { assert.Equal(t, existingRecord.GetUserID(), "user-1") assert.Equal(t, existingRecord.GetStatus(), versionanyapproval.ApprovalRecordStatusApproved) - // Verify the second record was not created - nonExistentRecord := repo.Get(ctx, "record-2") - assert.Assert(t, nonExistentRecord == nil) + // verify that the second record was not created + allRecords := repo.GetAll(ctx) + assert.Equal(t, len(allRecords), 1) + assert.Equal(t, allRecords[0].GetID(), "record-1") + assert.Equal(t, allRecords[0].GetVersionID(), "version-1") + assert.Equal(t, allRecords[0].GetEnvironmentID(), "environment-1") + assert.Equal(t, allRecords[0].GetUserID(), "user-1") + assert.Equal(t, allRecords[0].GetStatus(), versionanyapproval.ApprovalRecordStatusApproved) } func TestUserUniquenessForEnvAndVersion(t *testing.T) {