{"action":"created","issue":{"url":"https://api.github.com/repos/zalando-incubator/zally/issues/359","repository_url":"https://api.github.com/repos/zalando-incubator/zally","labels_url":"https://api.github.com/repos/zalando-incubator/zally/issues/359/labels{/name}","comments_url":"https://api.github.com/repos/zalando-incubator/zally/issues/359/comments","events_url":"https://api.github.com/repos/zalando-incubator/zally/issues/359/events","html_url":"https://github.com/zalando-incubator/zally/issues/359","id":232868887,"number":359,"title":"False-Positive result when checking for Unused Parameters","user":{"login":"maxim-tschumak","id":4802353,"avatar_url":"https://avatars2.githubusercontent.com/u/4802353?v=4","gravatar_id":"","url":"https://api.github.com/users/maxim-tschumak","html_url":"https://github.com/maxim-tschumak","followers_url":"https://api.github.com/users/maxim-tschumak/followers","following_url":"https://api.github.com/users/maxim-tschumak/following{/other_user}","gists_url":"https://api.github.com/users/maxim-tschumak/gists{/gist_id}","starred_url":"https://api.github.com/users/maxim-tschumak/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/maxim-tschumak/subscriptions","organizations_url":"https://api.github.com/users/maxim-tschumak/orgs","repos_url":"https://api.github.com/users/maxim-tschumak/repos","events_url":"https://api.github.com/users/maxim-tschumak/events{/privacy}","received_events_url":"https://api.github.com/users/maxim-tschumak/received_events","type":"User","site_admin":false},"labels":[{"id":503435882,"url":"https://api.github.com/repos/zalando-incubator/zally/labels/bug","name":"bug","color":"ee0701","default":true},{"id":508313415,"url":"https://api.github.com/repos/zalando-incubator/zally/labels/Server","name":"Server","color":"c2e0c6","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2017-06-01T12:52:11Z","updated_at":"2017-07-24T13:14:35Z","closed_at":null,"body":"**Current Behavior**\r\nFalse-Positive Violation when having two parameters with different parameter id (`definition.parameters.${parameter_id}`), but the same parameter name (`definition.parameters.${parameter_id}.name.${parameter_name}`)\r\n\r\nThe behavior is incorrect since the usage of the parameter can be verified only by the usage of the link to the parameter. And this is done via `parameter_id`.\r\n\r\n**How to Reproduce**\r\nPart of the API definition to reproduce the behavior:\r\n```\r\nparameters:\r\n  identifier_type:\r\n    name: identifier_type\r\n    description: Specifies the type of the customer identifier.\r\n    in: path\r\n    type: string\r\n    x-extensible-enum:\r\n      - customer_hash\r\n      - cookie_id\r\n      - fullvisitorid\r\n      - client_id\r\n      - idfa\r\n    required: true\r\n\r\n  target_identifier_type:\r\n    name: identifier_type\r\n    description: Specifies what type of customer identifiers to be returned\r\n    in: query\r\n    type: string\r\n    x-extensible-enum:\r\n      - customer_hash\r\n      - cookie_id\r\n      - fullvisitorid\r\n      - client_id\r\n      - idfa\r\n    required: false\r\n\r\npaths:\r\n  /identifier-types/{identifier_type}/source-ids/{source_identifier}:\r\n    get:\r\n      tags:\r\n        - id-mappings\r\n      description: List of identifiers associated with the source id.\r\n      parameters:\r\n        - $ref: '#/parameters/identifier_type'\r\n        - $ref: '#/parameters/source_identifier'\r\n        - $ref: '#/parameters/target_identifier_type'\r\n      responses:\r\n        200:\r\n          description: The identifiers associated with the source id.\r\n          schema:\r\n            $ref: '#/definitions/IdMappingResults'\r\n        401:\r\n          description: User is not authenticated\r\n        403:\r\n          description: User is not authorized\r\n        404:\r\n          description: Identifier is not found\r\n          schema:\r\n            $ref: '#definitions/Problem'\r\n      security:\r\n        - oauth2:\r\n          - 'cross-device-graph-service.read'\r\n```\r\n\r\n**Expected Behavior**\r\nIdentifying of the unused parameters should be done based on the `parameter_id` instead of `parameter_name`."},"comment":{"url":"https://api.github.com/repos/zalando-incubator/zally/issues/comments/317418629","html_url":"https://github.com/zalando-incubator/zally/issues/359#issuecomment-317418629","issue_url":"https://api.github.com/repos/zalando-incubator/zally/issues/359","id":317418629,"user":{"login":"innokenty","id":4004229,"avatar_url":"https://avatars0.githubusercontent.com/u/4004229?v=4","gravatar_id":"","url":"https://api.github.com/users/innokenty","html_url":"https://github.com/innokenty","followers_url":"https://api.github.com/users/innokenty/followers","following_url":"https://api.github.com/users/innokenty/following{/other_user}","gists_url":"https://api.github.com/users/innokenty/gists{/gist_id}","starred_url":"https://api.github.com/users/innokenty/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/innokenty/subscriptions","organizations_url":"https://api.github.com/users/innokenty/orgs","repos_url":"https://api.github.com/users/innokenty/repos","events_url":"https://api.github.com/users/innokenty/events{/privacy}","received_events_url":"https://api.github.com/users/innokenty/received_events","type":"User","site_admin":false},"created_at":"2017-07-24T13:14:35Z","updated_at":"2017-07-24T13:14:35Z","body":"I don't think the specified case is a valid swagger doc. `parameter.name` is the essential field that defines which parameter is which.\r\n\r\n### Proof №1: check the docs\r\nOpen this link and scroll a tiny bit up, because the header covers the title: https://swagger.io/specification/#fixed-fields-45\r\nThat's the spec for the parameters, and it says the `name` is required and only it identifies the parameter's name. Not the tagname above.\r\n![screen shot 2017-07-24 at 15 13 40](https://user-images.githubusercontent.com/4004229/28524926-c0902a8a-7082-11e7-92a1-5dde7e630dfe.png)\r\n\r\n\r\n### Proof №2: try the presented specification\r\nGo here: http://editor.swagger.io and paste the api definition, presented in this issue above (simplified):\r\n```yaml\r\nswagger: '2.0'\r\ninfo:\r\n  title: Product Service\r\n  version: 1.0.0\r\n\r\npaths:\r\n  '/identifier-types/{identifier_type}/source-ids':\r\n    get:\r\n      parameters:\r\n        - $ref: '#/parameters/identifier_type'\r\n        - $ref: '#/parameters/target_identifier_type'\r\n      responses:\r\n        '200':\r\n          description: The identifiers associated with the source id.\r\nparameters:\r\n  identifier_type:\r\n    name: identifier_type\r\n    description: Specifies the type of the customer identifier.\r\n    in: path\r\n    type: string\r\n    required: true\r\n  target_identifier_type:\r\n    name: identifier_type\r\n    description: Specifies what type of customer identifiers to be returned\r\n    in: query\r\n    type: string\r\n    required: false\r\n```\r\n\r\nIn the panel to the right, that shows the generated UI you can see that there is only one parameter: `target_identifier_type` is omitted. But if you change it to `name: target_identifier_type` - it appears in the UI as it should.\r\n\r\n### Proof №3: Java ignores it\r\nIn tests we do `SwaggerParser().read(\"api_definition.json\")`\r\nWhen I insert the presented example definition above into file, I can see in debug mode that the get operation node of the parsed swagger doc contains only 2 parameters, not 3. I think it matches them by names and therefore overrides the conflicting one.\r\n\r\n## Conclusion\r\nAny thoughts? I think it's not a valid spec and the issue should be closed."}}
{"actor":{"display_login":"innokenty"}}