feat: remove experimentals for modular models/schema 1.2 (#1520)

openfga · Apr 10, 2024 · 48b5992 · 48b5992
1 parent ab0d835
commit 48b5992
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 79 deletions.
diff --git a/.config-schema.json b/.config-schema.json
@@ -89,7 +89,7 @@
             "type": "array",
             "items": {
                 "type": "string",
-                "enum": ["enable-modular-models"]
+                "enum": []
             },
             "default": [],
             "x-env-variable": "OPENFGA_EXPERIMENTALS"

diff --git a/pkg/server/commands/write_authzmodel.go b/pkg/server/commands/write_authzmodel.go
@@ -22,7 +22,6 @@ type WriteAuthorizationModelCommand struct {
 	backend                          storage.TypeDefinitionWriteBackend
 	logger                           logger.Logger
 	maxAuthorizationModelSizeInBytes int
-	enableModularModels              bool
 }
 
 type WriteAuthModelOption func(*WriteAuthorizationModelCommand)
@@ -39,18 +38,11 @@ func WithWriteAuthModelMaxSizeInBytes(size int) WriteAuthModelOption {
 	}
 }
 
-func WithEnableModularModels(enable bool) WriteAuthModelOption {
-	return func(m *WriteAuthorizationModelCommand) {
-		m.enableModularModels = enable
-	}
-}
-
 func NewWriteAuthorizationModelCommand(backend storage.TypeDefinitionWriteBackend, opts ...WriteAuthModelOption) *WriteAuthorizationModelCommand {
 	model := &WriteAuthorizationModelCommand{
 		backend:                          backend,
 		logger:                           logger.NewNoopLogger(),
 		maxAuthorizationModelSizeInBytes: serverconfig.DefaultMaxAuthorizationModelSizeInBytes,
-		enableModularModels:              false,
 	}
 
 	for _, opt := range opts {
@@ -71,10 +63,6 @@ func (w *WriteAuthorizationModelCommand) Execute(ctx context.Context, req *openf
 		req.SchemaVersion = typesystem.SchemaVersion1_1
 	}
 
-	if !w.enableModularModels && req.GetSchemaVersion() == typesystem.SchemaVersion1_2 {
-		return nil, status.Error(codes.InvalidArgument, "modular models (schema version 1.2) are not supported")
-	}
-
 	model := &openfgav1.AuthorizationModel{
 		Id:              ulid.Make().String(),
 		SchemaVersion:   req.GetSchemaVersion(),

diff --git a/pkg/server/commands/write_authzmodel_test.go b/pkg/server/commands/write_authzmodel_test.go
@@ -9,14 +9,12 @@ import (
 	"github.com/stretchr/testify/require"
 	"go.uber.org/goleak"
 	"go.uber.org/mock/gomock"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 
 	mockstorage "github.com/openfga/openfga/internal/mocks"
 	"github.com/openfga/openfga/pkg/typesystem"
 )
 
-func TestWriteAuthorizationModelWithExperimentalEnableModularModels(t *testing.T) {
+func TestWriteAuthorizationModel(t *testing.T) {
 	t.Cleanup(func() {
 		goleak.VerifyNone(t)
 	})
@@ -31,41 +29,21 @@ func TestWriteAuthorizationModelWithExperimentalEnableModularModels(t *testing.T
 	mockDatastore.EXPECT().MaxTypesPerAuthorizationModel().AnyTimes().Return(100)
 
 	testCases := map[string]struct {
-		enableModularModules bool
-		inputSchema          string
-		expectAllowed        bool
+		inputSchema string
 	}{
-		`allow 1.2 when enableModularModules true`: {
-			enableModularModules: true,
-			inputSchema:          typesystem.SchemaVersion1_2,
-			expectAllowed:        true,
+		`allow 1.2`: {
+			inputSchema: typesystem.SchemaVersion1_2,
 		},
-		`forbid 1.2 when not enableModularModules false`: {
-			enableModularModules: false,
-			inputSchema:          typesystem.SchemaVersion1_2,
-			expectAllowed:        false,
-		},
-		`allow 1.1 when enableModularModules true`: {
-			enableModularModules: true,
-			inputSchema:          typesystem.SchemaVersion1_1,
-			expectAllowed:        true,
-		},
-		`allow 1.1 when enableModularModules false`: {
-			enableModularModules: false,
-			inputSchema:          typesystem.SchemaVersion1_1,
-			expectAllowed:        true,
+		`allow 1.1`: {
+			inputSchema: typesystem.SchemaVersion1_1,
 		},
 	}
 
 	for testName, test := range testCases {
 		t.Run(testName, func(t *testing.T) {
-			if test.expectAllowed {
-				mockDatastore.EXPECT().WriteAuthorizationModel(gomock.Any(), storeID, gomock.Any()).Return(nil)
-			}
+			mockDatastore.EXPECT().WriteAuthorizationModel(gomock.Any(), storeID, gomock.Any()).Return(nil)
 
-			cmd := NewWriteAuthorizationModelCommand(mockDatastore,
-				WithEnableModularModels(test.enableModularModules),
-			)
+			cmd := NewWriteAuthorizationModelCommand(mockDatastore)
 			_, err := cmd.Execute(ctx, &openfgav1.WriteAuthorizationModelRequest{
 				StoreId: storeID,
 				TypeDefinitions: []*openfgav1.TypeDefinition{
@@ -75,11 +53,7 @@ func TestWriteAuthorizationModelWithExperimentalEnableModularModels(t *testing.T
 				},
 				SchemaVersion: test.inputSchema,
 			})
-			if test.expectAllowed {
-				require.NoError(t, err)
-			} else {
-				require.ErrorIs(t, err, status.Error(codes.InvalidArgument, "modular models (schema version 1.2) are not supported"))
-			}
+			require.NoError(t, err)
 		})
 	}
 }
diff --git a/pkg/server/server.go b/pkg/server/server.go
@@ -6,7 +6,6 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
-	"slices"
 	"sort"
 	"strconv"
 	"time"
@@ -46,9 +45,8 @@ import (
 type ExperimentalFeatureFlag string
 
 const (
-	AuthorizationModelIDHeader                              = "Openfga-Authorization-Model-Id"
-	authorizationModelIDKey                                 = "authorization_model_id"
-	ExperimentalEnableModularModels ExperimentalFeatureFlag = "enable-modular-models"
+	AuthorizationModelIDHeader = "Openfga-Authorization-Model-Id"
+	authorizationModelIDKey    = "authorization_model_id"
 )
 
 var tracer = otel.Tracer("openfga/pkg/server")
@@ -899,12 +897,9 @@ func (s *Server) WriteAuthorizationModel(ctx context.Context, req *openfgav1.Wri
 		Method:  "WriteAuthorizationModel",
 	})
 
-	enableModularModels := slices.Contains(s.experimentals, ExperimentalEnableModularModels)
-
 	c := commands.NewWriteAuthorizationModelCommand(s.datastore,
 		commands.WithWriteAuthModelLogger(s.logger),
 		commands.WithWriteAuthModelMaxSizeInBytes(s.maxAuthorizationModelSizeInBytes),
-		commands.WithEnableModularModels(enableModularModels),
 	)
 	res, err := c.Execute(ctx, req)
 	if err != nil {

diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go
@@ -1564,7 +1564,7 @@ func TestDelegateCheckResolver(t *testing.T) {
 	})
 }
 
-func TestWriteAuthorizationModelWithExperimentalEnableModularModels(t *testing.T) {
+func TestWriteAuthorizationModelWithSchema12(t *testing.T) {
 	t.Cleanup(func() {
 		goleak.VerifyNone(t)
 	})
@@ -1576,34 +1576,12 @@ func TestWriteAuthorizationModelWithExperimentalEnableModularModels(t *testing.T
 
 	mockDatastore := mockstorage.NewMockOpenFGADatastore(mockController)
 
-	t.Run("rejects_request_with_schema_version_1.2", func(t *testing.T) {
+	t.Run("accepts_request_with_schema_version_1.2", func(t *testing.T) {
 		s := MustNewServerWithOpts(
 			WithDatastore(mockDatastore),
 		)
 		defer s.Close()
 
-		mockDatastore.EXPECT().MaxTypesPerAuthorizationModel().Return(100)
-
-		_, err := s.WriteAuthorizationModel(ctx, &openfgav1.WriteAuthorizationModelRequest{
-			StoreId:       storeID,
-			SchemaVersion: typesystem.SchemaVersion1_2,
-			TypeDefinitions: []*openfgav1.TypeDefinition{
-				{
-					Type: "user",
-				},
-			},
-		})
-
-		require.ErrorIs(t, err, status.Error(codes.InvalidArgument, "modular models (schema version 1.2) are not supported"))
-	})
-
-	t.Run("accepts_request_with_schema_version_1.2_if_experimental_flag_enabled", func(t *testing.T) {
-		s := MustNewServerWithOpts(
-			WithDatastore(mockDatastore),
-			WithExperimentals(ExperimentalEnableModularModels),
-		)
-		defer s.Close()
-
 		mockDatastore.EXPECT().MaxTypesPerAuthorizationModel().Return(100)
 		mockDatastore.EXPECT().WriteAuthorizationModel(gomock.Any(), storeID, gomock.Any()).Return(nil)