Merge pull request #561 from nateberkopec/refactor

Refactor

Author: nateberkopec

.rubocop.yml

@@ -4,7 +4,7 @@ AllCops:
     - 'lib/raven/utils/deep_merge.rb'
 
 Metrics/ClassLength:
-  Max: 230
+  Max: 231
   CountComments: false
 
 Metrics/AbcSize:

lib/raven/event.rb

@@ -23,19 +23,17 @@ class Event
     PLATFORM = "ruby".freeze
     SDK = { "name" => "sentry-raven", "version" => Raven::VERSION }.freeze
 
-    attr_reader :id
-    attr_accessor :message, :timestamp, :time_spent, :level, :logger,
+    attr_accessor :id, :timestamp, :time_spent, :level, :logger,
                   :culprit, :server_name, :release, :modules, :extra, :tags,
                   :context, :configuration, :checksum, :fingerprint, :environment,
-                  :os, :runtime, :breadcrumbs
+                  :os, :runtime, :breadcrumbs, :user, :backtrace
 
     def initialize(init = {})
       @configuration = Raven.configuration
       @interfaces    = {}
       @breadcrumbs   = Raven.breadcrumbs
       @context       = Raven.context
       @id            = SecureRandom.uuid.delete("-")
-      @message       = nil
       @timestamp     = Time.now.utc
       @time_spent    = nil
       @level         = :error
@@ -44,11 +42,11 @@ def initialize(init = {})
       @server_name   = @configuration.server_name
       @release       = @configuration.release
       @modules       = list_gem_specs if @configuration.send_modules
-      @user          = {}
-      @extra         = {}
-      @os            = {}
-      @runtime       = {}
-      @tags          = {}
+      @user          = {} # TODO: contexts
+      @extra         = {} # TODO: contexts
+      @os            = {} # TODO: contexts
+      @runtime       = {} # TODO: contexts
+      @tags          = {} # TODO: contexts
       @checksum      = nil
       @fingerprint   = nil
       @environment   = @configuration.current_environment
@@ -61,24 +59,36 @@ def initialize(init = {})
         end
       end
 
-      if @context.rack_env
+      if @context.rack_env # TODO: contexts
         @context.user[:ip_address] = calculate_real_ip_from_rack
       end
 
-      init.each_pair { |key, val| instance_variable_set('@' + key.to_s, val) }
+      init.each_pair { |key, val| public_send(key.to_s + "=", val) }
 
-      @user = @context.user.merge(@user)
-      @extra = @context.extra.merge(@extra)
-      @tags = @configuration.tags.merge(@context.tags).merge(@tags)
-      @os = @context.os
-      @runtime = @context.runtime
+      @user = @context.user.merge(@user) # TODO: contexts
+      @extra = @context.extra.merge(@extra) # TODO: contexts
+      @tags = @configuration.tags.merge(@context.tags).merge(@tags) # TODO: contexts
+      @os = @context.os # TODO: contexts
+      @runtime = @context.runtime # TODO: contexts
 
       # Some type coercion
       @timestamp  = @timestamp.strftime('%Y-%m-%dT%H:%M:%S') if @timestamp.is_a?(Time)
       @time_spent = (@time_spent * 1000).to_i if @time_spent.is_a?(Float)
       @level      = LOG_LEVELS[@level.to_s.downcase] if @level.is_a?(String) || @level.is_a?(Symbol)
     end
 
+    def message
+      @interfaces[:logentry] && @interfaces[:logentry].unformatted_message
+    end
+
+    def message=(args)
+      message, params = *args
+      interface(:message) do |int|
+        int.message = message
+        int.params = params
+      end
+    end
+
     class << self
       def from_exception(exc, options = {}, &block)
         exception_context = get_exception_context(exc) || {}
@@ -112,11 +122,8 @@ def from_message(message, options = {})
 
         new(options) do |evt|
           evt.configuration = configuration
-          evt.message = message
           evt.level = options[:level] || :error
-          evt.interface :message do |int|
-            int.message = message
-          end
+          evt.message = message, options[:message_params] || []
           if options[:backtrace]
             evt.interface(:stacktrace) do |int|
               stacktrace_interface_from(int, evt, options[:backtrace])
@@ -201,10 +208,10 @@ def list_gem_specs
     end
 
     def interface(name, value = nil, &block)
-      int = Raven.find_interface(name)
+      int = Interface.registered[name]
       raise(Error, "Unknown interface: #{name}") unless int
-      @interfaces[int.name] = int.new(value, &block) if value || block
-      @interfaces[int.name]
+      @interfaces[int.sentry_alias] = int.new(value, &block) if value || block
+      @interfaces[int.sentry_alias]
     end
 
     def [](key)
@@ -218,7 +225,6 @@ def []=(key, value)
     def to_hash
       data = {
         :event_id => @id,
-        :message => @message,
         :timestamp => @timestamp,
         :time_spent => @time_spent,
         :level => @level,
@@ -227,6 +233,7 @@ def to_hash
         :os => @os,
         :runtime => @runtime
       }
+
       data[:logger] = @logger if @logger
       data[:culprit] = @culprit if @culprit
       data[:server_name] = @server_name if @server_name
@@ -239,9 +246,11 @@ def to_hash
       data[:user] = @user if @user
       data[:breadcrumbs] = @breadcrumbs.to_hash unless @breadcrumbs.empty?
       data[:checksum] = @checksum if @checksum
+
       @interfaces.each_pair do |name, int_data|
         data[name.to_sym] = int_data.to_hash
       end
+      data[:message] = message
       data
     end
 

lib/raven/interfaces.rb renamed to lib/raven/interface.rb

@@ -1,6 +1,4 @@
 module Raven
-  INTERFACES = {} # rubocop:disable Style/MutableConstant
-
   class Interface
     def initialize(attributes = nil)
       attributes.each do |attr, value|
@@ -10,23 +8,18 @@ def initialize(attributes = nil)
       yield self if block_given?
     end
 
-    def self.name(value = nil)
-      @interface_name ||= value
+    def self.inherited(klass)
+      name = klass.name.split("::").last.downcase.gsub("interface", "")
+      registered[name.to_sym] = klass
+      super
     end
 
-    def to_hash
-      Hash[instance_variables.map { |name| [name[1..-1].to_sym, instance_variable_get(name)] }]
+    def self.registered
+      @@registered ||= {} # rubocop:disable Style/ClassVars
     end
-  end
 
-  def self.register_interface(mapping)
-    mapping.each_pair do |key, klass|
-      INTERFACES[key.to_s] = klass
-      INTERFACES[klass.name] = klass
+    def to_hash
+      Hash[instance_variables.map { |name| [name[1..-1].to_sym, instance_variable_get(name)] }]
     end
   end
-
-  def self.find_interface(name)
-    INTERFACES[name.to_s]
-  end
 end

lib/raven/interfaces/exception.rb

@@ -1,16 +1,17 @@
-require 'raven/interfaces'
+require 'raven/interface'
 
 module Raven
   class ExceptionInterface < Interface
-    name 'exception'
     attr_accessor :values
 
+    def self.sentry_alias
+      :exception
+    end
+
     def to_hash(*args)
       data = super(*args)
       data[:values] = data[:values].map(&:to_hash) if data[:values]
       data
     end
   end
-
-  register_interface :exception => ExceptionInterface
 end

lib/raven/interfaces/http.rb

@@ -1,23 +1,18 @@
-require 'raven/interfaces'
+require 'raven/interface'
 
 module Raven
   class HttpInterface < Interface
-    name 'request'
-    attr_accessor :url
-    attr_accessor :method
-    attr_accessor :data
-    attr_accessor :query_string
-    attr_accessor :cookies
-    attr_accessor :headers
-    attr_accessor :env
+    attr_accessor :url, :method, :data, :query_string, :cookies, :headers, :env
 
     def initialize(*arguments)
       self.headers = {}
       self.env = {}
       self.cookies = nil
       super(*arguments)
     end
-  end
 
-  register_interface :http => HttpInterface
+    def self.sentry_alias
+      :request
+    end
+  end
 end

lib/raven/interfaces/message.rb

@@ -1,16 +1,20 @@
-require 'raven/interfaces'
+require 'raven/interface'
 
 module Raven
   class MessageInterface < Interface
-    name 'sentry.interfaces.Message'
-    attr_accessor :message
-    attr_accessor :params
+    attr_accessor :message, :params
 
     def initialize(*arguments)
       self.params = []
       super(*arguments)
     end
-  end
 
-  register_interface :message => MessageInterface
+    def unformatted_message
+      message % params
+    end
+
+    def self.sentry_alias
+      :logentry
+    end
+  end
 end

lib/raven/interfaces/single_exception.rb

@@ -1,4 +1,4 @@
-require 'raven/interfaces'
+require 'raven/interface'
 
 module Raven
   class SingleExceptionInterface < Interface

lib/raven/interfaces/stack_trace.rb

@@ -1,15 +1,18 @@
-require 'raven/interfaces'
+require 'raven/interface'
 
 module Raven
   class StacktraceInterface < Interface
-    name 'stacktrace'
     attr_accessor :frames
 
     def initialize(*arguments)
       self.frames = []
       super(*arguments)
     end
 
+    def self.sentry_alias
+      :stacktrace
+    end
+
     def to_hash(*args)
       data = super(*args)
       data[:frames] = data[:frames].map(&:to_hash)
@@ -71,6 +74,4 @@ def to_hash(*args)
       end
     end
   end
-
-  register_interface :stack_trace => StacktraceInterface
 end

spec/raven/interface_spec.rb

@@ -0,0 +1,26 @@
+require 'spec_helper'
+
+class TestInterface < Raven::Interface
+  attr_accessor :some_attr
+end
+
+describe Raven::Interface do
+  it "should register an interface when a new class is defined" do
+    expect(Raven::Interface.registered[:test]).to eq(TestInterface)
+  end
+
+  it "can be initialized with some attributes" do
+    interface = TestInterface.new(:some_attr => "test")
+    expect(interface.some_attr).to eq("test")
+  end
+
+  it "can initialize with a block" do
+    interface = TestInterface.new { |int| int.some_attr = "test" }
+    expect(interface.some_attr).to eq("test")
+  end
+
+  it "serializes to a Hash" do
+    interface = TestInterface.new(:some_attr => "test")
+    expect(interface.to_hash).to eq(:some_attr => "test")
+  end
+end