ChurchSuite / churchsuite-api

API documentation for ChurchSuite https://churchsuite.com
45 stars 11 forks source link

Location{} or Location [] #11

Closed DavidStrickland0 closed 4 years ago

DavidStrickland0 commented 4 years ago

Per this documentation Location in SmallGroups is a singular instance. However upon calling the endpoint I find that an empty or unset location returns an empty array. IE instead of a blank location returning as "location":"" or "location":null

as expected it returns as

"location":[]

DavidStrickland0 commented 4 years ago

Looks Like Images has the same issue. Per the Documentation "images" is an instance but if left unset an Empty array is returned "images":{"thumb":{"px":128,"square":true,...... if unset becomes "images":[]

Basically saying if its not set its an empty array but when set its a singular instance.

If it matters or helps the code I'm working on is here: https://github.com/DavidStrickland0/ChurchSuiteSharp at the moment I've got the serialization set to JsonIgnore in the code so that the unit tests pass.

jeremyjcpaul commented 4 years ago

Hi David,

The "location" and "images" is returned as a json encoded array and so if the array is empty you should expect to get [] since that's a valid json encoded empty array.

Hope that helps, thanks, Jeremy

DavidStrickland0 commented 4 years ago

The problem is they arent ALWAYS json arrays. Sometimes the API is encoding them as Simple objects. If its an array then it should always be an array and thus always have [ ].

If there is only 1 location the api is returning "location":{ "address":"NG9 1PA", "latitude":"52.9211461693166", "longitude":"-1.20475036320128" }, when it should be "location":[{ "address":"NG9 1PA", "latitude":"52.9211461693166", "longitude":"-1.20475036320128" }],

DavidStrickland0 commented 4 years ago

Looks like ChurchSuite isn't the only Api with the issue. Not certain if this helps but thought it worth posting just in case https://stackoverflow.com/questions/13575280/jersey-json-array-with-1-element-is-serialized-as-object

jeremyjcpaul commented 4 years ago

Hi David,

I'm not saying that they are json arrays, I'm saying that they are json encoded arrays. In PHP this can be shown: echo json_encode(['foo' => 'bar']); // outputs: {"foo":"bar"} echo json_encode([]); // outputs: [] This is how most languages json encode arrays; it will return either a json object or an empty array. Your suggestion of "location":[{ ... }] is an array of json objects.

Thanks, Jeremy

DavidStrickland0 commented 4 years ago

No worries I can work around it. But that's just the point it has to be worked around.

Ambiguity between null, [], object and array just make my life as a consumer of the api more difficult and ultimately make the Api less user friendly and approachable. When something you assume would be either object or null instead turns out to be object, array or [] with no way to anticipate it its just frustrating and makes one wonder what other properties are suddenly going to blow up at some later date.

Granted I understand it makes it easier as a publisher to just json_encode it and let the consumer sort it out.

Anyway thanks for looking at it.

edwardsph commented 4 years ago

@DavidStrickland0 - I am with you on this issue. The return type should be consistent. If a single object is expected then an API should return an object or null. An empty array is a different type of value and should only be returned when a field is expected to return multiple items. Yes the API user can work around it but I would argue it is better for the API developer to ensure consistent responses to make the API easier to use.

jeremyjcpaul commented 4 years ago

Please see https://stackoverflow.com/questions/9619852/what-is-the-convention-in-json-for-empty-vs-null/9623521. An empty collection should be represented by an empty array. Null should be used to say that the value hasn't been set.

mattss commented 4 years ago

An empty collection should be represented by an empty array. Null should be used to say that the value hasn't been set.

@jeremyjcpaul Going back to the original question, isn't the point here that the ChurchSuite API isn't actually doing this. It is returning an empty array when the small groups location has no value, instead of 'null' as recommended.

lukesalter commented 4 years ago

Hi Gents,

Thanks for your input - I agree to some extent that null might be better than an empty array. In some languages an empty array implies that you could, and should test for, receiving multiple instances of something (locations, images, sandwiches, unicorns etc.). For now though the ChurchSuite API will continue to return an empty array.

The ChurchSuite API is first and foremost the means by which the ChurchSuite applications talk to one another. They're mostly PHP applications so a simple 'empty()' check works fine for us. For those of you using other languages I'm afraid it isn't priority for us to adhere to stricter standards (sorry!).

We'll bare this in mind and consider doing so for future updates to the API.

DavidStrickland0 commented 4 years ago

I appreciate that the API is a second class citizen and really only an offshoot of the internal work that is php centric. I do appreciate that its been released dispite this better something then having to do convuluted web scrapes to accomplish it.

However is there any chance of getting documentation that states what properties might be arrays and return '[]' and what aren't.

Is Location and Images the only places where I might run into this or are there other properties that might turn from an array into a object or from a object into a array?

lukesalter commented 4 years ago

@DavidStrickland0 You can safely assume that anything which would be returned as an object will always be [] when empty and everything else would be string or null. There are a couple of booleans in there but otherwise it's pretty consistent.

We're confident that the documentation clearly outlines which elements of the response will be objects and which won't be.

DavidStrickland0 commented 4 years ago

Thanks that clears it up.

In Case any other .Net users stumble on this I'm using

resultString = resultString.Replace("[]", "null");

just before I

JsonConvert.DeserializeObject<List<SmallGroup>>(resultString );

and it seems to have resolved the newtonsoft deserialization issues I was having.