Spring Boot 3 #1623

shubham1g5 · 2024-09-04T17:07:56Z

Technical Summary

JIRA

Update to Spring Boot 3

Safety Assurance

Safety story

Relying on automated unit tests and staging scripts
Will keep on staging for a week or so to catch anything unexpected

QA Plan

https://dimagi.atlassian.net/browse/QA-7246

Special deploy instructions

This PR can be deployed after merge with no further considerations.

Rollback instructions

This PR can be reverted after deploy with no further considerations.

Review

The set of people pinged as reviewers is appropriate for the level of risk of the change.

codecov · 2024-11-02T09:40:34Z

Codecov Report

Attention: Patch coverage is 90.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 70.27%. Comparing base (bb7cdbb) to head (b5fa164).

Files with missing lines	Patch %	Lines
...yer/application/GlobalDefaultExceptionHandler.java	0.00%	1 Missing ⚠️
.../org/commcare/formplayer/web/client/WebClient.java	50.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1623      +/-   ##
============================================
+ Coverage     70.22%   70.27%   +0.04%     
- Complexity     2007     2010       +3     
============================================
  Files           254      254              
  Lines          7902     7908       +6     
  Branches        745      745              
============================================
+ Hits           5549     5557       +8     
  Misses         2070     2070              
+ Partials        283      281       -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

shubham1g5 · 2024-11-02T13:39:30Z

src/main/java/org/commcare/formplayer/configuration/WebSecurityConfig.java

+        // By setting the csrfRequestAttributeName to null, the CsrfToken must first be loaded to determine what
+        // attribute name to use. This causes the CsrfToken to be loaded on every request.
+        CsrfTokenRequestAttributeHandler requestHandler = new CsrfTokenRequestAttributeHandler();
+        requestHandler.setCsrfRequestAttributeName(null);


Noting that this change was in response to a change made in Spring 3 that defers the csrf tokens lookup until needed optimising reads of HttpSession. Although that breaks all requests to FP as 403 error due to csrf token coming as undefined in request headers. In response, I first implemented the opt out steps mentioned here, that didn't solve the problem completely (Half the times requests were with csrf undefined and got errored with 403 while on trying again the csrf was there in request payloads, but I could not get to the bottom of why it was happening). Ultimately I decided to completely opt out of deferred tokens behaviour by follow the steps listed here.

Jtang-1 · 2024-11-04T21:46:30Z

src/main/resources/application.properties

@@ -26,7 +26,6 @@ management.health.diskspace.threshold=1073742000

 # metrics
 management.statsd.metrics.export.flavor=datadog
-management.metrics.web.server.request.autotime.enabled=false


the migration guide mentions "Should be applied at the ObservationRegistry level.". Do we need to do that?

Do you have a reference to this ? I just saw that it's been removed.

https://github.com/spring-projects/spring-boot/wiki/Spring-Boot-3.0.0-RC1-Configuration-Changelog#removed-in-300-rc1

More info here: https://github.com/spring-projects/spring-boot/wiki/Spring-Boot-3.0-Migration-Guide#micrometer-and-metrics-changes and here: spring-projects/spring-boot#41745

Overall I think this is OK though I don't recall why we turned that metric off.

snopoke

Overall this looks good to me.

snopoke · 2024-11-06T07:30:23Z

src/main/resources/application.properties

@@ -26,7 +26,6 @@ management.health.diskspace.threshold=1073742000

 # metrics
 management.statsd.metrics.export.flavor=datadog
-management.metrics.web.server.request.autotime.enabled=false


More info here: https://github.com/spring-projects/spring-boot/wiki/Spring-Boot-3.0-Migration-Guide#micrometer-and-metrics-changes and here: spring-projects/spring-boot#41745

Overall I think this is OK though I don't recall why we turned that metric off.

snopoke · 2024-11-06T07:31:11Z

src/test/java/org/commcare/formplayer/repo/FormSessionRepoTest.java

        formSessionRepo.saveAndFlush(loaded);
        assertThat(loaded.getDateCreated()).isEqualTo(dateCreated);
-        assertThat(loaded.getVersion()).isEqualTo(2);
+        assertThat(loaded.getVersion()).isEqualTo(1);


Can you explain these changes. They don't seem related to the Spring upgrade.

I could not really find the related change documentation for this in hibernate but after upgrading to SB 3, the version doesn't increment unless you make a change to the object. My guess is that the change is caused by shifting to the Jakarta persistence apis from javax but I don't really have a supportive reference here that can explain this change.

Read more: https://blogs.oracle.com/javamagazine/post/transition-from-java-ee-to-jakarta-ee

…ferred csrf token

…o load the full application context

…be able to check exception messages in tests

shubham1g5 · 2024-11-13T11:05:48Z

This has been on staging for a week and I have not seen anything related, as such I am planning to merge this after tomorrow's deploy so that it will get to prod in next week's deploy.

snopoke · 2024-11-13T11:34:43Z

This has been on staging for a week and I have not seen anything related, as such I am planning to merge this after tomorrow's deploy so that it will get to prod in next week's deploy.

Has someone tested SMS forms?

shubham1g5 · 2024-11-13T11:37:23Z

Has someone tested SMS forms?

Not really, are you concerned because of the security changes ?

snopoke · 2024-11-13T11:38:12Z

Has someone tested SMS forms?

Not really, are you concerned because of the security changes ?

yes, that's one place where auth differs from webapps

shubham1g5 · 2024-11-13T11:39:20Z

Great to know, I will make sure we get that tested. Thanks.

shubham1g5 changed the title ~~Sprint Boot 3~~ Spring Boot 3 Sep 15, 2024

shubham1g5 force-pushed the sb3 branch from e690a81 to 4671ece Compare November 2, 2024 09:36

shubham1g5 requested review from Robert-Costello, snopoke and Jtang-1 November 2, 2024 12:46

shubham1g5 commented Nov 2, 2024

View reviewed changes

shubham1g5 marked this pull request as ready for review November 2, 2024 14:03

Jtang-1 approved these changes Nov 4, 2024

View reviewed changes

snopoke reviewed Nov 6, 2024

View reviewed changes

shubham1g5 added 18 commits November 6, 2024 18:31

update to sb 3 and remove mockito-inline as its merged in mockito-core

5daf08c

Fix unrecorginsed javax imports to use jakarta namespace

d039c05

Read more: https://blogs.oracle.com/javamagazine/post/transition-from-java-ee-to-jakarta-ee

No antMatchers method anymore

d3de514

handle for no getRawStatusCode method anymore

652a49f

use correct return type

4a3de9b

fix import

b6fa265

remove unused repo method

744d7b7

correct exception type

f42db1d

use correct sentry dependency for spring boot 3

2dc5cc0

address method deprecations

0c70ae4

Explicitly cConfigure CsrfToken with 5.8 Defaults instead of using de…

2bc6bb9

…ferred csrf token

test fix: A change is necessary for version to be bumped

0e0a841

Fix for no bean named entityManagerFactory by using @SpringBootTest t…

7b52ff1

…o load the full application context

Rename property to the new name

56ed8d7

Property no longer present in spring boot 3

8da5e2d

bump Spring boot version to latest 3.x

3591724

Use correct flyway dependency for Flyway 10.10

74d9175

Enable INCLUDE_SOURCE_IN_LOCATION which is now disable by default to …

b5fa164

…be able to check exception messages in tests

shubham1g5 force-pushed the sb3 branch from a34064b to b5fa164 Compare November 7, 2024 05:13

shubham1g5 requested a review from snopoke November 13, 2024 11:04

shubham1g5 requested a review from Jtang-1 November 13, 2024 11:04

snopoke approved these changes Nov 13, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Spring Boot 3 #1623

Spring Boot 3 #1623

shubham1g5 commented Sep 4, 2024 •

edited

Loading

codecov bot commented Nov 2, 2024 •

edited

Loading

shubham1g5 Nov 2, 2024

Jtang-1 Nov 4, 2024

shubham1g5 Nov 5, 2024

Jtang-1 Nov 5, 2024

snopoke Nov 6, 2024

snopoke left a comment

snopoke Nov 6, 2024

snopoke Nov 6, 2024

shubham1g5 Nov 6, 2024

shubham1g5 commented Nov 13, 2024

snopoke commented Nov 13, 2024

shubham1g5 commented Nov 13, 2024

snopoke commented Nov 13, 2024

shubham1g5 commented Nov 13, 2024

Spring Boot 3 #1623

Are you sure you want to change the base?

Spring Boot 3 #1623

Conversation

shubham1g5 commented Sep 4, 2024 • edited Loading

Technical Summary

Safety Assurance

Safety story

QA Plan

Special deploy instructions

Rollback instructions

Review

codecov bot commented Nov 2, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snopoke left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shubham1g5 commented Nov 13, 2024

snopoke commented Nov 13, 2024

shubham1g5 commented Nov 13, 2024

snopoke commented Nov 13, 2024

shubham1g5 commented Nov 13, 2024

shubham1g5 commented Sep 4, 2024 •

edited

Loading

codecov bot commented Nov 2, 2024 •

edited

Loading