try to make rubocop happy

samvera · May 16, 2017 · 62e8be4 · 62e8be4
1 parent dadde68
commit 62e8be4
Show file tree

Hide file tree

Showing 9 changed files with 84 additions and 91 deletions.
diff --git a/app/jobs/fixity_check_job.rb b/app/jobs/fixity_check_job.rb
@@ -39,8 +39,6 @@ def perform(uri, file_set_id:, file_id:)
 
   protected
 
-
-
     def run_check(file_set_id, file_id, uri)
       begin
         fixity_ok = ActiveFedora::FixityService.new(uri).check

diff --git a/app/models/checksum_audit_log.rb b/app/models/checksum_audit_log.rb
@@ -1,7 +1,6 @@
 class ChecksumAuditLog < ActiveRecord::Base
-
   def failed?
-    ! passed?
+    !passed?
   end
 
   # Only the latest rows for a given file_set_id/checked_uri pair.
@@ -10,13 +9,13 @@ def failed?
   # LOTS of records.
   def self.latest_checks
     # one crazy SQL trick to get the latest for each fileset/checked_uri combo
+    # where there's no other self-join created_at greater -- only the greatest.
     joins("LEFT JOIN checksum_audit_logs c2 ON
             (checksum_audit_logs.file_set_id = c2.file_set_id AND
              checksum_audit_logs.checked_uri = c2.checked_uri AND
-             checksum_audit_logs.created_at < c2.created_at)").
-    # special trick, where there's no other self-join created_at greater -- only the greatest.
-    where("c2.id is NULL").
-    order("created_at desc, id desc")
+             checksum_audit_logs.created_at < c2.created_at)")
+      .where("c2.id is NULL")
+      .order("created_at desc, id desc")
   end
 
   # From all ChecksumAuditLogs related to this file set, returns only

diff --git a/app/services/hyrax/file_set_fixity_check_service.rb b/app/services/hyrax/file_set_fixity_check_service.rb
@@ -38,8 +38,8 @@ def initialize(file_set,
                    max_days_between_fixity_checks: Hyrax.config.max_days_between_fixity_checks,
                    latest_version_only: false)
       @max_days_between_fixity_checks = max_days_between_fixity_checks || 0
-      @async_jobs = !! async_jobs
-      @latest_version_only = !! latest_version_only
+      @async_jobs = async_jobs
+      @latest_version_only = latest_version_only
       if file_set.is_a?(String)
         @id = file_set
       else
@@ -57,9 +57,9 @@ def initialize(file_set,
     def fixity_check
       results = file_set.files.collect { |f| fixity_check_file(f) }
 
-      unless async_jobs
-        return results.flatten.group_by(&:file_id)
-      end
+      return if async_jobs
+
+      results.flatten.group_by(&:file_id)
     end
 
     # Return current fixity status for this FileSet based on
@@ -71,16 +71,16 @@ def logged_fixity_status
     end
 
     private
+
       # Retrieve or generate the fixity check for a file
       # (all versions are checked for versioned files unless latest_version_only set)
       # @param [ActiveFedora::File] file to fixity check
       # @param [Array] log container for messages
       def fixity_check_file(file)
         versions = file.has_versions? ? file.versions.all : [file]
 
-        if latest_version_only
-          versions = [versions.max_by(&:created)]
-        end
+        versions = [versions.max_by(&:created)] if latest_version_only
+
         versions.collect { |v| fixity_check_file_version(file.id, v.uri.to_s) }.flatten
       end
 

diff --git a/app/services/hyrax/fixity_status_service.rb b/app/services/hyrax/fixity_status_service.rb
@@ -24,61 +24,63 @@ def file_set_status
           content_tag("span", "passed", class: "label label-success") + ' ' + render_existing_check_summary
         else
           content_tag("span", "FAIL", class: "label label-danger") + ' ' + render_existing_check_summary + render_failed_compact
-          # TODO details on failures.
         end
     end
 
     protected
 
-    # A weird display, cause we need it to fit in that 180px column on
-    # FileSet show, and have no real UI to link to for files/versions :(
-    def render_failed_compact
-      safe_join(
-        ["<p><strong>Failed checks:</strong></p>".html_safe] +
-        failing_checks.collect do |log|
-          safe_join([
-            "<p>".html_safe,
-            "ChecksumAuditLog id: #{log.id}; ",
-            content_tag("a", "file", href: "#{Hydra::PCDM::File.translate_id_to_uri.call(log.file_id)}/fcr:metadata") + "; ",
-            content_tag("a", "checked_uri", href: "#{log.checked_uri}/fcr:metadata") + "; ",
-            "date: #{log.created_at.to_s}; ",
-            "expected_result: #{log.expected_result}",
-            "</p>".html_safe
-          ])
-        end
-      )
-    end
+      # A weird display, cause we need it to fit in that 180px column on
+      # FileSet show, and have no real UI to link to for files/versions :(
+      # rubocop:disable Metrics/MethodLength
+      def render_failed_compact
+        safe_join(
+          ["<p><strong>Failed checks:</strong></p>".html_safe] +
+          failing_checks.collect do |log|
+            safe_join(
+              [
+                "<p>".html_safe,
+                "ChecksumAuditLog id: #{log.id}; ",
+                content_tag("a", "file", href: "#{Hydra::PCDM::File.translate_id_to_uri.call(log.file_id)}/fcr:metadata") + "; ",
+                content_tag("a", "checked_uri", href: "#{log.checked_uri}/fcr:metadata") + "; ",
+                "date: #{log.created_at}; ",
+                "expected_result: #{log.expected_result}",
+                "</p>".html_safe
+              ]
+            )
+          end
+        )
+      end
+      # rubocop:enable Metrics/MethodLength
 
-    def render_existing_check_summary
-      @render_existing_check_summary ||=
-        "#{pluralize num_checked_files, 'File'} with #{pluralize relevant_log_records.count, "total version"} checked #{render_date_range}"
-    end
+      def render_existing_check_summary
+        @render_existing_check_summary ||=
+          "#{pluralize num_checked_files, 'File'} with #{pluralize relevant_log_records.count, 'total version'} checked #{render_date_range}"
+      end
 
-    def render_date_range
-      @render_date_range ||= begin
-        from = relevant_log_records.min_by(&:created_at).created_at.to_s
-        to   = relevant_log_records.max_by(&:created_at).created_at.to_s
-        if from == to
-          from
-        else
-          "between #{from} and #{to}"
+      def render_date_range
+        @render_date_range ||= begin
+          from = relevant_log_records.min_by(&:created_at).created_at.to_s
+          to   = relevant_log_records.max_by(&:created_at).created_at.to_s
+          if from == to
+            from
+          else
+            "between #{from} and #{to}"
+          end
         end
       end
-    end
 
-    # Should be all _latest_ ChecksumAuditLog about different files/versions
-    # currently existing in specified FileSet.
-    def relevant_log_records
-      @relevant_log_records = ChecksumAuditLog.latest_for_file_set_id(file_set_id)
-    end
-
-    def num_checked_files
-      @num_relevant_files ||= relevant_log_records.group_by(&:file_id).keys.count
-    end
+      # Should be all _latest_ ChecksumAuditLog about different files/versions
+      # currently existing in specified FileSet.
+      def relevant_log_records
+        @relevant_log_records = ChecksumAuditLog.latest_for_file_set_id(file_set_id)
+      end
 
-    def failing_checks
-      @failing_checks ||= relevant_log_records.find_all(&:failed?)
-    end
+      def num_checked_files
+        @num_relevant_files ||= relevant_log_records.group_by(&:file_id).keys.count
+      end
 
+      def failing_checks
+        @failing_checks ||= relevant_log_records.find_all(&:failed?)
+      end
   end
 end
diff --git a/spec/controllers/hyrax/fixity_checks_controller_spec.rb b/spec/controllers/hyrax/fixity_checks_controller_spec.rb
@@ -11,26 +11,24 @@
 
   context "when signed in" do
     describe "POST create" do
-      before { sign_in user }
-
-      it "returns json with the result" do
+      before do
+        sign_in user
         post :create, params: { file_set_id: file_set }, xhr: true
+      end
+      let(:json_response) { JSON.parse(response.body) }
+      it "returns json with the result" do
         expect(response).to be_success
-        json = JSON.parse(response.body)
         # json is a structure like this:
         #   { file_id => [{ "checked_uri" => "...4-4d71-83ba-1bc52a5e4300/fcr:versions/version1", "passed" => true },
         #                 { "checked_uri" => ".../version2", "passed" => false },
         #                 ...] }
-        json.each do |file_id, array_of_checks|
+        json_response.each do |_file_id, array_of_checks|
           array_of_checks.each do |check_hash|
-            ["file_set_id", "file_id", "checked_uri", "passed",
-             "expected_result", "created_at"].each do |internal_key|
-              expect(check_hash).to have_key(internal_key)
-            end
+            expect(check_hash.keys).to include("file_set_id", "file_id", "checked_uri", "passed", "expected_result", "created_at")
             expect(check_hash["passed"]).to be_in([true, false])
           end
         end
-        fixity_results = json.values.flatten.collect { |result| result["passed"] }
+        fixity_results = json_response.values.flatten.collect { |result| result["passed"] }
         expect(fixity_results.all? { |r| r == true }).to be true
       end
     end

diff --git a/spec/jobs/fixity_check_job_spec.rb b/spec/jobs/fixity_check_job_spec.rb
@@ -14,7 +14,6 @@
   describe "called with perform_now" do
     let(:log_record) { described_class.perform_now(uri, file_set_id: file_set.id, file_id: file_id) }
 
-
     describe 'fixity check the content' do
       let(:uri) { file_set.original_file.uri }
       it 'passes' do
@@ -63,5 +62,4 @@
       expect(ChecksumAuditLog.logs_for(file_set.id, checked_uri: uri).map(&:passed)).to eq [false, true, false, false, true, false, true]
     end
   end
-
 end
diff --git a/spec/models/checksum_audit_log_spec.rb b/spec/models/checksum_audit_log_spec.rb
@@ -27,18 +27,17 @@
   end
 
   describe "prune_history" do
-
     context "complex history" do
       let(:file_set_id) { "file_set_id" }
       let(:file_id) { "file_id" }
       let(:version_uri) { "#{file_id}/fcr:versions/version1" }
       before do
-        ChecksumAuditLog.create(file_set_id: file_set_id, file_id: file_id, checked_uri: version_uri, passed: true)
-        ChecksumAuditLog.create(file_set_id: file_set_id, file_id: file_id, checked_uri: version_uri, passed: true)
-        ChecksumAuditLog.create(file_set_id: file_set_id, file_id: file_id, checked_uri: version_uri, passed: false)
-        ChecksumAuditLog.create(file_set_id: file_set_id, file_id: file_id, checked_uri: version_uri, passed: true)
-        ChecksumAuditLog.create(file_set_id: file_set_id, file_id: file_id, checked_uri: version_uri, passed: true)
-        ChecksumAuditLog.create(file_set_id: file_set_id, file_id: file_id, checked_uri: version_uri, passed: true)
+        described_class.create(file_set_id: file_set_id, file_id: file_id, checked_uri: version_uri, passed: true)
+        described_class.create(file_set_id: file_set_id, file_id: file_id, checked_uri: version_uri, passed: true)
+        described_class.create(file_set_id: file_set_id, file_id: file_id, checked_uri: version_uri, passed: false)
+        described_class.create(file_set_id: file_set_id, file_id: file_id, checked_uri: version_uri, passed: true)
+        described_class.create(file_set_id: file_set_id, file_id: file_id, checked_uri: version_uri, passed: true)
+        described_class.create(file_set_id: file_set_id, file_id: file_id, checked_uri: version_uri, passed: true)
 
         described_class.prune_history(file_set_id, checked_uri: version_uri)
       end
@@ -47,7 +46,6 @@
       end
     end
 
-
     context 'after multiple checksum events where the checksum does not change' do
       specify 'only one of them should be kept' do
         success1 = described_class.create(file_set_id: f.id, file_id: content_id, checked_uri: version_uri, passed: true)
@@ -75,11 +73,11 @@
     let(:version_uri2) { f.original_file.versions.all.second.uri }
     before do
       described_class.create(file_set_id: f.id, file_id: content_id, checked_uri: version_uri, passed: true, created_at: 2.days.ago)
-      described_class.create(file_set_id: f.id, file_id: content_id, checked_uri: version_uri, passed: true, created_at: 1.days.ago)
+      described_class.create(file_set_id: f.id, file_id: content_id, checked_uri: version_uri, passed: true, created_at: 1.day.ago)
       described_class.create(file_set_id: f.id, file_id: content_id, checked_uri: version_uri, passed: true)
 
       described_class.create(file_set_id: f.id, file_id: content_id, checked_uri: version_uri2, passed: true, created_at: 2.days.ago)
-      described_class.create(file_set_id: f.id, file_id: content_id, checked_uri: version_uri2, passed: false, created_at: 1.days.ago)
+      described_class.create(file_set_id: f.id, file_id: content_id, checked_uri: version_uri2, passed: false, created_at: 1.day.ago)
       described_class.create(file_set_id: f.id, file_id: content_id, checked_uri: version_uri2, passed: true)
     end
     describe ".latest_checks" do
@@ -105,6 +103,4 @@
       end
     end
   end
-
-
 end
diff --git a/spec/services/hyrax/fixity_check_failure_service_spec.rb b/spec/services/hyrax/fixity_check_failure_service_spec.rb
@@ -3,7 +3,7 @@
   let!(:log_date) { '2015-07-15 03:06:59' }
   let(:inbox) { depositor.mailbox.inbox }
   let(:file) { Hydra::PCDM::File.new }
-  let(:version_uri) { "#{file.uri}/fcr:versions/version1"}
+  let(:version_uri) { "#{file.uri}/fcr:versions/version1" }
   let(:file_set) do
     create(:file_set, user: depositor, title: ["World Icon"]).tap { |fs| fs.original_file = file }
   end

diff --git a/spec/services/hyrax/fixity_status_service_spec.rb b/spec/services/hyrax/fixity_status_service_spec.rb
@@ -3,10 +3,12 @@
 RSpec.describe Hyrax::FixityStatusService do
   let(:file_set_id) { "xw42n7888" }
   let(:file_ids) do
-    [ "#{file_set_id}/files/3ec4c460-db36-49d4-914c-2d2036b0bfc6",
-      "#{file_set_id}/files/c3e8d90c-e47b-4c3b-9cff-50d20b5b0583"]
+    [
+      "#{file_set_id}/files/3ec4c460-db36-49d4-914c-2d2036b0bfc6",
+      "#{file_set_id}/files/c3e8d90c-e47b-4c3b-9cff-50d20b5b0583"
+    ]
   end
-  let(:service) { described_class.new(file_set_id)}
+  let(:service) { described_class.new(file_set_id) }
 
   describe "#file_set_status" do
     describe "no logs recorded" do
@@ -22,7 +24,7 @@
         # the FileSetFixityCheckService actually creats, as specs have before.
         file_ids.each do |file_id|
           ChecksumAuditLog.create!(passed: true, file_set_id: file_set_id, file_id: file_id, checked_uri: "#{file_id}/fcr:versions/version1", created_at: 2.days.ago)
-          ChecksumAuditLog.create!(passed: true, file_set_id: file_set_id, file_id: file_id, checked_uri: "#{file_id}/fcr:versions/version2", created_at: 1.days.ago)
+          ChecksumAuditLog.create!(passed: true, file_set_id: file_set_id, file_id: file_id, checked_uri: "#{file_id}/fcr:versions/version2", created_at: 1.day.ago)
         end
       end
       it "creates success message with details" do
@@ -38,8 +40,8 @@
       let(:failing_checked_uri) { "#{failing_file_id}/fcr:versions/version1" }
       before do
         ChecksumAuditLog.create!(passed: true, file_set_id: file_set_id, file_id: file_id, checked_uri: "#{file_id}/fcr:versions/version1", created_at: 2.days.ago)
-        ChecksumAuditLog.create!(passed: true, file_set_id: file_set_id, file_id: file_id, checked_uri: "#{file_id}/fcr:versions/version2", created_at: 1.days.ago)
-        ChecksumAuditLog.create!(passed: false, file_set_id: file_set_id, file_id: failing_file_id, checked_uri: failing_checked_uri, created_at: Time.now)
+        ChecksumAuditLog.create!(passed: true, file_set_id: file_set_id, file_id: file_id, checked_uri: "#{file_id}/fcr:versions/version2", created_at: 1.day.ago)
+        ChecksumAuditLog.create!(passed: false, file_set_id: file_set_id, file_id: failing_file_id, checked_uri: failing_checked_uri, created_at: Time.zone.now)
       end
       it "creates failure message with details" do
         result = service.file_set_status