Add body check

comet-ml · Sep 26, 2024 · 414e280 · 414e280
1 parent 49a4449
commit 414e280
Show file tree

Hide file tree

Showing 6 changed files with 48 additions and 34 deletions.
diff --git a/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/OpikMetadataConfig.java b/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/OpikMetadataConfig.java
@@ -8,15 +8,14 @@
 @Data
 public class OpikMetadataConfig {
 
-    public record UsageReport(@Valid @JsonProperty boolean enabled, @Valid @JsonProperty String url) {}
+    public record UsageReport(@Valid @JsonProperty boolean enabled, @Valid @JsonProperty String url) {
+    }
 
     @Valid
     @JsonProperty
-    @NotNull
-    private String version;
+    @NotNull private String version;
 
     @Valid
-    @NotNull
-    @JsonProperty
+    @NotNull @JsonProperty
     private UsageReport usageReport;
 }
diff --git a/...ik-backend/src/main/java/com/comet/opik/infrastructure/bi/ApplicationStartupListener.java b/...ik-backend/src/main/java/com/comet/opik/infrastructure/bi/ApplicationStartupListener.java
@@ -4,7 +4,6 @@
 import com.comet.opik.infrastructure.OpikConfiguration;
 import com.comet.opik.infrastructure.lock.LockService;
 import com.google.inject.Injector;
-import jakarta.inject.Singleton;
 import jakarta.ws.rs.client.Client;
 import jakarta.ws.rs.client.ClientBuilder;
 import jakarta.ws.rs.client.Entity;
@@ -27,7 +26,6 @@
 import static com.comet.opik.infrastructure.lock.LockService.Lock;
 
 @Slf4j
-@Singleton
 @RequiredArgsConstructor
 public class ApplicationStartupListener implements GuiceyLifecycleListener {
 
@@ -73,7 +71,8 @@ public void onEvent(GuiceyLifecycleEvent event) {
         }
     }
 
-    private Mono<Void> tryToReportStartupEvent(UsageReportDAO usageReport, IdGenerator generator, String eventType, OpikConfiguration config) {
+    private Mono<Void> tryToReportStartupEvent(UsageReportDAO usageReport, IdGenerator generator, String eventType,
+            OpikConfiguration config) {
         return Mono.fromCallable(() -> {
 
             String anonymousId = getAnonymousId(usageReport, generator);
@@ -107,15 +106,15 @@ private String getAnonymousId(UsageReportDAO usageReport, IdGenerator generator)
         return anonymousId.get();
     }
 
-    private void reportEvent(String anonymousId, String eventType, OpikConfiguration config, UsageReportDAO usageReport) {
+    private void reportEvent(String anonymousId, String eventType, OpikConfiguration config,
+            UsageReportDAO usageReport) {
 
         usageReport.addEvent(eventType);
 
         var startupEvent = new OpikStartupEvent(
-            anonymousId,
-            eventType,
-            Map.of("opik_app_version", config.getMetadata().getVersion())
-        );
+                anonymousId,
+                eventType,
+                Map.of("opik_app_version", config.getMetadata().getVersion()));
 
         try (Response response = client.target(URI.create(config.getMetadata().getUsageReport().url()))
                 .request()

diff --git a/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/bi/UsageReportDAO.java b/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/bi/UsageReportDAO.java
@@ -48,17 +48,19 @@ public void saveAnonymousId(@NonNull String id) {
     }
 
     public boolean isEventReported(@NonNull String eventType) {
-        return jdbi.inTransaction(handle -> handle.createQuery("SELECT COUNT(*) > 0 FROM usage_information WHERE event_type = :eventType AND reported_at IS NOT NULL")
+        return jdbi.inTransaction(handle -> handle.createQuery(
+                "SELECT COUNT(*) > 0 FROM usage_information WHERE event_type = :eventType AND reported_at IS NOT NULL")
                 .bind("eventType", eventType)
                 .mapTo(Boolean.class)
                 .one());
     }
 
     public void addEvent(@NonNull String eventType) {
         try {
-            jdbi.useHandle(handle -> handle.createUpdate("INSERT INTO usage_information (event_type) VALUES (:eventType)")
-                    .bind("eventType",eventType)
-                    .execute());
+            jdbi.useHandle(
+                    handle -> handle.createUpdate("INSERT INTO usage_information (event_type) VALUES (:eventType)")
+                            .bind("eventType", eventType)
+                            .execute());
         } catch (UnableToExecuteStatementException e) {
             if (e.getCause() instanceof SQLIntegrityConstraintViolationException) {
                 log.warn("Event type already exists: {}", eventType);
@@ -69,7 +71,9 @@ public void addEvent(@NonNull String eventType) {
     }
 
     public void markEventAsReported(@NonNull String eventType) {
-        jdbi.useHandle(handle -> handle.createUpdate("UPDATE usage_information SET reported_at = current_timestamp(6) WHERE event_type = :eventType")
+        jdbi.useHandle(handle -> handle
+                .createUpdate(
+                        "UPDATE usage_information SET reported_at = current_timestamp(6) WHERE event_type = :eventType")
                 .bind("eventType", eventType)
                 .execute());
     }

diff --git a/...end/src/test/java/com/comet/opik/api/resources/utils/TestDropwizardAppExtensionUtils.java b/...end/src/test/java/com/comet/opik/api/resources/utils/TestDropwizardAppExtensionUtils.java
@@ -37,7 +37,8 @@ public record AppContextConfig(
             String jdbcDriverClass,
             String awsJdbcDriverPlugins,
             boolean usageReportEnabled,
-            String usageReportUrl) {
+            String usageReportUrl,
+            String metadataVersion) {
     }
 
     public static TestDropwizardAppExtension newTestDropwizardAppExtension(String jdbcUrl,
@@ -153,6 +154,10 @@ public void run(GuiceyEnvironment environment) {
             }
         }
 
+        if (appContextConfig.metadataVersion() != null) {
+            list.add("metadata.version: %s".formatted(appContextConfig.metadataVersion()));
+        }
+
         if (appContextConfig.usageReportEnabled()) {
             list.add("metadata.usageReport.enabled: true");
 

diff --git a/.../opik-backend/src/test/java/com/comet/opik/infrastructure/aws/rds/MysqlRdsIamE2eTest.java b/.../opik-backend/src/test/java/com/comet/opik/infrastructure/aws/rds/MysqlRdsIamE2eTest.java
@@ -1,18 +1,15 @@
 package com.comet.opik.infrastructure.aws.rds;
 
 import com.comet.opik.api.Project;
-import com.comet.opik.api.resources.utils.AuthTestUtils;
 import com.comet.opik.api.resources.utils.ClickHouseContainerUtils;
 import com.comet.opik.api.resources.utils.ClientSupportUtils;
 import com.comet.opik.api.resources.utils.MigrationUtils;
 import com.comet.opik.api.resources.utils.MySQLContainerUtils;
 import com.comet.opik.api.resources.utils.RedisContainerUtils;
 import com.comet.opik.api.resources.utils.TestDropwizardAppExtensionUtils;
 import com.comet.opik.api.resources.utils.TestDropwizardAppExtensionUtils.AppContextConfig;
-import com.github.tomakehurst.wiremock.client.WireMock;
 import com.redis.testcontainers.RedisContainer;
 import jakarta.ws.rs.client.Entity;
-import jakarta.ws.rs.core.HttpHeaders;
 import jakarta.ws.rs.core.Response;
 import org.jdbi.v3.core.Jdbi;
 import org.junit.jupiter.api.BeforeAll;
@@ -22,12 +19,10 @@
 import org.junit.jupiter.api.extension.RegisterExtension;
 import org.testcontainers.containers.ClickHouseContainer;
 import org.testcontainers.junit.jupiter.Testcontainers;
-import reactor.core.publisher.Mono;
 import ru.vyarus.dropwizard.guice.test.ClientSupport;
 import ru.vyarus.dropwizard.guice.test.jupiter.ext.TestDropwizardAppExtension;
 
 import java.sql.SQLException;
-import java.time.Duration;
 import java.util.UUID;
 
 import static com.comet.opik.infrastructure.auth.RequestContext.WORKSPACE_HEADER;
@@ -38,7 +33,6 @@
 @TestInstance(TestInstance.Lifecycle.PER_CLASS)
 public class MysqlRdsIamE2eTest {
 
-
     private static final String URL_TEMPLATE = "%s/v1/private/projects";
 
     /// See PR: https://github.com/comet-ml/opik/pull/306
@@ -110,5 +104,4 @@ void testAwsRds__whenRdsIamDbAuthenticationIsEnabled__shouldAcceptRequest() {
         }
     }
 
-
 }
diff --git a/...ackend/src/test/java/com/comet/opik/infrastructure/bi/ApplicationStartupListenerTest.java b/...ackend/src/test/java/com/comet/opik/infrastructure/bi/ApplicationStartupListenerTest.java
@@ -24,10 +24,13 @@
 import ru.vyarus.dropwizard.guice.test.jupiter.ext.TestDropwizardAppExtension;
 
 import java.sql.SQLException;
+import java.util.Random;
 
 import static com.comet.opik.api.resources.utils.ClickHouseContainerUtils.DATABASE_NAME;
 import static com.comet.opik.api.resources.utils.MigrationUtils.CLICKHOUSE_CHANGELOG_FILE;
 import static com.github.tomakehurst.wiremock.client.WireMock.exactly;
+import static com.github.tomakehurst.wiremock.client.WireMock.matching;
+import static com.github.tomakehurst.wiremock.client.WireMock.matchingJsonPath;
 import static com.github.tomakehurst.wiremock.client.WireMock.post;
 import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor;
 import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo;
@@ -43,7 +46,11 @@ class ApplicationStartupListenerTest {
     class FirstStartupTest {
 
         private static final RedisContainer REDIS = RedisContainerUtils.newRedisContainer();
-        private static final ClickHouseContainer CLICK_HOUSE_CONTAINER = ClickHouseContainerUtils.newClickHouseContainer();
+        private static final ClickHouseContainer CLICK_HOUSE_CONTAINER = ClickHouseContainerUtils
+                .newClickHouseContainer();
+        private static final Random RANDOM = new Random();
+        private static final String VERSION = "%s.%s.%s".formatted(RANDOM.nextInt(10), RANDOM.nextInt(),
+                RANDOM.nextInt(99));
 
         @RegisterExtension
         private static final TestDropwizardAppExtension app;
@@ -61,7 +68,8 @@ class FirstStartupTest {
                     CLICK_HOUSE_CONTAINER, DATABASE_NAME);
 
             try {
-                MigrationUtils.runDbMigration(MYSQL_CONTAINER.createConnection(""), MySQLContainerUtils.migrationParameters());
+                MigrationUtils.runDbMigration(MYSQL_CONTAINER.createConnection(""),
+                        MySQLContainerUtils.migrationParameters());
             } catch (SQLException e) {
                 throw new RuntimeException(e);
             }
@@ -75,8 +83,12 @@ class FirstStartupTest {
 
             wireMock.server().stubFor(
                     post(urlPathEqualTo("/v1/notify/event"))
-                            .willReturn(WireMock.aResponse().withStatus(200).withBody("{ \"result\":\"OK\" }"))
-            );
+                            .withRequestBody(matchingJsonPath("$.anonymous_id", matching(
+                                    "^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$")))
+                            .withRequestBody(matchingJsonPath("$.event_type",
+                                    matching(GuiceyLifecycle.ApplicationStarted.name())))
+                            .withRequestBody(matchingJsonPath("$.event_properties.opik_app_version", matching(VERSION)))
+                            .willReturn(WireMock.aResponse().withStatus(200).withBody("{ \"result\":\"OK\" }")));
 
             app = TestDropwizardAppExtensionUtils.newTestDropwizardAppExtension(
                     AppContextConfig.builder()
@@ -85,6 +97,7 @@ class FirstStartupTest {
                             .redisUrl(REDIS.getRedisURI())
                             .usageReportEnabled(true)
                             .usageReportUrl("%s/v1/notify/event".formatted(wireMock.runtimeInfo().getHttpBaseUrl()))
+                            .metadataVersion(VERSION)
                             .build());
         }
 
@@ -103,7 +116,8 @@ void shouldNotifyEvent(UsageReportDAO reportDAO) {
     class SecondStartupTest {
 
         private static final RedisContainer REDIS = RedisContainerUtils.newRedisContainer();
-        private static final ClickHouseContainer CLICK_HOUSE_CONTAINER = ClickHouseContainerUtils.newClickHouseContainer();
+        private static final ClickHouseContainer CLICK_HOUSE_CONTAINER = ClickHouseContainerUtils
+                .newClickHouseContainer();
 
         @RegisterExtension
         private static final TestDropwizardAppExtension app;
@@ -121,7 +135,8 @@ class SecondStartupTest {
                     CLICK_HOUSE_CONTAINER, DATABASE_NAME);
 
             try {
-                MigrationUtils.runDbMigration(MYSQL_CONTAINER.createConnection(""), MySQLContainerUtils.migrationParameters());
+                MigrationUtils.runDbMigration(MYSQL_CONTAINER.createConnection(""),
+                        MySQLContainerUtils.migrationParameters());
             } catch (SQLException e) {
                 throw new RuntimeException(e);
             }
@@ -137,8 +152,7 @@ class SecondStartupTest {
 
             wireMock.server().stubFor(
                     post(urlPathEqualTo("/v1/notify/event"))
-                            .willReturn(WireMock.aResponse().withStatus(200).withBody("{ \"result\":\"OK\" }"))
-            );
+                            .willReturn(WireMock.aResponse().withStatus(200).withBody("{ \"result\":\"OK\" }")));
 
             app = TestDropwizardAppExtensionUtils.newTestDropwizardAppExtension(
                     AppContextConfig.builder()