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

Support nested classes/interfaces for repository fragments [DATACMNS-1754] #2170

Closed
spring-projects-issues opened this issue Jun 24, 2020 · 3 comments
Assignees
Labels
in: repository type: enhancement

Comments

@spring-projects-issues
Copy link

@spring-projects-issues spring-projects-issues commented Jun 24, 2020

kirmerzlikin opened DATACMNS-1754 and commented

Issue: if repository fragment interface is declared inside another class, its implementation is never found

Steps to reproduce: see reference URL

 

I did some digging and here's what I found:

  • NestedFragmentInterfaceImpl is not recognized as an implementation of a repository fragment interface NestedFragmentInterface
  • everything works fine if repository fragment interface is not nested
  • the root cause of this behaviour is DefaultImplementationLookupConfiguration not being able to match expected implementation class name for +nested interface+ and actual +nested class+ name

On the last point, it looks like in the case with a nested interface DefaultImplementationLookupConfiguration "normalizes" interface name and class name differently.

Method getImplementationClassName() only shortens interface name (i.e. trims package name) and adds implementation postfix. This means that for nested interface  EnclosingClass$NestedFragmentInterface we get "EnclosingClass.NestedFragmentInterfaceImpl" as a result.

In the same time method matches() not only shortens bean class name but also "localizes" it (i.e. removes enclosing class name). So even for nested class EnclosingClass$NestedFragmentInterfaceImpl we get "NestedFragmentInterfaceImpl".
 

In theory there are 3 options here:

  • leaving everything as is, accepting that repository fragment interfaces can not be nested (otherwise fragment implementations won't be found because bean class names will always be stripped of enclosing class name)
  • adjusting matches() to avoid using local bean class name if interface name is not local, this will allow fragment interfaces and implementation classes to be declared inside the same enclosing class (can be useful in tests)
  • adjusting getImplementationClassName() to use local interface name, this will allow repository interfaces and implementation classes to be declared anywhere (which also means repository interface nested in class A can have implementation nested in class B)

Suggested changes for option #2 - kirmerzlikin@4567d71
Suggested changes for option #3 - kirmerzlikin@736ceafed713ac662e8b2622760475196584eeed 


Reference URL: master...kirmerzlikin:nested-repository-fragment

Referenced from: pull request #460

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jul 10, 2020

kirmerzlikin commented

Oliver Drotbohm I would appreciate it if you could take a look at this issue.

Also in case one of suggested solutions seems good enough for you, I would be happy to polish it and prepare a pull request

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jul 14, 2020

Mark Paluch commented

It makes sense to support nested fragment interfaces and fragment implementation classes to be discovered when considerNestedRepositories is configured to true. Feel free to submit a pull request along with tests that verify the desired behavior

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jul 18, 2020

kirmerzlikin commented

Mark Paluch Thanks for your response.

Pull request is created, here's the link - #460

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

No branches or pull requests

2 participants