Skip to content

Commit

Permalink
Merge pull request #4954 from Konicai/feature/configurate
Browse files Browse the repository at this point in the history
Improve node ordering when updating configs
  • Loading branch information
Camotoy authored Aug 21, 2024
2 parents f7a677e + e506c14 commit 85d63de
Show file tree
Hide file tree
Showing 4 changed files with 196 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ private boolean loadConfig() {
int interval = pingPassthroughInterval.getInt();
if (interval < 15 && ViaProxy.getConfig().getTargetVersion() != null && ViaProxy.getConfig().getTargetVersion().olderThanOrEqualTo(LegacyProtocolVersion.r1_6_4)) {
// <= 1.6.4 servers sometimes block incoming connections from an IP address if too many connections are made
node.set(15);
pingPassthroughInterval.set(15);
}
} catch (SerializationException e) {
throw new RuntimeException(e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.spongepowered.configurate.CommentedConfigurationNode;
import org.spongepowered.configurate.interfaces.InterfaceDefaultOptions;
import org.spongepowered.configurate.transformation.ConfigurationTransformation;
import org.spongepowered.configurate.yaml.NodeStyle;
import org.spongepowered.configurate.yaml.YamlConfigurationLoader;

import java.io.File;
Expand Down Expand Up @@ -61,13 +62,16 @@ public static <T extends GeyserConfig> T load(File file, Class<T> configClass) t
public static <T extends GeyserConfig> T load(File file, Class<T> configClass, @Nullable Consumer<CommentedConfigurationNode> transformer) throws IOException {
var loader = YamlConfigurationLoader.builder()
.file(file)
.defaultOptions(options -> InterfaceDefaultOptions.addTo(options
.indent(2)
.nodeStyle(NodeStyle.BLOCK)
.defaultOptions(options -> InterfaceDefaultOptions.addTo(options)
.shouldCopyDefaults(false) // If we use ConfigurationNode#get(type, default), do not write the default back to the node.
.header(HEADER)
.serializers(builder -> builder.register(new LowercaseEnumSerializer()))))
.serializers(builder -> builder.register(new LowercaseEnumSerializer())))
.build();
var node = loader.load();
// temp fix for node.virtual() being broken
boolean virtual = file.exists();

CommentedConfigurationNode node = loader.load();
boolean originallyEmpty = !file.exists() || node.isNull();

// Note for Tim? Needed or else Configurate breaks.
var migrations = ConfigurationTransformation.versionedBuilder()
Expand Down Expand Up @@ -120,14 +124,31 @@ public static <T extends GeyserConfig> T load(File file, Class<T> configClass, @

T config = node.get(configClass);

if (virtual || currentVersion != newVersion) {
loader.save(node);
// Serialize the instance to ensure strict field ordering. Additionally, if we serialized back
// to the old node, existing nodes would only have their value changed, keeping their position
// at the top of the ordered map, forcing all new nodes to the bottom (regardless of field order).
// For that reason, we must also create a new node.
CommentedConfigurationNode newRoot = CommentedConfigurationNode.root(loader.defaultOptions());
newRoot.set(config);

if (originallyEmpty || currentVersion != newVersion) {

if (!originallyEmpty && currentVersion > 4) {
// Only copy comments over if the file already existed, and we are going to replace it

// Second case: Version 4 is pre-configurate where there were commented out nodes.
// These get treated as comments on lower nodes, which produces very undesirable results.

ConfigurationCommentMover.moveComments(node, newRoot);
}

loader.save(newRoot);
}

if (transformer != null) {
// Do not let the transformer change the node.
transformer.accept(node);
config = node.get(configClass);
// We transform AFTER saving so that these specific transformations aren't applied to file.
transformer.accept(newRoot);
config = newRoot.get(configClass);
}

return config;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/*
* Copyright (c) 2024 GeyserMC. http://geysermc.org
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*
* @author GeyserMC
* @link https://github.com/GeyserMC/Geyser
*/

package org.geysermc.geyser.configuration;

import org.checkerframework.checker.nullness.qual.NonNull;
import org.spongepowered.configurate.CommentedConfigurationNode;
import org.spongepowered.configurate.ConfigurationNode;
import org.spongepowered.configurate.ConfigurationVisitor;

/**
* Moves comments from a different node and puts them on this node
*/
public final class ConfigurationCommentMover implements ConfigurationVisitor.Stateless<RuntimeException> {

private final CommentedConfigurationNode otherRoot;

private ConfigurationCommentMover(@NonNull CommentedConfigurationNode otherNode) {
this.otherRoot = otherNode;
}

@Override
public void enterNode(final ConfigurationNode node) {
if (!(node instanceof CommentedConfigurationNode destination)) {
// Should not occur because all nodes in a tree are the same type,
// and our static method below ensures this visitor is only used on CommentedConfigurationNodes
throw new IllegalStateException(node.path() + " is not a CommentedConfigurationNode");
}
// Node with the same path
CommentedConfigurationNode source = otherRoot.node(node.path());

moveSingle(source, destination);
}

private static void moveSingle(@NonNull CommentedConfigurationNode source, @NonNull CommentedConfigurationNode destination) {
// Only transfer the comment, overriding if necessary
String comment = source.comment();
if (comment != null) {
destination.comment(comment);
}
}

/**
* Moves comments from a source node and its children to a destination node and its children (of a different tree), overriding if necessary.
* Comments are only moved to the destination node and its children which exist.
* Comments are only moved to and from nodes with the exact same path.
*
* @param source the source of the comments, which must be the topmost parent of a tree
* @param destination the destination of the comments, any node in a different tree
*/
public static void moveComments(@NonNull CommentedConfigurationNode source, @NonNull CommentedConfigurationNode destination) {
if (source.parent() != null) {
throw new IllegalArgumentException("source is not the base of the tree it is within: " + source.path());
}

if (source.isNull()) {
// It has no value(s), but may still have a comment on it. Don't both traversing the whole destination tree.
moveSingle(source, destination);
} else {
destination.visit(new ConfigurationCommentMover(source));
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/*
* Copyright (c) 2024 GeyserMC. http://geysermc.org
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*
* @author GeyserMC
* @link https://github.com/GeyserMC/Geyser
*/

package org.geysermc.geyser.configuration;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.spongepowered.configurate.CommentedConfigurationNode;
import org.spongepowered.configurate.util.CheckedConsumer;

import java.io.File;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class ConfigLoaderTest {

@TempDir
Path tempDirectory;

// Hack until improving ConfigLoader
CommentedConfigurationNode config1;
CommentedConfigurationNode config2;

@Test
void testCreateNewConfig() throws Exception {
// Test that the result of generating a config, and the result of reading it back after writing it, is the same

File file = tempDirectory.resolve("config.yml").toFile();

forAllConfigs(type -> {
ConfigLoader.load(file, type, n -> this.config1 = n.copy());
long initialModification = file.lastModified();
assertTrue(file.exists()); // should have been created
List<String> firstContents = Files.readAllLines(file.toPath());

ConfigLoader.load(file, type, n -> this.config2 = n.copy());
List<String> secondContents = Files.readAllLines(file.toPath());

assertEquals(initialModification, file.lastModified()); // should not have been touched
assertEquals(firstContents, secondContents);

// Must ignore this, as when the config is read back, the header is interpreted as a comment on the first node in the map
config1.node("java").comment(null);
config2.node("java").comment(null);
assertEquals(config1, config2);
});
}

void forAllConfigs(CheckedConsumer<Class<? extends GeyserConfig>, Exception> consumer) throws Exception {
consumer.accept(GeyserRemoteConfig.class);
consumer.accept(GeyserPluginConfig.class);
}
}

0 comments on commit 85d63de

Please sign in to comment.