feat: introduce SiloNotImplementedError

The standard NotImplementedError from Kotlin does not work, because in spring Throwable has precedence over the NotImplementedError, and it is therefore never caught.
GenSpectrum · Apr 27, 2023 · 4f94a06 · 4f94a06
1 parent cbae1a6
commit 4f94a06
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 15 deletions.
diff --git a/lapis2/src/main/kotlin/org/genspectrum/lapis/controller/ExceptionHandler.kt b/lapis2/src/main/kotlin/org/genspectrum/lapis/controller/ExceptionHandler.kt
@@ -2,6 +2,7 @@ package org.genspectrum.lapis.controller
 
 import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
 import mu.KotlinLogging
+import org.genspectrum.lapis.model.SiloNotImplementedError
 import org.genspectrum.lapis.silo.SiloException
 import org.springframework.http.HttpStatus
 import org.springframework.http.MediaType
@@ -14,7 +15,6 @@ private val log = KotlinLogging.logger {}
 
 @ControllerAdvice
 class ExceptionHandler : ResponseEntityExceptionHandler() {
-
     @ExceptionHandler(Throwable::class)
     fun handleUnexpectedException(e: Throwable): ResponseEntity<String> {
         log.error(e) { "Caught unexpected exception: ${e.message}" }
@@ -66,8 +66,9 @@ class ExceptionHandler : ResponseEntityExceptionHandler() {
             )
     }
 
-    @ExceptionHandler(NotImplementedError::class)
-    fun handleNotImplementedError(e: NotImplementedError): ResponseEntity<String> {
+    @ExceptionHandler(SiloNotImplementedError::class)
+    fun handleNotImplementedError(e: SiloNotImplementedError): ResponseEntity<String> {
+        log.error(e) { "Caught SiloNotImplementedError: ${e.message}" }
         return ResponseEntity
             .status(HttpStatus.NOT_IMPLEMENTED)
             .contentType(MediaType.APPLICATION_JSON)

diff --git a/lapis2/src/main/kotlin/org/genspectrum/lapis/model/VariantQueryCustomListener.kt b/lapis2/src/main/kotlin/org/genspectrum/lapis/model/VariantQueryCustomListener.kt
@@ -90,26 +90,28 @@ class VariantQueryCustomListener : VariantQueryBaseListener(), ParseTreeListener
     }
 
     override fun enterNucleotideInsertionQuery(ctx: NucleotideInsertionQueryContext?) {
-        throw NotImplementedError("Nucleotide insertions are not supported yet.")
+        throw SiloNotImplementedError("Nucleotide insertions are not supported yet.", NotImplementedError())
     }
 
     override fun enterAaMutationQuery(ctx: AaMutationQueryContext?) {
-        throw NotImplementedError("Amino acid mutations are not supported yet.")
+        throw SiloNotImplementedError("Amino acid mutations are not supported yet.", NotImplementedError())
     }
 
     override fun enterAaInsertionQuery(ctx: AaInsertionQueryContext?) {
-        throw NotImplementedError("Amino acid insertions are not supported yet.")
+        throw SiloNotImplementedError("Amino acid insertions are not supported yet.", NotImplementedError())
     }
 
     override fun enterNextcladePangolineageQuery(ctx: NextcladePangolineageQueryContext?) {
-        throw NotImplementedError("Nextclade pango lineages are not supported yet.")
+        throw SiloNotImplementedError("Nextclade pango lineages are not supported yet.", NotImplementedError())
     }
 
     override fun enterNextstrainCladeQuery(ctx: NextstrainCladeQueryContext?) {
-        throw NotImplementedError("Nextstrain clade lineages are not supported yet.")
+        throw SiloNotImplementedError("Nextstrain clade lineages are not supported yet.", NotImplementedError())
     }
 
     override fun enterGisaidCladeLineageQuery(ctx: GisaidCladeLineageQueryContext?) {
-        throw NotImplementedError("Gisaid clade lineages are not supported yet.")
+        throw SiloNotImplementedError("Gisaid clade lineages are not supported yet.", NotImplementedError())
     }
 }
+
+class SiloNotImplementedError(message: String?, cause: Throwable?) : Exception(message, cause)
diff --git a/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/ExceptionHandlerTest.kt b/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/ExceptionHandlerTest.kt
@@ -4,6 +4,7 @@ import com.ninjasquad.springmockk.MockkBean
 import io.mockk.MockKAnnotations
 import io.mockk.MockKMatcherScope
 import io.mockk.every
+import org.genspectrum.lapis.model.SiloNotImplementedError
 import org.genspectrum.lapis.response.AggregatedResponse
 import org.genspectrum.lapis.silo.SiloException
 import org.junit.jupiter.api.BeforeEach
@@ -96,4 +97,23 @@ class ExceptionHandlerTest(@Autowired val mockMvc: MockMvc) {
                 ),
             )
     }
+
+    @Test
+    fun `throw NOT_IMPLEMENTED(501) with additional info for request of a not implemented resource in SILO`() {
+        every { validControllerCall() } throws SiloNotImplementedError("SomeMessage", Exception("SomeCause"))
+
+        mockMvc.perform(get(validRoute))
+            .andExpect(status().isNotImplemented)
+            .andExpect(content().contentType(MediaType.APPLICATION_JSON))
+            .andExpect(
+                content().json(
+                    """
+                    {
+                      "title":"Not implemented",
+                      "message":"SomeMessage"
+                    }
+                    """,
+                ),
+            )
+    }
 }
diff --git a/lapis2/src/test/kotlin/org/genspectrum/lapis/model/VariantQueryFacadeTest.kt b/lapis2/src/test/kotlin/org/genspectrum/lapis/model/VariantQueryFacadeTest.kt
@@ -238,7 +238,7 @@ class VariantQueryFacadeTest {
     fun `given a variantQuery with a 'Insertion' expression then map should throw an error`() {
         val variantQuery = "ins_1234:GAG"
 
-        val exception = assertThrows<NotImplementedError> { underTest.map(variantQuery) }
+        val exception = assertThrows<SiloNotImplementedError> { underTest.map(variantQuery) }
 
         MatcherAssert.assertThat(
             exception.message,
@@ -250,7 +250,7 @@ class VariantQueryFacadeTest {
     fun `given a variant variantQuery with a 'AA mutation' expression then map should throw an error`() {
         val variantQuery = "S:N501Y"
 
-        val exception = assertThrows<NotImplementedError> { underTest.map(variantQuery) }
+        val exception = assertThrows<SiloNotImplementedError> { underTest.map(variantQuery) }
 
         MatcherAssert.assertThat(
             exception.message,
@@ -262,7 +262,7 @@ class VariantQueryFacadeTest {
     fun `given a valid variantQuery with a 'AA insertion' expression then map should throw an error`() {
         val variantQuery = "ins_S:501:EPE"
 
-        val exception = assertThrows<NotImplementedError> { underTest.map(variantQuery) }
+        val exception = assertThrows<SiloNotImplementedError> { underTest.map(variantQuery) }
 
         MatcherAssert.assertThat(
             exception.message,
@@ -274,7 +274,7 @@ class VariantQueryFacadeTest {
     fun `given a valid variantQuery with a 'nextclade pango lineage' expression then map should throw an error`() {
         val variantQuery = "nextcladePangoLineage:BA.5*"
 
-        val exception = assertThrows<NotImplementedError> { underTest.map(variantQuery) }
+        val exception = assertThrows<SiloNotImplementedError> { underTest.map(variantQuery) }
 
         MatcherAssert.assertThat(
             exception.message,
@@ -286,7 +286,7 @@ class VariantQueryFacadeTest {
     fun `given a valid variantQuery with a 'Nextstrain clade lineage' expression then map should throw an error`() {
         val variantQuery = "nextstrainClade:22B"
 
-        val exception = assertThrows<NotImplementedError> { underTest.map(variantQuery) }
+        val exception = assertThrows<SiloNotImplementedError> { underTest.map(variantQuery) }
 
         MatcherAssert.assertThat(
             exception.message,
@@ -298,7 +298,7 @@ class VariantQueryFacadeTest {
     fun `given a valid variantQuery with a 'Gisaid clade lineage' expression then map should throw an error`() {
         val variantQuery = "gisaid:AB"
 
-        val exception = assertThrows<NotImplementedError> { underTest.map(variantQuery) }
+        val exception = assertThrows<SiloNotImplementedError> { underTest.map(variantQuery) }
 
         MatcherAssert.assertThat(
             exception.message,