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

Add a constructor to S3ProtocolResolver allowing its usage outside of Spring context #1276

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

klach-ocado
Copy link

@klach-ocado klach-ocado commented Nov 15, 2024

Add a constructor to S3ProtocolResolver allowing its usage outside of Spring context

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

A new constructor was added to S3ProtocolResolver allowing setting required fields directly rather than lazily later using BeanFactory. But the method with BeanFactory is still used by default by S3AutoConfiguration which creates S3ProtocolResolver through @Import(S3ProtocolResolver.class), not through a method with @Bean annotation.

💡 Motivation and Context

In our application we need to fetch some configuration files from S3 before Spring context fully starts, so that properties from S3 will be included in @ConditionalOnProperty, etc. We decided to do that in EnvironmentPostProcessor, we have working proof-of-concept with a copy of S3ProtocolResolver from Spring Cloud AWS but it would be best to use original S3ProtocolResolver, and a lack of proper constructor is blocking that.

We are aware of #161 but we need something lighter.

💚 How did you test it?

S3ProtocolResolverTests which use the same approach as in S3AutoConfiguration, i.e. @Import(S3ProtocolResolver.class) still pass, and the new constructor is not called automatically even if the no-arg one is removed (in such a case there is BeanInstantiationException: Failed to instantiate [io.awspring.cloud.s3.S3ProtocolResolver]: No default constructor found). That also means that the change is backward compatible and it neither affects the existing code nor change the flow.

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • I updated reference documentation to reflect the change
  • All tests passing (plain Java tests are passing, I have problems running @Testcontainers on local machine)
  • No breaking changes

🔮 Next steps

The new constructor could potentially be used to create a bean in the standard way through @Bean method but that would be a breaking change, assuming the no-arg constructor would be removed with that change.

@github-actions github-actions bot added the component: s3 S3 integration related issue label Nov 15, 2024
@maciejwalkowiak maciejwalkowiak added this to the 3.3.0 M2 milestone Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: s3 S3 integration related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants