Fixes #25878: listBots API does not return botUser field

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;
@@ -283,6 +285,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<Bot> 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<Bot> 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<UUID> createdIds = java.util.Set.of(bot1.getId(), bot2.getId(), bot3.getId());
+    java.util.Map<UUID, UUID> expectedUserByBot =
+        java.util.Map.of(
+            bot1.getId(), user1.getId(),
+            bot2.getId(), user2.getId(),
+            bot3.getId(), user3.getId());
+
+    ListResponse<Bot> 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();

openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/BotRepository.java

@@ -13,8 +13,11 @@
 
 package org.openmetadata.service.jdbi3;
 
+import java.util.HashMap;
 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;
@@ -30,7 +33,8 @@
 
 @Slf4j
 public class BotRepository extends EntityRepository<Bot> {
-  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 +49,63 @@ 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<Bot> entities) {
+    if (entities == null || entities.isEmpty()) {
+      return;
+    }
+    if (fields.contains(BOT_USER_FIELD)) {
+      fetchAndSetBotUsers(entities);
+    }
+    super.setFieldsInBulk(fields, entities);
+  }
+
+  private void fetchAndSetBotUsers(List<Bot> bots) {
+    Map<UUID, EntityReference> botUserMap = batchFetchBotUsers(bots);
+    for (Bot bot : bots) {
+      EntityReference botUserRef = botUserMap.get(bot.getId());
+      if (botUserRef == null) {
+        botUserRef = getBotUser(bot);
+      }
+      bot.withBotUser(botUserRef);
+    }
+  }
+
+  private Map<UUID, EntityReference> batchFetchBotUsers(List<Bot> bots) {
+    if (bots == null || bots.isEmpty()) {
+      return Map.of();
+    }
+    List<CollectionDAO.EntityRelationshipObject> records =
+        daoCollection
+            .relationshipDAO()
+            .findToBatch(entityListToStrings(bots), Relationship.CONTAINS.ordinal(), Entity.USER);
+    if (records.isEmpty()) {
+      return Map.of();
+    }
+    List<UUID> userIds =
+        records.stream().map(r -> UUID.fromString(r.getToId())).distinct().toList();
+    Map<String, EntityReference> userRefById =
+        Entity.getEntityReferencesByIds(Entity.USER, userIds, Include.NON_DELETED).stream()
+            .collect(Collectors.toMap(ref -> ref.getId().toString(), ref -> ref));
+    Map<UUID, EntityReference> botUserMap = new HashMap<>();
+    for (CollectionDAO.EntityRelationshipObject record : records) {
+      EntityReference userRef = userRefById.get(record.getToId());
+      if (userRef != null) {
+        botUserMap.put(UUID.fromString(record.getFromId()), 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