-
Notifications
You must be signed in to change notification settings - Fork 211
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Multiple-create: Request Validation #2057
Multiple-create: Request Validation #2057
Conversation
…h/AuthZForNestedInsertions
…ithub.com/Azure/data-api-builder into dev/agarwalayush/AuthZForNestedInsertions
src/Service.Tests/SqlTests/GraphQLMutationTests/MultipleMutationIntegrationTests.cs
Outdated
Show resolved
Hide resolved
src/Service.Tests/SqlTests/GraphQLMutationTests/MultipleMutationIntegrationTests.cs
Outdated
Show resolved
Hide resolved
…garwalayush/requestValidationForNestedInsertions
…garwalayush/requestValidationForNestedInsertions
…garwalayush/requestValidationForNestedInsertions
/azp run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added feedback to clarify intent / comments/ add some more tests.
src/Service.Tests/SqlTests/GraphQLMutationTests/MultipleMutationIntegrationTests.cs
Show resolved
Hide resolved
src/Service.Tests/SqlTests/GraphQLMutationTests/MultipleMutationIntegrationTests.cs
Outdated
Show resolved
Hide resolved
src/Service.Tests/SqlTests/GraphQLMutationTests/MultipleMutationIntegrationTests.cs
Outdated
Show resolved
Hide resolved
src/Service.Tests/SqlTests/GraphQLMutationTests/MultipleMutationIntegrationTests.cs
Outdated
Show resolved
Hide resolved
src/Service.Tests/SqlTests/GraphQLMutationTests/MultipleMutationIntegrationTests.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pending more tests and further resolution of open feedback threads + resolving merge conflicts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit suggestion to re-name GraphQLRequestValidator
to MultipleMutationInputValidator
in the PR description.
LGTM, after addressing this and pending suggestions.
Thank you for adding logic to validate the Multiple Create mutation request :)
Why make this change?
With support for multiple-create, we introduce additional flexibility in the GraphQL schema where we make the referencing fields in an entity optional because their value can be derived from insertions in the referenced entity. So to say, the values of referencing fields in a referencing entity can have 3 sources of truth. The value for a referencing field can be derived via:
They can all exist at the same time, or none of them can. Presence of more than one sources of truth leads to possible conflicting values for the referencing column. Thus, at a time, we want exactly one of them to be present - so that we exactly one source of truth. This needs us to do request validation during request execution, so that only valid GQL mutations reach the query generation/query execution stage. This PR adds the logic for the same.
What is this change?
MultipleMutationInputValidator.cs
to perform the validations for a multiple-create mutation.MultipleMutationEntityInputValidationContext
to store relevant information while performing validations on a particular entity.MultipleMutationInputValidator.ValidateGraphQLValueNode()
will be made just after Authorization - request validation only occurs when the user is authorized to execute the mutation.MultipleMutationInputValidator.ValidateGraphQLValueNode()
makes recursive call to itself to parse out the entire create mutation request body based on an entity or relationship basis. Meaning, this method will be called on the top-level entity, followed by all the relationships for that entity, followed by all the relationships for the entity's relationships and so on....GraphQLRequestValidator.ValidateObjectFieldNodes()
which is called from the methodValidateGraphQLValueNode()
.ValidateObjectFieldNodes()
contains logic to ensure that when the current entity acts as the referencing entity for the relationships with its target entity, then the current entity should not specify a value for the referencing fields - as the values for the referencing fields will be derived from the insertion in the referenced target entity. Note: The determination of referencing entity is done via a call toMultipleCreateOrderHelper.GetReferencingEntityName()
method added via: Multiple-create: Order determination for insertions #2056.ValidateObjectFieldNodes()
makes calls to1.
ValidateAbsenceOfReferencingColumnsInTargetEntity()
: To validate that the current entity does not specify value for referencing columns to its parent entity (when parent entity is the referenced entity).2.
ProcessRelationshipField()
: To process a relationship field and does several things. Please refer to method summary.3.
ValidateAbsenceOfRepeatedReferencingColumn()
: This method is called for all the relationships with the current entity which are included in the request body. This ensures that referencing columns in the referencing entity (whichever entity, either source or target turns out as the referencing entity) does not hold references to multiple columns in the referenced entity - to avoid multiple sources of truth for the referencing column's value. (Issue: Allow a referencing column to reference only one referenced column in relationships #2019)4.
ValidatePresenceOfRequiredColumnsForInsertion()
: To ensure that we have one source of truth for values for referencing fields.5.
ValidateRelationshipFields()
: Recursively perform the same validations for all the relationships with the current entity included in the mutation request body.`
How was this tested?
MultipleMutationIntegrationTests.cs
to validate:Sample Request(s)
Response:
Response:
