From 0009fe16ca20512aa1d0446127a2516d127f7e55 Mon Sep 17 00:00:00 2001 From: Brandon Keepers Date: Sat, 20 Jan 2024 10:00:09 -0500 Subject: [PATCH 1/3] Refactor internal API to reduce duplication --- lib/dotenv.rb | 49 +++++++++------------------- lib/dotenv/environment.rb | 21 +++++++----- lib/dotenv/parser.rb | 10 +++--- lib/dotenv/substitutions/command.rb | 2 +- lib/dotenv/substitutions/variable.rb | 4 +-- spec/dotenv/environment_spec.rb | 28 ++++++++-------- spec/dotenv/parser_spec.rb | 2 +- spec/dotenv_spec.rb | 33 +++++++++---------- 8 files changed, 68 insertions(+), 81 deletions(-) diff --git a/lib/dotenv.rb b/lib/dotenv.rb index 8d543401..b594ed17 100644 --- a/lib/dotenv.rb +++ b/lib/dotenv.rb @@ -10,58 +10,41 @@ class << self module_function - def load(*filenames) - with(*filenames) do |f| - ignoring_nonexistent_files do - env = Environment.new(f, true) - instrument("dotenv.load", env: env) { env.apply } - end + def load(*filenames, **kwargs) + parse(*filenames, **kwargs) do |env| + instrument("dotenv.load", env: env) { env.apply } end end # same as `load`, but raises Errno::ENOENT if any files don't exist def load!(*filenames) - with(*filenames) do |f| - env = Environment.new(f, true) - instrument("dotenv.load", env: env) { env.apply } - end + load(*filenames, ignore: false) end # same as `load`, but will override existing values in `ENV` def overload(*filenames) - with(*filenames.reverse) do |f| - ignoring_nonexistent_files do - env = Environment.new(f, false) - instrument("dotenv.overload", env: env) { env.apply! } - end - end + load(*filenames, overwrite: true) end # same as `overload`, but raises Errno::ENOENT if any files don't exist def overload!(*filenames) - with(*filenames.reverse) do |f| - env = Environment.new(f, false) - instrument("dotenv.overload", env: env) { env.apply! } - end + load(*filenames, overwrite: true, ignore: false) end # returns a hash of parsed key/value pairs but does not modify ENV - def parse(*filenames) - with(*filenames) do |f| - ignoring_nonexistent_files do - Environment.new(f, false) - end - end - end - - # Internal: Helper to expand list of filenames. - # - # Returns a hash of all the loaded environment variables. - def with(*filenames) + def parse(*filenames, overwrite: false, ignore: true, &block) filenames << ".env" if filenames.empty? + filenames = filenames.reverse if overwrite filenames.reduce({}) do |hash, filename| - hash.merge!(yield(File.expand_path(filename)) || {}) + begin + env = Environment.new(File.expand_path(filename), overwrite: overwrite) + env = block.call(env) if block + rescue Errno::ENOENT + raise unless ignore + end + + hash.merge! env || {} end end diff --git a/lib/dotenv/environment.rb b/lib/dotenv/environment.rb index c7d21d09..25331134 100644 --- a/lib/dotenv/environment.rb +++ b/lib/dotenv/environment.rb @@ -4,13 +4,14 @@ module Dotenv class Environment < Hash attr_reader :filename - def initialize(filename, is_load = false) + def initialize(filename, overwrite: false) @filename = filename - load(is_load) + @overwrite = overwrite + load end - def load(is_load = false) - update Parser.call(read, is_load) + def load + update Parser.call(read, overwrite: @overwrite) end def read @@ -18,11 +19,13 @@ def read end def apply - each { |k, v| ENV[k] ||= v } - end - - def apply! - each { |k, v| ENV[k] = v } + each do |k, v| + if @overwrite + ENV[k] = v + else + ENV[k] ||= v + end + end end end end diff --git a/lib/dotenv/parser.rb b/lib/dotenv/parser.rb index ab78c2f4..f8509072 100644 --- a/lib/dotenv/parser.rb +++ b/lib/dotenv/parser.rb @@ -32,15 +32,15 @@ class Parser class << self attr_reader :substitutions - def call(string, is_load = false) - new(string, is_load).call + def call(...) + new(...).call end end - def initialize(string, is_load = false) + def initialize(string, overwrite: false) @string = string @hash = {} - @is_load = is_load + @overwrite = overwrite end def call @@ -104,7 +104,7 @@ def unescape_value(value, maybe_quote) def perform_substitutions(value, maybe_quote) if maybe_quote != "'" self.class.substitutions.each do |proc| - value = proc.call(value, @hash, @is_load) + value = proc.call(value, @hash, overwrite: @overwrite) end end value diff --git a/lib/dotenv/substitutions/command.rb b/lib/dotenv/substitutions/command.rb index 724f87d2..b2e0544f 100644 --- a/lib/dotenv/substitutions/command.rb +++ b/lib/dotenv/substitutions/command.rb @@ -20,7 +20,7 @@ class << self ) /x - def call(value, _env, _is_load) + def call(value, _env, overwrite: false) # Process interpolated shell commands value.gsub(INTERPOLATED_SHELL_COMMAND) do |*| # Eliminate opening and closing parentheses diff --git a/lib/dotenv/substitutions/variable.rb b/lib/dotenv/substitutions/variable.rb index 4dba441e..7ed8b727 100644 --- a/lib/dotenv/substitutions/variable.rb +++ b/lib/dotenv/substitutions/variable.rb @@ -18,8 +18,8 @@ class << self \}? # closing brace /xi - def call(value, env, is_load) - combined_env = is_load ? env.merge(ENV) : ENV.to_h.merge(env) + def call(value, env, overwrite: false) + combined_env = overwrite ? ENV.to_h.merge(env) : env.merge(ENV) value.gsub(VARIABLE) do |variable| match = $LAST_MATCH_INFO substitute(match, variable, combined_env) diff --git a/spec/dotenv/environment_spec.rb b/spec/dotenv/environment_spec.rb index f3dc54c1..dbda5b75 100644 --- a/spec/dotenv/environment_spec.rb +++ b/spec/dotenv/environment_spec.rb @@ -11,7 +11,7 @@ it "fails if file does not exist" do expect do - Dotenv::Environment.new(".does_not_exists", true) + Dotenv::Environment.new(".does_not_exists") end.to raise_error(Errno::ENOENT) end end @@ -27,27 +27,29 @@ subject.apply expect(ENV["OPTION_A"]).to eq("predefined") end - end - describe "apply!" do - it "sets variables in the ENV" do - subject.apply! - expect(ENV["OPTION_A"]).to eq("1") - end + context "with overwrite: true" do + subject { env("OPTION_A=1\nOPTION_B=2", overwrite: true) } - it "overrides defined variables" do - ENV["OPTION_A"] = "predefined" - subject.apply! - expect(ENV["OPTION_A"]).to eq("1") + it "sets variables in the ENV" do + subject.apply + expect(ENV["OPTION_A"]).to eq("1") + end + + it "overrides defined variables" do + ENV["OPTION_A"] = "predefined" + subject.apply + expect(ENV["OPTION_A"]).to eq("1") + end end end require "tempfile" - def env(text) + def env(text, ...) file = Tempfile.new("dotenv") file.write text file.close - env = Dotenv::Environment.new(file.path, true) + env = Dotenv::Environment.new(file.path, ...) file.unlink env end diff --git a/spec/dotenv/parser_spec.rb b/spec/dotenv/parser_spec.rb index 39fe9b74..dda20b40 100644 --- a/spec/dotenv/parser_spec.rb +++ b/spec/dotenv/parser_spec.rb @@ -2,7 +2,7 @@ describe Dotenv::Parser do def env(string) - Dotenv::Parser.call(string, true) + Dotenv::Parser.call(string) end it "parses unquoted values" do diff --git a/spec/dotenv_spec.rb b/spec/dotenv_spec.rb index 2676e6d9..dcaa2888 100644 --- a/spec/dotenv_spec.rb +++ b/spec/dotenv_spec.rb @@ -10,8 +10,7 @@ let(:env_files) { [] } it "defaults to .env" do - expect(Dotenv::Environment).to receive(:new).with(expand(".env"), anything) - .and_return(double(apply: {}, apply!: {})) + expect(Dotenv::Environment).to receive(:new).with(expand(".env"), anything).and_call_original subject end end @@ -23,7 +22,7 @@ expected = expand("~/.env") allow(File).to receive(:exist?) { |arg| arg == expected } expect(Dotenv::Environment).to receive(:new).with(expected, anything) - .and_return(double(apply: {}, apply!: {})) + .and_return(Dotenv::Environment.new(".env")) subject end end @@ -84,9 +83,9 @@ it_behaves_like "load" - it "initializes the Environment with a truthy is_load" do - expect(Dotenv::Environment).to receive(:new).with(anything, true) - .and_return(double(apply: {}, apply!: {})) + it "initializes the Environment with overwrite: false" do + expect(Dotenv::Environment).to receive(:new).with(anything, overwrite: false) + .and_call_original subject end @@ -106,9 +105,9 @@ it_behaves_like "load" - it "initializes Environment with truthy is_load" do - expect(Dotenv::Environment).to receive(:new).with(anything, true) - .and_return(double(apply: {}, apply!: {})) + it "initializes Environment with overwrite: false" do + expect(Dotenv::Environment).to receive(:new).with(anything, overwrite: false) + .and_call_original subject end @@ -127,9 +126,9 @@ it_behaves_like "load" it_behaves_like "overload" - it "initializes the Environment with a falsey is_load" do - expect(Dotenv::Environment).to receive(:new).with(anything, false) - .and_return(double(apply: {}, apply!: {})) + it "initializes the Environment overwrite: true" do + expect(Dotenv::Environment).to receive(:new).with(anything, overwrite: true) + .and_call_original subject end @@ -161,9 +160,9 @@ it_behaves_like "load" it_behaves_like "overload" - it "initializes the Environment with a falsey is_load" do - expect(Dotenv::Environment).to receive(:new).with(anything, false) - .and_return(double(apply: {}, apply!: {})) + it "initializes the Environment with overwrite: true" do + expect(Dotenv::Environment).to receive(:new).with(anything, overwrite: true) + .and_call_original subject end @@ -271,8 +270,8 @@ end end - it "initializes the Environment with a falsey is_load" do - expect(Dotenv::Environment).to receive(:new).with(anything, false) + it "initializes the Environment with overwrite: false" do + expect(Dotenv::Environment).to receive(:new).with(anything, overwrite: false) subject end From 136696f5509fe65bff61c6dc1f4132a0789817bd Mon Sep 17 00:00:00 2001 From: Brandon Keepers Date: Sat, 20 Jan 2024 10:13:49 -0500 Subject: [PATCH 2/3] Update standard config to latest supported Ruby version --- .standard.yml | 2 +- lib/dotenv/parser.rb | 2 +- lib/dotenv/substitutions/command.rb | 2 +- lib/dotenv/substitutions/variable.rb | 2 +- spec/dotenv/parser_spec.rb | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.standard.yml b/.standard.yml index 7a9789d0..eac53f7e 100644 --- a/.standard.yml +++ b/.standard.yml @@ -1,4 +1,4 @@ -ruby_version: 2.5 +ruby_version: 3.0 ignore: - lib/dotenv/parser.rb: diff --git a/lib/dotenv/parser.rb b/lib/dotenv/parser.rb index f8509072..14eba2ec 100644 --- a/lib/dotenv/parser.rb +++ b/lib/dotenv/parser.rb @@ -88,7 +88,7 @@ def expand_newlines(value) end def variable_not_set?(line) - !line.split[1..-1].all? { |var| @hash.member?(var) } + !line.split[1..].all? { |var| @hash.member?(var) } end def unescape_value(value, maybe_quote) diff --git a/lib/dotenv/substitutions/command.rb b/lib/dotenv/substitutions/command.rb index b2e0544f..2010bae1 100644 --- a/lib/dotenv/substitutions/command.rb +++ b/lib/dotenv/substitutions/command.rb @@ -28,7 +28,7 @@ def call(value, _env, overwrite: false) if $LAST_MATCH_INFO[:backslash] # Command is escaped, don't replace it. - $LAST_MATCH_INFO[0][1..-1] + $LAST_MATCH_INFO[0][1..] else # Execute the command and return the value `#{command}`.chomp diff --git a/lib/dotenv/substitutions/variable.rb b/lib/dotenv/substitutions/variable.rb index 7ed8b727..d35e3fbc 100644 --- a/lib/dotenv/substitutions/variable.rb +++ b/lib/dotenv/substitutions/variable.rb @@ -30,7 +30,7 @@ def call(value, env, overwrite: false) def substitute(match, variable, env) if match[1] == "\\" - variable[1..-1] + variable[1..] elsif match[3] env.fetch(match[3], "") else diff --git a/spec/dotenv/parser_spec.rb b/spec/dotenv/parser_spec.rb index dda20b40..60324e1a 100644 --- a/spec/dotenv/parser_spec.rb +++ b/spec/dotenv/parser_spec.rb @@ -129,14 +129,14 @@ def env(string) ENV["DOTENV_LINEBREAK_MODE"] = "strict" contents = [ - 'DOTENV_LINEBREAK_MODE=legacy', + "DOTENV_LINEBREAK_MODE=legacy", 'FOO="bar\nbaz\rfizz"' ].join("\n") expect(env(contents)).to eql("DOTENV_LINEBREAK_MODE" => "legacy", "FOO" => "bar\nbaz\rfizz") end it 'expands \n and \r in quoted strings with DOTENV_LINEBREAK_MODE=legacy in ENV' do - ENV['DOTENV_LINEBREAK_MODE'] = 'legacy' + ENV["DOTENV_LINEBREAK_MODE"] = "legacy" contents = 'FOO="bar\nbaz\rfizz"' expect(env(contents)).to eql("FOO" => "bar\nbaz\rfizz") end From 43ff6eefea69c008e307907ea5edf9f030a357d8 Mon Sep 17 00:00:00 2001 From: Brandon Keepers Date: Sat, 20 Jan 2024 10:38:53 -0500 Subject: [PATCH 3/3] Update docs, remove unused method --- lib/dotenv.rb | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/dotenv.rb b/lib/dotenv.rb index b594ed17..92166f8a 100644 --- a/lib/dotenv.rb +++ b/lib/dotenv.rb @@ -10,28 +10,35 @@ class << self module_function + # Loads environment variables from one or more `.env` files. See `#parse` for more details. def load(*filenames, **kwargs) parse(*filenames, **kwargs) do |env| instrument("dotenv.load", env: env) { env.apply } end end - # same as `load`, but raises Errno::ENOENT if any files don't exist + # Same as `#load`, but raises Errno::ENOENT if any files don't exist def load!(*filenames) load(*filenames, ignore: false) end - # same as `load`, but will override existing values in `ENV` + # same as `#load`, but will override existing values in `ENV` def overload(*filenames) load(*filenames, overwrite: true) end - # same as `overload`, but raises Errno::ENOENT if any files don't exist + # same as `#overload`, but raises Errno::ENOENT if any files don't exist def overload!(*filenames) load(*filenames, overwrite: true, ignore: false) end - # returns a hash of parsed key/value pairs but does not modify ENV + # Parses the given files, yielding for each file if a block is given. + # + # @param filenames [String, Array] Files to parse + # @param overwrite [Boolean] Overwrite existing `ENV` values + # @param ignore [Boolean] Ignore non-existent files + # @param block [Proc] Block to yield for each parsed `Dotenv::Environment` + # @return [Hash] parsed key/value pairs def parse(*filenames, overwrite: false, ignore: true, &block) filenames << ".env" if filenames.empty? filenames = filenames.reverse if overwrite @@ -61,9 +68,4 @@ def require_keys(*keys) return if missing_keys.empty? raise MissingKeys, missing_keys end - - def ignoring_nonexistent_files - yield - rescue Errno::ENOENT - end end