-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Changes to enable XML Output Formatters. #896
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on current Web API's xml media type formatter, this could throw in case the serializer doesn't support the given type...as in Web API should there be a check in CanWriteResult?...yeah, Json formatter doesn't do this check and it eagerly says true for all the types...the side effect of this here is that we do not let other formatters which can indeed handle the type...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WriterSettings.Encoding = context.SelectedEncoding;
This isn't thread-safe. Also a bad idea to mutate instance state based on per-request data. I think we'll need to make a copy of the writer settings if the encoding is different.
|
|
|
|
Addressing some PR comments.
|
@rynowak - I've updated the PR Ryan. Please have a look :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put this right below the other formatters in the file. See how it's already divided into sections?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI you can just do this like so:
[HttpPost]
public DummyClass GetDummyClass(int sampleInput)
{
var result = new ObjectResult(new DummyClass { SampleInt = sampleInput });
result.Formatters.Add(new XmlSerializerOutputFormatter());
return result;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this level of fine-grained control is in the sweet spot of object result
|
|
|
Checked in - 2920c55 |
Functional tests for Output Formatters are not a part of this PR. Will follow up with Functional Tests after @harshgMSFT's conneg check-in.