Skip to content

Fixes for Spring Boot 3.5.0 API #3007

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+22 −1
Open

Fixes for Spring Boot 3.5.0 API #3007

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bf52312
Fixes for Spring Boot 3.5.0 API
mschout May 23, 2025
3a403a5
Make isHalEnabled static
mschout Jun 3, 2025
fc5ef23
Avoid type cast
mschout Jun 3, 2025
ccf2d2d
Update springdoc-openapi-starter-common/src/main/java/org/springdoc/c…
mschout Jun 3, 2025
392aec6
Fix formatting
mschout Jun 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion ...openapi-starter-common/src/main/java/org/springdoc/core/providers/HateoasHalProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,15 @@

package org.springdoc.core.providers;

import java.util.List;
import java.util.Optional;

import jakarta.annotation.PostConstruct;

import org.springframework.boot.autoconfigure.hateoas.HateoasProperties;
import org.springframework.hateoas.mediatype.hal.Jackson2HalModule;
import org.springframework.lang.NonNull;
import org.springframework.util.ReflectionUtils;

/**
* The type Hateoas hal provider.
Expand Down Expand Up @@ -79,8 +82,26 @@ protected void init() {
*/
public boolean isHalEnabled() {
return hateoasPropertiesOptional
.map(HateoasProperties::getUseHalAsDefaultJsonMediaType)
.map(HateoasHalProvider::isHalEnabled)
.orElse(true);
}

private static boolean isHalEnabled(@NonNull HateoasProperties hateoasProperties) {
// In spring-boot 3.5, the method name was changed from getUseHalAsDefaultJsonMediaType to isUseHalAsDefaultJsonMediaType
var possibleMethodNames = List.of("isUseHalAsDefaultJsonMediaType", "getUseHalAsDefaultJsonMediaType");

for (var methodName : possibleMethodNames) {
var method = ReflectionUtils.findMethod(HateoasProperties.class, methodName);
if (method != null) {
var result = ReflectionUtils.invokeMethod(method, hateoasProperties);
if (result instanceof Boolean halEnabled) {
return halEnabled;
}

throw new IllegalStateException("Method " + methodName + " did not return a boolean value");
Copy link

@StefanSchrass StefanSchrass Jun 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Initial implementation defaults to true if no HateoasProperties are present. Could this also be applicable here?

Copy link
Contributor Author

@mschout mschout Jun 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

.orElse(true) will still be called if the Optional<HateoasProperties> hateoasPropertiesOptional is empty

Copy link

@StefanSchrass StefanSchrass Jun 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

totally, but its sheer existence doesn't mean that it is important to the application.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I'm not following. If HATEOAS isn't available, the .map() call is a no-op here. This project needs to be compatible with upstream one way or another. If this project doesn't know how to figure out if HAL is active or not, then the only options here (as suggested by upstream) is to either use Reflection (as this PR does), or, to bump the min required version of SB to 3.5.0 and call the new method directly, at the expense of breaking compatibility with older versions of Spring Boot). If upstream changes the property methods yet again, silently returning true if this project couldn't figure out how to determine if HAL is active or not is worse than bailing out IMO. But I'm not part of the springdoc team, so this is not my call.

FWIW, this is far from the only problem blocking me from personally migrating to 3.5.0 at this point. There was a rather serious regression in, for example, spring-data-jpa. Unless more people test -RC releases or milestone releases of upstream, the .0 release is really the first real shake out of problems and thats what we're seeing here.

}
}

throw new IllegalStateException("No suitable method found to determine if HAL is enabled");
Copy link

@StefanSchrass StefanSchrass Jun 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Initial implementation defaults to true if no HateoasProperties are present. Could this also be applicable here?

Copy link

@kimberninger kimberninger Jun 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is already the case. Have a look at line 86 (formerly line 83).

In my opinion, the exceptional behavior covered in isHalEnabled serves a separate use case. Here, the HateoasProperties are already known to exist, but for some unexpected reason they do neither respond to isUseHalAsDefaultJsonMediaType or getUseHalAsDefaultJsonMediaType. As this should never be the case unless something out of the ordinary is happening, simply returning true would just hide an error that would definitely need awareness.

Copy link

@StefanSchrass StefanSchrass Jun 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

No its not. Line 82 did map, this line throws--effectively bypassing the default.

I totally get the difference here, but my point is that it is actually harmful because even if you dont use HAL at all your application wont start (and thats why I am here)

Copy link
Contributor Author

@mschout mschout Jun 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The throw on line 105 should only happen if HateoasProperties exists (IE the Optional is non-empty), but we couldn't figure out the appropriate method name to call on it. We try the new method name in SB 3.5.0 first, and if we don't find that, we try the one from SB 3.4.x and ealier. @StefanSchrass do you have a small demo that does not start with the patch?

Copy link

Choose a reason for hiding this comment

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

No I haven't a demo. I am just in fearful expectation of experiencing this again. Bear in mind, for 10 days one is unable to use Spring-Boot 3.5.x with your product. To me this means "no security fixes to production the easy way" for 10 days. That is not really acceptable.
And here we are, preparing the same kind of fun again.

Please be aware of that Spring doesn't view the properties (the getters) as part of their API, so they will absolutely change them if they feel like it. This would mean no deplyoments for as long as it takes for you to add another reflection-action-method here.

God speed!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

3.4.x is still actively maintained FYI.

}
}