OPIK-647 Return 409 instead of throwing exception for workspaceId or …

…project mismatch (#977)

apps/opik-backend/src/main/java/com/comet/opik/domain/SpanService.java

@@ -45,7 +45,7 @@ public class SpanService {
     public static final String PARENT_SPAN_IS_MISMATCH = "parent_span_id does not match the existing span";
     public static final String TRACE_ID_MISMATCH = "trace_id does not match the existing span";
     public static final String SPAN_KEY = "Span";
-    public static final String PROJECT_NAME_MISMATCH = "Project name and workspace name do not match the existing span";
+    public static final String PROJECT_AND_WORKSPACE_NAME_MISMATCH = "Project name and workspace name do not match the existing span";
 
     private final @NonNull SpanDAO spanDAO;
     private final @NonNull ProjectService projectService;
@@ -124,7 +124,7 @@ private Mono<UUID> insertSpan(Span span, Project project, UUID id, Span existing
             }
 
             if (!project.id().equals(existingSpan.projectId())) {
-                return failWithConflict(PROJECT_NAME_MISMATCH);
+                return failWithConflict(PROJECT_AND_WORKSPACE_NAME_MISMATCH);
             }
 
             if (!Objects.equals(span.parentSpanId(), existingSpan.parentSpanId())) {
@@ -198,10 +198,9 @@ private Mono<Project> getProjectById(SpanUpdate spanUpdate) {
     private <T> Mono<T> handleSpanDBError(Throwable ex) {
         if (ex instanceof ClickHouseException
                 && ex.getMessage().contains("TOO_LARGE_STRING_SIZE")
-                && (ex.getMessage().contains("_CAST(project_id, FixedString(36))")
-                        || ex.getMessage()
-                                .contains(", CAST(leftPad(workspace_id, 40, '*'), 'FixedString(19)') ::"))) {
-            return failWithConflict(PROJECT_NAME_MISMATCH);
+                && ex.getMessage().contains("String too long for type FixedString")
+                && (ex.getMessage().contains("project_id") || ex.getMessage().contains("workspace_id"))) {
+            return failWithConflict(PROJECT_AND_WORKSPACE_NAME_MISMATCH);
         }
 
         if (ex instanceof ClickHouseException
@@ -224,7 +223,7 @@ private <T> Mono<T> handleSpanDBError(Throwable ex) {
 
     private Mono<Long> updateOrFail(SpanUpdate spanUpdate, UUID id, Span existingSpan, Project project) {
         if (!project.id().equals(existingSpan.projectId())) {
-            return failWithConflict(PROJECT_NAME_MISMATCH);
+            return failWithConflict(PROJECT_AND_WORKSPACE_NAME_MISMATCH);
         }
 
         if (!Objects.equals(existingSpan.parentSpanId(), spanUpdate.parentSpanId())) {

apps/opik-backend/src/main/java/com/comet/opik/domain/TraceService.java

@@ -48,6 +48,8 @@
 @ImplementedBy(TraceServiceImpl.class)
 public interface TraceService {
 
+    String PROJECT_NAME_AND_WORKSPACE_NAME_MISMATCH = "Project name and workspace name do not match the existing trace";
+
     Mono<UUID> create(Trace trace);
 
     Mono<Long> create(TraceBatch batch);
@@ -80,7 +82,6 @@ public interface TraceService {
 @RequiredArgsConstructor(onConstructor_ = @Inject)
 class TraceServiceImpl implements TraceService {
 
-    public static final String PROJECT_NAME_AND_WORKSPACE_NAME_MISMATCH = "Project name and workspace name do not match the existing trace";
     public static final String TRACE_KEY = "Trace";
 
     private final @NonNull TraceDAO dao;
@@ -168,8 +169,8 @@ private Mono<UUID> insertTrace(Trace newTrace, Project project, UUID id) {
     private <T> Mono<T> handleDBError(Throwable ex) {
         if (ex instanceof ClickHouseException
                 && ex.getMessage().contains("TOO_LARGE_STRING_SIZE")
-                && (ex.getMessage().contains("_CAST(project_id, FixedString(36))")
-                        && ex.getMessage().contains(", CAST(leftPad(workspace_id, 40, '*'), 'FixedString(19)') ::"))) {
+                && ex.getMessage().contains("String too long for type FixedString")
+                && (ex.getMessage().contains("project_id") || ex.getMessage().contains("workspace_id"))) {
 
             return failWithConflict(PROJECT_NAME_AND_WORKSPACE_NAME_MISMATCH);
         }

apps/opik-backend/src/test/java/com/comet/opik/api/resources/utils/resources/SpanResourceClient.java

@@ -10,6 +10,7 @@
 import jakarta.ws.rs.client.Entity;
 import jakarta.ws.rs.core.HttpHeaders;
 import jakarta.ws.rs.core.MediaType;
+import jakarta.ws.rs.core.Response;
 import lombok.RequiredArgsConstructor;
 import org.apache.http.HttpStatus;
 import ru.vyarus.dropwizard.guice.test.ClientSupport;
@@ -31,14 +32,7 @@ public class SpanResourceClient {
     private final String baseURI;
 
     public UUID createSpan(Span span, String apiKey, String workspaceName) {
-        try (var response = client.target(RESOURCE_PATH.formatted(baseURI))
-                .request()
-                .accept(MediaType.APPLICATION_JSON_TYPE)
-                .header(HttpHeaders.AUTHORIZATION, apiKey)
-                .header(WORKSPACE_HEADER, workspaceName)
-                .post(Entity.json(span))) {
-
-            assertThat(response.getStatus()).isEqualTo(HttpStatus.SC_CREATED);
+        try (var response = createSpan(span, apiKey, workspaceName, HttpStatus.SC_CREATED)) {
 
             var actualId = TestUtils.getIdFromLocation(response.getLocation());
 
@@ -50,6 +44,19 @@ public UUID createSpan(Span span, String apiKey, String workspaceName) {
         }
     }
 
+    public Response createSpan(Span span, String apiKey, String workspaceName, int expectedStatus) {
+        var response = client.target(RESOURCE_PATH.formatted(baseURI))
+                .request()
+                .accept(MediaType.APPLICATION_JSON_TYPE)
+                .header(HttpHeaders.AUTHORIZATION, apiKey)
+                .header(WORKSPACE_HEADER, workspaceName)
+                .post(Entity.json(span));
+
+        assertThat(response.getStatus()).isEqualTo(expectedStatus);
+
+        return response;
+    }
+
     public void feedbackScores(List<FeedbackScoreBatchItem> score, String apiKey, String workspaceName) {
 
         try (var response = client.target(RESOURCE_PATH.formatted(baseURI))

apps/opik-backend/src/test/java/com/comet/opik/api/resources/utils/resources/TraceResourceClient.java

@@ -14,6 +14,7 @@
 import jakarta.ws.rs.client.Entity;
 import jakarta.ws.rs.core.HttpHeaders;
 import jakarta.ws.rs.core.MediaType;
+import jakarta.ws.rs.core.Response;
 import lombok.RequiredArgsConstructor;
 import org.apache.http.HttpStatus;
 import ru.vyarus.dropwizard.guice.test.ClientSupport;
@@ -36,16 +37,8 @@ public class TraceResourceClient {
     private final PodamFactory podamFactory = PodamFactoryUtils.newPodamFactory();
 
     public UUID createTrace(Trace trace, String apiKey, String workspaceName) {
-        try (var response = client.target(RESOURCE_PATH.formatted(baseURI))
-                .request()
-                .accept(MediaType.APPLICATION_JSON_TYPE)
-                .header(HttpHeaders.AUTHORIZATION, apiKey)
-                .header(WORKSPACE_HEADER, workspaceName)
-                .post(Entity.json(trace))) {
-
-            assertThat(response.getStatus()).isEqualTo(HttpStatus.SC_CREATED);
-
-            var actualId = TestUtils.getIdFromLocation(response.getLocation());
+        try (var response = createTrace(trace, apiKey, workspaceName, HttpStatus.SC_CREATED)) {
+            UUID actualId = TestUtils.getIdFromLocation(response.getLocation());
 
             if (trace.id() != null) {
                 assertThat(actualId).isEqualTo(trace.id());
@@ -55,6 +48,19 @@ public UUID createTrace(Trace trace, String apiKey, String workspaceName) {
         }
     }
 
+    public Response createTrace(Trace trace, String apiKey, String workspaceName, int expectedStatus) {
+        var response = client.target(RESOURCE_PATH.formatted(baseURI))
+                .request()
+                .accept(MediaType.APPLICATION_JSON_TYPE)
+                .header(HttpHeaders.AUTHORIZATION, apiKey)
+                .header(WORKSPACE_HEADER, workspaceName)
+                .post(Entity.json(trace));
+
+        assertThat(response.getStatus()).isEqualTo(expectedStatus);
+
+        return response;
+    }
+
     public void feedbackScores(List<FeedbackScoreBatchItem> score, String apiKey, String workspaceName) {
 
         try (var response = client.target(RESOURCE_PATH.formatted(baseURI))

apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/SpansResourceTest.java

@@ -103,6 +103,7 @@
 import static com.comet.opik.api.resources.utils.MigrationUtils.CLICKHOUSE_CHANGELOG_FILE;
 import static com.comet.opik.api.resources.utils.StatsUtils.getProjectSpanStatItems;
 import static com.comet.opik.domain.ProjectService.DEFAULT_PROJECT;
+import static com.comet.opik.domain.SpanService.PROJECT_AND_WORKSPACE_NAME_MISMATCH;
 import static com.comet.opik.infrastructure.auth.RequestContext.SESSION_COOKIE;
 import static com.comet.opik.infrastructure.auth.RequestContext.WORKSPACE_HEADER;
 import static com.comet.opik.infrastructure.auth.TestHttpClientUtils.UNAUTHORIZED_RESPONSE;
@@ -3425,6 +3426,12 @@ private void createAndAssert(UUID entityId, FeedbackScore score, String workspac
         }
     }
 
+    private void createAndAssertErrorMessage(Span span, String apiKey, String workspaceName, int status, String errorMessage) {
+        try (var response = spanResourceClient.createSpan(span, apiKey, workspaceName, status)) {
+            assertThat(response.readEntity(ErrorMessage.class).errors().getFirst()).isEqualTo(errorMessage);
+        }
+    }
+
     @Test
     void createAndGetById() {
         var expectedSpan = podamFactory.manufacturePojo(Span.class).toBuilder()
@@ -3543,6 +3550,31 @@ void createSpansWithDifferentWorkspaces() {
         getAndAssert(expectedSpan2, API_KEY, TEST_WORKSPACE);
     }
 
+    @Test
+    void createSpansWithSameIdForDifferentWorkspacesReturnsConflict() {
+
+        var span1 = podamFactory.manufacturePojo(Span.class).toBuilder()
+                .projectId(null)
+                .parentSpanId(null)
+                .build();
+
+        createAndAssert(span1, API_KEY, TEST_WORKSPACE);
+
+        String apiKey = UUID.randomUUID().toString();
+        String workspaceName = UUID.randomUUID().toString();
+        String workspaceId = UUID.randomUUID().toString();
+
+        var span2 = podamFactory.manufacturePojo(Span.class).toBuilder()
+                .id(span1.id())
+                .projectId(null)
+                .parentSpanId(null)
+                .projectName(UUID.randomUUID().toString())
+                .build();
+
+        mockTargetWorkspace(apiKey, workspaceName, workspaceId);
+        createAndAssertErrorMessage(span2, apiKey, workspaceName, HttpStatus.SC_CONFLICT, PROJECT_AND_WORKSPACE_NAME_MISMATCH);
+    }
+
     @Test
     void createWhenTryingToCreateSpanTwice() {
         var expectedSpan = podamFactory.manufacturePojo(Span.class).toBuilder()

apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/TracesResourceTest.java

@@ -101,6 +101,7 @@
 import static com.comet.opik.api.resources.utils.MigrationUtils.CLICKHOUSE_CHANGELOG_FILE;
 import static com.comet.opik.api.resources.utils.StatsUtils.getProjectTraceStatItems;
 import static com.comet.opik.domain.ProjectService.DEFAULT_PROJECT;
+import static com.comet.opik.domain.TraceService.PROJECT_NAME_AND_WORKSPACE_NAME_MISMATCH;
 import static com.comet.opik.infrastructure.auth.RequestContext.SESSION_COOKIE;
 import static com.comet.opik.infrastructure.auth.RequestContext.WORKSPACE_HEADER;
 import static com.comet.opik.infrastructure.auth.TestHttpClientUtils.UNAUTHORIZED_RESPONSE;
@@ -3568,6 +3569,12 @@ private UUID create(Trace trace, String apiKey, String workspaceName) {
         return traceResourceClient.createTrace(trace, apiKey, workspaceName);
     }
 
+    private void createAndAssertErrorMessage(Trace trace, String apiKey, String workspaceName, int status, String errorMessage) {
+        try (var response = traceResourceClient.createTrace(trace, apiKey, workspaceName, status)) {
+            assertThat(response.readEntity(ErrorMessage.class).errors().getFirst()).isEqualTo(errorMessage);
+        }
+    }
+
     private void create(UUID entityId, FeedbackScore score, String workspaceName, String apiKey) {
         traceResourceClient.feedbackScore(entityId, score, workspaceName, apiKey);
     }
@@ -3622,8 +3629,8 @@ void createTrace() {
         }
 
         @Test
-        @DisplayName("when creating traces with different workspaces names, then return created traces")
-        void create__whenCreatingTracesWithDifferentWorkspacesNames__thenReturnCreatedTraces() {
+        @DisplayName("when creating traces with different project names, then return created traces")
+        void create__whenCreatingTracesWithDifferentProjectNames__thenReturnCreatedTraces() {
             var projectName = RandomStringUtils.randomAlphanumeric(10);
 
             var trace1 = factory.manufacturePojo(Trace.class)
@@ -3648,6 +3655,34 @@ void create__whenCreatingTracesWithDifferentWorkspacesNames__thenReturnCreatedTr
             getAndAssert(trace2, projectId2, API_KEY, TEST_WORKSPACE);
         }
 
+        @Test
+        @DisplayName("when creating traces with same Id for different workspaces, then return conflict")
+        void create__whenCreatingTracesWithSameIdForDifferentWorkspaces__thenReturnConflict() {
+
+            var trace1 = factory.manufacturePojo(Trace.class)
+                    .toBuilder()
+                    .projectName(DEFAULT_PROJECT)
+                    .usage(null)
+                    .feedbackScores(null)
+                    .build();
+            create(trace1, API_KEY, TEST_WORKSPACE);
+
+            String apiKey = UUID.randomUUID().toString();
+            String workspaceName = UUID.randomUUID().toString();
+            String workspaceId = UUID.randomUUID().toString();
+
+            mockTargetWorkspace(apiKey, workspaceName, workspaceId);
+
+            var trace2 = factory.manufacturePojo(Trace.class)
+                    .toBuilder()
+                    .id(trace1.id())
+                    .projectName(DEFAULT_PROJECT)
+                    .usage(null)
+                    .feedbackScores(null)
+                    .build();
+            createAndAssertErrorMessage(trace2, apiKey, workspaceName, HttpStatus.SC_CONFLICT, PROJECT_NAME_AND_WORKSPACE_NAME_MISMATCH);
+        }
+
         @Test
         void createWithMissingId() {
             var trace = factory.manufacturePojo(Trace.class).toBuilder()