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: "<workspaceId>:<objectId>" }] 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 <cursoragent@cursor.com>
This commit is contained in:
jmcqueen
parent
66255a7b0c
commit
b4bc6462a3
3 changed files with 59 additions and 19 deletions
|
|
@ -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: "<workspaceId>:<objectId>" }` —
|
||||
* 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": "<workspaceId>:<objectId>" } ]
|
||||
*
|
||||
* 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
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Reference in a new issue