From d6c3f1d5157bdfcb6ffdc8c5e32d320c81f88d44 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 18 Oct 2024 15:07:08 +0200 Subject: [PATCH] wip test against 20k URLs --- api-node/.gitignore | 2 + api-node/package.json | 7 +- api-node/src/common/registry.service.ts | 37 ++-- api-node/src/common/utils.ts | 3 + api-node/src/instrument.ts | 1 + api-node/src/main.ts | 4 +- api-node/src/packages/packages.controller.ts | 27 ++- api-node/test/everything.e2e-spec.ts | 57 ++++++ api-node/test/marketing.e2e-spec.ts | 2 +- api-node/test/packages.e2e-spec.ts | 85 +++++--- api-node/test/utils.ts | 3 + api-node/test/utils/getUrls.ts | 198 +++++++++++++++++++ api-server/apiserver.py | 2 + 13 files changed, 372 insertions(+), 56 deletions(-) create mode 100644 api-node/src/common/utils.ts create mode 100644 api-node/test/everything.e2e-spec.ts create mode 100644 api-node/test/utils/getUrls.ts diff --git a/api-node/.gitignore b/api-node/.gitignore index 4b56acfbe..ecddcbb6d 100644 --- a/api-node/.gitignore +++ b/api-node/.gitignore @@ -54,3 +54,5 @@ pids # Diagnostic reports (https://nodejs.org/api/report.html) report.[0-9]*.[0-9]*.[0-9]*.[0-9]*.json + +.urls.json diff --git a/api-node/package.json b/api-node/package.json index 950f8f3e2..9e6e34bdb 100644 --- a/api-node/package.json +++ b/api-node/package.json @@ -18,7 +18,9 @@ "test:watch": "jest --watch", "test:cov": "jest --coverage", "test:debug": "node --inspect-brk -r tsconfig-paths/register -r ts-node/register node_modules/.bin/jest --runInBand", - "test:e2e": "jest --config ./test/jest-e2e.json", + "test:e2e": "jest --config ./test/jest-e2e.json --testPathIgnorePatterns=everything.e2e-spec.ts", + "test:e2e:urls": "NODE_ENV=test jest --config ./test/jest-e2e.json everything.e2e-spec.ts", + "test:e2e:urls:generate": "ts-node ./test/utils/getUrls.ts", "python-api:build": "docker build -t registry-api-server ../", "python-api:start": "docker run -d -p 5031:5030 registry-api-server", "python-api:stop": "docker stop $(docker ps -q --filter ancestor=registry-api-server)", @@ -33,7 +35,8 @@ "@sentry/nestjs": "^8.34.0", "cache-manager": "^5.7.6", "reflect-metadata": "^0.2.0", - "rxjs": "^7.8.1" + "rxjs": "^7.8.1", + "semver": "^7.6.3" }, "devDependencies": { "@nestjs/cli": "^10.0.0", diff --git a/api-node/src/common/registry.service.ts b/api-node/src/common/registry.service.ts index a72b6eef2..88309b2ac 100644 --- a/api-node/src/common/registry.service.ts +++ b/api-node/src/common/registry.service.ts @@ -8,12 +8,9 @@ import type { MarketingSlugs, } from '../marketing/types'; import type { AppEntry, Apps } from '../apps/types'; -import type { - PackageEntry, - Packages, - PackageVersions, -} from '../packages/types'; +import type { PackageEntry, Packages } from '../packages/types'; import type { AwsLambdaLayers } from '../aws-lambda-layers/types'; +import * as semver from 'semver'; const SDKS_PATH = path.join('..', 'sdks'); const APPS_PATH = path.join('..', 'apps'); @@ -79,13 +76,13 @@ export class RegistryService { getSdkVersions(sdkId: string): SdkVersions { const latest = this.getSdk(sdkId); - const { versions } = this.getPackageVersions(latest.canonical); + const versions = this.getPackageVersions(latest.canonical); return { latest, versions }; } // Packages - getPackages(): Packages { + getPackages(strict: boolean = false): Packages { return this.#packages.reduce((acc, canonical) => { const packageDir = getPackageDirFromCanonical(canonical); const latestFilePath = path.join(packageDir, 'latest.json'); @@ -105,7 +102,7 @@ export class RegistryService { }, {}); } - getPackageVersions(packageName: string): PackageVersions { + getPackageVersions(packageName: string): string[] | null { const packageDir = getPackageDirFromCanonical(packageName); try { const versions = fs @@ -118,27 +115,31 @@ export class RegistryService { return versionFile.version; }); - const dedupedVersions = Array.from(new Set(versions)); - - const latest = JSON.parse( - fs.readFileSync(path.join(packageDir, 'latest.json')).toString(), - ); - - return { versions: dedupedVersions, latest }; + // dedupe and sort versions + return Array.from(new Set(versions)).sort((a, b) => { + return semver.parse(a).compare(semver.parse(b)); + }); } catch (e) { console.error(`Failed to read package versions: ${packageName}`); console.error(e); + return []; } } - getPackage(packageName: string, version: string = 'latest'): PackageEntry { + getPackage( + packageName: string, + version: string = 'latest', + ): PackageEntry | null { try { const packageDir = getPackageDirFromCanonical(packageName); const versionFilePath = path.join(packageDir, `${version}.json`); return JSON.parse(fs.readFileSync(versionFilePath).toString()); } catch (e) { - console.error(`Failed to read package by version: ${packageName}`); + console.error( + `Failed to read package ${packageName} for version ${version}`, + ); console.error(e); + return null; } } @@ -177,7 +178,7 @@ export class RegistryService { // Marketing getMarketingSlugs(): MarketingSlugs { - return { slugs: Object.keys(this.#slugs) }; + return { slugs: Object.keys(this.#slugs).sort() }; } resolveMarketingSlug(slug: string): ResolvedMarketingSlug | null { diff --git a/api-node/src/common/utils.ts b/api-node/src/common/utils.ts new file mode 100644 index 000000000..41e5e1f31 --- /dev/null +++ b/api-node/src/common/utils.ts @@ -0,0 +1,3 @@ +export function isTruthy(value: string): boolean { + return value.toLowerCase() === 'true' || value === '1' || value === 'yes'; +} diff --git a/api-node/src/instrument.ts b/api-node/src/instrument.ts index ab70a003e..3b1acf203 100644 --- a/api-node/src/instrument.ts +++ b/api-node/src/instrument.ts @@ -8,4 +8,5 @@ Sentry.init({ release: packageJson.version, environment: process.env.NODE_ENV || 'development', tracesSampleRate: 1.0, + enabled: process.env.NODE_ENV !== 'test', }); diff --git a/api-node/src/main.ts b/api-node/src/main.ts index 508b7be54..63ab90f21 100644 --- a/api-node/src/main.ts +++ b/api-node/src/main.ts @@ -4,7 +4,9 @@ import { NestFactory } from '@nestjs/core'; import { AppModule } from './app.module'; async function bootstrap(): Promise { - const app = await NestFactory.create(AppModule); + const app = await NestFactory.create(AppModule, { + logger: ['debug', 'error', 'fatal', 'verbose', 'log', 'warn'], + }); await app.listen(3000); } diff --git a/api-node/src/packages/packages.controller.ts b/api-node/src/packages/packages.controller.ts index 176e75dd1..94a777e57 100644 --- a/api-node/src/packages/packages.controller.ts +++ b/api-node/src/packages/packages.controller.ts @@ -1,4 +1,10 @@ -import { Controller, Get, Param } from '@nestjs/common'; +import { + Controller, + Get, + NotFoundException, + Param, + Query, +} from '@nestjs/common'; import { RegistryService } from '../common/registry.service'; import { PackageEntry, Packages, PackageVersions } from './types'; @@ -7,20 +13,29 @@ export class PackagesController { constructor(private registryService: RegistryService) {} @Get() - getPackages(): Packages { - return this.registryService.getPackages(); + getPackages(@Query('strict') strict: boolean = false): Packages { + return this.registryService.getPackages(strict); } @Get('/:package(*)/versions') getPackageVersions(@Param('package') pgkName: string): PackageVersions { - return this.registryService.getPackageVersions(pgkName); + const latest = this.registryService.getPackage(pgkName); + if (!latest) { + throw new NotFoundException(); + } + const versions = this.registryService.getPackageVersions(pgkName); + return { versions, latest }; } @Get('/:package(*)/:version') getPackageByVersion( @Param('package') pkgName: string, @Param('version') version: string, - ): PackageEntry { - return this.registryService.getPackage(pkgName, version); + ): PackageEntry | null { + const pkg = this.registryService.getPackage(pkgName, version); + if (!pkg) { + throw new NotFoundException(); + } + return pkg; } } diff --git a/api-node/test/everything.e2e-spec.ts b/api-node/test/everything.e2e-spec.ts new file mode 100644 index 000000000..4ef124585 --- /dev/null +++ b/api-node/test/everything.e2e-spec.ts @@ -0,0 +1,57 @@ +import { PYTHON_API_URL } from './utils'; +import * as fs from 'fs'; +describe('python and Node api responses match', () => { + const urls = JSON.parse( + fs.readFileSync(`${__dirname}/utils/.urls.json`, 'utf8'), + ); + + it.each(urls)('%s', async ({ url, status }) => { + let pythonResponse, nodeResponse, pythonResponseStatus, nodeResponseStatus; + let pythonResponseBody, nodeResponseBody; + let attempts = 0; + while (attempts < 3) { + try { + pythonResponse = await fetch(`${PYTHON_API_URL}${url}`, { + redirect: 'manual', + }); + nodeResponse = await fetch(`http://localhost:3000${url}`, { + redirect: 'manual', + }); + if (pythonResponse?.status < 400 && nodeResponse?.status < 400) { + pythonResponseStatus = pythonResponse?.status; + nodeResponseStatus = nodeResponse?.status; + pythonResponseBody = await pythonResponse?.text(); + nodeResponseBody = await nodeResponse?.text(); + break; + } + } catch (error) { + console.error(`Attempt ${attempts + 1} failed:`, error); + } + attempts++; + if (attempts < 3) { + pythonResponseStatus = pythonResponse?.status; + nodeResponseStatus = nodeResponse?.status; + pythonResponseBody = await pythonResponse?.text(); + nodeResponseBody = await nodeResponse?.text(); + await new Promise((resolve) => setTimeout(resolve, 1000)); // Wait 1 second before retrying + } + } + + if (attempts === 3) { + console.error(`Failed to fetch ${url} after 3 attempts`); + } + + expect(pythonResponseStatus).toBe(status); + expect(nodeResponseStatus).toBe(pythonResponseStatus); + + if (pythonResponseStatus === 200) { + try { + const pythonResponseJson = JSON.parse(pythonResponseBody); + const nodeResponseJson = JSON.parse(nodeResponseBody); + expect(nodeResponseJson).toEqual(pythonResponseJson); + } catch { + expect(nodeResponseBody).toEqual(pythonResponseBody); + } + } + }); +}); diff --git a/api-node/test/marketing.e2e-spec.ts b/api-node/test/marketing.e2e-spec.ts index 4ab90783e..f78a7ed59 100644 --- a/api-node/test/marketing.e2e-spec.ts +++ b/api-node/test/marketing.e2e-spec.ts @@ -24,7 +24,7 @@ describe('MarketingController (e2e)', () => { .get('/marketing-slugs') .expect((res) => { expect(res.status).toBe(200); - expect(res.body.slugs.sort()).toEqual(pythonApiData.slugs.sort()); + expect(res.body.slugs).toEqual(pythonApiData.slugs); }); }); diff --git a/api-node/test/packages.e2e-spec.ts b/api-node/test/packages.e2e-spec.ts index a37e35ff9..a99970923 100644 --- a/api-node/test/packages.e2e-spec.ts +++ b/api-node/test/packages.e2e-spec.ts @@ -29,39 +29,68 @@ describe('PackagesController (e2e)', () => { }); }); - it('/packages/:packageName/versions (GET)', async () => { - const packageName = 'npm:@sentry/angular'; + describe('/packages/:packageName/versions (GET)', () => { + it('returns versions for existing package', async () => { + const packageName = 'npm:@sentry/angular'; - const pythonApiResponse = await fetch( - `${PYTHON_API_URL}/packages/${packageName}/versions`, - ); - const pythonApiData = await pythonApiResponse.json(); + const pythonApiResponse = await fetch( + `${PYTHON_API_URL}/packages/${packageName}/versions`, + ); + const pythonApiData = await pythonApiResponse.json(); - return request(app.getHttpServer()) - .get(`/packages/${packageName}/versions`) - .expect((r) => { - expect(r.status).toEqual(200); - const { versions, latest } = r.body; - expect(versions.length).toEqual(pythonApiData.versions.length); - expect(versions.sort()).toEqual(pythonApiData.versions.sort()); - expect(latest).toEqual(pythonApiData.latest); - }); + return request(app.getHttpServer()) + .get(`/packages/${packageName}/versions`) + .expect((r) => { + expect(r.status).toEqual(200); + expect(r.body).toEqual(pythonApiData); + }); + }); + + it('returns 404 for non-existing package', async () => { + const nonExistingPackage = 'npm:@sentry/non-existing-package'; + + const pythonApiResponse = await fetch( + `${PYTHON_API_URL}/packages/${nonExistingPackage}/versions`, + ); + expect(pythonApiResponse.status).toEqual(404); + + return request(app.getHttpServer()) + .get(`/packages/${nonExistingPackage}/versions`) + .expect(404); + }); }); - it('/packages/:packageName/:version (GET)', async () => { - const packageName = 'npm:@sentry/angular'; - const version = '8.0.0'; + describe('/packages/:packageName/:version (GET)', () => { + it('returns package info for existing package', async () => { + const packageName = 'npm:@sentry/angular'; + const version = '8.0.0'; - const pythonApiResponse = await fetch( - `${PYTHON_API_URL}/packages/${packageName}/${version}`, - ); - const pythonApiData = await pythonApiResponse.json(); + const pythonApiResponse = await fetch( + `${PYTHON_API_URL}/packages/${packageName}/${version}`, + ); + const pythonApiData = await pythonApiResponse.json(); - return request(app.getHttpServer()) - .get(`/packages/${packageName}/${version}`) - .expect((r) => { - expect(r.status).toEqual(200); - expect(r.body).toEqual(pythonApiData); - }); + return request(app.getHttpServer()) + .get(`/packages/${packageName}/${version}`) + .expect((r) => { + expect(r.status).toEqual(200); + expect(r.body).toEqual(pythonApiData); + }); + }); + + it('returns 404 for non-existent package', async () => { + const nonExistentPackage = 'npm:@sentry/non-existent-package'; + const version = 'latest'; + + const pythonApiResponse = await fetch( + `${PYTHON_API_URL}/packages/${nonExistentPackage}/${version}`, + ); + + expect(pythonApiResponse.status).toEqual(404); + + return request(app.getHttpServer()) + .get(`/packages/${nonExistentPackage}/${version}`) + .expect(404); + }); }); }); diff --git a/api-node/test/utils.ts b/api-node/test/utils.ts index e8eab428a..3c9302694 100644 --- a/api-node/test/utils.ts +++ b/api-node/test/utils.ts @@ -1,2 +1,5 @@ const PYTHON_API_PORT = 5031; export const PYTHON_API_URL = `http://localhost:${PYTHON_API_PORT}`; + +import * as fs from 'fs'; +import * as path from 'path'; diff --git a/api-node/test/utils/getUrls.ts b/api-node/test/utils/getUrls.ts new file mode 100644 index 000000000..5df08cc08 --- /dev/null +++ b/api-node/test/utils/getUrls.ts @@ -0,0 +1,198 @@ +import { PYTHON_API_URL } from '../utils'; +import * as fs from 'fs'; +import * as path from 'path'; + +type URLsList = { + url: string; + status: number; +}[]; + +export async function getAllYesAllUrls(): Promise { + const urls: URLsList = [ + { url: '/packages', status: 200 }, + { url: '/sdks', status: 200 }, + { url: '/apps', status: 200 }, + { url: '/marketing-slugs', status: 200 }, + { url: '/aws-lambda-layers', status: 200 }, + { url: '/healthz', status: 200 }, + ]; + + // ------- packages ------- + + const packages = await fetch(`${PYTHON_API_URL}/packages`); + const packagesData = await packages.json(); + const canonicalPackageNames = Object.entries(packagesData).map( + // @ts-expect-error packagesData is not typed + (e) => e[1].canonical, + ); + + urls.push( + ...canonicalPackageNames.map((p) => ({ + url: `/packages/${p}/latest`, + status: 200, + })), + ); + + let errors = 0; + for (const packageName in packagesData) { + const versionsUrl = `/packages/${packageName}/versions`; + const versionsResponse = await fetch(`${PYTHON_API_URL}/${versionsUrl}`); + + if (versionsResponse.status !== 200) { + urls.push({ + url: versionsUrl, + status: versionsResponse.status, + }); + } else { + try { + const versionsData = await versionsResponse.json(); + + const allVersions: string[] = versionsData.versions; + + urls.push( + ...allVersions.map((v) => ({ + url: `/packages/${packageName}/${v}`, + status: 200, + })), + ); + } catch (e) { + console.debug({ versionsUrl }); + console.error(e); + ++errors; + } + } + } + + // ------- sdks ------- + + const sdks = await fetch(`${PYTHON_API_URL}/sdks`); + const sdksData = await sdks.json(); + const canonicalSdkNames = Object.keys(sdksData); + + urls.push( + ...canonicalSdkNames.map((s) => ({ + url: `/sdks/${s}/latest`, + status: 200, + })), + ); + + for (const sdkName in sdksData) { + const versionsUrl = `/sdks/${sdkName}/versions`; + const versionsResponse = await fetch(`${PYTHON_API_URL}${versionsUrl}`); + + if (versionsResponse.status !== 200) { + urls.push({ + url: versionsUrl, + status: versionsResponse.status, + }); + } else { + urls.push({ + url: versionsUrl, + status: 200, + }); + try { + const versionsData = await versionsResponse.json(); + + const allVersions: string[] = versionsData.versions; + + urls.push( + ...allVersions.map((v) => ({ + url: `/sdks/${sdkName}/${v}`, + status: 200, + })), + ); + } catch (e) { + console.debug({ versionsUrl }); + console.error(e); + ++errors; + } + } + } + + // ------- apps ------- + + const apps = await fetch(`${PYTHON_API_URL}/apps`); + const appsData = await apps.json(); + const appIds = Object.keys(appsData); + + urls.push( + ...appIds.map((a) => ({ + url: `/apps/${a}/latest`, + status: 200, + })), + ); + urls.push( + ...appIds.map((a) => ({ + url: `/apps/${a}/${appsData[a].version}`, + status: 200, + })), + ); + + for (const appEntry of Object.entries(appsData)) { + const appId = appEntry[0]; + // @ts-expect-error appsData is not typed + const fileUrls = appEntry[1].file_urls; + + for (const fileUrl of Object.entries(fileUrls)) { + const parts = fileUrl[0].split('/').at(-1).replace('.exe', '').split('-'); + const platform = parts.at(-2); + const arch = parts.at(-1); + const pkgName = parts.slice(0, -2).join('-'); + if (!pkgName) { + continue; + } + const downloadUrl = `/apps/${appId}/latest?response=download&platform=${platform}&arch=${arch}&package=${pkgName}`; + const url = new URL(downloadUrl, PYTHON_API_URL); + const downloadResponse = await fetch(url, { + redirect: 'manual', + }); + urls.push({ + url: downloadUrl, + status: downloadResponse.status, + }); + } + } + + // ------- marketing slugs ------- + + const marketingSlugs = await fetch(`${PYTHON_API_URL}/marketing-slugs`); + const marketingSlugsData = await marketingSlugs.json(); + const marketingSlugIds = marketingSlugsData.slugs; + + urls.push( + ...marketingSlugIds + .filter((s) => s !== 'createdAt') + .map((s) => ({ + url: `/marketing-slugs/${s}`, + status: 200, + })), + ); + + // ------- summary ------- + + const numberOfUrlsByStatus = urls.reduce( + (acc, url) => { + acc[url.status] = (acc[url.status] || 0) + 1; + return acc; + }, + {} as Record, + ); + + console.table([ + { + ...numberOfUrlsByStatus, + 'URL scraping errors': errors, + }, + ]); + fs.writeFileSync( + path.join(__dirname, '.urls.json'), + JSON.stringify(urls, null, 2), + { + flag: 'w', + }, + ); + + return urls; +} + +getAllYesAllUrls(); diff --git a/api-server/apiserver.py b/api-server/apiserver.py index 7580a6ea9..0e5874167 100644 --- a/api-server/apiserver.py +++ b/api-server/apiserver.py @@ -140,6 +140,8 @@ def get_package_versions(self, canonical): if ":" not in canonical: return registry, package = canonical.split(":", 1) + # Allow ":" to be used as a path separator + package = package.replace(":", "/") rv = set() for filename in os.listdir(self._path("packages", registry, package)): if filename.endswith(".json"):