Conversation
…rce and target entities (#1929) ## Why make this change? Currently, we allow defining multiple relationships between the same source and target entities. A user is allowed to define multiple relationships each with different source fields/target fields and even different cardinalities. This is not only logically incorrect and difficult to make sense out of, but will also pose a problem in the functioning of nested mutations. ## What is this change? Added the above explained validation in `development` mode when `GraphQL is enabled` for the source entity.. ## How was this tested? - Added unit test via `RuntimeConfigValidator.TestMultipleRelationshipsBetweenSourceAndTargetEntities`.
## Why make this change? - Closes #1950 - Introduces a feature flag in the config for nested mutation operations. - Feature Flag format: ```json "runtime":{ ... "graphql": { ... "nested-mutations": { "create": { "enabled": true/false } } } } ``` - CLI Option: `--graphql.nested-create.enabled`. This option can be used along with `init` command to enable/disable nested insert operation. - By default, the nested mutation operations will be **disabled**. - Nested Mutation operations are applicable only for MsSQL database type. So, when the option `--graphql.nested-create.enabled` is used along with other database types, it is not honored and nested mutation operations will be disabled. Nested Mutation section will not be written to the config file. In addition, a warning will be logged to let users know that the option is inapplicable. ## What is this change? - `dab.draft.schema.json` - The schema file is updated to contain details about the new fields - `InitOptions` - A new option `--graphql.nested-create.enabled` is introduced for the `init` command. - `NestedCreateOptionsConverter` - Custom converter to read & write the options for nested insert operation from/to the config file respectively. - `NestedMutationOptionsConverter` - Custom converter to read & write the options for nested mutation operations from/to the config file respectively. - `GraphQLRuntimeOptionsConverterFactory` - Updates the logic for reading and writing the graphQL runtime section of the config file. Incorporates logic for reading and writing the nested mutation operation options. - `dab-config.*.json`/`Multidab-config.*.json` - All the reference config files are updated to include the new Nested Mutation options ## How was this tested? - [x] Integration Tests - [x] Unit Tests - [x] Manual Tests ## Sample Commands - **Nested Create Operations are enabled**: `dab init --database-type mssql --connection-string connString --graphql.nested-create.enabled true`  - **Nested Create Operations are disabled**: `dab init --database-type mssql --connection-string connString --graphql.nested-create.enabled false`  - **When --graphql.nested-create.graphql option is not used in the init command**: `dab init --database-type mssql --connection-string connString`  - **When --graphql.nested-create.graphql option is used with a database type other than MsSQL**: 
…ultiple-create respectively (#2103) ## Why make this change? - Closes #2090 - Renames `nested-mutations` and `nested-create` to `multiple-mutations` and `multiple-create` respectively. - Nested Mutations has a significantly different meaning in the graphql specs (as explained in issue graphql/graphql-spec#252) compared to what we are trying to achieve. Hence, the decision to rename. ## What is this change? - All references to nested mutations/nested create - option name, field names in config JSON, class names, comments in code have been renamed. ## How was this tested? - Unit and Integration tests added as part of #1983 has been updated to validate the rename of CLI option, property in the config file, etc. ## Sample Commands 
…pi-builder into dev/NestedMutations
## Why make this change?
Currently, for a create mutation (or any graphql mutation essentially),
we can only do insertion/mutation in one table and not in another table
which is related to this table via some relationship (either defined in
config or in the database). This PR aims to:
1. Extend the functionality of the existing create mutations (which are
basically point insertions) to allow insertions in related table.
2. To generate additional `createMultiple` mutations which will allow
multiple nested insertions starting from the top-level entity.
**Note:** _All changes made in this PR are specific to MsSql. Nothing
changes for the other database flavors._
## Quick glossary:
1. **ObjectTypeDefinitionNode:** Represents a
table/view/stored-procedure. For a table, this contains all the column
fields which belong to the table and the relationship fields which are
defined in this table's entity configuration in the config file.
2. **InputObjectTypeDefinitionNode:** Represents an input object type we
generate for mutations.
## What is this change?
1. **We no more ignore relationship fields while generating the input
types for a create mutation**: Till date, we were only considering the
column fields in an object (of type `ObjectTypeDefinitionNode`) to
generate the input type (of type `InputObjectTypeDefinitionNode`) for a
table entity. But to support nested insertion, we would now also iterate
over relationship fields to generate the input type for a
**create**/**createMultiple** mutation.
2. **Addition of linking entities at the backend:** To support nested
insertions in tables which have a relationship with cardinality N:N, the
user can provide a linking table with the source defined in
`linking.object` field which we need the user to provide. We need to
generate an object type (of type `ObjectTypeDefinitionNode`) for this
linking entity. This object type is later be used to generate the input
object type for the linked tables.
Additional details about linking entities:
-> **A boolean property `IsLinkingEntity` is added with a _default value
of `false`_** to the `Entity` record. This ensures that we are backwards
compatible, _and all the entities provided in the config are not
considered as linking entities. For all the linking entities, this
boolean property will be set to `true`._
-> For a linking entity, **_the GraphQL and REST endpoints are disabled
(set to `false`) by default_**. This ensures that we don't accidentally
expose the linking entity to the user.
-> **MsSqlMetadataProvider.EntityToDatabaseObject contains database
objects for Entities in config + Linking Entities**. After we get all
the deserialized entities, we will create entities to represent linking
tables. The database objects for all the entities in config will be
generated by a call to
**SqlMetadataProvider.GenerateDatabaseObjectForEntities()** method which
in turn sequentially goes over all the entities in the config and call
the method **SqlMetadataProvider.PopulateDatabaseObjectForEntity()**
which actually populates the database object for an entity.
-> The method **SqlMetadataProvider.AddForeignKeysForRelationships()**
is renamed to **SqlMetadataProvider.ProcessRelationships()**. The method
now in addition to adding FKs to metadata, also creates linking entities
for M:N relationships. This is done via a call to method
**SqlMetadataProvider.PopulateMetadataForLinkingObject()**.
-> The method **PopulateMetadataForLinkingObject()** is a virtual method
which has an overridden implementation only for MsSql. Thus, linking
entities are generated only for MsSql.
3. **ReferencingFieldDirectiveType:** is a new directive type being
added. The presence of this directive for a column field implies that
the column is a referencing key to some column in another entity exposed
in the config. With nested insertions support, the values for such
fields can come via insertion in the referenced table. _And hence while
generating the input object for a `create`/`createMultiple` mutation, we
mark such a field as not required_.
4. **Name of the create multiple mutation:** is generated via a call to
the newly added helper method
`CreateMutationBuilder.GetInsertMultipleMutationName(singularName,
pluralName)` which takes in the singular/plural names of the entity. If
singularName == pluralName, the createMultiple mutation will be
generated with the name: `create{singularName}_Multiple` else, it will
be generated with the name `create{pluralName}`. The pluralName for an
entity is fetched using another newly added helper method
`GraphQLNaming.GetDefinedPluralName(entityName, configEntity)`
5. **For create multiple mutations, the input argument name will be
`items`** which is stored in a constant string
`MutationBuilder.ARRAY_INPUT_ARGUMENT_NAME`.
6. **Object types (of type `ObjectTypeDefinitionNode`) for linking
entities** will be generated using a method
`GraphQLSchemaCreator.GenerateObjectDefinitionsForLinkingEntities(linkingEntityNames,
entities)`.
7. **sourceTargetLinkingNode:** The object type for linking entity is
later used to generate object type for a `sourceTargetLinkingNode` which
will contain:
-> Fields present in the linking table (but not used to relate
source/target)
-> All the fields from the target entity.
This is done using a call to the newly added method
`GraphQLSchemaCreator.GenerateSourceTargetLinkingObjectDefinitions()`.
8. Renamed method `GraphQLSchemaCreator.FromDatabaseObject()` to
`GraphQLSchemaCreator.GenerateObjectTypeDefinitionForDatabaseObject()`
to better convey what the method is doing.
9. To make code more streamlined and bug-free the method
**SchemaConverter.FromDatabaseObject()** is broken down into smaller
chunks which handle smaller and easy to read/comprehend
responsibilities. Passing of existing test cases confirm the refactor.
10. Added a method
`GraphQLUtils.GenerateLinkingEntityName(sourceEntityName,
targetEntityName)` which concatenates the names of the source and target
entities with a delimiter (`ENTITY_NAME_DELIMITER = "$"`) between the
names and prefixes the concatenated string with: `LINKING_ENTITY_PREFIX
= "LinkingEntity"` string.
11. Added a method
`GraphQLUtils.GetSourceAndTargetEntityNameFromLinkingEntityName(string
linkingEntityName)` which returns the the names of the source and target
entities from the linking entity name.
12. Split the `CreateMutationBuilder.GenerateCreateInputType()` method
which was used to create input types for Relational (Pg/My/Ms/Dw Sql)
and non-relational dbs (Cosmos_NoSql) into two new methods:
`CreateMutationBuilder.GenerateCreateInputTypeForRelationalDb()` and
`CreateMutationBuilder.GenerateCreateInputTypeForNonRelationalDb()`.
13. Split the `CreateMutationBuilder.GetComplexInputType()` method which
was used to build input types for Relational (Pg/My/Ms/Dw Sql) and
non-relational dbs (Cosmos_NoSql) into two new methods:
`CreateMutationBuilder.GenerateComplexInputTypeForRelationalDb()` and
`CreateMutationBuilder.GenerateComplexInputTypeForNonRelationalDb()`.
## Validations Required:
1. **Ensure non-conflicting names of fields in sourceTargetLinkingNode
for N:N relationships:** Object types are generated for
`sourceTargetLinkingNode` for a source, target pair of tables (refer
above for what all fields would be present in this object type). The
name of the field (either a relationship field or a column field) in the
target entity might conflict with the name of the column field in the
linking table. Hence, we need a validation in place to ensure that if
there is a conflict, we catch it during startup (currently this check is
done during schema generation) and throw an appropriate exception with
an actionable message.
2. **Ensure non-conflicting names of relationship fields in column
fields in one entity:** This is a validation which is required in the
current architecture as well and is a bug (see here:
#1937). This is required
to ensure that relationship field names don't conflict with the exposed
names of column fields in a table.
3. **Ensure we have values for FK referencing fields to perform
insertions:** All the fields in a table which hold a foreign key
reference to some other column in another entity are now nullable in the
generate input objects for create mutation. This is because we assume
that the value for such a field might come via insertion in the related
entity/table. However, if the create mutation does not include a nested
insert, the value of such a field still has to be specified by the user
(unless the field is nullable/has default at the database level as well
in which case we can give a null value/default value for the field-
although this is highly unlikely since the field is a foreign key). If
we don't get a value for such a FK referencing field:
-> Either via nested insertion , or
-> By the user
we should throw an exception accordingly. This will maintain backwards
compatibility. **UNLESS THIS VALIDATION IS IN, WE ARE NOT BACKWARDS
COMPATIBLE.**
4. **Ensure there is only one source of value for FK referencing
fields:** If the user is providing a value for an FK referencing field,
and there is a nested insertion involved which also returns a value for
the same field, we should throw an exception as there are two
conflicting sources of truth. **In short, exactly one of the sources
should provide a value.**
## How was this tested?
- [x] Unit Tests - Added to `NestedMutationBuilderTests.cs` class.
## Sample Request(s)
1.
<img width="608" alt="image"
src="https://github.com/Azure/data-api-builder/assets/34566234/1296e972-2017-4396-8157-7be30b7b36f5">
## Why make this change?
Since GraphQL insertions now support nested insertions, we need to
authorize entity and fields not only for the top-level entity in the
insertion, but also the nested entities and fields. This PR aims to
address that logic of collecting all the unique entities and fields
belonging to those entities in a data structure, and then sequentially
iterate over all entities and fields to check whether the given role is
authorized to perform the action (here nested insertion).
## What is this change?
1. The presence of the model directive on the output type of the
mutation object will be used to determine whether we are dealing with a
point mutation or a nested insertion. Presence of model directive
indicates we are doing a point insertion while its absence indicates the
opposite. This logic is added to `SqlMutationEngine.ExecuteAsync()`
method. This logic determines whether the input argument name is `item`
(for point mutation) or `items` (for insert many).
2. A new method
`SqlMutationEngine.AuthorizeEntityAndFieldsForMutation()` is added. The
name is kept generic (instead of using 'Insertion') because the same
method can be used later for nested updates as well. As the name
indicates, this method iterates over all the entities and fields and
does the required authorization checks.
3. The method mentioned in the point above depends on another newly
added method `SqlMutationEngine.PopulateMutationFieldsToAuthorize()`
whose job is to populate all the unique entities referred in the
mutation and their corresponding fields into a data structure of the
format:
`Dictionary<string, HashSet<string>> entityAndFieldsToAuthorize` - where
for each entry in the dictionary:
-> Key represents the entity name
-> Value represents the unique set of fields referenced from the entity
4. The method `SqlMutationEngine.PopulateMutationFieldsToAuthorize()`
recursively calls itself for nested entities based on different criteria
explained in code comments.
5. When the field in a nested mutation is a list of ObjectFieldNode
(fieldName: fieldValue) or the field has an object value, the fields are
added to the data structure mentioned in (3) using a newly added method:
`SqlMutationEngine.ProcessObjectFieldNodesForAuthZ()` which sequentially
goes over all the fields and add it to the list of fields to be
authorized. Since a field might represent a relationship- and hence a
nested entity, this method again calls its parent caller i.e.
`SqlMutationEngine.PopulateMutationFieldsToAuthorize()`.
6. The method `SqlMutationEngine.ProcessObjectFieldNodesForAuthZ()`
contains the logic to ensure that the fields belonging to linking tables
are not added to the list of fields to be authorized.
7. Moved the method `GetRoleOfGraphQLRequest()` from
`Cosmos`/`SqlMutationEngine` to `AuthorizationResolver`.
## How was this tested?
To be added.
## Sample Request(s)
1. Config:
1. Request/Response - AuthZ failure because `piecesAvailable` field is
not accessible to `test_role_with_excluded_fields_on_create` role.
2. Request/Response: Removing `piecesAvailable` field from request body
leads to successful authz checks (request fails during query
generation).
---------
Co-authored-by: Sean Leonard <sean.leonard@microsoft.com>
…ue (#2116) ## Why make this change? - Closes #1951 - PR #1983, #2103 add CLI options to enable or disable multiple mutation/multiple create operation through CLI. With the changes introduced in the mentioned PRs, the configuration properties successfully gets written to the config file. Also, during deserialization, the properties are read and the `MultipleMutationOptions`, `MultipleCreateOptions`, `GraphQLRuntimeOptions` objects are created accordingly. - The above-mentioned PRs do not introduce any change in DAB engine behavior depending on the configuration property values. - This PR introduces changes to read these fields and enable/disable multiple create operation depending on whether the feature is enabled/disabled through the config file. This is achieved by introducing behavior changes in the schema generation. ## What is this change? - This PR builds on top of a) Schema generation PR #1902 b) Rename nested-create to multiple create PR #2103 - When multiple create operation is disabled, > i) Fields belonging to the related entities are not created in the input object type are not created. > ii) Many type multiple create mutation nodes (ex: `createbooks`, `createpeople_multiple` ) are not created. > iii) ReferencingField directive is not applied on relationship fields, so they continue to remain required fields for the create mutation operation. > iv) Entities for linking objects are not created as they are relevant only in the context of multiple create operations. ## How was this tested? - [x] Unit Tests and Integration Tests - [x] Manual Tests **Note:** At the moment, multiple create operation is disabled in the config file generated for integration tests. This is because of the plan to merge in the Schema generation, AuthZ/N branches separately to the main branch. With just these 2 PRs, a multiple create operation will fail, hence, the disabling multiple create operation. At the moment, tests that perform validations specific to multiple create feature enable it by i) updating the runtime object (or) ii) creating a custom config in which the operation is enabled. ## Sample Request(s) ### When Multiple Create operation is enabled - MsSQL #### Related entity fields are created in the input object type  #### Multiple type create operation is created in addition to point create operation  #### Querying related entities continue to work successfully  ### When Multiple Create operation is disabled - MsSQL #### Only fields belonging to the given entity are created in the input object type  #### Multiple type create operation is not created ### When Multiple Create operation is enabled - Other relational database types #### Only fields belonging to the given entity are created in the input object type  #### Multiple type create operation is not created --------- Co-authored-by: Ayush Agarwal <ayusha083@gmail.com>
Contributor
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
Contributor
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
severussundar
requested review from
Aniruddh25,
Mathos1432,
mbhaskar and
seantleonard
as code owners
severussundar
requested review from
aaronburtle,
aaronpowell,
abhishekkumams,
gledis69,
neeraj-sharma2592,
ravishetye,
rohkhann,
severussundar,
sourabh1007 and
tarazou9
as code owners
severussundar
added
the
Multiple mutations
Contributor
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
severussundar
approved these changes
Mar 27, 2024
seantleonard
approved these changes
Mar 28, 2024
Contributor
seantleonard
left a comment
seantleonard
left a comment
There was a problem hiding this comment.
Thanks for adding this.
Nit for next time: please rebase this branch onto latest main before opening so the commit history is cleaner.
- Noise of the merge and revert of Adding validation to disallow multiple relationships between same source and target entities
- Noise of merging latest main
and documentation of additional commits that are relevant beyond the PRs:
Contributor
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why make this change?
dev/NestedMutationsbranch.mainbranch in preparation for the0.12.* rc1releaseWhat is this change?
dev/NestedMutationsbranch contains the code changes for the following components of Multiple Create featuredev/NestedMutationsbranch.mainbranchHow was this tested?
dev/NestedMutations