Add type for direction

Azure · May 10, 2024 · 0246f2b · 0246f2b
1 parent 518d4a7
commit 0246f2b
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 17 deletions.
diff --git a/internal/controllers/reconciliation/controller.go b/internal/controllers/reconciliation/controller.go
@@ -150,7 +150,7 @@ func (c *Controller) Reconcile(ctx context.Context, req *reconstitution.Request)
 
 	// Evaluate the readiness of resources in the previous readiness group
 	if (status == nil || !status.Reconciled) && !resource.Deleted() {
-		dependencies := c.resourceClient.RangeByReadinessGroup(ctx, synRef, resource.ReadinessGroup, -1)
+		dependencies := c.resourceClient.RangeByReadinessGroup(ctx, synRef, resource.ReadinessGroup, reconstitution.RangeDesc)
 		for _, dep := range dependencies {
 			slice := &apiv1.ResourceSlice{}
 			err = c.client.Get(ctx, dep.ManifestRef.Slice, slice)

diff --git a/internal/reconstitution/cache.go b/internal/reconstitution/cache.go
@@ -72,11 +72,11 @@ func (c *Cache) Get(ctx context.Context, comp *SynthesisRef, ref *resource.Ref)
 	return res, ok
 }
 
-func (c *Cache) RangeByReadinessGroup(ctx context.Context, comp *SynthesisRef, group uint, dir int) []*Resource {
+func (c *Cache) RangeByReadinessGroup(ctx context.Context, comp *SynthesisRef, group uint, dir RangeDirection) []*Resource {
 	c.mut.Lock()
 	defer c.mut.Unlock()
 
-	if group == 0 && dir == -1 {
+	if group == 0 && !dir {
 		return nil
 	}
 
@@ -91,7 +91,7 @@ func (c *Cache) RangeByReadinessGroup(ctx context.Context, comp *SynthesisRef, g
 	}
 
 	// If we're adjacent...
-	if dir > 0 {
+	if dir {
 		if node.Right != nil {
 			return node.Right.Value
 		}
@@ -102,7 +102,7 @@ func (c *Cache) RangeByReadinessGroup(ctx context.Context, comp *SynthesisRef, g
 	}
 
 	// ...otherwise we need to find it
-	if dir > 0 {
+	if dir {
 		node, ok = resources.ByReadinessGroup.Ceiling(group + 1)
 	} else {
 		node, ok = resources.ByReadinessGroup.Floor(group - 1)

diff --git a/internal/reconstitution/cache_test.go b/internal/reconstitution/cache_test.go
@@ -287,39 +287,39 @@ func TestCacheRangeByReadinessGroup(t *testing.T) {
 	require.NoError(t, err)
 
 	// Ranging backwards from 0 should never return anything
-	refs := c.RangeByReadinessGroup(ctx, compRef, 0, -1)
+	refs := c.RangeByReadinessGroup(ctx, compRef, 0, RangeDesc)
 	assert.Equal(t, []string{}, reqsToNames(refs))
 
 	// Ranging forwards after all groups should not return anything
-	refs = c.RangeByReadinessGroup(ctx, compRef, 100, 1)
+	refs = c.RangeByReadinessGroup(ctx, compRef, 100, RangeAsc)
 	assert.Equal(t, []string{}, reqsToNames(refs))
 
 	// Prove synthesis refs are honored
-	refs = c.RangeByReadinessGroup(ctx, &SynthesisRef{CompositionName: "nope"}, 1, 1)
+	refs = c.RangeByReadinessGroup(ctx, &SynthesisRef{CompositionName: "nope"}, 1, RangeAsc)
 	assert.Equal(t, []string{}, reqsToNames(refs))
 
-	refs = c.RangeByReadinessGroup(ctx, &SynthesisRef{CompositionName: "nope"}, 1, -1)
+	refs = c.RangeByReadinessGroup(ctx, &SynthesisRef{CompositionName: "nope"}, 1, RangeDesc)
 	assert.Equal(t, []string{}, reqsToNames(refs))
 
 	// This node doesn't exist in the tree, this isn't possible at runtime
-	refs = c.RangeByReadinessGroup(ctx, compRef, 100, -1)
+	refs = c.RangeByReadinessGroup(ctx, compRef, 100, RangeDesc)
 	assert.Equal(t, []string{}, reqsToNames(refs))
 
 	// Ranging forwards returns all resources in the next group, but not groups after that
-	refs = c.RangeByReadinessGroup(ctx, compRef, 0, 1)
+	refs = c.RangeByReadinessGroup(ctx, compRef, 0, RangeAsc)
 	assert.Equal(t, []string{"group-1", "group-also-1"}, reqsToNames(refs))
 
-	refs = c.RangeByReadinessGroup(ctx, compRef, 1, 1)
+	refs = c.RangeByReadinessGroup(ctx, compRef, 1, RangeAsc)
 	assert.Equal(t, []string{"group-3"}, reqsToNames(refs))
 
-	refs = c.RangeByReadinessGroup(ctx, compRef, 3, 1)
+	refs = c.RangeByReadinessGroup(ctx, compRef, 3, RangeAsc)
 	assert.Equal(t, []string{}, reqsToNames(refs))
 
 	// Ranging backwards returns all resources in the previous group, but not groups before that
-	refs = c.RangeByReadinessGroup(ctx, compRef, 1, -1)
+	refs = c.RangeByReadinessGroup(ctx, compRef, 1, RangeDesc)
 	assert.Equal(t, []string{"default-group"}, reqsToNames(refs))
 
-	refs = c.RangeByReadinessGroup(ctx, compRef, 3, -1)
+	refs = c.RangeByReadinessGroup(ctx, compRef, 3, RangeDesc)
 	assert.Equal(t, []string{"group-1", "group-also-1"}, reqsToNames(refs))
 }
 

diff --git a/internal/reconstitution/controller.go b/internal/reconstitution/controller.go
@@ -147,7 +147,7 @@ func (r *controller) HandleReadinessTransition(ctx context.Context, req ctrl.Req
 		}
 
 		synRef := &SynthesisRef{CompositionName: owner.Name, Namespace: req.Namespace, UUID: slice.Spec.SynthesisUUID}
-		resources := r.Cache.RangeByReadinessGroup(ctx, synRef, res.ReadinessGroup, 1)
+		resources := r.Cache.RangeByReadinessGroup(ctx, synRef, res.ReadinessGroup, RangeAsc)
 		for _, res := range resources {
 			// TODO: This can be optimized by skipping the Add call if `res` is already ready
 			r.queue.Add(Request{

diff --git a/internal/reconstitution/reconstitution.go b/internal/reconstitution/reconstitution.go
@@ -19,9 +19,16 @@ type Reconciler interface {
 // Client provides read/write access to a collection of reconstituted resources.
 type Client interface {
 	Get(ctx context.Context, syn *SynthesisRef, res *resource.Ref) (*resource.Resource, bool)
-	RangeByReadinessGroup(ctx context.Context, syn *SynthesisRef, group uint, dir int) []*Resource
+	RangeByReadinessGroup(ctx context.Context, syn *SynthesisRef, group uint, dir RangeDirection) []*Resource
 }
 
+type RangeDirection bool
+
+var (
+	RangeAsc  RangeDirection = true
+	RangeDesc RangeDirection = false
+)
+
 // SynthesisRef refers to a specific synthesis of a composition.
 type SynthesisRef struct {
 	CompositionName, Namespace, UUID string