-
Notifications
You must be signed in to change notification settings - Fork 731
fix: re-use canonicalizeRepoUrl & insert package_repos in go and nuget #4331
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
cdd8bf0
9ac2223
4d69373
36f39e4
59d7e15
5f7d880
db5be1a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ import { | |
| } from './types' | ||
|
|
||
| const SERVICE_INDEX_URL = 'https://api.nuget.org/v3/index.json' | ||
| const FLAT_CONTAINER_BASE_URL = 'https://api.nuget.org/v3-flatcontainer' | ||
|
|
||
| interface ServiceIndexResource { | ||
| '@id': string | ||
|
|
@@ -144,3 +145,29 @@ export async function fetchRegistration( | |
| throw err | ||
| } | ||
| } | ||
|
|
||
| // The registration API never exposes the nuspec <repository> element — it must be read from the | ||
| // raw nuspec XML, served by the flat-container endpoint. | ||
|
Comment on lines
+149
to
+150
|
||
| export async function fetchNuspec( | ||
| packageId: string, | ||
| version: string, | ||
| ): Promise<string | NuGetFetchError> { | ||
| const lowerId = packageId.toLowerCase() | ||
| const lowerVersion = version.toLowerCase() | ||
|
|
||
| try { | ||
| const resp = await axios.get<string>( | ||
| `${FLAT_CONTAINER_BASE_URL}/${lowerId}/${lowerVersion}/${lowerId}.nuspec`, | ||
| { | ||
| responseType: 'text', | ||
| transformResponse: (data) => data, | ||
| timeout: 15000, | ||
| }, | ||
|
Comment on lines
+159
to
+165
|
||
| ) | ||
| return resp.data | ||
| } catch (err) { | ||
| const classified = classifyError(err) | ||
| if (classified) return classified | ||
| throw err | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nuspec fetch aborts enrichmentMedium Severity
Additional Locations (1)Reviewed by Cursor Bugbot for commit db5be1a. Configure here. |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,7 @@ | ||
| import { XMLParser } from 'fast-xml-parser' | ||
|
|
||
| import { canonicalizeRepoUrl } from '../utils/canonicalizeRepoUrl' | ||
|
|
||
| import { | ||
| NormalizedNuGetPackage, | ||
| NormalizedNuGetVersion, | ||
|
|
@@ -26,18 +30,20 @@ function parsePublishedDate(published: string | undefined): Date | null { | |
| return !isNaN(date.getTime()) && date.getUTCFullYear() > 1900 ? date : null | ||
| } | ||
|
|
||
| const SCM_HOSTS = ['github.com', 'gitlab.com', 'bitbucket.org'] | ||
| const nuspecParser = new XMLParser({ ignoreAttributes: false, attributeNamePrefix: '@_' }) | ||
|
|
||
| function normalizeRepoUrl(url: string | undefined): string | null { | ||
| if (!url) return null | ||
| return url | ||
| .trim() | ||
| .replace(/\.git$/, '') | ||
| .replace(/^git\+/, '') | ||
| .replace(/^git:\/\//, 'https://') | ||
| .replace(/^http:\/\/github\.com\//, 'https://github.com/') | ||
| export function parseNuspecRepositoryUrl(nuspecXml: string): string | null { | ||
| try { | ||
| const doc = nuspecParser.parse(nuspecXml) | ||
| const url = doc?.package?.metadata?.repository?.['@_url'] | ||
| return typeof url === 'string' && url.trim() !== '' ? url.trim() : null | ||
| } catch { | ||
| return null | ||
| } | ||
|
Comment on lines
+33
to
+42
|
||
| } | ||
|
|
||
| const SCM_HOSTS = ['github.com', 'gitlab.com', 'bitbucket.org'] | ||
|
|
||
| function isScmUrl(url: string | undefined): boolean { | ||
| if (!url) return false | ||
| try { | ||
|
|
@@ -64,6 +70,7 @@ export function normalizeNuGetPackage( | |
| packageId: string, | ||
| searchResult: NuGetSearchItem | null, | ||
| registration: NuGetRegistrationIndex, | ||
| nuspecXml?: string | null, | ||
| ): NormalizedNuGetPackage { | ||
| const allLeaves = registration.items.flatMap((page) => page.items ?? []) | ||
| const allEntries: NuGetCatalogEntry[] = allLeaves.map((leaf) => leaf.catalogEntry) | ||
|
|
@@ -96,10 +103,13 @@ export function normalizeNuGetPackage( | |
| ...(latestEntry ? [latestEntry] : []), | ||
| ] | ||
| : [...allEntries].reverse() | ||
| const nuspecRepoUrl = entriesForRepo.find((e) => e.repository?.url)?.repository?.url | ||
| const catalogRepoUrl = entriesForRepo.find((e) => e.repository?.url)?.repository?.url | ||
| const fetchedNuspecRepoUrl = nuspecXml ? parseNuspecRepositoryUrl(nuspecXml) : null | ||
| const nuspecRepoUrl = fetchedNuspecRepoUrl ?? catalogRepoUrl | ||
| const declaredRepositoryUrl = nuspecRepoUrl ?? null | ||
| const repoCandidate = nuspecRepoUrl ?? (isScmUrl(homepage) ? homepage : null) | ||
| const repositoryUrl = normalizeRepoUrl(repoCandidate ?? undefined) | ||
| const repo = | ||
| (nuspecRepoUrl ? canonicalizeRepoUrl(nuspecRepoUrl) : null) ?? | ||
| (isScmUrl(homepage) ? canonicalizeRepoUrl(homepage) : null) | ||
|
mbani01 marked this conversation as resolved.
Comment on lines
+110
to
+112
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mbani01 do you think it maybe worth toreview this ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 4d69373 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Homepage used when declared repo failsMedium Severity
Reviewed by Cursor Bugbot for commit 36f39e4. Configure here.
Comment on lines
+108
to
+112
|
||
|
|
||
| const keywords = searchResult?.tags && searchResult.tags.length > 0 ? searchResult.tags : null | ||
|
|
||
|
|
@@ -154,7 +164,7 @@ export function normalizeNuGetPackage( | |
| description, | ||
| homepage: homepage || null, | ||
| declaredRepositoryUrl, | ||
| repositoryUrl, | ||
| repo, | ||
| licenses, | ||
| licensesRaw, | ||
| keywords, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ import { | |
| IDbNuGetVersionUpsert, | ||
| NuGetPackageToSync, | ||
| QueryExecutor, | ||
| getOrCreateRepoByUrl, | ||
| listNuGetPackagesToSync, | ||
| logAuditFieldChange, | ||
| markNuGetPackageError, | ||
|
|
@@ -10,12 +11,13 @@ import { | |
| upsertMaintainer, | ||
| upsertNuGetPackage, | ||
| upsertNuGetVersionsBatch, | ||
| upsertPackageRepo, | ||
| } from '@crowd/data-access-layer' | ||
| import { getServiceChildLogger } from '@crowd/logging' | ||
|
|
||
| import { getNuGetConfig } from '../config' | ||
|
|
||
| import { fetchRegistration, fetchSearch } from './client' | ||
| import { fetchNuspec, fetchRegistration, fetchSearch } from './client' | ||
| import { normalizeNuGetPackage } from './normalize' | ||
| import { BatchResult, isNuGetFetchError } from './types' | ||
|
|
||
|
|
@@ -96,7 +98,15 @@ async function processPackage( | |
|
|
||
| const searchItem = isNuGetFetchError(searchResult) ? null : searchResult | ||
|
|
||
| const normalized = normalizeNuGetPackage(packageId, searchItem, registrationResult) | ||
| const preliminary = normalizeNuGetPackage(packageId, searchItem, registrationResult) | ||
|
|
||
| let nuspecXml: string | null = null | ||
| if (preliminary.latestVersion) { | ||
| const nuspecResult = await fetchNuspec(packageId, preliminary.latestVersion) | ||
|
Comment on lines
+101
to
+105
|
||
| nuspecXml = isNuGetFetchError(nuspecResult) ? null : nuspecResult | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nuspec fetch failure blocks package syncMedium Severity After registration and search succeed, Additional Locations (1)Reviewed by Cursor Bugbot for commit 9ac2223. Configure here. |
||
| } | ||
|
|
||
| const normalized = normalizeNuGetPackage(packageId, searchItem, registrationResult, nuspecXml) | ||
|
|
||
| await withDeadlockRetry(() => | ||
| qx.tx(async (t) => { | ||
|
|
@@ -108,7 +118,7 @@ async function processPackage( | |
| description: normalized.description, | ||
| homepage: normalized.homepage, | ||
| declaredRepositoryUrl: normalized.declaredRepositoryUrl, | ||
| repositoryUrl: normalized.repositoryUrl, | ||
| repositoryUrl: normalized.repo?.url ?? null, | ||
| licenses: normalized.licenses, | ||
| licensesRaw: normalized.licensesRaw, | ||
| keywords: normalized.keywords, | ||
|
|
@@ -122,6 +132,24 @@ async function processPackage( | |
| }) | ||
| pkgChanged.forEach((f) => changed.add(f)) | ||
|
|
||
| if (normalized.repo) { | ||
|
|
||
| const { id: repoId, changedFields: repoChanged } = await getOrCreateRepoByUrl( | ||
| t, | ||
| normalized.repo.url, | ||
| normalized.repo.host, | ||
| ) | ||
| repoChanged.forEach((f) => changed.add(f)) | ||
|
|
||
| const linkChanged = await upsertPackageRepo( | ||
|
mbani01 marked this conversation as resolved.
|
||
| t, | ||
| packageDbId.toString(), | ||
| repoId, | ||
| 'declared', | ||
|
Comment on lines
+143
to
+147
|
||
| 0.8, | ||
|
mbani01 marked this conversation as resolved.
|
||
| ) | ||
|
mbani01 marked this conversation as resolved.
Comment on lines
+143
to
+149
|
||
| linkChanged.forEach((f) => changed.add(f)) | ||
| } | ||
|
|
||
| if (normalized.versions.length > 0) { | ||
| const versionRows: IDbNuGetVersionUpsert[] = normalized.versions.map((v) => ({ | ||
| packageId: packageDbId, | ||
|
|
||


Uh oh!
There was an error while loading. Please reload this page.