Skip to content

Commit

Permalink
Renaming method from whitelisted to registered
Browse files Browse the repository at this point in the history
The words we use matter.  They perpetuate stereotypes, enforcing the
idea that "black" is unacceptable and "white" is acceptable.

There continue to be references to white and black list. However,
those appear to come from upstream dependencies.
  • Loading branch information
jeremyf committed Jun 7, 2020
1 parent 1bde709 commit ad5e645
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 22 deletions.
6 changes: 3 additions & 3 deletions app/actors/hyrax/actors/create_with_remote_files_actor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ def update(env)

private

def whitelisted_ingest_dirs
Hyrax.config.whitelisted_ingest_dirs
def registered_ingest_dirs
Hyrax.config.registered_ingest_dirs
end

# @param uri [URI] the uri fo the resource to import
def validate_remote_url(uri)
if uri.scheme == 'file'
path = File.absolute_path(CGI.unescape(uri.path))
whitelisted_ingest_dirs.any? do |dir|
registered_ingest_dirs.any? do |dir|
path.start_with?(dir) && path.length > dir.length
end
else
Expand Down
8 changes: 7 additions & 1 deletion app/models/content_block.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,18 @@ class ContentBlock < ActiveRecord::Base
# `for` is a reserved word in Ruby.
def self.for(key)
key = key.respond_to?(:to_sym) ? key.to_sym : key
raise ArgumentError, "#{key} is not a ContentBlock name" unless whitelisted?(key)
raise ArgumentError, "#{key} is not a ContentBlock name" unless registered?(key)
ContentBlock.public_send(NAME_REGISTRY[key])
end

class << self
# @deprecated
def whitelisted?(key)
Deprecation.warn(self, "Samvera is deprecating '#{self}.whitelisted?' in Hyrax 3.0. Use #{self}.registered? instead.")
registered?(key)
end

def registered?(key)
NAME_REGISTRY.include?(key)
end

Expand Down
4 changes: 2 additions & 2 deletions lib/generators/hyrax/templates/config/initializers/hyrax.rb
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@
config.browse_everything = nil
end

## Whitelist all directories which can be used to ingest from the local file
## Register all directories which can be used to ingest from the local file
# system.
#
# Any file, and only those, that is anywhere under one of the specified
Expand All @@ -275,7 +275,7 @@
# ingest files from the file system that are not part of the BrowseEverything
# mount point.
#
# config.whitelisted_ingest_dirs = []
# config.registered_ingest_dirs = []

##
# Set the system-wide virus scanner
Expand Down
22 changes: 18 additions & 4 deletions lib/hyrax/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -195,11 +195,25 @@ def virus_scanner
end
end

# @!attribute [w] whitelisted_ingest_dirs
# List of directories which can be used for local file system ingestion.
attr_writer :whitelisted_ingest_dirs
# @deprecated
def whitelisted_ingest_dirs
@whitelisted_ingest_dirs ||= \
Deprecation.warn(self, "Samvera is deprecating #{self.class}#whitelisted_ingest_dirs " \
"in Hyrax 3.0. Instead use #{self.class}#registered_ingest_dirs.")
registered_ingest_dirs
end

# @deprecated
def whitelisted_ingest_dirs=(input)
Deprecation.warn(self, "Samvera is deprecating #{self.class}#whitelisted_ingest_dirs= " \
"in Hyrax 3.0. Instead use #{self.class}#registered_ingest_dirs=.")
self.registered_ingest_dirs = input
end

# @!attribute [w] registered_ingest_dirs
# List of directories which can be used for local file system ingestion.
attr_writer :registered_ingest_dirs
def registered_ingest_dirs
@registered_ingest_dirs ||= \
if defined? BrowseEverything
file_system_dirs = Array.wrap(BrowseEverything.config['file_system'].try(:[], :home)).compact
# Include the Rails tmp directory for cases where the BrowseEverything provider is required to download the file to a temporary directory first
Expand Down
10 changes: 5 additions & 5 deletions spec/actors/hyrax/actors/create_with_remote_files_actor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,15 @@
end

before do
allow(Hyrax.config).to receive(:whitelisted_ingest_dirs).and_return(["/local/file/"])
allow(Hyrax.config).to receive(:registered_ingest_dirs).and_return(["/local/file/"])
end

it "attaches files" do
expect(IngestLocalFileJob).to receive(:perform_later).with(FileSet, "/local/file/here.txt", user)
expect(actor.create(environment)).to be true
end

context "with files from non-whitelisted directories" do
context "with files from non-registered directories" do
let(:file) { "file:///local/otherdir/test.txt" }

it "doesn't attach files" do
Expand All @@ -114,16 +114,16 @@

describe "#validate_remote_url" do
before do
allow(Hyrax.config).to receive(:whitelisted_ingest_dirs).and_return(['/test/', '/local/file/'])
allow(Hyrax.config).to receive(:registered_ingest_dirs).and_return(['/test/', '/local/file/'])
end

it "accepts file: urls in whitelisted directories" do
it "accepts file: urls in registered directories" do
expect(actor.send(:validate_remote_url, URI('file:///local/file/test.txt'))).to be true
expect(actor.send(:validate_remote_url, URI('file:///local/file/subdirectory/test.txt'))).to be true
expect(actor.send(:validate_remote_url, URI('file:///test/test.txt'))).to be true
end

it "rejects file: urls outside whitelisted directories" do
it "rejects file: urls outside registered directories" do
expect(actor.send(:validate_remote_url, URI('file:///tmp/test.txt'))).to be false
expect(actor.send(:validate_remote_url, URI('file:///test/../tmp/test.txt'))).to be false
expect(actor.send(:validate_remote_url, URI('file:///test/'))).to be false
Expand Down
8 changes: 4 additions & 4 deletions spec/lib/hyrax/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,13 @@
it { is_expected.to respond_to(:translate_id_to_uri) }
it { is_expected.to respond_to(:translate_uri_to_id) }
it { is_expected.to respond_to(:upload_path) }
it { is_expected.to respond_to(:whitelisted_ingest_dirs) }
it { is_expected.to respond_to(:whitelisted_ingest_dirs=) }
it { is_expected.to respond_to(:registered_ingest_dirs) }
it { is_expected.to respond_to(:registered_ingest_dirs=) }
it { is_expected.to respond_to(:work_requires_files?) }

describe "#whitelisted_ingest_dirs" do
describe "#registered_ingest_dirs" do
it "provides the Rails tmp directory for temporary downloads for cloud files" do
expect(configuration.whitelisted_ingest_dirs).to include(Rails.root.join('tmp').to_s)
expect(configuration.registered_ingest_dirs).to include(Rails.root.join('tmp').to_s)
end
end
end
6 changes: 3 additions & 3 deletions spec/models/content_block_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
expect { described_class.for(nil) }.to raise_error(ArgumentError)
end
end
context 'with a non-whitelisted value' do
context 'with a non-registered value' do
it 'raises an ArgumentError' do
expect { described_class.for(:destroy_all) }.to raise_error(ArgumentError)
end
end
context 'with a whitelisted value as a symbol' do
context 'with a registered value as a symbol' do
subject { described_class.for(:about) }

it 'returns a new instance' do
Expand All @@ -19,7 +19,7 @@
expect(subject).to be_persisted
end
end
context 'with a whitelisted value as a string' do
context 'with a registered value as a string' do
subject { described_class.for('about') }

it 'returns a new instance' do
Expand Down

0 comments on commit ad5e645

Please sign in to comment.