-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support mounting an attribute at the top level of a JSON column instead of mounting it to a key _within_ a JSON column container #45
Comments
Yeah, I'm leaning on the default ActiveRecord json serialization to serialize the top-level. I do see the value of this, although I don't need it myself. I'm scared of any increase in code complexity. :) I still wonder if you need attr_json for it. Hmm. Just brainstorming too here... So:
Would work, if SomeActiveModelType.new is something that adheres to the ActiveModel Type interface to succesfully turn something to and from it's database representation. Which, yeah, an AttrJson::Model does not. How does Rails handle ordinary json/jsonb columns? Check out the ActiveRecord::Type::Json. It pretty much does what you already figured out -- just serializes and deserializes to actual strings. We can see that the type used for postgres jsonb doesn't seem to do anything different than that either. (Confirmed with introspection that the parent class of So, yes, when you do AttrJsonModel.to_type, you get an instance of one of our AttrJson::Type::Model objects. Which you can see, yes, converts to/from a hash, not a string representation. Rather than make AttrJson::Type::Model do more than one thing, I think you want a new type. The important methods for complying with the Rails' own ActiveModel Type interface (the one used in rails Due to the good OO design of Rails ActiveModel type, which we follow here, I think you could make a new type something like the following, just delegating to both relevant existing types. You don't actually need AttrJson changes to make this work, so I'd suggest you try it out and if you get it working well and well-tested, perhaps PR it here to share. It might look something like this (I am typing this into this comment, haven't even had MRI parse it) class TopLevelAttrJsonModelType
attr_reader :model_type
def initialize(attr_json_model)
@model_type = attr_json_model.to_type
@wrapper_type = ActiveRecord::Type::Json.new
end
def type
model_type.type
end
def cast(*args)
wrapper_type.cast( model_type.cast(*args) )
end
def serialize(*args)
wrapper_type.serialize( model_type.serialize(*args) )
end
def deserialize
wrapper_type.deserialize( model_type.deserialize(*args) )
end
def changed_in_place?(*args)
model_type.changed_in_place?(*args)
end
end Then you'd just tell rails, using the standard activerecord
The beauty of the newish ActiveRecord/Model Type API, is you can just give it anything that complies with the fairly simple interface. Which is how attr_json does it's stuff too. The above might Just Work. I suggest playing around with that in your app and see what you think. If you can get it well-tested and solid and be sure you understand all the implications, I'd be happy to consider it as a PR. |
I realize my above psuedo-code isn't quite right, the order of def deserialize
model_type.deserialize( wrapper_type.deserialize(*args) )
end Get why? You just gotta play around with it, but I think something basically along these lines would probably work. |
I have no opinion on if this would actually be a good idea in notification-rails or not. I'm not familiar with that gem, but adding attr_json into it would add significant complexity, may not be worth it unless it adds a lot of value. Or it might be! No opinion! |
There is another new gem that provides support for "top-level" serialized-to-json models. https://github.com/DmitryTsepelev/store_model It doesn't do everything attr_json does, and I'm not sure if it would allow "nested" models. The existence of that gem does kind of make me want to add the "top-level" model feature to attr_json to "compete" with it, although I'm not sure I'll have time to get to it anytime soon, but happy to review a PR! |
Hey, I'm coming back to this, not sure if you're still around and interested, @TylerRick ? I think I figured out a way to do this supported by the not-so-well-known ActiveRecord serialization feature, that pretty much already works, with very little additional code. It needs a tiny shim in your local app, but I'm looking to write a tiny bit of code in attr_json to support it in a documented and tested way. If anyone with this use case wants to try it out, that'd be super helpful! |
attr_json
provides good support for rich data types as values inside a JSON object (ruby hash). I think it would be useful to also be able to use a rich data type directly on a JSON attribute itself (at the "top level" instead of just for keys inside it).I tried this:
but not surprisingly, it didn't work (caused
stack level too deep
) because callingsettings
tries to look inside its "container",settings
. But really, there is actually no container; we just want that JSON attribute itself to have a given type.(
would probably be more accurate)
You'll probably tell me, that's not what
attr_json
is for—just use Rails's built-inattribute
API! :)And then I'll say, I tried that, but it doesn't work out of the box currently.
I tried this:
... which got me pretty close, but it ran into this error when it tried to cast:
When I don't add an explicit
attribute :settings
line, it automatically casts between a Ruby Hash and JSON string automatically, so it must implicitly set the attribute type of JSONB columns somehow.I don't know how to introspect into the type of attributes, but I figured out how to get the type of the underlying column:
So I tried adding this to
Model#cast
inattr_json/type/model.rb
:And it works!
... except it still doesn't work for saving...
(Same error if you set it directly to a hash:
)
When I dug into it a bit to see how it was able to save regular
attr_json
fields that use custom type without getting that error, I discovered that even though the custom type serializes to a Hash, the container attribute that it's contained within takes care of serializing those Hashes and everything inside of it to JSON:lib/attr_json/type/container_attribute.rb
:(
AttrJson::Type::ContainerAttribute
's superclass isActiveRecord::Type::Internal::AbstractJson
(I'm on Rails 5.1).)How could we get it to work?
It seems like we just need the
AttrJson::Model
to serialize to a Hash when in a container and to a JSON string when used directly on a JSON attribute. What's the best way to do that?I don't suppose there's an easy way to detect if it's in a container not within the
serialize
method...What about adding another conversion, like
to_json_serialized_type
or something, that you can call instead ofto_json
if you want it to serialize directly to/from JSON instead of to/from a Hash?Why would you want this?
Mostly so you can reuse the same types both directly on an attribute and for attributes stored inside a container attribute without having to duplicate the logic in your own custom type.
A random example...
notifications-rails
The particular use case that led me to this idea was was trying to make a nicer API for accessing settings in a
NotificationSettings::Setting::Setting
model object.Since hashes are such a pain to work with, I was trying to change it to use jsonb, so we could do something like:
instead of
The text was updated successfully, but these errors were encountered: