ActiveModel::Serializer::type now accepts a block

[groyoh]

A block passed to the type method will be instance_eval'd to the serializer.
This allows us to avoid overriding the #_type method to have a dynamic type.

[bf4]

The structure of this code change makes me a bit uncomfortable. Since the change is that type is a value or a callable, I'd think that either the value is also made callable in the setter, or that it is consumed with a respond_to?(:call) and no more-- I'm surprised to see so many changes both here and in the tests. I'm going to take a closer look when I have some time. Would you mind sharing your design thoughts?

[groyoh]

Concerning your first point, I based my change on the link method and how it is used in the adapter (https://github.com/groyoh/active_model_serializers/blob/dynamic_type/lib/active_model/serializer/adapter/json_api.rb#L214). I'd be happy to change it so that the setter creates a callable out of the value if you consider that's a win.
Concerning the changes I made in this function they were mainly DRYing up the code, but I'll revert it to keep only the necessary code (and maybe create another PR for the unnecessary changes).
Concerning the changes in the specs, I mainly refactored the code to use a helper method to avoid repeating the code everywhere in the test file and also added specs to test that using the method in a subclass doesn't break it in the superclass.

[groyoh]

Simply moved this method to be private.

[bf4]

I think I'd rather see in implementation more like in #1370 and #1383 and note https://github.com/rails-api/active_model_serializers/pull/1370/files#r48116422 hasn't yet be resolved.

cc @beauby

Does that make sense? There's a lot going on in this method because there's uncertainty about how to handle the value being passed way down.

[beauby]

Could you describe a use-case where the type of a resource should be dynamically resolved? I'm guessing some polymorphic associations but I'm not sure exactly.

[groyoh]

@beauby indeed, I was thinking of polymorphic classes where the attributes and associations are the same and where the client should handle the resource differently depending of the type.

[beauby]

@groyoh Oh right, you mean polymorphic classes and not polymorphic associations. Could you sketch a real-life example to help me understand your use-case properly?

[groyoh]

@beauby I'll give you a simple example that does not necessarily involve polymorphism.

If for example you want to define a special inflection by defining a type block in a base class:

class BaseSerializer < ActiveModel::Serializer
  type do
    object.class.name.camelize
  end
  # Right now you'd have to do it like so
  # def _type
  #   object.class.name.camelize
  # end
end

class SomeSerializer < BaseSerializer
end 

This will allow you to avoid defining a static type within every serializer.
It you want a more complex example with polymorphism, let me know and I'll give you one.

[beauby]

I see. The use-case you're mentioning (globally setting the format of the type) should be taken care of by some pending PRs (ref missing). Polymorphic associations should work out of the box as the correct serializier would be inferred. The only possible case left would be something like:

class AbstractObject
  ...
end

class ConcreteObject1 < AbstractObject
  ...
end

class ConcreteObject2 < AbstractObject
  ...
end

class AbstractObjectSerializer < ActiveModel::Serializer
  ...
end

where the same serializer would be used to serialize different kinds of objects. What I'm wondering is whether this situation wouldn't be better addressed by using polymorphic associations instead of polymorphism at the object level. So I'd really like to see your use-case.

[groyoh]

Let's say you have a legacy system (that you can't change) used to track a statistics of a fair over the time. You want to track the number of incoming visitors, outgoing visitors, number of tweets, etc. and sample these values as a samples attribute. For some statistics this value may be a simple json and for some other may be encoded as a string to reduce data size so the clients needs to know the type to handle them properly. Furthermore, all the statistics for a fair have the same database id as the fair but stored with a type column (both combined are the primary key). You may have an index request that looks like:

{
  "data": [
    {
      "id": "1234",
      "type": "incoming_vistors",
      "attributes": {
        "samples": "a certain encoded string"
      }
    },
    {
      "id": "1234",
      "type": "outgoing_visitors",
      "attributes": {
        "samples": [
          {
            "timestamp": 1231241241,
            "value": 56
          }
        ]
      }
    }
  ]
}

To solve this, I would use a single model and serializer that take the type out of the database entry. Thus the type would have to be dynamically set at the serializer level.

Another way to solve this issue would be to combined the id and the type of the statistic within the JSONAPI id and use a static type (e.g. { "id": "1234-outgoing_visitor", "type": "fair_statistic" } ), but I'd prefer avoid that as it had a bit more complexity and is a bit more fuzzy.

I hope my example was clear enough.

[beauby]

@groyoh For the sake of clarity, would you mind sketching the db schema for your fair_statistics?

[groyoh]

Table: fair_statistics

column	type
id	string
type	string
created_at	timestamp
updated_at	timestamp
samples	blob

Model:

class FairStatistic < ActiveRecord::Base
end

Serializer:

class FairStatisticSerializer < ActiveModel::Serializer
  type do
    object.type
  end

  attributes :created_at, :updated_at, :samples
end

[beauby]

So a direct workaround for your situation would be to have

attribute :type, key: :statistic_type

so that your client would have full information (under the statistic_type attribute). I understand that in your case it would make things slightly cleaner to have it directly in the JSON API type but I believe this is quite an edge case. I'm not opposed to a dynamic type though, I'm just trying to ponder complexity vs usefulness.

[groyoh]

@beauby thanks for the proposal but the issue that I stated within #1399 (comment) is that every statistic have the same id as the fair (for legacy reason) but the primary key is the combination of id and type. So if I do as you stated, I might have multiple statistics with the same jsonapi type and id e.g.:

{
  "data": [
    {
      "id": "1234",
      "type": "fair_statistic",
      "attributes": {
        "statistic_type": "incoming_vistors",
        "samples": "a certain encoded string"
      }
    },
    {
      "id": "1234",
      "type": "fair_statistic",
      "attributes": {
        "statistic_type": "outgoing_vistors",
        "samples": [
          {
            "timestamp": 1231241241,
            "value": 56
          }
        ]
      }
    }
  ]
}

Sorry for not being clear enough once again.

[bf4]

Something in your scenario doesn't make sense. Two records can share the same type and id?

On Jan 1, 2016, at 12:00 AM, Yohan Robert notifications@github.com wrote:

@beauby thanks for the proposal but the issue that I stated within #1399 (comment) is that every statistic have the same id as the fair (for legacy reason) but the primary key is the combination of id and type. So if I do as you stated, I might have multiple statistics with the same jsonapi type and id e.g.:

{
"data": [
{
"id": "1234",
"type": "fair_statistic",
"attributes": {
"statistic_type": "incoming_vistors",
"samples": "a certain encoded string"
}
},
{
"id": "1234",
"type": "fair_statistic",
"attributes": {
"statistic_type": "outgoing_vistors",
"samples": [
{
"timestamp": 1231241241,
"value": 56
}
]
}
}
]
}
Sorry for not being clear enough once again.

—
Reply to this email directly or view it on GitHub.

[bf4]

So type is an attribute, right?

B mobile phone

On Dec 31, 2015, at 3:29 AM, Yohan Robert notifications@github.com wrote:

Table: fair_statistics

column type
id string
type string
created_at timestamp
updated_at timestamp
samples blob
Model:

class FairStatistic < ActiveRecord::Base
end
Serializer:

class FairStatisticSerializer < ActiveModel::Serializer
type do
object.type
end

attributes :created_at, :updated_at, :samples
end
—
Reply to this email directly or view it on GitHub.

[groyoh]

Two records can share the same id. Two record can share the same type. But two records cannot have the same id and the same type. The statistic id is some kind of "foreign key" which reference a fair's id (and could have been called fair_id but cannot be migrated at this point) meaning every statistics for the same fair have the same id. The pair type/id is actually the primary key.

This is why the only two solution I see is:

• use the db id as jsonapi id and the db type as jsonapi type (e.g. { "id": "1234", "type": "outgoing_visitors" })
• use a combination of the db id and type as jsonapi id and "fair_statistics" as jsonapi type (e.g. { "id": "1234-outgoing_visitors", "type": "fair_statistics" })

[beauby]

@groyoh So basically your fair_statistics table has no primary key, right?

[groyoh]

@beauby the primary key is a composed primary key using the columns id and type.

[bf4]

Is this sti? Sounds like you're overloading the word 'type' to mean both the record's type and one of its attributes.

B mobile phone

On Jan 2, 2016, at 9:38 AM, Yohan Robert notifications@github.com wrote:

@beauby the primary key is a composed primary key using the columns id and type.

—
Reply to this email directly or view it on GitHub.

[groyoh]

@bf4 I wouldn't say it is sti as there is only one table and one model. AFAIK, sti would imply to have one model per 'type'.

[bf4]

Can you maybe describe your record/model without using the word type? Anf how it might represent two Distinct API resources?

B mobile phone

On Jan 3, 2016, at 2:14 PM, Yohan Robert notifications@github.com wrote:

@bf4 I wouldn't say it is sti as there is only one table and one model. AFAIK, sti would imply to have one model per 'type'.

—
Reply to this email directly or view it on GitHub.

[groyoh]

@bf4

Can you maybe describe your record/model without using the word type?

I'll remove the use of type and replace it by category.

CREATE TABLE fair_statistics
(
  fair_id       INTEGER,
  category      VARCHAR(255),
  created_at    TIMESTAMP,
  updated_at    TIMESTAMP,
  samples       BLOB,
  CONSTRAINT pk_fair_statistics PRIMARY KEY (fair_id, category)
)

The fair_id column references a fair's id. That means that we might have two different statistics for the same fair and they would have the same fair_id but their primary key would be different:

INSERT INTO fair_statistics VALUES (1, "ingoing_visitors", now(), now(), some_binary_data);
INSERT INTO fair_statistics VALUES (1, "outgoing_visitors", now(), now(), some_binary_data);

Within the JSONAPI spec, the id and type pair can be considered as a primary key since the combination of the two has to be unique API-wide. So we define our database column category to be the JSONAPI type and our database column fair_id to be the JSONAPI id:

class FairStatistic < ActiveRecord::Base
  # some logic for the `samples` attribute
end
class FairStatisticSerializer < ActiveModel::Serializer
  attributes :created_at, :updated_at, :samples
  type do
    object.category
  end

  def id
    object.fair_id
  end
end

And how it might represent two distinct API resources?

{
  "data": [
    {
      "id": "1",
      "type": "outgoing_visitors",
      "attributes": {
        "created_at": "2016-01-14T16:15:09+00:00",
        "updated_at": "2016-01-14T16:15:09+00:00",
        "samples": "some encoded string"
      }
    },
    {
      "id": "1",
      "type": "ingoing_visitors",
      "attributes": {
        "created_at": "2016-01-14T16:15:09+00:00",
        "updated_at": "2016-01-14T16:15:09+00:00",
        "samples": "some encoded string"
      }
    }
  ]
}

The workaround to avoid using a dynamic type would be to concatenate the fair_id and the category within the JSONAPI id:

class FairStatistic < ActiveRecord::Base
  # some logic for the `samples` attribute
end
class FairStatisticSerializer < ActiveModel::Serializer
  attributes :created_at, :updated_at, :samples

#  type "fair_statistics"

  def id
    "#{object.fair_id}-#{object.category}"
  end
end

{
  "data": [
    {
      "id": "1-outgoing_visitors",
      "type": "fair_statistics",
      "attributes": {
        "created_at": "2016-01-14T16:15:09+00:00",
        "updated_at": "2016-01-14T16:15:09+00:00",
        "samples": "some encoded string"
      }
    },
    {
      "id": "1-ingoing_visitors",
      "type": "fair_statistics",
      "attributes": {
        "created_at": "2016-01-14T16:15:09+00:00",
        "updated_at": "2016-01-14T16:15:09+00:00",
        "samples": "some encoded string"
      }
    }
  ]
}

The latter is rather ugly and not convenient as it implies to define a specific concatenation/splitting to the id.

[bf4]

@groyoh gotcha. So, the issues are:

1. FairStatistics is a model and table with a compound primary key (fair_id, category), where fair_id is a foreign_key to fairs.id

2. Thus, FairStatistics behaves somewhat like STI, but there are no OutgoingVisitors, IncomingVisitors models

3. JSON API wants us to represent FairStatistics by id and type

   http://jsonapi.org/format/#document-resource-object-identification
   Every resource object MUST contain an id member and a type member. The values of the id and type members MUST be strings.

   Within a given API, each resource object's type and id pair MUST identify a single, unique resource. (The set of URIs controlled by a server, or multiple servers acting as one, constitute an API.)

   The type member is used to describe resource objects that share common attributes and relationships.

   Within a given API, each resource object's "type" and "id" pair MUST identify a single, unique resource. (The set of URIs controlled by a server, or multiple servers acting as one, constitute an API.)

4. It is unclear how FairStatistics maps to JSON API resource identifiers.

   • Is the resource a fair statistic with ids composed of the compound primary key fields? Or
   • Is the resource of type category and id fair_id?
   • What's the url to reference one of these resources?

So, I think part of the problem here is that we want to refer to a FairStatistic by its category, not by its model name. When you're talking about it, how do you refer to it? Do you call it an IncomingVisitorStatistic?
This might be a good question to post to https://github.com/json-api/json-api/issues

[beauby]

👍 Nice recap @bf4

[bf4]

possibly related to #1420

[groyoh]

@bf4 thanks for the recap. Indeed it would be referred as incoming_visitor_statistic. I will check the jsonapi issues and discuss, then ask them if I can't find anything.

[groyoh]

Closing this for now. I'll reopen with more info if feel there is a real need for it 😉