feat(policy): 1603 policy improve upsertattrfqn

service/integration/namespaces_test.go

@@ -164,9 +164,6 @@ func (s *NamespacesSuite) Test_UpdateNamespace() {
 		},
 	})
 	metadata := created.GetMetadata()
-	// only GET returns populated created/updated times
-	s.Nil(metadata.GetCreatedAt())
-	s.Nil(metadata.GetUpdatedAt())
 
 	s.Require().NoError(err)
 	s.NotNil(created)

service/policy/db/attribute_fqn.go

@@ -4,57 +4,12 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"log/slog"
 	"strings"
 
 	"github.com/opentdf/platform/protocol/go/policy/attributes"
 	"github.com/opentdf/platform/service/pkg/db"
 )
 
-// These values are optional, but at least one must be set. The other values will be derived from
-// the set values.
-type attrFqnUpsertOptions struct {
-	namespaceID string
-	attributeID string
-	valueID     string
-}
-
-// This logic is a bit complex. What we are trying to achieve is to upsert the fqn based on the
-// combination of namespaceId, attributeId, and valueId. However, instead of requiring all three
-// we want to support partial attribute FQNs. This means that we need to support the following
-// combinations:
-// 1. namespaceId
-// 2. namespaceId, attributeId
-// 3. namespaceId, attributeId, valueId
-//
-// This is a side effect -- errors will be swallowed and the fqn will be returned as an empty string
-func (c *PolicyDBClient) upsertAttrFqn(ctx context.Context, opts attrFqnUpsertOptions) string {
-	var (
-		fqn string
-		err error
-	)
-
-	switch {
-	case opts.valueID != "":
-		fqn, err = c.Queries.UpsertAttributeValueFqn(ctx, opts.valueID)
-	case opts.attributeID != "":
-		fqn, err = c.Queries.UpsertAttributeDefinitionFqn(ctx, opts.attributeID)
-	case opts.namespaceID != "":
-		fqn, err = c.Queries.UpsertAttributeNamespaceFqn(ctx, opts.namespaceID)
-	default:
-		err = fmt.Errorf("at least one of namespaceId, attributeId, or valueId must be set")
-	}
-
-	if err != nil {
-		wrappedErr := db.WrapIfKnownInvalidQueryErr(err)
-		c.logger.ErrorContext(ctx, "could not update FQN", slog.Any("opts", opts), slog.String("error", wrappedErr.Error()))
-		return ""
-	}
-
-	c.logger.DebugContext(ctx, "updated FQN", slog.String("fqn", fqn), slog.Any("opts", opts))
-	return fqn
-}
-
 // AttrFqnReindex will reindex all namespace, attribute, and attribute_value FQNs
 func (c *PolicyDBClient) AttrFqnReindex(ctx context.Context) (res struct { //nolint:nonamedreturns // Used to initialize an anonymous struct
 	Namespaces []struct {
@@ -77,40 +32,37 @@ func (c *PolicyDBClient) AttrFqnReindex(ctx context.Context) (res struct { //nol
 		panic(fmt.Errorf("could not get namespaces: %w", err))
 	}
 
-	// Get all attributes
-	attrs, err := c.ListAllAttributes(ctx)
-	if err != nil {
-		panic(fmt.Errorf("could not get attributes: %w", err))
-	}
-
-	// Get all attribute values
-	values, err := c.ListAllAttributeValues(ctx)
-	if err != nil {
-		panic(fmt.Errorf("could not get attribute values: %w", err))
-	}
-
 	// Reindex all namespaces
+	reindexedRecords := []UpsertAttributeNamespaceFqnRow{}
 	for _, n := range ns {
-		res.Namespaces = append(res.Namespaces, struct {
-			ID  string
-			Fqn string
-		}{ID: n.GetId(), Fqn: c.upsertAttrFqn(ctx, attrFqnUpsertOptions{namespaceID: n.GetId()})})
-	}
-
-	// Reindex all attributes
-	for _, a := range attrs {
-		res.Attributes = append(res.Attributes, struct {
-			ID  string
-			Fqn string
-		}{ID: a.GetId(), Fqn: c.upsertAttrFqn(ctx, attrFqnUpsertOptions{attributeID: a.GetId()})})
-	}
-
-	// Reindex all attribute values
-	for _, av := range values {
-		res.Values = append(res.Values, struct {
-			ID  string
-			Fqn string
-		}{ID: av.GetId(), Fqn: c.upsertAttrFqn(ctx, attrFqnUpsertOptions{valueID: av.GetId()})})
+		rows, err := c.Queries.UpsertAttributeNamespaceFqn(ctx, n.GetId())
+		if err != nil {
+			panic(fmt.Errorf("could not update namespace [%s] FQN: %w", n.GetId(), err))
+		}
+		reindexedRecords = append(reindexedRecords, rows...)
+	}
+
+	for _, r := range reindexedRecords {
+		switch {
+		case r.AttributeID == "" && r.ValueID == "":
+			// namespace record
+			res.Namespaces = append(res.Namespaces, struct {
+				ID  string
+				Fqn string
+			}{ID: r.NamespaceID, Fqn: r.Fqn})
+		case r.ValueID == "":
+			// attribute definition record
+			res.Attributes = append(res.Attributes, struct {
+				ID  string
+				Fqn string
+			}{ID: r.AttributeID, Fqn: r.Fqn})
+		default:
+			// attribute value record
+			res.Values = append(res.Values, struct {
+				ID  string
+				Fqn string
+			}{ID: r.ValueID, Fqn: r.Fqn})
+		}
 	}
 
 	return res

service/policy/db/attribute_values.go

@@ -18,7 +18,7 @@ import (
 func (c PolicyDBClient) CreateAttributeValue(ctx context.Context, attributeID string, r *attributes.CreateAttributeValueRequest) (*policy.Value, error) {
 	value := strings.ToLower(r.GetValue())
 
-	metadataJSON, metadata, err := db.MarshalCreateMetadata(r.GetMetadata())
+	metadataJSON, _, err := db.MarshalCreateMetadata(r.GetMetadata())
 	if err != nil {
 		return nil, err
 	}
@@ -33,23 +33,12 @@ func (c PolicyDBClient) CreateAttributeValue(ctx context.Context, attributeID st
 	}
 
 	// Update FQN
-	fqn := c.upsertAttrFqn(ctx, attrFqnUpsertOptions{valueID: createdID})
-	if fqn != "" {
-		c.logger.Debug("created new attribute value FQN",
-			slog.String("value_id", createdID),
-			slog.String("value", value),
-			slog.String("fqn", fqn),
-		)
+	_, err = c.Queries.UpsertAttributeValueFqn(ctx, createdID)
+	if err != nil {
+		return nil, db.WrapIfKnownInvalidQueryErr(err)
 	}
 
-	return &policy.Value{
-		Id:        createdID,
-		Attribute: &policy.Attribute{Id: attributeID},
-		Value:     value,
-		Metadata:  metadata,
-		Active:    &wrapperspb.BoolValue{Value: true},
-		Fqn:       fqn,
-	}, nil
+	return c.GetAttributeValue(ctx, createdID)
 }
 
 func (c PolicyDBClient) GetAttributeValue(ctx context.Context, id string) (*policy.Value, error) {
@@ -176,18 +165,12 @@ func (c PolicyDBClient) UnsafeUpdateAttributeValue(ctx context.Context, r *unsaf
 	}
 
 	// Update FQN
-	fqn := c.upsertAttrFqn(ctx, attrFqnUpsertOptions{valueID: id})
-	c.logger.Debug("upserted fqn for unsafely updated value",
-		slog.String("id", id),
-		slog.String("value", value),
-		slog.String("fqn", fqn),
-	)
+	_, err = c.Queries.UpsertAttributeValueFqn(ctx, id)
+	if err != nil {
+		return nil, db.WrapIfKnownInvalidQueryErr(err)
+	}
 
-	return &policy.Value{
-		Id:    id,
-		Value: value,
-		Fqn:   fqn,
-	}, nil
+	return c.GetAttributeValue(ctx, id)
 }
 
 func (c PolicyDBClient) DeactivateAttributeValue(ctx context.Context, id string) (*policy.Value, error) {

service/policy/db/attributes.go

@@ -5,7 +5,6 @@ import (
 	"database/sql"
 	"encoding/json"
 	"fmt"
-	"log/slog"
 	"strings"
 
 	"github.com/google/uuid"
@@ -282,7 +281,7 @@ func (c PolicyDBClient) GetAttributesByNamespace(ctx context.Context, namespaceI
 func (c PolicyDBClient) CreateAttribute(ctx context.Context, r *attributes.CreateAttributeRequest) (*policy.Attribute, error) {
 	name := strings.ToLower(r.GetName())
 	namespaceID := r.GetNamespaceId()
-	metadataJSON, metadata, err := db.MarshalCreateMetadata(r.GetMetadata())
+	metadataJSON, _, err := db.MarshalCreateMetadata(r.GetMetadata())
 	if err != nil {
 		return nil, err
 	}
@@ -299,48 +298,24 @@ func (c PolicyDBClient) CreateAttribute(ctx context.Context, r *attributes.Creat
 	}
 
 	// Add values
-	var values []*policy.Value
 	for _, v := range r.GetValues() {
 		req := &attributes.CreateAttributeValueRequest{
 			AttributeId: createdID,
 			Value:       v,
 		}
-		value, err := c.CreateAttributeValue(ctx, createdID, req)
+		_, err := c.CreateAttributeValue(ctx, createdID, req)
 		if err != nil {
 			return nil, err
 		}
-		values = append(values, value)
 	}
 
 	// Update the FQNs
-	fqn := c.upsertAttrFqn(ctx, attrFqnUpsertOptions{
-		namespaceID: namespaceID,
-		attributeID: createdID,
-	})
-	c.logger.DebugContext(ctx, "upserted fqn with new attribute definition", slog.Any("fqn", fqn))
-
-	for _, v := range values {
-		fqn = c.upsertAttrFqn(ctx, attrFqnUpsertOptions{
-			namespaceID: namespaceID,
-			attributeID: createdID,
-			valueID:     v.GetId(),
-		})
-		c.logger.DebugContext(ctx, "upserted fqn with new attribute value on new definition create", slog.Any("fqn", fqn))
+	_, err = c.Queries.UpsertAttributeDefinitionFqn(ctx, createdID)
+	if err != nil {
+		return nil, db.WrapIfKnownInvalidQueryErr(err)
 	}
 
-	a := &policy.Attribute{
-		Id:       createdID,
-		Name:     name,
-		Rule:     r.GetRule(),
-		Metadata: metadata,
-		Namespace: &policy.Namespace{
-			Id: namespaceID,
-		},
-		Active: &wrapperspb.BoolValue{Value: true},
-		Values: values,
-		Fqn:    fqn,
-	}
-	return a, nil
+	return c.GetAttribute(ctx, createdID)
 }
 
 func (c PolicyDBClient) UnsafeUpdateAttribute(ctx context.Context, r *unsafe.UnsafeUpdateAttributeRequest) (*policy.Attribute, error) {
@@ -396,27 +371,15 @@ func (c PolicyDBClient) UnsafeUpdateAttribute(ctx context.Context, r *unsafe.Uns
 		return nil, db.ErrNotFound
 	}
 
-	attribute := &policy.Attribute{
-		Id:   id,
-		Name: name,
-		Rule: rule,
-	}
-
 	// Upsert all the FQNs with the definition name mutation
 	if name != "" {
-		namespaceID := before.GetNamespace().GetId()
-		attrFqn := c.upsertAttrFqn(ctx, attrFqnUpsertOptions{namespaceID: namespaceID, attributeID: id})
-		c.logger.Debug("upserted attribute fqn with new definition name", slog.Any("fqn", attrFqn))
-		if len(before.GetValues()) > 0 {
-			for _, v := range before.GetValues() {
-				fqn := c.upsertAttrFqn(ctx, attrFqnUpsertOptions{namespaceID: namespaceID, attributeID: id, valueID: v.GetId()})
-				c.logger.Debug("upserted attribute value fqn with new definition name", slog.Any("fqn", fqn))
-			}
+		_, err = c.Queries.UpsertAttributeDefinitionFqn(ctx, id)
+		if err != nil {
+			return nil, db.WrapIfKnownInvalidQueryErr(err)
 		}
-		attribute.Fqn = attrFqn
 	}
 
-	return attribute, nil
+	return c.GetAttribute(ctx, id)
 }
 
 func (c PolicyDBClient) UpdateAttribute(ctx context.Context, id string, r *attributes.UpdateAttributeRequest) (*policy.Attribute, error) {

service/policy/db/namespaces.go

@@ -80,7 +80,7 @@ func (c PolicyDBClient) ListNamespaces(ctx context.Context, state string) ([]*po
 
 func (c PolicyDBClient) CreateNamespace(ctx context.Context, r *namespaces.CreateNamespaceRequest) (*policy.Namespace, error) {
 	name := strings.ToLower(r.GetName())
-	metadataJSON, metadata, err := db.MarshalCreateMetadata(r.GetMetadata())
+	metadataJSON, _, err := db.MarshalCreateMetadata(r.GetMetadata())
 	if err != nil {
 		return nil, err
 	}
@@ -94,16 +94,12 @@ func (c PolicyDBClient) CreateNamespace(ctx context.Context, r *namespaces.Creat
 	}
 
 	// Update FQN
-	fqn := c.upsertAttrFqn(ctx, attrFqnUpsertOptions{namespaceID: createdID})
-	c.logger.Debug("upserted fqn for created namespace", slog.Any("fqn", fqn))
+	_, err = c.Queries.UpsertAttributeNamespaceFqn(ctx, createdID)
+	if err != nil {
+		return nil, err
+	}
 
-	return &policy.Namespace{
-		Id:       createdID,
-		Name:     name,
-		Active:   &wrapperspb.BoolValue{Value: true},
-		Metadata: metadata,
-		Fqn:      fqn,
-	}, nil
+	return c.GetNamespace(ctx, createdID)
 }
 
 func (c PolicyDBClient) UpdateNamespace(ctx context.Context, id string, r *namespaces.UpdateNamespaceRequest) (*policy.Namespace, error) {
@@ -157,27 +153,12 @@ func (c PolicyDBClient) UnsafeUpdateNamespace(ctx context.Context, id string, na
 	}
 
 	// Update all FQNs that may contain the namespace name
-	nsFqn := c.upsertAttrFqn(ctx, attrFqnUpsertOptions{namespaceID: id})
-	c.logger.Debug("upserted fqn for unsafely updated namespace", slog.Any("fqn", nsFqn))
-
-	attrs, err := c.ListAttributes(ctx, StateAny, id)
+	_, err = c.Queries.UpsertAttributeNamespaceFqn(ctx, id)
 	if err != nil {
 		return nil, err
 	}
-	for _, attr := range attrs {
-		fqn := c.upsertAttrFqn(ctx, attrFqnUpsertOptions{namespaceID: id, attributeID: attr.GetId()})
-		c.logger.Debug("upserted definition fqn for unsafely updated namespace", slog.Any("fqn", fqn))
-		for _, value := range attr.GetValues() {
-			fqn = c.upsertAttrFqn(ctx, attrFqnUpsertOptions{namespaceID: id, attributeID: attr.GetId(), valueID: value.GetId()})
-			c.logger.Debug("upserted value fqn for unsafely updated namespace", slog.Any("fqn", fqn))
-		}
-	}
 
-	return &policy.Namespace{
-		Id:   id,
-		Name: name,
-		Fqn:  nsFqn,
-	}, nil
+	return c.GetNamespace(ctx, id)
 }
 
 func (c PolicyDBClient) DeactivateNamespace(ctx context.Context, id string) (*policy.Namespace, error) {