jhoerr / box-csharp-sdk-v2

A C# client for the Box API (v2).
http://developers.box.com/docs/
11 stars 15 forks source link

Folder Serialization #42

Closed ericrtodd closed 11 years ago

ericrtodd commented 11 years ago

When attempting to serialize a Folder object into JSON, an exception is thrown because the child folder objects don't contain a populated ItemCollection.

jhoerr commented 11 years ago

Hi Eric, can you post some code that reproduces the issue?

ericrtodd commented 11 years ago
var manager = new BoxApi.V2.BoxManager(_accessToken);
var folder = manager.GetFolder("0");
var json = new RestSharp.Serializers.JsonSerializer().Serialize(folder);

The problem appears to lie in Folder.FromEntriesGetAll because child folders lack any items.

(Edited for brevity by jhoerr on 4/18)

jhoerr commented 11 years ago

I have a couple thoughts on this -- bear with me. Folder.Folders and Folder.Files are valid enumerations, as far as I can tell. That said, both the RestSharp and Json.Net serializers choke on them, and at first glance I don't understand why that is. That said, there's no reason for those properties to be serialized in the first place. Unfortunately, I've found no way to get the RestSharp serializer to ignore a property. If I'm overlooking something there, please let me know.

Json.Net, on the other hand, supports ignoring properties by using the [JsonIgnore] attribute. One possible resolution here is that we [JsonIgnore] the troublesome attributes, and you start using the Json.Net serialzer instead of RestSharp's. Does that seem reasonable?

(It's worth noting that this SDK does not use RestSharp's serializer. It uses a custom Json.Net serializer, BoxApi.V2.Serialization.AttributableJsonSerializer, to work around differences in naming conventions between .Net and the API.)

ericrtodd commented 11 years ago

I think that the problem is twofold. First, the reason for the exception is the following:

private IEnumerable FromEntriesGetAll(ResourceType value) { // Child folders never have the ItemCollection populated; hence it is null. Because the JSON serialization // doesn't ignore the Files/Folders properties, this line of code is executed resulting in an underlying // NullReferenceException. I was able to suppress the exception by using the following: // return ItemCollection != null ? // ItemCollection.Entries.Where(i => i.Type.Equals(value)).Cast() : new T[0]; return ItemCollection.Entries.Where(i => i.Type.Equals(value)).Cast(); }

This prevents the exception from occurring during serialization or just trying to iterate Folders through a recursive method.

Second, even if the properties aren't throwing exceptions during serialization, empty arrays will be placed into the JSON for properties that are only accessors for other backed properties (redundant; slightly deceptive). JsonIgnore functionality would likely handle this scenario as you have described.

I have been developing a RESTful web service layer (using this project as Box access) to transfer Box data back and forth to the client. My first inclination (for at least this phase) was to send the original JSON back to the client via serializing the Folder/File objects. First glitch was the exception, but after resolving the exception I found that the resultant JSON didn't particularly match the JSON being sent from Box directly. After some consideration, I feel that it is best for me to create my own layer of Folder/File information in order to abstract the backend a little more (had a meeting yesterday that introduced the idea of using swappable document stores for the backend).

So , for my project, the serialization structure ceases to be an issue (and I may have been going off the design reservation to start with). The underlying problem of iterating the folders/files likely still needs resolution, however. A simple foreach statement will blow up if the ItemsCollection is either not initialized or the Files/Folders properties are allowed to access the uninitialized ItemsCollection.

jhoerr commented 11 years ago

Child folders never have the ItemCollection populated; hence it is null.

This makes total sense; thanks. I was looking for the problem in the wrong place. I'd be happy to accept that fix if you'd be inclined to open a pull request.

...after resolving the exception I found that the resultant JSON didn't particularly match the JSON being sent from Box directly

Is this the case when using RestSharp's native serializer, or the BoxApi.V2.Serialization.AttributableJsonSerializer? The later is able to massage property names and enum values into the Box API's specified format, and it's what the SDK uses to serialize data that's sent to Box. (Which isn't to say that it's perfect, but it's reasonably well tested and baked.)

ericrtodd commented 11 years ago

Sent pull request.