From ad5e64590f91a8fdf2c3ff812b278ba73eee487e Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Sun, 7 Jun 2020 15:39:36 -0400 Subject: [PATCH] Renaming method from whitelisted to registered 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. --- .../actors/create_with_remote_files_actor.rb | 6 ++--- app/models/content_block.rb | 8 ++++++- .../templates/config/initializers/hyrax.rb | 4 ++-- lib/hyrax/configuration.rb | 22 +++++++++++++++---- .../create_with_remote_files_actor_spec.rb | 10 ++++----- spec/lib/hyrax/configuration_spec.rb | 8 +++---- spec/models/content_block_spec.rb | 6 ++--- 7 files changed, 42 insertions(+), 22 deletions(-) diff --git a/app/actors/hyrax/actors/create_with_remote_files_actor.rb b/app/actors/hyrax/actors/create_with_remote_files_actor.rb index 0e737fb982..83261d8c07 100644 --- a/app/actors/hyrax/actors/create_with_remote_files_actor.rb +++ b/app/actors/hyrax/actors/create_with_remote_files_actor.rb @@ -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 diff --git a/app/models/content_block.rb b/app/models/content_block.rb index ed004dcbc8..cf4f6556a4 100644 --- a/app/models/content_block.rb +++ b/app/models/content_block.rb @@ -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 diff --git a/lib/generators/hyrax/templates/config/initializers/hyrax.rb b/lib/generators/hyrax/templates/config/initializers/hyrax.rb index 619c4dc351..afb4d26008 100644 --- a/lib/generators/hyrax/templates/config/initializers/hyrax.rb +++ b/lib/generators/hyrax/templates/config/initializers/hyrax.rb @@ -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 @@ -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 diff --git a/lib/hyrax/configuration.rb b/lib/hyrax/configuration.rb index acea83ff19..a334111753 100644 --- a/lib/hyrax/configuration.rb +++ b/lib/hyrax/configuration.rb @@ -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 diff --git a/spec/actors/hyrax/actors/create_with_remote_files_actor_spec.rb b/spec/actors/hyrax/actors/create_with_remote_files_actor_spec.rb index 93f41b9d4f..0e5d6878e5 100644 --- a/spec/actors/hyrax/actors/create_with_remote_files_actor_spec.rb +++ b/spec/actors/hyrax/actors/create_with_remote_files_actor_spec.rb @@ -84,7 +84,7 @@ 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 @@ -92,7 +92,7 @@ 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 @@ -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 diff --git a/spec/lib/hyrax/configuration_spec.rb b/spec/lib/hyrax/configuration_spec.rb index 8663b8f33b..f59ed68e4a 100644 --- a/spec/lib/hyrax/configuration_spec.rb +++ b/spec/lib/hyrax/configuration_spec.rb @@ -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 diff --git a/spec/models/content_block_spec.rb b/spec/models/content_block_spec.rb index 4a31e1b127..80d923df6f 100644 --- a/spec/models/content_block_spec.rb +++ b/spec/models/content_block_spec.rb @@ -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 @@ -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