diff --git a/app/controllers/bundle_contexts_controller.rb b/app/controllers/bundle_contexts_controller.rb index c9ab1aa7..5ef50f6f 100644 --- a/app/controllers/bundle_contexts_controller.rb +++ b/app/controllers/bundle_contexts_controller.rb @@ -5,6 +5,9 @@ def index def create @bundle_context = BundleContext.new(bundle_contexts_params) + + # TODO: present a nicer error in the app than default Rails error page (#310) + # (there is a before_save callback to create the bc.output_dir) if @bundle_context.save render :create else diff --git a/app/models/bundle_context.rb b/app/models/bundle_context.rb index dcb65e80..f26c6a03 100644 --- a/app/models/bundle_context.rb +++ b/app/models/bundle_context.rb @@ -1,6 +1,9 @@ +require 'fileutils' + class BundleContext < ApplicationRecord belongs_to :user has_many :job_runs + before_save :verify_output_dir validates :bundle_dir, :content_metadata_creation, :content_structure, :project_name, presence: true validates :staging_style_symlink, inclusion: { in: [true, false] } @@ -44,37 +47,41 @@ def assembly_staging_dir Settings.assembly_staging_dir end - def normalize_bundle_dir - self[:bundle_dir].chomp("/") if bundle_dir + def output_dir + @output_dir ||= "#{normalize_dir(Settings.job_output_parent_dir)}/#{user.sunet_id}/#{bundle_dir}" end def progress_log_file - '/dor/preassembly' # FIXME: (#78) + @progress_log_file ||= File.join(output_dir, "#{project_name}_progress.yml") end + # TODO: See #274. Possibly need to keep for SMPL style projects (if they don't use manifest?) def stageable_discovery {} end + # TODO: delete everywhere in code as single commit (#262) def accession_items nil end + # TODO: Delete everywhere in code as single commit (#227) def content_exclusion - # FIXME: Delete everywhere in code (#227) nil end + # TODO: Delete everywhere in code as single commit (#228) def file_attr - nil # FIXME: can get rid of this (#228) + nil end + # TODO: delete everwhere in code as single commit (#230) def validate_files? - false # FIXME: delete everwhere in code (#230) + false end + # TODO: find where this is used as a conditional and delete code that won't be executed and this method (#231) def content_tag_override? - # TODO: find where this is used as a conditional and delete code that won't be executed (#231) true end @@ -98,8 +105,8 @@ def manifest_rows def manifest_cols { - label: 'label', - source_id: 'sourceid', + label: 'label', # only used by SMPL manifests + source_id: 'sourceid', # only used by SMPL manifests object_container: 'object', # object referring to filename or foldername druid: 'druid' } @@ -107,12 +114,39 @@ def manifest_cols private + def job_output_parent_dir + @job_output_parent_dir ||= Settings.job_output_parent_dir + end + + def normalize_dir(dir) + dir.chomp('/') if dir + end + + def normalize_bundle_dir + self[:bundle_dir] = normalize_dir(bundle_dir) + end + + def verify_output_dir + if persisted? + # FIXME: or should we just try to create it? + raise "Output directory (#{output_dir}) should already exist, but doesn't" unless Dir.exist?(output_dir) + else + raise "Output directory (#{output_dir}) should not already exist" if Dir.exist?(output_dir) + begin + FileUtils.mkdir_p(output_dir) + rescue SystemCallError => e + raise "Unable to create output directory (#{@output_dir}): #{e.message}" + end + end + end + def verify_bundle_directory return if errors.key?(:bundle_dir) errors.add(:bundle_dir, "Bundle directory: #{bundle_dir} not found.") unless File.directory?(bundle_dir) end def verify_content_metadata_creation - errors.add(:content_metadata_creation, "The SMPL manifest #{smpl_manifest} was not found in #{bundle_dir}.") if smpl_cm_style? && !File.exist?(File.join(bundle_dir, smpl_manifest)) + return unless smpl_cm_style? + errors.add(:content_metadata_creation, "The SMPL manifest #{smpl_manifest} was not found in #{bundle_dir}.") unless File.exist?(File.join(bundle_dir, smpl_manifest)) end end diff --git a/db/schema.rb b/db/schema.rb index 6f48cccf..bf30d58a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -12,13 +12,16 @@ ActiveRecord::Schema.define(version: 2018_09_14_014133) do + # These are extensions that must be enabled in order to support this database + enable_extension "plpgsql" + create_table "bundle_contexts", force: :cascade do |t| t.string "project_name", null: false t.integer "content_structure", null: false t.string "bundle_dir", null: false t.boolean "staging_style_symlink", default: false, null: false t.integer "content_metadata_creation", null: false - t.integer "user_id", null: false + t.bigint "user_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false t.index ["user_id"], name: "index_bundle_contexts_on_user_id" @@ -27,7 +30,7 @@ create_table "job_runs", force: :cascade do |t| t.string "output_location" t.integer "job_type", null: false - t.integer "bundle_context_id", null: false + t.bigint "bundle_context_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false t.index ["bundle_context_id"], name: "index_job_runs_on_bundle_context_id" @@ -40,4 +43,6 @@ t.index ["sunet_id"], name: "index_users_on_sunet_id", unique: true end + add_foreign_key "bundle_contexts", "users" + add_foreign_key "job_runs", "bundle_contexts" end diff --git a/spec/controllers/bundle_contexts_controller_spec.rb b/spec/controllers/bundle_contexts_controller_spec.rb index bd612c15..c0985490 100644 --- a/spec/controllers/bundle_contexts_controller_spec.rb +++ b/spec/controllers/bundle_contexts_controller_spec.rb @@ -38,7 +38,11 @@ context "POST create" do context "Valid Parameters" do - before { post :create, params: params } + let(:output_dir) { "#{Settings.job_output_parent_dir}/#{subject.current_user.sunet_id}/spec/test_data/smpl_multimedia" } + before do + Dir.delete(output_dir) if Dir.exist?(output_dir) + post :create, params: params + end it 'passes newly created object' do expect(assigns(:bundle_context)).to be_a(BundleContext).and be_persisted @@ -53,10 +57,12 @@ expect(bc.bundle_dir).to eq "spec/test_data/smpl_multimedia" end it "persists the JobRun" do + Dir.delete(output_dir) if Dir.exist?(output_dir) expect { post :create, params: params }.to change(JobRun, :count).by(1) end it "fails if job_type is nil" do + Dir.delete(output_dir) if Dir.exist?(output_dir) params[:bundle_context].merge!(job_runs_attributes: {"0" => { job_type: "" }}) expect { post :create, params: params }.not_to change(JobRun, :count) end diff --git a/spec/models/bundle_context_spec.rb b/spec/models/bundle_context_spec.rb index c9a8ccc7..a90e0457 100644 --- a/spec/models/bundle_context_spec.rb +++ b/spec/models/bundle_context_spec.rb @@ -48,6 +48,10 @@ it { is_expected.to validate_presence_of(:content_metadata_creation) } it { is_expected.to belong_to(:user) } + it 'bundle_dir has trailing slash removed' do + expect(bc.bundle_dir).to eq "spec/test_data/images_jp2_tif" + end + describe '#bundle' do it 'returns a PreAssembly::Bundle' do expect(bc.bundle).to be_a(PreAssembly::Bundle) @@ -63,12 +67,6 @@ end end - describe "#normalize_bundle_dir" do - it "removes the trailing forward slash" do - expect(bc.normalize_bundle_dir).to eq "spec/test_data/images_jp2_tif" - end - end - describe "#content_tag_override?" do it "is set to true" do expect(described_class.new.content_tag_override?).to be true @@ -93,8 +91,19 @@ end end + describe 'output_dir' do + it 'returns "Settings.job_output_parent_dir/user_id/bundle_dir"' do + # Settings.job_output_parent_dir for test from config/settings/test.yml as 'log/test_jobs' + # bc.user.user_id above as 'Jdoe' + # bc.bundle_dir above as 'spec/test_data/images_jp2_tif' + expect(bc.output_dir).to eq 'log/test_jobs/Jdoe/spec/test_data/images_jp2_tif' + end + end + describe "#progress_log_file" do - skip("Need to figure out where to set this path via planning meeting 9/10/18") + it 'is project_name + "_progress.yml" in output_dir' do + expect(bc.progress_log_file).to eq "#{bc.output_dir}/#{bc.project_name}_progress.yml" + end end describe "manifest_rows" do @@ -129,4 +138,44 @@ ) end end + + describe '#verify_output_dir (private method)' do + before(:each) do + FileUtils.mkdir_p(bc.send(:job_output_parent_dir)) + end + context 'bundle_context is new' do + it 'creates directory' do + bc.bundle_dir = 'i_do_not_exist' + Dir.delete(bc.output_dir) if Dir.exist?(bc.output_dir) + expect(Dir.exist?(bc.output_dir)).to eq false + bc.send(:verify_output_dir) + expect(Dir.exist?(bc.output_dir)).to eq true + Dir.delete(bc.output_dir) + end + it 'raises error if directory already exists' do + bc.bundle_dir = 'i_exist' + FileUtils.mkdir_p(bc.output_dir) unless Dir.exist?(bc.output_dir) + expect(Dir.exist?(bc.output_dir)).to eq true + exp_msg = "Output directory (#{bc.output_dir}) should not already exist" + expect { bc.send(:verify_output_dir) }.to raise_error(RuntimeError, exp_msg) + Dir.delete(bc.output_dir) + end + it "raises error if directory can't be created" do + orig = Settings.job_output_parent_dir + Settings.job_output_parent_dir = '/boot' + regex_exp_msg = Regexp.new(Regexp.escape("Unable to create output directory (#{bc.output_dir}): Permission denied @ dir_s_mkdir - /boot")) + expect { bc.send(:verify_output_dir) }.to raise_error(RuntimeError, regex_exp_msg) + Settings.job_output_parent_dir = orig + end + end + context 'bundle_context is not new' do + it "raises error if directory doesn't exist" do + Dir.delete(bc.output_dir) if Dir.exist?(bc.output_dir) + bc.save # creates output dir + Dir.delete(bc.output_dir) if Dir.exist?(bc.output_dir) + exp_msg = "Output directory (#{bc.output_dir}) should already exist, but doesn't" + expect { bc.send(:verify_output_dir) }.to raise_error(RuntimeError, exp_msg) + end + end + end end