improve and fix instrumentation (#163)

* only notify about load.cached if docs are actually cached
* add payload (ids, docs) to events

lib/couch_potato/database.rb

@@ -150,7 +150,7 @@ def load_document(id)
       cached = cache && cache[id]
       if cache
         if cached
-          ActiveSupport::Notifications.instrument('couch_potato.load.cached') do
+          ActiveSupport::Notifications.instrument('couch_potato.load.cached', id: id, doc: cached) do
             cached
           end
         else
@@ -168,15 +168,18 @@ def load_documents(ids)
 
       uncached_ids = ids - (cache&.keys || [])
       uncached_docs_by_id = bulk_load(uncached_ids).index_by {|doc| doc.id if doc.respond_to?(:id) }
+      cached_docs_by_id = cache&.slice(*ids) || {}
+      if cached_docs_by_id.any?
+        ActiveSupport::Notifications.instrument('couch_potato.load.cached', ids: cached_docs_by_id.keys, docs: cached_docs_by_id.values) do
+          cached_docs_by_id
+        end
+      end
       if cache
         uncached_ids.each do |id|
           doc = uncached_docs_by_id[id]
           cache[id] = doc if doc
         end
       end
-      cached_docs_by_id = ActiveSupport::Notifications.instrument('couch_potato.load.cached') do
-        cache&.slice(*ids) || {}
-      end
       ids.filter_map { |id| (cached_docs_by_id[id]) || uncached_docs_by_id[id] }
     end
 
@@ -278,9 +281,11 @@ def raw_view(spec)
     def load_document_without_caching(id)
       raise "Can't load a document without an id (got nil)" if id.nil?
 
-      ActiveSupport::Notifications.instrument('couch_potato.load') do
+      payload = {id: id}
+      ActiveSupport::Notifications.instrument('couch_potato.load', payload) do
         instance = couchrest_database.get(id)
         instance.database = self if instance
+        payload[:doc] = instance
         instance
     end
     end
@@ -321,9 +326,11 @@ def save_document_without_conflict_handling(document, validate = true)
     def bulk_load(ids)
       return [] if ids.empty?
 
-      ActiveSupport::Notifications.instrument('couch_potato.load') do
+      payload = {ids: ids}
+      ActiveSupport::Notifications.instrument('couch_potato.load', payload) do
         response = couchrest_database.bulk_load ids
-        docs = response['rows'].map { |row| row['doc'] }.compact
+        docs = response["rows"].map { |row| row["doc"] }.compact
+        payload[:docs] = docs
         docs.each do |doc|
           doc.database = self if doc.respond_to?(:database=)
           doc.database_collection = docs if doc.respond_to?(:database_collection=)

lib/couch_potato/persistence/ghost_attributes.rb

@@ -1,12 +1,16 @@
 module CouchPotato
-  module GhostAttributes #:nodoc:
-    def method_missing(name, *args)
-      if(value = _document && _document[name.to_s])
+  module GhostAttributes # :nodoc:
+    def method_missing(name, *)
+      if (value = _document && _document[name.to_s])
         value
       else
         super
       end
     end
+
+    def respond_to_missing?(name, *)
+      _document && _document[name.to_s]
+    end
   end
 end
 

spec/unit/caching_spec.rb

@@ -18,6 +18,10 @@
     {}
   end
 
+  after(:each) do
+    ActiveSupport::Notifications.unsubscribe(@subscriber) if @subscriber
+  end
+
   context 'for a single document' do
     it 'gets an object from the cache the 2nd time via #load_documemt' do
       expect(couchrest_db).to receive(:get).with('1').exactly(1).times
@@ -47,6 +51,29 @@
       db.load_document '1'
       expect(db.load_document('1')).to eql(doc)
     end
+
+    it 'instruments the load call' do
+      doc = double("doc").as_null_object
+      allow(couchrest_db).to receive(:get).and_return(doc)
+      events = []
+      @subscriber = ActiveSupport::Notifications.subscribe(
+        'couch_potato.load.cached'
+      ) do |event|
+        events << event
+      end
+
+      db.load("1")
+      db.load("1")
+
+      expect(events.size).to eq(1)
+      expect(events.first.payload).to eq(
+        {
+          id: "1",
+          doc: doc
+        }
+      )
+
+    end
   end
 
   context 'for multiple documents' do
@@ -65,6 +92,31 @@
       expect(couchrest_db).to have_received(:bulk_load).with(['2']).exactly(1).times
     end
 
+    it 'instruments the load call' do
+      allow(couchrest_db).to receive(:bulk_load).with(['1'])
+        .and_return('rows' => [{'doc' => doc1}])
+      allow(couchrest_db).to receive(:bulk_load).with(['2'])
+        .and_return('rows' => [{'doc' => doc2}])
+      events = []
+      @subscriber = ActiveSupport::Notifications.subscribe(
+        'couch_potato.load.cached'
+      ) do |event|
+        events << event
+      end
+
+
+      db.load_document(['1'])
+      db.load_document(['1', '2'])
+
+      expect(events.size).to eq(1)
+      expect(events.first.payload).to eq(
+        {
+          ids: ["1"],
+          docs: [doc1]
+        }
+      )
+    end
+
     it 'loads nothing if all documents are cached' do
       allow(couchrest_db).to receive(:bulk_load).with(['1', '2'])
         .and_return('rows' => [{'doc' => doc1}, {'doc' => doc2}])
@@ -94,7 +146,7 @@
         'id' => '2',
       }
       allow(couchrest_db).to receive(:bulk_load).with(['1', '2'])
-        .and_return('rows' => [{'doc' => doc1}, {'doc' => doc1}])
+        .and_return('rows' => [{'doc' => doc1}, {'doc' => doc2}])
 
       db.load_document(['1', '2'])
       db.load_document(['1', '2'])

spec/unit/database_spec.rb

@@ -42,6 +42,11 @@ class Child
   let(:couchrest_db) { double('couchrest db', info: nil).as_null_object }
   let(:db) { CouchPotato::Database.new couchrest_db }
 
+
+  after(:each) do
+    ActiveSupport::Notifications.unsubscribe(@subscriber) if @subscriber
+  end
+
   it 'should raise an exception if nil given' do
     expect do
       db.load nil
@@ -130,6 +135,46 @@ class Child
     it 'returns an empty array when passing an empty array' do
       expect(db.load([])).to eq([])
     end
+
+    it 'instruments the load call' do
+      events = []
+      @subscriber = ActiveSupport::Notifications.subscribe(
+        'couch_potato.load'
+      ) do |event|
+        events << event
+      end
+
+      db.load(["1", "2"])
+
+      expect(events.size).to eq(1)
+      expect(events.first.payload).to eq(
+        {
+          ids: ["1", "2"],
+          docs: [doc1, doc2]
+        }
+      )
+    end
+  end
+
+  it 'instruments the load call' do
+    doc = double("doc").as_null_object
+    allow(couchrest_db).to receive(:get).and_return(doc)
+    events = []
+    @subscriber = ActiveSupport::Notifications.subscribe(
+      'couch_potato.load'
+    ) do |event|
+      events << event
+    end
+
+    db.load("1")
+
+    expect(events.size).to eq(1)
+    expect(events.first.payload).to eq(
+      {
+        id: "1",
+        doc: doc
+      }
+    )
   end
 end