Use HashWithIndifferentAccess for event payloads, tags, and context

Author: adrianna-chang-shopify

activesupport/lib/active_support/event_reporter.rb

@@ -1,11 +1,12 @@
 # typed: true
 # frozen_string_literal: true
 
+require "active_support/core_ext/hash/indifferent_access"
 require_relative "event_reporter/encoders"
 
 module ActiveSupport
   class TagStack # :nodoc:
-    EMPTY_TAGS = {}.freeze
+    EMPTY_TAGS = {}.with_indifferent_access.freeze
     FIBER_KEY = :event_reporter_tags
 
     class << self
@@ -29,24 +30,27 @@ def with_tags(*args, **kwargs)
 
       private
         def resolve_tags(args, kwargs)
-          tags = args.each_with_object({}) do |arg, tags|
+          tags = {}.with_indifferent_access
+
+          args.each do |arg|
             case arg
             when String, Symbol
-              tags[arg.to_sym] = true
+              tags[arg] = true
             when Hash
-              arg.each { |key, value| tags[key.to_sym] = value }
+              arg.each { |key, value| tags[key] = value }
             else
-              tags[arg.class.name.to_sym] = arg
+              tags[arg.class.name] = arg
             end
           end
-          kwargs.each { |key, value| tags[key.to_sym] = value }
+
+          kwargs.each { |key, value| tags[key] = value }
           tags
         end
     end
   end
 
   class EventContext # :nodoc:
-    EMPTY_CONTEXT = {}.freeze
+    EMPTY_CONTEXT = {}.with_indifferent_access.freeze
     FIBER_KEY = :event_reporter_context
 
     class << self
@@ -56,7 +60,7 @@ def context
 
       def set_context(context_hash)
         new_context = self.context.dup
-        context_hash.each { |key, value| new_context[key.to_sym] = value }
+        context_hash.each { |key, value| new_context[key] = value }
 
         Fiber[FIBER_KEY] = new_context.freeze
       end
@@ -265,6 +269,8 @@ def subscribe(subscriber)
     #     #    source_location: { filepath: "path/to/file.rb", lineno: 123, label: "UserService#create" }
     #     #  }
     #
+    # Note that the payload is converted to a HashWithIndifferentAccess, so keys can be accessed as strings or symbols.
+    #
     # Alternatively, an event object can be provided:
     #
     #   Rails.event.notify(UserCreatedEvent.new(id: 123))
@@ -395,6 +401,8 @@ def debug(name_or_object, payload = nil, caller_depth: 1, **kwargs)
     #   #    source_location: { filepath: "path/to/file.rb", lineno: 123, label: "UserService#create" }
     #   #  }
     #
+    #  Note that tags are converted to a HashWithIndifferentAccess, so keys can be accessed as strings or symbols.
+    #
     # The +tagged+ API can also receive a tag object:
     #
     #   graphql_tag = GraphqlTag.new(operation_name: "user_created", operation_type: "mutation")
@@ -465,9 +473,9 @@ def resolve_payload(name_or_object, payload, **kwargs)
         when String, Symbol
           handle_unexpected_args(name_or_object, payload, kwargs) if payload && kwargs.any?
           if kwargs.any?
-            kwargs
+            kwargs.with_indifferent_access
           elsif payload
-            payload
+            payload.with_indifferent_access
           end
         else
           handle_unexpected_args(name_or_object, payload, kwargs) if payload || kwargs.any?

activesupport/lib/active_support/event_reporter/test_helper.rb

@@ -16,17 +16,33 @@ def emit(event)
   def event_matcher(name:, payload: nil, tags: {}, context: {}, source_location: nil)
     ->(event) {
       return false unless event[:name] == name
-      return false unless event[:payload] == payload
-      return false unless event[:tags] == tags
-      return false unless event[:context] == context
+      return false unless hash_matches?(event[:payload], payload)
+      return false unless hash_matches?(event[:tags], tags)
+      return false unless hash_matches?(event[:context], context)
 
       if source_location
-        return false unless event[:source_location][:filepath] == source_location[:filepath] if source_location[:filepath]
-        return false unless event[:source_location][:lineno] == source_location[:lineno] if source_location[:lineno]
-        return false unless event[:source_location][:label] == source_location[:label] if source_location[:label]
+        [:filepath, :lineno, :label].each do |key|
+          return false unless event[:source_location][key] == source_location[key] if source_location[key]
+        end
       end
 
       true
     }
   end
+
+  private
+    def hash_matches?(actual, expected)
+      return true if actual.nil? && expected.nil?
+      return false if actual.nil? || expected.nil?
+
+      return actual == expected unless actual.is_a?(Hash) && expected.is_a?(Hash)
+
+      return false unless actual.size == expected.size
+
+      expected.each do |key, value|
+        return false unless actual[key] == value
+      end
+
+      true
+    end
 end

activesupport/test/event_reporter_test.rb

@@ -447,24 +447,20 @@ class ContextStoreTest < ActiveSupport::TestCase
 
     test "#set_context sets context data" do
       @reporter.set_context(shop_id: 123)
-      assert_equal({ shop_id: 123 }, @reporter.context)
+      assert_equal(123, @reporter.context[:shop_id])
     end
 
     test "#set_context merges with existing context" do
       @reporter.set_context(shop_id: 123)
       @reporter.set_context(user_id: 456)
-      assert_equal({ shop_id: 123, user_id: 456 }, @reporter.context)
+      assert_equal(123, @reporter.context[:shop_id])
+      assert_equal(456, @reporter.context[:user_id])
     end
 
     test "#set_context overwrites existing keys" do
       @reporter.set_context(shop_id: 123)
       @reporter.set_context(shop_id: 456)
-      assert_equal({ shop_id: 456 }, @reporter.context)
-    end
-
-    test "#set_context with string keys converts them to symbols" do
-      @reporter.set_context("shop_id" => 123)
-      assert_equal({ shop_id: 123 }, @reporter.context)
+      assert_equal(456, @reporter.context[:shop_id])
     end
 
     test "#clear_context removes all context data" do
@@ -525,7 +521,7 @@ class ContextStoreTest < ActiveSupport::TestCase
       @reporter.set_context(shop_id: 123)
 
       @reporter.tagged(request_id: "abc") do
-        assert_equal({ shop_id: 123 }, @reporter.context)
+        assert_equal(123, @reporter.context[:shop_id])
 
         assert_called_with(@subscriber, :emit, [
           event_matcher(name: "test_event", payload: { key: "value" }, tags: { request_id: "abc" }, context: { shop_id: 123 })