[INTERNAL][UI] Use new XMPPStore API

srossbach · srossbach · commit e4ba631b6a5e · 2019-08-05T20:55:14.000+02:00
This patch introduces the usage of the new XMPPStore API.

Renaming UI buttons is not done yet as changing UI values is likely to
crash the whole STF.
diff --git a/eclipse/src/saros/ui/actions/ChangeXMPPAccountAction.java b/eclipse/src/saros/ui/actions/ChangeXMPPAccountAction.java
@@ -4,6 +4,7 @@
 import org.eclipse.jface.action.Action;
 import org.eclipse.jface.action.ActionContributionItem;
 import org.eclipse.jface.action.IMenuCreator;
+import org.eclipse.jface.dialogs.MessageDialog;
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.widgets.Control;
 import org.eclipse.swt.widgets.Menu;
@@ -74,12 +75,23 @@ public ChangeXMPPAccountAction() {
   @Override
   public void run() {
 
+    final XMPPAccount defaultAccount = accountService.getDefaultAccount();
+    final boolean isEmpty = accountService.isEmpty();
+
+    if (defaultAccount == null || isEmpty) {
+      if (!MessageDialog.openQuestion(
+          SWTUtils.getShell(),
+          "Default account missing",
+          "A default account has not been set yet. Do you want set a default account?")) return;
+
+      SWTUtils.runSafeSWTAsync(LOG, this::openPreferences);
+      return;
+    }
+
     if (connectionHandler.isConnected()) {
       XMPPConnectionSupport.getInstance().disconnect();
     } else {
-      XMPPConnectionSupport.getInstance()
-          .connect(
-              accountService.isEmpty() ? null : accountService.getActiveAccount(), true, false);
+      XMPPConnectionSupport.getInstance().connect(accountService.getDefaultAccount(), true, false);
     }
   }
 
@@ -97,12 +109,16 @@ public Menu getMenu(Menu parent) {
   public Menu getMenu(Control parent) {
     accountMenu = new Menu(parent);
 
-    XMPPAccount activeAccount = null;
+    XMPPAccount defaultAccount = null;
 
-    if (connectionHandler.isConnected()) activeAccount = accountService.getActiveAccount();
+    /* FIXME obtain the current JID and the discard the entry.
+     * This logic here only works because we set the account that should connect to be the default one.
+     * If the user is interested in such a behavior is another question.
+     */
+    if (connectionHandler.isConnected()) defaultAccount = accountService.getDefaultAccount();
 
     for (XMPPAccount account : accountService.getAllAccounts()) {
-      if (!account.equals(activeAccount)) addMenuItem(account);
+      if (!account.equals(defaultAccount)) addMenuItem(account);
     }
 
     new MenuItem(accountMenu, SWT.SEPARATOR);
diff --git a/eclipse/src/saros/ui/eventhandler/ConnectingFailureHandler.java b/eclipse/src/saros/ui/eventhandler/ConnectingFailureHandler.java
@@ -64,15 +64,17 @@ private void handleConnectionFailed(Exception exception) {
         return;
       }
 
-      /* FIXME the active/default account is not always the account that is currently used for connecting */
       if (DialogUtils.popUpYesNoQuestion(
           "Connecting Error",
           generateHumanReadableErrorMessage((XMPPException) exception),
           false)) {
 
-        if (WizardUtils.openEditXMPPAccountWizard(accountStore.getActiveAccount()) == null) return;
+        /* FIXME the active/default account might not always be the account that is currently used for connecting */
+        final XMPPAccount accountUsedDuringConnection = accountStore.getDefaultAccount();
 
-        final XMPPAccount account = accountStore.getActiveAccount();
+        if (WizardUtils.openEditXMPPAccountWizard(accountUsedDuringConnection) == null) return;
+
+        final XMPPAccount account = accountStore.getDefaultAccount();
 
         SWTUtils.runSafeSWTAsync(
             LOG, () -> XMPPConnectionSupport.getInstance().connect(account, false));
diff --git a/eclipse/src/saros/ui/preferencePages/GeneralPreferencePage.java b/eclipse/src/saros/ui/preferencePages/GeneralPreferencePage.java
@@ -144,13 +144,16 @@ public void widgetDefaultSelected(SelectionEvent e) {
 
           private void handleEvent() {
 
+            XMPPAccount selectedAccount = getSelectedAccount();
+
+            if (selectedAccount == null) return;
+
             activateAccountButton.setEnabled(true);
             removeAccountButton.setEnabled(true);
             editAccountButton.setEnabled(true);
 
-            if (getSelectedAccount().equals(accountStore.getActiveAccount())) {
+            if (selectedAccount.equals(accountStore.getDefaultAccount())) {
               activateAccountButton.setEnabled(false);
-              removeAccountButton.setEnabled(false);
             }
           }
         });
@@ -206,11 +209,11 @@ private void updateList() {
   }
 
   private void updateInfoLabel() {
-    if (!accountStore.isEmpty())
-      infoLabel.setText(
-          Messages.GeneralPreferencePage_active
-              + createHumanDisplayAbleName(accountStore.getActiveAccount()));
-    else infoLabel.setText("");
+    final XMPPAccount defaultAccount = accountStore.getDefaultAccount();
+
+    if (defaultAccount != null)
+      infoLabel.setText("Default account: " + createHumanDisplayAbleName(defaultAccount));
+    else infoLabel.setText("Default account: none");
   }
 
   private String createHumanDisplayAbleName(XMPPAccount account) {
@@ -276,6 +279,8 @@ public void handleEvent(Event event) {
                   activateAccountButton.setEnabled(false);
                   removeAccountButton.setEnabled(false);
                   editAccountButton.setEnabled(false);
+
+                  updateInfoLabel();
                   updateList();
                 }
               }
@@ -292,10 +297,9 @@ private void createActivateAccountButton(Composite composite) {
             new Listener() {
               @Override
               public void handleEvent(Event event) {
-                accountStore.setAccountActive(getSelectedAccount());
+                accountStore.setDefaultAccount(getSelectedAccount());
                 updateInfoLabel();
                 activateAccountButton.setEnabled(false);
-                removeAccountButton.setEnabled(false);
                 MessageDialog.openInformation(
                     GeneralPreferencePage.this.getShell(),
                     Messages.GeneralPreferencePage_ACTIVATE_ACCOUNT_DIALOG_TITLE,
diff --git a/eclipse/src/saros/ui/util/XMPPConnectionSupport.java b/eclipse/src/saros/ui/util/XMPPConnectionSupport.java
@@ -42,7 +42,11 @@ public XMPPConnectionSupport(
   }
 
   /**
-   * Connects with the current active / default account.
+   * Connects with the default account.
+   *
+   * <p><b>Note:</b> If no default account is available this method silently returns regardless if
+   * <code>
+   * failSilently</code> is set to <code>false</code> !
    *
    * @param failSilently if <code>true</code> suppresses any further error handling
    */
@@ -51,17 +55,27 @@ public void connect(boolean failSilently) {
   }
 
   /**
-   * Connects with given account. If the given account is <code>null<code> the active / default one will be used.
-   * @param account the account to use
+   * Connects with given account.
+   *
+   * <p><b>Note:</b> If the default account should be used an no default account is available this
+   * method silently returns regardless if <code>
+   * failSilently</code> is set to <code>false</code> !
+   *
+   * @param account the account to use or <code>null</code> to use the default one
    * @param failSilently if <code>true</code> suppresses any further error handling
    */
   public void connect(final XMPPAccount account, boolean failSilently) {
     connect(account, false, failSilently);
   }
 
   /**
-   * Connects with given account. If the given account is <code>null<code> the active / default one will be used.
-   * @param account the account to use
+   * Connects with given account.
+   *
+   * <p><b>Note:</b> If the default account should be used an no default account is available this
+   * method silently returns regardless if <code>
+   * failSilently</code> is set to <code>false</code> !
+   *
+   * @param account account the account to use or <code>null</code> to use the default one
    * @param setAsDefault if <code>true</code> the account is set as the default one
    * @param failSilently if <code>true</code> suppresses any further error handling
    */
@@ -129,18 +143,18 @@ public void connect(final XMPPAccount account, boolean setAsDefault, boolean fai
 
     final XMPPAccount accountToConnect;
 
-    if (account == null && !store.isEmpty()) accountToConnect = store.getActiveAccount();
-    else if (account != null) accountToConnect = account;
-    else accountToConnect = null;
+    accountToConnect = account != null ? account : store.getDefaultAccount();
 
-    /*
-     * some magic, if we connect with null we will trigger an exception that is processed by
-     * the ConnectingFailureHandler which in turn will open the ConfigurationWizard
-     */
-    if (setAsDefault && accountToConnect != null) {
-      store.setAccountActive(accountToConnect);
+    if (accountToConnect == null) {
+      log.warn(
+          "unable to establish a connection - no account was provided and no default account could be found");
+
+      isConnecting = false;
+      return;
     }
 
+    if (setAsDefault) store.setDefaultAccount(accountToConnect);
+
     final boolean disconnectFirst = mustDisconnect;
 
     ThreadUtils.runSafeAsync(
diff --git a/eclipse/src/saros/ui/wizards/ConfigurationWizard.java b/eclipse/src/saros/ui/wizards/ConfigurationWizard.java
@@ -72,19 +72,18 @@ public void addPages() {
   public boolean performFinish() {
     setConfiguration();
 
-    final XMPPAccount accountToConnect;
+    final XMPPAccount accountToConnect = addOrGetXMPPAccount();
 
-    if (!enterXMPPAccountWizardPage.isExistingAccount()) {
-      accountToConnect = addXMPPAccount();
-    } else accountToConnect = null;
+    assert (accountToConnect != null);
 
-    assert accountStore.getActiveAccount() != null;
+    /* it is possible to finish the wizard multiple times
+     * (also it makes no sense) so ensure the behavior is always the same.
+     */
+
+    accountStore.setDefaultAccount(accountToConnect);
 
     if (preferences.getBoolean(PreferenceConstants.AUTO_CONNECT)) {
-      getShell()
-          .getDisplay()
-          .asyncExec(
-              () -> XMPPConnectionSupport.getInstance().connect(accountToConnect, true, false));
+      getShell().getDisplay().asyncExec(() -> XMPPConnectionSupport.getInstance().connect(false));
     }
 
     return true;
@@ -144,8 +143,9 @@ protected void setConfiguration() {
   }
 
   /** Adds the {@link EnterXMPPAccountWizardPage}'s account data to the {@link XMPPAccountStore}. */
-  private XMPPAccount addXMPPAccount() {
+  private XMPPAccount addOrGetXMPPAccount() {
 
+    boolean isExistingAccount = enterXMPPAccountWizardPage.isExistingAccount();
     JID jid = enterXMPPAccountWizardPage.getJID();
 
     String username = jid.getName();
@@ -162,6 +162,9 @@ private XMPPAccount addXMPPAccount() {
     boolean useTLS = enterXMPPAccountWizardPage.isUsingTLS();
     boolean useSASL = enterXMPPAccountWizardPage.isUsingSASL();
 
-    return accountStore.createAccount(username, password, domain, server, port, useTLS, useSASL);
+    if (isExistingAccount)
+      return accountStore.createAccount(username, password, domain, server, port, useTLS, useSASL);
+
+    return accountStore.getAccount(username, domain, server, port);
   }
 }
diff --git a/intellij/src/saros/intellij/ui/actions/ConnectServerAction.java b/intellij/src/saros/intellij/ui/actions/ConnectServerAction.java
@@ -9,6 +9,7 @@
 import saros.account.XMPPAccount;
 import saros.account.XMPPAccountStore;
 import saros.communication.connection.ConnectionHandler;
+import saros.net.xmpp.JID;
 import saros.repackaged.picocontainer.annotations.Inject;
 
 /** Connects to XMPP/Jabber server with given account or active account */
@@ -34,15 +35,22 @@ public String getActionName() {
 
   /** Connects with the given user. */
   public void executeWithUser(String user) {
-    XMPPAccount account = accountStore.findAccount(user);
-    accountStore.setAccountActive(account);
+    JID jid = new JID(user);
+    XMPPAccount account = accountStore.getAccount(jid.getName(), jid.getDomain());
+
+    if (account == null) return;
+
+    accountStore.setDefaultAccount(account);
     connectAccount(account);
   }
 
   /** Connects with active account from the {@link XMPPAccountStore}. */
   @Override
   public void execute() {
-    XMPPAccount account = accountStore.getActiveAccount();
+    XMPPAccount account = accountStore.getDefaultAccount();
+
+    if (account == null) return;
+
     connectAccount(account);
   }
 
diff --git a/server/src/saros/server/ServerLifecycle.java b/server/src/saros/server/ServerLifecycle.java
@@ -65,11 +65,13 @@ private void connectToXMPPServer(final ContainerContext context) {
      * instead
      */
     XMPPAccountStore store = context.getComponent(XMPPAccountStore.class);
-    XMPPAccount account = store.findAccount(jidString);
-    if (account == null) {
-      JID jid = new JID(jidString);
-      account = store.createAccount(jid.getName(), password, jid.getDomain(), "", 0, true, true);
-    }
+
+    // ensure we do not save anything and that the store is empty
+    store.setAccountFile(null, null);
+
+    JID jid = new JID(jidString);
+    XMPPAccount account =
+        store.createAccount(jid.getName(), password, jid.getDomain(), "", 0, true, true);
 
     ConnectionHandler connectionHandler = context.getComponent(ConnectionHandler.class);
     connectionHandler.connect(account, false);
diff --git a/stf.test/test/saros/stf/test/account/AccountPreferenceTest.java b/stf.test/test/saros/stf/test/account/AccountPreferenceTest.java
@@ -125,8 +125,8 @@ public void testRemoveAccountButton() throws Exception {
 
     shell.bot().listInGroup(GROUP_TITLE_XMPP_JABBER_ACCOUNTS).select(ALICE.getBaseJid());
 
-    assertFalse(
-        "remove account button must no be enabled when the active account is already selected",
+    assertTrue(
+        "remove account button must be enabled when the active account is already selected",
         shell
             .bot()
             .buttonInGroup(BUTTON_REMOVE_ACCOUNT, GROUP_TITLE_XMPP_JABBER_ACCOUNTS)
diff --git a/stf/src/saros/stf/server/rmi/controlbot/manipulation/impl/AccountManipulatorImpl.java b/stf/src/saros/stf/server/rmi/controlbot/manipulation/impl/AccountManipulatorImpl.java
@@ -29,7 +29,7 @@ public void restoreDefaultAccount(String username, String password, String domai
 
     for (XMPPAccount account : accountStore.getAllAccounts()) {
 
-      if (account.equals(accountStore.getActiveAccount())) continue;
+      if (account.equals(accountStore.getDefaultAccount())) continue;
 
       LOG.debug("deleting account: " + account);
 
@@ -40,19 +40,6 @@ public void restoreDefaultAccount(String username, String password, String domai
       accountStore.createAccount(username, password, domain, "", 0, true, true);
       return;
     }
-
-    XMPPAccount activeAccount = accountStore.getActiveAccount();
-
-    if (accountStore.existsAccount(username, domain, "", 0)) return;
-
-    XMPPAccount defaultAccount =
-        accountStore.createAccount(username, password, domain, "", 0, true, true);
-
-    LOG.debug("activating account: " + defaultAccount);
-    accountStore.setAccountActive(defaultAccount);
-
-    LOG.debug("deleting account: " + activeAccount);
-    accountStore.deleteAccount(activeAccount);
   }
 
   @Override
@@ -75,27 +62,13 @@ public boolean activateAccount(String username, String domain) throws RemoteExce
 
     final XMPPAccountStore accountStore = getXmppAccountStore();
 
-    XMPPAccount activeAccount = null;
+    final XMPPAccount accountToSetAsDefault = accountStore.getAccount(username, domain);
 
-    try {
-      activeAccount = accountStore.getActiveAccount();
-    } catch (IllegalStateException e) {
-      // ignore
-    }
-
-    for (XMPPAccount account : accountStore.getAllAccounts()) {
-      if (account.getUsername().equals(username) && account.getDomain().equals(domain)) {
+    if (accountToSetAsDefault == null)
+      throw new IllegalArgumentException(
+          "an account with username '" + username + "' and domain '" + domain + "' does not exist");
 
-        if (!account.equals(activeAccount)) {
-          LOG.debug("activating account: " + account);
-          accountStore.setAccountActive(account);
-        } else {
-          LOG.debug("account is already activated: " + account);
-        }
-
-        return !account.equals(activeAccount);
-      }
-    }
+    accountStore.setDefaultAccount(accountToSetAsDefault);
 
     throw new IllegalArgumentException(
         "an account with username '" + username + "' and domain '" + domain + "' does not exist");
diff --git a/stf/src/saros/stf/server/rmi/superbot/component/menubar/menu/submenu/impl/SarosPreferences.java b/stf/src/saros/stf/server/rmi/superbot/component/menubar/menu/submenu/impl/SarosPreferences.java
diff --git a/ui/src/saros/ui/browser_functions/SetActiveAccount.java b/ui/src/saros/ui/browser_functions/SetActiveAccount.java
diff --git a/ui/src/saros/ui/core_facades/ConnectionFacade.java b/ui/src/saros/ui/core_facades/ConnectionFacade.java
