Update SpringDoc json/yaml generator to make the results more deterministic. #7574
Conversation
…nistic. This will make it easier to compare different versions between gn releases. These are the properties that were enabled. springdoc.writer-with-order-by-keys=true springdoc.writer-with-default-pretty-printer=true
There was a problem hiding this comment.
@ianwallen tested and looks working fine, but please check the comments added.
| @@ -47,6 +47,8 @@ | |||
| import org.springframework.web.servlet.mvc.method.RequestMappingInfoHandlerMapping; | |||
|
|
|||
| import javax.servlet.http.HttpServletRequest; | |||
|
|
|||
| import java.nio.charset.StandardCharsets; | |||
There was a problem hiding this comment.
@ianwallen there are 2 unused imports, not related to the pull request, but would be good to clean up:
import org.springframework.core.annotation.AnnotationUtils;
...
import org.springframework.web.bind.annotation.ResponseBody;
| @@ -180,9 +189,14 @@ protected boolean isRestController(Map<String, Object> restControllers, | |||
| || !ModelAndView.class.isAssignableFrom(handlerMethod.getMethod().getReturnType())); | |||
| } | |||
|
|
|||
| protected void calculateServerUrl(HttpServletRequest request, String apiDocsUrl) { | |||
There was a problem hiding this comment.
Can you describe a bit the changes related to calculateServerUrl / getServerBaseUrl
There was a problem hiding this comment.
Then I noticed that calculateServerUrl was not only calculating a url, it was calculating a url and then setting the serverBaseUrl. I would have expected that calculateServerUrl would have returned a value but it was setting a value so I changed it to setServerBaseUrl to make it clearer.
I was also working on some other code and I needed to call the functionality from getServerBaseUrl in a couple places so I separated the function to avoid duplicate code.
I no longer needed to call getServerBaseUrl in multiple places but thought it was still good to keep it separated.
If you want I can merge the code back into a calculateServerUrl function?
| this.openAPIService.setServerBaseUrl(calculatedUrl); | ||
| private String getServerBaseUrl(HttpServletRequest request) { | ||
| String contextPath = request.getContextPath(); | ||
| StringBuffer requestURL = request.getRequestURL(); |
There was a problem hiding this comment.
In the previous code it was used with decode, was not really required?
There was a problem hiding this comment.
The old logic did not make any sense to me.
It was using the requestUrl and then subtracting the apiDocsUrl which has no relationship to the current url.
i.e.
requestUrl was http://localhost:8080/geonetwork/srv/api/doc
apiDocsUrl was based on @value(API_DOCS_URL) which was "/v3/api-docs"
It was subtracting the length of "/v3/api-docs" which just happened to be the same length as "/srv/api/doc" which was just luck that this did not fail?
It looked to me like it was just trying to get the base URL and content path from the URL. So the new logic achieves that correctly.
There was a problem hiding this comment.
@ianwallen thanks for the clarification. Please update the PR to remove the unused imports, it's all fine for me.
…terministic. (#7574) * Update SpringDoc json/yaml generator to make the results more deterministic. This will make it easier to compare different versions between gn releases. These are the properties that were enabled. springdoc.writer-with-order-by-keys=true springdoc.writer-with-default-pretty-printer=true * Cleanup imports
Update SpringDoc json/yaml generator to make the results more deterministic.
This will make it easier to compare different versions between gn releases.
These are the properties that were enabled.
springdoc.writer-with-order-by-keys=true
springdoc.writer-with-default-pretty-printer=true
Checklist
mainbranch, backports managed with labelREADME.mdfilespom.xmldependency management. Update build documentation with intended library use and library tutorials or documentation