From ea0ad3ef82db0ce0a30b7596344b6ed648248d13 Mon Sep 17 00:00:00 2001 From: Borys Tkachenko Date: Fri, 20 Dec 2024 17:07:18 +0100 Subject: [PATCH] OPIK-645 Return all feedback score names for project ids endpoint --- .../resources/v1/priv/DatasetsResource.java | 6 +-- .../v1/priv/ExperimentsResource.java | 4 +- .../resources/v1/priv/ProjectsResource.java | 31 ++++++++++++ ...sValidator.java => IdParamsValidator.java} | 8 ++-- .../comet/opik/domain/FeedbackScoreDAO.java | 47 +++++++++++++++++++ .../opik/domain/FeedbackScoreService.java | 9 ++++ .../resources/ProjectResourceClient.java | 23 +++++++++ .../v1/priv/ExperimentsResourceTest.java | 26 ++++++---- 8 files changed, 136 insertions(+), 18 deletions(-) rename apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/priv/validate/{ExperimentParamsValidator.java => IdParamsValidator.java} (75%) diff --git a/apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/priv/DatasetsResource.java b/apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/priv/DatasetsResource.java index 7a895adca5..30e5b2c5db 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/priv/DatasetsResource.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/priv/DatasetsResource.java @@ -15,7 +15,7 @@ import com.comet.opik.api.PageColumns; import com.comet.opik.api.filter.ExperimentsComparisonFilter; import com.comet.opik.api.filter.FiltersFactory; -import com.comet.opik.api.resources.v1.priv.validate.ExperimentParamsValidator; +import com.comet.opik.api.resources.v1.priv.validate.IdParamsValidator; import com.comet.opik.api.sorting.SortingFactoryDatasets; import com.comet.opik.api.sorting.SortingField; import com.comet.opik.domain.DatasetItemService; @@ -375,7 +375,7 @@ public Response findDatasetItemsWithExperimentItems( @QueryParam("filters") String filters, @QueryParam("truncate") boolean truncate) { - var experimentIds = ExperimentParamsValidator.getExperimentIds(experimentIdsQueryParam); + var experimentIds = IdParamsValidator.getIds(experimentIdsQueryParam); var queryFilters = filtersFactory.newFilters(filters, ExperimentsComparisonFilter.LIST_TYPE_REFERENCE); @@ -413,7 +413,7 @@ public Response getDatasetItemsOutputColumns( var experimentIds = Optional.ofNullable(experimentIdsQueryParam) .filter(Predicate.not(String::isEmpty)) - .map(ExperimentParamsValidator::getExperimentIds) + .map(IdParamsValidator::getIds) .orElse(null); String workspaceId = requestContext.get().getWorkspaceId(); diff --git a/apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/priv/ExperimentsResource.java b/apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/priv/ExperimentsResource.java index 7812805917..47a0a0bf8b 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/priv/ExperimentsResource.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/priv/ExperimentsResource.java @@ -11,7 +11,7 @@ import com.comet.opik.api.FeedbackDefinition; import com.comet.opik.api.FeedbackScoreNames; import com.comet.opik.api.Identifier; -import com.comet.opik.api.resources.v1.priv.validate.ExperimentParamsValidator; +import com.comet.opik.api.resources.v1.priv.validate.IdParamsValidator; import com.comet.opik.domain.ExperimentItemService; import com.comet.opik.domain.ExperimentService; import com.comet.opik.domain.FeedbackScoreService; @@ -281,7 +281,7 @@ public Response deleteExperimentItems( public Response findFeedbackScoreNames(@QueryParam("experiment_ids") String experimentIdsQueryParam) { var experimentIds = Optional.ofNullable(experimentIdsQueryParam) - .map(ExperimentParamsValidator::getExperimentIds) + .map(IdParamsValidator::getIds) .map(List::copyOf) .orElse(null); diff --git a/apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/priv/ProjectsResource.java b/apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/priv/ProjectsResource.java index 2f7249c429..08872355db 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/priv/ProjectsResource.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/priv/ProjectsResource.java @@ -2,6 +2,7 @@ import com.codahale.metrics.annotation.Timed; import com.comet.opik.api.BatchDelete; +import com.comet.opik.api.FeedbackScoreNames; import com.comet.opik.api.Page; import com.comet.opik.api.Project; import com.comet.opik.api.ProjectCriteria; @@ -10,8 +11,10 @@ import com.comet.opik.api.error.ErrorMessage; import com.comet.opik.api.metrics.ProjectMetricRequest; import com.comet.opik.api.metrics.ProjectMetricResponse; +import com.comet.opik.api.resources.v1.priv.validate.IdParamsValidator; import com.comet.opik.api.sorting.SortingFactoryProjects; import com.comet.opik.api.sorting.SortingField; +import com.comet.opik.domain.FeedbackScoreService; import com.comet.opik.domain.ProjectMetricsService; import com.comet.opik.domain.ProjectService; import com.comet.opik.infrastructure.auth.RequestContext; @@ -49,6 +52,7 @@ import lombok.extern.slf4j.Slf4j; import java.util.List; +import java.util.Optional; import java.util.UUID; import static com.comet.opik.domain.ProjectMetricsService.ERR_START_BEFORE_END; @@ -67,6 +71,7 @@ public class ProjectsResource { private final @NonNull Provider requestContext; private final @NonNull SortingFactoryProjects sortingFactory; private final @NonNull ProjectMetricsService metricsService; + private final @NonNull FeedbackScoreService feedbackScoreService; @GET @Operation(operationId = "findProjects", summary = "Find projects", description = "Find projects", responses = { @@ -232,6 +237,32 @@ public Response getProjectMetrics( return Response.ok().entity(response).build(); } + @GET + @Path("/feedback-scores/names") + @Operation(operationId = "findFeedbackScoreNamesByProjectIds", summary = "Find Feedback Score names By Project Ids", description = "Find Feedback Score names By Project Ids", responses = { + @ApiResponse(responseCode = "200", description = "Feedback Scores resource", content = @Content(schema = @Schema(implementation = FeedbackScoreNames.class))) + }) + public Response findFeedbackScoreNames(@QueryParam("project_ids") String projectIdsQueryParam) { + + var projectIds = Optional.ofNullable(projectIdsQueryParam) + .map(IdParamsValidator::getIds) + .map(List::copyOf) + .orElse(null); + + String workspaceId = requestContext.get().getWorkspaceId(); + + log.info("Find feedback score names by project_ids '{}', on workspaceId '{}'", + projectIds, workspaceId); + FeedbackScoreNames feedbackScoreNames = feedbackScoreService + .getProjectsFeedbackScoreNames(projectIds) + .contextWrite(ctx -> setRequestContext(ctx, requestContext)) + .block(); + log.info("Found feedback score names '{}' by project_ids '{}', on workspaceId '{}'", + feedbackScoreNames.scores().size(), projectIds, workspaceId); + + return Response.ok(feedbackScoreNames).build(); + } + private void validate(ProjectMetricRequest request) { if (!request.intervalStart().isBefore(request.intervalEnd())) { throw new BadRequestException(ERR_START_BEFORE_END); diff --git a/apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/priv/validate/ExperimentParamsValidator.java b/apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/priv/validate/IdParamsValidator.java similarity index 75% rename from apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/priv/validate/ExperimentParamsValidator.java rename to apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/priv/validate/IdParamsValidator.java index 11f91f47fb..15a7faff86 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/priv/validate/ExperimentParamsValidator.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/priv/validate/IdParamsValidator.java @@ -13,15 +13,15 @@ @UtilityClass @Slf4j -public class ExperimentParamsValidator { +public class IdParamsValidator { private static final TypeReference> LIST_UUID_TYPE_REFERENCE = new TypeReference<>() { }; - public static Set getExperimentIds(String experimentIds) { - var message = "Invalid query param experiment ids '%s'".formatted(experimentIds); + public static Set getIds(String idsQueryParam) { + var message = "Invalid query param ids '%s'".formatted(idsQueryParam); try { - return JsonUtils.readValue(experimentIds, LIST_UUID_TYPE_REFERENCE) + return JsonUtils.readValue(idsQueryParam, LIST_UUID_TYPE_REFERENCE) .stream() .collect(Collectors.toUnmodifiableSet()); } catch (RuntimeException exception) { diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/FeedbackScoreDAO.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/FeedbackScoreDAO.java index 5dd80144de..2ed2b80c5a 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/FeedbackScoreDAO.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/FeedbackScoreDAO.java @@ -72,6 +72,8 @@ Mono scoreEntity(EntityType entityType, UUID entityId, FeedbackScore score Mono> getSpanFeedbackScoreNames(@NonNull UUID projectId, SpanType type); Mono> getExperimentsFeedbackScoreNames(List experimentIds); + + Mono> getProjectsFeedbackScoreNames(List projectIds); } @Singleton @@ -203,6 +205,23 @@ INNER JOIN ( ; """; + private static final String SELECT_PROJECTS_FEEDBACK_SCORE_NAMES = """ + SELECT + distinct name + FROM ( + SELECT + name + FROM feedback_scores + WHERE workspace_id = :workspace_id + + AND project_id IN :project_ids + + ORDER BY entity_id DESC, last_updated_at DESC + LIMIT 1 BY entity_id, name + ) AS names + ; + """; + private final static String SELECT_SPAN_FEEDBACK_SCORE_NAMES = """ SELECT distinct name @@ -433,6 +452,34 @@ public Mono> getExperimentsFeedbackScoreNames(List experiment }); } + @Override + @WithSpan + public Mono> getProjectsFeedbackScoreNames(List projectIds) { + return asyncTemplate.nonTransaction(connection -> { + + ST template = new ST(SELECT_PROJECTS_FEEDBACK_SCORE_NAMES); + + if (CollectionUtils.isNotEmpty(projectIds)) { + template.add("project_ids", projectIds); + } + + var statement = connection.createStatement(template.render()); + + if (CollectionUtils.isNotEmpty(projectIds)) { + template.add("project_ids", projectIds); + } + + if (CollectionUtils.isNotEmpty(projectIds)) { + statement.bind("project_ids", projectIds.toArray(UUID[]::new)); + } + + return makeMonoContextAware(bindWorkspaceIdToMono(statement)) + .flatMapMany(result -> result.map((row, rowMetadata) -> row.get("name", String.class))) + .distinct() + .collect(Collectors.toList()); + }); + } + @Override @WithSpan public Mono> getSpanFeedbackScoreNames(@NonNull UUID projectId, SpanType type) { 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 f599e11b0d..992bfa6ec3 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 @@ -51,6 +51,8 @@ public interface FeedbackScoreService { Mono getSpanFeedbackScoreNames(UUID projectId, SpanType type); Mono getExperimentsFeedbackScoreNames(List experimentIds); + + Mono getProjectsFeedbackScoreNames(List projectIds); } @Slf4j @@ -256,6 +258,13 @@ public Mono getExperimentsFeedbackScoreNames(List expe .map(FeedbackScoreNames::new); } + @Override + public Mono getProjectsFeedbackScoreNames(List projectIds) { + return dao.getProjectsFeedbackScoreNames(projectIds) + .map(names -> names.stream().map(FeedbackScoreNames.ScoreName::new).toList()) + .map(FeedbackScoreNames::new); + } + private Mono failWithNotFound(String errorMessage) { log.info(errorMessage); return Mono.error(new NotFoundException(Response.status(404) diff --git a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/utils/resources/ProjectResourceClient.java b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/utils/resources/ProjectResourceClient.java index a87f6c39f8..f67a4ecb8a 100644 --- a/apps/opik-backend/src/test/java/com/comet/opik/api/resources/utils/resources/ProjectResourceClient.java +++ b/apps/opik-backend/src/test/java/com/comet/opik/api/resources/utils/resources/ProjectResourceClient.java @@ -1,9 +1,11 @@ package com.comet.opik.api.resources.utils.resources; +import com.comet.opik.api.FeedbackScoreNames; import com.comet.opik.api.Project; import com.comet.opik.api.resources.utils.TestUtils; import com.comet.opik.infrastructure.auth.RequestContext; import jakarta.ws.rs.client.Entity; +import jakarta.ws.rs.client.WebTarget; import jakarta.ws.rs.core.HttpHeaders; import lombok.RequiredArgsConstructor; import org.apache.hc.core5.http.HttpStatus; @@ -74,4 +76,25 @@ public Project getByName(String projectName, String apiKey, String workspaceName } } + public FeedbackScoreNames findFeedbackScoreNames(String projectIdsQueryParam, String apiKey, String workspaceName) { + WebTarget webTarget = client.target(RESOURCE_PATH.formatted(baseURI)) + .path("feedback-scores") + .path("names"); + + if (projectIdsQueryParam != null) { + webTarget = webTarget.queryParam("project_ids", projectIdsQueryParam); + } + + try (var actualResponse = webTarget + .request() + .header(HttpHeaders.AUTHORIZATION, apiKey) + .header(RequestContext.WORKSPACE_HEADER, workspaceName) + .get()) { + + // then + assertThat(actualResponse.getStatusInfo().getStatusCode()).isEqualTo(org.apache.http.HttpStatus.SC_OK); + + return actualResponse.readEntity(FeedbackScoreNames.class); + } + } } 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 6bda89b881..2ea2e084af 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 @@ -2659,20 +2659,22 @@ void getFeedbackScoreNames__whenGetFeedbackScoreNames__thenReturnFeedbackScoreNa createExperimentsItems(apiKey, workspaceName, unexpectedScores, List.of()); - fetchAndAssertResponse(userExperimentId, experimentId, names, otherNames, apiKey, workspaceName); + fetchAndAssertResponse(userExperimentId, experimentId, projectId, names, otherNames, apiKey, workspaceName); } } - private void fetchAndAssertResponse(boolean userExperimentId, UUID experimentId, List names, + private void fetchAndAssertResponse(boolean userExperimentId, UUID experimentId, UUID projectId, List names, List otherNames, String apiKey, String workspaceName) { WebTarget webTarget = client.target(URL_TEMPLATE.formatted(baseURI)) .path("feedback-scores") .path("names"); + String projectIdsQueryParam = null; if (userExperimentId) { var ids = JsonUtils.writeValueAsString(List.of(experimentId)); webTarget = webTarget.queryParam("experiment_ids", ids); + projectIdsQueryParam = JsonUtils.writeValueAsString(List.of(projectId)); } List expectedNames = userExperimentId @@ -2688,14 +2690,20 @@ private void fetchAndAssertResponse(boolean userExperimentId, UUID experimentId, // then assertThat(actualResponse.getStatusInfo().getStatusCode()).isEqualTo(HttpStatus.SC_OK); var actualEntity = actualResponse.readEntity(FeedbackScoreNames.class); - - assertThat(actualEntity.scores()).hasSize(expectedNames.size()); - assertThat(actualEntity - .scores() - .stream() - .map(FeedbackScoreNames.ScoreName::name) - .toList()).containsExactlyInAnyOrderElementsOf(expectedNames); + assertFeedbackScoreNames(actualEntity, expectedNames); } + + var feedbackScoreNamesByProjectId = projectResourceClient.findFeedbackScoreNames(projectIdsQueryParam, apiKey, workspaceName); + assertFeedbackScoreNames(feedbackScoreNamesByProjectId, expectedNames); + } + + private void assertFeedbackScoreNames(FeedbackScoreNames actual, List expectedNames) { + assertThat(actual.scores()).hasSize(expectedNames.size()); + assertThat(actual + .scores() + .stream() + .map(FeedbackScoreNames.ScoreName::name) + .toList()).containsExactlyInAnyOrderElementsOf(expectedNames); } private List> createMultiValueScores(List multipleValuesFeedbackScores,