From c459acebcd92079156ce9d70262ca76492d0bdcb Mon Sep 17 00:00:00 2001 From: Rody van Sambeek Date: Mon, 29 May 2023 22:06:30 +0200 Subject: [PATCH] Added new settings to force GraphQL naming conventions per entity and as a global setting --- src/Config/Entity.cs | 41 +++++++++- src/Config/GlobalSettings.cs | 7 +- .../Sql/SchemaConverter.cs | 16 +++- .../Configuration/ConfigurationTests.cs | 74 +++++++++++++++++ src/Service.Tests/DatabaseSchema-MsSql.sql | 13 +++ .../Sql/SchemaConverterTests.cs | 79 +++++++++++++------ .../Sql/StoredProcedureBuilderTests.cs | 3 +- src/Service.Tests/dab-config.MsSql.json | 21 +++++ src/Service/Services/GraphQLSchemaCreator.cs | 6 +- 9 files changed, 227 insertions(+), 33 deletions(-) diff --git a/src/Config/Entity.cs b/src/Config/Entity.cs index 8518919c44..b99db9ffde 100644 --- a/src/Config/Entity.cs +++ b/src/Config/Entity.cs @@ -120,6 +120,10 @@ public bool TryProcessGraphQLNamingConfig() { typeConfiguration = JsonSerializer.Deserialize(nameTypeSettings)!; } + else if (nameTypeSettings.ValueKind is JsonValueKind.Null) + { + typeConfiguration = null; + } else { // Not Supported Type @@ -127,6 +131,15 @@ public bool TryProcessGraphQLNamingConfig() } } + bool forceNamingStyle = false; + if (configElement.TryGetProperty(propertyName: "force-naming-style", out JsonElement forceNamingStyleSetting)) + { + if (forceNamingStyleSetting.ValueKind is JsonValueKind.True || forceNamingStyleSetting.ValueKind is JsonValueKind.False) + { + forceNamingStyle = JsonSerializer.Deserialize(forceNamingStyleSetting); + } + } + // Only stored procedure configuration can override the GraphQL operation type. // When the entity is a stored procedure, GraphQL metadata will either be: // - GraphQLStoredProcedureEntityOperationSettings when only operation is configured. @@ -181,7 +194,7 @@ error is InvalidOperationException || } else { - GraphQL = new GraphQLEntitySettings(Type: typeConfiguration); + GraphQL = new GraphQLEntitySettings(Type: typeConfiguration, ForceNamingStyle: forceNamingStyle); } } } @@ -319,7 +332,25 @@ error is InvalidOperationException || { throw new JsonException("Unsupported GraphQL Type"); } + } + + /// + /// Gets an entity's GraphQL force naming style configuration setting. + /// + /// Boolean to indicate whether to force using the GraphQL naming conventions for this entity. + public bool FetchGraphQLForceNamingStyle() + { + if (GraphQL is null) + { + return false; + } + if (GraphQL is GraphQLEntitySettings settings) + { + return settings.ForceNamingStyle; + } + + return false; } /// @@ -604,8 +635,14 @@ public record RestStoredProcedureEntityVerboseSettings(object? Path, /// Can be a string or Singular-Plural type. /// If string, a default plural route will be added as per the rules at /// + /// Forces the naming style of this entity to follow the GraphQL naming style conventions. + /// /// - public record GraphQLEntitySettings([property: JsonPropertyName("type")] object? Type = null); + /// + public record GraphQLEntitySettings( + [property: JsonPropertyName("type")] object? Type = null, + [property: JsonPropertyName("force-naming-style")] bool ForceNamingStyle = false + ); /// /// Describes the GraphQL settings applicable to an entity which is backed by a stored procedure. diff --git a/src/Config/GlobalSettings.cs b/src/Config/GlobalSettings.cs index 025b455303..7c4c95bc39 100644 --- a/src/Config/GlobalSettings.cs +++ b/src/Config/GlobalSettings.cs @@ -44,11 +44,16 @@ public record RestGlobalSettings( /// The URL path at which the graphql endpoint will be exposed. /// Defines if the GraphQL introspection file /// will be generated by the runtime. If GraphQL is disabled, this will be ignored. + /// Forces the naming style of all GraphQL entities to follow + /// the default GraphQL naming style conventions. + /// public record GraphQLGlobalSettings( bool Enabled = true, string Path = GlobalSettings.GRAPHQL_DEFAULT_PATH, [property: JsonPropertyName("allow-introspection")] - bool AllowIntrospection = true) + bool AllowIntrospection = true, + [property: JsonPropertyName("force-naming-style")] + bool ForceNamingStyle = false) : ApiSettings(Enabled, Path); /// diff --git a/src/Service.GraphQLBuilder/Sql/SchemaConverter.cs b/src/Service.GraphQLBuilder/Sql/SchemaConverter.cs index 602aa40d79..e3b73b5d2b 100644 --- a/src/Service.GraphQLBuilder/Sql/SchemaConverter.cs +++ b/src/Service.GraphQLBuilder/Sql/SchemaConverter.cs @@ -36,7 +36,8 @@ public static ObjectTypeDefinitionNode FromDatabaseObject( [NotNull] Entity configEntity, Dictionary entities, IEnumerable rolesAllowedForEntity, - IDictionary> rolesAllowedForFields) + IDictionary> rolesAllowedForFields, + bool forceNamingStyle) { Dictionary fields = new(); List objectTypeDirectives = new(); @@ -96,6 +97,11 @@ public static ObjectTypeDefinitionNode FromDatabaseObject( exposedColumnName = columnAlias; } + if (forceNamingStyle || configEntity.FetchGraphQLForceNamingStyle()) + { + exposedColumnName = FormatNameForField(exposedColumnName); + } + NamedTypeNode fieldType = new(GetGraphQLTypeFromSystemType(column.SystemType)); FieldDefinitionNode field = new( location: null, @@ -169,9 +175,15 @@ public static ObjectTypeDefinitionNode FromDatabaseObject( subStatusCode: DataApiBuilderException.SubStatusCodes.GraphQLMapping), }; + string exposedRelationshipName = relationshipName; + if (forceNamingStyle) + { + exposedRelationshipName = FormatNameForField(exposedRelationshipName); + } + FieldDefinitionNode relationshipField = new( location: null, - new NameNode(relationshipName), + new NameNode(exposedRelationshipName), description: null, new List(), isNullableRelationship ? targetField : new NonNullTypeNode(targetField), diff --git a/src/Service.Tests/Configuration/ConfigurationTests.cs b/src/Service.Tests/Configuration/ConfigurationTests.cs index 43767b7048..9fa9db80cc 100644 --- a/src/Service.Tests/Configuration/ConfigurationTests.cs +++ b/src/Service.Tests/Configuration/ConfigurationTests.cs @@ -1365,6 +1365,80 @@ public async Task TestSchemaIntrospectionQuery(bool enableIntrospection, bool ex } } + /// + /// Integration test that validates naming conventions based on the setting + /// of force-naming-style setting in the configuration. + /// TestCategory is required for CI/CD pipeline to inject a connection string. + /// + [TestCategory(TestCategory.MSSQL)] + [DataTestMethod] + [DataRow(false, "movieName", true, "does not exist on the type")] + [DataRow(false, "MovieName", false, "{\"items\":[{\"MovieName\":\"Example Movie 1\"},{\"MovieName\":\"Example Movie 2\"}]}")] + [DataRow(true, "movieName", false, "{\"items\":[{\"movieName\":\"Example Movie 1\"},{\"movieName\":\"Example Movie 2\"}]}")] + [DataRow(true, "MovieName", true, "does not exist on the type")] + public async Task TestForceNamingStyleQuery(bool enableForceNamingStyle, string queryPropertyName, bool expectError, string expected) + { + Entity graphQLEntityForceNaming = new( + Source: JsonSerializer.SerializeToElement("movies"), + Rest: JsonSerializer.SerializeToElement(false), + GraphQL: JsonSerializer.SerializeToElement(new GraphQLEntitySettings + { + ForceNamingStyle = enableForceNamingStyle + }), + Permissions: new PermissionSetting[] { GetMinimalPermissionConfig(AuthorizationResolver.ROLE_ANONYMOUS) }, + Relationships: null, + Mappings: null); + + Dictionary entityMap = new() + { + { "Movie", graphQLEntityForceNaming } + }; + + CreateCustomConfigFile(globalRestEnabled: true, entityMap); + + string[] args = new[] + { + $"--ConfigFileName={CUSTOM_CONFIG_FILENAME}" + }; + + using (TestServer server = new(Program.CreateWebHostBuilder(args))) + using (HttpClient client = server.CreateClient()) + { + string graphQLQueryName = "movies"; + string graphQLQuery = @"{ + movies { + items { + " + queryPropertyName + @" + } + } + }"; + + Mock> logger = new(); + RuntimeConfigProvider configProvider = new(new RuntimeConfigPath { ConfigFileName = CUSTOM_CONFIG_FILENAME }, logger.Object); + + JsonElement actual = await GraphQLRequestExecutor.PostGraphQLRequestAsync( + client, + configProvider, + query: graphQLQuery, + queryName: graphQLQueryName, + variables: null, + clientRoleHeader: null + ); + + if (expectError) + { + SqlTestHelper.TestForErrorInGraphQLResponse( + response: actual.ToString(), + message: expected + ); + } + else + { + SqlTestHelper.PerformTestEqualJsonStrings(expected, actual.ToString()); + } + } + } + /// /// Indirectly tests IsGraphQLReservedName(). Runtime config provided to engine which will /// trigger SqlMetadataProvider PopulateSourceDefinitionAsync() to pull column metadata from diff --git a/src/Service.Tests/DatabaseSchema-MsSql.sql b/src/Service.Tests/DatabaseSchema-MsSql.sql index 040997a9e6..940153fa57 100644 --- a/src/Service.Tests/DatabaseSchema-MsSql.sql +++ b/src/Service.Tests/DatabaseSchema-MsSql.sql @@ -47,6 +47,7 @@ DROP TABLE IF EXISTS graphql_incompatible; DROP TABLE IF EXISTS GQLmappings; DROP TABLE IF EXISTS bookmarks; DROP TABLE IF EXISTS mappedbookmarks; +DROP TABLE IF EXISTS movies; DROP SCHEMA IF EXISTS [foo]; COMMIT; @@ -254,6 +255,12 @@ CREATE TABLE mappedbookmarks bkname nvarchar(50) NOT NULL ) +CREATE TABLE movies +( + id int IDENTITY(5001, 1) PRIMARY KEY, + MovieName text NULL +); + ALTER TABLE books ADD CONSTRAINT book_publisher_fk FOREIGN KEY (publisher_id) @@ -358,6 +365,12 @@ FROM cteN WHERE [Number] <= @UpperBound; SET IDENTITY_INSERT mappedbookmarks OFF +SET IDENTITY_INSERT movies ON +INSERT INTO movies(id, MovieName) +VALUES (1, 'Example Movie 1'), +(2, 'Example Movie 2'); +SET IDENTITY_INSERT movies OFF + SET IDENTITY_INSERT books ON INSERT INTO books(id, title, publisher_id) VALUES (1, 'Awesome book', 1234), diff --git a/src/Service.Tests/GraphQLBuilder/Sql/SchemaConverterTests.cs b/src/Service.Tests/GraphQLBuilder/Sql/SchemaConverterTests.cs index bb58ac00f3..b010fbc587 100644 --- a/src/Service.Tests/GraphQLBuilder/Sql/SchemaConverterTests.cs +++ b/src/Service.Tests/GraphQLBuilder/Sql/SchemaConverterTests.cs @@ -31,11 +31,15 @@ public class SchemaConverterTests const string REFD_COLNAME = "fk_col"; [DataTestMethod] - [DataRow("test", "test")] - [DataRow("Test", "Test")] - [DataRow("T_est", "T_est")] - [DataRow("Test1", "Test1")] - public void EntityNameBecomesObjectName(string entityName, string expected) + [DataRow("test", false, "test")] + [DataRow("Test", false, "Test")] + [DataRow("T_est", false, "T_est")] + [DataRow("Test1", false, "Test1")] + [DataRow("test", true, "test")] + [DataRow("Test", true, "Test")] + [DataRow("T_est", true, "T_est")] + [DataRow("Test1", true, "Test1")] + public void EntityNameBecomesObjectName(string entityName, bool forceNamingStyle, string expected) { DatabaseObject dbObject = new DatabaseTable() { TableDefinition = new() }; @@ -45,7 +49,8 @@ public void EntityNameBecomesObjectName(string entityName, string expected) GenerateEmptyEntity(), new(), rolesAllowedForEntity: GetRolesAllowedForEntity(), - rolesAllowedForFields: GetFieldToRolesMap() + rolesAllowedForFields: GetFieldToRolesMap(), + forceNamingStyle: forceNamingStyle ); Assert.AreEqual(expected, od.Name.Value); @@ -57,9 +62,13 @@ public void EntityNameBecomesObjectName(string entityName, string expected) /// /// [DataTestMethod] - [DataRow("test", "test")] - [DataRow("Test", "Test")] - public void ColumnNameBecomesFieldName(string columnName, string expected) + [DataRow("test", false, "test")] + [DataRow("Test", false, "Test")] + [DataRow("test", true, "test")] + [DataRow("Test", true, "test")] + [DataRow("testTest", true, "testTest")] + [DataRow("TestTest", true, "testTest")] + public void ColumnNameBecomesFieldName(string columnName, bool forceNamingStyle, string expected) { SourceDefinition table = new(); table.Columns.Add(columnName, new ColumnDefinition @@ -75,7 +84,8 @@ public void ColumnNameBecomesFieldName(string columnName, string expected) GenerateEmptyEntity(), new(), rolesAllowedForEntity: GetRolesAllowedForEntity(), - rolesAllowedForFields: GetFieldToRolesMap(columnName: table.Columns.First().Key) + rolesAllowedForFields: GetFieldToRolesMap(columnName: table.Columns.First().Key), + forceNamingStyle: forceNamingStyle ); Assert.AreEqual(expected, od.Fields[0].Name.Value); @@ -117,7 +127,9 @@ public void FieldNameMatchesMappedValue(bool setMappings, string backingColumnNa configEntity, entities: new(), rolesAllowedForEntity: GetRolesAllowedForEntity(), - rolesAllowedForFields: GetFieldToRolesMap(columnName: table.Columns.First().Key)); + rolesAllowedForFields: GetFieldToRolesMap(columnName: table.Columns.First().Key), + forceNamingStyle: false + ); string errorMessage = "Object field representing database column has an unexpected name value."; if (expectMappedName) @@ -150,7 +162,8 @@ public void PrimaryKeyColumnHasAppropriateDirective() GenerateEmptyEntity(), new(), rolesAllowedForEntity: GetRolesAllowedForEntity(), - rolesAllowedForFields: GetFieldToRolesMap() + rolesAllowedForFields: GetFieldToRolesMap(), + forceNamingStyle: false ); FieldDefinitionNode field = od.Fields.First(f => f.Name.Value == columnName); @@ -179,7 +192,8 @@ public void MultiplePrimaryKeysAllMappedWithDirectives() GenerateEmptyEntity(), new(), rolesAllowedForEntity: GetRolesAllowedForEntity(), - rolesAllowedForFields: GetFieldToRolesMap() + rolesAllowedForFields: GetFieldToRolesMap(), + forceNamingStyle: false ); foreach (FieldDefinitionNode field in od.Fields) @@ -209,7 +223,8 @@ public void MultipleColumnsAllMapped() GenerateEmptyEntity(), new(), rolesAllowedForEntity: GetRolesAllowedForEntity(), - rolesAllowedForFields: GetFieldToRolesMap(additionalColumns: customColumnCount) + rolesAllowedForFields: GetFieldToRolesMap(additionalColumns: customColumnCount), + forceNamingStyle: false ); Assert.AreEqual(table.Columns.Count, od.Fields.Count); @@ -247,7 +262,8 @@ public void SystemTypeMapsToCorrectGraphQLType(Type systemType, string graphQLTy GenerateEmptyEntity(), new(), rolesAllowedForEntity: GetRolesAllowedForEntity(), - rolesAllowedForFields: GetFieldToRolesMap() + rolesAllowedForFields: GetFieldToRolesMap(), + forceNamingStyle: false ); FieldDefinitionNode field = od.Fields.First(f => f.Name.Value == columnName); @@ -274,7 +290,8 @@ public void NullColumnBecomesNullField() GenerateEmptyEntity(), new(), rolesAllowedForEntity: GetRolesAllowedForEntity(), - rolesAllowedForFields: GetFieldToRolesMap() + rolesAllowedForFields: GetFieldToRolesMap(), + forceNamingStyle: false ); FieldDefinitionNode field = od.Fields.First(f => f.Name.Value == columnName); @@ -301,7 +318,8 @@ public void NonNullColumnBecomesNonNullField() GenerateEmptyEntity(), new(), rolesAllowedForEntity: GetRolesAllowedForEntity(), - rolesAllowedForFields: GetFieldToRolesMap() + rolesAllowedForFields: GetFieldToRolesMap(), + forceNamingStyle: false ); FieldDefinitionNode field = od.Fields.First(f => f.Name.Value == columnName); @@ -372,7 +390,8 @@ public void WhenForeignKeyDefinedButNoRelationship_GraphQLWontModelIt() configEntity, new() { { TARGET_ENTITY, relationshipEntity } }, rolesAllowedForEntity: GetRolesAllowedForEntity(), - rolesAllowedForFields: GetFieldToRolesMap() + rolesAllowedForFields: GetFieldToRolesMap(), + forceNamingStyle: false ); Assert.AreEqual(2, od.Fields.Count); @@ -409,7 +428,8 @@ public void SingularNamingRulesDeterminedByRuntimeConfig(string entityName, stri configEntity, new(), rolesAllowedForEntity: GetRolesAllowedForEntity(), - rolesAllowedForFields: GetFieldToRolesMap() + rolesAllowedForFields: GetFieldToRolesMap(), + forceNamingStyle: false ); Assert.AreEqual(expected, od.Name.Value); @@ -441,7 +461,8 @@ public void AutoGeneratedFieldHasDirectiveIndicatingSuch() configEntity, new(), rolesAllowedForEntity: GetRolesAllowedForEntity(), - rolesAllowedForFields: GetFieldToRolesMap() + rolesAllowedForFields: GetFieldToRolesMap(), + forceNamingStyle: false ); Assert.IsTrue(od.Fields[0].Directives.Any(d => d.Name.Value == AutoGeneratedDirectiveType.DirectiveName)); @@ -492,7 +513,8 @@ public void DefaultValueGetsSetOnDirective(object defaultValue, string fieldName configEntity, new(), rolesAllowedForEntity: GetRolesAllowedForEntity(), - rolesAllowedForFields: GetFieldToRolesMap() + rolesAllowedForFields: GetFieldToRolesMap(), + forceNamingStyle: false ); // @authorize directive is implicitly created so the count to compare to is 2 @@ -536,7 +558,8 @@ public void AutoGeneratedFieldHasAuthorizeDirective(string[] rolesForField) configEntity, new(), rolesAllowedForEntity: GetRolesAllowedForEntity(), - rolesAllowedForFields: GetFieldToRolesMap(rolesForField: rolesForField) + rolesAllowedForFields: GetFieldToRolesMap(rolesForField: rolesForField), + forceNamingStyle: false ); // Ensures all fields added have the appropriate @authorize directive. @@ -576,7 +599,8 @@ public void FieldWithAnonymousAccessHasNoAuthorizeDirective(string[] rolesForFie configEntity, new(), rolesAllowedForEntity: GetRolesAllowedForEntity(), - rolesAllowedForFields: GetFieldToRolesMap(rolesForField: rolesForField) + rolesAllowedForFields: GetFieldToRolesMap(rolesForField: rolesForField), + forceNamingStyle: false ); // Ensures no field has the @authorize directive. @@ -618,7 +642,8 @@ public void EntityObjectTypeDefinition_AuthorizeDirectivePresence(string[] roles configEntity, new(), rolesAllowedForEntity: roles, - rolesAllowedForFields: GetFieldToRolesMap(rolesForField: roles) + rolesAllowedForFields: GetFieldToRolesMap(rolesForField: roles), + forceNamingStyle: false ); // Prepares error message, only used if assertion fails. @@ -666,7 +691,8 @@ public void EntityObjectTypeDefinition_AuthorizeDirectivePresenceMixed(string[] configEntity, new(), rolesAllowedForEntity: rolesForEntity, - rolesAllowedForFields: GetFieldToRolesMap(rolesForField: rolesForFields) + rolesAllowedForFields: GetFieldToRolesMap(rolesForField: rolesForFields), + forceNamingStyle: false ); // Prepares error message, only used if assertion fails. @@ -771,7 +797,8 @@ private static ObjectTypeDefinitionNode GenerateObjectWithRelationship(Cardinali dbObject, configEntity, new() { { TARGET_ENTITY, relationshipEntity } }, rolesAllowedForEntity: GetRolesAllowedForEntity(), - rolesAllowedForFields: GetFieldToRolesMap() + rolesAllowedForFields: GetFieldToRolesMap(), + forceNamingStyle: false ); } diff --git a/src/Service.Tests/GraphQLBuilder/Sql/StoredProcedureBuilderTests.cs b/src/Service.Tests/GraphQLBuilder/Sql/StoredProcedureBuilderTests.cs index 81f3bd32f7..6143b32211 100644 --- a/src/Service.Tests/GraphQLBuilder/Sql/StoredProcedureBuilderTests.cs +++ b/src/Service.Tests/GraphQLBuilder/Sql/StoredProcedureBuilderTests.cs @@ -194,7 +194,8 @@ public static ObjectTypeDefinitionNode CreateGraphQLTypeForEntity(Entity spEntit configEntity: spEntity, entities: new(), rolesAllowedForEntity: SchemaConverterTests.GetRolesAllowedForEntity(), - rolesAllowedForFields: SchemaConverterTests.GetFieldToRolesMap() + rolesAllowedForFields: SchemaConverterTests.GetFieldToRolesMap(), + forceNamingStyle: false ); return objectTypeDefinitionNode; diff --git a/src/Service.Tests/dab-config.MsSql.json b/src/Service.Tests/dab-config.MsSql.json index 12e6f4ed78..0ddb14c7d1 100644 --- a/src/Service.Tests/dab-config.MsSql.json +++ b/src/Service.Tests/dab-config.MsSql.json @@ -1680,6 +1680,27 @@ "rest": true, "graphql": true }, + "Movies": { + "source": "movies", + "permissions": [ + { + "role": "anonymous", + "actions": [ + "*" + ] + }, + { + "role": "authenticated", + "actions": [ + "*" + ] + } + ], + "rest": true, + "graphql": { + "force-naming-style": true + } + }, "PublisherNF": { "source": "publishers", "permissions": [ diff --git a/src/Service/Services/GraphQLSchemaCreator.cs b/src/Service/Services/GraphQLSchemaCreator.cs index e1d539f303..2e824edc40 100644 --- a/src/Service/Services/GraphQLSchemaCreator.cs +++ b/src/Service/Services/GraphQLSchemaCreator.cs @@ -41,6 +41,7 @@ public class GraphQLSchemaCreator private readonly ISqlMetadataProvider _sqlMetadataProvider; private readonly DatabaseType _databaseType; private readonly Dictionary _entities; + private readonly GraphQLGlobalSettings _graphQLGlobalSettings; private readonly IAuthorizationResolver _authorizationResolver; /// @@ -62,6 +63,7 @@ public GraphQLSchemaCreator( _databaseType = runtimeConfig.DatabaseType; _entities = runtimeConfig.Entities; + _graphQLGlobalSettings = runtimeConfig.GraphQLGlobalSettings; _queryEngine = queryEngine; _mutationEngine = mutationEngine; _sqlMetadataProvider = sqlMetadataProvider; @@ -156,6 +158,7 @@ DatabaseType.postgresql or Dictionary> rolesAllowedForFields = new(); SourceDefinition sourceDefinition = _sqlMetadataProvider.GetSourceDefinition(entityName); bool isStoredProcedure = entity.ObjectType is SourceType.StoredProcedure; + bool forceNamingStyle = _graphQLGlobalSettings.ForceNamingStyle; foreach (string column in sourceDefinition.Columns.Keys) { Config.Operation operation = isStoredProcedure ? Config.Operation.Execute : Config.Operation.Read; @@ -180,7 +183,8 @@ DatabaseType.postgresql or entity, entities, rolesAllowedForEntity, - rolesAllowedForFields + rolesAllowedForFields, + forceNamingStyle ); if (databaseObject.SourceType is not SourceType.StoredProcedure)