Skip to content

Commit

Permalink
Fix PUT/PATCH/DELETE requests when translations of the ID field are used
Browse files Browse the repository at this point in the history
Change-type: patch
  • Loading branch information
thgreasi committed Aug 26, 2024
1 parent 522ae49 commit d3be662
Show file tree
Hide file tree
Showing 3 changed files with 200 additions and 23 deletions.
72 changes: 71 additions & 1 deletion src/odata-to-abstract-sql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import stringHash = require('string-hash');
import {
isAliasNode,
isFromNode,
isSelectNode,
isSelectQueryNode,
isTableNode,
} from '@balena/abstract-sql-compiler';
import type {
Expand Down Expand Up @@ -69,6 +71,7 @@ import type {
IsNotDistinctFromNode,
IsDistinctFromNode,
UnknownTypeNodes,
FromTypeNode,
} from '@balena/abstract-sql-compiler';
import type {
ODataBinds,
Expand Down Expand Up @@ -176,6 +179,58 @@ const containsQueryOption = (opts?: object): boolean => {
return false;
};

const addNestedFieldSelect = (
selectNode: SelectNode[1],
fromNode: FromNode[1],
fieldName: string,
fieldNameAlias: string,
) => {
let aliasName: string | undefined;
let tableOrSubqueryNode: FromTypeNode[keyof FromTypeNode];
if (isAliasNode(fromNode)) {
tableOrSubqueryNode = fromNode[1];
aliasName = fromNode[2];
} else {
tableOrSubqueryNode = fromNode;
if (!isTableNode(tableOrSubqueryNode)) {
throw new Error('');
}
}
if (isTableNode(tableOrSubqueryNode)) {
selectNode.push([
'Alias',
['ReferencedField', aliasName ?? tableOrSubqueryNode[1], fieldName],
fieldNameAlias,
]);
return;
}
if (!isSelectQueryNode(tableOrSubqueryNode)) {
throw new Error(
`Adding a nested field select to a subquery containing a ${tableOrSubqueryNode[0]} is not supported`,
);
}
if (aliasName == null) {
// This should never happen but we are checking it to make TS happy.
throw new Error('Found unaliased SelectQueryNode');
}
const nestedSelectNode = tableOrSubqueryNode.find(isSelectNode);
if (nestedSelectNode == null) {
throw new Error(`Cannot find SelectNode in subquery`);
}
const nestedFromNode = tableOrSubqueryNode.find(isFromNode);
if (nestedFromNode == null) {
throw new Error(`Cannot find FromNode in subquery`);
}
addNestedFieldSelect(
nestedSelectNode[1],
nestedFromNode[1],
fieldName,
fieldNameAlias,
);
selectNode.push(['ReferencedField', aliasName, fieldNameAlias]);
return;
};

class Query {
public select: Array<
| ReferencedFieldNode
Expand Down Expand Up @@ -215,6 +270,14 @@ class Query {
);
this.from.push(tableRef);
}
addNestedFieldSelect(fieldName: string, fieldNameAlias: string): void {
if (this.from.length !== 1) {
throw new Error(
`Adding nested field SELECTs is only supported for queries with exactly 1 FROM clause. Found ${this.from.length}`,
);
}
addNestedFieldSelect(this.select, this.from[0], fieldName, fieldNameAlias);
}
compile(queryType: 'SelectQuery'): SelectQueryNode;
compile(queryType: 'InsertQuery'): InsertQueryNode;
compile(queryType: 'UpdateQuery'): UpdateQueryNode;
Expand Down Expand Up @@ -717,8 +780,15 @@ export class OData2AbstractSQL {
) {
// For update/delete statements we need to use a style query
const subQuery = new Query();
subQuery.select.push(referencedIdField);
subQuery.fromResource(this, resource);
subQuery.addNestedFieldSelect(
resource.idField,
method === 'PATCH'
? '$updateid'
: method === 'DELETE'
? '$deleteid'
: '$upsertid',
);
if (hasQueryOpts) {
this.AddQueryOptions(resource, path, subQuery);
}
Expand Down
141 changes: 121 additions & 20 deletions test/filterby.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@ const methodMaps = {
now: 'CurrentTimestamp',
};

const methodToWhereClauseIdMap = {
PATCH: '$updateid',
PUT: '$upsertid',
DELETE: '$deleteid',
};

const createExpression = function (lhs, op, rhs) {
if (lhs === 'not') {
return {
Expand Down Expand Up @@ -473,12 +479,24 @@ run(function () {
)
.from('pilot');
};
const updateWhere = [

const getUpdateWhere = (
/** @type keyof typeof methodToWhereClauseIdMap */ method,
) => [
'In',
['ReferencedField', 'pilot', 'id'],
[
'SelectQuery',
['Select', [['ReferencedField', 'pilot', 'id']]],
[
'Select',
[
[
'Alias',
['ReferencedField', 'pilot', 'id'],
methodToWhereClauseIdMap[method],
],
],
],
['From', ['Table', 'pilot']],
[
'From',
Expand Down Expand Up @@ -524,21 +542,25 @@ run(function () {
.to.be.a.query.that.updates.fields('name')
.values(['Bind', 'pilot', 'name'])
.from('pilot')
.where(updateWhere);
.where(getUpdateWhere('PATCH'));
}),
);

test(`/pilot?$filter=${odata}`, 'POST', { name: 'Peter' }, (result) =>
it(`should insert pilot where '${odata}'`, () => insertTest(result)),
it(`should insert pilot where '${odata}'`, () => {
insertTest(result);
}),
);

test(`/pilot?$filter=${odata}`, 'PUT', { name: 'Peter' }, (result) =>
describe(`should select from pilot where '${odata}'`, function () {
it('should be an upsert', () => {
expect(result).to.be.a.query.that.upserts;
});
it('that inserts', () => insertTest(result[1]));
return it('and updates', () =>
it('that inserts', () => {
insertTest(result[1]);
});
it('and updates', () => {
expect(result[2])
.to.be.a.query.that.updates.fields(
'created at',
Expand Down Expand Up @@ -569,11 +591,12 @@ run(function () {
'Default',
)
.from('pilot')
.where(updateWhere));
.where(getUpdateWhere('PUT'));
});
}),
);

return test('/pilot?$filter=' + odata, 'DELETE', (result) =>
test('/pilot?$filter=' + odata, 'DELETE', (result) =>
it('should delete from pilot where "' + odata + '"', () => {
expect(result)
.to.be.a.query.that.deletes.from('pilot')
Expand All @@ -582,7 +605,10 @@ run(function () {
['ReferencedField', 'pilot', 'id'],
[
'SelectQuery',
['Select', [['ReferencedField', 'pilot', 'id']]],
[
'Select',
[['Alias', ['ReferencedField', 'pilot', 'id'], '$deleteid']],
],
['From', ['Table', 'pilot']],
[
'From',
Expand Down Expand Up @@ -709,25 +735,37 @@ run([['Number', 1]], function () {
)
.from('pilot');
};
const updateWhere = [

const getUpdateWhere = (
/** @type keyof typeof methodToWhereClauseIdMap */ method,
) => [
'And',
['IsNotDistinctFrom', ['ReferencedField', 'pilot', 'id'], ['Bind', 0]],
[
'In',
['ReferencedField', 'pilot', 'id'],
[
'SelectQuery',
['Select', [['ReferencedField', 'pilot', 'id']]],
[
'Select',
[
[
'Alias',
['ReferencedField', 'pilot', 'id'],
methodToWhereClauseIdMap[method],
],
],
],
['From', ['Table', 'pilot']],
['Where', abstractsql],
],
],
];

test('/pilot(1)?$filter=' + odata, 'POST', { name }, (result) =>
it('should insert into pilot where "' + odata + '"', () =>
insertTest(result),
),
it('should insert into pilot where "' + odata + '"', () => {
insertTest(result);
}),
);

test('/pilot(1)?$filter=' + odata, 'PATCH', { name }, (result) =>
Expand All @@ -736,16 +774,18 @@ run([['Number', 1]], function () {
.to.be.a.query.that.updates.fields('name')
.values(['Bind', 'pilot', 'name'])
.from('pilot')
.where(updateWhere);
.where(getUpdateWhere('PATCH'));
}),
);

return test('/pilot(1)?$filter=' + odata, 'PUT', { name }, (result) =>
test('/pilot(1)?$filter=' + odata, 'PUT', { name }, (result) =>
describe('should upsert the pilot with id 1', function () {
it('should be an upsert', () =>
expect(result).to.be.a.query.that.upserts);
it('that inserts', () => insertTest(result[1]));
return it('and updates', () => {
it('that inserts', () => {
insertTest(result[1]);
});
it('and updates', () => {
expect(result[2])
.to.be.a.query.that.updates.fields(
'created at',
Expand Down Expand Up @@ -776,7 +816,7 @@ run([['Number', 1]], function () {
'Default',
)
.from('pilot')
.where(updateWhere);
.where(getUpdateWhere('PUT'));
});
}),
);
Expand Down Expand Up @@ -1505,7 +1545,7 @@ test(
['ReferencedField', 'copilot', 'id'],
[
'SelectQuery',
['Select', [['ReferencedField', 'copilot', 'id']]],
['Select', [['ReferencedField', 'copilot', '$updateid']]],
[
'From',
[
Expand All @@ -1518,6 +1558,11 @@ test(
['Field', '*'],
['Alias', ['Boolean', false], 'is blocked'],
['Alias', ['Text', 'Junior'], 'rank'],
[
'Alias',
['ReferencedField', 'copilot', 'id'],
'$updateid',
],
],
],
['From', ['Table', 'copilot']],
Expand All @@ -1541,3 +1586,59 @@ test(
]);
}),
);

test(
`/copilot?$select=id,rank&$filter=rank eq 'major'`,
'DELETE',
{ assists__pilot: 1 },
(result) =>
it(`should DELETE copilot based on filtered computed field rank`, () => {
expect(result).to.be.a.query.to.deep.equal([
'DeleteQuery',
['From', ['Table', 'copilot']],
[
'Where',
[
'In',
['ReferencedField', 'copilot', 'id'],
[
'SelectQuery',
['Select', [['ReferencedField', 'copilot', '$deleteid']]],
[
'From',
[
'Alias',
[
'SelectQuery',
[
'Select',
[
['Field', '*'],
['Alias', ['Boolean', false], 'is blocked'],
['Alias', ['Text', 'Junior'], 'rank'],
[
'Alias',
['ReferencedField', 'copilot', 'id'],
'$deleteid',
],
],
],
['From', ['Table', 'copilot']],
],
'copilot',
],
],
[
'Where',
[
'IsNotDistinctFrom',
['ReferencedField', 'copilot', 'rank'],
['Bind', 0],
],
],
],
],
],
]);
}),
);
10 changes: 8 additions & 2 deletions test/paging.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@ test('/pilot?$top=5&$skip=100', 'PATCH', { name }, (result) =>
['ReferencedField', 'pilot', 'id'],
[
'SelectQuery',
['Select', [['ReferencedField', 'pilot', 'id']]],
[
'Select',
[['Alias', ['ReferencedField', 'pilot', 'id'], '$updateid']],
],
['From', ['Table', 'pilot']],
['Limit', ['Number', 5]],
['Offset', ['Number', 100]],
Expand All @@ -52,7 +55,10 @@ test('/pilot?$top=5&$skip=100', 'DELETE', (result) =>
['ReferencedField', 'pilot', 'id'],
[
'SelectQuery',
['Select', [['ReferencedField', 'pilot', 'id']]],
[
'Select',
[['Alias', ['ReferencedField', 'pilot', 'id'], '$deleteid']],
],
['From', ['Table', 'pilot']],
['Limit', ['Number', 5]],
['Offset', ['Number', 100]],
Expand Down

0 comments on commit d3be662

Please sign in to comment.