Merge branch 'feature/simplify-folder-config'

lib/imap/backup/account/backup_folders.rb

@@ -24,7 +24,7 @@ def each(&block)
       configured =
         case
         when account.folders&.any?
-          account.folders.map { |af| af[:name] }
+          account.folders
         when account.folder_blacklist
           []
         else

lib/imap/backup/configuration.rb

@@ -11,7 +11,8 @@ module Imap; end
 module Imap::Backup
   class Configuration
     CONFIGURATION_DIRECTORY = File.expand_path("~/.imap-backup")
-    VERSION = "2.1".freeze
+    VERSION_2_1 = "2.1".freeze
+    VERSION = "2.2".freeze
     DEFAULT_STRATEGY = "delay_metadata".freeze
     DOWNLOAD_STRATEGIES = [
       {key: "direct", description: "write straight to disk"},
@@ -121,12 +122,30 @@ def data
             else
               DEFAULT_STRATEGY
             end
-          data
+          if data[:version] == VERSION_2_1
+            dehashify_folders(data)
+          else
+            data
+          end
         else
           {accounts: [], download_strategy: DEFAULT_STRATEGY}
         end
     end
 
+    def dehashify_folders(data)
+      data[:version] = VERSION
+
+      data[:accounts].each do |account|
+        next if !account.key?(:folders)
+
+        folders = account[:folders]
+        names = folders.map { |f| f[:name] }
+        account[:folders] = names
+      end
+
+      data
+    end
+
     def remove_modified_flags
       @download_strategy_modified = false
       accounts.each(&:clear_changes)

lib/imap/backup/setup/account/header.rb

@@ -78,7 +78,7 @@ def folders
       list =
         case
         when items.any?
-          items.map { |f| f[:name] }.join(", ")
+          items.join(", ")
         when !account.folder_blacklist
           "(all folders)"
         else

lib/imap/backup/setup/folder_chooser.rb

@@ -63,18 +63,18 @@ def selected?(folder_name)
       config_folders = account.folders
       return false if config_folders.nil?
 
-      config_folders.find { |f| f[:name] == folder_name }
+      config_folders.find { |f| f == folder_name }
     end
 
     def remove_missing
       removed = []
       config_folders = []
       account.folders.each do |f|
-        found = folder_names.find { |folder| folder == f[:name] }
+        found = folder_names.find { |folder| folder == f }
         if found
           config_folders << f
         else
-          removed << f[:name]
+          removed << f
         end
       end
 
@@ -91,11 +91,11 @@ def remove_missing
 
     def toggle_selection(folder_name)
       if selected?(folder_name)
-        new_list = account.folders.reject { |f| f[:name] == folder_name }
+        new_list = account.folders.reject { |f| f == folder_name }
         account.folders = new_list
       else
         existing = account.folders || []
-        account.folders = existing + [{name: folder_name}]
+        account.folders = existing << folder_name
       end
     end
 

spec/features/backup_spec.rb

@@ -5,15 +5,15 @@
 RSpec.describe "imap-backup backup", :docker, type: :aruba do
   include_context "message-fixtures"
 
-  let(:backup_folders) { [{name: folder}] }
+  let(:backup_folders) { [folder] }
   let(:folder) { "my-stuff" }
   let(:messages_as_mbox) do
     to_mbox_entry(**message_one) + to_mbox_entry(**message_two)
   end
   let(:account_config) do
     test_server_connection_parameters.merge(
       download_strategy: "delay_metadata",
-      folders: [{name: folder}]
+      folders: [folder]
     )
   end
   let(:account) { Imap::Backup::Account.new(account_config) }

spec/features/mirror_spec.rb

@@ -13,7 +13,7 @@
   let(:config_options) do
     {
       accounts: [
-        test_server_connection_parameters.merge(folders: [{name: source_folder}]),
+        test_server_connection_parameters.merge(folders: [source_folder]),
         other_server_connection_parameters
       ]
     }

spec/features/restore_spec.rb

@@ -3,9 +3,7 @@
 RSpec.describe "imap-backup restore", :docker, type: :aruba do
   include_context "message-fixtures"
 
-  let(:account_config) do
-    test_server_connection_parameters.merge(folders: [{name: folder}])
-  end
+  let(:account_config) { test_server_connection_parameters.merge(folders: [folder]) }
   let(:folder) { "my-stuff" }
   let(:messages_as_mbox) do
     to_mbox_entry(**message_one) + to_mbox_entry(**message_two)

spec/features/utils/export_to_thunderbird_spec.rb

@@ -3,9 +3,7 @@
 RSpec.describe "imap-backup utils export-to-thunderbird", :docker, type: :aruba do
   include_context "message-fixtures"
 
-  let(:account_config) do
-    test_server_connection_parameters.merge(folders: [{name: folder}])
-  end
+  let(:account_config) { test_server_connection_parameters.merge(folders: [folder]) }
   let(:email) { account_config[:username] }
   let(:folder) { "Foo" }
   let(:config_options) { {accounts: [account_config]} }

spec/features/utils/ignore_history_spec.rb

@@ -3,9 +3,7 @@
 RSpec.describe "imap-backup utils ignore-history", :docker, type: :aruba do
   include_context "message-fixtures"
 
-  let(:account_config) do
-    test_server_connection_parameters.merge(folders: [{name: folder}])
-  end
+  let(:account_config) { test_server_connection_parameters.merge(folders: [folder]) }
   let(:email) { account_config[:username] }
   let(:folder) { "my_folder" }
   let(:config_options) { {accounts: [account_config]} }

spec/performance/backup_spec.rb

@@ -11,8 +11,7 @@
   results = []
 
   before(:all) do
-    existing = test_server.folders.map(&:name)
-    existing.each do |folder|
+    test_server.folders.each do |folder|
       next if !folder.start_with?("bulk-")
 
       test_server.delete_folder folder
@@ -34,7 +33,7 @@
             context "with run #{run}" do
               let(:account_config) do
                 test_server_connection_parameters.merge(
-                  folders: [{name: folder}],
+                  folders: [folder],
                   multi_fetch_size: multi_fetch_size
                 )
               end
@@ -74,8 +73,7 @@
   end
 
   after(:all) do
-    existing = test_server.folders.map(&:name)
-    existing.each do |folder|
+    test_server.folders.each do |folder|
       next if !folder.start_with?("bulk-")
 
       test_server.delete_folder folder

spec/unit/imap/backup/account/backup_folders_spec.rb

@@ -10,7 +10,7 @@ module Imap::Backup
       )
     end
     let(:client) { instance_double(Client::Default, list: %w(foo bar baz)) }
-    let(:account_folders) { [{name: "foo"}] }
+    let(:account_folders) { ["foo"] }
     let(:folder_blacklist) { false }
     let(:result) { subject.each }
 

spec/unit/imap/backup/configuration_spec.rb

@@ -9,8 +9,7 @@ module Imap::Backup
     let(:file_path) { File.join(directory, "/config.json") }
     let(:file_exists) { true }
     let(:directory_exists) { true }
-    let(:configuration) { data.to_json }
-    let(:data) { {accounts: accounts.map(&:to_h)} }
+    let(:configuration) { {accounts: accounts.map(&:to_h)}.to_json }
     let(:accounts) do
       [
         Account.new({username: "username1"}),
@@ -30,6 +29,8 @@ module Imap::Backup
       allow(Serializer::PermissionChecker).to receive(:new) { permission_checker }
       allow(File).to receive(:read).and_call_original
       allow(File).to receive(:read).with(file_path) { configuration }
+      allow(FileUtils).to receive(:chmod)
+      allow(FileUtils).to receive(:mkdir_p)
     end
 
     describe ".exist?" do
@@ -80,11 +81,8 @@ module Imap::Backup
       let(:file) { instance_double(File, write: nil) }
 
       before do
-        allow(FileUtils).to receive(:mkdir_p)
-        allow(FileUtils).to receive(:chmod)
         allow(File).to receive(:open).and_call_original
         allow(File).to receive(:open).with(file_path, "w").and_yield(file)
-        allow(JSON).to receive(:pretty_generate) { "JSON output" }
       end
 
       it "creates the config directory" do
@@ -94,7 +92,13 @@ module Imap::Backup
       end
 
       it "saves the configuration" do
-        expect(file).to receive(:write).with("JSON output")
+        expect(file).to receive(:write).with(/^\{/)
+
+        subject.save
+      end
+
+      it "saves the version" do
+        expect(file).to receive(:write).with(/"version": "[\d\.]+"/)
 
         subject.save
       end
@@ -103,8 +107,7 @@ module Imap::Backup
         allow(subject.accounts[0]).to receive(:to_h) { "Account1" }
         allow(subject.accounts[1]).to receive(:to_h) { "Account2" }
 
-        expect(JSON).to receive(:pretty_generate).
-          with(hash_including({accounts: %w[Account1 Account2]}))
+        expect(file).to receive(:write).with(/"accounts": \[\s+"Account1",\s+"Account2"\s+\]/)
 
         subject.save
       end
@@ -185,6 +188,31 @@ module Imap::Backup
         end
       end
     end
+
+    context "when a version 2.1 file is present" do
+      let(:file) { instance_double(File, write: nil) }
+      let(:configuration) do
+        {version: "2.1", accounts: [{username: "account", folders: [{name: "foo"}]}]}.to_json
+      end
+
+      before do
+        allow(File).to receive(:open).and_call_original
+        allow(File).to receive(:open).with(file_path, "w").and_yield(file)
+      end
+
+      it "changes account folders from an array of hashes to an array" do
+        subject.save
+
+        expect(file).to have_received(:write).
+          with(
+            /
+            "folders":\s+\[\s+
+              "foo"\s+
+            \]
+            /x
+          )
+      end
+    end
   end
 end
 

spec/unit/imap/backup/setup/account/header_spec.rb

@@ -11,7 +11,7 @@ module Imap::Backup
         username: "user@example.com",
         password: existing_password,
         local_path: "/backup/path",
-        folders: [{name: "my_folder"}],
+        folders: ["my_folder"],
         mirror_mode: mirror_mode,
         server: "imap.example.com",
         connection_options: connection_options,

spec/unit/imap/backup/setup/folder_chooser_spec.rb

@@ -42,7 +42,7 @@ module Imap::Backup
       end
 
       describe "folder listing" do
-        let(:configured_folders) { [{name: "my_folder"}] }
+        let(:configured_folders) { ["my_folder"] }
         let(:online_folders) do
           # N.B. my_folder is already backed up
           %w(my_folder another_folder)
@@ -69,7 +69,7 @@ module Imap::Backup
 
           specify "are added to the account" do
             expect(account).to have_received(:"folders=").
-              with([{name: "my_folder"}, {name: "another_folder"}])
+              with(%w(my_folder another_folder))
           end
         end
 
@@ -87,9 +87,7 @@ module Imap::Backup
       end
 
       context "with missing remote folders" do
-        let(:configured_folders) do
-          [{name: "on_server"}, {name: "not_on_server"}]
-        end
+        let(:configured_folders) { %w(on_server not_on_server) }
 
         before do
           allow(Kernel).to receive(:puts)
@@ -98,7 +96,7 @@ module Imap::Backup
 
         specify "are removed from the account" do
           expect(account).to have_received(:"folders=").
-            with([{name: "on_server"}])
+            with(["on_server"])
         end
       end