Validate Account options

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]

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/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/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) }
@@ -114,8 +114,8 @@ module Imap::Backup
       context "when accounts are to be deleted" do
         let(:accounts) do
           [
-            {name: "keep_me"},
-            {name: "delete_me", delete: true}
+            {username: "keep_me", password: "password1"},
+            {username: "delete_me", password: "password2"}
           ]
         end
 
@@ -191,7 +191,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