Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bundle_context class to bundle_context AR Model #232

Merged
merged 1 commit into from
Sep 11, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 83 additions & 0 deletions app/models/bundle_context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ class BundleContext < ApplicationRecord
validates :staging_style_symlink, inclusion: { in: [ true, false ] }
validates :content_metadata_creation, presence: true, null: false

validate :verify_bundle_directory
validate :verify_content_metadata_creation

after_initialize :normalize_bundle_dir

enum content_structure: {
"simple_image_structure" => 0,
"simple_book_structure" => 1,
Expand All @@ -21,4 +26,82 @@ class BundleContext < ApplicationRecord
"smpl_style" => 2
}

def staging_dir
'/dor/assembly'
end

def normalize_bundle_dir
self[:bundle_dir].chomp("/") if bundle_dir
end

def progress_log_file
Tempfile.new.path(id) #FIXME: (#78)
end

def content_exclusion #FIXME: Delete everywhere in code (#227)
nil
end

def file_attr
nil # FIXME can get rid of this (#228)
end

def validate_files?
false #FIXME delete everwhere in code (#230)
end

def content_tag_override? #TODO: find where this is used as a conditional and delete code that won't be executed (#231)
true
end

def smpl_manifest
'smpl_manifest.csv'
end

def manifest
'manifest.csv'
end

def path_in_bundle(rel_path)
File.join(bundle_dir, rel_path)
end

# TODO: BundleContext is not really a logical home for this util method
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hrmm - not sure I agree, as reading the file from the user supplied location is needed to flesh out the bundle context data ...

Also - why is this a class method? should it be a private instance method directly operating on path_in_bundle(manifest) ? I think this is the only place where we read a csv file, unless @atz envisions writing out discovery report data as a csv (which could be a good idea, if folks work with the old discovery report that way - as it was designed)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, i'd left this comment a while back (after splitting out BundleContext from Bundle, but before sarav moved it to its own file). at that time, Bundle also used import_csv. but i just looked, and it doesn't use it any longer. so yeah, now it could just be a private instance method in this class, which is now a logical home for it.

i think that also means we can get rid of this delegation, and the require 'csv' at the top of bundle.rb:
https://github.com/sul-dlss/pre-assembly/blob/master/app/lib/pre_assembly/bundle.rb#L3
https://github.com/sul-dlss/pre-assembly/blob/master/app/lib/pre_assembly/bundle.rb#L41-L43

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cleaned up in #244

# load CSV allowing UTF-8 to pass through, deleting blank columns
# @param [String] filename
# @return [Array<ActiveSupport::HashWithIndifferentAccess>]
# @raise if file missing/unreadable
def self.import_csv(filename)
raise BundleUsageError, "CSV filename required" unless filename.present?
raise BundleUsageError, "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)
csv.map { |row| row.to_hash.with_indifferent_access }
end

# On first call, loads the manifest data, caches results
# @return [Array<ActiveSupport::HashWithIndifferentAccess>]
def manifest_rows
@manifest_rows ||= self.class.import_csv(path_in_bundle(manifest))
end

def manifest_cols
{
label: 'label',
source_id: 'sourceid',
object_container: 'object', #object referring to filename or foldername
druid: 'druid'
}
end

private

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_style? && !File.exist?(File.join(bundle_dir, smpl_manifest))
jmartin-sul marked this conversation as resolved.
Show resolved Hide resolved
end
end
1 change: 0 additions & 1 deletion config/projects/TEMPLATE.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ stageable_discovery:
# 00/
# 1.tif
# 2.tif
# 01/
jmartin-sul marked this conversation as resolved.
Show resolved Hide resolved
# 1.jp2
# 2.jp2
#
Expand Down
119 changes: 109 additions & 10 deletions spec/models/bundle_context_spec.rb
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
require 'rails_helper'

RSpec.describe BundleContext, type: :model do
context "validation" do
let(:user) { User.new(sunet_id: "Jdoe")}
subject(:bc) { BundleContext.new(project_name: "SmokeTest",
content_structure: 1,
bundle_dir: "spec/test_data/bundle_input_g",
staging_style_symlink: false,
content_metadata_creation: 1,
user: user) }
subject(:bc) do
jmartin-sul marked this conversation as resolved.
Show resolved Hide resolved
BundleContext.new(
project_name: "SmokeTest",
content_structure: 1,
bundle_dir: "spec/test_data/bundle_input_g/",
staging_style_symlink: false,
content_metadata_creation: 1,
user: user
)
end

let(:user) { User.new(sunet_id: "Jdoe") }

context "validation" do
it "is not valid unless it has all required attributes" do
expect(BundleContext.new).not_to be_valid
expect(bc).to be_valid
jmartin-sul marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -61,10 +64,106 @@
end
end

context "bundle_dir path does not exist" do
it "object does not pass validation" do
expect { bc.bundle_dir = 'does/not/exist' }.to change { bc.valid? }.to(false)
end
end

it { is_expected.to validate_presence_of(:project_name) }
it { is_expected.to validate_presence_of(:content_structure) }
it { is_expected.to validate_presence_of(:bundle_dir) }
it { is_expected.to validate_presence_of(:content_metadata_creation) }
it { is_expected.to belong_to(:user) }
end

describe "#staging_dir" do
it 'is hardcoded to the correct path' do
expect(described_class.new.staging_dir).to eq '/dor/assembly'
end
end

describe "#normalize_bundle_dir" do
it "removes the trailing forward slash" do
expect(bc.normalize_bundle_dir).to eq "spec/test_data/bundle_input_g"
end
end

describe "#content_tag_override?" do
it "is set to true" do
expect(described_class.new.content_tag_override?).to be true
end
end

describe "#smpl_manifest" do
it "returns the file name" do
expect(described_class.new.smpl_manifest).to eq 'smpl_manifest.csv'
end
end

describe "#manifest" do
it "returns the file name" do
expect(described_class.new.manifest).to eq 'manifest.csv'
end
end

describe "#path_in_bundle" do
it "creates a relative path" do
expect(bc.path_in_bundle("manifest.csv")).to eq "spec/test_data/bundle_input_g/manifest.csv"
end
end

describe "#progress_log_file" do
skip("Need to figure out where to set this path via planning meeting 9/10/18")
end

describe '#import_csv' do
let(:manifest) do
described_class.import_csv("#{Rails.root}/spec/test_data/bundle_input_a/manifest.csv")
end

it "loads a CSV as a hash with indifferent access" do
expect(manifest).to be_an(Array)
expect(manifest.size).to eq(3)
headers = %w{format sourceid filename label year inst_notes prod_notes has_more_metadata description}
expect(manifest).to all(be_an(ActiveSupport::HashWithIndifferentAccess)) # accessible w/ string and symbols
expect(manifest).to all(include(*headers))
expect(manifest[0][:description]).to be_nil
expect(manifest[1][:description]).to eq('')
expect(manifest[2][:description]).to eq('yo, this is a description')
end
end

describe "manifest_rows" do
jmartin-sul marked this conversation as resolved.
Show resolved Hide resolved
it "loads the manifest CSV" do
expect(described_class).to receive(:import_csv).with("spec/test_data/bundle_input_g/manifest.csv")
bc.manifest_rows
end

it "memoizes the manifest rows" do
expect(described_class).to receive(:import_csv).once.with("spec/test_data/bundle_input_g/manifest.csv").and_call_original
2.times { bc.manifest_rows }
end

it "expect the content of manifest rows" do
expect(bc.manifest_rows).to eq(
[
{"druid"=>"druid:jy812bp9403", "sourceid"=>"bar-1.0", "folder"=>"jy812bp9403", "label"=>"Label 1", "description"=>"This is a description for label 1"},
{"druid"=>"druid:tz250tk7584", "sourceid"=>"bar-2.1", "folder"=>"tz250tk7584", "label"=>"Label 2", "description"=>"This is a description for label 2"},
{"druid"=>"druid:gn330dv6119", "sourceid"=>"bar-3.1", "folder"=>"gn330dv6119", "label"=>"Label 3", "description"=>"This is a description for label 3"}
]
)
end
end

describe "manifest_cols" do
it "sets the column names" do
expect(bc.manifest_cols).to eq(
label: 'label',
source_id: 'sourceid',
object_container: 'object', # object referring to filename or foldername
druid: 'druid'
)
end
end
end