joukevandermaas / saule

JSON API library for ASP.Net Web API 2.
https://joukevandermaas.github.io/saule
MIT License
76 stars 37 forks source link

Serialise complex objects correctly #149

Closed laurence79 closed 7 years ago

laurence79 commented 7 years ago

This is essentially a reimplementation of the ResourceSerializer that copes with a couple of edge cases that have tripped me up in my own use:

Take a look at ResourceGraphSerializerTests to see tests that pass with the new serialiser. The new serialiser also passes all the existing tests (with a couple of exceptions where I have overridden the existing test with comments as to why).

The new serialiser approach differs from the original by first building a graph of objects to serialise using the IncludingContext, and then generates the response JSON from that.

This is anecdotal but I've also seen modest performance improvements from this approach.

By default the original serialiser is still used unless a new configuration option is set config.ConfigureJsonApi(new JsonApiConfiguration { UseGraphSerializer = true });

I appreciate this is a large change, but hopefully it will be a benefit to the project 😊

joukevandermaas commented 7 years ago

This sounds great, I'll try to make some time to review this soon. Thanks so much!

laurence79 commented 7 years ago

Thanks @joukevandermaas

laurence79 commented 7 years ago

Fixes #141 (when using the graph serialiser)

bjornharrtell commented 7 years ago

Seems like nice work @laurence79, looking forward to testing it. I wonder if it also resolves https://github.com/joukevandermaas/saule/issues/99?

laurence79 commented 7 years ago

Thanks @bjornharrtell, yes it seems like it might because only the attributes and relationships declared in the resource are read from the target objects.

laurence79 commented 7 years ago

AppVeyor had a timeout on package restore. I couldn't see a way to easily retry the build, so I bumped the Newtonsoft.Json version to 10.0.2, hope thats ok 😊

laurence79 commented 7 years ago

Ok @joukevandermaas, I think this is ready to be reviewed again 😊

joukevandermaas commented 7 years ago

@laurence79 Thanks a lot for this! I published a new pre-release with these changes in it.