RESTWS-649 - Correct host property in OpenAPI Spec #302
Conversation
| String resourceName, String searchHandlerId, int queryIndex) { | ||
| List<org.openmrs.module.webservices.docs.swagger.Parameter> parameters = new ArrayList<org.openmrs.module.webservices.docs.swagger.Parameter>(); | ||
| String resourceURL = getResourceUrl(getBaseUrl(), resourceName); | ||
| for (SearchHandlerDoc searchDoc : searchHandlerDocs) { |
There was a problem hiding this comment.
Is removing this block of code related to? "Correct host property in OpenAPI Spec"
| @@ -87,10 +86,27 @@ | |||
|
|
|||
| private Logger log = Logger.getLogger(this.getClass()); | |||
|
|
|||
| public SwaggerSpecificationCreator(String baseUrl) { | |||
| this.baseUrl = baseUrl; | |||
There was a problem hiding this comment.
Is removing the lines below related to? "Correct host property in OpenAPI Spec"
There was a problem hiding this comment.
We no longer use the baseUrl constructor. However the searchHandlerDocs instance variable requires baseUrl param to initiate. searchHandlerDocs is only referenced by getParametersListForSearchHandlers() which is unused. So it makes sense to remove these here.
There was a problem hiding this comment.
I do not refute the fact that we no longer need some code. What am simply saying is that if removing it is not related to "Correct host property in OpenAPI Spec", then it should be in a separate pull request.
|
@gayanW this pull request seems to have more changes that needed for just "Correct host property in OpenAPI Spec". Can you create a separate pull request for other things instead of including them all in this? |
|
Please see now. baseUrl was being used to generate the host, basePath, and scheme. Yet the logic there was invalid. So it needs to be removed. Now in the controller we pass the accurate values. |
|
@gayanW do you mind squashing the commits? |
* Make host and basePath not mandatory in SwaggerSpecificationCreator * Set host and basePath to correct values in SwaggerSpecificationController
|
Done. |
|
|
||
| baseUrl.append(scheme); // http, https | ||
| baseUrl.append("://"); | ||
| baseUrl.append(request.getServerName()); |
There was a problem hiding this comment.
Does this ticket involve removing all these lines of code (from line 30 - 68)?
Are you sure we are not having any regressions here?
| else if (scheme.equals("https")) | ||
| urlWithoutScheme = resourcesUrl.replace("https://", ""); | ||
|
|
||
| SwaggerSpecificationCreator creator = new SwaggerSpecificationCreator(urlWithoutScheme); |
There was a problem hiding this comment.
All this logic does nothing but build the baseUrl (urlWithoutScheme) string and then pass it to the SwaggerSpecificationCreator(..). As noted before generated string was invalid. Thus results in invalid host, and basePath in the spec. No there's no regression.
Issue: https://issues.openmrs.org/browse/RESTWS-649