Skip to content

Commit f6c11d8

Browse files
authored
Database policy support for PUT/PATCH operations - MsSql (#1428)
## Why make this change? As per the behaviour expected from PUT/PATCH operations with database policies discussed in #1430, implement db policy support for MsSql to fix #1370. ## What is this change? 1. Prior to this change, there was only one database policy for each operation. Since now database policies will be supported for both insert (or create)/update actions via PUT/PATCH operations, these 2 operations can have 2 database policies defined for them, one for each action. Hence, the `DbPolicyPredicates` (`string`) property in `BaseSqlQueryStructure` class is replaced by `DbPolicyPredicatesForOperations` (dictionary: `Config.Action`, `string`). 2. The query generated by `MsSqlQueryBuilder.Build(SqlUpsertQueryStructure structure)` is modified such that only one (or none) of the insert/update operations execute, depending on: - Existence of record in the table/view - Nature of PK (auto gen, non-auto gen) Previously, we always used to perform the update operation irrespective of whether the row existed or not. From now onwards, we would first determine if the row actually exists or not. We will perform update only if it exists. 3. The method `IQueryExecutor.GetMultipleResultSetsIfAnyAsync` has been provided another implementation specific to MsSql in `MsSqlQueryExecutor`. The DbDataReader instance for the query being executed for the PUT/PATCH return operation would always contain atleast 1 result set and atmost 2 result sets. - The first result set will give us the number of records(0/1) corresponding to given PK - The second result set will give us the rows affected by the operation executed. It maybe the case that we were not able to execute either of the update/insert operations. This happens when we are dealing with auto gen PK table/view, and there is no record for given PK. So we cannot perform either of the operations in this case. - If the first result sets says we have zero records present corresponding to given PK, we will attempt an insert operation(provided we are not dealing with auto gen PK table/view for which insert is not possible). - If the result set says we have one record present corresponding to given PK, we go on to perform the update. - The database policies are not taken into consideration when determining number of records for given PK in the first result set. They are only taken into consideration for the actual update/insert query. 4. The property `IS_FIRST_RESULT_SET` in `SqlMutationEngine` is renamed to `IS_UPDATE_RESULT_SET` to better denote its purpose. 5. Different scenarios are added to the method `MsSqlQueryExecutor.GetMultipleResultSetsIfAnyAsync` to throw appropriate exceptions (Forbidden/Authorization failure - 403 and NotFound - 404). Appropriate comments are added within the code to demonstrate each case. ## How was this tested? 1. Integration Tests - Done
1 parent 8ab6c18 commit f6c11d8

29 files changed

+666
-122
lines changed

config-generators/mssql-commands.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,8 @@ update Comic --config "dab-config.MsSql.json" --permissions "TestNestedFilterOne
167167
update Comic --config "dab-config.MsSql.json" --permissions "TestNestedFilterOneMany_EntityReadForbidden:create,update,delete"
168168
update Stock --config "dab-config.MsSql.json" --permissions "TestNestedFilterFieldIsNull_ColumnForbidden:read"
169169
update Stock --config "dab-config.MsSql.json" --permissions "TestNestedFilterFieldIsNull_EntityReadForbidden:read"
170+
update Stock --config "dab-config.MsSql.json" --permissions "database_policy_tester:update" --policy-database "@item.pieceid ne 1"
171+
update Stock --config "dab-config.MsSql.json" --permissions "database_policy_tester:create" --policy-database "@item.pieceid ne 6 and @item.piecesAvailable gt 0"
170172
update series --config "dab-config.MsSql.json" --permissions "TestNestedFilterManyOne_ColumnForbidden:read" --fields.exclude "name"
171173
update series --config "dab-config.MsSql.json" --permissions "TestNestedFilterManyOne_EntityReadForbidden:create,update,delete"
172174
update series --config "dab-config.MsSql.json" --permissions "TestNestedFilterOneMany_ColumnForbidden:read"

config-generators/postgresql-commands.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ update Publisher --config "dab-config.PostgreSql.json" --permissions "database_p
5050
update Publisher --config "dab-config.PostgreSql.json" --permissions "database_policy_tester:create"
5151
update Publisher --config "dab-config.PostgreSql.json" --permissions "database_policy_tester:update" --policy-database "@item.id ne 1234"
5252
update Stock --config "dab-config.PostgreSql.json" --permissions "authenticated:create,read,update,delete" --rest commodities --graphql true --relationship stocks_price --target.entity stocks_price --cardinality one
53+
update Stock --config "dab-config.PostgreSql.json" --permissions "database_policy_tester:create"
54+
update Stock --config "dab-config.PostgreSql.json" --permissions "database_policy_tester:update" --policy-database "@item.pieceid ne 1"
5355
update Book --config "dab-config.PostgreSql.json" --permissions "authenticated:create,read,update,delete"
5456
update Book --config "dab-config.PostgreSql.json" --relationship publishers --target.entity Publisher --cardinality one
5557
update Book --config "dab-config.PostgreSql.json" --relationship websiteplacement --target.entity BookWebsitePlacement --cardinality one

src/Config/DataApiBuilderException.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ public class DataApiBuilderException : Exception
1616
public const string CONNECTION_STRING_ERROR_MESSAGE = "A valid Connection String should be provided.";
1717
public const string GRAPHQL_FILTER_ENTITY_AUTHZ_FAILURE = "Access forbidden to the target entity described in the filter.";
1818
public const string GRAPHQL_FILTER_FIELD_AUTHZ_FAILURE = "Access forbidden to a field referenced in the filter.";
19+
public const string AUTHORIZATION_FAILURE = "Authorization Failure: Access Not Allowed.";
1920

2021
public enum SubStatusCodes
2122
{
@@ -37,6 +38,10 @@ public enum SubStatusCodes
3738
/// </summary>
3839
AuthorizationCheckFailed,
3940
/// <summary>
41+
/// Request did not satisfy database policy for the operation.
42+
/// </summary>
43+
DatabasePolicyFailure,
44+
/// <summary>
4045
/// The requested operation failed on the database.
4146
/// </summary>
4247
DatabaseOperationFailed,

src/Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationTestBase.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public async Task InsertMutationFailingDatabasePolicy(string dbQuery, string err
6464
SqlTestHelper.TestForErrorInGraphQLResponse(
6565
result.ToString(),
6666
message: errorMessage,
67-
statusCode: $"{DataApiBuilderException.SubStatusCodes.AuthorizationCheckFailed}"
67+
statusCode: $"{DataApiBuilderException.SubStatusCodes.DatabasePolicyFailure}"
6868
);
6969

7070
string dbResponse = await GetDatabaseResultAsync(dbQuery);
@@ -873,7 +873,7 @@ public virtual async Task TestDbPolicyForCreateOperationReferencingFieldAbsentIn
873873
SqlTestHelper.TestForErrorInGraphQLResponse(
874874
actual.ToString(),
875875
message: "One or more fields referenced by the database policy are not present in the request body.",
876-
statusCode: $"{DataApiBuilderException.SubStatusCodes.BadRequest}");
876+
statusCode: $"{DataApiBuilderException.SubStatusCodes.AuthorizationCheckFailed}");
877877
}
878878

879879
#endregion

src/Service.Tests/SqlTests/RestApiTests/Insert/InsertApiTestBase.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -664,7 +664,7 @@ await SetupAndRunRestApiTest(
664664
requestBody: requestBody,
665665
exceptionExpected: true,
666666
expectedStatusCode: HttpStatusCode.Forbidden,
667-
expectedSubStatusCode: DataApiBuilderException.SubStatusCodes.AuthorizationCheckFailed.ToString(),
667+
expectedSubStatusCode: DataApiBuilderException.SubStatusCodes.DatabasePolicyFailure.ToString(),
668668
expectedErrorMessage: "Could not insert row with given values.",
669669
clientRoleHeader: "database_policy_tester"
670670
);
@@ -693,8 +693,8 @@ await SetupAndRunRestApiTest(
693693
requestBody: requestBody,
694694
clientRoleHeader: "database_policy_tester",
695695
expectedErrorMessage: "One or more fields referenced by the database policy are not present in the request body.",
696-
expectedStatusCode: HttpStatusCode.BadRequest,
697-
expectedSubStatusCode: DataApiBuilderException.SubStatusCodes.BadRequest.ToString()
696+
expectedStatusCode: HttpStatusCode.Forbidden,
697+
expectedSubStatusCode: DataApiBuilderException.SubStatusCodes.AuthorizationCheckFailed.ToString()
698698
);
699699
}
700700

src/Service.Tests/SqlTests/RestApiTests/Patch/MsSqlPatchApiTests.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,22 @@ public class MsSqlPatchApiTests : PatchApiTestBase
9090
$"WHERE id = 1 AND title = 'The Hobbit Returns to The Shire' " +
9191
$"FOR JSON PATH, INCLUDE_NULL_VALUES, WITHOUT_ARRAY_WRAPPER"
9292
},
93+
{
94+
"PatchOneUpdateWithDatabasePolicy",
95+
$"SELECT [categoryid], [pieceid], [categoryName],[piecesAvailable]," +
96+
$"[piecesRequired] FROM { _Composite_NonAutoGenPK_TableName } " +
97+
$"WHERE [categoryid] = 100 AND [pieceid] = 99 AND [categoryName] = 'Historical' " +
98+
$"AND [piecesAvailable]= 4 AND [piecesRequired] = 0 AND [pieceid] != 1 " +
99+
$"FOR JSON PATH, INCLUDE_NULL_VALUES, WITHOUT_ARRAY_WRAPPER"
100+
},
101+
{
102+
"PatchOneInsertWithDatabasePolicy",
103+
$"SELECT [categoryid], [pieceid], [categoryName],[piecesAvailable]," +
104+
$"[piecesRequired] FROM { _Composite_NonAutoGenPK_TableName } " +
105+
$"WHERE [categoryid] = 0 AND [pieceid] = 7 AND [categoryName] = 'SciFi' " +
106+
$"AND [piecesAvailable]= 4 AND [piecesRequired] = 0 AND ([pieceid] != 6 AND [piecesAvailable] > 0) " +
107+
$"FOR JSON PATH, INCLUDE_NULL_VALUES, WITHOUT_ARRAY_WRAPPER"
108+
},
93109
{
94110
"PatchOne_Update_Default_Test",
95111
$"SELECT [id], [book_id], [content] FROM { _tableWithCompositePrimaryKey } " +

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

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,28 @@ public void PatchOneViewBadRequestTest()
213213

214214
[TestMethod]
215215
[Ignore]
216-
public override Task PatchOneUpdateInAccessibleRowWithDatabasePolicy()
216+
public override Task PatchOneUpdateWithUnsatisfiedDatabasePolicy()
217+
{
218+
throw new NotImplementedException();
219+
}
220+
221+
[TestMethod]
222+
[Ignore]
223+
public override Task PatchOneInsertWithUnsatisfiedDatabasePolicy()
224+
{
225+
throw new NotImplementedException();
226+
}
227+
228+
[TestMethod]
229+
[Ignore]
230+
public override Task PatchOneUpdateWithDatabasePolicy()
231+
{
232+
throw new NotImplementedException();
233+
}
234+
235+
[TestMethod]
236+
[Ignore]
237+
public override Task PatchOneInsertWithDatabasePolicy()
217238
{
218239
throw new NotImplementedException();
219240
}

src/Service.Tests/SqlTests/RestApiTests/Patch/PatchApiTestBase.cs

Lines changed: 92 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ await SetupAndRunRestApiTest(
334334
);
335335
}
336336
/// <summary>
337-
/// Tests the PatchOne functionality with a REST PUT request using
337+
/// Tests the PatchOne functionality with a REST PATCH request using
338338
/// headers that include as a key "If-Match" with an item that does exist,
339339
/// resulting in an update occuring. Verify update with Find.
340340
/// </summary>
@@ -361,6 +361,58 @@ await SetupAndRunRestApiTest(
361361
);
362362
}
363363

364+
/// <summary>
365+
/// Test to validate successful execution of PATCH operation which satisfies the database policy for the update operation it resolves into.
366+
/// </summary>
367+
[TestMethod]
368+
public virtual async Task PatchOneUpdateWithDatabasePolicy()
369+
{
370+
// PATCH operation resolves to update because we have a record present for given PK.
371+
// Since the database policy for update operation ("@item.pieceid ne 1") is satisfied by the operation, it executes successfully.
372+
string requestBody = @"
373+
{
374+
""piecesAvailable"": 4
375+
}";
376+
377+
await SetupAndRunRestApiTest(
378+
primaryKeyRoute: "categoryid/100/pieceid/99",
379+
queryString: null,
380+
entityNameOrPath: _Composite_NonAutoGenPK_EntityPath,
381+
sqlQuery: GetQuery("PatchOneUpdateWithDatabasePolicy"),
382+
operationType: Config.Operation.UpsertIncremental,
383+
requestBody: requestBody,
384+
expectedStatusCode: HttpStatusCode.OK,
385+
clientRoleHeader: "database_policy_tester"
386+
);
387+
}
388+
389+
/// <summary>
390+
/// Test to validate successful execution of PATCH operation which satisfies the database policy for the insert operation it resolves into.
391+
/// </summary>
392+
[TestMethod]
393+
public virtual async Task PatchOneInsertWithDatabasePolicy()
394+
{
395+
// PATCH operation resolves to insert because we don't have a record present for given PK.
396+
// Since the database policy for insert operation ("@item.pieceid ne 6 and @item.piecesAvailable gt 0") is satisfied by the operation, it executes successfully.
397+
string requestBody = @"
398+
{
399+
""piecesAvailable"": 4,
400+
""categoryName"": ""SciFi""
401+
}";
402+
403+
await SetupAndRunRestApiTest(
404+
primaryKeyRoute: "categoryid/0/pieceid/7",
405+
queryString: null,
406+
entityNameOrPath: _Composite_NonAutoGenPK_EntityPath,
407+
sqlQuery: GetQuery("PatchOneInsertWithDatabasePolicy"),
408+
operationType: Config.Operation.UpsertIncremental,
409+
requestBody: requestBody,
410+
expectedStatusCode: HttpStatusCode.Created,
411+
clientRoleHeader: "database_policy_tester",
412+
expectedLocationHeader: "categoryid/0/pieceid/7"
413+
);
414+
}
415+
364416
#endregion
365417

366418
#region Negative Tests
@@ -424,7 +476,7 @@ await SetupAndRunRestApiTest(
424476
}
425477

426478
/// <summary>
427-
/// Tests the PatchOne functionality with a REST PUT request using
479+
/// Tests the PatchOne functionality with a REST PATCH request using
428480
/// headers that include as a key "If-Match" with an item that does not exist,
429481
/// resulting in a DataApiBuilderException with status code of Precondition Failed.
430482
/// </summary>
@@ -589,53 +641,66 @@ await SetupAndRunRestApiTest(
589641
}
590642

591643
/// <summary>
592-
/// Test to validate that PATCH operation fails because the database policy("@item.id ne 1234")
593-
/// restricts modifying records where id is not 1234.
644+
/// Test to validate failure of PATCH operation failing to satisfy the database policy for the update operation.
645+
/// (because a record exists for given PK).
594646
/// </summary>
595647
[TestMethod]
596-
public virtual async Task PatchOneUpdateInAccessibleRowWithDatabasePolicy()
648+
public virtual async Task PatchOneUpdateWithUnsatisfiedDatabasePolicy()
597649
{
598-
// Perform PATCH update with upsert incrmental semantics.
650+
// PATCH operation resolves to update because we have a record present for given PK.
651+
// However, the update fails to execute successfully because the database policy ("@item.pieceid ne 1") for update operation is not satisfied.
599652
string requestBody = @"
600653
{
601-
""name"": ""New Publisher""
654+
""categoryName"": ""SciFi"",
655+
""piecesRequired"": 5,
656+
""piecesAvailable"": 2
602657
}";
603658

604659
await SetupAndRunRestApiTest(
605-
primaryKeyRoute: "id/1234",
660+
primaryKeyRoute: "categoryid/0/pieceid/1",
606661
queryString: null,
607-
entityNameOrPath: _foreignKeyEntityName,
608-
sqlQuery: string.Empty,
609-
operationType: Config.Operation.UpsertIncremental,
662+
entityNameOrPath: _Composite_NonAutoGenPK_EntityPath,
663+
operationType: Config.Operation.Upsert,
610664
requestBody: requestBody,
665+
sqlQuery: string.Empty,
611666
exceptionExpected: true,
612-
expectedErrorMessage: $"Cannot perform INSERT and could not find {_foreignKeyEntityName} with primary key <id: 1234> to perform UPDATE on.",
613-
expectedStatusCode: HttpStatusCode.NotFound,
614-
expectedSubStatusCode: DataApiBuilderException.SubStatusCodes.EntityNotFound.ToString(),
667+
expectedErrorMessage: DataApiBuilderException.AUTHORIZATION_FAILURE,
668+
expectedStatusCode: HttpStatusCode.Forbidden,
669+
expectedSubStatusCode: DataApiBuilderException.SubStatusCodes.DatabasePolicyFailure.ToString(),
615670
clientRoleHeader: "database_policy_tester"
616671
);
672+
}
617673

618-
// Perform PATCH update with update semantics.
619-
Dictionary<string, StringValues> headerDictionary = new()
674+
/// <summary>
675+
/// Test to validate failure of PATCH operation failing to satisfy the database policy for the update operation.
676+
/// (because no record exists for given PK).
677+
/// </summary>
678+
[TestMethod]
679+
public virtual async Task PatchOneInsertWithUnsatisfiedDatabasePolicy()
680+
{
681+
// PATCH operation resolves to insert because we don't have a record present for given PK.
682+
// However, the insert fails to execute successfully because the database policy ("@item.pieceid ne 6 and @item.piecesAvailable gt 6")
683+
// for insert operation is not satisfied.
684+
string requestBody = @"
620685
{
621-
{ "If-Match", "*" }
622-
};
686+
""categoryName"": ""SciFi"",
687+
""piecesRequired"": 5,
688+
""piecesAvailable"": 2
689+
}";
623690

624691
await SetupAndRunRestApiTest(
625-
primaryKeyRoute: "id/1234",
692+
primaryKeyRoute: "categoryid/0/pieceid/6",
626693
queryString: null,
627-
entityNameOrPath: _foreignKeyEntityName,
628-
sqlQuery: string.Empty,
629-
operationType: Config.Operation.UpsertIncremental,
694+
entityNameOrPath: _Composite_NonAutoGenPK_EntityPath,
695+
operationType: Config.Operation.Upsert,
630696
requestBody: requestBody,
631-
headers: new HeaderDictionary(headerDictionary),
697+
sqlQuery: string.Empty,
632698
exceptionExpected: true,
633-
expectedErrorMessage: $"No Update could be performed, record not found",
634-
expectedStatusCode: HttpStatusCode.PreconditionFailed,
635-
expectedSubStatusCode: DataApiBuilderException.SubStatusCodes.DatabaseOperationFailed.ToString(),
699+
expectedErrorMessage: DataApiBuilderException.AUTHORIZATION_FAILURE,
700+
expectedStatusCode: HttpStatusCode.Forbidden,
701+
expectedSubStatusCode: DataApiBuilderException.SubStatusCodes.DatabasePolicyFailure.ToString(),
636702
clientRoleHeader: "database_policy_tester"
637703
);
638-
639704
}
640705

641706
/// <summary>

src/Service.Tests/SqlTests/RestApiTests/Patch/PostgreSqlPatchApiTests.cs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4+
using System;
45
using System.Collections.Generic;
56
using System.Threading.Tasks;
67
using Microsoft.VisualStudio.TestTools.UnitTesting;
@@ -117,6 +118,18 @@ SELECT to_jsonb(subq) AS data
117118
) AS subq
118119
"
119120
},
121+
{
122+
"PatchOneUpdateWithDatabasePolicy",
123+
@"
124+
SELECT to_jsonb(subq) AS data
125+
FROM (
126+
SELECT categoryid, pieceid, ""categoryName"", ""piecesAvailable"", ""piecesRequired""
127+
FROM " + _Composite_NonAutoGenPK_TableName + @"
128+
WHERE categoryid = 100 AND pieceid = 99 AND ""categoryName"" = 'Historical'
129+
AND ""piecesAvailable"" = 4 AND ""piecesRequired"" = 0 AND pieceid != 1
130+
) AS subq
131+
"
132+
},
120133
{
121134
"PatchOne_Update_Default_Test",
122135
@"
@@ -207,6 +220,26 @@ await base.PatchOneViewBadRequestTest(
207220

208221
#region overridden tests
209222

223+
[TestMethod]
224+
[Ignore]
225+
public override Task PatchOneUpdateWithUnsatisfiedDatabasePolicy()
226+
{
227+
throw new NotImplementedException();
228+
}
229+
230+
[TestMethod]
231+
[Ignore]
232+
public override Task PatchOneInsertWithUnsatisfiedDatabasePolicy()
233+
{
234+
throw new NotImplementedException();
235+
}
236+
237+
[TestMethod]
238+
[Ignore]
239+
public override Task PatchOneInsertWithDatabasePolicy()
240+
{
241+
throw new NotImplementedException();
242+
}
210243
#endregion
211244

212245
#region Test Fixture Setup

src/Service.Tests/SqlTests/RestApiTests/Put/MsSqlPutApiTests.cs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,19 @@ public class MsSqlPutApiTests : PutApiTestBase
3030
$"FOR JSON PATH, INCLUDE_NULL_VALUES, WITHOUT_ARRAY_WRAPPER"
3131
},
3232
{
33-
"PutOneUpdateAccessibleRowWithDatabasePolicy",
34-
$"SELECT * FROM { _foreignKeyTableName } " +
35-
$"WHERE id = 2345 AND name = 'New Publisher' AND (id != 1234) " +
33+
"PutOneUpdateWithDatabasePolicy",
34+
$"SELECT [categoryid], [pieceid], [categoryName],[piecesAvailable]," +
35+
$"[piecesRequired] FROM { _Composite_NonAutoGenPK_TableName } " +
36+
$"WHERE [categoryid] = 100 AND [pieceid] = 99 AND [categoryName] = 'SciFi' " +
37+
$"AND [piecesAvailable]= 4 AND [piecesRequired] = 5 AND [pieceid] != 1 " +
38+
$"FOR JSON PATH, INCLUDE_NULL_VALUES, WITHOUT_ARRAY_WRAPPER"
39+
},
40+
{
41+
"PutOneInsertWithDatabasePolicy",
42+
$"SELECT [categoryid], [pieceid], [categoryName],[piecesAvailable]," +
43+
$"[piecesRequired] FROM { _Composite_NonAutoGenPK_TableName } " +
44+
$"WHERE [categoryid] = 0 AND [pieceid] = 7 AND [categoryName] = 'SciFi' " +
45+
$"AND [piecesAvailable]= 4 AND [piecesRequired] = 0 AND ([pieceid] != 6 AND [piecesAvailable] > 0) " +
3646
$"FOR JSON PATH, INCLUDE_NULL_VALUES, WITHOUT_ARRAY_WRAPPER"
3747
},
3848
{

0 commit comments

Comments
 (0)