Skip to content

Commit a9be6d4

Browse files
committed
fix: npm repository url gap
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
1 parent 6fb17af commit a9be6d4

6 files changed

Lines changed: 338 additions & 7 deletions

File tree

services/apps/packages_worker/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@
5656
"backfill:maven:local": "set -a && . ../../../backend/.env.dist.local && . ../../../backend/.env.override.local && set +a && SERVICE=maven LOG_LEVEL=info tsx src/bin/maven-backfill.ts",
5757
"backfill:maven-repo-url": "SERVICE=maven tsx src/bin/maven-repo-url-backfill.ts",
5858
"backfill:maven-repo-url:local": "set -a && . ../../../backend/.env.dist.local && . ../../../backend/.env.override.local && set +a && SERVICE=maven LOG_LEVEL=info tsx src/bin/maven-repo-url-backfill.ts",
59+
"backfill:npm-repo-url": "SERVICE=npm tsx src/bin/npm-repo-url-backfill.ts",
60+
"backfill:npm-repo-url:local": "set -a && . ../../../backend/.env.dist.local && . ../../../backend/.env.override.local && set +a && SERVICE=npm LOG_LEVEL=info tsx src/bin/npm-repo-url-backfill.ts",
5961
"import:maven-maintainers": "SERVICE=maven tsx src/maven/scripts/importMaintainersFromCsv.ts",
6062
"import:maven-maintainers:local": "set -a && . ../../../backend/.env.dist.local && . ../../../backend/.env.override.local && set +a && SERVICE=maven tsx src/maven/scripts/importMaintainersFromCsv.ts",
6163
"import:sonatype-popularity": "SERVICE=maven tsx src/maven/scripts/importSonatypePopularityFromCsv.ts",
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
import { getServiceLogger } from '@crowd/logging'
2+
3+
import { getPackagesDb } from '../db'
4+
import { backfillNpmRepositoryUrls } from '../npm/backfillRepositoryUrl'
5+
6+
const log = getServiceLogger()
7+
8+
let shuttingDown = false
9+
10+
// Graceful stop: finish the in-flight batch, then exit. Safe to interrupt — every
11+
// write is an idempotent UPDATE recomputed from declared_repository_url, so
12+
// re-running simply reprocesses and skips rows that already match.
13+
const shutdown = () => {
14+
if (shuttingDown) return
15+
shuttingDown = true
16+
log.info('Shutting down npm repo-url backfill (stopping after the current batch)...')
17+
}
18+
19+
process.on('SIGINT', shutdown)
20+
process.on('SIGTERM', shutdown)
21+
22+
const DEFAULT_BATCH_SIZE = 5000
23+
24+
const main = async () => {
25+
const dryRun = process.argv.includes('--dry-run')
26+
const criticalOnly = process.argv.includes('--critical-only')
27+
const rawBatchSize = process.env.NPM_REPO_URL_BACKFILL_BATCH_SIZE
28+
const parsedBatchSize = rawBatchSize === undefined ? DEFAULT_BATCH_SIZE : Number(rawBatchSize)
29+
if (!Number.isInteger(parsedBatchSize) || parsedBatchSize <= 0) {
30+
log.error(
31+
{ NPM_REPO_URL_BACKFILL_BATCH_SIZE: rawBatchSize },
32+
'NPM_REPO_URL_BACKFILL_BATCH_SIZE must be a positive integer',
33+
)
34+
process.exit(1)
35+
}
36+
const batchSize = parsedBatchSize
37+
38+
log.info(
39+
{ dryRun, criticalOnly, batchSize },
40+
'npm repo-url backfill starting (recompute canonicalizeRepoUrl from declared_repository_url, no packument fetch)...',
41+
)
42+
43+
const qx = await getPackagesDb()
44+
await qx.selectOne('SELECT 1')
45+
log.info('Connected to packages-db.')
46+
47+
const totals = await backfillNpmRepositoryUrls(qx, {
48+
batchSize,
49+
dryRun,
50+
criticalOnly,
51+
isShuttingDown: () => shuttingDown,
52+
})
53+
54+
log.info({ ...totals, dryRun }, 'npm repo-url backfill complete')
55+
process.exit(0)
56+
}
57+
58+
main().catch((err) => {
59+
log.error({ err }, 'npm repo-url backfill fatal error')
60+
process.exit(1)
61+
})
Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
import { deleteMavenPackageRepoLinks } from '@crowd/data-access-layer'
2+
import {
3+
NpmRepoUrlRow,
4+
getOrCreateRepoByUrl,
5+
listNpmPackagesForRepoUrlRecompute,
6+
updateNpmRepositoryUrls,
7+
upsertPackageRepo,
8+
} from '@crowd/data-access-layer/src/packages'
9+
import { QueryExecutor } from '@crowd/data-access-layer/src/queryExecutor'
10+
import { getServiceChildLogger } from '@crowd/logging'
11+
12+
import { canonicalizeRepoUrl } from '../utils/canonicalizeRepoUrl'
13+
14+
const log = getServiceChildLogger('npm-repo-url-backfill')
15+
16+
// Postgres deadlock (40P01) is transient: concurrent transactions upserting the same shared
17+
// rows (e.g. the same repo linked from many packages) can form a lock cycle. Re-running the
18+
// whole transaction resolves it — the upserts are idempotent. Mirrors Maven's
19+
// withDeadlockRetry (src/maven/runMavenEnrichmentLoop.ts).
20+
async function withDeadlockRetry<T>(fn: () => Promise<T>, maxAttempts = 4): Promise<T> {
21+
for (let attempt = 1; ; attempt++) {
22+
try {
23+
return await fn()
24+
} catch (err) {
25+
const code = (err as { code?: string }).code
26+
const isDeadlock =
27+
code === '40P01' || /deadlock detected/i.test(String((err as Error)?.message))
28+
if (isDeadlock && attempt < maxAttempts) {
29+
await new Promise((r) => setTimeout(r, 50 * attempt + Math.random() * 100))
30+
log.debug({ attempt }, 'Deadlock detected — retrying transaction')
31+
continue
32+
}
33+
throw err
34+
}
35+
}
36+
}
37+
38+
export type RepoUrlBackfillTotals = {
39+
scanned: number
40+
filled: number // NULL → canonical value
41+
cleared: number // canonical value → NULL (e.g. a stricter normalizer no longer accepts it)
42+
rewritten: number // non-canonical value → different canonical value
43+
unchanged: number
44+
linked: number // repos/package_repos link (re)written for a fill or rewrite
45+
pruned: number // stale 'declared' link removed for a clear or rewrite
46+
}
47+
48+
/**
49+
* Recomputes `repository_url` for every npm row directly from the stored
50+
* `declared_repository_url`, applying the current `canonicalizeRepoUrl`. No
51+
* packument is re-fetched — the raw declared value is already in the DB. Fills
52+
* recoverable NULLs (e.g. scp-form ssh:// URLs the previous normalizer dropped).
53+
*
54+
* `cleared` only accounts for values a *future* normalizer change stops
55+
* accepting for a row that already has a repository_url — the current
56+
* canonicalizeRepoUrl only ever adds recognized cases, so this backfill
57+
* doesn't clear any values on its own today.
58+
*
59+
* Link table: package_repos is kept consistent with the recomputed
60+
* repository_url. For a fill (previously null) or a rewrite (value changes) the
61+
* canonical repo link is (re)written via getOrCreateRepoByUrl/upsertPackageRepo —
62+
* the same pair the ingest path uses. A rewrite first prunes the stale
63+
* source='declared' link so a package never carries two 'declared' links to
64+
* different repos.
65+
*
66+
* Idempotent and resumable: the id cursor is derived from the scan, so a
67+
* re-run after an interrupt simply reprocesses from the start and skips rows
68+
* that already match.
69+
*/
70+
export async function backfillNpmRepositoryUrls(
71+
qx: QueryExecutor,
72+
options: {
73+
batchSize: number
74+
dryRun: boolean
75+
criticalOnly: boolean
76+
isShuttingDown: () => boolean
77+
},
78+
): Promise<RepoUrlBackfillTotals> {
79+
const { batchSize, dryRun, criticalOnly, isShuttingDown } = options
80+
const totals: RepoUrlBackfillTotals = {
81+
scanned: 0,
82+
filled: 0,
83+
cleared: 0,
84+
rewritten: 0,
85+
unchanged: 0,
86+
linked: 0,
87+
pruned: 0,
88+
}
89+
90+
let afterId = '0'
91+
for (;;) {
92+
if (isShuttingDown()) {
93+
log.info('Shutdown requested — stopping after the current batch.')
94+
break
95+
}
96+
97+
const rows: NpmRepoUrlRow[] = await listNpmPackagesForRepoUrlRecompute(qx, {
98+
afterId,
99+
limit: batchSize,
100+
criticalOnly,
101+
})
102+
if (rows.length === 0) break
103+
104+
const updates: { id: string; repositoryUrl: string | null }[] = []
105+
// Rows that had a link and now change to a different one — their stale 'declared' link is pruned.
106+
const pruneTargets: number[] = []
107+
// Rows that gained/changed a canonical URL — their repo link is (re)written after the update.
108+
const linkTargets: { id: string; repositoryUrl: string; host: string }[] = []
109+
for (const row of rows) {
110+
totals.scanned++
111+
// declaredRepositoryUrl is guaranteed non-null by the query's IS NOT NULL filter.
112+
const canonical = canonicalizeRepoUrl(row.declaredRepositoryUrl as string)
113+
const desired = canonical?.url ?? null
114+
if (desired === row.repositoryUrl) {
115+
totals.unchanged++
116+
continue
117+
}
118+
if (row.repositoryUrl === null) totals.filled++
119+
else if (desired === null) totals.cleared++
120+
else totals.rewritten++
121+
updates.push({ id: row.id, repositoryUrl: desired })
122+
if (row.repositoryUrl !== null && desired !== row.repositoryUrl) {
123+
pruneTargets.push(Number(row.id))
124+
}
125+
if (canonical)
126+
linkTargets.push({ id: row.id, repositoryUrl: canonical.url, host: canonical.host })
127+
}
128+
129+
if (updates.length > 0 && !dryRun) {
130+
// Atomic per batch: the repository_url UPDATE, the stale-link prune, and the
131+
// relink must commit together — otherwise an interrupt between them leaves
132+
// packages.repository_url updated but package_repos out of sync, and a
133+
// re-run would skip the row (its repository_url already matches `desired`),
134+
// so the inconsistency would never be repaired.
135+
await withDeadlockRetry(() =>
136+
qx.tx(async (t: QueryExecutor) => {
137+
await updateNpmRepositoryUrls(t, updates)
138+
if (pruneTargets.length > 0) await deleteMavenPackageRepoLinks(t, pruneTargets)
139+
for (const target of linkTargets) {
140+
const { id: repoId } = await getOrCreateRepoByUrl(t, target.repositoryUrl, target.host)
141+
await upsertPackageRepo(t, target.id, repoId, 'declared', 0.8)
142+
}
143+
}),
144+
)
145+
totals.pruned += pruneTargets.length
146+
totals.linked += linkTargets.length
147+
}
148+
149+
afterId = rows[rows.length - 1].id
150+
log.info(
151+
{ afterId, changes: updates.length, dryRun, criticalOnly, ...totals },
152+
'Backfill progress',
153+
)
154+
}
155+
156+
return totals
157+
}

services/apps/packages_worker/src/utils/__tests__/canonicalizeRepoUrl.test.ts

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,12 @@ describe('canonicalizeRepoUrl', () => {
2222
['gitlab:group/project', 'https://gitlab.com/group/project', 'gitlab'],
2323
['bitbucket:team/repo', 'https://bitbucket.org/team/repo', 'bitbucket'],
2424
['https://example.com/owner/repo', 'https://example.com/owner/repo', 'other'],
25+
['git+https://github.com/1aGh/md-claude.git', 'https://github.com/1agh/md-claude', 'github'],
26+
[
27+
'ssh://git@github.com:1inch/limit-order-protocol-utils.git',
28+
'https://github.com/1inch/limit-order-protocol-utils',
29+
'github',
30+
],
2531
])('canonicalizes %s', (input, expectedUrl, expectedHost) => {
2632
expect(canonicalizeRepoUrl(input)).toEqual({ url: expectedUrl, host: expectedHost })
2733
})
@@ -44,10 +50,14 @@ describe('canonicalizeRepoUrl', () => {
4450
})
4551
})
4652

47-
it.each([['not a url'], ['https://github.com/onlyowner'], [''], [' ']])(
48-
'returns null for unparseable input %s',
49-
(input) => {
50-
expect(canonicalizeRepoUrl(input)).toBeNull()
51-
},
52-
)
53+
it.each([
54+
['not a url'],
55+
['https://github.com/onlyowner'],
56+
[''],
57+
[' '],
58+
['123'],
59+
['https://github.com/Wscats'],
60+
])('returns null for unparseable input %s', (input) => {
61+
expect(canonicalizeRepoUrl(input)).toBeNull()
62+
})
5363
})

services/apps/packages_worker/src/utils/canonicalizeRepoUrl.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,15 @@ export function canonicalizeRepoUrl(raw: string): CanonicalRepo | null {
5454
s = `https://${scp[1]}/${scp[2]}`
5555
}
5656

57-
s = s.replace(/^ssh:\/\/git@([^/]+)\//, 'https://$1/')
57+
// ssh:// with an scp-style `host:path` (colon instead of slash) is not valid URL
58+
// syntax — the part after `:` looks like a port to the URL parser and throws.
59+
// Rewrite it before the generic ssh://git@host/path case below.
60+
const sshScp = s.match(/^ssh:\/\/git@([^/:]+):(.+)$/)
61+
if (sshScp) {
62+
s = `https://${sshScp[1]}/${sshScp[2]}`
63+
} else {
64+
s = s.replace(/^ssh:\/\/git@([^/]+)\//, 'https://$1/')
65+
}
5866
s = s.replace(/^git:\/\//, 'https://')
5967

6068
let u: URL

services/libs/data-access-layer/src/packages/packages.ts

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,99 @@ export async function getTrackedNpmPackages(
226226
}))
227227
}
228228

229+
// ─── npm repository_url backfill ──────────────────────────────────────────────
230+
231+
export type NpmRepoUrlRow = {
232+
id: string
233+
declaredRepositoryUrl: string | null
234+
repositoryUrl: string | null
235+
}
236+
237+
/**
238+
* Keyset-paginated scan of npm rows that carry a declared repository value.
239+
* The backfill recomputes repository_url *from* declared_repository_url, so rows
240+
* with no declared value are skipped — there is nothing to recompute from, and a
241+
* null declaration must never be used to clear an existing repository_url that a
242+
* different source may have set.
243+
*
244+
* `criticalOnly` restricts the scan to is_critical rows — used for a fast,
245+
* consumer-facing first pass.
246+
*/
247+
export async function listNpmPackagesForRepoUrlRecompute(
248+
qx: QueryExecutor,
249+
options: { afterId: string; limit: number; criticalOnly?: boolean },
250+
): Promise<NpmRepoUrlRow[]> {
251+
const rows: Array<{
252+
id: string
253+
declared_repository_url: string | null
254+
repository_url: string | null
255+
}> = await qx.select(
256+
`
257+
SELECT
258+
id::text AS id,
259+
declared_repository_url,
260+
repository_url
261+
FROM packages
262+
WHERE ecosystem = 'npm'
263+
${options.criticalOnly ? 'AND is_critical' : ''}
264+
AND id > $(afterId)::bigint
265+
AND declared_repository_url IS NOT NULL
266+
ORDER BY id ASC
267+
LIMIT $(limit)
268+
`,
269+
{ afterId: options.afterId, limit: options.limit },
270+
)
271+
return rows.map((r) => ({
272+
id: r.id,
273+
declaredRepositoryUrl: r.declared_repository_url,
274+
repositoryUrl: r.repository_url,
275+
}))
276+
}
277+
278+
/**
279+
* Applies a batch of recomputed repository_url values via direct UPDATE — the
280+
* only way to clear a stale value, since the enrichment upsert writes
281+
* EXCLUDED.repository_url unconditionally but is only ever called with a fresh
282+
* packument fetch. Splits clears (→ NULL) from sets to avoid NULLs inside a
283+
* text[] array literal. Also bumps last_synced_at so the correction is picked
284+
* up by any downstream export keyed off it.
285+
*/
286+
export async function updateNpmRepositoryUrls(
287+
qx: QueryExecutor,
288+
updates: { id: string; repositoryUrl: string | null }[],
289+
): Promise<void> {
290+
if (updates.length === 0) return
291+
292+
const toClear = updates.filter((u) => u.repositoryUrl === null).map((u) => u.id)
293+
const toSet = updates.filter(
294+
(u): u is { id: string; repositoryUrl: string } => u.repositoryUrl !== null,
295+
)
296+
297+
if (toClear.length > 0) {
298+
await qx.result(
299+
`UPDATE packages SET repository_url = NULL, last_synced_at = NOW()
300+
WHERE id = ANY($(ids)::bigint[]) AND repository_url IS NOT NULL`,
301+
{ ids: toClear },
302+
)
303+
}
304+
305+
if (toSet.length > 0) {
306+
await qx.result(
307+
`
308+
UPDATE packages p
309+
SET repository_url = v.repository_url, last_synced_at = NOW()
310+
FROM (
311+
SELECT unnest($(ids)::bigint[]) AS id,
312+
unnest($(urls)::text[]) AS repository_url
313+
) v
314+
WHERE p.id = v.id
315+
AND p.repository_url IS DISTINCT FROM v.repository_url
316+
`,
317+
{ ids: toSet.map((u) => u.id), urls: toSet.map((u) => u.repositoryUrl) },
318+
)
319+
}
320+
}
321+
229322
// How many critical PyPI packages exist — a cheap guard so the daily downloads workflow can
230323
// skip its BigQuery scan entirely when there are none to ingest (the merge scopes to is_critical).
231324
export async function getCriticalPypiPackageCount(qx: QueryExecutor): Promise<number> {

0 commit comments

Comments
 (0)