You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@juneau.apache.org by ja...@apache.org on 2020/09/25 13:49:23 UTC

[juneau] 02/02: REST API refactoring.

This is an automated email from the ASF dual-hosted git repository.

jamesbognar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/juneau.git

commit f017e678a31da6f2a7aa20e52640612d1e7ac788
Author: JamesBognar <ja...@salesforce.com>
AuthorDate: Fri Sep 25 09:49:17 2020 -0400

    REST API refactoring.
---
 .../annotation/RestMethod_ClientVersion_Test.java  |   6 +-
 .../main/java/org/apache/juneau/rest/RestCall.java |  22 +++
 .../java/org/apache/juneau/rest/RestContext.java   | 151 ++++++++++-----------
 .../org/apache/juneau/rest/RestMethodContext.java  |  94 ++++++++++---
 4 files changed, 169 insertions(+), 104 deletions(-)

diff --git a/juneau-rest/juneau-rest-server-utest/src/test/java/org/apache/juneau/rest/annotation/RestMethod_ClientVersion_Test.java b/juneau-rest/juneau-rest-server-utest/src/test/java/org/apache/juneau/rest/annotation/RestMethod_ClientVersion_Test.java
index 6d2b052..fac19aa 100644
--- a/juneau-rest/juneau-rest-server-utest/src/test/java/org/apache/juneau/rest/annotation/RestMethod_ClientVersion_Test.java
+++ b/juneau-rest/juneau-rest-server-utest/src/test/java/org/apache/juneau/rest/annotation/RestMethod_ClientVersion_Test.java
@@ -55,13 +55,13 @@ public class RestMethod_ClientVersion_Test {
 		RestClient a = MockRestClient.build(A.class);
 		a.get("/").run().assertBody().is("no-version");
 		for (String s : "1, 1.0, 1.0.0, 1.0.1".split("\\s*,\\s*")) {
-			a.get("/").clientVersion(s).run().assertBody().is("[1.0,1.0]");
+			a.get("/").clientVersion(s).run().assertBody().msg("s=[{0}]",s).is("[1.0,1.0]");
 		}
 		for (String s : "1.1, 1.1.1, 1.2, 1.9.9".split("\\s*,\\s*")) {
-			a.get("/").clientVersion(s).run().assertBody().is("[1.1,2)");
+			a.get("/").clientVersion(s).run().assertBody().msg("s=[{0}]").is("[1.1,2)");
 		}
 		for (String s : "2, 2.0, 2.1, 9, 9.9".split("\\s*,\\s*")) {
-			a.get("/").clientVersion(s).run().assertBody().is("2");
+			a.get("/").clientVersion(s).run().assertBody().msg("s=[{0}]").is("2");
 		}
 	}
 
diff --git a/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/RestCall.java b/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/RestCall.java
index fc79cad..aa034d6 100644
--- a/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/RestCall.java
+++ b/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/RestCall.java
@@ -43,6 +43,8 @@ public class RestCall {
 	private RestCallLogger logger;
 	private RestCallLoggerConfig loggerConfig;
 
+	private UrlPathPatternMatch urlPathPatternMatch;
+
 	/**
 	 * Constructor.
 	 *
@@ -303,6 +305,26 @@ public class RestCall {
 		return this;
 	}
 
+	/**
+	 * Sets the URL path pattern match on this call.
+	 *
+	 * @param urlPathPatternMatch The match pattern.
+	 * @return This object (for method chaining).
+	 */
+	public RestCall urlPathPatternMatch(UrlPathPatternMatch urlPathPatternMatch) {
+		this.urlPathPatternMatch = urlPathPatternMatch;
+		return this;
+	}
+
+	/**
+	 * Returns the URL path pattern match on this call.
+	 *
+	 * @return The URL path pattern match on this call.
+	 */
+	public UrlPathPatternMatch getUrlPathPatternMatch() {
+		return urlPathPatternMatch;
+	}
+
 	//------------------------------------------------------------------------------------------------------------------
 	// Lifecycle methods.
 	//------------------------------------------------------------------------------------------------------------------
diff --git a/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/RestContext.java b/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/RestContext.java
index 762264f..582c481 100644
--- a/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/RestContext.java
+++ b/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/RestContext.java
@@ -3911,17 +3911,15 @@ public class RestContext extends BeanContext {
 							sm = new RestMethodContext(smb) {
 
 								@Override
-								int invoke(RestCall call) throws Throwable {
+								void invoke(RestCall call) throws Throwable {
 
-									int rc = super.invoke(call);
-									if (rc != SC_OK)
-										return rc;
+									super.invoke(call);
 
 									final Object o = call.getOutput();
 
 									if ("GET".equals(call.getMethod())) {
 										call.output(rim.getMethodsByPath().keySet());
-										return SC_OK;
+										return;
 
 									} else if ("POST".equals(call.getMethod())) {
 										String pip = call.getUrlPathInfo().getPath();
@@ -3945,13 +3943,13 @@ public class RestContext extends BeanContext {
 												}
 												Object output = m.invoke(o, args);
 												call.output(output);
-												return SC_OK;
+												return;
 											} catch (Exception e) {
 												throw toHttpException(e, InternalServerError.class);
 											}
 										}
 									}
-									return SC_NOT_FOUND;
+									throw new NotFound();
 								}
 							};
 
@@ -4109,32 +4107,6 @@ public class RestContext extends BeanContext {
 		}
 	}
 
-	static class MethodMapBuilder  {
-		TreeMap<String,TreeSet<RestMethodContext>> map = new TreeMap<>();
-		Set<RestMethodContext> list = ASet.of();
-
-
-		MethodMapBuilder add(String httpMethodName, RestMethodContext mc) {
-			httpMethodName = httpMethodName.toUpperCase();
-			if (! map.containsKey(httpMethodName))
-				map.put(httpMethodName, new TreeSet<>());
-			map.get(httpMethodName).add(mc);
-			list.add(mc);
-			return this;
-		}
-
-		Map<String,List<RestMethodContext>> getMap() {
-			Map<String,List<RestMethodContext>> m = new LinkedHashMap<>();
-			for (Map.Entry<String,TreeSet<RestMethodContext>> e : map.entrySet())
-				m.put(e.getKey(), AList.of(e.getValue()));
-			return Collections.unmodifiableMap(m);
-		}
-
-		List<RestMethodContext> getList() {
-			return AList.of(list);
-		}
-	}
-
 	/**
 	 * Returns the resource resolver associated with this context.
 	 *
@@ -5223,18 +5195,13 @@ public class RestContext extends BeanContext {
 			} else {
 
 				// If the specified method has been defined in a subclass, invoke it.
-				int rc = findMethodAndInvoke(call);
-
-				// Should be 404 if URL pattern didn't match.
-				if (rc == 0)
-					rc = SC_NOT_FOUND;
-
-				// If not invoked above, see if it's an OPTIONs request
-				if (rc != SC_OK)
-					handleNotFound(call.status(rc));
-
-				if (call.getStatus() == 0)
-					call.status(rc);
+				try {
+					findMethod(call).invoke(call);
+				} catch (NotFound e) {
+					if (call.getStatus() == 0)
+						call.status(404);
+					handleNotFound(call);
+				}
 			}
 
 			if (call.hasOutput()) {
@@ -5254,50 +5221,44 @@ public class RestContext extends BeanContext {
 		finishCall(call);
 	}
 
-	private int findMethodAndInvoke(RestCall call) throws Throwable {
+	private RestMethodContext findMethod(RestCall call) throws Throwable {
 		String m = call.getMethod();
 
 		int rc = 0;
-		if (methodMap.containsKey(m))
-			rc = invoke(methodMap.get(m), call);
-
-		if ((rc == 0 || rc == 404) && methodMap.containsKey("*"))
-			rc = invoke(methodMap.get("*"), call);
-
-		// Should be 405 if the URL pattern matched but HTTP method did not.
-		if (rc == 0)
-			for (List<RestMethodContext> rcc : methodMap.values())
-				if (matches(rcc, call))
-					rc = SC_METHOD_NOT_ALLOWED;
-
-		return rc;
-	}
-
-	private boolean matches(List<RestMethodContext> mc, RestCall call) throws Throwable {
-		UrlPathInfo pi = call.getUrlPathInfo();
-		for (RestMethodContext m : mc)
-			if (m.matches(pi))
-				return true;
-		return false;
-	}
+		if (methodMap.containsKey(m)) {
+			for (RestMethodContext mc : methodMap.get(m)) {
+				int mrc = mc.match(call);
+				if (mrc == 2)
+					return mc;
+				rc = Math.max(rc, mrc);
+			}
+		}
 
-	private int invoke(List<RestMethodContext> mc, RestCall call) throws Throwable {
-		if (mc.size() == 1) {
-			call.restMethodContext(mc.get(0));
-			return mc.get(0).invoke(call);
+		if (methodMap.containsKey("*")) {
+			for (RestMethodContext mc : methodMap.get("*")) {
+				int mrc = mc.match(call);
+				if (mrc == 2)
+					return mc;
+				rc = Math.max(rc, mrc);
+			}
 		}
 
-		int maxRc = 0;
-		for (RestMethodContext m : mc) {
-			int rc = m.invoke(call);
-			if (rc == SC_OK) {
-				call.restMethodContext(m);
-				return SC_OK;
+		// If no paths matched, see if the path matches any other methods.
+		// Note that we don't want to match against "/*" patterns such as getOptions().
+		if (rc == 0) {
+			for (RestMethodContext mc : methods) {
+				if (! mc.getPathPattern().endsWith("/*")) {
+					int mrc = mc.match(call);
+					if (mrc == 2)
+						throw new MethodNotAllowed();
+				}
 			}
-			maxRc = Math.max(maxRc, rc);
 		}
-		return maxRc;
 
+		if (rc == 1)
+			throw new PreconditionFailed("Method ''{0}'' not found on resource on path ''{1}'' with matching matcher.", m, call.getPathInfo());
+
+		throw new NotFound();
 	}
 
 	private boolean isDebug(RestCall call) {
@@ -5743,4 +5704,34 @@ public class RestContext extends BeanContext {
 				.a("useClasspathResourceCaching", useClasspathResourceCaching)
 			);
 	}
+
+	//-----------------------------------------------------------------------------------------------------------------
+	// Helpers.
+	//-----------------------------------------------------------------------------------------------------------------
+
+	static class MethodMapBuilder  {
+		TreeMap<String,TreeSet<RestMethodContext>> map = new TreeMap<>();
+		Set<RestMethodContext> set = ASet.of();
+
+
+		MethodMapBuilder add(String httpMethodName, RestMethodContext mc) {
+			httpMethodName = httpMethodName.toUpperCase();
+			if (! map.containsKey(httpMethodName))
+				map.put(httpMethodName, new TreeSet<>());
+			map.get(httpMethodName).add(mc);
+			set.add(mc);
+			return this;
+		}
+
+		Map<String,List<RestMethodContext>> getMap() {
+			AMap<String,List<RestMethodContext>> m = AMap.of();
+			for (Map.Entry<String,TreeSet<RestMethodContext>> e : map.entrySet())
+				m.put(e.getKey(), AList.of(e.getValue()));
+			return m.unmodifiable();
+		}
+
+		List<RestMethodContext> getList() {
+			return AList.of(set).unmodifiable();
+		}
+	}
 }
diff --git a/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/RestMethodContext.java b/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/RestMethodContext.java
index 25b20cc..f980e58 100644
--- a/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/RestMethodContext.java
+++ b/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/RestMethodContext.java
@@ -13,7 +13,6 @@
 package org.apache.juneau.rest;
 
 import static org.apache.juneau.rest.Enablement.*;
-import static javax.servlet.http.HttpServletResponse.*;
 import static org.apache.juneau.internal.ClassUtils.*;
 import static org.apache.juneau.internal.ObjectUtils.*;
 import static org.apache.juneau.internal.StringUtils.*;
@@ -929,21 +928,87 @@ public class RestMethodContext extends BeanContext implements Comparable<RestMet
 	}
 
 	/**
-	 * Workhorse method.
+	 * Identifies if this method can process the specified call.
 	 *
-	 * @param pathInfo The value of {@link HttpServletRequest#getPathInfo()} (sorta)
-	 * @return The HTTP response code.
+	 * <p>
+	 * To process the call, the following must be true:
+	 * <ul>
+	 * 	<li>Path pattern must match.
+	 * 	<li>Matchers (if any) must match.
+	 * </ul>
+	 *
+	 * @param call The call to check.
+	 * @return
+	 * 	One of the following values:
+	 * 	<ul>
+	 * 		<li><c>0</c> - Path doesn't match.
+	 * 		<li><c>1</c> - Path matched but matchers did not.
+	 * 		<li><c>2</c> - Matches.
+	 * 	</ul>
 	 */
-	int invoke(RestCall call) throws Throwable {
+	protected int match(RestCall call) {
 
-		UrlPathPatternMatch pm = null;
+		UrlPathPatternMatch pm = matchPattern(call);
+
+		if (pm == null)
+			return 0;
+
+		if (requiredMatchers.length == 0 && optionalMatchers.length == 0) {
+			call.urlPathPatternMatch(pm);  // Cache so we don't have to recalculate.
+			return 2;
+		}
+
+		try {
+			RestRequest req = call.getRestRequest();
+			RestResponse res = call.getRestResponse();
+
+			@SuppressWarnings("deprecation")
+			RequestProperties requestProperties = new RequestProperties(req.getVarResolverSession(), properties);
+
+			req.init(this, requestProperties);
+			res.init(this, requestProperties);
+
+			// If the method implements matchers, test them.
+			for (RestMatcher m : requiredMatchers)
+				if (! m.matches(req))
+					return 1;
+			if (optionalMatchers.length > 0) {
+				boolean matches = false;
+				for (RestMatcher m : optionalMatchers)
+					matches |= m.matches(req);
+				if (! matches)
+					return 1;
+			}
 
+			call.urlPathPatternMatch(pm);  // Cache so we don't have to recalculate.
+			return 2;
+		} catch (Exception e) {
+			throw new InternalServerError(e);
+		}
+	}
+
+	private UrlPathPatternMatch matchPattern(RestCall call) {
+		UrlPathPatternMatch pm = null;
 		for (UrlPathPattern pp : pathPatterns)
 			if (pm == null)
 				pm = pp.match(call.getUrlPathInfo());
+		return pm;
+	}
 
+
+	/**
+	 * Workhorse method.
+	 *
+	 * @param pathInfo The value of {@link HttpServletRequest#getPathInfo()} (sorta)
+	 */
+	void invoke(RestCall call) throws Throwable {
+
+		UrlPathPatternMatch pm = call.getUrlPathPatternMatch();
 		if (pm == null)
-			return SC_NOT_FOUND;
+			pm = matchPattern(call);
+
+		if (pm == null)
+			throw new NotFound();
 
 		RestRequest req = call.getRestRequest();
 		RestResponse res = call.getRestResponse();
@@ -960,18 +1025,6 @@ public class RestMethodContext extends BeanContext implements Comparable<RestMet
 		req.init(this, requestProperties);
 		res.init(this, requestProperties);
 
-		// If the method implements matchers, test them.
-		for (RestMatcher m : requiredMatchers)
-			if (! m.matches(req))
-				return SC_PRECONDITION_FAILED;
-		if (optionalMatchers.length > 0) {
-			boolean matches = false;
-			for (RestMatcher m : optionalMatchers)
-				matches |= m.matches(req);
-			if (! matches)
-				return SC_PRECONDITION_FAILED;
-		}
-
 		context.preCall(call);
 
 		call.loggerConfig(callLoggerConfig);
@@ -1006,7 +1059,7 @@ public class RestMethodContext extends BeanContext implements Comparable<RestMet
 
 			for (RestGuard guard : guards)
 				if (! guard.guard(req, res))
-					return SC_OK;
+					return;
 
 			Object output;
 			try {
@@ -1055,7 +1108,6 @@ public class RestMethodContext extends BeanContext implements Comparable<RestMet
 		} catch (InvocationTargetException e) {
 			throw e.getTargetException();
 		}
-		return SC_OK;
 	}
 //
 //	protected void addStatusCode(int code) {