PR(REFAC): Reomve Explicit PolicyID from AddPolicy & UpdateSchema

- Just assert policyID is non-empty for PolicyID
- Make PolicyID index substitution easier with `UpdateSchema`
- Optional if we still want to do exact assertions

tests/integration/acp.go

@@ -20,6 +20,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"slices"
 	"strings"
 	"sync/atomic"
 	"time"
@@ -93,7 +94,11 @@ type AddPolicy struct {
 	Identity immutable.Option[int]
 
 	// The expected policyID generated based on the Policy loaded in to the ACP system.
-	ExpectedPolicyID string
+	//
+	// This is an optional attribute, for situations where a test might want to assert
+	// the exact policyID. When this is not provided the test will just assert that
+	// the resulting policyID is not empty.
+	ExpectedPolicyID immutable.Option[string]
 
 	// Any error expected from the action. Optional.
 	//
@@ -107,12 +112,23 @@ func addPolicyACP(
 	s *state,
 	action AddPolicy,
 ) {
-	// If we expect an error, then ExpectedPolicyID should be empty.
-	if action.ExpectedError != "" && action.ExpectedPolicyID != "" {
+	// If we expect an error, then ExpectedPolicyID should never be provided.
+	if action.ExpectedError != "" && !action.ExpectedPolicyID.HasValue() {
 		require.Fail(s.t, "Expected error should not have an expected policyID with it.", s.testCase.Description)
 	}
 
 	nodeIDs, nodes := getNodesWithIDs(action.NodeID, s.nodes)
+	maxNodeID := slices.Max(nodeIDs)
+	policyIDsAddedSoFar := len(s.policyIDs)
+	// Expand the policyIDs slice once, so we can minimize how many times we need to expaind it.
+	// We use the maximum nodeID provided to make sure policyIDs slice can accomodate upto that nodeID.
+	if policyIDsAddedSoFar <= maxNodeID {
+		// Expand the slice if required, so that the policyID can be accessed by node index
+		policyIDs := make([][]string, maxNodeID+1)
+		copy(policyIDs, s.policyIDs)
+		s.policyIDs = policyIDs
+	}
+
 	for index, node := range nodes {
 		nodeID := nodeIDs[index]
 		identity := getIdentity(s, nodeID, action.Identity)
@@ -124,7 +140,13 @@ func addPolicyACP(
 
 		if !expectedErrorRaised {
 			require.Equal(s.t, action.ExpectedError, "")
-			require.Equal(s.t, action.ExpectedPolicyID, policyResult.PolicyID)
+			if action.ExpectedPolicyID.HasValue() {
+				require.Equal(s.t, action.ExpectedPolicyID.Value(), policyResult.PolicyID)
+			} else {
+				require.NotEqual(s.t, policyResult.PolicyID, "")
+			}
+
+			s.policyIDs[nodeID] = append(s.policyIDs[nodeID], policyResult.PolicyID)
 		}
 
 		// The policy should only be added to a SourceHub chain once - there is no need to loop through

tests/integration/state.go

@@ -132,6 +132,9 @@ type state struct {
 	// Identities by node index, by identity index.
 	identities [][]identity.Identity
 
+	// Policy IDs, by node index, by policyID index (in the order they were added).
+	policyIDs [][]string
+
 	// Will recieve an item once all actions have finished processing.
 	allActionsDone chan struct{}
 
@@ -218,6 +221,7 @@ func newState(
 		collectionNames:          collectionNames,
 		collectionIndexesByRoot:  map[uint32]int{},
 		docIDs:                   [][]client.DocID{},
+		policyIDs:                [][]string{},
 		indexes:                  [][][]client.IndexDescription{},
 		isBench:                  false,
 	}

tests/integration/test_case.go

@@ -129,6 +129,51 @@ type SchemaUpdate struct {
 	// The schema update.
 	Schema string
 
+	// PolicyIDs is an optional list argument which makes it easier to substitute/embed PolicyIDs
+	// into the schema string where place holder labels ("%policyID%") are at, without us using
+	// explicit policy identifiers.
+	//
+	// Note:
+	// - The number of placeholder labels (%policyID%) must exactly match the number of policyID
+	//   indexes in this list.
+	// - The list must contain the policyID Indexes in the same order they are to be substituted in.
+	// - Can substitute the same policyID at an index, by specifying that index multiple times.
+	// - The indexes must be valid (correspond to the number of policies added so far).
+	//
+	// Example:
+	//
+	// Consider we have one policy that was added resulting in the following policyID:
+	// PolicyID="94eb195c0e459aa79e02a1986c7e731c5015721c18a373f2b2a0ed140a04b454"
+	//
+	// Then using this attribute like:
+	// PolicyIDs: immutable.Some([]int{0, 0}),
+	//
+	// On a Schema string like:
+	// ```
+	//	type Users1 @policy(id: "%policyID%", resource: "users") {
+	//		name: String
+	//		age: Int
+	//	}
+	//
+	//	type Users2 @policy(id: "%policyID%", resource: "users") {
+	//		name: String
+	//		age: Int
+	//	}
+	// ```
+	// The Schema that will be loaded will be this modified one:
+	// ```
+	//	type Users1 @policy(id: "94eb195c0e459aa79e02a1986c7e731c5015721c18a373f2b2a0ed140a04b454", resource: "users") {
+	//		name: String
+	//		age: Int
+	//	}
+	//
+	//	type Users2 @policy(id: "94eb195c0e459aa79e02a1986c7e731c5015721c18a373f2b2a0ed140a04b454", resource: "users") {
+	//		name: String
+	//		age: Int
+	//	}
+	// ```
+	PolicyIDs immutable.Option[[]int]
+
 	// Optionally, the expected results.
 	//
 	// Each item will be compared individually, if ID, RootID, SchemaVersionID or Fields on the

tests/integration/utils.go

@@ -1052,9 +1052,69 @@ func updateSchema(
 	s *state,
 	action SchemaUpdate,
 ) {
-	_, nodes := getNodesWithIDs(action.NodeID, s.nodes)
-	for _, node := range nodes {
-		results, err := node.AddSchema(s.ctx, action.Schema)
+	policyIDPlaceHolderLabel := "%policyID%"
+
+	// Do some sanitation checks if PolicyIDs are to be substituted, and error out early if invalid usage.
+	if action.PolicyIDs.HasValue() {
+		policyIDIndexes := action.PolicyIDs.Value()
+		if len(policyIDIndexes) == 0 {
+			require.Fail(s.t, "Specified policyID substitution but none given.", s.testCase.Description)
+		}
+
+		howManyPlaceHolders := strings.Count(action.Schema, policyIDPlaceHolderLabel)
+		if howManyPlaceHolders == 0 {
+			require.Fail(
+				s.t,
+				"Can't do substitution because no place holder labels:"+policyIDPlaceHolderLabel,
+				s.testCase.Description,
+			)
+		}
+
+		if howManyPlaceHolders != len(policyIDIndexes) {
+			require.Fail(
+				s.t,
+				"Can't do substitution when place holders are not equal to specified indexes",
+				s.testCase.Description,
+			)
+		}
+
+	}
+
+	nodeIDs, nodes := getNodesWithIDs(action.NodeID, s.nodes)
+	for index, node := range nodes {
+
+		// This schema might be modified if the caller needs some substitution magic done.
+		var modifiedSchema = action.Schema
+
+		// We need to substitute the policyIDs into the `%policyID% place holders.
+		if action.PolicyIDs.HasValue() {
+			policyIDIndexes := action.PolicyIDs.Value()
+			nodeID := nodeIDs[index]
+
+			nodesPolicyIDs := s.policyIDs[nodeID]
+			for _, policyIDIndex := range policyIDIndexes {
+				// Ensure policy index specified is valid (compared the existing policyIDs) for this node.
+				if policyIDIndex >= len(nodesPolicyIDs) {
+					require.Fail(
+						s.t,
+						"a policyID index is out of range, number of added policies is smaller",
+						s.testCase.Description,
+					)
+				}
+
+				policyID := nodesPolicyIDs[policyIDIndex]
+
+				// Now actually perform the substitution as all the sanity checks are done.
+				modifiedSchema = strings.Replace(
+					modifiedSchema,
+					policyIDPlaceHolderLabel,
+					policyID,
+					1,
+				)
+			}
+		}
+
+		results, err := node.AddSchema(s.ctx, modifiedSchema)
 		expectedErrorRaised := AssertError(s.t, s.testCase.Description, err, action.ExpectedError)
 
 		assertExpectedErrorRaised(s.t, s.testCase.Description, action.ExpectedError, expectedErrorRaised)