Transform keys referenced in values in serialized documents #1645

remear · 2016-03-31T21:05:55Z

Purpose

Apply key transforms to keys referenced in values in serialized documents.

Changes

Update key transforms to support multiple value types
Rename dashed key transform to dash

Related GitHub issues

#1613
#1625
#1574

Additional helpful information

Some helpful tests were provided by master...rthbound:bug-in-1574

Benchmark report

caching on: caching serializers: gc off 559.2443580027782/ips; 1810 objects
caching off: caching serializers: gc off 544.6921957270715/ips; 1810 objects
caching on: non-caching serializers: gc off 798.2542479087573/ips; 1246 objects
caching off: non-caching serializers: gc off 755.3988495824224/ips; 1246 objects
caching on: caching serializers: gc off 578.7006695551428/ips; 1810 objects
caching off: caching serializers: gc off 557.2192450384376/ips; 1810 objects
caching on: non-caching serializers: gc off 822.167102680962/ips; 1246 objects
caching off: non-caching serializers: gc off 793.4553596057367/ips; 1246 objects
caching on: caching serializers: gc off 580.5075760354737/ips; 1810 objects
caching off: caching serializers: gc off 557.0442339080246/ips; 1810 objects
caching on: non-caching serializers: gc off 832.2253431569914/ips; 1246 objects
caching off: non-caching serializers: gc off 774.6527647156513/ips; 1246 objects
caching on: caching serializers: gc off 581.7701133415057/ips; 1810 objects
caching off: caching serializers: gc off 544.7992003451257/ips; 1810 objects
caching on: non-caching serializers: gc off 851.9271723317336/ips; 1246 objects
caching off: non-caching serializers: gc off 826.8414637806952/ips; 1246 objects
caching on: caching serializers: gc off 602.3541757573917/ips; 1810 objects
caching off: caching serializers: gc off 571.5713299612428/ips; 1810 objects
caching on: non-caching serializers: gc off 863.8563683877113/ips; 1246 objects
caching off: non-caching serializers: gc off 808.5430729217755/ips; 1246 objects
Benchmark results:
{
  "commit_hash": "becf31a",
  "version": "0.10.0.rc4",
  "rails_version": "4.2.6",
  "benchmark_run[environment]": "2.2.2p95",
  "runs": [
    {
      "benchmark_type[category]": "caching on: caching serializers: gc off",
      "benchmark_run[result][iterations_per_second]": 602.354,
      "benchmark_run[result][total_allocated_objects_per_iteration]": 1810
    },
    {
      "benchmark_type[category]": "caching off: caching serializers: gc off",
      "benchmark_run[result][iterations_per_second]": 571.571,
      "benchmark_run[result][total_allocated_objects_per_iteration]": 1810
    },
    {
      "benchmark_type[category]": "caching on: non-caching serializers: gc off",
      "benchmark_run[result][iterations_per_second]": 863.856,
      "benchmark_run[result][total_allocated_objects_per_iteration]": 1246
    },
    {
      "benchmark_type[category]": "caching off: non-caching serializers: gc off",
      "benchmark_run[result][iterations_per_second]": 826.841,
      "benchmark_run[result][total_allocated_objects_per_iteration]": 1246
    }
  ]
}
camel 444.72195002379516/ips; 3569 objects
camel_lower 575.3975560772479/ips; 2487 objects
dash 2780.2742509858344/ips; 719 objects
unaltered 5971343.618915383/ips; 1 objects
underscore 4102.5819244858/ips; 313 objects
camel 430.83735091669/ips; 3569 objects
camel_lower 582.118042933472/ips; 2487 objects
dash 2862.6015904915635/ips; 719 objects
unaltered 6845976.792527086/ips; 1 objects
underscore 4232.043748664645/ips; 313 objects
camel 445.69972284071025/ips; 3569 objects
camel_lower 586.6072310145361/ips; 2487 objects
dash 2794.555574503196/ips; 719 objects
unaltered 6383218.323449024/ips; 1 objects
underscore 4109.766302245609/ips; 313 objects
camel 416.70362998385446/ips; 3569 objects
camel_lower 567.9704092061108/ips; 2487 objects
dash 2767.0964650677665/ips; 719 objects
unaltered 6268428.531397386/ips; 1 objects
underscore 4201.016484849122/ips; 313 objects
camel 436.77251219145313/ips; 3569 objects
camel_lower 549.8656927219089/ips; 2487 objects
dash 2844.8626655425846/ips; 719 objects
unaltered 6523871.688062123/ips; 1 objects
underscore 4304.981606429804/ips; 313 objects
Benchmark results:
{
  "commit_hash": "3498647",
  "version": "0.10.0.rc4",
  "rails_version": "4.2.6",
  "benchmark_run[environment]": "2.2.2p95",
  "runs": [
    {
      "benchmark_type[category]": "camel",
      "benchmark_run[result][iterations_per_second]": 445.7,
      "benchmark_run[result][total_allocated_objects_per_iteration]": 3569
    },
    {
      "benchmark_type[category]": "camel_lower",
      "benchmark_run[result][iterations_per_second]": 586.607,
      "benchmark_run[result][total_allocated_objects_per_iteration]": 2487
    },
    {
      "benchmark_type[category]": "dash",
      "benchmark_run[result][iterations_per_second]": 2862.602,
      "benchmark_run[result][total_allocated_objects_per_iteration]": 719
    },
    {
      "benchmark_type[category]": "unaltered",
      "benchmark_run[result][iterations_per_second]": 6845976.793,
      "benchmark_run[result][total_allocated_objects_per_iteration]": 1
    },
    {
      "benchmark_type[category]": "underscore",
      "benchmark_run[result][iterations_per_second]": 4304.982,
      "benchmark_run[result][total_allocated_objects_per_iteration]": 313
    }
  ]
}

NullVoxPopuli · 2016-03-31T21:22:43Z

lib/active_model_serializers/adapter/base.rb

-        :unaltered
-      end
+      class << self
+        def default_transform


should there be docs on this method?

More docs than mentioning that there are default transforms specified for each adapter? https://github.com/remear/active_model_serializers/blob/transforms/docs/general/key_transform.md#adapter-default

Which reminds me, I need to rename that md file.

I just mean something real quick for devs browsing the code, in case they haven't discovered that we do key transforms yet

Yes, there should be some docs in the code.

bf4 · 2016-03-31T21:37:13Z

@remear this might be a good one to benchmark. esp compare to other libs that do the same thing e.g. https://github.com/cerebris/jsonapi-resources/blob/d2a9d0f40105e246b0fcffd9e1cf1d3b634053f1/lib/jsonapi/formatter.rb#L64-L103

NullVoxPopuli · 2016-03-31T21:54:00Z

@bf4, is there an easy way to run your benchmarker? like...

rake benchmark

or something

bf4 · 2016-04-01T04:05:19Z

docs/general/transform.md

+Each adapter has a default transform configured:
+
+- `Json` - `:unaltered`
+- `JsonApi` - `:dash`


you can use a table here and above for better layout. see https://github.com/rails-api/active_model_serializers/tree/master/docs#integrations

ianks · 2016-04-01T04:28:32Z

lib/active_model_serializers/adapter/base.rb

+        # @return [Symbol] the transform to use
+        def transform(options)
+          return options[:transform] if options && options[:transform]
+          ActiveModelSerializers.config.transform || default_transform


Let's raise a deprecation notice here.

bf4 · 2016-04-01T06:39:20Z

@NullVoxPopuli

easy way to run your benchmark

bin/bench and see #1575

bf4 · 2016-04-01T06:40:01Z

test/benchmark/controllers.rb

@@ -8,7 +8,7 @@ class PostController < ActionController::Base
      else
        comments = [Comment.new(id: 1, body: 'ZOMG A COMMENT')]
      end
-      author = Author.new(id: 42, name: 'Joao Moura.')
+      author = Author.new(id: 42, first_name: 'Joao', last_name: 'Moura')


bf4 · 2016-04-01T19:19:44Z

active_model_serializers.gemspec

+  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.


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

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?

@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?

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.

Basically, issue a deprecation in the default_key_transform method.

bf4 · 2016-04-01T19:23:16Z

lib/active_model_serializers/adapter/json.rb

@@ -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)


how come this is a class method on the adapter?

(philosophical/stategic/curious)

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.

bf4 · 2016-04-04T15:03:45Z

ref or https://github.com/rails-api/active_model_serializers/blob/0-9-stable/lib/active_model/serializer.rb#L260-L282 and https://github.com/rails-api/active_model_serializers/blob/0-9-stable/lib/active_model/serializer.rb#L294

NullVoxPopuli reviewed Mar 31, 2016
View reviewed changes

remear force-pushed the transforms branch from 5c87f69 to 01e39c6 Compare April 1, 2016 01:21

remear added a commit to remear/active_model_serializers that referenced this pull request Apr 1, 2016

Update CHANGELOG with rails-api#1645 and breaking change information

025d021

remear force-pushed the transforms branch from a84e1c1 to a958542 Compare April 1, 2016 02:01

bf4 added C: Feature V: 0.10.x C: Keys and Transform labels Apr 1, 2016

bf4 reviewed Apr 1, 2016
View reviewed changes

bf4 mentioned this pull request Apr 1, 2016

Issue deprecation notice for key_transform default changes #1646

Closed

ianks reviewed Apr 1, 2016
View reviewed changes

bf4 reviewed Apr 1, 2016
View reviewed changes

bf4 mentioned this pull request Apr 1, 2016

Deserialization gives an empty hash with JSON Api #1640

Closed

remear force-pushed the transforms branch from a9a0e09 to c82a4af Compare April 1, 2016 18:23

remear changed the title ~~Transforms~~ Transform keys referenced in values in serialized documents Apr 1, 2016

remear force-pushed the transforms branch 2 times, most recently from 9583963 to becf31a Compare April 1, 2016 18:45

bf4 reviewed Apr 1, 2016
View reviewed changes

remear force-pushed the transforms branch 3 times, most recently from c2c04e5 to 039d08c Compare April 1, 2016 20:53

chrisdpeters mentioned this pull request Apr 3, 2016

Error serialization keys not matching #1654

Closed

remear force-pushed the transforms branch from 039d08c to 70c9741 Compare April 3, 2016 21:24

remear mentioned this pull request Apr 4, 2016

Namespace separator setting #1609

Closed

remear force-pushed the transforms branch from 70c9741 to 26a7479 Compare April 4, 2016 16:12

Apply key transforms to keys referenced in values

3498647

remear force-pushed the transforms branch from 26a7479 to 3498647 Compare April 4, 2016 16:16

remear merged commit 0e82f6b into rails-api:master Apr 4, 2016

remear deleted the transforms branch April 13, 2016 17:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Transform keys referenced in values in serialized documents #1645

Transform keys referenced in values in serialized documents #1645

remear commented Mar 31, 2016

NullVoxPopuli Mar 31, 2016

remear Mar 31, 2016

NullVoxPopuli Mar 31, 2016

remear Mar 31, 2016

bf4 commented Mar 31, 2016

NullVoxPopuli commented Mar 31, 2016

bf4 Apr 1, 2016

ianks Apr 1, 2016

bf4 commented Apr 1, 2016

bf4 Apr 1, 2016

bf4 Apr 1, 2016

ianks Apr 1, 2016

bf4 Apr 1, 2016

ianks Apr 1, 2016

ianks Apr 1, 2016

bf4 Apr 1, 2016

bf4 Apr 1, 2016

remear Apr 1, 2016

bf4 commented Apr 4, 2016

Transform keys referenced in values in serialized documents #1645

Transform keys referenced in values in serialized documents #1645

Conversation

remear commented Mar 31, 2016

Purpose

Changes

Related GitHub issues

Additional helpful information

Benchmark report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bf4 commented Mar 31, 2016

NullVoxPopuli commented Mar 31, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bf4 commented Apr 1, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bf4 commented Apr 4, 2016