Merge branch 'fix/leagcy-mailbox-migration'

Author: joeyates

lib/imap/backup/serializer.rb

@@ -28,7 +28,41 @@ def self.folder_path_for(path:, folder:)
     extend Forwardable
 
     def_delegator :mbox, :pathname, :mbox_pathname
-    def_delegators :imap, :get, :messages, :uid_validity, :uids, :update_uid
+
+    # Get message metadata
+    # @param uid [Integer] a message UID
+    # @return [Serializer::Message]
+    def get(uid)
+      validate!
+      imap.get(uid)
+    end
+
+    # @return [Array<Hash>]
+    def messages
+      validate!
+      imap.messages
+    end
+
+    # @return [Integer] the UID validity for the folder
+    def uid_validity
+      validate!
+      imap.uid_validity
+    end
+
+    # @return [Array<Integer>] The uids of all messages
+    def uids
+      validate!
+      imap.uids
+    end
+
+    # Update a message's metadata, replacing its UID
+    # @param old [Integer] the existing message UID
+    # @param new [Integer] the new UID to apply to the message
+    # @return [void]
+    def update_uid(old, new)
+      validate!
+      imap.update_uid(old, new)
+    end
 
     # @return [String] a folder name
     attr_reader :folder
@@ -62,10 +96,14 @@ def validate!
 
       optionally_migrate2to3
 
-      if imap.valid? && mbox.valid?
+      imap_valid = imap.valid?
+      mbox_valid = mbox.valid?
+      if imap_valid && mbox_valid
         @validated = true
         return true
       end
+      Logger.logger.info("Metadata file '#{imap.pathname}' is invalid") if !imap_valid
+      Logger.logger.info("Mailbox '#{mbox.pathname}' is invalid") if !mbox_valid
 
       delete
 
@@ -247,6 +285,8 @@ def optionally_migrate2to3
       MESSAGE
 
       migrator.run
+      # Ensure new metadata gets loaded
+      @imap = nil
     end
 
     def ensure_containing_directory

lib/imap/backup/serializer/imap.rb

@@ -108,6 +108,7 @@ def get(uid)
     def delete
       return if !exist?
 
+      Logger.logger.info("Deleting metadata file '#{pathname}'")
       FileUtils.rm(pathname)
       @loaded = false
       @messages = nil

lib/imap/backup/serializer/mbox.rb

@@ -70,6 +70,7 @@ def read(offset, length)
     def delete
       return if !exist?
 
+      Logger.logger.info("Deleting mailbox '#{pathname}'")
       FileUtils.rm(pathname)
     end
 

spec/features/regressions/migrate_legacy_backups_spec.rb

@@ -0,0 +1,53 @@
+require "features/helper"
+
+RSpec.describe "imap-backup migrate: avoid regression in migrating legacy backups",
+               :container, type: :aruba do
+  def overwrite_metadata_with_old_version(email, folder)
+    content = imap_parsed(email, folder)
+    uids = content[:messages].map { |m| m[:uid] }
+    uid_validity = content[:uid_validity]
+    old_metadata = {version: 2, uids: uids, uid_validity: uid_validity}
+    path = imap_path(email, folder)
+    File.open(path, "w") { |f| f.write(JSON.pretty_generate(old_metadata)) }
+  end
+
+  let(:email) { "me@example.com" }
+  let(:folder) { "migrate-folder" }
+  let(:source_account) do
+    {
+      username: email,
+      local_path: File.join(config_path, email.gsub("@", "_"))
+    }
+  end
+  let(:destination_account) { test_server_connection_parameters }
+  let(:destination_server) { test_server }
+  let(:config_options) { {accounts: [source_account, destination_account]} }
+
+  let!(:setup) do
+    test_server.warn_about_non_default_folders
+    create_config(**config_options)
+    append_local(
+      email: email, folder: folder, subject: "Ciao", flags: [:Draft, :$CUSTOM]
+    )
+    overwrite_metadata_with_old_version(email, folder)
+  end
+
+  after do
+    destination_server.delete_folder folder
+    destination_server.disconnect
+  end
+
+  it "copies emails to the destination account" do
+    run_command_and_stop "imap-backup migrate #{email} #{destination_account[:username]}"
+
+    messages = test_server.folder_messages(folder)
+    expected = <<~MESSAGE.gsub("\n", "\r\n")
+      From: sender@example.com
+      Subject: Ciao
+
+      body
+
+    MESSAGE
+    expect(messages[0]["BODY[]"]).to eq(expected)
+  end
+end

spec/unit/serializer_spec.rb

@@ -21,8 +21,10 @@
     let(:mbox_valid) { true }
 
     before do
+      allow(imap).to receive(:pathname) { "imap pathname" }
       allow(imap).to receive(:valid?) { imap_valid }
       allow(imap).to receive(:delete)
+      allow(mbox).to receive(:pathname) { "mbox pathname" }
       allow(mbox).to receive(:valid?) { mbox_valid }
       allow(mbox).to receive(:delete)
 
@@ -33,23 +35,23 @@
       let(:imap_valid) { false }
 
       it "deletes the imap file" do
-        expect(imap).to have_received(:delete)
+        expect(imap).to have_received(:delete).at_least(:once)
       end
 
       it "deletes the mbox file" do
-        expect(mbox).to have_received(:delete)
+        expect(mbox).to have_received(:delete).at_least(:once)
       end
     end
 
     context "when the mbox file is not valid" do
       let(:mbox_valid) { false }
 
       it "deletes the imap file" do
-        expect(imap).to have_received(:delete)
+        expect(imap).to have_received(:delete).at_least(:once)
       end
 
       it "deletes the mbox file" do
-        expect(mbox).to have_received(:delete)
+        expect(mbox).to have_received(:delete).at_least(:once)
       end
     end
   end
@@ -307,6 +309,7 @@ module Imap::Backup
           Serializer::Imap, "Old Imap",
           uid_validity: 1,
           uids: [1],
+          valid?: true,
           get: message,
           delete: nil,
           folder_path: "existing/imap"
@@ -316,7 +319,8 @@ module Imap::Backup
         instance_double(
           Serializer::Mbox, "Old Mbox",
           delete: nil,
-          folder_path: "existing/mbox"
+          folder_path: "existing/mbox",
+          valid?: true
         )
       end
       let(:imap) do