Skip to content

Commit 79e3552

Browse files
committed
fix: inconsistencies
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
1 parent 2e50265 commit 79e3552

3 files changed

Lines changed: 16 additions & 6 deletions

File tree

services/apps/packages_worker/src/npm/backfillRepositoryUrl.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { deleteMavenPackageRepoLinks } from '@crowd/data-access-layer'
21
import {
32
NpmRepoUrlRow,
43
getOrCreateRepoByUrl,
@@ -103,7 +102,8 @@ export async function backfillNpmRepositoryUrls(
103102

104103
const updates: { id: string; repositoryUrl: string | null }[] = []
105104
// Rows that had a link and now change to a different one — their stale 'declared' link is pruned.
106-
const pruneTargets: number[] = []
105+
// Kept as strings (packages.id is bigint) — Number() coercion would lose precision above 2^53.
106+
const pruneTargets: string[] = []
107107
// Rows that gained/changed a canonical URL — their repo link is (re)written after the update.
108108
const linkTargets: { id: string; repositoryUrl: string; host: string }[] = []
109109
for (const row of rows) {
@@ -120,7 +120,7 @@ export async function backfillNpmRepositoryUrls(
120120
else totals.rewritten++
121121
updates.push({ id: row.id, repositoryUrl: desired })
122122
if (row.repositoryUrl !== null && desired !== row.repositoryUrl) {
123-
pruneTargets.push(Number(row.id))
123+
pruneTargets.push(row.id)
124124
}
125125
if (canonical)
126126
linkTargets.push({ id: row.id, repositoryUrl: canonical.url, host: canonical.host })
@@ -135,7 +135,13 @@ export async function backfillNpmRepositoryUrls(
135135
await withDeadlockRetry(() =>
136136
qx.tx(async (t: QueryExecutor) => {
137137
await updateNpmRepositoryUrls(t, updates)
138-
if (pruneTargets.length > 0) await deleteMavenPackageRepoLinks(t, pruneTargets)
138+
if (pruneTargets.length > 0) {
139+
await t.result(
140+
`DELETE FROM package_repos
141+
WHERE package_id = ANY($(packageIds)::bigint[]) AND source = 'declared'`,
142+
{ packageIds: pruneTargets },
143+
)
144+
}
139145
for (const target of linkTargets) {
140146
const { id: repoId } = await getOrCreateRepoByUrl(t, target.repositoryUrl, target.host)
141147
await upsertPackageRepo(t, target.id, repoId, 'declared', 0.8)

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ describe('canonicalizeRepoUrl', () => {
2828
'https://github.com/1inch/limit-order-protocol-utils',
2929
'github',
3030
],
31+
['ssh://git@github.com:2222/foo/bar.git', 'https://github.com/foo/bar', 'github'],
3132
])('canonicalizes %s', (input, expectedUrl, expectedHost) => {
3233
expect(canonicalizeRepoUrl(input)).toEqual({ url: expectedUrl, host: expectedHost })
3334
})

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,12 @@ export function canonicalizeRepoUrl(raw: string): CanonicalRepo | null {
5656

5757
// ssh:// with an scp-style `host:path` (colon instead of slash) is not valid URL
5858
// 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.
59+
// Rewrite it before the generic ssh://git@host/path case below. A numeric-only
60+
// segment before the next `/` is a real port (e.g. `ssh://git@host:2222/owner/repo`),
61+
// not an scp-style owner — leave those for the generic case, which URL parses fine.
6062
const sshScp = s.match(/^ssh:\/\/git@([^/:]+):(.+)$/)
61-
if (sshScp) {
63+
const sshScpIsPort = sshScp ? /^\d+$/.test(sshScp[2].split('/')[0]) : false
64+
if (sshScp && !sshScpIsPort) {
6265
s = `https://${sshScp[1]}/${sshScp[2]}`
6366
} else {
6467
s = s.replace(/^ssh:\/\/git@([^/]+)\//, 'https://$1/')

0 commit comments

Comments
 (0)