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 basePath validation and assign servers URL's context as basePath [In Progress] #1954

Closed
wants to merge 12 commits into from

Conversation

VirajSalaka
Copy link
Contributor

@VirajSalaka VirajSalaka commented Apr 22, 2021

Purpose

$subject

Issues

Fixes #1958
Fixes #1952
Fixes #1959

Automation tests

  • Unit tests added: Yes
  • Integration tests added: Yes

Tested environments

Not Tested


Maintainers: Check before merge

  • Assigned 'Type' label
  • Assigned the project
  • Validated respective github issues
  • Assigned milestone to the github issue(s)

if override && apiKey == GenerateIdentifierForAPI(apiContent.VHost, mgwSwagger.GetTitle(), mgwSwagger.GetVersion()) {
continue
}
if swaggerEntry.GetXWso2Basepath() == mgwSwagger.GetXWso2Basepath() && apiContent.VHost == vhost {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

use an additional map

// See the License for the specific language governing permissions and
// limitations under the License.
//

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add License header

@@ -294,6 +334,7 @@ func UpdateAPI(apiContent config.APIContent) {
if svcdiscovery.IsServiceDiscoveryEnabled {
startConsulServiceDiscovery() //consul service discovery starting point
}
return
Copy link
Contributor Author

Choose a reason for hiding this comment

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

useless return, remove

@@ -461,7 +462,8 @@ func createRoute(params *routeCreateParams) *routev3.Route {
Value: b.Bytes(),
}

if xWso2Basepath != "" {
// if xWso2Basepath is not different compared to endpointBasepath, no need to substitute.
if xWso2Basepath != "" && xWso2Basepath != endpointBasepath {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

remove the condition check to follow one path all the time.

err := swagger.validateBasePath()
if err != nil {
logger.LoggerOasparser.Errorf("Error while parsing the API %s:%s - %v", swagger.title, swagger.version, err)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

return error

func (swagger *MgwSwagger) validateBasePath() error {
if xWso2BasePath == "" {
return errors.New("Empty Basepath is provided. use x-wso2-basePath extension, basePath (if OpenAPI v2 definition is used)" +
" servers entry (if OpenAPI v3 definition is used) with non empty context")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

change the error message

@@ -106,7 +106,8 @@ public void handleSuccessRequest(RequestContext requestContext) {
FilterUtils.getTenantDomainFromRequestURL(
requestContext.getMatchedAPI().getAPIConfig().getBasePath()) == null
? APIConstants.SUPER_TENANT_DOMAIN_NAME
: requestContext.getMatchedAPI().getAPIConfig().getBasePath());
: FilterUtils.getTenantDomainFromRequestURL(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

store the function's return value and reuse it

@VirajSalaka
Copy link
Contributor Author

closed as the fixes are provided with #1970

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