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

fix(entity-store): split into asset and entry map to prevent id collision #406

Merged
merged 1 commit into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
48 changes: 19 additions & 29 deletions packages/visual-sdk/src/store/EditorEntityStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,30 +67,20 @@ export class EditorEntityStore extends EntityStore {
return id.length === 1 ? id[0] : id.join(this.cacheIdSeperator);
}

private findMissingEntites(ids: string[]) {
const missing = [];

for (const id of ids) {
const entry = this.entitiesMap.get(id);
if (!entry) {
missing.push(id);
}
}

return missing;
}

private async fetchEntity(ids: string[]): Promise<Array<Entry>>;
private async fetchEntity(ids: string[], isAsset: true): Promise<Array<Asset>>;
private async fetchEntity(ids: string[], isAsset?: boolean): Promise<Array<Entry | Asset>> {
const missingIds = this.findMissingEntites(ids);

if (missingIds.length === 0) {
private async fetchEntity(type: 'Asset', ids: string[]): Promise<Array<Asset>>;
private async fetchEntity(type: 'Entry', ids: string[]): Promise<Array<Entry>>;
private async fetchEntity(
type: 'Asset' | 'Entry',
ids: string[]
): Promise<Array<Entry> | Array<Asset>> {
const { missing, resolved } = this.getEntitiesFromMap(type, ids);

if (missing.length === 0) {
// everything is already in cache
return ids.map((id) => this.entitiesMap.get(id)) as Array<Entry | Asset>;
return resolved as Array<Entry> | Array<Asset>;
}

const cacheId = this.getCacheId(missingIds);
const cacheId = this.getCacheId(missing);
const openRequest = this.requestCache.get(cacheId);

if (openRequest) {
Expand All @@ -101,7 +91,7 @@ export class EditorEntityStore extends EntityStore {
const unsubscribe = this.subscribe(
PostMessageMethods.REQUESTED_ENTITIES,
(message: RequestedEntitiesMessage) => {
if (missingIds.every((id) => message.entities.find((entity) => entity.sys.id === id))) {
if (missing.every((id) => message.entities.find((entity) => entity.sys.id === id))) {
clearTimeout(timeout);
resolve(message.entities);

Expand All @@ -119,8 +109,8 @@ export class EditorEntityStore extends EntityStore {
}, this.timeoutDuration);

this.sendMessage(PostMessageMethods.REQUEST_ENTITIES, {
entityIds: missingIds,
entityType: isAsset ? 'Asset' : 'Entry',
entityIds: missing,
entityType: type,
locale: this.locale,
});
});
Expand All @@ -130,13 +120,13 @@ export class EditorEntityStore extends EntityStore {
this.requestCache.set(cid, newPromise);
});

const result = (await newPromise) as Array<Entry | Asset>;
const result = (await newPromise) as Array<Entry> | Array<Asset>;

result.forEach((value) => {
this.entitiesMap.set(value.sys.id, value);
this.addEntity(value);
});

return ids.map((id) => this.entitiesMap.get(id)) as Array<Entry | Asset>;
return this.getEntitiesFromMap(type, ids).resolved as Array<Entry> | Array<Asset>;
}

public async fetchAsset(id: string): Promise<Asset | undefined> {
Expand All @@ -150,7 +140,7 @@ export class EditorEntityStore extends EntityStore {
}

public fetchAssets(ids: string[]): Promise<Asset[]> {
return this.fetchEntity(ids, true);
return this.fetchEntity('Asset', ids);
}

public async fetchEntry(id: string): Promise<Entry | undefined> {
Expand All @@ -164,6 +154,6 @@ export class EditorEntityStore extends EntityStore {
}

public fetchEntries(ids: string[]): Promise<Entry[]> {
return this.fetchEntity(ids);
return this.fetchEntity('Entry', ids);
}
}
15 changes: 15 additions & 0 deletions packages/visual-sdk/src/store/EditorStore.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,21 @@ describe('EntityStore', () => {
expect(createStore().entities).toEqual(entities);
});

it('prevents id collision for assets and entries', async () => {
const patchedAsset = { ...asset, sys: { ...asset.sys, id: '1' } };
const patchedEntry = { ...entry, sys: { ...entry.sys, id: '1' } };

const store = new EntityStore({ entities: [patchedAsset, patchedEntry], locale });

const [resolvedAsset, resolvedEntry] = await Promise.all([
store.fetchAsset('1'),
Copy link
Collaborator

Choose a reason for hiding this comment

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

What happens with?

public get entities() {
  return [...this.entryMap.values(), ...this.assetMap.values()];
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

() is it possible to have duplicates? EntityStore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes there it is possible to have duplicate sys.id, but you also have the sys.type to differentiate between them, so that should be fine :)

store.fetchEntry('1'),
]);

expect(resolvedAsset).toEqual(patchedAsset);
expect(resolvedEntry).toEqual(patchedEntry);
});

describe('getValue', () => {
it('should return the value based on entityId and path', () => {
const store = createStore();
Expand Down
84 changes: 59 additions & 25 deletions packages/visual-sdk/src/store/EntityStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,28 +8,33 @@ import { get } from './utils';
*/
export class EntityStore {
protected locale: string;
protected entitiesMap: Map<string, Entry | Asset>;
protected entryMap = new Map<string, Entry>();
protected assetMap = new Map<string, Asset>();

constructor({ entities, locale }: { entities: Array<Entry | Asset>; locale: string }) {
this.entitiesMap = new Map(entities.map((entity) => [entity.sys.id, entity]));
this.locale = locale;

for (const entity of entities) {
this.addEntity(entity);
}
}

public get entities() {
return [...this.entitiesMap.values()];
return [...this.entryMap.values(), ...this.assetMap.values()];
}

public updateEntity(entity: Entry | Asset) {
this.entitiesMap.set(entity.sys.id, entity);
this.addEntity(entity);
}

public getValue(
entityLink: UnresolvedLink<'Entry' | 'Asset'>,
path: string[]
): string | undefined {
const entity = this.entitiesMap.get(entityLink.sys.id);
entityLink.sys.type;
const entity = this.getEntity(entityLink.sys.linkType, entityLink.sys.id);

if (!entity || entity.sys.type !== entityLink.sys.linkType) {
if (!entity) {
// TODO: move to `debug` utils once it is extracted
console.warn(`Unresolved entity reference: ${entityLink}`);
return;
Expand All @@ -38,50 +43,79 @@ export class EntityStore {
return get<string>(entity, path);
}

private getEntitiesFromMap(ids: string[]) {
const entity = [];
const missingEntityIds = [];
protected getEntitiesFromMap(type: 'Entry' | 'Asset', ids: string[]) {
const resolved = [];
const missing = [];

for (const id of ids) {
const entry = this.entitiesMap.get(id);
if (entry) {
entity.push(entry);
const entity = this.getEntity(type, id);
if (entity) {
resolved.push(entity);
} else {
missingEntityIds.push(id);
missing.push(id);
}
}

if (missingEntityIds.length) {
throw new Error(`Missing entity in the store (${missingEntityIds.join(',')})`);
}
return {
resolved,
missing,
};
}

return entity as Array<Asset | Entry>;
protected addEntity(entity: Entry | Asset): void {
if (this.isAsset(entity)) {
this.assetMap.set(entity.sys.id, entity);
} else {
this.entryMap.set(entity.sys.id, entity);
}
}

public async fetchAsset(id: string): Promise<Asset | undefined> {
try {
return this.getEntitiesFromMap([id])[0] as Asset;
} catch (err) {
const { resolved, missing } = this.getEntitiesFromMap('Asset', [id]);
if (missing.length) {
// TODO: move to `debug` utils once it is extracted
console.warn(`Asset "${id}" is not in the store`);
return undefined;
}

return resolved[0] as Asset;
}
public async fetchAssets(ids: string[]): Promise<Asset[]> {
return this.getEntitiesFromMap(ids) as Asset[];
const { resolved, missing } = this.getEntitiesFromMap('Asset', ids);
if (missing.length) {
throw new Error(`Missing assets in the store (${missing.join(',')})`);
}
return resolved as Asset[];
}

public async fetchEntry(id: string): Promise<Entry | undefined> {
try {
return this.getEntitiesFromMap([id])[0] as Entry;
} catch (err) {
const { resolved, missing } = this.getEntitiesFromMap('Entry', [id]);
if (missing.length) {
// TODO: move to `debug` utils once it is extracted
console.warn(`Entry "${id}" is not in the store`);
return undefined;
}

return resolved[0] as Entry;
}

public async fetchEntries(ids: string[]): Promise<Entry[]> {
return this.getEntitiesFromMap(ids) as Entry[];
const { resolved, missing } = this.getEntitiesFromMap('Entry', ids);
if (missing.length) {
throw new Error(`Missing assets in the store (${missing.join(',')})`);
}
return resolved as Entry[];
}

private isAsset(entity: Entry | Asset): entity is Asset {
return entity.sys.type === 'Asset';
}

private getEntity(type: 'Asset' | 'Entry', id: string) {
if (type === 'Asset') {
return this.assetMap.get(id);
}

return this.entryMap.get(id);
}
}