Skip to content
This repository has been archived by the owner on Apr 26, 2020. It is now read-only.

Updated JsonNetBodyDeserializer.cs to support array types #69

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

logiclrd
Copy link

After encountering errors doing model binding to array types, I investigated the root cause and decided to fix it. It looks like JsonNetBodyDeserializer.cs creates a separate instance of the target type from that created during deserialization, and then manually copies properties over in order to support blacklisting of properties (a Nancy feature not directly integrated with Newtonsoft.Json). As part of this, JsonNetBodyDeserializer.cs assumes that it can use Activator.CreateInstance on the destination type. This does not work for arrays, as they do not have a parameterless constructor. The approach I took, since the deserialization is going to require the use of a List<T> anyway, is to leverage the existing List<T> deserialization, and then if the destination type is actually T[], call List<T>'s ToArray method before returning the final value.

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the Nancy code style guidelines
  • I have provided test coverage for my change (where applicable)

Description

  • To avoid performance issues that might be associated with using late binding to dynamically call ToArray, I generate dynamic method specializations to perform the call to List<T>.ToArray, and I set up a simple cache of delegates referring to these methods.
  • If CreateObjectWithBlacklistExcluded detects an array type T[], a new method ConvertArray is called which retrieves this converter from the cache (constructing it if necessary) and applies it to the result of ConvertCollection on List<T>.
  • The order of operations in CreateObjectWithBlacklistExcluded is altered so that Activator.CreateInstance is not called on the destination type in this circumstance.
  • Unit tests verify the new functionality with arrays of various element types.

…ize into them by generating methods to convert from List<T> to T[] using List<T>'s ToArray method.

Added associated unit tests to JsonNetBodyDeserializerFixture.cs.
@logiclrd
Copy link
Author

So... I did fill in the CLA, I thought @dnfclas was supposed to replace the cla-required label with cla-signed? I went through the "working on behalf of my employer" flow, though, so maybe that requires manual attention...

@NancyFx NancyFx deleted a comment from dnfclas Dec 11, 2017
@MichaelTsengLZ
Copy link

All CLA requirements met now. Check out the statuses. We are currently migrating to new CLA system which uses status instead of labels. :)
image

@logiclrd
Copy link
Author

Ahh, okay :-) Cool beans, then.

@logiclrd
Copy link
Author

Ping!

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

Successfully merging this pull request may close these issues.

4 participants