From e98168f88056400ab55192ae6fa9e0f5dc6a08fa Mon Sep 17 00:00:00 2001 From: Sarav Shah Date: Thu, 13 Sep 2018 15:47:07 -0700 Subject: [PATCH] Delete BundleContextTemporary - will delete the class and the RSPec tests. - rename bundle_setup_ar_model to bundle_setup - rename bundle_context_from_hash --- app/lib/bundle_context_temporary.rb | 171 ------------------ spec/lib/bundle_context_temporary_spec.rb | 83 --------- spec/lib/pre_assembly/bundle_spec.rb | 10 +- spec/services/discover_report_spec.rb | 2 +- spec/support/bundle_setup.rb | 14 +- .../discovery_report/show.text.erb_spec.rb | 2 +- 6 files changed, 9 insertions(+), 273 deletions(-) delete mode 100644 app/lib/bundle_context_temporary.rb delete mode 100644 spec/lib/bundle_context_temporary_spec.rb diff --git a/app/lib/bundle_context_temporary.rb b/app/lib/bundle_context_temporary.rb deleted file mode 100644 index 4f6842af1..000000000 --- a/app/lib/bundle_context_temporary.rb +++ /dev/null @@ -1,171 +0,0 @@ -class BundleUsageError < StandardError - # An exception class used to pass usage error messages - # back to users of the bin/pre-assemble script. -end - -# This is a temporary BundleConext class that will be replaced with the BundleContext ActiveRecord Model. -class BundleContextTemporary - # Paramaters passed via YAML config files. - YAML_PARAMS = [ - :accession_items, - :bundle_dir, - :config_filename, - :content_exclusion, - :content_md_creation, - :file_attr, - :manifest_cols, - :manifest, - :progress_log_file, - :project_name, - :project_style, - :stageable_discovery, - :staging_dir, - :staging_style, - :validate_files - ] - - YAML_PARAMS.each { |p| attr_accessor p } - - attr_accessor :user_params - attr_writer :manifest_rows - - # Unpack the user-supplied parameters, after converting - # all hash keys and some hash values to symbols. - def initialize(params) - params.deep_symbolize_keys! - raise ArgumentError, ':bundle_dir is required' unless params[:bundle_dir] # TODO: replace w/ AR validation - [:content_md_creation, :project_style, :stageable_discovery].each { |k| params[k] ||= {} } - params[:project_style].transform_values! { |v| v.is_a?(String) ? v.to_sym : v } - params[:project_style][:content_structure] ||= :simple_image - params[:content_md_creation][:style] ||= :default - params[:content_md_creation][:style] = params[:content_md_creation][:style].to_sym - params[:file_attr] ||= params[:publish_attr] - self.user_params = params - YAML_PARAMS.each { |p| instance_variable_set "@#{p}", params[p] } - setup_paths - setup_other - setup_defaults - validate_usage - end - - def validate_files? - return @validate_files unless @validate_files.nil? - false - end - - #### - # grab bag - #### - - def path_in_bundle(rel_path) - File.join(bundle_dir, rel_path) - end - - # On first call, loads the manifest data, caches results - # @return [Array] - def manifest_rows - @manifest_rows ||= CsvImporter.parse_to_hash(manifest) - end - - #### - # initialization helpers - #### - - def setup_paths - bundle_dir.chomp!('/') # get rid of any trailing slash on the bundle directory - self.manifest &&= path_in_bundle(manifest) - self.staging_dir ||= '/dor/assembly' - self.progress_log_file ||= File.join(File.dirname(config_filename), File.basename(config_filename, '.yaml') + '_progress.yaml') - end - - def setup_other - self.content_exclusion &&= Regexp.new(content_exclusion) - self.file_attr ||= {} - self.file_attr.delete_if { |_k, v| v.nil? } - end - - def setup_defaults - self.validate_files = true if @validate_files.nil? # FIXME: conflict between attribute and non-getter method #validate_files - self.staging_style ||= 'copy' - project_style[:content_tag_override] = false if project_style[:content_tag_override].nil? - content_md_creation[:smpl_manifest] ||= 'smpl_manifest.csv' - end - - #### - # Usage validation. - #### - - # allowed controlled vocabulary for various configuration paramaters - def allowed_values - { - :project_style => { - :content_structure => [:simple_image, :simple_book, :book_as_image, :book_with_pdf, :file, :smpl], - }, - :content_md_creation => { - :style => [:default, :filename, :dpg, :smpl, :salt, :none], - } - } - end - - def required_dirs - [bundle_dir, staging_dir] - end - - def required_user_params - YAML_PARAMS - non_required_user_params - end - - def non_required_user_params - [ - :config_filename, - :validate_files, - :staging_style, - ] - end - - # Validate parameters supplied via user script. - # Unit testing often bypasses such checks. - def validate_usage - validation_errors = [] - - required_user_params.each do |p| - next if user_params.has_key? p - validation_errors << "Missing parameter: #{p}." - end - - required_dirs.each do |d| - next if File.directory? d - validation_errors << "Required directory not found: #{d}." - end - - validation_errors << "Bundle directory not specified." if bundle_dir.nil? || bundle_dir == '' - validation_errors << "Bundle directory #{bundle_dir} not found." unless File.directory?(bundle_dir) - validation_errors << "Staging directory '#{staging_dir}' not writable." unless File.writable?(staging_dir) - validation_errors << "Progress log file '#{progress_log_file}' or directory not writable." unless File.writable?(File.dirname(progress_log_file)) - - # validation_errors << "The APO and SET DRUIDs should not be set." if apo_druid_id # APO should not be set - if manifest.blank? - validation_errors << "A manifest file must be provided." - elsif manifest_rows.size == 0 - validation_errors << "Manifest does not have any rows!" - elsif manifest_cols.blank? || manifest_cols[:object_container].blank? - validation_errors << "You must specify the name of your column which represents your object container in a parameter called 'object_container' under 'manifest_cols'" - else - first_row_keys = manifest_rows.first.keys - validation_errors << "Manifest does not have a column called '#{manifest_cols[:object_container]}'" unless first_row_keys.include?(manifest_cols[:object_container].to_s) - validation_errors << "Manifest does not have a column called '#{manifest_cols[:source_id]}'" if !manifest_cols[:source_id].blank? && !first_row_keys.include?(manifest_cols[:source_id].to_s) - validation_errors << "Manifest does not have a column called '#{manifest_cols[:label]}'" if !manifest_cols[:label].blank? && !first_row_keys.include?(manifest_cols[:label].to_s) - validation_errors << "Manifest does not have a column called 'druid'" unless first_row_keys.include?('druid') - end - - # check parameters that are part of a controlled vocabulary to be sure they don't have bogus values - validation_errors << "The project_style:content_structure value of '#{project_style[:content_structure]}' is not valid." unless allowed_values[:project_style][:content_structure].include? project_style[:content_structure] - validation_errors << "The content_md_creation:style value of '#{content_md_creation[:style]}' is not valid." unless allowed_values[:content_md_creation][:style].include? content_md_creation[:style] - validation_errors << "The SMPL manifest #{content_md_creation[:smpl_manifest]} was not found in #{bundle_dir}." if content_md_creation[:style] == :smpl && !File.exist?(File.join(bundle_dir, content_md_creation[:smpl_manifest])) - - unless validation_errors.blank? - validation_errors = ['Configuration errors found:'] + validation_errors - raise BundleUsageError, validation_errors.join(' ') unless validation_errors.blank? - end - end -end diff --git a/spec/lib/bundle_context_temporary_spec.rb b/spec/lib/bundle_context_temporary_spec.rb deleted file mode 100644 index 1b9bc5873..000000000 --- a/spec/lib/bundle_context_temporary_spec.rb +++ /dev/null @@ -1,83 +0,0 @@ -RSpec.describe BundleContextTemporary do - let(:revs_context) { context_from_proj(:proj_revs) } - - before { allow_any_instance_of(described_class).to receive(:validate_usage) } # to be replaced w/ AR validation - - describe "initialize() and other setup" do - it 'requires params' do - expect { described_class.new }.to raise_error(ArgumentError) - expect { described_class.new({}) }.to raise_error(ArgumentError) - end - - it "trims the trailing slash from the bundle directory" do - expect(revs_context.bundle_dir).to eq('spec/test_data/flat_dir_images') - end - - it '#setup_other should prune @file_attr' do - # All keys are present. - expect(revs_context.file_attr.keys.map(&:to_s).sort).to eq(%w(preserve publish shelve)) - # Keys with nil values should be removed. - revs_context.file_attr[:preserve] = nil - revs_context.file_attr[:publish] = nil - revs_context.setup_other - expect(revs_context.file_attr.keys).to eq([:shelve]) - end - end - - describe '#validate_usage' do - let(:fake_manifest) do - [{ druid: '123', sourceid: 'xyz', label: 'obj_1', filename: 'foo.jpg' }.with_indifferent_access] - end - - before do - revs_context.user_params = Hash[revs_context.required_user_params.map { |p| [p, ''] }] - allow_any_instance_of(described_class).to receive(:validate_usage).and_call_original # re-enable validation - end - - it "does not raise an exception if requirements are satisfied" do - allow(revs_context).to receive(:manifest_rows).and_return(fake_manifest) - expect { revs_context.validate_usage }.not_to raise_error - end - - it "raises exception if a user parameter is missing" do - revs_context.user_params.delete :bundle_dir - exp_msg = /^Configuration errors found: Missing parameter: / - expect { revs_context.validate_usage }.to raise_error(BundleUsageError, exp_msg) - end - - it "raises exception if required directory not found" do - revs_context.bundle_dir = '__foo_bundle_dir###' - exp_msg = /^Configuration errors found: Required directory not found/ - expect { revs_context.validate_usage }.to raise_error(BundleUsageError, exp_msg) - end - - it "raises exception if required file not found" do - revs_context.manifest = '__foo_manifest###' - expect(File).to receive(:readable?).with(revs_context.manifest).and_return(false) - expect { revs_context.validate_usage }.to raise_error(ArgumentError, /Required file not found/) - end - - it "raises an exception since the sourceID column is misspelled" do - exp_msg = /Manifest does not have a column called 'sourceid'/ - expect { context_from_proj(:proj_revs_bad_manifest).validate_usage }.to raise_error(BundleUsageError, exp_msg) - end - end - - describe '#setup_paths and defaults' do - it "sets the staging_dir to the value specified in YAML" do - expect(revs_context.staging_dir).to eq('tmp') - end - it "sets the progress log file to match the input yaml file if no progress log is specified in YAML" do - expect(context_from_proj(:proj_sohp3).progress_log_file).to eq('spec/test_data/project_config_files/proj_sohp3_progress.yaml') - end - it "sets content_tag_override to the default value when not specified" do - expect(revs_context.project_style[:content_tag_override]).to be_falsey - end - end - - describe '#staging_dir' do - it 'takes default value' do - expect(context_from_proj(:proj_sohp2).staging_dir).to eq('/dor/assembly') - end - end -end diff --git a/spec/lib/pre_assembly/bundle_spec.rb b/spec/lib/pre_assembly/bundle_spec.rb index 34af67b19..406b377ff 100644 --- a/spec/lib/pre_assembly/bundle_spec.rb +++ b/spec/lib/pre_assembly/bundle_spec.rb @@ -1,8 +1,8 @@ RSpec.describe PreAssembly::Bundle do let(:md5_regex) { /^[0-9a-f]{32}$/ } - let(:revs_context) { context_from_proj_ar_model(:proj_revs)} + let(:revs_context) { bundle_context_from_hash(:proj_revs)} let(:rumsey_context) do - context_from_proj_ar_model(:proj_rumsey).tap do |c| + bundle_context_from_hash(:proj_rumsey).tap do |c| c.manifest_cols[:object_container] = 'folder' allow(c).to receive(:path_in_bundle).with(any_args).and_call_original allow(c).to receive(:path_in_bundle).with("manifest.csv").and_return('spec/test_data/bundle_input_e/manifest_of_3.csv') @@ -17,7 +17,7 @@ allow(RestClient).to receive(:post).with(a_string_matching(exp_workflow_svc_url), {}).and_return(instance_double(RestClient::Response, code: 200)) end it 'runs images_jp2_tif cleanly using images_jp2_tif.yaml for options' do - bc = context_from_proj_ar_model('images_jp2_tif') + bc = bundle_context_from_hash('images_jp2_tif') # need to delete progress log to ensure this test doesn't skip objects already run File.delete(bc.progress_log_file) if File.exist?(bc.progress_log_file) @@ -60,7 +60,7 @@ describe '#digital_objects' do it "finds the correct number of objects" do - b = bundle_setup_ar_model(:folder_manifest) + b = bundle_setup(:folder_manifest) b.manifest_rows.each {|row| row.merge!("object" => row["folder"]) } expect(b.digital_objects.size).to eq(3) @@ -292,7 +292,7 @@ "cp898cs9946/descMetadata.xml", ], }.each do |proj, files| - b = described_class.new(context_from_proj_ar_model(proj)) + b = described_class.new(bundle_context_from_hash(proj)) exp_files = files.map { |f| b.path_in_bundle f } expect(b.find_files_recursively(b.bundle_dir).sort).to eq(exp_files) end diff --git a/spec/services/discover_report_spec.rb b/spec/services/discover_report_spec.rb index 3d7800c07..064936391 100644 --- a/spec/services/discover_report_spec.rb +++ b/spec/services/discover_report_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' RSpec.describe DiscoveryReport do - let(:bundle) { bundle_setup_ar_model(:proj_revs) } + let(:bundle) { bundle_setup(:proj_revs) } subject(:report) { described_class.new(bundle) } before do diff --git a/spec/support/bundle_setup.rb b/spec/support/bundle_setup.rb index 458be9d1d..01eeeec33 100644 --- a/spec/support/bundle_setup.rb +++ b/spec/support/bundle_setup.rb @@ -1,11 +1,6 @@ -# @param [#to_s] proj basename of YAML fixture file # @return [PreAssembly::Bundle] def bundle_setup(proj) - PreAssembly::Bundle.new(context_from_proj(proj)) -end - -def bundle_setup_ar_model(proj) - PreAssembly::Bundle.new(context_from_proj_ar_model(proj)) + PreAssembly::Bundle.new(bundle_context_from_hash(proj)) end def hash_from_proj(proj) @@ -13,16 +8,11 @@ def hash_from_proj(proj) YAML.load(File.read(filename)).merge('config_filename' => filename) end -def context_from_proj(proj) - BundleContextTemporary.new(hash_from_proj(proj)) -end - def noko_doc(x) Nokogiri.XML(x) { |conf| conf.default_xml.noblanks } end - -def context_from_proj_ar_model(proj) +def bundle_context_from_hash(proj) user = User.create(sunet_id: "Jdoe") cmc = hash_from_proj(proj)["content_md_creation"]["style"] diff --git a/spec/views/discovery_report/show.text.erb_spec.rb b/spec/views/discovery_report/show.text.erb_spec.rb index d752ced45..5f8ad0fba 100644 --- a/spec/views/discovery_report/show.text.erb_spec.rb +++ b/spec/views/discovery_report/show.text.erb_spec.rb @@ -1,5 +1,5 @@ RSpec.describe 'discovery_report/show.text.erb' do - let(:bundle) { bundle_setup_ar_model(:proj_revs) } + let(:bundle) { bundle_setup(:proj_revs) } subject(:report) { DiscoveryReport.new(bundle) } before do