Merge pull request #694 from bugsnag/improve-metadata-api

Improve metadata API in Report/Event

CHANGELOG.md

@@ -24,6 +24,8 @@ Changelog
   | [#692](https://github.com/bugsnag/bugsnag-ruby/pull/692)
 * Add `request` to `Report`/`Event` containing HTTP request metadata
   | [#693](https://github.com/bugsnag/bugsnag-ruby/pull/693)
+* Add `add_metadata` and `clear_metadata` to `Report`/`Event`
+  | [#694](https://github.com/bugsnag/bugsnag-ruby/pull/694)
 
 ### Fixes
 
@@ -46,6 +48,7 @@ Changelog
 * `Report#exceptions` has been deprecated in favour of the new `errors` property
 * `Report#raw_exceptions` has been deprecated in favour of the new `original_error` property
 * Accessing request data via `Report#metadata` has been deprecated in favour of using the new `request` property. Request data will be moved out of metadata in the next major version
+* The `Report#add_tab` and `Report#remove_tab` methods have been deprecated in favour of the new `add_metadata` and `clear_metadata` methods
 
 ## v6.22.1 (11 August 2021)
 

lib/bugsnag.rb

@@ -34,6 +34,8 @@
 require "bugsnag/breadcrumbs/breadcrumb"
 require "bugsnag/breadcrumbs/breadcrumbs"
 
+require "bugsnag/utility/metadata_delegate"
+
 # rubocop:todo Metrics/ModuleLength
 module Bugsnag
   LOCK = Mutex.new

lib/bugsnag/report.rb

@@ -143,6 +143,8 @@ def initialize(exception, passed_configuration, auto_notify=false)
       self.severity = auto_notify ? "error" : "warning"
       self.severity_reason = auto_notify ? {:type => UNHANDLED_EXCEPTION} : {:type => HANDLED_EXCEPTION}
       self.user = {}
+
+      @metadata_delegate = Utility::MetadataDelegate.new
     end
 
     ##
@@ -172,6 +174,8 @@ def context
     #   exists, this will be merged with the existing values. If a Hash is not
     #   given, the value will be placed into the 'custom' tab
     # @return [void]
+    #
+    # @deprecated Use {#add_metadata} instead
     def add_tab(name, value)
       return if name.nil?
 
@@ -190,6 +194,8 @@ def add_tab(name, value)
     #
     # @param name [String]
     # @return [void]
+    #
+    # @deprecated Use {#clear_metadata} instead
     def remove_tab(name)
       return if name.nil?
 
@@ -316,6 +322,43 @@ def request
       @meta_data[:request]
     end
 
+    ##
+    # Add values to metadata
+    #
+    # @overload add_metadata(section, data)
+    #   Merges data into the given section of metadata
+    #   @param section [String, Symbol]
+    #   @param data [Hash]
+    #
+    # @overload add_metadata(section, key, value)
+    #   Sets key to value in the given section of metadata. If the value is nil
+    #   the key will be deleted
+    #   @param section [String, Symbol]
+    #   @param key [String, Symbol]
+    #   @param value
+    #
+    # @return [void]
+    def add_metadata(section, key_or_data, *args)
+      @metadata_delegate.add_metadata(@meta_data, section, key_or_data, *args)
+    end
+
+    ##
+    # Clear values from metadata
+    #
+    # @overload clear_metadata(section)
+    #   Clears the given section of metadata
+    #   @param section [String, Symbol]
+    #
+    # @overload clear_metadata(section, key)
+    #   Clears the key in the given section of metadata
+    #   @param section [String, Symbol]
+    #   @param key [String, Symbol]
+    #
+    # @return [void]
+    def clear_metadata(section, *args)
+      @metadata_delegate.clear_metadata(@meta_data, section, *args)
+    end
+
     private
 
     def generate_exception_list

lib/bugsnag/utility/metadata_delegate.rb

@@ -0,0 +1,102 @@
+module Bugsnag::Utility
+  # @api private
+  class MetadataDelegate
+    # nil is a valid metadata value, so we need a sentinel object so we can tell
+    # if the value parameter has been provided
+    NOT_PROVIDED = Object.new
+
+    ##
+    # Add values to metadata
+    #
+    # @overload add_metadata(metadata, section, data)
+    #   Merges data into the given section of metadata
+    #   @param metadata [Hash] The metadata hash to operate on
+    #   @param section [String, Symbol]
+    #   @param data [Hash]
+    #
+    # @overload add_metadata(metadata, section, key, value)
+    #   Sets key to value in the given section of metadata. If the value is nil
+    #   the key will be deleted
+    #   @param metadata [Hash] The metadata hash to operate on
+    #   @param section [String, Symbol]
+    #   @param key [String, Symbol]
+    #   @param value
+    #
+    # @return [void]
+    def add_metadata(metadata, section, key_or_data, value = NOT_PROVIDED)
+      case value
+      when NOT_PROVIDED
+        merge_metadata(metadata, section, key_or_data)
+      when nil
+        clear_metadata(metadata, section, key_or_data)
+      else
+        overwrite_metadata(metadata, section, key_or_data, value)
+      end
+    end
+
+    ##
+    # Clear values from metadata
+    #
+    # @overload clear_metadata(metadata, section)
+    #   Clears the given section of metadata
+    #   @param metadata [Hash] The metadata hash to operate on
+    #   @param section [String, Symbol]
+    #
+    # @overload clear_metadata(metadata, section, key)
+    #   Clears the key in the given section of metadata
+    #   @param metadata [Hash] The metadata hash to operate on
+    #   @param section [String, Symbol]
+    #   @param key [String, Symbol]
+    #
+    # @return [void]
+    def clear_metadata(metadata, section, key = nil)
+      if key.nil?
+        metadata.delete(section)
+      elsif metadata[section]
+        metadata[section].delete(key)
+      end
+    end
+
+    private
+
+    ##
+    # Merge new metadata into the existing metadata
+    #
+    # Any keys with a 'nil' value in the new metadata will be deleted from the
+    # existing metadata
+    #
+    # @param existing_metadata [Hash]
+    # @param section [String, Symbol]
+    # @param new_metadata [Hash]
+    # @return [void]
+    def merge_metadata(existing_metadata, section, new_metadata)
+      return unless new_metadata.is_a?(Hash)
+
+      existing_metadata[section] ||= {}
+      data = existing_metadata[section]
+
+      new_metadata.each do |key, value|
+        if value.nil?
+          data.delete(key)
+        else
+          data[key] = value
+        end
+      end
+    end
+
+    ##
+    # Overwrite the value in metadata's section & key
+    #
+    # @param metadata [Hash]
+    # @param section [String, Symbol]
+    # @param key [String, Symbol]
+    # @param value
+    # @return [void]
+    def overwrite_metadata(metadata, section, key, value)
+      return unless key.is_a?(String) || key.is_a?(Symbol)
+
+      metadata[section] ||= {}
+      metadata[section][key] = value
+    end
+  end
+end

spec/report_spec.rb

@@ -2,6 +2,7 @@
 require_relative './spec_helper'
 require 'securerandom'
 require 'ostruct'
+require 'support/shared_examples_for_metadata'
 
 module ActiveRecord; class RecordNotFound < RuntimeError; end; end
 class NestedException < StandardError; attr_accessor :original_exception; end
@@ -28,6 +29,24 @@ def gloops
 end
 
 shared_examples "Report or Event tests" do |class_to_test|
+  context "metadata" do
+    include_examples(
+      "metadata delegate",
+      lambda do |metadata, *args|
+        report = class_to_test.new(RuntimeError.new, Bugsnag.configuration)
+        report.metadata = metadata
+
+        report.add_metadata(*args)
+      end,
+      lambda do |metadata, *args|
+        report = class_to_test.new(RuntimeError.new, Bugsnag.configuration)
+        report.metadata = metadata
+
+        report.clear_metadata(*args)
+      end
+    )
+  end
+
   it "#headers should return the correct request headers" do
     fake_now = Time.gm(2020, 1, 2, 3, 4, 5, 123456)
     expect(Time).to receive(:now).twice.and_return(fake_now)
@@ -349,7 +368,53 @@ def gloops
     end
   end
 
-  # TODO: nested context
+  it "metadata added with 'add_metadata' ends up in the payload" do
+    Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report|
+      report.add_metadata(
+        :some_tab,
+        { info: "here", data: "also here" }
+      )
+
+      report.add_metadata(:some_other_tab, :info, true)
+      report.add_metadata(:some_other_tab, :data, "very true")
+    end
+
+    expect(Bugsnag).to(have_sent_notification { |payload, _headers|
+      event = get_event_from_payload(payload)
+      expect(event["metaData"]).to eq({
+        "some_tab" => {
+          "info" => "here",
+          "data" => "also here"
+        },
+        "some_other_tab" => {
+          "info" => true,
+          "data" => "very true"
+        }
+      })
+    })
+  end
+
+  it "metadata removed with 'clear_metadata' does not end up in the payload" do
+    Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report|
+      report.add_metadata(
+        :some_tab,
+        { info: "here", data: "also here" }
+      )
+
+      report.add_metadata(:some_other_tab, :info, true)
+      report.add_metadata(:some_other_tab, :data, "very true")
+
+      report.clear_metadata(:some_tab)
+      report.clear_metadata(:some_other_tab, :info)
+    end
+
+    expect(Bugsnag).to(have_sent_notification { |payload, _headers|
+      event = get_event_from_payload(payload)
+      expect(event["metaData"]).to eq({
+        "some_other_tab" => { "data" => "very true" }
+      })
+    })
+  end
 
   it "accepts tabs in overrides and adds them to metaData" do
     Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report|