Ensure version 2->3 migration happens before any .imap access

The delegators were allowing access to the metadata before it
was migrated from the legacy version.
This meant that the list of UIDs was empty.
The second problem was that the Serializer::Imap instance
was not reloaded. This meant that it continued not be '#valid?'.
For this reason, after version migration both the '.imap' and '.mbox'
files were deleted.

Now, the migration happens on the first attempt at access to the
metadata and the fresh metadata is then loaded.

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

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

@@ -35,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
@@ -309,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"
@@ -318,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