From 38cc217cff374e3a33bf3d61b7c794d936c0f148 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 21 Nov 2015 07:41:56 +0100 Subject: [PATCH 1/2] Revert certain JavaDoc changes Ideally JavaDoc should provide additional information to the developer as to the method's purpose and usage. Typically you do not add the return type of the method and the parameter's types since this can be seen in the code. A short description of what the parameter really is (e.g. a String can hold many types of information) is a lot more beneficial. A JavaDoc statement simply restating the parameter types and the method name is, put bluntly, simply noise, since all of these things are already contained in the code itself. Similarly, @see references are great for pointing to other, related methods but aren't very helpful to point to a superclass method (the implemented or overriden method) since it is implied by @Override. A developer can navigate easily to the superclass method with any reasonable IDE. --- pom.xml | 2 +- src/main/java/fr/xephi/authme/AuthMe.java | 118 ++++-------------- .../java/fr/xephi/authme/Log4JFilter.java | 56 +-------- 3 files changed, 28 insertions(+), 148 deletions(-) diff --git a/pom.xml b/pom.xml index 93fdb5f2e..1a9a64396 100644 --- a/pom.xml +++ b/pom.xml @@ -606,7 +606,7 @@ true - + junit junit diff --git a/src/main/java/fr/xephi/authme/AuthMe.java b/src/main/java/fr/xephi/authme/AuthMe.java index 908dacc2b..29d7091c9 100644 --- a/src/main/java/fr/xephi/authme/AuthMe.java +++ b/src/main/java/fr/xephi/authme/AuthMe.java @@ -60,6 +60,7 @@ import net.milkbowl.vault.permission.Permission; import net.minelink.ctplus.CombatTagPlus; /** + * The AuthMe main class. */ public class AuthMe extends JavaPlugin { @@ -506,8 +507,8 @@ public class AuthMe extends JavaPlugin { /** * Get the permissions manager instance. * - - * @return Permissions Manager instance. */ + * @return Permissions Manager instance. + */ public PermissionsManager getPermissionsManager() { return this.permsMan; } @@ -649,10 +650,6 @@ public class AuthMe extends JavaPlugin { } // Save Player Data - /** - * Method savePlayer. - * @param player Player - */ public void savePlayer(Player player) { if ((Utils.isNPC(player)) || (Utils.isUnrestricted(player))) { return; @@ -681,11 +678,6 @@ public class AuthMe extends JavaPlugin { } // Select the player to kick when a vip player join the server when full - /** - * Method generateKickPlayer. - * @param collection Collection - - * @return Player */ public Player generateKickPlayer(Collection collection) { Player player = null; for (Player p : collection) { @@ -730,11 +722,6 @@ public class AuthMe extends JavaPlugin { } // Return the spawn location of a player - /** - * Method getSpawnLocation. - * @param player Player - - * @return Location */ public Location getSpawnLocation(Player player) { World world = player.getWorld(); String[] spawnPriority = Settings.spawnPriority.split(","); @@ -757,21 +744,11 @@ public class AuthMe extends JavaPlugin { } // Return the default spawnpoint of a world - /** - * Method getDefaultSpawn. - * @param world World - - * @return Location */ private Location getDefaultSpawn(World world) { return world.getSpawnLocation(); } // Return the multiverse spawnpoint of a world - /** - * Method getMultiverseSpawn. - * @param world World - - * @return Location */ private Location getMultiverseSpawn(World world) { if (multiverse != null && Settings.multiverse) { try { @@ -784,10 +761,6 @@ public class AuthMe extends JavaPlugin { } // Return the essentials spawnpoint - /** - * Method getEssentialsSpawn. - - * @return Location */ private Location getEssentialsSpawn() { if (essentialsSpawn != null) { return essentialsSpawn; @@ -796,11 +769,6 @@ public class AuthMe extends JavaPlugin { } // Return the authme soawnpoint - /** - * Method getAuthMeSpawn. - * @param player Player - - * @return Location */ private Location getAuthMeSpawn(Player player) { if ((!database.isAuthAvailable(player.getName().toLowerCase()) || !player.hasPlayedBefore()) && (Spawn.getInstance().getFirstSpawn() != null)) { return Spawn.getInstance().getFirstSpawn(); @@ -811,19 +779,11 @@ public class AuthMe extends JavaPlugin { return player.getWorld().getSpawnLocation(); } - /** - * Method switchAntiBotMod. - * @param mode boolean - */ public void switchAntiBotMod(boolean mode) { this.antibotMod = mode; Settings.switchAntiBotMod(mode); } - /** - * Method getAntiBotModMode. - - * @return boolean */ public boolean getAntiBotModMode() { return this.antibotMod; } @@ -850,12 +810,7 @@ public class AuthMe extends JavaPlugin { }, 1, 1200 * Settings.delayRecall); } - /** - * Method replaceAllInfos. - * @param message String - * @param player Player - - * @return String */ + public String replaceAllInfos(String message, Player player) { int playersOnline = Utils.getOnlinePlayers().size(); message = message.replace("&", "\u00a7"); @@ -871,11 +826,7 @@ public class AuthMe extends JavaPlugin { return message; } - /** - * Method getIP. - * @param player Player - - * @return String */ + public String getIP(Player player) { String name = player.getName().toLowerCase(); String ip = player.getAddress().getAddress().getHostAddress(); @@ -889,12 +840,7 @@ public class AuthMe extends JavaPlugin { return ip; } - /** - * Method isLoggedIp. - * @param name String - * @param ip String - - * @return boolean */ + public boolean isLoggedIp(String name, String ip) { int count = 0; for (Player player : Utils.getOnlinePlayers()) { @@ -904,12 +850,7 @@ public class AuthMe extends JavaPlugin { return count >= Settings.getMaxLoginPerIp; } - /** - * Method hasJoinedIp. - * @param name String - * @param ip String - - * @return boolean */ + public boolean hasJoinedIp(String name, String ip) { int count = 0; for (Player player : Utils.getOnlinePlayers()) { @@ -919,21 +860,18 @@ public class AuthMe extends JavaPlugin { return count >= Settings.getMaxJoinPerIp; } - /** - * Method getModuleManager. - - * @return ModuleManager */ + public ModuleManager getModuleManager() { return moduleManager; } /** - * Get Player real IP through VeryGames method + * Gets a player's real IP through VeryGames method. * - * @param player - * player - - * @return String */ + * @param player the player to process + * + * @return the real IP of the player + */ @Deprecated public String getVeryGamesIP(Player player) { String realIP = player.getAddress().getAddress().getHostAddress(); @@ -952,31 +890,19 @@ public class AuthMe extends JavaPlugin { return realIP; } - /** - * Method getCountryCode. - * @param ip String - - * @return String */ + @Deprecated public String getCountryCode(String ip) { return Utils.getCountryCode(ip); } - /** - * Method getCountryName. - * @param ip String - - * @return String */ + @Deprecated public String getCountryName(String ip) { return Utils.getCountryName(ip); } - /** - * Get the command handler instance. - * - - * @return Command handler. */ + public CommandHandler getCommandHandler() { return this.commandHandler; } @@ -993,9 +919,7 @@ public class AuthMe extends JavaPlugin { * @param args * The command arguments (Bukkit). * - - - * @return True if the command was executed, false otherwise. * @see org.bukkit.command.CommandExecutor#onCommand(CommandSender, Command, String, String[]) * @see org.bukkit.command.CommandExecutor#onCommand(CommandSender, Command, String, String[]) + * @return True if the command was executed, false otherwise. */ @Override public boolean onCommand(CommandSender sender, Command cmd, @@ -1012,9 +936,9 @@ public class AuthMe extends JavaPlugin { /** * Get the current installed AuthMeReloaded version name. * - * @return The version name of the currently installed AuthMeReloaded - * instance. */ + * instance. + */ public static String getVersionName() { return PLUGIN_VERSION_NAME; } @@ -1022,9 +946,9 @@ public class AuthMe extends JavaPlugin { /** * Get the current installed AuthMeReloaded version code. * - * @return The version code of the currently installed AuthMeReloaded - * instance. */ + * instance. + */ public static int getVersionCode() { return PLUGIN_VERSION_CODE; } diff --git a/src/main/java/fr/xephi/authme/Log4JFilter.java b/src/main/java/fr/xephi/authme/Log4JFilter.java index 5eebc7cb7..917a75369 100644 --- a/src/main/java/fr/xephi/authme/Log4JFilter.java +++ b/src/main/java/fr/xephi/authme/Log4JFilter.java @@ -24,12 +24,6 @@ public class Log4JFilter implements org.apache.logging.log4j.core.Filter { public Log4JFilter() { } - /** - * Method filter. - * @param record LogEvent - - - * @return Result * @see org.apache.logging.log4j.core.Filter#filter(LogEvent) */ @Override public Result filter(LogEvent record) { if (record == null) { @@ -38,32 +32,12 @@ public class Log4JFilter implements org.apache.logging.log4j.core.Filter { return validateMessage(record.getMessage()); } - /** - * Method filter. - * @param arg0 Logger - * @param arg1 Level - * @param arg2 Marker - * @param message String - * @param arg4 Object[] - - - * @return Result * @see org.apache.logging.log4j.core.Filter#filter(Logger, Level, Marker, String, Object[]) */ @Override public Result filter(Logger arg0, Level arg1, Marker arg2, String message, Object... arg4) { return validateMessage(message); } - /** - * Method filter. - * @param arg0 Logger - * @param arg1 Level - * @param arg2 Marker - * @param message Object - * @param arg4 Throwable - - - * @return Result * @see org.apache.logging.log4j.core.Filter#filter(Logger, Level, Marker, Object, Throwable) */ @Override public Result filter(Logger arg0, Level arg1, Marker arg2, Object message, Throwable arg4) { @@ -73,37 +47,17 @@ public class Log4JFilter implements org.apache.logging.log4j.core.Filter { return validateMessage(message.toString()); } - /** - * Method filter. - * @param arg0 Logger - * @param arg1 Level - * @param arg2 Marker - * @param message Message - * @param arg4 Throwable - - - * @return Result * @see org.apache.logging.log4j.core.Filter#filter(Logger, Level, Marker, Message, Throwable) */ @Override public Result filter(Logger arg0, Level arg1, Marker arg2, Message message, Throwable arg4) { return validateMessage(message); } - /** - * Method getOnMatch. - - - * @return Result * @see org.apache.logging.log4j.core.Filter#getOnMatch() */ @Override public Result getOnMatch() { return Result.NEUTRAL; } - /** - * Method getOnMismatch. - - - * @return Result * @see org.apache.logging.log4j.core.Filter#getOnMismatch() */ @Override public Result getOnMismatch() { return Result.NEUTRAL; @@ -115,8 +69,9 @@ public class Log4JFilter implements org.apache.logging.log4j.core.Filter { * data. * * @param message the Message object to verify - - * @return the Result value */ + * + * @return the Result value + */ private static Result validateMessage(Message message) { if (message == null) { return Result.NEUTRAL; @@ -129,8 +84,9 @@ public class Log4JFilter implements org.apache.logging.log4j.core.Filter { * depending on whether the message contains sensitive AuthMe data. * * @param message the message to verify - - * @return the Result value */ + * + * @return the Result value + */ private static Result validateMessage(String message) { if (message == null) { return Result.NEUTRAL; From 9a68aa5517afa61efdaec465808e219c1ccaf24d Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 21 Nov 2015 08:28:53 +0100 Subject: [PATCH 2/2] Proper Javadoc example / add test for StringUtils - Proper example for the purpose of javadoc and how it could look like - Fix containsAny to be null safe - Add tests --- .../fr/xephi/authme/util/StringUtils.java | 26 ++++++----- .../fr/xephi/authme/util/StringUtilsTest.java | 46 +++++++++++++++++++ 2 files changed, 61 insertions(+), 11 deletions(-) create mode 100644 src/test/java/fr/xephi/authme/util/StringUtilsTest.java diff --git a/src/main/java/fr/xephi/authme/util/StringUtils.java b/src/main/java/fr/xephi/authme/util/StringUtils.java index c84fc74a5..e9b495bcd 100644 --- a/src/main/java/fr/xephi/authme/util/StringUtils.java +++ b/src/main/java/fr/xephi/authme/util/StringUtils.java @@ -5,17 +5,18 @@ import net.ricecode.similarity.StringSimilarityService; import net.ricecode.similarity.StringSimilarityServiceImpl; /** + * Utility class for String operations. */ public class StringUtils { /** * Get the difference of two strings. * - * @param first First string. - * @param second Second string. + * @param first First string + * @param second Second string * - - * @return The difference value. */ + * @return The difference value + */ public static double getDifference(String first, String second) { // Make sure the strings are valid. if(first == null || second == null) @@ -27,22 +28,25 @@ public class StringUtils { // Determine the difference value, return the result return Math.abs(service.score(first, second) - 1.0); } - + /** - * Method containsAny. - * @param str String - * @param pieces String[] - - * @return boolean */ + * Returns whether the given string contains any of the provided elements. + * + * @param str the string to analyze + * @param pieces the items to check the string for + * + * @return true if the string contains at least one of the items + */ public static boolean containsAny(String str, String... pieces) { if (str == null) { return false; } for (String piece : pieces) { - if (str.contains(piece)) { + if (piece != null && str.contains(piece)) { return true; } } return false; } + } diff --git a/src/test/java/fr/xephi/authme/util/StringUtilsTest.java b/src/test/java/fr/xephi/authme/util/StringUtilsTest.java new file mode 100644 index 000000000..668f2059b --- /dev/null +++ b/src/test/java/fr/xephi/authme/util/StringUtilsTest.java @@ -0,0 +1,46 @@ +package fr.xephi.authme.util; + +import org.junit.Test; + +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; + +/** + * Test for {@link StringUtils}. + */ +public class StringUtilsTest { + + @Test + public void shouldFindContainedItem() { + // given + String text = "This is a test of containsAny()"; + String piece = "test"; + + // when + boolean result = StringUtils.containsAny(text, "some", "words", "that", "do not", "exist", piece); + + // then + assertThat(result, equalTo(true)); + } + + @Test + public void shouldReturnFalseIfNoneFound() { + // given + String text = "This is a test string"; + + // when + boolean result = StringUtils.containsAny(text, "some", "other", "words", null); + + // then + assertThat(result, equalTo(false)); + } + + @Test + public void shouldReturnFalseForNullString() { + // given/when + boolean result = StringUtils.containsAny(null, "some", "words", "to", "check"); + + // then + assertThat(result, equalTo(false)); + } +}