Skip to content

Commit

Permalink
Web Actions Tab: Add null handling to Application/Network Interceptors
Browse files Browse the repository at this point in the history
* Application or Network Interceptors are appearing null when they lack a qualified class name
* New `ClassNameFormatter` that handles formatting a valid class name
  • Loading branch information
adrw committed Mar 12, 2019
1 parent 26ff62a commit d484dac
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 15 deletions.
31 changes: 16 additions & 15 deletions misk/src/main/kotlin/misk/web/actions/WebActionMetadataAction.kt
Expand Up @@ -7,6 +7,7 @@ import misk.web.NetworkInterceptor
import misk.web.PathPattern
import misk.web.RequestContentType
import misk.web.ResponseContentType
import misk.web.formatter.ClassNameFormatter
import misk.web.jetty.WebActionsServlet
import misk.web.mediatype.MediaRange
import misk.web.mediatype.MediaTypes
Expand Down Expand Up @@ -49,19 +50,19 @@ data class WebActionMetadata(
)

internal fun WebActionMetadata(
name: String,
function: Function<*>,
functionAnnotations: List<Annotation>,
acceptedMediaRanges: List<MediaRange>,
responseContentType: MediaType?,
parameterTypes: List<KType>,
returnType: KType,
pathPattern: PathPattern,
applicationInterceptors: List<ApplicationInterceptor>,
networkInterceptors: List<NetworkInterceptor>,
dispatchMechanism: DispatchMechanism,
allowedServices: Set<String>,
allowedRoles: Set<String>
name: String,
function: Function<*>,
functionAnnotations: List<Annotation>,
acceptedMediaRanges: List<MediaRange>,
responseContentType: MediaType?,
parameterTypes: List<KType>,
returnType: KType,
pathPattern: PathPattern,
applicationInterceptors: List<ApplicationInterceptor>,
networkInterceptors: List<NetworkInterceptor>,
dispatchMechanism: DispatchMechanism,
allowedServices: Set<String>,
allowedRoles: Set<String>
): WebActionMetadata {
return WebActionMetadata(
name = name,
Expand All @@ -72,8 +73,8 @@ internal fun WebActionMetadata(
parameterTypes = parameterTypes.map { it.toString() },
returnType = returnType.toString(),
pathPattern = pathPattern.toString(),
applicationInterceptors = applicationInterceptors.map { it::class.qualifiedName.toString() },
networkInterceptors = networkInterceptors.map { it::class.qualifiedName.toString() },
applicationInterceptors = applicationInterceptors.map { ClassNameFormatter().format(it::class) },
networkInterceptors = networkInterceptors.map { ClassNameFormatter().format(it::class) },
dispatchMechanism = dispatchMechanism,
allowedServices = allowedServices,
allowedRoles = allowedRoles
Expand Down
27 changes: 27 additions & 0 deletions misk/src/main/kotlin/misk/web/formatter/ClassNameFormatter.kt
@@ -0,0 +1,27 @@
package misk.web.formatter

import com.google.common.base.CharMatcher
import kotlin.reflect.KClass

class ClassNameFormatter {
fun <T : Any> format(kclass: KClass<T>): String {
return when (kclass.qualifiedName) {
null -> kclass.toString().split("class ").last()
else -> kclass.qualifiedName.toString()
}
}

fun isValid(className: String): Boolean {
require(className.isNotBlank()) {
"Formatted class name can not be blank"
}
require(className != "null") {
"Formatted class name can not be null"
}
require(
CharMatcher.inRange('A', 'Z').or(CharMatcher.inRange('a', 'z')).matchesAnyOf(className)) {
"Formatted class name must include alpha characters [A-Z, a-z]"
}
return true
}
}
3 changes: 3 additions & 0 deletions misk/src/test/kotlin/misk/web/actions/TestWebActionModule.kt
Expand Up @@ -11,6 +11,7 @@ import misk.web.Get
import misk.web.ResponseContentType
import misk.web.WebActionModule
import misk.web.WebTestingModule
import misk.web.formatter.ClassNameFormatter
import misk.web.mediatype.MediaTypes
import misk.web.toResponseBody
import javax.inject.Inject
Expand All @@ -29,6 +30,8 @@ class TestWebActionModule : KAbstractModule() {
multibind<AccessAnnotationEntry>().toInstance(
AccessAnnotationEntry<CustomRoleAccess>(roles = listOf("admin")))
multibind<MiskCallerAuthenticator>().to<FakeCallerAuthenticator>()

bind<ClassNameFormatter>().toInstance(ClassNameFormatter())
}

class CustomServiceAccessAction @Inject constructor() : WebAction {
Expand Down
Expand Up @@ -2,6 +2,7 @@ package misk.web.actions

import misk.testing.MiskTest
import misk.testing.MiskTestModule
import misk.web.formatter.ClassNameFormatter
import misk.web.mediatype.MediaTypes
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test
Expand All @@ -13,6 +14,7 @@ class WebActionMetadataActionTest {
val module = TestAdminDashboardActionModule()

@Inject lateinit var webActionMetadataAction: WebActionMetadataAction
@Inject lateinit var classNameFormatter: ClassNameFormatter

@Test fun webActionMetadata() {
val response = webActionMetadataAction.getAll()
Expand All @@ -27,6 +29,12 @@ class WebActionMetadataActionTest {
assertThat(customServiceActionMetadata.pathPattern).isEqualTo("/custom_service_access")
assertThat(customServiceActionMetadata.allowedServices).containsOnly("payments")
assertThat(customServiceActionMetadata.allowedRoles).isEmpty()
assertThat(
customServiceActionMetadata.applicationInterceptors.all {
classNameFormatter.isValid(it)
})
assertThat(
customServiceActionMetadata.networkInterceptors.all { classNameFormatter.isValid(it) })

val customRoleActionMetadata = response.webActionMetadata.find {
it.name.equals(TestWebActionModule.CustomRoleAccessAction::class.simpleName)
Expand All @@ -37,5 +45,9 @@ class WebActionMetadataActionTest {
assertThat(customRoleActionMetadata.pathPattern).isEqualTo("/custom_role_access")
assertThat(customRoleActionMetadata.allowedServices).isEmpty()
assertThat(customRoleActionMetadata.allowedRoles).containsOnly("admin")
assertThat(
customRoleActionMetadata.applicationInterceptors.all { classNameFormatter.isValid(it) })
assertThat(
customRoleActionMetadata.networkInterceptors.all { classNameFormatter.isValid(it) })
}
}
57 changes: 57 additions & 0 deletions misk/src/test/kotlin/misk/web/formatter/ClassNameFormatterTest.kt
@@ -0,0 +1,57 @@
package misk.web.formatter

import misk.inject.KAbstractModule
import misk.testing.MiskTest
import misk.testing.MiskTestModule
import misk.web.NetworkChain
import misk.web.NetworkInterceptor
import misk.web.Response
import org.junit.jupiter.api.Test
import javax.inject.Inject
import kotlin.test.assertEquals
import kotlin.test.assertNull

@MiskTest(startService = false)
class ClassNameFormatterTest {
@MiskTestModule
val module = ClassNameFormatterTestModule()

@Inject lateinit var classNameFormatter: ClassNameFormatter

@Test fun validQualifiedName() {
val formatted = ClassNameFormatter().format(ValidQualifiedNameClass::class)
classNameFormatter.isValid(formatted)
assertEquals(formatted, ValidQualifiedNameClass::class.qualifiedName.toString())
}

@Test fun missingQualifiedName() {
val noQualifiedNameClass = NoQualifiedNameFactory().create()
assertNull(noQualifiedNameClass::class.qualifiedName)

val formatted = ClassNameFormatter().format(noQualifiedNameClass::class)
classNameFormatter.isValid(formatted)
assertEquals(formatted, noQualifiedNameClass::class.toString().split("class ").last())
}
}

class ValidQualifiedNameClass {}

class NoQualifiedNameFactory {
fun create(): NetworkInterceptor {
return NoQualifiedNameClass
}

private companion object {
val NoQualifiedNameClass = object : NetworkInterceptor {
override fun intercept(chain: NetworkChain): Response<*> {
return Response("False")
}
}
}
}

class ClassNameFormatterTestModule : KAbstractModule() {
override fun configure() {
bind<ClassNameFormatter>().toInstance(ClassNameFormatter())
}
}

0 comments on commit d484dac

Please sign in to comment.