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

Do not include the validation starter in web starters by default #19550

Closed
bclozel opened this issue Jan 7, 2020 · 26 comments
Closed

Do not include the validation starter in web starters by default #19550

bclozel opened this issue Jan 7, 2020 · 26 comments
Assignees
Labels
status: noteworthy A noteworthy issue to call out in the release notes type: enhancement A general enhancement
Milestone

Comments

@bclozel
Copy link
Member

bclozel commented Jan 7, 2020

We've noticed that many applications are not using the validation features in web applications, while validator libraries on the classpath come with a cost.

We could do the following:

  • remove the spring-boot-starter-validation dependency from spring-boot-starter-web and spring-boot-starter-webflux
  • create a new spring-boot-starter-validation-web that brings in the validation dependencies without the tomcat-embed-el which is currently excluded by the web starters
  • properly document that change in the release notes and the reference docs
@bclozel bclozel added type: enhancement A general enhancement for: team-attention An issue we'd like other members of the team to review status: noteworthy A noteworthy issue to call out in the release notes labels Jan 7, 2020
@bclozel bclozel added this to the 2.3.x milestone Jan 7, 2020
@bclozel bclozel self-assigned this Jan 7, 2020
@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label Jan 10, 2020
@bclozel
Copy link
Member Author

bclozel commented Jan 10, 2020

We'll do the following:

  • remove the spring-boot-starter-validation from spring-boot-starter-web by default
  • change spring-boot-starter-validation to use org.glassfish:jakarta.el instead of the current choice of default implementation
  • update all the web servers starters to use that same el implementation for all

With this change, we don't need to add a new starter and we can consistently use the same el implementation.

@ofabricio
Copy link

I'm surprised to figure out not most projects are not using validations, because all my web apps use them. I had to include this new starter.

I'm wondering what is people using instead?

@bclozel
Copy link
Member Author

bclozel commented May 22, 2020

The rationale behind this decision is not that most applications aren’t using validation, but rather that there is a significant amount of them and that the (startup and runtime) cost associated with the validation starter is far from negligible.

We usually try to strike the right balance between doing as much as possible with the developer’s intent and keeping setups efficient and lightweight. « Not all web applications should use validation and we want to optimize efficiency » is all that you should read into this decision.

@springframeworkguru
Copy link

This change caught me by surprise a little bit. In upgrading to 2.3.0, I added the validation API, but not the hibernate impl. My unit tests started covering validation started to fail. Without test coverage, might not have caught this.

Maybe consider failing startup if the API is present, and impl is not?

@bclozel
Copy link
Member Author

bclozel commented May 23, 2020

@springframeworkguru Any reason why you're not using the spring-boot-starter-validation instead of bringing the dependencies manually in your build?

@springframeworkguru
Copy link

@bclozel - Just didn't know of it, until I saw this thread. I def will now though.

@abccbaandy
Copy link

+1 for add some warning or even fail to start, it's strange to silent skip valid check if valid stuff not work.

@Saljack
Copy link

Saljack commented Jun 25, 2020

I don't understand why it has been changed. If the people who don't want to use validation can exclude the spring-boot-starter-validation in spring-boot-starter-web and spring-boot-starter-webflux.

@wilkinsona
Copy link
Member

@Saljack Please see this comment above for the reasoning behind the change.

@mykolap
Copy link

mykolap commented Jul 28, 2020

Decision about using jakarta-el instead of default el from tomcat was not the best choice for our project...
Now we got jsp compilation errors in many places
For example this function in jsp is ok for tomcat-el, but not ok for jakarta:
${eco:translate("JSON Pointer can be used for accessing body\'s parameters.")}
I had to exclude jakarta-el on project level.

@bclozel
Copy link
Member Author

bclozel commented Jul 28, 2020

@mkopylec that's unfortunate. Did you raise an issue with the el library about this? Could you edit your comment linking to that issue for other community members? We chose the Jakarta implementation here as validation doesn't necessarily happen in a web application (so with a server) and pushing the Tomcat implementation for apps using another Servlet container didn't feel right.

@mykolap
Copy link

mykolap commented Jul 28, 2020

But jakarta-el is now used for compiling jsp, that doesn't look right too. It is present now as a dependency for tomcat starter, not only for validation starter

@mykolap
Copy link

mykolap commented Jul 28, 2020

See - it is compile dependency for tomcat starter, not only for validation starter...
https://mvnrepository.com/artifact/org.springframework.boot/spring-boot-starter-tomcat/2.3.2.RELEASE

@bclozel
Copy link
Member Author

bclozel commented Jul 28, 2020

@mykolap if you've got a solution in mind, please submit a PR and we can reconsider this arrangement.

@mykolap
Copy link

mykolap commented Jul 28, 2020

I don't know how to resolve it properly...
If validation starter is not used - put tomcat-el as a compile dependency to tomcat starter looks good.
But if validation starter is used - we will have 2 libs with el, from jakarta and tomcat, and it should be resolved by excluding one of them...
It doesn't look nice... I will think about better solution, now I see the only way manual resolving, that's what I did in my project...

@Arturo0911
Copy link

Arturo0911 commented Feb 18, 2021

Hi. In my case, @Valid not work, in GitHub repository specify that in the newest version of spring "Do not include the validation starter in web starters by default ". Now how can i use the validator @Valid in the restcontrollers

My Pom.xml

<dependency>
	<groupId>org.springframework.boot</groupId>
	<artifactId>spring-boot-starter-web</artifactId>
</dependency>
<dependency>
	<groupId>org.springframework.boot</groupId>
	<artifactId>spring-boot-starter-validation</artifactId>
	<version>2.4.2</version>
</dependency>

My Entity class:

@Entity
@Table(name = "todos")
@AllArgsConstructor @NoArgsConstructor
public class ToDo {

    @Id
    @GeneratedValue(strategy = GenerationType.AUTO)
    @Column(name = "ID")
    @Getter @Setter
    private Integer id;

    @Column(name = "DESCRIPTION")
    @Getter @Setter
    @NotNull @NotEmpty @NotBlank
    private String description;

    @Column(name = "DATE")
    @Getter @Setter
    @NotNull @NotEmpty @NotBlank
    private Date date;

    @Column(name = "PRIORITY")
    @Getter @Setter
    @NotNull @NotEmpty @NotBlank
    private String priority;

    @Column(name = "FK_USER")
    @Getter @Setter
    @NotNull @NotEmpty @NotBlank
    private String fkUser;


    @PrePersist
    void getTimeOperation(){
        this.date = new Date();
    }



}

and my RestController class

@RequestMapping("/todo")
public String todoInInput2(@Valid ToDo toDo){
    return "Todo description: "+toDo.getDescription()+", todo priority "+toDo.getPriority();
}

@wilkinsona
Copy link
Member

A dependency on spring-boot-starter-validation should be sufficient. If that's not working and you'd like some help in figuring it out, please come and chat on Gitter or ask a question on Stack Overflow.

@jdriver2
Copy link

jdriver2 commented Apr 16, 2021

what if you are using gradle? This tip is not working. springboot 2.4.4

@wilkinsona
Copy link
Member

@jdriver2 To which tip are you referring? The build system shouldn't make any difference and adding a dependency on spring-boot-starter-validation should be sufficient with both Maven and Gradle. As I said above, if that's not working and you'd like some help in figuring it out, please come and chat on Gitter or ask a question on Stack Overflow.

@jdriver2
Copy link

i solved the issue. Using gradle, but required me to restart the IDE. Thanks!

@avdept
Copy link

avdept commented Apr 12, 2022

Wanted to point, if you use kotlin, you need to annotate your fields like @field: NotNull @field:NotEmpty @field:NotBlank, without it - validations won't work using kotlin

@gabopushups
Copy link

@Arturo0911 Did you find a solution for your issue? I'm facing the same exact problem as you

@141319nms
Copy link

i come with the same problem.i use the spring-boot-starter-parent v2.7.5,given the spring-boot-start-validation,but the validation never works,who has the answer ,please.

@141319nms
Copy link

image

image

image

@141319nms
Copy link

i get

@bclozel
Copy link
Member Author

bclozel commented Dec 5, 2022

Thanks for getting in touch, but it feels like this is a question that would be better suited to Stack Overflow. As mentioned in the guidelines for contributing, we prefer to use the issue tracker only for bugs and enhancements. Feel free to update this issue with a link to the re-posted question (so that other people can find it) or add some more details if you feel this is a genuine bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: noteworthy A noteworthy issue to call out in the release notes type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests