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

Add support for nested modules #578

Open
odrotbohm opened this issue May 3, 2024 · 10 comments
Open

Add support for nested modules #578

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

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.

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

2 participants