From a11ca51ee13a07e4dfd629633333180a4ffdec93 Mon Sep 17 00:00:00 2001 From: CraftBukkit/Spigot Date: Mon, 25 Nov 2024 07:35:28 +1100 Subject: [PATCH] #1515: Add a Class reader and Class node argument provider By: DerFrZocker --- .../bukkit/event/EntityRemoveEventTest.java | 219 +++++------------- .../provider/ClassNodeArgumentProvider.java | 70 ++++++ .../provider/ClassReaderArgumentProvider.java | 210 +++++++++++++++++ .../bukkit/support/test/ClassNodeTest.java | 29 +++ .../bukkit/support/test/ClassReaderTest.java | 29 +++ 5 files changed, 396 insertions(+), 161 deletions(-) create mode 100644 paper-server/src/test/java/org/bukkit/support/provider/ClassNodeArgumentProvider.java create mode 100644 paper-server/src/test/java/org/bukkit/support/provider/ClassReaderArgumentProvider.java create mode 100644 paper-server/src/test/java/org/bukkit/support/test/ClassNodeTest.java create mode 100644 paper-server/src/test/java/org/bukkit/support/test/ClassReaderTest.java diff --git a/paper-server/src/test/java/org/bukkit/event/EntityRemoveEventTest.java b/paper-server/src/test/java/org/bukkit/event/EntityRemoveEventTest.java index 04de9f1238..b5fd6d205f 100644 --- a/paper-server/src/test/java/org/bukkit/event/EntityRemoveEventTest.java +++ b/paper-server/src/test/java/org/bukkit/event/EntityRemoveEventTest.java @@ -2,31 +2,12 @@ package org.bukkit.event; import static org.junit.jupiter.api.Assertions.*; import com.google.common.base.Joiner; -import java.io.File; -import java.io.FileInputStream; -import java.io.FileNotFoundException; -import java.io.IOException; -import java.io.InputStream; -import java.net.URI; -import java.net.URISyntaxException; -import java.nio.file.Files; -import java.nio.file.Path; import java.util.ArrayList; import java.util.List; -import java.util.jar.JarFile; -import java.util.stream.Stream; -import net.minecraft.WorldVersion; -import net.minecraft.server.Main; import net.minecraft.world.level.entity.EntityAccess; import org.bukkit.support.environment.Normal; -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; -import org.objectweb.asm.ClassReader; +import org.bukkit.support.test.ClassNodeTest; import org.objectweb.asm.Handle; -import org.objectweb.asm.Opcodes; import org.objectweb.asm.tree.AbstractInsnNode; import org.objectweb.asm.tree.ClassNode; import org.objectweb.asm.tree.InvokeDynamicInsnNode; @@ -37,150 +18,77 @@ import org.objectweb.asm.tree.MethodNode; @Normal public class EntityRemoveEventTest { - // Needs to be a class, which is present in the source, and not a test class - private static final URI CRAFT_BUKKIT_CLASSES; - // Needs to be a class, which is from the minecraft package and not patch by CraftBukkit - private static final URI MINECRAFT_CLASSES; - - static { - try { - CRAFT_BUKKIT_CLASSES = Main.class.getProtectionDomain().getCodeSource().getLocation().toURI(); - MINECRAFT_CLASSES = WorldVersion.class.getProtectionDomain().getCodeSource().getLocation().toURI(); - } catch (URISyntaxException e) { - throw new RuntimeException(e); - } - } - - private static JarFile jarFile = null; - private static Stream files = null; - - public static Stream craftBukkitData() { - return files - .map(Path::toFile) - .filter(File::isFile) - .filter(file -> file.getName().endsWith(".class")) - .filter(file -> !file.getName().equals("EntityAccess.class")) - .map(file -> { - try { - return new FileInputStream(file); - } catch (FileNotFoundException e) { - throw new RuntimeException(e); - } - }).map(Arguments::of); - } - - public static Stream minecraftData() { - return jarFile - .stream() - .filter(entry -> entry.getName().endsWith(".class")) - .filter(entry -> !new File(CRAFT_BUKKIT_CLASSES.resolve(entry.getName())).exists()) - .filter(entry -> !entry.getName().startsWith("net/minecraft/gametest/framework")) - .map(entry -> { - try { - return jarFile.getInputStream(entry); - } catch (IOException e) { - throw new RuntimeException(e); - } - }).map(Arguments::arguments); - } - - @BeforeAll - public static void beforeAll() throws IOException { - assertNotEquals(CRAFT_BUKKIT_CLASSES, MINECRAFT_CLASSES, """ - The minecraft and craft bukkit uri point to the same directory / file. - Please make sure the CRAFT_BUKKIT_CLASSES points to the test class directory and MINECRAFT_CLASSES to the minecraft server jar. - """); - - jarFile = new JarFile(new File(MINECRAFT_CLASSES)); - files = Files.walk(Path.of(CRAFT_BUKKIT_CLASSES)); - } - - @ParameterizedTest - @MethodSource("minecraftData") - public void testMinecraftClasses(InputStream inputStream) throws IOException, ClassNotFoundException { - test(inputStream); - } - - @ParameterizedTest - @MethodSource("craftBukkitData") - public void testCraftBukkitModifiedClasses(InputStream inputStream) throws IOException, ClassNotFoundException { - test(inputStream); - } - - private void test(InputStream inputStream) throws IOException, ClassNotFoundException { + @ClassNodeTest(value = {ClassNodeTest.ClassType.CRAFT_BUKKIT, ClassNodeTest.ClassType.MINECRAFT_MODIFIED, ClassNodeTest.ClassType.MINECRAFT_UNMODIFIED}, + excludedClasses = EntityAccess.class, + excludedPackages = "net/minecraft/gametest/framework") + public void testForMissing(ClassNode classNode) throws ClassNotFoundException { List missingReason = new ArrayList<>(); - try (inputStream) { - ClassReader classReader = new ClassReader(inputStream); - ClassNode classNode = new ClassNode(Opcodes.ASM9); + boolean minecraftCause = false; + boolean bukkitCause = false; - classReader.accept(classNode, Opcodes.ASM9); - boolean minecraftCause = false; - boolean bukkitCause = false; + for (MethodNode methodNode : classNode.methods) { + if (methodNode.name.equals("remove") && methodNode.desc.contains("Lnet/minecraft/world/entity/Entity$RemovalReason;")) { + if (methodNode.desc.contains("Lorg/bukkit/event/entity/EntityRemoveEvent$Cause;")) { + bukkitCause = true; + } else { + minecraftCause = true; + } + } - for (MethodNode methodNode : classNode.methods) { - if (methodNode.name.equals("remove") && methodNode.desc.contains("Lnet/minecraft/world/entity/Entity$RemovalReason;")) { - if (methodNode.desc.contains("Lorg/bukkit/event/entity/EntityRemoveEvent$Cause;")) { - bukkitCause = true; - } else { - minecraftCause = true; - } + LineNumberNode lastLineNumber = null; + for (AbstractInsnNode instruction : methodNode.instructions) { + if (instruction instanceof LineNumberNode lineNumberNode) { + lastLineNumber = lineNumberNode; + continue; } - LineNumberNode lastLineNumber = null; - for (AbstractInsnNode instruction : methodNode.instructions) { - if (instruction instanceof LineNumberNode lineNumberNode) { - lastLineNumber = lineNumberNode; + if (instruction instanceof MethodInsnNode methodInsnNode) { + // Check for discard and remove method call + if (check(methodInsnNode.owner, methodInsnNode.name, methodInsnNode.desc)) { + // Add to list + missingReason.add(String.format("Method name: %s, name: %s, line number: %s", methodNode.name, methodInsnNode.name, lastLineNumber.line)); + } + } else if (instruction instanceof InvokeDynamicInsnNode dynamicInsnNode) { + // Check for discard and remove method call + if (!dynamicInsnNode.bsm.getOwner().equals("java/lang/invoke/LambdaMetafactory") + || !dynamicInsnNode.bsm.getName().equals("metafactory") || dynamicInsnNode.bsmArgs.length != 3) { continue; } - if (instruction instanceof MethodInsnNode methodInsnNode) { - // Check for discard and remove method call - if (check(methodInsnNode.owner, methodInsnNode.name, methodInsnNode.desc)) { - // Add to list - missingReason.add(String.format("Method name: %s, name: %s, line number: %s", methodNode.name, methodInsnNode.name, lastLineNumber.line)); - } - } else if (instruction instanceof InvokeDynamicInsnNode dynamicInsnNode) { - // Check for discard and remove method call - if (!dynamicInsnNode.bsm.getOwner().equals("java/lang/invoke/LambdaMetafactory") - || !dynamicInsnNode.bsm.getName().equals("metafactory") || dynamicInsnNode.bsmArgs.length != 3) { - continue; - } + Handle handle = (Handle) dynamicInsnNode.bsmArgs[1]; - Handle handle = (Handle) dynamicInsnNode.bsmArgs[1]; - - if (check(handle.getOwner(), handle.getName(), handle.getDesc())) { - // Add to list - missingReason.add(String.format("[D] Method name: %s, name: %s, line number: %s", methodNode.name, handle.getName(), lastLineNumber.line)); - } + if (check(handle.getOwner(), handle.getName(), handle.getDesc())) { + // Add to list + missingReason.add(String.format("[D] Method name: %s, name: %s, line number: %s", methodNode.name, handle.getName(), lastLineNumber.line)); } } } - - assertTrue(missingReason.isEmpty(), String.format(""" - The class %s has Entity#discard, Entity#remove and/or Entity#setRemoved method calls, which don't have a bukkit reason. - Please add a bukkit reason to them, if the event should not be called use null as reason. - - Following missing reasons where found: - %s""", classNode.name, Joiner.on('\n').join(missingReason))); - - if (minecraftCause == bukkitCause) { - return; - } - - if (minecraftCause) { - fail(String.format(""" - The class %s has the Entity#remove method override, but there is no bukkit override. - Please add a bukkit method override, which adds the bukkit cause. - """, classNode.name)); - return; // Will never reach ): - } - - fail(String.format(""" - The class %s has the Entity#remove method override, to add a bukkit cause, but there is no normal override. - Please remove the bukkit method override, since it is no longer needed. - """, classNode.name)); } + + assertTrue(missingReason.isEmpty(), String.format(""" + The class %s has Entity#discard, Entity#remove and/or Entity#setRemoved method calls, which don't have a bukkit reason. + Please add a bukkit reason to them, if the event should not be called use null as reason. + + Following missing reasons where found: + %s""", classNode.name, Joiner.on('\n').join(missingReason))); + + if (minecraftCause == bukkitCause) { + return; + } + + if (minecraftCause) { + fail(String.format(""" + The class %s has the Entity#remove method override, but there is no bukkit override. + Please add a bukkit method override, which adds the bukkit cause. + """, classNode.name)); + return; // Will never reach ): + } + + fail(String.format(""" + The class %s has the Entity#remove method override, to add a bukkit cause, but there is no normal override. + Please remove the bukkit method override, since it is no longer needed. + """, classNode.name)); } private boolean check(String owner, String name, String desc) throws ClassNotFoundException { @@ -207,15 +115,4 @@ public class EntityRemoveEventTest { return false; } - - @AfterAll - public static void clear() throws IOException { - if (jarFile != null) { - jarFile.close(); - } - - if (files != null) { - files.close(); - } - } } diff --git a/paper-server/src/test/java/org/bukkit/support/provider/ClassNodeArgumentProvider.java b/paper-server/src/test/java/org/bukkit/support/provider/ClassNodeArgumentProvider.java new file mode 100644 index 0000000000..522d3fdc1f --- /dev/null +++ b/paper-server/src/test/java/org/bukkit/support/provider/ClassNodeArgumentProvider.java @@ -0,0 +1,70 @@ +package org.bukkit.support.provider; + +import java.lang.annotation.Annotation; +import java.util.stream.Stream; +import org.bukkit.support.test.ClassNodeTest; +import org.bukkit.support.test.ClassReaderTest; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.ArgumentsProvider; +import org.junit.jupiter.params.support.AnnotationConsumer; +import org.objectweb.asm.ClassReader; +import org.objectweb.asm.Opcodes; +import org.objectweb.asm.tree.ClassNode; + +public class ClassNodeArgumentProvider implements ArgumentsProvider, AnnotationConsumer { + + private ClassNodeTest.ClassType[] classTypes; + private Class[] excludedClasses; + private String[] excludedPackages; + + @Override + public void accept(ClassNodeTest classNodeTest) { + this.classTypes = classNodeTest.value(); + this.excludedClasses = classNodeTest.excludedClasses(); + this.excludedPackages = classNodeTest.excludedPackages(); + + for (int i = 0; i < excludedPackages.length; i++) { + this.excludedPackages[i] = this.excludedPackages[i].replace('.', '/'); + } + } + + @Override + public Stream provideArguments(ExtensionContext extensionContext) throws Exception { + ClassReaderArgumentProvider classReaderArgumentProvider = new ClassReaderArgumentProvider(); + classReaderArgumentProvider.accept(new ClassReaderArguments(classReaderClassType(), excludedClasses, excludedPackages)); + + return classReaderArgumentProvider.getClassReaders().map(this::toClassNode).map(Arguments::of); + } + + private ClassReaderTest.ClassType[] classReaderClassType() { + ClassReaderTest.ClassType[] newValues = new ClassReaderTest.ClassType[classTypes.length]; + + for (int i = 0; i < classTypes.length; i++) { + newValues[i] = switch (classTypes[i]) { + case BUKKIT -> ClassReaderTest.ClassType.BUKKIT; + case CRAFT_BUKKIT -> ClassReaderTest.ClassType.CRAFT_BUKKIT; + case MINECRAFT_UNMODIFIED -> ClassReaderTest.ClassType.MINECRAFT_UNMODIFIED; + case MINECRAFT_MODIFIED -> ClassReaderTest.ClassType.MINECRAFT_MODIFIED; + }; + } + + return newValues; + } + + private ClassNode toClassNode(ClassReader classReader) { + ClassNode classNode = new ClassNode(Opcodes.ASM9); + + classReader.accept(classNode, Opcodes.ASM9); + + return classNode; + } + + private record ClassReaderArguments(ClassType[] value, Class[] excludedClasses, String[] excludedPackages) implements ClassReaderTest { + + @Override + public Class annotationType() { + return null; + } + } +} diff --git a/paper-server/src/test/java/org/bukkit/support/provider/ClassReaderArgumentProvider.java b/paper-server/src/test/java/org/bukkit/support/provider/ClassReaderArgumentProvider.java new file mode 100644 index 0000000000..f025491032 --- /dev/null +++ b/paper-server/src/test/java/org/bukkit/support/provider/ClassReaderArgumentProvider.java @@ -0,0 +1,210 @@ +package org.bukkit.support.provider; + +import static org.junit.jupiter.api.Assertions.*; +import java.io.File; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.io.InputStream; +import java.net.URI; +import java.net.URISyntaxException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.jar.JarEntry; +import java.util.jar.JarFile; +import java.util.stream.Stream; +import net.minecraft.WorldVersion; +import net.minecraft.server.Main; +import org.bukkit.Bukkit; +import org.bukkit.support.test.ClassReaderTest; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.ArgumentsProvider; +import org.junit.jupiter.params.support.AnnotationConsumer; +import org.objectweb.asm.ClassReader; + +public class ClassReaderArgumentProvider implements ArgumentsProvider, AnnotationConsumer { + + // Needs to be a class, which is present in the source, and not a test class + private static final URI CRAFT_BUKKIT_CLASSES; + // Needs to be a class, which is from the minecraft package and not patch by CraftBukkit + private static final URI MINECRAFT_CLASSES; + // Needs to be a class, which is from the bukkit package and not a CraftBukkit class + private static final URI BUKKIT_CLASSES; + + static { + try { + CRAFT_BUKKIT_CLASSES = Main.class.getProtectionDomain().getCodeSource().getLocation().toURI(); + MINECRAFT_CLASSES = WorldVersion.class.getProtectionDomain().getCodeSource().getLocation().toURI(); + BUKKIT_CLASSES = Bukkit.class.getProtectionDomain().getCodeSource().getLocation().toURI(); + } catch (URISyntaxException e) { + throw new RuntimeException(e); + } + } + + private ClassReaderTest.ClassType[] classTypes; + private Class[] excludedClasses; + private String[] excludedPackages; + + @Override + public void accept(ClassReaderTest classReaderTest) { + this.classTypes = classReaderTest.value(); + this.excludedClasses = classReaderTest.excludedClasses(); + this.excludedPackages = classReaderTest.excludedPackages(); + + for (int i = 0; i < excludedPackages.length; i++) { + this.excludedPackages[i] = this.excludedPackages[i].replace('.', '/'); + } + } + + @Override + public Stream provideArguments(ExtensionContext extensionContext) throws Exception { + return getClassReaders().map(Arguments::of); + } + + public Stream getClassReaders() { + assertNotEquals(CRAFT_BUKKIT_CLASSES, MINECRAFT_CLASSES, """ + The Minecraft and CraftBukkit uri point to the same directory / file. + Please make sure the CRAFT_BUKKIT_CLASSES points to the test class directory and MINECRAFT_CLASSES to the minecraft server jar. + """); + + Stream result = Stream.empty(); + + if (contains(ClassReaderTest.ClassType.MINECRAFT_UNMODIFIED)) { + result = Stream.concat(result, readMinecraftClasses()); + } + + if (contains(ClassReaderTest.ClassType.CRAFT_BUKKIT) || contains(ClassReaderTest.ClassType.MINECRAFT_MODIFIED)) { + result = Stream.concat(result, readCraftBukkitAndOrMinecraftModifiedClasses(contains(ClassReaderTest.ClassType.CRAFT_BUKKIT), contains(ClassReaderTest.ClassType.MINECRAFT_MODIFIED))); + } + + if (contains(ClassReaderTest.ClassType.BUKKIT)) { + result = Stream.concat(result, readBukkitClasses()); + } + + return result.map(this::toClassReader); + } + + private ClassReader toClassReader(InputStream stream) { + try (stream) { + return new ClassReader(stream); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + private boolean contains(ClassReaderTest.ClassType classType) { + for (ClassReaderTest.ClassType c : classTypes) { + if (c == classType) { + return true; + } + } + + return false; + } + + private Stream readMinecraftClasses() { + return readJarFile(MINECRAFT_CLASSES, true); + } + + private Stream readBukkitClasses() { + return readJarFile(BUKKIT_CLASSES, false); + } + + private Stream readJarFile(URI uri, boolean filterModified) { + try { + JarFile jarFile = new JarFile(new File(uri)); + return jarFile.stream().onClose(() -> closeJarFile(jarFile)) + .filter(entry -> entry.getName().endsWith(".class")) + .filter(entry -> filterModifiedIfNeeded(entry, filterModified)) + .filter(entry -> filterPackageNames(entry.getName())) + .filter(entry -> filterClass(entry.getName())) + .map(entry -> { + try { + return jarFile.getInputStream(entry); + } catch (IOException e) { + throw new RuntimeException(e); + } + }); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + private boolean filterModifiedIfNeeded(JarEntry entry, boolean needed) { + if (!needed) { + return true; + } + + return !new File(CRAFT_BUKKIT_CLASSES.resolve(entry.getName())).exists(); + } + + private boolean filterPackageNames(String name) { + for (String packageName : excludedPackages) { + if (name.startsWith(packageName)) { + return false; + } + } + + return true; + } + + private boolean filterClass(String name) { + for (Class clazz : excludedClasses) { + if (name.equals(clazz.getName().replace('.', '/') + ".class")) { + return false; + } + } + + return true; + } + + private Stream readCraftBukkitAndOrMinecraftModifiedClasses(boolean craftBukkit, boolean minecraftModified) { + try { + return Files.walk(Path.of(CRAFT_BUKKIT_CLASSES)) + .map(Path::toFile) + .filter(File::isFile) + .filter(file -> file.getName().endsWith(".class")) + .filter(file -> shouldInclude(removeHomeDirectory(file), craftBukkit, minecraftModified)) + .filter(file -> filterPackageNames(removeHomeDirectory(file))) + .filter(file -> filterClass(removeHomeDirectory(file))) + .map(file -> { + try { + return new FileInputStream(file); + } catch (FileNotFoundException e) { + throw new RuntimeException(e); + } + }); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + private String removeHomeDirectory(File file) { + return file.getAbsolutePath().substring(CRAFT_BUKKIT_CLASSES.getPath().length()); + } + + private boolean shouldInclude(String name, boolean craftBukkit, boolean minecraftModified) { + if (craftBukkit && minecraftModified) { + return true; + } + + if (craftBukkit) { + return name.startsWith("org/bukkit/craftbukkit/"); + } + + if (minecraftModified) { + return name.startsWith("net/minecraft/"); + } + + return false; + } + + private void closeJarFile(JarFile jarFile) { + try { + jarFile.close(); + } catch (IOException e) { + throw new RuntimeException(e); + } + } +} diff --git a/paper-server/src/test/java/org/bukkit/support/test/ClassNodeTest.java b/paper-server/src/test/java/org/bukkit/support/test/ClassNodeTest.java new file mode 100644 index 0000000000..41b284dbe5 --- /dev/null +++ b/paper-server/src/test/java/org/bukkit/support/test/ClassNodeTest.java @@ -0,0 +1,29 @@ +package org.bukkit.support.test; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import org.bukkit.support.provider.ClassNodeArgumentProvider; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ArgumentsSource; + +@Target({ElementType.METHOD, ElementType.ANNOTATION_TYPE}) +@Retention(RetentionPolicy.RUNTIME) +@ArgumentsSource(ClassNodeArgumentProvider.class) +@ParameterizedTest +public @interface ClassNodeTest { + + ClassType[] value() default {ClassType.BUKKIT, ClassType.CRAFT_BUKKIT, ClassType.MINECRAFT_UNMODIFIED, ClassType.MINECRAFT_MODIFIED}; + + Class[] excludedClasses() default {}; + + String[] excludedPackages() default {}; + + enum ClassType { + BUKKIT, + CRAFT_BUKKIT, + MINECRAFT_UNMODIFIED, + MINECRAFT_MODIFIED, + } +} diff --git a/paper-server/src/test/java/org/bukkit/support/test/ClassReaderTest.java b/paper-server/src/test/java/org/bukkit/support/test/ClassReaderTest.java new file mode 100644 index 0000000000..9ebec1ff65 --- /dev/null +++ b/paper-server/src/test/java/org/bukkit/support/test/ClassReaderTest.java @@ -0,0 +1,29 @@ +package org.bukkit.support.test; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import org.bukkit.support.provider.ClassReaderArgumentProvider; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ArgumentsSource; + +@Target({ElementType.METHOD, ElementType.ANNOTATION_TYPE}) +@Retention(RetentionPolicy.RUNTIME) +@ArgumentsSource(ClassReaderArgumentProvider.class) +@ParameterizedTest +public @interface ClassReaderTest { + + ClassType[] value() default {ClassType.BUKKIT, ClassType.CRAFT_BUKKIT, ClassType.MINECRAFT_UNMODIFIED, ClassType.MINECRAFT_MODIFIED}; + + Class[] excludedClasses() default {}; + + String[] excludedPackages() default {}; + + enum ClassType { + BUKKIT, + CRAFT_BUKKIT, + MINECRAFT_UNMODIFIED, + MINECRAFT_MODIFIED, + } +}