From 156e1ec330df8b940872be93c727c61cf9ea9cc4 Mon Sep 17 00:00:00 2001
From: Nick LaMuro <nicklamuro@gmail.com>
Date: Wed, 9 May 2018 20:47:45 -0500
Subject: [PATCH] Handle multi-value args in PostgresAdmin.backup_pg_dump

There are a few args passable to `pg_dump` that can take multiple
values.  Since it is safer (in my eyes) to do `-t table1 -t table2`
instead of trying our luck with `-t table1 table2`, this change makes it
so the options passed to AwesomeSpawn do the former and not the latter.

To accomplish this, this means we have to move the arguments outside of
the `opts` hash, and into the `args` array.

Also added a pretty decent amount of tests around this to make sure it
works as expected.
---
 lib/gems/pending/util/postgres_admin.rb | 27 ++++++++-
 spec/util/postgres_admin_spec.rb        | 78 +++++++++++++++++++++++++
 2 files changed, 104 insertions(+), 1 deletion(-)

diff --git a/lib/gems/pending/util/postgres_admin.rb b/lib/gems/pending/util/postgres_admin.rb
index d8be2e708..5e0f8c5a7 100644
--- a/lib/gems/pending/util/postgres_admin.rb
+++ b/lib/gems/pending/util/postgres_admin.rb
@@ -121,7 +121,11 @@ def self.restore_pg_basebackup(file)
   def self.backup_pg_dump(opts)
     opts = opts.dup
     dbname = opts.delete(:dbname)
-    runcmd("pg_dump", opts, :format => "c", :file => opts[:local_file], nil => dbname)
+
+    args = combine_command_args(opts, :format => "c", :file => opts[:local_file], nil => dbname)
+    args = handle_multi_value_pg_dump_args!(opts, args)
+
+    runcmd_with_logging("pg_dump", opts, args)
     opts[:local_file]
   end
 
@@ -228,4 +232,25 @@ def self.runcmd_with_logging(cmd_str, opts, params = {})
     default_args[:host]     = opts[:hostname] if opts[:hostname]
     default_args.merge(args)
   end
+
+  # rubocop:disable Style/SymbolArray
+  PG_DUMP_MULTI_VALUE_ARGS = [
+    :t, :table,  :T, :"exclude-table", :"exclude-table-data",
+    :n, :schema, :N, :"exclude-schema"
+  ].freeze
+  # rubocop:enable Style/SymbolArray
+  #
+  # NOTE:  Potentially mutates opts hash (args becomes new array and not
+  # mutated by this method)
+  private_class_method def self.handle_multi_value_pg_dump_args!(opts, args)
+    if opts.keys.any? { |key| PG_DUMP_MULTI_VALUE_ARGS.include?(key) }
+      args = args.to_a
+      PG_DUMP_MULTI_VALUE_ARGS.each do |table_key|
+        next unless opts.key?(table_key)
+        table_val = opts.delete(table_key)
+        args += Array.wrap(table_val).map! { |v| [table_key, v] }
+      end
+    end
+    args
+  end
 end
diff --git a/spec/util/postgres_admin_spec.rb b/spec/util/postgres_admin_spec.rb
index a034e4090..376865347 100644
--- a/spec/util/postgres_admin_spec.rb
+++ b/spec/util/postgres_admin_spec.rb
@@ -76,6 +76,84 @@
         expect(subject.backup_pg_dump(opts)).to eq(local_file)
       end
     end
+
+    shared_examples "for splitting multi value arg" do |arg_type|
+      arg_as_cmdline_opt = "-#{'-' if arg_type.size > 1}#{arg_type}"
+
+      let(:local_file) { "/some/path/to/pg.dump" }
+
+      context "with #{arg_as_cmdline_opt} as a single value" do
+        let(:expected_opts) { { :local_file => local_file } }
+        let(:expected_args) do
+          default_args.to_a << [arg_type, "value1"]
+        end
+
+        it "runs the command and returns the :local_file opt" do
+          opts = expected_opts.merge(arg_type => "value1")
+          expect(subject.backup_pg_dump(opts)).to eq(local_file)
+        end
+      end
+
+      context "with #{arg_as_cmdline_opt} as a single value array" do
+        let(:expected_opts) { { :local_file => local_file } }
+        let(:expected_args) do
+          default_args.to_a << [arg_type, "value1"]
+        end
+
+        it "runs the command and returns the :local_file opt" do
+          opts = expected_opts.merge(arg_type => ["value1"])
+          expect(subject.backup_pg_dump(opts)).to eq(local_file)
+        end
+      end
+
+      context "with #{arg_as_cmdline_opt} as a multi value array" do
+        let(:expected_opts) { { :local_file => local_file } }
+        let(:expected_args) do
+          default_args.to_a << [arg_type, "value1"] << [arg_type, "value2"]
+        end
+
+        it "runs the command and returns the :local_file opt" do
+          opts = expected_opts.merge(arg_type => %w[value1 value2])
+          expect(subject.backup_pg_dump(opts)).to eq(local_file)
+        end
+      end
+    end
+
+    context "with :local_file, :t in opts" do
+      include_examples "for splitting multi value arg", :t
+    end
+
+    context "with :local_file, :table in opts" do
+      include_examples "for splitting multi value arg", :table
+    end
+
+    context "with :local_file, :T in opts" do
+      include_examples "for splitting multi value arg", :T
+    end
+
+    context "with :local_file, :exclude-table in opts" do
+      include_examples "for splitting multi value arg", :"exclude-table"
+    end
+
+    context "with :local_file, :exclude-table-data in opts" do
+      include_examples "for splitting multi value arg", :"exclude-table-data"
+    end
+
+    context "with :local_file, :t in opts" do
+      include_examples "for splitting multi value arg", :n
+    end
+
+    context "with :local_file, :table in opts" do
+      include_examples "for splitting multi value arg", :schema
+    end
+
+    context "with :local_file, :T in opts" do
+      include_examples "for splitting multi value arg", :N
+    end
+
+    context "with :local_file, :exclude-table in opts" do
+      include_examples "for splitting multi value arg", :"exclude-schema"
+    end
   end
 
   context "ENV dependent" do