search

facebook-csharp-sdk / simple-json

JSON library for .NET 2.0+/SL4+/WP7/WindowsStore with optional support for dynamic and DataContract
MIT License
379 stars 143 forks source link

Bug deserializing to list when single object returned #63

Closed haagenson closed 10 years ago

haagenson commented 10 years ago

There is a bug I found using Generic Lists when the object to be deserialized is not part of a collection and is rather an object. This includes two commits. The first commit is the failing unit test. The second commit is the code to fix the bug.

haagenson commented 10 years ago

@prabirshrestha can you take a look at the proposed changes? I'm looking to incorporate these changes into the octokit repository as well and was hoping to get your opinion.

Thank you!

prabirshrestha commented 10 years ago

@haagenson Your root is not a json array. So trying to deserialize to a ItemList which is List<Item> should fail and it does fail. Not understanding this PR. Care to elaborate?

//ccing @shiftkey since octokit is referenced.

haagenson commented 10 years ago

@prabirshrestha I don't have control over the API that is delivering the object. The API returns List if there is more than one or simply Object for a single return. This implementation allows for stating that it should be deserialized as a List, even if a single object is returned.

My Test method is poorly named, it should be named something more along the lines of Can_Deserialize_Single_Root_Json_Object_To_Inherited_List.

prabirshrestha commented 10 years ago

@haagenson This doesn't actually make sense to include it in SimpleJson. Out of curiosity can you list the apis.

@haacked @shiftkey is there some parameters/headers you can send to always make it treat as lists?

I would love to see if any other vendors implements their api this way.

@haagenson Can you try this scenario by implementing your own IJsonSerializerStrategy. In fact octokit.net is using its own strategy - GitHubSerializerStrategy. Might be a patch there is a better solution instead. https://github.com/octokit/octokit.net/blob/9dacef686b0cfeaebea1248afd9d44029c6fe4c4/Octokit/Http/SimpleJsonSerializer.cs#L22

haagenson commented 10 years ago

@prabirshrestha I'm performing a GET API Call for references. The API uses the same signature for a single reference or a list of references.

I can definitely implement it in the GitHubSerializerStrategy instead. I think at this point I probably need more input from the GitHub guys on the direction they would like to go with this.

haacked commented 10 years ago

Yeah, definitely a GitHub issue.

haagenson commented 10 years ago

Closing this for now due to discussion on the Github repository.

Thanks so much for the review and comments @prabirshrestha

  • © Githubissues.
  • Githubissues is a development platform for aggregating issues.