Add treat default type as error #1654
Conversation
|
Can one of the admins verify this patch? |
|
Hi @csteegz, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution!
TTYL, AZPRBOT; |
| @@ -51,14 +50,13 @@ public Method BuildMethod(HttpMethod httpMethod, string url, string methodName, | |||
| { | |||
| EnsureUniqueMethodName(methodName, methodGroup); | |||
|
|
|||
| var method = New<Method>(new | |||
| { | |||
| var method = New<Method>(new { | |||
There was a problem hiding this comment.
While personally, I'd prefer using the 1TBS, the rest of the project uses the traditional ANSI-style braces, and I'd prefer to keep code changes in a single brace format.
If you could tweak the braces to be consistent that'd be great.
| @@ -96,16 +94,12 @@ public Method BuildMethod(HttpMethod httpMethod, string url, string methodName, | |||
|
|
|||
| var headerTypeName = string.Format(CultureInfo.InvariantCulture, | |||
| "{0}-{1}-Headers", methodGroup, methodName).Trim('-'); | |||
| var headerType = New<CompositeType>(headerTypeName,new | |||
| { | |||
| var headerType = New<CompositeType>(headerTypeName, new { | |||
There was a problem hiding this comment.
Switch to ANSI-style Braces
|
|
||
| var property = New<Property>(new | ||
| { | ||
| responseHeaders.ForEach(h => { |
There was a problem hiding this comment.
Switch to ANSI-style Braces
| @@ -142,8 +136,7 @@ public Method BuildMethod(HttpMethod httpMethod, string url, string methodName, | |||
| private static IEnumerable<SwaggerParameter> DeduplicateParameters(IEnumerable<SwaggerParameter> parameters) | |||
| { | |||
| return parameters | |||
| .Select(s => | |||
| { | |||
| .Select(s => { | |||
There was a problem hiding this comment.
Switch to ANSI-style Braces
| @@ -241,8 +261,7 @@ private Response BuildMethodReturnType(List<Stack<IModelType>> types, IModelType | |||
| } | |||
|
|
|||
| // BuildParameter up type inheritance tree | |||
| types.ForEach(typeStack => | |||
| { | |||
| types.ForEach(typeStack => { | |||
| if (errorModelType == null || method.Responses == null) return; | ||
| var keys = method.Responses.Keys; | ||
| var removeResponses = new List<HttpStatusCode>(); | ||
| foreach (var key in keys) |
There was a problem hiding this comment.
The response types for error conditions should be related (ie, default uses #/definitions/SomeError and all the other error responses should use something that either returns that type, or has an allOf: "#/definitions/SomeError" in it's type declaration.)
That way we can have very specific types for errors, and makes it very clear what's an acceptable error.
Of course that means we have to do three other things:
-
make sure that use of the
allOfdoesn't force type-inheritance, it's possible that we should be able to have two error types that bubble up to the same swagger model type (that doesn't even have any properties), but still generate individual classes without a hierarchy. -
change the generator to throw the correct exception type, rather than living with the default class
-
and add method documentation for known possible exceptions
|
closing due to inactivity (there are issues tracking and referring to this PR) |
Addresses #1523
This takes the opinion that there is one canonical error response, which is defined in the swagger as the default response. To reflect that, all responses that share the error response's model are treated as the default case, and ignored.