From 777e5cc8b671ad106622130c95e0554fa4a9d19c Mon Sep 17 00:00:00 2001 From: Krati Paliwal Date: Sun, 19 Apr 2026 16:00:02 +0000 Subject: [PATCH 1/2] Fixes #25878: listBots API does not return botUser field --- .../openmetadata/it/tests/BotResourceIT.java | 291 ++++++++++++++++++ .../service/jdbi3/BotRepository.java | 51 ++- .../service/resources/bots/BotResource.java | 24 +- 3 files changed, 360 insertions(+), 6 deletions(-) diff --git a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/BotResourceIT.java b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/BotResourceIT.java index e84986c45b87..4d6cafb8d870 100644 --- a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/BotResourceIT.java +++ b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/BotResourceIT.java @@ -2,7 +2,9 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.UUID; import org.junit.jupiter.api.Test; @@ -272,6 +274,295 @@ void test_botUserCannotBeShared(TestNamespace ns) { "Creating bot with already-used bot user should fail"); } + /** + * Verifies that GET /bots without ?fields=botUser does NOT populate botUser. The fields query + * parameter acts as an opt-in filter — consistent with how other resources handle optional + * fields (e.g. owners, tags). + */ + @Test + void test_listBots_withoutFieldsParam_omitsBotUser(TestNamespace ns) { + User botUser = createBotUser(ns); + CreateBot request = + new CreateBot() + .withName(ns.prefix("bot_list_omits_user")) + .withDescription("Bot for list-without-fields test") + .withBotUser(botUser.getName()); + Bot createdBot = createEntity(request); + + ListParams params = new ListParams().setLimit(1000); + ListResponse response = listEntities(params); + + assertNotNull(response, "List response should not be null"); + assertNotNull(response.getData(), "List data should not be null"); + + Bot listedBot = + response.getData().stream() + .filter(b -> createdBot.getId().equals(b.getId())) + .findFirst() + .orElseThrow(() -> new AssertionError("Created bot missing from list response")); + + assertNull( + listedBot.getBotUser(), + "botUser must NOT be populated when ?fields=botUser is not requested"); + } + + /** + * Regression test for issue #25878: verifies that passing ?fields=botUser is accepted and does + * not cause an error (the param was previously silently ignored). + */ + @Test + void test_listBots_withFieldsParam_returnsBotUser(TestNamespace ns) { + User botUser = createBotUser(ns); + CreateBot request = + new CreateBot() + .withName(ns.prefix("bot_list_fields_param")) + .withDescription("Bot for ?fields=botUser regression test") + .withBotUser(botUser.getName()); + Bot createdBot = createEntity(request); + + ListParams params = new ListParams().setLimit(1000).setFields("botUser"); + ListResponse response = listEntities(params); + + Bot listedBot = + response.getData().stream() + .filter(b -> createdBot.getId().equals(b.getId())) + .findFirst() + .orElseThrow(() -> new AssertionError("Created bot missing from list response")); + + assertNotNull( + listedBot.getBotUser(), "botUser must be populated when ?fields=botUser is provided"); + assertEquals(botUser.getId(), listedBot.getBotUser().getId()); + } + + /** + * Regression test for issue #25878: GET /bots/{id}?fields=botUser must return botUser (the + * fields query parameter was previously not exposed on this endpoint). + */ + @Test + void test_getBotById_withFieldsParam_returnsBotUser(TestNamespace ns) { + User botUser = createBotUser(ns); + CreateBot request = + new CreateBot() + .withName(ns.prefix("bot_get_fields_param")) + .withDescription("Bot for GET by id ?fields=botUser test") + .withBotUser(botUser.getName()); + Bot createdBot = createEntity(request); + + Bot retrieved = getEntityWithFields(createdBot.getId().toString(), "botUser"); + + assertNotNull(retrieved, "Bot must be retrievable by id with ?fields=botUser"); + assertNotNull(retrieved.getBotUser(), "botUser must be populated on GET by id"); + assertEquals(botUser.getId(), retrieved.getBotUser().getId()); + } + + /** + * Regression test for issue #25878: GET /bots/name/{name}?fields=botUser must return botUser. + */ + @Test + void test_getBotByName_withFieldsParam_returnsBotUser(TestNamespace ns) { + User botUser = createBotUser(ns); + CreateBot request = + new CreateBot() + .withName(ns.prefix("bot_get_by_name_fields")) + .withDescription("Bot for GET by name ?fields=botUser test") + .withBotUser(botUser.getName()); + Bot createdBot = createEntity(request); + + Bot retrieved = getEntityByNameWithFields(createdBot.getFullyQualifiedName(), "botUser"); + + assertNotNull(retrieved, "Bot must be retrievable by name with ?fields=botUser"); + assertNotNull(retrieved.getBotUser(), "botUser must be populated on GET by name"); + assertEquals(botUser.getId(), retrieved.getBotUser().getId()); + } + + /** + * Verifies all bots in a multi-bot list response have botUser populated — exercises the + * BotRepository bulk fetch path. + */ + @Test + void test_listBots_multipleBots_allHaveBotUser(TestNamespace ns) { + User user1 = createBotUser(ns); + User user2 = createBotUser(ns); + User user3 = createBotUser(ns); + + Bot bot1 = + createEntity( + new CreateBot() + .withName(ns.prefix("bot_bulk_1")) + .withDescription("Bulk test bot 1") + .withBotUser(user1.getName())); + Bot bot2 = + createEntity( + new CreateBot() + .withName(ns.prefix("bot_bulk_2")) + .withDescription("Bulk test bot 2") + .withBotUser(user2.getName())); + Bot bot3 = + createEntity( + new CreateBot() + .withName(ns.prefix("bot_bulk_3")) + .withDescription("Bulk test bot 3") + .withBotUser(user3.getName())); + + java.util.Set createdIds = java.util.Set.of(bot1.getId(), bot2.getId(), bot3.getId()); + java.util.Map expectedUserByBot = + java.util.Map.of( + bot1.getId(), user1.getId(), + bot2.getId(), user2.getId(), + bot3.getId(), user3.getId()); + + ListResponse response = listEntities(new ListParams().setLimit(1000).setFields("botUser")); + + long matched = + response.getData().stream() + .filter(b -> createdIds.contains(b.getId())) + .peek( + b -> { + assertNotNull( + b.getBotUser(), + "Every bot in list must have botUser populated (bulk fetch path)"); + assertEquals(expectedUserByBot.get(b.getId()), b.getBotUser().getId()); + }) + .count(); + + assertTrue( + matched == createdIds.size(), + "All three created bots should appear in the list response with botUser populated"); + } + + /** + * Regression test: verifies POST /bots returns botUser in the response body. The create flow + * uses the in-memory entity populated by {@link + * org.openmetadata.service.jdbi3.BotRepository#prepare}, which must still set botUser after the + * fields-gating refactor. + */ + @Test + void test_createBot_responseIncludesBotUser(TestNamespace ns) { + User botUser = createBotUser(ns); + CreateBot request = + new CreateBot() + .withName(ns.prefix("bot_create_response")) + .withDescription("Regression: create response must include botUser") + .withBotUser(botUser.getName()); + + Bot created = createEntity(request); + + assertNotNull(created, "Create response should not be null"); + assertNotNull(created.getBotUser(), "POST /bots response must populate botUser"); + assertEquals(botUser.getId(), created.getBotUser().getId()); + assertEquals("user", created.getBotUser().getType()); + } + + /** + * Regression test: patching a bot's description must not drop or alter its botUser + * relationship. Exercises the BotUpdater + restorePatchAttributes path. + */ + @Test + void test_patchBotDescription_preservesBotUser(TestNamespace ns) { + User botUser = createBotUser(ns); + CreateBot request = + new CreateBot() + .withName(ns.prefix("bot_patch_preserves_user")) + .withDescription("Initial description") + .withBotUser(botUser.getName()); + Bot created = createEntity(request); + UUID originalBotUserId = created.getBotUser().getId(); + + created.setDescription("Patched description"); + Bot patched = patchEntity(created.getId().toString(), created); + assertEquals("Patched description", patched.getDescription()); + + Bot refetched = getEntityWithFields(created.getId().toString(), "botUser"); + assertNotNull(refetched.getBotUser(), "botUser should still be resolvable after patch"); + assertEquals( + originalBotUserId, + refetched.getBotUser().getId(), + "Patching description must not change the botUser relationship"); + } + + /** + * Regression test: bot user cannot be changed via PATCH (enforced by + * {@link org.openmetadata.service.jdbi3.BotRepository#restorePatchAttributes}). Either the + * patch is silently reverted or the server rejects it — both outcomes are acceptable as long + * as the original relationship survives. + */ + @Test + void test_patchBot_cannotChangeBotUser(TestNamespace ns) { + User originalBotUser = createBotUser(ns); + User replacementBotUser = createBotUser(ns); + + Bot created = + createEntity( + new CreateBot() + .withName(ns.prefix("bot_patch_user_immutable")) + .withDescription("Bot for botUser immutability test") + .withBotUser(originalBotUser.getName())); + UUID originalBotUserId = created.getBotUser().getId(); + + created.setBotUser(replacementBotUser.getEntityReference()); + + try { + patchEntity(created.getId().toString(), created); + } catch (Exception ignored) { + // Server may reject the patch outright — also acceptable. + } + + Bot refetched = getEntityWithFields(created.getId().toString(), "botUser"); + assertEquals( + originalBotUserId, + refetched.getBotUser().getId(), + "Bot's underlying botUser relationship must be immutable via PATCH"); + } + + /** + * Backward-compatibility test: GET /bots/{id} must always return botUser, with or without the + * fields query param. Existing integrations rely on botUser being present in the default GET + * response — the fields-gating fix only applies to the list bulk path. + */ + @Test + void test_getBotById_alwaysReturnsBotUser(TestNamespace ns) { + User botUser = createBotUser(ns); + Bot created = + createEntity( + new CreateBot() + .withName(ns.prefix("bot_get_backcompat")) + .withDescription("Bot for GET backward-compat test") + .withBotUser(botUser.getName())); + + Bot fetchedNoFields = getEntity(created.getId().toString()); + assertEquals(created.getId(), fetchedNoFields.getId()); + assertNotNull( + fetchedNoFields.getBotUser(), + "GET /bots/{id} without ?fields must still return botUser (backward compat)"); + assertEquals(botUser.getId(), fetchedNoFields.getBotUser().getId()); + + Bot fetchedWithFields = getEntityWithFields(created.getId().toString(), "botUser"); + assertNotNull(fetchedWithFields.getBotUser()); + assertEquals(botUser.getId(), fetchedWithFields.getBotUser().getId()); + } + + /** + * Backward-compatibility test: GET /bots/name/{name} must always return botUser, matching the + * pre-existing contract for existing integrations. + */ + @Test + void test_getBotByName_alwaysReturnsBotUser(TestNamespace ns) { + User botUser = createBotUser(ns); + Bot created = + createEntity( + new CreateBot() + .withName(ns.prefix("bot_get_by_name_backcompat")) + .withDescription("Bot for GET-by-name backward-compat test") + .withBotUser(botUser.getName())); + + Bot fetchedNoFields = getEntityByName(created.getFullyQualifiedName()); + assertEquals(created.getId(), fetchedNoFields.getId()); + assertNotNull( + fetchedNoFields.getBotUser(), + "GET /bots/name/{name} without ?fields must still return botUser (backward compat)"); + assertEquals(botUser.getId(), fetchedNoFields.getBotUser().getId()); + } + @Test void test_botNameUniqueness(TestNamespace ns) { OpenMetadataClient client = SdkClients.adminClient(); diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/BotRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/BotRepository.java index bfa6675b5fd3..fe0c095e4199 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/BotRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/BotRepository.java @@ -13,7 +13,9 @@ package org.openmetadata.service.jdbi3; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.UUID; import lombok.extern.slf4j.Slf4j; import org.jdbi.v3.sqlobject.transaction.Transaction; @@ -30,7 +32,8 @@ @Slf4j public class BotRepository extends EntityRepository { - static final String BOT_UPDATE_FIELDS = "botUser"; + static final String BOT_USER_FIELD = "botUser"; + static final String BOT_UPDATE_FIELDS = BOT_USER_FIELD; public BotRepository() { super( @@ -45,12 +48,56 @@ public BotRepository() { @Override public void setFields(Bot entity, Fields fields, RelationIncludes relationIncludes) { + // Always populate botUser on single-entity GET to preserve backward-compatible behavior for + // existing callers of GET /bots/{id} and GET /bots/name/{name}. The list path (bulk) is opt-in + // via ?fields=botUser. entity.withBotUser(getBotUser(entity)); } + @Override + public void setFieldsInBulk(Fields fields, List entities) { + if (entities == null || entities.isEmpty()) { + return; + } + if (fields.contains(BOT_USER_FIELD)) { + fetchAndSetBotUsers(entities); + } + super.setFieldsInBulk(fields, entities); + } + + private void fetchAndSetBotUsers(List bots) { + Map botUserMap = batchFetchBotUsers(bots); + for (Bot bot : bots) { + EntityReference botUserRef = botUserMap.get(bot.getId()); + if (botUserRef == null) { + botUserRef = getBotUser(bot); + } + bot.withBotUser(botUserRef); + } + } + + private Map batchFetchBotUsers(List bots) { + Map botUserMap = new HashMap<>(); + if (bots == null || bots.isEmpty()) { + return botUserMap; + } + List records = + daoCollection + .relationshipDAO() + .findToBatch(entityListToStrings(bots), Relationship.CONTAINS.ordinal(), Entity.USER); + for (CollectionDAO.EntityRelationshipObject record : records) { + UUID botId = UUID.fromString(record.getFromId()); + EntityReference userRef = + Entity.getEntityReferenceById( + Entity.USER, UUID.fromString(record.getToId()), Include.NON_DELETED); + botUserMap.put(botId, userRef); + } + return botUserMap; + } + @Override public void clearFields(Bot entity, Fields fields) { - /* Do nothing */ + /* Do nothing — botUser is always returned on single-entity GET (see setFields). */ } @Override diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/resources/bots/BotResource.java b/openmetadata-service/src/main/java/org/openmetadata/service/resources/bots/BotResource.java index 6175e4eac8fd..4e37db2aeeec 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/resources/bots/BotResource.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/resources/bots/BotResource.java @@ -81,6 +81,7 @@ @Collection(name = "bots", order = 4, requiredForOps = true) // initialize after user resource public class BotResource extends EntityResource { public static final String COLLECTION_PATH = "/v1/bots/"; + public static final String FIELDS = "botUser"; private final BotMapper mapper = new BotMapper(); public BotResource(Authorizer authorizer, Limits limits) { @@ -156,6 +157,11 @@ public static class BotList extends ResultList { public ResultList list( @Context UriInfo uriInfo, @Context SecurityContext securityContext, + @Parameter( + description = "Fields requested in the returned resource", + schema = @Schema(type = "string", example = FIELDS)) + @QueryParam("fields") + String fieldsParam, @DefaultValue("10") @Min(value = 0, message = "must be greater than or equal to 0") @Max(value = 1000000, message = "must be less than or equal to 1000000") @@ -178,7 +184,7 @@ public ResultList list( @DefaultValue("non-deleted") Include include) { return listInternal( - uriInfo, securityContext, "", new ListFilter(include), limitParam, before, after); + uriInfo, securityContext, fieldsParam, new ListFilter(include), limitParam, before, after); } @GET @@ -202,8 +208,13 @@ public Bot get( @Context SecurityContext securityContext, @QueryParam("include") @DefaultValue("non-deleted") Include include, @Parameter(description = "Id of the bot", schema = @Schema(type = "UUID")) @PathParam("id") - UUID id) { - return getInternal(uriInfo, securityContext, id, "", include); + UUID id, + @Parameter( + description = "Fields requested in the returned resource", + schema = @Schema(type = "string", example = FIELDS)) + @QueryParam("fields") + String fieldsParam) { + return getInternal(uriInfo, securityContext, id, fieldsParam, include); } @GET @@ -228,6 +239,11 @@ public Bot getByName( @Parameter(description = "Name of the bot", schema = @Schema(type = "string")) @PathParam("name") String name, + @Parameter( + description = "Fields requested in the returned resource", + schema = @Schema(type = "string", example = FIELDS)) + @QueryParam("fields") + String fieldsParam, @Parameter( description = "Include all, deleted, or non-deleted entities.", schema = @Schema(implementation = Include.class)) @@ -235,7 +251,7 @@ public Bot getByName( @DefaultValue("non-deleted") Include include) { return getByNameInternal( - uriInfo, securityContext, EntityInterfaceUtil.quoteName(name), "", include); + uriInfo, securityContext, EntityInterfaceUtil.quoteName(name), fieldsParam, include); } @GET From f66ce06ad0e7790b7d0ed01507fb6b17a92740bb Mon Sep 17 00:00:00 2001 From: Krati Paliwal Date: Sat, 25 Apr 2026 15:07:16 +0000 Subject: [PATCH 2/2] address comments --- .../service/jdbi3/BotRepository.java | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/BotRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/BotRepository.java index fe0c095e4199..97617ef545fe 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/BotRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/BotRepository.java @@ -17,6 +17,7 @@ import java.util.List; import java.util.Map; import java.util.UUID; +import java.util.stream.Collectors; import lombok.extern.slf4j.Slf4j; import org.jdbi.v3.sqlobject.transaction.Transaction; import org.openmetadata.schema.entity.Bot; @@ -77,20 +78,27 @@ private void fetchAndSetBotUsers(List bots) { } private Map batchFetchBotUsers(List bots) { - Map botUserMap = new HashMap<>(); if (bots == null || bots.isEmpty()) { - return botUserMap; + return Map.of(); } List records = daoCollection .relationshipDAO() .findToBatch(entityListToStrings(bots), Relationship.CONTAINS.ordinal(), Entity.USER); + if (records.isEmpty()) { + return Map.of(); + } + List userIds = + records.stream().map(r -> UUID.fromString(r.getToId())).distinct().toList(); + Map userRefById = + Entity.getEntityReferencesByIds(Entity.USER, userIds, Include.NON_DELETED).stream() + .collect(Collectors.toMap(ref -> ref.getId().toString(), ref -> ref)); + Map botUserMap = new HashMap<>(); for (CollectionDAO.EntityRelationshipObject record : records) { - UUID botId = UUID.fromString(record.getFromId()); - EntityReference userRef = - Entity.getEntityReferenceById( - Entity.USER, UUID.fromString(record.getToId()), Include.NON_DELETED); - botUserMap.put(botId, userRef); + EntityReference userRef = userRefById.get(record.getToId()); + if (userRef != null) { + botUserMap.put(UUID.fromString(record.getFromId()), userRef); + } } return botUserMap; }