Fixes #25878: listBots API does not return botUser field#27527
Fixes #25878: listBots API does not return botUser field#27527kratipaliwal wants to merge 1 commit intoopen-metadata:mainfrom
Conversation
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
| 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); | ||
| } |
There was a problem hiding this comment.
💡 Performance: batchFetchBotUsers still does N individual DB lookups per user
The batchFetchBotUsers method correctly fetches all bot→user relationships in one query via findToBatch, but then calls Entity.getEntityReferenceById(Entity.USER, ...) inside the loop for each record (line 91-92). This results in N individual database queries to resolve each user entity reference, partially defeating the purpose of the batch optimization.
For small bot counts this is fine, but for large deployments the list endpoint could issue hundreds of individual queries.
Consider collecting all toId UUIDs first, then using a bulk fetch (e.g., Entity.getEntityReferencesByIds or a single IN-clause query) to resolve all user references in one round-trip.
Suggested fix:
List<UUID> userIds = records.stream()
.map(r -> UUID.fromString(r.getToId()))
.collect(Collectors.toList());
Map<UUID, EntityReference> userRefs = Entity.getEntityReferencesByIds(Entity.USER, userIds, Include.NON_DELETED);
for (CollectionDAO.EntityRelationshipObject record : records) {
UUID botId = UUID.fromString(record.getFromId());
UUID userId = UUID.fromString(record.getToId());
botUserMap.put(botId, userRefs.get(userId));
}
Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion
Code Review 👍 Approved with suggestions 0 resolved / 1 findingsUpdates the listBots API to include the missing botUser field. Refactor batchFetchBotUsers to eliminate N+1 database lookups to optimize performance. 💡 Performance: batchFetchBotUsers still does N individual DB lookups per user📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/BotRepository.java:88-94 The For small bot counts this is fine, but for large deployments the list endpoint could issue hundreds of individual queries. Consider collecting all Suggested fix🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
Describe your changes:
Fixes #25878
GET /v1/bots(list) was never returning thebotUserfield, even when callers explicitly asked for it with?fields=botUser. ExistingGET /v1/bots/{id}andGET /v1/bots/name/{name}continued to returnbotUseras before.Two independent bugs produced this:
BotResourcedid not expose afieldsquery parameter.list(),get(), andgetByName()hardcoded""for fields, so?fields=botUserwas silently ignored.BotRepositoryonly populatedbotUserviasetFields(single-entity path). The list endpoint usessetFieldsInBulk→fetchAndSetFields, which dispatches through registeredfieldFetchers. Combined withgetFieldsStrippedFromStorageJson()strippingbotUserfrom stored JSON, listed bots came back without it.Fix
BotResource: added aFIELDS = "botUser"constant and a@QueryParam("fields")onlist,get, andgetByName, forwarded to the internal handlers (previously the param was silently dropped).BotRepository: overrodesetFieldsInBulkto batch-fetchbotUserviarelationshipDAO.findToBatch(..., CONTAINS, USER)whenfields.contains("botUser"). Single-entitysetFieldsremains unconditional to preserve backward compatibility for existing UI and SDK callers ofGET /bots/{id}andGET /bots/name/{name}.Final behavior
?fieldsnot passed?fields=botUserGET /v1/bots/{id}botUserreturned (unchanged)botUserreturnedGET /v1/bots/name/{name}botUserreturned (unchanged)botUserreturnedGET /v1/bots(list)botUseromittedbotUserreturnedNo breaking change for existing consumers of the single-entity GET endpoints. The list endpoint gains the missing
?fields=botUsersupport described in the Swagger docs.Tests
Added regression integration tests in
BotResourceITcovering the list, get, create, and patch paths:test_listBots_withoutFieldsParam_omitsBotUser— list without?fieldsdoes not populatebotUser.test_listBots_withFieldsParam_returnsBotUser— list with?fields=botUserreturns the relationship.test_listBots_multipleBots_allHaveBotUser— exercises the bulk-fetch code path with multiple bots.test_getBotById_alwaysReturnsBotUser/test_getBotByName_alwaysReturnsBotUser— backward-compat guards: single-entity GETs must continue to returnbotUserwithout?fields.test_getBotById_withFieldsParam_returnsBotUser/test_getBotByName_withFieldsParam_returnsBotUser— explicit?fields=botUserstill works on single-entity GETs.test_createBot_responseIncludesBotUser— POST response continues to includebotUser.test_patchBotDescription_preservesBotUser— patching description does not drop thebotUserrelationship.test_patchBot_cannotChangeBotUser— guards the immutability contract inrestorePatchAttributes.Type of change:
Checklist:
Fixes <issue-number>: <short explanation>