From b74748af2affec31244f0bedd69fbd7c411d3277 Mon Sep 17 00:00:00 2001
From: Owen Li <owen.a.li@ericsson.com>
Date: Mon, 13 Nov 2017 10:16:08 -0500
Subject: [PATCH] Allow deactivation loop to continue running after an error

Currently, the deactivation cron job stops if it fails to deactivate an
account. Change the behaviour such that it logs which account failed,
and continues instead of stopping prematurely.

Also refactor code to reduce nesting and increase readability.

Change-Id: I8542e18368e47500cadb7cbcab27769175023181
---
 .../server/account/AccountDeactivator.java    | 28 +++++++++++++++----
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountDeactivator.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountDeactivator.java
index c222756d20..bd9af61aa7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountDeactivator.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountDeactivator.java
@@ -96,24 +96,40 @@ public class AccountDeactivator implements Runnable {
 
   @Override
   public void run() {
-    log.debug("Running account deactivations");
+    log.info("Running account deactivations");
     try {
       int numberOfAccountsDeactivated = 0;
       for (AccountState acc : accountQueryProvider.get().query(AccountPredicates.isActive())) {
-        log.debug("processing account " + acc.getUserName());
-        if (acc.getUserName() != null && !realm.isActive(acc.getUserName())) {
-          sif.deactivate(acc.getAccount().getId());
-          log.debug("deactivated accout " + acc.getUserName());
+        if (processAccount(acc)) {
           numberOfAccountsDeactivated++;
         }
       }
       log.info(
           "Deactivations complete, {} account(s) were deactivated", numberOfAccountsDeactivated);
     } catch (Exception e) {
-      log.error("Failed to deactivate inactive accounts " + e.getMessage(), e);
+      log.error("Failed to complete deactivation of accounts: " + e.getMessage(), e);
     }
   }
 
+  private boolean processAccount(AccountState account) {
+    log.debug("processing account " + account.getUserName());
+    try {
+      if (account.getUserName() != null && !realm.isActive(account.getUserName())) {
+        sif.deactivate(account.getAccount().getId());
+        log.info("deactivated account " + account.getUserName());
+        return true;
+      }
+    } catch (Exception e) {
+      log.error(
+          "Error deactivating account: {} ({}) {}",
+          account.getUserName(),
+          account.getAccount().getId(),
+          e.getMessage(),
+          e);
+    }
+    return false;
+  }
+
   @Override
   public String toString() {
     return "account deactivator";