Skip to content
This repository
Browse code

Change event namespace ordering to most-significant first [#4504 stat…

…e:resolved]

More work still needs to be done on some of these names
(render_template.action_view and render_template!.action_view particularly)
but this allows (for example) /^sql/ to subscribe to all
the various ORMs without further modification

Signed-off-by: José Valim <jose.valim@gmail.com>
  • Loading branch information...
commit 731d4392e478ff5526b595074d9caa999da8bd0c 1 parent 109d3ee
Justin George authored April 27, 2010 josevalim committed May 02, 2010
4  actionmailer/lib/action_mailer/base.rb
@@ -316,7 +316,7 @@ def default(value = nil)
316 316
       #     end
317 317
       #   end
318 318
       def receive(raw_mail)
319  
-        ActiveSupport::Notifications.instrument("action_mailer.receive") do |payload|
  319
+        ActiveSupport::Notifications.instrument("receive.action_mailer") do |payload|
320 320
           mail = Mail.new(raw_mail)
321 321
           set_payload_for_mail(payload, mail)
322 322
           new.receive(mail)
@@ -328,7 +328,7 @@ def receive(raw_mail)
328 328
       # when you call <tt>:deliver</tt> on the Mail::Message, calling +deliver_mail+ directly
329 329
       # and passing a Mail::Message will do nothing except tell the logger you sent the email.
330 330
       def deliver_mail(mail) #:nodoc:
331  
-        ActiveSupport::Notifications.instrument("action_mailer.deliver") do |payload|
  331
+        ActiveSupport::Notifications.instrument("deliver.action_mailer") do |payload|
332 332
           self.set_payload_for_mail(payload, mail)
333 333
           yield # Let Mail do the delivery actions
334 334
         end
2  actionpack/lib/action_controller/caching/fragments.rb
@@ -99,7 +99,7 @@ def expire_fragment(key, options = nil)
99 99
       end
100 100
 
101 101
       def instrument_fragment_cache(name, key)
102  
-        ActiveSupport::Notifications.instrument("action_controller.#{name}", :key => key){ yield }
  102
+        ActiveSupport::Notifications.instrument("#{name}.action_controller", :key => key){ yield }
103 103
       end
104 104
     end
105 105
   end
2  actionpack/lib/action_controller/caching/pages.rb
@@ -109,7 +109,7 @@ def page_cache_path(path)
109 109
           end
110 110
 
111 111
           def instrument_page_cache(name, path)
112  
-            ActiveSupport::Notifications.instrument("action_controller.#{name}", :path => path){ yield }
  112
+            ActiveSupport::Notifications.instrument("#{name}.action_controller", :path => path){ yield }
113 113
           end
114 114
       end
115 115
 
10  actionpack/lib/action_controller/metal/instrumentation.rb
@@ -23,9 +23,9 @@ def process_action(action, *args)
23 23
         :path       => (request.fullpath rescue "unknown")
24 24
       }
25 25
 
26  
-      ActiveSupport::Notifications.instrument("action_controller.start_processing", raw_payload.dup)
  26
+      ActiveSupport::Notifications.instrument("start_processing.action_controller", raw_payload.dup)
27 27
 
28  
-      ActiveSupport::Notifications.instrument("action_controller.process_action", raw_payload) do |payload|
  28
+      ActiveSupport::Notifications.instrument("process_action.action_controller", raw_payload) do |payload|
29 29
         result = super
30 30
         payload[:status] = response.status
31 31
         append_info_to_payload(payload)
@@ -42,20 +42,20 @@ def render(*args)
42 42
     end
43 43
 
44 44
     def send_file(path, options={})
45  
-      ActiveSupport::Notifications.instrument("action_controller.send_file",
  45
+      ActiveSupport::Notifications.instrument("send_file.action_controller",
46 46
         options.merge(:path => path)) do
47 47
         super
48 48
       end
49 49
     end
50 50
 
51 51
     def send_data(data, options = {})
52  
-      ActiveSupport::Notifications.instrument("action_controller.send_data", options) do
  52
+      ActiveSupport::Notifications.instrument("send_data.action_controller", options) do
53 53
         super
54 54
       end
55 55
     end
56 56
 
57 57
     def redirect_to(*args)
58  
-      ActiveSupport::Notifications.instrument("action_controller.redirect_to") do |payload|
  58
+      ActiveSupport::Notifications.instrument("redirect_to.action_controller") do |payload|
59 59
         result = super
60 60
         payload[:status]   = self.status
61 61
         payload[:location] = self.location
8  actionpack/lib/action_controller/test_case.rb
@@ -16,12 +16,12 @@ def setup_subscriptions
16 16
       @templates = Hash.new(0)
17 17
       @layouts = Hash.new(0)
18 18
 
19  
-      ActiveSupport::Notifications.subscribe("action_view.render_template") do |name, start, finish, id, payload|
  19
+      ActiveSupport::Notifications.subscribe("render_template.action_view") do |name, start, finish, id, payload|
20 20
         path = payload[:layout]
21 21
         @layouts[path] += 1
22 22
       end
23 23
 
24  
-      ActiveSupport::Notifications.subscribe("action_view.render_template!") do |name, start, finish, id, payload|
  24
+      ActiveSupport::Notifications.subscribe("!render_template.action_view") do |name, start, finish, id, payload|
25 25
         path = payload[:virtual_path]
26 26
         next unless path
27 27
         partial = path =~ /^.*\/_[^\/]*$/
@@ -36,8 +36,8 @@ def setup_subscriptions
36 36
     end
37 37
 
38 38
     def teardown_subscriptions
39  
-      ActiveSupport::Notifications.unsubscribe("action_view.render_template")
40  
-      ActiveSupport::Notifications.unsubscribe("action_view.render_template!")
  39
+      ActiveSupport::Notifications.unsubscribe("render_template.action_view")
  40
+      ActiveSupport::Notifications.unsubscribe("!render_template.action_view")
41 41
     end
42 42
 
43 43
     # Asserts that the request was rendered with the appropriate template file or partials
4  actionpack/lib/action_view/render/partials.rb
@@ -211,12 +211,12 @@ def render
211 211
         identifier = ((@template = find_template) ? @template.identifier : @path)
212 212
 
213 213
         if @collection
214  
-          ActiveSupport::Notifications.instrument("action_view.render_collection",
  214
+          ActiveSupport::Notifications.instrument("render_collection.action_view",
215 215
             :identifier => identifier || "collection", :count => @collection.size) do
216 216
             render_collection
217 217
           end
218 218
         else
219  
-          content = ActiveSupport::Notifications.instrument("action_view.render_partial",
  219
+          content = ActiveSupport::Notifications.instrument("render_partial.action_view",
220 220
             :identifier => identifier) do
221 221
             render_partial
222 222
           end
2  actionpack/lib/action_view/render/rendering.rb
@@ -52,7 +52,7 @@ def _render_template(template, layout = nil, options = {}) #:nodoc:
52 52
       locals = options[:locals] || {}
53 53
       layout = find_layout(layout) if layout
54 54
 
55  
-      ActiveSupport::Notifications.instrument("action_view.render_template",
  55
+      ActiveSupport::Notifications.instrument("render_template.action_view",
56 56
         :identifier => template.identifier, :layout => layout.try(:virtual_path)) do
57 57
 
58 58
         content = template.render(self, locals) { |*name| _layout_for(*name) }
2  actionpack/lib/action_view/template.rb
@@ -41,7 +41,7 @@ def initialize(source, identifier, handler, details)
41 41
     def render(view, locals, &block)
42 42
       # Notice that we use a bang in this instrumentation because you don't want to
43 43
       # consume this in production. This is only slow if it's being listened to.
44  
-      ActiveSupport::Notifications.instrument("action_view.render_template!", :virtual_path => @virtual_path) do
  44
+      ActiveSupport::Notifications.instrument("!render_template.action_view", :virtual_path => @virtual_path) do
45 45
         method_name = compile(locals, view)
46 46
         view.send(method_name, locals, &block)
47 47
       end
2  activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb
@@ -76,7 +76,7 @@ def columns_with_query_cache(*args)
76 76
         def cache_sql(sql)
77 77
           result =
78 78
             if @query_cache.has_key?(sql)
79  
-              ActiveSupport::Notifications.instrument("active_record.sql", 
  79
+              ActiveSupport::Notifications.instrument("sql.active_record",
80 80
                 :sql => sql, :name => "CACHE", :connection_id => self.object_id)
81 81
               @query_cache[sql]
82 82
             else
2  activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -197,7 +197,7 @@ def current_savepoint_name
197 197
         def log(sql, name)
198 198
           name ||= "SQL"
199 199
           result = nil
200  
-          ActiveSupport::Notifications.instrument("active_record.sql",
  200
+          ActiveSupport::Notifications.instrument("sql.active_record",
201 201
             :sql => sql, :name => name, :connection_id => self.object_id) do
202 202
             @runtime += Benchmark.ms { result = yield }
203 203
           end
2  activeresource/lib/active_resource/connection.rb
@@ -106,7 +106,7 @@ def head(path, headers = {})
106 106
     private
107 107
       # Makes a request to the remote service.
108 108
       def request(method, path, *arguments)
109  
-        result = ActiveSupport::Notifications.instrument("active_resource.request") do |payload|
  109
+        result = ActiveSupport::Notifications.instrument("request.active_resource") do |payload|
110 110
           payload[:method]      = method
111 111
           payload[:request_uri] = "#{site.scheme}://#{site.host}:#{site.port}#{path}"
112 112
           payload[:result]      = http.send(method, path, *arguments)
2  activesupport/lib/active_support/cache.rb
@@ -502,7 +502,7 @@ def instrument(operation, key, options = nil)
502 502
           if self.class.instrument
503 503
             payload = { :key => key }
504 504
             payload.merge!(options) if options.is_a?(Hash)
505  
-            ActiveSupport::Notifications.instrument("active_support.cache_#{operation}", payload){ yield }
  505
+            ActiveSupport::Notifications.instrument("cache_#{operation}.active_support", payload){ yield }
506 506
           else
507 507
             yield
508 508
           end
6  railties/lib/rails/log_subscriber.rb
@@ -22,7 +22,7 @@ module Rails
22 22
   #
23 23
   #   Rails::LogSubscriber.add :active_record, ActiveRecord::Railtie::LogSubscriber.new
24 24
   #
25  
-  # So whenever a "active_record.sql" notification arrive to Rails::LogSubscriber,
  25
+  # So whenever a "sql.active_record" notification arrive to Rails::LogSubscriber,
26 26
   # it will properly dispatch the event (ActiveSupport::Notifications::Event) to
27 27
   # the sql method.
28 28
   #
@@ -54,7 +54,7 @@ def self.add(namespace, log_subscriber, notifier = ActiveSupport::Notifications)
54 54
       log_subscribers << log_subscriber
55 55
 
56 56
       log_subscriber.public_methods(false).each do |event|
57  
-        notifier.subscribe("#{namespace}.#{event}") do |*args|
  57
+        notifier.subscribe("#{event}.#{namespace}") do |*args|
58 58
           next if log_subscriber.logger.nil?
59 59
 
60 60
           begin
@@ -105,4 +105,4 @@ def color(text, color, bold=false)
105 105
       "#{bold}#{color}#{text}#{CLEAR}"
106 106
     end
107 107
   end
108  
-end
  108
+end
2  railties/test/application/initializers/notifications_test.rb
@@ -38,7 +38,7 @@ def wait
38 38
       ActiveRecord::Base.logger = logger = MockLogger.new
39 39
 
40 40
       # Mimic ActiveRecord notifications
41  
-      instrument "active_record.sql", :name => "SQL", :sql => "SHOW tables"
  41
+      instrument "sql.active_record", :name => "SQL", :sql => "SHOW tables"
42 42
       wait
43 43
 
44 44
       assert_equal 1, logger.logged.size
18  railties/test/log_subscriber_test.rb
@@ -61,21 +61,21 @@ def test_does_not_set_color_if_colorize_logging_is_set_to_false
61 61
 
62 62
   def test_event_is_sent_to_the_registered_class
63 63
     Rails::LogSubscriber.add :my_log_subscriber, @log_subscriber
64  
-    instrument "my_log_subscriber.some_event"
  64
+    instrument "some_event.my_log_subscriber"
65 65
     wait
66  
-    assert_equal %w(my_log_subscriber.some_event), @logger.logged(:info)
  66
+    assert_equal %w(some_event.my_log_subscriber), @logger.logged(:info)
67 67
   end
68 68
 
69 69
   def test_event_is_an_active_support_notifications_event
70 70
     Rails::LogSubscriber.add :my_log_subscriber, @log_subscriber
71  
-    instrument "my_log_subscriber.some_event"
  71
+    instrument "some_event.my_log_subscriber"
72 72
     wait
73 73
     assert_kind_of ActiveSupport::Notifications::Event, @log_subscriber.event
74 74
   end
75 75
 
76 76
   def test_does_not_send_the_event_if_it_doesnt_match_the_class
77 77
     Rails::LogSubscriber.add :my_log_subscriber, @log_subscriber
78  
-    instrument "my_log_subscriber.unknown_event"
  78
+    instrument "unknown_event.my_log_subscriber"
79 79
     wait
80 80
     # If we get here, it means that NoMethodError was not raised.
81 81
   end
@@ -84,7 +84,7 @@ def test_does_not_send_the_event_if_logger_is_nil
84 84
     Rails.logger = nil
85 85
     @log_subscriber.expects(:some_event).never
86 86
     Rails::LogSubscriber.add :my_log_subscriber, @log_subscriber
87  
-    instrument "my_log_subscriber.some_event"
  87
+    instrument "some_event.my_log_subscriber"
88 88
     wait
89 89
   end
90 90
 
@@ -110,14 +110,14 @@ def test_flushes_the_same_logger_just_once
110 110
 
111 111
   def test_logging_does_not_die_on_failures
112 112
     Rails::LogSubscriber.add :my_log_subscriber, @log_subscriber
113  
-    instrument "my_log_subscriber.puke"
114  
-    instrument "my_log_subscriber.some_event"
  113
+    instrument "puke.my_log_subscriber"
  114
+    instrument "some_event.my_log_subscriber"
115 115
     wait
116 116
 
117 117
     assert_equal 1, @logger.logged(:info).size
118  
-    assert_equal 'my_log_subscriber.some_event', @logger.logged(:info).last
  118
+    assert_equal 'some_event.my_log_subscriber', @logger.logged(:info).last
119 119
 
120 120
     assert_equal 1, @logger.logged(:error).size
121  
-    assert_equal 'Could not log "my_log_subscriber.puke" event. RuntimeError: puke', @logger.logged(:error).last
  121
+    assert_equal 'Could not log "puke.my_log_subscriber" event. RuntimeError: puke', @logger.logged(:error).last
122 122
   end
123 123
 end

0 notes on commit 731d439

David Chelimsky

FYI - this breaks rspec-rails' monkey patch of AC::TC, so rspec-rails-2.0.0.beta.8 does not support rails from this commit forward. I just addressed this in rspec-rails, and will try to get an rspec-rails beta 9 out soon.

http://github.com/rspec/rspec-rails/commit/ccb61c5868e5670ecb9af50264407dd02226c7ab

Please sign in to comment.
Something went wrong with that request. Please try again.