fix(servlet): Allow overriding context set by servlet callback (#63)

Author: kattrali

build.gradle

@@ -32,6 +32,8 @@ dependencies {
 
     testCompile 'junit:junit:4.12'
     testCompile 'org.slf4j:slf4j-jdk14:1.7.25'
+    testCompile 'javax.servlet:javax.servlet-api:3.1.0'
+    testCompile 'org.mockito:mockito-core:2.10.0'
 }
 
 test {

examples/servlet/build.gradle

@@ -17,7 +17,7 @@ repositories {
 }
 
 dependencies {
-    runtime 'org.slf4j:slf4j-simple:1.+'
+    runtime 'org.slf4j:slf4j-simple:1.7.25'
     compile rootProject
 }
 

src/main/java/com/bugsnag/callbacks/ServletCallback.java

@@ -41,7 +41,9 @@ public void beforeNotify(Report report) {
                 .addToTab("request", "headers", getHeaderMap(request));
 
         // Set default context
-        report.setContext(request.getMethod() + " " + request.getRequestURI());
+        if (report.getContext() == null) {
+            report.setContext(request.getMethod() + " " + request.getRequestURI());
+        }
     }
 
     private String getClientIp(HttpServletRequest request) {

src/test/java/com/bugsnag/ServletCallbackTest.java

@@ -0,0 +1,114 @@
+package com.bugsnag.servlet;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import com.bugsnag.Bugsnag;
+import com.bugsnag.Report;
+import com.bugsnag.callbacks.ServletCallback;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import java.lang.StringBuffer;
+import java.util.Arrays;
+import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Vector;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletRequestEvent;
+import javax.servlet.http.HttpServletRequest;
+
+public class ServletCallbackTest {
+
+    /**
+     * Generate a new request instance which will be read by the servlet
+     * context and callback
+     */
+    @Before
+    public void setUp() {
+        HttpServletRequest request = mock(HttpServletRequest.class);
+
+        Map<String, String[]> params = new HashMap<String, String[]>();
+        params.put("account", new String[]{"Acme Co"});
+        params.put("name", new String[]{"Bill"});
+        when(request.getParameterMap()).thenReturn(params);
+
+        when(request.getMethod()).thenReturn("PATCH");
+        when(request.getRequestURL()).thenReturn(new StringBuffer("/foo/bar"));
+        when(request.getRequestURI()).thenReturn("/foo/bar");
+        when(request.getRemoteAddr()).thenReturn("12.0.4.57");
+
+        Enumeration<String> headers = new Vector<String>(
+                Arrays.asList("Content-Type", "Content-Length")).elements();
+        when(request.getHeaderNames()).thenReturn(headers);
+        when(request.getHeader("Content-Type")).thenReturn("application/json");
+        when(request.getHeader("Content-Length")).thenReturn("54");
+
+        ServletContext context = mock(ServletContext.class);
+        BugsnagServletRequestListener listener = new BugsnagServletRequestListener();
+        listener.requestInitialized(new ServletRequestEvent(context, request));
+    }
+
+    @SuppressWarnings("unchecked")
+    @Test
+    public void testRequestMetadataAdded() {
+        Report report = generateReport(new Exception("Spline reticulation failed"));
+        ServletCallback callback = new ServletCallback();
+        callback.beforeNotify(report);
+
+        Map<String, Object> metadata = (Map<String, Object>)report.getMetaData();
+        assertTrue(metadata.containsKey("request"));
+
+        Map<String, Object> request = (Map<String, Object>)metadata.get("request");
+        assertEquals("/foo/bar", request.get("url"));
+        assertEquals("PATCH", request.get("method"));
+        assertEquals("12.0.4.57", request.get("clientIp"));
+
+        assertTrue(request.containsKey("headers"));
+        Map<String, String> headers = (Map<String, String>)request.get("headers");
+        assertEquals("application/json", headers.get("Content-Type"));
+        assertEquals("54", headers.get("Content-Length"));
+
+        assertTrue(request.containsKey("params"));
+        Map<String, String[]> params = (Map<String, String[]>)request.get("params");
+        assertTrue(params.containsKey("account"));
+        String[] account = params.get("account");
+        assertEquals("Acme Co", account[0]);
+
+        assertTrue(params.containsKey("name"));
+        String[] name = params.get("name");
+        assertEquals("Bill", name[0]);
+    }
+
+    @Test
+    public void testRequestContextSet() {
+        Report report = generateReport(new Exception("Spline reticulation failed"));
+        ServletCallback callback = new ServletCallback();
+        callback.beforeNotify(report);
+
+        assertEquals("PATCH /foo/bar", report.getContext());
+    }
+
+    @Test
+    public void testExistingContextNotOverridden() {
+        Report report = generateReport(new Exception("Spline reticulation failed"));
+        report.setContext("Honey nut corn flakes");
+        ServletCallback callback = new ServletCallback();
+        callback.beforeNotify(report);
+
+        assertEquals("Honey nut corn flakes", report.getContext());
+    }
+
+    Report generateReport(Exception exception) {
+        Bugsnag bugsnag = new Bugsnag("apikey", false);
+        bugsnag.setDelivery(null);
+
+        return bugsnag.buildReport(exception);
+    }
+}