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

Bump project to use spring boot 2.x (Spring 1.x is EOL) #2056

Open
ecruz165 opened this issue Jul 22, 2022 · 11 comments
Open

Bump project to use spring boot 2.x (Spring 1.x is EOL) #2056

ecruz165 opened this issue Jul 22, 2022 · 11 comments

Comments

@ecruz165
Copy link
Contributor

ecruz165 commented Jul 22, 2022

Expected behavior

Platform should run all functions successfully using Spring 2.x

Actual behavior

Upgrading libraries breaks build.

Steps to reproduce behavior

Change spring boot version to 2.x and project will not compile.

@ecruz165
Copy link
Contributor Author

Hello my name is Edwin M. Cruz, a newbie staff member of Columbia University DBMI team. I'm an experienced Spring/Java engineer. I would love to become a contributor of this project. My work email is ec669@cumc.columbia.edu.

@chrisknoll
Copy link
Collaborator

chrisknoll commented Jul 22, 2022

This is definitely something we want to address, here are some other open issues related to dependencies:
#1771

I could have sworn that there were other issues that were related to updating library dependencies (I thought shrio was one) but I only searched through open issues). I could have sworn @t-abdul-basser did a summary of issues that he encountered when trying to update the Spring dependency, but I can't seem to find it.

We can use this issue to specifically track updating Spring boot to 2.x. But there are a number of dependency updates that we'd like to tackle in 1 release of Atlas (and considering the dependency changes, it will probably be the start of a 3.x line of Atlas):

  • Spring Boot (includes Hibernate, JPA, Batch and other 'core' spring functions)
  • JDK Update: We currently live on JDK8 but we'd want to move to a recent version of JDK. In fact, some of the library dependencies (such as Spring and Shiro may require a newer JDK version)
  • Other service dependencies. These could be logging, Json parsing (via Jackson) etc. We try to take whatever Spring Boot gives us as 'transient' dependencies, but other things may be injected for our own purposes (such as Apache Commons 3 lib)
  • Maven / toolchains: I'm positive that we're depending on some com.sun packages that are only exposed in the JDK8 version that we will need to replace with different build directives in maven.

I'd prefer to handle groups of dependencies in targeted PRs, but I expect that updating one dependency (such as Spring to 2.x) will involve a newer JDK, which will require other libraries to migrate in a big cascading dependency migration in 1 go.

Fortunately, we do have a good set of test cases that we can ensure the app behavior is maintained, but we probably don't have enough test coverage to be completely confident that everything 'just works' after making the changes.

@ecruz165
Copy link
Contributor Author

ecruz165 commented Jul 22, 2022

Spring Boot 2.7 supports java 8 if i recall correctly, but Spring Boot 3 will no longer support java 8 - so i would say stay on java 8 for now. I'll dig deeper and create/update list of libs and their ages. Once we are fully on Spring Boot 2.x and tested we can push forward for java 11 then 17. I'm a frontend ninja as well and have some ideas for frontend as well.

@t-abdul-basser
Copy link
Contributor

Thanks for welcoming @chrisknoll! @ecruz165 is a new-ish member of our team that will be working with us on OHDSI Atlas and WebAPI. Would you add him as a participant to the OHDSI Github org? Thanks.

@chrisknoll
Copy link
Collaborator

Done.

@ecruz165
Copy link
Contributor Author

ecruz165 commented Sep 8, 2022

@chrisknoll , @t-abdul-basser ,
Here is a high-level road map listing proposed change sets.

1. Bump several libraries that line up with dependency versions provided by Spring Boot 2.7.

Status: Partially Merged - Pull Request
Only those denoted with 2.12 were pull in.

  • org.apache.commons:commons-lang3 3.9 -> 3.12.0
  • org.freemarker:freemarker 2.3.30 -> 2.3.31
  • (2.12) org.apache.solr:solr-solrj 8.6.3 -> 8.11.2
  • org.ehcache:ehcache 3.8.1 -> 3.10.0
  • com.google.code.gson:gson 2.8.9 -> 2.9.0
  • com.zaxxer:HikariCP 3.3.1 -> 4.0.3
  • commons-httpclient:commons-httpclient 3.1 -> 4.5.13
  • javax.cache:cache-api 1.0.0 -> 1.1.1
  • (2.12) com.fasterxml.jackson.core:jackson-annotations ***2.12.7 -> 2.13.3
  • (2.12) com.fasterxml.jackson.core:jackson-core ***2.12.7 -> 2.13.3
  • com.microsoft.sqlserver:mssql-jdbc 9.2.1.jre8 -> 10.2.1.jre8
  • (2.12) org.postgresql:postgresql 42.3.3 -> 42.3.6
  • (2.12) org.yaml:snakeyaml 1.26 -> 1.3
  • (2.12) com.fasterxml.jackson.core:jackson-databind Dec-20 ***2.12.7 -> 2.13.3
  • Bump Shiro Libraries to 1.9.1
    -- (2.12) org.apache.shiro:shiro-core 1.8.0 -> 1.9.1
    -- (2.12 )org.apache.shiro:shiro-spring 1.8.0 -> 1.9.1
    -- (2.12) org.apache.shiro:shiro-web 1.8.0 -> 1.9.1
  • org.glassfish.jersey.media:jersey-media-multipart Jan-17 2.25.1 -> 2.35

Proposed PR Sets

  • PR#1: Bump Spring managed dependencies. (Story Points: 1)
    -- org.apache.commons:commons-lang3 3.9 -> 3.12.0
    -- org.ehcache:ehcache 3.8.1 -> 3.10.0
    -- com.google.code.gson:gson 2.8.9 -> 2.9.0
    -- com.zaxxer:HikariCP 3.3.1 -> 4.0.3
    -- commons-httpclient:commons-httpclient 3.1 -> 4.5.13
    -- javax.cache:cache-api 1.0.0 -> 1.1.1
    -- com.microsoft.sqlserver:mssql-jdbc 9.2.1.jre8 -> 10.2.1.jre8

  • PR#1: Bump Jersey dependencies. (Story Points: 5)

  • This is a larger effort! Curious why we have Jersey and Tomcat starter libraries as part of the codebase.
    -- org.glassfish.jersey.media:jersey-media-multipart Jan-17 2.25.1 -> 2.35

  • PR#2: Bump Jackson Libs (Story Points: 3)
    -- com.fasterxml.jackson.core:jackson-annotations ***2.12.7 -> 2.13.3
    -- com.fasterxml.jackson.core:jackson-core ***2.12.7 -> 2.13.3
    -- com.fasterxml.jackson.core:jackson-databind Dec-20 ***2.12.7 -> 2.13.3

  • PR#3: Bump Hibernate Libs to 5.5.9.Final (Story Points: 5)
    -- org.hibernate:hibernate-core 5.4.2.Final -> 5.5.9.Final
    -- org.hibernate:hibernate-entitymanager 5.4.2.Final -> 5.5.9.Final
    -- org.hibernate:hibernate-validator 5.4.2.Final -> 6.2.3.Final

  • PR#4: Bump Tomcat Libs to 9.0.65 (Story Points: 2)
    -- org.apache.tomcat.embed:tomcat-embed-el 8.5.81 -> 9.0.65
    -- org.apache.tomcat:tomcat-jdbc 8.5.43 -> 9.0.65
    -- org.apache.tomcat:tomcat-juli 8.5.81 -> 9.0.65

  • PR#5: Bump Flyway Libs (Story Points: 3)
    -- org.flywaydb:flyway-core 4.2.0 -> 8.5.13

6. Bump usage of JUnit 4 to JUnit 5 (Story Points: 8)

  • JUnit 5 is supported with SpringBoot 1.5. @chrisknoll
  • SpringBoot 2.x supports Java 1.8, so no need to disrupt until SpringBoot 3.x is targeted

7. Bump Hibernate Libs to 5.6.10.Final (Story Points: 3)

  • org.hibernate:hibernate-entitymanager 5.5.9.Final -> 5.6.10.Final

8. Bump Spring Boot to version 2.x and <=2.3 (Story Points: 3)

9. Bump remaining aged libraries that are not managed by Spring Boot (Story Points: 2)

10. Upgrade to Spring Boot 2.4 (Story Points: 3)

11. Upgrade to Spring Boot 2.7 (Story Points: 3)

12. Add Code Coverage report generation (Story Points: 1)

13. Bump JDK to Java 11 (Story Points: 5)

14. Bump JDK to Java 17 (Story Points: 3)

15. Analyze code with Sonar Lint and resolve issues (Story Points: 5)

16. Introduce Open Docs API, Swagger, and Actuator Endpoints (Story Points: 3)

17. Monorepo Project Layout (Story Points: 5)

18. Mocked Integrations tests (Story Points: 5)

19. Upgrade to Spring Boot 3.0. (Being released End of December) (Story Points: 8)

20. Upgrade JDK to Java 19 (Story Points: 3)

@alex-odysseus
Copy link
Contributor

Edwin, thank you for the roadmap compilation and sharing your thoughts, they look very detailed @ecruz165

You mentioned you will have some dedicated time for WebAPI / ATLAS development in the upcoming months.
If it is still the case do you see any anticipated target date for the Spring Boot 1.5.x -> Spring Boot 2.x (probably 2.4.x series first) migration contribution?

@ecruz165
Copy link
Contributor Author

ecruz165 commented Oct 25, 2022

Alex, thank you for reviewing the road map - almost thought it might not be discussed/viewed. And apologies for my overly ambitious PR on the first contribution. I am new to contributing to an open-source project. I would like to say I can safely promise 3 full working days at a minimum.

I believe mid/late December would be possible. If things go smoothly, maybe faster. Every improvement would be a PR, as described above. That would be about 16 PRs.

I would like to know what your checkout process is so I can do the same before creating a PR. I have all but 2 unit tests passing, and the 2 flaky unit tests do pass when run alone. Currently, the IT tests don't run on my setup. Maybe I am missing data cause the results are empty. And another thing, I had to look for a workaround to run tests with embedded Postgres, maybe cause I work on a mac. @alex-odysseus @chrisknoll @t-abdul-basser.

@chrisknoll
Copy link
Collaborator

So I might have been unclear about separate PR's in other threads, so I don't want to confuse anyone about when we want separate PRs: We can bundle these PRs together as one if the topic of a PR is to update dependencies. I believe that in other PRs where I've asked for separate PRs, it was because there was a suggestion to make an additional change unrelated to the topic, and I asked that it be dealt with separately.

One reason for separate PRs in this context is that if we have lower risk dependencies that can be grouped together, while an other series of PRs that are higher risk can be created, such that we can get a bunch of dependencies fixed on one shot, but then we have a set of other prs where half may work out, and the other half may not...if they were all in 1 PR we'd hold up 50% of fixes beccause of challenges in the other 50%. So, if you think we can nail all of them in 1 PR, fine. If we can group these into a few PRs because it makes logical sense to group them, also fine. I don't think we need to have 1 pr per dependency, that would be cumbersome and probably unnecessary.

@ecruz165
Copy link
Contributor Author

ecruz165 commented Oct 25, 2022

I updated work sets to mitigate risk with estimated story points and did a minor reorg based on what I believe is most important. @chrisknoll @alex-odysseus @t-abdul-basser

@t-abdul-basser
Copy link
Contributor

Thanks @ecruz165!

@RowanErasmus RowanErasmus moved this from Needs Review to Review Complete in Atlas/WebAPI Issue Triage Apr 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Review Complete
Development

No branches or pull requests

4 participants