fix: add force to backfill

Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>

Author: ulemons

services/apps/packages_worker/src/bin/maven-backfill.ts

@@ -2,7 +2,10 @@ import { getServiceLogger } from '@crowd/logging'
 
 import { getMavenConfig } from '../config'
 import { getPackagesDb } from '../db'
-import { runMavenCriticalBackfill } from '../maven/runMavenEnrichmentLoop'
+import {
+  runMavenCriticalBackfill,
+  runMavenCriticalForceBackfill,
+} from '../maven/runMavenEnrichmentLoop'
 
 const log = getServiceLogger()
 
@@ -25,7 +28,14 @@ const main = async () => {
     process.env.MAVEN_FETCHER_BASE_URL_BACKFILL ??
     'https://maven-central.storage-download.googleapis.com/maven2'
 
-  log.info('maven backfill starting (one-shot, full extraction)...')
+  // --force: re-run POM extraction over EVERY critical row, ignoring the
+  // staleness window. Use to fully re-apply extraction changes (e.g. SCM
+  // interpolation) after the queue has already drained. The default path only
+  // picks rows due by refreshDays and cannot be coaxed into a full pass by
+  // setting refreshDays=0 (that reprocesses the first batch forever).
+  const force = process.argv.includes('--force')
+
+  log.info({ force }, 'maven backfill starting (one-shot, full extraction)...')
 
   const config = getMavenConfig()
   log.info(config, 'Config loaded')
@@ -34,7 +44,9 @@ const main = async () => {
   await qx.selectOne('SELECT 1')
   log.info('Connected to packages-db.')
 
-  const totals = await runMavenCriticalBackfill(qx, config, () => shuttingDown)
+  const totals = force
+    ? await runMavenCriticalForceBackfill(qx, config, () => shuttingDown)
+    : await runMavenCriticalBackfill(qx, config, () => shuttingDown)
 
   log.info({ ...totals }, 'maven backfill complete')
   process.exit(0)

services/apps/packages_worker/src/maven/__tests__/normalize.test.ts

@@ -1,6 +1,6 @@
 import { describe, expect, it } from 'vitest'
 
-import { normalizeScmUrl } from '../extract'
+import { interpolateProperties, normalizeScmUrl } from '../extract'
 import { pickStableRelease } from '../metadata'
 import { isPrerelease, parseRepoUrl } from '../normalize'
 
@@ -195,6 +195,12 @@ describe('normalizeScmUrl', () => {
     expect(normalizeScmUrl('http://cvs.sourceforge.net/cgi-bin/viewcvs.cgi/foo')).toBeNull()
   })
 
+  it('returns null when an unresolved placeholder is embedded in the repo path', () => {
+    // Would otherwise parse to github.com/owner/%7BartifactId%7D and slip through.
+    expect(normalizeScmUrl('https://github.com/owner/${artifactId}')).toBeNull()
+    expect(normalizeScmUrl('scm:git:https://github.com/${owner}/repo.git')).toBeNull()
+  })
+
   // SCP colon form: "host:owner/repo" where the colon is a path separator, not a port
   it('recovers bare host:owner/repo SCP colon form', () => {
     expect(normalizeScmUrl('github.com:japgolly/scalacss.git')).toBe(
@@ -243,3 +249,56 @@ describe('normalizeScmUrl', () => {
     expect(normalizeScmUrl('https://android.googlesource.com/platform/tools/base')).toBeNull()
   })
 })
+
+describe('interpolateProperties', () => {
+  it('resolves a single ${...} placeholder from properties', () => {
+    expect(
+      interpolateProperties('${scm.github.url}', {
+        'scm.github.url': 'https://github.com/owner/repo',
+      }),
+    ).toBe('https://github.com/owner/repo')
+  })
+
+  it('resolves multiple placeholders in one string', () => {
+    expect(
+      interpolateProperties('https://gitlab.com/${projectPath}', {
+        projectPath: 'group/project',
+      }),
+    ).toBe('https://gitlab.com/group/project')
+  })
+
+  it('resolves nested placeholders recursively', () => {
+    expect(
+      interpolateProperties('${scm.url}', {
+        'scm.url': '${scm.base}/repo',
+        'scm.base': 'https://github.com/owner',
+      }),
+    ).toBe('https://github.com/owner/repo')
+  })
+
+  it('resolves built-in project.* style placeholders', () => {
+    expect(
+      interpolateProperties('https://github.com/acme/${project.artifactId}', {
+        'project.artifactId': 'my-lib',
+      }),
+    ).toBe('https://github.com/acme/my-lib')
+  })
+
+  it('leaves unresolved placeholders literal (missing property / method call)', () => {
+    expect(interpolateProperties('${scm.github.url}', {})).toBe('${scm.github.url}')
+    expect(
+      interpolateProperties('${pom.artifactId.substring(8)}', { 'pom.artifactId': 'foo' }),
+    ).toBe('${pom.artifactId.substring(8)}')
+  })
+
+  it('does not loop forever on self-referential placeholders', () => {
+    expect(interpolateProperties('${a}', { a: '${b}', b: '${a}' })).toContain('${')
+  })
+
+  it('composes with the normalizer end-to-end', () => {
+    const resolved = interpolateProperties('${scm.github.url}', {
+      'scm.github.url': 'scm:git:https://github.com/Owner/Repo.git',
+    })
+    expect(normalizeScmUrl(resolved)).toBe('https://github.com/owner/repo')
+  })
+})

services/apps/packages_worker/src/maven/extract.ts

@@ -47,6 +47,7 @@ interface PomData {
   developers?: { developer?: unknown }
   contributors?: { contributor?: unknown }
   parent?: { groupId?: unknown; artifactId?: unknown; version?: unknown }
+  properties?: unknown
 }
 
 interface PomPerson {
@@ -262,6 +263,9 @@ interface ResolvedFields {
   developers: PomMaintainer[]
   contributors: PomMaintainer[]
   hops: number
+  // Merged <properties> across the resolved parent chain (child overrides parent),
+  // used to interpolate ${...} placeholders in the SCM URL.
+  properties: Record<string, string>
 }
 
 // prettier-ignore
@@ -273,9 +277,12 @@ async function resolveWithInheritance(groupId: string, artifactId: string, versi
   const scmUrl = extractStr(pom.scm?.url ?? pom.scm?.connection)
   const developers = extractPersons(pom.developers?.developer, 'author')
   const contributors = extractPersons(pom.contributors?.contributor, 'maintainer')
+  const properties = extractProperties(pom)
 
   const missingLicense = licenses.length === 0
-  const missingScm = !scmUrl
+  // An unresolved ${...} placeholder counts as missing: the property that defines it
+  // may live in a parent POM, so we still need to walk the chain to collect it.
+  const missingScm = !scmUrl || scmUrl.includes('${')
   const missingDevelopers = developers.length === 0 || contributors.length === 0
   const parent = extractParent(pom)
 
@@ -306,6 +313,8 @@ async function resolveWithInheritance(groupId: string, artifactId: string, versi
         developers: developers.length > 0 ? developers : parentFields.developers,
         contributors: contributors.length > 0 ? contributors : parentFields.contributors,
         hops: parentFields.hops,
+        // Child properties override the parent's.
+        properties: { ...parentFields.properties, ...properties },
       }
     }
   }
@@ -319,6 +328,7 @@ async function resolveWithInheritance(groupId: string, artifactId: string, versi
     developers,
     contributors,
     hops: depth,
+    properties,
   }
 }
 
@@ -355,7 +365,12 @@ export async function extractArtifactDirect(groupId: string, artifactId: string,
   }
 
   const licenses = extractLicenses(pom)
-  const scmUrl = extractStr(pom.scm?.url ?? pom.scm?.connection)
+  const rawScmUrl = extractStr(pom.scm?.url ?? pom.scm?.connection)
+  const props = {
+    ...extractProperties(pom),
+    ...builtinProjectProperties(groupId, artifactId, version),
+  }
+  const scmUrl = rawScmUrl ? interpolateProperties(rawScmUrl, props) : null
   const developers = extractPersons(pom.developers?.developer, 'author')
   const contributors = extractPersons(pom.contributors?.contributor, 'maintainer')
 
@@ -414,6 +429,14 @@ export async function extractArtifact(groupId: string, artifactId: string, versi
       new Set(),
       baseUrl,
     )
+    // Resolve ${...} placeholders in the SCM URL using the merged chain properties
+    // plus the leaf's built-in project.* values. Best-effort: unresolved placeholders
+    // stay literal and are rejected downstream by normalizeScmUrl.
+    const props = {
+      ...resolved.properties,
+      ...builtinProjectProperties(groupId, artifactId, version),
+    }
+    const scmUrl = resolved.scmUrl ? interpolateProperties(resolved.scmUrl, props) : null
     return {
       groupId,
       artifactId,
@@ -422,7 +445,7 @@ export async function extractArtifact(groupId: string, artifactId: string, versi
       description: resolved.description,
       licenses: resolved.licenses,
       licensesRaw: resolved.licensesRaw,
-      scmUrl: resolved.scmUrl,
+      scmUrl,
       homepageUrl: resolved.homepageUrl,
       developers: resolved.developers,
       contributors: resolved.contributors,
@@ -524,6 +547,12 @@ export function normalizeScmUrl(raw: string | null): string | null {
   let s = raw.trim()
   if (!s) return null
 
+  // A leftover ${...} means best-effort interpolation upstream could not resolve it.
+  // Reject outright: a placeholder embedded in the path (e.g. github.com/owner/${x})
+  // otherwise survives URL parsing (percent-encoded to %7B…%7D) and would yield a junk
+  // repository_url instead of null.
+  if (s.includes('${')) return null
+
   // Strip Maven scm:git: / scm: prefix
   s = s.replace(/^scm:git:/i, '').replace(/^scm:/i, '')
 
@@ -612,6 +641,59 @@ function extractPersons(raw: unknown, role: 'author' | 'maintainer'): PomMaintai
     }))
 }
 
+/** Flattens a POM's <properties> block into a string→string map (non-string values skipped). */
+function extractProperties(pom: PomData): Record<string, string> {
+  const raw = pom.properties
+  if (!raw || typeof raw !== 'object') return {}
+  const out: Record<string, string> = {}
+  for (const [key, value] of Object.entries(raw as Record<string, unknown>)) {
+    if (typeof value === 'string') out[key] = value
+    else if (typeof value === 'number') out[key] = String(value)
+  }
+  return out
+}
+
+/** Maven built-in project.* / pom.* properties for the leaf coordinates. */
+function builtinProjectProperties(
+  groupId: string,
+  artifactId: string,
+  version: string,
+): Record<string, string> {
+  return {
+    'project.groupId': groupId,
+    'project.artifactId': artifactId,
+    'project.version': version,
+    'pom.groupId': groupId,
+    'pom.artifactId': artifactId,
+    'pom.version': version,
+    groupId,
+    artifactId,
+    version,
+  }
+}
+
+const MAX_INTERPOLATION_DEPTH = 10
+
+/**
+ * Best-effort Maven property interpolation of ${...} placeholders. Resolves
+ * recursively (a property value may itself reference another) up to a depth cap
+ * to guard against cycles. Placeholders with no matching property (e.g. defined
+ * in a profile/settings, or method calls like ${x.substring(8)}) are left as-is
+ * so the SCM normaliser rejects them.
+ */
+export function interpolateProperties(value: string, props: Record<string, string>): string {
+  let current = value
+  for (let i = 0; i < MAX_INTERPOLATION_DEPTH && current.includes('${'); i++) {
+    const next = current.replace(/\$\{([^{}]+)\}/g, (match, key) => {
+      const resolved = props[(key as string).trim()]
+      return resolved !== undefined ? resolved : match
+    })
+    if (next === current) break
+    current = next
+  }
+  return current
+}
+
 function extractParent(
   pom: PomData,
 ): { groupId: string; artifactId: string; version: string } | null {
@@ -634,5 +716,6 @@ function emptyFields(hops: number): ResolvedFields {
     developers: [],
     contributors: [],
     hops,
+    properties: {},
   }
 }

services/apps/packages_worker/src/maven/runMavenEnrichmentLoop.ts

@@ -1,6 +1,7 @@
 import {
   MavenPackageToSync,
   QueryExecutor,
+  listMavenCriticalPackagesById,
   listMavenPackagesToSync,
   logAuditFieldChange,
   replacePackageMaintainers,
@@ -497,3 +498,63 @@ async function runPhase(qx: QueryExecutor, config: MavenConfig, isCritical: bool
 export async function runMavenCriticalBackfill(qx: QueryExecutor, config: MavenConfig, isShuttingDown: () => boolean): Promise<BatchResult> {
   return runPhase(qx, config, true, isShuttingDown)
 }
+
+/**
+ * Force full-refresh backfill: re-runs POM extraction over EVERY critical Maven
+ * row, ignoring the staleness window. Unlike runMavenCriticalBackfill (which
+ * drains listMavenPackagesToSync by predicate and cannot terminate with
+ * refreshDays=0 — the freshly-synced top rows re-qualify every batch), this pages
+ * strictly by `id > afterId`, so each row is processed exactly once and the scan
+ * always terminates. Every page runs with forceFullExtraction=true.
+ *
+ * Trade-off vs. the normal path: rows are visited in id order, not
+ * dependent_count order, so if interrupted the most-depended-on packages are not
+ * guaranteed to be done first. Restart re-scans from id 0 (idempotent upserts).
+ */
+// prettier-ignore
+export async function runMavenCriticalForceBackfill(qx: QueryExecutor, config: MavenConfig, isShuttingDown: () => boolean): Promise<BatchResult> {
+  const total: BatchResult = { processed: 0, skipped: 0, error: 0, unchanged: 0 }
+  const startedAt = Date.now()
+  // Cursor kept as a string: id is a Postgres bigint, and Number() coercion would
+  // silently lose precision above 2^53, corrupting the cursor and skipping rows.
+  let afterId = '0'
+  let batchNum = 0
+
+  log.info('Force full-refresh started (all critical rows, keyset by id, ignoring refreshDays)')
+
+  while (!isShuttingDown()) {
+    const page = await listMavenCriticalPackagesById(qx, { afterId, limit: config.batchSize })
+    if (page.length === 0) break
+
+    // Capture the cursor before processPackages reorders the page in place (it
+    // clusters by namespace for the parent-POM cache). Rows come back id-ordered,
+    // so the max id is the last element — kept as a string to preserve bigint precision.
+    afterId = page[page.length - 1].id
+
+    const result = await processPackages(qx, config, page, true, true)
+    batchNum++
+    total.processed += result.processed
+    total.skipped += result.skipped
+    total.error += result.error
+    total.unchanged += result.unchanged
+
+    log.info(
+      {
+        batch: batchNum,
+        afterId,
+        totalProcessed: total.processed,
+        totalSkipped: total.skipped,
+        totalUnchanged: total.unchanged,
+        totalErrors: total.error,
+        elapsedSec: Math.round((Date.now() - startedAt) / 1000),
+      },
+      'Force batch done',
+    )
+  }
+
+  log.info(
+    { ...total, durationSec: Math.round((Date.now() - startedAt) / 1000) },
+    'Force full-refresh complete',
+  )
+  return total
+}

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

@@ -118,6 +118,41 @@ export async function listMavenPackagesToSync(
   )
 }
 
+/**
+ * Keyset-paginated scan of every critical Maven package, independent of the
+ * staleness window. Unlike `listMavenPackagesToSync` — which drains by predicate
+ * (a row leaves the set once it is fresh) and therefore cannot terminate with
+ * refreshDays=0 — this pages strictly by `id > afterId`, so each row is visited
+ * exactly once and the scan always terminates. Used by the `--force` full-refresh
+ * backfill to re-run POM extraction over the entire critical set regardless of
+ * last_synced_at / ingestion_source.
+ */
+export async function listMavenCriticalPackagesById(
+  qx: QueryExecutor,
+  options: { afterId: string; limit: number },
+): Promise<MavenPackageToSync[]> {
+  return qx.select(
+    `
+    SELECT
+      p.id,
+      p.purl,
+      p.namespace,
+      p.name,
+      p.dependent_count          AS "dependentPackagesCount",
+      p.dependent_repos_count    AS "dependentReposCount",
+      p.latest_version           AS "latestVersion"
+    FROM packages p
+    WHERE p.ecosystem = 'maven'
+      AND p.is_critical
+      AND p.namespace IS NOT NULL
+      AND p.id > $(afterId)::bigint
+    ORDER BY p.id ASC
+    LIMIT $(limit)
+    `,
+    { afterId: options.afterId, limit: options.limit },
+  )
+}
+
 // ─── repository_url backfill ──────────────────────────────────────────────────
 
 export type MavenRepoUrlRow = {