Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Module-internal dependencies rejected if allowed dependencies defined explicitly #660

Closed
mateuszradziszewski opened this issue Jun 18, 2024 · 5 comments
Assignees
Labels
in: core Core module meta model type: bug Something isn't working
Milestone

Comments

@mateuszradziszewski
Copy link

mateuszradziszewski commented Jun 18, 2024

Hi,

I came across issue where verification of application module dependencie fails even though it should not. Consider following example:

Packages structure

├── product 
│     ├── package-info.java             <-- Annotated with `@ApplicationModule(allowedDependencies={})`
│     └── repository
│             └── package-info.java     <-- Annotated with `@NamedInterface("repository")`
│             └── Product.java
│             └── ProductRepository.java
├── productmongodb 
│     └── package-info.java             <-- Annotated with `@ApplicationModule(allowedDependencies = {"product::repository"})`
│     └── MongoDbProductRepository.java
└── Application.java                       <-- Annotated with `@SpringBootApplication`

ProductRepository is interface with instance method save(Product product).
MongoDbProductRepository is concrete class implementing ProductRepository interface.

Expected: Verification should pass.
Actual: Verification fails with message "org.springframework.modulith.core.Violations: - Module 'product' depends on named interface(s) 'product :: repository' via com.example.demo.product.repository.ProductRepository -> com.example.demo.product.repository.Product. Allowed targets: none."

Hint:
The reason is that QualifiedDependency.fromMethodsOf(JavaClass source) method actually analyzes method ProductRepository.save(Product product) for passed argument of MongoDbProductRepository.
Exact problematic line of code: https://github.com/spring-projects/spring-modulith/blob/main/spring-modulith-core/src/main/java/org/springframework/modulith/core/ApplicationModule.java#L1124

@odrotbohm
Copy link
Member

Is there any chance you provide a minimal, complete reproducer? While it's obvious that a module-internal dependency should not be reported as invalid, I still don't quite get which class that's logically assigned to the product module is causing the offending dependency.

Am I deducing right, that the processing of MongoDbProductRepository (assumingly extending ProductRepository?) processes ProductRepository, its methods and finds save(…) which registers a dependency from ProductRepository to Product. In isValidDependency(…) the check on the declared dependencies then rejects Product as it does not consider it to reside in the same module. Correct?

@odrotbohm odrotbohm self-assigned this Jun 18, 2024
@odrotbohm odrotbohm added in: core Core module meta model type: bug Something isn't working labels Jun 18, 2024
@odrotbohm
Copy link
Member

Nevermind the reproducer. Got it already.

@mateuszradziszewski
Copy link
Author

I was about to give you the reproducer :) Your understanding is correct.

@odrotbohm odrotbohm changed the title False application module dependency violation for same-package dependencies Module-internal dependencies rejected if allowed dependencies defined explicitly Jun 18, 2024
@odrotbohm odrotbohm added this to the 1.3 M1 milestone Jun 18, 2024
odrotbohm added a commit that referenced this issue Jun 18, 2024
…encies.

As we process a type's entire type hierarchy for dependencies we might discover a foreign module's internal dependencies for modules that declare allowed dependencies explicitly. Explicitly declared dependencies so far solely verified the target being explicitly listed, which, for internal dependencies does not make sense. We now immediately start checking the source and target modules to be equivalent, in which case we can skip any further processing.
@odrotbohm
Copy link
Member

This should be fixed now. I've also added a backport into 1.2.x via GH-661 and will add further ones for 1.1.x and 1.0.x. Feel free to give the 1.2.1 snapshots a try.

@mateuszradziszewski
Copy link
Author

I verified that my case works well with version 1.2.1-SNAPSHOT.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Core module meta model type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants