Skip to content

Conversation

@ayush3797
Copy link
Contributor

@ayush3797 ayush3797 commented Apr 11, 2023

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

@ayush3797 ayush3797 changed the title Dev/agarwalayush/db policy for put patch Database policy for support PUT/PATCH operations - MsSql Apr 11, 2023
@ayush3797 ayush3797 self-assigned this Apr 11, 2023
@ayush3797 ayush3797 added enhancement New feature or request mssql an issue thats specific to mssql auth rest labels Apr 11, 2023
@ayush3797 ayush3797 added this to the Apr2023 milestone Apr 11, 2023
@ayush3797 ayush3797 changed the title Database policy for support PUT/PATCH operations - MsSql Database policy support for PUT/PATCH operations - MsSql Apr 11, 2023
Copy link
Collaborator

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your patience for waiting on the review! LGTM, left a few suggestions.

@ayush3797 ayush3797 enabled auto-merge (squash) May 3, 2023 20:18
@ayush3797 ayush3797 merged commit f6c11d8 into main May 3, 2023
@ayush3797 ayush3797 deleted the dev/agarwalayush/db-policy-for-put-patch branch May 3, 2023 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auth enhancement New feature or request mssql an issue thats specific to mssql rest

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Enhancement]: Support for database policy for PUT/PATCH operations - MsSql

4 participants