Cleanup filters design

resoluteworks · May 2, 2024 · a1a2e64 · a1a2e64
1 parent bd9ffaa
commit a1a2e64
Show file tree

Hide file tree

Showing 24 changed files with 57 additions and 149 deletions.
diff --git a/examples/gradle.properties b/examples/gradle.properties
@@ -1,2 +1,2 @@
-invirtVersion = 0.8.24
-http4kVersion = 5.16.2.0
+invirtVersion = 0.8.26
+http4kVersion = 5.17.0.0
diff --git a/examples/hotwire/src/main/kotlin/invirt/examples/hotwire/Application.kt b/examples/hotwire/src/main/kotlin/invirt/examples/hotwire/Application.kt
@@ -1,10 +1,8 @@
 package invirt.examples.hotwire
 
 import invirt.http4k.*
-import invirt.http4k.filters.CatchAllFilter
-import invirt.http4k.views.errorResponse
-import invirt.http4k.views.withView
-import invirt.pebble.invirtPebbleFilter
+import invirt.http4k.filters.CatchAll
+import invirt.http4k.views.*
 import io.github.oshai.kotlinlogging.KotlinLogging
 import org.http4k.cloudnative.env.Environment
 import org.http4k.cloudnative.env.EnvironmentKey
@@ -29,8 +27,8 @@ class Application {
 
         val userService = UserService()
         val appHandler = AppRequestContexts()
-            .then(CatchAllFilter())
-            .then(invirtPebbleFilter)
+            .then(CatchAll())
+            .then(StoreRequestOnThread())
             .then(
                 routes(
                     "/" GET {

diff --git a/examples/hotwire/src/main/kotlin/invirt/examples/hotwire/user-forms.kt b/examples/hotwire/src/main/kotlin/invirt/examples/hotwire/user-forms.kt
@@ -1,6 +1,6 @@
 package invirt.examples.hotwire
 
-import invirt.http4k.ViewResponse
+import invirt.http4k.views.ViewResponse
 import io.validk.ValidObject
 import io.validk.Validation
 import io.validk.email

diff --git a/gradle.properties b/gradle.properties
@@ -1,4 +1,4 @@
-invirtVersion = 0.8.24
+invirtVersion = 0.8.26
 kotlinVersion = 1.9.23
 http4kVersion = 5.17.0.0
 mockkVersion = 1.13.9

diff --git a/invirt-http4k/src/main/kotlin/invirt/http4k/current-request.kt b/invirt-http4k/src/main/kotlin/invirt/http4k/current-request.kt
@@ -0,0 +1,16 @@
+package invirt.http4k
+
+import org.http4k.core.Filter
+import org.http4k.core.Request
+
+internal val requestThreadLocal = ThreadLocal<Request>()
+val currentHttp4kRequest: Request? get() = requestThreadLocal.get()
+
+/**
+ * Stores the current http4k request on a thread local. The request can then be accessed with [currentHttp4kRequest]
+ */
+object StoreRequestOnThread {
+    operator fun invoke(): Filter {
+        return requestScopeValue(requestThreadLocal) { it }
+    }
+}
diff --git a/...n/invirt/http4k/filters/CatchAllFilter.kt → .../kotlin/invirt/http4k/filters/CatchAll.kt b/...n/invirt/http4k/filters/CatchAllFilter.kt → .../kotlin/invirt/http4k/filters/CatchAll.kt
@@ -8,7 +8,7 @@ import kotlin.reflect.KClass
 
 private val log = KotlinLogging.logger {}
 
-object CatchAllFilter {
+object CatchAll {
 
     operator fun invoke(vararg exceptionStatusMappings: Pair<KClass<out Throwable>, Status>): Filter {
         return invoke(exceptionStatusMappings.toMap())

diff --git a/...t/http4k/filters/DontCacheErrorsFilter.kt → .../invirt/http4k/filters/DontCacheErrors.kt b/...t/http4k/filters/DontCacheErrorsFilter.kt → .../invirt/http4k/filters/DontCacheErrors.kt
@@ -7,7 +7,7 @@ import org.http4k.core.noCache
 /**
  * Disable caching for certain error statuses
  */
-object DontCacheErrorsFilter {
+object DontCacheErrors {
 
     operator fun invoke(vararg errorStatuses: Status = arrayOf(Status.NOT_FOUND)): Filter {
         return invoke(errorStatuses.toSet())

diff --git a/...invirt/http4k/filters/ErrorPagesFilter.kt → ...otlin/invirt/http4k/filters/ErrorPages.kt b/...invirt/http4k/filters/ErrorPagesFilter.kt → ...otlin/invirt/http4k/filters/ErrorPages.kt
@@ -7,7 +7,7 @@ import org.http4k.core.Status
 /**
  * Renders a view based on a status code
  */
-object ErrorPagesFilter {
+object ErrorPages {
 
     operator fun invoke(vararg statusViewMappings: Pair<Status, String>): Filter {
         return invoke(statusViewMappings.toMap())

diff --git a/...irt/http4k/filters/HttpAccessLogFilter.kt → ...in/invirt/http4k/filters/HttpAccessLog.kt b/...irt/http4k/filters/HttpAccessLogFilter.kt → ...in/invirt/http4k/filters/HttpAccessLog.kt
@@ -5,7 +5,7 @@ import org.http4k.core.Filter
 import org.http4k.core.HttpTransaction
 import org.http4k.filter.ResponseFilters
 
-object HttpAccessLogFilter {
+object HttpAccessLog {
 
     internal val log = KotlinLogging.logger {}
 

diff --git a/...rt/http4k/filters/StatusOverrideFilter.kt → ...n/invirt/http4k/filters/StatusOverride.kt b/...rt/http4k/filters/StatusOverrideFilter.kt → ...n/invirt/http4k/filters/StatusOverride.kt
@@ -6,7 +6,7 @@ import org.http4k.core.Status
 
 private val log = KotlinLogging.logger {}
 
-object StatusOverrideFilter {
+object StatusOverride {
 
     operator fun invoke(vararg overrides: Pair<Status, Status>): Filter {
         return invoke(overrides.toMap())

diff --git a/invirt-http4k/src/main/kotlin/invirt/http4k/views/InvirtPebbleTemplates.kt b/invirt-http4k/src/main/kotlin/invirt/http4k/views/InvirtPebbleTemplates.kt
@@ -1,6 +1,6 @@
 package invirt.http4k.views
 
-import invirt.pebble.currentHttp4kRequest
+import invirt.http4k.currentHttp4kRequest
 import io.github.oshai.kotlinlogging.KotlinLogging
 import io.pebbletemplates.pebble.PebbleEngine
 import io.pebbletemplates.pebble.error.LoaderException

diff --git a/invirt-http4k/src/main/kotlin/invirt/pebble/InvirtPebbleExtension.kt b/invirt-http4k/src/main/kotlin/invirt/pebble/InvirtPebbleExtension.kt
@@ -1,6 +1,5 @@
 package invirt.pebble
 
-import invirt.http4k.requestScopeValue
 import invirt.pebble.filters.DateWithDaySuffixFilter
 import invirt.pebble.functions.*
 import invirt.utils.uuid7
@@ -9,8 +8,6 @@ import io.pebbletemplates.pebble.extension.Filter
 import io.pebbletemplates.pebble.extension.Function
 import java.time.LocalDate
 
-val invirtPebbleFilter: org.http4k.core.Filter = requestScopeValue(requestThreadLocal) { it }
-
 class InvirtPebbleExtension : AbstractExtension() {
 
     override fun getFunctions(): Map<String, Function> {

diff --git a/invirt-http4k/src/main/kotlin/invirt/pebble/RequestContext.kt b/invirt-http4k/src/main/kotlin/invirt/pebble/RequestContext.kt
diff --git a/invirt-http4k/src/main/kotlin/invirt/pebble/functions/uri.kt b/invirt-http4k/src/main/kotlin/invirt/pebble/functions/uri.kt
@@ -2,7 +2,6 @@ package invirt.pebble.functions
 
 import invirt.data.Page
 import invirt.http4k.*
-import invirt.pebble.currentHttp4kRequest
 
 val replaceQueryFunction = pebbleFunction("replaceQuery", "name", "value") {
     currentHttp4kRequest!!.uri.replaceQuery(args["name"] as String to args["value"]!!)

diff --git a/invirt-http4k/src/main/kotlin/invirt/pebble/functions/utils.kt b/invirt-http4k/src/main/kotlin/invirt/pebble/functions/utils.kt
@@ -1,7 +1,7 @@
 package invirt.pebble.functions
 
+import invirt.http4k.currentHttp4kRequest
 import invirt.http4k.views.validationErrorContextKey
-import invirt.pebble.currentHttp4kRequest
 import invirt.utils.minorUnitToString
 import java.util.*
 

diff --git a/invirt-http4k/src/test/kotlin/invirt/http4k/filters/CatchAllFilterTest.kt b/invirt-http4k/src/test/kotlin/invirt/http4k/filters/CatchAllFilterTest.kt
@@ -12,7 +12,7 @@ import org.http4k.routing.routes
 class CatchAllFilterTest : StringSpec({
 
     "exception to status" {
-        val handler = CatchAllFilter(
+        val handler = CatchAll(
             mapOf(
                 IllegalArgumentException::class to Status.BAD_REQUEST,
                 ClassCastException::class to Status.NOT_FOUND

diff --git a/invirt-http4k/src/test/kotlin/invirt/http4k/filters/DontCacheErrorsFilterTest.kt b/invirt-http4k/src/test/kotlin/invirt/http4k/filters/DontCacheErrorsFilterTest.kt
@@ -11,7 +11,7 @@ import org.http4k.routing.routes
 class DontCacheErrorsFilterTest : StringSpec({
 
     "errors don't allow cache" {
-        val handler = DontCacheErrorsFilter()
+        val handler = DontCacheErrors()
             .then(cacheOneYear())
             .then(routes("/test" GET { Response(Status.NOT_FOUND) }))
 
@@ -21,7 +21,7 @@ class DontCacheErrorsFilterTest : StringSpec({
     }
 
     "non-errors allow cache" {
-        val handler = DontCacheErrorsFilter()
+        val handler = DontCacheErrors()
             .then(cacheOneYear())
             .then(routes("/test" GET { Response(Status.OK) }))
 
@@ -31,7 +31,7 @@ class DontCacheErrorsFilterTest : StringSpec({
     }
 
     "custom statuses" {
-        val handler = DontCacheErrorsFilter(Status.INTERNAL_SERVER_ERROR)
+        val handler = DontCacheErrors(Status.INTERNAL_SERVER_ERROR)
             .then(cacheOneYear())
             .then(routes("/test" GET { Response(Status.INTERNAL_SERVER_ERROR) }))
 

diff --git a/invirt-http4k/src/test/kotlin/invirt/http4k/filters/ErrorPagesFilterTest.kt b/invirt-http4k/src/test/kotlin/invirt/http4k/filters/ErrorPagesFilterTest.kt
@@ -13,7 +13,7 @@ class ErrorPagesFilterTest : StringSpec({
     beforeAny { setDefaultViewLens(Views.Classpath("webapp/views")) }
 
     "404 page" {
-        val httpHandler = ErrorPagesFilter(Status.NOT_FOUND to "error/404")
+        val httpHandler = ErrorPages(Status.NOT_FOUND to "error/404")
             .then(
                 routes(
                     "/test" GET { Response(Status.NOT_FOUND) }
@@ -24,8 +24,8 @@ class ErrorPagesFilterTest : StringSpec({
     }
 
     "forbidden mapped to not found and custom error page" {
-        val httpHandler = ErrorPagesFilter(mapOf(Status.NOT_FOUND to "error/404"))
-            .then(StatusOverrideFilter(Status.FORBIDDEN to Status.NOT_FOUND))
+        val httpHandler = ErrorPages(mapOf(Status.NOT_FOUND to "error/404"))
+            .then(StatusOverride(Status.FORBIDDEN to Status.NOT_FOUND))
             .then(
                 routes(
                     "/test" GET { Response(Status.FORBIDDEN) }

diff --git a/invirt-http4k/src/test/kotlin/invirt/http4k/filters/HttpAccessLogFilterTest.kt b/invirt-http4k/src/test/kotlin/invirt/http4k/filters/HttpAccessLogFilterTest.kt
@@ -17,7 +17,7 @@ class HttpAccessLogFilterTest : StringSpec() {
     init {
 
         "defaults - errors only" {
-            val filter = HttpAccessLogFilter()
+            val filter = HttpAccessLog()
             testLogFilter(filter, Status.OK, false)
             testLogFilter(filter, Status.SEE_OTHER, false)
             testLogFilter(filter, Status.BAD_REQUEST, true)
@@ -28,7 +28,7 @@ class HttpAccessLogFilterTest : StringSpec() {
         }
 
         "all statuses" {
-            val filter = HttpAccessLogFilter(false)
+            val filter = HttpAccessLog(false)
             testLogFilter(filter, Status.OK, true)
             testLogFilter(filter, Status.SEE_OTHER, true)
             testLogFilter(filter, Status.BAD_REQUEST, true)
@@ -40,10 +40,10 @@ class HttpAccessLogFilterTest : StringSpec() {
     }
 
     private fun testLogFilter(filter: Filter, status: Status, expectCalled: Boolean) {
-        mockkObject(HttpAccessLogFilter) {
+        mockkObject(HttpAccessLog) {
             val log = mockk<KLogger>()
             every { log.atInfo(any<(KLoggingEventBuilder) -> Unit>()) } returns Unit
-            every { HttpAccessLogFilter.log } returns log
+            every { HttpAccessLog.log } returns log
 
             val httpHandler = filter.then(
                 routes(

diff --git a/invirt-http4k/src/test/kotlin/invirt/http4k/filters/StatusOverrideFilterTest.kt b/invirt-http4k/src/test/kotlin/invirt/http4k/filters/StatusOverrideFilterTest.kt
@@ -11,7 +11,7 @@ import org.http4k.kotest.shouldHaveStatus
 class StatusOverrideFilterTest : StringSpec({
 
     "forbidden as not found" {
-        val httpHandler = StatusOverrideFilter(Status.FORBIDDEN to Status.NOT_FOUND)
+        val httpHandler = StatusOverride(Status.FORBIDDEN to Status.NOT_FOUND)
             .then(
                 org.http4k.routing.routes(
                     "/test" GET { org.http4k.core.Response(Status.FORBIDDEN) }

diff --git a/invirt-http4k/src/test/kotlin/invirt/http4k/views/InvirtPebbleTemplatesTest.kt b/invirt-http4k/src/test/kotlin/invirt/http4k/views/InvirtPebbleTemplatesTest.kt
@@ -1,7 +1,7 @@
 package invirt.http4k.views
 
 import invirt.http4k.GET
-import invirt.pebble.invirtPebbleFilter
+import invirt.http4k.StoreRequestOnThread
 import io.kotest.core.spec.style.StringSpec
 import org.http4k.core.Method
 import org.http4k.core.Request
@@ -16,7 +16,7 @@ class InvirtPebbleTemplatesTest : StringSpec({
     beforeAny { setDefaultViewLens(Views.Classpath("webapp/views")) }
 
     "request object in pebble templates" {
-        val handler = invirtPebbleFilter.then(
+        val handler = StoreRequestOnThread().then(
             routes(
                 "/test" GET { renderTemplate("invirt-pebble-filter-request-object") }
             )

diff --git a/invirt-http4k/src/test/kotlin/invirt/pebble/PebbleFilterTest.kt b/invirt-http4k/src/test/kotlin/invirt/pebble/PebbleFilterTest.kt
@@ -1,6 +1,8 @@
 package invirt.pebble
 
 import invirt.http4k.GET
+import invirt.http4k.StoreRequestOnThread
+import invirt.http4k.currentHttp4kRequest
 import io.kotest.assertions.throwables.shouldThrowWithMessage
 import io.kotest.core.spec.style.StringSpec
 import io.kotest.matchers.shouldBe
@@ -12,7 +14,7 @@ class PebbleFilterTest : StringSpec({
 
     "request thread local" {
         lateinit var requestFromHandler: Request
-        val handler = invirtPebbleFilter.then(
+        val handler = StoreRequestOnThread().then(
             routes(
                 "/test" GET { request ->
                     requestFromHandler = currentHttp4kRequest!!
@@ -27,7 +29,7 @@ class PebbleFilterTest : StringSpec({
     }
 
     "request thread local cleared after request even when error occurs" {
-        val handler = invirtPebbleFilter
+        val handler = StoreRequestOnThread()
             .then(routes("/{value}" GET { throw IllegalStateException("Cannot proceed") }))
         shouldThrowWithMessage<IllegalStateException>("Cannot proceed") {
             handler(Request(Method.GET, "/test"))