Skip to content

Transform keys referenced in values in serialized documents #1645

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

Merged
merged 1 commit into from
Apr 4, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@

Breaking changes:

- [#1645](https://github.com/rails-api/active_model_serializers/pull/1645) Changed :dashed key transform to :dash. (@remear)
- [#1574](https://github.com/rails-api/active_model_serializers/pull/1574) Default key case for the JsonApi adapter changed to dashed. (@remear)

Features:
- [#1645](https://github.com/rails-api/active_model_serializers/pull/1645) Transform keys referenced in values. (@remear)
- [#1650](https://github.com/rails-api/active_model_serializers/pull/1650) Fix serialization scope options `scope`, `scope_name`
take precedence over `serialization_scope` in the controller.
Fix tests that required tearing down dynamic methods. (@bf4)
Expand Down
6 changes: 6 additions & 0 deletions active_model_serializers.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,10 @@ Gem::Specification.new do |spec|
spec.add_development_dependency 'grape', ['>= 0.13', '< 1.0']
spec.add_development_dependency 'json_schema'
spec.add_development_dependency 'rake', ['>= 10.0', '< 12.0']

spec.post_install_message = <<-EOF
NOTE: The default key case for the JsonApi adapter has changed to dashed.
See https://github.com/rails-api/active_model_serializers/blob/master/docs/general/key_transform.md
for more information on configuring this behavior.
Copy link
Member

Choose a reason for hiding this comment

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

This message will remain until the next rc5 or 0.10.0 whicher comes first ?

Copy link

Choose a reason for hiding this comment

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

Mehhhh, the post_install_message is not a good way to issue a deprecation I think. I mean, there is even a gem for ignoring post install messages that is pretty popular.... https://github.com/tpope/gem-shut-the-fuck-up

Why not just issue a real deprecation warning?

Copy link
Member

Choose a reason for hiding this comment

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

@ianks as our deprecation champion for this change, how would you recommend we do this? it's a changed default, not method, so we can't reroute.. right? or you think the change you be reverted for now?

Copy link

Choose a reason for hiding this comment

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

Champion, eh?! I guess I have a vested interest now 😆

The only time that this issue really affects people is when the options is nil, where it falls back to the default for the adapter. I would recommend that instead of accepting nil silently, we issue a deprecation notice when the option is not set.

Copy link

Choose a reason for hiding this comment

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

Basically, issue a deprecation in the default_key_transform method.

EOF
end
22 changes: 13 additions & 9 deletions docs/general/configuration_options.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,24 @@ When `false`, serializers must be explicitly specified.

##### key_transform

The [key transform](key_transform.md) to use.
The [key transform](key_transforms.md) to use.

Possible values:

- `:camel` - ExampleKey
- `:camel_lower` - exampleKey
- `:dashed` - example-key
- `:unaltered` - the original, unaltered key
- `nil` - use the adapter default
| Option | Result |
|----|----|
| `:camel` | ExampleKey |
| `:camel_lower` | exampleKey |
| `:dash` | example-key |
| `:unaltered` | the original, unaltered key |
| `:underscore` | example_key |
| `nil` | use the adapter default |

Each adapter has a default key transform configured:

- `Json` - `:unaltered`
- `JsonApi` - `:dashed`
| Adapter | Default Key Transform |
|----|----|
| `Json` | `:unaltered` |
| `JsonApi` | `:dash` |

`config.key_transform` is a global override of the adapter default. Adapters
still prefer the render option `:key_transform` over this setting.
Expand Down
34 changes: 0 additions & 34 deletions docs/general/key_transform.md

This file was deleted.

40 changes: 40 additions & 0 deletions docs/general/key_transforms.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
[Back to Guides](../README.md)

# Key Transforms

Key Transforms modify the casing of keys and keys referenced in values in
serialized responses.

Provided key transforms:

| Option | Result |
|----|----|
| `:camel` | ExampleKey |
| `:camel_lower` | exampleKey |
| `:dash` | example-key |
| `:unaltered` | the original, unaltered key |
| `:underscore` | example_key |
| `nil` | use the adapter default |

Key translation precedence is as follows:

##### Adapter option

`key_transform` is provided as an option via render.

```render json: posts, each_serializer: PostSerializer, key_transform: :camel_lower```

##### Configuration option

`key_transform` is set in `ActiveModelSerializers.config.key_transform`.

```ActiveModelSerializers.config.key_transform = :camel_lower```

##### Adapter default

Each adapter has a default transform configured:

| Adapter | Default Key Transform |
|----|----|
| `Json` | `:unaltered` |
| `JsonApi` | `:dash` |
41 changes: 24 additions & 17 deletions lib/active_model_serializers/adapter/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,25 +58,32 @@ def include_meta(json)
json
end

def default_key_transform
:unaltered
end
class << self
# Sets the default transform for the adapter.
#
# @return [Symbol] the default transform for the adapter
def default_key_transform
:unaltered
end

# Determines the transform to use in order of precedence:
# serialization context, global config, adapter default.
#
# @param serialization_context [Object] the SerializationContext
# @return [Symbol] the transform to use
def key_transform(serialization_context)
serialization_context.key_transform ||
ActiveModelSerializers.config.key_transform ||
default_key_transform
end
# Determines the transform to use in order of precedence:
# adapter option, global config, adapter default.
#
# @param options [Object]
# @return [Symbol] the transform to use
def transform(options)
return options[:key_transform] if options && options[:key_transform]
ActiveModelSerializers.config.key_transform || default_key_transform
end

def transform_key_casing!(value, serialization_context)
return value unless serialization_context
transform = key_transform(serialization_context)
KeyTransform.send(transform, value)
# Transforms the casing of the supplied value.
#
# @param value [Object] the value to be transformed
# @param options [Object] serializable resource options
# @return [Symbol] the default transform for the adapter
def transform_key_casing!(value, options)
KeyTransform.send(transform(options), value)
end
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/active_model_serializers/adapter/json.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class Json < Base
def serializable_hash(options = nil)
options ||= {}
serialized_hash = { root => Attributes.new(serializer, instance_options).serializable_hash(options) }
transform_key_casing!(serialized_hash, options[:serialization_context])
self.class.transform_key_casing!(serialized_hash, options)
Copy link
Member

Choose a reason for hiding this comment

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

how come this is a class method on the adapter?

Copy link
Member

Choose a reason for hiding this comment

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

(philosophical/stategic/curious)

Copy link
Member Author

Choose a reason for hiding this comment

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

To facilitate things like https://github.com/rails-api/active_model_serializers/pull/1645/files#diff-65abd6a4fd7fd55fe487ff138e0841dbR8. A class method here seemed like the best option at the time. I figured if there's a better way we could discuss refactoring and handle that in another PR instead of increasing the scope of this one.

end
end
end
Expand Down
55 changes: 28 additions & 27 deletions lib/active_model_serializers/adapter/json_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ def initialize(serializer, options = {})
@fieldset = options[:fieldset] || ActiveModel::Serializer::Fieldset.new(options.delete(:fields))
end

def default_key_transform
:dashed
def self.default_key_transform
:dash
Copy link
Member

Choose a reason for hiding this comment

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

oy

end

# {http://jsonapi.org/format/#crud Requests are transactional, i.e. success or failure}
Expand All @@ -48,9 +48,9 @@ def serializable_hash(options = nil)
document = if serializer.success?
success_document(options)
else
failure_document
failure_document(options)
Copy link
Contributor

Choose a reason for hiding this comment

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

do the options passed to here also trigger key transforming?

Copy link
Member Author

Choose a reason for hiding this comment

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

In the sense that if :transform is in those options that transform will be used instead of the global option or the adapter default (:dashed).

end
transform_key_casing!(document, options[:serialization_context])
self.class.transform_key_casing!(document, options)
Copy link
Contributor

Choose a reason for hiding this comment

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

why was the serialization_context passed before? and now just options?

Copy link
Member

Choose a reason for hiding this comment

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

I think #1643

Copy link
Contributor

Choose a reason for hiding this comment

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

nice

Copy link
Member Author

Choose a reason for hiding this comment

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

When I introduced this I thought it made more sense to pass just the serialization_context from the options. Now I think it makes more sense to just pass the options.

end

# {http://jsonapi.org/format/#document-top-level Primary data}
Expand All @@ -71,7 +71,7 @@ def serializable_hash(options = nil)
def success_document(options)
is_collection = serializer.respond_to?(:each)
serializers = is_collection ? serializer : [serializer]
primary_data, included = resource_objects_for(serializers)
primary_data, included = resource_objects_for(serializers, options)
Copy link
Member

Choose a reason for hiding this comment

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

seems strange that options is being passed into every method it seems

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I'm not a fan.


hash = {}
# toplevel_data
Expand Down Expand Up @@ -148,7 +148,7 @@ def success_document(options)
# }.reject! {|_,v| v.nil? }
# prs:
# https://github.com/rails-api/active_model_serializers/pull/1004
def failure_document
def failure_document(options)
hash = {}
# PR Please :)
# Jsonapi.add!(hash)
Expand All @@ -163,10 +163,10 @@ def failure_document
# ]
if serializer.respond_to?(:each)
hash[:errors] = serializer.flat_map do |error_serializer|
Error.resource_errors(error_serializer)
Error.resource_errors(error_serializer, options)
end
else
hash[:errors] = Error.resource_errors(serializer)
hash[:errors] = Error.resource_errors(serializer, options)
end
hash
end
Expand Down Expand Up @@ -224,21 +224,21 @@ def fragment_cache(cached_hash, non_cached_hash)
# [x] url helpers https://github.com/rails-api/active_model_serializers/issues/1269
# meta
# [x] https://github.com/rails-api/active_model_serializers/pull/1340
def resource_objects_for(serializers)
def resource_objects_for(serializers, options)
@primary = []
@included = []
@resource_identifiers = Set.new
serializers.each { |serializer| process_resource(serializer, true) }
serializers.each { |serializer| process_relationships(serializer, @include_tree) }
serializers.each { |serializer| process_resource(serializer, true, options) }
serializers.each { |serializer| process_relationships(serializer, @include_tree, options) }

[@primary, @included]
end

def process_resource(serializer, primary)
resource_identifier = ResourceIdentifier.new(serializer).as_json
def process_resource(serializer, primary, options)
resource_identifier = ResourceIdentifier.new(serializer, options).as_json
return false unless @resource_identifiers.add?(resource_identifier)

resource_object = resource_object_for(serializer)
resource_object = resource_object_for(serializer, options)
if primary
@primary << resource_object
else
Expand All @@ -248,21 +248,21 @@ def process_resource(serializer, primary)
true
end

def process_relationships(serializer, include_tree)
def process_relationships(serializer, include_tree, options)
serializer.associations(include_tree).each do |association|
process_relationship(association.serializer, include_tree[association.key])
process_relationship(association.serializer, include_tree[association.key], options)
end
end

def process_relationship(serializer, include_tree)
def process_relationship(serializer, include_tree, options)
if serializer.respond_to?(:each)
serializer.each { |s| process_relationship(s, include_tree) }
serializer.each { |s| process_relationship(s, include_tree, options) }
return
end
return unless serializer && serializer.object
return unless process_resource(serializer, false)
return unless process_resource(serializer, false, options)

process_relationships(serializer, include_tree)
process_relationships(serializer, include_tree, options)
end

# {http://jsonapi.org/format/#document-resource-object-attributes Document Resource Object Attributes}
Expand All @@ -286,9 +286,9 @@ def attributes_for(serializer, fields)
end

# {http://jsonapi.org/format/#document-resource-objects Document Resource Objects}
def resource_object_for(serializer)
def resource_object_for(serializer, options)
resource_object = cache_check(serializer) do
resource_object = ResourceIdentifier.new(serializer).as_json
resource_object = ResourceIdentifier.new(serializer, options).as_json

requested_fields = fieldset && fieldset.fields_for(resource_object[:type])
attributes = attributes_for(serializer, requested_fields)
Expand All @@ -297,7 +297,7 @@ def resource_object_for(serializer)
end

requested_associations = fieldset.fields_for(resource_object[:type]) || '*'
relationships = relationships_for(serializer, requested_associations)
relationships = relationships_for(serializer, requested_associations, options)
resource_object[:relationships] = relationships if relationships.any?

links = links_for(serializer)
Expand Down Expand Up @@ -425,15 +425,16 @@ def resource_object_for(serializer)
# id: 'required-id',
# meta: meta
# }.reject! {|_,v| v.nil? }
def relationships_for(serializer, requested_associations)
def relationships_for(serializer, requested_associations, options)
include_tree = ActiveModel::Serializer::IncludeTree.from_include_args(requested_associations)
serializer.associations(include_tree).each_with_object({}) do |association, hash|
hash[association.key] = Relationship.new(
serializer,
association.serializer,
association.options,
association.links,
association.meta
options,
options: association.options,
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for using named parameters. I love named parameters on anything with 2 or more parameters.

links: association.links,
meta: association.meta
).as_json
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,14 +182,14 @@ def parse_relationship(assoc_name, assoc_data, options)
prefix_key = field_key(assoc_name, options).to_s.singularize
hash =
if assoc_data.is_a?(Array)
{ "#{prefix_key}_ids".to_sym => assoc_data.map { |ri| ri[:id] } }
{ "#{prefix_key}_ids".to_sym => assoc_data.map { |ri| ri['id'] } }
else
{ "#{prefix_key}_id".to_sym => assoc_data && assoc_data.is_a?(Hash) ? assoc_data[:id] : nil }
{ "#{prefix_key}_id".to_sym => assoc_data ? assoc_data['id'] : nil }
end

polymorphic = (options[:polymorphic] || []).include?(assoc_name.to_sym)
if polymorphic
hash["#{prefix_key}_type".to_sym] = assoc_data.present? ? assoc_data[:type] : nil
hash["#{prefix_key}_type".to_sym] = assoc_data.present? ? assoc_data['type'] : nil
end

hash
Expand All @@ -198,7 +198,7 @@ def parse_relationship(assoc_name, assoc_data, options)
# @api private
def parse_relationships(relationships, options)
transform_keys(relationships, options)
.map { |(k, v)| parse_relationship(k, v[:data], options) }
.map { |(k, v)| parse_relationship(k, v['data'], options) }
Copy link
Member

Choose a reason for hiding this comment

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

I assume this is a bug fix in master?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if it's a bug. KeyTransform in master always returned symbols so a recent PR changes this code to work with that restriction. This PR makes Transform more flexible so that set of changes isn't necessary anymore.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

.reduce({}, :merge)
end

Expand Down
4 changes: 3 additions & 1 deletion lib/active_model_serializers/adapter/json_api/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ module Error
#
# @param [ActiveModel::Serializer::ErrorSerializer] error_serializer
# @return [Array<Symbol, Array<String>>] i.e. attribute_name, [attribute_errors]
def self.resource_errors(error_serializer)
def self.resource_errors(error_serializer, options)
error_serializer.as_json.flat_map do |attribute_name, attribute_errors|
attribute_name = JsonApi.send(:transform_key_casing!, attribute_name,
options)
attribute_error_objects(attribute_name, attribute_errors)
end
end
Expand Down
Loading