Skip to content

[ApiManagement] [SEMANTIC_VALIDATION_ERROR] Microsoft.ApiManagement/2017-03-01/apimreports.json #1678

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

Closed
vishrutshah opened this issue Sep 13, 2017 · 7 comments
Assignees
Labels
API Management bug This issue requires a change to an existing behavior in the product in order to be resolved. Semantic Validation Service Attention Workflow: This issue is responsible by Azure service team. Service-team

Comments

@vishrutshah
Copy link
Contributor

vishrutshah commented Sep 13, 2017

The existing swagger spec version 2017-03-01 of apimreports.json & apimidentityprovider has issues with Equivalent path already exists

https://travis-ci.org/Azure/azure-rest-api-specs/jobs/275144189#L1567
https://travis-ci.org/Azure/azure-rest-api-specs/jobs/275144189#L1597

@vishrutshah vishrutshah added API Management bug This issue requires a change to an existing behavior in the product in order to be resolved. Semantic Validation Service-team labels Sep 13, 2017
@olydis
Copy link
Contributor

olydis commented Sep 20, 2017

From @solankisamir

We have a set of Apis
/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.ApiManagement/service/{serviceName}/portalsettings/{signupName}
/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.ApiManagement/service/{serviceName}/portalsettings/{delegationName}
Here parameter signupName and delegationName have fixed values https://github.com/Azure/azure-rest-api-specs/pull/1679/files#diff-95497e5e275436cba7612b48ae3f13c0R624 and https://github.com/Azure/azure-rest-api-specs/pull/1679/files#diff-95497e5e275436cba7612b48ae3f13c0R654 respectively.
So, over the wire they would come out as
/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.ApiManagement/service/{serviceName}/portalsettings/signIn
/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.ApiManagement/service/{serviceName}/portalsettings/delegationName
Hence not equivalent path.
The reason we have them separate is due to the fact, that their response payload is different and it cannot be a single Enum sharing the same Contract
How should we got about this?

I think this case makes perfect sense, so the question is where we wanna go or what our recommendation is:
a) "fix the rule" - having multiple identical paths can be a quite intentional thing (as seen above, in BlobStorage I encountered the same). But I get that this was a common error in the past so we should flag it somehow.
b) request a suppression whenever it happens and the usecase is valid
c) recommend moving to x-ms-paths and adding pseudo-query-parameters for disambiguation

/cc @amarzavery @veronicagg

@solankisamir
Copy link
Member

@promoisha for pull request #1679

@veronicagg
Copy link
Contributor

veronicagg commented Sep 21, 2017

@olydis @solankisamir Equivalent paths are not supported by Open Api 2.0 - OAI/OpenAPI-Specification#788
For this case, I see 2 options:

  1. use the enum unique value directly in the path, which would disambiguate it: is there a reason why this is parameterized if it'll have a unique value?
  2. use x-ms-paths, which was created to help with the OpenAPI spec limitation. (https://github.com/Azure/autorest/blob/master/docs/extensions/readme.md#x-ms-paths)

@promoisha
Copy link
Contributor

@veronicagg The reason for parameterization is that the linter emits following warnings if we don't parameterize paths:

"message": "Type values \"signup\" have default value(s), please consider parameterizing them"

If that's OK to have these warnings, we can use enum values directly.

@veronicagg
Copy link
Contributor

@promoisha we've actually removed ParameterizedProperties rule for the next release, so please ignore that warning for the moment (it'll go away) - see #1504 and fd13a53#diff-933629df063d86e3d243a6fa347c0af4

@promoisha
Copy link
Contributor

@veronicagg Thanks for the info. I'll go ahead and use values directly in the URI.

@jhendrixMSFT
Copy link
Member

This has been fixed.

@bsiegel bsiegel added the Service Attention Workflow: This issue is responsible by Azure service team. label Sep 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Management bug This issue requires a change to an existing behavior in the product in order to be resolved. Semantic Validation Service Attention Workflow: This issue is responsible by Azure service team. Service-team
Projects
None yet
Development

No branches or pull requests

7 participants