Her JSON API support

[hubert]

Hi All. There's been great activity with issues and PRs lately. thank you.

One of the things, I'm keen to see with Her moving forward is first class support for JSON API. It's got great momentum behind it and addresses a lot of the issues we deal with building APIs.

I propose we introduce a module such that the way you specify your model consumes and produces JSON API is a one-liner.

class Contributor
  include Her::JsonApi::Model

  # defaults to demodulized, pluralized class name, e.g. contributors
  type :developers
end

JSON API response follows this general format:

{ data: {
  type: "developers",
  id: "6ab79c8c-ec5a-4426-ad38-8763bbede5a7",
  attributes: {
    language: "ruby",
    name: "avdi grimm",
  }
}

Underneath the hood, we introduce a parsing and serialization routine that translates to and from the JSON API format and the format Her expects.

Here are some benefits of this approach:

• Developer interaction with Her::JsonApi::Model would remain consistent with Her::Model, which should be easy for Rails developers to grok, since it's based on ActiveModel.
• This solution builds off of, rather than modifies, the existing codebase. This makes it a low risk addition which should have no impact on existing functionality.

The main downside is that we would need to combine the top-level data namespace with the attributes namespace. If an "id" or "type" were passed in attributes different than the top level id, we'd need to figure out a way to resolve the conflict. Perhaps top-level "id" and "type" get added as "data_id" and "data_type" or something...

[marshall-lee]

1. What's the purpose of format: :json_api then? Deprecate it?
2. Maybe just create a PR so we can look at it?

[marshall-lee]

The main downside is that we would need to combine the top-level data namespace with the attributes namespace. If an "id" or "type" were passed in attributes different than the top level id, we'd need to figure out a way to resolve the conflict.

This issue should definitely be resolved before merging.
First of all: what JSON API standard says about issues like this? Probably It says nothing because there is actually no such problem on JSON API side 😃

Perhaps top-level "id" and "type" get added as "data_id" and "data_type" or something...

wow, just don't do this! What if I have data_type or data_id in attributes? Just don't put in ActiveModel's attributes something that is actually not an attribute because it will produce bugs.

Surprisingly, the answer lies in my issue #338. Look at how ActiveModel is implemented: virtual module is injected in a model class that stores all the attribute methods. Okay what if I have a class attribute in the API response? Then :class attribute method will be created dynamically so It would not be possible to get an object class 😄 Attribute methods are superior and there's literally nothing to do with it. It's just an ActiveModel's caveat, that user should be warned about. Just don't struggle with it!

Soooo I propose this solution:

1. Don't put top-level type into attributes. If I get JSON API right the type is an API parameter — not an attribute. It can't be mutated. You could store it in instance variable @type and add a reader method for this but don't put it into attributes. If I call object.attributes[:type] = 'something' it's an assignment to type attribute but not to a type API parameter.
2. Define _type instance method for accessing @type instance variable. If someone has a type attribute he would have a possibility to access a not overriden type.
3. Put top-level id into attributes. Seriously, It's a different thing. Suppose a corner case when we mutate an id with PUT /somethings/old_id. Or just create an entity with custom id using POST /somethings. I don't know how the standard deals with this situation but this behavior seems not to break it.

[marshall-lee]

It's also worth to have ability to somehow pass a type API parameter separately from passing attributes when calling Her methods. I don't realized yet how to achieve this. Need some more use cases of advanced usage of type parameter.

But for now we could simply infer type value from model's name. Doing this we make a Her::JsonApi implementation being a compatible subset of JSON API standard. Not bad for the start!

[marshall-lee]

And I believe this all could be safely implemented without touching existing Her::Model code.

[hubert]

@marshall-lee

1. What's the purpose of format: :json_api then? Deprecate it?

The idea would be to deprecate it. Her should remain flexible enough to consume any api. However, for standards we want to support, they should have a first class representation which doesn't require any custom configuration to consume.

2. Maybe just create a PR so we can look at it?

I have a working PR in another fork. I'll create a PR so that people can look at it. It does not touch Her::Model which was one of my goals when writing this.

[marshall-lee]

4 I haven't described what to do when attributes: { id: '...' } is in the response. Well, I propose not doing anything with it for now. We should tell about this caveat in documentation but not make any workarounds in code. Just warn a user that using reserved keywords (class, id, etc) is dangerous and that's all.

[marshall-lee]

After all, API endpoint that sends id in attributes is strange and corner cased.

[hubert]

@marshall-lee i agree with most of what you say.

the top-level id and type attributes are both "api" rather than object attributes. namespacing the object attributes was done so that they could continue to add attrs as necessary and not conflict with whatever the user returns in attributes.

i think that adding _type and _id methods are reasonable ways to go, but those methods will still shadow _id and _type that are passed within attributes. the user could still dig in and access them directly via attributes which would not be the norm, but i think that's fair to put that burden on the api developer.

agree it is a corner case if id is passed within attributes.

[marshall-lee]

i think that adding _type and _id methods are reasonable ways to go, but those methods will still shadow _id and _type that are passed within attributes. the user could still dig in and access them directly via attributes which would not be the norm, but i think that's fair to put that burden on the api developer.

Yeah but be careful when adding _id. I propose not add it for now. We can store _type separately from attributes, but not the _id because in current Her architecture it assumes that id is a part of attributes. Ideally, Her::JsonApi should use that separately stored _id when constructing paths like /somethings/123 but not the value of id attribute. Does your implementation achieve this? If not, adding that _id will be useless and confusing. Achieving this may require changing the Her::Model code but maybe there is a simpler solution.

Also Her::Model adds a class method primary_key. Your Her::JsonApi should not add this method or just raise an exception because JSON API doesn't allow to setup different name for id.

[marshall-lee]

But maybe... Add that _id now and solve the issue with path constructing later? It's better than nothing 😃

But please undefine all these primary_key, parse_root_in_json and include_root_in_json in Her::JsonApi because they're simply not related to JSON API standard. Getting NoMethodError exception will show the user, that read documentation poorly, that these settings are not valid in case of JSON API.