Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serialization fails when both Input and Output formatters are present #143

Closed
aerotog opened this issue Mar 16, 2018 · 5 comments
Closed

Comments

@aerotog
Copy link

aerotog commented Mar 16, 2018

After testing a local build of the latest master, I've found that including both of the JSON formatters is causing responses to be empty. I believe this is related to the following code in the "ResourceConverter" class:

            var saveContext = serializer.Context;
            resource.ConverterContext = GetResourceConverterContext();
            serializer.Converters.Remove(this);
            serializer.Serialize(writer, resource);
            serializer.Converters.Add(this);
            serializer.Context = saveContext;

Both of the JSON formatters are adding their own instances of the following converters:

            SerializerSettings.Converters.Add(_linksConverter);
            SerializerSettings.Converters.Add(_resourceConverter);
            SerializerSettings.Converters.Add(_embeddedResourceConverter);

It doesn't seem the serialization logic is able to handle multiple instances of the same converter. This results in all responses being empty.

I have a naive solution running locally that fixes this. I may open a PR with the fix but would like confirmation that this is actually a bug.

@panmanphil
Copy link
Collaborator

Hoping to spend time on this this weekend, but what Accepts header are you sending in your requests? The example should content negotiate, selecting the correct outputter based on your accepts header (as I recall it)

@aerotog
Copy link
Author

aerotog commented Mar 17, 2018

I have tested both with and without the "Accept application/hal+json" header.

With only the Output formatter, I get valid response with or without the header.

With both Input and Output formatters, I get an empty response with or without the header.

As I mentioned in the Example Project issue, the fact that I'm getting valid responses without the the header might also be a bug but it's much less of a priority in my opinion.

@panmanphil
Copy link
Collaborator

@aerotog Could you review what i did against what you thought would fix this? Basically i gave the input formatter it's own JSON serializer settings.

@aerotog
Copy link
Author

aerotog commented Mar 18, 2018

@panmanphil looks like that works. I've added a comment to the PR to remove the TODO.

I don't fully understand how the formatters are used. Can you explain how the two formatters sharing the same instance of Json Serializer Settings causes a conflict?

@panmanphil
Copy link
Collaborator

@aerotog The formatters essentially register themselves with the data incoming or outgoing and take action when that condition hit. By having the input formatters replace the output formatters registration because of the shared serializer instance between them, the action used on output had nothing to say about the data from an object graph an output formatter would see. So it does nothing and the result a blank output. I didn't walk all through that logic, what you had already found made it trivial to find.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants