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

List Controller methods from parent class #225

Open
ST-DDT opened this issue Feb 28, 2017 · 8 comments
Open

List Controller methods from parent class #225

ST-DDT opened this issue Feb 28, 2017 · 8 comments

Comments

@ST-DDT
Copy link

ST-DDT commented Feb 28, 2017

Currently only the methods present in the topmost controller class are detected by jsonDoc.
However in some applications there is a lot of repetitive logic in the controllers so I moved them to a shared parent class. However now they will no longer be detected by jsonDoc. it would be nice if methods from parent classes will be detected by jsonDoc as well.

This can be fixed by adding a parent class check/recursion to this method:
AbstractSpringJSONDocScanner#jsondocMethods()

@ST-DDT
Copy link
Author

ST-DDT commented May 23, 2017

Example fix:

@Override
public Set<Method> jsondocMethods(final Class<?> controller) {
	// NEWLY ADDED - START
	if (controller == null) {
		return new LinkedHashSet<>();
	}
	// NEWLY ADDED - END
	final Set<Method> annotatedMethods = new LinkedHashSet<>();
	for (final Method method : controller.getDeclaredMethods()) {
		if (method.isAnnotationPresent(RequestMapping.class)) {
			annotatedMethods.add(method);
			System.out.println(controller.getSimpleName() + " " + method);
		}
	}
	// NEWLY ADDED - START
	annotatedMethods.addAll(jsondocMethods(controller.getSuperclass()));
	// NEWLY ADDED - END
	return annotatedMethods;
}

EDIT: See below for additional information.

@joffrey-bion
Copy link

joffrey-bion commented Aug 14, 2017

@ST-DDT Since controller methods are public, it would actually be much simpler and achieve the same purpose if we used controller.getMethods() instead of controller.getDeclaredMethods(). This way, we wouldn't even need the recursion.
Or do you have any example of non-public methods that you would like to see documented?

@ST-DDT
Copy link
Author

ST-DDT commented Sep 26, 2017

@joffrey-bion AFAIK Spring does not require Controller methods to be public (although they should be IMO) thus this approach is necessary

@ST-DDT
Copy link
Author

ST-DDT commented Sep 26, 2017

Unfortunately my solution/the parent scanning shows some other issues, because the @RequestMapping from the controller class is not present on parent classes thus the paths are wrong:

It will be displayed as /{id} instead of /myController/{id}

This can be fixed by

from

public abstract ApiMethodDoc initApiMethodDoc(Method method, Map<Class<?>, JSONDocTemplate> jsondocTemplates);

to

public abstract ApiMethodDoc initApiMethodDoc(Class<?> controller, Method method, Map<Class<?>, JSONDocTemplate> jsondocTemplates);

from

public static Set<String> buildPath(Method method) {
    Class<?> controller = method.getDeclaringClass();

to

public static Set<String> buildPath(Class<?> controller, Method method) {

PS: @joffrey-bion AFAICT your impl is likely to be also affected from this

@joffrey-bion
Copy link

joffrey-bion commented Sep 26, 2017

@ST-DDT Thanks for pointing it out. I'll double check Spring's implementation to see how paths are actually read. Or maybe you know already?

Suppose we have this situation:

@Controller
@RequestMapping("/parent")
public class ParentController {

    @RequestMapping("/parentMethod")
    public void parentMethod() { }
}

@Controller
@RequestMapping("/child")
public class ChildController extends ParentController {

    @RequestMapping("/childMethod")
    public void childMethod() { }
}

What API does Spring generate? I would say:

  • /child/childMethod
  • /child/parentMethod

But is the parent controller type-annotation really ignored?

@ST-DDT
Copy link
Author

ST-DDT commented Sep 26, 2017

Usually the parent would neither be annotated with @Controller nor with @RequestMapping. But yes the child wins.

For clarification:

// @Controller
// @RequestMapping("/parent")
public class ParentController {

    @RequestMapping("/parentMethod")
    public void parentMethod() { }
}

@Controller
@RequestMapping("/child")
public class ChildController extends ParentController {

    @RequestMapping("/childMethod")
    public void childMethod() { }
}

Will provide the following request urls:

  • /child/childMethod
  • /child/parentMethod

@Controller
@RequestMapping("/parent")
public class ParentController {

    @RequestMapping("/parentMethod")
    public void parentMethod() { }
}

@Controller
@RequestMapping("/child")
public class ChildController extends ParentController {

    @RequestMapping("/childMethod")
    public void childMethod() { }
}

Will provide the following request urls:

  • /child/childMethod
  • /child/parentMethod
  • /parent/parentMethod

// @Controller
// @RequestMapping("/parent")
public class ParentController {

    @RequestMapping("/parentMethod")
    public void parentMethod() { }
}

@Controller
@RequestMapping("/child")
public class ChildController extends ParentController {

    @Override
    @RequestMapping("/childMethod")
    public void parentMethod() { }
}

Will provide the following request urls:

  • /child/childMethod

package a;

// @Controller
// @RequestMapping("/parent")
public class ParentController {

    @RequestMapping("/parentMethod")
    void parentMethod() { }
}

package b;

@Controller
@RequestMapping("/child")
public class ChildController extends ParentController {

    @RequestMapping("/childMethod")
    void parentMethod() { }
}

Will provide the following request urls:

  • /child/childMethod
  • /child/parentMethod

RequestMappings are resolved in org.springframework.web.servlet.handler.AbstractHandlerMethodMapping.detectHandlerMethods(Object) using:

This produces an org.springframework.web.servlet.mvc.method.RequestMappingInfo and there you can extract all those paths and other stuff from. Unfortunately you will probably have to create a fake sub class of that class that exposes the RequestMappingHandlerMapping.getMappingForMethod(Method, Class<?>) Method.

public class DummyMapper extends RequestMappingHandlerMapping {

	@Override
	public RequestMappingInfo getMappingForMethod(Method method, Class<?> handlerType) {
		return super.getMappingForMethod(method, handlerType);
	}
	
	public static void main(String[] args) throws NoSuchMethodException, SecurityException {
		Class<?> controller = MyController.class;
		Method method = controller.getMethod("list");
		System.out.println(new DummyMapper().getMappingForMethod(method, controller).getPatternsCondition().getPatterns());
	}
}

On the other hand you might be able to read all registered methods from:

org.springframework.web.servlet.handler.AbstractHandlerMethodMapping.getHandlerMethods()'s keys.

But this would only work if the server is running.

Maybe this article helps: https://stackoverflow.com/questions/11082818/spring-mvc-get-all-request-mappings

@ST-DDT
Copy link
Author

ST-DDT commented Sep 26, 2017

@joffrey-bion Does this help you? Maybe we should move the discussion over to your fork.

@joffrey-bion
Copy link

joffrey-bion commented Sep 26, 2017

@ST-DDT Yes, thanks a lot for the detailed reply.
Let's carry on the discussion there: joffrey-bion/livedoc#34

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

No branches or pull requests

2 participants