-
Notifications
You must be signed in to change notification settings - Fork 364
Support .serialize …, coder: …
for Collections
#433
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
base: main
Are you sure you want to change the base?
Conversation
cd61a0a
to
64a7f52
Compare
lib/active_resource/coder.rb
Outdated
|
||
if collection | ||
raise ArgumentError.new("expected value to be Hash or Array, but was #{value.class}") unless value.is_a?(Hash) || value.is_a?(Array) | ||
resource_class.send(:instantiate_collection, value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make instantiate_collection
public and nodoc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
I've moved it above the private
line and added a # :nodoc:
comment. I'm not sure if this project's documentation incorporates # :nodoc
in the same way that rails/rails
does. For example, the ActiveResource::Collection class is marked as # :nodoc
, but it is still generating a documentation page.
Can you rebase please? |
7cf47c6
to
4798a43
Compare
4798a43
to
8aa1510
Compare
Follow-up to [rails#420][] Background --- The introduction of `ActiveResource::Base.coder` and the `ActiveResource::Coder` class added support for encoding and decoding instances of `Base`. However, collection-focused methods like `Base.where`, `Base.all`, and `Base.find(:all)` return `ActiveResource::Collection` instances. Problem --- While some collection instances are equivalent to `Array` instances, they are capable of being parsed into `Hash` values that include additional metadata (for example, pagination URLs, total counts, etc.). If applications were to dump results, there is a potential loss of that metadata. Proposal --- First, this commit modifies the `ActiveResource::Coder` class to accept a boolean `:collection` keyword to treat values as `ActiveResource::Collection` instances. Next, extend the `ActiveResource::Collection` class to retain the originally parsed values as a new `#original_parsed` attribute. It also defines the `#encode` method to rely on the resource class format for encoding. Additionally, extend the `ActiveResource::Serialization` module to also define a `.collection_coder` class attribute to serve as a convenience method for consumer to pass to `.serialize …, coder: …`: ```ruby class Person < ActiveResource::Base # … end class Team < ApplicationRecord serialize :people, coder: Person.collection_coder end ``` Like the instance-level coders, collection-level coders constructed with `collection: true` also accept an encoder proc to transform the value prior to dumping (for JSON/JSONB columns, for example): ```ruby class Person < ActiveResource::Base # … end class Team < ApplicationRecord # pass a to_proc-ready method name serialize :people, coder: ActiveResource::Coder.new(Person, :original_parsed, collection: true) # pass a block serialize :people, coder: ActiveResource::Coder.new(Person, collection: true) do |collection| collection.original_parsed end end ``` [rails#420]: rails#420
8aa1510
to
9a8053c
Compare
Follow-up to #420
Background
The introduction of
ActiveResource::Base.coder
and theActiveResource::Coder
class added support for encoding and decoding instances ofBase
.However, collection-focused methods like
Base.where
,Base.all
, andBase.find(:all)
returnActiveResource::Collection
instances.Problem
While some collection instances are equivalent to
Array
instances, they are capable of being parsed intoHash
values that include additional metadata (for example, pagination URLs, total counts, etc.). If applications were to dump results, there is a potential loss of that metadata.Proposal
First, this commit modifies the
ActiveResource::Coder
class to accept a boolean:collection
keyword to treat values asActiveResource::Collection
instances.Next, extend the
ActiveResource::Collection
class to retain the originally parsed values as a new#original_parsed
attribute. It also defines the#encode
method to rely on the resource class format for encoding.Additionally, extend the
ActiveResource::Serialization
module to also define a.collection_coder
class attribute to serve as a convenience method for consumer to pass to.serialize …, coder: …
:Like the instance-level coders, collection-level coders constructed with
collection: true
also accept an encoder proc to transform the value prior to dumping (for JSON/JSONB columns, for example):