diff --git a/core/src/saros/account/XMPPAccount.java b/core/src/saros/account/XMPPAccount.java index ededd8f90e..a3f44f0aab 100644 --- a/core/src/saros/account/XMPPAccount.java +++ b/core/src/saros/account/XMPPAccount.java @@ -51,6 +51,9 @@ public final class XMPPAccount implements Serializable { if (!domain.toLowerCase().equals(domain)) throw new IllegalArgumentException("domain url must be in lower case letters"); + // FIXME see https://tools.ietf.org/html/rfc6122#section-2 - the localpart/username is also + // case-insensitive + if (port < 0 || port >= 65536) throw new IllegalArgumentException("port number is not valid"); if ((server.trim().length() != 0 && port == 0) || (server.trim().length() == 0 && port != 0)) @@ -136,8 +139,7 @@ public String toString() { + ", TLS: " + useTLS + ", SASL: " - + useSASL - + " : "; + + useSASL; } @Override diff --git a/core/src/saros/account/XMPPAccountStore.java b/core/src/saros/account/XMPPAccountStore.java index f01a31850e..e8118b156f 100644 --- a/core/src/saros/account/XMPPAccountStore.java +++ b/core/src/saros/account/XMPPAccountStore.java @@ -13,12 +13,11 @@ import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.util.ArrayList; -import java.util.Collections; -import java.util.Comparator; import java.util.HashSet; import java.util.List; import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.stream.Collectors; import java.util.zip.DataFormatException; import java.util.zip.Deflater; import java.util.zip.Inflater; @@ -31,7 +30,6 @@ import org.apache.commons.io.IOUtils; import org.apache.log4j.Logger; import saros.annotations.Component; -import saros.net.xmpp.JID; /** * The XMPPAccountStore is responsible for administering XMPP account credentials. All data will @@ -57,7 +55,7 @@ public final class XMPPAccountStore { new CopyOnWriteArrayList(); private Set accounts; - private XMPPAccount activeAccount; + private XMPPAccount defaultAccount; private File accountFile; private String secretKey; @@ -94,9 +92,8 @@ private void notifyAccountStoreListeners() { } private void notifyActiveAccountListeners() { - XMPPAccount active = !isEmpty() ? getActiveAccount() : null; for (IAccountStoreListener listener : listeners) { - listener.activeAccountChanged(active); + listener.activeAccountChanged(defaultAccount); } } @@ -121,6 +118,7 @@ public void setAccountFile(final File file, final String key) { if (secretKey == null) secretKey = DEFAULT_SECRET_KEY; accounts = new HashSet(); + defaultAccount = null; if (accountFile != null) { File parent = accountFile.getParentFile(); @@ -138,7 +136,6 @@ public void setAccountFile(final File file, final String key) { notifyActiveAccountListeners(); } - @SuppressWarnings("unchecked") private synchronized void loadAccounts() { if (accountFile == null || !accountFile.exists()) return; @@ -166,7 +163,10 @@ private synchronized void loadAccounts() { new ByteArrayInputStream(Crypto.decrypt(encryptedAccountData, secretKey))); accounts = new HashSet<>(accountData.configuredAccounts); - activeAccount = accountData.configuredAccounts.get(accountData.activeAccountIndex); + defaultAccount = null; + + if (accountData.activeAccountIndex != -1) + defaultAccount = accountData.configuredAccounts.get(accountData.activeAccountIndex); } catch (RuntimeException e) { LOG.error("internal error while loading account data", e); @@ -178,13 +178,6 @@ private synchronized void loadAccounts() { IOUtils.closeQuietly(dataIn); } - /* - * remove us first and re add us, otherwise the active account object is - * not in the set and the wrong object will be updated - */ - accounts.remove(activeAccount); - accounts.add(activeAccount); - LOG.debug("loaded " + accounts.size() + " account(s)"); } @@ -211,8 +204,11 @@ private synchronized void saveAccounts() { dataOut = new FileOutputStream(accountFile); // use a pair in order to create a artificial xml root node ArrayList accountsToSave = new ArrayList<>(accounts); - int activeAccountIndex = accountsToSave.indexOf(activeAccount); - xStream.toXML(new AccountStoreInformation(activeAccountIndex, accountsToSave), out); + + int defaultAccountIndex = + defaultAccount == null ? -1 : accountsToSave.indexOf(defaultAccount); + + xStream.toXML(new AccountStoreInformation(defaultAccountIndex, accountsToSave), out); byte[] encryptedAccountData = Crypto.encrypt(out.toByteArray(), secretKey); @@ -245,31 +241,10 @@ private XStream createXStream() { * @return */ public synchronized List getAllAccounts() { - List accounts = new ArrayList(this.accounts); - - Comparator comparator = - new Comparator() { - - @Override - public int compare(XMPPAccount a, XMPPAccount b) { - int c = a.getUsername().compareToIgnoreCase(b.getUsername()); - - if (c != 0) return c; - - c = a.getDomain().compareToIgnoreCase(b.getDomain()); - - if (c != 0) return c; - - c = a.getServer().compareToIgnoreCase(b.getServer()); - - if (c != 0) return c; - - return Integer.valueOf(a.getPort()).compareTo(Integer.valueOf(b.getPort())); - } - }; - - Collections.sort(accounts, comparator); - return accounts; + return accounts + .stream() + .sorted(XMPPAccountStore::compare) + .collect(Collectors.toCollection(ArrayList::new)); } /** @@ -294,13 +269,12 @@ public int compare(XMPPAccount a, XMPPAccount b) { * * @return */ - public synchronized List getDomains() { - List domains = new ArrayList(); - for (XMPPAccount account : accounts) { - String domain = account.getDomain(); - if (!domains.contains(domain)) domains.add(domain); - } - return domains; + public List getDomains() { + final Set domains = new HashSet(); + + for (final XMPPAccount account : getAllAccounts()) domains.add(account.getDomain()); + + return new ArrayList(domains); } /** @@ -318,35 +292,33 @@ public synchronized List getDomains() { * the server list contains * *
    - *
  • jabber.org *
  • googlemail.com - *
  • saros-con.imp.fu-berlin.de *
* * @return */ public synchronized List getServers() { - List servers = new ArrayList(); - for (XMPPAccount account : accounts) { - String server = account.getServer(); - if (!servers.contains(server)) servers.add(server); - } - return servers; + + final Set servers = new HashSet(); + + for (final XMPPAccount account : getAllAccounts()) servers.add(account.getServer()); + + return new ArrayList(servers); } /** - * Makes the given account active. + * Sets the given account as the default one. * - * @param account the account to activate + * @param account the account to set as default or null * @throws IllegalArgumentException if the account is not found in the store */ - public void setAccountActive(XMPPAccount account) { + public void setDefaultAccount(final XMPPAccount account) { synchronized (this) { - if (!accounts.contains(account)) + if (account != null && !accounts.contains(account)) throw new IllegalArgumentException( "account '" + account + "' is not in the current account store"); - activeAccount = account; + defaultAccount = account; saveAccounts(); } @@ -355,32 +327,30 @@ public void setAccountActive(XMPPAccount account) { } /** - * Deletes an account. + * Deletes an account from the store. If this was the default account the default account is set + * to null. * * @param account the account to delete * @throws IllegalArgumentException if the account is not found in the store - * @throws IllegalStateException if the account is active */ public void deleteAccount(XMPPAccount account) { synchronized (this) { - if (!accounts.contains(account)) + if (!accounts.remove(account)) throw new IllegalArgumentException( "account '" + account + "' is not in the current account store"); - if (this.activeAccount == account) - throw new IllegalStateException( - "account '" + account + "' is active and cannot be deleted"); - - accounts.remove(account); + if (defaultAccount == account) defaultAccount = null; saveAccounts(); } notifyAccountStoreListeners(); + if (defaultAccount == null) notifyActiveAccountListeners(); } /** - * Creates an account. The account will automatically become active if the account store is empty. + * Creates an account. The account will automatically become the default if the account store is + * empty. If the account already exists the existing account is returned. * * @param username the user name of the new account as lower case string * @param password the password of the new account. @@ -406,16 +376,16 @@ public XMPPAccount createAccount( boolean useTLS, boolean useSASL) { - XMPPAccount newAccount = + final XMPPAccount newAccount = new XMPPAccount(username, password, domain, server, port, useTLS, useSASL); synchronized (this) { if (accounts.contains(newAccount)) throw new IllegalArgumentException("account already exists"); - if (accounts.isEmpty()) this.activeAccount = newAccount; + if (accounts.isEmpty()) defaultAccount = newAccount; - this.accounts.add(newAccount); + accounts.add(newAccount); saveAccounts(); } @@ -490,15 +460,12 @@ public void changeAccountData( } /** - * Returns the current active account. + * Returns the current default account. * - * @return the active account - * @throws IllegalStateException if the account store is empty + * @return the default account or null if the default account is not set */ - public synchronized XMPPAccount getActiveAccount() { - if (activeAccount == null) throw new IllegalStateException("the account store is empty"); - - return activeAccount; + public synchronized XMPPAccount getDefaultAccount() { + return defaultAccount; } /** @@ -512,54 +479,60 @@ public synchronized boolean isEmpty() { /** * Checks if the an account with the given arguments exists in the account - * store + * store. * * @param username * the username * @param domain * the domain name of the server * @param server - * the server ip / name + * the server address / name * @param port * the port of the server * @return true if such an account exists, false * otherwise */ - public synchronized boolean exists(String username, String domain, String server, int port) { - for (XMPPAccount a : getAllAccounts()) { - if (a.getServer().equalsIgnoreCase(server) - && a.getDomain().equalsIgnoreCase(domain) - && a.getUsername().equals(username) - && a.getPort() == port) { - return true; - } - } - return false; + public synchronized boolean existsAccount( + String username, String domain, String server, int port) { + return getAllAccounts() + .stream() + .anyMatch(a -> matchesAccount(a, username, domain, server, port)); } /** - * Searches for an account in the account store. + * Returns the account for the given username and domain. * - * @param jidString the jid of the user as string - * @return the matching XMPP account or null in case of no match - * @throws NullPointerException if jidString is null + *

Note: If the store contains multiple accounts for the given username and domain (e.g + * with different server address / name entries) the first account that matches will be returned. + * + * @param username the username to lookup + * @param domain the domain to lookup + * @return the account or null if the account does not exist */ - public XMPPAccount findAccount(String jidString) { - if (jidString == null) { - throw new NullPointerException("Null argument 'jidString'"); - } - JID jid = new JID(jidString); - String username = jid.getName(); - String domain = jid.getDomain(); - - for (XMPPAccount account : getAllAccounts()) { - if (domain.equalsIgnoreCase(account.getDomain()) - && username.equalsIgnoreCase(account.getUsername())) { - return account; - } - } + public XMPPAccount getAccount(final String username, final String domain) { + return getAllAccounts() + .stream() + .findFirst() + .filter(a -> matchesAnyServer(a, username, domain)) + .orElse(null); + } - return null; + /** + * Returns the account for the given username, domain, server address / name, and port. + * + * @param username the username to lookup + * @param domain the domain to lookup + * @param server the server/address to lookup + * @param port the port to lookup + * @return the account or null if the account does not exist + */ + public XMPPAccount getAccount( + final String username, final String domain, final String server, final int port) { + return getAllAccounts() + .stream() + .findFirst() + .filter(a -> matchesAccount(a, username, domain, server, port)) + .orElse(null); } /** @@ -695,4 +668,40 @@ private static byte[] xor(byte data[]) { return data; } } + + private static int compare(final XMPPAccount a, XMPPAccount b) { + int c = a.getUsername().compareToIgnoreCase(b.getUsername()); + + if (c != 0) return c; + + c = a.getDomain().compareToIgnoreCase(b.getDomain()); + + if (c != 0) return c; + + c = a.getServer().compareToIgnoreCase(b.getServer()); + + if (c != 0) return c; + + return Integer.valueOf(a.getPort()).compareTo(Integer.valueOf(b.getPort())); + } + + private static boolean matchesAnyServer( + final XMPPAccount account, final String username, final String domain) { + + return account.getUsername().equalsIgnoreCase(username) + && account.getDomain().equalsIgnoreCase(domain); + } + + private static boolean matchesAccount( + final XMPPAccount account, + final String username, + final String domain, + final String server, + final int port) { + + return account.getUsername().equalsIgnoreCase(username) + && account.getDomain().equalsIgnoreCase(domain) + && account.getServer().equalsIgnoreCase(server) + && account.getPort() == port; + } } diff --git a/core/test/junit/saros/account/XMPPAccountStoreTest.java b/core/test/junit/saros/account/XMPPAccountStoreTest.java index f9ed85401c..441e249c03 100644 --- a/core/test/junit/saros/account/XMPPAccountStoreTest.java +++ b/core/test/junit/saros/account/XMPPAccountStoreTest.java @@ -2,6 +2,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -32,11 +33,6 @@ public void testWithNoAccountFile() { assertTrue(store.isEmpty()); } - @Test(expected = IllegalStateException.class) - public void testActiveAccountWithEmptyAccountStore() { - store.getActiveAccount(); - } - /* * Test whether the currently expected XML format is accepted by the load * method. Otherwise saros could became incompatible with previous versions. @@ -69,61 +65,24 @@ public void testAccountsFileFormat() throws Exception { assertEquals(configuredAccounts.size(), store.getAllAccounts().size()); - assertTrue(store.exists("activeAccount", "activedomain", "activeserver", 3)); - assertTrue(store.exists("anotherAccount1", "anotherdomain1", "anotherserver1", 1)); - assertTrue(store.exists("anotherAccount2", "anotherdomain2", "anotherserver2", 2)); - - assertEquals("activeAccount", store.getActiveAccount().getUsername()); - } - - private void writeAccountFile(File accountFile, String key, String content) throws Exception { - final FileOutputStream dataOut = new FileOutputStream(accountFile); - final byte[] encryptedXmlContent = XMPPAccountStore.Crypto.encrypt(content.getBytes(), key); + assertTrue(store.existsAccount("activeAccount", "activedomain", "activeserver", 3)); + assertTrue(store.existsAccount("anotherAccount1", "anotherdomain1", "anotherserver1", 1)); + assertTrue(store.existsAccount("anotherAccount2", "anotherdomain2", "anotherserver2", 2)); - try { - dataOut.write(encryptedXmlContent); - dataOut.flush(); - } finally { - IOUtils.closeQuietly(dataOut); - } - } - - private String createAccountFileContent( - XMPPAccount activeAccount, ArrayList configuredAccounts) { - - int index = configuredAccounts.indexOf(activeAccount); - StringBuilder xmlContent = - new StringBuilder() - .append("\n") - .append(String.format(" %d\n", index)); - - xmlContent.append(" \n"); - for (XMPPAccount acc : configuredAccounts) { - xmlContent - .append(" \n") - .append(String.format(" %s\n", acc.getUsername())) - .append(String.format(" %s\n", acc.getPassword())) - .append(String.format(" %s\n", acc.getDomain())) - .append(String.format(" %s\n", acc.getServer())) - .append(String.format(" %d\n", acc.getPort())) - .append(String.format(" %s\n", acc.useTLS())) - .append(String.format(" %s\n", acc.useSASL())) - .append(" \n"); - } - xmlContent.append(" \n").append(""); - - return xmlContent.toString(); + assertEquals("activeAccount", store.getDefaultAccount().getUsername()); } @Test - public void testAutoActivation() throws Exception { + public void testGetDefaultOnAccountCreationInEmptyStore() throws Exception { store.createAccount("a", "a", "a", "a", 1, true, true); assertFalse(store.isEmpty()); - assertEquals("a", store.getActiveAccount().getUsername()); + assertNotNull(store.getDefaultAccount()); + + assertEquals("a", store.getDefaultAccount().getUsername()); // only the first account must be auto activated store.createAccount("b", "b", "b", "b", 1, true, true); - assertEquals("a", store.getActiveAccount().getUsername()); + assertEquals("a", store.getDefaultAccount().getUsername()); } @Test @@ -158,10 +117,12 @@ public void testDeleteExistingAccount() { assertEquals(1, store.getAllAccounts().size()); } - @Test(expected = IllegalStateException.class) - public void testDeleteActiveAccount() { + @Test + public void testDeleteDefaultAccount() { store.createAccount("a", "a", "a", "a", 1, true, true); - store.deleteAccount(store.getActiveAccount()); + store.deleteAccount(store.getDefaultAccount()); + assertNull(store.getDefaultAccount()); + assertTrue(store.isEmpty()); } @Test(expected = IllegalArgumentException.class) @@ -171,19 +132,19 @@ public void testCreateDuplicateAccount() { } @Test - public void setAccountActive() { + public void setAccountAsDefaultAccount() { store.createAccount("a", "a", "a", "a", 1, true, true); XMPPAccount account = store.createAccount("b", "a", "a", "a", 1, true, true); - store.setAccountActive(account); - assertEquals(account, store.getActiveAccount()); + store.setDefaultAccount(account); + assertEquals(account, store.getDefaultAccount()); } @Test(expected = IllegalArgumentException.class) - public void setNonExistingAccountActive() { + public void setNonExistingAccountAsDefault() { store.createAccount("a", "a", "a", "a", 1, true, true); XMPPAccount account = store.createAccount("b", "a", "a", "a", 1, true, true); store.deleteAccount(account); - store.setAccountActive(account); + store.setDefaultAccount(account); } @Test(expected = IllegalArgumentException.class) @@ -196,9 +157,9 @@ public void testChangeAccountDataToAlreadyExistingAccount() { @Test public void testChangeAccountData() { store.createAccount("a", "a", "a", "a", 1, true, true); - store.changeAccountData(store.getActiveAccount(), "b", "b", "b", "b", 5, false, false); + store.changeAccountData(store.getDefaultAccount(), "b", "b", "b", "b", 5, false, false); - XMPPAccount account = store.getActiveAccount(); + XMPPAccount account = store.getDefaultAccount(); assertEquals("b", account.getPassword()); assertEquals("b", account.getServer()); @@ -262,18 +223,17 @@ public void testChangeAccountDataAndThenDeleteAccount() { } @Test - public void testAccountexists() { + public void testAccountExists() { store.createAccount("alice", "alice", "b", "b", 1, true, true); - assertTrue(store.exists("alice", "b", "b", 1)); - assertFalse(store.exists("Alice", "b", "b", 1)); - assertFalse(store.exists("alice", "a", "b", 1)); - assertFalse(store.exists("alice", "b", "a", 1)); - assertFalse(store.exists("alice", "b", "b", 5)); + assertTrue(store.existsAccount("alice", "b", "b", 1)); + assertFalse(store.existsAccount("alice", "a", "b", 1)); + assertFalse(store.existsAccount("alice", "b", "a", 1)); + assertFalse(store.existsAccount("alice", "b", "b", 5)); } @Test - public void testChangeAccountDataAndActiveAccountAfterDeserialization() throws IOException { + public void testDefaultAccountIsModifiedCorrectlyAfterLoading() throws IOException { File tmpAccountFile = tmpFolder.newFile("saros_account.dat"); @@ -284,7 +244,7 @@ public void testChangeAccountDataAndActiveAccountAfterDeserialization() throws I store = new XMPPAccountStore(); store.setAccountFile(tmpAccountFile, null); - XMPPAccount account = store.getActiveAccount(); + XMPPAccount defaultAccount = store.getDefaultAccount(); XMPPAccount another = store.getAllAccounts().get(0); @@ -298,40 +258,103 @@ public void testChangeAccountDataAndActiveAccountAfterDeserialization() throws I true, true); - assertEquals(another, account); + assertEquals(another, defaultAccount); } @Test - public void testFindsExistingAccount() { + public void testSetDefaultToNullThenSaveAndLoadAgain() throws IOException { + File tmpAccountFile = tmpFolder.newFile("saros_account.dat"); + + XMPPAccount defaultAccount; + + store.setAccountFile(tmpAccountFile, null); + + assertEquals(0, store.getAllAccounts().size()); + + // this is the default one + defaultAccount = store.createAccount("alice", "alice", "b", "b", 1, true, true); + store.createAccount("bob", "bob", "b", "b", 1, true, true); + + assertEquals(defaultAccount, store.getDefaultAccount()); + store.setDefaultAccount(null); + + store = new XMPPAccountStore(); + store.setAccountFile(tmpAccountFile, null); + + defaultAccount = store.getDefaultAccount(); + + assertNull(defaultAccount); + + assertEquals(2, store.getAllAccounts().size()); + } + + @Test + public void testGetExistingAccount() { XMPPAccount created = store.createAccount("alice", "alice", "domain", "server", 12345, true, true); - XMPPAccount found = store.findAccount("alice@domain"); + XMPPAccount found = store.getAccount("alice", "domain"); assertEquals(created, found); } @Test - public void testUnsuccessfulFindAccount() { - XMPPAccount found = store.findAccount("alice@domain"); + public void testGetNonExistingAccount() { + XMPPAccount found = store.getAccount("i_want_to_win", "the_lottery"); assertNull(found); } @Test - public void testFindAccountIgnoresCase() { + public void testGetExisingAccountIgnoresCase() { XMPPAccount created = store.createAccount("alice", "alice", "domain", "server", 12345, true, true); - XMPPAccount found = store.findAccount("Alice@Domain"); + + XMPPAccount found = store.getAccount("AlIcE", "DoMaIn"); assertEquals(created, found); } - @Test(expected = NullPointerException.class) - public void testFindAccountWithNull() { - store.findAccount(null); + @Test + public void testFindAccountWithEmptyStringOrNull() { + assertNull(store.getAccount("", "")); + assertNull(store.getAccount(null, null)); } - @Test - public void testFindAccountWithEmptyString() { - XMPPAccount found = store.findAccount(""); - assertNull(found); + private static void writeAccountFile(File accountFile, String key, String content) + throws Exception { + final FileOutputStream dataOut = new FileOutputStream(accountFile); + final byte[] encryptedXmlContent = XMPPAccountStore.Crypto.encrypt(content.getBytes(), key); + + try { + dataOut.write(encryptedXmlContent); + dataOut.flush(); + } finally { + IOUtils.closeQuietly(dataOut); + } + } + + private static String createAccountFileContent( + XMPPAccount activeAccount, ArrayList configuredAccounts) { + + int index = configuredAccounts.indexOf(activeAccount); + StringBuilder xmlContent = + new StringBuilder() + .append("\n") + .append(String.format(" %d\n", index)); + + xmlContent.append(" \n"); + for (XMPPAccount acc : configuredAccounts) { + xmlContent + .append(" \n") + .append(String.format(" %s\n", acc.getUsername())) + .append(String.format(" %s\n", acc.getPassword())) + .append(String.format(" %s\n", acc.getDomain())) + .append(String.format(" %s\n", acc.getServer())) + .append(String.format(" %d\n", acc.getPort())) + .append(String.format(" %s\n", acc.useTLS())) + .append(String.format(" %s\n", acc.useSASL())) + .append(" \n"); + } + xmlContent.append(" \n").append(""); + + return xmlContent.toString(); } } 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..b4ac728355 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 (getSelectedAccount().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..cbe9264582 100644 --- a/eclipse/src/saros/ui/util/XMPPConnectionSupport.java +++ b/eclipse/src/saros/ui/util/XMPPConnectionSupport.java @@ -129,18 +129,19 @@ 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; - - /* - * 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 (account == null) accountToConnect = store.getDefaultAccount(); + else accountToConnect = account; + + 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/eclipse/src/saros/ui/wizards/pages/CreateXMPPAccountWizardPage.java b/eclipse/src/saros/ui/wizards/pages/CreateXMPPAccountWizardPage.java index 651bc0dcdf..5915ab9a9b 100644 --- a/eclipse/src/saros/ui/wizards/pages/CreateXMPPAccountWizardPage.java +++ b/eclipse/src/saros/ui/wizards/pages/CreateXMPPAccountWizardPage.java @@ -231,7 +231,7 @@ public void widgetSelected(SelectionEvent e) { private void updatePageCompletion() { - boolean accountExists = accountStore.exists(getUsername(), getServer(), "", 0); + boolean accountExists = accountStore.existsAccount(getUsername(), getServer(), "", 0); boolean isUsernameEmpty = getUsername().length() == 0; diff --git a/eclipse/src/saros/ui/wizards/pages/EditXMPPAccountWizardPage.java b/eclipse/src/saros/ui/wizards/pages/EditXMPPAccountWizardPage.java index ed3f42bf1b..77a5c48a4f 100644 --- a/eclipse/src/saros/ui/wizards/pages/EditXMPPAccountWizardPage.java +++ b/eclipse/src/saros/ui/wizards/pages/EditXMPPAccountWizardPage.java @@ -150,7 +150,7 @@ private void updatePageCompletion() { */ if ((isJIDValid && isPasswordValid && isServerValid && isPortValid)) accountExists = - accountStore.exists(getJID().getName(), getJID().getDomain(), getServer(), port); + accountStore.existsAccount(getJID().getName(), getJID().getDomain(), getServer(), port); // allow password modification if (accountExists != null diff --git a/eclipse/src/saros/ui/wizards/pages/EnterXMPPAccountWizardPage.java b/eclipse/src/saros/ui/wizards/pages/EnterXMPPAccountWizardPage.java index ab87ec351e..23b8bcd0f4 100644 --- a/eclipse/src/saros/ui/wizards/pages/EnterXMPPAccountWizardPage.java +++ b/eclipse/src/saros/ui/wizards/pages/EnterXMPPAccountWizardPage.java @@ -205,7 +205,7 @@ private void updatePageCompletion() { */ if ((isJIDValid && isPasswordValid && isServerValid && isPortValid)) accountExists = - accountStore.exists(getJID().getName(), getJID().getDomain(), getServer(), port); + accountStore.existsAccount(getJID().getName(), getJID().getDomain(), getServer(), port); if (!isJIDValid && wasJIDValid) { errorMessage = Messages.jid_format_errorMessage; 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/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 887a406d95..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.exists(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 @@ -60,7 +47,7 @@ public void addAccount(String username, String password, String domain) throws R XMPPAccountStore accountStore = getXmppAccountStore(); - if (accountStore.exists(username, domain, "", 0)) { + if (accountStore.existsAccount(username, domain, "", 0)) { LOG.debug( "account with username '" + username + "' and domain '" + domain + "' already exists"); return; @@ -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 94b5277dea..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,18 +342,16 @@ 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) { - return getXmppAccountStore().exists(jid.getName(), jid.getDomain(), "", 0); + return getXmppAccountStore().existsAccount(jid.getName(), jid.getDomain(), "", 0); } @Override 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); }