Various code householding

- Remove unused API on DataSource
- Add some sensible javadoc to DataSource
- Minor code simplification

src/main/java/fr/xephi/authme/command/executable/authme/AccountsCommand.java

@@ -30,7 +30,7 @@ public class AccountsCommand implements ExecutableCommand {
                         return;
                     }
 
-                    List<String> accountList = commandService.getDataSource().getAllAuthsByName(auth);
+                    List<String> accountList = commandService.getDataSource().getAllAuthsByIp(auth.getIp());
                     if (accountList.isEmpty()) {
                         commandService.send(sender, MessageKey.USER_NOT_REGISTERED);
                     } else if (accountList.size() == 1) {

src/main/java/fr/xephi/authme/datasource/CacheDataSource.java

@@ -115,24 +115,6 @@ public class CacheDataSource implements DataSource {
         return result;
     }
 
-    @Override
-    public int getIps(String ip) {
-        return source.getIps(ip);
-    }
-
-    @Override
-    public int purgeDatabase(long until) {
-        int cleared = source.purgeDatabase(until);
-        if (cleared > 0) {
-            for (Optional<PlayerAuth> auth : cachedAuths.asMap().values()) {
-                if (auth.isPresent() && auth.get().getLastLogin() < until) {
-                    cachedAuths.invalidate(auth.get().getNickname());
-                }
-            }
-        }
-        return cleared;
-    }
-
     @Override
     public List<String> autoPurgeDatabase(long until) {
         List<String> cleared = source.autoPurgeDatabase(until);
@@ -172,11 +154,6 @@ public class CacheDataSource implements DataSource {
         return result;
     }
 
-    @Override
-    public synchronized List<String> getAllAuthsByName(PlayerAuth auth) {
-        return source.getAllAuthsByName(auth);
-    }
-
     @Override
     public synchronized List<String> getAllAuthsByIp(final String ip) {
         return source.getAllAuthsByIp(ip);

src/main/java/fr/xephi/authme/datasource/DataSource.java

@@ -6,146 +6,119 @@ import fr.xephi.authme.security.crypts.HashedPassword;
 import java.util.List;
 
 /**
+ * Interface for manipulating {@link PlayerAuth} objects from a data source.
  */
 public interface DataSource {
 
     /**
-     * Method isAuthAvailable.
+     * Return whether there is a record for the given username.
      *
-     * @param user String
+     * @param user The username to look up
-     *
+     * @return True if there is a record, false otherwise
-     * @return boolean
      */
     boolean isAuthAvailable(String user);
 
     /**
-     * Method getPassword.
+     * Return the hashed password of the player.
      *
-     * @param user String
+     * @param user The user whose password should be retrieve
-     *
+     * @return The password hash of the player
-     * @return String
      */
     HashedPassword getPassword(String user);
 
     /**
-     * Method getAuth.
+     * Retrieve the entire PlayerAuth object associated with the username.
      *
-     * @param user String
+     * @param user The user to retrieve
-     *
+     * @return The PlayerAuth object for the given username
-     * @return PlayerAuth
      */
     PlayerAuth getAuth(String user);
 
     /**
-     * Method saveAuth.
+     * Save a new PlayerAuth object.
      *
-     * @param auth PlayerAuth
+     * @param auth The new PlayerAuth to persist
-     *
+     * @return True upon success, false upon failure
-     * @return boolean
      */
     boolean saveAuth(PlayerAuth auth);
 
     /**
-     * Method updateSession.
+     * Update the session of a record (IP, last login, real name).
      *
-     * @param auth PlayerAuth
+     * @param auth The PlayerAuth object to update in the database
-     *
+     * @return True upon success, false upon failure
-     * @return boolean
      */
     boolean updateSession(PlayerAuth auth);
 
     /**
-     * Method updatePassword.
+     * Update the password of the given PlayerAuth object.
      *
-     * @param auth PlayerAuth
+     * @param auth The PlayerAuth whose password should be updated
-     *
+     * @return True upon success, false upon failure
-     * @return boolean
      */
     boolean updatePassword(PlayerAuth auth);
 
+    /**
+     * Update the password of the given player.
+     *
+     * @param user The user whose password should be updated
+     * @param password The new password
+     * @return True upon success, false upon failure
+     */
     boolean updatePassword(String user, HashedPassword password);
 
     /**
-     * Method purgeDatabase.
+     * Purge all records in the database whose last login was longer ago than
+     * the given time.
      *
-     * @param until long
+     * @param until The minimum last login
-     *
+     * @return The account names that have been removed
-     * @return int
-     */
-    int purgeDatabase(long until);
-
-    /**
-     * Method autoPurgeDatabase.
-     *
-     * @param until long
-     *
-     * @return List of String
      */
     List<String> autoPurgeDatabase(long until);
 
     /**
-     * Method removeAuth.
+     * Remove a user record from the database.
      *
-     * @param user String
+     * @param user The user to remove
-     *
+     * @return True upon success, false upon failure
-     * @return boolean
      */
     boolean removeAuth(String user);
 
     /**
-     * Method updateQuitLoc.
+     * Update the quit location of a PlayerAuth.
      *
-     * @param auth PlayerAuth
+     * @param auth The entry whose quit location should be updated
-     *
+     * @return True upon success, false upon failure
-     * @return boolean
      */
     boolean updateQuitLoc(PlayerAuth auth);
 
     /**
-     * Method getIps.
+     * Return all usernames associated with the given IP address.
      *
-     * @param ip String
+     * @param ip The IP address to look up
-     *
+     * @return Usernames associated with the given IP address
-     * @return int
-     */
-    int getIps(String ip);
-
-    /**
-     * Method getAllAuthsByName.
-     *
-     * @param auth PlayerAuth
-     *
-     * @return List of String
-     */
-    List<String> getAllAuthsByName(PlayerAuth auth);
-
-    /**
-     * Method getAllAuthsByIp.
-     *
-     * @param ip String
-     *
-     * @return List of String * @throws Exception
      */
     List<String> getAllAuthsByIp(String ip);
 
     /**
-     * Method getAllAuthsByEmail.
+     * Return all usernames associated with the given email address.
      *
-     * @param email String
+     * @param email The email address to look up
-     *
+     * @return Users using the given email address
-     * @return List of String * @throws Exception
      */
     List<String> getAllAuthsByEmail(String email);
 
     /**
-     * Method updateEmail.
+     * Update the email of the PlayerAuth in the data source.
      *
-     * @param auth PlayerAuth
+     * @param auth The PlayerAuth whose email should be updated
-     *
+     * @return True upon success, false upon failure
-     * @return boolean
      */
     boolean updateEmail(PlayerAuth auth);
 
+    /**
+     * Close the underlying connections to the data source.
+     */
     void close();
 
     void reload();

src/main/java/fr/xephi/authme/datasource/FlatFile.java

@@ -278,82 +278,6 @@ public class FlatFile implements DataSource {
         return true;
     }
 
-    @Override
-    public int getIps(String ip) {
-        BufferedReader br = null;
-        int countIp = 0;
-        try {
-            br = new BufferedReader(new FileReader(source));
-            String line;
-            while ((line = br.readLine()) != null) {
-                String[] args = line.split(":");
-                if (args.length > 3 && args[2].equals(ip)) {
-                    countIp++;
-                }
-            }
-            return countIp;
-        } catch (FileNotFoundException ex) {
-            ConsoleLogger.showError(ex.getMessage());
-            return 0;
-        } catch (IOException ex) {
-            ConsoleLogger.showError(ex.getMessage());
-            return 0;
-        } finally {
-            if (br != null) {
-                try {
-                    br.close();
-                } catch (IOException ignored) {
-                }
-            }
-        }
-    }
-
-    @Override
-    public int purgeDatabase(long until) {
-        BufferedReader br = null;
-        BufferedWriter bw = null;
-        ArrayList<String> lines = new ArrayList<>();
-        int cleared = 0;
-        try {
-            br = new BufferedReader(new FileReader(source));
-            String line;
-            while ((line = br.readLine()) != null) {
-                String[] args = line.split(":");
-                if (args.length >= 4) {
-                    if (Long.parseLong(args[3]) >= until) {
-                        lines.add(line);
-                        continue;
-                    }
-                }
-                cleared++;
-            }
-            bw = new BufferedWriter(new FileWriter(source));
-            for (String l : lines) {
-                bw.write(l + "\n");
-            }
-        } catch (FileNotFoundException ex) {
-            ConsoleLogger.showError(ex.getMessage());
-            return cleared;
-        } catch (IOException ex) {
-            ConsoleLogger.showError(ex.getMessage());
-            return cleared;
-        } finally {
-            if (br != null) {
-                try {
-                    br.close();
-                } catch (IOException ignored) {
-                }
-            }
-            if (bw != null) {
-                try {
-                    bw.close();
-                } catch (IOException ignored) {
-                }
-            }
-        }
-        return cleared;
-    }
-
     @Override
     public List<String> autoPurgeDatabase(long until) {
         BufferedReader br = null;
@@ -532,36 +456,6 @@ public class FlatFile implements DataSource {
         return true;
     }
 
-    @Override
-    public List<String> getAllAuthsByName(PlayerAuth auth) {
-        BufferedReader br = null;
-        List<String> countIp = new ArrayList<>();
-        try {
-            br = new BufferedReader(new FileReader(source));
-            String line;
-            while ((line = br.readLine()) != null) {
-                String[] args = line.split(":");
-                if (args.length > 3 && args[2].equals(auth.getIp())) {
-                    countIp.add(args[0]);
-                }
-            }
-            return countIp;
-        } catch (FileNotFoundException ex) {
-            ConsoleLogger.showError(ex.getMessage());
-            return new ArrayList<>();
-        } catch (IOException ex) {
-            ConsoleLogger.showError(ex.getMessage());
-            return new ArrayList<>();
-        } finally {
-            if (br != null) {
-                try {
-                    br.close();
-                } catch (IOException ignored) {
-                }
-            }
-        }
-    }
-
     @Override
     public List<String> getAllAuthsByIp(String ip) {
         BufferedReader br = null;

src/main/java/fr/xephi/authme/datasource/MySQL.java

@@ -562,16 +562,14 @@ public class MySQL implements DataSource {
 
     @Override
     public synchronized boolean updateSession(PlayerAuth auth) {
-        try (Connection con = getConnection()) {
+        String sql = "UPDATE " + tableName + " SET "
-            String sql = "UPDATE " + tableName + " SET "
+            + col.IP + "=?, " + col.LAST_LOGIN + "=?, " + col.REAL_NAME + "=? WHERE " + col.NAME + "=?;";
-                + col.IP + "=?, " + col.LAST_LOGIN + "=?, " + col.REAL_NAME + "=? WHERE " + col.NAME + "=?;";
+        try (Connection con = getConnection(); PreparedStatement pst = con.prepareStatement(sql)) {
-            PreparedStatement pst = con.prepareStatement(sql);
             pst.setString(1, auth.getIp());
             pst.setTimestamp(2, new Timestamp(auth.getLastLogin()));
             pst.setString(3, auth.getRealName());
             pst.setString(4, auth.getNickname());
             pst.executeUpdate();
-            pst.close();
             return true;
         } catch (SQLException ex) {
             logSqlException(ex);
@@ -579,20 +577,6 @@ public class MySQL implements DataSource {
         return false;
     }
 
-    @Override
-    public synchronized int purgeDatabase(long until) {
-        int result = 0;
-        try (Connection con = getConnection()) {
-            String sql = "DELETE FROM " + tableName + " WHERE " + col.LAST_LOGIN + "<?;";
-            PreparedStatement pst = con.prepareStatement(sql);
-            pst.setLong(1, until);
-            result = pst.executeUpdate();
-        } catch (SQLException ex) {
-            logSqlException(ex);
-        }
-        return result;
-    }
-
     @Override
     public synchronized List<String> autoPurgeDatabase(long until) {
         List<String> list = new ArrayList<>();
@@ -669,25 +653,6 @@ public class MySQL implements DataSource {
         return false;
     }
 
-    @Override
-    public synchronized int getIps(String ip) {
-        int countIp = 0;
-        try (Connection con = getConnection()) {
-            String sql = "SELECT COUNT(*) FROM " + tableName + " WHERE " + col.IP + "=?;";
-            PreparedStatement pst = con.prepareStatement(sql);
-            pst.setString(1, ip);
-            ResultSet rs = pst.executeQuery();
-            while (rs.next()) {
-                countIp = rs.getInt(1);
-            }
-            rs.close();
-            pst.close();
-        } catch (SQLException ex) {
-            logSqlException(ex);
-        }
-        return countIp;
-    }
-
     @Override
     public synchronized boolean updateEmail(PlayerAuth auth) {
         try (Connection con = getConnection()) {
@@ -722,25 +687,6 @@ public class MySQL implements DataSource {
         }
     }
 
-    @Override
-    public synchronized List<String> getAllAuthsByName(PlayerAuth auth) {
-        List<String> result = new ArrayList<>();
-        try (Connection con = getConnection()) {
-            String sql = "SELECT " + col.NAME + " FROM " + tableName + " WHERE " + col.IP + "=?;";
-            PreparedStatement pst = con.prepareStatement(sql);
-            pst.setString(1, auth.getIp());
-            ResultSet rs = pst.executeQuery();
-            while (rs.next()) {
-                result.add(rs.getString(col.NAME));
-            }
-            rs.close();
-            pst.close();
-        } catch (SQLException ex) {
-            logSqlException(ex);
-        }
-        return result;
-    }
-
     @Override
     public synchronized List<String> getAllAuthsByIp(String ip) {
         List<String> result = new ArrayList<>();

src/main/java/fr/xephi/authme/datasource/SQLite.java

@@ -266,18 +266,6 @@ public class SQLite implements DataSource {
         return false;
     }
 
-    @Override
-    public int purgeDatabase(long until) {
-        String sql = "DELETE FROM " + tableName + " WHERE " + col.LAST_LOGIN + "<?;";
-        try (PreparedStatement pst = con.prepareStatement(sql)) {
-            pst.setLong(1, until);
-            return pst.executeUpdate();
-        } catch (SQLException ex) {
-            logSqlException(ex);
-        }
-        return 0;
-    }
-
     @Override
     public List<String> autoPurgeDatabase(long until) {
         PreparedStatement pst = null;
@@ -336,29 +324,6 @@ public class SQLite implements DataSource {
         return false;
     }
 
-    @Override
-    public int getIps(String ip) {
-        PreparedStatement pst = null;
-        ResultSet rs = null;
-        int countIp = 0;
-        try {
-            // TODO ljacqu 20151230: Simply fetch COUNT(1) and return that
-            pst = con.prepareStatement("SELECT * FROM " + tableName + " WHERE " + col.IP + "=?;");
-            pst.setString(1, ip);
-            rs = pst.executeQuery();
-            while (rs.next()) {
-                countIp++;
-            }
-            return countIp;
-        } catch (SQLException ex) {
-            logSqlException(ex);
-        } finally {
-            close(rs);
-            close(pst);
-        }
-        return 0;
-    }
-
     @Override
     public boolean updateEmail(PlayerAuth auth) {
         String sql = "UPDATE " + tableName + " SET " + col.EMAIL + "=? WHERE " + col.NAME + "=?;";
@@ -406,37 +371,13 @@ public class SQLite implements DataSource {
         }
     }
 
-    @Override
-    public List<String> getAllAuthsByName(PlayerAuth auth) {
-        PreparedStatement pst = null;
-        ResultSet rs = null;
-        List<String> names = new ArrayList<>();
-        try {
-            // TODO ljacqu 20160214: Use SELECT name if only the name is required
-            pst = con.prepareStatement("SELECT * FROM " + tableName + " WHERE " + col.IP + "=?;");
-            pst.setString(1, auth.getIp());
-            rs = pst.executeQuery();
-            while (rs.next()) {
-                names.add(rs.getString(col.NAME));
-            }
-            return names;
-        } catch (SQLException ex) {
-            logSqlException(ex);
-
-        } finally {
-            close(rs);
-            close(pst);
-        }
-        return new ArrayList<>();
-    }
-
     @Override
     public List<String> getAllAuthsByIp(String ip) {
         PreparedStatement pst = null;
         ResultSet rs = null;
         List<String> countIp = new ArrayList<>();
         try {
-            pst = con.prepareStatement("SELECT * FROM " + tableName + " WHERE " + col.IP + "=?;");
+            pst = con.prepareStatement("SELECT " + col.NAME + " FROM " + tableName + " WHERE " + col.IP + "=?;");
             pst.setString(1, ip);
             rs = pst.executeQuery();
             while (rs.next()) {

src/main/java/fr/xephi/authme/process/login/AsynchronousLogin.java

@@ -225,7 +225,7 @@ public class AsynchronousLogin {
             return;
         }
 
-        List<String> auths = this.database.getAllAuthsByName(auth);
+        List<String> auths = this.database.getAllAuthsByIp(auth.getIp());
         if (auths.size() < 2) {
             return;
         }

src/main/java/fr/xephi/authme/process/register/AsyncRegister.java

@@ -12,6 +12,7 @@ import fr.xephi.authme.security.crypts.HashedPassword;
 import fr.xephi.authme.security.crypts.TwoFactor;
 import fr.xephi.authme.settings.NewSetting;
 import fr.xephi.authme.settings.Settings;
+import fr.xephi.authme.util.StringUtils;
 import org.bukkit.Bukkit;
 import org.bukkit.entity.Player;
 
@@ -19,11 +20,11 @@ import org.bukkit.entity.Player;
  */
 public class AsyncRegister {
 
-    protected final Player player;
+    private final Player player;
-    protected final String name;
+    private final String name;
-    protected final String password;
+    private final String password;
     private final String ip;
-    private String email = "";
+    private final String email;
     private final AuthMe plugin;
     private final DataSource database;
     private final Messages m;
@@ -86,7 +87,7 @@ public class AsyncRegister {
 
     public void process() {
         if (preRegisterCheck()) {
-            if (email != null && !email.isEmpty()) {
+            if (!StringUtils.isEmpty(email)) {
                 emailRegister();
             } else {
                 passwordRegister();

src/test/java/fr/xephi/authme/command/executable/authme/AccountsCommandTest.java

@@ -16,7 +16,6 @@ import java.util.List;
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.mockito.BDDMockito.given;
-import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
@@ -48,8 +47,8 @@ public class AccountsCommandTest {
         // given
         given(sender.getName()).willReturn("Tester");
         List<String> arguments = Collections.EMPTY_LIST;
-        given(dataSource.getAuth("tester")).willReturn(mock(PlayerAuth.class));
+        given(dataSource.getAuth("tester")).willReturn(authWithIp("123.45.67.89"));
-        given(dataSource.getAllAuthsByName(any(PlayerAuth.class))).willReturn(Arrays.asList("Toaster", "Pester"));
+        given(dataSource.getAllAuthsByIp("123.45.67.89")).willReturn(Arrays.asList("Toaster", "Pester"));
 
         // when
         command.executeCommand(sender, arguments, service);
@@ -81,7 +80,7 @@ public class AccountsCommandTest {
         // given
         List<String> arguments = Collections.singletonList("SomeUser");
         given(dataSource.getAuth("someuser")).willReturn(mock(PlayerAuth.class));
-        given(dataSource.getAllAuthsByName(any(PlayerAuth.class))).willReturn(Collections.EMPTY_LIST);
+        given(dataSource.getAllAuthsByIp(anyString())).willReturn(Collections.EMPTY_LIST);
 
         // when
         command.executeCommand(sender, arguments, service);
@@ -96,8 +95,8 @@ public class AccountsCommandTest {
     public void shouldReturnSingleAccountMessage() {
         // given
         List<String> arguments = Collections.singletonList("SomeUser");
-        given(dataSource.getAuth("someuser")).willReturn(mock(PlayerAuth.class));
+        given(dataSource.getAuth("someuser")).willReturn(authWithIp("56.78.90.123"));
-        given(dataSource.getAllAuthsByName(any(PlayerAuth.class))).willReturn(Collections.singletonList("SomeUser"));
+        given(dataSource.getAllAuthsByIp("56.78.90.123")).willReturn(Collections.singletonList("SomeUser"));
 
         // when
         command.executeCommand(sender, arguments, service);
@@ -169,4 +168,11 @@ public class AccountsCommandTest {
         verify(sender, times(expectedCount)).sendMessage(captor.capture());
         return captor.getAllValues().toArray(new String[expectedCount]);
     }
+
+    private static PlayerAuth authWithIp(String ip) {
+        return PlayerAuth.builder()
+            .name("Test")
+            .ip(ip)
+            .build();
+    }
 }

src/test/java/fr/xephi/authme/settings/NewSettingTest.java

@@ -5,8 +5,7 @@ import fr.xephi.authme.settings.properties.TestConfiguration;
 import fr.xephi.authme.settings.properties.TestEnum;
 import org.bukkit.configuration.file.YamlConfiguration;
 import org.junit.Test;
-import org.mockito.invocation.InvocationOnMock;
+import org.mockito.internal.stubbing.answers.ReturnsArgumentAt;
-import org.mockito.stubbing.Answer;
 
 import java.io.File;
 
@@ -32,10 +31,10 @@ public class NewSettingTest {
     public void shouldLoadAllConfigs() {
         // given
         YamlConfiguration configuration = mock(YamlConfiguration.class);
-        given(configuration.getString(anyString(), anyString())).willAnswer(withDefaultArgument());
+        given(configuration.getString(anyString(), anyString())).willAnswer(new ReturnsArgumentAt(1));
-        given(configuration.getBoolean(anyString(), anyBoolean())).willAnswer(withDefaultArgument());
+        given(configuration.getBoolean(anyString(), anyBoolean())).willAnswer(new ReturnsArgumentAt(1));
-        given(configuration.getDouble(anyString(), anyDouble())).willAnswer(withDefaultArgument());
+        given(configuration.getDouble(anyString(), anyDouble())).willAnswer(new ReturnsArgumentAt(1));
-        given(configuration.getInt(anyString(), anyInt())).willAnswer(withDefaultArgument());
+        given(configuration.getInt(anyString(), anyInt())).willAnswer(new ReturnsArgumentAt(1));
 
         setReturnValue(configuration, TestConfiguration.VERSION_NUMBER, 20);
         setReturnValue(configuration, TestConfiguration.SKIP_BORING_FEATURES, true);
@@ -89,14 +88,4 @@ public class NewSettingTest {
             setting.getProperty(property).equals(property.getDefaultValue()), equalTo(true));
     }
 
-    private static <T> Answer<T> withDefaultArgument() {
-        return new Answer<T>() {
-            @Override
-            public T answer(InvocationOnMock invocation) throws Throwable {
-                // Return the second parameter -> the default
-                return (T) invocation.getArguments()[1];
-            }
-        };
-    }
-
 }