active_model_serializers icon indicating copy to clipboard operation
active_model_serializers copied to clipboard

Invalid resource identifiers are generated for null resources (with type and not id)

Open f1sherman opened this issue 7 years ago • 1 comments

Expected behavior vs actual behavior

Expected behavior: Serializing a null resource returns { data: null }

Actual behavior: Serializing a null resource returns { data: { type: '(something)' } }

Steps to reproduce

(e.g., detailed walkthrough, runnable script, example application)

Pass a blank id (representing a null or unpersisted record) to ResourceIdentifier.for_type_with_id.

Environment

ActiveModelSerializers Version (commit ref if not on tag):
0.10.7

Output of ruby -e "puts RUBY_DESCRIPTION": ruby 2.5.0p0 (2017-12-25 revision 61468) [x86_64-darwin17]

OS Type & Version: MacOS High Sierra

Integrated application and version (e.g., Rails, Grape, etc): Rails 5.1

Additonal helpful information

I noticed that this has been changed to the expected behavior and changed back again (see https://github.com/rails-api/active_model_serializers/pull/2216). However, per the JSONAPI spec it appears that a resource identifier with a type and without an ID is not valid:

A “resource identifier object” MUST contain type and id members.

http://jsonapi.org/format/#document-resource-identifier-objects

Also note that this is the behavior that Ember Data expects, and it will return an error if it encounters a resource identifier with only a type: https://github.com/emberjs/data/blob/b329a2c7018b86716d202610d3b6d0e080e370da/addon/-private/system/relationships/state/belongs-to.js#L208

This would be relatively trivial to change back, but obviously it would be a breaking change, so I wanted to start a discussion here of if/how this could be handled before submitting a PR.

f1sherman avatar Jan 24 '18 16:01 f1sherman

@f1sherman yeah, we need to make this configurable.

Problem is, that id is optional is in CREATE requests, so it's reasonable to assume that serializing a record with no id (not persisted) should just not include the id.

In general we've tried to avoid allowing passing nil to a serializer. But perhaps it makes sense for an adapter to intercept that nil object and interpret as a null resource, and skip the serializer. I'd have to look at the code to see how this could be handled

bf4 avatar Feb 07 '18 21:02 bf4