killbill-memoizeit

logging: prevent double logging by ExceptionMapperBase TimedResourceInterceptor

4/12/2016 4:58:21 PM

Details

diff --git a/jaxrs/src/main/java/org/killbill/billing/jaxrs/mappers/ExceptionMapperBase.java b/jaxrs/src/main/java/org/killbill/billing/jaxrs/mappers/ExceptionMapperBase.java
index 8794159..35cd19f 100644
--- a/jaxrs/src/main/java/org/killbill/billing/jaxrs/mappers/ExceptionMapperBase.java
+++ b/jaxrs/src/main/java/org/killbill/billing/jaxrs/mappers/ExceptionMapperBase.java
@@ -1,7 +1,9 @@
 /*
- * Copyright 2010-2013 Ning, Inc.
+ * Copyright 2010-2014 Ning, Inc.
+ * Copyright 2014-2016 Groupon, Inc
+ * Copyright 2014-2016 The Billing Project, LLC
  *
- * Ning licenses this file to you under the Apache License, version 2.0
+ * The Billing Project 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:
  *
@@ -112,55 +114,40 @@ public abstract class ExceptionMapperBase {
     }
 
     protected Response buildConflictingRequestResponse(final Exception e, final UriInfo uriInfo) {
-        // Log the full stacktrace
-        log.warn("Conflicting request", e);
-
         final Response.ResponseBuilder responseBuilder = Response.status(Status.CONFLICT);
         serializeException(e, uriInfo, responseBuilder);
-        return responseBuilder.build();
+        return new LoggingResponse(e, responseBuilder.build());
     }
 
     protected Response buildNotFoundResponse(final Exception e, final UriInfo uriInfo) {
-        // Log the full stacktrace
-        log.info("Not found", e);
-
         final Response.ResponseBuilder responseBuilder = Response.status(Status.NOT_FOUND);
         serializeException(e, uriInfo, responseBuilder);
-        return responseBuilder.build();
+        return new LoggingResponse(e, responseBuilder.build());
     }
 
     protected Response buildBadRequestResponse(final Exception e, final UriInfo uriInfo) {
-        // Log the full stacktrace
-        log.warn("Bad request", e);
-
         final Response.ResponseBuilder responseBuilder = Response.status(Status.BAD_REQUEST);
         serializeException(e, uriInfo, responseBuilder);
-        return responseBuilder.build();
+        return new LoggingResponse(e, responseBuilder.build());
     }
 
     protected Response buildAuthorizationErrorResponse(final Exception e, final UriInfo uriInfo) {
-        // Log the full stacktrace
-        log.warn("Authorization error", e);
-
         // TODO Forbidden?
         final Response.ResponseBuilder responseBuilder = Response.status(Status.UNAUTHORIZED);
         serializeException(e, uriInfo, responseBuilder);
-        return responseBuilder.build();
+        return new LoggingResponse(e, responseBuilder.build());
     }
 
     protected Response buildInternalErrorResponse(final Exception e, final UriInfo uriInfo) {
-        // Log the full stacktrace
-        log.warn("Internal error", e);
-
         final Response.ResponseBuilder responseBuilder = Response.status(Status.INTERNAL_SERVER_ERROR);
         serializeException(e, uriInfo, responseBuilder);
-        return responseBuilder.build();
+        return new LoggingResponse(e, responseBuilder.build());
     }
 
     protected Response buildPluginTimeoutResponse(final Exception e, final UriInfo uriInfo) {
         final Response.ResponseBuilder responseBuilder = Response.status(Status.ACCEPTED);
         serializeException(e, uriInfo, responseBuilder);
-        return responseBuilder.build();
+        return new LoggingResponse(e, responseBuilder.build());
     }
 
     private void serializeException(final Exception e, final UriInfo uriInfo, final Response.ResponseBuilder responseBuilder) {
diff --git a/jaxrs/src/main/java/org/killbill/billing/jaxrs/mappers/LoggingResponse.java b/jaxrs/src/main/java/org/killbill/billing/jaxrs/mappers/LoggingResponse.java
new file mode 100644
index 0000000..e3635ca
--- /dev/null
+++ b/jaxrs/src/main/java/org/killbill/billing/jaxrs/mappers/LoggingResponse.java
@@ -0,0 +1,66 @@
+/*
+ * Copyright 2016 Groupon, Inc
+ * Copyright 2016 The Billing Project, LLC
+ *
+ * The Billing Project 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.killbill.billing.jaxrs.mappers;
+
+import javax.ws.rs.core.MultivaluedMap;
+import javax.ws.rs.core.Response;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class LoggingResponse extends Response {
+
+    private static final Logger log = LoggerFactory.getLogger(LoggingResponse.class);
+
+    private final Exception e;
+    private final Response response;
+
+    public LoggingResponse(final Exception e, final Response response) {
+        this.e = e;
+        this.response = response;
+    }
+
+    @Override
+    public Object getEntity() {
+        // Delay logging until the entity is retrieved: this is to avoid double logging with TimedResourceInterceptor
+        // which needs to access exception mappers to get the response status
+        if (response.getStatus() == Status.CONFLICT.getStatusCode()) {
+            log.warn("Conflicting request", e);
+        } else if (response.getStatus() == Status.NOT_FOUND.getStatusCode()) {
+            log.debug("Not found", e);
+        } else if (response.getStatus() == Status.BAD_REQUEST.getStatusCode()) {
+            log.warn("Bad request", e);
+        } else if (response.getStatus() == Status.UNAUTHORIZED.getStatusCode()) {
+            log.debug("Authorization error", e);
+        } else if (response.getStatus() == Status.INTERNAL_SERVER_ERROR.getStatusCode()) {
+            log.warn("Internal error", e);
+        }
+
+        return response.getEntity();
+    }
+
+    @Override
+    public int getStatus() {
+        return response.getStatus();
+    }
+
+    @Override
+    public MultivaluedMap<String, Object> getMetadata() {
+        return response.getMetadata();
+    }
+}