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

feat!: Update to Spring 6, Boot 3 #13545

Merged
merged 20 commits into from
Jul 19, 2024
Merged

feat!: Update to Spring 6, Boot 3 #13545

merged 20 commits into from
Jul 19, 2024

Conversation

matrei
Copy link
Contributor

@matrei matrei commented Jul 12, 2024

No description provided.

matrei added 14 commits July 12, 2024 08:22
- Remove overridden methods that has the same behavior as the default implementations.
- `postProcessPropertyValues()` is removed in Spring 6 and replaced with `postProcessProperties()` but this is now removed.
- Add @nullable and @nonnull method parameter annotations to remove warning in Intellij.
This class is deprecated and uses API removed in Spring 6 -
`AbstractBeanFactory.getAccessControlContext()`

It also uses deprecated API in Java 17 - `java.lang.AccessController`
and `java.lang.SecurityManager`
The Spring CommonsMultipartResolver is removed in Spring 6.

The grails-web-fileupload project seems to have been created to address
a specific problem with the Safari browser. Hopefully this is resolved now
and this fix no longer necessary.
The overridden methods in this class has been removed in Spring 6.
This class does not serve a purpose any longer and is therefore deprecated.
NestedIOException has been removed in Spring 6.
IOException should be used directly instead.
The class hierarchy for AnnotationMetadataReadingVisitor has been
removed in Spring 6 with no public replacement. This commit adds
them to the grails-core project instead as a workaround until
another solution is found.
Spring has introduced a new `spring.config.activate.on-profile` config
property to signal if a config document should be included or not.

This commit refactors the existing `shouldSkipBlock` method to take
that into consideration. However, the usage of `shouldSkipBlock` was
removed in commit 03bef04. I'm not sure if that was an oversight, but
I have added the usage of this method back (now named `springProfileExclude`).
…ers`

`grails-plugin-converters` has not yet been converted to jakarta.servlet.
This commit makes the test suite pass until that is fixed.

`GrailsMockHttpServletRequestTests` had to be converted from JUnit to Spock to be able to use `@PendingFeature`.
The `grails-gsp` module has not yet been converted to jakarta.servlet,
causing test failures. To ensure the test suite passes until this
conversion is complete, this commit disables the affected tests using
`@Ignore` (Spock) and `@Disabled` (JUnit5).

As the problem with the tests occurs in the initialization phase,
it was regrettably not possible to use `@PendingFeature`.
Use the new `spring.config.activate.on-profile` config property
instead of the old `spring.profiles`.
@matrei
Copy link
Contributor Author

matrei commented Jul 12, 2024

The @PendingFeature in DefaultXmlRenderSpec should have been in it's own commit.
I'm not sure why that started failing, but let's see if upgrading grails-plugin-converters fixes it later on.

As Micronaut 4 does no longer include `jackson-databind` as a transitive
dependency, it is added as a direct dependency where ObjectMapper is used.

@NonNull
@Override
public <A extends Annotation> Set<A> findAllAnnotationsOnBean(@NonNull String beanName, @NonNull Class<A> annotationType, boolean allowFactoryBeanInit) throws NoSuchBeanDefinitionException {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it intentional that this behavior is introduced in the license header update commit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@osscontributor No, that was not my intention. Must have missed to exclude that change from the commit.

Copy link
Contributor

@sdelamo sdelamo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were these classes ported from Spring because they had been removed from Spring the latest version?

  • RecursiveAnnotationAttributesVisitor
  • RecursiveAnnotationArrayVisitor
  • MethodMetadataReadingVisitor
  • ClassMetadataReadingVisitor
  • AnnotationReadingVisitorUtils
  • AnnotationMetadataReadingVisitor

If that it is the case, we should tell that in these classess javadoc header.

Can we replace?

@Ignore('grails-gsp is not on jakarta.servlet yet')

with:

@PendingFeature

gradle.properties Show resolved Hide resolved
grails-test-suite-web/build.gradle Outdated Show resolved Hide resolved
grails-test-suite-web/build.gradle Outdated Show resolved Hide resolved
@@ -1,5 +0,0 @@
## grails-web-fileupload
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did we remove this module?

super.setStatus(status, errorMessage);
}
}
@Deprecated(forRemoval = true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we remove it in Grails 7 and document the breaking change instead of deprecating it?

@matrei
Copy link
Contributor Author

matrei commented Jul 17, 2024

Were these classes ported from Spring because they had been removed from Spring the latest version?

* `RecursiveAnnotationAttributesVisitor`

* `RecursiveAnnotationArrayVisitor`

* `MethodMetadataReadingVisitor`

* `ClassMetadataReadingVisitor`

* `AnnotationReadingVisitorUtils`

* `AnnotationMetadataReadingVisitor`

The class hierarchy for AnnotationMetadataReadingVisitor has been
removed in Spring 6 with no public replacement. This commit adds
them to the grails-core project instead as a workaround until
another solution is found.

If that it is the case, we should tell that in these classess javadoc header.

Yes, I'll add that 👍

@matrei
Copy link
Contributor Author

matrei commented Jul 17, 2024

Can we replace?

@Ignore('grails-gsp is not on jakarta.servlet yet')

with:

@PendingFeature

As the problem with the tests occurs in the initialization phase,
it was regrettably not possible to use @PendingFeature.

@sdelamo sdelamo merged commit a6dd1ec into 7.0.x Jul 19, 2024
11 checks passed
@matrei matrei deleted the matrei/spring-6-boot-3 branch August 20, 2024 05:39
matrei added a commit that referenced this pull request Aug 22, 2024
This commit enables the tests that were disabled
before `grails-gsp` was migrated to `jakarta.servlet`.

See #13545
matrei added a commit that referenced this pull request Aug 22, 2024
…jakarta-servlet` (#13590)

* fix(deps): Update to `grails-testing-support:4.0.0-SNAPSHOT`

* test(enable): `grails-gsp` now on `jakarta.servlet`

This commit enables the tests that were disabled
before `grails-gsp` was migrated to `jakarta.servlet`.

See #13545
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants