Skip to content

Commit

Permalink
Merge pull request #188 from leoarnold/leoarnold/meta_request/fragmen…
Browse files Browse the repository at this point in the history
…t-caching

Prevent spurious SQL query execution via ActiveSupport::Cache events
  • Loading branch information
dejan authored Apr 12, 2024
2 parents 7ecc156 + 554626d commit 0b2eaf7
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 1 deletion.
4 changes: 4 additions & 0 deletions meta_request/.rubocop.yml
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
inherit_from: .rubocop_todo.yml

Metrics/BlockLength:
ExcludedMethods:
- describe
12 changes: 11 additions & 1 deletion meta_request/lib/meta_request/event.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# frozen_string_literal: true

require 'active_support'
require 'active_support/cache'
require 'active_support/json'
require 'active_support/core_ext'

Expand All @@ -13,6 +14,7 @@ class Event < ActiveSupport::Notifications::Event
attr_reader :duration

def initialize(name, start, ending, transaction_id, payload)
@name = name
super(name, start, ending, transaction_id, json_encodable(payload))
@duration = 1000.0 * (ending - start)
end
Expand All @@ -37,7 +39,7 @@ def self.events_for_exception(exception_wrapper)
def json_encodable(payload)
return {} unless payload.is_a?(Hash)

transform_hash(payload, deep: true) do |hash, key, value|
transform_hash(sanitize_hash(payload), deep: true) do |hash, key, value|
if value.class.to_s == 'ActionDispatch::Http::Headers'
value = value.to_h.select { |k, _| k.upcase == k }
elsif not_encodable?(value)
Expand All @@ -54,6 +56,14 @@ def json_encodable(payload)
end.with_indifferent_access
end

def sanitize_hash(payload)
if @name =~ /\Acache_\w+\.active_support\z/
payload[:key] = ActiveSupport::Cache::Store.new.send(:normalize_key, payload[:key])
end

payload
end

def not_encodable?(value)
(defined?(ActiveRecord) && value.is_a?(ActiveRecord::ConnectionAdapters::AbstractAdapter)) ||
(defined?(ActionDispatch) &&
Expand Down
49 changes: 49 additions & 0 deletions meta_request/spec/meta_request/event_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# frozen_string_literal: true

require 'spec_helper'

RSpec.describe MetaRequest::Event do
describe '.new' do
context 'when transforming an event from ActiveSupport::Cache' do
let(:active_support_cache_events) do
%w[
cache_read.active_support
cache_write.active_support
]
end

let(:payload) do
{
key: [
'views',
'users/show:118c2da025706846afc6874e76b33a5c',
active_record_relation
]
}
end

let(:active_record_relation) do
double('ActiveRecord::Relation', cache_key: 'users/query-e06f359ccb226f3021b50c0c7e457f79')
end

let(:full_cache_key) do
'views/users/show:118c2da025706846afc6874e76b33a5c/users/query-e06f359ccb226f3021b50c0c7e457f79'
end

it 'replaces the cache key with its String representation and does not trigger SQL queries' do
# This would result in an SQL query being executed
expect(active_record_relation).to_not receive(:as_json)

event = described_class.new(
active_support_cache_events.sample,
Time.parse('2023-08-02 23:10:00.759479575 +0200'),
Time.parse('2023-08-02 23:10:00.761230028 +0200'),
SecureRandom.hex(10),
payload
)

expect(event.payload[:key]).to eq(full_cache_key)
end
end
end
end

0 comments on commit 0b2eaf7

Please sign in to comment.