fix: cache logic

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

Author: ulemons

services/apps/packages_worker/src/maven/README.md

@@ -50,13 +50,15 @@ critical packages, where data quality matters.
 
 Parent POMs are shared across many artifacts of the same namespace (`org.apache:apache`,
 `org.springframework.boot:spring-boot-starter-parent`, `com.google.cloud:google-cloud-shared-config`, …).
-Because the queue is ordered by `rank_in_ecosystem`, those siblings are processed close
-together. A module-level, coordinate-keyed in-process cache in `extract.ts` collapses the
-repeated parent fetches into a **single** HTTP request, and also removes the redundant second
-fetch of each artifact's own POM (`extractArtifact` fetches the leaf, then
-`resolveWithInheritance` would fetch it again at depth 0). This is the **single biggest lever
-against Maven Central rate limiting** — and it works _because_ of the namespace clustering, so
-shuffling the batch (which the queue's `rank` ordering produces) would be counter-productive.
+The batch is **selected** by criticality (`rank_in_ecosystem` / `criticality_score` via the SQL
+`LIMIT`), but that ordering does **not** group same-namespace siblings — so before processing,
+`processPackages` re-sorts each critical batch by `(namespace, name)`. That sort is what puts the
+siblings adjacent, so a parent fetched for one is still cached when the next arrives. A
+module-level, coordinate-keyed in-process cache in `extract.ts` then collapses the repeated parent
+fetches into a **single** HTTP request, and also removes the redundant second fetch of each
+artifact's own POM (`extractArtifact` fetches the leaf, then `resolveWithInheritance` would fetch
+it again at depth 0). This is the **single biggest lever against Maven Central rate limiting** — and
+it works _because_ of the namespace sort, so re-shuffling the batch would be counter-productive.
 
 - **Only successful fetches are cached.** `fetchPom` returns `null` for both a real 404 and a
   transient failure (throttle/timeout), so caching `null` would poison the cache — it is never

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

@@ -283,6 +283,7 @@ async function processCriticalPackage(
         allVersions.map((v) => ({
           packageId,
           ecosystem: 'maven',
+          namespace: groupId,
           name: artifactId,
           number: v,
           isLatest: v === metadata.releaseVersion,
@@ -378,6 +379,18 @@ async function processPackages(
   if (packages.length === 0)
     return { processed: 0, skipped: 0, error: 0, unchanged: 0, hopLimitReached: 0 }
 
+  // Cluster the batch by namespace so artifacts sharing a parent POM are processed
+  // adjacently — this is what makes the parent-POM cache effective. The criticality
+  // ordering only decides *which* packages are in the batch (via the SQL LIMIT);
+  // it does not group same-namespace siblings, so we reorder here. Only matters on
+  // the critical path (the non-critical path issues no POM/parent HTTP).
+  if (isCritical) {
+    packages.sort(
+      (a, b) =>
+        (a.namespace ?? '').localeCompare(b.namespace ?? '') || a.name.localeCompare(b.name),
+    )
+  }
+
   log.info({ count: packages.length, isCritical }, 'Batch started')
 
   const counts = { processed: 0, skipped: 0, error: 0, unchanged: 0, hopLimitReached: 0 }

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

@@ -59,6 +59,7 @@ export type IDbPackageMaintainerUpsert = {
 export type IDbVersionUpsert = {
   packageId: number
   ecosystem: string
+  namespace: string | null
   name: string
   number: string
   isLatest: boolean

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

@@ -35,21 +35,23 @@ export async function upsertVersionsBatch(
        WHERE package_id = $(packageId)::bigint AND number = ANY($(numbers)::text[])
     ),
     ins AS (
-      INSERT INTO versions (package_id, ecosystem, name, number, is_latest, is_prerelease, licenses, last_synced_at)
+      INSERT INTO versions (package_id, ecosystem, namespace, name, number, is_latest, is_prerelease, licenses, last_synced_at)
       SELECT
-        t.package_id, t.ecosystem, t.name, t.number, t.is_latest, t.is_prerelease,
+        t.package_id, t.ecosystem, t.namespace, t.name, t.number, t.is_latest, t.is_prerelease,
         CASE WHEN t.license IS NULL THEN NULL ELSE ARRAY[t.license] END,
         NOW()
       FROM UNNEST(
         $(packageIds)::bigint[],
         $(ecosystems)::text[],
+        $(namespaces)::text[],
         $(names)::text[],
         $(numbers)::text[],
         $(isLatests)::bool[],
         $(isPreleases)::bool[],
         $(licenses)::text[]
-      ) AS t(package_id, ecosystem, name, number, is_latest, is_prerelease, license)
+      ) AS t(package_id, ecosystem, namespace, name, number, is_latest, is_prerelease, license)
       ON CONFLICT (package_id, number) DO UPDATE SET
+        namespace      = COALESCE(EXCLUDED.namespace, versions.namespace),
         is_latest      = EXCLUDED.is_latest,
         is_prerelease  = EXCLUDED.is_prerelease,
         licenses       = COALESCE(EXCLUDED.licenses, versions.licenses),
@@ -68,6 +70,7 @@ export async function upsertVersionsBatch(
       packageId: versions[0].packageId,
       packageIds: versions.map((v) => v.packageId),
       ecosystems: versions.map((v) => v.ecosystem),
+      namespaces: versions.map((v) => v.namespace),
       names: versions.map((v) => v.name),
       numbers: versions.map((v) => v.number),
       isLatests: versions.map((v) => v.isLatest),