DEV: Review fixes (#10641)

See comments in https://review.discourse.org/t/dev-imap-log-to-database-10435/14337/6 for context.

app/models/imap_sync_log.rb

@@ -1,17 +1,12 @@
 # frozen_string_literal: true
 
 class ImapSyncLog < ActiveRecord::Base
-  RETAIN_LOGS_DAYS = 5
+  RETAIN_LOGS_DAYS ||= 5
 
   belongs_to :group
 
   def self.levels
-    @levels ||= Enum.new(
+    @levels ||= Enum.new(:debug, :info, :warn, :error)
-      debug: 1,
-      info: 2,
-      warn: 3,
-      error: 4
-    )
   end
 
   def self.log(message, level, group_id = nil)

db/migrate/20200910020909_make_imap_sync_log_cols_not_null.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+class MakeImapSyncLogColsNotNull < ActiveRecord::Migration[6.0]
+  def change
+    change_column_null :imap_sync_logs, :message, false
+    change_column_null :imap_sync_logs, :level, false
+  end
+end

lib/demon/email_sync.rb

@@ -68,9 +68,7 @@ class Demon::EmailSync < ::Demon::Base
 
     @sync_data.each do |db, sync_data|
       sync_data.each do |_, data|
-        data[:thread].kill
+        kill_and_disconnect!(data)
-        data[:thread].join
-        data[:syncer]&.disconnect! rescue nil
       end
     end
 
@@ -104,9 +102,7 @@ class Demon::EmailSync < ::Demon::Base
         next true if all_dbs.include?(db)
 
         sync_data.each do |_, data|
-          data[:thread].kill
+          kill_and_disconnect!(data)
-          data[:thread].join
-          data[:syncer]&.disconnect!
         end
 
         false
@@ -130,10 +126,7 @@ class Demon::EmailSync < ::Demon::Base
               ImapSyncLog.warn("Thread for group is dead", group_id)
             end
 
-            data[:thread].kill
+            kill_and_disconnect!(data)
-            data[:thread].join
-            data[:syncer]&.disconnect!
-
             false
           end
 
@@ -166,4 +159,14 @@ class Demon::EmailSync < ::Demon::Base
     STDERR.puts e.backtrace.join("\n")
     exit 1
   end
+
+  def kill_and_disconnect!(data)
+    data[:thread].kill
+    data[:thread].join
+    begin
+      data[:syncer]&.disconnect!
+    rescue Net::IMAP::ResponseError => err
+      puts "[EmailSync] Encountered a response error when disconnecting: #{err.to_s}"
+    end
+  end
 end

spec/jobs/cleanup_imap_sync_log_spec.rb

@@ -10,8 +10,8 @@ describe Jobs::CleanupImapSyncLog do
     log2 = ImapSyncLog.log("Test log 2", :debug)
     log3 = ImapSyncLog.log("Test log 3", :debug)
 
-    log2.update(created_at: Time.now - 6.days)
+    log2.update(created_at: 6.days.ago)
-    log3.update(created_at: Time.now - 7.days)
+    log3.update(created_at: 7.days.ago)
 
     job_class.execute({})