Skip to content

Commit

Permalink
(PDK-1108) Remove deprecated PDK::Config method calls
Browse files Browse the repository at this point in the history
Previously many calls within the PDK used `.user` directly.  However now that
there is a formal get and set method, accessing these directly is no longer
required and is a private API.  This commit updates any references to these
private methods and changes them to use .get and .set appropriately.
  • Loading branch information
glennsarti committed Feb 28, 2020
1 parent a3b68db commit 3536bea
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 61 deletions.
2 changes: 1 addition & 1 deletion lib/pdk/cli/convert.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ module PDK::CLI
raise PDK::CLI::ExitWithError, _('You can not specify --template-url and --default-template.') if opts[:'template-url']

opts[:'template-url'] = PDK::Util::TemplateURI.default_template_addressable_uri.to_s
PDK.config.user['module_defaults']['template-url'] = nil
PDK.config.set(%w[user module_defaults template-url], nil)
end

PDK::CLI::Util.validate_template_opts(opts)
Expand Down
12 changes: 2 additions & 10 deletions lib/pdk/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,6 @@ def initialize(options = nil)
}.merge(options)
end

# The user configuration settings.
# @deprecated This method is only provided as a courtesy until the `pdk set config` CLI and associated changes in this class, are completed.
# Any read-only operations should be using `.get` or `.get_within_scopes`
# @return [PDK::Config::Namespace]
def user
user_config
end

# The system level configuration settings.
# @return [PDK::Config::Namespace]
# @api private
Expand Down Expand Up @@ -269,9 +261,9 @@ def self.analytics_config_interview!

if answers.nil?
PDK.logger.info _('No answer given, opting out of analytics collection.')
PDK.config.user['analytics']['disabled'] = true
PDK.config.set(%w[user analytics disabled], true)
else
PDK.config.user['analytics']['disabled'] = !answers['enabled']
PDK.config.set(%w[user analytics disabled], !answers['enabled'])
end

PDK.logger.info(text: post_message, wrap: true)
Expand Down
10 changes: 5 additions & 5 deletions lib/pdk/generate/module.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,11 @@ def self.invoke(opts = {})
# If the user specifies our default template url via the command
# line, remove the saved template-url answer so that the template_uri
# resolution can find new default URLs in the future.
PDK.config.user['module_defaults']['template-url'] = nil if opts.key?(:'template-url')
PDK.config.set(%w[user module_defaults template-url], nil) if opts.key?(:'template-url')
else
# Save the template-url answers if the module was generated using a
# template/reference other than ours.
PDK.config.user['module_defaults']['template-url'] = template_uri.metadata_format
PDK.config.set(%w[user module_defaults template-url], template_uri.metadata_format)
end

begin
Expand Down Expand Up @@ -342,9 +342,9 @@ def self.module_interview(metadata, opts = {})
end

require 'pdk/answer_file'
PDK.config.user['module_defaults']['forge_username'] = opts[:username] unless opts[:username].nil?
PDK.config.user['module_defaults']['author'] = answers['author'] unless answers['author'].nil?
PDK.config.user['module_defaults']['license'] = answers['license'] unless answers['license'].nil?
PDK.config.set(%w[user module_defaults forge_username], opts[:username]) unless opts[:username].nil?
PDK.config.set(%w[user module_defaults author], answers['author']) unless answers['author'].nil?
PDK.config.set(%w[user module_defaults license], answers['license']) unless answers['license'].nil?
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/unit/pdk/cli/convert_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@

it 'clears the saved template-url answer' do
run
expect(PDK.config.user['module_defaults']['template-url']).to be_nil
expect(PDK.config.get(%w[user module_defaults template-url])).to be_nil
end

it 'submits the command to analytics' do
Expand Down
44 changes: 19 additions & 25 deletions spec/unit/pdk/config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,6 @@ def mock_file(path, content)
end
end

describe '.user' do
it 'is an alias for user_config' do
expect(config.user).to be(config.user_config)
end
end

describe '.user_config' do
it 'returns a PDK::Config::Namespace' do
expect(config.user_config).to be_a(PDK::Config::Namespace)
Expand Down Expand Up @@ -359,12 +353,12 @@ def project_config

it 'takes a String for the setting name' do
config.set('user.foo.whizz', value)
expect(config.user['foo']['whizz']).to eq(value)
expect(config.get('user.foo.whizz')).to eq(value)
end

it 'takes an Array for the setting name' do
config.set(%w[user foo whizz], value)
expect(config.user['foo']['whizz']).to eq(value)
expect(config.get(%w[user foo whizz])).to eq(value)
end

context 'given a root name that does not exist' do
Expand Down Expand Up @@ -400,7 +394,7 @@ def project_config
it 'can be accessed via a normal hash syntax' do
config.set(setting, value)

expect(config.user['bar']['baz']['setting']).to eq(value)
expect(config.get(%w[user bar baz setting])).to eq(value)
end

it 'saves to the root file' do
Expand All @@ -423,8 +417,8 @@ def project_config
it 'can be accessed via a normal hash syntax' do
config.set(setting, value)

expect(config.user['foo']['bar']).to eq('baz' => { 'setting' => value })
expect(config.user['foo']['bar']['baz']['setting']).to eq(value)
expect(config.get(%w[user foo bar])).to eq('baz' => { 'setting' => value })
expect(config.get(%w[user foo bar baz setting])).to eq(value)
end

it_behaves_like 'a new setting file', "{\n \"bar\": {\n \"baz\": {\n \"setting\": \"mock_value\"\n }\n }\n}"
Expand All @@ -436,14 +430,14 @@ def project_config

it 'can be accessed via a normal hash syntax' do
# The old setting should exist
expect(config.user['foo']['bar']).to eq('existing_setting' => 'exists')
expect(config.get(%w[user foo bar])).to eq('existing_setting' => 'exists')

config.set(setting, value)

# Should still contain the old setting
expect(config.user['foo']['bar']).to eq('baz' => { 'setting' => 'mock_value' }, 'existing_setting' => 'exists')
expect(config.get(%w[user foo bar])).to eq('baz' => { 'setting' => 'mock_value' }, 'existing_setting' => 'exists')
# Should contain the new setting
expect(config.user['foo']['bar']['baz']['setting']).to eq(value)
expect(config.get(%w[user foo bar baz setting])).to eq(value)
end

it_behaves_like 'a new setting file', "{\n \"bar\": {\n \"existing_setting\": \"exists\",\n \"baz\": {\n \"setting\": \"mock_value\"\n }\n }\n}"
Expand All @@ -456,7 +450,7 @@ def project_config
context 'without forcing the change' do
it 'raises an error' do
# The old setting should exist
expect(config.user['foo']['bar']).to eq([])
expect(config.get(%w[user foo bar])).to eq([])

expect { config.set(setting, value) }.to raise_error(ArgumentError)
end
Expand All @@ -467,12 +461,12 @@ def project_config

it 'uses the new setting' do
# The old setting should exist
expect(config.user['foo']['bar']).to eq([])
expect(config.get(%w[user foo bar])).to eq([])

config.set(setting, value, set_options)

# Should contain only new setting
expect(config.user['foo']['bar']['baz']['setting']).to eq(value)
expect(config.get(%w[user foo bar baz setting])).to eq(value)
end

it_behaves_like 'a new setting file', "{\n \"bar\": {\n \"baz\": {\n \"setting\": \"mock_value\"\n }\n }\n}"
Expand All @@ -486,7 +480,7 @@ def project_config
context 'without forcing the change' do
it 'appends the new value' do
config.set(setting, value, set_options)
expect(config.user['foo']['bar']).to eq(['abc', 'def', value])
expect(config.get(%w[user foo bar])).to eq(['abc', 'def', value])
end

it_behaves_like 'a new setting file', "{\n \"bar\": [\n \"abc\",\n \"def\",\n \"mock_value\"\n ]\n}"
Expand All @@ -497,7 +491,7 @@ def project_config

it 'uses the new value' do
config.set(setting, value, set_options)
expect(config.user['foo']['bar']).to eq(value)
expect(config.get(%w[user foo bar])).to eq(value)
end

it_behaves_like 'a new setting file', "{\n \"bar\": \"mock_value\"\n}"
Expand All @@ -512,7 +506,7 @@ def project_config
it 'does not append the new value' do
expect(PDK::Util::Filesystem).not_to receive(:write_file).with(PDK::Util::Filesystem.expand_path(foo_file), anything)
config.set(setting, value, set_options)
expect(config.user['foo']['bar']).to eq(['abc', value])
expect(config.get(%w[user foo bar])).to eq(['abc', value])
end
end

Expand All @@ -521,7 +515,7 @@ def project_config

it 'uses the new value' do
config.set(setting, value, set_options)
expect(config.user['foo']['bar']).to eq(value)
expect(config.get(%w[user foo bar])).to eq(value)
end

it_behaves_like 'a new setting file', "{\n \"bar\": \"mock_value\"\n}"
Expand Down Expand Up @@ -679,7 +673,7 @@ def uuid_regex(uuid)

it 'sets user.analytics.disabled to false' do
described_class.analytics_config_interview!
expect(PDK.config.user_config['analytics']['disabled']).to be_falsey
expect(PDK.config.get(%w[user analytics disabled])).to be_falsey
end
end

Expand All @@ -688,7 +682,7 @@ def uuid_regex(uuid)

it 'sets user.analytics.disabled to true' do
described_class.analytics_config_interview!
expect(PDK.config.user_config['analytics']['disabled']).to be_truthy
expect(PDK.config.get(%w[user analytics disabled])).to be_truthy
end
end

Expand All @@ -697,7 +691,7 @@ def uuid_regex(uuid)

it 'sets user.analytics.disabled to false' do
described_class.analytics_config_interview!
expect(PDK.config.user_config['analytics']['disabled']).to be_falsey
expect(PDK.config.get(%w[user analytics disabled])).to be_falsey
end
end

Expand All @@ -706,7 +700,7 @@ def uuid_regex(uuid)

it 'sets user.analytics.disabled to true' do
described_class.analytics_config_interview!
expect(PDK.config.user_config['analytics']['disabled']).to be_truthy
expect(PDK.config.get(%w[user analytics disabled])).to be_truthy
end
end
end
Expand Down
26 changes: 13 additions & 13 deletions spec/unit/pdk/generate/module_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -224,24 +224,24 @@
end

it 'takes precedence over the template-url answer' do
PDK.config.user['module_defaults']['template-url'] = 'answer-template'
PDK.config.set(%w[user module_defaults template-url], 'answer-template')
expect(PDK::Module::TemplateDir).to receive(:with).with(Addressable::URI.parse('cli-template#master'), anything, anything).and_yield(test_template_dir)
described_class.invoke(invoke_opts.merge(:'template-url' => 'cli-template'))
end

it 'saves the template-url and template-ref to the answer file if it is not the default template' do
described_class.invoke(invoke_opts.merge(:'template-url' => 'cli-template'))
expect(PDK.config.user['module_defaults']['template-url']).to eq(Addressable::URI.parse('cli-template#master').to_s)
expect(PDK.config.get(%w[user module_defaults template-url])).to eq(Addressable::URI.parse('cli-template#master').to_s)
end

it 'saves the template-url and template-ref to the answer file if it is not the default ref' do
described_class.invoke(invoke_opts.merge(:'template-url' => default_template_url, :'template-ref' => '1.2.3'))
expect(PDK.config.user['module_defaults']['template-url']).to eq("#{default_template_url}#1.2.3")
expect(PDK.config.get(%w[user module_defaults template-url])).to eq("#{default_template_url}#1.2.3")
end

it 'clears the saved template-url answer if it is the default template' do
described_class.invoke(invoke_opts.merge(:'template-url' => default_template_url))
expect(PDK.config.user['module_defaults']['template-url']).to eq(nil)
expect(PDK.config.get(%w[user module_defaults template-url])).to eq(nil)
end
end

Expand All @@ -253,7 +253,7 @@

context 'and a template-url answer exists' do
it 'uses the template-url from the answer file to generate the module' do
PDK.config.user['module_defaults']['template-url'] = 'answer-template'
PDK.config.set(%w[user module_defaults template-url], 'answer-template')
expect(PDK::Module::TemplateDir).to receive(:with).with(Addressable::URI.parse('answer-template'), anything, anything).and_yield(test_template_dir)
expect(logger).to receive(:info).with(a_string_matching(%r{generated at path}i))
expect(logger).to receive(:info).with(a_string_matching(%r{In your module directory, add classes with the 'pdk new class' command}i))
Expand All @@ -272,10 +272,10 @@
it 'uses the vendored template url' do
template_uri = "file:///tmp/package/cache/pdk-templates.git##{PDK::Util::TemplateURI.default_template_ref}"
expect(PDK::Module::TemplateDir).to receive(:with).with(Addressable::URI.parse(template_uri), anything, anything).and_yield(test_template_dir)
before = PDK.config.user['module_defaults']['template-url']
before = PDK.config.get(%w[user module_defaults template-url])

described_class.invoke(invoke_opts)
expect(PDK.config.user['module_defaults']['template-url']).to eq(before)
expect(PDK.config.get(%w[user module_defaults template-url])).to eq(before)
end
end

Expand All @@ -286,10 +286,10 @@

it 'uses the default template to generate the module' do
expect(PDK::Module::TemplateDir).to receive(:with).with(any_args).and_yield(test_template_dir)
before = PDK.config.user['module_defaults']['template-url']
before = PDK.config.get(%w[user module_defaults template-url])

described_class.invoke(invoke_opts)
expect(PDK.config.user['module_defaults']['template-url']).to eq(before)
expect(PDK.config.get(%w[user module_defaults template-url])).to eq(before)
end
end
end
Expand All @@ -307,7 +307,7 @@

subject(:answers) do
interview_metadata
PDK.config.user['module_defaults']
PDK.config.get(%w[user module_defaults])
end

let(:module_name) { 'bar' }
Expand Down Expand Up @@ -723,9 +723,9 @@
before(:each) do
allow(described_class).to receive(:module_interview).with(any_args)

PDK.config.user['module_defaults']['forge_username'] = 'testuser123'
PDK.config.user['module_defaults']['license'] = 'MIT'
PDK.config.user['module_defaults']['author'] = 'Test User'
PDK.config.set(%w[user module_defaults forge_username], 'testuser123')
PDK.config.set(%w[user module_defaults license], 'MIT')
PDK.config.set(%w[user module_defaults author], 'Test User')
end

it 'uses the saved forge_username answer' do
Expand Down
12 changes: 6 additions & 6 deletions spec/unit/pdk/util/template_uri_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
include_context 'mock configuration'

before(:each) do
PDK.config.user['module_defaults']['template-url'] = nil
PDK.config.set(%w[user module_defaults template-url], nil)
allow(PDK::Util).to receive(:module_root).and_return(nil)
allow(PDK::Util).to receive(:package_install?).and_return(false)
end
Expand Down Expand Up @@ -116,7 +116,7 @@

context 'and there are only answers' do
before :each do
PDK.config.user['module_defaults']['template-url'] = 'answer-templates'
PDK.config.set(%w[user module_defaults template-url], 'answer-templates')
allow(PDK::Util::Filesystem).to receive(:file?).with(File.join(module_root, 'metadata.json')).and_return(false)
end

Expand All @@ -138,7 +138,7 @@

context 'and there are metadata and answers' do
before :each do
PDK.config.user['module_defaults']['template-url'] = 'answer-templates'
PDK.config.set(%w[user module_defaults template-url], 'answer-templates')
end

it 'returns the metadata template' do
Expand All @@ -152,7 +152,7 @@

context 'when there are metadata and answers' do
before :each do
PDK.config.user['module_defaults']['template-url'] = 'answer-templates'
PDK.config.set(%w[user module_defaults template-url], 'answer-templates')
allow(PDK::Util::Filesystem).to receive(:file?).with(File.join(PDK::Util.module_root, 'metadata.json')).and_return(true)
allow(PDK::Module::Metadata).to receive(:from_file).with(File.join(PDK::Util.module_root, 'metadata.json')).and_return(mock_metadata)
end
Expand Down Expand Up @@ -445,7 +445,7 @@

context 'when the answers file has saved template-url value' do
before(:each) do
PDK.config.user['module_defaults']['template-url'] = answers_template_url
PDK.config.set(%w[user module_defaults template-url], answers_template_url)
end

context 'that is the deprecated pdk-module-template' do
Expand Down Expand Up @@ -475,7 +475,7 @@

context 'when the answers file has no saved template-url value' do
before(:each) do
PDK.config.user['module_defaults']['template-url'] = nil
PDK.config.set(%w[user module_defaults template-url], nil)
end

it 'does not include a PDK answers template option' do
Expand Down

0 comments on commit 3536bea

Please sign in to comment.