KTOR-8310 DI tooling for application modules#4746
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR introduces the Changes
Possibly related PRs
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
| try { | ||
| monitor.raise(event, application) | ||
| } catch (cause: Throwable) { | ||
| environment.log.debug("One or more of the handlers thrown an exception", cause) | ||
| } |
There was a problem hiding this comment.
I added a log entry for failed event hooks. The monitor.raiseCatching(event, application) call would actually swallow exceptions, which seems quite bad for debugging.
| modules.forEach { module -> module(newInstance, currentClassLoader) } | ||
| } | ||
|
|
||
| safeRaiseEvent(ApplicationStarted, newInstance) |
There was a problem hiding this comment.
I changed this hook callback to throw when there's a failure. I think it makes more sense than eating the exception here, and it's the only logical point where we could prevent an application from starting after modules are loaded.
| return parameters.all { | ||
| isApplication(it) || isApplicationEnvironment(it) || it.kind == KParameter.Kind.INSTANCE || it.isOptional | ||
| } |
There was a problem hiding this comment.
All parameters are OK now.
bjhham
force-pushed
the
bjhham/di-module-param-support
branch
2 times, most recently
from
aa8783f to
ae1f87d
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
ktor-server/ktor-server-core/jvm/src/io/ktor/server/application/ApplicationModuleInjector.kt (1)
30-37: Check typed usage for future extensibility.The function returns
Any?, which is flexible but might complicate typed usage. If strong typing is necessary in the future, consider refining the return type or adding generics.ktor-server/ktor-server-plugins/ktor-server-di/jvm/test/io/ktor/server/plugins/di/DependencyInjectTestModules.kt (2)
18-18: Streamline this scoping reference.Using
this@createBankTellerNoArgs.resolve()is correct, but it might be more readable to rely onthis.resolve()unless there's a naming collision.- BankTeller(this@createBankTellerNoArgs.resolve(), this@createBankTellerNoArgs.resolve()) + BankTeller(this.resolve(), this.resolve())
51-61: Consider adding a test for the route.The
bankingModuledefines a new route/hello. Adding a simple test to validate the correct response ensures reliability and helps prevent regressions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
ktor-server/ktor-server-core/api/ktor-server-core.api(1 hunks)ktor-server/ktor-server-core/jvm/src/io/ktor/server/application/ApplicationModuleInjector.kt(1 hunks)ktor-server/ktor-server-core/jvm/src/io/ktor/server/engine/EmbeddedServerJvm.kt(5 hunks)ktor-server/ktor-server-core/jvm/src/io/ktor/server/engine/internal/AutoReloadUtils.kt(1 hunks)ktor-server/ktor-server-core/jvm/src/io/ktor/server/engine/internal/CallableUtils.kt(5 hunks)ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.api(8 hunks)ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api(8 hunks)ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyInjection.kt(5 hunks)ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyRegistry.kt(3 hunks)ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyResolution.kt(0 hunks)ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/utils/ClasspathReference.kt(1 hunks)ktor-server/ktor-server-plugins/ktor-server-di/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.kt(6 hunks)ktor-server/ktor-server-plugins/ktor-server-di/jvm/resources/META-INF/services/io.ktor.server.application.ApplicationModuleInjector(1 hunks)ktor-server/ktor-server-plugins/ktor-server-di/jvm/src/io/ktor/server/plugins/di/DependencyReflectionJvm.kt(1 hunks)ktor-server/ktor-server-plugins/ktor-server-di/jvm/src/io/ktor/server/plugins/di/PluginModuleInjector.kt(1 hunks)ktor-server/ktor-server-plugins/ktor-server-di/jvm/src/io/ktor/server/plugins/di/utils/ClasspathReference.jvm.kt(2 hunks)ktor-server/ktor-server-plugins/ktor-server-di/jvm/test/io/ktor/server/plugins/di/DependencyInjectTestModules.kt(3 hunks)ktor-server/ktor-server-plugins/ktor-server-di/jvm/test/io/ktor/server/plugins/di/DependencyInjectionJvmTest.kt(7 hunks)ktor-server/ktor-server-plugins/ktor-server-di/nonJvm/src/io/ktor/server/plugins/di/utils/ClasspathReferences.nonJvm.kt(1 hunks)
💤 Files with no reviewable changes (1)
- ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyResolution.kt
🧰 Additional context used
🧠 Learnings (2)
ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.api (1)
Learnt from: bjhham
PR: ktorio/ktor#4732
File: ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyRegistry.kt:80-84
Timestamp: 2025-03-20T11:34:27.272Z
Learning: The circular dependency handling in the DI plugin is designed to "fail fast" when circular dependencies are detected. The implementation intentionally keeps the dependency key in the `visited` set after detecting a circular dependency to prevent any attempts at recovery or resolution in a different context.
ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api (1)
Learnt from: bjhham
PR: ktorio/ktor#4732
File: ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyRegistry.kt:80-84
Timestamp: 2025-03-20T11:34:27.272Z
Learning: The circular dependency handling in the DI plugin is designed to "fail fast" when circular dependencies are detected. The implementation intentionally keeps the dependency key in the `visited` set after detecting a circular dependency to prevent any attempts at recovery or resolution in a different context.
🧬 Code Definitions (3)
ktor-server/ktor-server-core/jvm/src/io/ktor/server/engine/EmbeddedServerJvm.kt (1)
ktor-server/ktor-server-core/jvm/src/io/ktor/server/engine/internal/CallableUtils.kt (1) (1)
executeModuleFunction(17-78)
ktor-server/ktor-server-plugins/ktor-server-di/jvm/test/io/ktor/server/plugins/di/DependencyInjectionJvmTest.kt (2)
ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyProvider.kt (1) (1)
provide(215-219)ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyReflection.kt (1) (1)
provide(35-36)
ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyInjection.kt (3)
ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/utils/ClasspathReference.kt (1) (1)
installReference(36-39)ktor-server/ktor-server-plugins/ktor-server-di/jvm/src/io/ktor/server/plugins/di/utils/ClasspathReference.jvm.kt (1) (1)
installReference(14-64)ktor-server/ktor-server-plugins/ktor-server-di/nonJvm/src/io/ktor/server/plugins/di/utils/ClasspathReferences.nonJvm.kt (1) (1)
installReference(10-13)
🔇 Additional comments (59)
ktor-server/ktor-server-plugins/ktor-server-di/jvm/resources/META-INF/services/io.ktor.server.application.ApplicationModuleInjector (1)
1-2: SPI service provider configuration looks good.This file correctly implements the Java Service Provider Interface (SPI) pattern, registering
PluginModuleInjectoras the implementation ofApplicationModuleInjector. This allows the service to be discovered and loaded automatically at runtime, which aligns with the dependency injection enhancements mentioned in the PR objectives.ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/utils/ClasspathReference.kt (1)
36-39: Good refactoring to extension function.Converting
installReferencefrom a standalone function to an extension function onApplicationimproves readability and follows Kotlin idioms. This change allows for more natural invocation directly on Application instances, enhancing the API's fluency without changing functionality.ktor-server/ktor-server-plugins/ktor-server-di/jvm/src/io/ktor/server/plugins/di/DependencyReflectionJvm.kt (1)
42-45:Details
❓ Verification inconclusive
Exception type unification is a good practice.
Changing from
DependencyConstructionExceptiontoDependencyInjectionExceptionaligns with the effort to unify exception types mentioned in the PR summary. This promotes consistent error handling across the framework.Note that this is potentially a breaking change for code that specifically catches
DependencyConstructionException. Consider documenting this in release notes.
🏁 Script executed:
#!/bin/bash # Check if there are any direct references to DependencyConstructionException in the codebase # that might be affected by this change rg "DependencyConstructionException" --type kotlinLength of output: 50
Action Required: Verify Impact on Exception Handling
The update to replaceDependencyConstructionExceptionwithDependencyInjectionExceptionis in line with the unification and streamlining of exception types across the framework. This change improves consistency in error handling, but it could also potentially break downstream code that explicitly catches the old exception. Although our initial search for any references toDependencyConstructionExceptionin Kotlin files returned no results, this outcome is inconclusive. Please perform a thorough manual check (or run an expanded repository-wide search) to confirm that no external code depends onDependencyConstructionException. If any such dependencies exist, document the impact in the release notes.
- Key Considerations:
- The change unifies exception types, enhancing error handling consistency.
- There is a potential for breaking changes if any external code catches the old exception type.
- Ensure that changes are clearly communicated in the release notes.
ktor-server/ktor-server-core/jvm/src/io/ktor/server/engine/internal/AutoReloadUtils.kt (1)
59-60: Parameter validation responsibility shifted.The function now always returns
trueinstead of validating parameters. This aligns with the PR objective of enhancing DI capabilities by allowing injection of parameters that aren't of default types.Based on the previous comment that "All parameters are OK now", this change appears intentional as the validation responsibility has likely been moved to the new
ApplicationModuleInjectorimplementation.ktor-server/ktor-server-plugins/ktor-server-di/jvm/src/io/ktor/server/plugins/di/PluginModuleInjector.kt (1)
12-25: Implementation looks clean and focused.This new
PluginModuleInjectorimplements theApplicationModuleInjectorinterface and provides parameter resolution from the DI container. The implementation correctly handles fallback to a newDependencyReflectionJvmif one isn't available in the application's dependencies. Using a lazy delegate for the parameter value is a good approach to defer resolution until actually needed.ktor-server/ktor-server-core/jvm/src/io/ktor/server/engine/EmbeddedServerJvm.kt (4)
67-70: Good use of service loading for extension point.The implementation uses
loadServiceOrNull()to load an availableApplicationModuleInjectorservice, with a fallback to theDisabledimplementation. This follows the service locator pattern correctly and is well protected with theInternalAPIopt-in annotation.
233-237: Better error handling for event hooks.Replacing
monitor.raiseCatchingwith an explicit try-catch that logs exceptions is a good improvement. This provides better visibility into failures during event handling.
383-383: Allowing startup failures to propagate.Changing from
monitor.raiseCatchingtomonitor.raisefor theApplicationStartedevent allows exceptions to propagate, which prevents an application from starting after modules are loaded if there are issues. This is an important improvement for detecting failures early.
432-432: Correctly passing the moduleInjector to the module execution function.The injector is now forwarded to the
executeModuleFunction, enabling it to resolve module parameters. This is a key part of the DI tooling implementation for application modules.ktor-server/ktor-server-plugins/ktor-server-di/nonJvm/src/io/ktor/server/plugins/di/utils/ClasspathReferences.nonJvm.kt (1)
10-13: Consistent API pattern across platforms.Converting
installReferenceto an extension function onApplicationmaintains consistency with the JVM implementation. The error message correctly indicates the lack of reflection support on non-JVM platforms.ktor-server/ktor-server-plugins/ktor-server-di/jvm/src/io/ktor/server/plugins/di/utils/ClasspathReference.jvm.kt (3)
14-17: Extension function improves API usability.Converting
installReferenceto an extension function onApplicationimproves the API design and aligns with Kotlin's idiomatic extension function pattern. This makes it clearer that the function operates in the context of an Application.
51-51: Correctly updated reference to use extension receiver.Updated application environment reference to use the extension receiver with
this@installReference.environmentinstead of the previous parameter reference.
53-53: Explicit receiver for get method.Added explicit
thisqualifier for thegetmethod call to clearly indicate it's being called on the current DependencyResolver instance. This improves code clarity.ktor-server/ktor-server-core/api/ktor-server-core.api (1)
63-69: Interface addition looks good.This new interface declaration and its companion object appear consistent with the rest of the API. No significant concerns.
ktor-server/ktor-server-core/jvm/src/io/ktor/server/application/ApplicationModuleInjector.kt (1)
23-28: Graceful default injector.Using the
Disabledfield in the companion object provides a clear fallback behavior. This raises an immediate exception if no injector is installed, which is helpful for detecting configuration issues early.ktor-server/ktor-server-plugins/ktor-server-di/jvm/test/io/ktor/server/plugins/di/DependencyInjectTestModules.kt (1)
8-9: New imports approved.Imports for
io.ktor.server.response.*andio.ktor.server.routing.*are appropriate for implementing routes and responses.ktor-server/ktor-server-core/jvm/src/io/ktor/server/engine/internal/CallableUtils.kt (6)
20-21: Signature change for injection support.Introducing
moduleInjectoralongsideapplicationin the signature is a clean way to enable dependency injection. Ensure all call sites are updated accordingly.
41-41: Invoke injection function carefully.Calling
callFunctionWithInjectionwithnullinstance is valid for static functions. Verify no dynamic references or instance-bound parameters are missed.
71-72: Instance creation with injection.Creating the container via
createModuleContainerand then injecting parameters is consistent with the Ktor design. This helps keep reflection usage localized.
83-84: Additional constructor parameters.Forwarding both
applicationandmoduleInjectorensures external clients can configure custom injectors without breaking usage of this utility.
95-95: Constructor invocation consistency.Invoking
callFunctionWithInjection(...)for constructor resolution maintains a consistent approach for both static and instance-based module functions.
101-134: Validate exception handling and fallback logic.Within this parameter-mapping block, consider:
- Logging or handling
IllegalArgumentExceptionfor unsupported parameter types, especially for specializedApplicationsubtypes.- Confirming no concurrency issues arise from repeated reflective calls.
Overall, the structured fallback paths (optional, marked nullable, or direct injection) look robust.ktor-server/ktor-server-plugins/ktor-server-di/jvm/test/io/ktor/server/plugins/di/DependencyInjectionJvmTest.kt (7)
7-11: New imports for HTTP client usage and test assertionsThese imports support the newly added test methods that perform actual HTTP interactions and verify behavior through assertions.
Also applies to: 16-16
67-69: Improved code clarity with explicitthiscontextPrefixing
resolve()calls withthismakes the source context more explicit, improving readability and maintainability.
108-124: Simplified test structure for better readabilityThe test has been restructured to use a single
assertFailsWithblock, removing the nested context and making the test flow clearer and more direct.
278-290: New test for module parameter injectionThis test verifies the core functionality introduced in KTOR-8310, confirming that parameters can be properly injected into application modules and accessed via HTTP endpoints.
292-303: Good error handling verificationThis test confirms that the system properly fails when a required dependency is missing, with the appropriate exception type. This aligns with the goal of validating dependencies during startup mentioned in KTOR-8329.
305-310: Enhanced test configuration with module supportThe
testConfigFilemethod has been upgraded to support module configuration and provide a default test function, making it more flexible for testing different module scenarios.Also applies to: 318-323
329-329: Executing test functionsThe added call to
test()ensures that any test logic defined in the new parameter is executed, allowing for HTTP interaction testing within the configuration test.ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyRegistry.kt (5)
15-17: Updated documentation to reflect new validation capabilitiesThe description now correctly indicates this is a stateful type that can verify dependencies are resolvable, which aligns with the new functionality.
21-33: Added dependency validation methodsThese new methods allow identifying required dependencies and validating their availability, addressing KTOR-8329 by enabling dependency validation during application startup rather than on first access.
58-58: Storage for required dependenciesA mutable set is added to track dependencies that must be resolvable, supporting the validation system.
74-82: Implementation of validation methodsThe implementation correctly adds dependencies to the required set and performs validation by attempting to resolve each required dependency.
85-94: Added property delegation with automatic dependency trackingThis
provideDelegateoperator enables theval service: Service by dependenciessyntax while also marking the dependency as required through the newrequiremethod.ktor-server/ktor-server-plugins/ktor-server-di/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.kt (4)
7-11: Added imports for HTTP testingNew imports support HTTP request/response testing in the newly added tests.
61-67: Refactored test for consistent structureThe test has been restructured to use the
testDIblock consistently with other tests, improving readability and maintainability.
87-107: New test for startup validationThis test verifies that missing dependencies fail during application startup rather than at first access, which directly addresses the KTOR-8329 issue mentioned in the PR objectives.
121-132: Improved clarity with explicit contextSimilar to changes in the JVM test file,
resolve()calls are now prefixed withthisfor better readability.ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyInjection.kt (4)
72-86: Improved DI initialization with validation on startupThe initialization has been restructured for better separation of concerns:
- First create the registry instance
- Install all references from configuration
- Subscribe to ApplicationStarted to validate dependencies
- Store the registry in application attributes
This change directly supports KTOR-8329 by validating dependencies at startup.
181-185: New base exception for standardized error handlingAdding a common parent exception for all DI-related issues improves error handling consistency and makes it easier to catch all DI problems with a single catch block.
191-191: Updated exception hierarchyAll dependency injection exceptions now extend from
DependencyInjectionException, creating a standardized hierarchy that's easier to handle and categorize.Also applies to: 197-197, 203-203, 209-209, 215-215, 222-222, 231-231
105-105: Consistent exception usage in NoReflectionThe NoReflection implementation now throws the standardized
DependencyInjectionExceptioninstead of a platform-specific exception.ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api (8)
197-198: Consistent interface adherence inDependencyRegistryImpl.
Providingrequire()andvalidate()here aligns with the new interface contract. Great job on maintaining consistency.
217-217: UnifyingDuplicateDependencyExceptionunder the new base class.
Adopting a common superclass promotes simpler exception handling.
242-242:InvalidDependencyReferenceExceptionnow inherits fromDependencyInjectionException.
This is a neat step toward standardizing error handling across the DI module.
254-254: Refactor ofMissingDependencyException.
Extending from the newDependencyInjectionExceptionhelps maintain a single hierarchy for DI-related errors.
258-258:OutOfOrderDependencyExceptionmigrated to the new exception base.
It remains consistent with the other refactored exceptions. Good move.
273-274: NewDependencyInjectionExceptionbase class.
Exposing a uniform base for DI-related errors simplifies scenario-specific error handling.
37-40:Details
❓ Verification inconclusive
Great addition of
require()andvalidate()methods.
These methods effectively strengthen the contract for ensuring dependencies are present and valid at runtime. Please confirm they have adequate unit test coverage.
🏁 Script executed:
#!/bin/bash # Search for direct unit test references to the new require and validate methods in test files fd --type f --regex ".*test.*" | xargs rg -C 2 "require\\(|validate\\("Length of output: 11819
Action Required: Confirm and/or Add DI-Specific Unit Tests
Great addition of the
require()andvalidate()methods to enforce the dependency contract. However, after reviewing the test search output, I couldn’t find any explicit unit tests targeting these methods within the DI plugin’s scope. While similar functions are used elsewhere in the codebase, it’s unclear if the DI-specific implementations have been exercised.
- Please verify that there are dedicated tests in the DI module (e.g., under tests for
ktor-server-di) that cover both the expected successful behavior and failure cases forrequire()andvalidate().- If such tests do not yet exist, kindly add them to ensure robust and comprehensive coverage.
328-328:Details
❓ Verification inconclusive
New
provideDelegateextension function.
This extension enables convenient property delegated injection. Please confirm that usage or tests exist to ensure correctness.
🏁 Script executed:
#!/bin/bash # Search for usage of provideDelegate in code/tests to confirm coverage fd --type f --regex ".*(src|test).*" | xargs rg -C 2 "provideDelegate"Length of output: 70
Attention: Please manually verify test coverage for
provideDelegateThe new
provideDelegateextension function is intended to simplify property delegated injection. However, the automated search did not uncover any usage or tests for this functionality. Since the script didn’t produce any output, please manually verify whether there are tests or usage examples validating its behavior. If such tests don’t exist yet, it’s recommended to add them to ensure correctness before merging.
- Verify if tests or sample usage exist in other parts of the codebase.
- If not, add appropriate test cases or usage examples to cover this new functionality.
ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.api (9)
16-16:AmbiguousDependencyExceptionunification.
Now extendingDependencyInjectionException—this ensures consistency across all DI exceptions.
20-20:CircularDependencyExceptionconsistency.
Aligning with the new base class is good. Remember to retain the fail-fast approach to avoid partial resolutions in circular references (per KTOR-4732).
24-24: AligningDependencyAbstractTypeConstructionExceptionunder the new base class.
A straightforward move to unify DI-related exception handling.
119-122: Introducing the newDependencyInjectionException.
Centralizing DI exceptions under this class simplifies error categorization and future reuse.
234-234: UnifiedDuplicateDependencyExceptionunderDependencyInjectionException.
Ensures a standardized approach for managing multiple definitions of the same dependency.
252-252: RefactorInvalidDependencyReferenceExceptionto inherit from the new base.
Continues the trend to keep all DI exceptions under one hierarchy.
274-274:MissingDependencyExceptioninheritance update.
Keeps minimal duplication and consistent error handling.
283-283:OutOfOrderDependencyExceptionre-homed underDependencyInjectionException.
Completes the unified structure for DI exceptions.
287-290:Details
✅ Verification successful
New
PluginModuleInjectorclass.
This extendsApplicationModuleInjectorand introducesresolveParameterfor module DI. Solid improvement toward more flexible injection. Please confirm integration and test coverage.
🏁 Script executed:
#!/bin/bash # Search for references to PluginModuleInjector or resolveParameter in test files to confirm coverage fd --type f --regex ".*test.*" | xargs rg -C 2 "PluginModuleInjector|resolveParameter"Length of output: 1267
Integration and Test Coverage Confirmed for PluginModuleInjector
The newPluginModuleInjectorand itsresolveParametermethod are well-integrated. The tests, as seen in theresolveParametersFromCustomConfigcases within both thektor-server-servletandktor-server-servlet-jakartamodules, verify the intended behavior. No further changes are needed from a DI integration standpoint.
bjhham
force-pushed
the
bjhham/di-module-param-support
branch
2 times, most recently
from
8ffd440 to
d3eb9ce
Compare
bjhham
force-pushed
the
bjhham/di-module-param-support
branch
from
d3eb9ce to
5836e57
Compare
| import kotlin.getValue | ||
| import kotlin.reflect.KParameter | ||
|
|
||
| public class PluginModuleInjector : ApplicationModuleInjector { |
| pluginInstall: DependencyInjectionConfig.() -> Unit = {}, | ||
| block: Application.() -> Unit | ||
| ) = testApplication { | ||
| ): Unit = runTestApplication { |
There was a problem hiding this comment.
Please consider dropping Unitfrom the return type of the common tests
| listOf( | ||
| resolve<GreetingService>(), | ||
| resolve<List<String>>("my-strings"), | ||
| this.resolve<GreetingService>(), |
There was a problem hiding this comment.
Could you tell me why this is explicitly needed? I would try to rename clashing symbols if it is possible
There was a problem hiding this comment.
I think the IDE added it automatically, should be able to drop it.
| } | ||
| } | ||
|
|
||
| public inline operator fun <reified T> DependencyRegistry.provideDelegate( |
osipxd
left a comment
osipxd
left a comment
There was a problem hiding this comment.
DI is becoming better and better 👍
ktor-server/ktor-server-core/jvm/src/io/ktor/server/application/ApplicationModuleInjector.kt
Outdated
Show resolved
Hide resolved
| * | ||
| * Will trigger calls to resolve the parameters `param1` and `param2`. | ||
| * | ||
| * By default, parameters like `ApplicationEnvironment` are resolved automatically. |
There was a problem hiding this comment.
It would be great to have a full list of automatically resolved parameters, as it might be unobvious
...or-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyInjection.kt
Outdated
Show resolved
Hide resolved
...ver-plugins/ktor-server-di/jvm/src/io/ktor/server/plugins/di/utils/ClasspathReference.jvm.kt
Outdated
Show resolved
Hide resolved
bjhham
force-pushed
the
bjhham/di-module-param-support
branch
from
db603bc to
c49e4c4
Compare
vnikolova
left a comment
vnikolova
left a comment
There was a problem hiding this comment.
Looks good, just added some minor comments.
ktor-server/ktor-server-core/jvm/src/io/ktor/server/application/ModuleParametersInjector.kt
Outdated
Show resolved
Hide resolved
ktor-server/ktor-server-core/jvm/src/io/ktor/server/application/ModuleParametersInjector.kt
Outdated
Show resolved
Hide resolved
...tor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyRegistry.kt
Outdated
Show resolved
Hide resolved
...tor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyRegistry.kt
Outdated
Show resolved
Hide resolved
...tor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyRegistry.kt
Outdated
Show resolved
Hide resolved
Co-authored-by: Vik Nikolova <contact@viknikolova.com>
4 participants
Subsystem
Server, DI
Motivation
Solution
ApplicationorApplicationEnvironment), we'll delegate the injection of the parameter to the provided implementation ofApplicationModuleInjector.