Skip to content

Add support for nested modules #578

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

Closed
odrotbohm opened this issue May 3, 2024 · 13 comments
Closed

Add support for nested modules #578

odrotbohm opened this issue May 3, 2024 · 13 comments
Assignees
Labels
in: core Core module meta model in: documentation support Documentation generation type: enhancement Major enhanvements, new features
Milestone

Comments

@odrotbohm
Copy link
Member

The current module system is flat. In other words, only one level of modules is supported. In some cases, it would be nice to be able to structure a module into submodules, that would stay hidden from the outside, but allow hiding internals from the parent module. They would also only be visible to their parents and direct sibling modules.

Related discussions / issues

@odrotbohm odrotbohm added in: documentation support Documentation generation in: core Core module meta model type: enhancement Major enhanvements, new features labels May 3, 2024
@odrotbohm odrotbohm self-assigned this May 3, 2024
@jwedel
Copy link

jwedel commented Jun 4, 2024

What about the use case when having a top level package just for grouping modules?

Let's say:

- domain1
- domain2
- group.foo
- group.bar

In reality, group contains more packages in a very complex application. domain1+2 are core domains and top level modules. foo and bar are kind of auxiliary domains that are used by both domain1 and 2.

Would it be possible to put a package-info.java with ApplicationModule annotation inside foo and bar packages instead of the top level package group to just use the directory structure for grouping?

If this feature doesn't, would you consider this as a valid use case? If yes, I would create an issue for that.

@odrotbohm
Copy link
Member Author

If it's solely grouping, it's not nesting. You could tackle your scenario by providing a custom implementation of ApplicationModuleDetectionStrategy and declare the implementation in spring.factories like this:

org.springframework.modulith.core.ApplicationModuleDetectionStrategy=com.acme.YourImplementation

For convenience, we expose ApplicationModuleDetectionStrategies.explictlyAnnotated() that you could delegate to, to enforce that only explicitly annotated packages would be considered application module base packages. Thus, you'd need to use @ApplicationModule on domain1, domain2, foo and bar. group would be ignored. Does that work for you?

@jwedel
Copy link

jwedel commented Jun 4, 2024

@odrotbohm Yes, I think so! Thanks a lot. If that works, would it make sense to contribute this?

EDIT: Would this go under test or main? -> test works :)

@odrotbohm
Copy link
Member Author

As such an implementation would be entirely specific to your scenario, it would have to live in your implementation by definition, wouldn't it? Or do you mean us providing an implementation of that out of the box?

@jwedel
Copy link

jwedel commented Jun 4, 2024

@odrotbohm I mean contributing another ApplicationModuleDetectionStrategy that would also look for modules in sub packages.

@odrotbohm
Copy link
Member Author

I am not sure how you'd generally do that without explicit markers. How would you know you'd have to consider domain1/2 a module, but not group? Because it's not a leaf package? That feels pretty specific, and the extension point exists exactly to allow others to be creative with their detection algorithms, but at the same time us not having to explicitly support those scenarios.

I wouldn't mind seeing some means to make the “explicitly annotated packages only” mode a bit easier to use. A dedicated class for that might make sense. I guess we could swap out the current enums against dedicated classes then. I wonder if we could also inspect application.properties for a configuration value, but I don't know how to properly read those outside the lifecycle of an ApplicationContext. Bootstrapping one seems a bit of a heavy hammer to solely read a single property.

@jwedel
Copy link

jwedel commented Jun 4, 2024

It actually just works out of the box:

public class GroupingApplicationModuleDetectionStrategy implements ApplicationModuleDetectionStrategy {
   @Override
   public Stream<JavaPackage> getModuleBasePackages( JavaPackage basePackage ) {
      return ApplicationModuleDetectionStrategy.explictlyAnnotated().getModuleBasePackages( basePackage );
   }
}

No custom logic necessary. So would be great to switch this strategy. Why not adding an optional strategy to the Modulithic annotation?

@odrotbohm
Copy link
Member Author

ApplicationModuleDetectionStrategy lives inside spring-modulith-core, @Modulithic lives in …-api. I think I found a way to inspect the properties.

@jwedel
Copy link

jwedel commented Jun 5, 2024

@odrotbohm Don't you think that, putting aside that it currently has technical limitations, it would have been a lot better from a DX point of view to add it to the @Modulithic annotation? The module detection starts from the package of the spring application that has this annotation. So it would have been naturally the first place to look for a way to change the strategy.

I see two and a half ways this could still be done.

  1. Adding enums without implementation to the api package, including EXPLICITLY_ANNOTATED, DIRECT_SUB_PACKAGES and maybe CUSTOM. Then during runtime, the implementation would pick the correct class. When using custom, still the application.yaml or spring.factories could be used.
@Modulithic(
      moduleDetectionStrategy = Modulithic.ModuleDetectionStrategy.EXPLICITLY_ANNOTATED,
)
  1. Using a string type to resolve the strategy at runtime via reflection.
@Modulithic(
      moduleDetectionStrategy = "com.acme.MyCustomApplicationModuleDetectionStrategy",
)
  1. Or a mixture of both, allowing a custom strategy to pass a class reference or an enum to pick a pre configured strategy:
@Modulithic(
      moduleDetectionStrategy = Modulithic.ModuleDetectionStrategy.CUSTOM,
      customModuleDetectionStrategy = "com.acme.MyCustomApplicationModuleDetectionStrategy",
)

They all have some downsides, but at least there are a lot more accessible.

@odrotbohm
Copy link
Member Author

The reason I am hesitant about the annotation is twofold:

  1. We would have to mirror the existing annotations into the API module and map values around. This is a minor issue because it is only inconvenient for us. In fact, I got rid of the internal enum in the context of a spike to support an alternative approach already.
  2. And IMO, much more significant is the problem that we would want to be able to both allow selecting between the prepared and custom implemented strategies seamlessly. The annotation-based approach implies a two-property approach that implies weird naming decisions (they are both defining some strategy) and preference rules or decisions, what to do if we find conflicting configuration. A single-property approach does not suffer from this issue, but is impossible when using an annotation attribute.

I have a spike available that introduces an application property spring.modulith.detection-strategy and configuration metadata that both presents the predefined values plus any custom implementations of ApplicationModuleDetectionStrategy in the code completion for that property. I've filed GH-652 to keep track of those efforts. I would like to see that solution shipped, but wouldn't mind another ticket that could investigate what an annotation-based approach could look like in more detail as a follow-up.

odrotbohm added a commit that referenced this issue Aug 13, 2024
$ Conflicts:
$	spring-modulith-distribution/pom.xml
odrotbohm added a commit that referenced this issue Aug 13, 2024
The ApplicationModules bootstrap now triggers the module base package detection, followed by a new, additional pass of detecting nested application module packages. Those packages are now added to the ones we create ApplicationModule instances for and also handed into the module instance creation step as exclusions to make sure that parent modules do not include code residing in sub-modules.

Each module now operates on the Classes instance obtained from the JavaPackage instance that constitutes the module's base package but filtered by the given exclusions.
@odrotbohm
Copy link
Member Author

odrotbohm commented Aug 13, 2024

If anyone wants to try out nested application modules, please refer to 1.3.0-GH-578-SNAPSHOT. If you now place @ApplicationModule in packages nested inside a module (implicitly or explicitly declared) this will create a nested module of the parent arrangement. Accessibility rules still apply, i.e., only API code of these nested modules is accessible by the direct parent module. A nested module can depend on any top-level module's API code.

Nested modules are excluded from the overview component diagram, but have the excerpt diagrams and Application Module Canvases rendered for them. I guess we're going to refine the diagramming situation, as I can imagine that some visualization of the nesting might be helpful, but that needs further experimentation.

odrotbohm added a commit that referenced this issue Aug 16, 2024
The ApplicationModules bootstrap now triggers the module base package detection, followed by a new, additional pass of detecting nested application module packages. Those packages are now added to the ones we create ApplicationModule instances for and also handed into the module instance creation step as exclusions to make sure that parent modules do not include code residing in sub-modules.

The bootstrap of ApplicationModules now uses a dedicated ApplicationModuleSource to allow calculating a default module name relative to the application base package.

Each module now operates on the Classes instance obtained from the JavaPackage instance that constitutes the module's base package but filtered by the given exclusions.
odrotbohm added a commit that referenced this issue Aug 16, 2024
The ApplicationModules bootstrap now triggers the module base package detection, followed by a new, additional pass of detecting nested application module packages. Those packages are now added to the ones we create ApplicationModule instances for and also handed into the module instance creation step as exclusions to make sure that parent modules do not include code residing in sub-modules.

The bootstrap of ApplicationModules now uses a dedicated ApplicationModuleSource to allow calculating a default module name relative to the application base package.

Each module now operates on the Classes instance obtained from the JavaPackage instance that constitutes the module's base package but filtered by the given exclusions.
@odrotbohm odrotbohm modified the milestones: 1.3 M3, 1.3 M2 Aug 23, 2024
odrotbohm added a commit that referenced this issue Aug 30, 2024
Previously, we rejected references between sub-modules both contained in the same parent package.

Related ticket: GH-578.
@nilskasseckert
Copy link

nilskasseckert commented Oct 4, 2024

If anyone wants to try out nested application modules, please refer to 1.3.0-GH-578-SNAPSHOT. If you now place @ApplicationModule in packages nested inside a module (implicitly or explicitly declared) this will create a nested module of the parent arrangement. Accessibility rules still apply, i.e., only API code of these nested modules is accessible by the direct parent module. A nested module can depend on any top-level module's API code.

Thanks a lot! This works pretty well!

Nested modules are excluded from the overview component diagram, but have the excerpt diagrams and Application Module Canvases rendered for them. I guess we're going to refine the diagramming situation, as I can imagine that some visualization of the nesting might be helpful, but that needs further experimentation.
But why should the Overview work any differently? In our project, for example, we use the component architecture. The following structure (simplified):

-api
  - M1
  - M2
  - M3
- backend
   - bl
      - M1
      - M2
      - M3 
   - rest
      - M1
      - M2
      - M3
  - Main.java   
- shared
  - SharedM1
  - SharedM2 

In the BL package, there is a package for each module that contains the business logic. This exposes a service that can be accessed from outside. The Rest package has an equivalent structure. It accesses the service of the BL package and uses the entities that are included in the API package as entities. Shared code is also used

Now, of course, we would like to have a complete screen based on the main class. The individual screens do nothing other than display this for a module. Wouldn't it “only” have to be put together?

@odrotbohm
Copy link
Member Author

I am not sure I follow. The package setup doesn't seem to follow the Spring Modulith conventions at all. Also, there don't seem to be any submodules involved. I'd love this ticket to stay focussed on this particular aspect. Please open a general purpose discussion for input on structuring by the community.

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 in: documentation support Documentation generation type: enhancement Major enhanvements, new features
Projects
None yet
Development

No branches or pull requests

3 participants