Merge 02cd579 into ae5cddd

sul-dlss · Sep 27, 2018 · 1a4b2d5 · 1a4b2d5
2 parents ae5cddd + 02cd579
commit 1a4b2d5
Show file tree

Hide file tree

Showing 35 changed files with 310 additions and 349 deletions.
diff --git a/.rubocop.yml b/.rubocop.yml
@@ -26,13 +26,18 @@ Layout/SpaceBeforeBlockBraces:
 # Configuration parameters: AllowURI, URISchemes.
 Metrics/LineLength:
   Max: 200
+  Exclude:
+    - 'config/initializers/**/*.rb' # generated files
 
 RSpec/ContextWording:
   Enabled: false # too dogmatic
 
 RSpec/ExampleLength:
   Max: 25
 
+RSpec/ImplicitSubject: # we use this for `define_enum_for`, `validate_presence_of`, etc.
+  Enabled: false
+
 # we like 'expect(x).to receive' better than 'have_received'
 RSpec/MessageSpies:
   Enabled: false

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
@@ -1,25 +1,30 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2018-09-26 16:12:20 -0700 using RuboCop version 0.59.0.
+# on 2018-09-27 15:46:41 -0700 using RuboCop version 0.59.0.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
-# Offense count: 9
+# Offense count: 2
+Lint/DuplicateMethods:
+  Exclude:
+    - 'app/lib/pre_assembly/digital_object.rb'
+
+# Offense count: 10
 Metrics/AbcSize:
   Max: 74
 
-# Offense count: 27
+# Offense count: 28
 # Configuration parameters: CountComments, ExcludedMethods.
 # ExcludedMethods: refine
 Metrics/BlockLength:
-  Max: 309
+  Max: 298
 
 # Offense count: 2
 # Configuration parameters: CountComments.
 Metrics/ClassLength:
-  Max: 190
+  Max: 187
 
 # Offense count: 4
 Metrics/CyclomaticComplexity:
@@ -29,9 +34,9 @@ Metrics/CyclomaticComplexity:
 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
 # URISchemes: http, https
 Metrics/LineLength:
-  Max: 281
+  Max: 251
 
-# Offense count: 10
+# Offense count: 9
 # Configuration parameters: CountComments.
 Metrics/MethodLength:
   Max: 44
@@ -62,78 +67,36 @@ Naming/UncommunicativeMethodParamName:
   Exclude:
     - 'spec/support/bundle_setup.rb'
 
+# Offense count: 1
+RSpec/BeforeAfterAll:
+  Exclude:
+    - 'spec/spec_helper.rb'
+    - 'spec/rails_helper.rb'
+    - 'spec/support/**/*.rb'
+    - 'spec/lib/pre_assembly/digital_object_spec.rb'
+
 # Offense count: 2
 RSpec/DescribeClass:
   Exclude:
     - 'spec/views/bundle_context_spec.rb'
     - 'spec/views/job_runs/index.json.jbuilder_spec.rb'
 
-# Offense count: 1
-# Cop supports --auto-correct.
-# Configuration parameters: SkipBlocks, EnforcedStyle.
-# SupportedStyles: described_class, explicit
-RSpec/DescribedClass:
-  Exclude:
-    - 'spec/lib/pre_assembly/bundle_spec.rb'
-
-# Offense count: 5
-# Cop supports --auto-correct.
-RSpec/EmptyLineAfterFinalLet:
-  Exclude:
-    - 'spec/controllers/bundle_contexts_controller_spec.rb'
-    - 'spec/lib/pre_assembly/bundle_spec.rb'
-    - 'spec/models/bundle_context_spec.rb'
-    - 'spec/models/job_run_spec.rb'
-    - 'spec/services/discovery_report_spec.rb'
-
-# Offense count: 5
-# Cop supports --auto-correct.
-RSpec/EmptyLineAfterHook:
-  Exclude:
-    - 'spec/jobs/discovery_report_job_spec.rb'
-    - 'spec/jobs/preassembly_job_spec.rb'
-    - 'spec/lib/pre_assembly/bundle_spec.rb'
-    - 'spec/models/bundle_context_spec.rb'
-    - 'spec/models/job_run_spec.rb'
-
 # Offense count: 2
 # Configuration parameters: Max.
 RSpec/ExampleLength:
   Exclude:
     - 'spec/lib/pre_assembly/bundle_spec.rb'
 
-# Offense count: 1
-# Cop supports --auto-correct.
-# Configuration parameters: CustomTransform, IgnoredWords.
-RSpec/ExampleWording:
-  Exclude:
-    - 'spec/controllers/bundle_contexts_controller_spec.rb'
-
-# Offense count: 3
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle.
-# SupportedStyles: single_line_only, disallow
-RSpec/ImplicitSubject:
-  Exclude:
-    - 'spec/models/bundle_context_spec.rb'
-    - 'spec/models/job_run_spec.rb'
-
 # Offense count: 1
 # Configuration parameters: AssignmentOnly.
 RSpec/InstanceVariable:
   Exclude:
     - 'spec/lib/pre_assembly/object_file_spec.rb'
 
 # Offense count: 2
-# Cop supports --auto-correct.
-RSpec/LeadingSubject:
-  Exclude:
-    - 'spec/models/bundle_context_spec.rb'
-    - 'spec/services/discovery_report_spec.rb'
-
-# Offense count: 1
 RSpec/LetSetup:
   Exclude:
+    - 'spec/views/bundle_contexts/new.html.erb_spec.rb'
     - 'spec/views/job_runs/index.json.jbuilder_spec.rb'
 
 # Offense count: 1
@@ -172,35 +135,11 @@ Style/ClassVars:
   Exclude:
     - 'app/lib/pre_assembly/logging.rb'
 
-# Offense count: 8
+# Offense count: 6
 Style/CommentedKeyword:
   Exclude:
     - 'app/lib/pre_assembly/smpl.rb'
-    - 'spec/lib/pre_assembly/smpl_spec.rb'
 
 # Offense count: 19
 Style/Documentation:
   Enabled: false
-
-# Offense count: 140
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, UseHashRocketsWithSymbolValues, PreferHashRocketsForNonAlnumEndingSymbols.
-# SupportedStyles: ruby19, hash_rockets, no_mixed_keys, ruby19_no_mixed_keys
-Style/HashSyntax:
-  Exclude:
-    - 'app/lib/csv_importer.rb'
-    - 'app/lib/pre_assembly/bundle.rb'
-    - 'app/lib/pre_assembly/digital_object.rb'
-    - 'app/lib/pre_assembly/logging.rb'
-    - 'app/lib/pre_assembly/smpl.rb'
-    - 'spec/lib/pre_assembly/bundle_spec.rb'
-    - 'spec/lib/pre_assembly/digital_object_spec.rb'
-    - 'spec/lib/pre_assembly/object_file_spec.rb'
-    - 'spec/lib/pre_assembly/smpl_spec.rb'
-
-# Offense count: 210
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, ConsistentQuotesInMultiline.
-# SupportedStyles: single_quotes, double_quotes
-Style/StringLiterals:
-  Enabled: false
diff --git a/Rakefile b/Rakefile
@@ -4,3 +4,8 @@
 require_relative 'config/application'
 
 Rails.application.load_tasks
+
+require 'rubocop/rake_task'
+RuboCop::RakeTask.new
+
+task default: [:spec, :rubocop]
diff --git a/app/controllers/job_runs_controller.rb b/app/controllers/job_runs_controller.rb
@@ -1,6 +1,6 @@
 class JobRunsController < ApplicationController
   def index
-    flash[:notice] = "Success! Your job is queued. A link to your validation report will be emailed to you when it is ready." if params[:created]
+    flash[:notice] = 'Success! Your job is queued. A link to your validation report will be emailed to you when it is ready.' if params[:created]
     @job_runs = JobRun.order('created_at desc').page params[:page]
   end
 

diff --git a/app/lib/csv_importer.rb b/app/lib/csv_importer.rb
@@ -4,10 +4,10 @@ class CsvImporter
   # @return [Array<ActiveSupport::HashWithIndifferentAccess>]
   # @raise if file missing/unreadable
   def self.parse_to_hash(filename)
-    raise ArgumentError, "CSV filename required" unless filename.present?
+    raise ArgumentError, 'CSV filename required' unless filename.present?
     raise ArgumentError, "Required file not found: #{filename}." unless File.readable?(filename)
-    file_contents = IO.read(filename).encode("utf-8", replace: nil)
-    csv = CSV.parse(file_contents, :headers => true)
+    file_contents = IO.read(filename).encode('utf-8', replace: nil)
+    csv = CSV.parse(file_contents, headers: true)
     csv.map { |row| row.to_hash.with_indifferent_access }
   end
 end
diff --git a/app/lib/pre_assembly/bundle.rb b/app/lib/pre_assembly/bundle.rb
@@ -28,7 +28,7 @@ class Bundle
 
     def initialize(bundle_context)
       @bundle_context = bundle_context
-      self.smpl_manifest = PreAssembly::Smpl.new(:csv_filename => bundle_context.smpl_manifest, :bundle_dir => bundle_dir) if bundle_context.smpl_cm_style?
+      self.smpl_manifest = PreAssembly::Smpl.new(csv_filename: bundle_context.smpl_manifest, bundle_dir: bundle_dir) if bundle_context.smpl_cm_style?
       self.skippables = {}
       load_skippables
     end
@@ -55,11 +55,11 @@ def run_pre_assembly
 
     def run_log_msg
       log_params = {
-        :content_structure => content_structure,
-        :project_name => project_name,
-        :bundle_dir => bundle_dir,
-        :assembly_staging_dir => Settings.assembly_staging_dir,
-        :environment => ENV['RAILS_ENV']
+        content_structure: content_structure,
+        project_name: project_name,
+        bundle_dir: bundle_dir,
+        assembly_staging_dir: Settings.assembly_staging_dir,
+        environment: ENV['RAILS_ENV']
       }
       log_params.map { |k, v| "#{k}=#{v.inspect}" }.join(', ')
     end
@@ -82,14 +82,14 @@ def object_filenames_unique?(dobj)
     def digital_objects
       @digital_objects ||= discover_containers_via_manifest.each_with_index.map do |c, i|
         params = {
-          :container       => c,
-          :stageable_items => discover_items_via_crawl(c)
+          container: c,
+          stageable_items: discover_items_via_crawl(c)
         }
         params[:object_files] = discover_object_files(params[:stageable_items])
         DigitalObject.new(self, params).tap do |dobj|
           r = manifest_rows[i]
           # Get label and source_id from column names declared in YAML config.
-          dobj.label        = manifest_cols[:label] ? r[manifest_cols[:label]] : ""
+          dobj.label        = manifest_cols[:label] ? r[manifest_cols[:label]] : ''
           dobj.source_id    = r[manifest_cols[:source_id]] if manifest_cols[:source_id]
           # Also store a hash of all values from the manifest row, using column names as keys.
           dobj.manifest_row = r
@@ -146,9 +146,9 @@ def all_object_files
 
     def new_object_file(stageable, file_path)
       ObjectFile.new(
-        :path                 => file_path,
-        :relative_path        => relative_path(get_base_dir(stageable), file_path),
-        :exclude_from_content => exclude_from_content(file_path)
+        path: file_path,
+        relative_path: relative_path(get_base_dir(stageable), file_path),
+        exclude_from_content: exclude_from_content(file_path)
       )
     end
 
@@ -162,7 +162,7 @@ def exclude_from_content(file_path)
     # Takes a DigitalObject. For each of its ObjectFiles,
     # sets the checksum attribute.
     def load_checksums(dobj)
-      log "  - load_checksums()"
+      log '  - load_checksums()'
       dobj.object_files.each { |file| file.checksum = file.md5 }
     end
 
@@ -220,10 +220,10 @@ def objects_to_process
 
     def log_progress_info(dobj)
       {
-        :container          => dobj.container,
-        :pid                => dobj.pid,
-        :pre_assem_finished => dobj.pre_assem_finished,
-        :timestamp          => Time.now.strftime('%Y-%m-%d %H:%I:%S')
+        container: dobj.container,
+        pid: dobj.pid,
+        pre_assem_finished: dobj.pre_assem_finished,
+        timestamp: Time.now.strftime('%Y-%m-%d %H:%I:%S')
       }
     end
 

diff --git a/app/lib/pre_assembly/digital_object.rb b/app/lib/pre_assembly/digital_object.rb
@@ -25,7 +25,7 @@ class DigitalObject
 
     attr_writer :dor_object, :druid_tree_dir
 
-    INIT_PARAMS = [:container, :stageable_items, :object_files]
+    INIT_PARAMS = [:container, :stageable_items, :object_files].freeze
 
     # @param [PreAssembly::Bundle] bundle
     # @param [Hash<Symbol => Object>] params
@@ -45,7 +45,7 @@ def setup
 
     def stager(source, destination)
       if staging_style_symlink
-        FileUtils.ln_s source, destination, :force => true
+        FileUtils.ln_s source, destination, force: true
       else
         FileUtils.cp_r source, destination
       end
@@ -112,7 +112,7 @@ def pid
     end
 
     def query_dor_by_barcode(barcode)
-      Dor::SearchService.query_by_id :barcode => barcode
+      Dor::SearchService.query_by_id barcode: barcode
     end
 
     def get_dor_item_apos(_pid)
@@ -126,7 +126,7 @@ def dor_object
     end
 
     def content_type_tag
-      dor_object.nil? ? "" : dor_object.content_type_tag
+      dor_object.nil? ? '' : dor_object.content_type_tag
     end
 
     def container_basename
@@ -171,15 +171,15 @@ def create_technical_metadata
       return unless content_md_creation == 'smpl_cm_style'
 
       tm = Nokogiri::XML::Document.new
-      tm_node = Nokogiri::XML::Node.new("technicalMetadata", tm)
+      tm_node = Nokogiri::XML::Node.new('technicalMetadata', tm)
       tm_node['objectId'] = pid
-      tm_node['datetime'] = Time.now.utc.strftime("%Y-%m-%d-T%H:%M:%SZ")
+      tm_node['datetime'] = Time.now.utc.strftime('%Y-%m-%d-T%H:%M:%SZ')
       tm << tm_node
 
       # find all technical metadata files and just append the xml to the combined technicalMetadata
       current_directory = Dir.pwd
       FileUtils.cd(File.join(bundle_dir, container_basename))
-      Dir.glob("**/*_techmd.xml").sort.each do |filename|
+      Dir.glob('**/*_techmd.xml').sort.each do |filename|
         tech_md_xml = Nokogiri::XML(File.open(File.join(bundle_dir, container_basename, filename)))
         tm.root << tech_md_xml.root
       end
@@ -205,11 +205,11 @@ def generate_content_metadata
     # Invoke the contentMetadata creation method used by the project
     # if we are not using a standard known style of content metadata generation, pass the task off to a custom method
     def create_content_metadata
-      if content_md_creation == "smpl_cm_style"
+      if content_md_creation == 'smpl_cm_style'
         self.content_md_xml = smpl_manifest.generate_cm(druid.id)
       else
         # otherwise use the content metadata generation gem
-        params = { :druid => druid.id, :objects => content_object_files, :add_exif => false, :bundle => content_md_creation.to_sym, :style => content_md_creation_style }
+        params = { druid: druid.id, objects: content_object_files, add_exif: false, bundle: content_md_creation.to_sym, style: content_md_creation_style }
         self.content_md_xml = Assembly::ContentMetadata.create_content_metadata(params)
       end
     end
@@ -279,7 +279,7 @@ def content_md_file
       'contentMetadata.xml'
     end
 
-    def retry_handler(method_name, logger, params = {})
+    def retry_handler(method_name, _logger, params = {})
       proc do |_exception, attempt_number, total_delay|
         log("      ** #{method_name} FAILED **; with params of #{params.inspect}; and trying attempt #{attempt_number} of #{Dor::Config.dor_services.num_attempts}; delayed #{total_delay} seconds")
       end