diff --git a/lib/mapping/object-selector.js b/lib/mapping/object-selector.js index de25df62..50b36fd2 100644 --- a/lib/mapping/object-selector.js +++ b/lib/mapping/object-selector.js @@ -255,22 +255,33 @@ class ObjectSelector { throw new Error(`Table "${info.tables[i].name}" could not be retrieved`); } - // All partition and clustering keys from the table should be included in the document - const keyNames = table.partitionKeys.concat(table.clusteringKeys).map(k => k.name); - const columns = propertiesInfo.map(p => p.columnName); + if (keysAreIncluded(table.partitionKeys, propertiesInfo) !== keyMatches.all) { + return false; + } - for (let i = 0; i < keyNames.length; i++) { - if (columns.indexOf(keyNames[i]) === -1) { + const clusteringKeyNames = table.clusteringKeys.map(k => k.name); + const queriedClusteringKeyNames = propertiesInfo.filter(k => clusteringKeyNames.includes(k.columnName)).map(k => k.columnName); + for (const queriedClusteringKeyName of queriedClusteringKeyNames) { + if (clusteringKeyNames.indexOf(queriedClusteringKeyName) >= queriedClusteringKeyNames.length) { + // One of the clustering columns preceding queriedClusteringKeyName is omitted return false; } } + // The filter makes sure that the column doesn't appear in `doc` (but it may appear in `docInfo`) + const queriedColumnNames = propertiesInfo.filter(p => p.value).map(p => p.columnName); + const primaryKeyColumnNames = table.partitionKeys.concat(table.clusteringKeys).map(k => k.name); + if (queriedColumnNames.some(queriedColumnName => !primaryKeyColumnNames.includes(queriedColumnName))) { + // Only primary key columns are allowed + return false; + } + // "when" conditions should be contained in the table return when.reduce((acc, p) => acc && table.columnsByName[p.columnName] !== undefined, true); }); if (filteredTables.length === 0) { - let message = `No table matches (all PKs have to be specified) fields: [${ + let message = `No table matches (must specify all partition key and top-level clustering columns) fields: [${ propertiesInfo.map(p => p.columnName)}]`; if (when.length > 0) { diff --git a/test/unit/mapping/model-mapper-mutation-tests.js b/test/unit/mapping/model-mapper-mutation-tests.js index d28846c5..6142c2d1 100644 --- a/test/unit/mapping/model-mapper-mutation-tests.js +++ b/test/unit/mapping/model-mapper-mutation-tests.js @@ -28,15 +28,15 @@ describe('ModelMapper', () => { mapperTestHelper.testParameters('insert'); it('should retrieve the table that apply and make a single execution', () => { - const clientInfo = mapperTestHelper.getClient([ 'id1', 'id2', 'name'], [ 1, 1 ]); + const clientInfo = mapperTestHelper.getClient([ 'partition1', 'clustering1', 'name'], [ 1, 1 ]); const modelMapper = mapperTestHelper.getModelMapper(clientInfo); - const doc = { id2: 'value2' , id1: 'value1' }; + const doc = { clustering1: 'value2' , partition1: 'value1' }; return modelMapper.insert(doc) .then(() => { assert.strictEqual(clientInfo.executions.length, 1); const execution = clientInfo.executions[0]; - assert.strictEqual(execution.query, 'INSERT INTO ks1.table1 ("id2", "id1") VALUES (?, ?)'); + assert.strictEqual(execution.query, 'INSERT INTO ks1.table1 ("clustering1", "partition1") VALUES (?, ?)'); assert.deepStrictEqual(execution.params, Object.keys(doc).map(key => doc[key])); helper.assertProperties(execution.options, { prepare: true, isIdempotent: true }); }); @@ -44,9 +44,9 @@ describe('ModelMapper', () => { it('should mark LWT queries as non-idempotent', () => testQueries('insert', [ { - doc: { id2: 'value2' , id1: 'value1', name: 'name1' }, + doc: { clustering1: 'value2' , partition1: 'value1', name: 'name1' }, docInfo: { ifNotExists: true }, - query: 'INSERT INTO ks1.table1 ("id2", "id1", "name") VALUES (?, ?, ?) IF NOT EXISTS', + query: 'INSERT INTO ks1.table1 ("clustering1", "partition1", "name") VALUES (?, ?, ?) IF NOT EXISTS', params: [ 'value2', 'value1', 'name1' ], isIdempotent: false } @@ -54,9 +54,9 @@ describe('ModelMapper', () => { it('should set TTL', () => testQueries('insert', [ { - doc: { id2: 'value2' , id1: 'value1', name: 'name1' }, + doc: { clustering1: 'value2' , partition1: 'value1', name: 'name1' }, docInfo: { ttl: 1000 }, - query: 'INSERT INTO ks1.table1 ("id2", "id1", "name") VALUES (?, ?, ?) USING TTL ?', + query: 'INSERT INTO ks1.table1 ("clustering1", "partition1", "name") VALUES (?, ?, ?) USING TTL ?', params: [ 'value2', 'value1', 'name1', 1000 ] } ])); @@ -75,7 +75,7 @@ describe('ModelMapper', () => { let catchCalled = false; - return modelMapper.insert({ id1: 'value1' }) + return modelMapper.insert({ partition1: 'value1' }) .catch(err => { catchCalled = true; assert.strictEqual(err, error); @@ -85,15 +85,15 @@ describe('ModelMapper', () => { it('should throw an error when filter or conditions are not valid', () => testErrors('insert', [ { - doc: { id1: 'x', notAValidProp: 'y' }, - message: 'No table matches (all PKs have to be specified) fields: [id1,notAValidProp]' + doc: { partition1: 'x', notAValidProp: 'y' }, + message: 'No table matches (all PKs have to be specified) fields: [partition1,notAValidProp]' }, { - doc: { id1: 'x'}, + doc: { partition1: 'x'}, docInfo: { fields: ['notAValidProp'] }, message: 'No table matches (all PKs have to be specified) fields: [notAValidProp]' }, { - doc: { id1: 'x', name: 'y' }, - message: 'No table matches (all PKs have to be specified) fields: [id1,name]' + doc: { partition1: 'x', name: 'y' }, + message: 'No table matches (all PKs have to be specified) fields: [partition1,name]' }, { doc: {}, message: 'Expected object with keys' @@ -101,14 +101,14 @@ describe('ModelMapper', () => { ])); it('should warn when cache reaches 100 different queries', async () => { - const clientInfo = mapperTestHelper.getClient(['id1'], [ 1 ], 'ks1'); + const clientInfo = mapperTestHelper.getClient(['partition1'], [ 1 ], 'ks1'); const modelMapper = mapperTestHelper.getModelMapper(clientInfo); const cacheHighWaterMark = 100; const promises = []; for (let i = 0; i < 2 * cacheHighWaterMark; i++) { - promises.push(modelMapper.insert({ id1: 1, [`col${i % (cacheHighWaterMark-1)}`]: 1})); + promises.push(modelMapper.insert({ partition1: 1, [`col${i % (cacheHighWaterMark-1)}`]: 1})); } await Promise.all(promises); @@ -117,7 +117,7 @@ describe('ModelMapper', () => { assert.strictEqual(clientInfo.logMessages.filter(l => l.level === 'warning').length, 0); // One more query - await modelMapper.insert({ id1: 1, anotherColumn: 1 }); + await modelMapper.insert({ partition1: 1, anotherColumn: 1 }); const warnings = clientInfo.logMessages.filter(l => l.level === 'warning'); assert.strictEqual(warnings.length, 1); @@ -136,9 +136,9 @@ describe('ModelMapper', () => { }, items: [ { - doc: { id1: 'value_id1', id2: 'value_id2', name: { prop1: 1, prop2: 'two' } }, - query: 'INSERT INTO ks1.table1 ("id1", "id2", "name") VALUES (?, ?, ?)', - params: [ 'value_id1', 'value_id2', '{"prop1":1,"prop2":"two"}'] + doc: { partition1: 'value_partition1', clustering1: 'value_clustering1', name: { prop1: 1, prop2: 'two' } }, + query: 'INSERT INTO ks1.table1 ("partition1", "clustering1", "name") VALUES (?, ?, ?)', + params: [ 'value_partition1', 'value_clustering1', '{"prop1":1,"prop2":"two"}'] } ] })); @@ -149,93 +149,93 @@ describe('ModelMapper', () => { it('should retrieve the table that apply and make a single execution', () => testQueries('update', [ { - doc: { id2: 'value2' , id1: 'value1', name: 'name1' }, - query: 'UPDATE ks1.table1 SET "name" = ? WHERE "id2" = ? AND "id1" = ?', + doc: { clustering1: 'value2' , partition1: 'value1', name: 'name1' }, + query: 'UPDATE ks1.table1 SET "name" = ? WHERE "clustering1" = ? AND "partition1" = ?', params: ['name1', 'value2', 'value1'], isIdempotent: true }])); it('should mark LWT queries as non-idempotent', () => testQueries('update', [ { - doc: {id2: 'value2', id1: 'value1', name: 'name1'}, + doc: {clustering1: 'value2', partition1: 'value1', name: 'name1'}, docInfo: {when: {name: 'previous name'}}, - query: 'UPDATE ks1.table1 SET "name" = ? WHERE "id2" = ? AND "id1" = ? IF "name" = ?', + query: 'UPDATE ks1.table1 SET "name" = ? WHERE "clustering1" = ? AND "partition1" = ? IF "name" = ?', params: ['name1', 'value2', 'value1', 'previous name'], isIdempotent: false }])); it('should append/prepend to a list', () => testQueries('update', [ { - doc: { id2: 'value2' , id1: 'value1', name: 'name1', list1: q.append(['a', 'b']) }, - query: 'UPDATE ks1.table1 SET "name" = ?, "list1" = "list1" + ? WHERE "id2" = ? AND "id1" = ?', + doc: { clustering1: 'value2' , partition1: 'value1', name: 'name1', list1: q.append(['a', 'b']) }, + query: 'UPDATE ks1.table1 SET "name" = ?, "list1" = "list1" + ? WHERE "clustering1" = ? AND "partition1" = ?', params: ['name1', ['a', 'b'], 'value2', 'value1'], isIdempotent: false }, { - doc: { id2: 'value2' , id1: 'value1', name: 'name1', list1: q.prepend(['a', 'b']) }, - query: 'UPDATE ks1.table1 SET "name" = ?, "list1" = ? + "list1" WHERE "id2" = ? AND "id1" = ?', + doc: { clustering1: 'value2' , partition1: 'value1', name: 'name1', list1: q.prepend(['a', 'b']) }, + query: 'UPDATE ks1.table1 SET "name" = ?, "list1" = ? + "list1" WHERE "clustering1" = ? AND "partition1" = ?', params: ['name1', ['a', 'b'], 'value2', 'value1'], isIdempotent: false }])); it('should increment/decrement a counter', () => { - const clientInfo = mapperTestHelper.getClient([ 'id1', 'id2', { name: 'c1', type: { code: dataTypes.counter }}], [ 1, 1 ]); + const clientInfo = mapperTestHelper.getClient([ 'partition1', 'clustering1', { name: 'c1', type: { code: dataTypes.counter }}], [ 1, 1 ]); const modelMapper = mapperTestHelper.getModelMapper(clientInfo); const items = [ { - doc: { id2: 'value2' , id1: 'value1', c1: q.incr(10) }, - query: 'UPDATE ks1.table1 SET "c1" = "c1" + ? WHERE "id2" = ? AND "id1" = ?' + doc: { clustering1: 'value2' , partition1: 'value1', c1: q.incr(10) }, + query: 'UPDATE ks1.table1 SET "c1" = "c1" + ? WHERE "clustering1" = ? AND "partition1" = ?' }, { - doc: { id2: 'another id 2' , id1: 'another id 1', c1: q.decr(10) }, - query: 'UPDATE ks1.table1 SET "c1" = "c1" - ? WHERE "id2" = ? AND "id1" = ?' + doc: { clustering1: 'another id 2' , partition1: 'another id 1', c1: q.decr(10) }, + query: 'UPDATE ks1.table1 SET "c1" = "c1" - ? WHERE "clustering1" = ? AND "partition1" = ?' }]; return Promise.all(items.map((item, index) => modelMapper.update(item.doc).then(() => { assert.strictEqual(clientInfo.executions.length, items.length); const execution = clientInfo.executions[index]; assert.strictEqual(execution.query, item.query); - assert.deepStrictEqual(execution.params, [ 10, item.doc.id2, item.doc.id1 ]); + assert.deepStrictEqual(execution.params, [ 10, item.doc.clustering1, item.doc.partition1 ]); helper.assertProperties(execution.options, { prepare: true, isIdempotent: false }); }))); }); it('should throw an error when filter or conditions are not valid', () => testErrors('update', [ { - doc: { id1: 'x', notAValidProp: 'y' }, - message: 'No table matches (all PKs and columns to set have to be specified) fields: [id1,notAValidProp]' + doc: { partition1: 'x', notAValidProp: 'y' }, + message: 'No table matches (all PKs and columns to set have to be specified) fields: [partition1,notAValidProp]' }, { - doc: { id1: 'x'}, + doc: { partition1: 'x'}, docInfo: { fields: ['notAValidProp'] }, message: 'No table matches (all PKs and columns to set have to be specified) fields: [notAValidProp]' }, { - doc: { id1: 'x', name: 'y' }, - message: 'No table matches (all PKs and columns to set have to be specified) fields: [id1,name]' + doc: { partition1: 'x', name: 'y' }, + message: 'No table matches (all PKs and columns to set have to be specified) fields: [partition1,name]' }, { - doc: { id1: 'x', id2: 'y', name: 'z'}, + doc: { partition1: 'x', clustering1: 'y', name: 'z'}, docInfo: { when: { notAValidProp: 'm'} }, - message: 'No table matches (all PKs and columns to set have to be specified) fields: [id1,id2,name]; condition: [notAValidProp]' + message: 'No table matches (all PKs and columns to set have to be specified) fields: [partition1,clustering1,name]; condition: [notAValidProp]' }, { doc: {}, message: 'Expected object with keys' }, { - doc: { id1: 'x', id2: 'y' }, - message: 'No table matches (all PKs and columns to set have to be specified) fields: [id1,id2]' + doc: { partition1: 'x', clustering1: 'y' }, + message: 'No table matches (all PKs and columns to set have to be specified) fields: [partition1,clustering1]' } ])); it('should use fields when specified', () => testQueries('update', [ { - doc: { id2: 'value2', id1: 'value1', name: 'name1', description: 'description1' }, - docInfo: { fields: [ 'id1', 'id2', 'description' ] }, - query: 'UPDATE ks1.table1 SET "description" = ? WHERE "id1" = ? AND "id2" = ?', + doc: { clustering1: 'value2', partition1: 'value1', name: 'name1', description: 'description1' }, + docInfo: { fields: [ 'partition1', 'clustering1', 'description' ] }, + query: 'UPDATE ks1.table1 SET "description" = ? WHERE "partition1" = ? AND "clustering1" = ?', params: ['description1', 'value1', 'value2'] }])); it('should set TTL', () => testQueries('update', [ { - doc: { id1: 'value_id1', id2: 'value_id2', name: 'value_name1' }, + doc: { partition1: 'value_partition1', clustering1: 'value_clustering1', name: 'value_name1' }, docInfo: { ttl: 360 }, - query: 'UPDATE ks1.table1 USING TTL ? SET "name" = ? WHERE "id1" = ? AND "id2" = ?', - params: [ 360, 'value_name1', 'value_id1', 'value_id2' ] + query: 'UPDATE ks1.table1 USING TTL ? SET "name" = ? WHERE "partition1" = ? AND "clustering1" = ?', + params: [ 360, 'value_name1', 'value_partition1', 'value_clustering1' ] } ])); @@ -246,21 +246,21 @@ describe('ModelMapper', () => { tables: ['table1'], columns: { 'name': { fromModel: JSON.stringify }, - 'id2': { fromModel: v => v + "_suffix" } + 'clustering1': { fromModel: v => v + "_suffix" } } } }, items: [ { - doc: { id1: 'value_id1', id2: 'value_id2', name: { prop1: 1, prop2: 'two' } }, - query: 'UPDATE ks1.table1 SET "name" = ? WHERE "id1" = ? AND "id2" = ?', - params: [ '{"prop1":1,"prop2":"two"}', 'value_id1', 'value_id2_suffix' ] + doc: { partition1: 'value_partition1', clustering1: 'value_clustering1', name: { prop1: 1, prop2: 'two' } }, + query: 'UPDATE ks1.table1 SET "name" = ? WHERE "partition1" = ? AND "clustering1" = ?', + params: [ '{"prop1":1,"prop2":"two"}', 'value_partition1', 'value_clustering1_suffix' ] }, { - doc: { id1: 'value_id1', id2: 'value_id2', description: 'my description' }, + doc: { partition1: 'value_partition1', clustering1: 'value_clustering1', description: 'my description' }, docInfo: { when: { name: { a: 'a', b: 2 } } }, - query: 'UPDATE ks1.table1 SET "description" = ? WHERE "id1" = ? AND "id2" = ? IF "name" = ?', - params: [ 'my description', 'value_id1', 'value_id2_suffix', '{"a":"a","b":2}' ], + query: 'UPDATE ks1.table1 SET "description" = ? WHERE "partition1" = ? AND "clustering1" = ? IF "name" = ?', + params: [ 'my description', 'value_partition1', 'value_clustering1_suffix', '{"a":"a","b":2}' ], isIdempotent: false } ] @@ -272,46 +272,49 @@ describe('ModelMapper', () => { it('should throw an error when filter or conditions are not valid', () => testErrors('remove', [ { - doc: { id1: 'x', notAValidProp: 'y' }, - message: 'No table matches (all PKs have to be specified) fields: [id1,notAValidProp]' + doc: { partition1: 'x', notAValidProp: 'y' }, + message: 'No table matches (must specify all partition key and top-level clustering columns) fields: [partition1,notAValidProp]' }, { - doc: { id1: 'x'}, + doc: { partition1: 'x'}, docInfo: { fields: ['notAValidProp'] }, - message: 'No table matches (all PKs have to be specified) fields: [notAValidProp]' + message: 'No table matches (must specify all partition key and top-level clustering columns) fields: [notAValidProp]' }, { - doc: { id1: 'x', name: 'y' }, - message: 'No table matches (all PKs have to be specified) fields: [id1,name]' + doc: { partition1: 'x', name: 'y' }, + message: 'No table matches (must specify all partition key and top-level clustering columns) fields: [partition1,name]' }, { - doc: { id1: 'x', id2: 'y', name: 'z'}, + doc: { partition1: 'x', clustering1: 'y'}, docInfo: { when: { notAValidProp: 'm'} }, - message: 'No table matches (all PKs have to be specified) fields: [id1,id2,name]; condition: [notAValidProp]' + message: 'No table matches (must specify all partition key and top-level clustering columns) fields: [partition1,clustering1]; condition: [notAValidProp]' }, { doc: {}, message: 'Expected object with keys' + }, { + doc: { partition1: 'x', clustering2: 'y' }, + message: 'No table matches (must specify all partition key and top-level clustering columns) fields: [partition1,clustering2]' } ])); it('should generate the query, params and set the idempotency', () => testQueries('remove', [ { - doc: { id1: 'x', 'id2': 'y' }, - query: 'DELETE FROM ks1.table1 WHERE "id1" = ? AND "id2" = ?', + doc: { partition1: 'x', clustering1: 'y' }, + query: 'DELETE FROM ks1.table1 WHERE "partition1" = ? AND "clustering1" = ?', params: [ 'x', 'y' ] }, { - doc: { id1: 'x', 'id2': 'y' }, + doc: { partition1: 'x', clustering1: 'y' }, docInfo: { when: { name: 'a' }}, - query: 'DELETE FROM ks1.table1 WHERE "id1" = ? AND "id2" = ? IF "name" = ?', + query: 'DELETE FROM ks1.table1 WHERE "partition1" = ? AND "clustering1" = ? IF "name" = ?', params: [ 'x', 'y', 'a' ], isIdempotent: false }, { - doc: { id1: 'x', 'id2': 'y' }, + doc: { partition1: 'x', clustering1: 'y' }, docInfo: { ifExists: true }, - query: 'DELETE FROM ks1.table1 WHERE "id1" = ? AND "id2" = ? IF EXISTS', + query: 'DELETE FROM ks1.table1 WHERE "partition1" = ? AND "clustering1" = ? IF EXISTS', params: [ 'x', 'y' ], isIdempotent: false }, { - doc: { id1: 'x', 'id2': 'y' }, - docInfo: { fields: [ 'id1', 'id2', 'name' ], deleteOnlyColumns: true }, - query: 'DELETE "name" FROM ks1.table1 WHERE "id1" = ? AND "id2" = ?', + doc: { partition1: 'x', clustering1: 'y' }, + docInfo: { fields: [ 'partition1', 'clustering1', 'name' ], deleteOnlyColumns: true }, + query: 'DELETE "name" FROM ks1.table1 WHERE "partition1" = ? AND "clustering1" = ?', params: [ 'x', 'y' ] } ])); @@ -323,21 +326,21 @@ describe('ModelMapper', () => { tables: ['table1'], columns: { 'name': { fromModel: JSON.stringify }, - 'id2': { fromModel: v => v + "_suffix" } + 'clustering1': { fromModel: v => v + "_suffix" } } } }, items: [ { - doc: { id1: 'value_id1', id2: 'value_id2' }, - query: 'DELETE FROM ks1.table1 WHERE "id1" = ? AND "id2" = ?', - params: [ 'value_id1', 'value_id2_suffix' ] + doc: { partition1: 'value_partition1', clustering1: 'value_clustering1' }, + query: 'DELETE FROM ks1.table1 WHERE "partition1" = ? AND "clustering1" = ?', + params: [ 'value_partition1', 'value_clustering1_suffix' ] }, { - doc: { id1: 'value_id1', id2: 'value_id2' }, + doc: { partition1: 'value_partition1', clustering1: 'value_clustering1' }, docInfo: { when: { name: { a: 1 } }}, - query: 'DELETE FROM ks1.table1 WHERE "id1" = ? AND "id2" = ? IF "name" = ?', - params: [ 'value_id1', 'value_id2_suffix', '{"a":1}' ], + query: 'DELETE FROM ks1.table1 WHERE "partition1" = ? AND "clustering1" = ? IF "name" = ?', + params: [ 'value_partition1', 'value_clustering1_suffix', '{"a":1}' ], isIdempotent: false }, ] @@ -347,8 +350,8 @@ describe('ModelMapper', () => { function testErrors(methodName, items) { return Promise.all(items.map(item => { - const columns = [ 'id1', 'id2', 'name']; - const clientInfo = mapperTestHelper.getClient(columns, [ 1, 1 ], 'ks1'); + const columns = [ 'partition1', 'clustering1', 'clustering2', 'name']; + const clientInfo = mapperTestHelper.getClient(columns, [ 1, 2 ], 'ks1'); const modelMapper = mapperTestHelper.getModelMapper(clientInfo); let catchCalled = false; @@ -365,7 +368,7 @@ function testErrors(methodName, items) { async function testQueries(methodName, items) { let models = null; - const columns = [ 'id1', 'id2', 'name', { name: 'list1', type: { code: dataTypes.list }}, 'description']; + const columns = [ 'partition1', 'clustering1', 'name', { name: 'list1', type: { code: dataTypes.list }}, 'description']; if (typeof methodName === 'object') { // Its an object with properties as parameters