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

CB-5658 CB-5659 fix: pagination resolving #2922

Merged
merged 7 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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
8 changes: 5 additions & 3 deletions webapp/packages/core-authentication/src/UsersResource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,8 @@ export class UsersResource extends CachedMapResource<string, AdminUser, UserReso

usersList.push(user);
} else {
const pageKey =
this.aliases.isAlias(originalKey, CachedResourceOffsetPageKey) || this.aliases.isAlias(originalKey, CachedResourceOffsetPageListKey);
const pageListKey = this.aliases.isAlias(originalKey, CachedResourceOffsetPageListKey);
const pageKey = this.aliases.isAlias(originalKey, CachedResourceOffsetPageKey) || pageListKey;
const filterKey = this.aliases.isAlias(originalKey, UsersResourceFilterKey);
let offset = CACHED_RESOURCE_DEFAULT_PAGE_OFFSET;
let limit = CACHED_RESOURCE_DEFAULT_PAGE_LIMIT;
Expand Down Expand Up @@ -294,7 +294,9 @@ export class UsersResource extends CachedMapResource<string, AdminUser, UserReso
usersList.push(...users);

pages.push([
CachedResourceOffsetPageListKey(offset, users.length).setParent(filterKey!),
pageListKey
? CachedResourceOffsetPageListKey(offset, users.length).setParent(filterKey)
: CachedResourceOffsetPageKey(offset, users.length).setParent(filterKey),
users.map(user => user.userId),
users.length === limit,
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ export class DBObjectResource extends CachedMapResource<string, DBObject> {
let limit = this.navTreeResource.childrenLimit;
let offset = CACHED_RESOURCE_DEFAULT_PAGE_OFFSET;
const parentKey = this.aliases.isAlias(originalKey, DBObjectParentKey);
const pageKey =
this.aliases.isAlias(originalKey, CachedResourceOffsetPageKey) || this.aliases.isAlias(originalKey, CachedResourceOffsetPageListKey);
const pageListKey = this.aliases.isAlias(originalKey, CachedResourceOffsetPageListKey);
const pageKey = this.aliases.isAlias(originalKey, CachedResourceOffsetPageKey) || pageListKey;

if (pageKey) {
limit = pageKey.options.limit;
Expand All @@ -116,11 +116,11 @@ export class DBObjectResource extends CachedMapResource<string, DBObject> {
this.set(resourceKeyList(keys), dbObjects);

this.offsetPagination.setPage(
CachedResourceOffsetPageKey(offset, limit).setParent(CachedResourceOffsetPageTargetKey(originalKey)),
pageListKey
? CachedResourceOffsetPageListKey(offset, limit).setParent(parentKey || CachedResourceOffsetPageTargetKey(nodeId))
: CachedResourceOffsetPageKey(offset, limit).setParent(parentKey || CachedResourceOffsetPageTargetKey(nodeId)),
keys,
this.navTreeResource.offsetPagination.hasNextPage(
CachedResourceOffsetPageKey(offset, limit).setParent(CachedResourceOffsetPageTargetKey(nodeId)),
),
keys.length === limit,
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -461,8 +461,8 @@ export class NavTreeResource extends CachedMapResource<string, string[], Record<
}

protected async loader(originalKey: ResourceKey<string>): Promise<Map<string, string[]>> {
const pageKey =
this.aliases.isAlias(originalKey, CachedResourceOffsetPageKey) || this.aliases.isAlias(originalKey, CachedResourceOffsetPageListKey);
const pageListKey = this.aliases.isAlias(originalKey, CachedResourceOffsetPageListKey);
const pageKey = this.aliases.isAlias(originalKey, CachedResourceOffsetPageKey) || pageListKey;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see a copy-paste here and a little bit below for all resources with pagination.
Maybe we should add a helper here so it returns the correct pageKey and the page? So we don't miss this bug when we create a new feature with the exact pagination mechanism

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now you can use getOffsetPageKeyInfo helper

const allKey = this.aliases.isAlias(originalKey, CachedMapAllKey);
const pageTarget = this.aliases.isAlias(originalKey, CachedResourceOffsetPageTargetKey);

Expand All @@ -481,7 +481,9 @@ export class NavTreeResource extends CachedMapResource<string, string[], Record<
values.push(navNodeChildren);

pages.push([
CachedResourceOffsetPageKey(offset, navNodeChildren.navNodeChildren.length).setParent(CachedResourceOffsetPageTargetKey(nodeId)),
pageListKey
? CachedResourceOffsetPageListKey(offset, navNodeChildren.navNodeChildren.length).setParent(CachedResourceOffsetPageTargetKey(nodeId))
: CachedResourceOffsetPageKey(offset, navNodeChildren.navNodeChildren.length).setParent(CachedResourceOffsetPageTargetKey(nodeId)),
navNodeChildren.navNodeChildren.map(node => node.id),
navNodeChildren.navNodeChildren.length === limit,
]);
Expand Down
72 changes: 45 additions & 27 deletions webapp/packages/core-resource/src/Resource/CachedResource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import { isResourceAlias } from './ResourceAlias';
import { ResourceError } from './ResourceError';
import type { ResourceKey, ResourceKeyFlat } from './ResourceKey';
import { resourceKeyAlias } from './ResourceKeyAlias';
import { resourceKeyList } from './ResourceKeyList';
import { isResourceKeyList, resourceKeyList } from './ResourceKeyList';
import { resourceKeyListAlias } from './ResourceKeyListAlias';
import { ResourceOffsetPagination } from './ResourceOffsetPagination';

Expand Down Expand Up @@ -91,41 +91,58 @@ export abstract class CachedResource<
this.aliases.add(CachedResourceParamKey, () => defaultKey);
this.aliases.add(CachedResourceListEmptyKey, () => resourceKeyList([]));
this.aliases.add(CachedResourceOffsetPageTargetKey, key => key.options.target);
this.aliases.add(CachedResourceOffsetPageKey, key => {
const keys = [];
const pageInfo = this.offsetPagination.getPageInfo(key);
this.aliases.add(
CachedResourceOffsetPageListKey,
key => key.parent! as any,
(param, key) => {
if (!isResourceKeyList(key)) {
return key as any;
}

if (pageInfo) {
const from = key.options.offset;
const to = key.options.offset + key.options.limit;
const keys = new Set<any>();
const pageInfo = this.offsetPagination.getPageInfo(param);

for (const page of pageInfo.pages) {
if (page.isHasCommonSegment(from, to)) {
keys.push(...page.get(from, to));
if (pageInfo) {
const from = param.options.offset;
const to = param.options.offset + param.options.limit;

for (const page of pageInfo.pages) {
if (page.isHasCommonSegment(from, to)) {
for (const pageKey of page.get(from, to)) {
keys.add(pageKey);
}
}
}
}
}
return resourceKeyList(key.filter(value => keys.has(value)));
},
);
this.aliases.add(
CachedResourceOffsetPageKey,
key => key.parent! as any,
(param, key) => {
if (!isResourceKeyList(key)) {
return key as any;
}

// todo: return single element?
return resourceKeyList([...new Set(keys)]);
});
this.aliases.add(CachedResourceOffsetPageListKey, key => {
const keys = [];
const pageInfo = this.offsetPagination.getPageInfo(key);
const keys = new Set<any>();
const pageInfo = this.offsetPagination.getPageInfo(param);

if (pageInfo) {
const from = key.options.offset;
const to = key.options.offset + key.options.limit;
if (pageInfo) {
const from = param.options.offset;
const to = param.options.offset + param.options.limit;

for (const page of pageInfo.pages) {
if (page.isHasCommonSegment(from, to)) {
keys.push(...page.get(from, to));
for (const page of pageInfo.pages) {
if (page.isHasCommonSegment(from, to)) {
for (const pageKey of page.get(from, to)) {
keys.add(pageKey);
}
}
Comment on lines +135 to +140
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. we can define helper like getKeysFromPages(pages)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's keep it for now

}
}
}

return resourceKeyList([...new Set(keys)]);
});
return resourceKeyList(key.filter(value => keys.has(value)));
},
);

// this.logger.spy(this.beforeLoad, 'beforeLoad');
// this.logger.spy(this.onDataOutdated, 'onDataOutdated');
Expand Down Expand Up @@ -329,6 +346,7 @@ export abstract class CachedResource<
}

const pageKey = this.aliases.isAlias(param, CachedResourceOffsetPageKey) || this.aliases.isAlias(param, CachedResourceOffsetPageListKey);

if (pageKey) {
const pageInfo = this.offsetPagination.getPageInfo(pageKey);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export abstract class ResourceAlias<TKey, TOptions extends ResourceAliasOptions>
return undefined;
}

setParent(parent: ResourceAlias<TKey, any>): this {
setParent(parent: ResourceAlias<TKey, any> | undefined): this {
parent = this.parent ? this.parent.setParent(parent) : parent;
const copy = new (this.constructor as any)(this.id, this.options, parent) as this;
return copy;
Expand Down
45 changes: 36 additions & 9 deletions webapp/packages/core-resource/src/Resource/ResourceAliases.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,33 @@
import { toJS } from 'mobx';

import { isResourceAlias, type ResourceAlias, ResourceAliasFactory, type ResourceAliasOptions } from './ResourceAlias';
import type { ResourceKey } from './ResourceKey';
import type { ResourceKey, ResourceKeySimple } from './ResourceKey';
import type { ResourceKeyAlias } from './ResourceKeyAlias';
import { isResourceKeyList, ResourceKeyList } from './ResourceKeyList';
import type { ResourceKeyListAlias } from './ResourceKeyListAlias';
import type { ResourceLogger } from './ResourceLogger';

export type IParamAlias<TKey> = {
id: string;
getAlias: (param: ResourceAlias<TKey, any>) => ResourceKey<TKey>;
getAlias: ResourceAliasResolver<TKey, any>;
transformKey?: ResourceAliasKeyTransformer<TKey, any>;
};

export type ResourceAliasResolver<TKey, TOptions extends ResourceAliasOptions> = (param: ResourceAlias<TKey, TOptions>) => ResourceKey<TKey>;

export type ResourceAliasKeyTransformer<TKey, TOptions extends ResourceAliasOptions> = <T extends ResourceKeySimple<TKey>>(
param: ResourceAlias<TKey, TOptions>,
key: T,
) => T;

export class ResourceAliases<TKey> {
protected paramAliases: Array<IParamAlias<TKey>>;
private captureAliasGetterExecution: boolean;

constructor(private readonly logger: ResourceLogger, private readonly validateKey: (key: TKey) => boolean) {
constructor(
private readonly logger: ResourceLogger,
private readonly validateKey: (key: TKey) => boolean,
) {
this.paramAliases = [];

this.captureAliasGetterExecution = false;
Expand Down Expand Up @@ -52,21 +63,23 @@ export class ResourceAliases<TKey> {

add<TOptions extends ResourceAliasOptions>(
param: ResourceAlias<TKey, TOptions> | ResourceAliasFactory<TKey, TOptions>,
getAlias: (param: ResourceAlias<TKey, TOptions>) => ResourceKey<TKey>,
getAlias: ResourceAliasResolver<TKey, TOptions>,
transformKey?: ResourceAliasKeyTransformer<TKey, TOptions>,
): void {
this.paramAliases.push({ id: param.id, getAlias });
this.paramAliases.push({ id: param.id, getAlias, transformKey });
}

replace<TOptions extends ResourceAliasOptions>(
param: ResourceAlias<TKey, TOptions> | ResourceAliasFactory<TKey, TOptions>,
getAlias: (param: ResourceAlias<TKey, TOptions>) => ResourceKey<TKey>,
getAlias: ResourceAliasResolver<TKey, TOptions>,
transformKey?: ResourceAliasKeyTransformer<TKey, TOptions>,
): void {
const indexOf = this.paramAliases.findIndex(aliasInfo => aliasInfo.id === param.id);

if (indexOf === -1) {
this.add(param, getAlias);
this.add(param, getAlias, transformKey);
} else {
this.paramAliases.splice(indexOf, 1, { id: param.id, getAlias });
this.paramAliases.splice(indexOf, 1, { id: param.id, getAlias, transformKey });
}
}

Expand Down Expand Up @@ -111,6 +124,7 @@ export class ResourceAliases<TKey> {
transformToKey(param: ResourceKey<TKey>): TKey | ResourceKeyList<TKey> {
let deep = 0;

const transforms: Array<{ key: ResourceKeyAlias<TKey, any> | ResourceKeyListAlias<TKey, any>; alias: IParamAlias<TKey> }> = [];
while (deep < 10) {
if (!this.validateResourceKey(param)) {
let paramString = JSON.stringify(toJS(param));
Expand All @@ -124,7 +138,15 @@ export class ResourceAliases<TKey> {
if (isResourceAlias(param)) {
for (const alias of this.paramAliases) {
if (alias.id === param.id) {
param = this.captureGetAlias(alias, param);
transforms.push({ key: param, alias });
const nextParam = this.captureGetAlias(alias, param);

if (isResourceAlias(nextParam)) {
param = nextParam.setParent(param);
} else {
param = nextParam;
}

deep++;
break;
}
Expand All @@ -141,6 +163,11 @@ export class ResourceAliases<TKey> {
if (isResourceAlias(param)) {
throw new Error(`Alias ${param.toString()} is not registered in ${this.logger.getName()}`);
}

for (const { key, alias } of transforms) {
param = alias.transformKey ? alias.transformKey(key, param) : param;
}

return param;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { DefaultValueGetter, isPrimitive, MetadataMap } from '@cloudbeaver/core-

import { CachedResourceOffsetPageKey, CachedResourceOffsetPageListKey } from './CachedResourceOffsetPageKeys';
import type { ICachedResourceMetadata } from './ICachedResourceMetadata';
import { isResourceAlias } from './ResourceAlias';
import { isResourceAlias, ResourceAlias } from './ResourceAlias';
import type { ResourceAliases } from './ResourceAliases';
import type { ResourceKey, ResourceKeyFlat } from './ResourceKey';
import { isResourceKeyList, ResourceKeyList } from './ResourceKeyList';
Expand Down Expand Up @@ -111,6 +111,8 @@ export class ResourceMetadata<TKey, TMetadata extends ICachedResourceMetadata> {
if (this.some(param, predicate)) {
result = true;
}
} else if (predicate(this.get(param))) {
result = true;
}
}

Expand Down Expand Up @@ -194,11 +196,11 @@ export class ResourceMetadata<TKey, TMetadata extends ICachedResourceMetadata> {
if (isResourceAlias(key)) {
key = this.aliases.transformToAlias(key);

if (this.aliases.isAlias(key, CachedResourceOffsetPageKey) || this.aliases.isAlias(key, CachedResourceOffsetPageListKey)) {
if (isResourceAlias(key, CachedResourceOffsetPageKey) || isResourceAlias(key, CachedResourceOffsetPageListKey)) {
return this.getMetadataKeyRef(key.parent as any);
}

return key.toString() as TKey;
return (key as ResourceAlias<TKey, any>).toString() as TKey;
}

if (isPrimitive(key)) {
Expand Down
Loading