chore: remove unused code in typesystem (openfga#1008)

jcbertolucci · Sep 15, 2023 · eec8cb0 · eec8cb0
1 parent 639d053
commit eec8cb0
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 85 deletions.
diff --git a/pkg/typesystem/typesystem.go b/pkg/typesystem/typesystem.go
@@ -854,14 +854,6 @@ func NewAndValidate(ctx context.Context, model *openfgav1.AuthorizationModel) (*
 		}
 	}
 
-	if err := t.ensureNoCyclesInTupleToUsersetDefinitions(); err != nil {
-		return nil, err
-	}
-
-	if err := t.ensureNoCyclesInComputedRewrite(); err != nil {
-		return nil, err
-	}
-
 	return t, nil
 }
 
@@ -1071,83 +1063,6 @@ func (t *TypeSystem) validateTypeRestrictions(objectType string, relationName st
 	return nil
 }
 
-// ensureNoCyclesInTupleToUsersetDefinitions throws an error on the following models because `viewer` is a cycle.
-//
-//	type folder
-//	  relations
-//	    define parent: [folder] as self
-//	    define viewer as viewer from parent
-//
-// and
-//
-//	type folder
-//	  relations
-//	    define parent as self
-//	    define viewer as viewer from parent
-func (t *TypeSystem) ensureNoCyclesInTupleToUsersetDefinitions() error {
-	for objectType := range t.typeDefinitions {
-		relations, err := t.GetRelations(objectType)
-		if err == nil {
-			for relationName, relation := range relations {
-				switch cyclicDefinition := relation.GetRewrite().Userset.(type) {
-				case *openfgav1.Userset_TupleToUserset:
-					// define viewer as viewer from parent
-					if cyclicDefinition.TupleToUserset.ComputedUserset.GetRelation() == relationName {
-						tuplesetRelationName := cyclicDefinition.TupleToUserset.GetTupleset().GetRelation()
-						tuplesetRelation, err := t.GetRelation(objectType, tuplesetRelationName)
-						// define parent: [folder] as self
-						if err == nil {
-							switch tuplesetRelation.GetRewrite().Userset.(type) {
-							case *openfgav1.Userset_This:
-								if t.schemaVersion == SchemaVersion1_0 && len(t.typeDefinitions) == 1 {
-									return &InvalidRelationError{ObjectType: objectType, Relation: relationName, Cause: ErrCycle}
-								}
-								if t.schemaVersion == SchemaVersion1_1 && len(tuplesetRelation.TypeInfo.DirectlyRelatedUserTypes) == 1 && tuplesetRelation.TypeInfo.DirectlyRelatedUserTypes[0].Type == objectType {
-									return &InvalidRelationError{ObjectType: objectType, Relation: relationName, Cause: ErrCycle}
-								}
-							}
-						}
-					}
-				}
-			}
-		}
-	}
-
-	return nil
-}
-
-// ensureNoCyclesInComputedRewrite throws an error on the following model because `folder` type is a cycle.
-//
-//	 type folder
-//		 relations
-//		  define parent as child
-//		  define child as parent
-func (t *TypeSystem) ensureNoCyclesInComputedRewrite() error {
-	for objectType := range t.typeDefinitions {
-		relations, err := t.GetRelations(objectType)
-		if err == nil {
-			for sourceRelationName, relation := range relations {
-				switch source := relation.GetRewrite().Userset.(type) {
-				case *openfgav1.Userset_ComputedUserset:
-					target := source.ComputedUserset.GetRelation()
-					targetRelation, err := t.GetRelation(objectType, target)
-					if err == nil {
-						switch rewrite := targetRelation.GetRewrite().Userset.(type) {
-						case *openfgav1.Userset_ComputedUserset:
-							if rewrite.ComputedUserset.GetRelation() == sourceRelationName {
-								return &InvalidTypeError{ObjectType: objectType, Cause: ErrCycle}
-							}
-						}
-					}
-				}
-			}
-		}
-
-	}
-
-	return nil
-}
-
 func (t *TypeSystem) IsDirectlyAssignable(relation *openfgav1.Relation) bool {
 	return RewriteContainsSelf(relation.GetRewrite())
 }

diff --git a/pkg/typesystem/typesystem_test.go b/pkg/typesystem/typesystem_test.go
@@ -244,6 +244,23 @@ func TestInvalidRewriteValidations(t *testing.T) {
 			},
 			err: ErrInvalidUsersetRewrite,
 		},
+		{
+			name: "duplicate_types_is_invalid",
+			model: &openfgav1.AuthorizationModel{
+				SchemaVersion: SchemaVersion1_1,
+				TypeDefinitions: []*openfgav1.TypeDefinition{
+					{
+						Type:      "repo",
+						Relations: map[string]*openfgav1.Userset{},
+					},
+					{
+						Type:      "repo",
+						Relations: map[string]*openfgav1.Userset{},
+					},
+				},
+			},
+			err: ErrDuplicateTypes,
+		},
 		{
 			name: "invalid_relation:_self_reference_in_computedUserset",
 			model: &openfgav1.AuthorizationModel{
@@ -2183,3 +2200,50 @@ func TestDirectlyRelatedUsersets(t *testing.T) {
 		})
 	}
 }
+
+func TestHasTypeInfo(t *testing.T) {
+	tests := []struct {
+		name       string
+		schema     string
+		model      string
+		objectType string
+		relation   string
+		expected   bool
+	}{
+		{
+			name:   "has_type_info_true",
+			schema: SchemaVersion1_1,
+			model: `type user
+
+			type folder
+			  relations
+			    define allowed: [user] as self`,
+			objectType: "folder",
+			relation:   "allowed",
+			expected:   true,
+		},
+		{
+			name:   "has_type_info_false",
+			schema: SchemaVersion1_0,
+			model: `type user
+
+			type folder
+			  relations
+			    define allowed as self`,
+			objectType: "folder",
+			relation:   "allowed",
+			expected:   false,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			typesys := New(&openfgav1.AuthorizationModel{
+				SchemaVersion:   test.schema,
+				TypeDefinitions: parser.MustParse(test.model),
+			})
+			result, err := typesys.HasTypeInfo(test.objectType, test.relation)
+			require.NoError(t, err)
+			require.Equal(t, test.expected, result)
+		})
+	}
+}