Merge pull request #408 from projecthydra-labs/callbacks

Implements a callback api

curation_concerns-models/app/actors/curation_concerns/file_set_actor.rb

@@ -73,17 +73,15 @@ def revert_content(revision_id)
       working_file = copy_repository_resource_to_working_directory(file_set)
       make_derivative(file_set.id, working_file)
 
-      return true unless CurationConcerns.config.respond_to?(:after_revert_content)
-      CurationConcerns.config.after_revert_content.call(file_set, user, revision_id)
+      CurationConcerns.config.callback.run(:after_revert_content, file_set, user, revision_id)
       true
     end
 
     def update_content(file)
       working_file = copy_file_to_working_directory(file, file_set.id)
       IngestFileJob.perform_later(file_set.id, working_file, file.content_type, user.user_key)
       make_derivative(file_set.id, working_file)
-      return true unless CurationConcerns.config.respond_to?(:after_update_content)
-      CurationConcerns.config.after_update_content.call(file_set, user)
+      CurationConcerns.config.callback.run(:after_update_content, file_set, user)
       true
     end
 
@@ -93,16 +91,14 @@ def update_metadata(model_attributes, all_attributes)
       file_set.attributes = model_attributes
       file_set.date_modified = CurationConcerns::TimeService.time_in_utc
       save do
-        if CurationConcerns.config.respond_to?(:after_update_metadata)
-          CurationConcerns.config.after_update_metadata.call(file_set, user)
-        end
+        CurationConcerns.config.callback.run(:after_update_metadata, file_set, user)
       end
     end
 
     def destroy
       file_set.destroy
       # TODO: need to mend the linked list of proxies (possibly wrap with a lock)
-      CurationConcerns.config.after_destroy.call(file_set.id, user) if CurationConcerns.config.respond_to?(:after_destroy)
+      CurationConcerns.config.callback.run(:after_destroy, file_set.id, user)
     end
 
     private

curation_concerns-models/app/jobs/audit_job.rb

@@ -18,10 +18,10 @@ def perform(id, file_id, uri)
     log = run_audit
     fixity_ok = log.pass == 1
     unless fixity_ok
-      if CurationConcerns.config.respond_to?(:after_audit_failure)
+      if CurationConcerns.config.callback.set?(:after_audit_failure)
         login = file_set.depositor
         user = User.find_by_user_key(login)
-        CurationConcerns.config.after_audit_failure.call(file_set, user, log.created_at)
+        CurationConcerns.config.callback.run(:after_audit_failure, file_set, user, log.created_at)
       end
     end
     fixity_ok

curation_concerns-models/app/jobs/import_url_job.rb

@@ -13,17 +13,10 @@ def perform(id)
       copy_remote_file(file_set.import_url, f)
       # attach downloaded file to generic file stubbed out
       if CurationConcerns::FileSetActor.new(file_set, user).create_content(f)
-
         # send message to user on download success
-        if CurationConcerns.config.respond_to?(:after_import_url_success)
-          CurationConcerns.config.after_import_url_success.call(file_set, user)
-        end
+        CurationConcerns.config.callback.run(:after_import_url_success, file_set, user)
       else
-
-        # send message to user on download failure
-        if CurationConcerns.config.respond_to?(:after_import_url_failure)
-          CurationConcerns.config.after_import_url_failure.call(file_set, user)
-        end
+        CurationConcerns.config.callback.run(:after_import_url_failure, file_set, user)
       end
     end
   end

curation_concerns-models/app/jobs/ingest_file_job.rb

@@ -11,8 +11,6 @@ def perform(file_set_id, filename, mime_type, user_key)
     Hydra::Works::UploadFileToFileSet.call(file_set, file, versioning: false)
     file_set.save!
     CurationConcerns::VersioningService.create(file_set.original_file, user_key)
-
-    return unless CurationConcerns.config.respond_to?(:after_create_content)
-    CurationConcerns.config.after_create_content.call(file_set, user_key)
+    CurationConcerns.config.callback.run(:after_create_content, file_set, user_key)
   end
 end

curation_concerns-models/app/jobs/ingest_local_file_job.rb

@@ -19,17 +19,9 @@ def perform(file_set_id, directory, filename, user_key)
 
     if actor.create_content(File.open(path))
       FileUtils.rm(path)
-
-      # send message to user on import success
-      if CurationConcerns.config.respond_to?(:after_import_local_file_success)
-        CurationConcerns.config.after_import_local_file_success.call(file_set, user, filename)
-      end
+      CurationConcerns.config.callback.run(:after_import_local_file_success, file_set, user, filename)
     else
-
-      # send message to user on import failure
-      if CurationConcerns.config.respond_to?(:after_import_local_file_failure)
-        CurationConcerns.config.after_import_local_file_failure.call(file_set, user, filename)
-      end
+      CurationConcerns.config.callback.run(:after_import_local_file_failure, file_set, user, filename)
     end
   end
 end

curation_concerns-models/app/jobs/upload_set_update_job.rb

@@ -19,50 +19,50 @@ def perform(login, upload_set_id, title, file_attributes, visibility)
     upload_set = UploadSet.find_or_create(self.upload_set_id)
     user = User.find_by_user_key(self.login)
 
-    upload_set.file_sets.each do |gf|
-      update_file(gf, user)
+    upload_set.file_sets.each do |file|
+      update_file(file, user)
     end
 
     upload_set.update(status: ["Complete"])
 
     if denied.empty?
       unless saved.empty?
-        if CurationConcerns.config.respond_to?(:after_upload_set_update_success)
+        if CurationConcerns.config.callback.set?(:after_upload_set_update_success)
           login = upload_set.depositor
           user = User.find_by_user_key(login)
-          CurationConcerns.config.after_upload_set_update_failure.call(user, upload_set, log.created_at)
+          CurationConcerns.config.callback.run(:after_upload_set_update_success, user, upload_set, log.created_at)
         end
         return true
       end
     else
-      if CurationConcerns.config.respond_to?(:after_upload_set_update_failure)
+      if CurationConcerns.config.callback.set?(:after_upload_set_update_failure)
         login = upload_set.depositor
         user = User.find_by_user_key(login)
-        CurationConcerns.config.after_upload_set_update_failure.call(user, upload_set, log.created_at)
+        CurationConcerns.config.callback.run(:after_upload_set_update_failure. user, upload_set, log.created_at)
       end
       return false
     end
   end
 
-  def update_file(gf, user)
-    unless user.can? :edit, gf
-      ActiveFedora::Base.logger.error "User #{user.user_key} DENIED access to #{gf.id}!"
-      denied << gf
+  def update_file(file, user)
+    unless user.can? :edit, file
+      ActiveFedora::Base.logger.error "User #{user.user_key} DENIED access to #{file.id}!"
+      denied << file
       return
     end
     # update the file using the actor after setting the title
-    gf.title = title[gf.id] if title[gf.id]
-    CurationConcerns::FileSetActor.new(gf, user).update_metadata(file_attributes, visibility: visibility)
+    file.title = title[file.id] if title[file.id]
+    CurationConcerns::FileSetActor.new(file, user).update_metadata(file_attributes, visibility: visibility)
 
     # update the work to the same metadata as the file.
     # NOTE: For the moment we are assuming copied metadata.  This is likely to change.
     # NOTE2: TODO: stop assuming that files only belong to one work
-    work = gf.in_works.first
+    work = file.in_works.first
     unless work.nil?
-      work.title = title[gf.id] if title[gf.id]
+      work.title = title[file.id] if title[file.id]
       CurationConcerns::GenericWorkActor.new(work, user, work_attributes).update
     end
 
-    saved << gf
+    saved << file
   end
 end

curation_concerns-models/lib/curation_concerns/configuration.rb

@@ -1,3 +1,5 @@
+require 'curation_concerns/callbacks'
+
 module CurationConcerns
   extend Deprecation
   class << self
@@ -17,6 +19,8 @@ class Queue
   end
 
   class Configuration
+    include Callbacks
+
     # An anonymous function that receives a path to a file
     # and returns AntiVirusScanner::NO_VIRUS_FOUND_RETURN_VALUE if no
     # virus is found; Any other returned value means a virus was found
@@ -114,6 +118,12 @@ def lock_retry_delay
       @lock_retry_delay ||= 200 # milliseconds
     end
 
+    callback.enable :after_create_content, :after_update_content,
+                    :after_revert_content, :after_update_metadata,
+                    :after_import_local_file_success,
+                    :after_import_local_file_failure, :after_audit_failure,
+                    :after_destroy, :after_import_url_success, :after_import_url_failure
+
     def register_curation_concern(*curation_concern_types)
       Array(curation_concern_types).flatten.compact.each do |cc_type|
         class_name = normalize_concern_name(cc_type)

lib/curation_concerns/callbacks.rb

@@ -0,0 +1,26 @@
+require 'active_support/concern'
+require 'curation_concerns/callbacks/registry'
+
+module CurationConcerns
+  module Callbacks
+    extend ActiveSupport::Concern
+
+    included do
+      # Define class instance variable as endpoint to the
+      # Callback::Registry api.
+      @callback = Registry.new
+    end
+
+    module ClassMethods
+      # Reader for class instance variable containing callback definitions.
+      def callback
+        @callback
+      end
+    end
+
+    # Accessor to Callback::Registry api for instances.
+    def callback
+      self.class.callback
+    end
+  end
+end

lib/curation_concerns/callbacks/registry.rb

@@ -0,0 +1,48 @@
+module CurationConcerns
+  module Callbacks
+    class Registry
+      attr_reader :callbacks
+
+      def initialize
+        @callbacks = {}
+      end
+
+      # Enables a callback by specifying one or more hooks.
+      def enable(hook, *more_hooks)
+        ([hook] + more_hooks).each { |h| @callbacks[h] ||= nil }
+      end
+
+      # Returns all enabled callback hooks.
+      def enabled
+        @callbacks.keys
+      end
+
+      # Returns true if the callback hook has been enabled.
+      def enabled?(hook)
+        @callbacks.key? hook
+      end
+
+      # Defines a callback for a given hook.
+      def set(hook, &block)
+        raise NoBlockGiven, "a block is required when setting a callback" unless block_given?
+        @callbacks[hook] = proc(&block)
+      end
+
+      # Returns true if a callback has been defined for a given hook.
+      def set?(hook)
+        enabled?(hook) && @callbacks[hook].respond_to?(:call)
+      end
+
+      # Runs the callback defined for a given hook, with the arguments provided
+      def run(hook, *args)
+        raise NotEnabled unless enabled?(hook)
+        return nil unless set?(hook)
+        @callbacks[hook].call(*args)
+      end
+    end
+
+    # Custom exceptions
+    class NotEnabled < StandardError; end
+    class NoBlockGiven < StandardError; end
+  end
+end

spec/jobs/ingest_file_job_spec.rb

@@ -37,4 +37,9 @@
       expect(VersionCommitter.where(version_id: versions.last.uri).pluck(:committer_login)).to eq ['bess']
     end
   end
+
+  it 'runs the after_create_content callback with file_set and user arguments' do
+    expect(CurationConcerns.config.callback).to receive(:run).with(:after_create_content, file_set, 'bob').exactly(1).times
+    described_class.perform_now(file_set.id, filename, 'image/png', 'bob')
+  end
 end

spec/lib/curation_concerns/callbacks/registry_spec.rb

@@ -0,0 +1,54 @@
+require 'curation_concerns/callbacks/registry'
+
+describe CurationConcerns::Callbacks::Registry do
+  subject { described_class.new }
+
+  describe "#enabled?" do
+    it "returns false if the specified callback has not been enabled" do
+      expect(subject.enabled?(:foo))
+    end
+
+    it "returns true after enabling the specified callback" do
+      subject.enable(:foo)
+      expect(subject.enabled?(:foo)).to eq true
+    end
+  end
+
+  describe "#set?" do
+    it "returns false if the callback has not been set" do
+      expect(subject.set?(:foo)).to eq false
+    end
+
+    it "returns true if the callback has been set" do
+      subject.set(:foo) { nil }
+      expect(subject.set?(:foo)).to eq true
+    end
+  end
+
+  describe "#set" do
+    it "raises an error if a block is not given" do
+      expect { subject.set(:foo) }.to raise_error CurationConcerns::Callbacks::NoBlockGiven
+    end
+
+    it "raises an error if given no arguments" do
+      expect { subject.set }.to raise_error ArgumentError
+    end
+  end
+
+  describe "#run" do
+    it "raises a NotEnabled error if the callback has not been enabled" do
+      expect { subject.run(:foo) }.to raise_error CurationConcerns::Callbacks::NotEnabled
+    end
+
+    it "runs the specified callback with parameters" do
+      subject.set(:foo) { |x, y| x + y }
+      expect(subject.run(:foo, 1, 2)).to eq 3
+    end
+
+    it "runs the most recently set callback" do
+      subject.set(:foo) { "first" }
+      subject.set(:foo) { "second" }
+      expect(subject.run(:foo)).to eq "second"
+    end
+  end
+end

spec/lib/curation_concerns/callbacks_spec.rb

@@ -0,0 +1,27 @@
+require 'curation_concerns/callbacks'
+
+describe CurationConcerns::Callbacks do
+  context 'when included in a class,' do
+    before do
+      class TestClass
+        include CurationConcerns::Callbacks
+      end
+    end
+
+    after do
+      Object.send(:remove_const, :TestClass)
+    end
+
+    describe '.callback' do
+      it 'returns an instance of Callbacks::Registry' do
+        expect(TestClass.callback).to be_a CurationConcerns::Callbacks::Registry
+      end
+    end
+
+    describe '#callback' do
+      it 'is an instance method shortcut to the class method of the same name' do
+        expect(TestClass.new.callback).to eq TestClass.callback
+      end
+    end
+  end
+end