From 6734c5977151ed9135a578fb5b4c7dd3fa6999ce Mon Sep 17 00:00:00 2001 From: Robin Date: Sat, 25 Jul 2026 08:19:43 +0200 Subject: [PATCH 1/4] test: add per-platform plugin startup/DI regression tests Add plugin startup/smoke tests for every supported Connect platform (Velocity, Spigot, Bungee) plus a core shared-graph test, so a startup/DI-provisioning regression is caught in CI instead of by users. This is the prevention for the class of bug behind BOTH the Velocity 4.0.0 Guice 7 failure (BedrockIdentityKeyProvider unprovisionable) and the Java-26 Libp2pEndpointRuntime constructor arity failure. - core testFixtures StartupGraphProvisioning: pure-JDK reflective @Inject-graph walker + a Guice 7 injectable-constructor rule replica + javax.inject scan, generalizing BedrockVelocityGuice7ProvisioningTest to the whole per-platform graph so new DI classes are covered automatically. - Per-platform tests provision each platform's real Guice module graph (Velocity boots the real ProxyCommonModule + VelocityPlatformModule child injector; Spigot/Bungee wire Server/ProxyCommonModule with platform SDK stubbed, documented) and assert Guice-7 provisionability. The Velocity test fails on the pre-fix javax annotations and passes on the fix. - Java-26 reflective-arity class stays guarded by the existing signature tests (Libp2pEndpointRuntimeInitTest / Libp2pRuntimeBoundaryTest). - CI: pullrequest.yml runs ./gradlew build on a JDK 17/21 matrix; artifacts from 17. Rationale documented (Gradle 8.5 caps the runnable JDK). --- .github/workflows/pullrequest.yml | 22 +- AGENTS.md | 15 + bungee/build.gradle.kts | 12 + .../connect/BungeePluginStartupTest.java | 121 +++++++ core/build.gradle.kts | 1 + .../startup/PluginGraphStartupTest.java | 142 ++++++++ .../startup/StartupGraphProvisioning.java | 342 ++++++++++++++++++ spigot/build.gradle.kts | 2 + .../connect/SpigotPluginStartupTest.java | 122 +++++++ velocity/build.gradle.kts | 1 + .../connect/VelocityPluginStartupTest.java | 139 +++++++ 11 files changed, 914 insertions(+), 5 deletions(-) create mode 100644 bungee/src/test/java/com/minekube/connect/BungeePluginStartupTest.java create mode 100644 core/src/test/java/com/minekube/connect/startup/PluginGraphStartupTest.java create mode 100644 core/src/testFixtures/java/com/minekube/connect/startup/StartupGraphProvisioning.java create mode 100644 spigot/src/test/java/com/minekube/connect/SpigotPluginStartupTest.java create mode 100644 velocity/src/test/java/com/minekube/connect/VelocityPluginStartupTest.java diff --git a/.github/workflows/pullrequest.yml b/.github/workflows/pullrequest.yml index df264dfd3..cb335fde3 100644 --- a/.github/workflows/pullrequest.yml +++ b/.github/workflows/pullrequest.yml @@ -8,17 +8,29 @@ jobs: build: runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + # Run the full build (incl. the per-platform plugin startup tests) on more than one JDK so a + # JDK-version-dependent DI/reflective startup regression is caught in CI. 17 is the primary + # (release) toolchain and the only one that publishes artifacts; 21 is the highest JDK + # Gradle 8.5 can run on. The Java-26-class reflective/DI failures (Guice 7 provisioning, + # Libp2pEndpointRuntime constructor arity) are additionally guarded by signature-level + # reflective tests that are independent of the running JDK, so they are covered even though + # the build cannot run on JDK 26 until Gradle is upgraded. + java: ['17', '21'] + steps: - name: Checkout repository uses: actions/checkout@v4 with: fetch-depth: 0 # Fetch all history for git describe - - name: Set up JDK 17 + - name: Set up JDK ${{ matrix.java }} uses: actions/setup-java@v4 with: distribution: 'temurin' - java-version: '17' + java-version: ${{ matrix.java }} cache: 'gradle' - name: Setup Gradle @@ -29,21 +41,21 @@ jobs: - name: Archive artifacts (Connect Bungee) uses: actions/upload-artifact@v4 - if: success() + if: success() && matrix.java == '17' with: name: Connect Bungee path: bungee/build/libs/connect-bungee.jar - name: Archive artifacts (Connect Spigot) uses: actions/upload-artifact@v4 - if: success() + if: success() && matrix.java == '17' with: name: Connect Spigot path: spigot/build/libs/connect-spigot.jar - name: Archive artifacts (Connect Velocity) uses: actions/upload-artifact@v4 - if: success() + if: success() && matrix.java == '17' with: name: Connect Velocity path: velocity/build/libs/connect-velocity.jar diff --git a/AGENTS.md b/AGENTS.md index 25f6faab5..84ef39ea3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -99,6 +99,21 @@ Do not call production fixed from a Connect Java release alone. Confirm the released jar is in the hub image, the hub pod logs the expected plugin version, and Moxy accepts a tunnel with the same `connectorVersion`. +### Per-platform startup/DI regression guard + +- Each platform (Velocity/Spigot/Bungee) has a plugin startup smoke test + (`/src/test/.../PluginStartupTest`) plus core + `startup/PluginGraphStartupTest`, sharing the `core` test fixture + `core/src/testFixtures/.../startup/StartupGraphProvisioning`: it walks the real + `@Inject` graph and replays Guice 7's injectable-constructor rule, so a provider + made unprovisionable on any platform's injector (the Velocity 4 / Guice 7 class + of bug) fails the suite. The Velocity test fails on the pre-fix `javax.inject` + annotations and passes on the fix. Add new platform DI classes to that + platform test's `*GraphRoots()`. +- `pullrequest.yml` runs `./gradlew build` on a JDK matrix (17, 21); the + Java-26-class reflective bugs (Guice 7 DI, `Libp2pEndpointRuntime` ctor arity) + are guarded by signature-level tests independent of the running JDK. + ## Maintaining this file Keep this file for knowledge useful to almost every future agent session in this project. diff --git a/bungee/build.gradle.kts b/bungee/build.gradle.kts index 04b866e0d..6d8317b1e 100644 --- a/bungee/build.gradle.kts +++ b/bungee/build.gradle.kts @@ -6,6 +6,18 @@ var guavaVersion = "21.0" dependencies { api(projects.core) implementation("cloud.commandframework", "cloud-bungee", Versions.cloudVersion) + + testImplementation("org.junit.jupiter:junit-jupiter:5.10.5") + testImplementation("org.mockito:mockito-core:4.11.0") + testImplementation(testFixtures(projects.core)) + // Bungee plugin classes (e.g. BungeeListener) are needed on the test classpath for the + // per-platform startup test's reflective DI-graph walk. + testImplementation("net.md-5", "bungeecord-api", bungeeApiVersion) + testRuntimeOnly("org.junit.platform:junit-platform-launcher") +} + +tasks.test { + useJUnitPlatform() } relocate("com.google.inject") diff --git a/bungee/src/test/java/com/minekube/connect/BungeePluginStartupTest.java b/bungee/src/test/java/com/minekube/connect/BungeePluginStartupTest.java new file mode 100644 index 000000000..7e449e0a0 --- /dev/null +++ b/bungee/src/test/java/com/minekube/connect/BungeePluginStartupTest.java @@ -0,0 +1,121 @@ +/* + * Copyright (c) 2019-2022 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/Floodgate + */ + +package com.minekube.connect; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; + +import com.google.inject.AbstractModule; +import com.google.inject.Guice; +import com.google.inject.Injector; +import com.google.inject.name.Names; +import com.minekube.connect.api.ConnectApi; +import com.minekube.connect.api.logger.ConnectLogger; +import com.minekube.connect.api.packet.PacketHandlers; +import com.minekube.connect.bedrock.BedrockAdmissionCoordinator; +import com.minekube.connect.bedrock.BedrockIdentityEnforcer; +import com.minekube.connect.bedrock.BedrockIdentityKeyProvider; +import com.minekube.connect.inject.CommonPlatformInjector; +import com.minekube.connect.listener.BungeeListener; +import com.minekube.connect.module.ProxyCommonModule; +import com.minekube.connect.platform.util.PlatformUtils; +import com.minekube.connect.startup.StartupGraphProvisioning; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Per-platform startup / smoke test for the BungeeCord plugin. + * + *

Asserts the Bungee plugin DI graph — the {@code ProxyCommonModule} + {@code BungeePlatformModule} + * graph {@link BungeePlugin#onLoad()} builds — is provisionable under Velocity 4-class Guice 7 and + * that a real Guice injector wires the shared graph up cleanly with no {@code ProvisionException}. + * Like every platform's injector it carries the Bedrock identity graph, so it fails on the pre-fix + * commit (javax-annotated Bedrock providers) and passes on the fix — see {@link + * StartupGraphProvisioning}. + * + *

Test limitation. {@code BungeePlugin} extends Bungee's {@code Plugin} and relies on + * server-side {@code init(...)} for its data folder/proxy, so the real-provision half wires the + * Bungee proxy graph ({@code ProxyCommonModule}) with the platform-supplied bindings stubbed rather + * than booting a BungeeCord proxy. That still provisions the full Connect object graph a DI + * regression would break; the Guice-7 provisionability check additionally covers the Bungee-specific + * DI classes ({@code BungeeListener}, …). + */ +class BungeePluginStartupTest { + @TempDir Path tempDir; + + private static List> bungeeGraphRoots() { + List> roots = new ArrayList<>(StartupGraphProvisioning.coreRuntimeGraphRoots()); + roots.add(BungeeListener.class); + return roots; + } + + @Test + void bungeePluginGraphIsProvisionableUnderGuice7() { + Set> graph = StartupGraphProvisioning.reachableInjectedTypes(bungeeGraphRoots()); + + List violations = StartupGraphProvisioning.guice7ProvisioningViolations(graph); + + assertTrue(violations.isEmpty(), + "Bungee plugin DI classes are not provisionable under Velocity 4.0.0's Guice 7:\n" + + String.join("\n", violations)); + assertTrue(graph.contains(BedrockIdentityKeyProvider.class), + "walk must reach BedrockIdentityKeyProvider (the class that failed on Velocity 4)"); + } + + /** + * Real Guice provisioning of the Bungee proxy graph ({@code ProxyCommonModule}) with the + * platform-supplied bindings stubbed. Proves the modules wire up and the Bedrock/config graph + * resolves without a provisioning error. Config load is not driven (it makes a network call); + * the Guice-7 check above covers the parts not instantiated here. + */ + @Test + void bungeeProxyGraphProvisionsKeySingletonsWithoutError() { + Injector injector = Guice.createInjector( + new ProxyCommonModule(tempDir), + new AbstractModule() { + @Override + protected void configure() { + bind(ConnectLogger.class).toInstance(mock(ConnectLogger.class)); + bind(PlatformUtils.class).toInstance(mock(PlatformUtils.class)); + bind(CommonPlatformInjector.class) + .toInstance(mock(CommonPlatformInjector.class)); + bind(String.class).annotatedWith(Names.named("platformName")) + .toInstance("BungeeCord"); + } + }); + + assertDoesNotThrow(() -> injector.getInstance(ConnectApi.class)); + assertDoesNotThrow(() -> injector.getInstance(PacketHandlers.class)); + assertDoesNotThrow(() -> injector.getInstance(BedrockAdmissionCoordinator.class)); + assertDoesNotThrow(() -> injector.getInstance(BedrockIdentityEnforcer.class)); + assertDoesNotThrow(() -> injector.getInstance(BedrockIdentityKeyProvider.class)); + } +} diff --git a/core/build.gradle.kts b/core/build.gradle.kts index 26a1ffacb..82ddfac9d 100644 --- a/core/build.gradle.kts +++ b/core/build.gradle.kts @@ -3,6 +3,7 @@ import net.kyori.blossom.BlossomExtension plugins { idea // used to let Intellij recognize protobuf generated sources + `java-test-fixtures` // shared startup/DI provisioning checks reused by per-platform tests id("net.kyori.blossom") id("com.google.protobuf") } diff --git a/core/src/test/java/com/minekube/connect/startup/PluginGraphStartupTest.java b/core/src/test/java/com/minekube/connect/startup/PluginGraphStartupTest.java new file mode 100644 index 000000000..11225dd59 --- /dev/null +++ b/core/src/test/java/com/minekube/connect/startup/PluginGraphStartupTest.java @@ -0,0 +1,142 @@ +/* + * Copyright (c) 2019-2022 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/Floodgate + */ + +package com.minekube.connect.startup; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; + +import com.google.inject.AbstractModule; +import com.google.inject.Guice; +import com.google.inject.Injector; +import com.google.inject.name.Names; +import com.minekube.connect.api.ConnectApi; +import com.minekube.connect.api.logger.ConnectLogger; +import com.minekube.connect.api.packet.PacketHandlers; +import com.minekube.connect.bedrock.BedrockAdmissionCoordinator; +import com.minekube.connect.bedrock.BedrockIdentityEnforcer; +import com.minekube.connect.bedrock.BedrockIdentityKeyProvider; +import com.minekube.connect.config.ConfigHolder; +import com.minekube.connect.inject.CommonPlatformInjector; +import com.minekube.connect.module.ServerCommonModule; +import com.minekube.connect.platform.util.PlatformUtils; +import java.nio.file.Path; +import java.util.List; +import java.util.Set; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Startup smoke test for the shared Connect plugin object graph — the portion of the DI graph every + * platform (Velocity/Spigot/Bungee) provisions identically. It is the workhorse behind the + * per-platform startup tests: those reuse {@link StartupGraphProvisioning#coreRuntimeGraphRoots()} + * and add their platform-specific roots on top. + * + *

This is the captain-directed prevention for the class of DI/startup regression that produced + * the Velocity 4.0.0 {@code BedrockIdentityKeyProvider} "Cant create plugin connect" failure. It + * fails on the pre-fix code (which annotated the Bedrock providers with {@code javax.inject}, + * invisible to Velocity 4's Guice 7) and passes on the fixed code. See + * {@link StartupGraphProvisioning} for why a Guice 7 rule replica is used instead of a live Guice 7 + * injector, and {@code BedrockVelocityGuice7ProvisioningTest} for the original narrower guard. + * + *

The reflective-constructor regression class (the Java-26 {@code Libp2pEndpointRuntime} + * constructor arity mismatch) is guarded by {@code Libp2pEndpointRuntimeInitTest} and + * {@code Libp2pRuntimeBoundaryTest}, which run in this same module's test task; {@code Libp2pEndpoint} + * and {@code Libp2pTunnelTransport} are also part of the graph asserted here. + */ +class PluginGraphStartupTest { + @TempDir Path tempDir; + + /** + * The whole shared runtime graph must be provisionable under Velocity-4-class Guice 7. This is + * the assertion that fails on the pre-fix commit (javax-annotated Bedrock providers) and passes + * on the fixed code. + */ + @Test + void sharedRuntimeGraphIsProvisionableUnderGuice7() { + Set> graph = + StartupGraphProvisioning.reachableInjectedTypes( + StartupGraphProvisioning.coreRuntimeGraphRoots()); + + List violations = StartupGraphProvisioning.guice7ProvisioningViolations(graph); + + assertTrue(violations.isEmpty(), + "Connect DI classes on the shared plugin graph are not provisionable under " + + "Velocity 4.0.0's Guice 7:\n" + String.join("\n", violations)); + } + + /** + * Guards the guard: the reflective walk must actually reach the class that regressed + * ({@code BedrockIdentityKeyProvider}) and the rest of the Bedrock identity graph, otherwise + * {@link #sharedRuntimeGraphIsProvisionableUnderGuice7()} could pass vacuously by covering + * nothing. + */ + @Test + void reflectiveWalkReachesTheClassesThatRegressed() { + Set> graph = + StartupGraphProvisioning.reachableInjectedTypes( + StartupGraphProvisioning.coreRuntimeGraphRoots()); + + assertTrue(graph.contains(BedrockIdentityKeyProvider.class), + "walk must reach BedrockIdentityKeyProvider (the class that failed on Velocity 4)"); + assertTrue(graph.contains(BedrockAdmissionCoordinator.class), + "walk must reach BedrockAdmissionCoordinator"); + assertTrue(graph.contains(BedrockIdentityEnforcer.class), + "walk must reach BedrockIdentityEnforcer"); + } + + /** + * Real Guice-6 provisioning of the hermetic slice of the server graph: proves the modules wire + * up and the Bedrock/config graph resolves without a {@code ProvisionException}. Config loading + * ({@code ConnectPlatform.init()}) is intentionally not driven here because it performs a + * network call to generate an endpoint name; the reflective Guice-7 check above covers the parts + * of the graph this real provision does not instantiate. + */ + @Test + void serverGraphProvisionsKeySingletonsWithoutError() { + Injector injector = Guice.createInjector( + new ServerCommonModule(tempDir), + new AbstractModule() { + @Override + protected void configure() { + // Bindings the platform module would normally supply. + bind(ConnectLogger.class).toInstance(mock(ConnectLogger.class)); + bind(PlatformUtils.class).toInstance(mock(PlatformUtils.class)); + bind(CommonPlatformInjector.class) + .toInstance(mock(CommonPlatformInjector.class)); + bind(String.class).annotatedWith(Names.named("platformName")) + .toInstance("test"); + } + }); + + assertNotNull(injector.getInstance(ConnectApi.class)); + assertNotNull(injector.getInstance(ConfigHolder.class)); + assertNotNull(injector.getInstance(BedrockAdmissionCoordinator.class)); + assertNotNull(injector.getInstance(BedrockIdentityEnforcer.class)); + assertNotNull(injector.getInstance(BedrockIdentityKeyProvider.class)); + assertNotNull(injector.getInstance(PacketHandlers.class)); + } +} diff --git a/core/src/testFixtures/java/com/minekube/connect/startup/StartupGraphProvisioning.java b/core/src/testFixtures/java/com/minekube/connect/startup/StartupGraphProvisioning.java new file mode 100644 index 000000000..9e0f7b16f --- /dev/null +++ b/core/src/testFixtures/java/com/minekube/connect/startup/StartupGraphProvisioning.java @@ -0,0 +1,342 @@ +/* + * Copyright (c) 2019-2022 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/Floodgate + */ + +package com.minekube.connect.startup; + +import com.minekube.connect.ConnectPlatform; +import com.minekube.connect.bedrock.BedrockAdmissionCoordinator; +import com.minekube.connect.bedrock.BedrockIdentityEnforcer; +import com.minekube.connect.bedrock.BedrockIdentityKeyProvider; +import com.minekube.connect.bedrock.VerifiedBedrockIdentityRegistry; +import com.minekube.connect.packet.PacketHandlersImpl; +import com.minekube.connect.register.WatchHealthServer; +import com.minekube.connect.register.WatcherRegister; +import com.minekube.connect.tunnel.Tunneler; +import com.minekube.connect.tunnel.WebSocketTunnelTransport; +import com.minekube.connect.tunnel.p2p.Libp2pEndpoint; +import com.minekube.connect.tunnel.p2p.Libp2pTunnelTransport; +import com.minekube.connect.util.Metrics; +import com.minekube.connect.util.UpdateChecker; +import com.minekube.connect.watch.WatchClient; +import java.lang.annotation.Annotation; +import java.lang.reflect.AnnotatedElement; +import java.lang.reflect.Constructor; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.lang.reflect.Parameter; +import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Deque; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; + +/** + * Reusable startup / DI-provisioning checks shared by the per-platform plugin startup tests + * ({@code VelocityPluginStartupTest}, {@code SpigotPluginStartupTest}, + * {@code BungeePluginStartupTest}) and the core {@code PluginGraphStartupTest}. + * + *

It is static and dependency-free (pure JDK reflection) so every platform module can consume it + * as a {@code testFixtures} artifact without pulling Guice 7 or {@code jakarta.inject} onto its + * classpath. It has two jobs: + * + *

    + *
  1. {@link #reachableInjectedTypes(Collection)} walks the real {@code @Inject} object graph the + * plugin provisions from a set of root types, so a new DI class added anywhere on a platform's + * graph is covered automatically instead of having to be re-listed by hand.
    2. + *
  2. {@link #guice7ProvisioningViolations(Collection)} replays Guice 7's injectable-constructor + * discovery rule (the Guice shipped by Velocity 4.0.0) plus a {@code javax.inject} scan, so a + * regression that makes any provider unprovisionable — like the 0.12.x + * {@code BedrockIdentityKeyProvider} "Cant create plugin connect" failure — is caught on every + * platform's injector, not just Velocity's.
    3. + *
+ * + *

Why a Guice 7 replica instead of a real Guice 7 injector. This repo compiles and tests + * against Guice 6 (see {@code Versions.guiceVersion}), which recognizes BOTH {@code javax.inject} + * and {@code com.google.inject} injectable-constructor markers. Provisioning the graph through the + * real Guice-6 injector therefore succeeds even with the {@code javax.inject} annotations that broke + * Velocity 4.0.0 and cannot reproduce the failure. No Guice 7 / Velocity 4.0.0 harness is on the + * classpath, so this fixture replicates Guice 7's exact injectable-constructor discovery rule and + * forbids {@code javax.inject} annotations across the plugin DI graph. The checks fail before the + * Velocity-4 DI fix and pass after it. This mirrors the reasoning in + * {@code BedrockVelocityGuice7ProvisioningTest}, generalized from a hand-listed set of classes to + * the whole per-platform graph. + */ +public final class StartupGraphProvisioning { + + private static final String CONNECT_PACKAGE = "com.minekube.connect"; + + /** Injectable-constructor markers that Guice recognizes across versions (used for traversal). */ + private static final Set INJECT_ANNOTATIONS = Set.of( + "com.google.inject.Inject", "javax.inject.Inject", "jakarta.inject.Inject"); + + /** + * Markers Guice 7 accepts as an injectable-constructor annotation. Referenced by + * fully-qualified name so this fixture needs neither Guice 7 nor {@code jakarta.inject} on its + * classpath. + */ + private static final Set GUICE7_INJECT_ANNOTATIONS = Set.of( + "com.google.inject.Inject", "jakarta.inject.Inject"); + + private static final String JAVAX_INJECT_PREFIX = "javax.inject."; + + private StartupGraphProvisioning() { + } + + /** + * Core types Guice constructs while the plugin loads and enables, on every platform. These are + * the singletons the platform entrypoints materialize through {@code getInstance(...)} / + * {@code asEagerSingleton()} plus the Bedrock identity graph reached during construction. Used + * as traversal roots by all per-platform startup tests so the shared graph is exercised + * identically on each injector. + */ + public static List> coreRuntimeGraphRoots() { + return List.of( + ConnectPlatform.class, + PacketHandlersImpl.class, + Libp2pEndpoint.class, + Libp2pTunnelTransport.class, + WebSocketTunnelTransport.class, + Tunneler.class, + WatcherRegister.class, + WatchHealthServer.class, + WatchClient.class, + UpdateChecker.class, + Metrics.class, + BedrockIdentityEnforcer.class, + BedrockAdmissionCoordinator.class, + BedrockIdentityKeyProvider.class, + VerifiedBedrockIdentityRegistry.class); + } + + /** + * Transitively walks the {@code @Inject} object graph reachable from {@code roots} and returns + * every concrete {@code com.minekube.connect.*} class Guice would just-in-time instantiate to + * satisfy those injection points. Edges followed: the single injectable constructor's + * parameters, {@code @Inject} fields, and {@code @Inject} method parameters, across each class's + * hierarchy. Interfaces/abstract types and non-Connect types are traversal boundaries (Guice + * resolves them through explicit bindings or platform SDK objects, not JIT construction). + * + *

The returned set is exactly the population that a DI regression could make unprovisionable, + * so it is what {@link #guice7ProvisioningViolations(Collection)} is run against. + */ + public static Set> reachableInjectedTypes(Collection> roots) { + Set> discovered = new LinkedHashSet<>(); + Deque> queue = new ArrayDeque<>(); + Set> visited = new LinkedHashSet<>(); + + // A root that Guice itself constructs (has an injectable constructor) is part of the graph + // to check; roots created inside @Provides methods (no injectable constructor) are seeds + // only and are covered through the dependencies they expose. + for (Class root : roots) { + if (isConnectConcrete(root) && hasInjectConstructor(root)) { + discovered.add(root); + } + enqueue(root, queue, visited); + } + + while (!queue.isEmpty()) { + Class type = queue.poll(); + for (Class dependency : injectedDependencies(type)) { + if (isConnectConcrete(dependency)) { + discovered.add(dependency); + } + enqueue(dependency, queue, visited); + } + } + return discovered; + } + + /** + * Applies Guice 7's injectable-constructor discovery rule and a {@code javax.inject} scan to + * each type and returns a human-readable violation for every type Guice 7 could NOT provision + * (or that leans on {@code javax.inject} annotations Guice 7 ignores). An empty list means the + * whole graph is provisionable on a Velocity-4-class (Guice 7) injector. + */ + public static List guice7ProvisioningViolations(Collection> types) { + List violations = new ArrayList<>(); + for (Class type : types) { + violations.addAll(guice7ProvisioningViolations(type)); + } + return violations; + } + + private static List guice7ProvisioningViolations(Class type) { + List violations = new ArrayList<>(); + + List> injectConstructors = new ArrayList<>(); + List> guice7InjectConstructors = new ArrayList<>(); + for (Constructor constructor : type.getDeclaredConstructors()) { + if (hasAnnotationNamed(constructor, INJECT_ANNOTATIONS)) { + injectConstructors.add(constructor); + } + if (hasAnnotationNamed(constructor, GUICE7_INJECT_ANNOTATIONS)) { + guice7InjectConstructors.add(constructor); + } + } + + // A type with no @Inject constructor is not just-in-time constructed by Guice: it is + // supplied by an @Provides method / toInstance / linked binding (e.g. ConfigLoader, + // SimpleConnectApi, BedrockIdentityReadiness are built with `new` inside CommonModule), so + // Guice never inspects it for an injectable constructor and the javax regression cannot + // apply. Only the classes Guice itself instantiates via constructor injection are subject to + // Guice 7's discovery rule — exactly the population the Velocity 4 failure came from. + if (injectConstructors.isEmpty()) { + return violations; + } + + if (guice7InjectConstructors.size() > 1) { + violations.add(type.getName() + " has more than one Guice 7 @Inject constructor: " + + guice7InjectConstructors); + } + + boolean provisionable = + guice7InjectConstructors.size() == 1 || hasInjectableNoArgConstructor(type); + if (!provisionable) { + violations.add(type.getName() + " has an @Inject constructor that Guice 7 does NOT " + + "recognize (it uses only javax.inject, which Guice 7 on Velocity 4.0.0 " + + "ignores) and NO no-arg constructor, so Guice 7 cannot provision it — " + + "reproducing the \"Cant create plugin connect\" failure."); + } + + for (String offender : javaxInjectAnnotations(type)) { + violations.add(type.getName() + " uses a javax.inject annotation invisible to Guice 7 " + + "on Velocity 4.0.0: " + offender); + } + return violations; + } + + // --- graph traversal ---------------------------------------------------------------------- + + private static void enqueue(Class type, Deque> queue, Set> visited) { + if (type != null && type.getName().startsWith(CONNECT_PACKAGE) && visited.add(type)) { + queue.add(type); + } + } + + private static Set> injectedDependencies(Class type) { + Set> dependencies = new LinkedHashSet<>(); + for (Class current = type; current != null && current != Object.class; + current = current.getSuperclass()) { + try { + Constructor injectConstructor = injectConstructor(current); + if (injectConstructor != null) { + for (Class parameterType : injectConstructor.getParameterTypes()) { + dependencies.add(parameterType); + } + } + for (Field field : current.getDeclaredFields()) { + if (hasAnnotationNamed(field, INJECT_ANNOTATIONS)) { + dependencies.add(field.getType()); + } + } + for (Method method : current.getDeclaredMethods()) { + if (hasAnnotationNamed(method, INJECT_ANNOTATIONS)) { + for (Class parameterType : method.getParameterTypes()) { + dependencies.add(parameterType); + } + } + } + } catch (NoClassDefFoundError | RuntimeException ignored) { + // A dependency type not on this module's test classpath is not part of the Connect + // DI graph we assert over; skip it defensively rather than fail the walk. + } + } + return dependencies; + } + + private static Constructor injectConstructor(Class type) { + for (Constructor constructor : type.getDeclaredConstructors()) { + if (hasAnnotationNamed(constructor, INJECT_ANNOTATIONS)) { + return constructor; + } + } + return null; + } + + private static boolean hasInjectConstructor(Class type) { + return injectConstructor(type) != null; + } + + private static boolean isConnectConcrete(Class type) { + return type.getName().startsWith(CONNECT_PACKAGE) + && !type.isInterface() + && !type.isAnnotation() + && !type.isEnum() + && !Modifier.isAbstract(type.getModifiers()); + } + + // --- Guice 7 injectable-constructor rule replica ------------------------------------------ + + private static boolean hasInjectableNoArgConstructor(Class type) { + try { + Constructor noArg = type.getDeclaredConstructor(); + return !Modifier.isPrivate(noArg.getModifiers()); + } catch (NoSuchMethodException e) { + return false; + } + } + + private static boolean hasAnnotationNamed(AnnotatedElement element, Set annotationNames) { + for (Annotation annotation : element.getAnnotations()) { + if (annotationNames.contains(annotation.annotationType().getName())) { + return true; + } + } + return false; + } + + private static List javaxInjectAnnotations(Class type) { + List offenders = new ArrayList<>(); + collectJavaxInject(type, offenders); // class-level scope, e.g. @Singleton + for (Constructor constructor : type.getDeclaredConstructors()) { + collectJavaxInject(constructor, offenders); + for (Parameter parameter : constructor.getParameters()) { + collectJavaxInject(parameter, offenders); + } + } + for (Field field : type.getDeclaredFields()) { + collectJavaxInject(field, offenders); + } + for (Method method : type.getDeclaredMethods()) { + collectJavaxInject(method, offenders); + for (Parameter parameter : method.getParameters()) { + collectJavaxInject(parameter, offenders); + } + } + return offenders; + } + + private static void collectJavaxInject(AnnotatedElement element, List offenders) { + for (Annotation annotation : element.getAnnotations()) { + String name = annotation.annotationType().getName(); + if (name.startsWith(JAVAX_INJECT_PREFIX)) { + offenders.add(name + " on " + element); + } + } + } +} diff --git a/spigot/build.gradle.kts b/spigot/build.gradle.kts index cbab061b8..323b0be8a 100644 --- a/spigot/build.gradle.kts +++ b/spigot/build.gradle.kts @@ -16,6 +16,8 @@ dependencies { } testImplementation("org.junit.jupiter:junit-jupiter:5.10.5") + testImplementation("org.mockito:mockito-core:4.11.0") + testImplementation(testFixtures(projects.core)) testImplementation("com.mojang", "authlib", authlibVersion) testImplementation("io.netty", "netty-transport", Versions.nettyVersion) testImplementation("dev.folia", "folia-api", Versions.spigotVersion) { diff --git a/spigot/src/test/java/com/minekube/connect/SpigotPluginStartupTest.java b/spigot/src/test/java/com/minekube/connect/SpigotPluginStartupTest.java new file mode 100644 index 000000000..c6cf20a48 --- /dev/null +++ b/spigot/src/test/java/com/minekube/connect/SpigotPluginStartupTest.java @@ -0,0 +1,122 @@ +/* + * Copyright (c) 2019-2022 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/Floodgate + */ + +package com.minekube.connect; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; + +import com.google.inject.AbstractModule; +import com.google.inject.Guice; +import com.google.inject.Injector; +import com.google.inject.name.Names; +import com.minekube.connect.addon.data.SpigotDataAddon; +import com.minekube.connect.api.ConnectApi; +import com.minekube.connect.api.logger.ConnectLogger; +import com.minekube.connect.api.packet.PacketHandlers; +import com.minekube.connect.bedrock.BedrockAdmissionCoordinator; +import com.minekube.connect.bedrock.BedrockIdentityEnforcer; +import com.minekube.connect.bedrock.BedrockIdentityKeyProvider; +import com.minekube.connect.inject.CommonPlatformInjector; +import com.minekube.connect.module.ServerCommonModule; +import com.minekube.connect.platform.util.PlatformUtils; +import com.minekube.connect.startup.StartupGraphProvisioning; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Per-platform startup / smoke test for the Bukkit/Spigot/Paper plugin. + * + *

Asserts the Spigot plugin DI graph — the {@code ServerCommonModule} + {@code SpigotPlatform} + * graph {@link SpigotPlugin#onLoad()} builds — is provisionable under Velocity 4-class Guice 7 and + * that a real Guice injector wires the shared graph up cleanly with no {@code ProvisionException}. + * Like every platform's injector it carries the Bedrock identity graph, so it fails on the pre-fix + * commit (javax-annotated Bedrock providers) and passes on the fix — see {@link + * StartupGraphProvisioning}. + * + *

Test limitation. {@code SpigotPlugin} extends Bukkit's {@code JavaPlugin} and cannot be + * instantiated outside a running server, so the real-provision half wires the Spigot server graph + * ({@code ServerCommonModule}) with the platform-supplied bindings stubbed, rather than booting a + * Bukkit server. That still provisions the full Connect object graph a DI regression would break; + * the Guice-7 provisionability check additionally covers the Spigot-specific DI classes + * ({@code SpigotPlatform}, {@code SpigotDataAddon}, …). + */ +class SpigotPluginStartupTest { + @TempDir Path tempDir; + + private static List> spigotGraphRoots() { + List> roots = new ArrayList<>(StartupGraphProvisioning.coreRuntimeGraphRoots()); + roots.add(SpigotPlatform.class); + roots.add(SpigotDataAddon.class); + return roots; + } + + @Test + void spigotPluginGraphIsProvisionableUnderGuice7() { + Set> graph = StartupGraphProvisioning.reachableInjectedTypes(spigotGraphRoots()); + + List violations = StartupGraphProvisioning.guice7ProvisioningViolations(graph); + + assertTrue(violations.isEmpty(), + "Spigot plugin DI classes are not provisionable under Velocity 4.0.0's Guice 7:\n" + + String.join("\n", violations)); + assertTrue(graph.contains(BedrockIdentityKeyProvider.class), + "walk must reach BedrockIdentityKeyProvider (the class that failed on Velocity 4)"); + } + + /** + * Real Guice provisioning of the Spigot server graph ({@code ServerCommonModule}) with the + * platform-supplied bindings stubbed. Proves the modules wire up and the Bedrock/config graph + * resolves without a provisioning error. Config load is not driven (it makes a network call); + * the Guice-7 check above covers the parts not instantiated here. + */ + @Test + void spigotServerGraphProvisionsKeySingletonsWithoutError() { + Injector injector = Guice.createInjector( + new ServerCommonModule(tempDir), + new AbstractModule() { + @Override + protected void configure() { + bind(ConnectLogger.class).toInstance(mock(ConnectLogger.class)); + bind(PlatformUtils.class).toInstance(mock(PlatformUtils.class)); + bind(CommonPlatformInjector.class) + .toInstance(mock(CommonPlatformInjector.class)); + bind(String.class).annotatedWith(Names.named("platformName")) + .toInstance("Spigot"); + } + }); + + assertDoesNotThrow(() -> injector.getInstance(ConnectApi.class)); + assertDoesNotThrow(() -> injector.getInstance(PacketHandlers.class)); + assertDoesNotThrow(() -> injector.getInstance(BedrockAdmissionCoordinator.class)); + assertDoesNotThrow(() -> injector.getInstance(BedrockIdentityEnforcer.class)); + assertDoesNotThrow(() -> injector.getInstance(BedrockIdentityKeyProvider.class)); + } +} diff --git a/velocity/build.gradle.kts b/velocity/build.gradle.kts index 11f27eae4..8a22001ee 100644 --- a/velocity/build.gradle.kts +++ b/velocity/build.gradle.kts @@ -16,6 +16,7 @@ dependencies { testImplementation("org.junit.jupiter:junit-jupiter:5.10.5") testImplementation("io.netty", "netty-transport", Versions.nettyVersion) testImplementation("org.mockito:mockito-core:4.11.0") + testImplementation(testFixtures(projects.core)) testRuntimeOnly("com.velocitypowered", "velocity-api", velocityVersion) testRuntimeOnly("org.junit.platform:junit-platform-launcher") } diff --git a/velocity/src/test/java/com/minekube/connect/VelocityPluginStartupTest.java b/velocity/src/test/java/com/minekube/connect/VelocityPluginStartupTest.java new file mode 100644 index 000000000..760fe6a8d --- /dev/null +++ b/velocity/src/test/java/com/minekube/connect/VelocityPluginStartupTest.java @@ -0,0 +1,139 @@ +/* + * Copyright (c) 2019-2022 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/Floodgate + */ + +package com.minekube.connect; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; + +import com.google.inject.AbstractModule; +import com.google.inject.Guice; +import com.google.inject.Injector; +import com.minekube.connect.api.ConnectApi; +import com.minekube.connect.api.inject.PlatformInjector; +import com.minekube.connect.api.logger.ConnectLogger; +import com.minekube.connect.api.packet.PacketHandlers; +import com.minekube.connect.bedrock.BedrockAdmissionCoordinator; +import com.minekube.connect.bedrock.BedrockIdentityEnforcer; +import com.minekube.connect.bedrock.BedrockIdentityKeyProvider; +import com.minekube.connect.listener.VelocityListener; +import com.minekube.connect.module.ProxyCommonModule; +import com.minekube.connect.module.VelocityPlatformModule; +import com.minekube.connect.startup.StartupGraphProvisioning; +import com.velocitypowered.api.event.EventManager; +import com.velocitypowered.api.plugin.annotation.DataDirectory; +import com.velocitypowered.api.proxy.ProxyServer; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.slf4j.Logger; + +/** + * Per-platform startup / smoke test for the Velocity plugin. + * + *

This is the direct regression guard for the Velocity 4.0.0 "Cant create plugin connect" + * failure: it asserts the whole Velocity plugin DI graph — {@code ProxyCommonModule} + + * {@code VelocityPlatformModule}, the exact modules {@link VelocityPlugin} builds — is provisionable + * under Velocity 4's Guice 7, and that a real Guice injector wires the graph up cleanly with no + * {@code ProvisionException}/{@code CreationException}. + * + *

Fails before the fix, passes after. {@link #velocityPluginGraphIsProvisionableUnderGuice7()} + * fails on the pre-fix commit (the Bedrock providers were annotated with {@code javax.inject}, which + * Guice 7 ignores) and passes on the fixed code. See {@link StartupGraphProvisioning} for why a + * Guice 7 rule replica is used rather than a live Guice 7 injector (this repo builds against Guice + * 6, which accepts both annotation packages and so cannot reproduce the failure through a real + * provision). + */ +class VelocityPluginStartupTest { + @TempDir Path tempDir; + + /** Velocity-specific classes Guice touches, on top of the shared core graph. */ + private static List> velocityGraphRoots() { + List> roots = new ArrayList<>(StartupGraphProvisioning.coreRuntimeGraphRoots()); + roots.add(VelocityPlugin.class); + roots.add(VelocityListener.class); + return roots; + } + + /** + * The reintroduction-detector: every {@code com.minekube.connect} class Guice would construct on + * the Velocity injector must be provisionable under Guice 7. This is the assertion that fails on + * the pre-fix code and passes on the fix. + */ + @Test + void velocityPluginGraphIsProvisionableUnderGuice7() { + Set> graph = + StartupGraphProvisioning.reachableInjectedTypes(velocityGraphRoots()); + + List violations = StartupGraphProvisioning.guice7ProvisioningViolations(graph); + + assertTrue(violations.isEmpty(), + "Velocity plugin DI classes are not provisionable under Velocity 4.0.0's Guice 7:\n" + + String.join("\n", violations)); + // Guard the guard: the walk must actually reach the class that regressed. + assertTrue(graph.contains(BedrockIdentityKeyProvider.class), + "walk must reach BedrockIdentityKeyProvider (the class that failed on Velocity 4)"); + } + + /** + * Real Guice provisioning of the actual Velocity module graph. Boots the same child injector + * {@link VelocityPlugin} constructs ({@code ProxyCommonModule} + {@code VelocityPlatformModule}) + * and resolves the plugin's key singletons, proving the graph wires up with no provisioning + * error. {@code ConnectPlatform.init()} (config load) is intentionally not driven — it performs + * a network call to generate an endpoint name — so the hermetic singletons are resolved + * directly; the Guice-7 check above covers the rest of the graph. + */ + @Test + void velocityModuleGraphProvisionsKeySingletonsWithoutError() { + Injector parent = Guice.createInjector(new AbstractModule() { + @Override + protected void configure() { + bind(ProxyServer.class).toInstance(mock(ProxyServer.class)); + bind(EventManager.class).toInstance(mock(EventManager.class)); + bind(Logger.class).toInstance(mock(Logger.class)); + // Lets Guice just-in-time bind VelocityPlugin (needed by the listener-registration + // provider) without constructing it — construction would run ConnectPlatform.init(), + // which makes a network call. We never request VelocityPlugin here. + bind(Path.class).annotatedWith(DataDirectory.class).toInstance(tempDir); + } + }); + + Injector child = parent.createChildInjector( + new ProxyCommonModule(tempDir), + new VelocityPlatformModule(parent)); + + assertDoesNotThrow(() -> child.getInstance(ConnectLogger.class)); + assertDoesNotThrow(() -> child.getInstance(ConnectApi.class)); + assertDoesNotThrow(() -> child.getInstance(PlatformInjector.class)); + assertDoesNotThrow(() -> child.getInstance(PacketHandlers.class)); + assertDoesNotThrow(() -> child.getInstance(BedrockAdmissionCoordinator.class)); + assertDoesNotThrow(() -> child.getInstance(BedrockIdentityEnforcer.class)); + assertDoesNotThrow(() -> child.getInstance(BedrockIdentityKeyProvider.class)); + } +} From e30363d31c1ae5653451b0ec094b69da0d8d431e Mon Sep 17 00:00:00 2001 From: Robin Date: Sat, 25 Jul 2026 08:38:13 +0200 Subject: [PATCH 2/4] no-mistakes(review): Expand startup graph coverage for member-injected platform bindings --- .../connect/BungeePluginStartupTest.java | 15 ++- .../startup/PluginGraphStartupTest.java | 3 + .../startup/StartupGraphProvisioning.java | 98 ++++++++++++------- .../connect/SpigotPluginStartupTest.java | 25 ++++- .../connect/VelocityPluginStartupTest.java | 14 ++- 5 files changed, 116 insertions(+), 39 deletions(-) diff --git a/bungee/src/test/java/com/minekube/connect/BungeePluginStartupTest.java b/bungee/src/test/java/com/minekube/connect/BungeePluginStartupTest.java index 7e449e0a0..52545bfb6 100644 --- a/bungee/src/test/java/com/minekube/connect/BungeePluginStartupTest.java +++ b/bungee/src/test/java/com/minekube/connect/BungeePluginStartupTest.java @@ -40,10 +40,15 @@ import com.minekube.connect.bedrock.BedrockIdentityEnforcer; import com.minekube.connect.bedrock.BedrockIdentityKeyProvider; import com.minekube.connect.inject.CommonPlatformInjector; +import com.minekube.connect.inject.bungee.BungeeInjector; import com.minekube.connect.listener.BungeeListener; +import com.minekube.connect.listener.BungeeListenerRegistration; import com.minekube.connect.module.ProxyCommonModule; import com.minekube.connect.platform.util.PlatformUtils; +import com.minekube.connect.pluginmessage.BungeeSkinApplier; import com.minekube.connect.startup.StartupGraphProvisioning; +import com.minekube.connect.util.BungeeCommandUtil; +import com.minekube.connect.util.BungeePlatformUtils; import java.nio.file.Path; import java.util.ArrayList; import java.util.List; @@ -55,7 +60,8 @@ * Per-platform startup / smoke test for the BungeeCord plugin. * *

Asserts the Bungee plugin DI graph — the {@code ProxyCommonModule} + {@code BungeePlatformModule} - * graph {@link BungeePlugin#onLoad()} builds — is provisionable under Velocity 4-class Guice 7 and + * graph {@link BungeePlugin#onLoad()} builds — including the concrete platform/listener bindings — + * is provisionable under Velocity 4-class Guice 7 and * that a real Guice injector wires the shared graph up cleanly with no {@code ProvisionException}. * Like every platform's injector it carries the Bedrock identity graph, so it fails on the pre-fix * commit (javax-annotated Bedrock providers) and passes on the fix — see {@link @@ -74,6 +80,11 @@ class BungeePluginStartupTest { private static List> bungeeGraphRoots() { List> roots = new ArrayList<>(StartupGraphProvisioning.coreRuntimeGraphRoots()); roots.add(BungeeListener.class); + roots.add(BungeeCommandUtil.class); + roots.add(BungeePlatformUtils.class); + roots.add(BungeeInjector.class); + roots.add(BungeeSkinApplier.class); + roots.add(BungeeListenerRegistration.class); return roots; } @@ -88,6 +99,8 @@ void bungeePluginGraphIsProvisionableUnderGuice7() { + String.join("\n", violations)); assertTrue(graph.contains(BedrockIdentityKeyProvider.class), "walk must reach BedrockIdentityKeyProvider (the class that failed on Velocity 4)"); + assertTrue(graph.contains(BungeeListener.class), + "walk must include member-injected BungeeListener"); } /** diff --git a/core/src/test/java/com/minekube/connect/startup/PluginGraphStartupTest.java b/core/src/test/java/com/minekube/connect/startup/PluginGraphStartupTest.java index 11225dd59..9ca7feca6 100644 --- a/core/src/test/java/com/minekube/connect/startup/PluginGraphStartupTest.java +++ b/core/src/test/java/com/minekube/connect/startup/PluginGraphStartupTest.java @@ -43,6 +43,7 @@ import com.minekube.connect.inject.CommonPlatformInjector; import com.minekube.connect.module.ServerCommonModule; import com.minekube.connect.platform.util.PlatformUtils; +import com.minekube.connect.register.WatcherRegister; import java.nio.file.Path; import java.util.List; import java.util.Set; @@ -106,6 +107,8 @@ void reflectiveWalkReachesTheClassesThatRegressed() { "walk must reach BedrockAdmissionCoordinator"); assertTrue(graph.contains(BedrockIdentityEnforcer.class), "walk must reach BedrockIdentityEnforcer"); + assertTrue(graph.contains(WatcherRegister.class), + "walk must include member-injected WatcherRegister"); } /** diff --git a/core/src/testFixtures/java/com/minekube/connect/startup/StartupGraphProvisioning.java b/core/src/testFixtures/java/com/minekube/connect/startup/StartupGraphProvisioning.java index 9e0f7b16f..b5bd8aea9 100644 --- a/core/src/testFixtures/java/com/minekube/connect/startup/StartupGraphProvisioning.java +++ b/core/src/testFixtures/java/com/minekube/connect/startup/StartupGraphProvisioning.java @@ -149,11 +149,10 @@ public static Set> reachableInjectedTypes(Collection> roots) { Deque> queue = new ArrayDeque<>(); Set> visited = new LinkedHashSet<>(); - // A root that Guice itself constructs (has an injectable constructor) is part of the graph - // to check; roots created inside @Provides methods (no injectable constructor) are seeds - // only and are covered through the dependencies they expose. + // Roots created inside @Provides methods with no injection members are seeds only and are + // covered through the dependencies they expose. for (Class root : roots) { - if (isConnectConcrete(root) && hasInjectConstructor(root)) { + if (isConnectConcrete(root) && hasInjectMember(root)) { discovered.add(root); } enqueue(root, queue, visited); @@ -162,7 +161,7 @@ public static Set> reachableInjectedTypes(Collection> roots) { while (!queue.isEmpty()) { Class type = queue.poll(); for (Class dependency : injectedDependencies(type)) { - if (isConnectConcrete(dependency)) { + if (isConnectConcrete(dependency) && hasInjectMember(dependency)) { discovered.add(dependency); } enqueue(dependency, queue, visited); @@ -199,28 +198,28 @@ private static List guice7ProvisioningViolations(Class type) { } } - // A type with no @Inject constructor is not just-in-time constructed by Guice: it is - // supplied by an @Provides method / toInstance / linked binding (e.g. ConfigLoader, - // SimpleConnectApi, BedrockIdentityReadiness are built with `new` inside CommonModule), so - // Guice never inspects it for an injectable constructor and the javax regression cannot - // apply. Only the classes Guice itself instantiates via constructor injection are subject to - // Guice 7's discovery rule — exactly the population the Velocity 4 failure came from. - if (injectConstructors.isEmpty()) { + // Types with no injection members are supplied by an @Provides method / toInstance / + // linked binding (e.g. ConfigLoader, SimpleConnectApi, BedrockIdentityReadiness are built + // with `new` inside CommonModule), so Guice never inspects them for injection and the javax + // regression cannot apply. + if (!hasInjectMember(type)) { return violations; } - if (guice7InjectConstructors.size() > 1) { - violations.add(type.getName() + " has more than one Guice 7 @Inject constructor: " - + guice7InjectConstructors); - } + if (!injectConstructors.isEmpty()) { + if (guice7InjectConstructors.size() > 1) { + violations.add(type.getName() + " has more than one Guice 7 @Inject constructor: " + + guice7InjectConstructors); + } - boolean provisionable = - guice7InjectConstructors.size() == 1 || hasInjectableNoArgConstructor(type); - if (!provisionable) { - violations.add(type.getName() + " has an @Inject constructor that Guice 7 does NOT " - + "recognize (it uses only javax.inject, which Guice 7 on Velocity 4.0.0 " - + "ignores) and NO no-arg constructor, so Guice 7 cannot provision it — " - + "reproducing the \"Cant create plugin connect\" failure."); + boolean provisionable = + guice7InjectConstructors.size() == 1 || hasInjectableNoArgConstructor(type); + if (!provisionable) { + violations.add(type.getName() + " has an @Inject constructor that Guice 7 does NOT " + + "recognize (it uses only javax.inject, which Guice 7 on Velocity 4.0.0 " + + "ignores) and NO no-arg constructor, so Guice 7 cannot provision it — " + + "reproducing the \"Cant create plugin connect\" failure."); + } } for (String offender : javaxInjectAnnotations(type)) { @@ -282,6 +281,30 @@ private static boolean hasInjectConstructor(Class type) { return injectConstructor(type) != null; } + private static boolean hasInjectMember(Class type) { + for (Class current = type; current != null && current != Object.class; + current = current.getSuperclass()) { + try { + if (hasInjectConstructor(current)) { + return true; + } + for (Field field : current.getDeclaredFields()) { + if (hasAnnotationNamed(field, INJECT_ANNOTATIONS)) { + return true; + } + } + for (Method method : current.getDeclaredMethods()) { + if (hasAnnotationNamed(method, INJECT_ANNOTATIONS)) { + return true; + } + } + } catch (NoClassDefFoundError | RuntimeException ignored) { + return false; + } + } + return false; + } + private static boolean isConnectConcrete(Class type) { return type.getName().startsWith(CONNECT_PACKAGE) && !type.isInterface() @@ -312,20 +335,23 @@ private static boolean hasAnnotationNamed(AnnotatedElement element, Set private static List javaxInjectAnnotations(Class type) { List offenders = new ArrayList<>(); - collectJavaxInject(type, offenders); // class-level scope, e.g. @Singleton - for (Constructor constructor : type.getDeclaredConstructors()) { - collectJavaxInject(constructor, offenders); - for (Parameter parameter : constructor.getParameters()) { - collectJavaxInject(parameter, offenders); + for (Class current = type; current != null && current != Object.class; + current = current.getSuperclass()) { + collectJavaxInject(current, offenders); // class-level scope, e.g. @Singleton + for (Constructor constructor : current.getDeclaredConstructors()) { + collectJavaxInject(constructor, offenders); + for (Parameter parameter : constructor.getParameters()) { + collectJavaxInject(parameter, offenders); + } } - } - for (Field field : type.getDeclaredFields()) { - collectJavaxInject(field, offenders); - } - for (Method method : type.getDeclaredMethods()) { - collectJavaxInject(method, offenders); - for (Parameter parameter : method.getParameters()) { - collectJavaxInject(parameter, offenders); + for (Field field : current.getDeclaredFields()) { + collectJavaxInject(field, offenders); + } + for (Method method : current.getDeclaredMethods()) { + collectJavaxInject(method, offenders); + for (Parameter parameter : method.getParameters()) { + collectJavaxInject(parameter, offenders); + } } } return offenders; diff --git a/spigot/src/test/java/com/minekube/connect/SpigotPluginStartupTest.java b/spigot/src/test/java/com/minekube/connect/SpigotPluginStartupTest.java index c6cf20a48..4aec70b69 100644 --- a/spigot/src/test/java/com/minekube/connect/SpigotPluginStartupTest.java +++ b/spigot/src/test/java/com/minekube/connect/SpigotPluginStartupTest.java @@ -34,6 +34,7 @@ import com.google.inject.Injector; import com.google.inject.name.Names; import com.minekube.connect.addon.data.SpigotDataAddon; +import com.minekube.connect.addon.data.SpigotDataHandler; import com.minekube.connect.api.ConnectApi; import com.minekube.connect.api.logger.ConnectLogger; import com.minekube.connect.api.packet.PacketHandlers; @@ -41,9 +42,16 @@ import com.minekube.connect.bedrock.BedrockIdentityEnforcer; import com.minekube.connect.bedrock.BedrockIdentityKeyProvider; import com.minekube.connect.inject.CommonPlatformInjector; +import com.minekube.connect.inject.spigot.SpigotInjector; +import com.minekube.connect.listener.PaperProfileListener; +import com.minekube.connect.listener.SpigotListener; +import com.minekube.connect.listener.SpigotListenerRegistration; import com.minekube.connect.module.ServerCommonModule; import com.minekube.connect.platform.util.PlatformUtils; import com.minekube.connect.startup.StartupGraphProvisioning; +import com.minekube.connect.util.SpigotCommandUtil; +import com.minekube.connect.util.SpigotPlatformUtils; +import com.minekube.connect.util.SpigotVersionSpecificMethods; import java.nio.file.Path; import java.util.ArrayList; import java.util.List; @@ -55,7 +63,8 @@ * Per-platform startup / smoke test for the Bukkit/Spigot/Paper plugin. * *

Asserts the Spigot plugin DI graph — the {@code ServerCommonModule} + {@code SpigotPlatform} - * graph {@link SpigotPlugin#onLoad()} builds — is provisionable under Velocity 4-class Guice 7 and + * graph {@link SpigotPlugin#onLoad()} builds — including the concrete platform/listener/addon + * bindings — is provisionable under Velocity 4-class Guice 7 and * that a real Guice injector wires the shared graph up cleanly with no {@code ProvisionException}. * Like every platform's injector it carries the Bedrock identity graph, so it fails on the pre-fix * commit (javax-annotated Bedrock providers) and passes on the fix — see {@link @@ -75,6 +84,14 @@ private static List> spigotGraphRoots() { List> roots = new ArrayList<>(StartupGraphProvisioning.coreRuntimeGraphRoots()); roots.add(SpigotPlatform.class); roots.add(SpigotDataAddon.class); + roots.add(SpigotCommandUtil.class); + roots.add(SpigotPlatformUtils.class); + roots.add(SpigotInjector.class); + roots.add(SpigotVersionSpecificMethods.class); + roots.add(SpigotListenerRegistration.class); + roots.add(SpigotListener.class); + roots.add(PaperProfileListener.class); + roots.add(SpigotDataHandler.class); return roots; } @@ -89,6 +106,12 @@ void spigotPluginGraphIsProvisionableUnderGuice7() { + String.join("\n", violations)); assertTrue(graph.contains(BedrockIdentityKeyProvider.class), "walk must reach BedrockIdentityKeyProvider (the class that failed on Velocity 4)"); + assertTrue(graph.contains(SpigotDataAddon.class), + "walk must include member-injected SpigotDataAddon"); + assertTrue(graph.contains(SpigotListener.class), + "walk must include member-injected SpigotListener"); + assertTrue(graph.contains(PaperProfileListener.class), + "walk must include member-injected PaperProfileListener"); } /** diff --git a/velocity/src/test/java/com/minekube/connect/VelocityPluginStartupTest.java b/velocity/src/test/java/com/minekube/connect/VelocityPluginStartupTest.java index 760fe6a8d..c748aa0c3 100644 --- a/velocity/src/test/java/com/minekube/connect/VelocityPluginStartupTest.java +++ b/velocity/src/test/java/com/minekube/connect/VelocityPluginStartupTest.java @@ -39,10 +39,15 @@ import com.minekube.connect.bedrock.BedrockAdmissionCoordinator; import com.minekube.connect.bedrock.BedrockIdentityEnforcer; import com.minekube.connect.bedrock.BedrockIdentityKeyProvider; +import com.minekube.connect.inject.velocity.VelocityInjector; import com.minekube.connect.listener.VelocityListener; +import com.minekube.connect.listener.VelocityListenerRegistration; import com.minekube.connect.module.ProxyCommonModule; import com.minekube.connect.module.VelocityPlatformModule; import com.minekube.connect.startup.StartupGraphProvisioning; +import com.minekube.connect.util.VelocityCommandUtil; +import com.minekube.connect.util.VelocityPlatformUtils; +import com.minekube.connect.util.VelocitySkinApplier; import com.velocitypowered.api.event.EventManager; import com.velocitypowered.api.plugin.annotation.DataDirectory; import com.velocitypowered.api.proxy.ProxyServer; @@ -73,11 +78,16 @@ class VelocityPluginStartupTest { @TempDir Path tempDir; - /** Velocity-specific classes Guice touches, on top of the shared core graph. */ + /** Velocity module bindings and providers, on top of the shared core graph. */ private static List> velocityGraphRoots() { List> roots = new ArrayList<>(StartupGraphProvisioning.coreRuntimeGraphRoots()); roots.add(VelocityPlugin.class); roots.add(VelocityListener.class); + roots.add(VelocityCommandUtil.class); + roots.add(VelocityPlatformUtils.class); + roots.add(VelocityInjector.class); + roots.add(VelocitySkinApplier.class); + roots.add(VelocityListenerRegistration.class); return roots; } @@ -99,6 +109,8 @@ void velocityPluginGraphIsProvisionableUnderGuice7() { // Guard the guard: the walk must actually reach the class that regressed. assertTrue(graph.contains(BedrockIdentityKeyProvider.class), "walk must reach BedrockIdentityKeyProvider (the class that failed on Velocity 4)"); + assertTrue(graph.contains(VelocityListener.class), + "walk must include member-injected VelocityListener"); } /** From d0f3d5014eadce8bb13e57c5b5b1d7958dc0cd24 Mon Sep 17 00:00:00 2001 From: Robin Date: Sat, 25 Jul 2026 08:43:21 +0200 Subject: [PATCH 3/4] no-mistakes(review): Cover Spigot addon bindings in startup graph tests --- .../minekube/connect/SpigotPluginStartupTest.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/spigot/src/test/java/com/minekube/connect/SpigotPluginStartupTest.java b/spigot/src/test/java/com/minekube/connect/SpigotPluginStartupTest.java index 4aec70b69..3f5b61076 100644 --- a/spigot/src/test/java/com/minekube/connect/SpigotPluginStartupTest.java +++ b/spigot/src/test/java/com/minekube/connect/SpigotPluginStartupTest.java @@ -33,6 +33,9 @@ import com.google.inject.Guice; import com.google.inject.Injector; import com.google.inject.name.Names; +import com.minekube.connect.addon.AddonManagerAddon; +import com.minekube.connect.addon.DebugAddon; +import com.minekube.connect.addon.PacketHandlerAddon; import com.minekube.connect.addon.data.SpigotDataAddon; import com.minekube.connect.addon.data.SpigotDataHandler; import com.minekube.connect.api.ConnectApi; @@ -84,6 +87,9 @@ private static List> spigotGraphRoots() { List> roots = new ArrayList<>(StartupGraphProvisioning.coreRuntimeGraphRoots()); roots.add(SpigotPlatform.class); roots.add(SpigotDataAddon.class); + roots.add(AddonManagerAddon.class); + roots.add(DebugAddon.class); + roots.add(PacketHandlerAddon.class); roots.add(SpigotCommandUtil.class); roots.add(SpigotPlatformUtils.class); roots.add(SpigotInjector.class); @@ -112,6 +118,12 @@ void spigotPluginGraphIsProvisionableUnderGuice7() { "walk must include member-injected SpigotListener"); assertTrue(graph.contains(PaperProfileListener.class), "walk must include member-injected PaperProfileListener"); + assertTrue(graph.contains(AddonManagerAddon.class), + "walk must include member-injected AddonManagerAddon"); + assertTrue(graph.contains(DebugAddon.class), + "walk must include member-injected DebugAddon"); + assertTrue(graph.contains(PacketHandlerAddon.class), + "walk must include member-injected PacketHandlerAddon"); } /** From 3640af0d465c6f1570777e763cfd0eec255025f2 Mon Sep 17 00:00:00 2001 From: Robin Date: Sat, 25 Jul 2026 08:47:21 +0200 Subject: [PATCH 4/4] no-mistakes(review): Cover enable-time registrars in startup graph tests --- .../minekube/connect/startup/PluginGraphStartupTest.java | 6 ++++++ .../connect/startup/StartupGraphProvisioning.java | 8 ++++++++ .../com/minekube/connect/SpigotPluginStartupTest.java | 4 ++++ 3 files changed, 18 insertions(+) diff --git a/core/src/test/java/com/minekube/connect/startup/PluginGraphStartupTest.java b/core/src/test/java/com/minekube/connect/startup/PluginGraphStartupTest.java index 9ca7feca6..f8955f99c 100644 --- a/core/src/test/java/com/minekube/connect/startup/PluginGraphStartupTest.java +++ b/core/src/test/java/com/minekube/connect/startup/PluginGraphStartupTest.java @@ -43,6 +43,8 @@ import com.minekube.connect.inject.CommonPlatformInjector; import com.minekube.connect.module.ServerCommonModule; import com.minekube.connect.platform.util.PlatformUtils; +import com.minekube.connect.register.CommandRegister; +import com.minekube.connect.register.ListenerRegister; import com.minekube.connect.register.WatcherRegister; import java.nio.file.Path; import java.util.List; @@ -109,6 +111,10 @@ void reflectiveWalkReachesTheClassesThatRegressed() { "walk must reach BedrockIdentityEnforcer"); assertTrue(graph.contains(WatcherRegister.class), "walk must include member-injected WatcherRegister"); + assertTrue(graph.contains(CommandRegister.class), + "walk must include eager CommandRegister"); + assertTrue(graph.contains(ListenerRegister.class), + "walk must include eager ListenerRegister"); } /** diff --git a/core/src/testFixtures/java/com/minekube/connect/startup/StartupGraphProvisioning.java b/core/src/testFixtures/java/com/minekube/connect/startup/StartupGraphProvisioning.java index b5bd8aea9..ce316de7a 100644 --- a/core/src/testFixtures/java/com/minekube/connect/startup/StartupGraphProvisioning.java +++ b/core/src/testFixtures/java/com/minekube/connect/startup/StartupGraphProvisioning.java @@ -30,7 +30,11 @@ import com.minekube.connect.bedrock.BedrockIdentityEnforcer; import com.minekube.connect.bedrock.BedrockIdentityKeyProvider; import com.minekube.connect.bedrock.VerifiedBedrockIdentityRegistry; +import com.minekube.connect.command.TestCommand; +import com.minekube.connect.command.main.MainCommand; import com.minekube.connect.packet.PacketHandlersImpl; +import com.minekube.connect.register.CommandRegister; +import com.minekube.connect.register.ListenerRegister; import com.minekube.connect.register.WatchHealthServer; import com.minekube.connect.register.WatcherRegister; import com.minekube.connect.tunnel.Tunneler; @@ -118,6 +122,10 @@ public static List> coreRuntimeGraphRoots() { return List.of( ConnectPlatform.class, PacketHandlersImpl.class, + CommandRegister.class, + ListenerRegister.class, + TestCommand.class, + MainCommand.class, Libp2pEndpoint.class, Libp2pTunnelTransport.class, WebSocketTunnelTransport.class, diff --git a/spigot/src/test/java/com/minekube/connect/SpigotPluginStartupTest.java b/spigot/src/test/java/com/minekube/connect/SpigotPluginStartupTest.java index 3f5b61076..0db3ac629 100644 --- a/spigot/src/test/java/com/minekube/connect/SpigotPluginStartupTest.java +++ b/spigot/src/test/java/com/minekube/connect/SpigotPluginStartupTest.java @@ -51,6 +51,7 @@ import com.minekube.connect.listener.SpigotListenerRegistration; import com.minekube.connect.module.ServerCommonModule; import com.minekube.connect.platform.util.PlatformUtils; +import com.minekube.connect.register.AddonRegister; import com.minekube.connect.startup.StartupGraphProvisioning; import com.minekube.connect.util.SpigotCommandUtil; import com.minekube.connect.util.SpigotPlatformUtils; @@ -86,6 +87,7 @@ class SpigotPluginStartupTest { private static List> spigotGraphRoots() { List> roots = new ArrayList<>(StartupGraphProvisioning.coreRuntimeGraphRoots()); roots.add(SpigotPlatform.class); + roots.add(AddonRegister.class); roots.add(SpigotDataAddon.class); roots.add(AddonManagerAddon.class); roots.add(DebugAddon.class); @@ -124,6 +126,8 @@ void spigotPluginGraphIsProvisionableUnderGuice7() { "walk must include member-injected DebugAddon"); assertTrue(graph.contains(PacketHandlerAddon.class), "walk must include member-injected PacketHandlerAddon"); + assertTrue(graph.contains(AddonRegister.class), + "walk must include eager AddonRegister"); } /**