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
Spring Boot 3 #1230
Spring Boot 3 #1230
Conversation
...xample-shared/src/test/java/com/netflix/graphql/dgs/example/shared/HelloDataFetcherTest.java
Outdated
Show resolved
Hide resolved
...otlin/com/netflix/graphql/dgs/metrics/micrometer/utils/SimpleQuerySignatureRepositoryTest.kt
Outdated
Show resolved
Hide resolved
import org.springframework.boot.context.properties.NestedConfigurationProperty | ||
import org.springframework.boot.context.properties.bind.DefaultValue | ||
|
||
@ConfigurationProperties(prefix = DgsAPQSupportProperties.PREFIX) | ||
@ConstructorBinding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spring Boot 3 @ConstructorBinding
update
... This means that the binder will find a constructor with the parameters that you wish to have bound. If your class has multiple constructors, the @ConstructorBinding annotation can be used to specify which constructor to use for constructor binding.
Thanks for the doing the pre-work and investigating this. Just to provide some context, we have some constraints with switching the framework to Spring Boot 3 due to internal usage and that will affect how we make this available. We don't have concrete timelines yet, but will be planning for it. I will post updates on this thread as and when we have more information from our side. |
Is there a way to test the WIP version? |
You'll have to check out the branch locally, publish to your local maven repo using |
@@ -33,7 +34,7 @@ import org.springframework.context.annotation.Configuration | |||
havingValue = "true", | |||
matchIfMissing = true | |||
) | |||
@Configuration(proxyBeanMethods = false) | |||
@AutoConfiguration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another notable change in functionality as of 3.0.0-M5.
With this release, support for registering auto-configurations in spring.factories has been removed in favor of the imports file.
A new
@AutoConfiguration
annotation has been introduced. It should be used to annotate top-level auto-configuration classes that are listed in the newMETA-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports
file, replacing@Configuration
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Under the hood, @AutoConfiguration
is basically a more advanced @Configuration
with proxyBeanMethods
set to false.
@srinivasankavitha & @berngp, thank you both for your continued support and eyes on this PR. I wanted to provide a quick update on the Spring Boot 3 release timeline. The release candidate process has begun. Last week the Spring team began publishing release candidates for Spring Framework & Spring Security, which this PR is now using. Spring announced today that they will be releasing the Spring Boot 3.0.0-RC1 later this evening. https://spring.io/blog/2022/10/20/spring-framework-6-0-0-rc2-available-now I will update this PR with the latest release candidates as they become available. It appears that the Spring team is on track for a GA release in mid-late November. Cheers! |
Thanks for all the work on the PR so far! |
@NestedConfigurationProperty | ||
var autotime: AutoTimeProperties = AutoTimeProperties(), | ||
var autotime: PropertiesAutoTimer = PropertiesAutoTimer(autotimeProperties), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one probably needs some input from @srinivasankavitha & @berngp.
This change wasn't well documented and I had to do some digging to figure out why this change was made. Spring Boot 3.0.0-RC1 attempts to decouple Micrometer from MetricsProperties
and MetricsProperties
references AutoTimeProperties
. Seems reasonable...
See spring boot change here: spring-projects/spring-boot@180d0ed
I composed the properties and timer in this data class to provide easy access to AutoTimeProperties
, but very open to different approaches / ideas.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach looks good to me!
Another quick update! The release candidates seem to be stabilizing from the Spring team. I'm seeing less refactoring or feature addition in these latest updates. I've updated this branch again to use the latest release candidates. We also now have a targeted release date, the Spring team announced:
|
Sounds great, thanks for the update @srinivasankavitha! Should I continue to target FWIW in my experience, master usually becomes the most recent release and another branch is maintained for backwards compatibility. Preserves trunk based development for the new work, but provides the flexibility of a short-lived release branch for these weird times when internal limitations complicate ideal workflows. 3.x support would == master, corresponding with a DGS 6.x release. Please let me know if I can help in anyway!
Thanks for bringing this back to my attention. I would be happy to address this update in my PR. |
import java.lang.annotation.*; | ||
|
||
@Target(ElementType.METHOD) | ||
@Retention(RetentionPolicy.RUNTIME) | ||
@DgsData(parentType = "Mutation") | ||
@Inherited | ||
public @interface DgsMutation { | ||
@AliasFor(annotation = DgsData.class) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spring Framework 6.0 was logging a semi-noisy warning about an upcoming deprecation. It was triggered by all of @DgsData
's children. Example output:
o.s.c.annotation.AnnotationTypeMapping : Support for convention-based annotation attribute overrides is deprecated and will be removed in Spring Framework 6.1. Please annotate the following attributes in @com.netflix.graphql.dgs.DgsMutation with appropriate @AliasFor declarations: [field]
The change is discussed here: spring-projects/spring-framework#28760
Since explicit overrides are favorable to implicit overrides, and since the support for convention-based overrides increases the complexity of Spring's annotation search algorithms, we will deprecate convention-based overrides in 6.0 and remove the support in 6.1.
For this PR, I kept @Inherited
in place on all annotations. I did some digging into why @Inherited
was being used in DGS and found: #1110.
In a separate PR, I'd be happy to implement Spring's AnnotationUtils
to allow us to remove @Inherited
. Just LMK!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. Yes, we can tackle that as part of a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finally managed to go through all the whole process independently, and all the changes listed here look great.
@NestedConfigurationProperty | ||
var autotime: AutoTimeProperties = AutoTimeProperties(), | ||
var autotime: PropertiesAutoTimer = PropertiesAutoTimer(autotimeProperties), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach looks good to me!
import java.lang.annotation.*; | ||
|
||
@Target(ElementType.METHOD) | ||
@Retention(RetentionPolicy.RUNTIME) | ||
@DgsData(parentType = "Mutation") | ||
@Inherited | ||
public @interface DgsMutation { | ||
@AliasFor(annotation = DgsData.class) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. Yes, we can tackle that as part of a separate PR.
Thanks so much for all the great work in this PR! 👏 I still need to do some more testing with all these changes, so will work on that over the next few days.
Our current plan is to aim for early Jan for a target 3.x compatible DGS release once the above is ready. We'll backport only essential changes and fixes to 2.7 branch, but new feature development will be enabled on master going forward. |
Hello - Thanks for all your work on this PR - We are moving to Spring Boot 3 across our projects and so anxiously awaiting this update. I'm trying to get a ahead and give this update a test. I built the branch out to my local maven repo and pulled it in successfully However when I start my app, I am not seeing the DGS framework initialize and no /graphql endpoint is available. Do you know if there are other updates to ensure Spring Boot is autowiring all the needed DGS components? Also looks like Spring Cloud final is supposed to be released today: https://calendar.spring.io/ Thank you! |
@Stuckya - I am thinking of merging your PR to a spring-boot-3 branch to further test and add any fixes on top of. Once we are ready, I can merge that branch to main (likely beginning of Jan). This will allow for us to make further fixes, if needed based on our testing. Does that sound ok? |
I updated this branch today to make use of the spring-cloud-dependencies 2022 GA release, which means we are no longer relying on the milestone spring repository.
@srinivasankavitha - Of course, that sounds like it will make things easier for you all to continue making updates util early Jan. Please let me know if I can help in any way. |
Great, I'll create a new branch and merge your PR there and continue with my testing. It's looking really good so far! Thanks for the change you just made to update the spring-cloud-dependencies to GA. I am updating the example projects so we can get the build passing here and will take it from there. Thanks again for the contributions! |
@GrantGochnauer Have you checked to make sure you are on the same branch as this PR? My guess is you don't have the changes to make it work with spring boot 3. I am able to use a local snapshot built from this branch in my test projects successfully. |
@Stuckya - could you please make the following changes as well as part of the PR. I updated example projects to Spring Boot 3 and this branch needs to use the updated examples for the build to pass. I cannot merge to a branch until then anyway.
|
You got it, update coming shortly. |
@srinivasankavitha, should be good to go! |
Thanks @Stuckya - build seems to have passed ci checks as well with your latest commit. 🥳 I'll merge it to a spring-boot-3-candidate branch and continue testing it. As mentioned earlier, I'll merge the branch to master once we are ready and have created a separate branch for 2.7. So early Jan, 2023. |
Pull request checklist
Pull Request type
Changes in this PR
Describe the new behavior from this PR, and why it's needed
Issue #948
Spring Boot 3 has been released! This PR has prepared
dgs-framework
for Spring Boot 3. Many project maintainers are currently validating their projects against Sprint Boot 3 to prepare for the upgrade. I have tested this branch extensively against my own Spring Boot 3 project.Version upgrades:
3.0.0
6.0.3
6.0.1
2022.0.0
17
Alternatives considered
Describe alternative implementation you have considered
N/A