Merge pull request #188 from leoarnold/leoarnold/meta_request/fragment-caching

Prevent spurious SQL query execution via ActiveSupport::Cache events

Author: dejan

meta_request/.rubocop.yml

@@ -1 +1,5 @@
 inherit_from: .rubocop_todo.yml
+
+Metrics/BlockLength:
+  ExcludedMethods:
+    - describe

meta_request/lib/meta_request/event.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require 'active_support'
+require 'active_support/cache'
 require 'active_support/json'
 require 'active_support/core_ext'
 
@@ -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
@@ -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)
@@ -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) &&

meta_request/spec/meta_request/event_spec.rb

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