koin-annotations icon indicating copy to clipboard operation
koin-annotations copied to clipboard

feat: @ComponentScan glob support

Open OffRange opened this issue 1 year ago • 2 comments
trafficstars

Summary

This PR enhances the @ComponentScan annotation in Koin Annotations to support glob patterns. Currently, @ComponentScan only allows specifying a single, exact package name per Koin module. With this change, developers can use wildcards (* and **) to scan multiple packages or subpackages in a more flexible and concise way.

Motivation

In modern, modular JVM applications, components (like services, repositories, or utilities) are often spread across multiple subpackages. The current @ComponentScan requires developers to either:

  1. Create a Koin module for each subpackage, leading to module proliferation.
  2. Place all components in a single package, which can become unwieldy.

By supporting glob patterns, we allow developers to organize their code more naturally without compromising on Koin's autowiring capabilities.

Implementation

  1. Added GlobToRegex.kt utility:

    • Converts glob patterns to regular expressions.
    • Supports * (single-level) and ** (multi-level) wildcards.
    • Case-sensitive by default, as package names in JVM are case-sensitive.
  2. Modified KoinMetaDataScanner and KoinMetaData:

    • Now uses custom extension function to interact with the GlobToRegex utility
  3. Tests

    • Still passes

Examples

Before:

@ComponentScan("com.app.service")
class ServiceModule
@ComponentScan("com.app.service.user")
class UserServiceModule
@ComponentScan("com.app.service.order")
class OrderServiceModule

After:

@ComponentScan("com.app.service.**")
class ServiceModule

However we can still use:

@ComponentScan("com.app.core")
class CoreModule

OffRange avatar Jun 07 '24 20:06 OffRange

Hello,

great addition. Could you add some local unit tests to help be sure we cover those regex?

Thanks 🙏

arnaudgiuliani avatar Jun 25 '24 12:06 arnaudgiuliani

Hello,

great addition. Could you add some local unit tests to help be sure we cover those regex?

Thanks 🙏

Of course, since all tests are located in modules within the examples module, I assume my tests should be placed there as well. Do you have any preferences regarding a specific module? Alternatively, would you prefer that I create a new module within examples for the regex testing, or should I create a new test source set for the koin-ksp-compiler module within the projects module?

OffRange avatar Jun 26 '24 08:06 OffRange

@OffRange can you reupdate/rebase your branch?

arnaudgiuliani avatar Dec 23 '24 15:12 arnaudgiuliani

rebased

OffRange avatar Jan 07 '25 16:01 OffRange

arf sorry, tried to solve the conflict. Can you revert/fix my commit? 🙏

arnaudgiuliani avatar Jan 27 '25 17:01 arnaudgiuliani

I identified and resolved a bug that compromised backward compatibility by modifying the regex to allow com.example.foo.bar when the component scan string is com.example.foo. Additionally, I enhanced the testing suite by duplicating the coffee-maker example and adapting it to use glob regexes, thereby verifying the fix and providing a template for future tests.

Now it should pass any tests

OffRange avatar Jan 27 '25 23:01 OffRange

What do you think of this PR: https://github.com/InsertKoinIO/koin-annotations/pull/182 this aim to bring multiple path scanning with the annotation

arnaudgiuliani avatar Jan 28 '25 15:01 arnaudgiuliani

Yeah, this PR looks great. However devs are required to type out every package to scan, with my proposal, devs could just use the glob pattern to include packages. Maybe a combination of both is suitable to give devs the option to choose. This would also allow to combine multiple "glob-branches", e.g. com.example.feat_a.** and com.example.feat_b.*

OffRange avatar Jan 28 '25 16:01 OffRange

@OffRange sure, your proposal is powerful. Do you mind merging your update after #182 ? Else we won't be able to port your changes :/

arnaudgiuliani avatar Feb 06 '25 16:02 arnaudgiuliani

Absolutely! I'll wait for #182

OffRange avatar Feb 06 '25 18:02 OffRange

@OffRange the previous PR is merged. You can add yours

arnaudgiuliani avatar Feb 19 '25 07:02 arnaudgiuliani

@arnaudgiuliani I merged and resolved conflicts to adopt #182. Let me know if this looks good

OffRange avatar Feb 19 '25 23:02 OffRange

Excellent @OffRange 👍

arnaudgiuliani avatar Feb 20 '25 11:02 arnaudgiuliani

Hey @arnaudgiuliani, sorry about the CI issues—I just noticed it's failing because of changes in #230, and it's triggering errors in the CoffeeGlobAppTest. https://github.com/InsertKoinIO/koin-annotations/blob/4206178e1ef8d8cefb92db4d33f6d1b133d5b176/examples/coffee-maker-glob/src/test/java/CoffeeGlobAppTest.kt#L81

I realize that copying the coffee-maker test module for the glob feature wasn’t the best approach, as any change to the API means updating both modules. To fix this, I see a couple of options:

  • Merge the Tests: I can open a new PR to combine the two test modules into one, so we only need to update one place.
  • Extract Common Logic: Alternatively, I can open a new PR to refactor the shared parts into a common base module that both tests can extend, keeping things DRY while still supporting both scenarios.

Let me know what you think, and thanks for your understanding!

OffRange avatar Feb 20 '25 13:02 OffRange

FYI @arnaudgiuliani, this was a breaking change for people that were using @ComponentScan with no parameter value, depending on its default behaviour.

With these changes, Koin started to generate defaultModules in my project, in multi-module projects it'd be normal to generate multiple default modules, which is causing naming conflicts and failing to compile.

It was working back in RC1, and does not work in RC3 and RC4.

Declaring exact package value for @ComponentScan solves this particular issue.

However, the second and more prominent issue is the dependencies in nested packages are not found at all. .** just does not work to me and the previous, default behavior has changed as described above, so there is no way to add them automatically to the module.

 * 3. Multi-level wildcard (`**`): `"com.example.**"`
 *    - Matches any number of package levels.
 *    - E.g., `com.example`, `com.example.service`, `com.example.service.user`.

This does not work to me either for com.example dependencies, nor for nested packages like com.example.service. This should be fixed or reverted because IMHO it just does not work as expected and breaks projects :confused:

krzdabrowski avatar Feb 22 '25 20:02 krzdabrowski

Hey @krzdabrowski, sorry to hear you're experiencing this issue. I tried reproducing the problem with ComponentScan using no parameters, but I wasn't able to replicate the results. Could you please provide a sample code snippet to help me diagnose the issue further?

Regarding the wildcard behavior: when using com.example.**, it correctly matches modules defined in subpackages like com.example.foo or deeper (e.g. com.example.foo.bar). However, as designed, this pattern does not match modules directly within com.example because it only scans its subpackages. Thanks for pointing out the documentation mistake—I’ll submit a PR to correct it.

If you want to match modules both directly under com.example and in its subpackages, you have three options:

  1. Use @ComponentScan("com.example")
  2. Use @ComponentScan("com.example**")
  3. Use @ComponentScan(value = ["com.example", "com.example.**"])

OffRange avatar Feb 23 '25 13:02 OffRange

Hey @OffRange, glad to hear it was the documentation with the mistake, not the code :slightly_smiling_face:

I believe it's kinda hard to give the code snippet because after some analysis on my side, it sounds like an edge case that happened to me. Still, I believe bug occurs. Here are some traits that I believe might be helpful to reproduce it:

  • Kotlin Multiplatform project, with commonMain, androidMain and iosMain sets
  • also, Gradle multi-module project
  • module names can look 'similar' to Koin when generating code, for example I had issue with modules :feature:login:data and :feature:settings:data

With no specific parameter value used in @ComponentScan, it looked like it couldn't "make a hit" with my scanned dependencies into the module I'd expect, resulting in hitting into defaultModule. This happened at least for few of my dependencies, landing into its defaultModules in each Gradle module. But somehow, for Koin, in the setup of multi-module KMP project, the paths were conflicting somehow, seeing "redeclaration of defaultModule error" while compilation. That's all what I have.

EDIT: I think I might have found the scenario. Have a module like this - both scanning for components, and declaring some on its own:

@Module
@ComponentScan
object SettingsModule {

    @Factory
    (...)
}

Then, create a sub-package and have another dependency there you want to scan, like:

@Single
internal class SettingsRepositoryImpl (...)

This dependency will not be added to generated module, but rather lands in defaultModule. When such situation happens for multiple Gradle and Koin modules, it creates multiple defaultModules, hence redeclaration and failing the compilation.

EDIT2: It also happened to me with the Koin module without any declarations inside, and other annotated dependency located in the same package - however, I don't know why it happened for that one module. But the result is the same - creating defaultModule, which causes issues.

EDIT3: All my problematic modules are located in commonMain so I believe the project needs to be a KMP one.

Declaring the package name in @ComponentScan parameter value fixes the issue but it feels like a regression because it worked in RC1.

krzdabrowski avatar Feb 23 '25 17:02 krzdabrowski

@krzdabrowski can you open an issue?

arnaudgiuliani avatar Feb 24 '25 08:02 arnaudgiuliani

@arnaudgiuliani done

krzdabrowski avatar Feb 24 '25 17:02 krzdabrowski