Browse Source

Optimize total (#802)

* Cleanup.

* Small bugfix.

* Fix tests.
pull/806/head
Sebastian Stehle 4 years ago
committed by GitHub
parent
commit
9234fce68b
No known key found for this signature in database GPG Key ID: 4AEE18F83AFDEB23
  1. 66
      frontend/app/shared/services/assets.service.spec.ts
  2. 154
      frontend/app/shared/services/assets.service.ts
  3. 7
      frontend/app/shared/services/contents.service.spec.ts
  4. 83
      frontend/app/shared/services/contents.service.ts
  5. 13
      frontend/app/shared/state/assets.state.spec.ts
  6. 24
      frontend/app/shared/state/assets.state.ts
  7. 8
      frontend/app/shared/state/contents.state.ts
  8. 4
      frontend/app/shared/state/query.ts

66
frontend/app/shared/services/assets.service.spec.ts

@ -7,7 +7,7 @@
import { HttpClientTestingModule, HttpTestingController } from '@angular/common/http/testing';
import { inject, TestBed } from '@angular/core/testing';
import { AnalyticsService, ApiUrlConfig, AssetDto, AssetFolderDto, AssetFoldersDto, AssetsDto, AssetsService, DateTime, encodeQuery, ErrorDto, MathHelper, Resource, ResourceLinks, sanitize, Version } from '@app/shared/internal';
import { AnalyticsService, ApiUrlConfig, AssetDto, AssetFolderDto, AssetFoldersDto, AssetsDto, AssetsService, DateTime, ErrorDto, MathHelper, Resource, ResourceLinks, sanitize, Version } from '@app/shared/internal';
describe('AssetsService', () => {
const version = new Version('1');
@ -80,7 +80,7 @@ describe('AssetsService', () => {
});
}));
it('should make get request to get assets',
it('should make post request to get assets',
inject([AssetsService, HttpTestingController], (assetsService: AssetsService, httpMock: HttpTestingController) => {
let assets: AssetsDto;
@ -88,12 +88,14 @@ describe('AssetsService', () => {
assets = result;
});
const query = { take: 17, skip: 13 };
const expectedQuery = { take: 17, skip: 13 };
const req = httpMock.expectOne(`http://service/p/api/apps/my-app/assets?q=${encodeQuery(query)}`);
const req = httpMock.expectOne('http://service/p/api/apps/my-app/assets/query');
expect(req.request.method).toEqual('GET');
expect(req.request.method).toEqual('POST');
expect(req.request.headers.get('If-Match')).toBeNull();
expect(req.request.headers.get('X-NoTotal')).toBeNull();
expect(req.request.body).toEqual({ q: sanitize(expectedQuery) });
req.flush({
total: 10,
@ -116,10 +118,10 @@ describe('AssetsService', () => {
it('should make get request to get asset folders',
inject([AssetsService, HttpTestingController], (assetsService: AssetsService, httpMock: HttpTestingController) => {
let assets: AssetFoldersDto;
let assetFolders: AssetFoldersDto;
assetsService.getAssetFolders('my-app', 'parent1', 'Path').subscribe(result => {
assets = result;
assetFolders = result;
});
const req = httpMock.expectOne('http://service/p/api/apps/my-app/assets/folders?parentId=parent1&scope=Path');
@ -138,7 +140,7 @@ describe('AssetsService', () => {
],
});
expect(assets!).toEqual(
expect(assetFolders!).toEqual(
new AssetFoldersDto(10, [
createAssetFolder(22),
createAssetFolder(23),
@ -165,27 +167,11 @@ describe('AssetsService', () => {
expect(asset!).toEqual(createAsset(12));
}));
it('should make get request to get assets by name',
inject([AssetsService, HttpTestingController], (assetsService: AssetsService, httpMock: HttpTestingController) => {
const query = { fullText: 'my-query' };
assetsService.getAssets('my-app', { take: 17, skip: 13, query }).subscribe();
const expectedQuery = { filter: { and: [{ path: 'fileName', op: 'contains', value: 'my-query' }] }, take: 17, skip: 13 };
const req = httpMock.expectOne(`http://service/p/api/apps/my-app/assets?q=${encodeQuery(expectedQuery)}`);
expect(req.request.method).toEqual('GET');
expect(req.request.headers.get('If-Match')).toBeNull();
req.flush({ total: 10, items: [] });
}));
it('should make post request to get assets by name if request limit reached',
it('should make post request to get assets by name',
inject([AssetsService, HttpTestingController], (assetsService: AssetsService, httpMock: HttpTestingController) => {
const query = { fullText: 'my-query' };
assetsService.getAssets('my-app', { take: 17, skip: 13, query, maxLength: 5 }).subscribe();
assetsService.getAssets('my-app', { take: 17, skip: 13, query, noTotal: true }).subscribe();
const expectedQuery = { filter: { and: [{ path: 'fileName', op: 'contains', value: 'my-query' }] }, take: 17, skip: 13 };
@ -193,50 +179,42 @@ describe('AssetsService', () => {
expect(req.request.method).toEqual('POST');
expect(req.request.headers.get('If-Match')).toBeNull();
expect(req.request.headers.get('X-NoTotal')).toEqual('1');
expect(req.request.body).toEqual({ q: sanitize(expectedQuery) });
req.flush({ total: 10, items: [] });
}));
it('should make get request to get assets by tag',
it('should make post request to get assets by tag',
inject([AssetsService, HttpTestingController], (assetsService: AssetsService, httpMock: HttpTestingController) => {
assetsService.getAssets('my-app', { take: 17, skip: 13, tags: ['tag1'] }).subscribe();
const expectedQuery = { filter: { and: [{ path: 'tags', op: 'eq', value: 'tag1' }] }, take: 17, skip: 13 };
const req = httpMock.expectOne(`http://service/p/api/apps/my-app/assets?q=${encodeQuery(expectedQuery)}`);
expect(req.request.method).toEqual('GET');
expect(req.request.headers.get('If-Match')).toBeNull();
req.flush({ total: 10, items: [] });
}));
it('should make get request to get assets by tag if request limit reached',
inject([AssetsService, HttpTestingController], (assetsService: AssetsService, httpMock: HttpTestingController) => {
assetsService.getAssets('my-app', { take: 17, skip: 13, tags: ['tag1'], maxLength: 5 }).subscribe();
const expectedQuery = { filter: { and: [{ path: 'tags', op: 'eq', value: 'tag1' }] }, take: 17, skip: 13 };
const req = httpMock.expectOne('http://service/p/api/apps/my-app/assets/query');
expect(req.request.method).toEqual('POST');
expect(req.request.headers.get('If-Match')).toBeNull();
expect(req.request.headers.get('X-NoTotal')).toBeNull();
expect(req.request.body).toEqual({ q: sanitize(expectedQuery) });
req.flush({ total: 10, items: [] });
}));
it('should make get request to get assets by ids',
it('should make post request to get assets by ids',
inject([AssetsService, HttpTestingController], (assetsService: AssetsService, httpMock: HttpTestingController) => {
const ids = ['1', '2'];
assetsService.getAssets('my-app', { ids }).subscribe();
const req = httpMock.expectOne('http://service/p/api/apps/my-app/assets?ids=1,2');
const expectedBody = { ids };
expect(req.request.method).toEqual('GET');
const req = httpMock.expectOne('http://service/p/api/apps/my-app/assets/query');
expect(req.request.method).toEqual('POST');
expect(req.request.headers.get('If-Match')).toBeNull();
expect(req.request.headers.get('X-NoTotal')).toBeNull();
expect(req.request.body).toEqual(expectedBody);
req.flush({ total: 10, items: [] });
}));

154
frontend/app/shared/services/assets.service.ts

@ -10,7 +10,7 @@ import { Injectable } from '@angular/core';
import { AnalyticsService, ApiUrlConfig, DateTime, ErrorDto, getLinkUrl, hasAnyLink, HTTP, Metadata, pretifyError, Resource, ResourceLinks, ResultSet, StringHelper, Types, Version, Versioned } from '@app/framework';
import { Observable, throwError } from 'rxjs';
import { catchError, filter, map, tap } from 'rxjs/operators';
import { encodeQuery, Query } from './../state/query';
import { Query, sanitize } from './../state/query';
import { AuthService } from './auth.service';
const SVG_PREVIEW_LIMIT = 10 * 1024;
@ -160,8 +160,20 @@ export type RenameAssetTagDto =
export type MoveAssetItemDto =
Readonly<{ parentId?: string }>;
export type AssetQueryDto =
Readonly<{ ids?: Tags; maxLength?: number; parentId?: string; query?: Query; skip?: number; tags?: Tags; take?: number }>;
export type AssetsQuery =
Readonly<{ noTotal?: boolean; parentId?: string }>;
export type AssetsByIds =
Readonly<{ ids: ReadonlyArray<string> }> & AssetsQuery;
export type AssetsByQuery =
Readonly<{ query?: Query; skip?: number; tags?: Tags; take?: number; noTotal?: boolean }> & AssetsQuery;
type AssetsResponse =
Readonly<{ total: number; items: any[]; folders: any[] } & Resource>;
type AssetFolderResponse =
Readonly<{ total: number; items: any[]; folders: any[]; path: any[] } & Resource>;
@Injectable()
export class AssetsService {
@ -186,90 +198,34 @@ export class AssetsService {
pretifyError('i18n:assets.loadTagsFailed'));
}
public getAssets(appName: string, q?: AssetQueryDto): Observable<AssetsDto> {
const { ids, maxLength, parentId, query, skip, tags, take } = q || {};
let fullQuery: string;
let queryObj: Query | undefined;
if (ids && ids.length > 0) {
fullQuery = `ids=${ids.join(',')}`;
} else {
queryObj = {};
const filters: any[] = [];
public getAssets(appName: string, q?: AssetsByQuery | AssetsByIds): Observable<AssetsDto> {
const body = buildQuery(q as any);
if (query && query.fullText && query.fullText.length > 0) {
filters.push({ path: 'fileName', op: 'contains', value: query.fullText });
}
const url = this.apiUrl.buildUrl(`api/apps/${appName}/assets/query`);
if (tags) {
for (const tag of tags) {
if (tag && tag.length > 0) {
filters.push({ path: 'tags', op: 'eq', value: tag });
}
}
}
if (filters.length > 0) {
queryObj.filter = { and: filters };
}
let options = {};
if (take && take > 0) {
queryObj.take = take;
}
if (skip && skip > 0) {
queryObj.skip = skip;
}
fullQuery = `q=${encodeQuery(queryObj)}`;
if (parentId) {
fullQuery = StringHelper.appendToUrl(fullQuery, 'parentId', parentId, true);
}
if (q?.noTotal) {
options = {
headers: {
'X-NoTotal': '1',
},
};
}
if (fullQuery.length > (maxLength || 2000)) {
const body: any = {};
if (ids && ids.length > 0) {
body.ids = ids;
} else if (queryObj) {
body.q = queryObj;
}
if (parentId) {
body.parentId = parentId;
}
const url = this.apiUrl.buildUrl(`api/apps/${appName}/assets/query`);
return this.http.post<{ total: number; items: any[]; folders: any[] } & Resource>(url, body).pipe(
map(({ total, items, _links }) => {
const assets = items.map(parseAsset);
return new AssetsDto(total, assets, _links);
}),
pretifyError('i18n:assets.loadFailed'));
} else {
const url = this.apiUrl.buildUrl(`api/apps/${appName}/assets?${fullQuery}`);
return this.http.get<{ total: number; items: any[]; folders: any[] } & Resource>(url).pipe(
map(({ total, items, _links }) => {
const assets = items.map(parseAsset);
return this.http.post<AssetsResponse>(url, body, options).pipe(
map(({ total, items, _links }) => {
const assets = items.map(parseAsset);
return new AssetsDto(total, assets, _links);
}),
pretifyError('i18n:assets.loadFailed'));
}
return new AssetsDto(total, assets, _links);
}),
pretifyError('i18n:assets.loadFailed'));
}
public getAssetFolders(appName: string, parentId: string, scope: AssetFolderScope): Observable<AssetFoldersDto> {
const url = this.apiUrl.buildUrl(`api/apps/${appName}/assets/folders?parentId=${parentId}&scope=${scope}`);
return this.http.get<{ total: number; items: any[]; folders: any[]; path: any[] } & Resource>(url).pipe(
return this.http.get<AssetFolderResponse>(url).pipe(
map(({ total, items, path, _links }) => {
const assetFolders = items.map(parseAssetFolder);
const assetPath = path.map(parseAssetFolder);
@ -437,6 +393,52 @@ export class AssetsService {
}
}
function buildQuery(q?: AssetsByQuery & AssetsByIds) {
const { ids, parentId, query, skip, tags, take } = q || {};
const body: any = {};
if (parentId) {
body.parentId = parentId;
}
if (Types.isArray(ids)) {
body.ids = ids;
} else {
const queryObj: Query = {};
const filters: any[] = [];
if (query && query.fullText && query.fullText.length > 0) {
filters.push({ path: 'fileName', op: 'contains', value: query.fullText });
}
if (tags) {
for (const tag of tags) {
if (tag && tag.length > 0) {
filters.push({ path: 'tags', op: 'eq', value: tag });
}
}
}
if (filters.length > 0) {
queryObj.filter = { and: filters };
}
if (take && take > 0) {
queryObj.take = take;
}
if (skip && skip > 0) {
queryObj.skip = skip;
}
body.q = sanitize(queryObj);
}
return body;
}
function parseAsset(response: any) {
return new AssetDto(response._links, response._meta,
response.id,

7
frontend/app/shared/services/contents.service.spec.ts

@ -38,7 +38,7 @@ describe('ContentsService', () => {
let contents: ContentsDto;
contentsService.getContents('my-app', 'my-schema', { take: 17, skip: 13, query }).subscribe(result => {
contentsService.getContents('my-app', 'my-schema', { take: 17, skip: 13, query, noTotal: true }).subscribe(result => {
contents = result;
});
@ -48,6 +48,7 @@ describe('ContentsService', () => {
expect(req.request.method).toEqual('POST');
expect(req.request.headers.get('If-Match')).toBeNull();
expect(req.request.headers.get('X-NoTotal')).toBe('1');
expect(req.request.body).toEqual({ q: sanitize(expectedQuery) });
req.flush({
@ -78,6 +79,7 @@ describe('ContentsService', () => {
expect(req.request.method).toEqual('POST');
expect(req.request.headers.get('If-Match')).toBeNull();
expect(req.request.headers.get('X-NoTotal')).toBeNull();
expect(req.request.body).toEqual({ odata: '$filter=my-filter&$top=17&$skip=13' });
req.flush({ total: 10, items: [] });
@ -87,12 +89,13 @@ describe('ContentsService', () => {
inject([ContentsService, HttpTestingController], (contentsService: ContentsService, httpMock: HttpTestingController) => {
const ids = ['1', '2', '3'];
contentsService.getAllContents('my-app', { ids }).subscribe();
contentsService.getAllContents('my-app', { ids, noTotal: true }).subscribe();
const req = httpMock.expectOne('http://service/p/api/content/my-app');
expect(req.request.method).toEqual('POST');
expect(req.request.headers.get('If-Match')).toBeNull();
expect(req.request.headers.get('X-NoTotal')).toBe('1');
expect(req.request.body).toEqual({ ids });
req.flush({ total: 10, items: [] });

83
frontend/app/shared/services/contents.service.ts

@ -10,7 +10,7 @@ import { Injectable } from '@angular/core';
import { AnalyticsService, ApiUrlConfig, DateTime, ErrorDto, hasAnyLink, HTTP, mapVersioned, pretifyError, Resource, ResourceLinks, ResultSet, Version, Versioned } from '@app/framework';
import { Observable } from 'rxjs';
import { map, tap } from 'rxjs/operators';
import { encodeQuery, Query, StatusInfo } from './../state/query';
import { Query, sanitize, StatusInfo } from './../state/query';
import { parseField, RootFieldDto } from './schemas.service';
export class ScheduleDto {
@ -125,14 +125,20 @@ export type BulkUpdateDto =
export type BulkUpdateJobDto =
Readonly<{ id: string; type: BulkUpdateType; status?: string; schema?: string; dueTime?: string | null; expectedVersion?: number }>;
export type ContentsQuery =
Readonly<{ noTotal?: boolean }>;
export type ContentsByIds =
Readonly<{ ids: ReadonlyArray<string> }>;
Readonly<{ ids: ReadonlyArray<string> }> & ContentsQuery;
export type ContentsBySchedule =
Readonly<{ scheduledFrom: string | null; scheduledTo: string | null }>;
Readonly<{ scheduledFrom: string | null; scheduledTo: string | null }> & ContentsQuery;
type ContentsByQuery =
Readonly<{ query?: Query; skip?: number; take?: number }> & ContentsQuery;
export type ContentsByQuery =
Readonly<{ query?: Query; skip?: number; take?: number }>;
type ContentsResponse =
Readonly<{ total: number; items: []; statuses: StatusInfo[] } & Resource>;
@Injectable()
export class ContentsService {
@ -144,19 +150,21 @@ export class ContentsService {
}
public getContents(appName: string, schemaName: string, q?: ContentsByQuery): Observable<ContentsDto> {
const { odataParts, queryObj } = buildQuery(q);
const body = buildQuery(q);
const body: any = {};
const url = this.apiUrl.buildUrl(`/api/content/${appName}/${schemaName}/query`);
if (odataParts.length > 0) {
body.odata = odataParts.join('&');
} else if (queryObj) {
body.q = queryObj;
}
let options = {};
const url = this.apiUrl.buildUrl(`/api/content/${appName}/${schemaName}/query`);
if (q?.noTotal) {
options = {
headers: {
'X-NoTotal': '1',
},
};
}
return this.http.post<{ total: number; items: []; statuses: StatusInfo[] } & Resource>(url, body).pipe(
return this.http.post<ContentsResponse>(url, body, options).pipe(
map(({ total, items, statuses, _links }) => {
const contents = items.map(parseContent);
@ -183,9 +191,21 @@ export class ContentsService {
}
public getAllContents(appName: string, q: ContentsByIds | ContentsBySchedule): Observable<ContentsDto> {
const { noTotal, ...body } = q;
const url = this.apiUrl.buildUrl(`/api/content/${appName}`);
return this.http.post<{ total: number; items: []; statuses: StatusInfo[] } & Resource>(url, q).pipe(
let options = {};
if (noTotal) {
options = {
headers: {
'X-NoTotal': '1',
},
};
}
return this.http.post<ContentsResponse>(url, body, options).pipe(
map(({ total, items, statuses, _links }) => {
const contents = items.map(parseContent);
@ -199,7 +219,17 @@ export class ContentsService {
const url = this.apiUrl.buildUrl(`/api/content/${appName}/${schemaName}/${id}/references?${fullQuery}`);
return this.http.get<{ total: number; items: []; statuses: StatusInfo[] } & Resource>(url).pipe(
let options = {};
if (q?.noTotal) {
options = {
headers: {
'X-NoTotal': '1',
},
};
}
return this.http.get<ContentsResponse>(url, options).pipe(
map(({ total, items, statuses, _links }) => {
const contents = items.map(parseContent);
@ -213,7 +243,7 @@ export class ContentsService {
const url = this.apiUrl.buildUrl(`/api/content/${appName}/${schemaName}/${id}/referencing?${fullQuery}`);
return this.http.get<{ total: number; items: []; statuses: StatusInfo[] } & Resource>(url).pipe(
return this.http.get<ContentsResponse>(url).pipe(
map(({ total, items, statuses, _links }) => {
const contents = items.map(parseContent);
@ -337,13 +367,12 @@ export class ContentsService {
function buildQuery(q?: ContentsByQuery) {
const { query, skip, take } = q || {};
const queryParts: string[] = [];
const odataParts: string[] = [];
let queryObj: Query | undefined;
const body: any = {};
if (query && query.fullText && query.fullText.indexOf('$') >= 0) {
odataParts.push(`${query.fullText.trim()}`);
const odataParts: string[] = [
`${query.fullText.trim()}`,
];
if (take && take > 0) {
odataParts.push(`$top=${take}`);
@ -352,8 +381,10 @@ function buildQuery(q?: ContentsByQuery) {
if (skip && skip > 0) {
odataParts.push(`$skip=${skip}`);
}
body.odata = odataParts.join('&');
} else {
queryObj = { ...query };
const queryObj: Query = { ...query };
if (take && take > 0) {
queryObj.take = take;
@ -363,12 +394,10 @@ function buildQuery(q?: ContentsByQuery) {
queryObj.skip = skip;
}
queryParts.push(`q=${encodeQuery(queryObj)}`);
body.q = sanitize(queryObj);
}
const fullQuery = [...queryParts, ...odataParts].join('&');
return { fullQuery, odataParts, queryObj };
return body;
}
function parseContent(response: any) {

13
frontend/app/shared/state/assets.state.spec.ts

@ -107,6 +107,19 @@ describe('AssetsState', () => {
expect().nothing();
});
it('should skip page size if loaded before', () => {
assetsService.setup(x => x.getAssets(app, { take: 30, skip: 0, parentId: MathHelper.EMPTY_GUID }))
.returns(() => of(new AssetsDto(200, [asset1, asset2]))).verifiable();
assetsService.setup(x => x.getAssets(app, { take: 30, skip: 30, parentId: MathHelper.EMPTY_GUID, noTotal: true }))
.returns(() => of(new AssetsDto(200, []))).verifiable();
assetsState.load().subscribe();
assetsState.page({ page: 1, pageSize: 30 }).subscribe();
expect().nothing();
});
});
describe('Navigating', () => {

24
frontend/app/shared/state/assets.state.ts

@ -6,7 +6,7 @@
*/
import { Injectable } from '@angular/core';
import { compareStrings, DialogService, ErrorDto, getPagingInfo, ListState, MathHelper, shareSubscribed, State } from '@app/framework';
import { compareStrings, DialogService, ErrorDto, getPagingInfo, ListState, MathHelper, shareSubscribed, State, Types } from '@app/framework';
import { EMPTY, forkJoin, Observable, of, throwError } from 'rxjs';
import { catchError, finalize, switchMap, tap } from 'rxjs/operators';
import { AnnotateAssetDto, AssetDto, AssetFolderDto, AssetFoldersDto, AssetsService, RenameAssetFolderDto } from './../services/assets.service';
@ -166,7 +166,8 @@ export abstract class AssetsStateBase extends State<Snapshot> {
const { items: assets, total } = assetsResult;
this.next({
this.next(s => ({
...s,
assets,
folders: foldersResult.items,
canCreate: assetsResult.canCreate,
@ -177,8 +178,8 @@ export abstract class AssetsStateBase extends State<Snapshot> {
isLoading: false,
path,
tagsAvailable,
total,
}, 'Loading Success');
total: total >= 0 ? total : s.total,
}), 'Loading Success');
}),
finalize(() => {
this.next({ isLoading: false }, 'Loading Done');
@ -462,26 +463,29 @@ function createQuery(snapshot: Snapshot) {
pageSize,
query,
tagsSelected,
total,
} = snapshot;
const result: any = { take: pageSize, skip: pageSize * page };
const hasQuery = !!query?.fullText || Object.keys(tagsSelected).length > 0;
const tags = Object.keys(tagsSelected);
if (hasQuery) {
if (Types.isString(query?.fullText) || tags.length > 0) {
if (query) {
result.query = query;
}
const searchTags = Object.keys(snapshot.tagsSelected);
if (searchTags.length > 0) {
result.tags = searchTags;
if (tags.length > 0) {
result.tags = tags;
}
} else {
result.parentId = snapshot.parentId;
}
if (page > 0 && total > 0) {
result.noTotal = true;
}
return result;
}

8
frontend/app/shared/state/contents.state.ts

@ -163,7 +163,7 @@ export abstract class ContentsStateBase extends State<Snapshot> {
this.next({ isLoading: true }, 'Loading Done');
const { page, pageSize, query, reference, referencing } = this.snapshot;
const { page, pageSize, query, reference, referencing, total } = this.snapshot;
const q: any = { take: pageSize, skip: pageSize * page };
@ -171,6 +171,10 @@ export abstract class ContentsStateBase extends State<Snapshot> {
q.query = query;
}
if (page > 0 && total > 0) {
q.noTotal = true;
}
let content$: Observable<ContentsDto>;
if (referencing) {
@ -205,7 +209,7 @@ export abstract class ContentsStateBase extends State<Snapshot> {
contents,
selectedContent,
statuses,
total,
total: total >= 0 ? total : s.total,
};
}, 'Loading Success');
}),

4
frontend/app/shared/state/query.ts

@ -194,10 +194,6 @@ export function serializeQuery(query?: Query | null) {
return JSON.stringify(sanitize(query));
}
export function encodeQuery(query?: Query | null) {
return encodeURIComponent(serializeQuery(query));
}
export function deserializeQuery(raw?: string): Query | undefined {
let query: Query | undefined;

Loading…
Cancel
Save