GH-1224: symbol provider for request mappings now take superclasses and interfaces into account for all mapping attributes

This commit is contained in:
Martin Lippert
2024-04-10 13:06:42 +02:00
parent 5b74513e9d
commit 72ab00f009
22 changed files with 418 additions and 33 deletions

View File

@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2017, 2023 Pivotal, Inc.
* Copyright (c) 2017, 2024 Pivotal, Inc.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
@@ -20,7 +20,10 @@ import java.util.stream.Stream;
import org.eclipse.jdt.core.dom.ASTNode;
import org.eclipse.jdt.core.dom.Annotation;
import org.eclipse.jdt.core.dom.Expression;
import org.eclipse.jdt.core.dom.IAnnotationBinding;
import org.eclipse.jdt.core.dom.IMemberValuePairBinding;
import org.eclipse.jdt.core.dom.ITypeBinding;
import org.eclipse.jdt.core.dom.IVariableBinding;
import org.eclipse.jdt.core.dom.MemberValuePair;
import org.eclipse.jdt.core.dom.MethodDeclaration;
import org.eclipse.jdt.core.dom.NormalAnnotation;
@@ -55,12 +58,7 @@ public class RequestMappingSymbolProvider extends AbstractSymbolProvider {
stream.filter(Objects::nonNull)
.flatMap(parent -> (path == null ? Stream.<String>empty() : Arrays.stream(path))
.filter(Objects::nonNull).map(p -> {
String separator = !parent.endsWith("/") && !p.startsWith("/") ? "/" : "";
String resultPath = parent + separator + p;
if (resultPath.endsWith("/")) {
resultPath = resultPath.substring(0, resultPath.length() - 1);
}
return resultPath.startsWith("/") ? resultPath : "/" + resultPath;
return calculatePath(parent, p);
}))
.map(p -> RouteUtils.createRouteSymbol(location, p, methods, contentTypes, acceptTypes, null))
.forEach((enhancedSymbol) -> context.getGeneratedSymbols().add(new CachedSymbol(context.getDocURI(), context.getLastModified(), enhancedSymbol)));
@@ -70,6 +68,14 @@ public class RequestMappingSymbolProvider extends AbstractSymbolProvider {
}
}
private String calculatePath(String parent, String path) {
String separator = !parent.endsWith("/") && !path.startsWith("/") && !path.isEmpty() ? "/" : "";
String resultPath = parent + separator + path;
String result = resultPath.startsWith("/") ? resultPath : "/" + resultPath;
return result;
}
private String[] getMethod(Annotation node, SpringIndexerJavaContext context) {
String[] methods = null;
@@ -102,11 +108,14 @@ public class RequestMappingSymbolProvider extends AbstractSymbolProvider {
if (parentAnnotation != null) {
methods = getMethod(parentAnnotation, context);
}
else {
methods = getAttributeValuesFromSupertypes("method", node, context);
}
}
return methods;
}
private String[] getPath(Annotation node, SpringIndexerJavaContext context) {
if (node.isNormalAnnotation()) {
NormalAnnotation normNode = (NormalAnnotation) node;
@@ -133,10 +142,43 @@ public class RequestMappingSymbolProvider extends AbstractSymbolProvider {
private String[] getParentPath(Annotation node, SpringIndexerJavaContext context) {
Annotation parentAnnotation = getParentAnnotation(node);
return parentAnnotation == null ? null : getPath(parentAnnotation, context);
if (parentAnnotation != null) {
return getPath(parentAnnotation, context);
}
else {
return getPathFromSupertypes(node, context);
}
}
private String[] getPathFromSupertypes(Annotation node, SpringIndexerJavaContext context) {
IAnnotationBinding annotationBinding = getAnnotationFromSupertypes(node, context);
IMemberValuePairBinding valuePair = getValuePair(annotationBinding, "value", "path");
if (valuePair != null) {
Object value = valuePair.getValue();
if (value instanceof Object[]) {
Object[] values = (Object[]) value;
String[] result = new String[values.length];
for (int k = 0; k < result.length; k++) {
result[k] = values[k].toString();
}
return result;
}
else if (value instanceof String[]) {
return (String[]) value;
}
else if (value != null) {
return new String[] {value.toString()};
}
}
return null;
}
private Annotation getParentAnnotation(Annotation node) {
// lookup class level request mapping annotation
ASTNode parent = node.getParent() != null ? node.getParent().getParent() : null;
while (parent != null && !(parent instanceof TypeDeclaration)) {
parent = parent.getParent();
@@ -158,6 +200,7 @@ public class RequestMappingSymbolProvider extends AbstractSymbolProvider {
}
}
}
return null;
}
@@ -196,9 +239,21 @@ public class RequestMappingSymbolProvider extends AbstractSymbolProvider {
}
}
}
// lookup accept types on class level (same class or supertypes)
if (node.getParent() instanceof MethodDeclaration) {
Annotation parentAnnotation = getParentAnnotation(node);
if (parentAnnotation != null) {
return getAcceptTypes(parentAnnotation, context);
}
else {
return getAttributeValuesFromSupertypes("consumes", node, context);
}
}
return new String[0];
}
private String[] getContentTypes(Annotation node, SpringIndexerJavaContext context) {
if (node.isNormalAnnotation()) {
NormalAnnotation normNode = (NormalAnnotation) node;
@@ -215,7 +270,136 @@ public class RequestMappingSymbolProvider extends AbstractSymbolProvider {
}
}
}
// lookup content types on class level (same class or supertypes)
if (node.getParent() instanceof MethodDeclaration) {
Annotation parentAnnotation = getParentAnnotation(node);
if (parentAnnotation != null) {
return getContentTypes(parentAnnotation, context);
}
else {
return getAttributeValuesFromSupertypes("produces", node, context);
}
}
return new String[0];
}
private String[] getAttributeValuesFromSupertypes(String attributeName, Annotation node, SpringIndexerJavaContext context) {
IAnnotationBinding annotationBinding = getAnnotationFromSupertypes(node, context);
IMemberValuePairBinding valuePair = getValuePair(annotationBinding, attributeName);
if (valuePair != null) {
Object value = valuePair.getValue();
if (value instanceof Object[]) {
Object[] values = (Object[]) value;
String[] result = new String[values.length];
for (int k = 0; k < result.length; k++) {
Object v = values[k];
if (v instanceof IVariableBinding) {
IVariableBinding varBinding = (IVariableBinding) v;
result[k] = varBinding.getName();
}
else if (v instanceof String) {
result[k] = (String) v;
}
}
return result;
}
else if (value instanceof String[]) {
return (String[]) value;
}
else if (value != null) {
return new String[] {value.toString()};
}
}
return null;
}
private IMemberValuePairBinding getValuePair(IAnnotationBinding annotationBinding, String... names) {
if (annotationBinding != null) {
IMemberValuePairBinding[] valuePairs = annotationBinding.getDeclaredMemberValuePairs();
if (valuePairs != null ) {
for (int j = 0; j < valuePairs.length; j++) {
String valueName = valuePairs[j].getName();
if (valueName != null) {
for (int i = 0; i < names.length; i++) {
if (names[i] != null && names[i].equals(valueName)) {
return valuePairs[j];
}
}
}
}
}
}
return null;
}
private IAnnotationBinding getAnnotationFromSupertypes(Annotation node, SpringIndexerJavaContext context) {
ASTNode parent = node.getParent() != null ? node.getParent().getParent() : null;
while (parent != null && !(parent instanceof TypeDeclaration)) {
parent = parent.getParent();
}
if (parent != null) {
TypeDeclaration type = (TypeDeclaration) parent;
ITypeBinding typeBinding = type.resolveBinding();
if (typeBinding != null) {
return findFirstRequestMappingAnnotation(typeBinding);
}
}
return null;
}
private IAnnotationBinding findFirstRequestMappingAnnotation(ITypeBinding start) {
if (start == null) {
return null;
}
IAnnotationBinding found = getRequestMappingAnnotation(start);
if (found != null) {
return found;
}
else {
// search interfaces first
ITypeBinding[] interfaces = start.getInterfaces();
if (interfaces != null) {
for (int i = 0; i < interfaces.length; i++) {
found = findFirstRequestMappingAnnotation(interfaces[i]);
if (found != null) {
return found;
}
}
}
// search superclass second
ITypeBinding superclass = start.getSuperclass();
if (superclass != null) {
return findFirstRequestMappingAnnotation(superclass);
}
// nothing found
return null;
}
}
private IAnnotationBinding getRequestMappingAnnotation(ITypeBinding typeBinding) {
IAnnotationBinding[] annotations = typeBinding.getAnnotations();
for (int i = 0; i < annotations.length; i++) {
if (annotations[i].getAnnotationType() != null && Annotations.SPRING_REQUEST_MAPPING.equals(annotations[i].getAnnotationType().getQualifiedName())) {
return annotations[i];
}
}
return null;
}
}

View File

@@ -204,8 +204,6 @@ public class ASTUtils {
ITypeBinding klass = varBinding.getDeclaringClass();
if (klass!=null) {
dependencies.accept(klass);
}
Object constValue = varBinding.getConstantValue();
if (constValue != null) {

View File

@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2017, 2022 Pivotal, Inc.
* Copyright (c) 2017, 2024 Pivotal, Inc.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
@@ -184,7 +184,7 @@ public class RequestMappingSymbolProviderTest {
String docUri = directory.toPath().resolve("src/main/java/org/test/ParentMappingClass.java").toUri().toString();
List<? extends WorkspaceSymbol> symbols = indexer.getSymbols(docUri);
assertEquals(1, symbols.size());
assertTrue(containsSymbol(symbols, "@/parent/greeting -- GET", docUri, 8, 1, 8, 47));
assertTrue(containsSymbol(symbols, "@/parent/greeting -- GET", docUri, 8, 1, 8, 51));
}
@Test
@@ -192,7 +192,55 @@ public class RequestMappingSymbolProviderTest {
String docUri = directory.toPath().resolve("src/main/java/org/test/ParentMappingClass2.java").toUri().toString();
List<? extends WorkspaceSymbol> symbols = indexer.getSymbols(docUri);
assertEquals(1, symbols.size());
assertTrue(containsSymbol(symbols, "@/parent2 -- GET,POST,DELETE", docUri, 8, 1, 8, 16));
assertTrue(containsSymbol(symbols, "@/parent2 -- GET,POST,DELETE", docUri, 11, 1, 11, 16));
}
@Test
void testParentRequestMappingSymbolWithPathAttribute() throws Exception {
String docUri = directory.toPath().resolve("src/main/java/org/test/ParentMappingClass3.java").toUri().toString();
List<? extends WorkspaceSymbol> symbols = indexer.getSymbols(docUri);
assertEquals(1, symbols.size());
assertTrue(containsSymbol(symbols, "@/parent3/greeting -- GET", docUri, 8, 1, 8, 51));
}
@Test
void testMappingPathFromSuperclass() throws Exception {
String docUri = directory.toPath().resolve("src/main/java/org/test/inheritance/SubclassWithMappingFromParent.java").toUri().toString();
List<? extends WorkspaceSymbol> symbols = indexer.getSymbols(docUri);
assertEquals(1, symbols.size());
assertTrue(containsSymbol(symbols, "@/superclasspath/", docUri, 6, 1, 6, 21));
}
@Test
void testMappingPathFromSuperclassWithConstant() throws Exception {
String docUri = directory.toPath().resolve("src/main/java/org/test/inheritance/SubclassWithMappingFromParentWithConstant.java").toUri().toString();
List<? extends WorkspaceSymbol> symbols = indexer.getSymbols(docUri);
assertEquals(1, symbols.size());
assertTrue(containsSymbol(symbols, "@/path/from/constant/", docUri, 6, 1, 6, 21));
}
@Test
void testMappingPathFromSuperclassWithMethodsAndPathAttribute() throws Exception {
String docUri = directory.toPath().resolve("src/main/java/org/test/inheritance/SubclassWithMappingFromParentWithMethods.java").toUri().toString();
List<? extends WorkspaceSymbol> symbols = indexer.getSymbols(docUri);
assertEquals(1, symbols.size());
assertTrue(containsSymbol(symbols, "@/superclasspath -- POST,PUT - Accept: testconsume - Content-Type: text/plain", docUri, 6, 1, 6, 16));
}
@Test
void testMappingPathFromMultiLevelClassHierarchy() throws Exception {
String docUri = directory.toPath().resolve("src/main/java/org/test/inheritance/SuperControllerLevel4.java").toUri().toString();
List<? extends WorkspaceSymbol> symbols = indexer.getSymbols(docUri);
assertEquals(1, symbols.size());
assertTrue(containsSymbol(symbols, "@/path-level2/final-subclass-path", docUri, 6, 1, 6, 39));
}
@Test
void testMappingPathFromSuperInterfaceEvenIfSuperclassContainsMappingPath() throws Exception {
String docUri = directory.toPath().resolve("src/main/java/org/test/inheritance/ControllerAsSubclassAndInterfaceHierarchy.java").toUri().toString();
List<? extends WorkspaceSymbol> symbols = indexer.getSymbols(docUri);
assertEquals(1, symbols.size());
assertTrue(containsSymbol(symbols, "@/superinterface-path/last-path-segment -- GET - Accept: testconsume - Content-Type: text/plain", docUri, 6, 1, 6, 33));
}
@Test
@@ -208,63 +256,63 @@ public class RequestMappingSymbolProviderTest {
void testGetMappingSymbol() throws Exception {
String docUri = directory.toPath().resolve("src/main/java/org/test/RequestMethodClass.java").toUri().toString();
List<? extends WorkspaceSymbol> symbols = indexer.getSymbols(docUri);
assertTrue(containsSymbol(symbols, "@/getData -- GET", docUri, 12, 1, 12, 24));
assertTrue(containsSymbol(symbols, "@/getData -- GET", docUri, 13, 1, 13, 24));
}
@Test
void testGetMappingSymbolWithoutPath() throws Exception {
String docUri = directory.toPath().resolve("src/main/java/org/test/RequestMethodClass.java").toUri().toString();
List<? extends WorkspaceSymbol> symbols = indexer.getSymbols(docUri);
assertTrue(containsSymbol(symbols, "@/ -- GET", docUri, 40, 1, 40, 16));
assertTrue(containsSymbol(symbols, "@/ -- GET", docUri, 41, 1, 41, 16));
}
@Test
void testGetMappingSymbolWithoutAnything() throws Exception {
String docUri = directory.toPath().resolve("src/main/java/org/test/RequestMethodClass.java").toUri().toString();
List<? extends WorkspaceSymbol> symbols = indexer.getSymbols(docUri);
assertTrue(containsSymbol(symbols, "@/ -- GET", docUri, 44, 1, 44, 14));
assertTrue(containsSymbol(symbols, "@/ -- GET", docUri, 45, 1, 45, 14));
}
@Test
void testDeleteMappingSymbol() throws Exception {
String docUri = directory.toPath().resolve("src/main/java/org/test/RequestMethodClass.java").toUri().toString();
List<? extends WorkspaceSymbol> symbols = indexer.getSymbols(docUri);
assertTrue(containsSymbol(symbols, "@/deleteData -- DELETE", docUri, 20, 1, 20, 30));
assertTrue(containsSymbol(symbols, "@/deleteData -- DELETE", docUri, 21, 1, 21, 30));
}
@Test
void testPostMappingSymbol() throws Exception {
String docUri = directory.toPath().resolve("src/main/java/org/test/RequestMethodClass.java").toUri().toString();
List<? extends WorkspaceSymbol> symbols = indexer.getSymbols(docUri);
assertTrue(containsSymbol(symbols, "@/postData -- POST", docUri, 24, 1, 24, 26));
assertTrue(containsSymbol(symbols, "@/postData -- POST", docUri, 25, 1, 25, 26));
}
@Test
void testPutMappingSymbol() throws Exception {
String docUri = directory.toPath().resolve("src/main/java/org/test/RequestMethodClass.java").toUri().toString();
List<? extends WorkspaceSymbol> symbols = indexer.getSymbols(docUri);
assertTrue(containsSymbol(symbols, "@/putData -- PUT", docUri, 16, 1, 16, 24));
assertTrue(containsSymbol(symbols, "@/putData -- PUT", docUri, 17, 1, 17, 24));
}
@Test
void testPatchMappingSymbol() throws Exception {
String docUri = directory.toPath().resolve("src/main/java/org/test/RequestMethodClass.java").toUri().toString();
List<? extends WorkspaceSymbol> symbols = indexer.getSymbols(docUri);
assertTrue(containsSymbol(symbols, "@/patchData -- PATCH", docUri, 28, 1, 28, 28));
assertTrue(containsSymbol(symbols, "@/patchData -- PATCH", docUri, 29, 1, 29, 28));
}
@Test
void testGetRequestMappingSymbol() throws Exception {
String docUri = directory.toPath().resolve("src/main/java/org/test/RequestMethodClass.java").toUri().toString();
List<? extends WorkspaceSymbol> symbols = indexer.getSymbols(docUri);
assertTrue(containsSymbol(symbols, "@/getHello -- GET", docUri, 32, 1, 32, 61));
assertTrue(containsSymbol(symbols, "@/getHello -- GET", docUri, 33, 1, 33, 61));
}
@Test
void testMultiRequestMethodMappingSymbol() throws Exception {
String docUri = directory.toPath().resolve("src/main/java/org/test/RequestMethodClass.java").toUri().toString();
List<? extends WorkspaceSymbol> symbols = indexer.getSymbols(docUri);
assertTrue(containsSymbol(symbols, "@/postAndPutHello -- POST,PUT", docUri, 36, 1, 36, 76));
assertTrue(containsSymbol(symbols, "@/postAndPutHello -- POST,PUT", docUri, 37, 1, 37, 76));
}
@Test

View File

@@ -2,7 +2,7 @@ package org.test;
import org.springframework.web.bind.annotation.RequestMapping;
public class ChainedRequestMappingOverMultipleClasses {
public class ChainedRequestMappingPathOverMultipleClasses {
@RequestMapping(ChainElement1.MAPPING_PATH_1)
public String hello() {

View File

@@ -1,12 +1,12 @@
package org.test;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestMethod.*;
import static org.springframework.web.bind.annotation.RequestMethod.*;
@RequestMapping(value="parent", method= {GET,POST,DELETE})
@RequestMapping(value = "parent", method = {GET,POST,DELETE})
public class ParentMappingClass {
@RequestMapping(value="/greeting", method=GET)
@RequestMapping(value = "/greeting", method = GET)
public String hello() {
return "Hello";
}

View File

@@ -1,9 +1,12 @@
package org.test;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestMethod.*;
import static org.springframework.web.bind.annotation.RequestMethod.DELETE;
import static org.springframework.web.bind.annotation.RequestMethod.GET;
import static org.springframework.web.bind.annotation.RequestMethod.POST;
@RequestMapping(value="parent2", method= {GET,POST,DELETE})
import org.springframework.web.bind.annotation.RequestMapping;
@RequestMapping(value = "parent2", method = {GET,POST,DELETE})
public class ParentMappingClass2 {
@RequestMapping

View File

@@ -0,0 +1,14 @@
package org.test;
import org.springframework.web.bind.annotation.RequestMapping;
import static org.springframework.web.bind.annotation.RequestMethod.*;
@RequestMapping(path = "parent3", method = {GET,POST,DELETE})
public class ParentMappingClass3 {
@RequestMapping(value = "/greeting", method = GET)
public String hello() {
return "Hello";
}
}

View File

@@ -6,7 +6,8 @@ import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.PatchMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestMethod.*;
import org.springframework.web.bind.annotation.RequestMethod;
import static org.springframework.web.bind.annotation.RequestMethod.PUT;
public class RequestMethodClass {
@@ -31,7 +32,7 @@ public class RequestMethodClass {
}
@RequestMapping(value="/getHello", method=RequestMethod.GET)
public getHello() {
public void getHello() {
}
@RequestMapping(path="/postAndPutHello", method= {RequestMethod.POST, PUT})

View File

@@ -0,0 +1,12 @@
package org.test.inheritance;
import org.springframework.web.bind.annotation.GetMapping;
public class ControllerAsSubclassAndInterfaceHierarchy extends SuperclassWithMappingPath implements EmptyInterfaceWithinHierarchy {
@GetMapping("last-path-segment")
public String sayHello() {
return "hello";
}
}

View File

@@ -0,0 +1,4 @@
package org.test.inheritance;
public interface EmptyInterfaceWithinHierarchy extends SuperInterfaceWithMappingPath {
}

View File

@@ -0,0 +1,12 @@
package org.test.inheritance;
import org.springframework.web.bind.annotation.RequestMapping;
public class SubclassWithMappingFromParent extends SuperclassWithMappingPath {
@RequestMapping("/")
public String hello() {
return "Hello";
}
}

View File

@@ -0,0 +1,12 @@
package org.test.inheritance;
import org.springframework.web.bind.annotation.RequestMapping;
public class SubclassWithMappingFromParentWithConstant extends SuperclassWithMappingPathFromConstant {
@RequestMapping("/")
public String hello() {
return "Hello";
}
}

View File

@@ -0,0 +1,12 @@
package org.test.inheritance;
import org.springframework.web.bind.annotation.RequestMapping;
public class SubclassWithMappingFromParentWithMethods extends SuperclassWithMappingPathAndMethods {
@RequestMapping
public String hello() {
return "Hello";
}
}

View File

@@ -0,0 +1,9 @@
package org.test.inheritance;
import static org.springframework.web.bind.annotation.RequestMethod.POST;
import org.springframework.web.bind.annotation.RequestMapping;
@RequestMapping(method = {POST})
public class SuperControllerLevel1 {
}

View File

@@ -0,0 +1,7 @@
package org.test.inheritance;
import org.springframework.web.bind.annotation.RequestMapping;
@RequestMapping(path = "path-level2")
public class SuperControllerLevel2 extends SuperControllerLevel1 {
}

View File

@@ -0,0 +1,4 @@
package org.test.inheritance;
public class SuperControllerLevel3 extends SuperControllerLevel2 {
}

View File

@@ -0,0 +1,12 @@
package org.test.inheritance;
import org.springframework.web.bind.annotation.RequestMapping;
public class SuperControllerLevel4 extends SuperControllerLevel3 {
@RequestMapping("final-subclass-path")
public String details() {
return "subclass-details";
}
}

View File

@@ -0,0 +1,8 @@
package org.test.inheritance;
import org.springframework.web.bind.annotation.RestController;
@RestController
public class SuperControllerLevel5 extends SuperControllerLevel4 {
}

View File

@@ -0,0 +1,15 @@
package org.test.inheritance;
import static org.springframework.web.bind.annotation.RequestMethod.GET;
import static org.springframework.web.bind.annotation.RequestMethod.POST;
import org.springframework.http.MediaType;
import org.springframework.web.bind.annotation.RequestMapping;
@RequestMapping(
path = "/superinterface-path",
method = {GET, POST},
produces = {MediaType.TEXT_PLAIN_VALUE},
consumes = {"testconsume"})
public interface SuperInterfaceWithMappingPath {
}

View File

@@ -0,0 +1,7 @@
package org.test.inheritance;
import org.springframework.web.bind.annotation.RequestMapping;
@RequestMapping(value="/superclasspath")
public class SuperclassWithMappingPath {
}

View File

@@ -0,0 +1,15 @@
package org.test.inheritance;
import static org.springframework.web.bind.annotation.RequestMethod.POST;
import static org.springframework.web.bind.annotation.RequestMethod.PUT;
import org.springframework.http.MediaType;
import org.springframework.web.bind.annotation.RequestMapping;
@RequestMapping(
path = "/superclasspath",
method = {POST, PUT},
produces = {MediaType.TEXT_PLAIN_VALUE},
consumes = {"testconsume"})
public class SuperclassWithMappingPathAndMethods {
}

View File

@@ -0,0 +1,8 @@
package org.test.inheritance;
import org.springframework.web.bind.annotation.RequestMapping;
import org.test.Constants;
@RequestMapping(Constants.REQUEST_MAPPING_PATH)
public class SuperclassWithMappingPathFromConstant {
}