From a8edfbad56a76f89ce5ae7154d844d47e5af46cd Mon Sep 17 00:00:00 2001 From: Stefan Rossbach Date: Wed, 31 Jul 2019 12:52:34 +0200 Subject: [PATCH 1/3] [INTERNAL][CORE] Amends XMPPAccountStore API Introduces get/set defaultAccount methods which should be a replacement for set/get activeAccount in the future. They allow null values making the call to isEmpty obsolete. And now it is also possible to completely remove all account data. Add getAccount methods as there was no way to retrieve specific account unless you called getAllAccounts and perform the search by yourself. --- core/src/saros/account/XMPPAccount.java | 3 +- core/src/saros/account/XMPPAccountStore.java | 243 ++++++++++++------ .../saros/account/XMPPAccountStoreTest.java | 78 +++++- .../pages/CreateXMPPAccountWizardPage.java | 2 +- .../pages/EditXMPPAccountWizardPage.java | 2 +- .../pages/EnterXMPPAccountWizardPage.java | 2 +- .../impl/AccountManipulatorImpl.java | 4 +- .../menu/submenu/impl/SarosPreferences.java | 2 +- 8 files changed, 235 insertions(+), 101 deletions(-) diff --git a/core/src/saros/account/XMPPAccount.java b/core/src/saros/account/XMPPAccount.java index ededd8f90e..b9f759994f 100644 --- a/core/src/saros/account/XMPPAccount.java +++ b/core/src/saros/account/XMPPAccount.java @@ -136,8 +136,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..293c010e5f 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; @@ -57,7 +56,7 @@ public final class XMPPAccountStore { new CopyOnWriteArrayList(); private Set accounts; - private XMPPAccount activeAccount; + private XMPPAccount defaultAccount; private File accountFile; private String secretKey; @@ -94,9 +93,8 @@ private void notifyAccountStoreListeners() { } private void notifyActiveAccountListeners() { - XMPPAccount active = !isEmpty() ? getActiveAccount() : null; for (IAccountStoreListener listener : listeners) { - listener.activeAccountChanged(active); + listener.activeAccountChanged(defaultAccount); } } @@ -121,6 +119,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 +137,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 +164,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 +179,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 +205,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 +242,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 +270,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,20 +293,38 @@ 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); + + final Set servers = new HashSet(); + + for (final XMPPAccount account : getAllAccounts()) servers.add(account.getServer()); + + return new ArrayList(servers); + } + + /** + * Sets the given account as the default one. + * + * @param account the account to set as default or null + * @throws IllegalArgumentException if the account is not found in the store + */ + public void setDefaultAccount(final XMPPAccount account) { + synchronized (this) { + if (account != null && !accounts.contains(account)) + throw new IllegalArgumentException( + "account '" + account + "' is not in the current account store"); + + defaultAccount = account; + + saveAccounts(); } - return servers; + + notifyActiveAccountListeners(); } /** @@ -339,14 +332,16 @@ public synchronized List getServers() { * * @param account the account to activate * @throws IllegalArgumentException if the account is not found in the store + * @deprecated Will be removed soon, do not use */ + @Deprecated public void setAccountActive(XMPPAccount account) { synchronized (this) { if (!accounts.contains(account)) throw new IllegalArgumentException( "account '" + account + "' is not in the current account store"); - activeAccount = account; + defaultAccount = account; saveAccounts(); } @@ -355,32 +350,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 +399,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(); } @@ -489,16 +482,34 @@ public void changeAccountData( notifyAccountStoreListeners(); } + /** + * Returns the current default account. + * + * @return the default account or null if the default account is not set + */ + public synchronized XMPPAccount getDefaultAccount() { + return defaultAccount; + } + /** * Returns the current active account. * * @return the active account * @throws IllegalStateException if the account store is empty + * @deprecated Will be removed soon, do not use */ + @Deprecated public synchronized XMPPAccount getActiveAccount() { - if (activeAccount == null) throw new IllegalStateException("the account store is empty"); + if (defaultAccount != null) return defaultAccount; + + if (accounts.isEmpty()) throw new IllegalStateException("the account store is empty"); + + // backward compatibility for now, just pick one + setAccountActive(accounts.iterator().next()); - return activeAccount; + assert defaultAccount != null; + + return defaultAccount; } /** @@ -512,29 +523,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)); + } + + /** + * Returns the account for the given username and domain. + * + *

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 getAccount(final String username, final String domain) { + return getAllAccounts() + .stream() + .findFirst() + .filter(a -> matchesAnyServer(a, username, domain)) + .orElse(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); } /** @@ -543,7 +585,9 @@ public synchronized boolean exists(String username, String domain, String server * @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 + * @deprecated Will be removed soon, do not use */ + @Deprecated public XMPPAccount findAccount(String jidString) { if (jidString == null) { throw new NullPointerException("Null argument 'jidString'"); @@ -695,4 +739,39 @@ 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().equals(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().equals(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..bf39af62ce 100644 --- a/core/test/junit/saros/account/XMPPAccountStoreTest.java +++ b/core/test/junit/saros/account/XMPPAccountStoreTest.java @@ -69,9 +69,9 @@ 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)); + assertTrue(store.existsAccount("activeAccount", "activedomain", "activeserver", 3)); + assertTrue(store.existsAccount("anotherAccount1", "anotherdomain1", "anotherserver1", 1)); + assertTrue(store.existsAccount("anotherAccount2", "anotherdomain2", "anotherserver2", 2)); assertEquals("activeAccount", store.getActiveAccount().getUsername()); } @@ -158,10 +158,11 @@ public void testDeleteExistingAccount() { assertEquals(1, store.getAllAccounts().size()); } - @Test(expected = IllegalStateException.class) - public void testDeleteActiveAccount() { + @Test + public void deleteDefaultAccount() { store.createAccount("a", "a", "a", "a", 1, true, true); - store.deleteAccount(store.getActiveAccount()); + store.deleteAccount(store.getDefaultAccount()); + assertNull(store.getDefaultAccount()); } @Test(expected = IllegalArgumentException.class) @@ -265,11 +266,11 @@ public void testChangeAccountDataAndThenDeleteAccount() { 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", "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 @@ -301,6 +302,61 @@ public void testChangeAccountDataAndActiveAccountAfterDeserialization() throws I assertEquals(another, account); } + @Test + public void testChangeAccountAfterDeserialization() throws IOException { + + File tmpAccountFile = tmpFolder.newFile("saros_account.dat"); + + store.setAccountFile(tmpAccountFile, null); + + store.createAccount("alice", "alice", "b", "b", 1, true, true); + + store = new XMPPAccountStore(); + store.setAccountFile(tmpAccountFile, null); + + XMPPAccount defaultAccount = store.getDefaultAccount(); + + XMPPAccount another = store.getAllAccounts().get(0); + + store.changeAccountData( + another, + another.getUsername(), + another.getPassword(), + another.getDomain(), + "", + 0, + true, + true); + + assertEquals(another, defaultAccount); + } + + @Test + public void testSetDefaultToNullAndThenDeserializeAgain() 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 testFindsExistingAccount() { XMPPAccount created = 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/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..1c8e541e31 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 @@ -43,7 +43,7 @@ public void restoreDefaultAccount(String username, String password, String domai XMPPAccount activeAccount = accountStore.getActiveAccount(); - if (accountStore.exists(username, domain, "", 0)) return; + if (accountStore.existsAccount(username, domain, "", 0)) return; XMPPAccount defaultAccount = accountStore.createAccount(username, password, domain, "", 0, true, true); @@ -60,7 +60,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; 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..6e2309835a 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 @@ -353,7 +353,7 @@ private boolean isAccountActiveNoGUI(JID jid) { } private boolean isAccountExistNoGUI(JID jid) { - return getXmppAccountStore().exists(jid.getName(), jid.getDomain(), "", 0); + return getXmppAccountStore().existsAccount(jid.getName(), jid.getDomain(), "", 0); } @Override From a6f20a1cfb1378e48ec96584515804fc12b29ac7 Mon Sep 17 00:00:00 2001 From: Stefan Rossbach Date: Wed, 31 Jul 2019 15:21:10 +0200 Subject: [PATCH 2/3] [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 | 21 +++++----- .../saros/ui/wizards/ConfigurationWizard.java | 25 ++++++------ .../ui/actions/ConnectServerAction.java | 14 +++++-- .../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 +- 11 files changed, 92 insertions(+), 87 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..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/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 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); } From a7849707c2d197dc681e75617c34d853d453bff1 Mon Sep 17 00:00:00 2001 From: Stefan Rossbach Date: Sat, 3 Aug 2019 00:10:46 +0200 Subject: [PATCH 3/3] [INTERNAL][CORE] Removes deprecated methods from XMPPAccountStore This patch also fixes an issue in the getAccountsMethods so they lookup the account correctly by ignoring the case sensitivity of the username. Adds a FIXME to the XMPPAccount class regarding RFC 6122 --- core/src/saros/account/XMPPAccount.java | 3 + core/src/saros/account/XMPPAccountStore.java | 76 +------- .../saros/account/XMPPAccountStoreTest.java | 171 +++++++----------- 3 files changed, 75 insertions(+), 175 deletions(-) diff --git a/core/src/saros/account/XMPPAccount.java b/core/src/saros/account/XMPPAccount.java index b9f759994f..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)) diff --git a/core/src/saros/account/XMPPAccountStore.java b/core/src/saros/account/XMPPAccountStore.java index 293c010e5f..e8118b156f 100644 --- a/core/src/saros/account/XMPPAccountStore.java +++ b/core/src/saros/account/XMPPAccountStore.java @@ -30,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 @@ -327,28 +326,6 @@ public void setDefaultAccount(final XMPPAccount account) { notifyActiveAccountListeners(); } - /** - * Makes the given account active. - * - * @param account the account to activate - * @throws IllegalArgumentException if the account is not found in the store - * @deprecated Will be removed soon, do not use - */ - @Deprecated - public void setAccountActive(XMPPAccount account) { - synchronized (this) { - if (!accounts.contains(account)) - throw new IllegalArgumentException( - "account '" + account + "' is not in the current account store"); - - defaultAccount = account; - - saveAccounts(); - } - - notifyActiveAccountListeners(); - } - /** * Deletes an account from the store. If this was the default account the default account is set * to null. @@ -491,27 +468,6 @@ public synchronized XMPPAccount getDefaultAccount() { return defaultAccount; } - /** - * Returns the current active account. - * - * @return the active account - * @throws IllegalStateException if the account store is empty - * @deprecated Will be removed soon, do not use - */ - @Deprecated - public synchronized XMPPAccount getActiveAccount() { - if (defaultAccount != null) return defaultAccount; - - if (accounts.isEmpty()) throw new IllegalStateException("the account store is empty"); - - // backward compatibility for now, just pick one - setAccountActive(accounts.iterator().next()); - - assert defaultAccount != null; - - return defaultAccount; - } - /** * Returns if the account store is currently empty * @@ -579,33 +535,6 @@ public XMPPAccount getAccount( .orElse(null); } - /** - * Searches for an account in the account store. - * - * @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 - * @deprecated Will be removed soon, do not use - */ - @Deprecated - 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; - } - } - - return null; - } - /** * class which is used for serialization of account information * @@ -759,7 +688,8 @@ private static int compare(final XMPPAccount a, XMPPAccount b) { private static boolean matchesAnyServer( final XMPPAccount account, final String username, final String domain) { - return account.getUsername().equals(username) && account.getDomain().equalsIgnoreCase(domain); + return account.getUsername().equalsIgnoreCase(username) + && account.getDomain().equalsIgnoreCase(domain); } private static boolean matchesAccount( @@ -769,7 +699,7 @@ private static boolean matchesAccount( final String server, final int port) { - return account.getUsername().equals(username) + 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 bf39af62ce..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. @@ -73,57 +69,20 @@ public void testAccountsFileFormat() throws Exception { assertTrue(store.existsAccount("anotherAccount1", "anotherdomain1", "anotherserver1", 1)); assertTrue(store.existsAccount("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); - - 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 @@ -159,10 +118,11 @@ public void testDeleteExistingAccount() { } @Test - public void deleteDefaultAccount() { + public void testDeleteDefaultAccount() { store.createAccount("a", "a", "a", "a", 1, true, true); store.deleteAccount(store.getDefaultAccount()); assertNull(store.getDefaultAccount()); + assertTrue(store.isEmpty()); } @Test(expected = IllegalArgumentException.class) @@ -172,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) @@ -197,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()); @@ -263,47 +223,17 @@ public void testChangeAccountDataAndThenDeleteAccount() { } @Test - public void testAccountexists() { + public void testAccountExists() { store.createAccount("alice", "alice", "b", "b", 1, true, true); assertTrue(store.existsAccount("alice", "b", "b", 1)); - assertFalse(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 { - - File tmpAccountFile = tmpFolder.newFile("saros_account.dat"); - - store.setAccountFile(tmpAccountFile, null); - - store.createAccount("alice", "alice", "b", "b", 1, true, true); - - store = new XMPPAccountStore(); - store.setAccountFile(tmpAccountFile, null); - - XMPPAccount account = store.getActiveAccount(); - - XMPPAccount another = store.getAllAccounts().get(0); - - store.changeAccountData( - another, - another.getUsername(), - another.getPassword(), - another.getDomain(), - "", - 0, - true, - true); - - assertEquals(another, account); - } - - @Test - public void testChangeAccountAfterDeserialization() throws IOException { + public void testDefaultAccountIsModifiedCorrectlyAfterLoading() throws IOException { File tmpAccountFile = tmpFolder.newFile("saros_account.dat"); @@ -332,7 +262,7 @@ public void testChangeAccountAfterDeserialization() throws IOException { } @Test - public void testSetDefaultToNullAndThenDeserializeAgain() throws IOException { + public void testSetDefaultToNullThenSaveAndLoadAgain() throws IOException { File tmpAccountFile = tmpFolder.newFile("saros_account.dat"); XMPPAccount defaultAccount; @@ -340,6 +270,7 @@ public void testSetDefaultToNullAndThenDeserializeAgain() throws IOException { 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); @@ -358,36 +289,72 @@ public void testSetDefaultToNullAndThenDeserializeAgain() throws IOException { } @Test - public void testFindsExistingAccount() { + 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(); } }