From 880e8bf92dee27f01a26f001cf23ea6eb16bd017 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Hlad=C3=ADk?= Date: Fri, 21 Jun 2019 19:50:08 +0200 Subject: [PATCH 1/6] Bad field name in GraphQL schema description (#370) * change nested field name source * nested field unique name * QueryGraphTypeVisitor property --- .../Contents/GraphQL/GraphQLModel.cs | 4 ++-- .../Contents/GraphQL/IGraphModel.cs | 2 +- .../Contents/GraphQL/Types/ContentDataGraphType.cs | 2 +- .../Contents/GraphQL/Types/NestedGraphType.cs | 10 +++++----- .../Contents/GraphQL/Types/QueryGraphTypeVisitor.cs | 6 ++++-- 5 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/GraphQLModel.cs b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/GraphQLModel.cs index 1373ee4379..9cb3dc67e7 100644 --- a/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/GraphQLModel.cs +++ b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/GraphQLModel.cs @@ -136,9 +136,9 @@ public IFieldPartitioning ResolvePartition(Partitioning key) return partitionResolver(key); } - public (IGraphType ResolveType, ValueResolver Resolver) GetGraphType(ISchemaEntity schema, IField field) + public (IGraphType ResolveType, ValueResolver Resolver) GetGraphType(ISchemaEntity schema, IField field, string fieldName) { - return field.Accept(new QueryGraphTypeVisitor(schema, GetContentType, this, assetListType)); + return field.Accept(new QueryGraphTypeVisitor(schema, GetContentType, this, assetListType, fieldName)); } public IGraphType GetAssetType() diff --git a/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/IGraphModel.cs b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/IGraphModel.cs index f44c36b585..1ee303d6aa 100644 --- a/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/IGraphModel.cs +++ b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/IGraphModel.cs @@ -35,6 +35,6 @@ public interface IGraphModel IGraphType GetContentDataType(Guid schemaId); - (IGraphType ResolveType, ValueResolver Resolver) GetGraphType(ISchemaEntity schema, IField field); + (IGraphType ResolveType, ValueResolver Resolver) GetGraphType(ISchemaEntity schema, IField field, string fieldName); } } 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 7dfd1480bc..eb6ef19f5e 100644 --- a/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/ContentDataGraphType.cs +++ b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/ContentDataGraphType.cs @@ -27,7 +27,7 @@ public void Initialize(IGraphModel model, ISchemaEntity schema) foreach (var (field, fieldName, typeName) in schema.SchemaDef.Fields.SafeFields()) { - var (resolvedType, valueResolver) = model.GetGraphType(schema, field); + var (resolvedType, valueResolver) = model.GetGraphType(schema, field, fieldName); if (valueResolver != null) { 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 7f64227bbe..2964ed2010 100644 --- a/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/NestedGraphType.cs +++ b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/NestedGraphType.cs @@ -16,18 +16,18 @@ namespace Squidex.Domain.Apps.Entities.Contents.GraphQL.Types { public sealed class NestedGraphType : ObjectGraphType { - public NestedGraphType(IGraphModel model, ISchemaEntity schema, IArrayField field) + public NestedGraphType(IGraphModel model, ISchemaEntity schema, IArrayField field, string fieldName) { var schemaType = schema.TypeName(); var schemaName = schema.DisplayName(); - var fieldName = field.DisplayName(); + var fieldDisplayName = field.DisplayName(); Name = $"{schemaType}{fieldName}ChildDto"; foreach (var (nestedField, nestedName, _) in field.Fields.SafeFields()) { - var fieldInfo = model.GetGraphType(schema, nestedField); + var fieldInfo = model.GetGraphType(schema, nestedField, nestedName); if (fieldInfo.ResolveType != null) { @@ -38,12 +38,12 @@ public NestedGraphType(IGraphModel model, ISchemaEntity schema, IArrayField fiel Name = nestedName, Resolver = resolver, ResolvedType = fieldInfo.ResolveType, - Description = $"The {fieldName}/{nestedField.DisplayName()} nested field." + Description = $"The {fieldDisplayName}/{nestedField.DisplayName()} nested field." }); } } - Description = $"The structure of the {schemaName}.{fieldName} nested schema."; + Description = $"The structure of the {schemaName}.{fieldDisplayName} nested schema."; } private static FuncFieldResolver ValueResolver(NestedField nestedField, (IGraphType ResolveType, ValueResolver Resolver) fieldInfo) diff --git a/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/QueryGraphTypeVisitor.cs b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/QueryGraphTypeVisitor.cs index 32a9a308f6..195eee1a7f 100644 --- a/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/QueryGraphTypeVisitor.cs +++ b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/QueryGraphTypeVisitor.cs @@ -22,13 +22,15 @@ public sealed class QueryGraphTypeVisitor : IFieldVisitor<(IGraphType ResolveTyp private readonly Func schemaResolver; private readonly IGraphModel model; private readonly IGraphType assetListType; + private readonly string fieldName; - public QueryGraphTypeVisitor(ISchemaEntity schema, Func schemaResolver, IGraphModel model, IGraphType assetListType) + public QueryGraphTypeVisitor(ISchemaEntity schema, Func schemaResolver, IGraphModel model, IGraphType assetListType, string fieldName) { this.model = model; this.assetListType = assetListType; this.schema = schema; this.schemaResolver = schemaResolver; + this.fieldName = fieldName; } public (IGraphType ResolveType, ValueResolver Resolver) Visit(IArrayField field) @@ -93,7 +95,7 @@ private static (IGraphType ResolveType, ValueResolver Resolver) ResolveDefault(I private (IGraphType ResolveType, ValueResolver Resolver) ResolveNested(IArrayField field) { - var schemaFieldType = new ListGraphType(new NonNullGraphType(new NestedGraphType(model, schema, field))); + var schemaFieldType = new ListGraphType(new NonNullGraphType(new NestedGraphType(model, schema, field, this.fieldName))); return (schemaFieldType, NoopResolver); } From 4b7fa30f4aff2d9b8214e4862f89b885f5645bb5 Mon Sep 17 00:00:00 2001 From: Sebastian Date: Fri, 21 Jun 2019 20:05:00 +0200 Subject: [PATCH 2/6] Fix for scheduling. --- .../Contents/ScheduleJob.cs | 11 ++++++----- .../Api/Controllers/Contents/Models/ScheduleJobDto.cs | 10 ++++++---- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/Squidex.Domain.Apps.Entities/Contents/ScheduleJob.cs b/src/Squidex.Domain.Apps.Entities/Contents/ScheduleJob.cs index 9936e98eff..4145dd9544 100644 --- a/src/Squidex.Domain.Apps.Entities/Contents/ScheduleJob.cs +++ b/src/Squidex.Domain.Apps.Entities/Contents/ScheduleJob.cs @@ -1,4 +1,5 @@ -// ========================================================================== + +// ========================================================================== // Squidex Headless CMS // ========================================================================== // Copyright (c) Squidex UG (haftungsbeschraenkt) @@ -22,17 +23,17 @@ public sealed class ScheduleJob public Instant DueTime { get; } - public ScheduleJob(Guid id, Status status, RefToken scheduledBy, Instant due) + public ScheduleJob(Guid id, Status status, RefToken scheduledBy, Instant dueTime) { Id = id; ScheduledBy = scheduledBy; Status = status; - DueTime = due; + DueTime = dueTime; } - public static ScheduleJob Build(Status status, RefToken by, Instant due) + public static ScheduleJob Build(Status status, RefToken scheduledBy, Instant dueTime) { - return new ScheduleJob(Guid.NewGuid(), status, by, due); + return new ScheduleJob(Guid.NewGuid(), status, scheduledBy, dueTime); } } } diff --git a/src/Squidex/Areas/Api/Controllers/Contents/Models/ScheduleJobDto.cs b/src/Squidex/Areas/Api/Controllers/Contents/Models/ScheduleJobDto.cs index b0e7b34cbb..ef7ebda7bf 100644 --- a/src/Squidex/Areas/Api/Controllers/Contents/Models/ScheduleJobDto.cs +++ b/src/Squidex/Areas/Api/Controllers/Contents/Models/ScheduleJobDto.cs @@ -6,6 +6,7 @@ // ========================================================================== using System; +using System.ComponentModel.DataAnnotations; using NodaTime; using Squidex.Domain.Apps.Core.Contents; using Squidex.Infrastructure; @@ -25,13 +26,14 @@ public sealed class ScheduleJobDto public Status Status { get; set; } /// - /// The user who schedule the content. + /// The target date and time when the content should be scheduled. /// - public RefToken ScheduledBy { get; set; } + public Instant DueTime { get; set; } /// - /// The target date and time when the content should be scheduled. + /// The user who schedule the content. /// - public Instant DueTime { get; set; } + [Required] + public RefToken ScheduledBy { get; set; } } } From c1bc9e7edaf22131217e4adbb7c265dcc0075ef9 Mon Sep 17 00:00:00 2001 From: Sebastian Date: Fri, 21 Jun 2019 22:01:55 +0200 Subject: [PATCH 3/6] DataLoader implemented. --- .../Contents/GraphQL/CachingGraphQLService.cs | 46 ++++------- .../GraphQL/GraphQLExecutionContext.cs | 78 ++++++++++++++++--- .../Contents/GraphQL/GraphQLModel.cs | 18 ++--- .../Contents/ScheduleJob.cs | 3 +- src/Squidex/Config/Domain/EntitiesServices.cs | 14 ++++ .../Contents/GraphQL/GraphQLTestBase.cs | 41 +++++++--- 6 files changed, 137 insertions(+), 63 deletions(-) diff --git a/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/CachingGraphQLService.cs b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/CachingGraphQLService.cs index e56e28cb38..1a01b229a4 100644 --- a/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/CachingGraphQLService.cs +++ b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/CachingGraphQLService.cs @@ -8,43 +8,25 @@ using System; using System.Linq; using System.Threading.Tasks; +using GraphQL; using Microsoft.Extensions.Caching.Memory; using Squidex.Domain.Apps.Entities.Apps; using Squidex.Domain.Apps.Entities.Assets; using Squidex.Infrastructure; -using Squidex.Infrastructure.Log; namespace Squidex.Domain.Apps.Entities.Contents.GraphQL { public sealed class CachingGraphQLService : CachingProviderBase, IGraphQLService { private static readonly TimeSpan CacheDuration = TimeSpan.FromMinutes(10); - private readonly IContentQueryService contentQuery; - private readonly IGraphQLUrlGenerator urlGenerator; - private readonly ISemanticLog log; - private readonly IAssetQueryService assetQuery; - private readonly IAppProvider appProvider; - - public CachingGraphQLService( - IMemoryCache cache, - IAppProvider appProvider, - IAssetQueryService assetQuery, - IContentQueryService contentQuery, - IGraphQLUrlGenerator urlGenerator, - ISemanticLog log) + private readonly IDependencyResolver resolver; + + public CachingGraphQLService(IMemoryCache cache, IDependencyResolver resolver) : base(cache) { - Guard.NotNull(appProvider, nameof(appProvider)); - Guard.NotNull(assetQuery, nameof(assetQuery)); - Guard.NotNull(contentQuery, nameof(contentQuery)); - Guard.NotNull(urlGenerator, nameof(urlGenerator)); - Guard.NotNull(log, nameof(log)); - - this.appProvider = appProvider; - this.assetQuery = assetQuery; - this.contentQuery = contentQuery; - this.urlGenerator = urlGenerator; - this.log = log; + Guard.NotNull(resolver, nameof(resolver)); + + this.resolver = resolver; } public async Task<(bool HasError, object Response)> QueryAsync(QueryContext context, params GraphQLQuery[] queries) @@ -54,7 +36,7 @@ public CachingGraphQLService( var model = await GetModelAsync(context.App); - var ctx = new GraphQLExecutionContext(context, assetQuery, contentQuery, urlGenerator); + var ctx = new GraphQLExecutionContext(context, resolver); var result = await Task.WhenAll(queries.Select(q => QueryInternalAsync(model, ctx, q))); @@ -68,7 +50,7 @@ public CachingGraphQLService( var model = await GetModelAsync(context.App); - var ctx = new GraphQLExecutionContext(context, assetQuery, contentQuery, urlGenerator); + var ctx = new GraphQLExecutionContext(context, resolver); var result = await QueryInternalAsync(model, ctx, query); @@ -82,7 +64,7 @@ public CachingGraphQLService( return (false, new { data = new object() }); } - var result = await model.ExecuteAsync(ctx, query, log); + var result = await model.ExecuteAsync(ctx, query); if (result.Errors?.Any() == true) { @@ -102,9 +84,13 @@ private Task GetModelAsync(IAppEntity app) { entry.AbsoluteExpirationRelativeToNow = CacheDuration; - var allSchemas = await appProvider.GetSchemasAsync(app.Id); + var allSchemas = await resolver.Resolve().GetSchemasAsync(app.Id); - return new GraphQLModel(app, allSchemas, contentQuery.DefaultPageSizeGraphQl, assetQuery.DefaultPageSizeGraphQl, urlGenerator); + return new GraphQLModel(app, + allSchemas, + resolver.Resolve().DefaultPageSizeGraphQl, + resolver.Resolve().DefaultPageSizeGraphQl, + resolver.Resolve()); }); } diff --git a/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/GraphQLExecutionContext.cs b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/GraphQLExecutionContext.cs index 54a2793ab2..c2587b7474 100644 --- a/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/GraphQLExecutionContext.cs +++ b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/GraphQLExecutionContext.cs @@ -7,37 +7,93 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; +using GraphQL; +using GraphQL.DataLoader; using Squidex.Domain.Apps.Entities.Assets; using Squidex.Infrastructure.Json.Objects; +using Squidex.Infrastructure.Log; namespace Squidex.Domain.Apps.Entities.Contents.GraphQL { public sealed class GraphQLExecutionContext : QueryExecutionContext { + private static readonly List EmptyAssets = new List(); + private static readonly List EmptyContents = new List(); + private readonly IDataLoaderContextAccessor dataLoaderContextAccessor; + private readonly IDependencyResolver resolver; + public IGraphQLUrlGenerator UrlGenerator { get; } - public GraphQLExecutionContext(QueryContext context, - IAssetQueryService assetQuery, - IContentQueryService contentQuery, - IGraphQLUrlGenerator urlGenerator) - : base(context, assetQuery, contentQuery) + public ISemanticLog Log { get; } + + public GraphQLExecutionContext(QueryContext context, IDependencyResolver resolver) + : base(context, + resolver.Resolve(), + resolver.Resolve()) + { + UrlGenerator = resolver.Resolve(); + + dataLoaderContextAccessor = resolver.Resolve(); + + this.resolver = resolver; + } + + public void Setup(ExecutionOptions execution) { - UrlGenerator = urlGenerator; + var loader = resolver.Resolve(); + + execution.Listeners.Add(loader); + execution.FieldMiddleware.Use(LoggingMiddleware.Create(resolver.Resolve())); + + execution.UserContext = this; } - public Task> GetReferencedAssetsAsync(IJsonValue value) + public async Task> GetReferencedAssetsAsync(IJsonValue value) { var ids = ParseIds(value); - return GetReferencedAssetsAsync(ids); + if (ids == null) + { + return EmptyAssets; + } + + var dataLoader = + dataLoaderContextAccessor.Context.GetOrAddBatchLoader("Assets", + async batch => + { + var result = await GetReferencedAssetsAsync(new List(batch)); + + return result.ToDictionary(x => x.Id); + }); + + var assets = await Task.WhenAll(ids.Select(x => dataLoader.LoadAsync(x))); + + return assets.Where(x => x != null).ToList(); } - public Task> GetReferencedContentsAsync(Guid schemaId, IJsonValue value) + public async Task> GetReferencedContentsAsync(Guid schemaId, IJsonValue value) { var ids = ParseIds(value); - return GetReferencedContentsAsync(schemaId, ids); + if (ids == null) + { + return EmptyContents; + } + + var dataLoader = + dataLoaderContextAccessor.Context.GetOrAddBatchLoader($"Schema_{schemaId}", + async batch => + { + var result = await GetReferencedContentsAsync(schemaId, new List(batch)); + + return result.ToDictionary(x => x.Id); + }); + + var contents = await Task.WhenAll(ids.Select(x => dataLoader.LoadAsync(x))); + + return contents.Where(x => x != null).ToList(); } private static ICollection ParseIds(IJsonValue value) @@ -58,7 +114,7 @@ private static ICollection ParseIds(IJsonValue value) } catch { - return new List(); + return null; } } } diff --git a/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/GraphQLModel.cs b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/GraphQLModel.cs index 9cb3dc67e7..5f4ee3be38 100644 --- a/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/GraphQLModel.cs +++ b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/GraphQLModel.cs @@ -20,7 +20,6 @@ using Squidex.Domain.Apps.Entities.Contents.GraphQL.Types.Utils; using Squidex.Domain.Apps.Entities.Schemas; using Squidex.Infrastructure; -using Squidex.Infrastructure.Log; using GraphQLSchema = GraphQL.Types.Schema; #pragma warning disable IDE0003 @@ -172,20 +171,17 @@ public IGraphType GetContentType(Guid schemaId) return contentTypes.GetOrAdd(schema, s => new ContentGraphType()); } - public async Task<(object Data, object[] Errors)> ExecuteAsync(GraphQLExecutionContext context, GraphQLQuery query, ISemanticLog log) + public async Task<(object Data, object[] Errors)> ExecuteAsync(GraphQLExecutionContext context, GraphQLQuery query) { Guard.NotNull(context, nameof(context)); - var inputs = query.Variables?.ToInputs(); - - var result = await new DocumentExecuter().ExecuteAsync(options => + var result = await new DocumentExecuter().ExecuteAsync(execution => { - options.FieldMiddleware.Use(LoggingMiddleware.Create(log)); - options.OperationName = query.OperationName; - options.UserContext = context; - options.Schema = graphQLSchema; - options.Inputs = inputs; - options.Query = query.Query; + context.Setup(execution); + + execution.Schema = graphQLSchema; + execution.Inputs = query.Variables?.ToInputs(); + execution.Query = query.Query; }).ConfigureAwait(false); return (result.Data, result.Errors?.Select(x => (object)new { x.Message, x.Locations }).ToArray()); diff --git a/src/Squidex.Domain.Apps.Entities/Contents/ScheduleJob.cs b/src/Squidex.Domain.Apps.Entities/Contents/ScheduleJob.cs index 4145dd9544..fe48b89e41 100644 --- a/src/Squidex.Domain.Apps.Entities/Contents/ScheduleJob.cs +++ b/src/Squidex.Domain.Apps.Entities/Contents/ScheduleJob.cs @@ -1,5 +1,4 @@ - -// ========================================================================== +// ========================================================================== // Squidex Headless CMS // ========================================================================== // Copyright (c) Squidex UG (haftungsbeschraenkt) diff --git a/src/Squidex/Config/Domain/EntitiesServices.cs b/src/Squidex/Config/Domain/EntitiesServices.cs index 1b378c1732..99092c7452 100644 --- a/src/Squidex/Config/Domain/EntitiesServices.cs +++ b/src/Squidex/Config/Domain/EntitiesServices.cs @@ -6,6 +6,8 @@ // ========================================================================== using System; +using GraphQL; +using GraphQL.DataLoader; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; @@ -74,6 +76,18 @@ public static void AddMyEntitiesServices(this IServiceCollection services, IConf services.AddSingletonAs() .As().As(); + services.AddSingletonAs(x => new FuncDependencyResolver(t => x.GetRequiredService(t))) + .As(); + + services.AddSingletonAs() + .As(); + + services.AddSingletonAs() + .AsSelf(); + + services.AddSingletonAs() + .As(); + services.AddSingletonAs() .As(); 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 53e1554495..7012ae1fd7 100644 --- a/tests/Squidex.Domain.Apps.Entities.Tests/Contents/GraphQL/GraphQLTestBase.cs +++ b/tests/Squidex.Domain.Apps.Entities.Tests/Contents/GraphQL/GraphQLTestBase.cs @@ -9,6 +9,8 @@ using System.Collections.Generic; using System.Security.Claims; using FakeItEasy; +using GraphQL; +using GraphQL.DataLoader; using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Options; using Newtonsoft.Json; @@ -34,7 +36,6 @@ namespace Squidex.Domain.Apps.Entities.Contents.GraphQL { public class GraphQLTestBase { - protected readonly Schema schemaDef; protected readonly Guid schemaId = Guid.NewGuid(); protected readonly Guid appId = Guid.NewGuid(); protected readonly string appName = "my-app"; @@ -42,8 +43,7 @@ public class GraphQLTestBase protected readonly IAssetQueryService assetQuery = A.Fake(); protected readonly ISchemaEntity schema = A.Fake(); protected readonly IJsonSerializer serializer = TestUtils.CreateSerializer(TypeNameHandling.None); - protected readonly IMemoryCache cache = new MemoryCache(Options.Create(new MemoryCacheOptions())); - protected readonly IAppProvider appProvider = A.Fake(); + protected readonly IDependencyResolver dependencyResolver; protected readonly IAppEntity app = A.Dummy(); protected readonly QueryContext context; protected readonly ClaimsPrincipal user = new ClaimsPrincipal(); @@ -51,7 +51,7 @@ public class GraphQLTestBase public GraphQLTestBase() { - schemaDef = + var schemaDef = new Schema("my-schema") .AddJson(1, "my-json", Partitioning.Invariant, new JsonFieldProperties()) @@ -93,11 +93,7 @@ public GraphQLTestBase() A.CallTo(() => schema.Id).Returns(schemaId); A.CallTo(() => schema.SchemaDef).Returns(schemaDef); - var allSchemas = new List { schema }; - - A.CallTo(() => appProvider.GetSchemasAsync(appId)).Returns(allSchemas); - - sut = new CachingGraphQLService(cache, appProvider, assetQuery, contentQuery, new FakeUrlGenerator(), A.Fake()); + sut = CreateSut(); } protected static IContentEntity CreateContent(Guid id, Guid refId, Guid assetId, NamedContentData data = null, NamedContentData dataDraft = null) @@ -210,5 +206,32 @@ private string Serialize((bool HasErrors, object Response) result) { return serializer.Serialize(result); } + + private CachingGraphQLService CreateSut() + { + var appProvider = A.Fake(); + + A.CallTo(() => appProvider.GetSchemasAsync(appId)) + .Returns(new List { schema }); + + var dataLoaderContext = new DataLoaderContextAccessor(); + + var services = new Dictionary + { + [typeof(IAppProvider)] = appProvider, + [typeof(IAssetQueryService)] = assetQuery, + [typeof(IContentQueryService)] = contentQuery, + [typeof(IDataLoaderContextAccessor)] = dataLoaderContext, + [typeof(IGraphQLUrlGenerator)] = new FakeUrlGenerator(), + [typeof(ISemanticLog)] = A.Fake(), + [typeof(DataLoaderDocumentListener)] = new DataLoaderDocumentListener(dataLoaderContext) + }; + + var resolver = new FuncDependencyResolver(t => services[t]); + + var cache = new MemoryCache(Options.Create(new MemoryCacheOptions())); + + return new CachingGraphQLService(cache, resolver); + } } } From 0df169ea3430d40009fff50a6b7df383ce2c1786 Mon Sep 17 00:00:00 2001 From: Sebastian Date: Fri, 21 Jun 2019 22:23:02 +0200 Subject: [PATCH 4/6] Just some formatting. --- .../Contents/GraphQL/GraphQLExecutionContext.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/GraphQLExecutionContext.cs b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/GraphQLExecutionContext.cs index c2587b7474..aeb62077ba 100644 --- a/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/GraphQLExecutionContext.cs +++ b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/GraphQLExecutionContext.cs @@ -44,8 +44,10 @@ public void Setup(ExecutionOptions execution) { var loader = resolver.Resolve(); + var logger = LoggingMiddleware.Create(resolver.Resolve()); + execution.Listeners.Add(loader); - execution.FieldMiddleware.Use(LoggingMiddleware.Create(resolver.Resolve())); + execution.FieldMiddleware.Use(logger); execution.UserContext = this; } From af6c3808a9c99d9dec57800f5474ee2d9ea7ebb6 Mon Sep 17 00:00:00 2001 From: Sebastian Date: Fri, 21 Jun 2019 23:08:01 +0200 Subject: [PATCH 5/6] More optimizations. --- .../Assets/MongoAssetRepository.cs | 7 +-- .../Contents/MongoContentCollection.cs | 9 +-- .../GraphQL/GraphQLExecutionContext.cs | 59 ++++++++++++------- .../Contents/GraphQL/Types/Extensions.cs | 9 +++ .../Contents/QueryExecutionContext.cs | 4 +- src/Squidex.Domain.Apps.Entities/Q.cs | 5 ++ .../Contents/GraphQL/GraphQLQueriesTests.cs | 48 ++++++++------- 7 files changed, 87 insertions(+), 54 deletions(-) diff --git a/src/Squidex.Domain.Apps.Entities.MongoDb/Assets/MongoAssetRepository.cs b/src/Squidex.Domain.Apps.Entities.MongoDb/Assets/MongoAssetRepository.cs index 44d93d8d49..6444ba6384 100644 --- a/src/Squidex.Domain.Apps.Entities.MongoDb/Assets/MongoAssetRepository.cs +++ b/src/Squidex.Domain.Apps.Entities.MongoDb/Assets/MongoAssetRepository.cs @@ -119,12 +119,9 @@ public async Task> QueryAsync(Guid appId, HashSet ids.Contains(x.Id)).SortByDescending(x => x.LastModified); - var assetItems = find.ToListAsync(); - var assetCount = find.CountDocumentsAsync(); + var assetItems = await find.ToListAsync(); - await Task.WhenAll(assetItems, assetCount); - - return ResultList.Create(assetCount.Result, assetItems.Result.OfType()); + return ResultList.Create(assetItems.Count, assetItems.OfType()); } } diff --git a/src/Squidex.Domain.Apps.Entities.MongoDb/Contents/MongoContentCollection.cs b/src/Squidex.Domain.Apps.Entities.MongoDb/Contents/MongoContentCollection.cs index 1bb4c61283..9424333c0a 100644 --- a/src/Squidex.Domain.Apps.Entities.MongoDb/Contents/MongoContentCollection.cs +++ b/src/Squidex.Domain.Apps.Entities.MongoDb/Contents/MongoContentCollection.cs @@ -137,17 +137,14 @@ public async Task> QueryAsync(ISchemaEntity schema, { var find = Collection.Find(FilterFactory.IdsBySchema(schema.Id, ids, status)); - var contentItems = find.WithoutDraft(includeDraft).ToListAsync(); - var contentCount = find.CountDocumentsAsync(); - - await Task.WhenAll(contentItems, contentCount); + var contentItems = await find.WithoutDraft(includeDraft).ToListAsync(); - foreach (var entity in contentItems.Result) + foreach (var entity in contentItems) { entity.ParseData(schema.SchemaDef, serializer); } - return ResultList.Create(contentCount.Result, contentItems.Result); + return ResultList.Create(contentItems.Count, contentItems); } public async Task FindContentAsync(ISchemaEntity schema, Guid id, Status[] status, bool includeDraft) diff --git a/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/GraphQLExecutionContext.cs b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/GraphQLExecutionContext.cs index aeb62077ba..c5dbf60243 100644 --- a/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/GraphQLExecutionContext.cs +++ b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/GraphQLExecutionContext.cs @@ -12,6 +12,7 @@ using GraphQL; using GraphQL.DataLoader; using Squidex.Domain.Apps.Entities.Assets; +using Squidex.Domain.Apps.Entities.Contents.GraphQL.Types; using Squidex.Infrastructure.Json.Objects; using Squidex.Infrastructure.Log; @@ -52,6 +53,20 @@ public void Setup(ExecutionOptions execution) execution.UserContext = this; } + public override Task FindAssetAsync(Guid id) + { + var dataLoader = GetAssetsLoader(); + + return dataLoader.LoadAsync(id); + } + + public override Task FindContentAsync(Guid schemaId, Guid id) + { + var dataLoader = GetContentsLoader(schemaId); + + return dataLoader.LoadAsync(id); + } + public async Task> GetReferencedAssetsAsync(IJsonValue value) { var ids = ParseIds(value); @@ -61,18 +76,9 @@ public async Task> GetReferencedAssetsAsync(IJsonVal return EmptyAssets; } - var dataLoader = - dataLoaderContextAccessor.Context.GetOrAddBatchLoader("Assets", - async batch => - { - var result = await GetReferencedAssetsAsync(new List(batch)); - - return result.ToDictionary(x => x.Id); - }); + var dataLoader = GetAssetsLoader(); - var assets = await Task.WhenAll(ids.Select(x => dataLoader.LoadAsync(x))); - - return assets.Where(x => x != null).ToList(); + return await dataLoader.LoadManyAsync(ids); } public async Task> GetReferencedContentsAsync(Guid schemaId, IJsonValue value) @@ -84,18 +90,31 @@ public async Task> GetReferencedContentsAsync(Guid return EmptyContents; } - var dataLoader = - dataLoaderContextAccessor.Context.GetOrAddBatchLoader($"Schema_{schemaId}", - async batch => - { - var result = await GetReferencedContentsAsync(schemaId, new List(batch)); + var dataLoader = GetContentsLoader(schemaId); + + return await dataLoader.LoadManyAsync(ids); + } + + private IDataLoader GetAssetsLoader() + { + return dataLoaderContextAccessor.Context.GetOrAddBatchLoader("Assets", + async batch => + { + var result = await GetReferencedAssetsAsync(new List(batch)); - return result.ToDictionary(x => x.Id); - }); + return result.ToDictionary(x => x.Id); + }); + } - var contents = await Task.WhenAll(ids.Select(x => dataLoader.LoadAsync(x))); + private IDataLoader GetContentsLoader(Guid schemaId) + { + return dataLoaderContextAccessor.Context.GetOrAddBatchLoader($"Schema_{schemaId}", + async batch => + { + var result = await GetReferencedContentsAsync(schemaId, new List(batch)); - return contents.Where(x => x != null).ToList(); + return result.ToDictionary(x => x.Id); + }); } private static ICollection ParseIds(IJsonValue value) diff --git a/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/Extensions.cs b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/Extensions.cs index 9531680b0c..5b78b27285 100644 --- a/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/Extensions.cs +++ b/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/Extensions.cs @@ -7,6 +7,8 @@ using System.Collections.Generic; using System.Linq; +using System.Threading.Tasks; +using GraphQL.DataLoader; using Squidex.Domain.Apps.Core.Schemas; using Squidex.Infrastructure; @@ -37,5 +39,12 @@ private static string SafeString(this string value, int index) return value; } + + public static async Task> LoadManyAsync(this IDataLoader dataLoader, ICollection keys) where T : class + { + var contents = await Task.WhenAll(keys.Select(x => dataLoader.LoadAsync(x))); + + return contents.Where(x => x != null).ToList(); + } } } diff --git a/src/Squidex.Domain.Apps.Entities/Contents/QueryExecutionContext.cs b/src/Squidex.Domain.Apps.Entities/Contents/QueryExecutionContext.cs index 666b7474d4..b1615f3a11 100644 --- a/src/Squidex.Domain.Apps.Entities/Contents/QueryExecutionContext.cs +++ b/src/Squidex.Domain.Apps.Entities/Contents/QueryExecutionContext.cs @@ -34,7 +34,7 @@ public QueryExecutionContext(QueryContext context, IAssetQueryService assetQuery this.context = context; } - public async Task FindAssetAsync(Guid id) + public virtual async Task FindAssetAsync(Guid id) { var asset = cachedAssets.GetOrDefault(id); @@ -51,7 +51,7 @@ public async Task FindAssetAsync(Guid id) return asset; } - public async Task FindContentAsync(Guid schemaId, Guid id) + public virtual async Task FindContentAsync(Guid schemaId, Guid id) { var content = cachedContents.GetOrDefault(id); diff --git a/src/Squidex.Domain.Apps.Entities/Q.cs b/src/Squidex.Domain.Apps.Entities/Q.cs index 8bd9b0f390..966bdd279d 100644 --- a/src/Squidex.Domain.Apps.Entities/Q.cs +++ b/src/Squidex.Domain.Apps.Entities/Q.cs @@ -25,6 +25,11 @@ public Q WithODataQuery(string odataQuery) return Clone(c => c.ODataQuery = odataQuery); } + public Q WithIds(params Guid[] ids) + { + return Clone(c => c.Ids = ids.ToList()); + } + public Q WithIds(IEnumerable ids) { return Clone(c => c.Ids = ids.ToList()); 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 09b5afd920..2e503ffdf0 100644 --- a/tests/Squidex.Domain.Apps.Entities.Tests/Contents/GraphQL/GraphQLQueriesTests.cs +++ b/tests/Squidex.Domain.Apps.Entities.Tests/Contents/GraphQL/GraphQLQueriesTests.cs @@ -212,8 +212,8 @@ public async Task Should_return_single_asset_when_finding_asset() } }".Replace("", assetId.ToString()); - A.CallTo(() => assetQuery.FindAssetAsync(MatchsAssetContext(), assetId)) - .Returns(asset); + A.CallTo(() => assetQuery.QueryAsync(MatchsAssetContext(), MatchId(assetId))) + .Returns(ResultList.Create(1, asset)); var result = await sut.QueryAsync(context, new GraphQLQuery { Query = query }); @@ -544,8 +544,8 @@ public async Task Should_return_single_content_with_duplicate_names() } }".Replace("", contentId.ToString()); - A.CallTo(() => contentQuery.FindContentAsync(MatchsContentContext(), schemaId.ToString(), contentId, EtagVersion.Any)) - .Returns(content); + A.CallTo(() => contentQuery.QueryAsync(MatchsContentContext(), schemaId.ToString(), MatchId(contentId))) + .Returns(ResultList.Create(1, content)); var result = await sut.QueryAsync(context, new GraphQLQuery { Query = query }); @@ -635,8 +635,8 @@ public async Task Should_return_single_content_when_finding_content() } }".Replace("", contentId.ToString()); - A.CallTo(() => contentQuery.FindContentAsync(MatchsContentContext(), schemaId.ToString(), contentId, EtagVersion.Any)) - .Returns(content); + A.CallTo(() => contentQuery.QueryAsync(MatchsContentContext(), schemaId.ToString(), MatchId(contentId))) + .Returns(ResultList.Create(1, content)); var result = await sut.QueryAsync(context, new GraphQLQuery { Query = query }); @@ -730,12 +730,12 @@ public async Task Should_also_fetch_referenced_contents_when_field_is_included_i } }".Replace("", contentId.ToString()); - A.CallTo(() => contentQuery.FindContentAsync(MatchsContentContext(), schemaId.ToString(), contentId, EtagVersion.Any)) - .Returns(content); - A.CallTo(() => contentQuery.QueryAsync(MatchsContentContext(), schemaId.ToString(), A.Ignored)) .Returns(ResultList.Create(0, contentRef)); + A.CallTo(() => contentQuery.QueryAsync(MatchsContentContext(), schemaId.ToString(), MatchId(contentId))) + .Returns(ResultList.Create(1, content)); + var result = await sut.QueryAsync(context, new GraphQLQuery { Query = query }); var expected = new @@ -788,8 +788,8 @@ public async Task Should_also_fetch_referenced_assets_when_field_is_included_in_ } }".Replace("", contentId.ToString()); - A.CallTo(() => contentQuery.FindContentAsync(MatchsContentContext(), schemaId.ToString(), contentId, EtagVersion.Any)) - .Returns(content); + A.CallTo(() => contentQuery.QueryAsync(MatchsContentContext(), schemaId.ToString(), MatchId(contentId))) + .Returns(ResultList.Create(1, content)); A.CallTo(() => assetQuery.QueryAsync(MatchsAssetContext(), A.Ignored)) .Returns(ResultList.Create(0, assetRef)); @@ -844,10 +844,11 @@ public async Task Should_make_multiple_queries() } }".Replace("", assetId2.ToString()); - A.CallTo(() => assetQuery.FindAssetAsync(MatchsAssetContext(), assetId1)) - .Returns(asset1); - A.CallTo(() => assetQuery.FindAssetAsync(MatchsAssetContext(), assetId2)) - .Returns(asset2); + A.CallTo(() => assetQuery.QueryAsync(MatchsAssetContext(), MatchId(assetId1))) + .Returns(ResultList.Create(0, asset1)); + + A.CallTo(() => assetQuery.QueryAsync(MatchsAssetContext(), MatchId(assetId2))) + .Returns(ResultList.Create(0, asset2)); var result = await sut.QueryAsync(context, new GraphQLQuery { Query = query1 }, new GraphQLQuery { Query = query2 }); @@ -902,8 +903,8 @@ public async Task Should_not_return_data_when_field_not_part_of_content() } }".Replace("", contentId.ToString()); - A.CallTo(() => contentQuery.FindContentAsync(MatchsContentContext(), schemaId.ToString(), contentId, EtagVersion.Any)) - .Returns(content); + A.CallTo(() => contentQuery.QueryAsync(MatchsContentContext(), schemaId.ToString(), MatchId(contentId))) + .Returns(ResultList.Create(1, content)); var result = await sut.QueryAsync(context, new GraphQLQuery { Query = query }); @@ -940,8 +941,8 @@ public async Task Should_return_draft_content_when_querying_dataDraft() } }".Replace("", contentId.ToString()); - A.CallTo(() => contentQuery.FindContentAsync(MatchsContentContext(), schemaId.ToString(), contentId, EtagVersion.Any)) - .Returns(content); + A.CallTo(() => contentQuery.QueryAsync(MatchsContentContext(), schemaId.ToString(), MatchId(contentId))) + .Returns(ResultList.Create(1, content)); var result = await sut.QueryAsync(context, new GraphQLQuery { Query = query }); @@ -986,8 +987,8 @@ public async Task Should_return_null_when_querying_dataDraft_and_no_draft_conten } }".Replace("", contentId.ToString()); - A.CallTo(() => contentQuery.FindContentAsync(MatchsContentContext(), schemaId.ToString(), contentId, EtagVersion.Any)) - .Returns(content); + A.CallTo(() => contentQuery.QueryAsync(MatchsContentContext(), schemaId.ToString(), MatchId(contentId))) + .Returns(ResultList.Create(1, content)); var result = await sut.QueryAsync(context, new GraphQLQuery { Query = query }); @@ -1005,6 +1006,11 @@ public async Task Should_return_null_when_querying_dataDraft_and_no_draft_conten AssertResult(expected, result); } + private static Q MatchId(Guid contentId) + { + return A.That.Matches(x => x.Ids.Count == 1 && x.Ids[0] == contentId); + } + private QueryContext MatchsAssetContext() { return A.That.Matches(x => x.App == app && x.User == user); From fc172c8bd92b83b9e2c677ee574907eac44c3bc7 Mon Sep 17 00:00:00 2001 From: Sebastian Date: Sat, 22 Jun 2019 08:38:18 +0200 Subject: [PATCH 6/6] Virtual everywhere. --- .../Contents/QueryExecutionContext.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Squidex.Domain.Apps.Entities/Contents/QueryExecutionContext.cs b/src/Squidex.Domain.Apps.Entities/Contents/QueryExecutionContext.cs index b1615f3a11..8b186cbd45 100644 --- a/src/Squidex.Domain.Apps.Entities/Contents/QueryExecutionContext.cs +++ b/src/Squidex.Domain.Apps.Entities/Contents/QueryExecutionContext.cs @@ -68,7 +68,7 @@ public virtual async Task FindContentAsync(Guid schemaId, Guid i return content; } - public async Task> QueryAssetsAsync(string query) + public virtual async Task> QueryAssetsAsync(string query) { var assets = await assetQuery.QueryAsync(context, Q.Empty.WithODataQuery(query)); @@ -80,7 +80,7 @@ public async Task> QueryAssetsAsync(string query) return assets; } - public async Task> QueryContentsAsync(string schemaIdOrName, string query) + public virtual async Task> QueryContentsAsync(string schemaIdOrName, string query) { var result = await contentQuery.QueryAsync(context, schemaIdOrName, Q.Empty.WithODataQuery(query)); @@ -92,7 +92,7 @@ public async Task> QueryContentsAsync(string schemaI return result; } - public async Task> GetReferencedAssetsAsync(ICollection ids) + public virtual async Task> GetReferencedAssetsAsync(ICollection ids) { Guard.NotNull(ids, nameof(ids)); @@ -111,7 +111,7 @@ public async Task> GetReferencedAssetsAsync(ICollect return ids.Select(cachedAssets.GetOrDefault).Where(x => x != null).ToList(); } - public async Task> GetReferencedContentsAsync(Guid schemaId, ICollection ids) + public virtual async Task> GetReferencedContentsAsync(Guid schemaId, ICollection ids) { Guard.NotNull(ids, nameof(ids));