Code review verbeteringen en documentatie updates (#24)

ericwout-overheid · claude · web-flow · commit 4094f9af50db · 2026-02-19T13:55:43.000+01:00
* Code review verbeteringen en documentatie updates - SQL injection preventie: table name validatie in ClickHouseRepository - Connection leak fix: shutdown() sluit ClickHouse client via repository.close() - Veilige cast naar ReadableSpan in LogboekInterceptor (as? i.p.v. as) - Lazy initialisatie van serviceName in ProcessingHandler - Provided scope voor jakarta.ws.rs-api en microprofile-config-api - All-open plugin uitgebreid met @interceptor annotatie - ClickHouse Docker image gepind naar versie 25 - README hernoemd naar JVM, Kotlin voorbeeld toegevoegd - Workflow hernoemd naar "CI with Maven" - Typo fix in ClickHouseSpanExporterTest Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Code review bevindingen verwerken - shutdown() vangt nu exceptions op en retourneert ofFailure() - Test verificatie toegevoegd voor repository.close() in shutdown - Parameterized tests voor table name validatie - ClickHouse image gepind naar versie 25.3 (LTS) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Tabelnaam als veld van ClickHouseRepository valideren bij initialisatie De tabelnaam werd bij elke insert en ensureSchema opnieuw uit de config gelezen en gevalideerd. Nu wordt de tabelnaam eenmalig bij constructie opgeslagen en gevalideerd. De table parameter is verwijderd uit insertJsonEachRow en de tableName is verwijderd uit ClickHouseSpanExporter. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml
@@ -6,7 +6,7 @@
 # separate terms of service, privacy policy, and support
 # documentation.
 
-name: Java CI with Maven
+name: CI with Maven
 
 permissions: read-all
 
diff --git a/README.md b/README.md
@@ -1,9 +1,9 @@
-# Logboek Dataverwerkingen Java implementatie
+# Logboek Dataverwerkingen JVM implementatie
 
 ![Project Pre-Alpha Status](https://img.shields.io/badge/life_cycle-pre_alpha-red)
 [![OpenSSF Scorecard](https://api.scorecard.dev/projects/github.com/MinBZK/moza-logboekdataverwerking/badge)](https://scorecard.dev/viewer/?uri=github.com/MinBZK/moza-logboekdataverwerking)
 
-Dit is een Kotlin implementatie van de - in ontwikkeling zijnde - standaard Logboek Dataverwerkingen (LDV) van Logius.
+Dit is een Kotlin implementatie van de - in ontwikkeling zijnde - standaard Logboek Dataverwerkingen (LDV) van Logius. De library is bruikbaar vanuit zowel Kotlin als Java projecten.
 
 ## Inleiding
 
@@ -22,7 +22,8 @@ Dit Open Source project is opgezet om de LDV standaard eenvoudig aan nieuwe of b
 
 Om deze package te gebruiken moet je in je (maven) project de volgende variablen in je `application.properties` file toevoegen:
 
-```
+```properties
+logboekdataverwerking.enabled=true
 logboekdataverwerking.service-name=service-name
 logboekdataverwerking.clickhouse.endpoint=http://localhost:8123
 logboekdataverwerking.clickhouse.username=user
@@ -33,8 +34,9 @@ logboekdataverwerking.clickhouse.table=table_name
 
 of `application.yml`:
 
-```
+```yaml
 logboekdataverwerking:
+    enabled: true
     service-name: service-name
     clickhouse:
         endpoint: http://localhost:8123
@@ -52,35 +54,63 @@ Hierbij is `name` de beschrijving van je eigen trace log en `processingActivityI
 
 Daarnaast kan er in de betreffende functie extra informatie aan de Span worden toegevoegd:
 
-```java
-
-    @Inject
-    ProcessingHandler handler;
-
-    @Inject
-    LogboekContext logboekContext;
-
-    @GET
-    @Path("/{identificatieType}/{identificatieNummer}")
-    @Logboek(name = "test", processingActivityId = "1")
-    public Response test() throws InterruptedException {
-        var innerSpan = handler.startSpan("span-2", null);
-        Thread.sleep(1000);
-        LogboekContext innerContext = new LogboekContext();
-        innerContext.setStatus(StatusCode.ERROR);
-        innerContext.setDataSubjectId("123");
-        innerContext.setDataSubjectType("BSN");
-        innerContext.setProcessingActivityId("4321");
-        innerContext.addLogboekContextToSpan(innerSpan);
-        innerSpan.end();
+**Kotlin:**
+```kotlin
+@Inject
+lateinit var handler: ProcessingHandler
+
+@Inject
+lateinit var logboekContext: LogboekContext
+
+@GET
+@Path("/{identificatieType}/{identificatieNummer}")
+@Logboek(name = "test", processingActivityId = "1")
+fun test(): Response {
+    val innerSpan = handler.startSpan("span-2", null)
+    val innerContext = LogboekContext().apply {
+        status = StatusCode.ERROR
+        dataSubjectId = "123"
+        dataSubjectType = "BSN"
+        processingActivityId = "4321"
+    }
+    handler.addLogboekContextToSpan(innerSpan, innerContext)
+    innerSpan.end()
 
-        logboekContext.setDataSubjectId("000000000");
-        logboekContext.setDataSubjectType("KVK");
-        logboekContext.setStatus(StatusCode.OK);
+    logboekContext.dataSubjectId = "000000000"
+    logboekContext.dataSubjectType = "KVK"
+    logboekContext.status = StatusCode.OK
 
+    return Response.ok("Hello world").build()
+}
+```
 
-        return Response.ok("Hello world").build();
-    }
+**Java:**
+```java
+@Inject
+ProcessingHandler handler;
+
+@Inject
+LogboekContext logboekContext;
+
+@GET
+@Path("/{identificatieType}/{identificatieNummer}")
+@Logboek(name = "test", processingActivityId = "1")
+public Response test() {
+    var innerSpan = handler.startSpan("span-2", null);
+    LogboekContext innerContext = new LogboekContext();
+    innerContext.setStatus(StatusCode.ERROR);
+    innerContext.setDataSubjectId("123");
+    innerContext.setDataSubjectType("BSN");
+    innerContext.setProcessingActivityId("4321");
+    handler.addLogboekContextToSpan(innerSpan, innerContext);
+    innerSpan.end();
+
+    logboekContext.setDataSubjectId("000000000");
+    logboekContext.setDataSubjectType("KVK");
+    logboekContext.setStatus(StatusCode.OK);
+
+    return Response.ok("Hello world").build();
+}
 ```
 ### Uitschakelen tijdens testen
 
diff --git a/compose.yml b/compose.yml
@@ -1,6 +1,6 @@
 services:
   clickhouse:
-    image: clickhouse/clickhouse-server:latest
+    image: clickhouse/clickhouse-server:25.3
     ports:
     - "8123:8123"
     environment:
diff --git a/pom.xml b/pom.xml
@@ -9,7 +9,7 @@
     <version>1.1.0-SNAPSHOT</version>
 
     <name>Logboek Dataverwerkingen Clickhouse</name>
-    <description>Java utilities to integrate the Logboek Dataverwerkingen (LDV) standard using a Clickhouse DB by Logius.</description>
+    <description>JVM utilities to integrate the Logboek Dataverwerkingen (LDV) standard using a Clickhouse DB by Logius.</description>
     <url>https://github.com/MinBZK/moza-logboekdataverwerking</url>
 
     <licenses>
@@ -120,6 +120,7 @@
             <groupId>jakarta.ws.rs</groupId>
             <artifactId>jakarta.ws.rs-api</artifactId>
             <version>4.0.0</version>
+            <scope>provided</scope>
         </dependency>
 
         <!-- https://mvnrepository.com/artifact/jakarta.servlet/jakarta.servlet-api -->
@@ -135,6 +136,7 @@
             <groupId>org.eclipse.microprofile.config</groupId>
             <artifactId>microprofile-config-api</artifactId>
             <version>3.1</version>
+            <scope>provided</scope>
         </dependency>
 
         <!-- Kotlin -->
@@ -287,6 +289,7 @@
                     <pluginOptions>
                         <option>all-open:annotation=jakarta.enterprise.context.ApplicationScoped</option>
                         <option>all-open:annotation=jakarta.enterprise.context.RequestScoped</option>
+                        <option>all-open:annotation=jakarta.interceptor.Interceptor</option>
                     </pluginOptions>
                 </configuration>
                 <dependencies>
diff --git a/src/main/kotlin/nl/mijnoverheidzakelijk/ldv/exporter/ClickHouseSpanExporter.kt b/src/main/kotlin/nl/mijnoverheidzakelijk/ldv/exporter/ClickHouseSpanExporter.kt
@@ -4,7 +4,6 @@ import com.fasterxml.jackson.databind.ObjectMapper
 import io.opentelemetry.sdk.common.CompletableResultCode
 import io.opentelemetry.sdk.trace.data.SpanData
 import io.opentelemetry.sdk.trace.export.SpanExporter
-import nl.mijnoverheidzakelijk.ldv.config.ConfigurationLoader
 import nl.mijnoverheidzakelijk.ldv.repository.ClickHouseRepository
 import java.util.concurrent.TimeUnit
 import java.util.logging.Level
@@ -20,7 +19,6 @@ import java.util.logging.Logger
  */
 class ClickHouseSpanExporter (
         private val repository: ClickHouseRepository = ClickHouseRepository(),
-        private val tableName: String = ConfigurationLoader.clickhouseTable,
         private val objectMapper: ObjectMapper = ObjectMapper()
     ) : SpanExporter {
 
@@ -51,7 +49,7 @@ class ClickHouseSpanExporter (
             }
 
             if (payload.isNotEmpty()) {
-                repository.insertJsonEachRow(tableName, payload.toString())
+                repository.insertJsonEachRow(payload.toString())
             }
         } catch (e: Exception) {
             LOGGER.log(Level.SEVERE, "Failed to insert spans into ClickHouse", e)
@@ -89,11 +87,19 @@ class ClickHouseSpanExporter (
     override fun flush(): CompletableResultCode = CompletableResultCode.ofSuccess()
 
     /**
-     * Shuts down the exporter. No resources to free, returns success.
-     * 
+     * Shuts down the exporter and closes the underlying ClickHouse client.
+     *
      * @return success result code
      */
-    override fun shutdown(): CompletableResultCode = CompletableResultCode.ofSuccess()
+    override fun shutdown(): CompletableResultCode {
+        return try {
+            repository.close()
+            CompletableResultCode.ofSuccess()
+        } catch (e: Exception) {
+            LOGGER.log(Level.SEVERE, "Failed to close ClickHouse client", e)
+            CompletableResultCode.ofFailure()
+        }
+    }
 
     companion object {
         private val LOGGER: Logger = Logger.getLogger(ClickHouseSpanExporter::class.java.getName())
diff --git a/src/main/kotlin/nl/mijnoverheidzakelijk/ldv/logboekdataverwerking/LogboekInterceptor.kt b/src/main/kotlin/nl/mijnoverheidzakelijk/ldv/logboekdataverwerking/LogboekInterceptor.kt
@@ -65,10 +65,10 @@ class LogboekInterceptor {
             span.setStatus(StatusCode.ERROR)
             throw e
         } finally {
-            val spanData = (span as ReadableSpan).toSpanData()
-
             if (headers.getHeaderString("traceparent") != null) {
-                span.setAttribute("dpl.core.foreign_operation.span_id", spanData.parentSpanId)
+                (span as? ReadableSpan)?.toSpanData()?.let { spanData ->
+                    span.setAttribute("dpl.core.foreign_operation.span_id", spanData.parentSpanId)
+                }
 
                 //todo hoe krijgen we de url, bijv. header. Hier is het team van LDV nog mee bezig.
                 //todo How do we get the url, ex. header. This is still being worked on by the LDV team.
diff --git a/src/main/kotlin/nl/mijnoverheidzakelijk/ldv/logboekdataverwerking/ProcessingHandler.kt b/src/main/kotlin/nl/mijnoverheidzakelijk/ldv/logboekdataverwerking/ProcessingHandler.kt
@@ -70,7 +70,7 @@ class ProcessingHandler {
                 _openTelemetry = value
             }
 
-        val serviceName: String = ConfigurationLoader.serviceName
+        val serviceName: String by lazy { ConfigurationLoader.serviceName }
 
         /**
          * Initializes and returns the global [OpenTelemetry] instance.
diff --git a/src/main/kotlin/nl/mijnoverheidzakelijk/ldv/repository/ClickHouseRepository.kt b/src/main/kotlin/nl/mijnoverheidzakelijk/ldv/repository/ClickHouseRepository.kt
@@ -13,22 +13,26 @@ import java.util.concurrent.TimeUnit
  * Repository encapsulating basic ClickHouse operations used by the exporter.
  */
 class ClickHouseRepository {
+    private val table: String = ConfigurationLoader.clickhouseTable
     private val client: Client = Client.Builder()
         .addEndpoint(ConfigurationLoader.clickhouseEndpoint)
         .setUsername(ConfigurationLoader.clickhouseUsername)
         .setPassword(ConfigurationLoader.clickhousePassword)
         .setDefaultDatabase(ConfigurationLoader.clickhouseDatabase)
         .build()
 
+    init {
+        requireValidTableName(table)
+    }
+
     /**
      * Ensures that the target table exists with the expected schema.
-     * 
+     *
      * @throws ConfigurationException if the table name cannot be resolved
      * @throws RuntimeException       if the DDL operation fails
      */
     @Throws(ConfigurationException::class)
     fun ensureSchema() {
-        val table = ConfigurationLoader.clickhouseTable
         try {
             // Schema matching SpanData structure (camelCase)
             client.query(
@@ -54,18 +58,31 @@ class ClickHouseRepository {
     }
 
     /**
-     * Inserts a JSON payload into the specified table.
+     * Inserts a JSON payload into the configured table.
      *
-     * @param table              the target table name
      * @param jsonEachRowPayload payload where each line is a JSON object
      * @throws RuntimeException if the insert fails
      */
-    fun insertJsonEachRow(table: String, jsonEachRowPayload: String) {
+    fun insertJsonEachRow(jsonEachRowPayload: String) {
         try {
             val data: InputStream = ByteArrayInputStream(jsonEachRowPayload.toByteArray(StandardCharsets.UTF_8))
             client.insert(table, data, ClickHouseFormat.JSONEachRow).get()
         } catch (e: Exception) {
             throw RuntimeException("Failed to insert into ClickHouse", e)
         }
     }
+
+    fun close() {
+        client.close()
+    }
+
+    companion object {
+        private val TABLE_NAME_PATTERN = Regex("^[a-zA-Z_][a-zA-Z0-9_.]*$")
+
+        private fun requireValidTableName(table: String) {
+            require(TABLE_NAME_PATTERN.matches(table)) {
+                "Invalid table name: $table"
+            }
+        }
+    }
 }
diff --git a/src/test/kotlin/nl/mijnoverheidzakelijk/ldv/exporter/ClickHouseSpanExporterTest.kt b/src/test/kotlin/nl/mijnoverheidzakelijk/ldv/exporter/ClickHouseSpanExporterTest.kt
diff --git a/src/test/kotlin/nl/mijnoverheidzakelijk/ldv/repository/ClickHouseRepositoryTest.kt b/src/test/kotlin/nl/mijnoverheidzakelijk/ldv/repository/ClickHouseRepositoryTest.kt

Original file line number	Diff line number	Diff line change
`@@ -70,7 +70,7 @@ class ProcessingHandler {`
`70`	`70`	`_openTelemetry = value`
`71`	`71`	`}`
`72`	`72`
`73`		`- val serviceName: String = ConfigurationLoader.serviceName`
	`73`	`+ val serviceName: String by lazy { ConfigurationLoader.serviceName }`
`74`	`74`
`75`	`75`	`/**`
`76`	`76`	`* Initializes and returns the global [OpenTelemetry] instance.`