Various fixes, enhancements to csharp-refactor client
#1711
Conversation
| ConstructorHandling = ConstructorHandling.AllowNonPublicDefaultConstructor, | ||
| ContractResolver = new DefaultContractResolver | ||
| { | ||
| NamingStrategy = new CamelCaseNamingStrategy() |
There was a problem hiding this comment.
We'll want to create an issue to support naming strategy here.
| public static bool IsJsonMime(String mime) | ||
| { | ||
| var jsonRegex = new Regex("(?i)^(application/json|[^;/ \t]+/[^;/ \t]+[+]json)[ \t]*(;.*)?$"); | ||
| return mime != null && (jsonRegex.IsMatch(mime) || mime.Equals("application/json-patch+json")); |
There was a problem hiding this comment.
This could be optimized by returning earlier using String.IsNullOrWhitespace(mime) before unnecessarily constructing the regex object… could also cache the regex object as a static variable to reduce its effect on heap:
public static Regex jsonRegex = new Regex("(?i)^(application/json|[^;/ \t]+/[^;/ \t]+[+]json)[ \t]*(;.*)?$");
public static bool IsJsonMime(String mime)
{
if (String.IsNullOrWhiteSpace(mime)) return false;
return jsonRegex.IsMatch(mime) || mime.Equals("application/json-patch+json");
}
|
|
||
| foreach (var contentType in contentTypes) | ||
| { | ||
| if (IsJsonMime(contentType.ToLower())) |
There was a problem hiding this comment.
I think this might be incorrect… this would return true for APPLICATION/JSON; CHARSET=UTF-8, then apply that as a header when it's not going to be a useful or relevant header. Although HTTP spec defines header names (e.g. 'Content-Type') as case insensitive, the values are case sensitive.
|
@jimschubert thanks for the feedback. As discussed we'll address that in follow-up PRs |
PR checklist
./bin/to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.shand./bin/security/{LANG}-petstore.shif updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in.\bin\windows\.master,. Default:3.4.x,4.0.xmaster.Description of the PR
NOTE: file upload is still broken and will be addressed in another PR instead.
cc @mandrean (2017/08) @jimschubert (2017/09)