From 793477b898bec18e4b9a506e2ab441bca57cbb6d Mon Sep 17 00:00:00 2001 From: Mark Vainomaa Date: Thu, 25 Oct 2018 12:37:37 +0300 Subject: [PATCH 1/7] Improve code style --- .../proxy/config/VelocityConfiguration.java | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java b/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java index c15824dfe..d3074fcfc 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java @@ -34,7 +34,8 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi @Comment({ "What should we display for the maximum number of players? (Velocity does not support a cap", - "on the number of players online.)"}) + "on the number of players online.)" + }) @ConfigKey("show-max-players") private int showMaxPlayers = 500; @@ -50,7 +51,8 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi "- \"legacy\": Forward player IPs and UUIDs in BungeeCord-compatible fashion. Use this if you run", " servers using Minecraft 1.12 or lower.", "- \"modern\": Forward player IPs and UUIDs as part of the login process using Velocity's native", - " forwarding. Only applicable for Minecraft 1.13 or higher."}) + " forwarding. Only applicable for Minecraft 1.13 or higher." + }) @ConfigKey("player-info-forwarding-mode") private PlayerInfoForwarding playerInfoForwardingMode = PlayerInfoForwarding.NONE; @@ -77,6 +79,7 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi @Ignore private Component motdAsComponent; + @Ignore private Favicon favicon; @@ -389,7 +392,6 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi } private static class Servers { - @IsMap @Comment("Configure your servers here.") private Map servers = ImmutableMap.of("lobby", "127.0.0.1:30066", "factions", "127.0.0.1:30067", "minigames", "127.0.0.1:30068"); @@ -398,8 +400,7 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi @ConfigKey("try") private List attemptConnectionOrder = Arrays.asList("lobby"); - private Servers() { - } + private Servers() {} private Servers(Toml toml) { if (toml != null) { @@ -446,7 +447,6 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi + ", attemptConnectionOrder=" + attemptConnectionOrder + '}'; } - } private static class ForcedHosts { @@ -505,32 +505,37 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi } private static class Advanced { - @Comment({ "How large a Minecraft packet has to be before we compress it. Setting this to zero will compress all packets, and", - "setting it to -1 will disable compression entirely."}) + "setting it to -1 will disable compression entirely." + }) @ConfigKey("compression-threshold") private int compressionThreshold = 1024; + @Comment("How much compression should be done (from 0-9). The default is -1, which uses zlib's default level of 6.") @ConfigKey("compression-level") private int compressionLevel = -1; + @Comment({ "How fast (in miliseconds) are clients allowed to connect after the last connection? Default: 3000", - "Disable by setting to 0"}) + "Disable by setting to 0" + }) @ConfigKey("login-ratelimit") private int loginRatelimit = 3000; + @Comment({"Specify a custom timeout for connection timeouts here. The default is five seconds."}) @ConfigKey("connection-timeout") private int connectionTimeout = 5000; + @Comment({"Specify a read timeout for connections here. The default is 30 seconds."}) @ConfigKey("read-timeout") private int readTimeout = 30000; + @Comment("Enables compatibility with HAProxy.") @ConfigKey("proxy-protocol") private boolean proxyProtocol = false; - private Advanced() { - } + private Advanced() {} private Advanced(Toml toml) { if (toml != null) { @@ -581,13 +586,14 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi } private static class Query { - @Comment("Whether to enable responding to GameSpy 4 query responses or not") @ConfigKey("enabled") private boolean queryEnabled = false; + @Comment("If query responding is enabled, on what port should query response listener listen on?") @ConfigKey("port") private int queryPort = 25577; + @Comment("This is the map name that is reported to the query services.") @ConfigKey("map") private String queryMap = "Velocity"; @@ -596,8 +602,7 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi @ConfigKey("show-plugins") private boolean showPlugins = false; - private Query() { - } + private Query() {} private Query(boolean queryEnabled, int queryPort) { this.queryEnabled = queryEnabled; From fdc83261cb0010b2e228b44d5301ebe7a8cdb420 Mon Sep 17 00:00:00 2001 From: Mark Vainomaa Date: Thu, 25 Oct 2018 12:42:01 +0300 Subject: [PATCH 2/7] Add missing constructor parameter --- .../com/velocitypowered/proxy/config/VelocityConfiguration.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java b/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java index d3074fcfc..91bbe46bc 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java @@ -604,7 +604,7 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi private Query() {} - private Query(boolean queryEnabled, int queryPort) { + private Query(boolean queryEnabled, int queryPort, String queryMap) { this.queryEnabled = queryEnabled; this.queryPort = queryPort; this.queryMap = queryMap; From 85e43727cfc17fb1e910173665ef84b1d8b7c16d Mon Sep 17 00:00:00 2001 From: Mark Vainomaa Date: Thu, 25 Oct 2018 12:36:04 +0300 Subject: [PATCH 3/7] Improve code style, grammar and other fixes --- .../proxy/config/AnnotatedConfig.java | 142 +++++++++++------- 1 file changed, 86 insertions(+), 56 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/config/AnnotatedConfig.java b/proxy/src/main/java/com/velocitypowered/proxy/config/AnnotatedConfig.java index 55b87d405..61c967ead 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/config/AnnotatedConfig.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/config/AnnotatedConfig.java @@ -3,7 +3,6 @@ package com.velocitypowered.proxy.config; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import java.io.File; import java.io.IOException; import java.lang.annotation.ElementType; import java.lang.annotation.Retention; @@ -11,9 +10,12 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import java.lang.reflect.Field; import java.nio.charset.StandardCharsets; -import java.nio.file.*; +import java.nio.file.AtomicMoveNotSupportedException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; import java.util.ArrayList; -import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -35,7 +37,6 @@ public class AnnotatedConfig { @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.FIELD, ElementType.TYPE}) public @interface Table { - String value(); } @@ -45,7 +46,6 @@ public class AnnotatedConfig { @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.FIELD, ElementType.TYPE}) public @interface Comment { - String[] value(); } @@ -55,7 +55,6 @@ public class AnnotatedConfig { @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.FIELD, ElementType.TYPE}) public @interface ConfigKey { - String value(); } @@ -65,96 +64,125 @@ public class AnnotatedConfig { */ @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.FIELD, ElementType.TYPE}) - public @interface IsMap { - } + public @interface IsMap {} /** * Indicates that a field is a string converted to byte[] */ @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.FIELD, ElementType.TYPE}) - public @interface StringAsBytes { - } + public @interface StringAsBytes {} /** - * Indicates that a field should be skiped + * Indicates that a field should be skipped */ @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.FIELD}) - public @interface Ignore { - } + public @interface Ignore {} + /** + * Dumps this configuration to list of strings using {@link #dumpConfig(Object)} + * @return configuration dump + */ public List dumpConfig() { - List lines = new ArrayList<>(); - dumpFields(this, lines); - return lines; + return dumpConfig(this); } /** - * Dump all field and they annotations to List + * Creates TOML configuration from supplied 
dumpable
object. * - * @param toSave object those we need to dump - * @param lines a list where store dumped lines + * @param dumpable object which is going to be dumped + * @throws RuntimeException if reading field value(s) fail + * @return string list of configuration file lines */ - private void dumpFields(Object toSave, List lines) { - + private static List dumpConfig(Object dumpable) { + List lines = new ArrayList<>(); try { - for (Field field : toSave.getClass().getDeclaredFields()) { - if (field.getAnnotation(Ignore.class) != null) { //Skip this field + for (Field field : dumpable.getClass().getDeclaredFields()) { + // Skip fields with @Ignore annotation + if (field.getAnnotation(Ignore.class) != null) { continue; } + + // Make field accessible + field.setAccessible(true); + + // Add comments Comment comment = field.getAnnotation(Comment.class); - if (comment != null) { //Add comments + if (comment != null) { for (String line : comment.value()) { lines.add("# " + line); } } - ConfigKey key = field.getAnnotation(ConfigKey.class); //Get a key name for config - String name = key == null ? field.getName() : key.value(); // Use a field name if name in annotation is not present - field.setAccessible(true); // Make field accessible + + // Get a key name for config + ConfigKey key = field.getAnnotation(ConfigKey.class); + String name = key == null ? field.getName() : key.value(); // Use field name if @ConfigKey annotation is not present + + // Check if field is table. Table table = field.getAnnotation(Table.class); - if (table != null) { // Check if field is table. - lines.add(table.value()); // Write [name] - dumpFields(field.get(toSave), lines); // dump fields of table class - } else { - if (field.getAnnotation(IsMap.class) != null) { // check if field is map - Map map = (Map) field.get(toSave); - for (Entry entry : map.entrySet()) { - lines.add(safeKey(entry.getKey()) + " = " + toString(entry.getValue())); // Save a map data - } - lines.add(""); //Add empty line - continue; - } - Object value = field.get(toSave); - if (field.getAnnotation(StringAsBytes.class) != null) { // Check if field is a byte[] representation of a string - value = new String((byte[]) value, StandardCharsets.UTF_8); - } - lines.add(name + " = " + toString(value)); // save field to config - lines.add(""); // add empty line + if (table != null) { + lines.add(table.value()); // Write [name] + lines.addAll(dumpConfig(field.get(dumpable))); // dump fields of table + continue; } + + if (field.getAnnotation(IsMap.class) != null) { // check if field is map + @SuppressWarnings("unchecked") + Map map = (Map) field.get(dumpable); + for (Entry entry : map.entrySet()) { + lines.add(entry.getKey() + " = " + serialize(entry.getValue())); // Save map data + } + lines.add(""); //Add empty line + continue; + } + + Object value = field.get(dumpable); + + // Check if field is a byte[] representation of a string + if (field.getAnnotation(StringAsBytes.class) != null) { + value = new String((byte[]) value, StandardCharsets.UTF_8); + } + + // Save field to config + lines.add(name + " = " + serialize(value)); + lines.add(""); // add empty line } } catch (IllegalAccessException | IllegalArgumentException | SecurityException e) { - throw new RuntimeException("Can not dump config", e); + throw new RuntimeException("Could not dump configuration", e); } + + return lines; } - private String toString(Object value) { + /** + * Serializes 
value
so it could be parsed by TOML specification + * + * @param value object to serialize + * @return Serialized object + */ + private static String serialize(Object value) { if (value instanceof List) { - Collection listValue = (Collection) value; + List listValue = (List) value; if (listValue.isEmpty()) { return "[]"; } + StringBuilder m = new StringBuilder(); m.append("["); + for (Object obj : listValue) { - m.append(System.lineSeparator()).append(" ").append(toString(obj)).append(","); + m.append(System.lineSeparator()).append(" ").append(serialize(obj)).append(","); } + m.deleteCharAt(m.length() - 1).append(System.lineSeparator()).append("]"); return m.toString(); } + if (value instanceof Enum) { value = value.toString(); } + if (value instanceof String) { String stringValue = (String) value; if (stringValue.isEmpty()) { @@ -162,6 +190,7 @@ public class AnnotatedConfig { } return "\"" + stringValue.replace("\n", "\\n") + "\""; } + return value != null ? value.toString() : "null"; } @@ -173,23 +202,24 @@ public class AnnotatedConfig { } /** - * Saves lines to file + * Writes list of strings to file * - * @param lines Lines to save - * @param to A path of file where to save lines - * @throws IOException if lines is empty or was error during saving + * @param lines list of strings to write + * @param to Path of file where lines should be written + * @throws IOException if error occurred during writing + * @throws IllegalArgumentException if 
lines
is empty list */ public static void saveConfig(List lines, Path to) throws IOException { if (lines.isEmpty()) { - throw new IOException("Can not save config because list is empty"); + throw new IllegalArgumentException("lines cannot be empty"); } - Path temp = new File(to.toFile().getParent(), "__tmp").toPath(); + + Path temp = to.getParent().resolve(to.getFileName().toString() + "__tmp"); Files.write(temp, lines, StandardCharsets.UTF_8, StandardOpenOption.CREATE); try { Files.move(temp, to, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE); } catch (AtomicMoveNotSupportedException e) { Files.move(temp, to, StandardCopyOption.REPLACE_EXISTING); } - } } From 1e0ec8ad66d1807ad9744753a5ffe3f7293c7e34 Mon Sep 17 00:00:00 2001 From: Mark Vainomaa Date: Sat, 27 Oct 2018 00:35:16 +0300 Subject: [PATCH 4/7] Better description, make class abstract --- .../com/velocitypowered/proxy/config/AnnotatedConfig.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/config/AnnotatedConfig.java b/proxy/src/main/java/com/velocitypowered/proxy/config/AnnotatedConfig.java index 61c967ead..4cce31526 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/config/AnnotatedConfig.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/config/AnnotatedConfig.java @@ -21,10 +21,9 @@ import java.util.Map; import java.util.Map.Entry; /** - * Only for simple configs + * Simple annotation and fields based TOML configuration serializer */ -public class AnnotatedConfig { - +public abstract class AnnotatedConfig { private static final Logger logger = LogManager.getLogger(AnnotatedConfig.class); public static Logger getLogger() { From b86622b849af9b23c623a114f4fc2110e452529c Mon Sep 17 00:00:00 2001 From: Mark Vainomaa Date: Sat, 27 Oct 2018 00:39:13 +0300 Subject: [PATCH 5/7] Improve comments --- .../velocitypowered/proxy/config/AnnotatedConfig.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/config/AnnotatedConfig.java b/proxy/src/main/java/com/velocitypowered/proxy/config/AnnotatedConfig.java index 4cce31526..6c1dc1fd1 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/config/AnnotatedConfig.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/config/AnnotatedConfig.java @@ -121,18 +121,18 @@ public abstract class AnnotatedConfig { // Check if field is table. Table table = field.getAnnotation(Table.class); if (table != null) { - lines.add(table.value()); // Write [name] - lines.addAll(dumpConfig(field.get(dumpable))); // dump fields of table + lines.add(table.value()); // Write [name] + lines.addAll(dumpConfig(field.get(dumpable))); // Dump fields of table continue; } - if (field.getAnnotation(IsMap.class) != null) { // check if field is map + if (field.getAnnotation(IsMap.class) != null) { // Check if field is a map @SuppressWarnings("unchecked") Map map = (Map) field.get(dumpable); for (Entry entry : map.entrySet()) { lines.add(entry.getKey() + " = " + serialize(entry.getValue())); // Save map data } - lines.add(""); //Add empty line + lines.add(""); // Add empty line continue; } @@ -145,7 +145,7 @@ public abstract class AnnotatedConfig { // Save field to config lines.add(name + " = " + serialize(value)); - lines.add(""); // add empty line + lines.add(""); // Add empty line } } catch (IllegalAccessException | IllegalArgumentException | SecurityException e) { throw new RuntimeException("Could not dump configuration", e); From 2b4b77ef95df9782a9f375b28b3c565019716531 Mon Sep 17 00:00:00 2001 From: Mark Vainomaa Date: Sat, 27 Oct 2018 22:52:04 +0300 Subject: [PATCH 6/7] Fix missing constructor parameter and not loading show-plugins option from configuration --- .../velocitypowered/proxy/config/VelocityConfiguration.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java b/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java index 91bbe46bc..5bcec44c3 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java @@ -604,10 +604,11 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi private Query() {} - private Query(boolean queryEnabled, int queryPort, String queryMap) { + private Query(boolean queryEnabled, int queryPort, String queryMap, boolean showPlugins) { this.queryEnabled = queryEnabled; this.queryPort = queryPort; this.queryMap = queryMap; + this.showPlugins = showPlugins; } private Query(Toml toml) { @@ -615,6 +616,7 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi this.queryEnabled = toml.getBoolean("enabled", false); this.queryPort = toml.getLong("port", 25577L).intValue(); this.queryMap = toml.getString("map", "Velocity"); + this.showPlugins = toml.getBoolean("show-plugins", false); } } From 25f6ca410c8a6be7a18bd2eac5de6e04d4fdb339 Mon Sep 17 00:00:00 2001 From: Mark Vainomaa Date: Sat, 27 Oct 2018 22:54:10 +0300 Subject: [PATCH 7/7] Make servers list option more readable --- .../velocitypowered/proxy/config/VelocityConfiguration.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java b/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java index 5bcec44c3..5b935fd41 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java @@ -394,7 +394,11 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi private static class Servers { @IsMap @Comment("Configure your servers here.") - private Map servers = ImmutableMap.of("lobby", "127.0.0.1:30066", "factions", "127.0.0.1:30067", "minigames", "127.0.0.1:30068"); + private Map servers = ImmutableMap.of( + "lobby", "127.0.0.1:30066", + "factions", "127.0.0.1:30067", + "minigames", "127.0.0.1:30068" + ); @Comment("In what order we should try servers when a player logs in or is kicked from a server.") @ConfigKey("try")