Skip to content
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

Add support for single record serialization #50

Conversation

felixbuenemann
Copy link
Collaborator

@felixbuenemann felixbuenemann commented Apr 15, 2016

This will handle serialization of single records as used for example in show actions using the postres array serializer. To be able to do this, the record is wrapped in a relation using its primary key for lookup.

It's also possible to use this feature with a relation by passing the root: 'singular_name' and single_record: true options to render or the serializer, but you have to ensure that the relation only returns a single record, for example by using .limit(1).

The single record serialization format does use a singular root key name for the main resource and does not wrap the object in an array:

Collection: {"notes":[{"id":1,"tag_ids":[1]}],"tags":[...]}
Single Record: {"note":{"id":1,"tag_ids":[1]},"tags":[...]}

The latter format is required by ember-data with the active model adapter when using find('note', 1).

This PR does not add support for the root: false option (see #13). For single records a fallback to the regular serializer will be used when root is disabled.

@felixbuenemann
Copy link
Collaborator Author

Currently this PR switches the default serializer for single records from ruby to postgres and is likely to break apps that currently us a mix of ruby and postgres serialization, if the resources currently serialized by ruby are not fully compatible with postgres_ext-serializers.

As such merging this PR should require a major version bump.

Another option would be to make the :single_record option opt-in and change it's default when releasing the next version with breaking changes.

end

it 'generates the proper json output' do
# XXX Not sure why the key order is inverted here...
Copy link
Collaborator Author

@felixbuenemann felixbuenemann Apr 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is, because it's not actually using the database serialization. We should alter the test to ensure the original serializers are not called.

@felixbuenemann
Copy link
Collaborator Author

felixbuenemann commented Apr 15, 2016

This needs some more work. The "serialize singular record" test is not working as intended.

Currently we need to manually pass a record to the ArraySerializer to be able to use database serialization. In order for this to happen for render json: record we need to alter the ActiveModel::Serializer.build_json class method to pass the record to the ArraySerializer instead of the default serializer.

@felixbuenemann felixbuenemann force-pushed the add-support-for-single-record-serialization branch from 378918d to 34fd48f Compare April 15, 2016 21:59
@felixbuenemann
Copy link
Collaborator Author

I've revised the code and moved the relation wrapping into a patch to ActiveModel::Serializer.build_json which is cleaner and fixes automatic database serialization when passing records to render :json.

I've also extended the tests to make sure the ArraySerializer is used for serialization and that using custom serializers works as expected.

This will handle serialization of single records as used for example in
show actions using the postres array serializer. To be able to do this,
the record is wrapped in a relation using its primary key for lookup.

It's also possible to use this feature with a relation by passing the
`root: 'singular_name'` and `single_record: true` options to render or
the serializer, but you have to ensure that the relation only returns a
single record, for example by using `.limit(1)`.

The single record serialization format does use a singular root key name
for the main resource and does not wrap the object in an array:

Collection: `{"notes":[{"id":1,"tag_ids":[1]}],"tags":[...]}`
Single Record: `{"note":{"id":1,"tag_ids":[1]},"tags":[...]}`

This adds a patch to ActiveModel::Serializer.build_json which adds the
required wrapping in a relation.

Note that this method will load and instantiate the record and it's
serializer once and then reload it using the primary key in a relation.
To avoid this you can skip the find call in the controller and replace
it with a relation and pass `:root` and `:single_record` along with the
singular relation in the call to `render`.
@felixbuenemann felixbuenemann force-pushed the add-support-for-single-record-serialization branch from 34fd48f to 52fb8d1 Compare April 15, 2016 22:28
@felixbuenemann
Copy link
Collaborator Author

Rebased on current master.

@felixbuenemann
Copy link
Collaborator Author

Instead of adding an option to enable/disable single record database serialization, there should probably be an option like use_postgres_serializer: false to disable postgres_ext-serializers on a case by case basis, eg. using default_serializer_options in the controller, or an option passed to render :json.

This would make it easier to work with projects where not all serializers are compatible with postgres_ext-serializers and ease migration towards full database serialization.

@felixbuenemann
Copy link
Collaborator Author

felixbuenemann commented Apr 15, 2016

I'm going to merge this as is and add the option to conditionally disable database serialization in a separate PR. This means the next release should be 1.0.0.

@felixbuenemann felixbuenemann merged commit 0c2d483 into DavyJonesLocker:master Apr 15, 2016
@felixbuenemann felixbuenemann deleted the add-support-for-single-record-serialization branch April 15, 2016 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant