Custom_fields simultaneous save with json becomes an array

bug
1

Hi there, I think I hit what might be a bug in core, but I’m not sure what the desired functionality is.

I noticed this from working on the discourse-push-notifications plugin where the custom_field is being set as an array sometimes. I suggested a fix to merge the arrays and it was suggested that it could be a bug in core’s custom_fields.

I just investigated and reproduced this behavior in a test. From what I can tell if two separate threads are saving simultaneously, it combines the data into an array as two separate values.

Is this the desired behavior? How should custom_fields behave when being saved like this?

4 Likes
3

Fixed in

https://github.com/discourse/discourse/commit/4bb454d8892ee3bf5461f8a8fcaddc379278d968

@awole20 Thank you for digging into this and for the test case :+1:

4 Likes
4

I’ve ran into this in the past with polls as well where a json custom field would be mysteriously converted into an array. Hmm I’m not sure how many people are affected but we might need to run migrations to fix fields that have incorrectly been converted to arrays.

4 Likes
5

I have spent some time wrestling with JSON custom fields in the past. One thing I would add/raise here, given this topic is still semi-live (and I just spent 20 mins dealing with a similar issue).

You have to be very careful to ensure that all values you save to custom fields cast as JSON are in fact valid JSON objects. Every time you save any custom field, all custom fields associated with the relevant model instance are reloaded and re-cast, so any invalid JSON will cause an exception (here). This trips me up all the time.

For example, if you have an array of JSON objects, as opposed to an actual JSON object, you can’t cast the field as type :json, even though you may think it would parse as JSON in ruby.

Say you have an array of hashes you want to save to a single custom field. The initial save will actually work if you first convert the array into a JSON string:

category.custom_fields['election_list'] = JSON.generate([{"topic_id"=>"24","topic_url"=>"/t/moderator-election/24","status"=>2,"position"=>"moderator","banner"=>false}])
category.save_custom_fields(true)

You will end up with this string saved in category_custom_fields and no errors will be thrown:

[{"topic_id":"24","topic_url":"/t/moderator-election/24","status":2,"position":"moderator","banner":false}]

However whenever the class instance associated with this custom field tries to save any new custom field, our example string is loaded and converted to a string containing a hash (and no longer wrapped in an array):

'{"topic_id"=>"24","topic_url"=>"/t/moderator-election/24","status"=>2,"position"=>"moderator","banner"=>false}'

Then, when the operation comes to refresh_custom_fields_from_db where all the custom_fields associated with the model instance are re-cast for a second time, an exception is thrown when this string containing a hash is given to ::JSON.parse

This is probably the intended behaviour and just represents me coming to grips with ruby & JSON.

Currently, if I want to save an array of objects in a single custom field, I typically just use the String type and use ::JSON.parse and ::JSON.generate myself.

4 Likes