Merge pull request ManageIQ#9127 from isimluk/rhbz#1341502

FileDepotFtp: FTP.nlst cannot distinguish empty from non-existent dir

app/models/file_depot_ftp.rb

@@ -93,12 +93,15 @@ def file_exists?(file_or_directory)
   private
 
   def create_directory_structure(directory_path)
-    Pathname.new(directory_path).descend do |path|
-      next if file_exists?(path)
-
-      _log.info("creating #{path}")
-      ftp.mkdir(path.to_s)
+    pwd = ftp.pwd
+    directory_path.to_s.split('/').each do |directory|
+      unless ftp.nlst.include?(directory)
+        _log.info("creating #{directory}")
+        ftp.mkdir(directory)
+      end
+      ftp.chdir(directory)
     end
+    ftp.chdir(pwd)
   end
 
   def upload(source, destination)

spec/models/file_depot_ftp_spec.rb

@@ -29,17 +29,28 @@
   end
 
   context "#upload_file" do
-    it "does not already exist" do
-      expect(file_depot_ftp).to receive(:connect).and_return(connection)
-      expect(file_depot_ftp).to receive(:file_exists?).exactly(4).times.and_return(false)
-      expect(connection).to receive(:mkdir).with("uploads")
-      expect(connection).to receive(:mkdir).with("uploads/#{@zone.name}_#{@zone.id}")
-      expect(connection).to receive(:mkdir).with("uploads/#{@zone.name}_#{@zone.id}/#{@miq_server.name}_#{@miq_server.id}")
-      expect(connection).to receive(:putbinaryfile)
-      expect(log_file).to receive(:post_upload_tasks)
-      expect(connection).to receive(:close)
+    it 'uploads file to vsftpd with existing directory structure' do
+      vsftpd = VsftpdMock.new('uploads' =>
+                              {"#{@zone.name}_#{@zone.id}" =>
+                              {"#{@miq_server.name}_#{@miq_server.id}" => {}}})
+      expect(file_depot_ftp).to receive(:connect).and_return(vsftpd)
+      file_depot_ftp.upload_file(log_file)
+      expect(vsftpd.content).to eq('uploads' =>
+                                   {"#{@zone.name}_#{@zone.id}" =>
+                                   {"#{@miq_server.name}_#{@miq_server.id}" =>
+                                   {"Current_region_unknown_#{@zone.name}_#{@zone.id}_#{@miq_server.name}_#{@miq_server.id}_unknown_unknown.txt" =>
+                                   log_file.local_file}}})
+    end
 
+    it 'uploads file to vsftpd with empty /uploads directory' do
+      vsftpd = VsftpdMock.new('uploads' => {})
+      expect(file_depot_ftp).to receive(:connect).and_return(vsftpd)
       file_depot_ftp.upload_file(log_file)
+      expect(vsftpd.content).to eq('uploads' =>
+                                   {"#{@zone.name}_#{@zone.id}" =>
+                                   {"#{@miq_server.name}_#{@miq_server.id}" =>
+                                   {"Current_region_unknown_#{@zone.name}_#{@zone.id}_#{@miq_server.name}_#{@miq_server.id}_unknown_unknown.txt" =>
+                                   log_file.local_file}}})
     end
 
     it "already exists" do
@@ -53,4 +64,51 @@
       file_depot_ftp.upload_file(log_file)
     end
   end
+
+  class FtpMock
+    attr_reader :pwd, :content
+    def initialize(content = {})
+      @pwd = '/'
+      @content = content
+    end
+
+    def chdir(dir)
+      newpath = (Pathname.new(pwd) + dir).to_s
+      if local(newpath).kind_of? Hash
+        @pwd = newpath
+      end
+    end
+
+    private
+
+    def local(path)
+      local = @content
+      path.split('/').each do |dir|
+        next if dir.empty?
+        local = local[dir]
+        raise Net::FTPPermError, '550 Failed to change directory.' if local.nil?
+      end
+      local
+    end
+  end
+
+  class VsftpdMock < FtpMock
+    def nlst(path = '')
+      l = local(pwd + path)
+      l.respond_to?(:keys) ? l.keys : []
+    rescue
+      return []
+    end
+
+    def mkdir(dir)
+      l = local(pwd)
+      l[dir] = {}
+    end
+
+    def putbinaryfile(local_path, remote_path)
+      dir, base = Pathname.new(remote_path).split
+      l = local(dir.to_s)
+      l[base.to_s] = local_path
+    end
+  end
 end