Spring upgrade to 3.3.6 #8325

DavidMcClatchey · 2024-11-27T21:38:58Z

BACKEND PULL REQUEST

Related Issue

Resolves Spring 3.3 Upgrade #8209

Changes Proposed

Updates all relevant dependencies in the build.gradle and gradle.lockfile in order to update overall to Spring 3.3.6
Adds new getter for the isDeleted field in EternalAuditedEntity that starts with get in order for GraphQL introspection from the Spring GraphQL dependency to succeed
Updates deprecated GraphQL methods and method to retrieve HTTP status code

Additional Information

During this upgrade, Spring 3.4 came out, so we should upgrade to that in the future

Testing

Build and run locally
Deployed on dev3

Checklist for Primary Reviewer

Any large-scale changes have been deployed to test, dev, or pentest and smoke tested
Changes comply with the SimpleReport Style Guide
Changes with security implications have been approved by a security engineer (changes to authentication, encryption, handling of PII, etc.)
Any dependencies introduced have been vetted and discussed

DavidMcClatchey · 2024-12-03T20:08:21Z

.../src/main/java/gov/cdc/usds/simplereport/config/scalars/datetime/DateTimeScalarCoercion.java


 @Slf4j
-class DateTimeScalarCoercion implements Coercing<Date, Object> {
+public class DateTimeScalarCoercion implements Coercing<Date, Object> {


Class and methods were made public for unit testing, similar to FlexibleDateCoercion.java, which also implements Coercing.

DavidMcClatchey · 2024-12-03T20:11:10Z

.../src/main/java/gov/cdc/usds/simplereport/config/scalars/datetime/DateTimeScalarCoercion.java

+      @NotNull Object dataFetcherResult,
+      @NotNull GraphQLContext graphQLContext,
+      @NotNull Locale locale)
+      throws CoercingSerializeException {


Previous overridden method signature was deprecated in favor of this.

Question for my own understanding... (I have never upgraded Spring before 😅) How did you know this method should now look like this? Was it trial and error or is there some resource you are taking this from? 🤔 This question applies in general for the overridden methods that changed 👀

IIRC, I was mainly alerted through Intellij after updating the dependency related to the change. e.g. after updating the dependency but before updating the code, it looked like this:

So then after seeing that, I'd look into the Coercing interface for what should replace it. Luckily there are usually replacements for deprecated methods and they're not just wiped out entirely.

Another Intellij trick is to right click on the root folder, then Analyze -> Run Inspection by Name and type in "deprecated" and then click on the one for Java:

Then it'll show you anything that's deprecated or that altogether doesn't work anymore 🧐

Thank you for the detailed explanation and going over this in eng sync as well 🙌

DavidMcClatchey · 2024-12-03T20:11:16Z

.../src/main/java/gov/cdc/usds/simplereport/config/scalars/datetime/DateTimeScalarCoercion.java

-    if (((String) input).length() == 0) {
+  public Date parseValue(
+      @NotNull Object input, @NotNull GraphQLContext graphQLContext, @NotNull Locale locale)
+      throws CoercingParseValueException {


Previous overridden method signature was deprecated in favor of this.

DavidMcClatchey · 2024-12-03T20:11:21Z

.../src/main/java/gov/cdc/usds/simplereport/config/scalars/datetime/DateTimeScalarCoercion.java

+      @NotNull CoercedVariables coercedVariables,
+      @NotNull GraphQLContext graphQLContext,
+      @NotNull Locale locale)
+      throws CoercingParseLiteralException {


Previous overridden method signature was deprecated in favor of this.

DavidMcClatchey · 2024-12-03T20:11:32Z

...d/src/main/java/gov/cdc/usds/simplereport/config/scalars/localdate/FlexibleDateCoercion.java

+  public Object serialize(
+      @NotNull Object dataFetcherResult,
+      @NotNull GraphQLContext graphQLContext,
+      @NotNull Locale locale) {


Previous overridden method signature was deprecated in favor of this.

DavidMcClatchey · 2024-12-03T20:11:36Z

...d/src/main/java/gov/cdc/usds/simplereport/config/scalars/localdate/FlexibleDateCoercion.java

@@ -45,7 +53,8 @@ public Object serialize(Object dataFetcherResult) {
  }

  @Override
-  public Object parseValue(Object input) {
+  public Object parseValue(
+      @NotNull Object input, @NotNull GraphQLContext graphQLContext, @NotNull Locale locale) {


Previous overridden method signature was deprecated in favor of this.

DavidMcClatchey · 2024-12-03T20:11:40Z

...d/src/main/java/gov/cdc/usds/simplereport/config/scalars/localdate/FlexibleDateCoercion.java

+      @NotNull Value<?> input,
+      @NotNull CoercedVariables coercedVariables,
+      @NotNull GraphQLContext graphQLContext,
+      @NotNull Locale locale) {


Previous overridden method signature was deprecated in favor of this.

DavidMcClatchey · 2024-12-03T20:12:39Z

backend/src/main/java/gov/cdc/usds/simplereport/db/model/EternalAuditedEntity.java

@@ -21,6 +21,10 @@ public boolean isDeleted() {
    return isDeleted;
  }

+  public boolean getIsDeleted() {
+    return isDeleted;
+  }


This getter is needed in order for GraphQL introspection to work properly or else startup fails. It seems to be looking for a method with "get" in the name.

Is this something that would be helpful to add a comment about in the code? 🤔

Good call! We wouldn't want somebody seeing there are two getters and deleting this one.

why not rename the method in Eternal.java to avoid having two getters?

Thanks for calling that out, I'll add that now

DavidMcClatchey · 2024-12-03T20:15:14Z

backend/src/main/java/gov/cdc/usds/simplereport/logging/AuditLoggingInstrumentation.java

@@ -31,11 +32,11 @@
 @Component
 @Slf4j
 @RequiredArgsConstructor
-public class AuditLoggingInstrumentation extends SimpleInstrumentation {
+public class AuditLoggingInstrumentation extends SimplePerformantInstrumentation {


SimpleInstrumentation is deprecated in favor of SimplePerformantInstrumentation.

good to know it is performant now

DavidMcClatchey · 2024-12-03T20:16:27Z

backend/src/test/java/gov/cdc/usds/simplereport/service/PatientBulkUploadServiceAsyncTest.java

@@ -406,7 +406,7 @@ void requiresPermissionAddPatientsToFacility() throws IOException {
    CompletableFuture<Set<Person>> futurePatients =
        this._service.savePatients(content, secondFacilityId);
    ExecutionException caught = assertThrows(ExecutionException.class, futurePatients::get);
-    assertThat(caught.getCause().getClass()).isEqualTo(AccessDeniedException.class);
+    assertThat(caught.getCause().getClass()).isEqualTo(AuthorizationDeniedException.class);


AccessDeniedException deprecated in favor of AuthorizationDeniedException

Thanks for calling this out! I'm definitely making use of this in one of my PRs so if this goes in I'll update my usage as well!

mpbrown

Great work on this upgrade David!

sonarqubecloud · 2024-12-10T19:46:41Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
80.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

DavidMcClatchey added 16 commits November 8, 2024 12:28

builds but doesn't run

85bddab

Merge branch 'main' into david/spring-upgrade

1aa7443

deprecated method and graphql-java update

55ad1a1

merging main

424c444

Spring 3.3 pause to do 3.2

6311f57

spring 3.2 building and running

fb5a562

spring 3.2.11 building and running

a41a16a

minimum dependencies for 3.3.0 failure

7d99c32

spring 3.3.0 building and running

38dfce1

Merge branch 'main' into david/spring-upgrade

d63802a

3.3.1 building and running

87f3f03

3.3.3 building and running

970183b

3.3.4 building and running

2dc3418

3.3.6 building and running

8160849

fixing tests

fe1289d

cleanup

3c2e7c4

DavidMcClatchey temporarily deployed to dev3 November 27, 2024 21:40 — with GitHub Actions Inactive

DavidMcClatchey temporarily deployed to dev3 November 27, 2024 21:48 — with GitHub Actions Inactive

fixing backend linting

7b3187f

DavidMcClatchey temporarily deployed to dev3 November 27, 2024 21:53 — with GitHub Actions Inactive

DavidMcClatchey added 7 commits December 2, 2024 12:28

increasing test coverage for sonar

5e5ca77

timezone agnostic tests

df2e3d5

formatting

242f222

testing linting precommit

1d7ed9d

testing linting precommit

7a49424

checkstyleTest precommit takes too long

ef61f28

sonar fix

dd11197

DavidMcClatchey marked this pull request as ready for review December 3, 2024 17:23

DavidMcClatchey commented Dec 3, 2024

View reviewed changes

emyl3 mentioned this pull request Dec 4, 2024

Bump org.springframework:spring-web from 6.0.18 to 6.2.0 in /backend #8285

Merged

DavidMcClatchey added 2 commits December 4, 2024 14:28

adding getter comment

c2f3202

more comment detail

b384d60

DavidMcClatchey assigned mehansen, DanielSass, emyl3, mpbrown and bobbywells52 Dec 4, 2024

comment format change

87ed870

mehansen self-requested a review December 5, 2024 17:31

renaming isDeleted method to getIsDeleted

33fff01

mpbrown previously approved these changes Dec 10, 2024

View reviewed changes

DanielSass previously approved these changes Dec 10, 2024

View reviewed changes

DavidMcClatchey dismissed stale reviews from DanielSass and mpbrown via 33fff01 December 10, 2024 19:28

DanielSass approved these changes Dec 10, 2024

View reviewed changes

mpbrown approved these changes Dec 10, 2024

View reviewed changes

DavidMcClatchey added this pull request to the merge queue Dec 10, 2024

Merged via the queue into main with commit 3b272d0 Dec 10, 2024
34 checks passed

DavidMcClatchey deleted the david/spring-upgrade branch December 10, 2024 21:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Spring upgrade to 3.3.6 #8325

Spring upgrade to 3.3.6 #8325

DavidMcClatchey commented Nov 27, 2024

DavidMcClatchey Dec 3, 2024 •

edited

Loading

DavidMcClatchey Dec 3, 2024

emyl3 Dec 4, 2024 •

edited

Loading

DavidMcClatchey Dec 4, 2024

emyl3 Dec 6, 2024

DavidMcClatchey Dec 3, 2024

DavidMcClatchey Dec 3, 2024

DavidMcClatchey Dec 3, 2024

DavidMcClatchey Dec 3, 2024

DavidMcClatchey Dec 3, 2024

DavidMcClatchey Dec 3, 2024

emyl3 Dec 4, 2024

DavidMcClatchey Dec 4, 2024

mehansen Dec 6, 2024

DavidMcClatchey Dec 6, 2024 •

edited

Loading

DavidMcClatchey Dec 3, 2024 •

edited

Loading

DanielSass Dec 4, 2024

DavidMcClatchey Dec 3, 2024

emyl3 Dec 4, 2024

mpbrown left a comment

sonarqubecloud bot commented Dec 10, 2024

Spring upgrade to 3.3.6 #8325

Spring upgrade to 3.3.6 #8325

Conversation

DavidMcClatchey commented Nov 27, 2024

BACKEND PULL REQUEST

Related Issue

Changes Proposed

Additional Information

Testing

Checklist for Primary Reviewer

DavidMcClatchey Dec 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emyl3 Dec 4, 2024 • edited Loading

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DavidMcClatchey Dec 6, 2024 • edited Loading

Choose a reason for hiding this comment

DavidMcClatchey Dec 3, 2024 • edited Loading

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

mpbrown left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Dec 10, 2024

Quality Gate passed

DavidMcClatchey Dec 3, 2024 •

edited

Loading

emyl3 Dec 4, 2024 •

edited

Loading

DavidMcClatchey Dec 6, 2024 •

edited

Loading

DavidMcClatchey Dec 3, 2024 •

edited

Loading