From 2b102fd90ad12fa5f8974a7bb4b74238c7978b82 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 20 Oct 2022 02:57:23 +0530 Subject: [PATCH 01/21] updating stored-procedure design doc --- docs/internals/StoredProcedureDesignDoc.md | 86 +++++++++++++++++++++- 1 file changed, 83 insertions(+), 3 deletions(-) diff --git a/docs/internals/StoredProcedureDesignDoc.md b/docs/internals/StoredProcedureDesignDoc.md index 6fcb00e8a8..4235856440 100644 --- a/docs/internals/StoredProcedureDesignDoc.md +++ b/docs/internals/StoredProcedureDesignDoc.md @@ -35,11 +35,20 @@ Thus a stored procedure entity might look like: parameters can either be fixed as above or passed at runtime through - query parameters for GET request - request body for POST, PUT, PATCH, DELETE +- For GraphQL, the calling stored-procedure will look something like this passed in the body +``` +{ + GetBooks(param1:value, param2:value) { + result + } +} +``` > **Spec Interpretation**: > - since not explicitly stated in the specification, request body for GET will be ignored altogether. > - Parameter resolution will go as follows, from highest to lowest priority: request (query string or body) > config defaults > sql defaults > - NOTE: sql defaults not so easy to infer for parameters, so we explicitly require all parameters to be provided either in the request or config in this first version. +> - if the request doesn't contain the parameter values, default values from the config will be picked up. ### Stored Procedure Permissions @@ -122,7 +131,7 @@ Implementation was segmented into 5 main sections: > - Added overriden method to map Sql data type returned from metadata into the CLR/.NET type equivalent. Used/necessary for metadata parsing in `FillSchemaForStoredProcedureAsync()` in `SqlMetadataProvider`. > - Left as TODOs in MySql and Postgres. -### 3. Request Context + Validation +### 3 REST Request Context + Validation > ### `RestRequestContext.cs` > - Since multiple derived classes are implementing/duplicating logic for populating their `FieldValuePairsInBody` dictionary with the Json request body, moved that logic into a method in this class, `PopulateFieldValuePairsInBody`. @@ -150,7 +159,71 @@ Implementation was segmented into 5 main sections: > - there were missing parameters in the request and no default was found in config > - Condition to avoid the `ColumnsPermissionsRequirement` AuthZ check, since specification hasn't defined what this would look like yet -### 4. Structure + Query Building +### 4. GRAPHQL Schema Generation + +> ### `GraphQLSchemaCreator.cs` +> - `GenerateSqlGraphQLObjects` method requires updating to allow it to process stored-procedure (currently it skips). +> - It will have to call a separate method to generate the inputType for Stored Procedure. +> **Question** +> 1. How do we know the result fields since we do not know the result of stored-procedure before hand? +> - Ask users to provide schema for procedures - Not convenient for users. +> - Just return the execution results as Text. - We will go for this one, since we will not support +> pagination/ordering/filtering for stored-procedures in graphQL. +``` +{StoredProcedureName(param1:value1, param2:value2): String} +``` + +> ### `QueryBuilder.cs` and `MutationBuilder.cs` +> - TBH, both query and mutation doesn't seem to relate to stored-Procedure, but it can be any of the two: Query/Mutation. But for now we are considering it to be a query type. +> - It will have a method called `GenerateStoredProcedureQuery` to create the graphql schema for our Stored Procedure. +> - The above method will be similar to `GenerateByPKQuery`,only difference will be that we will use parameters instead of primary keys. +> - it will contain the input fields, i.e. parameters in case of Stored Procedure. +> - we will get the default value from config. +> - For Example: here id represents param +``` +{ + "Execute Stored-Procedure GetBook and get results from the database" + GetBook("parameters for GetBook stored-procedure" id: String = "1"): String +} +``` + +> **Question** +> 1. Should we just keep the return type as String like +``` +{"Get a book from the database by its ID\/primary key" +book_by_pk(id: Int!): book} +``` +> or have a return type with field such as {result: String} like this +``` +{"Get a list of all the book items from the database" +books( + first: Int + after: String + filter: bookFilterInput + orderBy: bookOrderByInput): bookConnection!} + +{"The return object from a filter query that supports a pagination token for paging through results" +type bookConnection { + "The list of items that matched the filter" + items: [book!]! + "A pagination token to provide to subsequent pages of a query" + endCursor: String + "Indicates if there are more pages of items to return" + hasNextPage: Boolean! +}} +``` +> My thoughts: The first one makes more sense since we only need to display the results obtained from the +> stored procedure call, but the second one is useful if in future we decide to support order/pagination/filter on graphQL +> requests for stored-procedure. +> 2. How do we decide if a stored-procedure is Query or mutation? +> This question made me realize how un-natural it feels to add support for stored procedures in graphQL. Currently I'm not +> sure if there is a way to figure out if the call would make any changes in the database or not apart from looking at the read/write permissions. + +### `SchemaConvertor.cs` +> - Add directives for parameter fields added for stored-procedure. +> - Create the directive for parameters for stored-procedure.(**Confirm if that is required**). + +### 5. Structure + Query Building > ### `SqlExecuteStructure.cs` > - Contains all needed info to build an `EXECUTE {stored_proc_name} {parameters}` query @@ -170,7 +243,7 @@ Implementation was segmented into 5 main sections: > ### `MySqlQueryBuilder.cs` & `PostgresQueryBuilder.cs` > - Added method stubs as TODOs for `Build(SqlExecuteStructure)` -### 5. Query Execution + Result Formatting +### 5. REST Query Execution + Result Formatting ### `SqlQueryEngine.cs` > - Separated `ExecuteAsync(RestRequestContext)` into `ExecuteAsync(FindRequestContext)` and `ExecuteAsync(StoredProcedureRequestContext)`. Seems to be better practice than doing type checking and conditional downcasting. @@ -189,6 +262,13 @@ Implementation was segmented into 5 main sections: > - Insert request returns `201 Created` with **first result set** as json response. If none/empty result set, an empty array is returned. Discussion: prefer to instead return no json at all? > - Update/upsert behaves same as insert but with `200 OK` response. +> ### 6. GRAPHQL Query Execution + +> ### `ResolverMiddleware.cs` & `SqlQueryEngine.cs` +> - Call `ExecuteAsync` with `IMiddlewareContext` and `parameters`. +> - We would have to convert `IMiddlewareContext` to `StoredProcedureRequestContext`, similar to how the `dispatchQuery` method in `RestService.cs` is doing. It converts `RestRequestContext` to `StoredProcedureRequestContext`. +> - Then we can simply call `ExecuteAsync(StoredProcedureRequestContext context)` and return the parsed json request. + ## TODO 1. MySql/Postgres support - changes really should be minimal. Foundation is already laid, just may need minor updates to metadata and then obviously adding `Build` methods in respective query builders. 2. Ease up type checking of parameters specified in config. Try to parse them instead of just doing direct type equality check in `SqlMetadataProvider`. From 257be5598ce20d03e2a7aa7d4c92bde4ead69f22 Mon Sep 17 00:00:00 2001 From: abhishekkumams <102276754+abhishekkumams@users.noreply.github.com> Date: Thu, 20 Oct 2022 09:21:28 +0530 Subject: [PATCH 02/21] Update StoredProcedureDesignDoc.md --- docs/internals/StoredProcedureDesignDoc.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/internals/StoredProcedureDesignDoc.md b/docs/internals/StoredProcedureDesignDoc.md index 4235856440..24c91949e9 100644 --- a/docs/internals/StoredProcedureDesignDoc.md +++ b/docs/internals/StoredProcedureDesignDoc.md @@ -39,7 +39,6 @@ parameters can either be fixed as above or passed at runtime through ``` { GetBooks(param1:value, param2:value) { - result } } ``` From 31b73b653561db7526d61871cfec314a4d5134a2 Mon Sep 17 00:00:00 2001 From: abhishekkumams <102276754+abhishekkumams@users.noreply.github.com> Date: Thu, 20 Oct 2022 09:23:27 +0530 Subject: [PATCH 03/21] Update StoredProcedureDesignDoc.md --- docs/internals/StoredProcedureDesignDoc.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/internals/StoredProcedureDesignDoc.md b/docs/internals/StoredProcedureDesignDoc.md index 24c91949e9..e3199b2cf4 100644 --- a/docs/internals/StoredProcedureDesignDoc.md +++ b/docs/internals/StoredProcedureDesignDoc.md @@ -261,6 +261,8 @@ type bookConnection { > - Insert request returns `201 Created` with **first result set** as json response. If none/empty result set, an empty array is returned. Discussion: prefer to instead return no json at all? > - Update/upsert behaves same as insert but with `200 OK` response. +
+ > ### 6. GRAPHQL Query Execution > ### `ResolverMiddleware.cs` & `SqlQueryEngine.cs` From cea68af0a891449b7464afc84242f4c0c6f5c77d Mon Sep 17 00:00:00 2001 From: abhishekkumams <102276754+abhishekkumams@users.noreply.github.com> Date: Thu, 20 Oct 2022 09:24:44 +0530 Subject: [PATCH 04/21] Update StoredProcedureDesignDoc.md --- docs/internals/StoredProcedureDesignDoc.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/docs/internals/StoredProcedureDesignDoc.md b/docs/internals/StoredProcedureDesignDoc.md index e3199b2cf4..ee2bc437d8 100644 --- a/docs/internals/StoredProcedureDesignDoc.md +++ b/docs/internals/StoredProcedureDesignDoc.md @@ -261,9 +261,7 @@ type bookConnection { > - Insert request returns `201 Created` with **first result set** as json response. If none/empty result set, an empty array is returned. Discussion: prefer to instead return no json at all? > - Update/upsert behaves same as insert but with `200 OK` response. -
- -> ### 6. GRAPHQL Query Execution +### 6. GRAPHQL Query Execution > ### `ResolverMiddleware.cs` & `SqlQueryEngine.cs` > - Call `ExecuteAsync` with `IMiddlewareContext` and `parameters`. From 4456a1c04b1caa6bd25382ea57fffd76f13ba594 Mon Sep 17 00:00:00 2001 From: abhishekkumams <102276754+abhishekkumams@users.noreply.github.com> Date: Tue, 25 Oct 2022 14:04:34 +0530 Subject: [PATCH 05/21] Adding examples --- docs/internals/StoredProcedureDesignDoc.md | 36 +++++++--------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/docs/internals/StoredProcedureDesignDoc.md b/docs/internals/StoredProcedureDesignDoc.md index ee2bc437d8..bfd3105489 100644 --- a/docs/internals/StoredProcedureDesignDoc.md +++ b/docs/internals/StoredProcedureDesignDoc.md @@ -186,31 +186,17 @@ Implementation was segmented into 5 main sections: } ``` -> **Question** -> 1. Should we just keep the return type as String like -``` -{"Get a book from the database by its ID\/primary key" -book_by_pk(id: Int!): book} -``` -> or have a return type with field such as {result: String} like this -``` -{"Get a list of all the book items from the database" -books( - first: Int - after: String - filter: bookFilterInput - orderBy: bookOrderByInput): bookConnection!} - -{"The return object from a filter query that supports a pagination token for paging through results" -type bookConnection { - "The list of items that matched the filter" - items: [book!]! - "A pagination token to provide to subsequent pages of a query" - endCursor: String - "Indicates if there are more pages of items to return" - hasNextPage: Boolean! -}} -``` +> ### Example +> Consider stored-procedure `GetBooks` and `GetBook`. the former has no param, while the later has one param, which not provided will pick the value from config. +> Below is the generated documentation from GraphQL +![image](https://user-images.githubusercontent.com/102276754/197724270-241e37e6-6459-4cce-a959-19a5031b63d1.png) + + + +> **Question/Thoughts** +> 1. we just keep the return type as String like + + > My thoughts: The first one makes more sense since we only need to display the results obtained from the > stored procedure call, but the second one is useful if in future we decide to support order/pagination/filter on graphQL > requests for stored-procedure. From a76061e5e16bbfb004aed06cd80bc1fb2c4cbd0f Mon Sep 17 00:00:00 2001 From: abhishekkumams <102276754+abhishekkumams@users.noreply.github.com> Date: Tue, 25 Oct 2022 15:12:03 +0530 Subject: [PATCH 06/21] Adding graphQL response example --- docs/internals/StoredProcedureDesignDoc.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/docs/internals/StoredProcedureDesignDoc.md b/docs/internals/StoredProcedureDesignDoc.md index bfd3105489..641b44177d 100644 --- a/docs/internals/StoredProcedureDesignDoc.md +++ b/docs/internals/StoredProcedureDesignDoc.md @@ -252,7 +252,14 @@ Implementation was segmented into 5 main sections: > ### `ResolverMiddleware.cs` & `SqlQueryEngine.cs` > - Call `ExecuteAsync` with `IMiddlewareContext` and `parameters`. > - We would have to convert `IMiddlewareContext` to `StoredProcedureRequestContext`, similar to how the `dispatchQuery` method in `RestService.cs` is doing. It converts `RestRequestContext` to `StoredProcedureRequestContext`. -> - Then we can simply call `ExecuteAsync(StoredProcedureRequestContext context)` and return the parsed json request. +> - Then we can simply call `ExecuteAsync(StoredProcedureRequestContext context)` and return the parsed json request as String. + +> ### Example +> 1. With param as id +> ![image](https://user-images.githubusercontent.com/102276754/197740155-d6b800aa-acda-4fe2-a82c-1c0fdc45696f.png) +> 2. No param +> ![image](https://user-images.githubusercontent.com/102276754/197740411-747be3e1-a8df-4dc7-8d87-0521b63169da.png) + ## TODO 1. MySql/Postgres support - changes really should be minimal. Foundation is already laid, just may need minor updates to metadata and then obviously adding `Build` methods in respective query builders. From 8b0166a99f3790e5c06fdab5b8414161a67750e8 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 31 Oct 2022 14:57:24 +0530 Subject: [PATCH 07/21] updating examples --- docs/internals/StoredProcedureDesignDoc.md | 38 ++++++++++++---------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/docs/internals/StoredProcedureDesignDoc.md b/docs/internals/StoredProcedureDesignDoc.md index 641b44177d..7f53fe7ebf 100644 --- a/docs/internals/StoredProcedureDesignDoc.md +++ b/docs/internals/StoredProcedureDesignDoc.md @@ -33,9 +33,9 @@ Thus a stored procedure entity might look like: } ``` parameters can either be fixed as above or passed at runtime through -- query parameters for GET request -- request body for POST, PUT, PATCH, DELETE -- For GraphQL, the calling stored-procedure will look something like this passed in the body +- [REST] query parameters for GET request +- [REST] request body for POST, PUT, PATCH, DELETE +- [GRAPHQL] the calling stored-procedure will look something like this passed in the body ``` { GetBooks(param1:value, param2:value) { @@ -173,7 +173,10 @@ Implementation was segmented into 5 main sections: ``` > ### `QueryBuilder.cs` and `MutationBuilder.cs` -> - TBH, both query and mutation doesn't seem to relate to stored-Procedure, but it can be any of the two: Query/Mutation. But for now we are considering it to be a query type. +> - TBH, both query and mutation doesn't seem to relate to stored-Procedure, but every GraphQL request has to be one of the two: Query/Mutation. +> - Q. How the engine will know if a stored-procedure is Query or Mutation? +> - If any roles in the permission has only READ access, then it's a Query, else Mutation. +> #### Query > - It will have a method called `GenerateStoredProcedureQuery` to create the graphql schema for our Stored Procedure. > - The above method will be similar to `GenerateByPKQuery`,only difference will be that we will use parameters instead of primary keys. > - it will contain the input fields, i.e. parameters in case of Stored Procedure. @@ -185,6 +188,17 @@ Implementation was segmented into 5 main sections: GetBook("parameters for GetBook stored-procedure" id: String = "1"): String } ``` +> #### Mutation +> - It will have a method called `GenerateStoredProcedureMutation` to create the graphql schema for our Stored Procedure. +> - The above method will be similar to `GenerateStoredProcedureQuery`, except how the results will be formed. +> - We can't infer what the stored procedure actually did beyond the HasRows and RecordsAffected attributes of the DbDataReader. +> - So, we will check this fields similar to how it is done for the REST calls. +``` +{ + "Execute Stored-Procedure GetBook and get results from the database" + InsertBook("parameters for GetBook stored-procedure" name: String): String +} +``` > ### Example > Consider stored-procedure `GetBooks` and `GetBook`. the former has no param, while the later has one param, which not provided will pick the value from config. @@ -192,18 +206,6 @@ Implementation was segmented into 5 main sections: ![image](https://user-images.githubusercontent.com/102276754/197724270-241e37e6-6459-4cce-a959-19a5031b63d1.png) - -> **Question/Thoughts** -> 1. we just keep the return type as String like - - -> My thoughts: The first one makes more sense since we only need to display the results obtained from the -> stored procedure call, but the second one is useful if in future we decide to support order/pagination/filter on graphQL -> requests for stored-procedure. -> 2. How do we decide if a stored-procedure is Query or mutation? -> This question made me realize how un-natural it feels to add support for stored procedures in graphQL. Currently I'm not -> sure if there is a way to figure out if the call would make any changes in the database or not apart from looking at the read/write permissions. - ### `SchemaConvertor.cs` > - Add directives for parameter fields added for stored-procedure. > - Create the directive for parameters for stored-procedure.(**Confirm if that is required**). @@ -249,7 +251,7 @@ Implementation was segmented into 5 main sections: ### 6. GRAPHQL Query Execution -> ### `ResolverMiddleware.cs` & `SqlQueryEngine.cs` +> ### `ResolverMiddleware.cs`, `SqlQueryEngine.cs`, and `SqlMutationEngine.cs` > - Call `ExecuteAsync` with `IMiddlewareContext` and `parameters`. > - We would have to convert `IMiddlewareContext` to `StoredProcedureRequestContext`, similar to how the `dispatchQuery` method in `RestService.cs` is doing. It converts `RestRequestContext` to `StoredProcedureRequestContext`. > - Then we can simply call `ExecuteAsync(StoredProcedureRequestContext context)` and return the parsed json request as String. @@ -259,6 +261,8 @@ Implementation was segmented into 5 main sections: > ![image](https://user-images.githubusercontent.com/102276754/197740155-d6b800aa-acda-4fe2-a82c-1c0fdc45696f.png) > 2. No param > ![image](https://user-images.githubusercontent.com/102276754/197740411-747be3e1-a8df-4dc7-8d87-0521b63169da.png) +> 3. Mutaion operation +> ## TODO From 841383a6edd39a23ca9d297f90121b7c5801390b Mon Sep 17 00:00:00 2001 From: abhishekkumams <102276754+abhishekkumams@users.noreply.github.com> Date: Mon, 31 Oct 2022 14:58:39 +0530 Subject: [PATCH 08/21] Update StoredProcedureDesignDoc.md --- docs/internals/StoredProcedureDesignDoc.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/internals/StoredProcedureDesignDoc.md b/docs/internals/StoredProcedureDesignDoc.md index 7f53fe7ebf..b31dc2aecb 100644 --- a/docs/internals/StoredProcedureDesignDoc.md +++ b/docs/internals/StoredProcedureDesignDoc.md @@ -262,7 +262,7 @@ Implementation was segmented into 5 main sections: > 2. No param > ![image](https://user-images.githubusercontent.com/102276754/197740411-747be3e1-a8df-4dc7-8d87-0521b63169da.png) > 3. Mutaion operation -> +> ![image](https://user-images.githubusercontent.com/102276754/198975915-f8fefd85-6f8e-4b1b-8fc3-be156084e6ae.png) ## TODO From a15fa5b3c06c091383f06b95dbaf25fcae751442 Mon Sep 17 00:00:00 2001 From: abhishekkumams <102276754+abhishekkumams@users.noreply.github.com> Date: Mon, 31 Oct 2022 15:00:13 +0530 Subject: [PATCH 09/21] Update StoredProcedureDesignDoc.md --- docs/internals/StoredProcedureDesignDoc.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/internals/StoredProcedureDesignDoc.md b/docs/internals/StoredProcedureDesignDoc.md index b31dc2aecb..e478565a1f 100644 --- a/docs/internals/StoredProcedureDesignDoc.md +++ b/docs/internals/StoredProcedureDesignDoc.md @@ -130,7 +130,7 @@ Implementation was segmented into 5 main sections: > - Added overriden method to map Sql data type returned from metadata into the CLR/.NET type equivalent. Used/necessary for metadata parsing in `FillSchemaForStoredProcedureAsync()` in `SqlMetadataProvider`. > - Left as TODOs in MySql and Postgres. -### 3 REST Request Context + Validation +### 3. REST Request Context + Validation > ### `RestRequestContext.cs` > - Since multiple derived classes are implementing/duplicating logic for populating their `FieldValuePairsInBody` dictionary with the Json request body, moved that logic into a method in this class, `PopulateFieldValuePairsInBody`. @@ -261,7 +261,7 @@ Implementation was segmented into 5 main sections: > ![image](https://user-images.githubusercontent.com/102276754/197740155-d6b800aa-acda-4fe2-a82c-1c0fdc45696f.png) > 2. No param > ![image](https://user-images.githubusercontent.com/102276754/197740411-747be3e1-a8df-4dc7-8d87-0521b63169da.png) -> 3. Mutaion operation +> 3. Mutation operation > ![image](https://user-images.githubusercontent.com/102276754/198975915-f8fefd85-6f8e-4b1b-8fc3-be156084e6ae.png) From 397b617ced83f9bf68197aa744dd4b2905058584 Mon Sep 17 00:00:00 2001 From: abhishekkumams <102276754+abhishekkumams@users.noreply.github.com> Date: Wed, 2 Nov 2022 17:37:28 +0530 Subject: [PATCH 10/21] Update StoredProcedureDesignDoc.md --- docs/internals/StoredProcedureDesignDoc.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/docs/internals/StoredProcedureDesignDoc.md b/docs/internals/StoredProcedureDesignDoc.md index e478565a1f..6b4c12e0c6 100644 --- a/docs/internals/StoredProcedureDesignDoc.md +++ b/docs/internals/StoredProcedureDesignDoc.md @@ -206,10 +206,6 @@ Implementation was segmented into 5 main sections: ![image](https://user-images.githubusercontent.com/102276754/197724270-241e37e6-6459-4cce-a959-19a5031b63d1.png) -### `SchemaConvertor.cs` -> - Add directives for parameter fields added for stored-procedure. -> - Create the directive for parameters for stored-procedure.(**Confirm if that is required**). - ### 5. Structure + Query Building > ### `SqlExecuteStructure.cs` From 08e8d7940e879f1cb78dc9e67e708e68e186c81b Mon Sep 17 00:00:00 2001 From: abhishekkumams <102276754+abhishekkumams@users.noreply.github.com> Date: Mon, 14 Nov 2022 14:27:27 +0530 Subject: [PATCH 11/21] Update StoredProcedureDesignDoc.md --- docs/internals/StoredProcedureDesignDoc.md | 34 ++++++++++------------ 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/docs/internals/StoredProcedureDesignDoc.md b/docs/internals/StoredProcedureDesignDoc.md index 6b4c12e0c6..9094131281 100644 --- a/docs/internals/StoredProcedureDesignDoc.md +++ b/docs/internals/StoredProcedureDesignDoc.md @@ -107,9 +107,9 @@ Implementation was segmented into 5 main sections: > ### `DatabaseObject.cs` > - Tables and views have metadata that does not apply to stored procedures, most notably Columns, Primary Keys, and Relationships. -> - As such, we create a `StoredProcedureDefinition` class to hold procedure-relevant info - i.e. procedure parameter metadata. +> - As such, we create a `StoredProcedureDefinition` class to hold procedure-relevant info - i.e. procedure parameter metadata. > - We also add an `ObjectType` attribute on a `DatabaseObject` for more robust checking of whether it represents a table, view, or stored procedure vs. just null-checking the `TableDefinition` and `StoredProcedureDefinition` attributes. -> - The `StoredProcedureDefinition` class houses a Dictionary, `Parameters`, of strings to `ParameterDefinition`s, where the strings are procedure parameter names as defined in sql. +> - The `StoredProcedureDefinition` class houses a Dictionary, `Parameters`, of strings to `ParameterDefinition`s, where the strings are procedure parameter names as defined in sql and `ResultSet` containing resultSet fieldName and ResultSet fieldType of the stpred procedure result. > - `ParameterDefinition` class houses all useful parameter metadata, such as its data type in sql, corresponding CLR/.NET data type, whether it has a default value specified in config, and the default value config defines. It also holds parameter type (IN/OUT), but this isn't currently being used.
@@ -123,6 +123,7 @@ Implementation was segmented into 5 main sections: > - `SELECT * FROM INFORMATION_SCHEMA.PARAMETERS WHERE SPECIFIC_NAME = {procedure_name}` > - Using metadata, we first verify procedures specified in config are present in the database. Then, we verify all parameters for each procedure specified in config are actually present in the database, otherwise we fail runtime initialization. We also verify that any parameters specified in config match the type retrieved from the parameter metadata. > - TODO: more logic is needed for determining whether a value specified in config can be parsed into the metadata type. For example, providing a string in config might be parse-able into a datetime value, but right now it will be rejected. We should relax this constraint at runtime initialization and potentially fall back to request-time validation. +> - For finding the coulmns in the result set of a stored Procedure, we retrieve metadata using `sys.dm_exec_describe_first_result_set_for_object`.
@@ -161,21 +162,16 @@ Implementation was segmented into 5 main sections: ### 4. GRAPHQL Schema Generation > ### `GraphQLSchemaCreator.cs` -> - `GenerateSqlGraphQLObjects` method requires updating to allow it to process stored-procedure (currently it skips). -> - It will have to call a separate method to generate the inputType for Stored Procedure. -> **Question** -> 1. How do we know the result fields since we do not know the result of stored-procedure before hand? -> - Ask users to provide schema for procedures - Not convenient for users. -> - Just return the execution results as Text. - We will go for this one, since we will not support -> pagination/ordering/filtering for stored-procedures in graphQL. +> - `GenerateSqlGraphQLObjects` method requires updating to allow it to process stored-procedure. +> - Here we create StoredProcedure GraphQL Object Types. + ``` -{StoredProcedureName(param1:value1, param2:value2): String} +{StoredProcedureName(param1:value1, param2:value2): StoredProcedureName} ``` > ### `QueryBuilder.cs` and `MutationBuilder.cs` -> - TBH, both query and mutation doesn't seem to relate to stored-Procedure, but every GraphQL request has to be one of the two: Query/Mutation. -> - Q. How the engine will know if a stored-procedure is Query or Mutation? -> - If any roles in the permission has only READ access, then it's a Query, else Mutation. +> - We will allow only single CRUD action to be specified in the config for StoredProcedure. +> - If any roles in the permission has READ access, then it's a Query, else Mutation. > #### Query > - It will have a method called `GenerateStoredProcedureQuery` to create the graphql schema for our Stored Procedure. > - The above method will be similar to `GenerateByPKQuery`,only difference will be that we will use parameters instead of primary keys. @@ -185,7 +181,7 @@ Implementation was segmented into 5 main sections: ``` { "Execute Stored-Procedure GetBook and get results from the database" - GetBook("parameters for GetBook stored-procedure" id: String = "1"): String + GetBook("parameters for GetBook stored-procedure" id: String = "1"): GetBook } ``` > #### Mutation @@ -203,7 +199,7 @@ Implementation was segmented into 5 main sections: > ### Example > Consider stored-procedure `GetBooks` and `GetBook`. the former has no param, while the later has one param, which not provided will pick the value from config. > Below is the generated documentation from GraphQL -![image](https://user-images.githubusercontent.com/102276754/197724270-241e37e6-6459-4cce-a959-19a5031b63d1.png) +!![image](https://user-images.githubusercontent.com/102276754/201593001-46f34d0b-0290-4946-901d-42f6f32b546b.png) ### 5. Structure + Query Building @@ -226,7 +222,7 @@ Implementation was segmented into 5 main sections: > ### `MySqlQueryBuilder.cs` & `PostgresQueryBuilder.cs` > - Added method stubs as TODOs for `Build(SqlExecuteStructure)` -### 5. REST Query Execution + Result Formatting +### 6. REST Query Execution + Result Formatting ### `SqlQueryEngine.cs` > - Separated `ExecuteAsync(RestRequestContext)` into `ExecuteAsync(FindRequestContext)` and `ExecuteAsync(StoredProcedureRequestContext)`. Seems to be better practice than doing type checking and conditional downcasting. @@ -245,7 +241,7 @@ Implementation was segmented into 5 main sections: > - Insert request returns `201 Created` with **first result set** as json response. If none/empty result set, an empty array is returned. Discussion: prefer to instead return no json at all? > - Update/upsert behaves same as insert but with `200 OK` response. -### 6. GRAPHQL Query Execution +### 7. GRAPHQL Query Execution > ### `ResolverMiddleware.cs`, `SqlQueryEngine.cs`, and `SqlMutationEngine.cs` > - Call `ExecuteAsync` with `IMiddlewareContext` and `parameters`. @@ -254,9 +250,9 @@ Implementation was segmented into 5 main sections: > ### Example > 1. With param as id -> ![image](https://user-images.githubusercontent.com/102276754/197740155-d6b800aa-acda-4fe2-a82c-1c0fdc45696f.png) +> ![image](https://user-images.githubusercontent.com/102276754/201590249-57c03f98-2a88-4acd-a951-3e8779df4f4d.png) > 2. No param -> ![image](https://user-images.githubusercontent.com/102276754/197740411-747be3e1-a8df-4dc7-8d87-0521b63169da.png) +> ![image](https://user-images.githubusercontent.com/102276754/201590101-59931ff4-fba8-4901-8a18-9f8a4ffd2cfa.png) > 3. Mutation operation > ![image](https://user-images.githubusercontent.com/102276754/198975915-f8fefd85-6f8e-4b1b-8fc3-be156084e6ae.png) From d54aeb444f5fa55df881a0d674d1e1a8ed13a941 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 25 Nov 2022 14:33:18 +0530 Subject: [PATCH 12/21] updating docs --- docs/internals/StoredProcedureDesignDoc.md | 46 +++++++++++++++------- 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/docs/internals/StoredProcedureDesignDoc.md b/docs/internals/StoredProcedureDesignDoc.md index 9094131281..c754bfec9b 100644 --- a/docs/internals/StoredProcedureDesignDoc.md +++ b/docs/internals/StoredProcedureDesignDoc.md @@ -89,9 +89,14 @@ Justification **against** supporting all CRUD operations: ``` Justification **for** allowing permission configuration for all CRUD operations: - Davide: the "simplification" would complicate authorization with no real benefit. True in that the authorization logic would need to change conditioned on whether the entity source was a stored procedure. -- Aniruddh: we should leave the responsibility for the developer to properly configure hawaii; it's not our fault if they configure against the API guidelines +- Aniruddh: we should leave the responsibility for the developer to properly configure hawaii; it's not our fault if they configure against the API guidelines -**Conclusion**: we treat stored procedures as any other entity when it comes to CRUD support and role/action AuthZ. +Users can provide only one CRUD operation for stored-procedure. However, in cases where the stored-procedure is also returning some values, a READ permission would also be required +or else the stored procedure will execute but no result would be displayed. +One of create/update/delete(CUD) operation is required to qualify as mutation. Along with one of the CUD operation, READ permission could also be given. +But providing more than one (CUD) operation would throw an error. As we don’t know what exactly the stored-procedure will be doing, From DABs perspective, we need just one of the CUD operation to label it as Mutation. + +**Conclusion**: we treat stored procedures as any other entity when it comes to CRUD support and role/action AuthZ except that they can provide only one CRUD operation for stored-procedure alone with READ. ## Implementation Overview Implementation was segmented into 5 main sections: @@ -107,9 +112,9 @@ Implementation was segmented into 5 main sections: > ### `DatabaseObject.cs` > - Tables and views have metadata that does not apply to stored procedures, most notably Columns, Primary Keys, and Relationships. -> - As such, we create a `StoredProcedureDefinition` class to hold procedure-relevant info - i.e. procedure parameter metadata. +> - As such, we create a `StoredProcedureDefinition` class with base class as `SourceDefinition` to hold procedure-relevant info - i.e. procedure parameter metadata. > - We also add an `ObjectType` attribute on a `DatabaseObject` for more robust checking of whether it represents a table, view, or stored procedure vs. just null-checking the `TableDefinition` and `StoredProcedureDefinition` attributes. -> - The `StoredProcedureDefinition` class houses a Dictionary, `Parameters`, of strings to `ParameterDefinition`s, where the strings are procedure parameter names as defined in sql and `ResultSet` containing resultSet fieldName and ResultSet fieldType of the stpred procedure result. +> - The `StoredProcedureDefinition` class houses a Dictionary, `Parameters`, of strings to `ParameterDefinition`s, where the strings are procedure parameter names as defined in sql and SourceDefinition.Columns will store the result set as Columns containing resultSet fieldName and ResultSet fieldType of the stpred procedure result. > - `ParameterDefinition` class houses all useful parameter metadata, such as its data type in sql, corresponding CLR/.NET data type, whether it has a default value specified in config, and the default value config defines. It also holds parameter type (IN/OUT), but this isn't currently being used.
@@ -123,7 +128,7 @@ Implementation was segmented into 5 main sections: > - `SELECT * FROM INFORMATION_SCHEMA.PARAMETERS WHERE SPECIFIC_NAME = {procedure_name}` > - Using metadata, we first verify procedures specified in config are present in the database. Then, we verify all parameters for each procedure specified in config are actually present in the database, otherwise we fail runtime initialization. We also verify that any parameters specified in config match the type retrieved from the parameter metadata. > - TODO: more logic is needed for determining whether a value specified in config can be parsed into the metadata type. For example, providing a string in config might be parse-able into a datetime value, but right now it will be rejected. We should relax this constraint at runtime initialization and potentially fall back to request-time validation. -> - For finding the coulmns in the result set of a stored Procedure, we retrieve metadata using `sys.dm_exec_describe_first_result_set_for_object`. +> - We introduced a new method `PopulateResultSetDefinitionsForStoredProcedureAsync` for finding the coulmns in the result set of a stored Procedure, it retrieves the metadata using `sys.dm_exec_describe_first_result_set_for_object`.
@@ -162,18 +167,22 @@ Implementation was segmented into 5 main sections: ### 4. GRAPHQL Schema Generation > ### `GraphQLSchemaCreator.cs` -> - `GenerateSqlGraphQLObjects` method requires updating to allow it to process stored-procedure. +> - Updated `GenerateSqlGraphQLObjects` method to allow it to process stored-procedure. > - Here we create StoredProcedure GraphQL Object Types. +> - the return type of GraphQL Query/ Mutation will be of List Type with the same name as StoredProcedure name. + +> ### `SchemaConvertor.cs` +- Here we create the objectTypeDefinitionNode, if the stored procedure does not return anything, we add "results" as field +for return type object ``` -{StoredProcedureName(param1:value1, param2:value2): StoredProcedureName} +{StoredProcedureName(param1:value1, param2:value2): [StoredProcedureName!]} ``` > ### `QueryBuilder.cs` and `MutationBuilder.cs` -> - We will allow only single CRUD action to be specified in the config for StoredProcedure. -> - If any roles in the permission has READ access, then it's a Query, else Mutation. +> - If any roles in the permission has READ access, then we generate a GraphQL Query. > #### Query -> - It will have a method called `GenerateStoredProcedureQuery` to create the graphql schema for our Stored Procedure. +> - It will have a method called `GenerateStoredProcedureSchema` to create the graphql schema for our Stored Procedure. > - The above method will be similar to `GenerateByPKQuery`,only difference will be that we will use parameters instead of primary keys. > - it will contain the input fields, i.e. parameters in case of Stored Procedure. > - we will get the default value from config. @@ -185,10 +194,10 @@ Implementation was segmented into 5 main sections: } ``` > #### Mutation -> - It will have a method called `GenerateStoredProcedureMutation` to create the graphql schema for our Stored Procedure. -> - The above method will be similar to `GenerateStoredProcedureQuery`, except how the results will be formed. -> - We can't infer what the stored procedure actually did beyond the HasRows and RecordsAffected attributes of the DbDataReader. -> - So, we will check this fields similar to how it is done for the REST calls. +> - If any roles in the permission has CREATE/UPDATE/DELETE access, then we generate a GraphQL Mutation. +> - It will have a method called `GenerateStoredProcedureSchema` to create the graphql schema for our Stored Procedure. +> - The above method will be called by `AddMutationsForStoredProcedure`, which checks if the roles are allowed for mutation and add it to mutation fields. +> - We simply execute the stored-procedure and if it doesn't return anything or if the user doesn't have read permission empty result array will be returned. ``` { "Execute Stored-Procedure GetBook and get results from the database" @@ -196,7 +205,7 @@ Implementation was segmented into 5 main sections: } ``` -> ### Example +> ### Examples > Consider stored-procedure `GetBooks` and `GetBook`. the former has no param, while the later has one param, which not provided will pick the value from config. > Below is the generated documentation from GraphQL !![image](https://user-images.githubusercontent.com/102276754/201593001-46f34d0b-0290-4946-901d-42f6f32b546b.png) @@ -229,6 +238,7 @@ Implementation was segmented into 5 main sections: > - `ExecuteAsync(StoredProcedureRequestContext)` initializes the `SqlExecuteStructure` with the context's resolved parameters, and calls a new `ExecuteAsync(SqlExecuteStructure)` method. Result returned simply as an `OkResponse` instead of using the `FormatFindResult` method. The pagination methods of `FormatFindResult` don't play well with stored procedures at the moment, since it relies on fields from a `TableDefinition`, but pagination can and should be added to a future version. > - The response from this method will always be a `200 OK` if there were no database errors. If there was no result set returned, we will return an empty json array. **NOTE: Only the first result set returned by the procedure is considered.** > - `ExecuteAsync(SqlExecuteStructure)` is needed because **we have no guarantee the result set is JSON**. As such, we can't use the same strategy that `ExecuteAsync(SqlQueryStructure)` does in using `GetJsonStringFromDbReader`. Instead, we do basically identical fetching as the mutation engine does in using `ExtractRowFromDbDataReader`. As such, one could argue that stored procedures could entirely be the responsibility of the mutation engine. +> - `ExecuteListAsync` is called for stored procedure. here we create an object of `SqlExecuteStructure` and calls `ExecuteAsync(SqlExecuteStructure)`. We format the result as list of JsonDocument. If the user has no read permission, it will return an empty list.
@@ -255,6 +265,12 @@ Implementation was segmented into 5 main sections: > ![image](https://user-images.githubusercontent.com/102276754/201590101-59931ff4-fba8-4901-8a18-9f8a4ffd2cfa.png) > 3. Mutation operation > ![image](https://user-images.githubusercontent.com/102276754/198975915-f8fefd85-6f8e-4b1b-8fc3-be156084e6ae.png) +> 4. InsertMutation Stored Procedure +> ![image](https://user-images.githubusercontent.com/102276754/201590249-57c03f98-2a88-4acd-a951-3e8779df4f4d.png) +> 2. Insert And Select with Read permission +> ![image](https://user-images.githubusercontent.com/102276754/201590101-59931ff4-fba8-4901-8a18-9f8a4ffd2cfa.png) +> 3. Insert And Select without Read permission +> ![image](https://user-images.githubusercontent.com/102276754/198975915-f8fefd85-6f8e-4b1b-8fc3-be156084e6ae.png) ## TODO From a1e6711beaaad332f08c2fd99a0db42787d341af Mon Sep 17 00:00:00 2001 From: abhishekkumams <102276754+abhishekkumams@users.noreply.github.com> Date: Fri, 25 Nov 2022 14:37:26 +0530 Subject: [PATCH 13/21] Update StoredProcedureDesignDoc.md --- docs/internals/StoredProcedureDesignDoc.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/internals/StoredProcedureDesignDoc.md b/docs/internals/StoredProcedureDesignDoc.md index c754bfec9b..76d0e977c4 100644 --- a/docs/internals/StoredProcedureDesignDoc.md +++ b/docs/internals/StoredProcedureDesignDoc.md @@ -259,18 +259,18 @@ for return type object > - Then we can simply call `ExecuteAsync(StoredProcedureRequestContext context)` and return the parsed json request as String. > ### Example -> 1. With param as id +> 1. Query With param as id > ![image](https://user-images.githubusercontent.com/102276754/201590249-57c03f98-2a88-4acd-a951-3e8779df4f4d.png) -> 2. No param +> 2. Query No param > ![image](https://user-images.githubusercontent.com/102276754/201590101-59931ff4-fba8-4901-8a18-9f8a4ffd2cfa.png) -> 3. Mutation operation -> ![image](https://user-images.githubusercontent.com/102276754/198975915-f8fefd85-6f8e-4b1b-8fc3-be156084e6ae.png) -> 4. InsertMutation Stored Procedure -> ![image](https://user-images.githubusercontent.com/102276754/201590249-57c03f98-2a88-4acd-a951-3e8779df4f4d.png) +> 3. Insert Mutation operation +> ![image](https://user-images.githubusercontent.com/102276754/202139861-4e65ba4a-a5db-4c39-9de5-aa10d971e489.png) +> 4. Count rows Stored Procedure +> ![image](https://user-images.githubusercontent.com/102276754/203942475-ac5e1e40-df5d-4708-99b4-9a8c34a4c042.png) > 2. Insert And Select with Read permission -> ![image](https://user-images.githubusercontent.com/102276754/201590101-59931ff4-fba8-4901-8a18-9f8a4ffd2cfa.png) +> ![image](https://user-images.githubusercontent.com/102276754/202140414-6aa1312d-8a83-47b3-a123-ac6da0dcf129.png) > 3. Insert And Select without Read permission -> ![image](https://user-images.githubusercontent.com/102276754/198975915-f8fefd85-6f8e-4b1b-8fc3-be156084e6ae.png) +> ![image](https://user-images.githubusercontent.com/102276754/203942396-8c54cd8e-b217-4eb2-9359-6ce923a428e5.png) ## TODO From 3df9deb64029cdb4850e95e55f57496621ccd316 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 25 Nov 2022 14:51:05 +0530 Subject: [PATCH 14/21] updating docs --- docs/internals/StoredProcedureDesignDoc.md | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/docs/internals/StoredProcedureDesignDoc.md b/docs/internals/StoredProcedureDesignDoc.md index c754bfec9b..44e2f230b8 100644 --- a/docs/internals/StoredProcedureDesignDoc.md +++ b/docs/internals/StoredProcedureDesignDoc.md @@ -35,7 +35,7 @@ Thus a stored procedure entity might look like: parameters can either be fixed as above or passed at runtime through - [REST] query parameters for GET request - [REST] request body for POST, PUT, PATCH, DELETE -- [GRAPHQL] the calling stored-procedure will look something like this passed in the body +- [GRAPHQL] query field arguments in the request body of a GraphQL request ``` { GetBooks(param1:value, param2:value) { @@ -91,7 +91,7 @@ Justification **for** allowing permission configuration for all CRUD operations: - Davide: the "simplification" would complicate authorization with no real benefit. True in that the authorization logic would need to change conditioned on whether the entity source was a stored procedure. - Aniruddh: we should leave the responsibility for the developer to properly configure hawaii; it's not our fault if they configure against the API guidelines -Users can provide only one CRUD operation for stored-procedure. However, in cases where the stored-procedure is also returning some values, a READ permission would also be required +Users can provide only one CRUD operation for stored-procedure. However, Cases where the stored-procedure is also returning some values, a READ permission would also be required or else the stored procedure will execute but no result would be displayed. One of create/update/delete(CUD) operation is required to qualify as mutation. Along with one of the CUD operation, READ permission could also be given. But providing more than one (CUD) operation would throw an error. As we don’t know what exactly the stored-procedure will be doing, From DABs perspective, we need just one of the CUD operation to label it as Mutation. @@ -164,7 +164,7 @@ Implementation was segmented into 5 main sections: > - there were missing parameters in the request and no default was found in config > - Condition to avoid the `ColumnsPermissionsRequirement` AuthZ check, since specification hasn't defined what this would look like yet -### 4. GRAPHQL Schema Generation +### 4. GraphQL Schema Generation > ### `GraphQLSchemaCreator.cs` > - Updated `GenerateSqlGraphQLObjects` method to allow it to process stored-procedure. @@ -172,8 +172,7 @@ Implementation was segmented into 5 main sections: > - the return type of GraphQL Query/ Mutation will be of List Type with the same name as StoredProcedure name. > ### `SchemaConvertor.cs` -- Here we create the objectTypeDefinitionNode, if the stored procedure does not return anything, we add "results" as field -for return type object +- Here we create the objectTypeDefinitionNode, if the stored procedure does not return anything, we add "results" as field for return type object ``` {StoredProcedureName(param1:value1, param2:value2): [StoredProcedureName!]} @@ -238,7 +237,6 @@ for return type object > - `ExecuteAsync(StoredProcedureRequestContext)` initializes the `SqlExecuteStructure` with the context's resolved parameters, and calls a new `ExecuteAsync(SqlExecuteStructure)` method. Result returned simply as an `OkResponse` instead of using the `FormatFindResult` method. The pagination methods of `FormatFindResult` don't play well with stored procedures at the moment, since it relies on fields from a `TableDefinition`, but pagination can and should be added to a future version. > - The response from this method will always be a `200 OK` if there were no database errors. If there was no result set returned, we will return an empty json array. **NOTE: Only the first result set returned by the procedure is considered.** > - `ExecuteAsync(SqlExecuteStructure)` is needed because **we have no guarantee the result set is JSON**. As such, we can't use the same strategy that `ExecuteAsync(SqlQueryStructure)` does in using `GetJsonStringFromDbReader`. Instead, we do basically identical fetching as the mutation engine does in using `ExtractRowFromDbDataReader`. As such, one could argue that stored procedures could entirely be the responsibility of the mutation engine. -> - `ExecuteListAsync` is called for stored procedure. here we create an object of `SqlExecuteStructure` and calls `ExecuteAsync(SqlExecuteStructure)`. We format the result as list of JsonDocument. If the user has no read permission, it will return an empty list.
@@ -251,13 +249,17 @@ for return type object > - Insert request returns `201 Created` with **first result set** as json response. If none/empty result set, an empty array is returned. Discussion: prefer to instead return no json at all? > - Update/upsert behaves same as insert but with `200 OK` response. -### 7. GRAPHQL Query Execution +### 7. GRAPHQL Query Execution + Result Formatting > ### `ResolverMiddleware.cs`, `SqlQueryEngine.cs`, and `SqlMutationEngine.cs` > - Call `ExecuteAsync` with `IMiddlewareContext` and `parameters`. > - We would have to convert `IMiddlewareContext` to `StoredProcedureRequestContext`, similar to how the `dispatchQuery` method in `RestService.cs` is doing. It converts `RestRequestContext` to `StoredProcedureRequestContext`. > - Then we can simply call `ExecuteAsync(StoredProcedureRequestContext context)` and return the parsed json request as String. +> ### `SqlQueryEngine.cs` and `SqlMutationEngine.cs` +> - `ExecuteListAsync` is called where we create an object of `SqlExecuteStructure` and calls `ExecuteAsync(SqlExecuteStructure)`. +> - We format the result as list of JsonDocument. If the user has no read permission, it will return an empty list. + > ### Example > 1. With param as id > ![image](https://user-images.githubusercontent.com/102276754/201590249-57c03f98-2a88-4acd-a951-3e8779df4f4d.png) From cf4362ab9380acdc48022703e6d4cf80d0004ed0 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 29 Nov 2022 15:02:33 +0530 Subject: [PATCH 15/21] updating examples --- docs/internals/StoredProcedureDesignDoc.md | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/internals/StoredProcedureDesignDoc.md b/docs/internals/StoredProcedureDesignDoc.md index 537e559f04..7a6069424e 100644 --- a/docs/internals/StoredProcedureDesignDoc.md +++ b/docs/internals/StoredProcedureDesignDoc.md @@ -91,9 +91,8 @@ Justification **for** allowing permission configuration for all CRUD operations: - Davide: the "simplification" would complicate authorization with no real benefit. True in that the authorization logic would need to change conditioned on whether the entity source was a stored procedure. - Aniruddh: we should leave the responsibility for the developer to properly configure hawaii; it's not our fault if they configure against the API guidelines -Users can provide only one CRUD operation for stored-procedure. However, Cases where the stored-procedure is also returning some values, a READ permission would also be required -or else the stored procedure will execute but no result would be displayed. -One of create/update/delete(CUD) operation is required to qualify as mutation. Along with one of the CUD operation, READ permission could also be given. +Users can provide only one CRUD operation for stored-procedure. +CREATE/UPDATE/DELETE(CUD) action will create mutation operation, while READ will create a Query operation for GraphQL. But providing more than one (CUD) operation would throw an error. As we don’t know what exactly the stored-procedure will be doing, From DABs perspective, we need just one of the CUD operation to label it as Mutation. **Conclusion**: we treat stored procedures as any other entity when it comes to CRUD support and role/action AuthZ except that they can provide only one CRUD operation for stored-procedure alone with READ. @@ -196,7 +195,7 @@ Implementation was segmented into 5 main sections: > - If any roles in the permission has CREATE/UPDATE/DELETE access, then we generate a GraphQL Mutation. > - It will have a method called `GenerateStoredProcedureSchema` to create the graphql schema for our Stored Procedure. > - The above method will be called by `AddMutationsForStoredProcedure`, which checks if the roles are allowed for mutation and add it to mutation fields. -> - We simply execute the stored-procedure and if it doesn't return anything or if the user doesn't have read permission empty result array will be returned. +> - We simply execute the stored-procedure and response is an empty array. ``` { "Execute Stored-Procedure GetBook and get results from the database" @@ -265,14 +264,15 @@ Implementation was segmented into 5 main sections: > ![image](https://user-images.githubusercontent.com/102276754/201590249-57c03f98-2a88-4acd-a951-3e8779df4f4d.png) > 2. Query No param > ![image](https://user-images.githubusercontent.com/102276754/201590101-59931ff4-fba8-4901-8a18-9f8a4ffd2cfa.png) -> 3. Insert Mutation operation +> 3. Insert (Mutation) > ![image](https://user-images.githubusercontent.com/102276754/202139861-4e65ba4a-a5db-4c39-9de5-aa10d971e489.png) -> 4. Count rows Stored Procedure -> ![image](https://user-images.githubusercontent.com/102276754/203942475-ac5e1e40-df5d-4708-99b4-9a8c34a4c042.png) -> 2. Insert And Select with Read permission -> ![image](https://user-images.githubusercontent.com/102276754/202140414-6aa1312d-8a83-47b3-a123-ac6da0dcf129.png) -> 3. Insert And Select without Read permission -> ![image](https://user-images.githubusercontent.com/102276754/203942396-8c54cd8e-b217-4eb2-9359-6ce923a428e5.png) +> 4. Count (Query) +> ![image](https://user-images.githubusercontent.com/102276754/202140115-bc380631-c403-4871-8b40-bfc28438d602.png) +> 5. Update(Mutation) +> ![image](https://user-images.githubusercontent.com/102276754/204460055-a14e6a85-bf89-4547-b62d-8ff6a4c96caf.png) +> 6. Delete(Mutation) +> ![image](https://user-images.githubusercontent.com/102276754/204484061-c32412dd-673a-4b2c-a3b0-ae34f3662b22.png) + ## TODO From 44fc06f27ccde1142b988075414935697652cb64 Mon Sep 17 00:00:00 2001 From: abhishekkumams <102276754+abhishekkumams@users.noreply.github.com> Date: Tue, 29 Nov 2022 22:06:02 +0530 Subject: [PATCH 16/21] Update StoredProcedureDesignDoc.md --- docs/internals/StoredProcedureDesignDoc.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/internals/StoredProcedureDesignDoc.md b/docs/internals/StoredProcedureDesignDoc.md index 7a6069424e..89ae5a1e4d 100644 --- a/docs/internals/StoredProcedureDesignDoc.md +++ b/docs/internals/StoredProcedureDesignDoc.md @@ -93,9 +93,9 @@ Justification **for** allowing permission configuration for all CRUD operations: Users can provide only one CRUD operation for stored-procedure. CREATE/UPDATE/DELETE(CUD) action will create mutation operation, while READ will create a Query operation for GraphQL. -But providing more than one (CUD) operation would throw an error. As we don’t know what exactly the stored-procedure will be doing, From DABs perspective, we need just one of the CUD operation to label it as Mutation. +Providing more than one (CRUD) operation would throw an error and the engine will fail to start during initialization. -**Conclusion**: we treat stored procedures as any other entity when it comes to CRUD support and role/action AuthZ except that they can provide only one CRUD operation for stored-procedure alone with READ. +**Conclusion**: we treat stored procedures as any other entity when it comes to CRUD support and role/action AuthZ. ## Implementation Overview Implementation was segmented into 5 main sections: From ac42bddc3a412159e4c1b7288b3e23844d4d8150 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 6 Dec 2022 10:05:25 +0530 Subject: [PATCH 17/21] fixing doc --- docs/internals/StoredProcedureDesignDoc.md | 47 +++++++++++----------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/docs/internals/StoredProcedureDesignDoc.md b/docs/internals/StoredProcedureDesignDoc.md index 89ae5a1e4d..8272676dfd 100644 --- a/docs/internals/StoredProcedureDesignDoc.md +++ b/docs/internals/StoredProcedureDesignDoc.md @@ -3,7 +3,7 @@ ## Design/Spec Summary ### Entity Source Object Config Currently, the `source` attribute is always a string. With the addition of stored procedure support and the next version of the JSON schema: [here](https://github.com/Azure/project-hawaii/blob/db619b4175719c83d540bc30ef5acc5faa6faa6d/playground/hawaii.draft-02.schema.json), the `source` attribute is now optionally an object with attributes like so: -``` +```json "type": { "type": "string", "enum": [ "table", "view", "stored-procedure" ], @@ -36,7 +36,7 @@ parameters can either be fixed as above or passed at runtime through - [REST] query parameters for GET request - [REST] request body for POST, PUT, PATCH, DELETE - [GRAPHQL] query field arguments in the request body of a GraphQL request -``` +```graphql { GetBooks(param1:value, param2:value) { } @@ -52,7 +52,7 @@ parameters can either be fixed as above or passed at runtime through ### Stored Procedure Permissions Stored procedures have identical role/action permissions to any other entity. I.e. same familiar format: -``` +```json "permissions": [ { "role": "anonymous", @@ -75,7 +75,7 @@ Justification **against** supporting all CRUD operations: - Other solutions usually limit to `POST`: https://learn.microsoft.com/rest/api/cosmos-db/execute-a-stored-procedure - [PostgREST](https://postgrest.org/en/stable/api.html#stored-procedures) support `POST` and `GET` if marked `IMMUTABLE` or `STABLE` - Proposed permission: -``` +```json "permissions": [ { "role": "anonymous", @@ -113,21 +113,21 @@ Implementation was segmented into 5 main sections: > - Tables and views have metadata that does not apply to stored procedures, most notably Columns, Primary Keys, and Relationships. > - As such, we create a `StoredProcedureDefinition` class with base class as `SourceDefinition` to hold procedure-relevant info - i.e. procedure parameter metadata. > - We also add an `ObjectType` attribute on a `DatabaseObject` for more robust checking of whether it represents a table, view, or stored procedure vs. just null-checking the `TableDefinition` and `StoredProcedureDefinition` attributes. -> - The `StoredProcedureDefinition` class houses a Dictionary, `Parameters`, of strings to `ParameterDefinition`s, where the strings are procedure parameter names as defined in sql and SourceDefinition.Columns will store the result set as Columns containing resultSet fieldName and ResultSet fieldType of the stpred procedure result. -> - `ParameterDefinition` class houses all useful parameter metadata, such as its data type in sql, corresponding CLR/.NET data type, whether it has a default value specified in config, and the default value config defines. It also holds parameter type (IN/OUT), but this isn't currently being used. +> - The `StoredProcedureDefinition` class houses a Dictionary, `Parameters` which will contain the parameterName as Key and value will be the default param value and `SourceDefinition.Columns` will store the result set as Columns containing resultSet fieldName and ResultSet fieldType of the stored procedure result. +> - `ParameterDefinition` class inside `DatabaseObject` class houses all useful parameter metadata, such as its data type in sql, corresponding CLR/.NET data type, whether it has a default value specified in config, and the default value config defines. It also holds parameter type (IN/OUT), but this isn't currently being used.
> ### `SqlMetadataProvider.cs` > - Problem: when we draw metadata from the database schema, we implicitly check if each entity specified in config exists in the database. Path: in `Startup.cs`, the `PerformOnConfigChangeAsync()` method invokes `InitializeAsync()` of the metadata provider bound at runtime, which then invokes `PopulateTableDefinitionForEntities()`. Several steps down the stack `FillSchemaForTableAsync()` is called, which performs a `SELECT * FROM {table_name}` for the given entity and adds the resulting `DataTable` object to the `EntitiesDataSet` class variable. Unfortunately, stored procedure metadata cannot be queried using the same syntax. As such, running the select statement on a stored procedure source name will cause a Sql exception to surface and result in runtime initialization failure. -> - Instead, we introduce the `PopulateStoredProcedureDefinitionForEntities()` method, which iterates over only entites labeled as stored procedures to fill their metadata, and we simply skip over entities labeled as Stored Procedures in the `PopulateTableDefinitionForEntities()` method. +> - Instead, we introduce the `PopulateStoredProcedureDefinitionForEntities()` method, which iterates over only entities labeled as stored procedures to fill their metadata, and we simply skip over entities labeled as Stored Procedures in the `PopulateTableDefinitionForEntities()` method. > - We use the ADO.NET `GetSchemaAsync` method to retrieve procedure metadata and parameter metadata from the Procedures and Procedure Parameters collections respectively: https://learn.microsoft.com/dotnet/framework/data/adonet/sql-server-schema-collections. These collections are listed as SQL Server-specific, so this implementation might not be extensible to MySQL and Postgres. > - In this case, we should just directly access the ANSI-standard information_schema and retrieve metadata using: > - `SELECT * FROM INFORMATION_SCHEMA.ROUTINES WHERE SPECIFIC_NAME = {procedure_name}` > - `SELECT * FROM INFORMATION_SCHEMA.PARAMETERS WHERE SPECIFIC_NAME = {procedure_name}` > - Using metadata, we first verify procedures specified in config are present in the database. Then, we verify all parameters for each procedure specified in config are actually present in the database, otherwise we fail runtime initialization. We also verify that any parameters specified in config match the type retrieved from the parameter metadata. > - TODO: more logic is needed for determining whether a value specified in config can be parsed into the metadata type. For example, providing a string in config might be parse-able into a datetime value, but right now it will be rejected. We should relax this constraint at runtime initialization and potentially fall back to request-time validation. -> - We introduced a new method `PopulateResultSetDefinitionsForStoredProcedureAsync` for finding the coulmns in the result set of a stored Procedure, it retrieves the metadata using `sys.dm_exec_describe_first_result_set_for_object`. +> - Added the method `PopulateResultSetDefinitionsForStoredProcedureAsync()` which retrieves stored procedure metadata using SQL Server's `sys.dm_exec_describe_first_result_set_for_object` and then uses the result set column definitions from that metadata
@@ -166,37 +166,36 @@ Implementation was segmented into 5 main sections: ### 4. GraphQL Schema Generation > ### `GraphQLSchemaCreator.cs` -> - Updated `GenerateSqlGraphQLObjects` method to allow it to process stored-procedure. -> - Here we create StoredProcedure GraphQL Object Types. -> - the return type of GraphQL Query/ Mutation will be of List Type with the same name as StoredProcedure name. +> - Updated `GenerateSqlGraphQLObjects` method to allow it to process stored-procedure metadata retrieved from SQL Server. +> - Create StoredProcedure GraphQL Object Types for use in the generated GraphQL schema. +> - The return type of the generated GraphQL Query/Mutation is `List listName` where `listName` is the stored procedure name. > ### `SchemaConvertor.cs` -- Here we create the objectTypeDefinitionNode, if the stored procedure does not return anything, we add "results" as field for return type object +- Create the `objectTypeDefinitionNode` to represent the columns of the stored-procedure result. If the stored procedure does not return anything, the return type object (`objectTypeDefinitionNode`) will only have one field added named `results`. -``` +```graphql {StoredProcedureName(param1:value1, param2:value2): [StoredProcedureName!]} ``` > ### `QueryBuilder.cs` and `MutationBuilder.cs` -> - If any roles in the permission has READ access, then we generate a GraphQL Query. +> - If the stored-procedure entity in the runtime config defines a role permission allowing READ access, a GraphQL Query field is created to be included in the generated GraphQL schema. > #### Query -> - It will have a method called `GenerateStoredProcedureSchema` to create the graphql schema for our Stored Procedure. -> - The above method will be similar to `GenerateByPKQuery`,only difference will be that we will use parameters instead of primary keys. -> - it will contain the input fields, i.e. parameters in case of Stored Procedure. -> - we will get the default value from config. +> - `GraphQLStoredProcedureBuilder` contains the method called `GenerateStoredProcedureSchema` which gets called inside QueryBuilder class to create the GraphQL schema for our Stored Procedure. +> - The method `GenerateStoredProcedureSchema` is similar to `GenerateByPKQuery`, where the only difference is that we will use procedure parameters instead of primary keys as arguments. +> - Default values for procedure parameters will be sourced from the runtime config. > - For Example: here id represents param -``` +```graphql { "Execute Stored-Procedure GetBook and get results from the database" GetBook("parameters for GetBook stored-procedure" id: String = "1"): GetBook } ``` > #### Mutation -> - If any roles in the permission has CREATE/UPDATE/DELETE access, then we generate a GraphQL Mutation. -> - It will have a method called `GenerateStoredProcedureSchema` to create the graphql schema for our Stored Procedure. -> - The above method will be called by `AddMutationsForStoredProcedure`, which checks if the roles are allowed for mutation and add it to mutation fields. -> - We simply execute the stored-procedure and response is an empty array. -``` +> - If the stored-procedure entity in the runtime config defines a role permission allowing CREATE/UPDATE/DELETE access, a GraphQL Mutation field is created to be included in the generated GraphQL schema. +> - MutationBuilder calls the method `GenerateStoredProcedureSchema` to create the GraphQL schema for our Stored Procedure. +> - The method `GenerateStoredProcedureSchema` will be called by `AddMutationsForStoredProcedure` to add the generated stored procedure mutation field to the GraphQL schema when mutations are permitted by any of the stored procedures role permissions in the runtime config. +> - Response for Mutation operation in stored-procedure will be an empty array. +```graphql { "Execute Stored-Procedure GetBook and get results from the database" InsertBook("parameters for GetBook stored-procedure" name: String): String From cce08f3115148ad09d01999f7fe402f05ea4a58d Mon Sep 17 00:00:00 2001 From: abhishekkumams <102276754+abhishekkumams@users.noreply.github.com> Date: Tue, 6 Dec 2022 10:19:06 +0530 Subject: [PATCH 18/21] Update docs/internals/StoredProcedureDesignDoc.md Co-authored-by: Ravi Shetye --- docs/internals/StoredProcedureDesignDoc.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/internals/StoredProcedureDesignDoc.md b/docs/internals/StoredProcedureDesignDoc.md index 8272676dfd..96e1f0d17c 100644 --- a/docs/internals/StoredProcedureDesignDoc.md +++ b/docs/internals/StoredProcedureDesignDoc.md @@ -51,7 +51,7 @@ parameters can either be fixed as above or passed at runtime through ### Stored Procedure Permissions -Stored procedures have identical role/action permissions to any other entity. I.e. same familiar format: +Stored procedures have identical role/action permissions to any other entity. i.e. same familiar format: ```json "permissions": [ { From a085254071fa3cf37d1d1d7f401bedbf522ed612 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 6 Dec 2022 10:58:26 +0530 Subject: [PATCH 19/21] fixing quotation --- docs/internals/StoredProcedureDesignDoc.md | 36 +++++++++++----------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/docs/internals/StoredProcedureDesignDoc.md b/docs/internals/StoredProcedureDesignDoc.md index 8272676dfd..3018c6a502 100644 --- a/docs/internals/StoredProcedureDesignDoc.md +++ b/docs/internals/StoredProcedureDesignDoc.md @@ -51,7 +51,7 @@ parameters can either be fixed as above or passed at runtime through ### Stored Procedure Permissions -Stored procedures have identical role/action permissions to any other entity. I.e. same familiar format: +Stored procedures have identical role/action permissions to any other entity. i.e. same familiar format: ```json "permissions": [ { @@ -101,7 +101,7 @@ Providing more than one (CRUD) operation would throw an error and the engine wil Implementation was segmented into 5 main sections: ### 1. Support in Config -> ### `Entity.cs` +#### `Entity.cs` > - Exposing the `DatabaseObjectSource` object rather than `source` being a simple string. > - Stored procedures must be specified as "stored-procedure" source type > - See [sample source config](#sample-source) @@ -109,7 +109,7 @@ Implementation was segmented into 5 main sections: ### 2. Metadata Validation -> ### `DatabaseObject.cs` +#### `DatabaseObject.cs` > - Tables and views have metadata that does not apply to stored procedures, most notably Columns, Primary Keys, and Relationships. > - As such, we create a `StoredProcedureDefinition` class with base class as `SourceDefinition` to hold procedure-relevant info - i.e. procedure parameter metadata. > - We also add an `ObjectType` attribute on a `DatabaseObject` for more robust checking of whether it represents a table, view, or stored procedure vs. just null-checking the `TableDefinition` and `StoredProcedureDefinition` attributes. @@ -118,7 +118,7 @@ Implementation was segmented into 5 main sections:
-> ### `SqlMetadataProvider.cs` +#### `SqlMetadataProvider.cs` > - Problem: when we draw metadata from the database schema, we implicitly check if each entity specified in config exists in the database. Path: in `Startup.cs`, the `PerformOnConfigChangeAsync()` method invokes `InitializeAsync()` of the metadata provider bound at runtime, which then invokes `PopulateTableDefinitionForEntities()`. Several steps down the stack `FillSchemaForTableAsync()` is called, which performs a `SELECT * FROM {table_name}` for the given entity and adds the resulting `DataTable` object to the `EntitiesDataSet` class variable. Unfortunately, stored procedure metadata cannot be queried using the same syntax. As such, running the select statement on a stored procedure source name will cause a Sql exception to surface and result in runtime initialization failure. > - Instead, we introduce the `PopulateStoredProcedureDefinitionForEntities()` method, which iterates over only entities labeled as stored procedures to fill their metadata, and we simply skip over entities labeled as Stored Procedures in the `PopulateTableDefinitionForEntities()` method. > - We use the ADO.NET `GetSchemaAsync` method to retrieve procedure metadata and parameter metadata from the Procedures and Procedure Parameters collections respectively: https://learn.microsoft.com/dotnet/framework/data/adonet/sql-server-schema-collections. These collections are listed as SQL Server-specific, so this implementation might not be extensible to MySQL and Postgres. @@ -131,19 +131,19 @@ Implementation was segmented into 5 main sections:
-> ### `MsSqlMetadataProvider.cs`, `MySqlMetadataProvider.cs`, & `PostgreSqlMetadataProvider.cs` +#### `MsSqlMetadataProvider.cs`, `MySqlMetadataProvider.cs`, & `PostgreSqlMetadataProvider.cs` > - Added overriden method to map Sql data type returned from metadata into the CLR/.NET type equivalent. Used/necessary for metadata parsing in `FillSchemaForStoredProcedureAsync()` in `SqlMetadataProvider`. > - Left as TODOs in MySql and Postgres. ### 3. REST Request Context + Validation -> ### `RestRequestContext.cs` +#### `RestRequestContext.cs` > - Since multiple derived classes are implementing/duplicating logic for populating their `FieldValuePairsInBody` dictionary with the Json request body, moved that logic into a method in this class, `PopulateFieldValuePairsInBody`. > - Added `DispatchExecute(IQueryEngine)` and `DispatchExecute(IMutationEngine)` as virtual methods, as an implementation of the visitor pattern/double dispatch strategy. Helps prevent the sometimes-bad practice of downcasting in calling the overloaded `ExecuteAsync` methods in the query and mutation engines (between `FindRequestContext` and `StoredProcedureRequestContext` in the query engine, for example).
-> ### `StoredProcedureRequestContext.cs` +#### `StoredProcedureRequestContext.cs` > - Since it was agreed not to change the Operation enum, as we are keeping the same general authorization logic, we need a way to conditionally split constructing and building the appropriate query structure in the query and mutation engine. The best way I found to do so was introduce a request context representing a stored procedure request for both Query and Mutation scenarios. > - Populates the request body on construction. > - Contains `PopulateResolvedParameters` method to populate its `ResolvedParameters` dictionary, which houses the final, resolved parameters that will be passed in constructing the sql query. Populates this dictionary with the query string or request body depending on the `OperationType` field. Should only be called after `ParsedQueryString` and/or `FieldValuePairsInBody` are appropriately populated. @@ -151,7 +151,7 @@ Implementation was segmented into 5 main sections:
-> ### `RestService.cs` +#### `RestService.cs` > - Condition in `ExecuteAsync` based on the database object type whether the entity type requested is a stored procedure. > - If so, initialize a `StoredProcedureRequestContext`. > - If we have a read operation, parse the query string into `ParsedQueryString`. Note: for stored procedures, ODataFilters do not apply for this iteration. As such, keys like `$filter` and `$select` will be treated as any other parameters. Request body is ignored, as we pass in `null` to constructor. @@ -165,13 +165,13 @@ Implementation was segmented into 5 main sections: ### 4. GraphQL Schema Generation -> ### `GraphQLSchemaCreator.cs` +#### `GraphQLSchemaCreator.cs` > - Updated `GenerateSqlGraphQLObjects` method to allow it to process stored-procedure metadata retrieved from SQL Server. > - Create StoredProcedure GraphQL Object Types for use in the generated GraphQL schema. > - The return type of the generated GraphQL Query/Mutation is `List listName` where `listName` is the stored procedure name. -> ### `SchemaConvertor.cs` -- Create the `objectTypeDefinitionNode` to represent the columns of the stored-procedure result. If the stored procedure does not return anything, the return type object (`objectTypeDefinitionNode`) will only have one field added named `results`. +#### `SchemaConvertor.cs` +> - Create the `objectTypeDefinitionNode` to represent the columns of the stored-procedure result. If the stored procedure does not return anything, the return type object (`objectTypeDefinitionNode`) will only have one field added named `results`. ```graphql {StoredProcedureName(param1:value1, param2:value2): [StoredProcedureName!]} @@ -210,7 +210,7 @@ Implementation was segmented into 5 main sections: ### 5. Structure + Query Building -> ### `SqlExecuteStructure.cs` +#### `SqlExecuteStructure.cs` > - Contains all needed info to build an `EXECUTE {stored_proc_name} {parameters}` query > - Contains a dictionary, `ProcedureParameters`, mapping stored procedure parameter names to engine-generated parameterized values. I.e. keys are the user-defined procedure parameters (@id), and values are @param0, @param1... > - Constructor populates `ProcedureParameters` dictionary and the base class's `Parameters` dictionary mapping parameterized values to the procedure parameter values. Confusing, I know. @@ -218,19 +218,19 @@ Implementation was segmented into 5 main sections:
-> ### `MsSqlQueryBuilder.cs` +#### `MsSqlQueryBuilder.cs` > - Added the `Build(SqlExecuteStructure)` that builds the query string for execute requests as `EXECUTE {schema_name}.{stored_proc_name} {parameters}` > - Added `BuildProcedureParameterList` to build the list of parameters from the `ProcedureParameters` dictionary. The result of this method might look like `@id = @param0, @title = @param1`. > - Found the naive string concatenation implementation faster than StringBuilder or Linq + string.Join
-> ### `MySqlQueryBuilder.cs` & `PostgresQueryBuilder.cs` +#### `MySqlQueryBuilder.cs` & `PostgresQueryBuilder.cs` > - Added method stubs as TODOs for `Build(SqlExecuteStructure)` ### 6. REST Query Execution + Result Formatting -### `SqlQueryEngine.cs` +#### `SqlQueryEngine.cs` > - Separated `ExecuteAsync(RestRequestContext)` into `ExecuteAsync(FindRequestContext)` and `ExecuteAsync(StoredProcedureRequestContext)`. Seems to be better practice than doing type checking and conditional downcasting. > - `ExecuteAsync(StoredProcedureRequestContext)` initializes the `SqlExecuteStructure` with the context's resolved parameters, and calls a new `ExecuteAsync(SqlExecuteStructure)` method. Result returned simply as an `OkResponse` instead of using the `FormatFindResult` method. The pagination methods of `FormatFindResult` don't play well with stored procedures at the moment, since it relies on fields from a `TableDefinition`, but pagination can and should be added to a future version. > - The response from this method will always be a `200 OK` if there were no database errors. If there was no result set returned, we will return an empty json array. **NOTE: Only the first result set returned by the procedure is considered.** @@ -238,7 +238,7 @@ Implementation was segmented into 5 main sections:
-### `SqlMutationEngine.cs` +#### `SqlMutationEngine.cs` > - Instead of trying to refactor `ExecuteAsync(RestRequestContext)` and `PerformMutationOperation`, which conditionally initializes query structures and builds them based on context `OperationType` - which is not a distinguishing factor of a stored procedure request - I found it easier and clearer to add an overloaded `ExecuteAsync(StoredProcedureRequestContext)`, which will be responsible for all stored procedure mutation requests (POST, PUT, PATCH, DELETE). > - In `ExecuteAsync(StoredProcedureRequestContext)`, we initialize the structure and build it the same way as in the query engine. The difference comes in formatting the response. We're leaving it up to the developers to configure their procedures appropriately and to call them with the appropriate HTTP verb. As such, we will return the expected response for each successful request based on its Operation. @@ -249,12 +249,12 @@ Implementation was segmented into 5 main sections: ### 7. GRAPHQL Query Execution + Result Formatting -> ### `ResolverMiddleware.cs`, `SqlQueryEngine.cs`, and `SqlMutationEngine.cs` +#### `ResolverMiddleware.cs`, `SqlQueryEngine.cs`, and `SqlMutationEngine.cs` > - Call `ExecuteAsync` with `IMiddlewareContext` and `parameters`. > - We would have to convert `IMiddlewareContext` to `StoredProcedureRequestContext`, similar to how the `dispatchQuery` method in `RestService.cs` is doing. It converts `RestRequestContext` to `StoredProcedureRequestContext`. > - Then we can simply call `ExecuteAsync(StoredProcedureRequestContext context)` and return the parsed json request as String. -> ### `SqlQueryEngine.cs` and `SqlMutationEngine.cs` +#### `SqlQueryEngine.cs` and `SqlMutationEngine.cs` > - `ExecuteListAsync` is called where we create an object of `SqlExecuteStructure` and calls `ExecuteAsync(SqlExecuteStructure)`. > - We format the result as list of JsonDocument. If the user has no read permission, it will return an empty list. From 9667c9b91cb3ba6592f51b45606f599ec0a8848d Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 6 Dec 2022 11:04:17 +0530 Subject: [PATCH 20/21] fixing quotation --- docs/internals/StoredProcedureDesignDoc.md | 34 +++++++++++----------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/docs/internals/StoredProcedureDesignDoc.md b/docs/internals/StoredProcedureDesignDoc.md index 3018c6a502..1052ffeaf7 100644 --- a/docs/internals/StoredProcedureDesignDoc.md +++ b/docs/internals/StoredProcedureDesignDoc.md @@ -101,7 +101,7 @@ Providing more than one (CRUD) operation would throw an error and the engine wil Implementation was segmented into 5 main sections: ### 1. Support in Config -#### `Entity.cs` +### `Entity.cs` > - Exposing the `DatabaseObjectSource` object rather than `source` being a simple string. > - Stored procedures must be specified as "stored-procedure" source type > - See [sample source config](#sample-source) @@ -109,7 +109,7 @@ Implementation was segmented into 5 main sections: ### 2. Metadata Validation -#### `DatabaseObject.cs` +### `DatabaseObject.cs` > - Tables and views have metadata that does not apply to stored procedures, most notably Columns, Primary Keys, and Relationships. > - As such, we create a `StoredProcedureDefinition` class with base class as `SourceDefinition` to hold procedure-relevant info - i.e. procedure parameter metadata. > - We also add an `ObjectType` attribute on a `DatabaseObject` for more robust checking of whether it represents a table, view, or stored procedure vs. just null-checking the `TableDefinition` and `StoredProcedureDefinition` attributes. @@ -118,7 +118,7 @@ Implementation was segmented into 5 main sections:
-#### `SqlMetadataProvider.cs` +### `SqlMetadataProvider.cs` > - Problem: when we draw metadata from the database schema, we implicitly check if each entity specified in config exists in the database. Path: in `Startup.cs`, the `PerformOnConfigChangeAsync()` method invokes `InitializeAsync()` of the metadata provider bound at runtime, which then invokes `PopulateTableDefinitionForEntities()`. Several steps down the stack `FillSchemaForTableAsync()` is called, which performs a `SELECT * FROM {table_name}` for the given entity and adds the resulting `DataTable` object to the `EntitiesDataSet` class variable. Unfortunately, stored procedure metadata cannot be queried using the same syntax. As such, running the select statement on a stored procedure source name will cause a Sql exception to surface and result in runtime initialization failure. > - Instead, we introduce the `PopulateStoredProcedureDefinitionForEntities()` method, which iterates over only entities labeled as stored procedures to fill their metadata, and we simply skip over entities labeled as Stored Procedures in the `PopulateTableDefinitionForEntities()` method. > - We use the ADO.NET `GetSchemaAsync` method to retrieve procedure metadata and parameter metadata from the Procedures and Procedure Parameters collections respectively: https://learn.microsoft.com/dotnet/framework/data/adonet/sql-server-schema-collections. These collections are listed as SQL Server-specific, so this implementation might not be extensible to MySQL and Postgres. @@ -131,19 +131,19 @@ Implementation was segmented into 5 main sections:
-#### `MsSqlMetadataProvider.cs`, `MySqlMetadataProvider.cs`, & `PostgreSqlMetadataProvider.cs` +### `MsSqlMetadataProvider.cs`, `MySqlMetadataProvider.cs`, & `PostgreSqlMetadataProvider.cs` > - Added overriden method to map Sql data type returned from metadata into the CLR/.NET type equivalent. Used/necessary for metadata parsing in `FillSchemaForStoredProcedureAsync()` in `SqlMetadataProvider`. > - Left as TODOs in MySql and Postgres. ### 3. REST Request Context + Validation -#### `RestRequestContext.cs` +### `RestRequestContext.cs` > - Since multiple derived classes are implementing/duplicating logic for populating their `FieldValuePairsInBody` dictionary with the Json request body, moved that logic into a method in this class, `PopulateFieldValuePairsInBody`. > - Added `DispatchExecute(IQueryEngine)` and `DispatchExecute(IMutationEngine)` as virtual methods, as an implementation of the visitor pattern/double dispatch strategy. Helps prevent the sometimes-bad practice of downcasting in calling the overloaded `ExecuteAsync` methods in the query and mutation engines (between `FindRequestContext` and `StoredProcedureRequestContext` in the query engine, for example).
-#### `StoredProcedureRequestContext.cs` +### `StoredProcedureRequestContext.cs` > - Since it was agreed not to change the Operation enum, as we are keeping the same general authorization logic, we need a way to conditionally split constructing and building the appropriate query structure in the query and mutation engine. The best way I found to do so was introduce a request context representing a stored procedure request for both Query and Mutation scenarios. > - Populates the request body on construction. > - Contains `PopulateResolvedParameters` method to populate its `ResolvedParameters` dictionary, which houses the final, resolved parameters that will be passed in constructing the sql query. Populates this dictionary with the query string or request body depending on the `OperationType` field. Should only be called after `ParsedQueryString` and/or `FieldValuePairsInBody` are appropriately populated. @@ -151,7 +151,7 @@ Implementation was segmented into 5 main sections:
-#### `RestService.cs` +### `RestService.cs` > - Condition in `ExecuteAsync` based on the database object type whether the entity type requested is a stored procedure. > - If so, initialize a `StoredProcedureRequestContext`. > - If we have a read operation, parse the query string into `ParsedQueryString`. Note: for stored procedures, ODataFilters do not apply for this iteration. As such, keys like `$filter` and `$select` will be treated as any other parameters. Request body is ignored, as we pass in `null` to constructor. @@ -165,19 +165,19 @@ Implementation was segmented into 5 main sections: ### 4. GraphQL Schema Generation -#### `GraphQLSchemaCreator.cs` +### `GraphQLSchemaCreator.cs` > - Updated `GenerateSqlGraphQLObjects` method to allow it to process stored-procedure metadata retrieved from SQL Server. > - Create StoredProcedure GraphQL Object Types for use in the generated GraphQL schema. > - The return type of the generated GraphQL Query/Mutation is `List listName` where `listName` is the stored procedure name. -#### `SchemaConvertor.cs` +### `SchemaConvertor.cs` > - Create the `objectTypeDefinitionNode` to represent the columns of the stored-procedure result. If the stored procedure does not return anything, the return type object (`objectTypeDefinitionNode`) will only have one field added named `results`. ```graphql {StoredProcedureName(param1:value1, param2:value2): [StoredProcedureName!]} ``` -> ### `QueryBuilder.cs` and `MutationBuilder.cs` +### `QueryBuilder.cs` and `MutationBuilder.cs` > - If the stored-procedure entity in the runtime config defines a role permission allowing READ access, a GraphQL Query field is created to be included in the generated GraphQL schema. > #### Query > - `GraphQLStoredProcedureBuilder` contains the method called `GenerateStoredProcedureSchema` which gets called inside QueryBuilder class to create the GraphQL schema for our Stored Procedure. @@ -210,7 +210,7 @@ Implementation was segmented into 5 main sections: ### 5. Structure + Query Building -#### `SqlExecuteStructure.cs` +### `SqlExecuteStructure.cs` > - Contains all needed info to build an `EXECUTE {stored_proc_name} {parameters}` query > - Contains a dictionary, `ProcedureParameters`, mapping stored procedure parameter names to engine-generated parameterized values. I.e. keys are the user-defined procedure parameters (@id), and values are @param0, @param1... > - Constructor populates `ProcedureParameters` dictionary and the base class's `Parameters` dictionary mapping parameterized values to the procedure parameter values. Confusing, I know. @@ -218,19 +218,19 @@ Implementation was segmented into 5 main sections:
-#### `MsSqlQueryBuilder.cs` +### `MsSqlQueryBuilder.cs` > - Added the `Build(SqlExecuteStructure)` that builds the query string for execute requests as `EXECUTE {schema_name}.{stored_proc_name} {parameters}` > - Added `BuildProcedureParameterList` to build the list of parameters from the `ProcedureParameters` dictionary. The result of this method might look like `@id = @param0, @title = @param1`. > - Found the naive string concatenation implementation faster than StringBuilder or Linq + string.Join
-#### `MySqlQueryBuilder.cs` & `PostgresQueryBuilder.cs` +### `MySqlQueryBuilder.cs` & `PostgresQueryBuilder.cs` > - Added method stubs as TODOs for `Build(SqlExecuteStructure)` ### 6. REST Query Execution + Result Formatting -#### `SqlQueryEngine.cs` +### `SqlQueryEngine.cs` > - Separated `ExecuteAsync(RestRequestContext)` into `ExecuteAsync(FindRequestContext)` and `ExecuteAsync(StoredProcedureRequestContext)`. Seems to be better practice than doing type checking and conditional downcasting. > - `ExecuteAsync(StoredProcedureRequestContext)` initializes the `SqlExecuteStructure` with the context's resolved parameters, and calls a new `ExecuteAsync(SqlExecuteStructure)` method. Result returned simply as an `OkResponse` instead of using the `FormatFindResult` method. The pagination methods of `FormatFindResult` don't play well with stored procedures at the moment, since it relies on fields from a `TableDefinition`, but pagination can and should be added to a future version. > - The response from this method will always be a `200 OK` if there were no database errors. If there was no result set returned, we will return an empty json array. **NOTE: Only the first result set returned by the procedure is considered.** @@ -238,7 +238,7 @@ Implementation was segmented into 5 main sections:
-#### `SqlMutationEngine.cs` +### `SqlMutationEngine.cs` > - Instead of trying to refactor `ExecuteAsync(RestRequestContext)` and `PerformMutationOperation`, which conditionally initializes query structures and builds them based on context `OperationType` - which is not a distinguishing factor of a stored procedure request - I found it easier and clearer to add an overloaded `ExecuteAsync(StoredProcedureRequestContext)`, which will be responsible for all stored procedure mutation requests (POST, PUT, PATCH, DELETE). > - In `ExecuteAsync(StoredProcedureRequestContext)`, we initialize the structure and build it the same way as in the query engine. The difference comes in formatting the response. We're leaving it up to the developers to configure their procedures appropriately and to call them with the appropriate HTTP verb. As such, we will return the expected response for each successful request based on its Operation. @@ -249,12 +249,12 @@ Implementation was segmented into 5 main sections: ### 7. GRAPHQL Query Execution + Result Formatting -#### `ResolverMiddleware.cs`, `SqlQueryEngine.cs`, and `SqlMutationEngine.cs` +### `ResolverMiddleware.cs`, `SqlQueryEngine.cs`, and `SqlMutationEngine.cs` > - Call `ExecuteAsync` with `IMiddlewareContext` and `parameters`. > - We would have to convert `IMiddlewareContext` to `StoredProcedureRequestContext`, similar to how the `dispatchQuery` method in `RestService.cs` is doing. It converts `RestRequestContext` to `StoredProcedureRequestContext`. > - Then we can simply call `ExecuteAsync(StoredProcedureRequestContext context)` and return the parsed json request as String. -#### `SqlQueryEngine.cs` and `SqlMutationEngine.cs` +### `SqlQueryEngine.cs` and `SqlMutationEngine.cs` > - `ExecuteListAsync` is called where we create an object of `SqlExecuteStructure` and calls `ExecuteAsync(SqlExecuteStructure)`. > - We format the result as list of JsonDocument. If the user has no read permission, it will return an empty list. From f50e36ec43ff7bacabac6c0cbd0e980cc549fe6b Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 6 Dec 2022 11:22:06 +0530 Subject: [PATCH 21/21] replaced JsonDocument to JsonElement in doc --- docs/internals/StoredProcedureDesignDoc.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/internals/StoredProcedureDesignDoc.md b/docs/internals/StoredProcedureDesignDoc.md index 1052ffeaf7..e5ac80f453 100644 --- a/docs/internals/StoredProcedureDesignDoc.md +++ b/docs/internals/StoredProcedureDesignDoc.md @@ -256,7 +256,7 @@ Implementation was segmented into 5 main sections: ### `SqlQueryEngine.cs` and `SqlMutationEngine.cs` > - `ExecuteListAsync` is called where we create an object of `SqlExecuteStructure` and calls `ExecuteAsync(SqlExecuteStructure)`. -> - We format the result as list of JsonDocument. If the user has no read permission, it will return an empty list. +> - We format the result as list of JsonElement. If the user has no read permission, it will return an empty list. > ### Example > 1. Query With param as id