From 179fd9c965e61609c49c51f2c7c2b56aa209f75e Mon Sep 17 00:00:00 2001 From: Sven Dunemann Date: Mon, 18 May 2020 09:47:25 +0200 Subject: [PATCH 1/6] Feat: Decision Inheritance --- lib/license_finder/decisions.rb | 62 +++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 19 deletions(-) diff --git a/lib/license_finder/decisions.rb b/lib/license_finder/decisions.rb index e2b0b0d7f..1779dd7b8 100644 --- a/lib/license_finder/decisions.rb +++ b/lib/license_finder/decisions.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'open-uri' + module LicenseFinder class Decisions ###### @@ -75,37 +77,37 @@ def initialize end def add_package(name, version, txn = {}) - @decisions << [:add_package, name, version, txn] + add_decision [:add_package, name, version, txn] @packages << ManualPackage.new(name, version) self end def remove_package(name, txn = {}) - @decisions << [:remove_package, name, txn] + add_decision [:remove_package, name, txn] @packages.delete(ManualPackage.new(name)) self end def license(name, lic, txn = {}) - @decisions << [:license, name, lic, txn] + add_decision [:license, name, lic, txn] @licenses[name] << License.find_by_name(lic) self end def unlicense(name, lic, txn = {}) - @decisions << [:unlicense, name, lic, txn] + add_decision [:unlicense, name, lic, txn] @licenses[name].delete(License.find_by_name(lic)) self end def homepage(name, homepage, txn = {}) - @decisions << [:homepage, name, homepage, txn] + add_decision [:homepage, name, homepage, txn] @homepages[name] = homepage self end def approve(name, txn = {}) - @decisions << [:approve, name, txn] + add_decision [:approve, name, txn] versions = [] versions = @approvals[name][:safe_versions] if @approvals.key?(name) @@ -115,71 +117,94 @@ def approve(name, txn = {}) end def unapprove(name, txn = {}) - @decisions << [:unapprove, name, txn] + add_decision [:unapprove, name, txn] @approvals.delete(name) self end def permit(lic, txn = {}) - @decisions << [:permit, lic, txn] + add_decision [:permit, lic, txn] @permitted << License.find_by_name(lic) self end def unpermit(lic, txn = {}) - @decisions << [:unpermit, lic, txn] + add_decision [:unpermit, lic, txn] @permitted.delete(License.find_by_name(lic)) self end def restrict(lic, txn = {}) - @decisions << [:restrict, lic, txn] + add_decision [:restrict, lic, txn] @restricted << License.find_by_name(lic) self end def unrestrict(lic, txn = {}) - @decisions << [:unrestrict, lic, txn] + add_decision [:unrestrict, lic, txn] @restricted.delete(License.find_by_name(lic)) self end def ignore(name, txn = {}) - @decisions << [:ignore, name, txn] + add_decision [:ignore, name, txn] @ignored << name self end def heed(name, txn = {}) - @decisions << [:heed, name, txn] + add_decision [:heed, name, txn] @ignored.delete(name) self end def ignore_group(name, txn = {}) - @decisions << [:ignore_group, name, txn] + add_decision [:ignore_group, name, txn] @ignored_groups << name self end def heed_group(name, txn = {}) - @decisions << [:heed_group, name, txn] + add_decision [:heed_group, name, txn] @ignored_groups.delete(name) self end def name_project(name, txn = {}) - @decisions << [:name_project, name, txn] + add_decision [:name_project, name, txn] @project_name = name self end def unname_project(txn = {}) - @decisions << [:unname_project, txn] + add_decision [:unname_project, txn] @project_name = nil self end + def inherit_from(filepath) + decisions = + if filepath =~ %r{^https?://} + open(filepath).read + else + Pathname(filepath).read + end + + add_decision [:inherit_from, filepath] + restore_inheritance(decisions) + end + + def add_decision(decision) + @decisions << decision unless @inherited + end + + def restore_inheritance(decisions) + @inherited = true + self.class.restore(decisions, self) + @inherited = false + self + end + ######### # PERSIST ######### @@ -192,8 +217,7 @@ def save!(file) write!(persist, file) end - def self.restore(persisted) - result = new + def self.restore(persisted, result = new) return result unless persisted actions = YAML.load(persisted) From ea6850254dd6dbfaf30a075bc9ec895b60a6a1a7 Mon Sep 17 00:00:00 2001 From: Sven Dunemann Date: Mon, 18 May 2020 10:22:31 +0200 Subject: [PATCH 2/6] Add specs for inherit_from decisions --- spec/lib/license_finder/decisions_spec.rb | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/spec/lib/license_finder/decisions_spec.rb b/spec/lib/license_finder/decisions_spec.rb index 837a9c92f..1641028c3 100644 --- a/spec/lib/license_finder/decisions_spec.rb +++ b/spec/lib/license_finder/decisions_spec.rb @@ -280,6 +280,22 @@ module LicenseFinder end end + describe '.inherit_from' do + let(:yml) { YAML.dump([[:permit, 'MIT']]) } + + it 'inheritates rules from local decision file' do + allow_any_instance_of(Pathname).to receive(:read).and_return(yml) + decisions = subject.inherit_from('./config/inherit.yml') + expect(decisions).to be_permitted(License.find_by_name('MIT')) + end + + it 'inheritates rules from remote decision file' do + stub_request(:get, 'https://example.com/config/inherit.yml').to_return(status: 200, body: yml, headers: {}) + decisions = subject.inherit_from('https://example.com/config/inherit.yml') + expect(decisions).to be_permitted(License.find_by_name('MIT')) + end + end + describe 'persistence' do def roundtrip(decisions) described_class.restore(decisions.persist) @@ -446,6 +462,12 @@ def roundtrip(decisions) expect(decisions.project_name).to be_nil end + it 'does not store decisions from inheritance' do + allow_any_instance_of(Pathname).to receive(:read).and_return(YAML.dump([[:permit, 'MIT']])) + decisions = subject.inherit_from('./config/inherit.yml') + expect(decisions.persist).to eql(YAML.dump([[:inherit_from, './config/inherit.yml']])) + end + it 'ignores empty or missing persisted decisions' do described_class.restore('') described_class.restore(nil) From ffe4a5376c153fc3f9e2f84f3c644b473701a0ac Mon Sep 17 00:00:00 2001 From: Sven Dunemann Date: Mon, 18 May 2020 10:58:46 +0200 Subject: [PATCH 3/6] Add new inherited_decisions command --- lib/license_finder/cli.rb | 1 + lib/license_finder/cli/inherited_decisions.rb | 32 +++++++++++++++++++ lib/license_finder/cli/main.rb | 1 + lib/license_finder/decisions.rb | 9 +++++- 4 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 lib/license_finder/cli/inherited_decisions.rb diff --git a/lib/license_finder/cli.rb b/lib/license_finder/cli.rb index 8d0be8247..ef764aba5 100644 --- a/lib/license_finder/cli.rb +++ b/lib/license_finder/cli.rb @@ -8,6 +8,7 @@ module CLI require 'license_finder/cli/patched_thor' require 'license_finder/cli/base' require 'license_finder/cli/makes_decisions' +require 'license_finder/cli/inherited_decisions' require 'license_finder/cli/permitted_licenses' require 'license_finder/cli/restricted_licenses' require 'license_finder/cli/dependencies' diff --git a/lib/license_finder/cli/inherited_decisions.rb b/lib/license_finder/cli/inherited_decisions.rb new file mode 100644 index 000000000..3fdcece35 --- /dev/null +++ b/lib/license_finder/cli/inherited_decisions.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module LicenseFinder + module CLI + class InheritedDecisions < Base + extend Subcommand + include MakesDecisions + + desc 'list', 'List all the inherited decision files' + def list + say 'Inherited Decision Files:', :blue + say_each(decisions.inherited_decisions) + end + + auditable + desc 'add DECISION_FILE...', 'Add one or more decision files to the inherited decisions' + def add(*decision_files) + assert_some decision_files + modifying { decision_files.each { |filepath| decisions.inherit_from(filepath) } } + say "Added #{decision_files.join(', ')} to the inherited decisions" + end + + auditable + desc 'remove DECISION_FILE...', 'Remove one or more decision files from the inherited decisions' + def remove(*decision_files) + assert_some decision_files + modifying { decision_files.each { |filepath| decisions.remove_inheritance(filepath) } } + say "Removed #{decision_files.join(', ')} from the inherited decisions" + end + end + end +end diff --git a/lib/license_finder/cli/main.rb b/lib/license_finder/cli/main.rb index 6a2083701..9021b6077 100644 --- a/lib/license_finder/cli/main.rb +++ b/lib/license_finder/cli/main.rb @@ -172,6 +172,7 @@ def diff(file1, file2) subcommand 'permitted_licenses', PermittedLicenses, 'Automatically approve any dependency that has a permitted license' subcommand 'restricted_licenses', RestrictedLicenses, 'Forbid approval of any dependency whose licenses are all restricted' subcommand 'project_name', ProjectName, 'Set the project name, for display in reports' + subcommand 'inherited_decisions', InheritedDecisions, 'Add or remove decision files you want to inherit from' private diff --git a/lib/license_finder/decisions.rb b/lib/license_finder/decisions.rb index 1779dd7b8..9bc8ba89b 100644 --- a/lib/license_finder/decisions.rb +++ b/lib/license_finder/decisions.rb @@ -8,7 +8,7 @@ class Decisions # READ ###### - attr_reader :packages, :permitted, :restricted, :ignored, :ignored_groups, :project_name + attr_reader :packages, :permitted, :restricted, :ignored, :ignored_groups, :project_name, :inherited_decisions def licenses_of(name) @licenses[name] @@ -74,6 +74,7 @@ def initialize @restricted = Set.new @ignored = Set.new @ignored_groups = Set.new + @inherited_decisions = Set.new end def add_package(name, version, txn = {}) @@ -191,9 +192,15 @@ def inherit_from(filepath) end add_decision [:inherit_from, filepath] + @inherited_decisions << filepath restore_inheritance(decisions) end + def remove_inheritance(filepath) + @decisions = @decisions - [[:inherit_from, filepath]] + self + end + def add_decision(decision) @decisions << decision unless @inherited end From 57f4e33ffe0e8864c91051e4c3e902c720341b6a Mon Sep 17 00:00:00 2001 From: Sven Dunemann Date: Mon, 18 May 2020 11:09:52 +0200 Subject: [PATCH 4/6] add spec for removing inheritance --- lib/license_finder/decisions.rb | 5 +++-- spec/lib/license_finder/decisions_spec.rb | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/lib/license_finder/decisions.rb b/lib/license_finder/decisions.rb index 9bc8ba89b..b5fec2785 100644 --- a/lib/license_finder/decisions.rb +++ b/lib/license_finder/decisions.rb @@ -186,7 +186,7 @@ def unname_project(txn = {}) def inherit_from(filepath) decisions = if filepath =~ %r{^https?://} - open(filepath).read + URI.open(filepath).read else Pathname(filepath).read end @@ -197,7 +197,8 @@ def inherit_from(filepath) end def remove_inheritance(filepath) - @decisions = @decisions - [[:inherit_from, filepath]] + @decisions -= [[:inherit_from, filepath]] + @inherited_decisions.delete(filepath) self end diff --git a/spec/lib/license_finder/decisions_spec.rb b/spec/lib/license_finder/decisions_spec.rb index 1641028c3..3130de968 100644 --- a/spec/lib/license_finder/decisions_spec.rb +++ b/spec/lib/license_finder/decisions_spec.rb @@ -296,6 +296,17 @@ module LicenseFinder end end + describe '.remove_inheritance' do + it 'reports inheritanced decisions' do + allow_any_instance_of(Pathname).to receive(:read).and_return('---') + decisions = subject.inherit_from('./config/inherit.yml') + expect(decisions.inherited_decisions).to include('./config/inherit.yml') + + decisions = subject.remove_inheritance('./config/inherit.yml') + expect(decisions.inherited_decisions).to be_empty + end + end + describe 'persistence' do def roundtrip(decisions) described_class.restore(decisions.persist) @@ -462,6 +473,15 @@ def roundtrip(decisions) expect(decisions.project_name).to be_nil end + it 'can restore inherited decisions' do + allow_any_instance_of(Pathname).to receive(:read).and_return(YAML.dump([[:permit, 'MIT']])) + decisions = roundtrip( + subject + .inherit_from('./config/inherit.yml') + ) + expect(decisions.inherited_decisions).to include('./config/inherit.yml') + end + it 'does not store decisions from inheritance' do allow_any_instance_of(Pathname).to receive(:read).and_return(YAML.dump([[:permit, 'MIT']])) decisions = subject.inherit_from('./config/inherit.yml') From f1084187753c81c0e1cb159ff717d08b8c85b20a Mon Sep 17 00:00:00 2001 From: Sven Dunemann Date: Mon, 18 May 2020 14:08:45 +0200 Subject: [PATCH 5/6] use open, disable rubocop, add note --- lib/license_finder/decisions.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/license_finder/decisions.rb b/lib/license_finder/decisions.rb index b5fec2785..926c4ab12 100644 --- a/lib/license_finder/decisions.rb +++ b/lib/license_finder/decisions.rb @@ -186,7 +186,10 @@ def unname_project(txn = {}) def inherit_from(filepath) decisions = if filepath =~ %r{^https?://} - URI.open(filepath).read + # ruby < 2.5.0 URI.open is private + # rubocop:disable Security/Open + open(filepath).read + # rubocop:enable Security/Open else Pathname(filepath).read end From c3afcc531fe346c80a3435f6c839907255127f7b Mon Sep 17 00:00:00 2001 From: Sven Dunemann Date: Wed, 20 May 2020 06:59:43 +0200 Subject: [PATCH 6/6] check RUBY_VERSION --- lib/license_finder/decisions.rb | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/lib/license_finder/decisions.rb b/lib/license_finder/decisions.rb index 926c4ab12..9e11f1d8a 100644 --- a/lib/license_finder/decisions.rb +++ b/lib/license_finder/decisions.rb @@ -186,10 +186,7 @@ def unname_project(txn = {}) def inherit_from(filepath) decisions = if filepath =~ %r{^https?://} - # ruby < 2.5.0 URI.open is private - # rubocop:disable Security/Open - open(filepath).read - # rubocop:enable Security/Open + open_uri(filepath).read else Pathname(filepath).read end @@ -216,6 +213,17 @@ def restore_inheritance(decisions) self end + def open_uri(uri) + # ruby < 2.5.0 URI.open is private + if Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.5.0') + # rubocop:disable Security/Open + open(uri) + # rubocop:enable Security/Open + else + URI.open(uri) + end + end + ######### # PERSIST #########