Merge pull request #431 from josephschorr/remove-wildcard-fix

Fix handling of removing allowed wildcards on relations

internal/namespace/diff.go

@@ -40,11 +40,11 @@ const (
 
 	// RelationDirectWildcardTypeAdded indicates that an allowed relation wildcard type has been added to
 	// the relation.
-	RelationDirectWildcardTypeAdded DeltaType = "relation-direct-type-added"
+	RelationDirectWildcardTypeAdded DeltaType = "relation-wildcard-type-added"
 
 	// RelationDirectWildcardTypeRemoved indicates that an allowed relation wildcard type has been removed from
 	// the relation.
-	RelationDirectWildcardTypeRemoved DeltaType = "relation-direct-type-removed"
+	RelationDirectWildcardTypeRemoved DeltaType = "relation-wildcard-type-removed"
 )
 
 // NamespaceDiff holds the diff between two namespaces.

internal/namespace/diff_test.go

@@ -224,7 +224,6 @@ func TestNamespaceDiff(t *testing.T) {
 				{Type: RelationDirectWildcardTypeAdded, RelationName: "somerel", WildcardType: "foo2"},
 			},
 		},
-
 		{
 			"wildcard type changed",
 			ns.Namespace(
@@ -249,6 +248,24 @@ func TestNamespaceDiff(t *testing.T) {
 				}},
 			},
 		},
+		{
+			"wildcard type changed no rewrite",
+			ns.Namespace(
+				"document",
+				ns.Relation("somerel", nil, ns.AllowedPublicNamespace("user")),
+			),
+			ns.Namespace(
+				"document",
+				ns.Relation("somerel", nil, ns.AllowedRelation("organization", "user")),
+			),
+			[]Delta{
+				{Type: RelationDirectWildcardTypeRemoved, RelationName: "somerel", WildcardType: "user"},
+				{Type: RelationDirectTypeAdded, RelationName: "somerel", DirectType: &v0.RelationReference{
+					Namespace: "organization",
+					Relation:  "user",
+				}},
+			},
+		},
 	}
 
 	for _, tc := range testCases {

internal/services/shared/schema.go

@@ -13,6 +13,7 @@ import (
 	"github.com/authzed/spicedb/internal/datastore"
 	"github.com/authzed/spicedb/internal/datastore/options"
 	"github.com/authzed/spicedb/internal/namespace"
+	"github.com/authzed/spicedb/pkg/tuple"
 )
 
 // EnsureNoRelationshipsExist ensures that no relationships exist within the namespace with the given name.
@@ -109,6 +110,30 @@ func SanityCheckExistingRelationships(ctx context.Context, ds datastore.Datastor
 				return err
 			}
 
+		case namespace.RelationDirectWildcardTypeRemoved:
+			qy, qyErr := ds.ReverseQueryTuples(
+				ctx,
+				&v1.SubjectFilter{
+					SubjectType:       delta.WildcardType,
+					OptionalSubjectId: tuple.PublicWildcard,
+				},
+				headRevision,
+				options.WithResRelation(&options.ResourceRelation{
+					Namespace: nsdef.Name,
+					Relation:  delta.RelationName,
+				}),
+				options.WithReverseLimit(options.LimitOne),
+			)
+			err = ErrorIfTupleIteratorReturnsTuples(
+				ctx,
+				qy,
+				qyErr,
+				"cannot remove allowed wildcard type `%s:*` from Relation `%s` in Object Definition `%s`, as a Relationship exists with it",
+				delta.WildcardType, delta.RelationName, nsdef.Name)
+			if err != nil {
+				return err
+			}
+
 		case namespace.RelationDirectTypeRemoved:
 			qy, qyErr := ds.ReverseQueryTuples(
 				ctx,

internal/services/v1/schema_test.go

@@ -172,3 +172,64 @@ func TestSchemaDeleteDefinition(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, `definition example/user {}`, readback.SchemaText)
 }
+
+func TestSchemaRemoveWildcard(t *testing.T) {
+	conn, cleanup, _ := testserver.NewTestServer(require.New(t), 0, memdb.DisableGC, 0, true, tf.EmptyDatastore)
+	t.Cleanup(cleanup)
+	client := v1.NewSchemaServiceClient(conn)
+	v0client := v0.NewACLServiceClient(conn)
+
+	// Write a basic schema.
+	_, err := client.WriteSchema(context.Background(), &v1.WriteSchemaRequest{
+		Schema: `definition example/user {}
+	
+		definition example/document {
+			relation somerelation: example/user:*
+		}`,
+	})
+	require.NoError(t, err)
+
+	// Write the wildcard relationship.
+	_, err = v0client.Write(context.Background(), &v0.WriteRequest{
+		Updates: []*v0.RelationTupleUpdate{tuple.Create(
+			tuple.MustParse("example/document:somedoc#somerelation@example/user:*"),
+		)},
+	})
+	require.Nil(t, err)
+
+	newSchema := `definition example/document {
+	relation somerelation: example/organization#user
+}
+
+definition example/organization {
+	relation user: example/user
+}
+
+definition example/user {}`
+
+	// Attempt to change the wildcard type, which should fail.
+	_, err = client.WriteSchema(context.Background(), &v1.WriteSchemaRequest{
+		Schema: newSchema,
+	})
+	grpcutil.RequireStatus(t, codes.InvalidArgument, err)
+	require.Equal(t, "rpc error: code = InvalidArgument desc = cannot remove allowed wildcard type `example/user:*` from Relation `somerelation` in Object Definition `example/document`, as a Relationship exists with it", err.Error())
+
+	// Delete the relationship.
+	_, err = v0client.Write(context.Background(), &v0.WriteRequest{
+		Updates: []*v0.RelationTupleUpdate{tuple.Delete(
+			tuple.MustParse("example/document:somedoc#somerelation@example/user:*"),
+		)},
+	})
+	require.Nil(t, err)
+
+	// Attempt to delete the wildcard type, which should work now.
+	_, err = client.WriteSchema(context.Background(), &v1.WriteSchemaRequest{
+		Schema: newSchema,
+	})
+	require.Nil(t, err)
+
+	// Ensure it was deleted.
+	readback, err := client.ReadSchema(context.Background(), &v1.ReadSchemaRequest{})
+	require.NoError(t, err)
+	require.Equal(t, newSchema, readback.SchemaText)
+}