Merge e788385 into 4de93ed

.rubocop_todo.yml

@@ -206,7 +206,6 @@ Style/AndOr:
     - 'lib/moab/file_signature.rb'
     - 'lib/moab/storage_object_version.rb'
     - 'lib/moab/storage_repository.rb'
-    - 'lib/moab/verification_result.rb'
     - 'lib/serializer/serializable.rb'
     - 'lib/stanford/content_inventory.rb'
 
@@ -279,7 +278,6 @@ Style/Documentation:
   Exclude:
     - 'spec/**/*'
     - 'test/**/*'
-    - 'lib/moab/verification_result.rb'
 
 # Offense count: 47
 # Cop supports --auto-correct.
@@ -386,7 +384,6 @@ Style/ParenthesesAroundCondition:
   Exclude:
     - 'lib/moab/file_manifestation.rb'
     - 'lib/moab/file_signature.rb'
-    - 'lib/moab/verification_result.rb'
     - 'spec/unit_tests/moab/verification_result_spec.rb'
 
 # Offense count: 8

lib/moab/storage_object_validator.rb

@@ -89,15 +89,11 @@ def version_dir_format?(dirname)
 
     # call only if the version directories are "correctly named" vdddd
     def check_sequential_version_dirs
-      errors = []
       version_directories.each_with_index do |dir_name, index|
-        expected_vers_num = index + 1 # version numbering starts at 1, array indexing starts at 0
-        if dir_name[1..-1].to_i != expected_vers_num
-          errors << result_hash(VERSIONS_NOT_IN_ORDER, version_directories)
-          break
-        end
+        next if dir_name[1..-1].to_i == index + 1 # version numbering starts at 1, array indexing at 0
+        return [result_hash(VERSIONS_NOT_IN_ORDER, version_directories)]
       end
-      errors
+      []
     end
 
     def check_correctly_formed_moab(allow_content_subdirs = true)
@@ -113,17 +109,12 @@ def check_correctly_formed_moab(allow_content_subdirs = true)
     end
 
     def check_version_sub_dirs(version_path, version)
-      errors = []
       version_sub_dirs = directory_entries(version_path)
-      version_sub_dir_count = version_sub_dirs.size
-      if version_sub_dir_count == EXPECTED_VERSION_SUB_DIRS.size
-        errors.concat expected_version_sub_dirs(version_path, version)
-      elsif version_sub_dir_count > EXPECTED_VERSION_SUB_DIRS.size
-        errors.concat found_unexpected(version_sub_dirs, version, EXPECTED_VERSION_SUB_DIRS)
-      elsif version_sub_dir_count < EXPECTED_VERSION_SUB_DIRS.size
-        errors.concat missing_dir(version_sub_dirs, version, EXPECTED_VERSION_SUB_DIRS)
-      end
-      errors
+      count = version_sub_dirs.size
+      return expected_version_sub_dirs(version_path, version) if count == EXPECTED_VERSION_SUB_DIRS.size
+      return found_unexpected(version_sub_dirs, version, EXPECTED_VERSION_SUB_DIRS) if count > EXPECTED_VERSION_SUB_DIRS.size
+      return missing_dir(version_sub_dirs, version, EXPECTED_VERSION_SUB_DIRS) if count < EXPECTED_VERSION_SUB_DIRS.size
+      []
     end
 
     def check_data_directory(version_path, version, allow_content_subdirs = true)
@@ -137,15 +128,11 @@ def check_data_directory(version_path, version, allow_content_subdirs = true)
     end
 
     def check_data_sub_dirs(version, data_sub_dirs)
+      return found_unexpected(data_sub_dirs, version, EXPECTED_DATA_SUB_DIRS) if data_sub_dirs.size > EXPECTED_DATA_SUB_DIRS.size
       errors = []
-      if data_sub_dirs.size > EXPECTED_DATA_SUB_DIRS.size
-        errors.concat found_unexpected(data_sub_dirs, version, EXPECTED_DATA_SUB_DIRS)
-      else
-        errors.concat missing_dir(data_sub_dirs, version, [METADATA_DIR]) unless data_sub_dirs.include?(METADATA_DIR)
-        errors.concat found_unexpected(data_sub_dirs, version, EXPECTED_DATA_SUB_DIRS) unless subset?(data_sub_dirs,
-                                                                                                      EXPECTED_DATA_SUB_DIRS)
-      end
-      errors
+      errors.concat missing_dir(data_sub_dirs, version, [METADATA_DIR]) unless data_sub_dirs.include?(METADATA_DIR)
+      return errors if data_sub_dirs.to_set.subset?(EXPECTED_DATA_SUB_DIRS.to_set)
+      errors.concat found_unexpected(data_sub_dirs, version, EXPECTED_DATA_SUB_DIRS)
     end
 
     def check_optional_content_dir(version_path, allow_content_subdirs = true)
@@ -167,10 +154,6 @@ def content_sub_dir_forbidden?(dirname)
       FORBIDDEN_CONTENT_SUB_DIRS.include?(dirname) || version_dir_format?(dirname)
     end
 
-    def subset?(first_array, second_array)
-      first_array.to_set.subset?(second_array.to_set)
-    end
-
     def check_metadata_dir_files_only(version_path)
       errors = []
       metadata_dir_path = File.join(version_path, DATA_DIR, METADATA_DIR)
@@ -230,24 +213,15 @@ def error_code_msg(response_code, addl = nil)
     end
 
     def check_required_manifest_files(dir, version)
-      unless contains_file?(File.join(dir, MANIFESTS_DIR))
-        return [result_hash(NO_FILES_IN_MANIFEST_DIR, version)]
-      end
+      return [result_hash(NO_FILES_IN_MANIFEST_DIR, version)] unless contains_file?(File.join(dir, MANIFESTS_DIR))
       errors = []
-      unless File.exist?(File.join(dir, MANIFEST_INVENTORY_PATH))
-        errors << result_hash(NO_MANIFEST_INVENTORY, version)
-      end
-      unless File.exist?(File.join(dir, SIGNATURE_CATALOG_PATH))
-        errors << result_hash(NO_SIGNATURE_CATALOG, version)
-      end
+      errors << result_hash(NO_MANIFEST_INVENTORY, version) unless File.exist?(File.join(dir, MANIFEST_INVENTORY_PATH))
+      errors << result_hash(NO_SIGNATURE_CATALOG, version) unless File.exist?(File.join(dir, SIGNATURE_CATALOG_PATH))
       errors
     end
 
-    def latest_manifest_inventory
-      File.join(storage_obj_path, version_directories.last, MANIFEST_INVENTORY_PATH)
-    end
-
     def object_id_from_manifest_inventory
+      latest_manifest_inventory = File.join(storage_obj_path, version_directories.last, MANIFEST_INVENTORY_PATH)
       Nokogiri::XML(File.open(latest_manifest_inventory)).at_xpath('//fileInventory/@objectId').value
     end
   end

lib/moab/storage_object_version.rb

@@ -217,7 +217,7 @@ def verify_version_storage()
       result
     end
 
-    # @return [Boolean] return true if the manifest inventory matches the actual files
+    # @return [VerificationResult] return true if the manifest inventory matches the actual files
     def verify_manifest_inventory
       # read/parse manifestInventory.xml
       result = VerificationResult.new("manifest_inventory")
@@ -240,6 +240,7 @@ def verify_manifest_inventory
       result
     end
 
+    # @return [VerificationResult]
     def verify_signature_catalog
       result = VerificationResult.new("signature_catalog")
       signature_catalog = self.signature_catalog

lib/moab/verification_result.rb

@@ -1,6 +1,7 @@
 require 'moab'
 
 module Moab
+  # A recursive "Tree" type object for verifications
   class VerificationResult
     # @return [String] The name of the entity that was verified
     attr_accessor :entity
@@ -15,34 +16,30 @@ class VerificationResult
     attr_accessor :subentities
 
     # @param entity [Object] The name of the entity being verified
-    def initialize(entity)
+    # @param verified [Boolean]
+    # @param details [Hash]
+    def initialize(entity, verified = false, details = nil)
       @entity = entity
-      @verified = false
-      @details = nil
+      @verified = verified
+      @details = details
       @subentities = Array.new
     end
 
-    # @param entity [Symbol] The name of the entity being verified
+    # @param entity [#to_s] The name of the entity being verified
     # @param expected [Object] The expected value
     # @param found [Object] The found value
     # @return [VerificationResult] The result of comparing the expected and found values
     def self.verify_value(entity, expected, found)
-      result = VerificationResult.new(entity.to_s)
-      result.verified = (expected == found)
-      result.details = { 'expected' => expected, 'found' => found }
-      result
+      new(entity.to_s, (expected == found), 'expected' => expected, 'found' => found)
     end
 
-    # @param entity [Symbol] The name of the entity being verified
+    # @param entity [#to_s] The name of the entity being verified
     # @param expression [Object] The expression that will be evaluated as true or false
     # @param details [Object] optional details that could be reported
     # @return [VerificationResult] The result of evaluating the expression
     def self.verify_truth(entity, expression, details = nil)
-      result = VerificationResult.new(entity.to_s)
       # TODO: add expression.empty?
-      result.verified = !(expression.nil? or (expression == false))
-      result.details = details
-      result
+      new(entity.to_s, !(expression.nil? || (expression == false)), details)
     end
 
     # @param verbose [Boolean] If true, always provide all details of the verification
@@ -55,27 +52,21 @@ def to_json(verbose = false)
     # @param level [Integer] Used to test the depth of recursion
     # @return [Hash] The verification result serialized to a hash
     def to_hash(verbose = false, level = 0)
-      hash = Hash.new
-      hash['verified'] = @verified
-      if (verbose or @verified == false)
-        hash['details'] = @details ? @details : subentities_to_hash(verbose, level)
-      end
-      if level > 0
-        hash
-      else
-        { @entity => hash }
+      hash = { 'verified' => verified }
+      if verbose || verified == false
+        hash['details'] = details || subentities_to_hash(verbose, level)
       end
+      return hash if level > 0
+      { entity => hash }
     end
 
+    private
+
     # @param verbose [Boolean] If true, always provide all details of the verification
     # @param level [Integer] Used to increment the depth of recursion
     # @return [Hash] The verification result of subentities serialized to a hash
     def subentities_to_hash(verbose, level)
-      hash = Hash.new
-      @subentities.each do |s|
-        hash[s.entity] = s.to_hash(verbose, level + 1)
-      end
-      hash
+      subentities.map { |s| [s.entity, s.to_hash(verbose, level + 1)] }.to_h
     end
   end
 end