From ad4ec3a76ec00f3b15b5f7aa8956ded6a877d5db Mon Sep 17 00:00:00 2001
From: Nick LaMuro <nicklamuro@gmail.com>
Date: Mon, 5 Nov 2018 15:40:09 -0600
Subject: [PATCH] Fix uri dirname parsing for swift restore

The swift protocol is the only file storage used by `EvmDatabaseOps`
at this time that uses query params as part of setting it's options.

The previous implementation of `.with_file_storage` would use a simple
`File.dirname` to determine the mount point.  This causes the query
params to be completely stripped from the uri, and messes with the
configuration set by the user via the query params.

There is also a quirk with the implementation of `URI::FTP` (stdlib)
that overrides `.path` differently since it swaps it's `"/"` characters
for `"%2F"`, which we don't happening unannounced.

So for the fix, we use `URI::Generic.new` directly, and avoid the
`URI.parse` method since it will make use of `URI::FTP` in those cases.
Split is used as just a convenient way of getting the args for
`URI::Generic.new`.
---
 lib/evm_database_ops.rb           |  5 ++++-
 spec/lib/evm_database_ops_spec.rb | 18 ++++++++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/lib/evm_database_ops.rb b/lib/evm_database_ops.rb
index a2ae54822c0f..ee5391ce32bf 100644
--- a/lib/evm_database_ops.rb
+++ b/lib/evm_database_ops.rb
@@ -111,7 +111,10 @@ def self.restore(db_opts, connect_opts = {})
     if db_opts[:local_file].nil?
       if action == :restore
         uri = connect_opts[:uri]
-        connect_opts[:uri] = File.dirname(connect_opts[:uri])
+
+        connect_uri        = URI::Generic.new(*URI.split(uri))
+        connect_uri.path   = File.dirname(connect_uri.path)
+        connect_opts[:uri] = connect_uri.to_s
       else
         connect_opts[:remote_file_name] ||= File.basename(backup_file_name(action))
         #
diff --git a/spec/lib/evm_database_ops_spec.rb b/spec/lib/evm_database_ops_spec.rb
index 397a02b36129..8f93f16c8ed9 100644
--- a/spec/lib/evm_database_ops_spec.rb
+++ b/spec/lib/evm_database_ops_spec.rb
@@ -291,10 +291,28 @@ def execute_with_file_storage(action = :backup)
           execute_with_file_storage(:restore)
         end
 
+        it "parses the dirname of the `uri` and passes that in `connect_opts`" do
+          expected_connect_opts = { :uri => "smb://tmp/" }
+          allow(file_storage).to receive(:download)
+          expect(MiqFileStorage).to receive(:with_interface_class).with(expected_connect_opts)
+          execute_with_file_storage(:restore)
+        end
+
         it "returns calculated uri" do
           allow(file_storage).to receive(:download).and_yield(input_path)
           expect(execute_with_file_storage(:restore) { "block result" }).to eq("smb://tmp/foo")
         end
+
+        context "with query_params in the URI" do
+          let(:connect_opts) { { :uri => "swift://container/foo.gz?2plus2=5" } }
+
+          it "retains query_params when parsing dirname" do
+            expected_connect_opts = { :uri => "swift://container/?2plus2=5" }
+            allow(file_storage).to receive(:download)
+            expect(MiqFileStorage).to receive(:with_interface_class).with(expected_connect_opts)
+            execute_with_file_storage(:restore)
+          end
+        end
       end
     end
   end