From e4ba631b6a5eead8c1ed3cf731e42089625da14f Mon Sep 17 00:00:00 2001 From: Stefan Rossbach Date: Wed, 31 Jul 2019 15:21:10 +0200 Subject: [PATCH] [INTERNAL][UI] Use new XMPPStore API 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. --- .../ui/actions/ChangeXMPPAccountAction.java | 28 ++++++++++--- .../ConnectingFailureHandler.java | 8 ++-- .../GeneralPreferencePage.java | 22 ++++++---- .../saros/ui/util/XMPPConnectionSupport.java | 42 ++++++++++++------- .../saros/ui/wizards/ConfigurationWizard.java | 25 ++++++----- .../ui/actions/ConnectServerAction.java | 14 +++++-- server/src/saros/server/ServerLifecycle.java | 12 +++--- .../test/account/AccountPreferenceTest.java | 4 +- .../impl/AccountManipulatorImpl.java | 39 +++-------------- .../menu/submenu/impl/SarosPreferences.java | 14 +++---- .../browser_functions/SetActiveAccount.java | 2 +- .../ui/core_facades/ConnectionFacade.java | 2 +- 12 files changed, 116 insertions(+), 96 deletions(-) diff --git a/eclipse/src/saros/ui/actions/ChangeXMPPAccountAction.java b/eclipse/src/saros/ui/actions/ChangeXMPPAccountAction.java index 8e937d11e5..e30320446d 100644 --- 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 index 007e321285..68bb6d945a 100644 --- 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 index 014ac66447..19aafbe677 100644 --- 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 index 337064dc96..40e1a7d81a 100644 --- 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. + * + *

Note: If no default account is available this method silently returns regardless if + * + * failSilently is set to false ! * * @param failSilently if true suppresses any further error handling */ @@ -51,8 +55,13 @@ public void connect(boolean failSilently) { } /** - * Connects with given account. If the given account is null the active / default one will be used. - * @param account the account to use + * Connects with given account. + * + *

Note: If the default account should be used an no default account is available this + * method silently returns regardless if + * failSilently is set to false ! + * + * @param account the account to use or null to use the default one * @param failSilently if true suppresses any further error handling */ public void connect(final XMPPAccount account, boolean failSilently) { @@ -60,8 +69,13 @@ public void connect(final XMPPAccount account, boolean failSilently) { } /** - * Connects with given account. If the given account is null the active / default one will be used. - * @param account the account to use + * Connects with given account. + * + *

Note: If the default account should be used an no default account is available this + * method silently returns regardless if + * failSilently is set to false ! + * + * @param account account the account to use or null to use the default one * @param setAsDefault if true the account is set as the default one * @param failSilently if true 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 index a74dbe1c88..481288b7e1 100644 --- 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 index 2092a84d57..5dd4214243 100644 --- 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 index c69635ca6d..f9856af27d 100644 --- 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 index c28d79e44e..6f62b39cb3 100644 --- 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 index 1c8e541e31..d5de103025 100644 --- 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 index 6e2309835a..fc6974cc37 100644 --- 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 @@ -342,14 +342,12 @@ private void clickMenuSarosPreferences() throws RemoteException { } private boolean isAccountActiveNoGUI(JID jid) { - XMPPAccount account = null; - try { - account = getXmppAccountStore().getActiveAccount(); - return account.getUsername().equals(jid.getName()) - && account.getDomain().equals(jid.getDomain()); - } catch (IllegalStateException e) { - return false; - } + final XMPPAccount account = getXmppAccountStore().getDefaultAccount(); + + if (account == null) return false; + + return account.getUsername().equals(jid.getName()) + && account.getDomain().equals(jid.getDomain()); } private boolean isAccountExistNoGUI(JID jid) { diff --git a/ui/src/saros/ui/browser_functions/SetActiveAccount.java b/ui/src/saros/ui/browser_functions/SetActiveAccount.java index afc0857418..b25a714903 100644 --- a/ui/src/saros/ui/browser_functions/SetActiveAccount.java +++ b/ui/src/saros/ui/browser_functions/SetActiveAccount.java @@ -38,7 +38,7 @@ public SetActiveAccount(XMPPAccountStore accountStore) { @BrowserFunction public void setActiveAccount(XMPPAccount account) { try { - accountStore.setAccountActive(account); + accountStore.setDefaultAccount(account); } catch (IllegalArgumentException e) { LOG.error("Couldn't activate account " + account.toString() + ". Error:" + e.getMessage(), e); JavaScriptAPI.showError(browser, HTMLUIStrings.ERR_ACCOUNT_SET_ACTIVE_FAILED); diff --git a/ui/src/saros/ui/core_facades/ConnectionFacade.java b/ui/src/saros/ui/core_facades/ConnectionFacade.java index 8cd7164677..d85ca35cf8 100644 --- a/ui/src/saros/ui/core_facades/ConnectionFacade.java +++ b/ui/src/saros/ui/core_facades/ConnectionFacade.java @@ -31,7 +31,7 @@ public ConnectionFacade(ConnectionHandler connectionHandler, XMPPAccountStore ac * @param account representing an XMPP account */ public void connect(XMPPAccount account) { - accountStore.setAccountActive(account); + accountStore.setDefaultAccount(account); connectionHandler.connect(account, false); }