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

feat(config): ensure ids are prefixed before querying with them #2322

Merged
merged 2 commits into from
Jul 18, 2022
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
132 changes: 132 additions & 0 deletions packages/config/src/dynamo/__tests__/config.dynamo.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
import DynamoDB from 'aws-sdk/clients/dynamodb.js';
import o from 'ospec';
import sinon from 'sinon';
import { Config } from '../../base.config.js';
import { ConfigImagery } from '../../config/imagery.js';
import { ConfigTileSet } from '../../config/tile.set.js';
import { ConfigDynamoCached } from '../dynamo.config.cached.js';
import { ConfigProviderDynamo } from '../dynamo.config.js';

const sandbox = sinon.createSandbox();

class FakeDynamoDb {
values: Map<string, Record<string, unknown>> = new Map();
get: unknown[] = [];
getAll: { RequestItems: { Foo: { Keys: { id: { S: string } }[] } } }[] = [];
getItem(req: any): unknown {
this.get.push(req);
const reqId = req.Key.id.S;
const val = this.values.get(reqId);
return {
promise(): Promise<unknown> {
if (val) return Promise.resolve({ Item: DynamoDB.Converter.marshall(val) });
return Promise.resolve(null);
},
};
}

batchGetItem(req: any): unknown {
this.getAll.push(req);
const keys = req.RequestItems.Foo.Keys.map((c: any) => DynamoDB.Converter.unmarshall(c).id);
const output = keys.map((c: string) => this.values.get(c)).filter((f: unknown) => f != null);
return {
promise(): Promise<unknown> {
if (output.length === 0) return Promise.resolve({ Responses: {} });
return Promise.resolve({ Responses: { Foo: output.map((c: any) => DynamoDB.Converter.marshall(c)) } });
},
};
}
}

o.spec('ConfigDynamo', () => {
let provider: ConfigProviderDynamo;
let fakeDynamo: FakeDynamoDb;

o.beforeEach(() => {
provider = new ConfigProviderDynamo('Foo');
Config.setConfigProvider(provider);
fakeDynamo = new FakeDynamoDb();
provider.dynamo = fakeDynamo as any;
});

o.afterEach(() => sandbox.restore());

o('should not get if missing', async () => {
const ret = await Config.TileSet.get('ts_abc123');

o(fakeDynamo.get).deepEquals([{ Key: { id: { S: 'ts_abc123' } }, TableName: 'Foo' }]);
o(ret).equals(null);
});

o('should get', async () => {
fakeDynamo.values.set('ts_abc123', { id: 'ts_abc123' });
const ret = await Config.TileSet.get('ts_abc123');

o(fakeDynamo.get).deepEquals([{ Key: { id: { S: 'ts_abc123' } }, TableName: 'Foo' }]);
o(ret).deepEquals({ id: 'ts_abc123' } as ConfigTileSet);
});

o('should get-all', async () => {
fakeDynamo.values.set('ts_abc123', { id: 'ts_abc123' });
fakeDynamo.values.set('ts_abc456', { id: 'ts_abc456' });
const ret = await Config.TileSet.getAll(new Set(fakeDynamo.values.keys()));

o(fakeDynamo.getAll[0].RequestItems.Foo.Keys).deepEquals([{ id: { S: 'ts_abc123' } }, { id: { S: 'ts_abc456' } }]);
o([...ret.values()]).deepEquals([...fakeDynamo.values.values()] as any);
});

o('should throw without prefix', async () => {
fakeDynamo.values.set('ts_abc123', { id: 'ts_abc123' });
const ret = await Config.TileSet.get('abc123').catch((e) => e);

o(fakeDynamo.get).deepEquals([]);
o(String(ret)).deepEquals('Error: Trying to query "abc123" expected prefix of ts');
});

o('should get-all partial', async () => {
fakeDynamo.values.set('ts_abc123', { id: 'ts_abc123' });
const ret = await Config.TileSet.getAll(new Set(['ts_abc123', 'ts_abc456']));
o(fakeDynamo.getAll[0].RequestItems.Foo.Keys).deepEquals([{ id: { S: 'ts_abc123' } }, { id: { S: 'ts_abc456' } }]);
o([...ret.values()]).deepEquals([...fakeDynamo.values.values()] as any);
});

o('should throw if on wrong prefix', async () => {
const ret = await Config.TileSet.get('im_abc123').catch((e) => e);
o(fakeDynamo.get).deepEquals([]);
o(String(ret)).deepEquals('Error: Trying to query "im_abc123" expected prefix of ts');
});

o('should throw on prefixed and un-prefixed', async () => {
fakeDynamo.values.set('ts_abc123', { id: 'ts_abc123' });

const ret = Config.TileSet.getAll(new Set(['abc123', 'ts_abc123']));
const err = await ret.then(() => null).catch((e) => e);
o(String(err)).equals('Error: Trying to query "abc123" expected prefix of ts');
o(fakeDynamo.getAll).deepEquals([]);
});

o.spec('DynamoCached', () => {
o('should get-all with cache', async () => {
fakeDynamo.values.set('im_abc123', { id: 'im_abc123' });
fakeDynamo.values.set('im_abc456', { id: 'im_abc456' });

const cached = Config.Imagery as ConfigDynamoCached<ConfigImagery>;
cached.cache.set('im_abc123', { id: 'im_abc123' } as ConfigImagery);
const ret = await Config.Imagery.getAll(new Set(['im_abc123', 'im_abc456']));

o(fakeDynamo.getAll[0].RequestItems.Foo.Keys).deepEquals([{ id: { S: 'im_abc456' } }]);
o([...ret.values()]).deepEquals([...fakeDynamo.values.values()] as any);
});

o('should get with cache', async () => {
fakeDynamo.values.set('im_abc123', { id: 'im_abc123' });

const cached = Config.Imagery as ConfigDynamoCached<ConfigImagery>;
cached.cache.set('im_abc123', { id: 'im_abc123' } as ConfigImagery);
const ret = await Config.Imagery.get('im_abc123');

o(fakeDynamo.get).deepEquals([]);
o(ret).deepEquals({ id: 'im_abc123' } as ConfigImagery);
});
});
});
10 changes: 1 addition & 9 deletions packages/config/src/dynamo/__tests__/config.imagery.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Epsg, EpsgCode, NamedBounds, QuadKey, TileMatrixSet, TileMatrixSets } from '@basemaps/geo';
import { Epsg } from '@basemaps/geo';
import DynamoDB from 'aws-sdk/clients/dynamodb.js';
import o from 'ospec';
import sinon from 'sinon';
Expand All @@ -9,14 +9,6 @@ import { ConfigProviderDynamo } from '../dynamo.config.js';

const sandbox = sinon.createSandbox();

export function qkToNamedBounds(quadKeys: string[]): NamedBounds[] {
const tms = TileMatrixSets.get(EpsgCode.Google);
return quadKeys.map((qk) => ({
name: TileMatrixSet.tileToName(QuadKey.toTile(qk)),
...tms.tileToSourceBounds(QuadKey.toTile(qk)),
}));
}

o.spec('ConfigProvider.Imagery', () => {
const provider = new ConfigProviderDynamo('Foo');

Expand Down
18 changes: 14 additions & 4 deletions packages/config/src/dynamo/dynamo.config.base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import { BaseConfig } from '../config/base.js';
import { ConfigPrefix } from '../config/prefix.js';
import { ConfigProviderDynamo } from './dynamo.config.js';

function toId(id: string): { id: { S: string } } {
export type IdQuery = { id: { S: string } };
function toId(id: string): IdQuery {
return { id: { S: id } };
}

Expand All @@ -16,6 +17,12 @@ export class ConfigDynamoBase<T extends BaseConfig = BaseConfig> extends Basemap
this.cfg = cfg;
}

/** Ensure the ID is prefixed before querying */
ensureId(id: string): string {
if (id.startsWith(this.prefix + '_')) return id;
throw new Error(`Trying to query "${id}" expected prefix of ${this.prefix}`);
}

private get db(): DynamoDB {
return this.cfg.dynamo;
}
Expand All @@ -29,7 +36,9 @@ export class ConfigDynamoBase<T extends BaseConfig = BaseConfig> extends Basemap
}

public async get(key: string): Promise<T | null> {
const item = await this.db.getItem({ Key: { id: { S: key } }, TableName: this.cfg.tableName }).promise();
const item = await this.db
.getItem({ Key: { id: { S: this.ensureId(key) } }, TableName: this.cfg.tableName })
.promise();
if (item == null || item.Item == null) return null;
const obj = DynamoDB.Converter.unmarshall(item.Item) as BaseConfig;
if (this.is(obj)) return obj;
Expand All @@ -38,7 +47,8 @@ export class ConfigDynamoBase<T extends BaseConfig = BaseConfig> extends Basemap

/** Get all records with the id */
public async getAll(keys: Set<string>): Promise<Map<string, T>> {
let mappedKeys = Array.from(keys, toId);
let mappedKeys: IdQuery[] = [];
for (const key of keys) mappedKeys.push(toId(this.ensureId(key)));

const output: Map<string, T> = new Map();

Expand All @@ -52,7 +62,7 @@ export class ConfigDynamoBase<T extends BaseConfig = BaseConfig> extends Basemap
const items = await this.db.batchGetItem({ RequestItems }).promise();

const metadataItems = items.Responses?.[this.cfg.tableName];
if (metadataItems == null) throw new Error('Failed to fetch metadata from ' + this.cfg.tableName);
if (metadataItems == null) throw new Error('Failed to fetch from ' + this.cfg.tableName);

for (const row of metadataItems) {
const item = DynamoDB.Converter.unmarshall(row) as BaseConfig;
Expand Down
14 changes: 8 additions & 6 deletions packages/config/src/dynamo/dynamo.config.cached.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ export class ConfigDynamoCached<T extends BaseConfig> extends ConfigDynamoBase<T
cache: Map<string, T> = new Map();

public async get(id: string): Promise<T | null> {
let existing: T | null | undefined = this.cache.get(id);
const queryKey = this.ensureId(id);
let existing: T | null | undefined = this.cache.get(queryKey);
if (existing == null) {
existing = await super.get(id);
existing = await super.get(queryKey);
if (existing == null) return null;
this.cache.set(id, existing);
this.cache.set(queryKey, existing);
}

return existing;
Expand All @@ -20,11 +21,12 @@ export class ConfigDynamoCached<T extends BaseConfig> extends ConfigDynamoBase<T
const toFetch = new Set<string>();

for (const id of ids) {
const existing = this.cache.get(id);
const queryKey = this.ensureId(id);
const existing = this.cache.get(queryKey);
if (existing == null) {
toFetch.add(id);
toFetch.add(queryKey);
} else {
output.set(id, existing);
output.set(queryKey, existing);
}
}

Expand Down