From 0a7bd6c81a33cfaaa2f4d2456c6b237792f38fe6 Mon Sep 17 00:00:00 2001 From: DerFrZocker Date: Tue, 15 Oct 2024 21:05:19 +1100 Subject: [PATCH] #1493: Improve reroute performance and add some tests --- .../org/bukkit/craftbukkit/CraftServer.java | 2 + .../reroute/RequirePluginVersionData.java | 8 +- .../craftbukkit/legacy/reroute/Reroute.java | 120 ++++++++++ .../legacy/reroute/RerouteBuilder.java | 141 +++++++---- .../legacy/reroute/RerouteMethodData.java | 4 +- .../bukkit/craftbukkit/util/Commodore.java | 133 ++++------- .../craftbukkit/util/CraftMagicNumbers.java | 10 +- .../legacy/MaterialReroutingTest.java | 9 +- .../legacy/reroute/AbstractRerouteTest.java | 76 ++++++ .../legacy/reroute/DoNotRerouteTest.java | 23 ++ .../reroute/InjectCompatibilityTest.java | 55 +++++ .../legacy/reroute/InjectPluginNameTest.java | 55 +++++ .../reroute/InjectPluginVersionTest.java | 56 +++++ .../legacy/reroute/NotInBukkitTest.java | 41 ++++ .../reroute/RequireCompatibilityTest.java | 47 ++++ .../reroute/RequirePluginVersionDataTest.java | 121 ++++++++++ .../reroute/RequirePluginVersionTest.java | 42 ++++ .../reroute/RerouteArgumentTypeTest.java | 71 ++++++ .../legacy/reroute/RerouteBuilderTest.java | 221 ++++++++++++++++++ .../legacy/reroute/RerouteMethodNameTest.java | 41 ++++ .../legacy/reroute/RerouteReturnTypeTest.java | 41 ++++ .../legacy/reroute/RerouteStaticTest.java | 41 ++++ .../{ => reroute}/RerouteValidationTest.java | 27 ++- 23 files changed, 1229 insertions(+), 156 deletions(-) create mode 100644 src/main/java/org/bukkit/craftbukkit/legacy/reroute/Reroute.java create mode 100644 src/test/java/org/bukkit/craftbukkit/legacy/reroute/AbstractRerouteTest.java create mode 100644 src/test/java/org/bukkit/craftbukkit/legacy/reroute/DoNotRerouteTest.java create mode 100644 src/test/java/org/bukkit/craftbukkit/legacy/reroute/InjectCompatibilityTest.java create mode 100644 src/test/java/org/bukkit/craftbukkit/legacy/reroute/InjectPluginNameTest.java create mode 100644 src/test/java/org/bukkit/craftbukkit/legacy/reroute/InjectPluginVersionTest.java create mode 100644 src/test/java/org/bukkit/craftbukkit/legacy/reroute/NotInBukkitTest.java create mode 100644 src/test/java/org/bukkit/craftbukkit/legacy/reroute/RequireCompatibilityTest.java create mode 100644 src/test/java/org/bukkit/craftbukkit/legacy/reroute/RequirePluginVersionDataTest.java create mode 100644 src/test/java/org/bukkit/craftbukkit/legacy/reroute/RequirePluginVersionTest.java create mode 100644 src/test/java/org/bukkit/craftbukkit/legacy/reroute/RerouteArgumentTypeTest.java create mode 100644 src/test/java/org/bukkit/craftbukkit/legacy/reroute/RerouteBuilderTest.java create mode 100644 src/test/java/org/bukkit/craftbukkit/legacy/reroute/RerouteMethodNameTest.java create mode 100644 src/test/java/org/bukkit/craftbukkit/legacy/reroute/RerouteReturnTypeTest.java create mode 100644 src/test/java/org/bukkit/craftbukkit/legacy/reroute/RerouteStaticTest.java rename src/test/java/org/bukkit/craftbukkit/legacy/{ => reroute}/RerouteValidationTest.java (82%) diff --git a/src/main/java/org/bukkit/craftbukkit/CraftServer.java b/src/main/java/org/bukkit/craftbukkit/CraftServer.java index e16de43eb..abf3a2801 100644 --- a/src/main/java/org/bukkit/craftbukkit/CraftServer.java +++ b/src/main/java/org/bukkit/craftbukkit/CraftServer.java @@ -383,6 +383,7 @@ public final class CraftServer implements Server { minimumAPI = ApiVersion.getOrCreateVersion(configuration.getString("settings.minimum-api")); loadIcon(); loadCompatibilities(); + CraftMagicNumbers.INSTANCE.getCommodore().updateReroute(activeCompatibilities::contains); // Set map color cache if (configuration.getBoolean("settings.use-map-color-cache")) { @@ -930,6 +931,7 @@ public final class CraftServer implements Server { console.autosavePeriod = configuration.getInt("ticks-per.autosave"); loadIcon(); loadCompatibilities(); + CraftMagicNumbers.INSTANCE.getCommodore().updateReroute(activeCompatibilities::contains); try { playerList.getIpBans().load(); diff --git a/src/main/java/org/bukkit/craftbukkit/legacy/reroute/RequirePluginVersionData.java b/src/main/java/org/bukkit/craftbukkit/legacy/reroute/RequirePluginVersionData.java index 8f344e500..5cc95b2a7 100644 --- a/src/main/java/org/bukkit/craftbukkit/legacy/reroute/RequirePluginVersionData.java +++ b/src/main/java/org/bukkit/craftbukkit/legacy/reroute/RequirePluginVersionData.java @@ -7,7 +7,7 @@ public record RequirePluginVersionData(ApiVersion minInclusive, ApiVersion maxIn public static RequirePluginVersionData create(RequirePluginVersion requirePluginVersion) { if (!requirePluginVersion.value().isBlank()) { if (!requirePluginVersion.minInclusive().isBlank() || !requirePluginVersion.maxInclusive().isBlank()) { - throw new RuntimeException("When setting value, min inclusive and max inclusive data is not allowed."); + throw new IllegalArgumentException("When setting value, min inclusive and max inclusive data is not allowed."); } return new RequirePluginVersionData(ApiVersion.getOrCreateVersion(requirePluginVersion.value()), ApiVersion.getOrCreateVersion(requirePluginVersion.value())); @@ -23,6 +23,12 @@ public record RequirePluginVersionData(ApiVersion minInclusive, ApiVersion maxIn maxInclusive = ApiVersion.getOrCreateVersion(requirePluginVersion.maxInclusive()); } + if (minInclusive != null && maxInclusive != null) { + if (minInclusive.isNewerThan(maxInclusive)) { + throw new IllegalArgumentException("Min inclusive cannot be newer than max inclusive."); + } + } + return new RequirePluginVersionData(minInclusive, maxInclusive); } diff --git a/src/main/java/org/bukkit/craftbukkit/legacy/reroute/Reroute.java b/src/main/java/org/bukkit/craftbukkit/legacy/reroute/Reroute.java new file mode 100644 index 000000000..1f828c37d --- /dev/null +++ b/src/main/java/org/bukkit/craftbukkit/legacy/reroute/Reroute.java @@ -0,0 +1,120 @@ +package org.bukkit.craftbukkit.legacy.reroute; + +import java.util.HashMap; +import java.util.Map; +import java.util.function.Consumer; +import org.bukkit.craftbukkit.util.ApiVersion; +import org.bukkit.craftbukkit.util.ClassTraverser; +import org.jetbrains.annotations.VisibleForTesting; +import org.objectweb.asm.Type; + +public class Reroute { + + @VisibleForTesting + final Map rerouteDataMap; + + Reroute(Map rerouteDataMap) { + this.rerouteDataMap = rerouteDataMap; + } + + /* + This method looks (and probably is) overengineered, but it gives the most flexible when it comes to remapping normal methods to static one. + The problem with normal owner and desc replacement is that child classes have them as an owner, instead there parents for there parents methods + + For example, if we have following two interfaces org.bukkit.BlockData and org.bukkit.Orientable extents BlockData + and BlockData has the method org.bukkit.Material getType which we want to reroute to the static method + org.bukkit.Material org.bukkit.craftbukkit.legacy.EnumEvil#getType(org.bukkit.BlockData) + + If we now call BlockData#getType we get as the owner org/bukkit/BlockData and as desc ()Lorg/bukkit/Material; + Which we can nicely reroute by checking if the owner is BlockData and the name getType + The problem, starts if we use Orientable#getType no we get as owner org/bukkit/Orientable and as desc ()Lorg/bukkit/Material; + Now we can now longer safely say to which getType method we need to reroute (assume there are multiple getType methods from different classes, + which are not related to BlockData), simple using the owner class will not work, since would reroute to + EnumEvil#getType(org.bukkit.Orientable) which is not EnumEvil#getType(org.bukkit.BlockData) and will throw a method not found error + at runtime. + + Meaning we would need to add checks for each subclass, which would be pur insanity. + + To solve this, we go through each super class and interfaces (and their super class and interfaces etc.) and try to get an owner + which matches with one of our replacement methods. Based on how inheritance works in java, this method should be safe to use. + + As a site note: This method could also be used for the other method reroute, e.g. legacy method rerouting, where only the replacement + method needs to be written, and this method figures out the rest, which could reduce the size and complexity of the Commodore class. + The question then becomes one about performance (since this is not the most performance way) and convenience. + But since it is only applied for each class and method call once when they get first loaded, it should not be that bad. + (Although some load time testing could be done) + */ + public boolean apply(ApiVersion pluginVersion, String owner, String name, String desc, boolean staticCall, Consumer consumer) { + RerouteDataHolder rerouteData = rerouteDataMap.get(desc + name); + if (rerouteData == null) { + return false; + } + + Type ownerType = Type.getObjectType(owner); + RerouteMethodData data = rerouteData.get(ownerType); + + if (staticCall && data == null) { + return false; + } + + if (data != null) { + if (data.requiredPluginVersion() != null && !data.requiredPluginVersion().test(pluginVersion)) { + return false; + } + + consumer.accept(data); + return true; + } + + Class ownerClass; + try { + ownerClass = Class.forName(ownerType.getClassName(), false, Reroute.class.getClassLoader()); + } catch (ClassNotFoundException e) { + return false; + } + + ClassTraverser it = new ClassTraverser(ownerClass); + while (it.hasNext()) { + Class clazz = it.next(); + + data = rerouteData.get(Type.getType(clazz)); + + if (data == null) { + continue; + } + + if (data.requiredPluginVersion() != null && !data.requiredPluginVersion().test(pluginVersion)) { + return false; + } + + consumer.accept(data); + return true; + } + + return false; + } + + static class RerouteDataHolder { + + @VisibleForTesting + final Map rerouteMethodDataMap = new HashMap<>(); + + public RerouteMethodData get(Class clazz) { + return rerouteMethodDataMap.get(Type.getInternalName(clazz)); + } + + private RerouteMethodData get(Type owner) { + return rerouteMethodDataMap.get(owner.getInternalName()); + } + + void add(RerouteMethodData value) { + RerouteMethodData rerouteMethodData = get(value.sourceOwner()); + + if (rerouteMethodData != null) { + throw new IllegalStateException("Reroute method data already exists: " + rerouteMethodData); + } + + rerouteMethodDataMap.put(value.sourceOwner().getInternalName(), value); + } + } +} diff --git a/src/main/java/org/bukkit/craftbukkit/legacy/reroute/RerouteBuilder.java b/src/main/java/org/bukkit/craftbukkit/legacy/reroute/RerouteBuilder.java index c4ff44aec..435672a3a 100644 --- a/src/main/java/org/bukkit/craftbukkit/legacy/reroute/RerouteBuilder.java +++ b/src/main/java/org/bukkit/craftbukkit/legacy/reroute/RerouteBuilder.java @@ -1,53 +1,69 @@ package org.bukkit.craftbukkit.legacy.reroute; import com.google.common.base.Preconditions; +import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.lang.reflect.Parameter; import java.util.ArrayList; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.function.Predicate; import org.bukkit.craftbukkit.util.ApiVersion; import org.objectweb.asm.Type; public class RerouteBuilder { - public static Map buildFromClass(Class clazz) { - Preconditions.checkArgument(!clazz.isInterface(), "Interface Classes are currently not supported"); + private final List> classes = new ArrayList<>(); + private final Predicate compatibilityPresent; - Map result = new HashMap<>(); - - for (Method method : clazz.getDeclaredMethods()) { - if (method.isBridge()) { - continue; - } - - if (method.isSynthetic()) { - continue; - } - - if (!Modifier.isPublic(method.getModifiers())) { - continue; - } - - if (!Modifier.isStatic(method.getModifiers())) { - continue; - } - - if (method.isAnnotationPresent(DoNotReroute.class)) { - continue; - } - - RerouteMethodData rerouteMethodData = buildFromMethod(method); - result.put(rerouteMethodData.source(), rerouteMethodData); - } - - return Collections.unmodifiableMap(result); + private RerouteBuilder(Predicate compatibilityPresent) { + this.compatibilityPresent = compatibilityPresent; } - public static RerouteMethodData buildFromMethod(Method method) { + public static RerouteBuilder create(Predicate compatibilityPresent) { + return new RerouteBuilder(compatibilityPresent); + } + + public RerouteBuilder forClass(Class clazz) { + classes.add(clazz); + return this; + } + + public Reroute build() { + Map rerouteDataHolderMap = new HashMap<>(); + + for (Class clazz : classes) { + List data = buildFromClass(clazz, compatibilityPresent); + data.forEach(value -> rerouteDataHolderMap.computeIfAbsent(value.methodKey(), v -> new Reroute.RerouteDataHolder()).add(value)); + } + + return new Reroute(rerouteDataHolderMap); + } + + private static List buildFromClass(Class clazz, Predicate compatibilityPresent) { + Preconditions.checkArgument(!clazz.isInterface(), "Interface Classes are currently not supported"); + + List result = new ArrayList<>(); + boolean shouldInclude = shouldInclude(getRequireCompatibility(clazz), true, compatibilityPresent); + + for (Method method : clazz.getDeclaredMethods()) { + if (!isMethodValid(method)) { + continue; + } + + if (!shouldInclude(getRequireCompatibility(method), shouldInclude, compatibilityPresent)) { + continue; + } + + result.add(buildFromMethod(method)); + } + + return result; + } + + private static RerouteMethodData buildFromMethod(Method method) { RerouteReturn rerouteReturn = new RerouteReturn(Type.getReturnType(method)); List arguments = new ArrayList<>(); List sourceArguments = new ArrayList<>(); @@ -113,7 +129,10 @@ public class RerouteBuilder { if (rerouteStatic != null) { sourceOwner = Type.getObjectType(rerouteStatic.value()); } else { - RerouteArgument argument = sourceArguments.get(0); + if (sourceArguments.isEmpty()) { + throw new RuntimeException("Source argument list is empty, no owner class found"); + } + RerouteArgument argument = sourceArguments.getFirst(); sourceOwner = argument.sourceType(); sourceArguments.remove(argument); } @@ -135,23 +154,12 @@ public class RerouteBuilder { methodName = method.getName(); } - String methodKey = sourceOwner.getInternalName() - + " " - + sourceDesc.getDescriptor() - + " " - + methodName; + String methodKey = sourceDesc.getDescriptor() + methodName; Type targetType = Type.getType(method); boolean inBukkit = !method.isAnnotationPresent(NotInBukkit.class) && !method.getDeclaringClass().isAnnotationPresent(NotInBukkit.class); - String requiredCompatibility = null; - if (method.isAnnotationPresent(RequireCompatibility.class)) { - requiredCompatibility = method.getAnnotation(RequireCompatibility.class).value(); - } else if (method.getDeclaringClass().isAnnotationPresent(RequireCompatibility.class)) { - requiredCompatibility = method.getDeclaringClass().getAnnotation(RequireCompatibility.class).value(); - } - RequirePluginVersionData requiredPluginVersion = null; if (method.isAnnotationPresent(RequirePluginVersion.class)) { requiredPluginVersion = RequirePluginVersionData.create(method.getAnnotation(RequirePluginVersion.class)); @@ -159,6 +167,47 @@ public class RerouteBuilder { requiredPluginVersion = RequirePluginVersionData.create(method.getDeclaringClass().getAnnotation(RequirePluginVersion.class)); } - return new RerouteMethodData(methodKey, sourceDesc, sourceOwner, methodName, rerouteStatic != null, targetType, Type.getInternalName(method.getDeclaringClass()), method.getName(), arguments, rerouteReturn, inBukkit, requiredCompatibility, requiredPluginVersion); + return new RerouteMethodData(methodKey, sourceDesc, sourceOwner, methodName, rerouteStatic != null, targetType, Type.getInternalName(method.getDeclaringClass()), method.getName(), arguments, rerouteReturn, inBukkit, requiredPluginVersion); + } + + private static boolean isMethodValid(Method method) { + if (method.isBridge()) { + return false; + } + + if (method.isSynthetic()) { + return false; + } + + if (!Modifier.isPublic(method.getModifiers())) { + return false; + } + + if (!Modifier.isStatic(method.getModifiers())) { + return false; + } + + if (method.isAnnotationPresent(DoNotReroute.class)) { + return false; + } + + return true; + } + + private static String getRequireCompatibility(AnnotatedElement element) { + RequireCompatibility annotation = element.getAnnotation(RequireCompatibility.class); + if (annotation == null) { + return null; + } + + return annotation.value(); + } + + private static boolean shouldInclude(String string, boolean parent, Predicate compatibilityPresent) { + if (string == null) { + return parent; + } + + return compatibilityPresent.test(string); } } diff --git a/src/main/java/org/bukkit/craftbukkit/legacy/reroute/RerouteMethodData.java b/src/main/java/org/bukkit/craftbukkit/legacy/reroute/RerouteMethodData.java index 6b6bf143a..cf94b8054 100644 --- a/src/main/java/org/bukkit/craftbukkit/legacy/reroute/RerouteMethodData.java +++ b/src/main/java/org/bukkit/craftbukkit/legacy/reroute/RerouteMethodData.java @@ -4,8 +4,8 @@ import java.util.List; import org.jetbrains.annotations.Nullable; import org.objectweb.asm.Type; -public record RerouteMethodData(String source, Type sourceDesc, Type sourceOwner, String sourceName, +public record RerouteMethodData(String methodKey, Type sourceDesc, Type sourceOwner, String sourceName, boolean staticReroute, Type targetType, String targetOwner, String targetName, List arguments, RerouteReturn rerouteReturn, boolean isInBukkit, - @Nullable String requiredCompatibility, @Nullable RequirePluginVersionData requiredPluginVersion) { + @Nullable RequirePluginVersionData requiredPluginVersion) { } diff --git a/src/main/java/org/bukkit/craftbukkit/util/Commodore.java b/src/main/java/org/bukkit/craftbukkit/util/Commodore.java index 54a2600e3..7d1feb489 100644 --- a/src/main/java/org/bukkit/craftbukkit/util/Commodore.java +++ b/src/main/java/org/bukkit/craftbukkit/util/Commodore.java @@ -1,5 +1,6 @@ package org.bukkit.craftbukkit.util; +import com.google.common.base.Predicates; import com.google.common.collect.HashMultimap; import com.google.common.collect.Multimap; import com.google.common.io.ByteStreams; @@ -16,6 +17,7 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.function.Consumer; +import java.util.function.Predicate; import java.util.jar.JarEntry; import java.util.jar.JarFile; import java.util.jar.JarOutputStream; @@ -28,6 +30,7 @@ import org.bukkit.craftbukkit.legacy.FieldRename; import org.bukkit.craftbukkit.legacy.MaterialRerouting; import org.bukkit.craftbukkit.legacy.MethodRerouting; import org.bukkit.craftbukkit.legacy.enums.EnumEvil; +import org.bukkit.craftbukkit.legacy.reroute.Reroute; import org.bukkit.craftbukkit.legacy.reroute.RerouteArgument; import org.bukkit.craftbukkit.legacy.reroute.RerouteBuilder; import org.bukkit.craftbukkit.legacy.reroute.RerouteMethodData; @@ -87,18 +90,38 @@ public class Commodore { "org/bukkit/block/banner/PatternType", "NOP" ); - private static Map createReroutes(Class clazz) { - Map reroutes = RerouteBuilder.buildFromClass(clazz); - REROUTES.add(reroutes); - return reroutes; + private final List reroutes = new ArrayList<>(); // only for testing + private Reroute materialReroute; + private Reroute reroute; + + public Commodore() { + } + + public Commodore(Predicate compatibilityPresent) { + updateReroute(compatibilityPresent); + } + + public void updateReroute(Predicate compatibilityPresent) { + materialReroute = RerouteBuilder + .create(compatibilityPresent) + .forClass(MaterialRerouting.class) + .build(); + reroute = RerouteBuilder + .create(compatibilityPresent) + .forClass(FieldRename.class) + .forClass(MethodRerouting.class) + .forClass(EnumEvil.class) + .build(); + + reroutes.clear(); + reroutes.add(materialReroute); + reroutes.add(reroute); } @VisibleForTesting - public static final List> REROUTES = new ArrayList<>(); // Only used for testing - private static final Map FIELD_RENAME_METHOD_REROUTE = createReroutes(FieldRename.class); - private static final Map MATERIAL_METHOD_REROUTE = createReroutes(MaterialRerouting.class); - private static final Map METHOD_REROUTE = createReroutes(MethodRerouting.class); - private static final Map ENUM_METHOD_REROUTE = createReroutes(EnumEvil.class); + public List getReroutes() { + return reroutes; + } public static void main(String[] args) { OptionParser parser = new OptionParser(); @@ -110,6 +133,8 @@ public class Commodore { File input = options.valueOf(inputFlag); File output = options.valueOf(outputFlag); + Commodore commodore = new Commodore(Predicates.alwaysTrue()); + if (input.isDirectory()) { if (!output.isDirectory()) { System.err.println("If input directory specified, output directory required too"); @@ -118,15 +143,15 @@ public class Commodore { for (File in : input.listFiles()) { if (in.getName().endsWith(".jar")) { - convert(in, new File(output, in.getName())); + convert(in, new File(output, in.getName()), commodore); } } } else { - convert(input, output); + convert(input, output, commodore); } } - private static void convert(File in, File out) { + private static void convert(File in, File out, Commodore commodore) { System.out.println("Attempting to convert " + in + " to " + out); try { @@ -144,7 +169,7 @@ public class Commodore { byte[] b = ByteStreams.toByteArray(is); if (entry.getName().endsWith(".class")) { - b = convert(b, "dummy", ApiVersion.NONE, Collections.emptySet()); + b = commodore.convert(b, "dummy", ApiVersion.NONE, Collections.emptySet()); entry = new JarEntry(entry.getName()); } @@ -162,7 +187,7 @@ public class Commodore { } } - public static byte[] convert(byte[] b, final String pluginName, final ApiVersion pluginVersion, final Set activeCompatibilities) { + public byte[] convert(byte[] b, final String pluginName, final ApiVersion pluginVersion, final Set activeCompatibilities) { final boolean modern = pluginVersion.isNewerThanOrSameAs(ApiVersion.FLATTENING); final boolean enumCompatibility = pluginVersion.isOlderThanOrSameAs(ApiVersion.getOrCreateVersion("1.20.6")) && activeCompatibilities.contains("enum-compatibility-mode"); ClassReader cr = new ClassReader(b); @@ -329,10 +354,7 @@ public class Commodore { } private void handleMethod(MethodPrinter visitor, int opcode, String owner, String name, String desc, boolean itf, Type samMethodType, Type instantiatedMethodType) { - if (checkReroute(visitor, FIELD_RENAME_METHOD_REROUTE, opcode, owner, name, desc, samMethodType, instantiatedMethodType)) { - return; - } - if (checkReroute(visitor, METHOD_REROUTE, opcode, owner, name, desc, samMethodType, instantiatedMethodType)) { + if (checkReroute(visitor, reroute, opcode, owner, name, desc, samMethodType, instantiatedMethodType)) { return; } @@ -353,10 +375,6 @@ public class Commodore { } } - if (checkReroute(visitor, ENUM_METHOD_REROUTE, opcode, owner, name, desc, samMethodType, instantiatedMethodType)) { - return; - } - // SPIGOT-4496 if (owner.equals("org/bukkit/map/MapView") && name.equals("getId") && desc.equals("()S")) { // Should be same size on stack so just call other method @@ -448,15 +466,15 @@ public class Commodore { } // TODO 2024-05-21: Move this up, when material gets fully replaced with ItemType and BlockType - if (owner.startsWith("org/bukkit") && checkReroute(visitor, MATERIAL_METHOD_REROUTE, opcode, owner, name, desc, samMethodType, instantiatedMethodType)) { + if (owner.startsWith("org/bukkit") && checkReroute(visitor, materialReroute, opcode, owner, name, desc, samMethodType, instantiatedMethodType)) { return; } visitor.visit(opcode, owner, name, desc, itf, samMethodType, instantiatedMethodType); } - private boolean checkReroute(MethodPrinter visitor, Map rerouteMethodDataMap, int opcode, String owner, String name, String desc, Type samMethodType, Type instantiatedMethodType) { - return rerouteMethods(activeCompatibilities, pluginVersion, rerouteMethodDataMap, opcode == Opcodes.INVOKESTATIC || opcode == Opcodes.H_INVOKESTATIC, owner, name, desc, data -> { + private boolean checkReroute(MethodPrinter visitor, Reroute reroute, int opcode, String owner, String name, String desc, Type samMethodType, Type instantiatedMethodType) { + return rerouteMethods(pluginVersion, reroute, opcode == Opcodes.INVOKESTATIC || opcode == Opcodes.H_INVOKESTATIC, owner, name, desc, data -> { visitor.visit(Opcodes.INVOKESTATIC, className, buildMethodName(data), buildMethodDesc(data), isInterface, samMethodType, instantiatedMethodType); rerouteMethodData.add(data); }); @@ -599,69 +617,8 @@ public class Commodore { }; } - /* - This method looks (and probably is) overengineered, but it gives the most flexible when it comes to remapping normal methods to static one. - The problem with normal owner and desc replacement is that child classes have them as an owner, instead there parents for there parents methods - - For example, if we have following two interfaces org.bukkit.BlockData and org.bukkit.Orientable extents BlockData - and BlockData has the method org.bukkit.Material getType which we want to reroute to the static method - org.bukkit.Material org.bukkit.craftbukkit.legacy.EnumEvil#getType(org.bukkit.BlockData) - - If we now call BlockData#getType we get as the owner org/bukkit/BlockData and as desc ()Lorg/bukkit/Material; - Which we can nicely reroute by checking if the owner is BlockData and the name getType - The problem, starts if we use Orientable#getType no we get as owner org/bukkit/Orientable and as desc ()Lorg/bukkit/Material; - Now we can now longer safely say to which getType method we need to reroute (assume there are multiple getType methods from different classes, - which are not related to BlockData), simple using the owner class will not work, since would reroute to - EnumEvil#getType(org.bukkit.Orientable) which is not EnumEvil#getType(org.bukkit.BlockData) and will throw a method not found error - at runtime. - - Meaning we would need to add checks for each subclass, which would be pur insanity. - - To solve this, we go through each super class and interfaces (and their super class and interfaces etc.) and try to get an owner - which matches with one of our replacement methods. Based on how inheritance works in java, this method should be safe to use. - - As a site note: This method could also be used for the other method reroute, e.g. legacy method rerouting, where only the replacement - method needs to be written, and this method figures out the rest, which could reduce the size and complexity of the Commodore class. - The question then becomes one about performance (since this is not the most performance way) and convenience. - But since it is only applied for each class and method call once when they get first loaded, it should not be that bad. - (Although some load time testing could be done) - */ - public static boolean rerouteMethods(Set activeCompatibilities, ApiVersion pluginVersion, Map rerouteMethodDataMap, boolean staticCall, String owner, String name, String desc, Consumer consumer) { - Type ownerType = Type.getObjectType(owner); - Class ownerClass; - try { - ownerClass = Class.forName(ownerType.getClassName()); - } catch (ClassNotFoundException e) { - return false; - } - - ClassTraverser it = new ClassTraverser(ownerClass); - while (it.hasNext()) { - Class clazz = it.next(); - - String methodKey = Type.getInternalName(clazz) + " " + desc + " " + name; - - RerouteMethodData data = rerouteMethodDataMap.get(methodKey); - if (data == null) { - if (staticCall) { - return false; - } - continue; - } - - if (data.requiredCompatibility() != null && !activeCompatibilities.contains(data.requiredCompatibility())) { - return false; - } - - if (data.requiredPluginVersion() != null && !data.requiredPluginVersion().test(pluginVersion)) { - return false; - } - - consumer.accept(data); - return true; - } - - return false; + public static boolean rerouteMethods(ApiVersion pluginVersion, Reroute reroute, boolean staticCall, String owner, String name, String desc, Consumer consumer) { + return reroute.apply(pluginVersion, owner, name, desc, staticCall, consumer); } private static List getMethodSignatures(byte[] clazz) { diff --git a/src/main/java/org/bukkit/craftbukkit/util/CraftMagicNumbers.java b/src/main/java/org/bukkit/craftbukkit/util/CraftMagicNumbers.java index 80ab7eaf2..0ab6b0a5f 100644 --- a/src/main/java/org/bukkit/craftbukkit/util/CraftMagicNumbers.java +++ b/src/main/java/org/bukkit/craftbukkit/util/CraftMagicNumbers.java @@ -74,7 +74,9 @@ import org.bukkit.potion.PotionType; @SuppressWarnings("deprecation") public final class CraftMagicNumbers implements UnsafeValues { - public static final UnsafeValues INSTANCE = new CraftMagicNumbers(); + public static final CraftMagicNumbers INSTANCE = new CraftMagicNumbers(); + + private final Commodore commodore = new Commodore(); private CraftMagicNumbers() {} @@ -165,6 +167,10 @@ public final class CraftMagicNumbers implements UnsafeValues { return CraftLegacy.toLegacyData(data); } + public Commodore getCommodore() { + return this.commodore; + } + @Override public Material toLegacy(Material material) { return CraftLegacy.toLegacy(material); @@ -320,7 +326,7 @@ public final class CraftMagicNumbers implements UnsafeValues { @Override public byte[] processClass(PluginDescriptionFile pdf, String path, byte[] clazz) { try { - clazz = Commodore.convert(clazz, pdf.getName(), ApiVersion.getOrCreateVersion(pdf.getAPIVersion()), ((CraftServer) Bukkit.getServer()).activeCompatibilities); + clazz = commodore.convert(clazz, pdf.getName(), ApiVersion.getOrCreateVersion(pdf.getAPIVersion()), ((CraftServer) Bukkit.getServer()).activeCompatibilities); } catch (Exception ex) { Bukkit.getLogger().log(Level.SEVERE, "Fatal error trying to convert " + pdf.getFullName() + ":" + path, ex); } diff --git a/src/test/java/org/bukkit/craftbukkit/legacy/MaterialReroutingTest.java b/src/test/java/org/bukkit/craftbukkit/legacy/MaterialReroutingTest.java index f18ae913b..3eac642d6 100644 --- a/src/test/java/org/bukkit/craftbukkit/legacy/MaterialReroutingTest.java +++ b/src/test/java/org/bukkit/craftbukkit/legacy/MaterialReroutingTest.java @@ -2,20 +2,19 @@ package org.bukkit.craftbukkit.legacy; import static org.junit.jupiter.api.Assertions.*; import com.google.common.base.Joiner; +import com.google.common.base.Predicates; import java.io.File; import java.io.IOException; import java.io.InputStream; import java.net.URI; import java.net.URISyntaxException; import java.util.ArrayList; -import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.jar.JarFile; import java.util.stream.Stream; import org.bukkit.Bukkit; +import org.bukkit.craftbukkit.legacy.reroute.Reroute; import org.bukkit.craftbukkit.legacy.reroute.RerouteBuilder; -import org.bukkit.craftbukkit.legacy.reroute.RerouteMethodData; import org.bukkit.craftbukkit.util.ApiVersion; import org.bukkit.craftbukkit.util.Commodore; import org.bukkit.support.environment.Normal; @@ -34,7 +33,7 @@ public class MaterialReroutingTest { // Needs to be a bukkit class private static final URI BUKKIT_CLASSES; - private static final Map MATERIAL_METHOD_REROUTE = RerouteBuilder.buildFromClass(MaterialRerouting.class); + private static final Reroute MATERIAL_METHOD_REROUTE = RerouteBuilder.create(Predicates.alwaysTrue()).forClass(MaterialRerouting.class).build(); static { try { @@ -95,7 +94,7 @@ public class MaterialReroutingTest { } } - if (!Commodore.rerouteMethods(Collections.emptySet(), ApiVersion.CURRENT, MATERIAL_METHOD_REROUTE, (methodNode.access & Opcodes.ACC_STATIC) != 0, classNode.name, methodNode.name, methodNode.desc, a -> { })) { + if (!Commodore.rerouteMethods(ApiVersion.CURRENT, MATERIAL_METHOD_REROUTE, (methodNode.access & Opcodes.ACC_STATIC) != 0, classNode.name, methodNode.name, methodNode.desc, a -> { })) { missingReroute.add(methodNode.name + " " + methodNode.desc + " " + methodNode.signature); } } diff --git a/src/test/java/org/bukkit/craftbukkit/legacy/reroute/AbstractRerouteTest.java b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/AbstractRerouteTest.java new file mode 100644 index 000000000..46ec5a54d --- /dev/null +++ b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/AbstractRerouteTest.java @@ -0,0 +1,76 @@ +package org.bukkit.craftbukkit.legacy.reroute; + +import static org.junit.jupiter.api.Assertions.*; +import com.google.common.base.Predicate; +import com.google.common.base.Predicates; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import org.objectweb.asm.Type; + +public abstract class AbstractRerouteTest { + + void test(Class testClass, Map dataMap) { + test(testClass, Predicates.alwaysTrue(), dataMap); + } + + void test(Class testClass, Predicate predicate, Map dataMap) { + test(testClass, Predicates.and(predicate), dataMap.entrySet().stream().map(entry -> new TestDataHolder(entry.getKey(), List.of(entry.getValue()))).toList()); + } + + void test(Class testClass, Predicate predicate, List dataList) { + Reroute reroute = RerouteBuilder + .create(predicate) + .forClass(testClass) + .build(); + + assertEquals(dataList.size(), reroute.rerouteDataMap.size(), String.format("First layer reroute data amount are not the same. Expected: %s, Actual: %s", dataList, reroute.rerouteDataMap)); + + for (TestDataHolder testHolder : dataList) { + Reroute.RerouteDataHolder holder = reroute.rerouteDataMap.get(testHolder.methodKey()); + assertEquals(testHolder.rerouteMethodDataList().size(), holder.rerouteMethodDataMap.size(), String.format("Second layer reroute data amount are not the same. Expected: %s, Actual: %s", testHolder.rerouteMethodDataList, holder.rerouteMethodDataMap)); + + for (RerouteMethodData testData : testHolder.rerouteMethodDataList()) { + RerouteMethodData actual = holder.rerouteMethodDataMap.get(testData.sourceOwner().getInternalName()); + assertNotNull(actual, String.format("No reroute method data found for %s", testData)); + + check(actual, testData); + } + } + } + + RerouteArgument create(String type, String sourceType, boolean injectPluginName, boolean injectPluginVersion, String injectCompatibility) { + return new RerouteArgument(Type.getType(type), Type.getType(sourceType), injectPluginName, injectPluginVersion, injectCompatibility); + } + + List create(RerouteArgument... arguments) { + return Arrays.asList(arguments); + } + + RerouteMethodData create(String methodKey, String sourceDesc, String sourceOwner, String sourceName, + boolean staticReroute, String targetType, String targetOwner, String targetName, + List arguments, String rerouteReturn, boolean isInBukkit, + RequirePluginVersionData requiredPluginVersion) { + return new RerouteMethodData(methodKey, Type.getType(sourceDesc), Type.getObjectType(sourceOwner), sourceName, + staticReroute, Type.getType(targetType), targetOwner, targetName, arguments, new RerouteReturn(Type.getType(rerouteReturn)), isInBukkit, requiredPluginVersion); + } + + private void check(RerouteMethodData actual, RerouteMethodData expected) { + assertEquals(expected.methodKey(), actual.methodKey(), "Method key are not the same"); + assertEquals(expected.sourceDesc(), actual.sourceDesc(), "Source desc are not the same"); + assertEquals(expected.sourceOwner(), actual.sourceOwner(), "Source owner are not the same"); + assertEquals(expected.sourceName(), actual.sourceName(), "Source name are not the same"); + assertEquals(expected.staticReroute(), actual.staticReroute(), "Static reroute flag are not the same"); + assertEquals(expected.targetType(), actual.targetType(), "Target type are not the same"); + assertEquals(expected.targetOwner(), actual.targetOwner(), "Target owner are not the same"); + assertEquals(expected.targetName(), actual.targetName(), "Target name are not the same"); + assertEquals(expected.arguments().size(), actual.arguments().size(), "Arguments count are not the same"); + assertEquals(expected.arguments(), actual.arguments(), "Arguments are not the same"); + assertEquals(expected.rerouteReturn(), actual.rerouteReturn(), "Reroute return flag are not the same"); + assertEquals(expected.isInBukkit(), actual.isInBukkit(), "In Bukkit flag are not the same"); + assertEquals(expected.requiredPluginVersion(), actual.requiredPluginVersion(), "Required plugin version are not the same"); + } + + public record TestDataHolder(String methodKey, List rerouteMethodDataList) { + } +} diff --git a/src/test/java/org/bukkit/craftbukkit/legacy/reroute/DoNotRerouteTest.java b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/DoNotRerouteTest.java new file mode 100644 index 000000000..701bdcc7c --- /dev/null +++ b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/DoNotRerouteTest.java @@ -0,0 +1,23 @@ +package org.bukkit.craftbukkit.legacy.reroute; + +import java.util.List; +import java.util.Map; +import org.bukkit.support.environment.Normal; +import org.junit.jupiter.api.Test; + +@Normal +public class DoNotRerouteTest extends AbstractRerouteTest { + + @Test + public void testDoNotReroute() { + test(DoNotRerouteTestData.class, Map.of()); + } + + public static class DoNotRerouteTestData { + + @DoNotReroute + public static List getList(Object o) { + return null; + } + } +} diff --git a/src/test/java/org/bukkit/craftbukkit/legacy/reroute/InjectCompatibilityTest.java b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/InjectCompatibilityTest.java new file mode 100644 index 000000000..5471af287 --- /dev/null +++ b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/InjectCompatibilityTest.java @@ -0,0 +1,55 @@ +package org.bukkit.craftbukkit.legacy.reroute; + +import static org.junit.jupiter.api.Assertions.*; +import com.google.common.base.Predicates; +import java.util.List; +import java.util.Map; +import org.bukkit.support.environment.Normal; +import org.junit.jupiter.api.Test; + +@Normal +public class InjectCompatibilityTest extends AbstractRerouteTest { + + @Test + public void testInjectCompatibility() { + test(InjectCompatibilityTestData.class, Map.of( + "()Ljava/util/List;getList", create( + "()Ljava/util/List;getList", + "()Ljava/util/List;", + "java/lang/Object", + "getList", + false, + "(Ljava/lang/Object;Z)Ljava/util/List;", + "org/bukkit/craftbukkit/legacy/reroute/InjectCompatibilityTest$InjectCompatibilityTestData", + "getList", + create( + create("Ljava/lang/Object;", "Ljava/lang/Object;", false, false, null), + create("Z", "Z", false, false, "test-value") + ), + "Ljava/util/List;", + true, + null + ) + ) + ); + } + + @Test + public void testInjectCompatibilityIncorrectType() { + assertThrows(RuntimeException.class, () -> RerouteBuilder.create(Predicates.alwaysTrue()).forClass(InjectCompatibilityIncorrectTypeTestData.class).build()); + } + + public static class InjectCompatibilityTestData { + + public static List getList(Object o, @InjectCompatibility("test-value") boolean value) { + return null; + } + } + + public static class InjectCompatibilityIncorrectTypeTestData { + + public static List getList(Object o, @InjectCompatibility("test-value") String value) { + return null; + } + } +} diff --git a/src/test/java/org/bukkit/craftbukkit/legacy/reroute/InjectPluginNameTest.java b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/InjectPluginNameTest.java new file mode 100644 index 000000000..41f803c85 --- /dev/null +++ b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/InjectPluginNameTest.java @@ -0,0 +1,55 @@ +package org.bukkit.craftbukkit.legacy.reroute; + +import static org.junit.jupiter.api.Assertions.*; +import com.google.common.base.Predicates; +import java.util.List; +import java.util.Map; +import org.bukkit.support.environment.Normal; +import org.junit.jupiter.api.Test; + +@Normal +public class InjectPluginNameTest extends AbstractRerouteTest { + + @Test + public void testInjectPluginName() { + test(InjectPluginNameTestData.class, Map.of( + "()Ljava/util/List;getList", create( + "()Ljava/util/List;getList", + "()Ljava/util/List;", + "java/lang/Object", + "getList", + false, + "(Ljava/lang/Object;Ljava/lang/String;)Ljava/util/List;", + "org/bukkit/craftbukkit/legacy/reroute/InjectPluginNameTest$InjectPluginNameTestData", + "getList", + create( + create("Ljava/lang/Object;", "Ljava/lang/Object;", false, false, null), + create("Ljava/lang/String;", "Ljava/lang/String;", true, false, null) + ), + "Ljava/util/List;", + true, + null + ) + ) + ); + } + + @Test + public void testInjectPluginNameIncorrectType() { + assertThrows(RuntimeException.class, () -> RerouteBuilder.create(Predicates.alwaysTrue()).forClass(InjectPluginNameIncorrectTypeTestData.class).build()); + } + + public static class InjectPluginNameTestData { + + public static List getList(Object o, @InjectPluginName String value) { + return null; + } + } + + public static class InjectPluginNameIncorrectTypeTestData { + + public static List getList(Object o, @InjectPluginName Object value) { + return null; + } + } +} diff --git a/src/test/java/org/bukkit/craftbukkit/legacy/reroute/InjectPluginVersionTest.java b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/InjectPluginVersionTest.java new file mode 100644 index 000000000..25e7a9e91 --- /dev/null +++ b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/InjectPluginVersionTest.java @@ -0,0 +1,56 @@ +package org.bukkit.craftbukkit.legacy.reroute; + +import static org.junit.jupiter.api.Assertions.*; +import com.google.common.base.Predicates; +import java.util.List; +import java.util.Map; +import org.bukkit.craftbukkit.util.ApiVersion; +import org.bukkit.support.environment.Normal; +import org.junit.jupiter.api.Test; + +@Normal +public class InjectPluginVersionTest extends AbstractRerouteTest { + + @Test + public void testInjectPluginVersion() { + test(InjectPluginVersionTestData.class, Map.of( + "()Ljava/util/List;getList", create( + "()Ljava/util/List;getList", + "()Ljava/util/List;", + "java/lang/Object", + "getList", + false, + "(Ljava/lang/Object;Lorg/bukkit/craftbukkit/util/ApiVersion;)Ljava/util/List;", + "org/bukkit/craftbukkit/legacy/reroute/InjectPluginVersionTest$InjectPluginVersionTestData", + "getList", + create( + create("Ljava/lang/Object;", "Ljava/lang/Object;", false, false, null), + create("Lorg/bukkit/craftbukkit/util/ApiVersion;", "Lorg/bukkit/craftbukkit/util/ApiVersion;", false, true, null) + ), + "Ljava/util/List;", + true, + null + ) + ) + ); + } + + @Test + public void testInjectPluginVersionIncorrectType() { + assertThrows(RuntimeException.class, () -> RerouteBuilder.create(Predicates.alwaysTrue()).forClass(InjectPluginVersionIncorrectTypeTestData.class).build()); + } + + public static class InjectPluginVersionTestData { + + public static List getList(Object o, @InjectPluginVersion ApiVersion value) { + return null; + } + } + + public static class InjectPluginVersionIncorrectTypeTestData { + + public static List getList(Object o, @InjectPluginVersion String value) { + return null; + } + } +} diff --git a/src/test/java/org/bukkit/craftbukkit/legacy/reroute/NotInBukkitTest.java b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/NotInBukkitTest.java new file mode 100644 index 000000000..1188802c2 --- /dev/null +++ b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/NotInBukkitTest.java @@ -0,0 +1,41 @@ +package org.bukkit.craftbukkit.legacy.reroute; + +import java.util.List; +import java.util.Map; +import org.bukkit.support.environment.Normal; +import org.junit.jupiter.api.Test; + +@Normal +public class NotInBukkitTest extends AbstractRerouteTest { + + @Test + public void testNotInBukkit() { + test(NotInBukkitTestData.class, Map.of( + "()Ljava/util/List;getList", create( + "()Ljava/util/List;getList", + "()Ljava/util/List;", + "java/lang/Object", + "getList", + false, + "(Ljava/lang/Object;)Ljava/util/List;", + "org/bukkit/craftbukkit/legacy/reroute/NotInBukkitTest$NotInBukkitTestData", + "getList", + create( + create("Ljava/lang/Object;", "Ljava/lang/Object;", false, false, null) + ), + "Ljava/util/List;", + false, + null + ) + ) + ); + } + + public static class NotInBukkitTestData { + + @NotInBukkit + public static List getList(Object o) { + return null; + } + } +} diff --git a/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RequireCompatibilityTest.java b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RequireCompatibilityTest.java new file mode 100644 index 000000000..118609fef --- /dev/null +++ b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RequireCompatibilityTest.java @@ -0,0 +1,47 @@ +package org.bukkit.craftbukkit.legacy.reroute; + +import com.google.common.base.Predicates; +import java.util.List; +import java.util.Map; +import org.bukkit.support.environment.Normal; +import org.junit.jupiter.api.Test; + +@Normal +public class RequireCompatibilityTest extends AbstractRerouteTest { + + @Test + public void testRequireCompatibility() { + test(RequireCompatibilityTestData.class, Map.of( + "()Ljava/util/List;getList", create( + "()Ljava/util/List;getList", + "()Ljava/util/List;", + "java/lang/Object", + "getList", + false, + "(Ljava/lang/Object;)Ljava/util/List;", + "org/bukkit/craftbukkit/legacy/reroute/RequireCompatibilityTest$RequireCompatibilityTestData", + "getList", + create( + create("Ljava/lang/Object;", "Ljava/lang/Object;", false, false, null) + ), + "Ljava/util/List;", + true, + null + ) + ) + ); + } + + @Test + public void testRequireCompatibilityNotPresent() { + test(RequireCompatibilityTestData.class, Predicates.alwaysFalse(), Map.of()); + } + + public static class RequireCompatibilityTestData { + + @RequireCompatibility("test-value") + public static List getList(Object o) { + return null; + } + } +} diff --git a/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RequirePluginVersionDataTest.java b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RequirePluginVersionDataTest.java new file mode 100644 index 000000000..c7c9ab3e6 --- /dev/null +++ b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RequirePluginVersionDataTest.java @@ -0,0 +1,121 @@ +package org.bukkit.craftbukkit.legacy.reroute; + +import static org.junit.jupiter.api.Assertions.*; +import java.lang.annotation.Annotation; +import org.bukkit.craftbukkit.util.ApiVersion; +import org.bukkit.support.environment.Normal; +import org.junit.jupiter.api.Test; + +@Normal +public class RequirePluginVersionDataTest { + + @Test + public void testSame() { + RequirePluginVersionData data = RequirePluginVersionData.create(new RequirePluginVersionImpl("", "1.21", "1.21")); + + assertEquals(ApiVersion.getOrCreateVersion("1.21"), data.minInclusive()); + assertEquals(ApiVersion.getOrCreateVersion("1.21"), data.maxInclusive()); + } + + @Test + public void testDifferent() { + RequirePluginVersionData data = RequirePluginVersionData.create(new RequirePluginVersionImpl("", "1.21", "1.23")); + + assertEquals(ApiVersion.getOrCreateVersion("1.21"), data.minInclusive()); + assertEquals(ApiVersion.getOrCreateVersion("1.23"), data.maxInclusive()); + } + + @Test + public void testValue() { + RequirePluginVersionData data = RequirePluginVersionData.create(new RequirePluginVersionImpl("1.42", "", "")); + + assertEquals(ApiVersion.getOrCreateVersion("1.42"), data.minInclusive()); + assertEquals(ApiVersion.getOrCreateVersion("1.42"), data.maxInclusive()); + } + + @Test + public void testValueAndMin() { + assertThrows(IllegalArgumentException.class, () -> RequirePluginVersionData.create(new RequirePluginVersionImpl("1.42", "1.52", ""))); + } + + @Test + public void testValueAndMax() { + assertThrows(IllegalArgumentException.class, () -> RequirePluginVersionData.create(new RequirePluginVersionImpl("1.42", "", "1.53"))); + } + + + @Test + public void testValueAndMinMax() { + assertThrows(IllegalArgumentException.class, () -> RequirePluginVersionData.create(new RequirePluginVersionImpl("1.42", "1.51", "1.53"))); + } + + @Test + public void testMinNewerThanMax() { + assertThrows(IllegalArgumentException.class, () -> RequirePluginVersionData.create(new RequirePluginVersionImpl("", "1.59", "1.57"))); + } + + @Test + public void testOnlyMin() { + RequirePluginVersionData data = RequirePluginVersionData.create(new RequirePluginVersionImpl("", "1.44", "")); + + assertEquals(ApiVersion.getOrCreateVersion("1.44"), data.minInclusive()); + assertNull(data.maxInclusive()); + } + + + @Test + public void testOnlyMax() { + RequirePluginVersionData data = RequirePluginVersionData.create(new RequirePluginVersionImpl("", "", "1.96")); + + assertNull(data.minInclusive()); + assertEquals(ApiVersion.getOrCreateVersion("1.96"), data.maxInclusive()); + } + + @Test + public void testExactData() { + RequirePluginVersionData data = new RequirePluginVersionData(ApiVersion.getOrCreateVersion("1.12"), ApiVersion.getOrCreateVersion("1.12")); + + assertTrue(data.test(ApiVersion.getOrCreateVersion("1.12"))); + assertFalse(data.test(ApiVersion.getOrCreateVersion("1.11"))); + assertFalse(data.test(ApiVersion.getOrCreateVersion("1.13"))); + } + + @Test + public void testRangeData() { + RequirePluginVersionData data = new RequirePluginVersionData(ApiVersion.getOrCreateVersion("1.12"), ApiVersion.getOrCreateVersion("1.13")); + + assertTrue(data.test(ApiVersion.getOrCreateVersion("1.12"))); + assertTrue(data.test(ApiVersion.getOrCreateVersion("1.12.5"))); + assertTrue(data.test(ApiVersion.getOrCreateVersion("1.13"))); + + assertFalse(data.test(ApiVersion.getOrCreateVersion("1.11"))); + assertFalse(data.test(ApiVersion.getOrCreateVersion("1.14"))); + } + + @Test + public void testOlderOpenRangeData() { + RequirePluginVersionData data = new RequirePluginVersionData(null, ApiVersion.getOrCreateVersion("1.13")); + + assertTrue(data.test(ApiVersion.getOrCreateVersion("1.1"))); + assertTrue(data.test(ApiVersion.getOrCreateVersion("1.13"))); + + assertFalse(data.test(ApiVersion.getOrCreateVersion("1.14"))); + } + + @Test + public void testNewerOpenRangeData() { + RequirePluginVersionData data = new RequirePluginVersionData(ApiVersion.getOrCreateVersion("1.12"), null); + + assertTrue(data.test(ApiVersion.getOrCreateVersion("1.12"))); + assertTrue(data.test(ApiVersion.getOrCreateVersion("1.14"))); + + assertFalse(data.test(ApiVersion.getOrCreateVersion("1.11"))); + } + + private record RequirePluginVersionImpl(String value, String minInclusive, String maxInclusive) implements RequirePluginVersion { + @Override + public Class annotationType() { + return null; + } + } +} diff --git a/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RequirePluginVersionTest.java b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RequirePluginVersionTest.java new file mode 100644 index 000000000..09c64116e --- /dev/null +++ b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RequirePluginVersionTest.java @@ -0,0 +1,42 @@ +package org.bukkit.craftbukkit.legacy.reroute; + +import java.util.List; +import java.util.Map; +import org.bukkit.craftbukkit.util.ApiVersion; +import org.bukkit.support.environment.Normal; +import org.junit.jupiter.api.Test; + +@Normal +public class RequirePluginVersionTest extends AbstractRerouteTest { + + @Test + public void testRequirePluginVersion() { + test(RequirePluginVersionTestData.class, Map.of( + "()Ljava/util/List;getList", create( + "()Ljava/util/List;getList", + "()Ljava/util/List;", + "java/lang/Object", + "getList", + false, + "(Ljava/lang/Object;)Ljava/util/List;", + "org/bukkit/craftbukkit/legacy/reroute/RequirePluginVersionTest$RequirePluginVersionTestData", + "getList", + create( + create("Ljava/lang/Object;", "Ljava/lang/Object;", false, false, null) + ), + "Ljava/util/List;", + true, + new RequirePluginVersionData(ApiVersion.getOrCreateVersion("1.42"), ApiVersion.getOrCreateVersion("1.42")) + ) + ) + ); + } + + public static class RequirePluginVersionTestData { + + @RequirePluginVersion("1.42") + public static List getList(Object o) { + return null; + } + } +} diff --git a/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RerouteArgumentTypeTest.java b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RerouteArgumentTypeTest.java new file mode 100644 index 000000000..097221bce --- /dev/null +++ b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RerouteArgumentTypeTest.java @@ -0,0 +1,71 @@ +package org.bukkit.craftbukkit.legacy.reroute; + +import java.util.List; +import java.util.Map; +import org.bukkit.support.environment.Normal; +import org.junit.jupiter.api.Test; + +@Normal +public class RerouteArgumentTypeTest extends AbstractRerouteTest { + + @Test + public void testRerouteArgumentType() { + test(RerouteArgumentTypeTestData.class, Map.of( + "()Ljava/util/List;getList", create( + "()Ljava/util/List;getList", + "()Ljava/util/List;", + "java/util/Map", + "getList", + false, + "(Ljava/lang/Object;)Ljava/util/List;", + "org/bukkit/craftbukkit/legacy/reroute/RerouteArgumentTypeTest$RerouteArgumentTypeTestData", + "getList", + create( + create("Ljava/lang/Object;", "Ljava/util/Map;", false, false, null) + ), + "Ljava/util/List;", + true, + null + ) + ) + ); + } + + @Test + public void testRerouteArgumentTypeSecond() { + test(RerouteArgumentTypeSecondTestData.class, Map.of( + "(Ljava/lang/String;)Ljava/util/List;getList", create( + "(Ljava/lang/String;)Ljava/util/List;getList", + "(Ljava/lang/String;)Ljava/util/List;", + "java/lang/Object", + "getList", + false, + "(Ljava/lang/Object;Ljava/util/Map;)Ljava/util/List;", + "org/bukkit/craftbukkit/legacy/reroute/RerouteArgumentTypeTest$RerouteArgumentTypeSecondTestData", + "getList", + create( + create("Ljava/lang/Object;", "Ljava/lang/Object;", false, false, null), + create("Ljava/util/Map;", "Ljava/lang/String;", false, false, null) + ), + "Ljava/util/List;", + true, + null + ) + ) + ); + } + + public static class RerouteArgumentTypeTestData { + + public static List getList(@RerouteArgumentType("java/util/Map") Object o) { + return null; + } + } + + public static class RerouteArgumentTypeSecondTestData { + + public static List getList(Object o, @RerouteArgumentType("java/lang/String") Map map) { + return null; + } + } +} diff --git a/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RerouteBuilderTest.java b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RerouteBuilderTest.java new file mode 100644 index 000000000..d249c0e26 --- /dev/null +++ b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RerouteBuilderTest.java @@ -0,0 +1,221 @@ +package org.bukkit.craftbukkit.legacy.reroute; + +import static org.junit.jupiter.api.Assertions.*; +import com.google.common.base.Predicates; +import java.util.List; +import java.util.Map; +import java.util.logging.Logger; +import org.bukkit.support.environment.Normal; +import org.junit.jupiter.api.Test; + +@Normal +public class RerouteBuilderTest extends AbstractRerouteTest { + + @Test + public void testReroute() { + test(FirstTest.class, Map.of( + "()VtestReroute", create( + "()VtestReroute", + "()V", + "org/bukkit/craftbukkit/legacy/reroute/RerouteBuilderTest", + "testReroute", + false, + "(Lorg/bukkit/craftbukkit/legacy/reroute/RerouteBuilderTest;)V", + "org/bukkit/craftbukkit/legacy/reroute/RerouteBuilderTest$FirstTest", + "testReroute", + create( + create("Lorg/bukkit/craftbukkit/legacy/reroute/RerouteBuilderTest;", "Lorg/bukkit/craftbukkit/legacy/reroute/RerouteBuilderTest;", false, false, null) + ), + "V", + true, + null + ) + )); + } + + @Test + public void testInvalidMethods() { + test(InvalidMethodTest.class, Map.of()); + } + + @Test + public void testMultipleMethods() { + test(MultipleMethodTest.class, Map.of( + "()Ljava/util/List;getList", create( + "()Ljava/util/List;getList", + "()Ljava/util/List;", + "java/lang/Object", + "getList", + false, + "(Ljava/lang/Object;)Ljava/util/List;", + "org/bukkit/craftbukkit/legacy/reroute/RerouteBuilderTest$MultipleMethodTest", + "getList", + create( + create("Ljava/lang/Object;", "Ljava/lang/Object;", false, false, null) + ), + "Ljava/util/List;", + true, + null + ), + "(Ljava/util/Map;)Ljava/util/Map;getMap", create( + "(Ljava/util/Map;)Ljava/util/Map;getMap", + "(Ljava/util/Map;)Ljava/util/Map;", + "java/lang/String", + "getMap", + false, + "(Ljava/lang/String;Ljava/util/Map;)Ljava/util/Map;", + "org/bukkit/craftbukkit/legacy/reroute/RerouteBuilderTest$MultipleMethodTest", + "getMap", + create( + create("Ljava/lang/String;", "Ljava/lang/String;", false, false, null), + create("Ljava/util/Map;", "Ljava/util/Map;", false, false, null) + ), + "Ljava/util/Map;", + true, + null + ), + "(ZS)IgetInt", create( + "(ZS)IgetInt", + "(ZS)I", + "java/util/logging/Logger", + "getInt", + false, + "(Ljava/util/logging/Logger;ZS)I", + "org/bukkit/craftbukkit/legacy/reroute/RerouteBuilderTest$MultipleMethodTest", + "getInt", + create( + create("Ljava/util/logging/Logger;", "Ljava/util/logging/Logger;", false, false, null), + create("Z", "Z", false, false, null), + create("S", "S", false, false, null) + ), + "I", + true, + null + ) + ) + ); + } + + @Test + public void testInterfaceRerouteClass() { + assertThrows(IllegalArgumentException.class, () -> RerouteBuilder.create(Predicates.alwaysTrue()).forClass(InterfaceTest.class).build()); + } + + @Test + public void testAnnotationRerouteClass() { + assertThrows(IllegalArgumentException.class, () -> RerouteBuilder.create(Predicates.alwaysTrue()).forClass(AnnotationTest.class).build()); + } + + @Test + public void testMissingOwner() { + assertThrows(RuntimeException.class, () -> RerouteBuilder.create(Predicates.alwaysTrue()).forClass(MissingOwnerTest.class).build()); + } + + @Test + public void testSameKey() { + test(SameKeyTest.class, Predicates.alwaysTrue(), List.of( + new TestDataHolder("()Ljava/util/List;getList", List.of(create( + "()Ljava/util/List;getList", + "()Ljava/util/List;", + "java/lang/Object", + "getList", + false, + "(Ljava/lang/Object;)Ljava/util/List;", + "org/bukkit/craftbukkit/legacy/reroute/RerouteBuilderTest$SameKeyTest", + "getList", + create( + create("Ljava/lang/Object;", "Ljava/lang/Object;", false, false, null) + ), + "Ljava/util/List;", + true, + null + ), + create( + "()Ljava/util/List;getList", + "()Ljava/util/List;", + "java/lang/String", + "getList", + false, + "(Ljava/lang/String;Ljava/lang/String;)Ljava/util/List;", + "org/bukkit/craftbukkit/legacy/reroute/RerouteBuilderTest$SameKeyTest", + "getList", + create( + create("Ljava/lang/String;", "Ljava/lang/String;", false, false, null), + create("Ljava/lang/String;", "Ljava/lang/String;", true, false, null) + ), + "Ljava/util/List;", + true, + null + ) + )) + )); + } + + public static class FirstTest { + + public static void testReroute(RerouteBuilderTest rerouteBuilderTest) { + } + } + + public static class InvalidMethodTest { + + public void testReroute(RerouteBuilderTest rerouteBuilderTest) { + } + + static void testReroute2(RerouteBuilderTest rerouteBuilderTest) { + } + + void testReroute3(RerouteBuilderTest rerouteBuilderTest) { + } + + private static void testReroute4(RerouteBuilderTest rerouteBuilderTest) { + } + + protected static void testReroute5(RerouteBuilderTest rerouteBuilderTest) { + } + } + + public static class MultipleMethodTest { + + public static List getList(Object o) { + return null; + } + + public static Map getMap(String value, Map map) { + return null; + } + + public static int getInt(Logger logger, boolean bool, short s) { + return 0; + } + } + + public interface InterfaceTest { + + static List getList(Object o) { + return null; + } + } + + public @interface AnnotationTest { + + } + + public static class MissingOwnerTest { + + public static List getList() { + return null; + } + } + + public class SameKeyTest { + + public static List getList(Object o) { + return null; + } + + public static List getList(String s, @InjectPluginName String name) { + return null; + } + } +} diff --git a/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RerouteMethodNameTest.java b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RerouteMethodNameTest.java new file mode 100644 index 000000000..24638da3f --- /dev/null +++ b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RerouteMethodNameTest.java @@ -0,0 +1,41 @@ +package org.bukkit.craftbukkit.legacy.reroute; + +import java.util.List; +import java.util.Map; +import org.bukkit.support.environment.Normal; +import org.junit.jupiter.api.Test; + +@Normal +public class RerouteMethodNameTest extends AbstractRerouteTest { + + @Test + public void testRerouteMethodName() { + test(RerouteMethodNameTestData.class, Map.of( + "()Ljava/util/List;getMap", create( + "()Ljava/util/List;getMap", + "()Ljava/util/List;", + "java/lang/Object", + "getMap", + false, + "(Ljava/lang/Object;)Ljava/util/List;", + "org/bukkit/craftbukkit/legacy/reroute/RerouteMethodNameTest$RerouteMethodNameTestData", + "getList", + create( + create("Ljava/lang/Object;", "Ljava/lang/Object;", false, false, null) + ), + "Ljava/util/List;", + true, + null + ) + ) + ); + } + + public static class RerouteMethodNameTestData { + + @RerouteMethodName("getMap") + public static List getList(Object o) { + return null; + } + } +} diff --git a/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RerouteReturnTypeTest.java b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RerouteReturnTypeTest.java new file mode 100644 index 000000000..3f1e337e9 --- /dev/null +++ b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RerouteReturnTypeTest.java @@ -0,0 +1,41 @@ +package org.bukkit.craftbukkit.legacy.reroute; + +import java.util.List; +import java.util.Map; +import org.bukkit.support.environment.Normal; +import org.junit.jupiter.api.Test; + +@Normal +public class RerouteReturnTypeTest extends AbstractRerouteTest { + + @Test + public void testRerouteReturnType() { + test(RerouteReturnTypeTestData.class, Map.of( + "()Ljava/util/Map;getList", create( + "()Ljava/util/Map;getList", + "()Ljava/util/Map;", + "java/lang/Object", + "getList", + false, + "(Ljava/lang/Object;)Ljava/util/List;", + "org/bukkit/craftbukkit/legacy/reroute/RerouteReturnTypeTest$RerouteReturnTypeTestData", + "getList", + create( + create("Ljava/lang/Object;", "Ljava/lang/Object;", false, false, null) + ), + "Ljava/util/List;", + true, + null + ) + ) + ); + } + + public static class RerouteReturnTypeTestData { + + @RerouteReturnType("java/util/Map") + public static List getList(Object o) { + return null; + } + } +} diff --git a/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RerouteStaticTest.java b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RerouteStaticTest.java new file mode 100644 index 000000000..7f1c045a0 --- /dev/null +++ b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RerouteStaticTest.java @@ -0,0 +1,41 @@ +package org.bukkit.craftbukkit.legacy.reroute; + +import java.util.List; +import java.util.Map; +import org.bukkit.support.environment.Normal; +import org.junit.jupiter.api.Test; + +@Normal +public class RerouteStaticTest extends AbstractRerouteTest { + + @Test + public void testStaticReroute() { + test(RerouteStaticTestData.class, Map.of( + "(Ljava/lang/Object;)Ljava/util/List;getList", create( + "(Ljava/lang/Object;)Ljava/util/List;getList", + "(Ljava/lang/Object;)Ljava/util/List;", + "java/util/Map", + "getList", + true, + "(Ljava/lang/Object;)Ljava/util/List;", + "org/bukkit/craftbukkit/legacy/reroute/RerouteStaticTest$RerouteStaticTestData", + "getList", + create( + create("Ljava/lang/Object;", "Ljava/lang/Object;", false, false, null) + ), + "Ljava/util/List;", + true, + null + ) + ) + ); + } + + public static class RerouteStaticTestData { + + @RerouteStatic("java/util/Map") + public static List getList(Object o) { + return null; + } + } +} diff --git a/src/test/java/org/bukkit/craftbukkit/legacy/RerouteValidationTest.java b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RerouteValidationTest.java similarity index 82% rename from src/test/java/org/bukkit/craftbukkit/legacy/RerouteValidationTest.java rename to src/test/java/org/bukkit/craftbukkit/legacy/reroute/RerouteValidationTest.java index ba2578296..55283fea5 100644 --- a/src/test/java/org/bukkit/craftbukkit/legacy/RerouteValidationTest.java +++ b/src/test/java/org/bukkit/craftbukkit/legacy/reroute/RerouteValidationTest.java @@ -1,13 +1,13 @@ -package org.bukkit.craftbukkit.legacy; +package org.bukkit.craftbukkit.legacy.reroute; import static org.junit.jupiter.api.Assertions.*; import com.google.common.base.Joiner; +import com.google.common.base.Predicates; import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.HashMap; import java.util.Map; import java.util.stream.Stream; -import org.bukkit.craftbukkit.legacy.reroute.RerouteMethodData; import org.bukkit.craftbukkit.util.Commodore; import org.bukkit.support.environment.Normal; import org.junit.jupiter.params.ParameterizedTest; @@ -19,25 +19,28 @@ import org.objectweb.asm.Type; public class RerouteValidationTest { public static Stream data() { - return Commodore.REROUTES.stream().map(Arguments::of); + Commodore commodore = new Commodore(Predicates.alwaysTrue()); + return commodore.getReroutes().stream().map(Arguments::of); } @ParameterizedTest @MethodSource("data") - public void testReroutes(Map reroutes) { + public void testReroutes(Reroute reroute) { Map wrongReroutes = new HashMap<>(); String owner = null; - for (Map.Entry entry : reroutes.entrySet()) { - owner = entry.getValue().targetOwner(); - if (!entry.getValue().isInBukkit()) { - continue; - } + for (Map.Entry value : reroute.rerouteDataMap.entrySet()) { + for (Map.Entry entry : value.getValue().rerouteMethodDataMap.entrySet()) { + owner = entry.getValue().targetOwner(); + if (!entry.getValue().isInBukkit()) { + continue; + } - String error = isValid(entry.getKey(), entry.getValue()); + String error = isValid(entry.getKey(), entry.getValue()); - if (error != null) { - wrongReroutes.put(entry.getKey(), error); + if (error != null) { + wrongReroutes.put(entry.getKey(), error); + } } }