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

java validation errors in graphql #23838

Closed
raoua-eng opened this issue Feb 21, 2022 · 11 comments · Fixed by #23877
Closed

java validation errors in graphql #23838

raoua-eng opened this issue Feb 21, 2022 · 11 comments · Fixed by #23877

Comments

@raoua-eng
Copy link

Describe the bug

I'm experiensing validation in GraphQL inputs with the Java Bean Validation API and I tried to customize message errors by using the expression language (EL) as below :

@Size(value = 5, message = "{ '${validatedValue}' must be at least {value} characters long. Length found : '${validatedValue.length()}'}")
private String civility;

Then in the graphql playground if we added a wrong civility with an invalid charachter long, here must be at least 5 we get this message error :

"message": "validation failed: updateAdherent.adherent.civility : 'TESTEST' must be at least 5 characters long. Length found : '${validatedValue.length()}'

Expected behavior

In the message error should be :

"message": "validation failed: updateAdherent.adherent.civility : 'TESTEST' must be at least 5 characters long. Length found : '7'

Actual behavior

"message": "validation failed: updateAdherent.adherent.civility : 'TESTEST' must be at least 5 characters long. Length found : '${validatedValue.length()}'

How to Reproduce?

As a reproducer we start by the GraphQL input

import java.time.LocalDate;
import javax.json.bind.annotation.JsonbDateFormat;
import javax.validation.constraints.Email;
import javax.validation.constraints.NotBlank;
import javax.validation.constraints.Past;
import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Data;
import lombok.NoArgsConstructor;

@Builder
@Data
@AllArgsConstructor
@NoArgsConstructor
public class CreatePersonUseCaseInput {
    @NotBlank(message="'${validatedValue}' cannot be null")
    private String name;
    @Past(message="Date must be in past. found '${validatedValue}'")
    @JsonbDateFormat(value = "dd/MM/yyyy")
    private LocalDate dateOfBirth;
    @Email(message="email should be valid")
    private String email;
  @Size(value = 5, message = "{ '${validatedValue}' must be at least {value} characters long. Length found : '${validatedValue.length()}'}")
  private String civility;
}

then the mutation code:

@Mutation
    @Description("create person")
    public String createPerson(@Valid @Name("person")CreatePersonUseCaseInput input) {
        /*
         * do something
         * */
        return "successful creation";
    }

Lastly the query in the graphql playground is

mutation{
  createPerson(
    person:{
      name:"test"
      dateOfBirth: "12/03/2002"
      email: "test.test@gg"
      civility: "TESTEST"
    }
  )
}

Output of uname -a or ver

No response

Output of java -version

"1.8.0_265"

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.5.1.Final

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

@raoua-eng raoua-eng added the kind/bug Something isn't working label Feb 21, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Feb 21, 2022

/cc @jmartisk, @phillip-kruger

@jmartisk
Copy link
Contributor

@gsmet could you have a look? To me it looks like a HV issue, not specific to GraphQL

@gsmet
Copy link
Member

gsmet commented Feb 21, 2022

@raoua-eng it would help if you could put together a full reproducer (i.e. a Maven project containing everything needed to reproduce the issue). Thanks!

@gsmet gsmet added the area/hibernate-validator Hibernate Validator label Feb 21, 2022
@geoand geoand added the triage/needs-reproducer We are waiting for a reproducer. label Feb 21, 2022
@raoua-eng
Copy link
Author

@gsmet Feel free to use a reproducer I made for this issue: https://github.com/raoua-eng/Quarkus

@raoua-eng
Copy link
Author

@gsmet do you think the issue is related to EL restrictions NONE, VARIABLES, BEAN_PROPERTIES(default), BEAN_METHODS ?

@gsmet
Copy link
Member

gsmet commented Feb 22, 2022

Yes exactly.

The default allows you to use bean properties but not call methods on beans. Thus why you cannot use the length() method.

I wonder if we should have some sort of allow list allowing some of the harmless but very useful methods. @yrodiere any opinion on that? It makes things a bit blurry though.

Also, for now, you cannot configure the default expression language level in Quarkus, we need to fix it.

@yrodiere
Copy link
Member

I wonder if we should have some sort of allow list allowing some of the harmless but very useful methods. @yrodiere any opinion on that? It makes things a bit blurry though.

Meh. Not a fan of things that needs to be updated continuously, and that list probably would. Maybe if it can be customized by users somehow...

Regardless, I think the main problem here is that users are not warned that their attempt to call a method is being ignored, and they are not provided with a helpful message telling them what to change in their application to make it work. But I suppose we don't have a way to warn something every time a method call is ignored? It doesn't work that way?

Also, for now, you cannot configure the default expression language level in Quarkus, we need to fix it.

👍

@raoua-eng
Copy link
Author

Thanks and I hope it will be fixed soon.

gsmet added a commit to gsmet/quarkus that referenced this issue Feb 22, 2022
…evel

I only exposed the constraint one and not the custom violation one as I
think it is bad practice to globally change the custom violation one.

Fix quarkusio#23838
@gsmet
Copy link
Member

gsmet commented Feb 22, 2022

I created a PR for that here: #23877

gsmet added a commit to gsmet/quarkus that referenced this issue Feb 28, 2022
…evel

I only exposed the constraint one and not the custom violation one as I
think it is bad practice to globally change the custom violation one.

Fix quarkusio#23838
yrodiere pushed a commit to gsmet/quarkus that referenced this issue Mar 1, 2022
…evel

I only exposed the constraint one and not the custom violation one as I
think it is bad practice to globally change the custom violation one.

Fix quarkusio#23838
@quarkus-bot quarkus-bot bot added this to the 2.8 - main milestone Mar 1, 2022
@gsmet gsmet modified the milestones: 2.8 - main, 2.7.3.Final Mar 1, 2022
gsmet added a commit to gsmet/quarkus that referenced this issue Mar 1, 2022
…evel

I only exposed the constraint one and not the custom violation one as I
think it is bad practice to globally change the custom violation one.

Fix quarkusio#23838

(cherry picked from commit 780d683)
@gsmet gsmet removed the triage/needs-reproducer We are waiting for a reproducer. label Mar 2, 2022
@gsmet
Copy link
Member

gsmet commented Mar 2, 2022

2.7.3.Final has been released and you can now configure the behavior with:

quarkus.hibernate-validator.expression-language.constraint-expression-feature-level=bean-methods

@gsmet gsmet added kind/enhancement New feature or request and removed kind/bug Something isn't working labels Mar 2, 2022
@raoua-eng
Copy link
Author

Great :) thank you for your support

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants