Skip to content
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

Automatically remove constraints when explicit NULL is used in query #742

Closed
5 tasks done
novoj opened this issue Nov 11, 2024 · 1 comment · Fixed by #747
Closed
5 tasks done

Automatically remove constraints when explicit NULL is used in query #742

novoj opened this issue Nov 11, 2024 · 1 comment · Fixed by #747
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@novoj
Copy link
Collaborator

novoj commented Nov 11, 2024

The Java version of the API automatically excludes constraints that are used in the query but don't make sense because they lack crucial information - i.e. key arguments are NULL. This is expected and desired behaviour, as it makes client code drastically simpler. The same behaviour seems to be beneficial for the GraphQL API, but there the arguments are marked as mandatory. This issue aims to change this behaviour in the GraphQL / REST API.

The nullability will be placed at the root constraint value only. No matter the GQL/REST value structure type of constraint. For example:

For constraint with primitive value:

entityPrimaryKeyInSet: [1,2]

the entire [1,2] value will be nullable, but not the individual items. Therefore, if there will be null instead of an array:

entityPrimaryKeyInSet: null

the entire constraint will be omitted:

For constraint with wrapper object value:

priceDiscount: {
  inPriceLists: "basic",
  order: DESC
}

the entire {...} object will be nullable, but not the nested properties. They will have proper nullability based on internal definition. Therefore, if there will be null instead of an top-most object:

priceDiscount: null

the entire constraint will be omitted. However, in any other case, e.g.:

priceDiscount: {}

the constraint will not be omitted and nested properties will be validated.

Same principle goes for constraint with constraints.

  • check why certain constraints - such as entityPrimaryKeysIn are not marked as mandatory in GQL schema even if they doesn't work with null values
  • propagate nullability into GQL and REST schemas
  • allow null values for constraint fields and ignore them in such case, properly handle arrays of constraints where all inner constraints will be omitted
  • document this behavior into user documentation
  • unit tests for this behavior
@novoj novoj added enhancement New feature or request breaking change Backward incompatible data model change labels Nov 11, 2024
@novoj novoj added this to the Beta milestone Nov 11, 2024
@lukashornych
Copy link
Collaborator

Regarding the first point

check why certain constraints - such as entityPrimaryKeysIn are not marked as mandatory in GQL schema even if they doesn't work with null values

Currently, each constraint value is already being built into a schema explicitly as nullable within the ConstraintSchemaBuilder (more specifically the GraphQLConstraintSchemaBuilder) even though the ConstraintResolver cannot work with the null values on input. And it is this way since the beginning since early prototypes of the GraphQLConstraintSchemaBuilder.

I've had to dig deep into my memory, but I remembered that it is this way because it is an input object and if a property in an input object is marked as non-nullable, it must be present in the sent JSON. Therefore, client would have to send filterBy constraint with every nested constraint specified for every combination.

Anyway, from schema point of view it should be actually implemented as we want in this issue and there shouldn't be any breaking changes so far. The only thing is to implement the resolving part.

@lukashornych lukashornych removed the breaking change Backward incompatible data model change label Nov 22, 2024
lukashornych added a commit that referenced this issue Nov 22, 2024
…luding such constraints from output query
lukashornych added a commit that referenced this issue Nov 22, 2024
…cluding such constraints from output query
lukashornych added a commit that referenced this issue Nov 22, 2024
…raints-when-explicit-null-is-used-in-query

fix(#742): allow null as constraint value on input, effectively excluding such constraints from output query
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants