Skip to content

Commit

Permalink
improve validation of abortSignal option, and add additional tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@autopulated
autopulated committed Apr 25, 2024
1 parent 7b7d958 commit c79fabc
Show file tree
Hide file tree
Showing 3 changed files with 119 additions and 8 deletions.
22 changes: 15 additions & 7 deletions lib/model.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,12 +147,20 @@ class BaseModel {
async remove() { return this.#remove(); }
async toObject({virtuals=true, ...otherOptions}={}) { return this.#toObject({virtuals, ...otherOptions}); }


// public static methods:
// options: {ConsistentRead: true, abortSignal: ...} ... dynamoDB consistent read option (defaults to false), and dynamoDB abortSignal options
static #abortSignalSchema = {
type: 'object',
apiArgument: {
validate: (data) => (typeof data.aborted === 'boolean') && (typeof data.addEventListener === 'function'),
error: 'Must be an AbortController Signal.'
}
};
static #getById_options_validate = ajv.compile({
type: 'object',
properties: {
abortSignal: {type:'object'},
abortSignal: this.#abortSignalSchema,
ConsistentRead: {type:'boolean'},
},
additionalProperties: false
Expand All @@ -173,7 +181,7 @@ class BaseModel {
static #getByIds_options_validate = ajv.compile({
type: 'object',
properties: {
abortSignal: {type:'object'},
abortSignal: this.#abortSignalSchema,
ConsistentRead: {type:'boolean'},
},
additionalProperties: false
Expand Down Expand Up @@ -234,7 +242,7 @@ class BaseModel {
type: 'object',
properties: {
limit: {type:'number', const: 1},
abortSignal: {type:'object'},
abortSignal: this.#abortSignalSchema,
startAfter: {type: 'object'},
rawQueryOptions: this.#rawQueryOptionsSchema,
rawFetchOptions: this.#rawFetchOptionsSchema
Expand All @@ -255,7 +263,7 @@ class BaseModel {
type: 'object',
properties: {
limit: {type:'number', const: 1},
abortSignal: {type:'object'},
abortSignal: this.#abortSignalSchema,
startAfter: {type: 'object'},
rawQueryOptions: this.#rawQueryOptionsSchema,
},
Expand All @@ -274,7 +282,7 @@ class BaseModel {
type: 'object',
properties: {
limit: {type:'number', default: 50},
abortSignal: {type:'object'},
abortSignal: this.#abortSignalSchema,
startAfter: {type: 'object'},
rawQueryOptions: this.#rawQueryOptionsSchema,
rawFetchOptions: this.#rawFetchOptionsSchema
Expand Down Expand Up @@ -336,7 +344,7 @@ class BaseModel {
type: 'object',
properties: {
limit: {type:'number', default:50},
abortSignal: {type:'object'},
abortSignal: this.#abortSignalSchema,
startAfter: {type: 'object'},
rawQueryOptions: this.#rawQueryOptionsSchema
},
Expand Down Expand Up @@ -566,8 +574,8 @@ class BaseModel {
const { ConsistentRead, abortSignal } = rawOptions ?? {};
const table = DerivedModel[kModelTable];
const schema = DerivedModel[kModelSchema];
/* c8 ignore next 3 */
if (!table[kTableIsReady]) {
/* c8 ignore next 2 */
await table.ready();
}
if ((typeof id !== 'string') || (!id.length)) {
Expand Down
24 changes: 23 additions & 1 deletion lib/shared.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const Ajv = require('ajv');
const kExtendedTypeDate = Symbol.for('dynamodm:extendedType:date');
const kExtendedTypeBuffer = Symbol.for('dynamodm:extendedType:buffer');

// this AJV instance is used for general validation, compiled schemas do not modify data
// this AJV instance is used for general validation (e.g. of API function arguments), compiled schemas do not modify data
const ajv = new Ajv({
useDefaults: true
});
Expand Down Expand Up @@ -54,6 +54,28 @@ const extendedTypeKeyword = {
ajv.addKeyword(extendedTypeKeyword);
defaultIgnoringAjv.addKeyword(extendedTypeKeyword);

// Keyword for validating that API arguments with a function and friendly error
// message defined in the schema:
const apiArgument = {
keyword: 'apiArgument',
type: 'object',
schemaType: 'object',
metaSchema: {
type: 'object',
properties: { validate:{} , error: {type:'string'} },
required: ['validate', 'error'],
additionalProperties: false
},
validate: function apiArgValidate(schema, data) {
if (!schema.validate(data)) {
apiArgValidate.errors = [{ message: schema.error, data }];
return false;
}
return true;
}
};
ajv.addKeyword(apiArgument);

// Marshalling of built-in types to dynamodb types:
marshallingAjv.addKeyword({
keyword: 'extendedType',
Expand Down
81 changes: 81 additions & 0 deletions test/queries.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,23 @@ t.test('queries:', async t => {
t.end();
});

t.test('aborting getById', async t => {
const ac0 = new AbortController();
ac0.abort(new Error('my reason 0 '));
t.rejects(Foo.getById(all_foos[0].id, {abortSignal: ac0.signal}), {name:'AbortError', message:'Request aborted'}, 'getById should be abortable with an AbortController that is already aborted');

const ac1 = new AbortController();
t.rejects(Foo.getById(all_foos[0].id, {abortSignal: ac1.signal}), {name:'AbortError', message:'Request aborted'}, 'getById should be abortable with an AbortController signal immediately');
ac1.abort(new Error('my reason'));

const ac2 = new AbortController();
t.rejects(Foo.getById(all_foos[0].id, {abortSignal: ac2.signal}), {name:'AbortError', message:'Request aborted'}, 'getById should be abortable with an AbortController signal asynchronously');
setTimeout(() => {
ac2.abort(new Error('my reason 2'));
}, 1);
t.end();
});

await t.test('rawQueryManyIds', async t => {
t.test('on type index', async t => {
const foo_ids = await Foo.rawQueryManyIds({
Expand Down Expand Up @@ -347,6 +364,60 @@ t.test('queries:', async t => {
t.equal(foo_ids.length, 7, 'should return all N of this type');
t.match(foo_ids, all_foos.slice(0,7).map(f => f.id), 'should return the correct Ids');
});
await t.test('aborting with already-aborted controller', async t => {
const ac0 = new AbortController();
ac0.abort(new Error('my reason 0'));
const iter0 = Foo.rawQueryIteratorIds({
IndexName:'type',
KeyConditionExpression:'#typeFieldName = :type',
ExpressionAttributeValues: { ':type': 'namespace.foo' },
ExpressionAttributeNames: { '#typeFieldName': 'type' }
}, {abortSignal: ac0.signal});
await t.rejects(arrayFromAsync(iter0), {name:'AbortError', message:'Request aborted'}, 'rawQueryIteratorIds should be abortable with an AbortController that is already aborted');
t.end();
});
await t.test('aborting with already-aborted controller', async t => {
const ac1 = new AbortController();
const iter1 = Foo.rawQueryIteratorIds({
IndexName:'type',
KeyConditionExpression:'#typeFieldName = :type',
ExpressionAttributeValues: { ':type': 'namespace.foo' },
ExpressionAttributeNames: { '#typeFieldName': 'type' }
}, {abortSignal: ac1.signal});
const testComplete = t.rejects(arrayFromAsync(iter1), {name:'AbortError', message:'Request aborted'}, 'rawQueryIteratorIds should be abortable with an AbortController signal immediately');
ac1.abort(new Error('my reason'));
await testComplete;
t.end();
});
await t.test('aborting with already-aborted controller', async t => {
const ac2 = new AbortController();
const iter2 = Foo.rawQueryIteratorIds({
IndexName:'type',
KeyConditionExpression:'#typeFieldName = :type',
ExpressionAttributeValues: { ':type': 'namespace.foo' },
ExpressionAttributeNames: { '#typeFieldName': 'type' }
}, {abortSignal: ac2.signal});
const testComplete = t.rejects(arrayFromAsync(iter2), {name:'AbortError', message:'Request aborted'}, 'rawQueryIteratorIds should be abortable with an AbortController signal asynchronously');
setTimeout(() => {
ac2.abort(new Error('my reason 2'));
}, 1);
await testComplete;
t.end();
});
await t.test('aborting after the fact', async t => {
// check that aborting after completion doesn't do anything bad:
const ac3 = new AbortController();
const iter3 = Foo.rawQueryIteratorIds({
IndexName:'type',
KeyConditionExpression:'#typeFieldName = :type',
ExpressionAttributeValues: { ':type': 'namespace.foo' },
ExpressionAttributeNames: { '#typeFieldName': 'type' }
}, {abortSignal: ac3.signal});
const foo_ids = await arrayFromAsync(iter3);
ac3.abort(new Error('my reason 3'));
t.equal(foo_ids[0].startsWith('namespace.foo'), true, 'should have still returned foos');
t.end();
});
t.end();
});

Expand Down Expand Up @@ -506,6 +577,16 @@ t.test('queries:', async t => {

t.end();
});
await t.test('throws with invalid abortSignal', async t => {
// check that accidentally passing the abort controller, instead of its signal, throws an error:
await t.rejects(Foo.queryMany({ type: 'namespace.foo' }, { abortSignal: new AbortController() }), {
message: "Invalid options: [ { message: 'Must be an AbortController Signal.', data: AbortController { signal: AbortSignal { aborted: false } }, instancePath: '/abortSignal', schemaPath: '#/properties/abortSignal/apiArgument"
}, 'passing a signal without an .aborted property should fail');
await t.rejects(Foo.queryMany({ type: 'namespace.foo' }, { abortSignal: { aborted: false } }), {
message: "Invalid options: [ { message: 'Must be an AbortController Signal.', data: { aborted: false }, instancePath: '/abortSignal', schemaPath: '#/properties/abortSignal/apiArgument"
}, 'passing a signal without an .addEventListener function should fail');
t.end();
});
t.end();
});

Expand Down

0 comments on commit c79fabc

Please sign in to comment.