fix: DeleteAccount fails mailbox startup during bootstrap causing installation to be stuck

store/src/main/java/com/zextras/mailbox/messageBroker/CreateMessageBrokerException.java

@@ -0,0 +1,9 @@
+package com.zextras.mailbox.messageBroker;
+
+public class CreateMessageBrokerException extends Exception {
+
+	public CreateMessageBrokerException(Exception e) {
+		super("Cannot create message broker client", e);
+	}
+
+}

store/src/main/java/com/zextras/mailbox/messageBroker/MessageBrokerFactory.java

@@ -0,0 +1,35 @@
+package com.zextras.mailbox.messageBroker;
+
+import com.zextras.carbonio.message_broker.MessageBrokerClient;
+import com.zextras.carbonio.message_broker.config.enums.Service;
+import com.zextras.mailbox.client.ServiceDiscoverHttpClient;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+public class MessageBrokerFactory {
+
+	public static MessageBrokerClient getMessageBrokerClientInstance()
+			throws CreateMessageBrokerException {
+		Path filePath = Paths.get("/etc/carbonio/mailbox/service-discover/token");
+		String token;
+		try {
+			token = Files.readString(filePath);
+			ServiceDiscoverHttpClient serviceDiscoverHttpClient =
+					ServiceDiscoverHttpClient.defaultURL("carbonio-message-broker")
+							.withToken(token);
+
+			return MessageBrokerClient.fromConfig(
+							"127.78.0.7",
+							20005,
+							serviceDiscoverHttpClient.getConfig("default/username")
+									.getOrElse("carbonio-message-broker"),
+							serviceDiscoverHttpClient.getConfig("default/password").getOrElse("")
+					)
+					.withCurrentService(Service.MAILBOX);
+		} catch (IOException e) {
+			throw new CreateMessageBrokerException(e);
+		}
+	}
+}

store/src/main/java/com/zimbra/cs/account/callback/AccountStatus.java

@@ -5,11 +5,9 @@
 
 package com.zimbra.cs.account.callback;
 
-import java.util.Map;
-import java.util.Set;
-
 import com.zextras.carbonio.message_broker.MessageBrokerClient;
 import com.zextras.carbonio.message_broker.events.services.mailbox.UserStatusChanged;
+import com.zextras.mailbox.messageBroker.MessageBrokerFactory;
 import com.zimbra.common.account.Key;
 import com.zimbra.common.service.ServiceException;
 import com.zimbra.common.util.ZimbraLog;
@@ -19,7 +17,8 @@
 import com.zimbra.cs.account.DistributionList;
 import com.zimbra.cs.account.Entry;
 import com.zimbra.cs.account.Provisioning;
-import com.zimbra.cs.service.admin.AdminService;
+import java.util.Map;
+import java.util.Set;
 
 public class AccountStatus extends AttributeCallback {
 
@@ -80,11 +79,7 @@ private void publishStatusChangedEvent(Account account) {
         String userId = account.getId();
 
         try {
-            MessageBrokerClient messageBrokerClient = AdminService.getMessageBrokerClientInstance();
-            if (!messageBrokerClient.healthCheck()) {
-              ZimbraLog.account.warn("Message broker is not reachable, this can happen if message broker is not installed");
-              return;
-            }
+            MessageBrokerClient messageBrokerClient = MessageBrokerFactory.getMessageBrokerClientInstance();
             boolean result = messageBrokerClient.publish(new UserStatusChanged(userId, status.toUpperCase()));
             if (result) {
                 ZimbraLog.account.info("Published status changed event for user: " + userId);

store/src/main/java/com/zimbra/cs/service/admin/AdminService.java

@@ -6,11 +6,9 @@
 package com.zimbra.cs.service.admin;
 
 import com.zextras.carbonio.message_broker.MessageBrokerClient;
-import com.zextras.carbonio.message_broker.config.enums.Service;
-import com.zextras.carbonio.message_broker.events.services.mailbox.UserDeleted;
 import com.zextras.mailbox.account.usecase.DeleteUserUseCase;
 import com.zextras.mailbox.acl.AclService;
-import com.zextras.mailbox.client.ServiceDiscoverHttpClient;
+import com.zextras.mailbox.messageBroker.MessageBrokerFactory;
 import com.zimbra.common.service.ServiceException;
 import com.zimbra.common.soap.AdminConstants;
 import com.zimbra.common.soap.Element;
@@ -20,11 +18,7 @@
 import com.zimbra.cs.mailbox.MailboxManager;
 import com.zimbra.soap.DocumentDispatcher;
 import com.zimbra.soap.DocumentService;
-
-import java.io.IOException;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.nio.file.Paths;
+import io.vavr.control.Try;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -401,29 +395,7 @@ public static Map<String, Object> getAttrs(Element request, boolean ignoreEmptyV
     return result;
   }
 
-  protected MessageBrokerClient getMessageBroker() {
-      return getMessageBrokerClientInstance();
-  }
-
-  public static MessageBrokerClient getMessageBrokerClientInstance() {
-    Path filePath = Paths.get("/etc/carbonio/mailbox/service-discover/token");
-    String token;
-    try {
-      token = Files.readString(filePath);
-    } catch (IOException e) {
-      throw new RuntimeException("Can't read consul token from file", e);
-    }
-
-    ServiceDiscoverHttpClient serviceDiscoverHttpClient =
-        ServiceDiscoverHttpClient.defaultURL("carbonio-message-broker")
-            .withToken(token);
-
-    return MessageBrokerClient.fromConfig(
-            "127.78.0.7",
-            20005,
-            serviceDiscoverHttpClient.getConfig("default/username").getOrElse("carbonio-message-broker"),
-            serviceDiscoverHttpClient.getConfig("default/password").getOrElse("")
-        )
-        .withCurrentService(Service.MAILBOX);
+  protected Try<MessageBrokerClient> getMessageBroker() {
+      return Try.of(MessageBrokerFactory::getMessageBrokerClientInstance);
   }
 }

store/src/main/java/com/zimbra/cs/service/admin/DeleteAccount.java

@@ -24,6 +24,7 @@
 import com.zimbra.soap.admin.message.DeleteAccountRequest;
 import com.zimbra.soap.admin.message.DeleteAccountResponse;
 
+import io.vavr.control.Try;
 import java.util.List;
 import java.util.Map;
 
@@ -35,11 +36,11 @@ public class DeleteAccount extends AdminDocumentHandler {
   private static final String[] TARGET_ACCOUNT_PATH = new String[] {AdminConstants.E_ID};
 
   private final DeleteUserUseCase deleteUserUseCase;
-  private final MessageBrokerClient messageBrokerClient;
+  private final Try<MessageBrokerClient> messageBrokerClientTry;
 
-  public DeleteAccount(DeleteUserUseCase deleteUserUseCase, MessageBrokerClient messageBrokerClient) {
+  public DeleteAccount(DeleteUserUseCase deleteUserUseCase, Try<MessageBrokerClient> messageBrokerClientTry) {
     this.deleteUserUseCase = deleteUserUseCase;
-    this.messageBrokerClient = messageBrokerClient;
+    this.messageBrokerClientTry = messageBrokerClientTry;
   }
 
   @Override
@@ -111,12 +112,9 @@ public void docRights(List<AdminRight> relatedRights, List<String> notes) {
 
   private void publishAccountDeletedEvent(Account account) {
     String userId = account.getId();
-    if (!messageBrokerClient.healthCheck()) {
-      ZimbraLog.account.warn("Message broker is not reachable, this can happen if message broker is not installed");
-      return;
-    }
     try {
-      boolean result = messageBrokerClient.publish(new UserDeleted(userId));
+			final MessageBrokerClient messageBrokerClient = messageBrokerClientTry.get();
+			boolean result = messageBrokerClient.publish(new UserDeleted(userId));
       if (result) {
         ZimbraLog.account.info("Published deleted account event for user: " + userId);
       } else {

store/src/test/java/com/zextras/mailbox/callbacks/AccountStatusChangedCallbackTest.java

@@ -6,12 +6,12 @@
 
 import com.zextras.carbonio.message_broker.MessageBrokerClient;
 import com.zextras.carbonio.message_broker.events.services.mailbox.UserStatusChanged;
+import com.zextras.mailbox.messageBroker.MessageBrokerFactory;
 import com.zextras.mailbox.util.MailboxTestUtil;
 import com.zimbra.cs.account.Account;
 import com.zimbra.cs.account.Provisioning;
 import com.zimbra.cs.account.callback.AccountStatus;
 import com.zimbra.cs.account.callback.CallbackContext;
-import com.zimbra.cs.service.admin.AdminService;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
@@ -33,7 +33,7 @@ void setUp() throws Exception {
    * This just tests that if calls are successful no other exceptions are thrown.
    */
   @Test
-  void shouldNotFailWhenExecutingUserStatusChangedCallback(){
+  void shouldNotFailWhenExecutingUserStatusChangedCallback() throws Exception {
     CallbackContext context = Mockito.mock(CallbackContext.class);
     String attrName = "fake";
     Account entry = Mockito.mock(Account.class);
@@ -43,7 +43,7 @@ void shouldNotFailWhenExecutingUserStatusChangedCallback(){
     Mockito.when(entry.getAccountStatus(any(Provisioning.class))).thenReturn("active");
     Mockito.when(entry.getId()).thenReturn("fake-account-id");
 
-    MessageBrokerClient mockedMessageBrokerClient = AdminService.getMessageBrokerClientInstance();
+    MessageBrokerClient mockedMessageBrokerClient = MessageBrokerFactory.getMessageBrokerClientInstance();
     Mockito.when(mockedMessageBrokerClient.publish(any(UserStatusChanged.class))).thenReturn(true);
 
     accountStatus.postModify(context, attrName, entry);

store/src/test/java/com/zextras/mailbox/messageBroker/MessageBrokerFactoryTest.java

@@ -0,0 +1,14 @@
+package com.zextras.mailbox.messageBroker;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+class MessageBrokerFactoryTest {
+
+	@Test
+	void shouldFailCreatingClient_WhenConsulTokenIsMissing() throws Exception {
+		Assertions.assertThrows(CreateMessageBrokerException.class,
+				MessageBrokerFactory::getMessageBrokerClientInstance);
+	}
+
+}

store/src/test/java/com/zextras/mailbox/util/MailboxTestUtil.java

@@ -4,7 +4,7 @@
 
 import com.unboundid.ldap.listener.InMemoryDirectoryServer;
 import com.zextras.carbonio.message_broker.MessageBrokerClient;
-import com.zextras.carbonio.message_broker.config.enums.Service;
+import com.zextras.mailbox.messageBroker.MessageBrokerFactory;
 import com.zextras.mailbox.util.InMemoryLdapServer.Builder;
 import com.zimbra.common.account.ZAttrProvisioning;
 import com.zimbra.common.localconfig.LC;
@@ -122,8 +122,8 @@ public static void initData() throws Exception {
   private static void mockMessageBrokerClient() {
     if(mockedMessageBrokerClient == null) {
       MessageBrokerClient messageBrokerClient = Mockito.mock(MessageBrokerClient.class);
-      MockedStatic<AdminService> mockedAdminServiceStatic = Mockito.mockStatic(AdminService.class, Mockito.CALLS_REAL_METHODS);
-      mockedAdminServiceStatic.when(AdminService::getMessageBrokerClientInstance).thenReturn(messageBrokerClient);
+      MockedStatic<MessageBrokerFactory> mockedMessageBrokerFactory = Mockito.mockStatic(MessageBrokerFactory.class, Mockito.CALLS_REAL_METHODS);
+      mockedMessageBrokerFactory.when(MessageBrokerFactory::getMessageBrokerClientInstance).thenReturn(messageBrokerClient);
       mockedMessageBrokerClient = messageBrokerClient;
     }
   }

store/src/test/java/com/zimbra/cs/service/admin/AdminServiceWithFakeBrokerClient.java

@@ -1,12 +1,13 @@
 package com.zimbra.cs.service.admin;
 
 import com.zextras.carbonio.message_broker.MessageBrokerClient;
+import io.vavr.control.Try;
 import org.mockito.Mockito;
 
 public class AdminServiceWithFakeBrokerClient extends AdminService {
 
 	@Override
-	protected MessageBrokerClient getMessageBroker() {
-		return Mockito.mock(MessageBrokerClient.class);
+	protected Try<MessageBrokerClient> getMessageBroker() {
+		return Try.of(() -> Mockito.mock(MessageBrokerClient.class));
 	}
 }

store/src/test/java/com/zimbra/cs/service/admin/DeleteAccountTest.java

@@ -8,6 +8,7 @@
 import com.zextras.carbonio.message_broker.events.services.mailbox.UserDeleted;
 import com.zextras.mailbox.account.usecase.DeleteUserUseCase;
 import com.zextras.mailbox.acl.AclService;
+import com.zextras.mailbox.messageBroker.MessageBrokerFactory;
 import com.zextras.mailbox.util.MailboxTestUtil;
 import com.zextras.mailbox.util.MailboxTestUtil.AccountCreator;
 import com.zimbra.common.account.ZAttrProvisioning;
@@ -65,15 +66,15 @@ static void setUp() throws Exception {
     final MailboxManager mailboxManager = MailboxManager.getInstance();
     provisioning = Provisioning.getInstance();
     accountCreatorFactory = new AccountCreator.Factory(provisioning);
-    mockMessageBrokerClient = AdminService.getMessageBrokerClientInstance();
+    mockMessageBrokerClient = MessageBrokerFactory.getMessageBrokerClientInstance();
     deleteAccount =
         new DeleteAccount(
             new DeleteUserUseCase(
                 provisioning,
                 mailboxManager,
                 new AclService(mailboxManager, provisioning),
                 ZimbraLog.security),
-            mockMessageBrokerClient);
+            Try.of(() -> mockMessageBrokerClient));
     provisioning.createDomain(OTHER_DOMAIN, new HashMap<>());
   }
 
@@ -319,7 +320,7 @@ void shouldDeleteUserThrowsException(Account caller, Account toDelete) throws Ex
         DeleteAccount deleteAccountHandler =
                 new DeleteAccount(
                         deleteUserUseCase,
-                        mockMessageBrokerClient);
+                        Try.of(() -> mockMessageBrokerClient));
         Mockito.when(deleteUserUseCase.delete(toDeleteId)).thenReturn(Try.failure(new RuntimeException("message")));
         DeleteAccountRequest deleteAccountRequest = new DeleteAccountRequest(toDeleteId);
         Element request = JaxbUtil.jaxbToElement(deleteAccountRequest);