Open onomated opened 8 years ago
In master I've removed the dynamic serializers. I'm curious if you can find a better solution there...
in master
def fetch_attributes_fragment(adapter_instance)
serializer_class._cache_options ||= {}
serializer_class._cache_options[:key] = serializer_class._cache_key if serializer_class._cache_key
#....
key = cache_key(adapter_instance)
so, key
option will be respected
def cache_key(adapter_instance)
return @cache_key if defined?(@cache_key)
parts = []
parts << object_cache_key
parts << adapter_instance.cache_key
parts << serializer_class._cache_digest unless serializer_class._skip_digest?
@cache_key = parts.join('/')
end
Also, any particular reason you're skipping the digest?
@bf4 No strong reason other than I purge all cache on deploy, so computing digests is extra computation that isn't required.
I'll try master
and see how that goes. Thanks!
Ok, so looks like @bf4 is off the project? All comments, mentions, and PRs and gone
@onomated github thinks I"m a robot
@bf4 You probably are a robot with all the activity you're balancing at a time.
Took a look at master and my object_cache_key
method override works as long as I provide the cache key
configuration option as well. Ideally, it'll be great if the AMS cache method took a namespace
or prefix
option so clients won't have to dig in and override methods. That way I can prefix api responses with a namespace such as api:objects:<serializer_class_name>:
. This is what I currently accomplish with the object_cache_key
override.
Thanks for your hard work!
Turns out I made too many links in an issue on my blog repo (was just using it to link dump)
@bf4 May be related to your recent changes to caching, but I started getting this error during cache fetch. See stack trace below. AMS commit hash is up-to-date with master. There have been no changes to application code since:
Rescued from TypeError
no _dump_data is defined for class OpenSSL::Digest
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/redis-store-1.1.7/lib/redis/store/marshalling.rb:29:in `dump'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/redis-store-1.1.7/lib/redis/store/marshalling.rb:29:in `_marshal'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/redis-store-1.1.7/lib/redis/store/marshalling.rb:5:in `set'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/redis-store-1.1.7/lib/redis/store/namespace.rb:5:in `block in set'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/redis-store-1.1.7/lib/redis/store/namespace.rb:74:in `namespace'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/redis-store-1.1.7/lib/redis/store/namespace.rb:5:in `set'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/redis-activesupport-4.1.5/lib/active_support/cache/redis_store.rb:223:in `block in write_entry'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/redis-activesupport-4.1.5/lib/active_support/cache/redis_store.rb:212:in `call'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/redis-activesupport-4.1.5/lib/active_support/cache/redis_store.rb:212:in `with'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/redis-activesupport-4.1.5/lib/active_support/cache/redis_store.rb:223:in `write_entry'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/redis-activesupport-4.1.5/lib/active_support/cache/redis_store.rb:60:in `block in write'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/cache.rb:547:in `block in instrument'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/notifications.rb:166:in `instrument'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/cache.rb:547:in `instrument'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/redis-activesupport-4.1.5/lib/active_support/cache/redis_store.rb:58:in `write'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/cache.rb:588:in `save_block_result_to_cache'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/cache.rb:299:in `fetch'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model/serializer/caching.rb:222:in `fetch'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model_serializers/adapter/json_api.rb:297:in `resource_object_for'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model_serializers/adapter/json_api.rb:248:in `process_resource'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model_serializers/adapter/json_api.rb:270:in `process_relationship'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model_serializers/adapter/json_api.rb:260:in `block in process_relationships'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model/serializer/associations.rb:93:in `yield'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model/serializer/associations.rb:93:in `block (2 levels) in associations'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model/serializer/associations.rb:89:in `each'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model/serializer/associations.rb:89:in `block in associations'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model_serializers/adapter/json_api.rb:259:in `each'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model_serializers/adapter/json_api.rb:259:in `each'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model_serializers/adapter/json_api.rb:259:in `process_relationships'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model_serializers/adapter/json_api.rb:239:in `block in resource_objects_for'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model/serializer/collection_serializer.rb:6:in `each'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model/serializer/collection_serializer.rb:6:in `each'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model_serializers/adapter/json_api.rb:239:in `resource_objects_for'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model_serializers/adapter/json_api.rb:90:in `success_document'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model_serializers/adapter/json_api.rb:59:in `serializable_hash'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model_serializers/adapter/base.rb:59:in `as_json'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model_serializers/serializable_resource.rb:8:in `to_json'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model_serializers/serializable_resource.rb:8:in `to_json'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model_serializers/logging.rb:69:in `block (3 levels) in notify'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:117:in `call'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:117:in `call'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:555:in `block (2 levels) in compile'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:505:in `call'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:505:in `call'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:498:in `block (2 levels) in around'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:343:in `call'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:343:in `block (2 levels) in simple'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model_serializers/logging.rb:22:in `call'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model_serializers/logging.rb:22:in `block (3 levels) in instrument_rendering'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model_serializers/logging.rb:79:in `block in notify_render'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/notifications.rb:164:in `block in instrument'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/notifications/instrumenter.rb:20:in `instrument'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/notifications.rb:164:in `instrument'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model_serializers/logging.rb:78:in `notify_render'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model_serializers/logging.rb:21:in `block (2 levels) in instrument_rendering'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model_serializers/logging.rb:95:in `block in tag_logger'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/tagged_logging.rb:68:in `block in tagged'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/tagged_logging.rb:26:in `tagged'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/tagged_logging.rb:68:in `tagged'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model_serializers/logging.rb:95:in `tag_logger'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model_serializers/logging.rb:20:in `block in instrument_rendering'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:441:in `instance_exec'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:441:in `block in make_lambda'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:342:in `call'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:342:in `block in simple'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:497:in `call'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:497:in `block in around'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:505:in `call'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:505:in `call'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:92:in `_run_callbacks'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:776:in `_run_render_callbacks'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:81:in `run_callbacks'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/bundler/gems/active_model_serializers-10ec8ece16f3/lib/active_model_serializers/logging.rb:68:in `block (2 levels) in notify'
/var/app/current/app/api/formatters/scoped_active_model_serializer.rb:8:in `call'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/grape-0.16.2/lib/grape/middleware/formatter.rb:44:in `block in build_formatted_response'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/grape-0.16.2/lib/grape/middleware/formatter.rb:44:in `collect'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/grape-0.16.2/lib/grape/middleware/formatter.rb:44:in `build_formatted_response'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/grape-0.16.2/lib/grape/middleware/formatter.rb:28:in `after'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/grape-0.16.2/lib/grape/middleware/base.rb:33:in `call!'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/grape-0.16.2/lib/grape/middleware/base.rb:23:in `call'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/newrelic_rpm-3.15.2.317/lib/new_relic/agent/instrumentation/middleware_tracing.rb:96:in `call'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/grape-0.16.2/lib/grape/middleware/base.rb:30:in `call!'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/grape-0.16.2/lib/grape/middleware/base.rb:23:in `call'
/opt/rubies/ruby-2.2.4/lib/ruby/gems/2.2.0/gems/newrelic_rpm-3.15.2.317/lib/new_relic/agent/instrumentation/middleware_tracing.rb:96:in `call'
...
@bf4 Error above is a result of fragment caching not being invoked. AMS is attempting to cache an excluded field in my entity. I've inspected my redis cache and confirmed that excluded fields are being cached. Stepping through caching.rb
it doesn't look like fetch_attributes_fragment is being invoked
Do you think you can narrow this down to a failing test? I can pair w you if it helps
B mobile phone
On Jun 15, 2016, at 6:12 AM, Onome notifications@github.com wrote:
@bf4 Error above is a result of fragment caching not being invoked. AMS is attempting to cache an excluded field in my entity. I've inspected my redis cache and confirmed that excluded fields are being cached. Stepping through caching.rb it doesn't look like fetch_attributes_fragment is being invoked
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.
Stepping through caching.rb it doesn't look like fetch_attributes_fragment is being invoked
@onomated any debug info you can provide here is helpful. I was going to release 0.10.1 but am hesitant if there's a bug
@bf4, Sorry just seeing this. I can take a look at writing specs for this later today. Cursory glance at the cache_test
shows direct invocations of fetch_attributes_fragment
. I'll try to write a test that renders directly from the adapter on a fragmented serializer. Any helpful starting points will be great.
Thanks for the hard work @onomated and @bf4. I can confirm I am facing the same issue with v0.10.1
. Going to try with master.
I just released 0.10.2 which fixes the fragmented cache with read_multi issue https://twitter.com/hazula/status/750394854115450880
@onomated What's needed to consider this issue resolved?
@bf4 Swamped with looming deadlines, but what I did in the time being was to disable caching all together for those models. I can re-enable and verify, but it is a pretty straightforward verification. I rendered json for my models and inspected the fields saved in cache expecting the my except
fields are omitted in the cached result.
As a side note, I use Rubymine and the current AMS specs doesn't play nice, so I have to execute by command line which doesn't quite help my debugging setup etc (yes, I know I'm spoiled), so I couldn't get to writing failing specs before the storm of deadlines hit.
I'll verify 0.10.2 later tonight.
Thanks for the hard work @bf4!
As a side note, I use Rubymine and the current AMS specs doesn't play nice,
would love to know what this entails.
@bf4 Still seeing excluded fields in cache with 0.10.2
. Will pull master and work on a failing spec.
I noticed my cache key: 'custom_key'
are being completely ignored. When i look at my cache keys in redis they are in the format of cache:{plurazied_model_name}/{id}-{updated_at}/attributes/{id}
@rromanchuk probably due to #1642. The object.cache_key
is preferred to the serializer cache key:
option. I personally think the priority should be switched so that we would first try to see if the user has defined the key
option, then fallback to object.cache_key
. @bf4 any thought about this?
@groyoh thanks for the heads up, i haven't dug into the cache mixin enough to actually know what is going on. I had a feeling i should maybe try and override cache_key as a sanity test. If i'm only trying to rename the key what's the best practice? Would something like "my_custom_key/#{object.cache_key}"
be OK?
@rromanchuk I'm not completely sure what you're trying to achieve so I guess that depends on whether you use multiple adapters or not. If you don't then I guess it's fine. Otherwise I would strongly recommend that you override the object_cache_key
method in a superclass to something like.:
class BaseSerializer < ActiveModel::Serializer
def object_cache_key
"#{serializer_class._cache_key}_#{object.cache_key}"
end
end
class MyCachedSerializer < BaseSerializer
cache key: 'custom_key'
end
That would result in the following key "custom_key/#{object.cache_key}/#{adapter_name}"
so that' the cache value is saved using a different key for each adapter.
Disclaimer: I'm just saying that out of my mind. I did not take the time to make sure it actually works properly.
Also if you have further questions, it may be better to open a new issue with a proper description of your troubles.
@groyoh this looks exactly like what i need, i'll let you know how it goes
cache:compact_user_users/
keys look way better!
If we are explicitly overriding the cache key in the serializer I would think that would take precedence. In Rails, every model implementscache_key
from ActiveRecord::Base so #1642 essentially removes the ability to cache the same model in different serializers without something like @groyoh commented.
Also, the current implementation requires the model's cache_key
to be updated whenever anything regarding serialization is changed such as adding a field to the JSON. I don't think this should be the model's concern, the serializer cache key should be able to bust it's own cache without the model caring about how it is serialized.
If the current implementation is really preferred, I would suggest that there is at least some warning when the serializer cache key is ignored. We were just burned by this in production because we added one existing field to the JSON and thought an update to the serializer cache key would fix the issue.
Having one serializer for User and one serializer for UserWithAuthToken is a pretty common use-case for multiple serializers for one model. I would not be surprised if there applications out there that are unknowingly caching UserWithAuthToken and then serving that JSON to other requests that use the normal UserSerializer. You can imagine how bad this would be, but the developer would never know about it unless he inspected the JSON response of his/her own application! Seems dangerous.
I was indeed leaking my authenticated user serializer. 🙈
@lserman sorry to hear that you had trouble with that. I'll try to dig and see if there is a deeper reason to that specific change. The issue right now is that changing this may imply a breaking change so I'll try to see what we can do.
@onomated
I am using 0.10.5
and facing the same problem that excluded fields in cache https://github.com/rails-api/active_model_serializers/issues/1804#issuecomment-230797892.
In my case except:
and only:
option both are not working.
May i know do you solve the problem?
@TONYHOKAN I have since disabled Fragment caching. In my case it actually was less performant than without caching, as noted here. For some of my endpoints, I cache the full response (including headers to retain pagination info) directly in rails cache, for data that doesn't change frequently.
@onomated Thanks for your reply. It seems that i can only disable fragment caching now and keep track of this problem.
Expected vs Actual Behavior
I have a model say
User
that I would like to present in two representations say aUserSerializer
andProfileSerializer
. My serializers are defined as follows:The generated cache keys do not follow the overidden
object_cache_key
and fall-back to the AMS default that is based on the model name. SoProfileSerializer
responses overwrite responses of theUserSerializer
in cache since they both act on the same model.Is there a way to provide a cache prefix or overwrite the default cache key generated in fragment caching? I have tried
cache key: 'some_prefix', skip_digest: true except: [:num_following]
and that also doesn't work.This issue only occurs with fragment caching due to the dynamic creation of Cached vs NonCached variants of the serializer. Works fine without fragment caching.
Steps to reproduce
See examples above
Environment
Rails 4.2.2, Grape 0.16