draftsman icon indicating copy to clipboard operation
draftsman copied to clipboard

Model attributes stored with JSON coder don't deserialize (via reify) correctly

Open npafundi opened this issue 10 years ago • 9 comments

This is a great gem! Even in the experimental state, it works incredibly well.

I found one issue when storing an attribute coded in JSON. When I reify from a draft, the attribute is returned as an encoded String rather than the expected JSON object.

As a brief example, assume I have a model with a settings attribute that looks something like this:

class Widget < ActiveRecord::Base
  store :settings, coder: JSON
end

In the database, the settings column is simply a text type field.

Assume I have a widget object and set its settings attribute to the following JSON object: {"key1":"val1", "key2":"val2"} If I run a draft_update, this will be saved into the drafts table correctly. Publishing also works as expected. However, reifying seems to return this object as a String rather than hash (or JSON object). widget.draft.reify.settings would return the following: "{\"key1\":\"val1\", \"key2\":\"val2\"}"

I haven't yet found a workaround, but I'm still looking. If there's something simple you might recommend, let me know.

Thanks for all the development work you've put into draftsman!

npafundi avatar Jul 25 '14 00:07 npafundi

Thanks for the compliment, @npafundi!

Looks like someone (perhaps I) would need to modify the reify logic to introspect whether or not the attribute has been configured to use a coder like you have. I'm surprised that the way it calls settings= in your case doesn't handle it automatically. (See https://github.com/live-editor/draftsman/blob/master/lib/draftsman/draft.rb#L200)

I must admit that I am not familiar with the store setting that you are using there. I'll have to look into that. I may also look to see if PaperTrail has done anything to address it too.

chrisdpeters avatar Jul 25 '14 02:07 chrisdpeters

Thanks for the fast reply, @chrisdpeters!

I looked into this a little more, and the simplest solution that I could find was to call JSON.parse(widget.settings) after reifying in my controllers (or serializers). Doing this doesn't require changes to draftsman, of course.

This does seem like an odd issue -- I was also surprised that the settings= didn't work when looking at the reify code. I'm not very familiar with what options can be determined with introspection, but it seems difficult to determine what coder is being used on a specific attribute.

It may be possible to check the data type of the model attribute (the actual data type of the column), but that would require that the column's data type be set to json (we're using Postgres, which has a json data type: http://www.postgresql.org/docs/9.3/static/datatype-json.html). However, this is only a partial solution. It's still possible to use store with a JSON coder when the column is another data type (text, varchar, etc.), and those cases wouldn't be covered.

Since I wasn't able to find a way to determine the attribute's coder type, I decided to call JSON.parse on my end, which works (albeit not as elegantly).

All that said, I'm not sure if this is something that's worth fixing. It's probably a small subset of projects using this serialization technique, and there is a relatively simple workaround by parsing the returned string. I'll keep it in the back of my mind, but if you'd like to close the issue that's fine by me.

Again, thanks for putting this gem together!

npafundi avatar Jul 25 '14 18:07 npafundi

I actually started working on adding support for the Postgres json datatype this morning. I'm mostly done with it, and I'd imagine it'll be done in a few days. You'd be more than welcome to help me battle-test it.

Do you think that would solve the issue for your situation?

P. S. I plan on trying this blog post's suggestion for converting my text columns storing JSON to json. I love any post that starts out with, "This is an 'I fucked up so you don’t have to' post." :)

chrisdpeters avatar Jul 25 '14 19:07 chrisdpeters

That's awesome.

It may work, depending on how you plan on handling the loading of those json objects. In my particular case, I believe I'd need the json attributes to not be serialized into a String, or to be parsed back out after reifying.

I'm not quite sure, but I'd love to see what you have and see if it'll fit in. Worst case, I can still run the parse on my end.

I'll probably have to make some adjustments to our code either way. Let me know when you have something, and I'll give it a shot.

Thanks!

npafundi avatar Jul 25 '14 21:07 npafundi

I am about to commit some code that adds PostgreSQL JSON support for the object, object_changes, and previous_draft columns. After some thought about your issue, I don't think this change will help you though. My change applies to the draft data, not your application's unique models.

On a side note, I'm wondering if changing your settings column to JSON would fix the issue. Would Rails then automatically deserialize the JSON string into a hash? If not, I think your workaround is the most sensible approach for the time being. It bugs me that it's not automatic though...

chrisdpeters avatar Jul 28 '14 14:07 chrisdpeters

Hey Chris, sorry for the slow reply. I had the same thought, and earlier today I reworked some of our code and migrated to the Postgres json data type. Good news - it seems to be working great! My testing has been limited, but from what I've seen, it looks like everything is serializing and deserializing correctly.

Because this removes our dependence of serializing to/from JSON using a coder, it looks like it fixed the whole issue. This may not work for MySQL users, but they should be able to use the workaround we discussed.

I'm still looking forward to pulling in your new changes. I haven't fully tested my own changes yet, so if anything related comes up, I'll let you know.

Thanks for the suggestions/solution!

npafundi avatar Jul 29 '14 05:07 npafundi

You're welcome and no problem.

I just cut version 0.3.0 of the gem. I added support for PostgreSQL JSON because I needed to query some of the data within draft#object. Works very nice!

If you need to create a migration to convert object, object_changes, and previous_draft from text to json, again, I recommend using the query in this blog post.

Let me know if you experience any other issues or think the gem's API could be improved.

chrisdpeters avatar Jul 29 '14 11:07 chrisdpeters

I figured out why this wasn't working. Draftsman calls write_attribute in several places, and that doesn't use the default accessor (in your case, settings=): http://www.davidverhasselt.com/set-attributes-in-activerecord/

I'm working on fixing another issue, but I may try to get this fixed in the next release for our friends using databases other than PostgreSQL.

chrisdpeters avatar Aug 11 '14 12:08 chrisdpeters

Good find! I'm sure anyone who can't switch databases would find that useful.

Everything has been working great since we changed to the json datatype in PG. I haven't finished integrating draftsman completely (working on some other tasks), but so far, so good.

npafundi avatar Aug 11 '14 16:08 npafundi