From cd8331bc186729498d988b1f477bd5f8dd71253d Mon Sep 17 00:00:00 2001 From: Sebastian Date: Mon, 3 Jun 2019 08:36:49 +0200 Subject: [PATCH] Support for duplicate names in GraphQL. --- .../Schemas/FieldCollection.cs | 9 +- .../GraphQL/Types/ContentDataGraphType.cs | 57 ++-- .../Contents/GraphQL/Types/Extensions.cs | 41 +++ .../Contents/GraphQL/Types/NestedGraphType.cs | 31 ++- .../Contents/GraphQL/GraphQLQueriesTests.cs | 253 +++++++++++------- .../Contents/GraphQL/GraphQLTestBase.cs | 22 +- 6 files changed, 274 insertions(+), 139 deletions(-) create mode 100644 src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/Extensions.cs diff --git a/src/Squidex.Domain.Apps.Core.Model/Schemas/FieldCollection.cs b/src/Squidex.Domain.Apps.Core.Model/Schemas/FieldCollection.cs index d09841b55..f07114303 100644 --- a/src/Squidex.Domain.Apps.Core.Model/Schemas/FieldCollection.cs +++ b/src/Squidex.Domain.Apps.Core.Model/Schemas/FieldCollection.cs @@ -122,9 +122,14 @@ namespace Squidex.Domain.Apps.Core.Schemas { Guard.NotNull(field, nameof(field)); - if (ByName.ContainsKey(field.Name) || ById.ContainsKey(field.Id)) + if (ByName.ContainsKey(field.Name)) { - throw new ArgumentException($"A field with name '{field.Name}' and id {field.Id} already exists.", nameof(field)); + throw new ArgumentException($"A field with name '{field.Name}' already exists.", nameof(field)); + } + + if (ById.ContainsKey(field.Id)) + { + throw new ArgumentException($"A field with id {field.Id} already exists.", nameof(field)); } return Clone(clone => diff --git a/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/ContentDataGraphType.cs b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/ContentDataGraphType.cs index c25cd48bf..7dfd1480b 100644 --- a/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/ContentDataGraphType.cs +++ b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/ContentDataGraphType.cs @@ -25,18 +25,17 @@ namespace Squidex.Domain.Apps.Entities.Contents.GraphQL.Types Name = $"{schemaType}DataDto"; - foreach (var field in schema.SchemaDef.Fields.ForApi()) + foreach (var (field, fieldName, typeName) in schema.SchemaDef.Fields.SafeFields()) { var (resolvedType, valueResolver) = model.GetGraphType(schema, field); if (valueResolver != null) { - var fieldType = field.TypeName(); - var fieldName = field.DisplayName(); + var displayName = field.DisplayName(); var fieldGraphType = new ObjectGraphType { - Name = $"{schemaType}Data{fieldType}Dto" + Name = $"{schemaType}Data{typeName}Dto" }; var partition = model.ResolvePartition(field.Partitioning); @@ -45,45 +44,51 @@ namespace Squidex.Domain.Apps.Entities.Contents.GraphQL.Types { var key = partitionItem.Key; - var partitionResolver = new FuncFieldResolver(c => - { - if (((ContentFieldData)c.Source).TryGetValue(key, out var value)) - { - return valueResolver(value, c); - } - else - { - return null; - } - }); - fieldGraphType.AddField(new FieldType { Name = key.EscapePartition(), - Resolver = partitionResolver, + Resolver = PartitionResolver(valueResolver, key), ResolvedType = resolvedType, Description = field.RawProperties.Hints }); } - fieldGraphType.Description = $"The structure of the {fieldName} field of the {schemaName} content type."; - - var fieldResolver = new FuncFieldResolver>(c => - { - return c.Source.GetOrDefault(field.Name); - }); + fieldGraphType.Description = $"The structure of the {displayName} field of the {schemaName} content type."; AddField(new FieldType { - Name = field.Name.ToCamelCase(), - Resolver = fieldResolver, + Name = fieldName, + Resolver = FieldResolver(field), ResolvedType = fieldGraphType, - Description = $"The {fieldName} field." + Description = $"The {displayName} field." }); } } Description = $"The structure of the {schemaName} content type."; } + + private static FuncFieldResolver PartitionResolver(ValueResolver valueResolver, string key) + { + return new FuncFieldResolver(c => + { + if (((ContentFieldData)c.Source).TryGetValue(key, out var value)) + { + return valueResolver(value, c); + } + else + { + return null; + } + }); + } + + private static FuncFieldResolver> FieldResolver(RootField field) + { + return new FuncFieldResolver>(c => + { + return c.Source.GetOrDefault(field.Name); + }); + } } } diff --git a/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/Extensions.cs b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/Extensions.cs new file mode 100644 index 000000000..9531680b0 --- /dev/null +++ b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/Extensions.cs @@ -0,0 +1,41 @@ +// ========================================================================== +// Squidex Headless CMS +// ========================================================================== +// Copyright (c) Squidex UG (haftungsbeschraenkt) +// All rights reserved. Licensed under the MIT license. +// ========================================================================== + +using System.Collections.Generic; +using System.Linq; +using Squidex.Domain.Apps.Core.Schemas; +using Squidex.Infrastructure; + +namespace Squidex.Domain.Apps.Entities.Contents.GraphQL.Types +{ + public static class Extensions + { + public static IEnumerable<(T Field, string Name, string Type)> SafeFields(this IEnumerable fields) where T : IField + { + var allFields = + fields.ForApi() + .Select(f => (Field: f, Name: f.Name.ToCamelCase(), Type: f.TypeName())).GroupBy(x => x.Name) + .Select(g => + { + return g.Select((f, i) => (f.Field, f.Name.SafeString(i), f.Type.SafeString(i))); + }) + .SelectMany(x => x); + + return allFields; + } + + private static string SafeString(this string value, int index) + { + if (index > 0) + { + return value + (index + 1); + } + + return value; + } + } +} diff --git a/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/NestedGraphType.cs b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/NestedGraphType.cs index e7b954698..7f64227bb 100644 --- a/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/NestedGraphType.cs +++ b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/NestedGraphType.cs @@ -25,27 +25,17 @@ namespace Squidex.Domain.Apps.Entities.Contents.GraphQL.Types Name = $"{schemaType}{fieldName}ChildDto"; - foreach (var nestedField in field.Fields.ForApi()) + foreach (var (nestedField, nestedName, _) in field.Fields.SafeFields()) { var fieldInfo = model.GetGraphType(schema, nestedField); if (fieldInfo.ResolveType != null) { - var resolver = new FuncFieldResolver(c => - { - if (((JsonObject)c.Source).TryGetValue(nestedField.Name, out var value)) - { - return fieldInfo.Resolver(value, c); - } - else - { - return fieldInfo; - } - }); + var resolver = ValueResolver(nestedField, fieldInfo); AddField(new FieldType { - Name = nestedField.Name.ToCamelCase(), + Name = nestedName, Resolver = resolver, ResolvedType = fieldInfo.ResolveType, Description = $"The {fieldName}/{nestedField.DisplayName()} nested field." @@ -55,5 +45,20 @@ namespace Squidex.Domain.Apps.Entities.Contents.GraphQL.Types Description = $"The structure of the {schemaName}.{fieldName} nested schema."; } + + private static FuncFieldResolver ValueResolver(NestedField nestedField, (IGraphType ResolveType, ValueResolver Resolver) fieldInfo) + { + return new FuncFieldResolver(c => + { + if (((JsonObject)c.Source).TryGetValue(nestedField.Name, out var value)) + { + return fieldInfo.Resolver(value, c); + } + else + { + return fieldInfo; + } + }); + } } } diff --git a/tests/Squidex.Domain.Apps.Entities.Tests/Contents/GraphQL/GraphQLQueriesTests.cs b/tests/Squidex.Domain.Apps.Entities.Tests/Contents/GraphQL/GraphQLQueriesTests.cs index 06f6143eb..09b5afd92 100644 --- a/tests/Squidex.Domain.Apps.Entities.Tests/Contents/GraphQL/GraphQLQueriesTests.cs +++ b/tests/Squidex.Domain.Apps.Entities.Tests/Contents/GraphQL/GraphQLQueriesTests.cs @@ -187,9 +187,9 @@ namespace Squidex.Domain.Apps.Entities.Contents.GraphQL var assetId = Guid.NewGuid(); var asset = CreateAsset(assetId); - var query = $@" - query {{ - findAsset(id: ""{assetId}"") {{ + var query = @" + query { + findAsset(id: """") { id version created @@ -209,8 +209,8 @@ namespace Squidex.Domain.Apps.Entities.Contents.GraphQL pixelHeight tags slug - }} - }}"; + } + }".Replace("", assetId.ToString()); A.CallTo(() => assetQuery.FindAssetAsync(MatchsAssetContext(), assetId)) .Returns(asset); @@ -372,12 +372,12 @@ namespace Squidex.Domain.Apps.Entities.Contents.GraphQL { new { - nestedNumber = 1, + nestedNumber = 10, nestedBoolean = true }, new { - nestedNumber = 2, + nestedNumber = 20, nestedBoolean = false } } @@ -518,15 +518,86 @@ namespace Squidex.Domain.Apps.Entities.Contents.GraphQL AssertResult(expected, result); } + [Fact] + public async Task Should_return_single_content_with_duplicate_names() + { + var contentId = Guid.NewGuid(); + var content = CreateContent(contentId, Guid.Empty, Guid.Empty); + + var query = @" + query { + findMySchemaContent(id: """") { + data { + myNumber { + iv + } + myNumber2 { + iv + } + myArray { + iv { + nestedNumber + nestedNumber2 + } + } + } + } + }".Replace("", contentId.ToString()); + + A.CallTo(() => contentQuery.FindContentAsync(MatchsContentContext(), schemaId.ToString(), contentId, EtagVersion.Any)) + .Returns(content); + + var result = await sut.QueryAsync(context, new GraphQLQuery { Query = query }); + + var expected = new + { + data = new + { + findMySchemaContent = new + { + data = new + { + myNumber = new + { + iv = 1 + }, + myNumber2 = new + { + iv = 2 + }, + myArray = new + { + iv = new[] + { + new + { + nestedNumber = 10, + nestedNumber2 = 11, + }, + new + { + nestedNumber = 20, + nestedNumber2 = 21, + } + } + } + } + } + } + }; + + AssertResult(expected, result); + } + [Fact] public async Task Should_return_single_content_when_finding_content() { var contentId = Guid.NewGuid(); var content = CreateContent(contentId, Guid.Empty, Guid.Empty); - var query = $@" - query {{ - findMySchemaContent(id: ""{contentId}"") {{ + var query = @" + query { + findMySchemaContent(id: """") { id version created @@ -535,34 +606,34 @@ namespace Squidex.Domain.Apps.Entities.Contents.GraphQL lastModifiedBy status url - data {{ - myString {{ + data { + myString { de - }} - myNumber {{ + } + myNumber { iv - }} - myBoolean {{ + } + myBoolean { iv - }} - myDatetime {{ + } + myDatetime { iv - }} - myJson {{ + } + myJson { iv - }} - myGeolocation {{ + } + myGeolocation { iv - }} - myTags {{ + } + myTags { iv - }} - myLocalized {{ + } + myLocalized { de_DE - }} - }} - }} - }}"; + } + } + } + }".Replace("", contentId.ToString()); A.CallTo(() => contentQuery.FindContentAsync(MatchsContentContext(), schemaId.ToString(), contentId, EtagVersion.Any)) .Returns(content); @@ -645,19 +716,19 @@ namespace Squidex.Domain.Apps.Entities.Contents.GraphQL var contentId = Guid.NewGuid(); var content = CreateContent(contentId, contentRefId, Guid.Empty); - var query = $@" - query {{ - findMySchemaContent(id: ""{contentId}"") {{ + var query = @" + query { + findMySchemaContent(id: """") { id - data {{ - myReferences {{ - iv {{ + data { + myReferences { + iv { id - }} - }} - }} - }} - }}"; + } + } + } + } + }".Replace("", contentId.ToString()); A.CallTo(() => contentQuery.FindContentAsync(MatchsContentContext(), schemaId.ToString(), contentId, EtagVersion.Any)) .Returns(content); @@ -703,19 +774,19 @@ namespace Squidex.Domain.Apps.Entities.Contents.GraphQL var contentId = Guid.NewGuid(); var content = CreateContent(contentId, Guid.Empty, assetRefId); - var query = $@" - query {{ - findMySchemaContent(id: ""{contentId}"") {{ + var query = @" + query { + findMySchemaContent(id: """") { id - data {{ - myAssets {{ - iv {{ + data { + myAssets { + iv { id - }} - }} - }} - }} - }}"; + } + } + } + } + }".Replace("", contentId.ToString()); A.CallTo(() => contentQuery.FindContentAsync(MatchsContentContext(), schemaId.ToString(), contentId, EtagVersion.Any)) .Returns(content); @@ -760,18 +831,18 @@ namespace Squidex.Domain.Apps.Entities.Contents.GraphQL var asset1 = CreateAsset(assetId1); var asset2 = CreateAsset(assetId2); - var query1 = $@" - query {{ - findAsset(id: ""{assetId1}"") {{ + var query1 = @" + query { + findAsset(id: """") { id - }} - }}"; - var query2 = $@" - query {{ - findAsset(id: ""{assetId2}"") {{ + } + }".Replace("", assetId1.ToString()); + var query2 = @" + query { + findAsset(id: """") { id - }} - }}"; + } + }".Replace("", assetId2.ToString()); A.CallTo(() => assetQuery.FindAssetAsync(MatchsAssetContext(), assetId1)) .Returns(asset1); @@ -813,9 +884,9 @@ namespace Squidex.Domain.Apps.Entities.Contents.GraphQL var contentId = Guid.NewGuid(); var content = CreateContent(contentId, Guid.Empty, Guid.Empty, new NamedContentData()); - var query = $@" - query {{ - findMySchemaContent(id: ""{contentId}"") {{ + var query = @" + query { + findMySchemaContent(id: """") { id version created @@ -823,13 +894,13 @@ namespace Squidex.Domain.Apps.Entities.Contents.GraphQL lastModified lastModifiedBy url - data {{ - myInvalid {{ + data { + myInvalid { iv - }} - }} - }} - }}"; + } + } + } + }".Replace("", contentId.ToString()); A.CallTo(() => contentQuery.FindContentAsync(MatchsContentContext(), schemaId.ToString(), contentId, EtagVersion.Any)) .Returns(content); @@ -855,19 +926,19 @@ namespace Squidex.Domain.Apps.Entities.Contents.GraphQL var contentId = Guid.NewGuid(); var content = CreateContent(contentId, Guid.Empty, Guid.Empty, null, dataDraft); - var query = $@" - query {{ - findMySchemaContent(id: ""{contentId}"") {{ - dataDraft {{ - myString {{ + var query = @" + query { + findMySchemaContent(id: """") { + dataDraft { + myString { de - }} - myNumber {{ + } + myNumber { iv - }} - }} - }} - }}"; + } + } + } + }".Replace("", contentId.ToString()); A.CallTo(() => contentQuery.FindContentAsync(MatchsContentContext(), schemaId.ToString(), contentId, EtagVersion.Any)) .Returns(content); @@ -904,16 +975,16 @@ namespace Squidex.Domain.Apps.Entities.Contents.GraphQL var contentId = Guid.NewGuid(); var content = CreateContent(contentId, Guid.Empty, Guid.Empty, null); - var query = $@" - query {{ - findMySchemaContent(id: ""{contentId}"") {{ - dataDraft {{ - myString {{ + var query = @" + query { + findMySchemaContent(id: """") { + dataDraft { + myString { de - }} - }} - }} - }}"; + } + } + } + }".Replace("", contentId.ToString()); A.CallTo(() => contentQuery.FindContentAsync(MatchsContentContext(), schemaId.ToString(), contentId, EtagVersion.Any)) .Returns(content); diff --git a/tests/Squidex.Domain.Apps.Entities.Tests/Contents/GraphQL/GraphQLTestBase.cs b/tests/Squidex.Domain.Apps.Entities.Tests/Contents/GraphQL/GraphQLTestBase.cs index 9edd65708..d3f6b5ea1 100644 --- a/tests/Squidex.Domain.Apps.Entities.Tests/Contents/GraphQL/GraphQLTestBase.cs +++ b/tests/Squidex.Domain.Apps.Entities.Tests/Contents/GraphQL/GraphQLTestBase.cs @@ -58,13 +58,15 @@ namespace Squidex.Domain.Apps.Entities.Contents.GraphQL new StringFieldProperties()) .AddNumber(3, "my-number", Partitioning.Invariant, new NumberFieldProperties()) - .AddAssets(4, "my-assets", Partitioning.Invariant, + .AddNumber(4, "my_number", Partitioning.Invariant, + new NumberFieldProperties()) + .AddAssets(5, "my-assets", Partitioning.Invariant, new AssetsFieldProperties()) - .AddBoolean(5, "my-boolean", Partitioning.Invariant, + .AddBoolean(6, "my-boolean", Partitioning.Invariant, new BooleanFieldProperties()) - .AddDateTime(6, "my-datetime", Partitioning.Invariant, + .AddDateTime(7, "my-datetime", Partitioning.Invariant, new DateTimeFieldProperties()) - .AddReferences(7, "my-references", Partitioning.Invariant, + .AddReferences(8, "my-references", Partitioning.Invariant, new ReferencesFieldProperties { SchemaId = schemaId }) .AddReferences(9, "my-invalid", Partitioning.Invariant, new ReferencesFieldProperties { SchemaId = Guid.NewGuid() }) @@ -76,7 +78,8 @@ namespace Squidex.Domain.Apps.Entities.Contents.GraphQL new StringFieldProperties()) .AddArray(13, "my-array", Partitioning.Invariant, f => f .AddBoolean(121, "nested-boolean") - .AddNumber(122, "nested-number")) + .AddNumber(122, "nested-number") + .AddNumber(123, "nested_number")) .ConfigureScripts(new SchemaScripts { Query = "" }) .Publish(); @@ -111,6 +114,9 @@ namespace Squidex.Domain.Apps.Entities.Contents.GraphQL .AddField("my-number", new ContentFieldData() .AddValue("iv", 1.0)) + .AddField("my_number", + new ContentFieldData() + .AddValue("iv", 2.0)) .AddField("my-boolean", new ContentFieldData() .AddValue("iv", true)) @@ -137,10 +143,12 @@ namespace Squidex.Domain.Apps.Entities.Contents.GraphQL .AddValue("iv", JsonValue.Array( JsonValue.Object() .Add("nested-boolean", true) - .Add("nested-number", 1), + .Add("nested-number", 10) + .Add("nested_number", 11), JsonValue.Object() .Add("nested-boolean", false) - .Add("nested-number", 2)))); + .Add("nested-number", 20) + .Add("nested_number", 21)))); var content = new ContentEntity {