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

Fix cache auto configuration #1484

Merged
merged 1 commit into from
Jul 3, 2023
Merged

Conversation

msdousti
Copy link
Contributor

@msdousti msdousti commented Jul 3, 2023

Using Riptide without caching enabled must not require httpclient5-cache.

Description

  • Fixed RiptideProperties such that it does not require org.apache.hc.client5.http.impl.cache.CacheConfig.
  • Fixed org.springframework.boot.autoconfigure.AutoConfiguration.imports to have separate configs for main and test (otherwise, production code would require MockRestServiceServer to be in class path)
  • Updated README.md to use httpclient5-cache instead of httpclient-cache.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.

@fatroom
Copy link
Member

fatroom commented Jul 3, 2023

👍

@fatroom fatroom merged commit 11d3af3 into zalando:main Jul 3, 2023
3 checks passed
@cberg-zalando cberg-zalando deleted the fix_cache_auto_config branch July 4, 2023 05:01
@@ -35,6 +36,7 @@
@Setter
@NoArgsConstructor
@AllArgsConstructor
@Component
Copy link
Member

Choose a reason for hiding this comment

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

Interesting that you need to add this annotation as @ConfigurationProperties should normally make this available to Spring.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Intellij showed an error saying not registered via @enableconfigurationproperties or marked as spring component.

The project compiled anyway, but I followed the advice here:
https://stackoverflow.com/a/57950415/459391

This is expected as @ConfigurationProperties does not make a class a Spring Component. Mark the class with @component and it should work. Note that a class can only be injected if it is a Component.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the insights. Yes, I remember that you have to add this annotation to your main class for example, to get this working. I guess, given that this is a library, it makes sense to follow that approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants