Exception on Delete

[StokedPrune]

Hi, I'm guessing this is me doing something wrong but I cant see what. I get an InvalidOperationException every time I delete a row, even though it deletes successfully.

Example code:

var soql = new SoqlQuery();

soql.Select("uniqueid,readingdate"); //must be LOWER CASE
soql.Where("readingdate < '" + cutOffTime + "'");

List<SodaObject> deleteRecords = _client.Query<SodaObject>(soql, "vmb6-se9c") as List<SodaObject> ?? new List<SodaObject>();

foreach (var item in deleteRecords)
{
    try
    {
        var result = _client.DeleteRow(item.UniqueId, "vmb6-se9c");
    }
    catch(Exception e)
    {
        Console.WriteLine(e);
    }
}

Exception:

System.InvalidOperationException: Couldn't deserialize the (JSON) response into an instance of type SODA.SodaResult. ---> Newtonsoft.Json.JsonSerializationException: Cannot deserialize the current JSON array (e.g. [1,2,3]) into type 'SODA.SodaResult' because the type requires a JSON object (e.g. {"name":"value"}) to deserialize correctly.
To fix this error either change the JSON to a JSON object (e.g. {"name":"value"}) or change the deserialized type to an array or a type that implements a collection interface (e.g. ICollection, IList) like List<T> that can be deserialized from a JSON array. JsonArrayAttribute can also be added to the type to force it to deserialize from a JSON array.
Path '', line 1, position 1.
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.EnsureArrayContract(JsonReader reader, Type objectType, JsonContract contract)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateList(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, Object existingValue, String id)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.Deserialize(JsonReader reader, Type objectType, Boolean checkAdditionalContent)
   at Newtonsoft.Json.JsonSerializer.DeserializeInternal(JsonReader reader, Type objectType)
   at Newtonsoft.Json.JsonConvert.DeserializeObject(String value, Type type, JsonSerializerSettings settings)
   at Newtonsoft.Json.JsonConvert.DeserializeObject[T](String value, JsonSerializerSettings settings)
   at Newtonsoft.Json.JsonConvert.DeserializeObject[T](String value)
   at SODA.SodaRequest.ParseResponse[TResult]()
   --- End of inner exception stack trace ---
   at SODA.SodaRequest.ParseResponse[TResult]()
   at SODA.SodaClient.DeleteRow(String rowId, String resourceId)
   at ConsoleApp2.MySingleton.<TrimRows>d__6.MoveNext() in C:\Users\Mick\source\repos\ConsoleApp2\Singleton.cs:line 45

Similar to this?? #5

Any ideas?
Thanks

[thekaveman]

Thank you @sp00n5 for the report! It looks like a problem with the library incorrectly parsing the DELETE response, not something you are doing wrong.

Based on the exception it appears there are two problems: 1) the raw JSON coming back is an array, where the delete operation is expecting an object and 2) the deserialization target object (SodaResult) doesn't have the appropriate attributes (similar to #5 as you mention).

If you are able, can you post the raw JSON response that comes back after your delete? If you place a breakpoint on the _client.DeleteRow row line and debug into SodaRequest.ParseResult you should be able to see the raw response. The docs are somewhat mysterious in what we should be expecting.

[StokedPrune]

Thanks Kegan but I cant seem to find the raw response. where should it be?

[thekaveman]

I just realized something... assuming you are using the Nuget package, the .pdb files aren't available and you won't be able to step into the SODA.NET source in a debug session.

If you've downloaded this repo and have a direct project reference in your solution, here's what you can do:

1. Place a breakpoint on the line just before the catch, the line that causes the exception soClient.DeleteRow(...)

2. Step into the soClient.DeleteRow call (press F11 after the breakpoint is hit)

3. From there step into SodaRequest.ParseResponse (this is on the last line of the DeleteRow method).

4. I think the raw JSON should be available on line 113 in SodaRequest: https://github.com/CityofSantaMonica/SODA.NET/blob/master/SODA/SodaRequest.cs#L113

[StokedPrune]

Thanks, Ive done that now.
Result content is [{"typ":"delete","id":"99974074-b1fd-4067-9a73-c8001fee3459"}]
See image:

[thekaveman]

Thanks @sp00n5! This confirms that it is a problem with how SodaClient.DeleteRow interprets the response:

1. DeleteRow currently expects a single object, but the response is an array of objects
2. The deserialization target class (SodaResult) must have properties typ (I thought this may have been a typo in the docs, but thanks for verifying!) and id.

Are you comfortable submitting a patch to fix this?

[StokedPrune]

Sure, I will give it a go... On Friday, 10 April 2020, 9:57:25 am AEST, Kegan Maher <notifications@github.com> wrote: Thanks @sp00n5! This confirms that it is a problem with how SodaClient.DeleteRow interprets the response: - The response is an array of objects, not a single object - The deserialization target must have properties typ (thought this may have been a typo in the docs, but thanks for verifying!) and id. Are you comfortable submitting a patch to fix this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[StokedPrune]

I have modified the SodaClient.DeleteRow to accept an Array (SodaResult[]) and added the required members to the SodaResult Class. Works fine now when the delete is successful, however, when the delete fails it returns an object, not an array eg:

{"message":"Row with identifier a06355c0-d528-4d8d-8a3c-c11f24a42d52 was not found","errorCode":"soda.row.not-found","data":{"value":"a06355c0-d528-4d8d-8a3c-c11f24a42d52"}}

So now we get the same exception as original but now on failure instead of success..
I'm not sure how best to deal with that???

[thekaveman]

Thanks for working on this, all I can say is, wow... how annoying that there are so many different return types depending on the use case/operation. This seems to be a problem with DELETE being the only "direct row manipulation" operation for which an array is returned (opposed to POSTing for create/update on a single row returning a single result object).

It also seems this error message doesn't conform to the docs which state that the member should be code and not errorCode.

At this point I can think of one ugly workaround, to make DeleteRow return a dynamic result - I really don't like this option. I'm going to reach out to Socrata support and see if they can weigh in on these inconsistencies. At least we'll have a final resort option if they aren't willing or able to clean this up on their end.

[thekaveman]

After reviewing Socrata's own Java-based tool "DataSync" for data publishing, there is another option that we can pursue which may be cleaner, if not a bit more complex.

Rather than issuing a DELETE directly against the row, we could issue a POST following the upsert functionality by crafting the a specialized payload like:

{
    "<row_id_field>": "<input_id>",
    ":deleted": true
}

The only issue here is the exact field name for <row_id_field> can be defined on a per-dataset basis, and so we'd first need to query the dataset's metadata to get the name of the identifier field.

Then just pass this JSON string into a call to Upsert and return the result from there.

This seems to be exactly what Socrata's DataSync does.

[StokedPrune]

I thought about trying the 'dynamic' approach also but it felt a bit like a work around rather than a solution. I'm not sure why the deleterow would need to return an array on success, it can only ever be a single row operation as far as I understand? Maybe they can review that?
Your above suggestion about deleting via an upsert, I dont fully understand it yet but isnt that also a workaround? Bypassing the delete operation to do a delete. However that could be good because it would allow a 'bulk' delete (??) rather than the very slow row by row approach..

[thekaveman]

I'm not sure why the deleterow would need to return an array on success, it can only ever be a single row operation as far as I understand?

I definitely agree, it seems very strange and inconsistent.

...deleting via an upsert... isnt that also a workaround? Bypassing the delete operation to do a delete. However that could be good because it would allow a 'bulk' delete (??)

Socrata's Upsert functionality supports create, update, and delete for one or more rows at once (with properly crafted payloads). So this isn't so much a a workaround - we would simply be using Upsert to mark a single row for deletion rather than direct row delete functionality. For a user of this library, it should be entirely transparent.

Since their desktop tool makes use of the Upsert functionality vs. direct row delete, it seems to be inline with established practice. They have another Java library (on which this library was based) where the direct row delete method returns void, thus avoiding (ahem) this problem altogether!

[StokedPrune]

Is the main goal here not to break anyone's existing code? I think your idea will work. I will give it a try.
Just wondering about the DeleteRow Method, it only accepts a string as the RowId. Since the Id column is user definable, what if the user has a number Id column? Should that be catered for? Or is it taken care of somewhere that I am not seeing?
Thanks
Mick

[thekaveman]

Yes, ensuring that existing code works is certainly a goal. I think since we already only accept a string for the rowId it shouldn't be a problem, but that is an area that should be tested before rolling this out.

In the worst case, we can interrogate the Column metadata to find out the type of the Id column, and cast as needed before sending the request.