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

Serializing an empty array should maintain the root key #1156

Conversation

NullVoxPopuli
Copy link
Contributor

Relates to: #1153

@NullVoxPopuli NullVoxPopuli force-pushed the infer-root-name-from-serializer branch from 82424ce to 91f1ea2 Compare September 16, 2015 02:05
Allow an empty array to derive the root key from the serializer name

why is the array serialized as an array, but a relationship serialized as a hash

arrays with the json adapter now have keys

fix rubocop stuff
@NullVoxPopuli NullVoxPopuli force-pushed the infer-root-name-from-serializer branch from 91f1ea2 to 6fdf461 Compare September 16, 2015 02:20
@NullVoxPopuli NullVoxPopuli changed the title [WIP] Serializing an empty array should maintain the root key Serializing an empty array should maintain the root key Sep 16, 2015
def inferred_json_key
serializer = options[:serializer]
return unless serializer
potential_model_name = serializer.name.sub('Serializer', '')
Copy link
Member

Choose a reason for hiding this comment

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

serializer.name.remove('Serializer') 🤷

@bf4
Copy link
Member

bf4 commented Sep 16, 2015

Let's hold off on merging this till we clarify which arguments, serializer vs. each_serializer we should be passing to manually specify the serializer for a non-compliant resource (i.e. would fail the Lint).

See my comment in the OP's issue: #1153 (comment)

@NullVoxPopuli
Copy link
Contributor Author

@bf4 for an empty object (Array, or Hash) - would it matter which serializer vs each_serializer we're using? there isn't really anything to serialize aside from the root key.

it would either be

{
  root: {}
}

or

{
  roots: []
}

@bf4
Copy link
Member

bf4 commented Sep 22, 2015

So, AMS doesn't handle primitives, and when they find their way it, they are passed through as is. ( Specifically, when you're not rendering an object with a defined serializer.)

How options are parsed

  1. serializable_resource = ActiveModel::SerializableResource.new(resource, options)
  2. if serializable_resource.serializer?
    • Where serializer? is use_adapter? && !!(serializer)
      • Where use_adapter?: 'True when no explicit adapter given, or explicit appear is truthy (non-nil); False when explicit adapter is falsy (nil or false)'
      • Where serializer:
        1. from explicit :serializer option, else
        2. implicitly from resource ActiveModel::Serializer.serializer_for(resource)
    • The :serializer option is removed from the serializer_opts hash
    • If the :each_serializer option is present, it is removed from the serializer_opts hash and set as the :serializer option
  3. The serializer and adapter are created as
    1. serializer_instance = serializer.new(resource, serializer_opts)
    2. adapter_instance = ActiveModel::Serializer::Adapter.create(serializer_instance, adapter_opts)
  4. If the serializer_instance was a ArraySerializer and the :serializer serializer_opts is present, then that serializer is passed into each resource:
    serializer_class = options.fetch(:serializer) do
    ActiveModel::Serializer.serializer_for(resource)
    end

The upshot of which is that

  • for a collection, the collection serializer is the :serializer option and :each_serializer is used as the serializer for each resource in the collection
  • for a single resource, the :serializer option is the resource serializer

Aside, the the collection serializer (ArraySerializer) cannot identify a serializer for a resource in its collection, it raises NoSerializerError which is rescued in build_association which sets the association value directly: reflection_options[:virtual_value] = association_value.try(:as_json) || association_value

@NullVoxPopuli
Copy link
Contributor Author

that is a fantastic explanation!

thanks -- I'll close this PR.

@NullVoxPopuli NullVoxPopuli deleted the infer-root-name-from-serializer branch September 22, 2015 15:42
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.

2 participants