Consider having a SerializableResource and DeserializableDocument

[bf4]

Whereas

• Having SerializableResource.serialize return an instance of SerializableResource is not intuitive.
• It makes more sense for SerializableResource.new to return a serializable_resource
• A SerializableResource takes a Ruby model and presents it with a serialization interface serializable_hash, as_json, to_json that is usually consumed by the Renderer in Rails
• The upcoming deserialization feature takes a JSON document and returns a Ruby model
• The code for SerializableResource.serialize and SerializableResource.deserialize would get complicated quite quickly as they're really different responsibilities

Therefore

• It may be more intuitive to have a SerializableResource that takes a resource and a DeserializableDocument that takes a JSON document. It might even make sense for the DeserializableDocument to be a pure function since there may not need to be an intermediate state, as there is with the SerializableResource.

Also, we might start a convention of calling subclass of an ActiveModel::Serializer an e.g. PostSerialization

Relevant PRS

• Outside Controller Documentation Use #1083
• WIP - Deserializer implementation #950
  • WIP - Deserializer implementation #950 (comment)
  • WIP - Deserializer implementation #950 (comment) )
  • joaomdmoura@5bc262f ? Joao renamed PostSerializer to PostSerialization
• Encapsulate serialization in ActiveModel::SerializableResource #954
  • Encapsulate serialization in ActiveModel::SerializableResource #954 (comment)
  • Encapsulate serialization in ActiveModel::SerializableResource #954 (comment)
  • Encapsulate serialization in ActiveModel::SerializableResource #954 (comment)
• Add a #to_json method to the serializer base #889 (comment)

Some (lightly amended) discussion of what SerializableResource does (from the slack team)

> Hmm why does AM::SerializableResource#serialize return an AM::SerializableResource? Is it not a bit confusing?

bf4 [7:43 PM]
b/c it's `AM::SerializableResource::serialize`, a class method returning an instance :simple_smile:

> @bf4: Yes but is 'serialize' the right name for that?
I mean, in which sense does it actually serialize the resource?

bf4 [8:03 PM] 
ok, so then you can call adapter methods on it to get the serialization you want

> I would expect a method called serialize to provide a serialized resource (i.e. some json string in our case)
I get that, I'm not saying this method is useless, I'm just questioning whether it should be called 'serialize' or not

bf4 [8:03 PM] 
can you think of a better name for what it does?
we discussed naming *a lot* in that PR, #954

> I'll read the discussion then

bf4 [8:04 PM] 
I expect that class to evolve as we use it, honestly
its top-level goal is just to encapsulate all the logic joining the serializers (naming!) and the adapter

> If somehow AMS provided also, say, xml serialization, then I would get the rationale behind having X.serialize.as_json and X.serialize.as_xml

bf4 [8:05 PM] 
for various reasons, including being able to serialize outside the controller...
it does provide xml, just sorta unintentional, but there

> but I was looking at the PR that adds doc about using AMS outside controllers, and the whole thing seemed weird to me

bf4 [8:06 PM] 
it's part of ActiveModel::Serialization
well, it'll make more sense when you want to test a serializer
and also when you're tired of explaining the complicated dance you'd have to do outside of the controller to serialize something :simple_smile:

> Oh yeah, using AMS outside controllers totally makes sense to me
I guess I haven't really wrapped my head about what the SerializableResource class does

bf4 [8:07 PM] 
One of my open prs is basically to right a serializer test so no one else needs to reinvent the wheel
it'll also significantly simplify the tests
95% of AMS::SR is the encapsulate the logic that was in the controller
(code-wise)

> I think I'd expect the Serializer to use a SerializableResource under the hood, but being able to do PostSerializer.serialize(post), and it would spit the json (as a convenience method for testing/using outside a controller)
In a perfect world, I'd love to be able to do `json_post = PostSerializer.serialize(post)` and `post = Post.new(PostSerializer.deserialize(params_or_json_hash))`

bf4 [8:16 PM]
So, SerializableResource is a pretty good name, because it takes in a resource, and makes it serializable.  i.e. as_json, to_json, serializable_hash, the ActiveModel::Serialization interfaced
But it's also a bad name, because it returns the response document
It might be a better name for what we call 'Serializer' today
The Serialize is a pretty bad name these days, because it's really a ResourceSerializer.. it just transforms a resource into a serializable object

> I agree with that. What I'm questioning is 1. whether this class should be exposed to users, since they are already familiar with Serializers, and 2. whether the current #serialize() method should be named what it is

bf4 [8:18 PM] 
1. should it be exposed? if not it, something that plays its current job a million times yes
2. whether `::serialize` is a nice name (not `#serialize`)... up for grabs

> 1. agreed. My point was that since users already defined Serializers for their resources, it might make sense that they would just call methods on those, rather than use another class

bf4 [8:19 PM] 
Yes, but why should a Serializer know how to generate a response?

> I think a serializer should know how to serialize a resource, and the way I understand it in AMS, that means building a json string

bf4 [8:23 PM] 
anyhow, the way I see it, the library has three jobs
1. given an object, present it so that it can be serialized differently from itself
2. each resource in the object (1 .. *)  is presented so that it can be serialized differently from itself
3. take the object and the options and generate a response document
@beauby: but that's what it does! But only with the FlattenJson adapter!
# ```render json: object, { # some options }
|_>
   for resource in object
     present as serializable
  |_>
    for serializable resources
      present as serializable response object
 #```
or an example
 #```render json: [Foo.new(bar: 'bar'), Bar.new(foo: 'foo')], adapter: json_api, meta: { amazing: true }
|_>
  FooSerializer.new(foo).serializable_hash #=> { foo: { bar: 'bar' } }
  BarSerializer.new(bar).serializable_hash # => { bar: { foo: 'foo' } }
  |_>
    JsonApi.new( ^ ).serializable_hash
   # => 
   { data: [ { foo: { bar: 'bar' } }, { bar: { foo: 'foo'} }], meta: { amazing: true }
# ```
see how the two need a controller to orchestrate the process?
also, see how its just is to prepare it for the renderer which calls `as_json` or `to_json` or whatever
( did you read my blog or the posting to the rails-core list? http://www.benjaminfleischer.com/2015/06/02/understanding-rails-model-serializers/ )
How Rails serialzers work now (Rails 4.2), and a proposal for how they could be better
I totally agree that words to describe this are made harder by the names of the classes in AMS. I wish it were in a different namespace

> @bf4 So a "serializable response object" is a json string here, right?

bf4 [9:03 PM] 
nope
Without AMS, this is the flow before the renderer
# ```render json: [Foo.new(bar: 'bar'), Bar.new(foo: 'foo')]
# ```
in both cases, the renderer just calls to_json what it gets (or as_json or whatever if you modify it) 

> right, right
yeah so AMS just builds a hash that will be serialized the right way
(here by "serialize" i mean transforming a hash into a json string)

bf4 [9:07 PM] 
nope
AMS does this (in a really abstract way)
# ```render json: object
ams = AMS.new(object)
# ```so the Renderer doesn't get `object`, it gets `ams` (edited)
does the last thing I wrote make more sense?
in a really formal sense, the Renderer is actually what serializes the object into a json string

> I see
but then, what does AMS expose to the renderer? a SerializableResource?

bf4 [9:12 PM] 
an instance of the adapter :simple_smile:
(which is what SR delegates to)

[bf4]

cc @beauby @joaomdmoura @kurko

[beauby]

My quick 2cts here:

• SerializableResource::serialize() is, indeed, confusing for the reasons stated above
• SerializableResource is, at least to me, a bit confusing as well, since a) the name implies that usual resources (ActiveModels) are not, in fact, serializable, and b) there is a history of AMS using actions as class names (mainly Serializer)
• In any case, I agree that deserialize() falls outside the scope of SerializableResource
• As stated, the interface that would make most sense to me currently (but I'm happy to be convinced otherwise) is
  • serializable_hash = PostSerializer.serialize(post) (and serializable_hash.to_json is the actual JSON string)
  • post = Post.new(PostSerializer.deserialize(params_or_hash)), i.e. PostSerializer::deserialize() takes a hash or an instance of ActionController::Parameters and spits out a hash that is ActiveRecord-friendly
• I agree that moving most of the serialization logic into a different class was a good move, but I am not sure that class should be exposed to the user

[bf4]

@beauby As much as I agree with you that serializable_hash = PostSerializer.serialize(post) (and serializable_hash.to_json) would be nice, given that the the thing we call ActiveModel::Serializer, which all resource serializers inherit from, 1) currently only serializes a single resource and 2) would need to add the responsibilities of both Adapters and collection serializers, I think it's just unfortunate that the naming will be imperfect. That is, an ActiveModel::Serializer would need to know not only how to serialize, say, a Post, but also how to generate a JsonApi resource object for that post. I can see this working maybe if we split adapters in two: Resource adapter and Document/Response adapter. That way the current code that SerializableResource encapsulates (which is the song and dance between serializers and an adapter in the action controller)

summarized:

serializer = ActiveModel::Serializer.serializer_for(resource)
serializer_instance = serializer.new(resource, serializer_opts)
adapter_instance = ActiveModel::Serializer.adapter.new(serializer_instance, adapter_opts)
adapter_instance.serializable_hash(options)

could be

serializer = ActiveModel::Serializer.serializer_for(resource)
serializer_instance = serializer.new(resource, serializer_opts)
resource_serialization = ActiveModel::Serializer.resource_adapter.new(serializer_instance.serializable_hash)
adapter_instance = ActiveModel::Serializer.response_adapter.new(resource_serialization, adapter_opts)
adapter_instance.serializable_hash(options)

Which since wouldn't really allow for ActiveModel::Serializer.serializer_for(post).new(post, adapter: :json).serializable_hash since you'd need to duplicate the logic that checks if post is a collection of posts and even other things.... and also know whether serializable_hash is for the post or for the response. e.g. { posts: { id: 1 } } vs. { data: [ { posts: [{ id: 1}, {id: 2}], authors: [1, 2] ] }, links: { self: "https://example.com/posts?include=authors" } }

Again, the serialization of Post as { id: 1} is a different responsibility from render post, include: :authors, adapter: :json_api, the former being just the PostSerializer, the latter being the PostSerializer, AuthorSerializer, and Adapter::JsonApi. I learned about these naming issues when trying to test my serializers. (Which is part of I wanted to address in #967 and #996 )

[bf4]

related: Rails PR 21496: Add configurable serializers in each renderer

[joaomdmoura]

Hey everyone showing up late here:

• Having SerializableResource.serialize return an instance of SerializableResource is not intuitive It makes more sense for SerializableResource.new to return a serializable_resource

👍 totally agree .new should be enough

• A SerializableResource takes a Ruby model and presents it with a serialization interface serializable_hash, as_json, to_json that is usually consumed by the Renderer in Rails

I really like how the serializer worked solo, but having SerializableResource to tie up Serializers and Adapters is a good thing for ppl trying to build get the json without going through render

• The upcoming deserialization feature takes a JSON document and returns a Ruby model. The code for SerializableResource.serialize and SerializableResource.deserialize would get complicated quite quickly as they're really different responsibilities

Agree, we will need to split this logic, not sure if DeserializableDocument is the best name but we need to have another place to handle this. (I know I still need to split deserialization PR).

• Split adapters in two: Resource adapter and Document/Response adapter.

👎 would be nice to keep adapter as simple as possible, if we fell the need to split it we might be putting more responsibilities in it than we should.

So abou the PR itself:

Consider having a SerializableResource and DeserializableDocument
Yup, we will need a different class to deal with deserialization, and I'd keep SerializableResource around, how do you feel about it?

[bf4]

re removing SerializableResource.serialize in #1129

[remear]

@rails-api/ams Has this been amply considered? Are we still interested in doing this?

[bf4]

@remear This has kind of been forgotten. I still think it's a goo idea, but...