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

getMappingForMethod failed when implement a interface method(with @RequestMapping) in super class [SPR-17223] #21756

Closed
spring-projects-issues opened this issue Aug 29, 2018 · 6 comments
Assignees
Labels
in: web status: declined type: enhancement

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Aug 29, 2018

s-mop opened SPR-17223 and commented

Here is the demo codes:

@RequestMapping(value = "order/customer")
@RestController
public interface CunstomerOrderClient {    
    @PostMapping("rating")
    String rating();
}
public class AbstractOrderService {    
    public String rating() {
        return "rated";
    }
}
@Service
public class CustomerOrderService extends AbstractOrderService implements CunstomerOrderClient {    
}

"order/customer/rating" dosnt registered as I expected

 I tried to find reason and get these codes in spring-webmvc:

 

 org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping.getMappingForMethod(Method, Class<?>)

protected RequestMappingInfo getMappingForMethod(Method method, Class<?> handlerType) {
    RequestMappingInfo info = createRequestMappingInfo(method); 
    if (info != null) { 
        RequestMappingInfo typeInfo = createRequestMappingInfo(handlerType); 
        if (typeInfo != null) { info = typeInfo.combine(info); } 
    } 
    return info; 
}

and 

 org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping.createRequestMappingInfo(AnnotatedElement)

private RequestMappingInfo createRequestMappingInfo(AnnotatedElement element) { 
    RequestMapping requestMapping = AnnotatedElementUtils.findMergedAnnotation(element, RequestMapping.class); 
    RequestCondition<?> condition = (element instanceof Class ? getCustomTypeCondition((Class<?>) element) : getCustomMethodCondition((Method) element));
    return (requestMapping != null ? createRequestMappingInfo(requestMapping, condition) : null); 
}

These codes seems like try to find @ReqeuestMapping in super class or interface of witch i defined my 'ranking' method

 

Of course they can't find @ReqeuestMapping because the 'ranking' method is defined in AbstractOrderService, and AbstractOrderService is not implements CunstomerOrderClient.


Affects: 5.0.5

Issue Links:

  • #15682 Enable REST controller method parameter annotations on an interface
  • #22028 AOP support for SPR-17223 & #1950

Referenced from: pull request #1950

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 4, 2018

s-mop commented

I wonder if you mind me creating a PR.

 

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 20, 2018

s-mop commented

Let me explain why I need declared methods in interfaces and implemented in super class

Usually we need declare a lot of APIs for various entities in one or more controllers like:

public User create(User user) {...};
public Order create(Order order) {...};
public Product create(Prodcut product) {...};

public User update(User user) {...};
public Order update(Order order) {...};
public Product update(Prodcut product) {...};

 

To avoid redundant code we can declare a parent class like this:

public abstract class CommonController<T> {
    public T create(T entity) {...}
    
    public T update(T entity) {...}

    public List<T> search(T entity) {...}
}

public class UserController extends CommonController<User> {...}

public class OrderController extends CommonController<Order> {...}

 

But, with the increasement of entities and common APIs number. We met with a problem.Every controller declared the APIs as many as the methods defined in CommonController.

In our production scenario, Most controllers(90%+) only need some of mothods(about 30%-50%) witch defined in CommonController.

I tried to split CommonController to spread classes like CommonCreateOrUpdateController, CommonQueryController, but it seems not very helpful.

After a few research on github and stackoverflow, I found that APIs defined in interface is supported.

So now I can declare APIs like this with this PR :

 

public interface FilterClient<T> {

    @RequestMapping(value = "filter", method = RequestMethod.POST) 
    PageRep<? extends T> filter(@RequestParam("pageNum") Integer pageNum, @RequestParam("pageSize") Integer pageSize, 
                                @RequestBody Filter filter);
}

public interface CreateClient<T> {

    @RequestMapping(value = "/create", method = RequestMethod.POST)
    T createByViewType(@RequestBody @Validated({Creating.class, Default.class}) T t);
}


@RequestMapping(value = "order")
@RestController
public interface OrderClient extends FilterClient<Order>, CreateClient<Order> {    

    @PostMapping("rating")
    public void rating();
}

public abstract class AbstractPersistenceService<T> { 
    public T create(T entity) {...} 
    public void update(T entity) {...} 
}

@Service
public class OrderService extends AbstractPersistenceService<Order> implements OrderClient {    
}

 

It works like a charm with other framework(like Hibernate-Validator,Swagger2) on our staging environment.

I saw the fix version of this issue is updated from 5.1 GA to 5.X backlog .But, at last, I realy hope it can be merged in an earlier version

Thank you for your patience and please let me know if there are any inadequacies.

 

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 25, 2018

s-mop commented

Juergen Hoeller any advice?

@rstoyanchev
Copy link
Contributor

@rstoyanchev rstoyanchev commented Apr 2, 2020

So the issue here is that the implementing method is on an abstract class that isn't itself implementing the interface. Would use of delegate classes be a better fit, especially in a situation akin to multiple inheritance (i.e. "Most controllers(90%+) only need some of methods (about 30%-50%) which are defined in CommonController")?

@s-mop
Copy link

@s-mop s-mop commented Apr 30, 2020

So the issue here is that the implementing method is on an abstract class that isn't itself implementing the interface. Would use of delegate classes be a better fit, especially in a situation akin to multiple inheritance (i.e. "Most controllers(90%+) only need some of methods (about 30%-50%) which are defined in CommonController")?

My purpose is let one Controller/Service register some common API by only Implement the common Interfaces what they need. Instead of implement every common API or call several Repositories
Im not quiet sure about the 'delegate classes'
If there is other ways to do so by minimum redundancy code. Please give me some hints

@rstoyanchev
Copy link
Contributor

@rstoyanchev rstoyanchev commented Dec 3, 2021

Sorry for the delayed response. To me it still feels like a case of multiple inheritance and the path for that in Java is through delegation. The scenario however is a bit involved and although you've provided some snippets, it's still not completely clear.

It would be best to provide an actual sample, sufficiently representative of the problem, which would allow us to reason about alternative approaches or possible changes in the framework. While the changes proposed in #1950 may be working for you, they could also cause side effects such as is the case with #22028.

I'm closing as possibly outdated, but we can re-consider if this is still of interest.

@rstoyanchev rstoyanchev removed this from the 6.x Backlog milestone Dec 3, 2021
@rstoyanchev rstoyanchev added the status: declined label Dec 3, 2021
@rstoyanchev rstoyanchev self-assigned this Dec 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web status: declined type: enhancement
Projects
None yet
Development

No branches or pull requests

3 participants