diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/resource/ResourceController.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/resource/ResourceController.java index 6bd904a8..a6a32807 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/resource/ResourceController.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/resource/ResourceController.java @@ -20,8 +20,6 @@ import java.io.IOException; import java.io.InputStream; import java.nio.charset.Charset; -import javax.servlet.http.HttpServletRequest; - import org.springframework.cloud.config.environment.Environment; import org.springframework.cloud.config.server.environment.EnvironmentRepository; import org.springframework.core.io.Resource; @@ -35,6 +33,7 @@ import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.ResponseStatus; import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.context.request.ServletWebRequest; import org.springframework.web.util.UrlPathHelper; import static org.springframework.cloud.config.server.support.EnvironmentPropertySource.prepareEnvironment; @@ -71,23 +70,23 @@ public class ResourceController { @RequestMapping("/{name}/{profile}/{label}/**") public String retrieve(@PathVariable String name, @PathVariable String profile, - @PathVariable String label, HttpServletRequest request, + @PathVariable String label, ServletWebRequest request, @RequestParam(defaultValue = "true") boolean resolvePlaceholders) throws IOException { String path = getFilePath(request, name, profile, label); - return retrieve(name, profile, label, path, resolvePlaceholders); + return retrieve(request, name, profile, label, path, resolvePlaceholders); } @RequestMapping(value = "/{name}/{profile}/**", params = "useDefaultLabel") public String retrieve(@PathVariable String name, @PathVariable String profile, - HttpServletRequest request, + ServletWebRequest request, @RequestParam(defaultValue = "true") boolean resolvePlaceholders) throws IOException { String path = getFilePath(request, name, profile, null); - return retrieve(name, profile, null, path, resolvePlaceholders); + return retrieve(request, name, profile, null, path, resolvePlaceholders); } - private String getFilePath(HttpServletRequest request, String name, String profile, + private String getFilePath(ServletWebRequest request, String name, String profile, String label) { String stem; if (label != null) { @@ -96,27 +95,22 @@ public class ResourceController { else { stem = String.format("/%s/%s/", name, profile); } - String path = this.helper.getPathWithinApplication(request); + String path = this.helper.getPathWithinApplication(request.getRequest()); path = path.substring(path.indexOf(stem) + stem.length()); return path; } - synchronized String retrieve(String name, String profile, String label, String path, - boolean resolvePlaceholders) throws IOException { - if (name != null && name.contains("(_)")) { - // "(_)" is uncommon in a git repo name, but "/" cannot be matched - // by Spring MVC - name = name.replace("(_)", "/"); + synchronized String retrieve(ServletWebRequest request, String name, String profile, + String label, String path, boolean resolvePlaceholders) throws IOException { + name = resolveName(name); + label = resolveLabel(label); + Resource resource = this.resourceRepository.findOne(name, profile, label, path); + if (checkNotModified(request, resource)) { + // Content was not modified. Just return. + return null; } - if (label != null && label.contains("(_)")) { - // "(_)" is uncommon in a git branch name, but "/" cannot be matched - // by Spring MVC - label = label.replace("(_)", "/"); - } - // ensure InputStream will be closed to prevent file locks on Windows - try (InputStream is = this.resourceRepository.findOne(name, profile, label, path) - .getInputStream()) { + try (InputStream is = resource.getInputStream()) { String text = StreamUtils.copyToString(is, Charset.forName("UTF-8")); if (resolvePlaceholders) { Environment environment = this.environmentRepository.findOne(name, @@ -127,31 +121,71 @@ public class ResourceController { } } - @RequestMapping(value = "/{name}/{profile}/{label}/**", produces = MediaType.APPLICATION_OCTET_STREAM_VALUE) - public byte[] binary(@PathVariable String name, @PathVariable String profile, - @PathVariable String label, HttpServletRequest request) throws IOException { - String path = getFilePath(request, name, profile, label); - return binary(name, profile, label, path); + /* + * Used only for unit tests. + */ + String retrieve(String name, String profile, String label, String path, + boolean resolvePlaceholders) throws IOException { + return retrieve(null, name, profile, label, path, resolvePlaceholders); } - synchronized byte[] binary(String name, String profile, String label, String path) + @RequestMapping(value = "/{name}/{profile}/{label}/**", produces = MediaType.APPLICATION_OCTET_STREAM_VALUE) + public byte[] binary(@PathVariable String name, @PathVariable String profile, + @PathVariable String label, ServletWebRequest request) throws IOException { + String path = getFilePath(request, name, profile, label); + return binary(request, name, profile, label, path); + } + + /* + * Used only for unit tests. + */ + byte[] binary(String name, String profile, String label, String path) throws IOException { + return binary(null, name, profile, label, path); + } + + private synchronized byte[] binary(ServletWebRequest request, String name, + String profile, String label, String path) throws IOException { + name = resolveName(name); + label = resolveLabel(label); + Resource resource = this.resourceRepository.findOne(name, profile, label, path); + if (checkNotModified(request, resource)) { + // Content was not modified. Just return. + return null; + } + // TODO: is this line needed for side effects? + prepareEnvironment(this.environmentRepository.findOne(name, profile, label)); + try (InputStream is = resource.getInputStream()) { + return StreamUtils.copyToByteArray(is); + } + } + + private boolean checkNotModified(ServletWebRequest request, Resource resource) { + try { + return request != null && request.checkNotModified(resource.lastModified()); + } + catch (Exception ex) { + // Ignore the exception since caching is optional. + } + return false; + } + + private String resolveName(String name) { if (name != null && name.contains("(_)")) { // "(_)" is uncommon in a git repo name, but "/" cannot be matched // by Spring MVC name = name.replace("(_)", "/"); } + return name; + } + + private String resolveLabel(String label) { if (label != null && label.contains("(_)")) { // "(_)" is uncommon in a git branch name, but "/" cannot be matched // by Spring MVC label = label.replace("(_)", "/"); } - // TODO: is this line needed for side effects? - prepareEnvironment(this.environmentRepository.findOne(name, profile, label)); - try (InputStream is = this.resourceRepository.findOne(name, profile, label, path) - .getInputStream()) { - return StreamUtils.copyToByteArray(is); - } + return label; } @ExceptionHandler(NoSuchResourceException.class) diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/resource/ResourceControllerTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/resource/ResourceControllerTests.java index 7353a100..dc247d61 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/resource/ResourceControllerTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/resource/ResourceControllerTests.java @@ -27,6 +27,8 @@ import org.springframework.cloud.config.server.environment.NativeEnvironmentRepo import org.springframework.cloud.config.server.environment.NativeEnvironmentRepositoryTests; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.web.context.request.ServletWebRequest; import static org.assertj.core.api.Assertions.assertThat; @@ -174,8 +176,10 @@ public class ResourceControllerTests { public void resourceWithSlashRequest() throws Exception { this.environmentRepository.setSearchLocations("classpath:/test"); MockHttpServletRequest request = new MockHttpServletRequest(); + ServletWebRequest webRequest = new ServletWebRequest(request, + new MockHttpServletResponse()); request.setRequestURI("/foo/bar/dev/" + "spam/foo.txt"); - String resource = this.controller.retrieve("foo", "bar", "dev", request, true); + String resource = this.controller.retrieve("foo", "bar", "dev", webRequest, true); assertThat(resource).isEqualToIgnoringNewLines("foo: dev_bar/spam"); } @@ -183,9 +187,11 @@ public class ResourceControllerTests { public void resourceWithSlashRequestAndServletPath() throws Exception { this.environmentRepository.setSearchLocations("classpath:/test"); MockHttpServletRequest request = new MockHttpServletRequest(); + ServletWebRequest webRequest = new ServletWebRequest(request, + new MockHttpServletResponse()); request.setServletPath("/spring"); request.setRequestURI("/foo/bar/dev/" + "spam/foo.txt"); - String resource = this.controller.retrieve("foo", "bar", "dev", request, true); + String resource = this.controller.retrieve("foo", "bar", "dev", webRequest, true); assertThat(resource).isEqualToIgnoringNewLines("foo: dev_bar/spam"); } @@ -209,8 +215,11 @@ public class ResourceControllerTests { public void resourceWithSlashForResolvePlaceholdersFalseRequest() throws Exception { this.environmentRepository.setSearchLocations("classpath:/test"); MockHttpServletRequest request = new MockHttpServletRequest(); + ServletWebRequest webRequest = new ServletWebRequest(request, + new MockHttpServletResponse()); request.setRequestURI("/foo/bar/dev/" + "spam/foo.txt"); - String resource = this.controller.retrieve("foo", "bar", "dev", request, false); + String resource = this.controller.retrieve("foo", "bar", "dev", webRequest, + false); assertThat(resource).isEqualToIgnoringNewLines("foo: dev_bar/spam"); } @@ -279,8 +288,10 @@ public class ResourceControllerTests { public void resourceWithSlashForBinaryRequest() throws Exception { this.environmentRepository.setSearchLocations("classpath:/test"); MockHttpServletRequest request = new MockHttpServletRequest(); + ServletWebRequest webRequest = new ServletWebRequest(request, + new MockHttpServletResponse()); request.setRequestURI("/foo/bar/dev/" + "spam/foo.txt"); - byte[] resource = this.controller.binary("foo", "bar", "dev", request); + byte[] resource = this.controller.binary("foo", "bar", "dev", webRequest); assertThat(new String(resource)).isEqualToIgnoringNewLines("foo: dev_bar/spam"); }