Merge pull request #4333 from joshiste/logfile-resource-handler

* pr/4333:
  Improve resource handling in LogFileMvcEndpoint

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/LogFileMvcEndpoint.java

@@ -18,6 +18,9 @@ package org.springframework.boot.actuate.endpoint.mvc;
 
 import java.io.IOException;
 
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 import javax.validation.constraints.NotNull;
 import javax.validation.constraints.Pattern;
 
@@ -32,19 +35,18 @@ import org.springframework.context.EnvironmentAware;
 import org.springframework.core.env.Environment;
 import org.springframework.core.io.FileSystemResource;
 import org.springframework.core.io.Resource;
+import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
-import org.springframework.http.ResponseEntity;
-import org.springframework.http.ResponseEntity.BodyBuilder;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestMethod;
-import org.springframework.web.bind.annotation.ResponseBody;
+import org.springframework.web.servlet.resource.ResourceHttpRequestHandler;
 
 /**
  * Controller that provides an API for logfiles, i.e. downloading the main logfile
  * configured in environment property 'logging.file' that is standard, but optional
  * property for spring-boot applications.
  *
- * @author Johannes Stelzer
+ * @author Johannes Edmeier
  * @author Phillip Webb
  * @since 1.3.0
  */
@@ -109,28 +111,15 @@ public class LogFileMvcEndpoint implements MvcEndpoint, EnvironmentAware {
 		return null;
 	}
 
-	@RequestMapping(method = RequestMethod.HEAD)
-	@ResponseBody
-	public ResponseEntity<?> available() {
-		return getResponse(false);
-	}
-
-	@RequestMapping(method = RequestMethod.GET)
-	@ResponseBody
-	public ResponseEntity<?> invoke() throws IOException {
-		return getResponse(true);
-	}
-
-	private ResponseEntity<?> getResponse(boolean includeBody) {
+	@RequestMapping(method = { RequestMethod.GET, RequestMethod.HEAD })
+	public void invoke(HttpServletRequest request, HttpServletResponse response)
+			throws ServletException, IOException {
 		if (!isEnabled()) {
-			return (includeBody ? DISABLED_RESPONSE : ResponseEntity.notFound().build());
+			response.setStatus(HttpStatus.NOT_FOUND.value());
+			return;
 		}
 		Resource resource = getLogFileResource();
-		if (resource == null) {
-			return ResponseEntity.notFound().build();
-		}
-		BodyBuilder response = ResponseEntity.ok().contentType(MediaType.TEXT_PLAIN);
-		return (includeBody ? response.body(resource) : response.build());
+		new Handler(resource).handleRequest(request, response);
 	}
 
 	private Resource getLogFileResource() {
@@ -149,4 +138,27 @@ public class LogFileMvcEndpoint implements MvcEndpoint, EnvironmentAware {
 		return resource;
 	}
 
+	/**
+	 * {@link ResourceHttpRequestHandler} to send the log file.
+	 */
+	private static class Handler extends ResourceHttpRequestHandler {
+
+		private final Resource resource;
+
+		public Handler(Resource resource) {
+			this.resource = resource;
+		}
+
+		@Override
+		protected Resource getResource(HttpServletRequest request) throws IOException {
+			return this.resource;
+		}
+
+		@Override
+		protected MediaType getMediaType(Resource resource) {
+			return MediaType.TEXT_PLAIN;
+		}
+
+	}
+
 }

spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/LogFileMvcEndpointTests.java

@@ -18,18 +18,17 @@ package org.springframework.boot.actuate.endpoint.mvc;
 
 import java.io.File;
 import java.io.IOException;
-import java.io.InputStream;
-import java.io.InputStreamReader;
 
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 
-import org.springframework.core.io.Resource;
+import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpStatus;
-import org.springframework.http.ResponseEntity;
 import org.springframework.mock.env.MockEnvironment;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.mock.web.MockHttpServletResponse;
 import org.springframework.util.FileCopyUtils;
 
 import static org.hamcrest.Matchers.equalTo;
@@ -39,7 +38,7 @@ import static org.junit.Assert.assertThat;
 /**
  * Tests for {@link LogFileMvcEndpoint}.
  *
- * @author Johannes Stelzer
+ * @author Johannes Edmeier
  * @author Phillip Webb
  */
 public class LogFileMvcEndpointTests {
@@ -63,37 +62,54 @@ public class LogFileMvcEndpointTests {
 	}
 
 	@Test
-	public void notAvailableWithoutLogFile() throws IOException {
-		assertThat(this.mvc.available().getStatusCode(), equalTo(HttpStatus.NOT_FOUND));
+	public void notAvailableWithoutLogFile() throws Exception {
+		MockHttpServletResponse response = new MockHttpServletResponse();
+		MockHttpServletRequest request = new MockHttpServletRequest(
+				HttpMethod.HEAD.name(), "/logfile");
+		this.mvc.invoke(request, response);
+		assertThat(response.getStatus(), equalTo(HttpStatus.NOT_FOUND.value()));
 	}
 
 	@Test
 	public void notAvailableWithMissingLogFile() throws Exception {
 		this.environment.setProperty("logging.file", "no_test.log");
-		assertThat(this.mvc.available().getStatusCode(), equalTo(HttpStatus.NOT_FOUND));
+		MockHttpServletResponse response = new MockHttpServletResponse();
+		MockHttpServletRequest request = new MockHttpServletRequest(
+				HttpMethod.HEAD.name(), "/logfile");
+		this.mvc.invoke(request, response);
+		assertThat(response.getStatus(), equalTo(HttpStatus.NOT_FOUND.value()));
 	}
 
 	@Test
 	public void availableWithLogFile() throws Exception {
 		this.environment.setProperty("logging.file", this.logFile.getAbsolutePath());
-		assertThat(this.mvc.available().getStatusCode(), equalTo(HttpStatus.OK));
+		MockHttpServletResponse response = new MockHttpServletResponse();
+		MockHttpServletRequest request = new MockHttpServletRequest(
+				HttpMethod.HEAD.name(), "/logfile");
+		this.mvc.invoke(request, response);
+		assertThat(response.getStatus(), equalTo(HttpStatus.OK.value()));
 	}
 
 	@Test
 	public void notAvailableIfDisabled() throws Exception {
 		this.environment.setProperty("logging.file", this.logFile.getAbsolutePath());
 		this.mvc.setEnabled(false);
-		assertThat(this.mvc.available().getStatusCode(), equalTo(HttpStatus.NOT_FOUND));
+		MockHttpServletResponse response = new MockHttpServletResponse();
+		MockHttpServletRequest request = new MockHttpServletRequest(
+				HttpMethod.HEAD.name(), "/logfile");
+		this.mvc.invoke(request, response);
+		assertThat(response.getStatus(), equalTo(HttpStatus.NOT_FOUND.value()));
 	}
 
 	@Test
-	public void invokeGetsContent() throws IOException {
+	public void invokeGetsContent() throws Exception {
 		this.environment.setProperty("logging.file", this.logFile.getAbsolutePath());
-		ResponseEntity<?> response = this.mvc.invoke();
-		assertEquals(HttpStatus.OK, response.getStatusCode());
-		InputStream inputStream = ((Resource) response.getBody()).getInputStream();
-		InputStreamReader reader = new InputStreamReader(inputStream);
-		assertEquals("--TEST--", FileCopyUtils.copyToString(reader));
+		MockHttpServletResponse response = new MockHttpServletResponse();
+		MockHttpServletRequest request = new MockHttpServletRequest(HttpMethod.GET.name(),
+				"/logfile");
+		this.mvc.invoke(request, response);
+		assertThat(response.getStatus(), equalTo(HttpStatus.OK.value()));
+		assertEquals("--TEST--", response.getContentAsString());
 	}
 
 }