{"action":"created","issue":{"url":"https://api.github.com/repos/Azure/azure-rest-api-specs/issues/790","repository_url":"https://api.github.com/repos/Azure/azure-rest-api-specs","labels_url":"https://api.github.com/repos/Azure/azure-rest-api-specs/issues/790/labels{/name}","comments_url":"https://api.github.com/repos/Azure/azure-rest-api-specs/issues/790/comments","events_url":"https://api.github.com/repos/Azure/azure-rest-api-specs/issues/790/events","html_url":"https://github.com/Azure/azure-rest-api-specs/issues/790","id":194494062,"number":790,"title":"Service responses don't always contain 'required' properties indicated in their schema definition","user":{"login":"veronicagg","id":17714137,"avatar_url":"https://avatars.githubusercontent.com/u/17714137?v=3","gravatar_id":"","url":"https://api.github.com/users/veronicagg","html_url":"https://github.com/veronicagg","followers_url":"https://api.github.com/users/veronicagg/followers","following_url":"https://api.github.com/users/veronicagg/following{/other_user}","gists_url":"https://api.github.com/users/veronicagg/gists{/gist_id}","starred_url":"https://api.github.com/users/veronicagg/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/veronicagg/subscriptions","organizations_url":"https://api.github.com/users/veronicagg/orgs","repos_url":"https://api.github.com/users/veronicagg/repos","events_url":"https://api.github.com/users/veronicagg/events{/privacy}","received_events_url":"https://api.github.com/users/veronicagg/received_events","type":"User","site_admin":false},"labels":[{"id":374571230,"url":"https://api.github.com/repos/Azure/azure-rest-api-specs/labels/discussion","name":"discussion","color":"5319e7","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2016-12-09T01:53:04Z","updated_at":"2016-12-09T19:11:03Z","closed_at":null,"body":"When running swagger validation tools on the existing specs, one class of error found is that responses from service are missing required properties. Examples: https://github.com/Azure/azure-rest-api-specs/issues/755, https://github.com/Azure/azure-rest-api-specs/issues/772\r\n\r\nThe cause of this issue is models shared for request and response, with required properties for the request, but those not coming back in the response. \r\n\r\nSDKs generated from AutoRest are generally not exposing this as a problem, because they are not validating responses against models. The consumer obtains the response even if it doesn't match the expected model. It's up to the consumer to use the response properly afterwards.\r\n\r\nAnswers to the following questions can help choose an solution for an accurate swagger representation:\r\n\t1. Are properties marked as required really required?\r\n\t2. Is GET->PUT pipeline scenario expected to work for the consumer of the SDK?\r\n\r\nIf the service does not send the 'required' properties in the response, then piping scenario would be broken even if the models (or model types) are the same, as sending the response of a 'GET' in a 'PUT' request, would result in an error indicating that's missing required properties. \r\n\r\nA common case for these 'required' properties are passwords/access keys, which are required in the request but won't be returned to the user in the response. In this case, the user would need to provide the password/access key for the PUT call to work\r\n\r\nOptions to resolve the above in the swagger spec:\r\n\r\nA. If the properties are accurately required for the request, but not expected to come back in the response (the GET->PUT piping scenario is not expected to work as is), then the model definition for the result could be broken up into 2 models: model 'A' would contain the shared properties, and model 'B' being an \"allOf\" of A. This way model 'B' expands on model 'A' with the additional required properties.\r\n\tGenerated code would represent these as different models, so the consumer of the SDK would 'GET' model A, and need to construct model B with the additional properties for a 'PUT'. \r\n     \r\nB. If the properties are not required under certain circumstances (depending on other properties for example) then the model for request and response would be the same and those properties should not be indicated as required in the spec.\r\n\r\n@amarzavery , @johanste, does this properly capture what we discussed yesterday? Feel free to comment. \r\n@lmazuel , @begoldsm, we'd really appreciate your feedback on this.\r\n@salameer"},"comment":{"url":"https://api.github.com/repos/Azure/azure-rest-api-specs/issues/comments/266096151","html_url":"https://github.com/Azure/azure-rest-api-specs/issues/790#issuecomment-266096151","issue_url":"https://api.github.com/repos/Azure/azure-rest-api-specs/issues/790","id":266096151,"user":{"login":"begoldsm","id":8781927,"avatar_url":"https://avatars.githubusercontent.com/u/8781927?v=3","gravatar_id":"","url":"https://api.github.com/users/begoldsm","html_url":"https://github.com/begoldsm","followers_url":"https://api.github.com/users/begoldsm/followers","following_url":"https://api.github.com/users/begoldsm/following{/other_user}","gists_url":"https://api.github.com/users/begoldsm/gists{/gist_id}","starred_url":"https://api.github.com/users/begoldsm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/begoldsm/subscriptions","organizations_url":"https://api.github.com/users/begoldsm/orgs","repos_url":"https://api.github.com/users/begoldsm/repos","events_url":"https://api.github.com/users/begoldsm/events{/privacy}","received_events_url":"https://api.github.com/users/begoldsm/received_events","type":"User","site_admin":false},"created_at":"2016-12-09T19:11:03Z","updated_at":"2016-12-09T19:11:03Z","body":"@veronicagg thank you so much for putting this summary together. From our side, I think A will work for the vast majority of cases. There is one other scenario where we \"re-use\" objects with required properties, and that is in LIST calls (get all the resources in a resource group or subscription). In this case, by design, the properties of those resources are omitted by the service to reduce the payload size. \r\nWe would not want to require another different object in this case that the user needs to know about/manage, but perhaps this would still fit under option A?\r\n\r\nFor example: An 'account' object in a PUT requires location, properties.varA, properties.VarB, properties.Password.\r\nA specific GET for that account returns an object with everything **except** properties.Password\r\nA GET on the resource group containing the account only returns the top level object (name, location, in this case).\r\n\r\nWould this mean that we need three difference object model definitions:\r\n1. baseAccount (contains all common properties, no required ones)\r\n2. specificGetAccount (allOf baseAccount + properties.varA, properties.VarB)\r\n3. putAccount (allOf specificGetAccount + properties.Password)\r\n\r\nThis seems a bit heavy handed, but it is the most explicit modelling. That being said, I am open to suggestions, but bear in mind that any of these changes will result in a breaking change for all existing SDKs and specs."}}
{"actor":{"display_login":"begoldsm"}}