Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Nesta::Config from_(environment|yaml) raise NotDefined
Previously these methods returned nil, which encouraged them to be called one
after the other in a boolean short-circuit style, leading to subtle bugs
in the way config values that evaluated to false were handled.

Raising an exception has also allowed needlessly complex conditional
statements to be simplified.
  • Loading branch information
gma committed Nov 7, 2013
1 parent 972f64b commit 7d1adf3
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 21 deletions.
6 changes: 6 additions & 0 deletions CHANGES
Expand Up @@ -17,6 +17,12 @@
value in the `NESTA_MY_SETTING` variable can be returned by
calling `Nesta::Config.fetch(:my_setting)`.

If the setting you're trying to read isn't defined a NotDefined
exception (a subclass of KeyError) will be raised. Similarly to
Ruby's Hash#fetch method, you can avoid the exception by specifying a
second argument (such as nil) that will be returned if the setting
isn't defined.

(Sean Redmond, Graham Ashton)

* Added 'Link text' metadata. When working out how to link to your
Expand Down
51 changes: 33 additions & 18 deletions lib/nesta/config.rb
Expand Up @@ -13,24 +13,20 @@ class NotDefined < KeyError; end
class << self
attr_accessor :settings, :author_settings, :yaml_conf
end
def self.fetch(key, *args)
setting = key.to_s
value = from_environment(setting) || from_yaml(setting)
if value.nil?
args.empty? && (raise NotDefined.new(setting)) || (return args.first)
else
return value

def self.fetch(key, *default)
from_environment(key.to_s)
rescue NotDefined
begin
from_yaml(key.to_s)
rescue NotDefined
default.empty? && raise || (return default.first)
end
end

def self.method_missing(method, *args)
if settings.include?(method.to_s)
begin
fetch(method)
rescue NotDefined
nil
end
fetch(method, nil)
else
super
end
Expand All @@ -43,6 +39,8 @@ def self.author
ENV[variable] && environment_config[setting] = ENV[variable]
end
environment_config.empty? ? from_yaml('author') : environment_config
rescue NotDefined
nil
end

def self.cache
Expand All @@ -67,12 +65,20 @@ def self.yaml_path
end

def self.read_more
default = 'Continue reading'
from_environment('read_more') || from_yaml('read_more') || default
from_environment('read_more')
rescue NotDefined
begin
from_yaml('read_more')
rescue NotDefined
'Continue reading'
end
end

def self.from_environment(setting)
value = ENV["NESTA_#{setting.upcase}"]
value = ENV.fetch("NESTA_#{setting.upcase}")
rescue KeyError
raise NotDefined.new(setting)
else
overrides = { "true" => true, "false" => false }
overrides.has_key?(value) ? overrides[value] : value
end
Expand All @@ -88,11 +94,20 @@ def self.can_use_yaml?
end
private_class_method :can_use_yaml?

def self.from_hash(hash, setting)
hash.fetch(setting) { raise NotDefined.new(setting) }
end
private_class_method :from_hash

def self.from_yaml(setting)
if can_use_yaml?
self.yaml_conf ||= YAML::load(ERB.new(IO.read(yaml_path)).result)
rack_env_conf = self.yaml_conf[Nesta::App.environment.to_s]
(rack_env_conf && rack_env_conf[setting]) || self.yaml_conf[setting]
env_config = self.yaml_conf[Nesta::App.environment.to_s] || {}
begin
from_hash(env_config, setting)
rescue NotDefined
from_hash(self.yaml_conf, setting)
end
end
rescue Errno::ENOENT # config file not found
raise unless Nesta::App.environment == :test
Expand Down
21 changes: 19 additions & 2 deletions spec/config_spec.rb
Expand Up @@ -4,6 +4,14 @@
after(:each) do
ENV.keys.each { |variable| ENV.delete(variable) if variable =~ /NESTA_/ }
end

it 'should return default value for "Read more"' do
Nesta::Config.read_more.should == 'Continue reading'
end

it 'should return nil for author when not defined' do
Nesta::Config.author.should == nil
end

describe "when settings defined in ENV" do
before(:each) do
Expand Down Expand Up @@ -64,9 +72,9 @@
Nesta::Config.author["email"].should be_nil
end

it "should override top level settings with RACK_ENV specific settings" do
it "should override top level settings with environment specific settings" do
stub_config_key('content', 'general/path')
stub_config_key('content', 'rack_env/path', rack_env: true)
stub_config_key('content', 'rack_env/path', for_environment: true)
Nesta::Config.content.should == 'rack_env/path'
end
end
Expand Down Expand Up @@ -97,5 +105,14 @@
it "should allow default value to be specified when they don't exist" do
Nesta::Config.fetch('who me?', 'yes you').should == 'yes you'
end

it 'should cope with non-truthy boolean values' do
ENV['NESTA_FALSY'] = 'false'
begin
Nesta::Config.fetch('falsy').should == false
ensure
ENV.delete('NESTA_FALSY')
end
end
end
end
2 changes: 1 addition & 1 deletion spec/spec_helper.rb
Expand Up @@ -24,7 +24,7 @@ def stub_yaml_config

def stub_config_key(key, value, options = {})
stub_yaml_config unless @config
if options[:rack_env]
if options[:for_environment]
@config['test'] ||= {}
@config['test'][key] = value
else
Expand Down

0 comments on commit 7d1adf3

Please sign in to comment.