Merge branch 'feature/mirror-mode-for-single-backup'

lib/imap/backup/account.rb

@@ -59,6 +59,7 @@ class Account
     attr_reader :reset_seen_flags_after_fetch
 
     def initialize(options)
+      check_options!(options)
       @username = options[:username]
       @password = options[:password]
       @local_path = options[:local_path]
@@ -97,14 +98,6 @@ def capabilities
       client.capability
     end
 
-    # Indicates whether the account has been configured, and is ready
-    # to be used
-    #
-    # @return [Boolean]
-    def valid?
-      username && password ? true : false
-    end
-
     def modified?
       changes.any?
     end
@@ -245,6 +238,23 @@ def reset_seen_flags_after_fetch=(value)
 
     attr_reader :changes
 
+    REQUIRED_ATTRIBUTES = %i[password username].freeze
+    OPTIONAL_ATTRIBUTES = %i[
+      connection_options download_strategy folders folder_blacklist local_path mirror_mode
+      multi_fetch_size reset_seen_flags_after_fetch server
+    ].freeze
+    KNOWN_ATTRIBUTES = REQUIRED_ATTRIBUTES + OPTIONAL_ATTRIBUTES
+
+    def check_options!(options)
+      missing_required = REQUIRED_ATTRIBUTES - options.keys
+      if missing_required.any?
+        raise ArgumentError, "Missing required options: #{missing_required.join(', ')}"
+      end
+
+      unknown = options.keys - KNOWN_ATTRIBUTES
+      raise ArgumentError, "Unknown options: #{unknown.join(', ')}" if unknown.any?
+    end
+
     def update(field, value)
       key = :"@#{field}"
       if changes[field]

lib/imap/backup/cli/single/backup.rb

@@ -28,7 +28,7 @@ def run
         download_strategy: download_strategy,
         folder_blacklist: folder_blacklist,
         local_path: local_path,
-        mirror: mirror,
+        mirror_mode: mirror,
         reset_seen_flags_after_fetch: reset_seen_flags_after_fetch
       )
       account.connection_options = connection_options if connection_options

spec/features/backup_spec.rb

@@ -187,12 +187,8 @@
 
     let!(:setup) do
       create_directory account_config[:local_path]
-      message = "existing mbox"
-      valid_imap_data = {
-        version: 3, uid_validity: 1, messages: [{uid: 1, offset: 0, length: message.length}]
-      }
-      File.write(imap_path, valid_imap_data.to_json)
-      File.write(mbox_path, message)
+      File.write(imap_path, "existing imap")
+      File.write(mbox_path, "existing mbox")
       super()
     end
 

spec/features/local/list_accounts_spec.rb

@@ -1,7 +1,7 @@
 require "features/helper"
 
 RSpec.describe "imap-backup local accounts", type: :aruba do
-  let(:config_options) { {accounts: [{username: "me@example.com"}]} }
+  let(:config_options) { {accounts: [{username: "me@example.com", password: "password1"}]} }
   let(:command) { "imap-backup local accounts" }
 
   before do
@@ -24,7 +24,9 @@
 
   context "when a config path is supplied" do
     let(:custom_config_path) { File.join(File.expand_path("~/.imap-backup"), "foo.json") }
-    let(:config_options) { {path: custom_config_path, accounts: [{username: "other@example.com"}]} }
+    let(:config_options) do
+      {path: custom_config_path, accounts: [{username: "other@example.com", password: "password1"}]}
+    end
     let(:command) { "imap-backup local accounts --config #{custom_config_path}" }
 
     it "lists accounts from the supplied config file" do

spec/features/migrate_spec.rb

@@ -7,6 +7,7 @@
   let(:source_account) do
     {
       username: email,
+      password: "password1",
       local_path: File.join(config_path, email.gsub("@", "_"))
     }
   end

spec/features/regressions/migrate_legacy_backups_spec.rb

@@ -16,6 +16,7 @@ def overwrite_metadata_with_old_version(email, folder)
   let(:source_account) do
     {
       username: email,
+      password: "password1",
       local_path: File.join(config_path, email.gsub("@", "_"))
     }
   end

spec/features/single/backup_spec.rb

@@ -36,4 +36,30 @@
     actual = mbox_content(account[:username], folder, local_path: account[:local_path])
     expect(actual).to eq(messages_as_mbox)
   end
+
+  context "in mirror mode" do
+    let(:imap_path) { File.join(account[:local_path], "Foo.imap") }
+    let(:mbox_path) { File.join(account[:local_path], "Foo.mbox") }
+    let(:command) { "#{super()} --mirror" }
+
+    before do
+      create_directory account[:local_path]
+      File.write(imap_path, "existing imap")
+      File.write(mbox_path, "existing mbox")
+    end
+
+    context "with folders that are not being backed up" do
+      it "deletes .imap files" do
+        run_command_and_stop command
+
+        expect(File.exist?(imap_path)).to be false
+      end
+
+      it "deletes .mbox files" do
+        run_command_and_stop command
+
+        expect(File.exist?(mbox_path)).to be false
+      end
+    end
+  end
 end

spec/unit/account_spec.rb

@@ -8,62 +8,83 @@ module Imap::Backup
 
     let(:options) { {username: "user", password: "pwd"} }
 
-    describe "#local_path" do
-      let(:options) { {username: "user", password: "pwd", local_path: "local_path"} }
+    describe "#initialize" do
+      context "with valid options" do
+        let(:options) do
+          {
+            username: "user", password: "pwd", folder_blacklist: true, mirror_mode: true,
+            local_path: "local_path", folders: ["folder"]
+          }
+        end
 
-      it "returns the supplied local_path" do
-        expect(subject.local_path).to eq("local_path")
-      end
-    end
+        it "sets the username" do
+          expect(subject.username).to eq("user")
+        end
 
-    describe "#folders" do
-      let(:options) { {username: "user", password: "pwd", folders: ["folder"]} }
+        it "sets the password" do
+          expect(subject.password).to eq("pwd")
+        end
 
-      it "returns the supplied folders" do
-        expect(subject.folders).to eq(["folder"])
-      end
-    end
+        it "sets the folders" do
+          expect(subject.folders).to eq(["folder"])
+        end
 
-    describe "#folder_blacklist" do
-      let(:options) { {username: "user", password: "pwd", folder_blacklist: true} }
+        it "sets the folder_blacklist" do
+          expect(subject.folder_blacklist).to be true
+        end
 
-      it "returns the supplied folder_blacklist" do
-        expect(subject.folder_blacklist).to be true
-      end
+        it "sets the local_path" do
+          expect(subject.local_path).to eq("local_path")
+        end
 
-      it "defaults to false" do
-        expect(described_class.new({}).folder_blacklist).to be false
+        it "sets the mirror_mode" do
+          expect(subject.mirror_mode).to be true
+        end
+
+        it "sets marked_for_deletion to false" do
+          expect(subject.marked_for_deletion?).to be false
+        end
       end
-    end
 
-    describe "#mirror_mode" do
-      let(:options) { {username: "user", password: "pwd", mirror_mode: true} }
+      context "without optional options" do
+        it "sets folder_blacklist to false" do
+          expect(subject.folder_blacklist).to be false
+        end
 
-      it "returns the supplied mirror_mode" do
-        expect(subject.mirror_mode).to be true
-      end
+        it "sets mirror_mode to false" do
+          expect(subject.mirror_mode).to be false
+        end
 
-      it "defaults to false" do
-        expect(described_class.new({}).mirror_mode).to be false
+        it "sets server to nil" do
+          expect(subject.server).to be_nil
+        end
       end
-    end
 
-    describe "#server" do
-      let(:options) { {username: "user", password: "pwd", server: "server"} }
+      context "with missing required options" do
+        let(:options) { {} }
 
-      it "returns the supplied server" do
-        expect(subject.server).to eq("server")
+        it "raises an error" do
+          expect do
+            described_class.new(options)
+          end.to raise_error(ArgumentError, /Missing required options: password, username/)
+        end
       end
 
-      it "defaults to nil" do
-        expect(described_class.new({}).server).to be_nil
+      context "with invalid options" do
+        let(:options) { {username: "a", password: "b", not_a_valid_option: "value"} }
+
+        it "raises an error" do
+          expect do
+            described_class.new(options)
+          end.to raise_error(ArgumentError, /Unknown options: not_a_valid_option/)
+        end
       end
     end
 
     describe "#connection_options" do
-      let(:options) { {username: "user", password: "pwd", connection_options: '{"foo": "bar"}'} }
-
       context "when the supplied connection_options is a String" do
+        let(:options) { {username: "user", password: "pwd", connection_options: '{"foo": "bar"}'} }
+
         it "returns the parsed connection_options" do
           expect(subject.connection_options).to eq({foo: "bar"})
         end
@@ -77,8 +98,10 @@ module Imap::Backup
         end
       end
 
-      it "defaults to nil" do
-        expect(described_class.new({}).connection_options).to be_nil
+      context "when not set" do
+        it "defaults to nil" do
+          expect(subject.connection_options).to be_nil
+        end
       end
     end
 
@@ -97,30 +120,6 @@ module Imap::Backup
       end
     end
 
-    describe "#valid?" do
-      context "with username and password" do
-        it "is true" do
-          expect(subject.valid?).to be true
-        end
-      end
-
-      context "without a username" do
-        let(:options) { {password: "pwd"} }
-
-        it "is false" do
-          expect(subject.valid?).to be false
-        end
-      end
-
-      context "without a password" do
-        let(:options) { {username: "user"} }
-
-        it "is false" do
-          expect(subject.valid?).to be false
-        end
-      end
-    end
-
     describe "#modified?" do
       context "with changes" do
         it "is true" do
@@ -265,7 +264,7 @@ module Imap::Backup
       [:connection_options, '{"some": "option"}', {some: "option"}]
     ].each do |attribute, value, expected|
       describe "setting ##{attribute}=" do
-        let(:options) { {} }
+        let(:options) { {username: "original", password: "original"} }
 
         before { subject.send(:"#{attribute}=", value) }
 

spec/unit/cli/single/backup_spec.rb

@@ -216,7 +216,7 @@ module Imap::Backup
         subject.run
 
         expect(Account).to have_received(:new).
-          with(hash_including(mirror: true))
+          with(hash_including(mirror_mode: true))
       end
     end
 
@@ -227,7 +227,7 @@ module Imap::Backup
         subject.run
 
         expect(Account).to have_received(:new).
-          with(hash_including(mirror: false))
+          with(hash_including(mirror_mode: false))
       end
     end
 

spec/unit/configuration_spec.rb

@@ -11,8 +11,8 @@ module Imap::Backup
     let(:configuration) { {accounts: accounts.map(&:to_h)}.to_json }
     let(:accounts) do
       [
-        Account.new({username: "username1"}),
-        Account.new({username: "username2"})
+        Account.new({username: "username1", password: "password1"}),
+        Account.new({username: "username2", password: "password2"})
       ]
     end
     let(:permission_checker) { instance_double(Serializer::PermissionChecker, run: nil) }
@@ -102,34 +102,30 @@ module Imap::Backup
         subject.save
       end
 
-      it "uses the Account#to_h method" do
-        allow(subject.accounts[0]).to receive(:to_h) { "Account1" }
-        allow(subject.accounts[1]).to receive(:to_h) { "Account2" }
-
-        expect(file).to receive(:write).with(/"accounts": \[\s+"Account1",\s+"Account2"\s+\]/)
+      it "serializes all Account data" do
+        serialized = nil
+        allow(file).to receive(:write) { |data| serialized = data }
 
         subject.save
-      end
 
-      context "when accounts are to be deleted" do
-        let(:accounts) do
-          [
-            {name: "keep_me"},
-            {name: "delete_me", delete: true}
-          ]
-        end
+        parsed = JSON.parse(serialized, symbolize_names: true)
 
-        before do
-          allow(subject.accounts[0]).to receive(:to_h) { "Account1" }
-          allow(subject.accounts[1]).to receive(:to_h) { "Account2" }
-          subject.accounts[0].mark_for_deletion
-        end
+        expect(parsed[:accounts].first).to eq(
+          {
+            username: "username1", password: "password1",
+            multi_fetch_size: 1
+          }
+        )
+      end
 
+      context "when accounts are to be deleted" do
         it "does not save them" do
-          expect(JSON).to receive(:pretty_generate).
-            with(hash_including({accounts: ["Account2"]}))
+          subject.accounts[0].mark_for_deletion
 
           subject.save
+
+          expect(file).to have_received(:write).with(/"username2"/)
+          expect(file).to_not have_received(:write).with(/"username1"/)
         end
       end
 
@@ -191,7 +187,7 @@ module Imap::Backup
     context "when a folders are stored as Hashes" do
       let(:file) { instance_double(File, write: nil) }
       let(:configuration) do
-        {accounts: [{username: "account", folders: [{name: "foo"}]}]}.to_json
+        {accounts: [{username: "account", password: "ciao", folders: [{name: "foo"}]}]}.to_json
       end
 
       before do