From 3e4eb135d3d90818c31acc11fde678b7a572366a Mon Sep 17 00:00:00 2001 From: Sebastian Stehle Date: Sat, 5 Oct 2019 17:24:33 +0200 Subject: [PATCH] Performance improved. --- .../Contents/Queries/ContentEnricher.cs | 13 ++---- .../Contents/Models/ContentsDto.cs | 18 ++++---- .../shared/contents-selector.component.ts | 2 +- .../workflows/workflows-page.component.ts | 10 +--- .../shared/services/contents.service.spec.ts | 32 +++++++++++++ .../app/shared/state/schema-tag-converter.ts | 18 +++----- .../app/shared/state/schemas.state.spec.ts | 46 ++++++++++++++++++- src/Squidex/app/shared/state/schemas.state.ts | 46 +++++++++++++++---- .../Contents/Queries/ContentEnricherTests.cs | 14 ++++++ 9 files changed, 150 insertions(+), 49 deletions(-) diff --git a/src/Squidex.Domain.Apps.Entities/Contents/Queries/ContentEnricher.cs b/src/Squidex.Domain.Apps.Entities/Contents/Queries/ContentEnricher.cs index 7808fe52a..612539c41 100644 --- a/src/Squidex.Domain.Apps.Entities/Contents/Queries/ContentEnricher.cs +++ b/src/Squidex.Domain.Apps.Entities/Contents/Queries/ContentEnricher.cs @@ -69,15 +69,12 @@ namespace Squidex.Domain.Apps.Entities.Contents.Queries { var result = SimpleMapper.Map(content, new ContentEntity()); - if (ShouldEnrich(context)) - { - await ResolveColorAsync(content, result, cache); + await ResolveColorAsync(content, result, cache); - if (ShouldEnrichWithStatuses(context)) - { - await ResolveNextsAsync(content, result, context); - await ResolveCanUpdateAsync(content, result); - } + if (ShouldEnrichWithStatuses(context)) + { + await ResolveNextsAsync(content, result, context); + await ResolveCanUpdateAsync(content, result); } results.Add(result); diff --git a/src/Squidex/Areas/Api/Controllers/Contents/Models/ContentsDto.cs b/src/Squidex/Areas/Api/Controllers/Contents/Models/ContentsDto.cs index 73b9d976a..cd02ddaa7 100644 --- a/src/Squidex/Areas/Api/Controllers/Contents/Models/ContentsDto.cs +++ b/src/Squidex/Areas/Api/Controllers/Contents/Models/ContentsDto.cs @@ -36,7 +36,8 @@ namespace Squidex.Areas.Api.Controllers.Contents.Models [Required] public StatusInfoDto[] Statuses { get; set; } - public static async Task FromContentsAsync(IResultList contents, Context context, ApiController controller, ISchemaEntity schema, IContentWorkflow workflow) + public static async Task FromContentsAsync(IResultList contents, Context context, ApiController controller, + ISchemaEntity schema, IContentWorkflow workflow) { var result = new ContentsDto { @@ -63,18 +64,15 @@ namespace Squidex.Areas.Api.Controllers.Contents.Models private ContentsDto CreateLinks(ApiController controller, string app, string schema) { - if (schema != null) - { - var values = new { app, name = schema }; + var values = new { app, name = schema }; - AddSelfLink(controller.Url(x => nameof(x.GetContents), values)); + AddSelfLink(controller.Url(x => nameof(x.GetContents), values)); - if (controller.HasPermission(Permissions.AppContentsCreate, app, schema)) - { - AddPostLink("create", controller.Url(x => nameof(x.PostContent), values)); + if (controller.HasPermission(Permissions.AppContentsCreate, app, schema)) + { + AddPostLink("create", controller.Url(x => nameof(x.PostContent), values)); - AddPostLink("create/publish", controller.Url(x => nameof(x.PostContent), values) + "?publish=true"); - } + AddPostLink("create/publish", controller.Url(x => nameof(x.PostContent), values) + "?publish=true"); } return this; diff --git a/src/Squidex/app/features/content/shared/contents-selector.component.ts b/src/Squidex/app/features/content/shared/contents-selector.component.ts index b6025a183..59e01bdc4 100644 --- a/src/Squidex/app/features/content/shared/contents-selector.component.ts +++ b/src/Squidex/app/features/content/shared/contents-selector.component.ts @@ -90,7 +90,7 @@ export class ContentsSelectorComponent extends ResourceOwner implements OnInit { selected = selected.id; } - this.schemasState.loadSchema(selected) + this.schemasState.loadSchema(selected, true) .subscribe(schema => { if (schema) { this.schema = schema; diff --git a/src/Squidex/app/features/settings/pages/workflows/workflows-page.component.ts b/src/Squidex/app/features/settings/pages/workflows/workflows-page.component.ts index 6b38856fa..d20f67a2f 100644 --- a/src/Squidex/app/features/settings/pages/workflows/workflows-page.component.ts +++ b/src/Squidex/app/features/settings/pages/workflows/workflows-page.component.ts @@ -5,7 +5,7 @@ * Copyright (c) Squidex UG (haftungsbeschränkt). All rights reserved. */ -import { Component, OnDestroy, OnInit } from '@angular/core'; +import { Component, OnInit } from '@angular/core'; import { RolesState, @@ -19,7 +19,7 @@ import { styleUrls: ['./workflows-page.component.scss'], templateUrl: './workflows-page.component.html' }) -export class WorkflowsPageComponent implements OnInit, OnDestroy { +export class WorkflowsPageComponent implements OnInit { constructor( public readonly rolesState: RolesState, public readonly schemasSource: SchemaTagConverter, @@ -30,15 +30,9 @@ export class WorkflowsPageComponent implements OnInit, OnDestroy { public ngOnInit() { this.rolesState.load(); - this.schemasSource.load(); - this.workflowsState.load(); } - public ngOnDestroy() { - this.schemasSource.destroy(); - } - public reload() { this.workflowsState.load(true); } diff --git a/src/Squidex/app/shared/services/contents.service.spec.ts b/src/Squidex/app/shared/services/contents.service.spec.ts index 5e72f8f28..9720a1560 100644 --- a/src/Squidex/app/shared/services/contents.service.spec.ts +++ b/src/Squidex/app/shared/services/contents.service.spec.ts @@ -114,6 +114,38 @@ describe('ContentsService', () => { req.flush({ total: 10, items: [] }); })); + it('should make get request to get contents by ids', + inject([ContentsService, HttpTestingController], (contentsService: ContentsService, httpMock: HttpTestingController) => { + + let contents: ContentsDto; + + contentsService.getContentsByIds('my-app', ['1', '2', '3']).subscribe(result => { + contents = result; + }); + + const req = httpMock.expectOne(`http://service/p/api/content/my-app/?ids=1,2,3`); + + expect(req.request.method).toEqual('GET'); + expect(req.request.headers.get('If-Match')).toBeNull(); + + req.flush({ + total: 10, + items: [ + contentResponse(12), + contentResponse(13) + ], + statuses: [{ + status: 'Draft', color: 'Gray' + }] + }); + + expect(contents!).toEqual( + new ContentsDto([{ status: 'Draft', color: 'Gray' }], 10, [ + createContent(12), + createContent(13) + ])); + })); + it('should make get request to get content', inject([ContentsService, HttpTestingController], (contentsService: ContentsService, httpMock: HttpTestingController) => { diff --git a/src/Squidex/app/shared/state/schema-tag-converter.ts b/src/Squidex/app/shared/state/schema-tag-converter.ts index 75b8ae5f9..4fc78b061 100644 --- a/src/Squidex/app/shared/state/schema-tag-converter.ts +++ b/src/Squidex/app/shared/state/schema-tag-converter.ts @@ -5,7 +5,7 @@ * Copyright (c) Squidex UG (haftungsbeschränkt). All rights reserved. */ -import { Injectable } from '@angular/core'; +import { Injectable, OnDestroy } from '@angular/core'; import { Subscription } from 'rxjs'; import { Converter, TagValue } from '@app/framework'; @@ -14,7 +14,7 @@ import { SchemaDto } from './../services/schemas.service'; import { SchemasState } from './schemas.state'; @Injectable() -export class SchemaTagConverter implements Converter { +export class SchemaTagConverter implements Converter, OnDestroy { private schemasSubscription: Subscription; private schemas: SchemaDto[] = []; @@ -24,20 +24,16 @@ export class SchemaTagConverter implements Converter { readonly schemasState: SchemasState ) { this.schemasSubscription = - schemasState.changes.subscribe(state => { - if (state.isLoaded) { - this.schemas = state.schemas.values; + schemasState.schemas.subscribe(schemas => { + this.schemas = schemas.values; - this.suggestions = this.schemas.map(x => new TagValue(x.id, x.name, x.id)); - } + this.suggestions = this.schemas.map(x => new TagValue(x.id, x.name, x.id)); }); - } - public load() { - this.schemasState.load(); + this.schemasState.loadIfNotLoaded(); } - public destroy() { + public ngOnDestroy() { this.schemasSubscription.unsubscribe(); } diff --git a/src/Squidex/app/shared/state/schemas.state.spec.ts b/src/Squidex/app/shared/state/schemas.state.spec.ts index 8529366d9..323ff8964 100644 --- a/src/Squidex/app/shared/state/schemas.state.spec.ts +++ b/src/Squidex/app/shared/state/schemas.state.spec.ts @@ -112,6 +112,30 @@ describe('SchemasState', () => { dialogs.verify(x => x.notifyInfo(It.isAnyString()), Times.once()); }); + + it('should not load if already loaded', () => { + schemasService.setup(x => x.getSchemas(app)) + .returns(() => of(oldSchemas)).verifiable(); + + schemasState.load(true).subscribe(); + schemasState.loadIfNotLoaded().subscribe(); + + expect().nothing(); + + dialogs.verify(x => x.notifyInfo(It.isAnyString()), Times.once()); + }); + + it('should load if not loaded yet', () => { + schemasService.setup(x => x.getSchemas(app)) + .returns(() => of(oldSchemas)).verifiable(); + + schemasState.load(true).subscribe(); + schemasState.loadIfNotLoaded().subscribe(); + + expect().nothing(); + + dialogs.verify(x => x.notifyInfo(It.isAnyString()), Times.once()); + }); }); describe('Updates', () => { @@ -167,7 +191,7 @@ describe('SchemasState', () => { expect().nothing(); }); - it('should return schema on select and load when not loaded', () => { + it('should return schema on select and reload always', () => { schemasService.setup(x => x.getSchema(app, schema1.name)) .returns(() => of(schema)).verifiable(); @@ -182,6 +206,26 @@ describe('SchemasState', () => { expect(schemasState.snapshot.selectedSchema).toBe(schemasState.snapshot.schemas.at(0)); }); + it('should return schema on get and cache it', () => { + schemasService.setup(x => x.getSchema(app, schema1.name)) + .returns(() => of(schema)).verifiable(Times.exactly(1)); + + schemasState.loadSchema(schema1.name, true).subscribe(); + schemasState.loadSchema(schema1.name, true).subscribe(); + + expect().nothing(); + }); + + it('should return schema on get and reuse it from select when caching', () => { + schemasService.setup(x => x.getSchema(app, schema1.name)) + .returns(() => of(schema)).verifiable(Times.exactly(1)); + + schemasState.select(schema1.name).subscribe(); + schemasState.loadSchema(schema1.name, true).subscribe(); + + expect().nothing(); + }); + it('should return null on select when loading failed', () => { schemasService.setup(x => x.getSchema(app, 'failed')) .returns(() => throwError({})).verifiable(); diff --git a/src/Squidex/app/shared/state/schemas.state.ts b/src/Squidex/app/shared/state/schemas.state.ts index 384bb0cd2..c213fd676 100644 --- a/src/Squidex/app/shared/state/schemas.state.ts +++ b/src/Squidex/app/shared/state/schemas.state.ts @@ -6,7 +6,7 @@ */ import { Injectable } from '@angular/core'; -import { Observable, of } from 'rxjs'; +import { empty, Observable, of } from 'rxjs'; import { catchError, tap } from 'rxjs/operators'; import { @@ -103,26 +103,52 @@ export class SchemasState extends State { return this.loadSchema(idOrName).pipe( tap(selectedSchema => { this.next(s => { - const schemas = selectedSchema ? s.schemas.replaceBy('id', selectedSchema) : s.schemas; - - return { ...s, selectedSchema, schemas }; + return { ...s, selectedSchema }; }); })); } - public loadSchema(idOrName: string | null) { - return !idOrName ? of(null) : - this.schemasService.getSchema(this.appName, idOrName).pipe( - catchError(() => of(null))); + public loadSchema(idOrName: string | null, cached = false) { + if (!idOrName) { + return of(null); + } + + if (cached) { + const found = this.snapshot.schemas.find(x => x.id === idOrName || x.name === idOrName); + + if (Types.is(found, SchemaDetailsDto)) { + return of(found); + } + } + + return this.schemasService.getSchema(this.appName, idOrName).pipe( + tap(schema => { + this.next(s => { + const schemas = s.schemas.replaceBy('id', schema); + + return { ...s, schemas }; + }); + }), + catchError(() => of(null))); } public load(isReload = false): Observable { if (!isReload) { - const selectedSchema = this.snapshot.selectedSchema; + this.resetState({ selectedSchema: this.snapshot.selectedSchema }); + } - this.resetState({ selectedSchema }); + return this.loadInternal(isReload); + } + + public loadIfNotLoaded(): Observable { + if (this.snapshot.isLoaded) { + return empty(); } + return this.loadInternal(false); + } + + private loadInternal(isReload = false): Observable { return this.schemasService.getSchemas(this.appName).pipe( tap(({ items, canCreate }) => { if (isReload) { diff --git a/tests/Squidex.Domain.Apps.Entities.Tests/Contents/Queries/ContentEnricherTests.cs b/tests/Squidex.Domain.Apps.Entities.Tests/Contents/Queries/ContentEnricherTests.cs index b27b61ecf..dc1619916 100644 --- a/tests/Squidex.Domain.Apps.Entities.Tests/Contents/Queries/ContentEnricherTests.cs +++ b/tests/Squidex.Domain.Apps.Entities.Tests/Contents/Queries/ContentEnricherTests.cs @@ -54,6 +54,20 @@ namespace Squidex.Domain.Apps.Entities.Contents.Queries Assert.Contains(schema.Version, result.CacheDependencies); } + [Fact] + public async Task Should_enrich_with_schema_infos() + { + var source = new ContentEntity { Status = Status.Published, SchemaId = schemaId }; + + A.CallTo(() => contentWorkflow.GetInfoAsync(source)) + .Returns(new StatusInfo(Status.Published, StatusColors.Published)); + + var result = await sut.EnrichAsync(source, requestContext.WithResolveSchema()); + + Assert.Equal("my-schema", result.SchemaName); + Assert.Equal("my-schema", result.SchemaDisplayName); + } + [Fact] public async Task Should_enrich_content_with_status_color() {