fix(core): reduces GetAttributeValuesByFqns calls in getDecisions (#1857)

### Proposed Changes

* getDecisions calls `GetAttributeValuesByFqns` per `ResourceAttribute`,
with bulk rewrap incoming, this can be quite expensive.
* rather than `GetAttributesValuesByFqns` per `ResourceAttribute` get
all FQNs in one call by collecting them in the `ResourceAttribute`.

### Checklist

- [ ] I have added or updated unit tests
- [ ] I have added or updated integration tests (if appropriate)
- [ ] I have added or updated documentation

### Testing Instructions

`go run ./examples benchmark-decision --insecurePlaintextConn --count
1000`
locally I got 1.3s vs 80ms, so a very important performance change.

Author: imdominicreed

examples/cmd/benchmark_decision.go

@@ -16,7 +16,7 @@ func init() {
 		Long:  `A OpenTDF benchmark tool to measure throughput and latency with configurable concurrency.`,
 		RunE:  runDecisionBenchmark,
 	}
-
+	benchmarkCmd.Flags().IntVar(&config.RequestCount, "count", 100, "Total number of requests")
 	ExamplesCmd.AddCommand(benchmarkCmd)
 }
 
@@ -28,7 +28,7 @@ func runDecisionBenchmark(cmd *cobra.Command, args []string) error {
 	}
 
 	ras := []*authorization.ResourceAttribute{}
-	for i := 0; i < 100; i++ {
+	for i := 0; i < config.RequestCount; i++ {
 		ras = append(ras, &authorization.ResourceAttribute{AttributeValueFqns: []string{"https://example.com/attr/attr1/value/value1"}})
 	}
 

service/authorization/authorization.go

@@ -200,20 +200,21 @@ func (as *AuthorizationService) GetDecisions(ctx context.Context, req *connect.R
 }
 
 func (as *AuthorizationService) getDecisions(ctx context.Context, dr *authorization.DecisionRequest) ([]*authorization.DecisionResponse, error) {
-	var attrDefsReqs [][]*policy.Attribute
-	var attrValsReqs [][]*policy.Value
-	var fqnsReqs [][]string
 	allPertinentFQNS := &authorization.ResourceAttribute{AttributeValueFqns: make([]string, 0)}
 	response := make([]*authorization.DecisionResponse, len(dr.GetResourceAttributes())*len(dr.GetEntityChains()))
-	for raIdx, ra := range dr.GetResourceAttributes() {
-		as.logger.DebugContext(ctx, "getting resource attributes", slog.String("FQNs", strings.Join(ra.GetAttributeValueFqns(), ", ")))
 
-		// get attribute definition/value combinations
-		dataAttrDefsAndVals, err := retrieveAttributeDefinitions(ctx, ra, as.sdk)
-		if err != nil {
-			// if attribute an FQN does not exist
-			// return deny for all entity chains aginst this RA set and continue to next
-			if errors.Is(err, status.Error(codes.NotFound, db.ErrTextNotFound)) || errors.Is(err, ErrEmptyStringAttribute) {
+	// TODO: fetching missing FQNs should not lead into a complete failure, rather a list of unknown FQNs would be preferred
+	var err error
+	var dataAttrDefsAndVals map[string]*attr.GetAttributeValuesByFqnsResponse_AttributeAndValue
+	allPertinentFQNS.AttributeValueFqns, err = getAttributesFromRas(dr.GetResourceAttributes())
+	if err == nil {
+		dataAttrDefsAndVals, err = retrieveAttributeDefinitions(ctx, allPertinentFQNS.GetAttributeValueFqns(), as.sdk)
+	}
+	if err != nil {
+		// if attribute an FQN does not exist
+		// return deny for all entity chains aginst this RAs
+		if errors.Is(err, status.Error(codes.NotFound, db.ErrTextNotFound)) || errors.Is(err, ErrEmptyStringAttribute) {
+			for raIdx, ra := range dr.GetResourceAttributes() {
 				for ecIdx, ec := range dr.GetEntityChains() {
 					decisionResp := &authorization.DecisionResponse{
 						Decision:      authorization.DecisionResponse_DECISION_DENY,
@@ -231,45 +232,28 @@ func (as *AuthorizationService) getDecisions(ctx context.Context, dr *authorizat
 					}
 					responseIdx := (raIdx * len(dr.GetEntityChains())) + ecIdx
 					response[responseIdx] = decisionResp
-					// append empty values to keep the order of the requests
-					attrDefsReqs = append(attrDefsReqs, []*policy.Attribute{})
-					attrValsReqs = append(attrValsReqs, []*policy.Value{})
-					fqnsReqs = append(fqnsReqs, []string{})
 				}
-				continue
 			}
-			return nil, db.StatusifyError(err, db.ErrTextGetRetrievalFailed, slog.String("fqns", strings.Join(ra.GetAttributeValueFqns(), ", ")))
+			return response, nil
 		}
+		return nil, db.StatusifyError(err, db.ErrTextGetRetrievalFailed, slog.String("fqns", strings.Join(allPertinentFQNS.GetAttributeValueFqns(), ", ")))
+	}
 
-		var attrDefs []*policy.Attribute
-		var attrVals []*policy.Value
-		var fqns []string
-
-		for fqn, v := range dataAttrDefsAndVals {
-			attrDefs = append(attrDefs, v.GetAttribute())
-			attrVal := v.GetValue()
-			fqns = append(fqns, fqn)
-			attrVal.Fqn = fqn
-			attrVals = append(attrVals, attrVal)
-		}
-
-		attrDefs, err = populateAttrDefValueFqns(attrDefs)
-		if err != nil {
-			return nil, connect.NewError(connect.CodeInternal, err)
-		}
-		allPertinentFQNS.AttributeValueFqns = append(allPertinentFQNS.GetAttributeValueFqns(), ra.GetAttributeValueFqns()...)
-
-		// get the relevant resource attribute fqns
-		for _, attrDef := range attrDefs {
-			if attrDef.GetRule() == policy.AttributeRuleTypeEnum_ATTRIBUTE_RULE_TYPE_ENUM_HIERARCHY {
-				for _, value := range attrDef.GetValues() {
-					allPertinentFQNS.AttributeValueFqns = append(allPertinentFQNS.AttributeValueFqns, value.GetFqn())
-				}
+	var allAttrDefs []*policy.Attribute
+	for _, v := range dataAttrDefsAndVals {
+		allAttrDefs = append(allAttrDefs, v.GetAttribute())
+	}
+	allAttrDefs, err = populateAttrDefValueFqns(allAttrDefs)
+	if err != nil {
+		return nil, connect.NewError(connect.CodeInternal, err)
+	}
+	// get the relevant resource attribute fqns
+	for _, attrDef := range allAttrDefs {
+		if attrDef.GetRule() == policy.AttributeRuleTypeEnum_ATTRIBUTE_RULE_TYPE_ENUM_HIERARCHY {
+			for _, value := range attrDef.GetValues() {
+				allPertinentFQNS.AttributeValueFqns = append(allPertinentFQNS.AttributeValueFqns, value.GetFqn())
 			}
 		}
-		attrDefsReqs = append(attrDefsReqs, attrDefs)
-		attrValsReqs = append(attrValsReqs, attrVals)
-		fqnsReqs = append(fqnsReqs, fqns)
 	}
 
 	var ecChainEntitlementsResponse []*connect.Response[authorization.GetEntitlementsResponse]
@@ -294,15 +278,31 @@ func (as *AuthorizationService) getDecisions(ctx context.Context, dr *authorizat
 	}
 
 	for raIdx, ra := range dr.GetResourceAttributes() {
+		var attrDefs []*policy.Attribute
+		var attrVals []*policy.Value
+		var fqns []string
+
+		for _, fqn := range ra.GetAttributeValueFqns() {
+			fqn = strings.ToLower(fqn)
+			fqns = append(fqns, fqn)
+			v := dataAttrDefsAndVals[fqn]
+			attrDefs = append(attrDefs, v.GetAttribute())
+			attrVal := v.GetValue()
+			attrVal.Fqn = fqn
+			attrVals = append(attrVals, attrVal)
+		}
+
+		attrDefs, err = populateAttrDefValueFqns(attrDefs)
+		if err != nil {
+			return nil, connect.NewError(connect.CodeInternal, err)
+		}
+
 		for ecIdx, ec := range dr.GetEntityChains() {
 			// check if we already have a decision for this entity chain
 			responseIdx := (raIdx * len(dr.GetEntityChains())) + ecIdx
 			if response[responseIdx] != nil {
 				continue
 			}
-			attrVals := attrValsReqs[raIdx]
-			attrDefs := attrDefsReqs[raIdx]
-			fqns := fqnsReqs[raIdx]
 
 			//
 			// TODO: we should already have the subject mappings here and be able to just use OPA to trim down the known data attr values to the ones matched up with the entities
@@ -651,27 +651,36 @@ func (as *AuthorizationService) GetEntitlements(ctx context.Context, req *connec
 	return resp, nil
 }
 
-func retrieveAttributeDefinitions(ctx context.Context, ra *authorization.ResourceAttribute, sdk *otdf.SDK) (map[string]*attr.GetAttributeValuesByFqnsResponse_AttributeAndValue, error) {
-	attrFqns := ra.GetAttributeValueFqns()
-	if len(attrFqns) == 0 {
-		return make(map[string]*attr.GetAttributeValuesByFqnsResponse_AttributeAndValue), nil
-	}
-	// remove empty strings
-	attrFqnsNoEmpty := attrFqns[:0] // Use the same backing array to avoid allocation
-	for _, str := range attrFqns {
-		if str != "" {
-			attrFqnsNoEmpty = append(attrFqnsNoEmpty, str)
+func getAttributesFromRas(ras []*authorization.ResourceAttribute) ([]string, error) {
+	var attrFqns []string
+	repeats := make(map[string]bool)
+	moreThanOneAttr := false
+	for _, ra := range ras {
+		for _, str := range ra.GetAttributeValueFqns() {
+			moreThanOneAttr = true
+			if str != "" && !repeats[str] {
+				attrFqns = append(attrFqns, str)
+				repeats[str] = true
+			}
 		}
 	}
-	// if no attribute value FQNs after removal, return error
-	if len(attrFqnsNoEmpty) == 0 {
+
+	if moreThanOneAttr && len(attrFqns) == 0 {
 		return nil, ErrEmptyStringAttribute
 	}
+	return attrFqns, nil
+}
+
+func retrieveAttributeDefinitions(ctx context.Context, attrFqns []string, sdk *otdf.SDK) (map[string]*attr.GetAttributeValuesByFqnsResponse_AttributeAndValue, error) {
+	if len(attrFqns) == 0 {
+		return make(map[string]*attr.GetAttributeValuesByFqnsResponse_AttributeAndValue), nil
+	}
+
 	resp, err := sdk.Attributes.GetAttributeValuesByFqns(ctx, &attr.GetAttributeValuesByFqnsRequest{
 		WithValue: &policy.AttributeValueSelector{
 			WithSubjectMaps: false,
 		},
-		Fqns: attrFqnsNoEmpty,
+		Fqns: attrFqns,
 	})
 	if err != nil {
 		return nil, err