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 2018/10/10 16:13:21 UTC
[juneau] branch master updated: Slightly better initialization in
RestServlet.
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
The following commit(s) were added to refs/heads/master by this push:
new e05c5a3 Slightly better initialization in RestServlet.
e05c5a3 is described below
commit e05c5a357cf9490433fcafd4df82dad02bd7755f
Author: JamesBognar <ja...@apache.org>
AuthorDate: Wed Oct 10 12:13:08 2018 -0400
Slightly better initialization in RestServlet.
---
.../java/org/apache/juneau/utils/SearchArgs.java | 6 ++
.../apache/juneau/rest/BasicRestCallHandler.java | 2 +
.../java/org/apache/juneau/rest/RestContext.java | 12 ++++
.../java/org/apache/juneau/rest/RestServlet.java | 50 +++++++------
.../java/org/apache/juneau/rest/mock/MockRest.java | 13 +++-
.../apache/juneau/rest/ThreadLocalObjectsTest.java | 81 ++++++++++++++++++++++
6 files changed, 139 insertions(+), 25 deletions(-)
diff --git a/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/utils/SearchArgs.java b/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/utils/SearchArgs.java
index 26b2cff..fdcda4f 100644
--- a/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/utils/SearchArgs.java
+++ b/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/utils/SearchArgs.java
@@ -24,6 +24,12 @@ import org.apache.juneau.internal.*;
* Encapsulates arguments for basic search/view/sort/position/limit functionality.
*/
public class SearchArgs {
+
+ /**
+ * Default search args.
+ */
+ public static SearchArgs DEFAULT = SearchArgs.builder().build();
+
private final Map<String,String> search;
private final List<String> view;
private final Map<String,Boolean> sort;
diff --git a/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/BasicRestCallHandler.java b/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/BasicRestCallHandler.java
index de95ef6..238b084 100644
--- a/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/BasicRestCallHandler.java
+++ b/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/BasicRestCallHandler.java
@@ -199,6 +199,8 @@ public class BasicRestCallHandler implements RestCallHandler {
r1.setAttribute("Exception", e);
r1.setAttribute("ExecTime", System.currentTimeMillis() - startTime);
handleError(r1, r2, e);
+ } finally {
+ context.clearState();
}
context.finishCall(r1, r2);
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 5b5a505..99f6200 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
@@ -4777,6 +4777,9 @@ public final class RestContext extends BeanContext {
}
void setRequest(RestRequest req) {
+ // Must be careful not to bleed thread-locals.
+ if (this.req.get() != null)
+ throw new RuntimeException("Thread-local request object was not cleaned up from previous request. " + this + ", thread=["+Thread.currentThread().getId()+"]");
this.req.set(req);
}
@@ -4790,6 +4793,9 @@ public final class RestContext extends BeanContext {
}
void setResponse(RestResponse res) {
+ // Must be careful not to bleed thread-locals.
+ if (this.res.get() != null)
+ throw new RuntimeException("Thread-local response object was not cleaned up from previous request. " + this + ", thread=["+Thread.currentThread().getId()+"]");
this.res.set(res);
}
@@ -4801,4 +4807,10 @@ public final class RestContext extends BeanContext {
req.remove();
res.remove();
}
+
+ @Override
+ public String toString() {
+ Object r = getResource();
+ return "RestContext: hashCode=["+System.identityHashCode(this)+"], resource=["+(r == null ? null : r.getClass()+","+System.identityHashCode(r))+"]";
+ }
}
diff --git a/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/RestServlet.java b/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/RestServlet.java
index 05ebb3a..ee67915 100644
--- a/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/RestServlet.java
+++ b/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/RestServlet.java
@@ -39,20 +39,18 @@ public abstract class RestServlet extends HttpServlet {
private RestContextBuilder builder;
private volatile RestContext context;
- private boolean isInitialized = false;
- private Exception initException;
+ private volatile Exception initException;
+ private boolean isInitialized = false; // Should not be volatile.
@Override /* Servlet */
public final synchronized void init(ServletConfig servletConfig) throws ServletException {
try {
+ if (context != null)
+ return;
builder = RestContext.create(servletConfig, this.getClass(), null).init(this);
super.init(servletConfig);
- if (! isInitialized) {
- builder.servletContext(this.getServletContext());
- context = builder.build();
- isInitialized = true;
- }
- context.postInit();
+ builder.servletContext(this.getServletContext());
+ setContext(builder.build());
context.postInitChildFirst();
} catch (RestException e) {
// Thrown RestExceptions are simply caught and re-thrown on subsequent calls to service().
@@ -70,8 +68,6 @@ public abstract class RestServlet extends HttpServlet {
initException = new Exception(e);
log(SEVERE, e, "Servlet init error on class ''{0}''", getClass().getName());
throw new ServletException(e);
- } finally {
- isInitialized = true;
}
}
@@ -83,13 +79,17 @@ public abstract class RestServlet extends HttpServlet {
super.init(servletConfig);
}
- /*
+ /**
* Sets the context object for this servlet.
- * Used when subclasses of RestServlet are attached as child resources.
+ *
+ * @param context
+ * @throws ServletException
*/
- synchronized void setContext(RestContext context) {
+ public synchronized void setContext(RestContext context) throws ServletException {
this.builder = context.builder;
this.context = context;
+ isInitialized = true;
+ context.postInit();
}
@Override /* GenericServlet */
@@ -131,15 +131,17 @@ public abstract class RestServlet extends HttpServlet {
@Override /* Servlet */
public void service(HttpServletRequest r1, HttpServletResponse r2) throws ServletException, InternalServerError, IOException {
try {
- if (initException != null) {
- if (initException instanceof RestException)
- throw (RestException)initException;
- throw new InternalServerError(initException);
+ // To avoid checking the volatile field context on every call, use the non-volatile isInitialized field as a first-check check.
+ if (! isInitialized) {
+ if (initException != null) {
+ if (initException instanceof RestException)
+ throw (RestException)initException;
+ throw new InternalServerError(initException);
+ }
+ if (context == null)
+ throw new InternalServerError("Servlet {0} not initialized. init(ServletConfig) was not called. This can occur if you've overridden this method but didn't call super.init(RestConfig).", getClass().getName());
+ isInitialized = true;
}
- if (context == null)
- throw new InternalServerError("Servlet {0} not initialized. init(ServletConfig) was not called. This can occur if you've overridden this method but didn't call super.init(RestConfig).", getClass().getName());
- if (! isInitialized)
- throw new InternalServerError("Servlet {0} has not been initialized", getClass().getName());
context.getCallHandler().service(r1, r2);
@@ -147,8 +149,6 @@ public abstract class RestServlet extends HttpServlet {
r2.sendError(SC_INTERNAL_SERVER_ERROR, e.getLocalizedMessage());
} catch (Throwable e) {
r2.sendError(SC_INTERNAL_SERVER_ERROR, e.getLocalizedMessage());
- } finally {
- context.clearState();
}
}
@@ -213,6 +213,8 @@ public abstract class RestServlet extends HttpServlet {
* @return The current HTTP request, or <jk>null</jk> if it wasn't created.
*/
public RestRequest getRequest() {
+ if (context == null)
+ return null;
return context.getRequest();
}
@@ -222,6 +224,8 @@ public abstract class RestServlet extends HttpServlet {
* @return The current HTTP response, or <jk>null</jk> if it wasn't created.
*/
public RestResponse getResponse() {
+ if (context == null)
+ return null;
return context.getResponse();
}
diff --git a/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/mock/MockRest.java b/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/mock/MockRest.java
index 6742b7e..58ea9c0 100644
--- a/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/mock/MockRest.java
+++ b/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/mock/MockRest.java
@@ -61,8 +61,17 @@ public class MockRest implements MockHttpConnection {
private final RestContext rc;
private MockRest(Class<?> c, boolean debug) throws Exception {
- if (! CONTEXTS.containsKey(c))
- CONTEXTS.put(c, RestContext.create(c.newInstance()).logger(debug ? BasicRestLogger.class : NoOpRestLogger.class).build().postInit().postInitChildFirst());
+ if (! CONTEXTS.containsKey(c)) {
+ Object r = c.newInstance();
+ RestContext rc = RestContext.create(r).logger(debug ? BasicRestLogger.class : NoOpRestLogger.class).build();
+ if (r instanceof RestServlet) {
+ ((RestServlet)r).setContext(rc);
+ } else {
+ rc.postInit();
+ }
+ rc.postInitChildFirst();
+ CONTEXTS.put(c, rc);
+ }
rc = CONTEXTS.get(c);
}
diff --git a/juneau-rest/juneau-rest-server/src/test/java/org/apache/juneau/rest/ThreadLocalObjectsTest.java b/juneau-rest/juneau-rest-server/src/test/java/org/apache/juneau/rest/ThreadLocalObjectsTest.java
new file mode 100644
index 0000000..462060b
--- /dev/null
+++ b/juneau-rest/juneau-rest-server/src/test/java/org/apache/juneau/rest/ThreadLocalObjectsTest.java
@@ -0,0 +1,81 @@
+// ***************************************************************************************************************************
+// * Licensed to the Apache Software Foundation (ASF) under one or more contributor license agreements. See the NOTICE file *
+// * distributed with this work for additional information regarding copyright ownership. The ASF licenses this file *
+// * to you under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance *
+// * with the License. You may obtain a copy of the License at *
+// * *
+// * http://www.apache.org/licenses/LICENSE-2.0 *
+// * *
+// * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an *
+// * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the *
+// * specific language governing permissions and limitations under the License. *
+// ***************************************************************************************************************************
+package org.apache.juneau.rest;
+
+import static org.junit.Assert.*;
+
+import org.apache.juneau.rest.annotation.*;
+import org.apache.juneau.rest.mock.*;
+import org.junit.*;
+import org.junit.runners.*;
+
+/**
+ * Tests various aspects of URL path parts.
+ */
+@SuppressWarnings({"javadoc"})
+@FixMethodOrder(MethodSorters.NAME_ASCENDING)
+public class ThreadLocalObjectsTest {
+
+ //=================================================================================================================
+ // Thread-locals on top-level resource.
+ //=================================================================================================================
+
+ @SuppressWarnings("serial")
+ @RestResource(path="/a")
+ public static class A extends BasicRestServlet {
+ @RestMethod
+ public void getA01() throws Exception {
+ getResponse().getWriter().append(getRequest().getQuery("foo"));
+ }
+
+ @RestHook(HookEvent.END_CALL)
+ public void assertThreadsNotSet() {
+ assertNull(getRequest());
+ assertNull(getResponse());
+ }
+ }
+ static MockRest a = MockRest.create(A.class);
+
+ @Test
+ public void a01() throws Exception {
+ a.get("/a01?foo=bar").execute()
+ .assertBodyContains("bar")
+ ;
+ }
+
+ //=================================================================================================================
+ // Thread-locals on child resource.
+ //=================================================================================================================
+
+ @SuppressWarnings("serial")
+ @RestResource(
+ children={
+ A.class
+ }
+ )
+ public static class B extends BasicRestServletGroup {
+ @RestHook(HookEvent.END_CALL)
+ public void assertThreadsNotSet2() {
+ assertNull(getRequest());
+ assertNull(getResponse());
+ }
+ }
+ static MockRest b = MockRest.create(B.class);
+
+ @Test
+ public void b01() throws Exception {
+ b.get("/a/a01?foo=bar").execute()
+ .assertBodyContains("bar")
+ ;
+ }
+}
\ No newline at end of file