From d56664d318ad5fe278c1c328f470c3b30c74db8d Mon Sep 17 00:00:00 2001 From: Thiago Hora Date: Tue, 21 Jul 2026 16:00:10 +0200 Subject: [PATCH 1/7] [OPIK-7352] [BE] reject non-UUIDv7 referenced ids on ingest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Retention sweeps spans/traces via an id-range predicate that assumes the referenced id is a time-ordered UUIDv7 (SpanDAO orders by the trace_id range; TraceDAO by the id range). Ingest validated an entity's own id but not the ids it references, so a client could store a span with a non-UUIDv7 trace_id and have those rows ordered lexicographically instead of chronologically — deleted or retained against the wrong window. Validate referenced ids on ingest with a not-in-future policy (version 7 required, future-dated rejected, past allowed — referencing older entities such as late spans on old traces is legitimate). Renames validateIdForUpdate -> validateIdNotInFuture and reuses it for both the update paths and referenced ids; adds a public sync overload. Covered paths: span traceId/parentSpanId (create, batch, update, batch-update, and the experiment-items bulk path via batch create); single and batch feedback-score entity ids; trace/span/thread comment entity ids; attachment entity ids; dataset-item trace/span ids (batch, patch, from-traces, from-spans); annotation-queue item ids. Version enforcement is always on (mirrors own-id validation); the not-in-future check is gated by the existing uuidValidation kill-switch. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../opik/domain/AnnotationQueueService.java | 4 ++ .../com/comet/opik/domain/CommentService.java | 1 + .../comet/opik/domain/DatasetItemService.java | 17 +++++ .../opik/domain/FeedbackScoreService.java | 5 +- .../com/comet/opik/domain/IdGenerator.java | 24 ++++--- .../com/comet/opik/domain/SpanService.java | 23 +++++- .../com/comet/opik/domain/TraceService.java | 2 +- .../domain/attachment/AttachmentService.java | 6 ++ .../resources/v1/priv/SpansResourceTest.java | 72 +++++++++++++++++++ 9 files changed, 142 insertions(+), 12 deletions(-) diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/AnnotationQueueService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/AnnotationQueueService.java index 7847199fa48..055304fc4b3 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/AnnotationQueueService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/AnnotationQueueService.java @@ -134,6 +134,10 @@ public Mono addItems(@NonNull UUID queueId, @NonNull Set itemIds) { return Mono.just(0L); } + // Queue items reference trace/thread ids (v7 by construction); enforce so the referenced-id + // policy is uniform. Past allowed — queues commonly collect older traces/threads. + itemIds.forEach(itemId -> idGenerator.validateIdNotInFuture(itemId, "AnnotationQueue item")); + return annotationQueueDAO.findQueueInfoById(queueId) .switchIfEmpty(Mono.error(createNotFoundError(queueId))) .flatMap(queue -> annotationQueueDAO.addItems(queueId, itemIds, queue.projectId())) diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/CommentService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/CommentService.java index 5b5f0d3c505..f79c08ca083 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/CommentService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/CommentService.java @@ -53,6 +53,7 @@ class CommentServiceImpl implements CommentService { @Override public Mono create(@NonNull UUID entityId, @NonNull Comment comment, CommentDAO.EntityType entityType) { + idGenerator.validateIdNotInFuture(entityId, entityType.getType()); UUID id = idGenerator.generateId(); var monoProjectId = resolveProjectId(entityType, entityId); diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/DatasetItemService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/DatasetItemService.java index e23a6ff9ebc..b746de05d6d 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/DatasetItemService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/DatasetItemService.java @@ -187,6 +187,8 @@ public Mono createFromTraces( log.info("Creating dataset items from '{}' traces for dataset '{}'", traceIds.size(), datasetId); + traceIds.forEach(traceId -> idGenerator.validateIdNotInFuture(traceId, "dataset_item trace")); + // Verify dataset exists return Mono.deferContextual(ctx -> { String workspaceId = ctx.get(RequestContext.WORKSPACE_ID); @@ -240,6 +242,8 @@ public Mono createFromSpans( log.info("Creating dataset items from '{}' spans for dataset '{}'", spanIds.size(), datasetId); + spanIds.forEach(spanId -> idGenerator.validateIdNotInFuture(spanId, "dataset_item span")); + // Verify dataset exists return Mono.deferContextual(ctx -> { String workspaceId = ctx.get(RequestContext.WORKSPACE_ID); @@ -382,6 +386,7 @@ private Mono authorizeItem(Mono itemMono) { @Override @WithSpan public Mono patch(@NonNull UUID id, @NonNull DatasetItem item) { + validateReferencedTraceAndSpan(item); return Mono.deferContextual(ctx -> { String workspaceId = ctx.get(RequestContext.WORKSPACE_ID); String userName = ctx.get(RequestContext.USER_NAME); @@ -903,11 +908,23 @@ private List addIdIfAbsent(DatasetItemBatch batch) { .stream() .map(item -> { IdGenerator.validateVersion(item.id(), "dataset_item"); + validateReferencedTraceAndSpan(item); return item; }) .toList(); } + // The dataset_item's referenced trace_id / span_id must be a time-ordered UUIDv7 (past allowed: + // items are commonly linked to older traces/spans). Reuses the shared referenced-id policy. + private void validateReferencedTraceAndSpan(DatasetItem item) { + if (item.traceId() != null) { + idGenerator.validateIdNotInFuture(item.traceId(), "dataset_item trace"); + } + if (item.spanId() != null) { + idGenerator.validateIdNotInFuture(item.spanId(), "dataset_item span"); + } + } + private Mono failWithConflict(String message) { log.info(message); return Mono.error(new IdentifierMismatchException(new ErrorMessage(List.of(message)))); diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/FeedbackScoreService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/FeedbackScoreService.java index 34d6cc8f03a..1bc218e6e02 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/FeedbackScoreService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/FeedbackScoreService.java @@ -89,6 +89,7 @@ class FeedbackScoreServiceImpl implements FeedbackScoreService { private final @NonNull TraceThreadService traceThreadService; private final @NonNull Provider requestContext; private final @NonNull EventBus eventBus; + private final @NonNull IdGenerator idGenerator; @Builder(toBuilder = true) record ProjectDto(Project project, List scores) { @@ -100,6 +101,7 @@ public Mono scoreTrace(@NonNull UUID traceId, @NonNull FeedbackScore score String workspaceId = ctx.get(RequestContext.WORKSPACE_ID); String userName = ctx.get(RequestContext.USER_NAME); + idGenerator.validateIdNotInFuture(traceId, EntityType.TRACE.getType()); return traceDAO.getProjectIdFromTrace(traceId) .switchIfEmpty(Mono.error(failWithNotFound("Trace", traceId))) .flatMap(projectId -> getAuthor() @@ -118,6 +120,7 @@ public Mono scoreSpan(@NonNull UUID spanId, @NonNull FeedbackScore score) String workspaceId = ctx.get(RequestContext.WORKSPACE_ID); String userName = ctx.get(RequestContext.USER_NAME); + idGenerator.validateIdNotInFuture(spanId, EntityType.SPAN.getType()); return spanDAO.getProjectIdFromSpan(spanId) .switchIfEmpty(Mono.error(failWithNotFound("Span", spanId))) .flatMap(projectId -> getAuthor() @@ -173,7 +176,7 @@ private Mono processScoreBatch(EntityType entityType, List> scoresPerProject = scores .stream() .map(score -> { - IdGenerator.validateVersion(score.id(), entityType.getType()); // validate span/trace id + idGenerator.validateIdNotInFuture(score.id(), entityType.getType()); // validate span/trace id return score.toBuilder() .projectName(WorkspaceUtils.getProjectName(score.projectName())) diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/IdGenerator.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/IdGenerator.java index 312c85cb72d..7bcfd6bed18 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/IdGenerator.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/IdGenerator.java @@ -34,12 +34,19 @@ public interface IdGenerator { Mono validateIdAsync(UUID id, String resource); /** - * Validates an ingested {@code id} on the update path: it must be a version 7 UUID - * ({@link #validateVersion(UUID, String)}) and must not embed a timestamp far in the future (which - * would corrupt the partition layout). Unlike {@link #validateId}, old ids are allowed, because - * updating a long-lived entity (e.g. created months ago) is a legitimate operation. + * Validates an {@code id} that may legitimately point at an entity created in the past: it must be a + * version 7 UUID ({@link #validateVersion(UUID, String)}) and must not embed a timestamp far in the + * future (which would corrupt the partition layout / retention id-range). Unlike {@link #validateId}, + * old ids are allowed. + * + *

Used both on the update path (updating a long-lived entity created months ago is legitimate) and + * for referenced/foreign ids on ingest (e.g. a span's {@code traceId}: retention orders spans by the + * {@code trace_id} range assuming it is a time-ordered UUIDv7, and late spans on old traces are common, + * so old is fine but non-v7 or future-dated must be rejected). */ - Mono validateIdForUpdateAsync(UUID id, String resource); + void validateIdNotInFuture(UUID id, String resource); + + Mono validateIdNotInFutureAsync(UUID id, String resource); static Mono validateVersionAsync(@NonNull UUID id, String resource) { return Mono.fromCallable(() -> { @@ -91,15 +98,16 @@ public Mono validateIdAsync(@NonNull UUID id, String resource) { }); } - private void validateIdForUpdate(UUID id, String resource) { + @Override + public void validateIdNotInFuture(@NonNull UUID id, String resource) { IdGenerator.validateVersion(id, resource); uuidV7TimestampValidator.validateNotInFuture(id); } @Override - public Mono validateIdForUpdateAsync(@NonNull UUID id, String resource) { + public Mono validateIdNotInFutureAsync(@NonNull UUID id, String resource) { return Mono.fromCallable(() -> { - validateIdForUpdate(id, resource); + validateIdNotInFuture(id, resource); return id; }); } diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/SpanService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/SpanService.java index dffd115f792..f24002a1d0e 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/SpanService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/SpanService.java @@ -62,6 +62,8 @@ 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 SPAN_TRACE_KEY = "Span trace"; + public static final String SPAN_PARENT_KEY = "Span parent"; 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; @@ -159,6 +161,10 @@ public Mono create(@NonNull Span span) { var projectName = WorkspaceUtils.getProjectName(span.projectName()); return idGenerator .validateIdAsync(id, SPAN_KEY) + .then(idGenerator.validateIdNotInFutureAsync(span.traceId(), SPAN_TRACE_KEY)) + .then(span.parentSpanId() == null + ? Mono.empty() + : idGenerator.validateIdNotInFutureAsync(span.parentSpanId(), SPAN_PARENT_KEY)) .then(projectService.getOrCreate(projectName)) .flatMap(project -> lockService.executeWithLock( new LockService.Lock(id, SPAN_KEY), @@ -220,7 +226,11 @@ public Mono update(@NonNull UUID id, @NonNull SpanUpdate spanUpdate) { String userName = ctx.get(RequestContext.USER_NAME); return idGenerator - .validateIdForUpdateAsync(id, SPAN_KEY) + .validateIdNotInFutureAsync(id, SPAN_KEY) + .then(idGenerator.validateIdNotInFutureAsync(spanUpdate.traceId(), SPAN_TRACE_KEY)) + .then(spanUpdate.parentSpanId() == null + ? Mono.empty() + : idGenerator.validateIdNotInFutureAsync(spanUpdate.parentSpanId(), SPAN_PARENT_KEY)) .then(Mono.defer(() -> getProjectById(spanUpdate) .switchIfEmpty(Mono.defer(() -> projectService.getOrCreate(projectName))) .subscribeOn(Schedulers.boundedElastic())) @@ -247,7 +257,12 @@ public Mono batchUpdate(@NonNull SpanBatchUpdate batchUpdate) { String workspaceId = ctx.get(RequestContext.WORKSPACE_ID); String userName = ctx.get(RequestContext.USER_NAME); - return spanDAO.bulkUpdate(batchUpdate.ids(), batchUpdate.update(), mergeTags) + return idGenerator.validateIdNotInFutureAsync(batchUpdate.update().traceId(), SPAN_TRACE_KEY) + .then(batchUpdate.update().parentSpanId() == null + ? Mono.empty() + : idGenerator.validateIdNotInFutureAsync(batchUpdate.update().parentSpanId(), + SPAN_PARENT_KEY)) + .then(spanDAO.bulkUpdate(batchUpdate.ids(), batchUpdate.update(), mergeTags)) .onErrorResume(TagOperations::mapTagLimitError) .doOnSuccess(__ -> { log.info("Completed batch update for '{}' spans", batchUpdate.ids().size()); @@ -460,6 +475,10 @@ private List bindSpanToProjectAndId(List spans, List projec UUID id = span.id() == null ? idGenerator.generateId() : span.id(); idGenerator.validateId(id, SPAN_KEY); + idGenerator.validateIdNotInFuture(span.traceId(), SPAN_TRACE_KEY); + if (span.parentSpanId() != null) { + idGenerator.validateIdNotInFuture(span.parentSpanId(), SPAN_PARENT_KEY); + } return span.toBuilder().id(id).projectId(project.id()).build(); }) diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/TraceService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/TraceService.java index 1b3ddd9cc56..9cf964bc1cf 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/TraceService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/TraceService.java @@ -322,7 +322,7 @@ public Mono update(@NonNull TraceUpdate traceUpdate, @NonNull UUID id) { var projectName = WorkspaceUtils.getProjectName(traceUpdate.projectName()); return Mono.deferContextual(ctx -> idGenerator - .validateIdForUpdateAsync(id, TRACE_KEY) + .validateIdNotInFutureAsync(id, TRACE_KEY) .then(getProjectById(traceUpdate) .switchIfEmpty(Mono.defer(() -> projectService.getOrCreate(projectName))) .subscribeOn(Schedulers.boundedElastic()) diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/attachment/AttachmentService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/attachment/AttachmentService.java index 989a941d05d..2504374c689 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/attachment/AttachmentService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/attachment/AttachmentService.java @@ -9,6 +9,7 @@ import com.comet.opik.api.attachment.EntityType; import com.comet.opik.api.attachment.StartMultipartUploadRequest; import com.comet.opik.api.attachment.StartMultipartUploadResponse; +import com.comet.opik.domain.IdGenerator; import com.comet.opik.domain.ProjectService; import com.comet.opik.infrastructure.OpikConfiguration; import com.comet.opik.infrastructure.auth.RequestContext; @@ -119,12 +120,14 @@ class AttachmentServiceImpl implements AttachmentService { private final @NonNull ProjectService projectService; private final @NonNull OpikConfiguration config; private final @NonNull Provider requestContext; + private final @NonNull IdGenerator idGenerator; private static final Tika tika = new Tika(); private static final int MAX_ATTACHMENTS_PER_ENTITY = 1_000; @Override public StartMultipartUploadResponse startMultiPartUpload(@NonNull StartMultipartUploadRequest startUploadRequest, @NonNull String workspaceId, @NonNull String userName) { + idGenerator.validateIdNotInFuture(startUploadRequest.entityId(), startUploadRequest.entityType().getValue()); if (config.getS3Config().isMinIO()) { return prepareMinIOUploadResponse(startUploadRequest); } @@ -149,6 +152,8 @@ public StartMultipartUploadResponse startMultiPartUpload(@NonNull StartMultipart public void completeMultiPartUpload(@NonNull CompleteMultipartUploadRequest completeUploadRequest, @NonNull String workspaceId, @NonNull String userName) { + idGenerator.validateIdNotInFuture(completeUploadRequest.entityId(), + completeUploadRequest.entityType().getValue()); // In case of MinIO complete is not needed, file is uploaded directly via BE if (config.getS3Config().isMinIO()) { log.info("Skipping completeMultiPartUpload for MinIO"); @@ -190,6 +195,7 @@ public void uploadAttachment(@NonNull AttachmentInfo attachmentInfo, byte[] data @Override public void uploadAttachmentInternal(@NonNull AttachmentInfo attachmentInfo, byte[] data, @NonNull String workspaceId, @NonNull String userName) { + idGenerator.validateIdNotInFuture(attachmentInfo.entityId(), attachmentInfo.entityType().getValue()); attachmentInfo = attachmentInfo.toBuilder() .containerId(getProjectIdByName(attachmentInfo.projectName(), workspaceId, userName)) diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/SpansResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/SpansResourceTest.java index 9453f03dcfb..03cc568f410 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/SpansResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/SpansResourceTest.java @@ -344,6 +344,24 @@ static Stream invalidIdsForUpdate() { "UUID after window")); } + // Referenced ids (traceId, parentSpanId) use the not-in-future policy: non-v7 and future-dated are + // rejected, but past ids are allowed (spans are commonly attached to older traces), so unlike + // invalidIds() there is no TOO_OLD case here. + static Stream invalidTraceIds() { + var future = Instant.now().plus(Duration.ofHours(25)).toEpochMilli(); + var expectedDetails = "id with timestamp '%s' must be in the allowed ingestion window of '%s' around now, reason '%s'"; + var expectedWindow = Duration.ofHours(24); + return Stream.of( + arguments(UUID.randomUUID(), + "Span trace id must be a version 7 UUID", + "traceId not v7"), + arguments( + generator.construct(future), + expectedDetails.formatted( + Instant.ofEpochMilli(future), expectedWindow, Reason.TOO_FAR_FUTURE.getValue()), + "traceId after window")); + } + @Nested @DisplayName("Spans existence probe") class SpansExistence { @@ -1800,6 +1818,46 @@ void createWithInvalidIdThrowsBadRequest(UUID id, String expectedDetails, String } } + @MethodSource("com.comet.opik.api.resources.v1.priv.SpansResourceTest#invalidTraceIds") + @ParameterizedTest(name = "Create span with invalid traceId throws bad request: {2}") + void createWithInvalidTraceIdThrowsBadRequest(UUID traceId, String expectedDetails, String testName) { + var expectedEntity = new io.dropwizard.jersey.errors.ErrorMessage( + HttpStatus.SC_BAD_REQUEST, "Invalid UUID for id", expectedDetails); + var span = podamFactory.manufacturePojo(Span.class).toBuilder().traceId(traceId).build(); + try (var response = spanResourceClient.createSpan( + span, API_KEY, TEST_WORKSPACE, HttpStatus.SC_BAD_REQUEST)) { + var actualEntity = response.readEntity(io.dropwizard.jersey.errors.ErrorMessage.class); + assertThat(actualEntity).isEqualTo(expectedEntity); + } + } + + @Test + @DisplayName("Create span with non-v7 parentSpanId throws bad request") + void createWithNonV7ParentSpanIdThrowsBadRequest() { + var expectedEntity = new io.dropwizard.jersey.errors.ErrorMessage( + HttpStatus.SC_BAD_REQUEST, "Invalid UUID for id", "Span parent id must be a version 7 UUID"); + var span = podamFactory.manufacturePojo(Span.class).toBuilder().parentSpanId(UUID.randomUUID()).build(); + try (var response = spanResourceClient.createSpan( + span, API_KEY, TEST_WORKSPACE, HttpStatus.SC_BAD_REQUEST)) { + var actualEntity = response.readEntity(io.dropwizard.jersey.errors.ErrorMessage.class); + assertThat(actualEntity).isEqualTo(expectedEntity); + } + } + + @Test + @DisplayName("Create span with an old (past) v7 traceId succeeds — late spans on old traces are valid") + void createWithOldTraceIdSucceeds() { + var old = Instant.now().minus(Duration.ofDays(30)).toEpochMilli(); + var span = podamFactory.manufacturePojo(Span.class).toBuilder() + .traceId(generator.construct(old)) + .parentSpanId(null) + .build(); + + var id = spanResourceClient.createSpan(span, API_KEY, TEST_WORKSPACE); + + assertThat(id).isNotNull(); + } + @Test @DisplayName("when span is fetched with different truncate and strip_attachments flags, then response varies accordingly") void getByList__whenFetchedWithDifferentFlags__thenResponseVariesAccordingly() throws Exception { @@ -2292,6 +2350,20 @@ void batchCreateWithInvalidIdThrowsBadRequest(UUID id, String expectedDetails, S } } + @MethodSource("com.comet.opik.api.resources.v1.priv.SpansResourceTest#invalidTraceIds") + @ParameterizedTest(name = "Batch create span with invalid traceId throws bad request: {2}") + void batchCreateWithInvalidTraceIdThrowsBadRequest(UUID traceId, String expectedDetails, String testName) { + var expectedEntity = new io.dropwizard.jersey.errors.ErrorMessage( + HttpStatus.SC_BAD_REQUEST, "Invalid UUID for id", expectedDetails); + var span = podamFactory.manufacturePojo(Span.class).toBuilder().traceId(traceId).build(); + try (var response = spanResourceClient.callBatchCreateSpans( + List.of(span), API_KEY, TEST_WORKSPACE)) { + assertThat(response.getStatusInfo().getStatusCode()).isEqualTo(HttpStatus.SC_BAD_REQUEST); + var actualEntity = response.readEntity(io.dropwizard.jersey.errors.ErrorMessage.class); + assertThat(actualEntity).isEqualTo(expectedEntity); + } + } + private long readClickHouseErrorCount(TransactionTemplateAsync templateAsync, int errorCode) { return templateAsync.nonTransaction(connection -> { var statement = connection.createStatement( From 5f9f035eb5cde288784bb1c0630f99246c1f1bb9 Mon Sep 17 00:00:00 2001 From: Thiago Hora Date: Tue, 21 Jul 2026 16:29:00 +0200 Subject: [PATCH 2/7] [OPIK-7352] [BE] validate config-entity referenced ids on ingest Extends referenced-id validation to config-entity references (verify-then- enforce: confirmed zero non-UUIDv7 ids across prod projects, datasets, dataset_versions, prompts, prompt_versions, alerts, dashboards, webhooks, automation_rules, and guardrails, so enforcement rejects no existing data). Adds null-safe validateIdNotInFutureIfPresent(Async) helpers and validates: projectId across span/trace update + batch-update, feedback scores (single + batch), guardrails, assertion results, annotation queues, experiments, optimizations, alerts, automation-rule evaluators, prompts + prompt versions, dashboards, and thread open/close; feedback/guardrail sourceQueueId; guardrail secondaryId; experiment optimizationId + datasetVersionId; dataset-item datasetId + copy-from ids. Upgrades the guardrail/assertion referenced entityId from version-only to the shared not-in-future policy. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../com/comet/opik/domain/AlertService.java | 1 + .../opik/domain/AnnotationQueueService.java | 1 + .../opik/domain/AssertionResultService.java | 6 +++++- .../comet/opik/domain/DashboardService.java | 1 + .../comet/opik/domain/DatasetItemService.java | 4 ++++ .../comet/opik/domain/ExperimentService.java | 3 +++ .../opik/domain/FeedbackScoreService.java | 4 ++++ .../comet/opik/domain/GuardrailsService.java | 4 +++- .../com/comet/opik/domain/IdGenerator.java | 14 +++++++++++++ .../opik/domain/OptimizationService.java | 1 + .../com/comet/opik/domain/PromptService.java | 2 ++ .../com/comet/opik/domain/SpanService.java | 20 +++++++------------ .../com/comet/opik/domain/TraceService.java | 4 +++- .../AutomationRuleEvaluatorService.java | 3 +++ .../domain/threads/TraceThreadService.java | 4 ++++ 15 files changed, 56 insertions(+), 16 deletions(-) diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/AlertService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/AlertService.java index 196f5e36c60..4cb0e658540 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/AlertService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/AlertService.java @@ -555,6 +555,7 @@ private Alert prepareAlert(Alert alert, String userName, String workspaceId) { UUID id = alert.id() == null ? idGenerator.generateId() : alert.id(); IdGenerator.validateVersion(id, "Alert"); + idGenerator.validateIdNotInFutureIfPresent(alert.projectId(), "project"); UUID webhookId = alert.webhook().id() == null ? idGenerator.generateId() : alert.webhook().id(); IdGenerator.validateVersion(webhookId, "Webhook"); diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/AnnotationQueueService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/AnnotationQueueService.java index 055304fc4b3..40c865ee580 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/AnnotationQueueService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/AnnotationQueueService.java @@ -261,6 +261,7 @@ private Mono enhancePageWithProjectNames( private AnnotationQueue prepareAnnotationQueue(AnnotationQueue annotationQueue) { UUID id = annotationQueue.id() == null ? idGenerator.generateId() : annotationQueue.id(); IdGenerator.validateVersion(id, "AnnotationQueue"); + idGenerator.validateIdNotInFutureIfPresent(annotationQueue.projectId(), "project"); log.debug("Preparing annotation queue with id '{}', name '{}', project '{}'", id, annotationQueue.name(), annotationQueue.projectId()); diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/AssertionResultService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/AssertionResultService.java index d826b84d762..9c874b9f29f 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/AssertionResultService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/AssertionResultService.java @@ -43,6 +43,7 @@ class AssertionResultServiceImpl implements AssertionResultService { private final @NonNull AssertionResultDAO assertionResultDAO; private final @NonNull ProjectService projectService; private final @NonNull EventBus eventBus; + private final @NonNull IdGenerator idGenerator; @Override public Mono insertBatch(@NonNull EntityType entityType, @@ -63,7 +64,10 @@ public Mono saveBatch(@NonNull EntityType entityType, } // Validate up front so a bad id fails fast and independently of project-name normalisation. - assertionResults.forEach(item -> IdGenerator.validateVersion(item.entityId(), entityType.getType())); + assertionResults.forEach(item -> { + idGenerator.validateIdNotInFuture(item.entityId(), entityType.getType()); + idGenerator.validateIdNotInFutureIfPresent(item.projectId(), "project"); + }); return Mono.deferContextual(ctx -> { String workspaceId = ctx.get(RequestContext.WORKSPACE_ID); diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/DashboardService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/DashboardService.java index 8a9a1e1ae1f..24daa474f07 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/DashboardService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/DashboardService.java @@ -83,6 +83,7 @@ public Dashboard create(@NonNull Dashboard dashboard, @NonNull DashboardScope sc // Generate ID if not provided var dashboardId = dashboard.id() != null ? dashboard.id() : idGenerator.generateId(); IdGenerator.validateVersion(dashboardId, "dashboard"); + idGenerator.validateIdNotInFutureIfPresent(dashboard.projectId(), "project"); final UUID resolvedProjectId; if (StringUtils.isNotBlank(dashboard.projectName()) && dashboard.projectId() == null) { diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/DatasetItemService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/DatasetItemService.java index b746de05d6d..d3027abe8e9 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/DatasetItemService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/DatasetItemService.java @@ -1572,6 +1572,10 @@ private List prepareAddedItems(DatasetItemChanges changes, UUID dat @WithSpan public Mono save(@NonNull DatasetItemBatch batch) { + idGenerator.validateIdNotInFutureIfPresent(batch.datasetId(), "dataset"); + idGenerator.validateIdNotInFutureIfPresent(batch.copyFromDatasetId(), "dataset"); + idGenerator.validateIdNotInFutureIfPresent(batch.copyFromVersionId(), "dataset version"); + if (!featureFlags.isDatasetVersioningEnabled()) { // Legacy: save to legacy table log.info("Saving items to legacy table for dataset '{}'", batch.datasetId()); diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/ExperimentService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/ExperimentService.java index 3da57e5dd56..1aaedcba52e 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/ExperimentService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/ExperimentService.java @@ -495,6 +495,9 @@ private Set getPromptVersionIds(Experiment experiment) { public Mono create(@NonNull Experiment experiment) { var id = experiment.id() == null ? idGenerator.generateId() : experiment.id(); IdGenerator.validateVersion(id, "Experiment"); + idGenerator.validateIdNotInFutureIfPresent(experiment.projectId(), "project"); + idGenerator.validateIdNotInFutureIfPresent(experiment.optimizationId(), "optimization"); + idGenerator.validateIdNotInFutureIfPresent(experiment.datasetVersionId(), "dataset version"); var name = StringUtils.getIfBlank(experiment.name(), nameGenerator::generateName); return resolveProjectId(experiment) .flatMap(resolvedExperiment -> datasetService diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/FeedbackScoreService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/FeedbackScoreService.java index 1bc218e6e02..642f73510d4 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/FeedbackScoreService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/FeedbackScoreService.java @@ -102,6 +102,7 @@ public Mono scoreTrace(@NonNull UUID traceId, @NonNull FeedbackScore score String userName = ctx.get(RequestContext.USER_NAME); idGenerator.validateIdNotInFuture(traceId, EntityType.TRACE.getType()); + idGenerator.validateIdNotInFutureIfPresent(score.sourceQueueId(), "annotation queue"); return traceDAO.getProjectIdFromTrace(traceId) .switchIfEmpty(Mono.error(failWithNotFound("Trace", traceId))) .flatMap(projectId -> getAuthor() @@ -121,6 +122,7 @@ public Mono scoreSpan(@NonNull UUID spanId, @NonNull FeedbackScore score) String userName = ctx.get(RequestContext.USER_NAME); idGenerator.validateIdNotInFuture(spanId, EntityType.SPAN.getType()); + idGenerator.validateIdNotInFutureIfPresent(score.sourceQueueId(), "annotation queue"); return spanDAO.getProjectIdFromSpan(spanId) .switchIfEmpty(Mono.error(failWithNotFound("Span", spanId))) .flatMap(projectId -> getAuthor() @@ -177,6 +179,8 @@ private Mono processScoreBatch(EntityType entityType, List { idGenerator.validateIdNotInFuture(score.id(), entityType.getType()); // validate span/trace id + idGenerator.validateIdNotInFutureIfPresent(score.projectId(), "project"); + idGenerator.validateIdNotInFutureIfPresent(score.sourceQueueId(), "annotation queue"); return score.toBuilder() .projectName(WorkspaceUtils.getProjectName(score.projectName())) diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/GuardrailsService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/GuardrailsService.java index c4b3405e000..5faae049098 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/GuardrailsService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/GuardrailsService.java @@ -51,7 +51,9 @@ public Mono addTraceGuardrails(List guardrails) { .stream() .map(guardrail -> { UUID id = idGenerator.generateId(); - IdGenerator.validateVersion(guardrail.entityId(), entityType.getType()); // validate trace id + idGenerator.validateIdNotInFuture(guardrail.entityId(), entityType.getType()); + idGenerator.validateIdNotInFuture(guardrail.secondaryId(), "guardrail secondary"); + idGenerator.validateIdNotInFutureIfPresent(guardrail.projectId(), "project"); return guardrail.toBuilder() .id(id) diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/IdGenerator.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/IdGenerator.java index 7bcfd6bed18..a4ac620a266 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/IdGenerator.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/IdGenerator.java @@ -48,6 +48,20 @@ public interface IdGenerator { Mono validateIdNotInFutureAsync(UUID id, String resource); + /** + * Null-safe variant of {@link #validateIdNotInFuture} for optional referenced ids (e.g. an optional + * {@code projectId} that may be resolved by name instead). No-op when {@code id} is null. + */ + default void validateIdNotInFutureIfPresent(UUID id, String resource) { + if (id != null) { + validateIdNotInFuture(id, resource); + } + } + + default Mono validateIdNotInFutureIfPresentAsync(UUID id, String resource) { + return id == null ? Mono.empty() : validateIdNotInFutureAsync(id, resource); + } + static Mono validateVersionAsync(@NonNull UUID id, String resource) { return Mono.fromCallable(() -> { validateVersion(id, resource); diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/OptimizationService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/OptimizationService.java index 00131cc440c..014b09af622 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/OptimizationService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/OptimizationService.java @@ -176,6 +176,7 @@ private OptimizationSearchCriteria resolveDatasetNameFilter( public Mono upsert(@NonNull Optimization optimization) { UUID id = optimization.id() == null ? idGenerator.generateId() : optimization.id(); IdGenerator.validateVersion(id, "Optimization"); + idGenerator.validateIdNotInFutureIfPresent(optimization.projectId(), "project"); // Detect if this is a Studio optimization (has studioConfig in the request) boolean isStudioOptimization = optimization.studioConfig() != null; diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/PromptService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/PromptService.java index da883242466..22853f2e4b1 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/PromptService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/PromptService.java @@ -214,6 +214,7 @@ private PromptVersion createPromptVersionFromPromptRequest(Prompt createdPrompt, private Prompt savePrompt(String workspaceId, Prompt prompt) { IdGenerator.validateVersion(prompt.id(), "prompt"); + idGenerator.validateIdNotInFutureIfPresent(prompt.projectId(), "project"); transactionTemplate.inTransaction(WRITE, handle -> { PromptDAO promptDAO = handle.attach(PromptDAO.class); @@ -332,6 +333,7 @@ public PromptVersion createPromptVersion(@NonNull CreatePromptVersion createProm : createPromptVersion.version().commit(); IdGenerator.validateVersion(id, "prompt version"); + idGenerator.validateIdNotInFutureIfPresent(createPromptVersion.projectId(), "project"); TemplateStructure templateStructure = createPromptVersion.templateStructure(); diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/SpanService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/SpanService.java index f24002a1d0e..a90d012767c 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/SpanService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/SpanService.java @@ -162,9 +162,7 @@ public Mono create(@NonNull Span span) { return idGenerator .validateIdAsync(id, SPAN_KEY) .then(idGenerator.validateIdNotInFutureAsync(span.traceId(), SPAN_TRACE_KEY)) - .then(span.parentSpanId() == null - ? Mono.empty() - : idGenerator.validateIdNotInFutureAsync(span.parentSpanId(), SPAN_PARENT_KEY)) + .then(idGenerator.validateIdNotInFutureIfPresentAsync(span.parentSpanId(), SPAN_PARENT_KEY)) .then(projectService.getOrCreate(projectName)) .flatMap(project -> lockService.executeWithLock( new LockService.Lock(id, SPAN_KEY), @@ -228,9 +226,8 @@ public Mono update(@NonNull UUID id, @NonNull SpanUpdate spanUpdate) { return idGenerator .validateIdNotInFutureAsync(id, SPAN_KEY) .then(idGenerator.validateIdNotInFutureAsync(spanUpdate.traceId(), SPAN_TRACE_KEY)) - .then(spanUpdate.parentSpanId() == null - ? Mono.empty() - : idGenerator.validateIdNotInFutureAsync(spanUpdate.parentSpanId(), SPAN_PARENT_KEY)) + .then(idGenerator.validateIdNotInFutureIfPresentAsync(spanUpdate.parentSpanId(), SPAN_PARENT_KEY)) + .then(idGenerator.validateIdNotInFutureIfPresentAsync(spanUpdate.projectId(), "project")) .then(Mono.defer(() -> getProjectById(spanUpdate) .switchIfEmpty(Mono.defer(() -> projectService.getOrCreate(projectName))) .subscribeOn(Schedulers.boundedElastic())) @@ -258,10 +255,9 @@ public Mono batchUpdate(@NonNull SpanBatchUpdate batchUpdate) { String userName = ctx.get(RequestContext.USER_NAME); return idGenerator.validateIdNotInFutureAsync(batchUpdate.update().traceId(), SPAN_TRACE_KEY) - .then(batchUpdate.update().parentSpanId() == null - ? Mono.empty() - : idGenerator.validateIdNotInFutureAsync(batchUpdate.update().parentSpanId(), - SPAN_PARENT_KEY)) + .then(idGenerator.validateIdNotInFutureIfPresentAsync(batchUpdate.update().parentSpanId(), + SPAN_PARENT_KEY)) + .then(idGenerator.validateIdNotInFutureIfPresentAsync(batchUpdate.update().projectId(), "project")) .then(spanDAO.bulkUpdate(batchUpdate.ids(), batchUpdate.update(), mergeTags)) .onErrorResume(TagOperations::mapTagLimitError) .doOnSuccess(__ -> { @@ -476,9 +472,7 @@ private List bindSpanToProjectAndId(List spans, List projec UUID id = span.id() == null ? idGenerator.generateId() : span.id(); idGenerator.validateId(id, SPAN_KEY); idGenerator.validateIdNotInFuture(span.traceId(), SPAN_TRACE_KEY); - if (span.parentSpanId() != null) { - idGenerator.validateIdNotInFuture(span.parentSpanId(), SPAN_PARENT_KEY); - } + idGenerator.validateIdNotInFutureIfPresent(span.parentSpanId(), SPAN_PARENT_KEY); return span.toBuilder().id(id).projectId(project.id()).build(); }) diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/TraceService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/TraceService.java index 9cf964bc1cf..437c8043703 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/TraceService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/TraceService.java @@ -323,6 +323,7 @@ public Mono update(@NonNull TraceUpdate traceUpdate, @NonNull UUID id) { return Mono.deferContextual(ctx -> idGenerator .validateIdNotInFutureAsync(id, TRACE_KEY) + .then(idGenerator.validateIdNotInFutureIfPresentAsync(traceUpdate.projectId(), "project")) .then(getProjectById(traceUpdate) .switchIfEmpty(Mono.defer(() -> projectService.getOrCreate(projectName))) .subscribeOn(Schedulers.boundedElastic()) @@ -358,7 +359,8 @@ public Mono batchUpdate(@NonNull TraceBatchUpdate batchUpdate) { String workspaceId = ctx.get(RequestContext.WORKSPACE_ID); String userName = ctx.get(RequestContext.USER_NAME); String workspaceName = ctx.getOrDefault(RequestContext.WORKSPACE_NAME, ""); - return dao.getProjectIdsByTraceIds(new ArrayList<>(batchUpdate.ids())) + return idGenerator.validateIdNotInFutureIfPresentAsync(batchUpdate.update().projectId(), "project") + .then(dao.getProjectIdsByTraceIds(new ArrayList<>(batchUpdate.ids()))) .flatMap(traceToProjectMap -> { var projectIds = Set.copyOf(traceToProjectMap.values()); return dao.bulkUpdate(batchUpdate.ids(), batchUpdate.update(), mergeTags) diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/evaluators/AutomationRuleEvaluatorService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/evaluators/AutomationRuleEvaluatorService.java index dc664222279..7b9ac648e9d 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/evaluators/AutomationRuleEvaluatorService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/evaluators/AutomationRuleEvaluatorService.java @@ -117,6 +117,7 @@ public > T save(@No UUID id = idGenerator.generateId(); IdGenerator.validateVersion(id, "AutomationRuleEvaluator"); + projectIds.forEach(projectId -> idGenerator.validateIdNotInFutureIfPresent(projectId, "project")); // Dual-field sync: First projectId becomes the legacy project_id field UUID primaryProjectId = projectIds.isEmpty() ? null : projectIds.iterator().next(); @@ -227,6 +228,8 @@ public > T save(@No public void update(@NonNull UUID id, @NonNull Set projectIds, @NonNull String workspaceId, @NonNull String userName, @NonNull AutomationRuleEvaluatorUpdate evaluatorUpdate) { + projectIds.forEach(projectId -> idGenerator.validateIdNotInFutureIfPresent(projectId, "project")); + log.debug("Updating AutomationRuleEvaluator with id '{}' in projectIds '{}' and workspaceId '{}'", id, projectIds, workspaceId); diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/threads/TraceThreadService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/threads/TraceThreadService.java index 7991dcd4672..18f683ce23c 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/threads/TraceThreadService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/threads/TraceThreadService.java @@ -10,6 +10,7 @@ import com.comet.opik.api.events.ProjectWithPendingClosureTraceThreads; import com.comet.opik.api.events.TraceThreadsCreated; import com.comet.opik.api.resources.v1.events.TraceThreadBufferConfig; +import com.comet.opik.domain.IdGenerator; import com.comet.opik.domain.ProjectService; import com.comet.opik.domain.TagOperations; import com.comet.opik.domain.TraceService; @@ -86,6 +87,7 @@ class TraceThreadServiceImpl implements TraceThreadService { private static final Duration LOCK_DURATION = Duration.ofSeconds(5); + private final @NonNull IdGenerator idGenerator; private final @NonNull TraceThreadDAO traceThreadDAO; private final @NonNull TraceThreadIdService traceThreadIdService; private final @NonNull TraceService traceService; @@ -398,6 +400,7 @@ public Mono addToPendingQueue(@NonNull UUID projectId) { @Override public Mono openThread(UUID projectId, String projectName, @NonNull String threadId) { + idGenerator.validateIdNotInFutureIfPresent(projectId, "project"); return projectService.resolveProjectIdAndVerifyVisibility(projectId, projectName) .flatMap(verifiedProjectId -> getOrCreateThreadId(verifiedProjectId, threadId) .then(Mono.defer(() -> lockService.executeWithLockCustomExpire( @@ -411,6 +414,7 @@ public Mono openThread(UUID projectId, String projectName, @NonNull String @Override public Mono closeThreads(UUID projectId, String projectName, @NonNull Set threadIds) { + idGenerator.validateIdNotInFutureIfPresent(projectId, "project"); if (CollectionUtils.isEmpty(threadIds)) { return Mono.empty(); } From 7a47cac7db5f5b77dbc2e85db0e67c1746a8697e Mon Sep 17 00:00:00 2001 From: Thiago Hora Date: Tue, 21 Jul 2026 17:10:07 +0200 Subject: [PATCH 3/7] [OPIK-7352] [BE] only validate referenced ids that are persisted Refine scope to the rule: validate a referenced id iff the operation persists it to the entity's table. Drop validation where the id is not written by the operation: - Thread open/close: projectId only resolves/locates an already-existing thread (its project_id was written at trace ingestion), so nothing is persisted here. - Dataset item batch copy-from ids: only a read source for carry-forward rows, never stored as version lineage. All create/update paths that write the referenced id to a column keep their validation (spans trace_id/parent_span_id, feedback/comment/attachment/ guardrail/assertion entity ids, dataset-item trace/span + dataset ids, annotation-queue items, and projectId/optimizationId/datasetVersionId/ sourceQueueId on the rows that store them). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../main/java/com/comet/opik/domain/DatasetItemService.java | 2 -- .../com/comet/opik/domain/threads/TraceThreadService.java | 4 ---- 2 files changed, 6 deletions(-) diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/DatasetItemService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/DatasetItemService.java index d3027abe8e9..7795410c3df 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/DatasetItemService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/DatasetItemService.java @@ -1573,8 +1573,6 @@ private List prepareAddedItems(DatasetItemChanges changes, UUID dat public Mono save(@NonNull DatasetItemBatch batch) { idGenerator.validateIdNotInFutureIfPresent(batch.datasetId(), "dataset"); - idGenerator.validateIdNotInFutureIfPresent(batch.copyFromDatasetId(), "dataset"); - idGenerator.validateIdNotInFutureIfPresent(batch.copyFromVersionId(), "dataset version"); if (!featureFlags.isDatasetVersioningEnabled()) { // Legacy: save to legacy table diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/threads/TraceThreadService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/threads/TraceThreadService.java index 18f683ce23c..7991dcd4672 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/threads/TraceThreadService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/threads/TraceThreadService.java @@ -10,7 +10,6 @@ import com.comet.opik.api.events.ProjectWithPendingClosureTraceThreads; import com.comet.opik.api.events.TraceThreadsCreated; import com.comet.opik.api.resources.v1.events.TraceThreadBufferConfig; -import com.comet.opik.domain.IdGenerator; import com.comet.opik.domain.ProjectService; import com.comet.opik.domain.TagOperations; import com.comet.opik.domain.TraceService; @@ -87,7 +86,6 @@ class TraceThreadServiceImpl implements TraceThreadService { private static final Duration LOCK_DURATION = Duration.ofSeconds(5); - private final @NonNull IdGenerator idGenerator; private final @NonNull TraceThreadDAO traceThreadDAO; private final @NonNull TraceThreadIdService traceThreadIdService; private final @NonNull TraceService traceService; @@ -400,7 +398,6 @@ public Mono addToPendingQueue(@NonNull UUID projectId) { @Override public Mono openThread(UUID projectId, String projectName, @NonNull String threadId) { - idGenerator.validateIdNotInFutureIfPresent(projectId, "project"); return projectService.resolveProjectIdAndVerifyVisibility(projectId, projectName) .flatMap(verifiedProjectId -> getOrCreateThreadId(verifiedProjectId, threadId) .then(Mono.defer(() -> lockService.executeWithLockCustomExpire( @@ -414,7 +411,6 @@ public Mono openThread(UUID projectId, String projectName, @NonNull String @Override public Mono closeThreads(UUID projectId, String projectName, @NonNull Set threadIds) { - idGenerator.validateIdNotInFutureIfPresent(projectId, "project"); if (CollectionUtils.isEmpty(threadIds)) { return Mono.empty(); } From 7df5573f60e5380b28b7d568adb43fc350d29025 Mon Sep 17 00:00:00 2001 From: Thiago Hora Date: Wed, 22 Jul 2026 10:34:17 +0200 Subject: [PATCH 4/7] [OPIK-7352] [BE] test: use UUIDv7 for validated referenced ids Existing tests assigned random v4 UUIDs to referenced ids that are now validated as UUIDv7 (guardrail secondaryId, experiment optimizationId / datasetVersionId). Switch that test data to generator.generate() so the tests exercise the intended paths (the invalid-version conflict test now uses a v7-but-nonexistent id, preserving its 409 expectation). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../resources/v1/priv/AlertResourceTest.java | 13 +++++++---- .../v1/priv/DatasetVersionResourceTest.java | 5 +++- .../v1/priv/ExperimentsResourceTest.java | 4 ++-- .../priv/GetTracesByProjectResourceTest.java | 8 ++++--- .../v1/priv/GuardrailsResourceTest.java | 14 +++++++---- .../v1/priv/ProjectMetricsResourceTest.java | 23 +++++++++++-------- ...ojectMetricsWithBreakdownResourceTest.java | 5 +++- .../v1/priv/ProjectsResourceTest.java | 6 +++-- 8 files changed, 50 insertions(+), 28 deletions(-) diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/AlertResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/AlertResourceTest.java index ae5321dcf4b..b4ec843436f 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/AlertResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/AlertResourceTest.java @@ -57,6 +57,8 @@ import com.comet.opik.infrastructure.auth.WorkspaceUserPermission; import com.comet.opik.podam.PodamFactoryUtils; import com.comet.opik.utils.JsonUtils; +import com.fasterxml.uuid.Generators; +import com.fasterxml.uuid.impl.TimeBasedEpochGenerator; import com.github.tomakehurst.wiremock.WireMockServer; import com.redis.testcontainers.RedisContainer; import jakarta.ws.rs.core.HttpHeaders; @@ -169,6 +171,7 @@ class AlertResourceTest { } private final PodamFactory factory = PodamFactoryUtils.newPodamFactory(); + private final TimeBasedEpochGenerator generator = Generators.timeBasedEpochGenerator(); private AlertResourceClient alertResourceClient; private PromptResourceClient promptResourceClient; @@ -1785,7 +1788,7 @@ void whenGuardrailsAreTriggeredForTrace_thenWebhookIsCalledBasedOnProjectScope( // Create guardrails for the trace Guardrail guardrail = factory.manufacturePojo(Guardrail.class).toBuilder() .entityId(trace.id()) - .secondaryId(UUID.randomUUID()) + .secondaryId(generator.generate()) .projectName(projectName) .result(GuardrailResult.FAILED) .build(); @@ -1904,7 +1907,7 @@ void whenGuardrailScopedByProjectIdColumn__thenWebhookFiresOnlyForMatchingProjec Guardrail guardrailA = factory.manufacturePojo(Guardrail.class).toBuilder() .entityId(traceA.id()) - .secondaryId(UUID.randomUUID()) + .secondaryId(generator.generate()) .projectName(projectAName) .result(GuardrailResult.FAILED) .build(); @@ -1929,7 +1932,7 @@ void whenGuardrailScopedByProjectIdColumn__thenWebhookFiresOnlyForMatchingProjec Guardrail guardrailB = factory.manufacturePojo(Guardrail.class).toBuilder() .entityId(traceB.id()) - .secondaryId(UUID.randomUUID()) + .secondaryId(generator.generate()) .projectName(projectBName) .result(GuardrailResult.FAILED) .build(); @@ -2720,7 +2723,7 @@ void testGuardrailsTriggeredEvent(AlertType alertType) { List guardrails = IntStream.range(0, 2) .mapToObj(i -> factory.manufacturePojo(Guardrail.class).toBuilder() .entityId(trace.id()) - .secondaryId(UUID.randomUUID()) + .secondaryId(generator.generate()) .projectName(projectName) .projectId(projectId) .result(GuardrailResult.FAILED) @@ -2812,7 +2815,7 @@ void testGuardrailsTriggeredEventWithFallback() { return factory.manufacturePojo(Guardrail.class).toBuilder() .entityId(trace.id()) - .secondaryId(UUID.randomUUID()) + .secondaryId(generator.generate()) .projectName(projectName) .projectId(projectId) .result(GuardrailResult.FAILED) diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/DatasetVersionResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/DatasetVersionResourceTest.java index 8badd737da7..733fb8fb2ae 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/DatasetVersionResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/DatasetVersionResourceTest.java @@ -2819,6 +2819,9 @@ void createFromTraces__whenRegularDataset__thenDataContainsAllEnrichedFields() { @TestInstance(TestInstance.Lifecycle.PER_CLASS) class ExperimentDatasetVersionLinking { + private final com.fasterxml.uuid.impl.TimeBasedEpochGenerator generator = com.fasterxml.uuid.Generators + .timeBasedEpochGenerator(); + private Experiment getExperiment(UUID id) { return experimentResourceClient.getExperiment(id, API_KEY, TEST_WORKSPACE); } @@ -3059,7 +3062,7 @@ void createExperiment_whenInvalidVersionId_thenConflict() { var datasetId = createDataset(datasetName); createDatasetItems(datasetId, 1); - var nonExistentVersionId = UUID.randomUUID(); + var nonExistentVersionId = generator.generate(); // when - create experiment with non-existent version ID var experiment = experimentResourceClient.createPartialExperiment() diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ExperimentsResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ExperimentsResourceTest.java index 63665745ac1..85c1b2efb2a 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ExperimentsResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ExperimentsResourceTest.java @@ -1495,7 +1495,7 @@ void findByOptimizationIdAndType(ExperimentType type) { var apiKey = UUID.randomUUID().toString(); mockTargetWorkspace(apiKey, workspaceName, workspaceId); - UUID optimizationId = UUID.randomUUID(); + UUID optimizationId = GENERATOR.generate(); var experiments = experimentResourceClient.generateExperimentList() .stream() @@ -4785,7 +4785,7 @@ void createWithoutOptionalFieldsAndGet(String name) { void createWithOptimizationIdTypeAndGet(ExperimentType type) { var expectedExperiment = experimentResourceClient.createPartialExperiment() .type(type) - .optimizationId(UUID.randomUUID()) + .optimizationId(GENERATOR.generate()) .build(); var expectedId = createAndAssert(expectedExperiment, API_KEY, TEST_WORKSPACE); diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/GetTracesByProjectResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/GetTracesByProjectResourceTest.java index 9689eaa3fe2..6f61eddfc6f 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/GetTracesByProjectResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/GetTracesByProjectResourceTest.java @@ -60,6 +60,8 @@ import com.comet.opik.podam.PodamFactoryUtils; import com.comet.opik.utils.JsonUtils; import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.uuid.Generators; +import com.fasterxml.uuid.impl.TimeBasedEpochGenerator; import com.google.common.collect.Lists; import com.redis.testcontainers.RedisContainer; import jakarta.ws.rs.core.Response; @@ -117,7 +119,6 @@ import static com.comet.opik.api.resources.utils.ClickHouseContainerUtils.DATABASE_NAME; import static com.comet.opik.api.resources.utils.TestUtils.toURLEncodedQueryParam; import static com.comet.opik.api.resources.utils.traces.TraceAssertions.IGNORED_FIELDS_TRACES; -import static java.util.UUID.randomUUID; import static java.util.stream.Collectors.toCollection; import static java.util.stream.Collectors.toMap; import static org.assertj.core.api.Assertions.assertThat; @@ -172,6 +173,7 @@ class GetTracesByProjectResourceTest { } private final PodamFactory factory = PodamFactoryUtils.newPodamFactory(); + private final TimeBasedEpochGenerator generator = Generators.timeBasedEpochGenerator(); private final FilterQueryBuilder filterQueryBuilder = new FilterQueryBuilder(); private String baseURI; @@ -3947,7 +3949,7 @@ void whenFilterGuardrails__thenReturnTracesFiltered(String endpoint, TracePageTe var guardrailsByTraceId = traces.stream() .collect(Collectors.toMap(Trace::id, trace -> guardrailsGenerator.generateGuardrailsForTrace( - trace.id(), randomUUID(), trace.projectName()))); + trace.id(), generator.generate(), trace.projectName()))); // set the first trace with failed guardrails guardrailsByTraceId.put(traces.getFirst().id(), guardrailsByTraceId.get(traces.getFirst().id()).stream() @@ -5163,7 +5165,7 @@ void getTracesByProject__whenExcludeParamIdDefined__thenReturnSpanExcludingField .toList(); List guardrailsByTraceId = traces.stream() - .map(trace -> guardrailsGenerator.generateGuardrailsForTrace(trace.id(), randomUUID(), + .map(trace -> guardrailsGenerator.generateGuardrailsForTrace(trace.id(), generator.generate(), trace.projectName())) .flatMap(Collection::stream) .toList(); diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/GuardrailsResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/GuardrailsResourceTest.java index a655bc18bd5..7e9c8a54727 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/GuardrailsResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/GuardrailsResourceTest.java @@ -21,6 +21,8 @@ import com.comet.opik.extensions.DropwizardAppExtensionProvider; import com.comet.opik.extensions.RegisterApp; import com.comet.opik.podam.PodamFactoryUtils; +import com.fasterxml.uuid.Generators; +import com.fasterxml.uuid.impl.TimeBasedEpochGenerator; import com.redis.testcontainers.RedisContainer; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -82,6 +84,7 @@ public class GuardrailsResourceTest { } private final PodamFactory factory = PodamFactoryUtils.newPodamFactory(); + private final TimeBasedEpochGenerator generator = Generators.timeBasedEpochGenerator(); private TraceResourceClient traceResourceClient; private GuardrailsResourceClient guardrailsResourceClient; @@ -121,7 +124,8 @@ void testCreateGuardrails_getTraceById() { .build(); var traceId = traceResourceClient.createTrace(trace, API_KEY, TEST_WORKSPACE); - var guardrails = guardrailsGenerator.generateGuardrailsForTrace(traceId, randomUUID(), trace.projectName()); + var guardrails = guardrailsGenerator.generateGuardrailsForTrace(traceId, generator.generate(), + trace.projectName()); guardrailsResourceClient.addBatch(guardrails, API_KEY, TEST_WORKSPACE); Trace actual = traceResourceClient.getById(traceId, TEST_WORKSPACE, API_KEY); @@ -153,9 +157,11 @@ void testCreateGuardrails_findTraces() { var guardrailsByTraceId = traces.stream() .collect(Collectors.toMap(Trace::id, trace -> Stream.concat( // mimic two separate guardrails validation groups - guardrailsGenerator.generateGuardrailsForTrace(trace.id(), randomUUID(), trace.projectName()) + guardrailsGenerator.generateGuardrailsForTrace(trace.id(), generator.generate(), + trace.projectName()) .stream(), - guardrailsGenerator.generateGuardrailsForTrace(trace.id(), randomUUID(), trace.projectName()) + guardrailsGenerator.generateGuardrailsForTrace(trace.id(), generator.generate(), + trace.projectName()) .stream()) .toList())); @@ -189,7 +195,7 @@ void getTraceStats_containsGuardrails() { var guardrailsByTraceId = traces.stream() .collect(Collectors.toMap(Trace::id, trace -> guardrailsGenerator.generateGuardrailsForTrace( - trace.id(), randomUUID(), trace.projectName()))); + trace.id(), generator.generate(), trace.projectName()))); guardrailsByTraceId.values() .forEach(guardrail -> guardrailsResourceClient.addBatch(guardrail, API_KEY, diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ProjectMetricsResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ProjectMetricsResourceTest.java index 23484888cb8..144a7f91375 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ProjectMetricsResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ProjectMetricsResourceTest.java @@ -49,6 +49,8 @@ import com.comet.opik.infrastructure.DatabaseAnalyticsFactory; import com.comet.opik.podam.PodamFactoryUtils; import com.comet.opik.utils.JsonUtils; +import com.fasterxml.uuid.Generators; +import com.fasterxml.uuid.impl.TimeBasedEpochGenerator; import com.github.tomakehurst.wiremock.client.WireMock; import com.redis.testcontainers.RedisContainer; import jakarta.ws.rs.NotFoundException; @@ -131,7 +133,6 @@ import static com.github.tomakehurst.wiremock.client.WireMock.post; import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo; import static java.util.Collections.singletonMap; -import static java.util.UUID.randomUUID; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Named.named; import static org.junit.jupiter.params.provider.Arguments.arguments; @@ -188,6 +189,7 @@ class ProjectMetricsResourceTest { } private final PodamFactory factory = PodamFactoryUtils.newPodamFactory(); + private final TimeBasedEpochGenerator generator = Generators.timeBasedEpochGenerator(); private IdGenerator idGenerator; private String baseURI; @@ -424,7 +426,7 @@ void happyPathWithFilter(Function getFilter, List e // create guardrails for the first trace var guardrail = guardrailsGenerator.generateGuardrailsForTrace( - traces.getFirst().id(), randomUUID(), projectName).getFirst().toBuilder() + traces.getFirst().id(), generator.generate(), projectName).getFirst().toBuilder() .result(GuardrailResult.PASSED) .build(); @@ -650,7 +652,7 @@ void happyPathWithFilter(Function getFilter, List e // create guardrails for the first trace var guardrail = guardrailsGenerator.generateGuardrailsForTrace( - traceForFilter.id(), randomUUID(), projectName).getFirst().toBuilder() + traceForFilter.id(), generator.generate(), projectName).getFirst().toBuilder() .result(GuardrailResult.PASSED) .build(); @@ -1055,7 +1057,7 @@ void happyPathWithFilter(Function getFilter, List e // create guardrails for the first trace var guardrail = guardrailsGenerator.generateGuardrailsForTrace( - traceForFilter.id(), randomUUID(), projectName).getFirst().toBuilder() + traceForFilter.id(), generator.generate(), projectName).getFirst().toBuilder() .result(GuardrailResult.PASSED) .build(); @@ -1274,7 +1276,7 @@ void happyPathWithFilter(Function getFilter, List e // create guardrails for the first trace var guardrail = guardrailsGenerator.generateGuardrailsForTrace( - traceForFilter.id(), randomUUID(), projectName).getFirst().toBuilder() + traceForFilter.id(), generator.generate(), projectName).getFirst().toBuilder() .result(GuardrailResult.PASSED) .build(); @@ -1466,7 +1468,7 @@ void happyPathWithFilter(Function getFilter, List e // create guardrails for the first trace var guardrail = guardrailsGenerator.generateGuardrailsForTrace( - traceForFilter.getFirst().id(), randomUUID(), projectName).getFirst().toBuilder() + traceForFilter.getFirst().id(), generator.generate(), projectName).getFirst().toBuilder() .result(GuardrailResult.PASSED) .build(); @@ -1685,7 +1687,7 @@ private Long createTracesWithGuardrails(String projectName, Instant marker) { return traces.stream() .map(trace -> { List guardrails = guardrailsGenerator.generateGuardrailsForTrace(trace.id(), - randomUUID(), + generator.generate(), trace.projectName()); guardrailsResourceClient.addBatch(guardrails, API_KEY, WORKSPACE_NAME); return guardrails; @@ -1711,7 +1713,8 @@ private Pair, List> createTracesWithGuardrails(String projectN List guardrailCounts = traces.stream() .map(trace -> { - var guardrails = guardrailsGenerator.generateGuardrailsForTrace(trace.id(), randomUUID(), + var guardrails = guardrailsGenerator.generateGuardrailsForTrace(trace.id(), + generator.generate(), trace.projectName()); var guardrailsWithAtLeastOneFailed = IntStream.range(0, guardrails.size()) .mapToObj(i -> i == 0 @@ -3515,7 +3518,7 @@ void happyPathWithFilter(Function getFilter, List e traceResourceClient.feedbackScores(scores, API_KEY, WORKSPACE_NAME); var guardrail = guardrailsGenerator.generateGuardrailsForTrace( - traceForFilter.getFirst().id(), randomUUID(), projectName).getFirst().toBuilder() + traceForFilter.getFirst().id(), generator.generate(), projectName).getFirst().toBuilder() .result(GuardrailResult.PASSED) .build(); guardrailsResourceClient.addBatch(List.of(guardrail), API_KEY, WORKSPACE_NAME); @@ -3633,7 +3636,7 @@ void happyPathWithFilter(Function getFilter, List e traceResourceClient.feedbackScores(scores, API_KEY, WORKSPACE_NAME); var guardrail = guardrailsGenerator.generateGuardrailsForTrace( - traceForFilter.id(), randomUUID(), projectName).getFirst().toBuilder() + traceForFilter.id(), generator.generate(), projectName).getFirst().toBuilder() .result(GuardrailResult.PASSED) .build(); guardrailsResourceClient.addBatch(List.of(guardrail), API_KEY, WORKSPACE_NAME); diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ProjectMetricsWithBreakdownResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ProjectMetricsWithBreakdownResourceTest.java index 1d317207ec7..7b176302b24 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ProjectMetricsWithBreakdownResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ProjectMetricsWithBreakdownResourceTest.java @@ -31,6 +31,8 @@ import com.comet.opik.infrastructure.DatabaseAnalyticsFactory; import com.comet.opik.podam.PodamFactoryUtils; import com.comet.opik.utils.JsonUtils; +import com.fasterxml.uuid.Generators; +import com.fasterxml.uuid.impl.TimeBasedEpochGenerator; import com.redis.testcontainers.RedisContainer; import io.dropwizard.jersey.validation.ValidationErrorMessage; import jakarta.ws.rs.client.Entity; @@ -116,6 +118,7 @@ class ProjectMetricsWithBreakdownResourceTest { } private final PodamFactory factory = PodamFactoryUtils.newPodamFactory(); + private final TimeBasedEpochGenerator generator = Generators.timeBasedEpochGenerator(); private IdGenerator idGenerator; private String baseURI; @@ -1412,7 +1415,7 @@ private void createTracesWithGuardrails(String projectName, Instant marker, Brea traceResourceClient.createTrace(trace, API_KEY, WORKSPACE_NAME); // Add guardrail (alternating between pass and fail) - var guardrails = guardrailsGenerator.generateGuardrailsForTrace(trace.id(), UUID.randomUUID(), + var guardrails = guardrailsGenerator.generateGuardrailsForTrace(trace.id(), generator.generate(), projectName); // Set result to FAILED for even indices, PASSED for odd var guardrailsWithResult = guardrails.stream() diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ProjectsResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ProjectsResourceTest.java index 1dee24a875e..3f07e8cd358 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ProjectsResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ProjectsResourceTest.java @@ -59,6 +59,8 @@ import com.comet.opik.podam.PodamFactoryUtils; import com.comet.opik.utils.JsonUtils; import com.comet.opik.utils.ValidationUtils; +import com.fasterxml.uuid.Generators; +import com.fasterxml.uuid.impl.TimeBasedEpochGenerator; import com.github.tomakehurst.wiremock.client.WireMock; import com.redis.testcontainers.RedisContainer; import jakarta.ws.rs.HttpMethod; @@ -131,7 +133,6 @@ 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; -import static java.util.UUID.randomUUID; import static java.util.stream.Collectors.averagingDouble; import static java.util.stream.Collectors.groupingBy; import static java.util.stream.Collectors.toMap; @@ -184,6 +185,7 @@ class ProjectsResourceTest { } private final PodamFactory factory = PodamFactoryUtils.newPodamFactory(); + private final TimeBasedEpochGenerator generator = Generators.timeBasedEpochGenerator(); private String baseURI; private ClientSupport client; @@ -1857,7 +1859,7 @@ private Project buildProjectStats(Project project, String apiKey, String workspa var guardrailsByTraceId = traces.stream() .collect(Collectors.toMap(Trace::id, trace -> guardrailsGenerator.generateGuardrailsForTrace( - trace.id(), randomUUID(), trace.projectName()))); + trace.id(), generator.generate(), trace.projectName()))); guardrailsByTraceId.values().forEach(guardrail -> guardrailsResourceClient.addBatch( guardrail, apiKey, workspaceName)); From f1e48d33c7591962ef2b7cc2630584f719540425 Mon Sep 17 00:00:00 2001 From: Thiago Hora Date: Wed, 22 Jul 2026 11:21:55 +0200 Subject: [PATCH 5/7] [OPIK-7352] [BE] drop projectId validation where input isn't persisted; fix test data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address reviewer + CI findings: - Remove projectId validation in GuardrailsService and AssertionResultService: both resolve the project by name and overwrite projectId with the resolved project.id() before persistence, so the client-supplied projectId is never stored — validating it only breaks clients that send arbitrary project ids. - Fix remaining test data to use current UUIDv7 (never v4 or future-dated) for validated referenced ids: span traceId, dataset-item trace/span ids, annotation-queue item ids, feedback sourceQueueId, alert projectId, automation-rule projectIds, experiment optimizationId. 404/409 negative tests use a v7-but-nonexistent id to preserve their intent. - Strengthen createWithOldTraceIdSucceeds to round-trip and assert the old traceId is persisted verbatim; add a batch non-v7 parentSpanId rejection test. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../opik/domain/AssertionResultService.java | 5 +---- .../comet/opik/domain/GuardrailsService.java | 1 - .../v1/priv/AnnotationQueuesResourceTest.java | 17 ++++++++------ .../AutomationRuleEvaluatorsResourceTest.java | 2 +- .../DatasetsResourceCreateFromSpansTest.java | 2 +- .../DatasetsResourceCreateFromTracesTest.java | 2 +- ...ntsResourceFindProjectExperimentsTest.java | 5 ++++- .../v1/priv/FindSpansResourceTest.java | 4 ++-- .../priv/MultiValueFeedbackScoresE2ETest.java | 11 ++++++---- .../resources/v1/priv/SpansResourceTest.java | 22 ++++++++++++++++++- .../v1/priv/WorkspaceVersionResourceTest.java | 9 +++++--- 11 files changed, 54 insertions(+), 26 deletions(-) diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/AssertionResultService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/AssertionResultService.java index 9c874b9f29f..2eb95279520 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/AssertionResultService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/AssertionResultService.java @@ -64,10 +64,7 @@ public Mono saveBatch(@NonNull EntityType entityType, } // Validate up front so a bad id fails fast and independently of project-name normalisation. - assertionResults.forEach(item -> { - idGenerator.validateIdNotInFuture(item.entityId(), entityType.getType()); - idGenerator.validateIdNotInFutureIfPresent(item.projectId(), "project"); - }); + assertionResults.forEach(item -> idGenerator.validateIdNotInFuture(item.entityId(), entityType.getType())); return Mono.deferContextual(ctx -> { String workspaceId = ctx.get(RequestContext.WORKSPACE_ID); diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/GuardrailsService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/GuardrailsService.java index 5faae049098..df899dd553c 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/GuardrailsService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/GuardrailsService.java @@ -53,7 +53,6 @@ public Mono addTraceGuardrails(List guardrails) { UUID id = idGenerator.generateId(); idGenerator.validateIdNotInFuture(guardrail.entityId(), entityType.getType()); idGenerator.validateIdNotInFuture(guardrail.secondaryId(), "guardrail secondary"); - idGenerator.validateIdNotInFutureIfPresent(guardrail.projectId(), "project"); return guardrail.toBuilder() .id(id) diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/AnnotationQueuesResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/AnnotationQueuesResourceTest.java index d62f1160bee..211fd4a9de7 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/AnnotationQueuesResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/AnnotationQueuesResourceTest.java @@ -33,6 +33,8 @@ import com.comet.opik.infrastructure.auth.WorkspaceUserPermission; import com.comet.opik.infrastructure.db.TransactionTemplateAsync; import com.comet.opik.podam.PodamFactoryUtils; +import com.fasterxml.uuid.Generators; +import com.fasterxml.uuid.impl.TimeBasedEpochGenerator; import com.redis.testcontainers.RedisContainer; import org.apache.hc.core5.http.HttpStatus; import org.assertj.core.api.recursive.comparison.RecursiveComparisonConfiguration; @@ -122,6 +124,7 @@ class AnnotationQueuesResourceTest { } private final PodamFactory factory = PodamFactoryUtils.newPodamFactory(); + private static final TimeBasedEpochGenerator GENERATOR = Generators.timeBasedEpochGenerator(); private AnnotationQueue newAnnotationQueue() { return factory.manufacturePojo(AnnotationQueue.class) @@ -550,9 +553,9 @@ void addItemsToAnnotationQueue() { // Generate some item IDs to add var itemIds = Set.of( - UUID.randomUUID(), - UUID.randomUUID(), - UUID.randomUUID()); + GENERATOR.generate(), + GENERATOR.generate(), + GENERATOR.generate()); // When & Then annotationQueuesResourceClient.addItemsToAnnotationQueue( @@ -578,8 +581,8 @@ void removeItemsFromAnnotationQueue() { // Generate some item IDs to add first, then remove var itemIds = Set.of( - UUID.randomUUID(), - UUID.randomUUID()); + GENERATOR.generate(), + GENERATOR.generate()); // Add items first annotationQueuesResourceClient.addItemsToAnnotationQueue( @@ -598,8 +601,8 @@ void removeItemsFromAnnotationQueue() { @DisplayName("should return 404 when adding items to non-existent annotation queue") void addItemsToAnnotationQueueWhenQueueNotExistsShouldReturn404() { // Given - Non-existent queue ID - var nonExistentQueueId = UUID.randomUUID(); - var itemIds = Set.of(UUID.randomUUID()); + var nonExistentQueueId = GENERATOR.generate(); + var itemIds = Set.of(GENERATOR.generate()); // When & Then annotationQueuesResourceClient.addItemsToAnnotationQueue( diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/AutomationRuleEvaluatorsResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/AutomationRuleEvaluatorsResourceTest.java index 1f2c6e2122a..c0c8a2c15b6 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/AutomationRuleEvaluatorsResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/AutomationRuleEvaluatorsResourceTest.java @@ -425,7 +425,7 @@ void setUp() { @DisplayName("create evaluator definition: when api key is present, then return proper response") void createAutomationRuleEvaluator__whenSessionTokenIsPresent__thenReturnProperResponse( String sessionToken, boolean isAuthorized, String workspaceName) { - var projectId = UUID.randomUUID(); + var projectId = generator.generate(); var ruleEvaluator = factory.manufacturePojo(AutomationRuleEvaluatorLlmAsJudge.class).toBuilder() .projectIds(Set.of(projectId)) .build(); diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/DatasetsResourceCreateFromSpansTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/DatasetsResourceCreateFromSpansTest.java index c8e0cd91fb6..b47f0a1a8d3 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/DatasetsResourceCreateFromSpansTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/DatasetsResourceCreateFromSpansTest.java @@ -368,7 +368,7 @@ void createDatasetItemsFromSpans__whenDatasetNotFound__thenReturn404() { UUID nonExistentDatasetId = UUID.randomUUID(); var request = CreateDatasetItemsFromSpansRequest.builder() - .spanIds(Set.of(UUID.randomUUID())) + .spanIds(Set.of(GENERATOR.generate())) .enrichmentOptions( SpanEnrichmentOptions.builder().build()) .build(); diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/DatasetsResourceCreateFromTracesTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/DatasetsResourceCreateFromTracesTest.java index b175f09c400..ea304d19c4f 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/DatasetsResourceCreateFromTracesTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/DatasetsResourceCreateFromTracesTest.java @@ -351,7 +351,7 @@ void createDatasetItemsFromTraces__whenDatasetNotFound__thenReturn404() { UUID nonExistentDatasetId = UUID.randomUUID(); var request = CreateDatasetItemsFromTracesRequest.builder() - .traceIds(Set.of(UUID.randomUUID())) + .traceIds(Set.of(GENERATOR.generate())) .enrichmentOptions( TraceEnrichmentOptions.builder().build()) .build(); diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ExperimentsResourceFindProjectExperimentsTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ExperimentsResourceFindProjectExperimentsTest.java index 06ed3ef76a7..365eca1d7be 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ExperimentsResourceFindProjectExperimentsTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ExperimentsResourceFindProjectExperimentsTest.java @@ -39,6 +39,8 @@ import com.comet.opik.extensions.RegisterApp; import com.comet.opik.podam.PodamFactoryUtils; import com.comet.opik.utils.JsonUtils; +import com.fasterxml.uuid.Generators; +import com.fasterxml.uuid.impl.TimeBasedEpochGenerator; import com.google.common.eventbus.EventBus; import org.apache.commons.collections4.MapUtils; import org.apache.commons.lang3.RandomStringUtils; @@ -110,6 +112,7 @@ class ExperimentsResourceFindProjectExperimentsTest { private final TestDropwizardAppExtension APP = setup.APP; private final PodamFactory factory = PodamFactoryUtils.newPodamFactory(); + private final TimeBasedEpochGenerator generator = Generators.timeBasedEpochGenerator(); private String baseURI; private ExperimentResourceClient experimentResourceClient; @@ -1120,7 +1123,7 @@ void findByOptimizationIdAndType(ExperimentType type) { var project = factory.manufacturePojo(Project.class); var projectId = projectResourceClient.createProject(project, apiKey, workspaceName); - UUID optimizationId = UUID.randomUUID(); + UUID optimizationId = generator.generate(); var experiments = experimentResourceClient.generateExperimentList() .stream() diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/FindSpansResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/FindSpansResourceTest.java index 0fa794be813..6ff91e47975 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/FindSpansResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/FindSpansResourceTest.java @@ -1506,7 +1506,7 @@ void whenFilterTraceIdEqual__thenReturnSpansFiltered(String endpoint, SpanPageTe mockTargetWorkspace(apiKey, workspaceName, workspaceId); var projectName = generator.generate().toString(); - var traceId = UUID.randomUUID(); + var traceId = generator.generate(); // Create 5 spans with the same trace_id (these should be returned by the filter) var expectedSpanCount = 5; @@ -1528,7 +1528,7 @@ void whenFilterTraceIdEqual__thenReturnSpansFiltered(String endpoint, SpanPageTe .mapToObj(i -> podamFactory.manufacturePojo(Span.class).toBuilder() .projectId(null) .projectName(projectName) - .traceId(UUID.randomUUID()) + .traceId(generator.generate()) .name("other-span-" + i) .feedbackScores(null) .totalEstimatedCost(null) diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/MultiValueFeedbackScoresE2ETest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/MultiValueFeedbackScoresE2ETest.java index fd8ca6b5b68..af080b1a690 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/MultiValueFeedbackScoresE2ETest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/MultiValueFeedbackScoresE2ETest.java @@ -51,6 +51,8 @@ import com.comet.opik.extensions.RegisterApp; import com.comet.opik.infrastructure.auth.RequestContext; import com.comet.opik.podam.PodamFactoryUtils; +import com.fasterxml.uuid.Generators; +import com.fasterxml.uuid.impl.TimeBasedEpochGenerator; import com.redis.testcontainers.RedisContainer; import org.apache.commons.lang3.RandomStringUtils; import org.awaitility.Awaitility; @@ -125,6 +127,7 @@ class MultiValueFeedbackScoresE2ETest { } private final PodamFactory factory = PodamFactoryUtils.newPodamFactory(); + private final TimeBasedEpochGenerator generator = Generators.timeBasedEpochGenerator(); private TraceResourceClient traceResourceClient; private SpanResourceClient spanResourceClient; @@ -290,8 +293,8 @@ void deleteTraceFeedbackScoreScopedBySourceQueueId() { .build(); var traceId = traceResourceClient.createTrace(trace, API_KEY1, TEST_WORKSPACE); - var queueIdA = randomUUID(); - var queueIdB = randomUUID(); + var queueIdA = generator.generate(); + var queueIdB = generator.generate(); // Score from queue A var score = factory.manufacturePojo(FeedbackScore.class).toBuilder() @@ -343,8 +346,8 @@ void sameAuthorTwoQueuesProducesAverage() { .build(); var traceId = traceResourceClient.createTrace(trace, API_KEY1, TEST_WORKSPACE); - var queueIdA = randomUUID(); - var queueIdB = randomUUID(); + var queueIdA = generator.generate(); + var queueIdB = generator.generate(); var scoreName = randomUUID().toString(); var scoreFromQueueA = factory.manufacturePojo(FeedbackScore.class).toBuilder() diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/SpansResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/SpansResourceTest.java index 03cc568f410..3bff5c070dc 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/SpansResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/SpansResourceTest.java @@ -1848,14 +1848,20 @@ void createWithNonV7ParentSpanIdThrowsBadRequest() { @DisplayName("Create span with an old (past) v7 traceId succeeds — late spans on old traces are valid") void createWithOldTraceIdSucceeds() { var old = Instant.now().minus(Duration.ofDays(30)).toEpochMilli(); + var oldTraceId = generator.construct(old); var span = podamFactory.manufacturePojo(Span.class).toBuilder() - .traceId(generator.construct(old)) + .traceId(oldTraceId) .parentSpanId(null) .build(); var id = spanResourceClient.createSpan(span, API_KEY, TEST_WORKSPACE); assertThat(id).isNotNull(); + + // Round-trip: the old traceId must be persisted verbatim (not rewritten) and parentSpanId stays null. + var retrievedSpan = spanResourceClient.getById(id, TEST_WORKSPACE, API_KEY); + assertThat(retrievedSpan.traceId()).isEqualTo(oldTraceId); + assertThat(retrievedSpan.parentSpanId()).isNull(); } @Test @@ -2364,6 +2370,20 @@ void batchCreateWithInvalidTraceIdThrowsBadRequest(UUID traceId, String expected } } + @Test + @DisplayName("Batch create span with non-v7 parentSpanId throws bad request") + void batchCreateWithNonV7ParentSpanIdThrowsBadRequest() { + var expectedEntity = new io.dropwizard.jersey.errors.ErrorMessage( + HttpStatus.SC_BAD_REQUEST, "Invalid UUID for id", "Span parent id must be a version 7 UUID"); + var span = podamFactory.manufacturePojo(Span.class).toBuilder().parentSpanId(UUID.randomUUID()).build(); + try (var response = spanResourceClient.callBatchCreateSpans( + List.of(span), API_KEY, TEST_WORKSPACE)) { + assertThat(response.getStatusInfo().getStatusCode()).isEqualTo(HttpStatus.SC_BAD_REQUEST); + var actualEntity = response.readEntity(io.dropwizard.jersey.errors.ErrorMessage.class); + assertThat(actualEntity).isEqualTo(expectedEntity); + } + } + private long readClickHouseErrorCount(TransactionTemplateAsync templateAsync, int errorCode) { return templateAsync.nonTransaction(connection -> { var statement = connection.createStatement( diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/WorkspaceVersionResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/WorkspaceVersionResourceTest.java index 8a165bbe994..bf8ada0316f 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/WorkspaceVersionResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/WorkspaceVersionResourceTest.java @@ -29,6 +29,8 @@ import com.comet.opik.extensions.DropwizardAppExtensionProvider; import com.comet.opik.extensions.RegisterApp; import com.comet.opik.podam.PodamFactoryUtils; +import com.fasterxml.uuid.Generators; +import com.fasterxml.uuid.impl.TimeBasedEpochGenerator; import com.redis.testcontainers.RedisContainer; import org.apache.commons.lang3.RandomStringUtils; import org.awaitility.Awaitility; @@ -83,6 +85,7 @@ class WorkspaceVersionResourceTest { .build(); private final PodamFactory podamFactory = PodamFactoryUtils.newPodamFactory(); + private final TimeBasedEpochGenerator generator = Generators.timeBasedEpochGenerator(); @Nested @TestInstance(TestInstance.Lifecycle.PER_CLASS) @@ -526,7 +529,7 @@ void workspaceVersion__whenMultiProjectRule__returnsVersion1() { // Single-project rule does not trigger version_1 evaluatorClient.createEvaluator(podamFactory.manufacturePojo(AutomationRuleEvaluatorLlmAsJudge.class) .toBuilder() - .projectIds(Set.of(UUID.randomUUID())) + .projectIds(Set.of(generator.generate())) .build(), workspaceName, API_KEY); assertThat(workspaceClient.getWorkspaceVersion(API_KEY, workspaceName)).isEqualTo(V2_WORKSPACE_VERSION); @@ -534,7 +537,7 @@ void workspaceVersion__whenMultiProjectRule__returnsVersion1() { // Multi-project rule triggers version_1 evaluatorClient.createEvaluator(podamFactory.manufacturePojo(AutomationRuleEvaluatorLlmAsJudge.class) .toBuilder() - .projectIds(Set.of(UUID.randomUUID(), UUID.randomUUID())) + .projectIds(Set.of(generator.generate(), generator.generate())) .build(), workspaceName, API_KEY); assertThat(workspaceClient.getWorkspaceVersion(API_KEY, workspaceName)).isEqualTo(V1_WORKSPACE_VERSION); @@ -597,7 +600,7 @@ void workspaceVersion__whenAlertWithoutProject__returnsVersion1() { // Project-scoped alert (projectId column) does not trigger version_1 alertClient.createAlert( - AlertResourceTest.generateAlertForProject(podamFactory, UUID.randomUUID()), + AlertResourceTest.generateAlertForProject(podamFactory, generator.generate()), API_KEY, workspaceName, 201); assertThat(workspaceClient.getWorkspaceVersion(API_KEY, workspaceName)).isEqualTo(V2_WORKSPACE_VERSION); From 72118b6e9c64ea3608ad9d03a18f3ba41729d42a Mon Sep 17 00:00:00 2001 From: Thiago Hora Date: Wed, 22 Jul 2026 11:33:36 +0200 Subject: [PATCH 6/7] [OPIK-7352] [BE] validate span batch ids before side effects; dedupe checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses reviewer findings: - create(SpanBatch) validated traceId/parentSpanId inside bindSpanToProjectAndId, which runs AFTER deleteAutoStrippedAttachments + project getOrCreate — so a bad batch could delete attachments / create projects before failing 400. Move the id checks up front (before any side effect) so a rejected batch never mutates state. - Extract a shared validateSpanReferences(traceId, parentSpanId) helper used by every span write path (single/batch create, single/batch update) so the reference-id rules can't drift between them. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../com/comet/opik/domain/SpanService.java | 32 +++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/SpanService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/SpanService.java index a90d012767c..cbe4974408d 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/SpanService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/SpanService.java @@ -161,8 +161,7 @@ public Mono create(@NonNull Span span) { var projectName = WorkspaceUtils.getProjectName(span.projectName()); return idGenerator .validateIdAsync(id, SPAN_KEY) - .then(idGenerator.validateIdNotInFutureAsync(span.traceId(), SPAN_TRACE_KEY)) - .then(idGenerator.validateIdNotInFutureIfPresentAsync(span.parentSpanId(), SPAN_PARENT_KEY)) + .then(Mono.fromRunnable(() -> validateSpanReferences(span.traceId(), span.parentSpanId()))) .then(projectService.getOrCreate(projectName)) .flatMap(project -> lockService.executeWithLock( new LockService.Lock(id, SPAN_KEY), @@ -225,8 +224,8 @@ public Mono update(@NonNull UUID id, @NonNull SpanUpdate spanUpdate) { return idGenerator .validateIdNotInFutureAsync(id, SPAN_KEY) - .then(idGenerator.validateIdNotInFutureAsync(spanUpdate.traceId(), SPAN_TRACE_KEY)) - .then(idGenerator.validateIdNotInFutureIfPresentAsync(spanUpdate.parentSpanId(), SPAN_PARENT_KEY)) + .then(Mono.fromRunnable( + () -> validateSpanReferences(spanUpdate.traceId(), spanUpdate.parentSpanId()))) .then(idGenerator.validateIdNotInFutureIfPresentAsync(spanUpdate.projectId(), "project")) .then(Mono.defer(() -> getProjectById(spanUpdate) .switchIfEmpty(Mono.defer(() -> projectService.getOrCreate(projectName))) @@ -254,9 +253,9 @@ public Mono batchUpdate(@NonNull SpanBatchUpdate batchUpdate) { String workspaceId = ctx.get(RequestContext.WORKSPACE_ID); String userName = ctx.get(RequestContext.USER_NAME); - return idGenerator.validateIdNotInFutureAsync(batchUpdate.update().traceId(), SPAN_TRACE_KEY) - .then(idGenerator.validateIdNotInFutureIfPresentAsync(batchUpdate.update().parentSpanId(), - SPAN_PARENT_KEY)) + return Mono + .fromRunnable(() -> validateSpanReferences(batchUpdate.update().traceId(), + batchUpdate.update().parentSpanId())) .then(idGenerator.validateIdNotInFutureIfPresentAsync(batchUpdate.update().projectId(), "project")) .then(spanDAO.bulkUpdate(batchUpdate.ids(), batchUpdate.update(), mergeTags)) .onErrorResume(TagOperations::mapTagLimitError) @@ -378,6 +377,15 @@ public Mono create(@NonNull SpanBatch batch) { List dedupedSpans = dedupSpans(batch.spans()); + // Fail fast on invalid ids BEFORE any side effect below (auto-stripped attachment deletion, project + // creation), so a rejected batch never mutates state. + dedupedSpans.forEach(span -> { + if (span.id() != null) { + idGenerator.validateId(span.id(), SPAN_KEY); + } + validateSpanReferences(span.traceId(), span.parentSpanId()); + }); + List projectNames = dedupedSpans .stream() .map(Span::projectName) @@ -449,6 +457,13 @@ private List dedupSpans(List initialSpans) { return result; } + // Shared span reference-id policy: the trace (required) and parent (optional) must be time-ordered + // UUIDv7, past allowed. Used by every span write path so the rules can't drift between them. + private void validateSpanReferences(UUID traceId, UUID parentSpanId) { + idGenerator.validateIdNotInFuture(traceId, SPAN_TRACE_KEY); + idGenerator.validateIdNotInFutureIfPresent(parentSpanId, SPAN_PARENT_KEY); + } + private List bindSpanToProjectAndId(List spans, List projects) { Map projectPerName = projects.stream() .collect(Collectors.toMap( @@ -471,8 +486,7 @@ private List bindSpanToProjectAndId(List spans, List projec UUID id = span.id() == null ? idGenerator.generateId() : span.id(); idGenerator.validateId(id, SPAN_KEY); - idGenerator.validateIdNotInFuture(span.traceId(), SPAN_TRACE_KEY); - idGenerator.validateIdNotInFutureIfPresent(span.parentSpanId(), SPAN_PARENT_KEY); + // trace/parent references are validated up front in create(SpanBatch) before side effects. return span.toBuilder().id(id).projectId(project.id()).build(); }) From 76137a6b90aa99abc38ba2fe7573cbb7e929acad Mon Sep 17 00:00:00 2001 From: Thiago Hora Date: Wed, 22 Jul 2026 15:51:39 +0200 Subject: [PATCH 7/7] [OPIK-7352] [BE] scope config-id validation to unchecked refs; reviewer fixes Address reviewer feedback (andrescrz): - Drop UUIDv7 validation on referenced config ids that are already existence-checked (so a bad id fails with the proper 404/409, not 400): projectId on span/trace update, prompt, experiment, optimization, dashboard (all go through validateProjectIdExists / resolveProjectIdOrCreate / get); feedback-batch projectId (overwritten by the name-resolved project); datasetVersionId (FK-checked) and datasetId (findById). - Keep it where the ref is persisted WITHOUT an existence check, so a UUIDv4 orphan can't be ingested: alert projectId, annotation-queue projectId, automation-rule projectIds, experiment optimizationId, feedback sourceQueueId. - Move validateIdNotInFutureIfPresent(Async) bodies out of the IdGenerator interface into IdGeneratorImpl. - Tests: route added generators through TestIdGeneratorFactory (static final) instead of local TimeBasedEpochGenerator fields; parameterize the span invalid-reference tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../com/comet/opik/domain/AlertService.java | 1 + .../opik/domain/AnnotationQueueService.java | 1 + .../comet/opik/domain/DashboardService.java | 1 - .../comet/opik/domain/DatasetItemService.java | 2 - .../comet/opik/domain/ExperimentService.java | 3 +- .../opik/domain/FeedbackScoreService.java | 1 - .../com/comet/opik/domain/IdGenerator.java | 22 ++++--- .../opik/domain/OptimizationService.java | 1 - .../com/comet/opik/domain/PromptService.java | 2 - .../com/comet/opik/domain/SpanService.java | 2 - .../com/comet/opik/domain/TraceService.java | 4 +- .../AutomationRuleEvaluatorService.java | 1 + .../resources/v1/priv/AlertResourceTest.java | 16 ++--- .../v1/priv/AnnotationQueuesResourceTest.java | 20 +++--- .../v1/priv/DatasetVersionResourceTest.java | 7 +- ...ntsResourceFindProjectExperimentsTest.java | 8 +-- .../priv/GetTracesByProjectResourceTest.java | 10 +-- .../v1/priv/GuardrailsResourceTest.java | 14 ++-- .../priv/MultiValueFeedbackScoresE2ETest.java | 14 ++-- .../v1/priv/ProjectMetricsResourceTest.java | 23 ++++--- ...ojectMetricsWithBreakdownResourceTest.java | 7 +- .../v1/priv/ProjectsResourceTest.java | 8 +-- .../resources/v1/priv/SpansResourceTest.java | 64 +++++++------------ .../v1/priv/WorkspaceVersionResourceTest.java | 12 ++-- 24 files changed, 112 insertions(+), 132 deletions(-) diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/AlertService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/AlertService.java index 4cb0e658540..8c216be01fc 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/AlertService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/AlertService.java @@ -555,6 +555,7 @@ private Alert prepareAlert(Alert alert, String userName, String workspaceId) { UUID id = alert.id() == null ? idGenerator.generateId() : alert.id(); IdGenerator.validateVersion(id, "Alert"); + // projectId is persisted without an existence check here, so enforce v7 to avoid storing an orphan v4. idGenerator.validateIdNotInFutureIfPresent(alert.projectId(), "project"); UUID webhookId = alert.webhook().id() == null ? idGenerator.generateId() : alert.webhook().id(); diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/AnnotationQueueService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/AnnotationQueueService.java index 40c865ee580..85150bc7da2 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/AnnotationQueueService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/AnnotationQueueService.java @@ -261,6 +261,7 @@ private Mono enhancePageWithProjectNames( private AnnotationQueue prepareAnnotationQueue(AnnotationQueue annotationQueue) { UUID id = annotationQueue.id() == null ? idGenerator.generateId() : annotationQueue.id(); IdGenerator.validateVersion(id, "AnnotationQueue"); + // projectId is persisted without an existence check here, so enforce v7 to avoid storing an orphan v4. idGenerator.validateIdNotInFutureIfPresent(annotationQueue.projectId(), "project"); log.debug("Preparing annotation queue with id '{}', name '{}', project '{}'", diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/DashboardService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/DashboardService.java index 24daa474f07..8a9a1e1ae1f 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/DashboardService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/DashboardService.java @@ -83,7 +83,6 @@ public Dashboard create(@NonNull Dashboard dashboard, @NonNull DashboardScope sc // Generate ID if not provided var dashboardId = dashboard.id() != null ? dashboard.id() : idGenerator.generateId(); IdGenerator.validateVersion(dashboardId, "dashboard"); - idGenerator.validateIdNotInFutureIfPresent(dashboard.projectId(), "project"); final UUID resolvedProjectId; if (StringUtils.isNotBlank(dashboard.projectName()) && dashboard.projectId() == null) { diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/DatasetItemService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/DatasetItemService.java index 7795410c3df..b746de05d6d 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/DatasetItemService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/DatasetItemService.java @@ -1572,8 +1572,6 @@ private List prepareAddedItems(DatasetItemChanges changes, UUID dat @WithSpan public Mono save(@NonNull DatasetItemBatch batch) { - idGenerator.validateIdNotInFutureIfPresent(batch.datasetId(), "dataset"); - if (!featureFlags.isDatasetVersioningEnabled()) { // Legacy: save to legacy table log.info("Saving items to legacy table for dataset '{}'", batch.datasetId()); diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/ExperimentService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/ExperimentService.java index 1aaedcba52e..48d0a0d87eb 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/ExperimentService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/ExperimentService.java @@ -495,9 +495,8 @@ private Set getPromptVersionIds(Experiment experiment) { public Mono create(@NonNull Experiment experiment) { var id = experiment.id() == null ? idGenerator.generateId() : experiment.id(); IdGenerator.validateVersion(id, "Experiment"); - idGenerator.validateIdNotInFutureIfPresent(experiment.projectId(), "project"); + // optimizationId is stored without an existence check, so enforce v7 to avoid storing an orphan v4. idGenerator.validateIdNotInFutureIfPresent(experiment.optimizationId(), "optimization"); - idGenerator.validateIdNotInFutureIfPresent(experiment.datasetVersionId(), "dataset version"); var name = StringUtils.getIfBlank(experiment.name(), nameGenerator::generateName); return resolveProjectId(experiment) .flatMap(resolvedExperiment -> datasetService diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/FeedbackScoreService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/FeedbackScoreService.java index 642f73510d4..787aec1faf0 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/FeedbackScoreService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/FeedbackScoreService.java @@ -179,7 +179,6 @@ private Mono processScoreBatch(EntityType entityType, List { idGenerator.validateIdNotInFuture(score.id(), entityType.getType()); // validate span/trace id - idGenerator.validateIdNotInFutureIfPresent(score.projectId(), "project"); idGenerator.validateIdNotInFutureIfPresent(score.sourceQueueId(), "annotation queue"); return score.toBuilder() diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/IdGenerator.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/IdGenerator.java index a4ac620a266..8a754db3653 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/IdGenerator.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/IdGenerator.java @@ -52,15 +52,9 @@ public interface IdGenerator { * Null-safe variant of {@link #validateIdNotInFuture} for optional referenced ids (e.g. an optional * {@code projectId} that may be resolved by name instead). No-op when {@code id} is null. */ - default void validateIdNotInFutureIfPresent(UUID id, String resource) { - if (id != null) { - validateIdNotInFuture(id, resource); - } - } + void validateIdNotInFutureIfPresent(UUID id, String resource); - default Mono validateIdNotInFutureIfPresentAsync(UUID id, String resource) { - return id == null ? Mono.empty() : validateIdNotInFutureAsync(id, resource); - } + Mono validateIdNotInFutureIfPresentAsync(UUID id, String resource); static Mono validateVersionAsync(@NonNull UUID id, String resource) { return Mono.fromCallable(() -> { @@ -125,4 +119,16 @@ public Mono validateIdNotInFutureAsync(@NonNull UUID id, String resource) return id; }); } + + @Override + public void validateIdNotInFutureIfPresent(UUID id, String resource) { + if (id != null) { + validateIdNotInFuture(id, resource); + } + } + + @Override + public Mono validateIdNotInFutureIfPresentAsync(UUID id, String resource) { + return id == null ? Mono.empty() : validateIdNotInFutureAsync(id, resource); + } } diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/OptimizationService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/OptimizationService.java index 014b09af622..00131cc440c 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/OptimizationService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/OptimizationService.java @@ -176,7 +176,6 @@ private OptimizationSearchCriteria resolveDatasetNameFilter( public Mono upsert(@NonNull Optimization optimization) { UUID id = optimization.id() == null ? idGenerator.generateId() : optimization.id(); IdGenerator.validateVersion(id, "Optimization"); - idGenerator.validateIdNotInFutureIfPresent(optimization.projectId(), "project"); // Detect if this is a Studio optimization (has studioConfig in the request) boolean isStudioOptimization = optimization.studioConfig() != null; diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/PromptService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/PromptService.java index 22853f2e4b1..da883242466 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/PromptService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/PromptService.java @@ -214,7 +214,6 @@ private PromptVersion createPromptVersionFromPromptRequest(Prompt createdPrompt, private Prompt savePrompt(String workspaceId, Prompt prompt) { IdGenerator.validateVersion(prompt.id(), "prompt"); - idGenerator.validateIdNotInFutureIfPresent(prompt.projectId(), "project"); transactionTemplate.inTransaction(WRITE, handle -> { PromptDAO promptDAO = handle.attach(PromptDAO.class); @@ -333,7 +332,6 @@ public PromptVersion createPromptVersion(@NonNull CreatePromptVersion createProm : createPromptVersion.version().commit(); IdGenerator.validateVersion(id, "prompt version"); - idGenerator.validateIdNotInFutureIfPresent(createPromptVersion.projectId(), "project"); TemplateStructure templateStructure = createPromptVersion.templateStructure(); diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/SpanService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/SpanService.java index cbe4974408d..276a716a3c2 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/SpanService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/SpanService.java @@ -226,7 +226,6 @@ public Mono update(@NonNull UUID id, @NonNull SpanUpdate spanUpdate) { .validateIdNotInFutureAsync(id, SPAN_KEY) .then(Mono.fromRunnable( () -> validateSpanReferences(spanUpdate.traceId(), spanUpdate.parentSpanId()))) - .then(idGenerator.validateIdNotInFutureIfPresentAsync(spanUpdate.projectId(), "project")) .then(Mono.defer(() -> getProjectById(spanUpdate) .switchIfEmpty(Mono.defer(() -> projectService.getOrCreate(projectName))) .subscribeOn(Schedulers.boundedElastic())) @@ -256,7 +255,6 @@ public Mono batchUpdate(@NonNull SpanBatchUpdate batchUpdate) { return Mono .fromRunnable(() -> validateSpanReferences(batchUpdate.update().traceId(), batchUpdate.update().parentSpanId())) - .then(idGenerator.validateIdNotInFutureIfPresentAsync(batchUpdate.update().projectId(), "project")) .then(spanDAO.bulkUpdate(batchUpdate.ids(), batchUpdate.update(), mergeTags)) .onErrorResume(TagOperations::mapTagLimitError) .doOnSuccess(__ -> { diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/TraceService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/TraceService.java index 437c8043703..9cf964bc1cf 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/TraceService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/TraceService.java @@ -323,7 +323,6 @@ public Mono update(@NonNull TraceUpdate traceUpdate, @NonNull UUID id) { return Mono.deferContextual(ctx -> idGenerator .validateIdNotInFutureAsync(id, TRACE_KEY) - .then(idGenerator.validateIdNotInFutureIfPresentAsync(traceUpdate.projectId(), "project")) .then(getProjectById(traceUpdate) .switchIfEmpty(Mono.defer(() -> projectService.getOrCreate(projectName))) .subscribeOn(Schedulers.boundedElastic()) @@ -359,8 +358,7 @@ public Mono batchUpdate(@NonNull TraceBatchUpdate batchUpdate) { String workspaceId = ctx.get(RequestContext.WORKSPACE_ID); String userName = ctx.get(RequestContext.USER_NAME); String workspaceName = ctx.getOrDefault(RequestContext.WORKSPACE_NAME, ""); - return idGenerator.validateIdNotInFutureIfPresentAsync(batchUpdate.update().projectId(), "project") - .then(dao.getProjectIdsByTraceIds(new ArrayList<>(batchUpdate.ids()))) + return dao.getProjectIdsByTraceIds(new ArrayList<>(batchUpdate.ids())) .flatMap(traceToProjectMap -> { var projectIds = Set.copyOf(traceToProjectMap.values()); return dao.bulkUpdate(batchUpdate.ids(), batchUpdate.update(), mergeTags) diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/evaluators/AutomationRuleEvaluatorService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/evaluators/AutomationRuleEvaluatorService.java index 7b9ac648e9d..af4559509c9 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/evaluators/AutomationRuleEvaluatorService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/evaluators/AutomationRuleEvaluatorService.java @@ -117,6 +117,7 @@ public > T save(@No UUID id = idGenerator.generateId(); IdGenerator.validateVersion(id, "AutomationRuleEvaluator"); + // projectIds are persisted without an existence check, so enforce v7 to avoid storing orphan v4 ids. projectIds.forEach(projectId -> idGenerator.validateIdNotInFutureIfPresent(projectId, "project")); // Dual-field sync: First projectId becomes the legacy project_id field diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/AlertResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/AlertResourceTest.java index b4ec843436f..3e310eb5331 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/AlertResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/AlertResourceTest.java @@ -51,14 +51,14 @@ import com.comet.opik.api.sorting.SortableFields; import com.comet.opik.api.sorting.SortingField; import com.comet.opik.domain.GuardrailResult; +import com.comet.opik.domain.IdGenerator; +import com.comet.opik.domain.TestIdGeneratorFactory; import com.comet.opik.extensions.DropwizardAppExtensionProvider; import com.comet.opik.extensions.RegisterApp; import com.comet.opik.infrastructure.DatabaseAnalyticsFactory; import com.comet.opik.infrastructure.auth.WorkspaceUserPermission; import com.comet.opik.podam.PodamFactoryUtils; import com.comet.opik.utils.JsonUtils; -import com.fasterxml.uuid.Generators; -import com.fasterxml.uuid.impl.TimeBasedEpochGenerator; import com.github.tomakehurst.wiremock.WireMockServer; import com.redis.testcontainers.RedisContainer; import jakarta.ws.rs.core.HttpHeaders; @@ -171,7 +171,7 @@ class AlertResourceTest { } private final PodamFactory factory = PodamFactoryUtils.newPodamFactory(); - private final TimeBasedEpochGenerator generator = Generators.timeBasedEpochGenerator(); + private static final IdGenerator idGenerator = TestIdGeneratorFactory.create(); private AlertResourceClient alertResourceClient; private PromptResourceClient promptResourceClient; @@ -1788,7 +1788,7 @@ void whenGuardrailsAreTriggeredForTrace_thenWebhookIsCalledBasedOnProjectScope( // Create guardrails for the trace Guardrail guardrail = factory.manufacturePojo(Guardrail.class).toBuilder() .entityId(trace.id()) - .secondaryId(generator.generate()) + .secondaryId(idGenerator.generateId()) .projectName(projectName) .result(GuardrailResult.FAILED) .build(); @@ -1907,7 +1907,7 @@ void whenGuardrailScopedByProjectIdColumn__thenWebhookFiresOnlyForMatchingProjec Guardrail guardrailA = factory.manufacturePojo(Guardrail.class).toBuilder() .entityId(traceA.id()) - .secondaryId(generator.generate()) + .secondaryId(idGenerator.generateId()) .projectName(projectAName) .result(GuardrailResult.FAILED) .build(); @@ -1932,7 +1932,7 @@ void whenGuardrailScopedByProjectIdColumn__thenWebhookFiresOnlyForMatchingProjec Guardrail guardrailB = factory.manufacturePojo(Guardrail.class).toBuilder() .entityId(traceB.id()) - .secondaryId(generator.generate()) + .secondaryId(idGenerator.generateId()) .projectName(projectBName) .result(GuardrailResult.FAILED) .build(); @@ -2723,7 +2723,7 @@ void testGuardrailsTriggeredEvent(AlertType alertType) { List guardrails = IntStream.range(0, 2) .mapToObj(i -> factory.manufacturePojo(Guardrail.class).toBuilder() .entityId(trace.id()) - .secondaryId(generator.generate()) + .secondaryId(idGenerator.generateId()) .projectName(projectName) .projectId(projectId) .result(GuardrailResult.FAILED) @@ -2815,7 +2815,7 @@ void testGuardrailsTriggeredEventWithFallback() { return factory.manufacturePojo(Guardrail.class).toBuilder() .entityId(trace.id()) - .secondaryId(generator.generate()) + .secondaryId(idGenerator.generateId()) .projectName(projectName) .projectId(projectId) .result(GuardrailResult.FAILED) diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/AnnotationQueuesResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/AnnotationQueuesResourceTest.java index 211fd4a9de7..bd77f89f8f4 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/AnnotationQueuesResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/AnnotationQueuesResourceTest.java @@ -28,13 +28,13 @@ import com.comet.opik.api.sorting.Direction; import com.comet.opik.api.sorting.SortableFields; import com.comet.opik.api.sorting.SortingField; +import com.comet.opik.domain.IdGenerator; +import com.comet.opik.domain.TestIdGeneratorFactory; import com.comet.opik.extensions.DropwizardAppExtensionProvider; import com.comet.opik.extensions.RegisterApp; import com.comet.opik.infrastructure.auth.WorkspaceUserPermission; import com.comet.opik.infrastructure.db.TransactionTemplateAsync; import com.comet.opik.podam.PodamFactoryUtils; -import com.fasterxml.uuid.Generators; -import com.fasterxml.uuid.impl.TimeBasedEpochGenerator; import com.redis.testcontainers.RedisContainer; import org.apache.hc.core5.http.HttpStatus; import org.assertj.core.api.recursive.comparison.RecursiveComparisonConfiguration; @@ -124,7 +124,7 @@ class AnnotationQueuesResourceTest { } private final PodamFactory factory = PodamFactoryUtils.newPodamFactory(); - private static final TimeBasedEpochGenerator GENERATOR = Generators.timeBasedEpochGenerator(); + private static final IdGenerator idGenerator = TestIdGeneratorFactory.create(); private AnnotationQueue newAnnotationQueue() { return factory.manufacturePojo(AnnotationQueue.class) @@ -553,9 +553,9 @@ void addItemsToAnnotationQueue() { // Generate some item IDs to add var itemIds = Set.of( - GENERATOR.generate(), - GENERATOR.generate(), - GENERATOR.generate()); + idGenerator.generateId(), + idGenerator.generateId(), + idGenerator.generateId()); // When & Then annotationQueuesResourceClient.addItemsToAnnotationQueue( @@ -581,8 +581,8 @@ void removeItemsFromAnnotationQueue() { // Generate some item IDs to add first, then remove var itemIds = Set.of( - GENERATOR.generate(), - GENERATOR.generate()); + idGenerator.generateId(), + idGenerator.generateId()); // Add items first annotationQueuesResourceClient.addItemsToAnnotationQueue( @@ -601,8 +601,8 @@ void removeItemsFromAnnotationQueue() { @DisplayName("should return 404 when adding items to non-existent annotation queue") void addItemsToAnnotationQueueWhenQueueNotExistsShouldReturn404() { // Given - Non-existent queue ID - var nonExistentQueueId = GENERATOR.generate(); - var itemIds = Set.of(GENERATOR.generate()); + var nonExistentQueueId = idGenerator.generateId(); + var itemIds = Set.of(idGenerator.generateId()); // When & Then annotationQueuesResourceClient.addItemsToAnnotationQueue( diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/DatasetVersionResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/DatasetVersionResourceTest.java index 733fb8fb2ae..567d70d4697 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/DatasetVersionResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/DatasetVersionResourceTest.java @@ -47,7 +47,9 @@ import com.comet.opik.api.sorting.SortingField; import com.comet.opik.domain.DatasetVersionDAO; import com.comet.opik.domain.DatasetVersionService; +import com.comet.opik.domain.IdGenerator; import com.comet.opik.domain.SpanEnrichmentOptions; +import com.comet.opik.domain.TestIdGeneratorFactory; import com.comet.opik.domain.TraceEnrichmentOptions; import com.comet.opik.extensions.DropwizardAppExtensionProvider; import com.comet.opik.extensions.RegisterApp; @@ -2819,8 +2821,7 @@ void createFromTraces__whenRegularDataset__thenDataContainsAllEnrichedFields() { @TestInstance(TestInstance.Lifecycle.PER_CLASS) class ExperimentDatasetVersionLinking { - private final com.fasterxml.uuid.impl.TimeBasedEpochGenerator generator = com.fasterxml.uuid.Generators - .timeBasedEpochGenerator(); + private static final IdGenerator idGenerator = TestIdGeneratorFactory.create(); private Experiment getExperiment(UUID id) { return experimentResourceClient.getExperiment(id, API_KEY, TEST_WORKSPACE); @@ -3062,7 +3063,7 @@ void createExperiment_whenInvalidVersionId_thenConflict() { var datasetId = createDataset(datasetName); createDatasetItems(datasetId, 1); - var nonExistentVersionId = generator.generate(); + var nonExistentVersionId = idGenerator.generateId(); // when - create experiment with non-existent version ID var experiment = experimentResourceClient.createPartialExperiment() diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ExperimentsResourceFindProjectExperimentsTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ExperimentsResourceFindProjectExperimentsTest.java index 365eca1d7be..85ad50c9780 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ExperimentsResourceFindProjectExperimentsTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ExperimentsResourceFindProjectExperimentsTest.java @@ -34,13 +34,13 @@ import com.comet.opik.api.sorting.SortableFields; import com.comet.opik.api.sorting.SortingField; import com.comet.opik.domain.FeedbackScoreMapper; +import com.comet.opik.domain.IdGenerator; import com.comet.opik.domain.SpanType; +import com.comet.opik.domain.TestIdGeneratorFactory; import com.comet.opik.extensions.DropwizardAppExtensionProvider; import com.comet.opik.extensions.RegisterApp; import com.comet.opik.podam.PodamFactoryUtils; import com.comet.opik.utils.JsonUtils; -import com.fasterxml.uuid.Generators; -import com.fasterxml.uuid.impl.TimeBasedEpochGenerator; import com.google.common.eventbus.EventBus; import org.apache.commons.collections4.MapUtils; import org.apache.commons.lang3.RandomStringUtils; @@ -112,7 +112,7 @@ class ExperimentsResourceFindProjectExperimentsTest { private final TestDropwizardAppExtension APP = setup.APP; private final PodamFactory factory = PodamFactoryUtils.newPodamFactory(); - private final TimeBasedEpochGenerator generator = Generators.timeBasedEpochGenerator(); + private static final IdGenerator idGenerator = TestIdGeneratorFactory.create(); private String baseURI; private ExperimentResourceClient experimentResourceClient; @@ -1123,7 +1123,7 @@ void findByOptimizationIdAndType(ExperimentType type) { var project = factory.manufacturePojo(Project.class); var projectId = projectResourceClient.createProject(project, apiKey, workspaceName); - UUID optimizationId = generator.generate(); + UUID optimizationId = idGenerator.generateId(); var experiments = experimentResourceClient.generateExperimentList() .stream() diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/GetTracesByProjectResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/GetTracesByProjectResourceTest.java index 6f61eddfc6f..8232813e977 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/GetTracesByProjectResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/GetTracesByProjectResourceTest.java @@ -53,6 +53,7 @@ import com.comet.opik.domain.GuardrailsMapper; import com.comet.opik.domain.IdGenerator; import com.comet.opik.domain.SpanType; +import com.comet.opik.domain.TestIdGeneratorFactory; import com.comet.opik.domain.cost.CostService; import com.comet.opik.domain.filter.FilterQueryBuilder; import com.comet.opik.extensions.DropwizardAppExtensionProvider; @@ -60,8 +61,6 @@ import com.comet.opik.podam.PodamFactoryUtils; import com.comet.opik.utils.JsonUtils; import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.uuid.Generators; -import com.fasterxml.uuid.impl.TimeBasedEpochGenerator; import com.google.common.collect.Lists; import com.redis.testcontainers.RedisContainer; import jakarta.ws.rs.core.Response; @@ -173,7 +172,7 @@ class GetTracesByProjectResourceTest { } private final PodamFactory factory = PodamFactoryUtils.newPodamFactory(); - private final TimeBasedEpochGenerator generator = Generators.timeBasedEpochGenerator(); + private static final IdGenerator testIdGenerator = TestIdGeneratorFactory.create(); private final FilterQueryBuilder filterQueryBuilder = new FilterQueryBuilder(); private String baseURI; @@ -3949,7 +3948,7 @@ void whenFilterGuardrails__thenReturnTracesFiltered(String endpoint, TracePageTe var guardrailsByTraceId = traces.stream() .collect(Collectors.toMap(Trace::id, trace -> guardrailsGenerator.generateGuardrailsForTrace( - trace.id(), generator.generate(), trace.projectName()))); + trace.id(), testIdGenerator.generateId(), trace.projectName()))); // set the first trace with failed guardrails guardrailsByTraceId.put(traces.getFirst().id(), guardrailsByTraceId.get(traces.getFirst().id()).stream() @@ -5165,7 +5164,8 @@ void getTracesByProject__whenExcludeParamIdDefined__thenReturnSpanExcludingField .toList(); List guardrailsByTraceId = traces.stream() - .map(trace -> guardrailsGenerator.generateGuardrailsForTrace(trace.id(), generator.generate(), + .map(trace -> guardrailsGenerator.generateGuardrailsForTrace(trace.id(), + testIdGenerator.generateId(), trace.projectName())) .flatMap(Collection::stream) .toList(); diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/GuardrailsResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/GuardrailsResourceTest.java index 7e9c8a54727..1573cddb438 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/GuardrailsResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/GuardrailsResourceTest.java @@ -17,12 +17,12 @@ import com.comet.opik.api.resources.utils.resources.TraceResourceClient; import com.comet.opik.domain.GuardrailResult; import com.comet.opik.domain.GuardrailsMapper; +import com.comet.opik.domain.IdGenerator; +import com.comet.opik.domain.TestIdGeneratorFactory; import com.comet.opik.domain.stats.StatsMapper; import com.comet.opik.extensions.DropwizardAppExtensionProvider; import com.comet.opik.extensions.RegisterApp; import com.comet.opik.podam.PodamFactoryUtils; -import com.fasterxml.uuid.Generators; -import com.fasterxml.uuid.impl.TimeBasedEpochGenerator; import com.redis.testcontainers.RedisContainer; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -84,7 +84,7 @@ public class GuardrailsResourceTest { } private final PodamFactory factory = PodamFactoryUtils.newPodamFactory(); - private final TimeBasedEpochGenerator generator = Generators.timeBasedEpochGenerator(); + private static final IdGenerator idGenerator = TestIdGeneratorFactory.create(); private TraceResourceClient traceResourceClient; private GuardrailsResourceClient guardrailsResourceClient; @@ -124,7 +124,7 @@ void testCreateGuardrails_getTraceById() { .build(); var traceId = traceResourceClient.createTrace(trace, API_KEY, TEST_WORKSPACE); - var guardrails = guardrailsGenerator.generateGuardrailsForTrace(traceId, generator.generate(), + var guardrails = guardrailsGenerator.generateGuardrailsForTrace(traceId, idGenerator.generateId(), trace.projectName()); guardrailsResourceClient.addBatch(guardrails, API_KEY, TEST_WORKSPACE); @@ -157,10 +157,10 @@ void testCreateGuardrails_findTraces() { var guardrailsByTraceId = traces.stream() .collect(Collectors.toMap(Trace::id, trace -> Stream.concat( // mimic two separate guardrails validation groups - guardrailsGenerator.generateGuardrailsForTrace(trace.id(), generator.generate(), + guardrailsGenerator.generateGuardrailsForTrace(trace.id(), idGenerator.generateId(), trace.projectName()) .stream(), - guardrailsGenerator.generateGuardrailsForTrace(trace.id(), generator.generate(), + guardrailsGenerator.generateGuardrailsForTrace(trace.id(), idGenerator.generateId(), trace.projectName()) .stream()) .toList())); @@ -195,7 +195,7 @@ void getTraceStats_containsGuardrails() { var guardrailsByTraceId = traces.stream() .collect(Collectors.toMap(Trace::id, trace -> guardrailsGenerator.generateGuardrailsForTrace( - trace.id(), generator.generate(), trace.projectName()))); + trace.id(), idGenerator.generateId(), trace.projectName()))); guardrailsByTraceId.values() .forEach(guardrail -> guardrailsResourceClient.addBatch(guardrail, API_KEY, diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/MultiValueFeedbackScoresE2ETest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/MultiValueFeedbackScoresE2ETest.java index af080b1a690..fb82e48bb4d 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/MultiValueFeedbackScoresE2ETest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/MultiValueFeedbackScoresE2ETest.java @@ -47,12 +47,12 @@ import com.comet.opik.api.resources.utils.traces.TraceAssertions; import com.comet.opik.domain.EntityType; import com.comet.opik.domain.FeedbackScoreDAO; +import com.comet.opik.domain.IdGenerator; +import com.comet.opik.domain.TestIdGeneratorFactory; import com.comet.opik.extensions.DropwizardAppExtensionProvider; import com.comet.opik.extensions.RegisterApp; import com.comet.opik.infrastructure.auth.RequestContext; import com.comet.opik.podam.PodamFactoryUtils; -import com.fasterxml.uuid.Generators; -import com.fasterxml.uuid.impl.TimeBasedEpochGenerator; import com.redis.testcontainers.RedisContainer; import org.apache.commons.lang3.RandomStringUtils; import org.awaitility.Awaitility; @@ -127,7 +127,7 @@ class MultiValueFeedbackScoresE2ETest { } private final PodamFactory factory = PodamFactoryUtils.newPodamFactory(); - private final TimeBasedEpochGenerator generator = Generators.timeBasedEpochGenerator(); + private static final IdGenerator idGenerator = TestIdGeneratorFactory.create(); private TraceResourceClient traceResourceClient; private SpanResourceClient spanResourceClient; @@ -293,8 +293,8 @@ void deleteTraceFeedbackScoreScopedBySourceQueueId() { .build(); var traceId = traceResourceClient.createTrace(trace, API_KEY1, TEST_WORKSPACE); - var queueIdA = generator.generate(); - var queueIdB = generator.generate(); + var queueIdA = idGenerator.generateId(); + var queueIdB = idGenerator.generateId(); // Score from queue A var score = factory.manufacturePojo(FeedbackScore.class).toBuilder() @@ -346,8 +346,8 @@ void sameAuthorTwoQueuesProducesAverage() { .build(); var traceId = traceResourceClient.createTrace(trace, API_KEY1, TEST_WORKSPACE); - var queueIdA = generator.generate(); - var queueIdB = generator.generate(); + var queueIdA = idGenerator.generateId(); + var queueIdB = idGenerator.generateId(); var scoreName = randomUUID().toString(); var scoreFromQueueA = factory.manufacturePojo(FeedbackScore.class).toBuilder() diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ProjectMetricsResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ProjectMetricsResourceTest.java index 144a7f91375..00870805053 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ProjectMetricsResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ProjectMetricsResourceTest.java @@ -44,13 +44,12 @@ import com.comet.opik.domain.IdGenerator; import com.comet.opik.domain.ProjectMetricsDAO; import com.comet.opik.domain.ProjectMetricsService; +import com.comet.opik.domain.TestIdGeneratorFactory; import com.comet.opik.extensions.DropwizardAppExtensionProvider; import com.comet.opik.extensions.RegisterApp; import com.comet.opik.infrastructure.DatabaseAnalyticsFactory; import com.comet.opik.podam.PodamFactoryUtils; import com.comet.opik.utils.JsonUtils; -import com.fasterxml.uuid.Generators; -import com.fasterxml.uuid.impl.TimeBasedEpochGenerator; import com.github.tomakehurst.wiremock.client.WireMock; import com.redis.testcontainers.RedisContainer; import jakarta.ws.rs.NotFoundException; @@ -189,7 +188,7 @@ class ProjectMetricsResourceTest { } private final PodamFactory factory = PodamFactoryUtils.newPodamFactory(); - private final TimeBasedEpochGenerator generator = Generators.timeBasedEpochGenerator(); + private static final IdGenerator testIdGenerator = TestIdGeneratorFactory.create(); private IdGenerator idGenerator; private String baseURI; @@ -426,7 +425,7 @@ void happyPathWithFilter(Function getFilter, List e // create guardrails for the first trace var guardrail = guardrailsGenerator.generateGuardrailsForTrace( - traces.getFirst().id(), generator.generate(), projectName).getFirst().toBuilder() + traces.getFirst().id(), testIdGenerator.generateId(), projectName).getFirst().toBuilder() .result(GuardrailResult.PASSED) .build(); @@ -652,7 +651,7 @@ void happyPathWithFilter(Function getFilter, List e // create guardrails for the first trace var guardrail = guardrailsGenerator.generateGuardrailsForTrace( - traceForFilter.id(), generator.generate(), projectName).getFirst().toBuilder() + traceForFilter.id(), testIdGenerator.generateId(), projectName).getFirst().toBuilder() .result(GuardrailResult.PASSED) .build(); @@ -1057,7 +1056,7 @@ void happyPathWithFilter(Function getFilter, List e // create guardrails for the first trace var guardrail = guardrailsGenerator.generateGuardrailsForTrace( - traceForFilter.id(), generator.generate(), projectName).getFirst().toBuilder() + traceForFilter.id(), testIdGenerator.generateId(), projectName).getFirst().toBuilder() .result(GuardrailResult.PASSED) .build(); @@ -1276,7 +1275,7 @@ void happyPathWithFilter(Function getFilter, List e // create guardrails for the first trace var guardrail = guardrailsGenerator.generateGuardrailsForTrace( - traceForFilter.id(), generator.generate(), projectName).getFirst().toBuilder() + traceForFilter.id(), testIdGenerator.generateId(), projectName).getFirst().toBuilder() .result(GuardrailResult.PASSED) .build(); @@ -1468,7 +1467,7 @@ void happyPathWithFilter(Function getFilter, List e // create guardrails for the first trace var guardrail = guardrailsGenerator.generateGuardrailsForTrace( - traceForFilter.getFirst().id(), generator.generate(), projectName).getFirst().toBuilder() + traceForFilter.getFirst().id(), testIdGenerator.generateId(), projectName).getFirst().toBuilder() .result(GuardrailResult.PASSED) .build(); @@ -1687,7 +1686,7 @@ private Long createTracesWithGuardrails(String projectName, Instant marker) { return traces.stream() .map(trace -> { List guardrails = guardrailsGenerator.generateGuardrailsForTrace(trace.id(), - generator.generate(), + testIdGenerator.generateId(), trace.projectName()); guardrailsResourceClient.addBatch(guardrails, API_KEY, WORKSPACE_NAME); return guardrails; @@ -1714,7 +1713,7 @@ private Pair, List> createTracesWithGuardrails(String projectN List guardrailCounts = traces.stream() .map(trace -> { var guardrails = guardrailsGenerator.generateGuardrailsForTrace(trace.id(), - generator.generate(), + testIdGenerator.generateId(), trace.projectName()); var guardrailsWithAtLeastOneFailed = IntStream.range(0, guardrails.size()) .mapToObj(i -> i == 0 @@ -3518,7 +3517,7 @@ void happyPathWithFilter(Function getFilter, List e traceResourceClient.feedbackScores(scores, API_KEY, WORKSPACE_NAME); var guardrail = guardrailsGenerator.generateGuardrailsForTrace( - traceForFilter.getFirst().id(), generator.generate(), projectName).getFirst().toBuilder() + traceForFilter.getFirst().id(), testIdGenerator.generateId(), projectName).getFirst().toBuilder() .result(GuardrailResult.PASSED) .build(); guardrailsResourceClient.addBatch(List.of(guardrail), API_KEY, WORKSPACE_NAME); @@ -3636,7 +3635,7 @@ void happyPathWithFilter(Function getFilter, List e traceResourceClient.feedbackScores(scores, API_KEY, WORKSPACE_NAME); var guardrail = guardrailsGenerator.generateGuardrailsForTrace( - traceForFilter.id(), generator.generate(), projectName).getFirst().toBuilder() + traceForFilter.id(), testIdGenerator.generateId(), projectName).getFirst().toBuilder() .result(GuardrailResult.PASSED) .build(); guardrailsResourceClient.addBatch(List.of(guardrail), API_KEY, WORKSPACE_NAME); diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ProjectMetricsWithBreakdownResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ProjectMetricsWithBreakdownResourceTest.java index 7b176302b24..ca2c3baccb1 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ProjectMetricsWithBreakdownResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ProjectMetricsWithBreakdownResourceTest.java @@ -26,13 +26,12 @@ import com.comet.opik.domain.GuardrailResult; import com.comet.opik.domain.IdGenerator; import com.comet.opik.domain.SpanType; +import com.comet.opik.domain.TestIdGeneratorFactory; import com.comet.opik.extensions.DropwizardAppExtensionProvider; import com.comet.opik.extensions.RegisterApp; import com.comet.opik.infrastructure.DatabaseAnalyticsFactory; import com.comet.opik.podam.PodamFactoryUtils; import com.comet.opik.utils.JsonUtils; -import com.fasterxml.uuid.Generators; -import com.fasterxml.uuid.impl.TimeBasedEpochGenerator; import com.redis.testcontainers.RedisContainer; import io.dropwizard.jersey.validation.ValidationErrorMessage; import jakarta.ws.rs.client.Entity; @@ -118,7 +117,7 @@ class ProjectMetricsWithBreakdownResourceTest { } private final PodamFactory factory = PodamFactoryUtils.newPodamFactory(); - private final TimeBasedEpochGenerator generator = Generators.timeBasedEpochGenerator(); + private static final IdGenerator testIdGenerator = TestIdGeneratorFactory.create(); private IdGenerator idGenerator; private String baseURI; @@ -1415,7 +1414,7 @@ private void createTracesWithGuardrails(String projectName, Instant marker, Brea traceResourceClient.createTrace(trace, API_KEY, WORKSPACE_NAME); // Add guardrail (alternating between pass and fail) - var guardrails = guardrailsGenerator.generateGuardrailsForTrace(trace.id(), generator.generate(), + var guardrails = guardrailsGenerator.generateGuardrailsForTrace(trace.id(), testIdGenerator.generateId(), projectName); // Set result to FAILED for even indices, PASSED for odd var guardrailsWithResult = guardrails.stream() diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ProjectsResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ProjectsResourceTest.java index 3f07e8cd358..729fee90372 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ProjectsResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/ProjectsResourceTest.java @@ -48,7 +48,9 @@ import com.comet.opik.domain.FeedbackScoreDAO; import com.comet.opik.domain.GuardrailResult; import com.comet.opik.domain.GuardrailsMapper; +import com.comet.opik.domain.IdGenerator; import com.comet.opik.domain.ProjectService; +import com.comet.opik.domain.TestIdGeneratorFactory; import com.comet.opik.domain.retention.RetentionUtils; import com.comet.opik.domain.workspaces.WorkspacesService; import com.comet.opik.extensions.DropwizardAppExtensionProvider; @@ -59,8 +61,6 @@ import com.comet.opik.podam.PodamFactoryUtils; import com.comet.opik.utils.JsonUtils; import com.comet.opik.utils.ValidationUtils; -import com.fasterxml.uuid.Generators; -import com.fasterxml.uuid.impl.TimeBasedEpochGenerator; import com.github.tomakehurst.wiremock.client.WireMock; import com.redis.testcontainers.RedisContainer; import jakarta.ws.rs.HttpMethod; @@ -185,7 +185,7 @@ class ProjectsResourceTest { } private final PodamFactory factory = PodamFactoryUtils.newPodamFactory(); - private final TimeBasedEpochGenerator generator = Generators.timeBasedEpochGenerator(); + private static final IdGenerator idGenerator = TestIdGeneratorFactory.create(); private String baseURI; private ClientSupport client; @@ -1859,7 +1859,7 @@ private Project buildProjectStats(Project project, String apiKey, String workspa var guardrailsByTraceId = traces.stream() .collect(Collectors.toMap(Trace::id, trace -> guardrailsGenerator.generateGuardrailsForTrace( - trace.id(), generator.generate(), trace.projectName()))); + trace.id(), idGenerator.generateId(), trace.projectName()))); guardrailsByTraceId.values().forEach(guardrail -> guardrailsResourceClient.addBatch( guardrail, apiKey, workspaceName)); diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/SpansResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/SpansResourceTest.java index 3bff5c070dc..f40fd5e28fb 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/SpansResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/SpansResourceTest.java @@ -346,20 +346,29 @@ static Stream invalidIdsForUpdate() { // Referenced ids (traceId, parentSpanId) use the not-in-future policy: non-v7 and future-dated are // rejected, but past ids are allowed (spans are commonly attached to older traces), so unlike - // invalidIds() there is no TOO_OLD case here. - static Stream invalidTraceIds() { + // invalidIds() there is no TOO_OLD case here. Each argument sets exactly one referenced id to an + // invalid value on the span builder and pairs it with the expected validation message, so trace and + // parent cases share a single test body. + static Stream invalidReferencedIds() { var future = Instant.now().plus(Duration.ofHours(25)).toEpochMilli(); var expectedDetails = "id with timestamp '%s' must be in the allowed ingestion window of '%s' around now, reason '%s'"; var expectedWindow = Duration.ofHours(24); return Stream.of( - arguments(UUID.randomUUID(), + arguments( + (Function) builder -> builder.traceId(UUID.randomUUID()), "Span trace id must be a version 7 UUID", "traceId not v7"), arguments( - generator.construct(future), + (Function) builder -> builder + .traceId(generator.construct(future)), expectedDetails.formatted( Instant.ofEpochMilli(future), expectedWindow, Reason.TOO_FAR_FUTURE.getValue()), - "traceId after window")); + "traceId after window"), + arguments( + (Function) builder -> builder + .parentSpanId(UUID.randomUUID()), + "Span parent id must be a version 7 UUID", + "parentSpanId not v7")); } @Nested @@ -1818,25 +1827,13 @@ void createWithInvalidIdThrowsBadRequest(UUID id, String expectedDetails, String } } - @MethodSource("com.comet.opik.api.resources.v1.priv.SpansResourceTest#invalidTraceIds") - @ParameterizedTest(name = "Create span with invalid traceId throws bad request: {2}") - void createWithInvalidTraceIdThrowsBadRequest(UUID traceId, String expectedDetails, String testName) { + @MethodSource("com.comet.opik.api.resources.v1.priv.SpansResourceTest#invalidReferencedIds") + @ParameterizedTest(name = "Create span with invalid referenced id throws bad request: {2}") + void createWithInvalidReferencedIdThrowsBadRequest( + Function spanCustomizer, String expectedDetails, String testName) { var expectedEntity = new io.dropwizard.jersey.errors.ErrorMessage( HttpStatus.SC_BAD_REQUEST, "Invalid UUID for id", expectedDetails); - var span = podamFactory.manufacturePojo(Span.class).toBuilder().traceId(traceId).build(); - try (var response = spanResourceClient.createSpan( - span, API_KEY, TEST_WORKSPACE, HttpStatus.SC_BAD_REQUEST)) { - var actualEntity = response.readEntity(io.dropwizard.jersey.errors.ErrorMessage.class); - assertThat(actualEntity).isEqualTo(expectedEntity); - } - } - - @Test - @DisplayName("Create span with non-v7 parentSpanId throws bad request") - void createWithNonV7ParentSpanIdThrowsBadRequest() { - var expectedEntity = new io.dropwizard.jersey.errors.ErrorMessage( - HttpStatus.SC_BAD_REQUEST, "Invalid UUID for id", "Span parent id must be a version 7 UUID"); - var span = podamFactory.manufacturePojo(Span.class).toBuilder().parentSpanId(UUID.randomUUID()).build(); + var span = spanCustomizer.apply(podamFactory.manufacturePojo(Span.class).toBuilder()).build(); try (var response = spanResourceClient.createSpan( span, API_KEY, TEST_WORKSPACE, HttpStatus.SC_BAD_REQUEST)) { var actualEntity = response.readEntity(io.dropwizard.jersey.errors.ErrorMessage.class); @@ -2356,26 +2353,13 @@ void batchCreateWithInvalidIdThrowsBadRequest(UUID id, String expectedDetails, S } } - @MethodSource("com.comet.opik.api.resources.v1.priv.SpansResourceTest#invalidTraceIds") - @ParameterizedTest(name = "Batch create span with invalid traceId throws bad request: {2}") - void batchCreateWithInvalidTraceIdThrowsBadRequest(UUID traceId, String expectedDetails, String testName) { + @MethodSource("com.comet.opik.api.resources.v1.priv.SpansResourceTest#invalidReferencedIds") + @ParameterizedTest(name = "Batch create span with invalid referenced id throws bad request: {2}") + void batchCreateWithInvalidReferencedIdThrowsBadRequest( + Function spanCustomizer, String expectedDetails, String testName) { var expectedEntity = new io.dropwizard.jersey.errors.ErrorMessage( HttpStatus.SC_BAD_REQUEST, "Invalid UUID for id", expectedDetails); - var span = podamFactory.manufacturePojo(Span.class).toBuilder().traceId(traceId).build(); - try (var response = spanResourceClient.callBatchCreateSpans( - List.of(span), API_KEY, TEST_WORKSPACE)) { - assertThat(response.getStatusInfo().getStatusCode()).isEqualTo(HttpStatus.SC_BAD_REQUEST); - var actualEntity = response.readEntity(io.dropwizard.jersey.errors.ErrorMessage.class); - assertThat(actualEntity).isEqualTo(expectedEntity); - } - } - - @Test - @DisplayName("Batch create span with non-v7 parentSpanId throws bad request") - void batchCreateWithNonV7ParentSpanIdThrowsBadRequest() { - var expectedEntity = new io.dropwizard.jersey.errors.ErrorMessage( - HttpStatus.SC_BAD_REQUEST, "Invalid UUID for id", "Span parent id must be a version 7 UUID"); - var span = podamFactory.manufacturePojo(Span.class).toBuilder().parentSpanId(UUID.randomUUID()).build(); + var span = spanCustomizer.apply(podamFactory.manufacturePojo(Span.class).toBuilder()).build(); try (var response = spanResourceClient.callBatchCreateSpans( List.of(span), API_KEY, TEST_WORKSPACE)) { assertThat(response.getStatusInfo().getStatusCode()).isEqualTo(HttpStatus.SC_BAD_REQUEST); diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/WorkspaceVersionResourceTest.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/WorkspaceVersionResourceTest.java index bf8ada0316f..309d0b80586 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/WorkspaceVersionResourceTest.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/WorkspaceVersionResourceTest.java @@ -24,13 +24,13 @@ import com.comet.opik.api.resources.utils.resources.PromptResourceClient; import com.comet.opik.api.resources.utils.resources.WorkspaceResourceClient; import com.comet.opik.domain.DemoData; +import com.comet.opik.domain.IdGenerator; +import com.comet.opik.domain.TestIdGeneratorFactory; import com.comet.opik.domain.workspaces.Workspace; import com.comet.opik.domain.workspaces.WorkspacesService; import com.comet.opik.extensions.DropwizardAppExtensionProvider; import com.comet.opik.extensions.RegisterApp; import com.comet.opik.podam.PodamFactoryUtils; -import com.fasterxml.uuid.Generators; -import com.fasterxml.uuid.impl.TimeBasedEpochGenerator; import com.redis.testcontainers.RedisContainer; import org.apache.commons.lang3.RandomStringUtils; import org.awaitility.Awaitility; @@ -85,7 +85,7 @@ class WorkspaceVersionResourceTest { .build(); private final PodamFactory podamFactory = PodamFactoryUtils.newPodamFactory(); - private final TimeBasedEpochGenerator generator = Generators.timeBasedEpochGenerator(); + private static final IdGenerator idGenerator = TestIdGeneratorFactory.create(); @Nested @TestInstance(TestInstance.Lifecycle.PER_CLASS) @@ -529,7 +529,7 @@ void workspaceVersion__whenMultiProjectRule__returnsVersion1() { // Single-project rule does not trigger version_1 evaluatorClient.createEvaluator(podamFactory.manufacturePojo(AutomationRuleEvaluatorLlmAsJudge.class) .toBuilder() - .projectIds(Set.of(generator.generate())) + .projectIds(Set.of(idGenerator.generateId())) .build(), workspaceName, API_KEY); assertThat(workspaceClient.getWorkspaceVersion(API_KEY, workspaceName)).isEqualTo(V2_WORKSPACE_VERSION); @@ -537,7 +537,7 @@ void workspaceVersion__whenMultiProjectRule__returnsVersion1() { // Multi-project rule triggers version_1 evaluatorClient.createEvaluator(podamFactory.manufacturePojo(AutomationRuleEvaluatorLlmAsJudge.class) .toBuilder() - .projectIds(Set.of(generator.generate(), generator.generate())) + .projectIds(Set.of(idGenerator.generateId(), idGenerator.generateId())) .build(), workspaceName, API_KEY); assertThat(workspaceClient.getWorkspaceVersion(API_KEY, workspaceName)).isEqualTo(V1_WORKSPACE_VERSION); @@ -600,7 +600,7 @@ void workspaceVersion__whenAlertWithoutProject__returnsVersion1() { // Project-scoped alert (projectId column) does not trigger version_1 alertClient.createAlert( - AlertResourceTest.generateAlertForProject(podamFactory, generator.generate()), + AlertResourceTest.generateAlertForProject(podamFactory, idGenerator.generateId()), API_KEY, workspaceName, 201); assertThat(workspaceClient.getWorkspaceVersion(API_KEY, workspaceName)).isEqualTo(V2_WORKSPACE_VERSION);