Conversation
WalkthroughThis pull request refactors the application to demonstrate three dependency injection approaches: manual constructor injection, a custom reflection-based DI container, and Weld CDI. Changes include introducing interfaces (MessageService, Printer), multiple application classes implementing each pattern, a custom Container for manual registration and resolution, updates to Maven dependencies, and a new entry point replacing the original. Changes
Sequence Diagram(s)sequenceDiagram
participant Main
participant ApplicationManual
participant MessageService
participant Printer
rect rgb(200, 220, 240)
Note over Main,Printer: Part 1: Manual Constructor Injection
Main->>ApplicationManual: new ApplicationManual(ms, printer)
Main->>ApplicationManual: run()
ApplicationManual->>MessageService: getMessage()
MessageService-->>ApplicationManual: "message"
ApplicationManual->>Printer: print("message")
Printer-->>ApplicationManual: (void)
end
sequenceDiagram
participant Main
participant Container
participant ApplicationContainer
participant MessageService
participant Printer
rect rgb(200, 220, 240)
Note over Main,Printer: Part 2: Custom DI Container with Reflection
Main->>Container: register(MessageService, impl)
Main->>Container: register(Printer, impl)
Main->>Container: getInstance(ApplicationContainer)
Container->>Container: resolveConstructor via reflection
Container->>MessageService: getInstance(MessageService)
MessageService-->>Container: (instance)
Container->>Printer: getInstance(Printer)
Printer-->>Container: (instance)
Container->>ApplicationContainer: new ApplicationContainer(ms, printer)
ApplicationContainer-->>Main: (instance)
Main->>ApplicationContainer: run()
ApplicationContainer->>Printer: print("[Part 2] " + message)
end
sequenceDiagram
participant Main
participant WeldContainer
participant ApplicationCdi
participant HelloMessageService
participant ConsolePrinter
rect rgb(200, 220, 240)
Note over Main,ConsolePrinter: Part 3: Weld CDI Framework
Main->>WeldContainer: new Weld().initialize()
Main->>WeldContainer: select(ApplicationCdi)
WeldContainer->>HelloMessageService: create instance `@ApplicationScoped`
WeldContainer->>ConsolePrinter: create instance `@ApplicationScoped`
WeldContainer->>ApplicationCdi: inject dependencies via constructor
ApplicationCdi-->>Main: (instance)
Main->>ApplicationCdi: run()
ApplicationCdi->>HelloMessageService: getMessage()
HelloMessageService-->>ApplicationCdi: "Hello from CDI (Part 3)!"
ApplicationCdi->>ConsolePrinter: print(message)
ConsolePrinter-->>ApplicationCdi: "Printer output: Hello from CDI..."
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Areas requiring extra attention:
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/main/java/org/example/Main.java (1)
8-50: Entry point cleanly showcases the three DI approachesThe separation into
runPart1(manual DI),runPart2(custom container), andrunPart3(Weld CDI) is clear and matches the exercise goal. For slightly better ergonomics, you could optionally drive which part runs via a command‑line argument or environment flag instead of commenting/uncommenting calls inmain, but for a teaching scenario this is perfectly acceptable as is.src/main/java/org/example/Container.java (1)
11-44: Constructor selection ingetInstanceis order‑dependent; consider failing fast or choosing deterministicallyThe basic registration and recursive resolution logic is fine for a teaching‑level DI container, but relying on
implementationClass.getDeclaredConstructors()[0]makes behavior depend on reflection order if a class ever gains multiple constructors. That can lead to hard‑to‑debug wiring issues.To keep this simple container predictable, you could either:
- Enforce exactly one constructor per injectable and fail fast otherwise, or
- Encode a clear selection rule (e.g., prefer the only constructor, or the one with the most parameters).
Here’s an example of the first option (single‑constructor invariant) while reusing the chosen constructor in both branches:
- Constructor<?> constructor = implementationClass.getDeclaredConstructors()[0]; - Class<?>[] paramTypes = constructor.getParameterTypes(); - - if (paramTypes.length == 0) { - - return (T) implementationClass.getDeclaredConstructor().newInstance(); - } + Constructor<?>[] constructors = implementationClass.getDeclaredConstructors(); + if (constructors.length != 1) { + throw new RuntimeException( + "Expected exactly one constructor for " + implementationClass.getName() + + " but found " + constructors.length); + } + + Constructor<?> constructor = constructors[0]; + Class<?>[] paramTypes = constructor.getParameterTypes(); + + if (paramTypes.length == 0) { + constructor.setAccessible(true); + return (T) constructor.newInstance(); + } @@ - Object[] params = new Object[paramTypes.length]; - for (int i = 0; i < paramTypes.length; i++) { - params[i] = getInstance(paramTypes[i]); - } - - return (T) constructor.newInstance(params); + Object[] params = new Object[paramTypes.length]; + for (int i = 0; i < paramTypes.length; i++) { + params[i] = getInstance(paramTypes[i]); + } + + constructor.setAccessible(true); + return (T) constructor.newInstance(params);You may also want to keep in mind that cyclic dependencies will currently cause unbounded recursion; that’s acceptable for an exercise container but something to be aware of if you extend it further.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
pom.xml(1 hunks)src/main/java/org/example/App.java(0 hunks)src/main/java/org/example/ApplicationCdi.java(1 hunks)src/main/java/org/example/ApplicationContainer.java(1 hunks)src/main/java/org/example/ApplicationManual.java(1 hunks)src/main/java/org/example/ConsolePrinter.java(1 hunks)src/main/java/org/example/Container.java(1 hunks)src/main/java/org/example/HelloMessageService.java(1 hunks)src/main/java/org/example/Main.java(1 hunks)src/main/java/org/example/MessageService.java(1 hunks)src/main/java/org/example/Printer.java(1 hunks)src/main/resources/META-INF/beans.xml(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/org/example/App.java
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: jennymakki
Repo: fungover/exercise2025 PR: 204
File: src/main/java/org/example/config/SecurityConfig.java:19-24
Timestamp: 2025-11-16T13:56:11.263Z
Learning: The repository fungover/exercise2025 is an exercise/learning project where hard-coded credentials and other shortcuts for ease of testing are acceptable and intentional.
Learnt from: Bekhalm
Repo: fungover/exercise2025 PR: 188
File: src/main/java/container/SimpleContainer.java:12-13
Timestamp: 2025-10-16T13:39:46.749Z
Learning: In assignment or educational contexts, code that demonstrates different implementation approaches (e.g., manual DI vs CDI frameworks) should be retained even if not all variants are used in production, as they serve pedagogical purposes.
📚 Learning: 2025-10-16T13:39:46.749Z
Learnt from: Bekhalm
Repo: fungover/exercise2025 PR: 188
File: src/main/java/container/SimpleContainer.java:12-13
Timestamp: 2025-10-16T13:39:46.749Z
Learning: In assignment or educational contexts, code that demonstrates different implementation approaches (e.g., manual DI vs CDI frameworks) should be retained even if not all variants are used in production, as they serve pedagogical purposes.
Applied to files:
src/main/java/org/example/Main.java
📚 Learning: 2025-09-10T14:02:54.365Z
Learnt from: alfredbrannare
Repo: fungover/exercise2025 PR: 66
File: src/test/java/org/example/service/WarehouseTest.java:1-21
Timestamp: 2025-09-10T14:02:54.365Z
Learning: Mockito 5.x (specifically version 5.19.0 via mockito-junit-jupiter dependency) includes static mocking capabilities without requiring a separate mockito-inline dependency, unlike older Mockito versions.
Applied to files:
pom.xml
🧬 Code graph analysis (3)
src/main/java/org/example/ConsolePrinter.java (2)
src/main/java/org/example/ApplicationCdi.java (1)
ApplicationScoped(6-22)src/main/java/org/example/HelloMessageService.java (1)
ApplicationScoped(5-13)
src/main/java/org/example/ApplicationCdi.java (2)
src/main/java/org/example/ConsolePrinter.java (1)
ApplicationScoped(5-12)src/main/java/org/example/HelloMessageService.java (1)
ApplicationScoped(5-13)
src/main/java/org/example/HelloMessageService.java (2)
src/main/java/org/example/ApplicationCdi.java (1)
ApplicationScoped(6-22)src/main/java/org/example/ConsolePrinter.java (1)
ApplicationScoped(5-12)
🪛 GitHub Actions: .github/workflows/ci.yml
pom.xml
[error] 1-1: Maven enforcer rule 'RequireJavaVersion' failed: Detected JDK 25.0.1 which is not in the allowed range [24,25].
🔇 Additional comments (8)
src/main/resources/META-INF/beans.xml (1)
1-6: Minimal CDI descriptor looks goodThe beans.xml definition matches the Jakarta EE 4 / CDI 4 schema and is a clean, minimal setup for Weld SE. No changes needed.
src/main/java/org/example/HelloMessageService.java (1)
1-12: Straightforward CDIMessageServiceimplementation
HelloMessageServicecleanly implementsMessageServiceand is properly annotated for CDI. The fixed message is appropriate for this exercise/demo context.src/main/java/org/example/Printer.java (1)
1-5: Printer abstraction is clear and focusedThe
Printerinterface is minimal and expressive, which works well for swapping implementations across the different DI approaches.src/main/java/org/example/ConsolePrinter.java (1)
1-11: Console printer implementation is appropriate
ConsolePrintercorrectly implementsPrinterand the prefixed output is useful for distinguishing DI parts in the console. The CDI scope choice fits the example.src/main/java/org/example/MessageService.java (1)
1-5: MessageService interface is well-definedThe
MessageServiceinterface is minimal and clearly expresses the responsibility for producing a message; it works well as a shared contract across the three DI variants.src/main/java/org/example/ApplicationContainer.java (1)
1-15: Clean container-based DI exampleConstructor injection into
ApplicationContaineris straightforward, andrun()delegates correctly toPrinterandMessageServicewith the “[Part 2]” prefix. This is a clear illustration of the container-wired variant.src/main/java/org/example/ApplicationCdi.java (1)
1-21: CDI-based Application wiring is consistent and clear
ApplicationCdiuses constructor injection withMessageServiceandPrinterin a CDI-friendly way, andrun()mirrors the other parts with a clear “[Part 3 / CDI]” prefix. This is a solid CDI example with no issues from a wiring/correctness perspective.src/main/java/org/example/ApplicationManual.java (1)
5-16: Manual constructor injection is clean and idiomaticUsing final fields with constructor injection and a thin
run()that delegates toprinter.print(messageService.getMessage())is a clear, testable way to demonstrate manual DI. No issues from a correctness or design perspective for this exercise.
| <dependency> | ||
| <groupId>org.jboss.weld.se</groupId> | ||
| <artifactId>weld-se-core</artifactId> | ||
| <version>5.1.2.Final</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>jakarta.inject</groupId> | ||
| <artifactId>jakarta.inject-api</artifactId> | ||
| <version>2.0.1</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.slf4j</groupId> | ||
| <artifactId>slf4j-simple</artifactId> | ||
| <version>2.0.12</version> | ||
| </dependency> |
There was a problem hiding this comment.
🧩 Analysis chain
Fix Java version range to unblock Maven Enforcer in CI
The new CDI/logging dependencies look fine for the DI demo. The CI failure, however, comes from the requireJavaVersion rule: JDK 25.0.1 does not satisfy [24,25] (patch 25.0.1 is treated as greater than 25). With <maven.compiler.release>25</maven.compiler.release>, you probably want to allow all 25.x versions.
Consider narrowing the rule to Java 25 only and allowing all 25.x patch releases:
- <requireJavaVersion>
- <version>[24,25]</version>
- </requireJavaVersion>
+ <requireJavaVersion>
+ <!-- Allow Java 25.x but reject 24 and 26+ -->
+ <version>[25,26)</version>
+ </requireJavaVersion>Please verify that this matches the Java versions you intend to support (both locally and in your GitHub Actions runner).
Also applies to: 128-130
I need to verify the review comment by examining the pom.xml file and confirming the Maven Enforcer version range semantics.
🏁 Script executed:
#!/bin/bash
# Examine the pom.xml file at the specified lines
cat pom.xml | head -150 | tail -120Length of output: 4479
🌐 Web query:
Maven Enforcer requireJavaVersion version range syntax [24,25] vs [25,26)
💡 Result:
Short answer: they use standard Maven range syntax — square brackets [] are inclusive, parentheses () are exclusive.
- [24,25] means versions >= 24 and <= 25 (24 through 25 inclusive). [1][2]
- [25,26) means versions >= 25 and < 26 (25 included, 26 excluded). [1][2]
Note: a single value like 24 (in RequireJavaVersion) is treated as a minimum (>=24) by the Enforcer rule. [2]
---
🏁 Script executed:
```shell
#!/bin/bash
# Check for maven.compiler.release property in pom.xml
cat pom.xml | grep -n "compiler.release"
Length of output: 62
Fix Java version range to unblock Maven Enforcer in CI
The new CDI/logging dependencies (weld-se-core, jakarta.inject-api, slf4j-simple) are appropriate for the DI demo. The CI failure stems from the requireJavaVersion rule: Maven treats version "25" as "25.0.0", so JDK 25.0.1 exceeds the upper bound of [24,25] and fails validation. With <maven.compiler.release>25</maven.compiler.release>, update the enforcer rule to allow all 25.x patch releases:
- <requireJavaVersion>
- <version>[24,25]</version>
- </requireJavaVersion>
+ <requireJavaVersion>
+ <!-- Allow Java 25.x but reject 24 and 26+ -->
+ <version>[25,26)</version>
+ </requireJavaVersion>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In pom.xml around lines 36 to 50, the Maven Enforcer requireJavaVersion rule is
too strict and treats "25" as "25.0.0", causing JDK 25.0.1 to fail; update the
enforcer Java version range to allow all 25.x patch releases (for example change
the upper bound to 26 by using a range like [24,26) or equivalent) while keeping
<maven.compiler.release>25</maven.compiler.release> so CI accepts JDK 25.x.
1 participant
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.