Skip to content

Commit ac498a8

Browse files
aaronburtleaaronburtle
committed
mysql put fix
1 parent a4505f8 commit ac498a8

6 files changed

Lines changed: 215 additions & 48 deletions

File tree

‎config-generators/mysql-commands.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,9 @@ update Stock --config "dab-config.MySql.json" --permissions "test_role_with_excl
6161
update Stock --config "dab-config.MySql.json" --permissions "test_role_with_excluded_fields:read" --fields.exclude "categoryName"
6262
update Stock --config "dab-config.MySql.json" --permissions "test_role_with_policy_excluded_fields:create,update,delete"
6363
update Stock --config "dab-config.MySql.json" --permissions "test_role_with_policy_excluded_fields:read" --fields.exclude "categoryName" --policy-database "@item.piecesAvailable ne 0"
64+
update Stock --config "dab-config.MySql.json" --permissions "database_policy_tester:update" --policy-database "@item.pieceid ne 1"
65+
update Stock --config "dab-config.MySql.json" --permissions "database_policy_tester:create"
66+
update Stock --config "dab-config.MySql.json" --permissions "database_policy_tester:read"
6467
update Book --config "dab-config.MySql.json" --permissions "authenticated:create,read,update,delete"
6568
update Book --config "dab-config.MySql.json" --relationship publishers --target.entity Publisher --cardinality one
6669
update Book --config "dab-config.MySql.json" --relationship websiteplacement --target.entity BookWebsitePlacement --cardinality one

‎src/Core/Resolvers/MySqlQueryBuilder.cs‎

Lines changed: 50 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,13 @@ public class MySqlQueryBuilder : BaseSqlQueryBuilder, IQueryBuilder
1818
private static DbCommandBuilder _builder = new MySqlCommandBuilder();
1919
public const string DATABASE_NAME_PARAM = "databaseName";
2020

21+
/// <summary>
22+
/// Column alias under which the number of records already present for the given primary key
23+
/// is returned as the first result set of an upsert query. Used by the query executor to
24+
/// distinguish an update from an insert and to detect database policy failures.
25+
/// </summary>
26+
public const string COUNT_ROWS_WITH_GIVEN_PK = "cnt_rows_to_update";
27+
2128
/// <summary>
2229
/// Adds database specific quotes to string identifier
2330
/// </summary>
@@ -113,29 +120,55 @@ public string Build(SqlExecuteStructure structure)
113120
public string Build(SqlUpsertQueryStructure structure)
114121
{
115122
(string sets, string updates, string select) = MakeQuerySegmentsForUpdate(structure, structure.OutputColumns);
123+
string tableName = QuoteIdentifier(structure.DatabaseObject.Name);
124+
125+
// Predicates identifying the record by its primary key.
126+
string pkPredicates = Build(structure.Predicates);
127+
128+
// Predicates for the UPDATE: primary key + database policy configured for the update operation.
129+
// Applying the update policy here ensures a PUT/PATCH cannot overwrite a record the caller is
130+
// not authorized to modify (e.g. a row owned by a different user).
131+
string updatePredicates = JoinPredicateStrings(
132+
pkPredicates,
133+
structure.GetDbPolicyForOperation(EntityActionOperation.Update));
134+
135+
// Capture whether a record already exists for the given primary key BEFORE attempting the
136+
// update/insert. This count is surfaced as the first result set and is used by the query
137+
// executor to distinguish an update from an insert and to detect database policy failures.
138+
string countExistingRows =
139+
$"SET @cnt := (SELECT COUNT(*) FROM {tableName} WHERE {pkPredicates}); " +
140+
$"SELECT @cnt AS {QuoteIdentifier(COUNT_ROWS_WITH_GIVEN_PK)};";
141+
142+
// Update honoring the update database policy. When the policy is not satisfied, zero rows
143+
// match and the subsequent select returns no rows.
144+
string updateQuery =
145+
$"UPDATE {tableName} " +
146+
$"SET {Build(structure.UpdateOperations, ", ")} " +
147+
", " + updates +
148+
$" WHERE {updatePredicates}; " +
149+
$"SET @ROWCOUNT=ROW_COUNT(); " +
150+
$"SELECT " + select + $" WHERE @ROWCOUNT > 0;";
116151

117152
if (structure.IsFallbackToUpdate)
118153
{
119-
return sets + ";\n" +
120-
$"UPDATE {QuoteIdentifier(structure.DatabaseObject.Name)} " +
121-
$"SET {Build(structure.UpdateOperations, ", ")} " +
122-
", " + updates +
123-
$" WHERE {Build(structure.Predicates)}; " +
124-
$" SET @ROWCOUNT=ROW_COUNT(); " +
125-
$"SELECT " + select + $" WHERE @ROWCOUNT > 0;";
154+
// Update-only path (e.g. autogenerated primary key): no insert is attempted.
155+
return sets + ";\n" + countExistingRows + updateQuery;
126156
}
127157
else
128158
{
129-
string insert = $"INSERT INTO {QuoteIdentifier(structure.DatabaseObject.Name)} ({Build(structure.InsertColumns)}) " +
130-
$"VALUES ({string.Join(", ", (structure.Values))}) ";
131-
132-
return sets + ";\n" +
133-
insert + " ON DUPLICATE KEY " +
134-
$"UPDATE {Build(structure.UpdateOperations, ", ")}" +
135-
$", " + updates + ";" +
136-
$" SET @ROWCOUNT=ROW_COUNT(); " +
137-
$"SELECT " + select + $" WHERE @ROWCOUNT != 1;" +
138-
$"SELECT {MakeUpsertSelections(structure)} WHERE @ROWCOUNT = 1;";
159+
// Insert honoring the create database policy, but only when no record already exists for
160+
// the given primary key (@cnt = 0). Gating the insert on @cnt = 0 ensures the update
161+
// policy enforced above cannot be bypassed by falling through to an insert on an
162+
// existing record.
163+
string createPredicates = JoinPredicateStrings(structure.GetDbPolicyForOperation(EntityActionOperation.Create));
164+
string insertQuery =
165+
$"INSERT INTO {tableName} ({Build(structure.InsertColumns)}) " +
166+
$"SELECT {string.Join(", ", structure.Values)} FROM DUAL " +
167+
$"WHERE @cnt = 0 AND ({createPredicates}); " +
168+
$"SET @ROWCOUNT=ROW_COUNT(); " +
169+
$"SELECT {MakeUpsertSelections(structure)} WHERE @ROWCOUNT = 1;";
170+
171+
return sets + ";\n" + countExistingRows + updateQuery + insertQuery;
139172
}
140173
}
141174

‎src/Core/Resolvers/MySqlQueryExecutor.cs‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@
22
// Licensed under the MIT License.
33

44
using System.Data.Common;
5+
using System.Net;
56
using Azure.Core;
67
using Azure.DataApiBuilder.Config;
78
using Azure.DataApiBuilder.Config.ObjectModel;
89
using Azure.DataApiBuilder.Core.Configurations;
910
using Azure.DataApiBuilder.Core.Models;
11+
using Azure.DataApiBuilder.Service.Exceptions;
1012
using Azure.Identity;
1113
using Microsoft.AspNetCore.Http;
1214
using Microsoft.Extensions.Logging;
@@ -187,5 +189,103 @@ private bool IsDefaultAccessTokenValid()
187189

188190
return _defaultAccessToken?.Token;
189191
}
192+
193+
/// <summary>
194+
/// Interprets the result sets produced by an upsert (PUT/PATCH) query built by
195+
/// <see cref="MySqlQueryBuilder.Build(SqlUpsertQueryStructure)"/> to determine whether the
196+
/// operation resulted in an update or an insert, and to surface database policy failures.
197+
/// The upsert query returns:
198+
/// result set #1: the number of records already present for the given primary key.
199+
/// result set #2: the output of the UPDATE (non-empty only when a record was updated).
200+
/// result set #3 (non-fallback only): the output of the INSERT (non-empty only when a record was inserted).
201+
/// </summary>
202+
/// <param name="dbDataReader">A DbDataReader.</param>
203+
/// <param name="args">The arguments to this handler - args[0] = primary key in pretty format, args[1] = entity name.</param>
204+
public override async Task<DbResultSet> GetMultipleResultSetsIfAnyAsync(
205+
DbDataReader dbDataReader, List<string>? args = null)
206+
{
207+
// Result set #1: count (0/1) of records already present for the given primary key.
208+
DbResultSet countResultSet = await ExtractResultSetFromDbDataReaderAsync(dbDataReader);
209+
DbResultSetRow? countResultSetRow = countResultSet.Rows.FirstOrDefault();
210+
int numOfRecordsWithGivenPK;
211+
212+
if (countResultSetRow is not null &&
213+
countResultSetRow.Columns.TryGetValue(MySqlQueryBuilder.COUNT_ROWS_WITH_GIVEN_PK, out object? rowsWithGivenPK) &&
214+
rowsWithGivenPK is not null)
215+
{
216+
numOfRecordsWithGivenPK = Convert.ToInt32(rowsWithGivenPK);
217+
}
218+
else
219+
{
220+
throw new DataApiBuilderException(
221+
message: "Neither insert nor update could be performed.",
222+
statusCode: HttpStatusCode.InternalServerError,
223+
subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError);
224+
}
225+
226+
// Result set #2: output of the UPDATE. Non-empty only when a record was actually updated
227+
// (i.e. it existed and satisfied the update database policy).
228+
DbResultSet? updateResultSet = await dbDataReader.NextResultAsync()
229+
? await ExtractResultSetFromDbDataReaderAsync(dbDataReader)
230+
: null;
231+
232+
if (numOfRecordsWithGivenPK == 1)
233+
{
234+
// A record existed for the given primary key, so an update was attempted.
235+
if (updateResultSet is null || updateResultSet.Rows.Count == 0)
236+
{
237+
// Record exists but no record was updated - indicates the update database policy
238+
// was not satisfied (e.g. an attempt to modify another user's row).
239+
throw new DataApiBuilderException(
240+
message: DataApiBuilderException.AUTHORIZATION_FAILURE,
241+
statusCode: HttpStatusCode.Forbidden,
242+
subStatusCode: DataApiBuilderException.SubStatusCodes.DatabasePolicyFailure);
243+
}
244+
245+
// Identifies this as the result set of an update operation (used to return HTTP 200
246+
// instead of 201 and to omit the location header).
247+
updateResultSet.ResultProperties.Add(SqlMutationEngine.IS_UPDATE_RESULT_SET, true);
248+
return updateResultSet;
249+
}
250+
251+
// No record existed for the given primary key, so an insert was attempted. The insert output
252+
// is in result set #3. For the update-only (fallback) path there is no insert result set.
253+
DbResultSet? insertResultSet = await dbDataReader.NextResultAsync()
254+
? await ExtractResultSetFromDbDataReaderAsync(dbDataReader)
255+
: null;
256+
257+
if (insertResultSet is null)
258+
{
259+
// Update-only path (e.g. autogenerated primary key) and no record was found to update.
260+
if (args is not null && args.Count > 1)
261+
{
262+
string prettyPrintPk = args[0];
263+
string entityName = args[1];
264+
265+
throw new DataApiBuilderException(
266+
message: $"Cannot perform INSERT and could not find {entityName} " +
267+
$"with primary key {prettyPrintPk} to perform UPDATE on.",
268+
statusCode: HttpStatusCode.NotFound,
269+
subStatusCode: DataApiBuilderException.SubStatusCodes.ItemNotFound);
270+
}
271+
272+
throw new DataApiBuilderException(
273+
message: "Neither insert nor update could be performed.",
274+
statusCode: HttpStatusCode.InternalServerError,
275+
subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError);
276+
}
277+
278+
if (insertResultSet.Rows.Count == 0)
279+
{
280+
// No record existed but nothing was inserted - indicates the create database policy
281+
// was not satisfied.
282+
throw new DataApiBuilderException(
283+
message: DataApiBuilderException.AUTHORIZATION_FAILURE,
284+
statusCode: HttpStatusCode.Forbidden,
285+
subStatusCode: DataApiBuilderException.SubStatusCodes.DatabasePolicyFailure);
286+
}
287+
288+
return insertResultSet;
289+
}
190290
}
191291
}

‎src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMySql.verified.txt‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,23 @@
387387
Action: Delete
388388
}
389389
]
390+
},
391+
{
392+
Role: database_policy_tester,
393+
Actions: [
394+
{
395+
Action: Read
396+
},
397+
{
398+
Action: Create
399+
},
400+
{
401+
Action: Update,
402+
Policy: {
403+
Database: @item.pieceid ne 1
404+
}
405+
}
406+
]
390407
}
391408
],
392409
Relationships: {

‎src/Service.Tests/SqlTests/RestApiTests/Patch/MySqlPatchApiTests.cs‎

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,19 @@ SELECT JSON_OBJECT('categoryid', categoryid, 'pieceid', pieceid, 'categoryName',
9595
) AS subq
9696
"
9797
},
98+
{
99+
"PatchOneUpdateWithDatabasePolicy",
100+
@"
101+
SELECT JSON_OBJECT('categoryid', categoryid, 'pieceid', pieceid, 'categoryName', categoryName,
102+
'piecesAvailable',piecesAvailable,'piecesRequired',piecesRequired) AS data
103+
FROM (
104+
SELECT categoryid, pieceid, categoryName,piecesAvailable,piecesRequired
105+
FROM " + _Composite_NonAutoGenPK_TableName + @"
106+
WHERE categoryid = 100 AND pieceid = 99 AND categoryName ='Historical' AND piecesAvailable = 4
107+
AND piecesRequired = 0 AND pieceid != 1
108+
) AS subq
109+
"
110+
},
98111
{
99112
"PatchOne_Insert_Empty_Test",
100113
@"
@@ -295,51 +308,42 @@ SELECT JSON_OBJECT('categoryid', categoryid, 'pieceid', pieceid, 'piecesAvailabl
295308
};
296309

297310
#region overridden tests
298-
[TestMethod]
299-
[Ignore]
300-
public override Task PatchOneInsertInViewTest()
301-
{
302-
throw new NotImplementedException();
303-
}
304311

312+
// Create-action database policies are only supported for MSSQL and DWSQL. Since MySQL does not
313+
// support a database policy on the create action, the PATCH tests that rely on a create policy
314+
// (insert path) remain unsupported here. The update-policy path is validated by
315+
// PatchOneUpdateWithDatabasePolicy and PatchOneUpdateWithUnsatisfiedDatabasePolicy.
305316
[TestMethod]
306317
[Ignore]
307-
public override Task PatchOneUpdateViewTest()
308-
{
309-
throw new NotImplementedException();
310-
}
311-
312-
[TestMethod]
313-
[Ignore]
314-
public void PatchOneViewBadRequestTest()
318+
public override Task PatchOneInsertWithDatabasePolicy()
315319
{
316320
throw new NotImplementedException();
317321
}
318322

319323
[TestMethod]
320324
[Ignore]
321-
public override Task PatchOneUpdateWithUnsatisfiedDatabasePolicy()
325+
public override Task PatchOneInsertWithUnsatisfiedDatabasePolicy()
322326
{
323327
throw new NotImplementedException();
324328
}
325329

326330
[TestMethod]
327331
[Ignore]
328-
public override Task PatchOneInsertWithUnsatisfiedDatabasePolicy()
332+
public override Task PatchOneInsertInViewTest()
329333
{
330334
throw new NotImplementedException();
331335
}
332336

333337
[TestMethod]
334338
[Ignore]
335-
public override Task PatchOneUpdateWithDatabasePolicy()
339+
public override Task PatchOneUpdateViewTest()
336340
{
337341
throw new NotImplementedException();
338342
}
339343

340344
[TestMethod]
341345
[Ignore]
342-
public override Task PatchOneInsertWithDatabasePolicy()
346+
public void PatchOneViewBadRequestTest()
343347
{
344348
throw new NotImplementedException();
345349
}

0 commit comments

Comments
 (0)