Repository: tipsy/javalin
Pull request #954 (2020-05-09 09:44:12)

[static-files] Allow path prefixing


Patch:
diff --git a/javalin/src/main/java/io/javalin/core/JavalinConfig.java b/javalin/src/main/java/io/javalin/core/JavalinConfig.java
index 483dd5d4..cef739ae 100644
--- a/javalin/src/main/java/io/javalin/core/JavalinConfig.java
+++ b/javalin/src/main/java/io/javalin/core/JavalinConfig.java
@@ -113,9 +113,14 @@ public class JavalinConfig {
     }
 
     public JavalinConfig addStaticFiles(@NotNull String path, @NotNull Location location) {
+        addStaticFiles("/", path, location);
+        return this;
+    }
+
+    public JavalinConfig addStaticFiles(@NotNull String urlPathPrefix, @NotNull String path, @NotNull Location location) {
         JettyUtil.disableJettyLogger();
         if (inner.resourceHandler == null) inner.resourceHandler = new JettyResourceHandler();
-        inner.resourceHandler.addStaticFileConfig(new StaticFileConfig(path, location));
+        inner.resourceHandler.addStaticFileConfig(new StaticFileConfig(urlPathPrefix, path, location));
         return this;
     }
 
diff --git a/javalin/src/main/java/io/javalin/http/staticfiles/JettyResourceHandler.kt b/javalin/src/main/java/io/javalin/http/staticfiles/JettyResourceHandler.kt
index 30890ab6..b7b05822 100644
--- a/javalin/src/main/java/io/javalin/http/staticfiles/JettyResourceHandler.kt
+++ b/javalin/src/main/java/io/javalin/http/staticfiles/JettyResourceHandler.kt
@@ -11,8 +11,8 @@ import io.javalin.core.util.Header
 import io.javalin.core.util.Util
 import io.javalin.http.JavalinResponseWrapper
 import org.eclipse.jetty.server.Request
-import org.eclipse.jetty.server.Server
 import org.eclipse.jetty.server.handler.ResourceHandler
+import org.eclipse.jetty.util.resource.EmptyResource
 import org.eclipse.jetty.util.resource.Resource
 import java.io.File
 import javax.servlet.http.HttpServletRequest
@@ -22,18 +22,17 @@ class JettyResourceHandler : io.javalin.http.staticfiles.ResourceHandler {
 
     val handlers = mutableListOf<ResourceHandler>()
 
-    // It would work without a server, but if none is set jetty will log a warning.
-    private val dummyServer = Server()
-
     override fun addStaticFileConfig(config: StaticFileConfig) {
-        val handler = if (config.path == "/webjars") WebjarHandler() else ResourceHandler().apply {
+        val handler = if (config.path == "/webjars") WebjarHandler() else PrefixableHandler(config.urlPathPrefix).apply {
             resourceBase = getResourcePath(config)
             isDirAllowed = false
             isEtags = true
-            Javalin.log?.info("Static file handler added with path=${config.path} and location=${config.location}. Absolute path: '${getResourcePath(config)}'.")
+            Javalin.log?.info("""Static file handler added:
+                |    {urlPathPrefix: "${config.urlPathPrefix}", path: "${config.path}", location: Location.${config.location}}
+                |    Resolved path: '${getResourcePath(config)}'
+                """.trimMargin())
         }
         handlers.add(handler.apply {
-            server = dummyServer
             start()
         })
     }
@@ -42,7 +41,19 @@ class JettyResourceHandler : io.javalin.http.staticfiles.ResourceHandler {
         override fun getResource(path: String) = Resource.newClassPathResource("META-INF/resources$path") ?: super.getResource(path)
     }
 
-    fun getResourcePath(staticFileConfig: StaticFileConfig): String {
+    inner class PrefixableHandler(private var urlPathPrefix: String) : ResourceHandler() {
+        override fun getResource(path: String): Resource {
+            val targetResource by lazy { path.removePrefix(urlPathPrefix) }
+            return when {
+                urlPathPrefix == "/" -> super.getResource(path) // same as regular ResourceHandler
+                !path.startsWith(urlPathPrefix) -> EmptyResource.INSTANCE
+                !targetResource.startsWith("/") -> EmptyResource.INSTANCE
+                else -> super.getResource(targetResource)
+            }
+        }
+    }
+
+    private fun getResourcePath(staticFileConfig: StaticFileConfig): String {
         val nosuchdir = "Static resource directory with path: '${staticFileConfig.path}' does not exist."
         if (staticFileConfig.location == Location.CLASSPATH) {
             val classPathResource = Resource.newClassPathResource(staticFileConfig.path)
diff --git a/javalin/src/main/java/io/javalin/http/staticfiles/ResourceHandler.kt b/javalin/src/main/java/io/javalin/http/staticfiles/ResourceHandler.kt
index 1bd48c8d..7ee29acf 100644
--- a/javalin/src/main/java/io/javalin/http/staticfiles/ResourceHandler.kt
+++ b/javalin/src/main/java/io/javalin/http/staticfiles/ResourceHandler.kt
@@ -4,7 +4,7 @@ import javax.servlet.http.HttpServletRequest
 import javax.servlet.http.HttpServletResponse
 
 enum class Location { CLASSPATH, EXTERNAL; }
-data class StaticFileConfig(val path: String, val location: Location)
+data class StaticFileConfig(val urlPathPrefix: String, val path: String, val location: Location)
 
 interface ResourceHandler {
     fun handle(httpRequest: HttpServletRequest, httpResponse: HttpServletResponse): Boolean
diff --git a/javalin/src/main/java/io/javalin/plugin/metrics/MicrometerPlugin.kt b/javalin/src/main/java/io/javalin/plugin/metrics/MicrometerPlugin.kt
index fae0e9ab..b21311aa 100644
--- a/javalin/src/main/java/io/javalin/plugin/metrics/MicrometerPlugin.kt
+++ b/javalin/src/main/java/io/javalin/plugin/metrics/MicrometerPlugin.kt
@@ -1,25 +1,78 @@
+/*
+ * Javalin - https://javalin.io
+ * Copyright 2020 David Åse
+ * Licensed under Apache 2.0: https://github.com/tipsy/javalin/blob/master/LICENSE
+ */
+
 package io.javalin.plugin.metrics
 
 import io.javalin.Javalin
 import io.javalin.core.plugin.Plugin
-import io.javalin.core.util.OptionalDependency
-import io.javalin.core.util.Util
+import io.javalin.http.Context
+import io.javalin.http.ExceptionHandler
+import io.javalin.http.HandlerEntry
+import io.javalin.http.HandlerType
 import io.micrometer.core.instrument.MeterRegistry
 import io.micrometer.core.instrument.Metrics
 import io.micrometer.core.instrument.Tag
+import io.micrometer.core.instrument.Tags
+import io.micrometer.core.instrument.binder.http.DefaultHttpServletRequestTagsProvider
+import io.micrometer.core.instrument.binder.jetty.JettyConnectionMetrics
 import io.micrometer.core.instrument.binder.jetty.JettyServerThreadPoolMetrics
-import io.micrometer.core.instrument.binder.jetty.JettyStatisticsMetrics
-import org.eclipse.jetty.server.handler.StatisticsHandler
+import io.micrometer.core.instrument.binder.jetty.TimedHandler
+import org.apache.commons.lang3.StringUtils
+import javax.servlet.http.HttpServletRequest
+import javax.servlet.http.HttpServletResponse
 
-class MicrometerPlugin @JvmOverloads constructor(val registry: MeterRegistry = Metrics.globalRegistry) : Plugin {
+class MicrometerPlugin @JvmOverloads constructor(private val registry: MeterRegistry = Metrics.globalRegistry,
+                                                 private val tags: Iterable<Tag> = Tags.empty(),
+                                                 private val tagExceptionName: Boolean = false) : Plugin {
     override fun apply(app: Javalin) {
         app.server()?.server()?.let { server ->
-            Util.ensureDependencyPresent(OptionalDependency.MICROMETER)
-            val statisticsHandler = StatisticsHandler()
-            server.insertHandler(statisticsHandler)
-            JettyStatisticsMetrics.monitor(this.registry, statisticsHandler)
-            val threadPoolMetrics = JettyServerThreadPoolMetrics(server.threadPool, emptyList<Tag>())
-            threadPoolMetrics.bindTo(this.registry)
+            if(tagExceptionName) {
+                app.exception(Exception::class.java, EXCEPTION_HANDLER)
+            }
+
+            server.insertHandler(TimedHandler(registry, tags, object : DefaultHttpServletRequestTagsProvider() {
+                override fun getTags(request: HttpServletRequest, response: HttpServletResponse): Iterable<Tag> {
+                    val exceptionName = if(tagExceptionName) {
+                        response.getHeader(EXCEPTION_HEADER)
+                    } else {
+                        "Unknown"
+                    }
+
+                    response.setHeader(EXCEPTION_HEADER, null)
+                    val uri = app.servlet()
+                            .matcher
+                            .findEntries(HandlerType.valueOf(request.method), request.pathInfo)
+                            .stream()
+                            .findAny()
+                            .map(HandlerEntry::path)
+                            .map { path: String -> if (path == "/" || StringUtils.isBlank(path)) "root" else path }
+                            .map { path: String -> if (response.status in 300..399) "REDIRECTION" else path }
+                            .map { path: String -> if (response.status == 404) "NOT_FOUND" else path }
+                            .orElse("NOT_FOUND")
+                    return Tags.concat(
+                            super.getTags(request, response),
+                            "uri", uri,
+                            "exception", exceptionName ?: "None"
+                    )
+                }
+            }))
+
+            JettyServerThreadPoolMetrics(server.threadPool, tags).bindTo(registry)
+        }
+
+        JettyConnectionMetrics(registry, tags)
+    }
+
+    companion object {
+        private const val EXCEPTION_HEADER = "__micrometer_exception_name"
+
+        var EXCEPTION_HANDLER = ExceptionHandler { e: Exception, ctx: Context ->
+            val simpleName = e.javaClass.simpleName
+            ctx.header(EXCEPTION_HEADER, if (StringUtils.isNotBlank(simpleName)) simpleName else e.javaClass.name)
+            ctx.status(500)
         }
     }
 }
diff --git a/pom.xml b/pom.xml
index c6711e8e..c9e5459b 100644
--- a/pom.xml
+++ b/pom.xml
@@ -72,7 +72,7 @@
         <jtwig.version>5.87.0.RELEASE</jtwig.version>
         <jvmbrotli.version>0.2.0</jvmbrotli.version>
         <logback.version>1.2.3</logback.version>
-        <micrometer.version>1.3.6</micrometer.version>
+        <micrometer.version>1.5.1</micrometer.version>
         <mustache.version>0.9.6</mustache.version>
         <pebble.version>3.1.2</pebble.version>
         <redoc.version>2.0.0-rc.23</redoc.version>
@@ -87,7 +87,7 @@
         <assertj.version>3.15.0</assertj.version>
         <common.io.version>2.6</common.io.version>
         <gson.version>2.8.6</gson.version>
-        <java.websocket.version>1.4.1</java.websocket.version>
+        <java.websocket.version>1.5.0</java.websocket.version>
         <javax.servlet.api.version>3.1.0</javax.servlet.api.version>
         <junit.benchmark.version>0.7.2</junit.benchmark.version>
         <junit.version>4.13</junit.version>


Test patch:
diff --git a/javalin/src/test/java/io/javalin/TestMicrometerPlugin.kt b/javalin/src/test/java/io/javalin/TestMicrometerPlugin.kt
new file mode 100644
index 00000000..40efb883
--- /dev/null
+++ b/javalin/src/test/java/io/javalin/TestMicrometerPlugin.kt
@@ -0,0 +1,95 @@
+/*
+ * Javalin - https://javalin.io
+ * Copyright 2020 David Åse
+ * Licensed under Apache 2.0: https://github.com/tipsy/javalin/blob/master/LICENSE
+ */
+
+package io.javalin
+
+import io.javalin.plugin.metrics.MicrometerPlugin
+import io.javalin.testing.HttpUtil
+import io.micrometer.core.instrument.MeterRegistry
+import io.micrometer.core.instrument.Tags
+import io.micrometer.core.instrument.simple.SimpleMeterRegistry
+import org.junit.After
+import org.junit.Before
+import org.junit.Test
+
+class TestMicrometerPlugin {
+    private val meterRegistry: MeterRegistry = SimpleMeterRegistry()
+    val app: Javalin = Javalin.create { config -> config.registerPlugin(MicrometerPlugin(meterRegistry, Tags.empty(), true)) }.start(0);
+    val http = HttpUtil(app.port())
+
+    @Before
+    fun before() {
+        // must manually delegate to Micrometer exception handler for excepton tags to be correct
+        app.exception(IllegalArgumentException::class.java) { e, ctx ->
+            MicrometerPlugin.EXCEPTION_HANDLER.handle(e, ctx);
+            e.printStackTrace();
+        };
+    }
+
+    @After
+    fun after() {
+        app.stop()
+    }
+
+    @Test
+    fun `successful request`() {
+        app.get("/hello/:name") { ctx -> ctx.result("Hello: " + ctx.pathParam("name")) }
+        http.get("/hello/jon")
+        meterRegistry.get("jetty.server.requests")
+                .tag("uri", "/hello/:name")
+                .tag("method", "GET")
+                .tag("exception", "None")
+                .tag("status", "200")
+                .tag("outcome", "SUCCESS")
+                .timer()
+    }
+
+    @Test
+    fun `request throwing exception`() {
+        app.get("/boom") { ctx -> throw IllegalArgumentException("boom") }
+        http.get("/boom")
+        meterRegistry.get("jetty.server.requests")
+                .tag("uri", "/boom")
+                .tag("method", "GET")
+                .tag("exception", "IllegalArgumentException")
+                .tag("status", "500")
+                .tag("outcome", "SERVER_ERROR")
+                .timer()
+    }
+
+    @Test
+    fun redirect() {
+        app.get("/hello") { ctx -> ctx.result("Hello") }
+        app.get("/redirect") { ctx -> ctx.redirect("/hello") }
+        http.get("/redirect")
+        meterRegistry.get("jetty.server.requests")
+                .tag("uri", "REDIRECTION")
+                .tag("method", "GET")
+                .tag("exception", "None")
+                .tag("status", "302")
+                .tag("outcome", "REDIRECTION")
+                .timer()
+        meterRegistry.get("jetty.server.requests")
+                .tag("uri", "/hello")
+                .tag("method", "GET")
+                .tag("exception", "None")
+                .tag("status", "200")
+                .tag("outcome", "SUCCESS")
+                .timer()
+    }
+
+    @Test
+    fun `not found`() {
+        http.get("/doesNotExist")
+        meterRegistry.get("jetty.server.requests")
+                .tag("uri", "NOT_FOUND")
+                .tag("method", "GET")
+                .tag("exception", "None")
+                .tag("status", "404")
+                .tag("outcome", "CLIENT_ERROR")
+                .timer()
+    }
+}
diff --git a/javalin/src/test/java/io/javalin/TestStaticFiles.kt b/javalin/src/test/java/io/javalin/TestStaticFiles.kt
index b7a3ee62..53bca4e7 100644
--- a/javalin/src/test/java/io/javalin/TestStaticFiles.kt
+++ b/javalin/src/test/java/io/javalin/TestStaticFiles.kt
@@ -156,4 +156,28 @@ class TestStaticFiles {
         assertThat(http.get("/html.html").status).isEqualTo(200)
         assertThat(http.get("/html.html").headers.getFirst(Header.CONTENT_TYPE)).contains("text/html")
     }
+
+    @Test
+    fun `serving from custom url path works`() {
+        TestUtil.test(Javalin.create { servlet ->
+            servlet.addStaticFiles("/public")
+            servlet.addStaticFiles("/url-prefix", "/public", Location.CLASSPATH)
+        }) { _, http ->
+            assertThat(http.get("/styles.css").status).isEqualTo(200)
+            assertThat(http.get("/url-prefix/styles.css").status).isEqualTo(200)
+        }
+    }
+
+    @Test
+    fun `urlPathPrefix filters requests to a specific subfolder`() {
+        TestUtil.test(Javalin.create { servlet ->
+            // effectively equivalent to servlet.addStaticFiles("/public", Location.CLASSPATH)
+            // but with benefit of additional "filtering": only requests matching /assets/* will be matched against static resources handler
+            servlet.addStaticFiles("/assets", "/public/assets", Location.CLASSPATH)
+        }) { _, http ->
+            assertThat(http.get("/assets/filtered-styles.css").status).isEqualTo(200) // access to urls matching /assets/* is allowed
+            assertThat(http.get("/filtered-styles.css").status).isEqualTo(404) // direct access to a file in the subfolder is not allowed
+            assertThat(http.get("/styles.css").status).isEqualTo(404) // access to other locations in /public is not allowed
+        }
+    }
 }
diff --git a/javalin/src/test/resources/public/assets/filtered-styles.css b/javalin/src/test/resources/public/assets/filtered-styles.css
new file mode 100644
index 00000000..9c0e6d04
--- /dev/null
+++ b/javalin/src/test/resources/public/assets/filtered-styles.css
@@ -0,0 +1,3 @@
+.css-test:before {
+    content: "CSS works"
+}