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 ComponentScan#nameGenerator alias on @SpringBootApplication #19346

Closed
torsten-github opened this issue Dec 11, 2019 · 15 comments
Closed

Add ComponentScan#nameGenerator alias on @SpringBootApplication #19346

torsten-github opened this issue Dec 11, 2019 · 15 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@torsten-github
Copy link

torsten-github commented Dec 11, 2019

Please provide specialized BeanNameGenerator implementations and provide a property to choose one. E.g spring.bean-name-gernerator=package-aware (or default).

Problem
The default implementation fails, when two Components have the same class name. This issue raised at spring framework also suggests the use of a custom BeanNameGenerator.

The code needed for the custom implantation is about 10 Lines in the main class. The registration needs to be done twice if a Servlet is used and can easily be missed. Also needs special treatment for @SpringBootTest classes.

UPDATE: ... just a view Lines! But still a little anoying. Learned to use the @ComponentScan(nameGenerator = CustomGenerator.class ) (updated my example)

I think "good" structured code is likely to have classes with the same name, but in different packages. Especially when dealing with bounded contexts (DDD) where a Entity xyz can have different meanings in different contexts. These Entities are in no way beans, but they will raise some services like xyzService, xyzMapper, etc.

@philwebb
Copy link
Member

philwebb commented Dec 13, 2019

I'm not too keen to add an application.properties entry since class name resolution wouldn't be type safe. I think we could offer the same nameGenerator attribute on @SpringBootApplication instead. Would that work for you?

@philwebb philwebb added type: enhancement A general enhancement and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Dec 13, 2019
@philwebb philwebb added this to the 2.3.x milestone Dec 13, 2019
@torsten-github
Copy link
Author

Yes, I agree. That seems to be a more appropriate place.

@snicoll
Copy link
Member

snicoll commented Dec 30, 2019

I've started to look at this one and I am now wondering if that's worth it. SpringApplicationBuilder already offers the infrastructure to customize a BeanNameGenerator with a one-liner, e.g.

new SpringApplicationBuilder(DemoApplication.class)
    .beanNameGenerator(new CustomBeanNameGenerator())
    .run(args);

It should be noted that specifying an attribute override on @SpringBootApplication takes precedence to this (which was surprising).

@torsten-github what is wrong with the approach above? It also has the advantage of you providing the concrete instance (whereas the annotation approach forces you to have a default public constructor that we'd call to create it).

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Dec 30, 2019
@spring-projects-issues
Copy link
Collaborator

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Jan 6, 2020
@torsten-github
Copy link
Author

@torsten-github what is wrong with the approach above?
In my case, using a SpringBootServletInitializer, this does not work, since SpringApplication Builder... is executed on e.g. JBoss. Luckily the @ComponetnScan(nameGenerator = ...) works in both worlds :)

Main thought
The need for a package aware BeanNameGenerator is very likely. Please also see my issue at spring-framework

Would be nice to choose it out of the box. Maybe my own implementation is not spring-like, or misses some internal points I'm not aware of at the moment...? Makes me feel better, to know I'm use a spring-framework generator, which can handle same class names in different packages.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue labels Jan 6, 2020
@snicoll
Copy link
Member

snicoll commented Jan 6, 2020

In my case, using a SpringBootServletInitializer, this does not work, since SpringApplication Builder... is executed on e.g. JBoss. Luckily the @ComponetnScan(nameGenerator = ...) works in both worlds :)

Why is that? There is a configure method that takes the builder, which allows you to invoke the method I've mentioned.

Would be nice to choose it out of the box. Maybe my own implementation is not spring-like, or misses some internal points I'm not aware of at the moment...? Makes me feel better, to know I'm use a spring-framework generator, which can handle same class names in different packages.

I think you've lost me. I am merely suggesting a mechanism to set the BeanNameGenerator already exists using the builder. This isn't different from whatever the extra attribute on @SpringBootApplication would do.

@snicoll snicoll added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jan 6, 2020
@torsten-github
Copy link
Author

@snicoll What I'm trying to ask for, is: Please provide a package aware BeanNameGenerator class which I can setup. Instead of having me to write my own.

If you look at the spring-framework issue I mentioned above, you can see the code I need to write:

@SpringBootApplication
@ComponentScan(nameGenerator = CustomGenerator.class)
public class App extends SpringBootServletInitializer {

  public static void main(String[] args) {
    SpringApplication.run(App.class, args);
  }

  @Override
  protected SpringApplicationBuilder configure(SpringApplicationBuilder application) {
    return application
        .sources(App.class);
  }

  public static class CustomGenerator extends AnnotationBeanNameGenerator {
    @Override
    public String generateBeanName(BeanDefinition definition, BeanDefinitionRegistry registry) {
      return definition.getBeanClassName();
    }
  }
}

If I get your point, you want me to configure the BeanNameGenerator in the static main method. This will not work, on a Server, since it is run as a Servlet.

@torsten-github

This comment has been minimized.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 6, 2020
@snicoll
Copy link
Member

snicoll commented Jan 6, 2020

Please provide a package aware BeanNameGenerator class which I can setup. Instead of having me to write my own.

That's not the question I am asking, if you want to advocate for that more, please add a comment in the relevant issue.

If I get your point, you want me to configure the BeanNameGenerator in the static main method. This will not work, on a Server, since it is run as a Servlet.

No, I am not asking you to do that. You've exactly the setup I mentioned in my previous comment with a configure method. This should work:

@SpringBootApplication
public class App extends SpringBootServletInitializer {

  public static void main(String[] args) {
    SpringApplication.run(App.class, args);
  }

  @Override
  protected SpringApplicationBuilder configure(SpringApplicationBuilder application) {
    return application
        .beanNameGenerator(new CustomGenerator.class) 
        .sources(App.class);
  }

  public static class CustomGenerator extends AnnotationBeanNameGenerator {
    @Override
    public String generateBeanName(BeanDefinition definition, BeanDefinitionRegistry registry) {
      return definition.getBeanClassName();
    }
  }
}

I'll keep this one open until we get a chance to discuss it.

@wilkinsona
Copy link
Member

I think it makes sense to provide an alias for nameGenerator on @SpringBootApplication. Configuring the name generator in the main method (or configure method for war deployments) means that the configuration is lost when running an integration test. The alternative is to use @ComponentScan which is cumbersome when using sliced tests as the exclude filters need to be re-declared.

@snicoll
Copy link
Member

snicoll commented Jan 6, 2020

Ah, that's a good point Andy, I missed that use case.

@snicoll snicoll removed the status: feedback-provided Feedback has been provided label Jan 6, 2020
@torsten-github
Copy link
Author

That's not the question I am asking, if you want to advocate for that more, please add a comment in the relevant issue.

Oh, I thought this issue provide specialized BeanNameGenerator implementation... raised by me is the place for that. I hoped the boot project would make the configuration part a one-liner.

But, never mind, I think I made my point. And I'm not sure if I can still follow. It's OK for me. Thank you very much for your time.

This should work:

Ok, now I see. Missed that Builder in the parameter. I used to have the Builder in the main method.

Just for completeness: I changed my code from Annotation to your suggestion. But it will no longer start without a Server (using SpringBoot startup) not sure about the name. I think it's the build in tomcat?!

@snicoll snicoll self-assigned this Jan 6, 2020
@snicoll snicoll changed the title provide specialized BeanNameGenerator implementations via property setting Add ComponentScan#nameGenerator alias on @SpringBootApplication Jan 6, 2020
@snicoll snicoll modified the milestones: 2.3.x, 2.3.0.M1 Jan 6, 2020
@snicoll snicoll closed this as completed in ee75557 Jan 6, 2020
@torsten-github
Copy link
Author

I think we could offer the same nameGenerator attribute on @SpringBootApplication instead. Would that work for you?

Ah - now I get it. Sorry.
But it still implied for me the "provide ... implementation" part. Isn't that a core frature of spring-boot?
Would be nice to here a yes or no. Thanks

@snicoll
Copy link
Member

snicoll commented Jan 6, 2020

@torsten-github I already provided a link to the relevant issue. There is no reason for having that default in Spring Boot since Spring Framework can open the one it has privately at the moment. For further updates on the default implementation, please follow-up on this issue.

@sbrannen
Copy link
Member

sbrannen commented Jan 7, 2020

FYI: spring-projects/spring-framework@b4c91e7 introduced a new FullyQualifiedAnnotationBeanNameGenerator that will be available in Spring Framework 5.2.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

6 participants