From b4bc6462a3e45c79a3cf21d35d091ac0d7a5effa Mon Sep 17 00:00:00 2001 From: jmcqueen Date: Tue, 7 Jul 2026 10:38:28 -0400 Subject: [PATCH] Fix Assets sync: correct AQL pagination + Jira Cloud CMDB field format Two bugs found during end-to-end smoke test on the stores cache workaround (Forgejo #8): 1. Wrong pagination params on POST /object/aql. The endpoint uses startAt/maxResults as URL query params, not page/resultPerPage in the body. Passing the wrong param names caused the API to silently fall back to the default maxResults=25, so the first sync only cached 25 stores. After fix: 2,661 stores paginated across 6 pages in 5.3s. 2. Wrong request-field shape for CMDB-object custom fields on Jira Cloud. resolveStoreAssetReference was returning [{ objectId: "75974" }] which is the legacy Data Center / Server shape. Cloud requires [{ id: ":" }] The old shape is silently accepted (HTTP 204) by REST and by POST /rest/servicedeskapi/request, but the field is never actually persisted \u2014 verified via direct REST GET showing customfield_10261:[]. After fix: SS-20948 shows the store correctly populated. - assetsSyncClient.js: use URLSearchParams to pass startAt/maxResults/includeAttributes; drop the old page/resultPerPage opts; jsdoc updated with pointer to Atlassian Assets API v1 docs. - storesCache.js: pagination loop now advances by startAt += values.length and trusts isLast (Atlassian caps `total` at 1000 as a hint on this endpoint, so we can't rely on it). - assets.js: new buildStoreFieldRef(objectId) helper produces the correct Cloud shape from config.jira.assetsWorkspaceId; all three resolution paths (cache / live PAT / service-account fallback) route through it. Falls back to legacy shape with a loud error log if workspaceId is unset (misconfig). End-to-end verified: SS-20948 was created for store 00782 (objectId 75974) via cache-only lookup and shows the field populated on both the JSM request echo and a direct REST GET. Co-authored-by: Cursor --- src/services/jira/assets.js | 28 ++++++++++++++++++++++----- src/services/jira/assetsSyncClient.js | 22 +++++++++++++++++---- src/services/jira/storesCache.js | 28 +++++++++++++++++---------- 3 files changed, 59 insertions(+), 19 deletions(-) diff --git a/src/services/jira/assets.js b/src/services/jira/assets.js index 776ac54..963f978 100644 --- a/src/services/jira/assets.js +++ b/src/services/jira/assets.js @@ -236,10 +236,28 @@ function summarizeAssetsObject(obj) { }; } +/** + * Build the JSM/Jira Cloud request-field value for a CMDB-object custom field. + * On Jira Cloud the CMDB field expects `{ id: ":" }` — + * NOT the old `{ objectId }` shape from Data Center / Server, which is + * silently accepted (HTTP 204) but never actually persists to the ticket. + */ +function buildStoreFieldRef(objectId) { + const workspaceId = config.jira.assetsWorkspaceId; + if (!workspaceId) { + // Fall back to the legacy shape so at least *something* is sent. The + // caller will get an empty field on the created ticket, but we log the + // config problem loudly. + logger.error('JIRA_ASSETS_WORKSPACE_ID not configured; store custom field write will silently no-op on Jira Cloud'); + return [{ objectId: String(objectId) }]; + } + return [{ id: `${workspaceId}:${objectId}` }]; +} + /** * Resolve a store number (e.g. "00305" or 305) to the Assets object reference * used for the Store custom field on a JSM request: - * customfield_10261: [ { "objectId": "82288" } ] + * customfield_10261: [ { "id": ":" } ] * * Resolution order (each step logged so we can tell which path served the * lookup): @@ -269,7 +287,7 @@ export async function resolveStoreAssetReference(rawStoreNumber) { objectId: cached.objectId, cacheAge: storesCache.status().ageSeconds, }); - return [{ objectId: String(cached.objectId) }]; + return buildStoreFieldRef(cached.objectId); } // 2. Live lookup via the personal-PAT sync client (handles brand-new @@ -277,7 +295,7 @@ export async function resolveStoreAssetReference(rawStoreNumber) { if (isAssetsSyncConfigured()) { const ql = `objectTypeId = ${objectTypeId} AND "${attribute}" = "${normalized}"`; logger.info('Cache miss; trying live PAT lookup for store', { storeNumber: normalized, ql }); - const r = await syncAssetsAql(ql, { resultPerPage: 1, includeAttributes: false }); + const r = await syncAssetsAql(ql, { maxResults: 1, includeAttributes: false }); if (r.status === 200) { const values = Array.isArray(r.data?.values) ? r.data.values : []; if (values.length > 0) { @@ -287,7 +305,7 @@ export async function resolveStoreAssetReference(rawStoreNumber) { storeNumber: normalized, objectId: String(objectId), }); - return [{ objectId: String(objectId) }]; + return buildStoreFieldRef(objectId); } } logger.warn('Live PAT lookup returned 200 but no matching object', { @@ -350,7 +368,7 @@ export async function resolveStoreAssetReference(rawStoreNumber) { } logger.info('Resolved store via service-account AQL fallback', { storeNumber: normalized, objectId: String(objectId) }); - return [{ objectId: String(objectId) }]; + return buildStoreFieldRef(objectId); } // All three paths missed — surface a message that tells the caller which diff --git a/src/services/jira/assetsSyncClient.js b/src/services/jira/assetsSyncClient.js index 6e990c7..658ad0c 100644 --- a/src/services/jira/assetsSyncClient.js +++ b/src/services/jira/assetsSyncClient.js @@ -70,7 +70,16 @@ export function describeSyncIdentity() { * returns `{ status, statusText, data, headers, requestUrl, requestBody, error }`. * * `qlQuery` raw AQL string, e.g. `objectTypeId = 109` - * `opts` { page, resultPerPage, includeAttributes, extraBody, timeoutMs } + * `opts` { startAt, maxResults, includeAttributes, extraBody, timeoutMs } + * + * Endpoint reference: Atlassian Assets REST API v1 `POST /object/aql` + * - Query params: `startAt` (default 0), `maxResults` (default 25, cap + * varies by tenant but 500 is safe), `includeAttributes` (default true) + * - Body: `{ "qlQuery": "..." }` + * - Response: `{ startAt, maxResults, total, isLast, values: [...] }` + * + * The old `page` / `resultPerPage` params are for a different endpoint and + * are silently ignored here — always use startAt/maxResults on this one. */ export async function assetsAql(qlQuery, opts = {}) { const auth = getAuthHeader(); @@ -94,14 +103,19 @@ export async function assetsAql(qlQuery, opts = {}) { } const { - page = 1, - resultPerPage = 500, + startAt = 0, + maxResults = 500, includeAttributes = true, extraBody = {}, timeoutMs = 30000, } = opts; - const url = `${ASSETS_HOST}/jsm/assets/workspace/${workspaceId}/v1/object/aql?page=${page}&resultPerPage=${resultPerPage}&includeAttributes=${includeAttributes}`; + const qp = new URLSearchParams({ + startAt: String(startAt), + maxResults: String(maxResults), + includeAttributes: String(includeAttributes), + }); + const url = `${ASSETS_HOST}/jsm/assets/workspace/${workspaceId}/v1/object/aql?${qp.toString()}`; const body = { qlQuery, ...extraBody }; try { diff --git a/src/services/jira/storesCache.js b/src/services/jira/storesCache.js index 928580c..ec8647d 100644 --- a/src/services/jira/storesCache.js +++ b/src/services/jira/storesCache.js @@ -171,15 +171,19 @@ export async function refresh({ force = false } = {}) { try { const newCache = new Map(); - let page = 1; + // Atlassian Assets `/object/aql` paginates via startAt + maxResults + + // isLast. Some responses also cap `total` at 1000 as a hint, so we + // trust isLast (or an empty/short page) rather than the reported total. + let startAt = 0; + let page = 0; let seenTotal = 0; let expectedTotal = null; let orphaned = 0; - while (page <= maxPages) { + while (page < maxPages) { const r = await assetsAql(`objectTypeId = ${objectTypeId}`, { - page, - resultPerPage: pageSize, + startAt, + maxResults: pageSize, includeAttributes: true, }); @@ -188,11 +192,12 @@ export async function refresh({ force = false } = {}) { ? JSON.stringify(r.data).slice(0, 400) : String(r.data).slice(0, 400); throw new Error( - `Assets AQL page ${page} returned HTTP ${r.status} (${r.statusText}). Body: ${bodySnippet}` + `Assets AQL page ${page + 1} (startAt=${startAt}) returned HTTP ${r.status} (${r.statusText}). Body: ${bodySnippet}` ); } const values = Array.isArray(r.data?.values) ? r.data.values : []; + const returnedMaxResults = typeof r.data?.maxResults === 'number' ? r.data.maxResults : pageSize; if (expectedTotal === null && typeof r.data?.total === 'number') { expectedTotal = r.data.total; } @@ -211,13 +216,16 @@ export async function refresh({ force = false } = {}) { }); } seenTotal += values.length; + page++; const isLast = r.data?.isLast === true - || values.length < pageSize - || (expectedTotal !== null && seenTotal >= expectedTotal); + || values.length === 0 + || values.length < returnedMaxResults; logger.debug('Stores cache sync page', { page, + startAt, + returnedMaxResults, rowsThisPage: values.length, seenTotal, expectedTotal, @@ -226,11 +234,11 @@ export async function refresh({ force = false } = {}) { isLast, }); - if (values.length === 0 || isLast) break; - page++; + if (isLast) break; + startAt += values.length; } - if (page > maxPages) { + if (page >= maxPages) { logger.warn('Stores cache sync hit page cap; some stores may be missing', { maxPages, pageSize,