fix(core): support Velocity 4 Guice injection#64
Merged
Conversation
Velocity 4.0.0 fails to load the Connect plugin ("Cant create plugin
connect") because Guice cannot provision BedrockIdentityKeyProvider, the
2nd parameter of CommonModule.bedrockIdentityReadiness on the
VelocityPlugin injector.
Root cause: the Velocity plugin does not shade Guice (it is `provided`)
and VelocityPlugin builds a child of Velocity's own injector, so it runs
on the platform's Guice. Velocity 4.0.0 provides Guice 7, which dropped
javax.inject support and recognizes only com.google.inject/jakarta.inject
annotations. BedrockIdentityKeyProvider and BedrockAdmissionCoordinator
were annotated with javax.inject.Inject/@singleton, so under Guice 7 they
have no discoverable @Inject constructor and no no-arg constructor and
cannot be provisioned — failing the whole injector. Spigot/Bungee shade
Guice 6 and Velocity 3.x ships Guice 5; both still accept javax, so only
Velocity 4.0.0 broke. The javax dependency was introduced in #59 (which
replaced an explicit @provides factory with a JIT javax.inject.Inject
constructor), not #61.
Switch the five DI classes that still used javax.inject (BedrockIdentity-
KeyProvider, BedrockAdmissionCoordinator, BedrockIdentityEnforcer,
VerifiedBedrockIdentityRegistry, Metrics) to their com.google.inject.*
equivalents — the codebase standard everywhere else. This is portable
across Guice 5/6/7 and changes no DI semantics.
Add BedrockVelocityGuice7ProvisioningTest, which replicates Guice 7's
injectable-constructor discovery rule and forbids javax.inject on these
graph classes (no Guice 7 / Velocity 4.0.0 harness is on the classpath;
Guice 6 masks the failure). It fails before this change and passes after.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Fix the Connect (connect-java) plugin failing to load on Velocity 4.0.0 with 'Cant create plugin connect', a Guice dependency-injection provisioning failure: Guice cannot provision com.minekube.connect.bedrock.BedrockIdentityKeyProvider (no @Inject constructor / no no-arg constructor) as the 2nd param of CommonModule.bedrockIdentityReadiness, failing the whole VelocityPlugin injector.
Root cause (proven, not assumed): the Velocity plugin does NOT shade Guice (velocity/build.gradle.kts marks guice 'provided') and VelocityPlugin builds a CHILD of Velocity's own injector, so it runs on the platform's Guice. Velocity 4.0.0 (new major) provides Guice 7, which dropped javax.inject support and recognizes only com.google.inject/jakarta.inject annotations. Of every injectable class in the codebase, only the Bedrock DI classes used javax.inject; everything else already uses com.google.inject. So on Guice 7 the rest of the plugin works but the javax.inject-annotated classes are invisible to constructor discovery. Spigot/Bungee shade their own Guice 6 and Velocity 3.x ships Guice 5 (both accept javax), which is why only Velocity 4.0.0 broke. Git history shows the javax.inject JIT constructor was introduced in #59 (which replaced an explicit @provides factory with a JIT javax.inject.Inject constructor); #61 (initially suspected) touched only a test file and is NOT the cause.
Fix (smallest proven counterfactual): switch the five DI classes that still used javax.inject (BedrockIdentityKeyProvider, BedrockAdmissionCoordinator, BedrockIdentityEnforcer, VerifiedBedrockIdentityRegistry, Metrics) to their com.google.inject.* equivalents (Inject, Singleton, name.Named) — the codebase standard everywhere else. Deliberately chose com.google.inject over jakarta.inject because jakarta would break Velocity 3.x (Guice 5); com.google.inject is portable across Guice 5/6/7. This is a pure annotation-provider swap with identical DI semantics — no Bedrock-identity behavior change. Deliberately fixed all five javax.inject users, not just the reported BedrockIdentityKeyProvider, because the others (@singleton scope, @nAmed qualifier) would fail or misbehave next on the Guice 7 graph.
Regression test: added core/.../bedrock/BedrockVelocityGuice7ProvisioningTest. Because the repo compiles/tests against Guice 6 (which recognizes BOTH javax and com.google.inject) and no Guice 7 / Velocity 4.0.0 harness is on the classpath, a plain Guice-6 provisioning test cannot reproduce the failure; the test instead replicates Guice 7's exact injectable-constructor discovery rule and forbids javax.inject annotations on these graph classes. Verified it fails before the fix (3/4 tests) and passes after. This harness limitation is documented explicitly in the test Javadoc. Also added a concise AGENTS.md 'DI annotations (Guice provider portability)' entry. Validation: ./gradlew build is green (checkstyle, libp2p isolation verification, and core/velocity/spigot/bungee test suites all pass), confirming no regression to other platforms.
What Changed
javax.injectto portablecom.google.injectannotations.AGENTS.md.Risk Assessment
✅ Low: The change is a bounded annotation-provider swap across all five identified DI classes, and surrounding injector paths show no material source risk.
Testing
Fresh targeted runs exercised the Guice-7 constructor-discovery replica, all five DI classes’ annotation scan, real Guice graph provisioning, parent-injector startup guards, Bedrock behavior checks, and the Velocity plugin lifecycle test. No real Velocity 4/Guice 7 server harness is present, so the documented exact-rule replica is the runtime limitation.
Evidence: Targeted test cases
Evidence: Core targeted run
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
./gradlew :core:test --tests com.minekube.connect.bedrock.BedrockVelocityGuice7ProvisioningTest --tests com.minekube.connect.module.BedrockParentInjectorStartupTest --tests com.minekube.connect.bedrock.BedrockIdentityKeyProviderTest --tests com.minekube.connect.bedrock.BedrockAdmissionCoordinatorTest --rerun-tasks --console=plain --info./gradlew :velocity:test --tests com.minekube.connect.VelocityPluginTest --console=plain --infoVerified the JUnit XML evidence and confirmed the worktree has no generated build or Gradle-cache directories.✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.