Modify cache key to prevent errors and incorrect data returned. Fixes #1344 #1346
Conversation
… This will prevent objects PORO objects that don't have updated_at defined, from throwing an error. Not as big a deal now that PORO objects can inherit ActiveModelSerializers::Model, but still necessary if it's not inherited for whatever reason. Add the Adapter type to the cache key. This prevents incorrect results when the same object is serialized with different adapters.
kevintyll
changed the title
| @@ -55,7 +55,7 @@ def self.digest_caller_file(caller_line) | |||
|
|
|||
| serializer.class_attribute :_cache # @api private : the cache object | |||
| serializer.class_attribute :_fragmented # @api private : @see ::fragmented | |||
| serializer.class_attribute :_cache_key # @api private : when present, is first item in cache_key | |||
| serializer.class_attribute :_cache_key # @api private : when present, is first item in cache_key. Ignored if the serialized object defines #cache_key. | |||
There was a problem hiding this comment.
Don't know what you're asking here.
There was a problem hiding this comment.
in the diff, to change Ignored if the serialized object defines #cache_key. to Ignored if the serializable object defines #cache_key.
|
@kevintyll hi :) |
|
Hi, sorry I haven't responded. I haven't had a chance to work on your suggestions yet. I'm hoping to be able to get to them after the first of the year. |
For discussion: Consider evaluating association in serializer context That way, associations are really just anything that can be conditionally included. They no longer have to actually be methods on the object or serializer. e.g. ```diff has_many :comments do - last(1) + Comment.active.for_serialization(object).last(1) end ```
|
soon... :) |
Fix broken link in guides
Simplify attributes handling.
…leases Restore docs from older releases
|
🌅 |
|
Hoping to get to it Wednesday. |
| parts = [] | ||
| parts << object_cache_key | ||
| parts << adapter_instance.class.to_s.demodulize.underscore | ||
| parts << @klass._cache_digest unless @klass._cache_options && @klass._cache_options[:skip_digest] | ||
| parts.join('/') |
There was a problem hiding this comment.
- TODO: urgh, we need to document this option (note to self)
…ister to change it's signature. Not sure if that is okay or not.
|
Let me know about the change I made to the register method with the adapter#name. I'll continue tomorrow. |
|
The inclusion of adapter type is a good idea but the impl change is too big for this pr, needs to be discussed elsewhere... I'm ok surfacing the adapter name as a class method, but interface of registering shouldn't change here (reviewing on phone, pardon if I missed something.) |
|
I think serializer#cache_key is the more fundamental change #1346 (comment) |
… This will prevent objects PORO objects that don't have updated_at defined, from throwing an error. Not as big a deal now that PORO objects can inherit ActiveModelSerializers::Model, but still necessary if it's not inherited for whatever reason. Add the Adapter type to the cache key. This prevents incorrect results when the same object is serialized with different adapters.
…ister to change it's signature. Not sure if that is okay or not.
… on adapter, though that leaves a disconnect with the naming in the register method.
Conflicts: lib/active_model/serializer.rb lib/active_model/serializer/adapter/base.rb lib/active_model/serializer/adapter/cached_serializer.rb test/fixtures/poro.rb test/serializers/cache_test.rb
|
Okay, I think I got it all. Not sure what happened to the diff. Looks like it's picking things up from the merge of master into my fork. |
|
Probably rebase and force push helps B mobile phone
|
|
Rebasing didn't do anything. Had some issues merging upstream into my fork. I probably screwed up the commit history. I confident the code is correctly merged though. |
|
Continued in #1642 |
When caching, return the object's cache_key up front if it's defined. This will prevent objects PORO objects that don't have updated_at defined, from throwing an error. Not as big a deal now that PORO objects can inherit ActiveModelSerializers::Model, but still necessary if it's not inherited for whatever reason.
Add the Adapter type to the cache key. This prevents incorrect results when the same object is serialized with different adapters.
fixes #1344