Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Graphql naming conventions #1528

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 39 additions & 2 deletions src/Config/Entity.cs
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,26 @@ public bool TryProcessGraphQLNamingConfig()
{
typeConfiguration = JsonSerializer.Deserialize<SingularPlural>(nameTypeSettings)!;
}
else if (nameTypeSettings.ValueKind is JsonValueKind.Null)
{
typeConfiguration = null;
}
else
{
// Not Supported Type
return false;
}
}

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<bool>(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.
Expand Down Expand Up @@ -181,7 +194,7 @@ error is InvalidOperationException ||
}
else
{
GraphQL = new GraphQLEntitySettings(Type: typeConfiguration);
GraphQL = new GraphQLEntitySettings(Type: typeConfiguration, ForceNamingStyle: forceNamingStyle);
}
}
}
Expand Down Expand Up @@ -319,7 +332,25 @@ error is InvalidOperationException ||
{
throw new JsonException("Unsupported GraphQL Type");
}
}

/// <summary>
/// Gets an entity's GraphQL force naming style configuration setting.
/// </summary>
/// <returns>Boolean to indicate whether to force using the GraphQL naming conventions for this entity.</returns>
public bool FetchGraphQLForceNamingStyle()
{
if (GraphQL is null)
{
return false;
}

if (GraphQL is GraphQLEntitySettings settings)
{
return settings.ForceNamingStyle;
}

return false;
}

/// <summary>
Expand Down Expand Up @@ -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
/// </param>
/// <param name="ForceNamingStyle">Forces the naming style of this entity to follow the GraphQL naming style conventions.
/// </param>
/// <seealso cref="<https://engdic.org/singular-and-plural-noun-rules-definitions-examples/"/>
public record GraphQLEntitySettings([property: JsonPropertyName("type")] object? Type = null);
/// <seealso cref="https://github.com/hendrikniemann/graphql-style-guide#fields"/>
public record GraphQLEntitySettings(
[property: JsonPropertyName("type")] object? Type = null,
[property: JsonPropertyName("force-naming-style")] bool ForceNamingStyle = false
);

/// <summary>
/// Describes the GraphQL settings applicable to an entity which is backed by a stored procedure.
Expand Down
7 changes: 6 additions & 1 deletion src/Config/GlobalSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,16 @@ public record RestGlobalSettings(
/// <param name="Path">The URL path at which the graphql endpoint will be exposed.</param>
/// <param name="AllowIntrospection">Defines if the GraphQL introspection file
/// will be generated by the runtime. If GraphQL is disabled, this will be ignored.</param>
/// <param name="ForceNamingStyle">Forces the naming style of all GraphQL entities to follow
/// the default GraphQL naming style conventions.</param>
/// <seealso cref="https://github.com/hendrikniemann/graphql-style-guide#fields"/>
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);

/// <summary>
Expand Down
16 changes: 14 additions & 2 deletions src/Service.GraphQLBuilder/Sql/SchemaConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ public static ObjectTypeDefinitionNode FromDatabaseObject(
[NotNull] Entity configEntity,
Dictionary<string, Entity> entities,
IEnumerable<string> rolesAllowedForEntity,
IDictionary<string, IEnumerable<string>> rolesAllowedForFields)
IDictionary<string, IEnumerable<string>> rolesAllowedForFields,
bool forceNamingStyle)
{
Dictionary<string, FieldDefinitionNode> fields = new();
List<DirectiveNode> objectTypeDirectives = new();
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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<InputValueDefinitionNode>(),
isNullableRelationship ? targetField : new NonNullTypeNode(targetField),
Expand Down
74 changes: 74 additions & 0 deletions src/Service.Tests/Configuration/ConfigurationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1365,6 +1365,80 @@ public async Task TestSchemaIntrospectionQuery(bool enableIntrospection, bool ex
}
}

/// <summary>
/// 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.
/// </summary>
[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<string, Entity> 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<ILogger<RuntimeConfigProvider>> 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());
}
}
}

/// <summary>
/// Indirectly tests IsGraphQLReservedName(). Runtime config provided to engine which will
/// trigger SqlMetadataProvider PopulateSourceDefinitionAsync() to pull column metadata from
Expand Down
13 changes: 13 additions & 0 deletions src/Service.Tests/DatabaseSchema-MsSql.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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),
Expand Down
Loading