From 34257ec1cc15d0a7d310ebf55557b33042aa7d87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Fri, 11 Feb 2022 15:51:59 +0100 Subject: [PATCH 1/4] ref: Remove all Gradle-related code from Maven target --- README.md | 17 +++--- src/targets/__tests__/maven.test.ts | 87 +++-------------------------- src/targets/maven.ts | 72 +++--------------------- 3 files changed, 22 insertions(+), 154 deletions(-) diff --git a/README.md b/README.md index 723a91078..ccb469c0f 100644 --- a/README.md +++ b/README.md @@ -1013,14 +1013,13 @@ Note: in order to see the output of the commands, set the [logging level](#loggi **Configuration** -| Option | Description | -| ------------------- | --------------------------------------------------------------------- | -| `gradleCliPath` | Path to the Gradle CLI. It must be executable by the calling process. | -| `mavenCliPath` | Path to the Maven CLI. It must be executable by the calling process. | -| `mavenSettingsPath` | Path to the Maven `settings.xml` file. | -| `mavenRepoId` | ID of the Maven server in the `settings.xml`. | -| `mavenRepoUrl` | URL of the Maven repository. | -| `android` | Android configuration, see below. | +| Option | Description | +| ------------------- | -------------------------------------------------------------------- | +| `mavenCliPath` | Path to the Maven CLI. It must be executable by the calling process. | +| `mavenSettingsPath` | Path to the Maven `settings.xml` file. | +| `mavenRepoId` | ID of the Maven server in the `settings.xml`. | +| `mavenRepoUrl` | URL of the Maven repository. | +| `android` | Android configuration, see below. | If your project isn't related to Android, you don't need this configuration and can set the option to `false`. If not, set the following nested elements: @@ -1034,7 +1033,6 @@ can set the option to `false`. If not, set the following nested elements: ```yaml targets: - name: maven - gradleCliPath: ./gradlew mavenCliPath: scripts/mvnw.cmd mavenSettingsPath: scripts/settings.xml mavenRepoId: ossrh @@ -1047,7 +1045,6 @@ targets: ```yaml targets: - name: maven - gradleCliPath: ./gradlew mavenCliPath: scripts/mvnw.cmd mavenSettingsPath: scripts/settings.xml mavenRepoId: ossrh diff --git a/src/targets/__tests__/maven.test.ts b/src/targets/__tests__/maven.test.ts index d4a6e6d3e..5fe3e4a27 100644 --- a/src/targets/__tests__/maven.test.ts +++ b/src/targets/__tests__/maven.test.ts @@ -1,6 +1,3 @@ -import { promises as fsPromises } from 'fs'; -import { homedir } from 'os'; -import { join } from 'path'; import { NoneArtifactProvider } from '../../artifact_providers/none'; import { MavenTarget, @@ -53,7 +50,6 @@ function getFullTargetConfig(): any { GPG_PASSPHRASE: DEFAULT_OPTION_VALUE, OSSRH_USERNAME: DEFAULT_OPTION_VALUE, OSSRH_PASSWORD: DEFAULT_OPTION_VALUE, - gradleCliPath: DEFAULT_OPTION_VALUE, mavenCliPath: DEFAULT_OPTION_VALUE, mavenSettingsPath: DEFAULT_OPTION_VALUE, mavenRepoId: DEFAULT_OPTION_VALUE, @@ -71,7 +67,6 @@ function getRequiredTargetConfig(): any { GPG_PASSPHRASE: DEFAULT_OPTION_VALUE, OSSRH_USERNAME: DEFAULT_OPTION_VALUE, OSSRH_PASSWORD: DEFAULT_OPTION_VALUE, - gradleCliPath: DEFAULT_OPTION_VALUE, mavenCliPath: DEFAULT_OPTION_VALUE, mavenSettingsPath: DEFAULT_OPTION_VALUE, mavenRepoId: DEFAULT_OPTION_VALUE, @@ -105,7 +100,7 @@ describe('Maven target configuration', () => { test('env vars without options', () => { expect(() => createMavenTarget({})).toThrowErrorMatchingInlineSnapshot( - `"Required configuration gradleCliPath not found in configuration file. See the documentation for more details."` + `"Required configuration mavenCliPath not found in configuration file. See the documentation for more details."` ); }); @@ -203,38 +198,16 @@ describe('publish', () => { test('main flow', async () => { const callOrder: string[] = []; const mvnTarget = createMavenTarget(); - const createGradlePropsMock = jest.fn( - async () => void callOrder.push('createGradleProps') + mvnTarget.upload = jest.fn(async () => void callOrder.push('upload')); + mvnTarget.closeAndReleaseRepository = jest.fn( + async () => void callOrder.push('closeAndReleaseRepository') ); - mvnTarget.createUserGradlePropsFile = createGradlePropsMock; - const deleteGradlePropsMock = jest.fn( - async () => void callOrder.push('deleteGradleProps') - ); - mvnTarget.deleteUserGradlePropsFile = deleteGradlePropsMock; - const uploadMock = jest.fn(async () => void callOrder.push('upload')); - mvnTarget.upload = uploadMock; - (retrySpawnProcess as jest.MockedFunction< - typeof retrySpawnProcess - >).mockImplementationOnce( - async () => void callOrder.push('closeAndRelease') - ); - const revision = 'r3v1s10n'; await mvnTarget.publish('1.0.0', revision); - expect(createGradlePropsMock).toHaveBeenCalledTimes(1); - expect(uploadMock).toHaveBeenCalledTimes(1); - expect(uploadMock).toHaveBeenLastCalledWith(revision); - expect(deleteGradlePropsMock).toHaveBeenCalledTimes(1); - expect(retrySpawnProcess).toHaveBeenCalledTimes(1); - expect(retrySpawnProcess).toHaveBeenCalledWith(DEFAULT_OPTION_VALUE, [ - 'closeAndReleaseRepository', - ]); - expect(callOrder).toStrictEqual([ - 'createGradleProps', - 'upload', - 'closeAndRelease', - 'deleteGradleProps', - ]); + expect(mvnTarget.upload).toHaveBeenCalledTimes(1); + expect(mvnTarget.upload).toHaveBeenLastCalledWith(revision); + expect(mvnTarget.closeAndReleaseRepository).toHaveBeenCalledTimes(1); + expect(callOrder).toStrictEqual(['upload', 'closeAndReleaseRepository']); }); test('upload POM', async () => { @@ -330,47 +303,3 @@ describe('publish', () => { expect(cmdArgs[7]).toBe(DEFAULT_OPTION_VALUE); }); }); - -describe('get gradle home directory', () => { - const gradleHomeEnvVar = 'GRADLE_USER_HOME'; - - beforeEach(() => { - setTargetSecretsInEnv(); - // no need to check whether it already exists - delete process.env[gradleHomeEnvVar]; - }); - - test('with gradle home', () => { - const expectedHomeDir = 'testDirectory'; - process.env[gradleHomeEnvVar] = expectedHomeDir; - const actual = createMavenTarget().getGradleHomeDir(); - expect(actual).toEqual(expectedHomeDir); - }); - - test('without gradle home', () => { - const expected = join(homedir(), '.gradle'); - const actual = createMavenTarget().getGradleHomeDir(); - expect(actual).toEqual(expected); - }); -}); - -describe('createUserGradlePropsFile', () => { - const gradleHomeEnvVar = 'GRADLE_USER_HOME'; - - afterEach(() => { - delete process.env[gradleHomeEnvVar]; - }); - - test('should make sure that directory exists before writing gradle props file', async () => { - const randomTmpDir = '/random/depth/of/directories'; - process.env[gradleHomeEnvVar] = randomTmpDir; - await createMavenTarget().createUserGradlePropsFile(); - expect(fsPromises.mkdir).toHaveBeenCalledWith(randomTmpDir, { - recursive: true, - }); - expect(fsPromises.writeFile).toHaveBeenCalledWith( - `${randomTmpDir}/gradle.properties`, - `mavenCentralUsername=my_default_value\nmavenCentralPassword=my_default_value` - ); - }); -}); diff --git a/src/targets/maven.ts b/src/targets/maven.ts index a0c0a7429..80fce874c 100644 --- a/src/targets/maven.ts +++ b/src/targets/maven.ts @@ -4,7 +4,6 @@ import { RemoteArtifact, } from '../artifact_providers/base'; import { BaseTarget } from './base'; -import { homedir } from 'os'; import { basename, extname, join, parse } from 'path'; import { promises as fsPromises } from 'fs'; import { checkExecutableIsPresent, extractZipArchive } from '../utils/system'; @@ -15,13 +14,6 @@ import { stringToRegexp } from '../utils/filters'; import { checkEnvForPrerequisite } from '../utils/env'; import { importGPGKey } from '../utils/gpg'; -const GRADLE_PROPERTIES_FILENAME = 'gradle.properties'; - -/** - * Default gradle user home directory. See - * https://docs.gradle.org/current/userguide/build_environment.html#sec:gradle_environment_variables - */ -const DEFAULT_GRADLE_USER_HOME = join(homedir(), '.gradle'); export const POM_DEFAULT_FILENAME = 'pom-default.xml'; const POM_FILE_EXT = '.xml'; // Must include the leading `.` const BOM_FILE_KEY_REGEXP = new RegExp('pom'); @@ -34,7 +26,6 @@ export const targetSecrets = [ type SecretsType = typeof targetSecrets[number]; export const targetOptions = [ - 'gradleCliPath', 'mavenCliPath', 'mavenSettingsPath', 'mavenRepoId', @@ -181,11 +172,6 @@ export class MavenTarget extends BaseTarget { this.mavenConfig.mavenCliPath ); checkExecutableIsPresent(this.mavenConfig.mavenCliPath); - this.logger.debug( - 'Checking if Gradle CLI is available: ', - this.mavenConfig.gradleCliPath - ); - checkExecutableIsPresent(this.mavenConfig.gradleCliPath); this.logger.debug('Checking if GPG is available'); checkExecutableIsPresent('gpg'); } @@ -196,58 +182,8 @@ export class MavenTarget extends BaseTarget { * @param revision Git commit SHA to be published. */ public async publish(_version: string, revison: string): Promise { - await this.createUserGradlePropsFile(); await this.upload(revison); - - // Maven central is very flaky, so retrying with an exponential delay in - // in case it fails. - // TODO: close the repository by doing the requests from Craft - // What the gradle plugin does is a / some requests to close the repo, see - // https://github.com/vanniktech/gradle-maven-publish-plugin/tree/master/src/main/kotlin/com/vanniktech/maven/publish/nexus - // If Craft did those requests, it wouldn't be necessary to rely on a third - // party plugin for releases. - await retrySpawnProcess(this.mavenConfig.gradleCliPath, [ - 'closeAndReleaseRepository', - ]); - await this.deleteUserGradlePropsFile(); - } - - /** - * Creates the required user's `gradle.properties` file. - * - * If there's an existing one, it's overwritten. - * TODO: control when it's overwritten with an option. - */ - public async createUserGradlePropsFile(): Promise { - const gradleHomeDir = this.getGradleHomeDir(); - // Setting `recursive: true` allows `mkdir` to not fail in case directory already exists. - await fsPromises.mkdir(gradleHomeDir, { recursive: true }); - return fsPromises.writeFile( - join(gradleHomeDir, GRADLE_PROPERTIES_FILENAME), - [ - // OSSRH and Maven Central credentials are the same - `mavenCentralUsername=${this.mavenConfig.OSSRH_USERNAME}`, - `mavenCentralPassword=${this.mavenConfig.OSSRH_PASSWORD}`, - ].join('\n') - ); - } - - /** - * Deletes the user's `gradle.properties` file. - */ - public async deleteUserGradlePropsFile(): Promise { - return fsPromises.unlink( - join(this.getGradleHomeDir(), GRADLE_PROPERTIES_FILENAME) - ); - } - - /** - * Retrieves the Gradle Home path. - * - * @returns the gradle home path. - */ - public getGradleHomeDir(): string { - return process.env.GRADLE_USER_HOME || DEFAULT_GRADLE_USER_HOME; + await this.closeAndReleaseRepository(); } /** @@ -444,4 +380,10 @@ export class MavenTarget extends BaseTarget { return `${moduleName}.jar`; } + + // Maven central does not indicate when it completes the action, so we need to + // retry every so often and query it for the new state of repository. + public async closeAndReleaseRepository(): Promise { + // + } } From 701fa02eb14820b63e2ebf0bca0cce1b6cafdade Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Fri, 11 Feb 2022 16:51:37 +0100 Subject: [PATCH 2/4] feat: Publish maven packages without use of Gradle --- src/targets/maven.ts | 110 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 108 insertions(+), 2 deletions(-) diff --git a/src/targets/maven.ts b/src/targets/maven.ts index 80fce874c..2ef9cafa5 100644 --- a/src/targets/maven.ts +++ b/src/targets/maven.ts @@ -6,8 +6,9 @@ import { import { BaseTarget } from './base'; import { basename, extname, join, parse } from 'path'; import { promises as fsPromises } from 'fs'; +import fetch from 'node-fetch'; import { checkExecutableIsPresent, extractZipArchive } from '../utils/system'; -import { retrySpawnProcess } from '../utils/async'; +import { retrySpawnProcess, sleep } from '../utils/async'; import { withTempDir } from '../utils/files'; import { ConfigurationError } from '../utils/errors'; import { stringToRegexp } from '../utils/filters'; @@ -18,6 +19,16 @@ export const POM_DEFAULT_FILENAME = 'pom-default.xml'; const POM_FILE_EXT = '.xml'; // Must include the leading `.` const BOM_FILE_KEY_REGEXP = new RegExp('pom'); +const NEXUS_API_BASE_URL = 'https://oss.sonatype.org/service/local/staging'; +const NEXUS_RETRY_DELAY = 10 * 1000; // 10s +const NEXUS_RETRY_DEADLINE = 15 * 60 * 1000; // 15min + +type NexusRepository = { + repositoryId: string; + type: 'open' | 'closed'; + transitioning: boolean; +}; + export const targetSecrets = [ 'GPG_PASSPHRASE', 'OSSRH_USERNAME', @@ -383,7 +394,102 @@ export class MavenTarget extends BaseTarget { // Maven central does not indicate when it completes the action, so we need to // retry every so often and query it for the new state of repository. + // Based on: https://github.com/vanniktech/gradle-maven-publish-plugin/ implementation. public async closeAndReleaseRepository(): Promise { - // + const repository = await this.getRepository(); + const { repositoryId, type } = repository; + + if (type !== 'open') { + throw new Error( + 'No open repositories available. Go to Nexus Repository Manager to see what happened.' + ); + } + + await this.closeRepository(repositoryId); + await this.releaseRepository(repositoryId); + } + + private getNexusRequestHeaders(): Record { + return { + Accept: 'application/json', + Authorization: `Basic ${Buffer.from( + `${process.env.OSSRH_USERNAME}:${process.env.OSSRH_USERNAME}` + ).toString(`base64`)}`, + }; + } + + private async getRepository(): Promise { + const response = await fetch(`${NEXUS_API_BASE_URL}/profile_repositories`, { + headers: this.getNexusRequestHeaders(), + }); + if (!response.ok) { + throw new Error( + `Unable to fetch repository: ${response.status}, ${response.statusText}` + ); + } + + const body = await response.json(); + const repositories = body.data; + + if (repositories.length === 0) { + throw new Error(`No available repositories. Nothing to publish.`); + } + + if (repositories.length > 1) { + throw new Error( + `There are more than 1 active repositories. Please close unwanted deployments.` + ); + } + + return repositories[0]; + } + + private async closeRepository(repositoryId: string): Promise { + const response = await fetch(`${NEXUS_API_BASE_URL}/bulk/close`, { + headers: this.getNexusRequestHeaders(), + method: 'POST', + body: JSON.stringify({ + data: { stagedRepositoryIds: [repositoryId] }, + }), + }); + + if (!response.ok) { + throw new Error( + `Unable to close repository: ${response.status}, ${response.statusText}` + ); + } + + const poolingStartTime = Date.now(); + + while (true) { + if (Date.now() - poolingStartTime > NEXUS_RETRY_DEADLINE) { + throw new Error('Deadline for Nexus repository status change reached.'); + } + + await sleep(NEXUS_RETRY_DELAY); + + const repository = await this.getRepository(); + const { type, transitioning } = repository; + + if (type === 'closed' && !transitioning) { + break; + } + } + } + + private async releaseRepository(repositoryId: string): Promise { + const response = await fetch(`${NEXUS_API_BASE_URL}/bulk/promote`, { + headers: this.getNexusRequestHeaders(), + method: 'POST', + body: JSON.stringify({ + data: { stagedRepositoryIds: [repositoryId] }, + }), + }); + + if (!response.ok) { + throw new Error( + `Unable to release repository: ${response.status}, ${response.statusText}` + ); + } } } From 630d702543a5acb08e6fb65f70ae7b750d527dd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Mon, 14 Feb 2022 12:57:01 +0100 Subject: [PATCH 3/4] Add comprehensive tests around Nexus Repository --- package.json | 1 + src/targets/__tests__/maven.test.ts | 292 ++++++++++++++++++++++++++-- src/targets/maven.ts | 43 ++-- yarn.lock | 20 ++ 4 files changed, 326 insertions(+), 30 deletions(-) diff --git a/package.json b/package.json index e83fe9a93..309df49a7 100644 --- a/package.json +++ b/package.json @@ -66,6 +66,7 @@ "json-schema-to-typescript": "5.7.0", "mkdirp": "0.5.1", "mustache": "3.0.1", + "nock": "^13.2.4", "node-fetch": "^2.6.1", "nvar": "1.3.1", "ora": "5.4.0", diff --git a/src/targets/__tests__/maven.test.ts b/src/targets/__tests__/maven.test.ts index 5fe3e4a27..49b0aa074 100644 --- a/src/targets/__tests__/maven.test.ts +++ b/src/targets/__tests__/maven.test.ts @@ -1,11 +1,15 @@ +import { URL } from 'url'; +import nock from 'nock'; import { NoneArtifactProvider } from '../../artifact_providers/none'; import { MavenTarget, + NexusRepository, + NEXUS_API_BASE_URL, POM_DEFAULT_FILENAME, targetOptions, targetSecrets, } from '../maven'; -import { retrySpawnProcess } from '../../utils/async'; +import { retrySpawnProcess, sleep } from '../../utils/async'; import { withTempDir } from '../../utils/files'; import { importGPGKey } from '../../utils/gpg'; @@ -29,7 +33,15 @@ jest.mock('../../utils/system', () => ({ extractZipArchive: jest.fn(), })); -jest.mock('../../utils/async'); +jest.mock('../../utils/async', () => ({ + ...jest.requireActual('../../utils/async'), + retrySpawnProcess: jest.fn(() => Promise.resolve()), + sleep: jest.fn(() => + setTimeout(() => { + Promise.resolve(); + }, 10) + ), +})); const DEFAULT_OPTION_VALUE = 'my_default_value'; @@ -86,11 +98,25 @@ function createMavenTarget( return new MavenTarget(mergedConfig, new NoneArtifactProvider()); } -describe('Maven target configuration', () => { - beforeEach(() => setTargetSecretsInEnv()); +function getRepositoryInfo( + type: NexusRepository['type'], + transitioning: NexusRepository['transitioning'] +): NexusRepository { + return { + type, + repositoryId: 'sentry-java', + transitioning, + }; +} + +beforeEach(() => { + jest.resetAllMocks(); + setTargetSecretsInEnv(); +}); - afterEach(() => removeTargetSecretsFromEnv()); +afterEach(() => removeTargetSecretsFromEnv()); +describe('Maven target configuration', () => { test('no env vars and no options', () => { removeTargetSecretsFromEnv(); expect(createMavenTarget).toThrowErrorMatchingInlineSnapshot( @@ -187,14 +213,6 @@ describe('Maven target configuration', () => { }); describe('publish', () => { - const tmpDirName = 'tmpDir'; - - beforeAll(() => setTargetSecretsInEnv()); - - afterAll(() => removeTargetSecretsFromEnv()); - - beforeEach(() => jest.resetAllMocks()); - test('main flow', async () => { const callOrder: string[] = []; const mvnTarget = createMavenTarget(); @@ -205,10 +223,14 @@ describe('publish', () => { const revision = 'r3v1s10n'; await mvnTarget.publish('1.0.0', revision); expect(mvnTarget.upload).toHaveBeenCalledTimes(1); - expect(mvnTarget.upload).toHaveBeenLastCalledWith(revision); + expect(mvnTarget.upload).toHaveBeenCalledWith(revision); expect(mvnTarget.closeAndReleaseRepository).toHaveBeenCalledTimes(1); expect(callOrder).toStrictEqual(['upload', 'closeAndReleaseRepository']); }); +}); + +describe('upload', () => { + const tmpDirName = 'tmpDir'; test('upload POM', async () => { // simple mock to always use the same temporary directory, @@ -303,3 +325,245 @@ describe('publish', () => { expect(cmdArgs[7]).toBe(DEFAULT_OPTION_VALUE); }); }); + +describe('closeAndReleaseRepository', () => { + test('should throw if repository is not opened', async () => { + const mvnTarget = createMavenTarget(); + mvnTarget.getRepository = jest.fn(() => + Promise.resolve(getRepositoryInfo('closed', false)) + ); + await expect(mvnTarget.closeAndReleaseRepository()).rejects.toThrow(); + }); + + test('should call closeRepository and releaseRepository with fetched repository ID', async () => { + const mvnTarget = createMavenTarget(); + mvnTarget.getRepository = jest.fn(() => + Promise.resolve(getRepositoryInfo('open', false)) + ); + const callOrder: string[] = []; + mvnTarget.closeRepository = jest.fn(async () => { + callOrder.push('closeRepository'); + return true; + }); + mvnTarget.releaseRepository = jest.fn(async () => { + callOrder.push('releaseRepository'); + return true; + }); + + await mvnTarget.closeAndReleaseRepository(); + + expect(mvnTarget.closeRepository).toHaveBeenCalledWith('sentry-java'); + expect(mvnTarget.releaseRepository).toHaveBeenCalledWith('sentry-java'); + expect(callOrder).toStrictEqual(['closeRepository', 'releaseRepository']); + }); + + test('should not release repostiory if it was not closed properly', async () => { + const mvnTarget = createMavenTarget(); + mvnTarget.getRepository = jest.fn(() => + Promise.resolve(getRepositoryInfo('open', false)) + ); + mvnTarget.closeRepository = jest.fn(() => Promise.reject()); + mvnTarget.releaseRepository = jest.fn(() => Promise.resolve(true)); + + try { + await mvnTarget.closeAndReleaseRepository(); + } catch (e) { + // no-empty + } + + expect(mvnTarget.closeRepository).toHaveBeenCalledWith('sentry-java'); + expect(mvnTarget.releaseRepository).not.toHaveBeenCalledWith(); + }); +}); + +describe('getRepository', () => { + const url = new URL(NEXUS_API_BASE_URL); + const repositoryInfo = getRepositoryInfo('open', false); + + test('should return the repository if server responds correctly', async () => { + nock(url.origin) + .get(`${url.pathname}/profile_repositories`) + .reply(200, { + data: [repositoryInfo], + }); + + const mvnTarget = createMavenTarget(); + await expect(mvnTarget.getRepository()).resolves.toStrictEqual( + repositoryInfo + ); + }); + + test('should throw if server returns no active repositories', async () => { + nock(url.origin).get(`${url.pathname}/profile_repositories`).reply(200, { + data: [], + }); + + const mvnTarget = createMavenTarget(); + await expect(mvnTarget.getRepository()).rejects.toThrow( + new Error('No available repositories. Nothing to publish.') + ); + }); + + test('should throw if server returns more than one active repository', async () => { + nock(url.origin) + .get(`${url.pathname}/profile_repositories`) + .reply(200, { + data: [repositoryInfo, repositoryInfo], + }); + + const mvnTarget = createMavenTarget(); + await expect(mvnTarget.getRepository()).rejects.toThrow( + new Error( + 'There are more than 1 active repositories. Please close unwanted deployments.' + ) + ); + }); + + test('should throw if server doesnt accept the request', async () => { + nock(url.origin).get(`${url.pathname}/profile_repositories`).reply(500); + + const mvnTarget = createMavenTarget(); + await expect(mvnTarget.getRepository()).rejects.toThrow( + new Error('Unable to fetch repository: 500, Internal Server Error') + ); + }); +}); + +describe('closeRepository', () => { + const url = new URL(NEXUS_API_BASE_URL); + const repositoryId = 'sentry-java'; + + test('should return true if server responds correctly', async () => { + nock(url.origin) + .post(`${url.pathname}/bulk/close`, { + data: { stagedRepositoryIds: [repositoryId] }, + }) + .reply(200); + + const mvnTarget = createMavenTarget(); + mvnTarget.getRepository = jest.fn(() => + Promise.resolve(getRepositoryInfo('closed', false)) + ); + + await expect(mvnTarget.closeRepository(repositoryId)).resolves.toBe(true); + }); + + test('should throw if server doesnt accept the request', async () => { + nock(url.origin) + .post(`${url.pathname}/bulk/close`, { + data: { stagedRepositoryIds: [repositoryId] }, + }) + .reply(500); + + const mvnTarget = createMavenTarget(); + await expect(mvnTarget.closeRepository(repositoryId)).rejects.toThrow( + new Error('Unable to close repository: 500, Internal Server Error') + ); + }); + + test('should wait for the status of repository to be changed', async () => { + nock(url.origin) + .post(`${url.pathname}/bulk/close`, { + data: { stagedRepositoryIds: [repositoryId] }, + }) + .reply(200); + + const mvnTarget = createMavenTarget(); + mvnTarget.getRepository = jest + .fn() + .mockImplementationOnce(() => + Promise.resolve(getRepositoryInfo('open', false)) + ) + .mockImplementationOnce(() => + Promise.resolve(getRepositoryInfo('open', false)) + ) + .mockImplementationOnce(() => + Promise.resolve(getRepositoryInfo('closed', false)) + ); + + await expect(mvnTarget.closeRepository(repositoryId)).resolves.toBe(true); + expect(sleep).toHaveBeenCalledTimes(3); + expect(mvnTarget.getRepository).toHaveBeenCalledTimes(3); + }); + + test('should wait for the repository to not be in transitioning state', async () => { + nock(url.origin) + .post(`${url.pathname}/bulk/close`, { + data: { stagedRepositoryIds: [repositoryId] }, + }) + .reply(200); + + const mvnTarget = createMavenTarget(); + mvnTarget.getRepository = jest + .fn() + .mockImplementationOnce(() => + Promise.resolve(getRepositoryInfo('closed', true)) + ) + .mockImplementationOnce(() => + Promise.resolve(getRepositoryInfo('closed', true)) + ) + .mockImplementationOnce(() => + Promise.resolve(getRepositoryInfo('closed', false)) + ); + + await expect(mvnTarget.closeRepository(repositoryId)).resolves.toBe(true); + expect(sleep).toHaveBeenCalledTimes(3); + expect(mvnTarget.getRepository).toHaveBeenCalledTimes(3); + }); + + test('should throw when status change deadline is reached', async () => { + nock(url.origin) + .post(`${url.pathname}/bulk/close`, { + data: { stagedRepositoryIds: [repositoryId] }, + }) + .reply(200); + + const mvnTarget = createMavenTarget(); + mvnTarget.getRepository = jest.fn(() => + Promise.resolve(getRepositoryInfo('open', false)) + ); + + // Deadline is 30min, so we fake pooling start time and initial read to 1min + // and second iteration to something over 30min + jest + .spyOn(Date, 'now') + .mockImplementationOnce(() => 1 * 60 * 1000) + .mockImplementationOnce(() => 1 * 60 * 1000) + .mockImplementationOnce(() => 32 * 60 * 1000); + + await expect(mvnTarget.closeRepository(repositoryId)).rejects.toThrow( + new Error('Deadline for Nexus repository status change reached.') + ); + expect(sleep).toHaveBeenCalledTimes(1); + expect(mvnTarget.getRepository).toHaveBeenCalledTimes(1); + }); +}); + +describe('releaseRepository', () => { + const url = new URL(NEXUS_API_BASE_URL); + const repositoryId = 'sentry-java'; + + test('should return true if server responds correctly', async () => { + nock(url.origin) + .post(`${url.pathname}/bulk/promote`, { + data: { stagedRepositoryIds: [repositoryId] }, + }) + .reply(200); + + const mvnTarget = createMavenTarget(); + await expect(mvnTarget.releaseRepository(repositoryId)).resolves.toBe(true); + }); + + test('should throw if server doesnt accept the request', async () => { + nock(url.origin) + .post(`${url.pathname}/bulk/promote`, { + data: { stagedRepositoryIds: [repositoryId] }, + }) + .reply(500); + + const mvnTarget = createMavenTarget(); + await expect(mvnTarget.releaseRepository(repositoryId)).rejects.toThrow( + new Error('Unable to release repository: 500, Internal Server Error') + ); + }); +}); diff --git a/src/targets/maven.ts b/src/targets/maven.ts index 2ef9cafa5..222503f39 100644 --- a/src/targets/maven.ts +++ b/src/targets/maven.ts @@ -19,11 +19,13 @@ export const POM_DEFAULT_FILENAME = 'pom-default.xml'; const POM_FILE_EXT = '.xml'; // Must include the leading `.` const BOM_FILE_KEY_REGEXP = new RegExp('pom'); -const NEXUS_API_BASE_URL = 'https://oss.sonatype.org/service/local/staging'; +// TODO: Make it configurable to allow for sentry-clj releases? +export const NEXUS_API_BASE_URL = + 'https://oss.sonatype.org/service/local/staging'; const NEXUS_RETRY_DELAY = 10 * 1000; // 10s -const NEXUS_RETRY_DEADLINE = 15 * 60 * 1000; // 15min +const NEXUS_RETRY_DEADLINE = 30 * 60 * 1000; // 30min -type NexusRepository = { +export type NexusRepository = { repositoryId: string; type: 'open' | 'closed'; transitioning: boolean; @@ -409,19 +411,11 @@ export class MavenTarget extends BaseTarget { await this.releaseRepository(repositoryId); } - private getNexusRequestHeaders(): Record { - return { - Accept: 'application/json', - Authorization: `Basic ${Buffer.from( - `${process.env.OSSRH_USERNAME}:${process.env.OSSRH_USERNAME}` - ).toString(`base64`)}`, - }; - } - - private async getRepository(): Promise { + public async getRepository(): Promise { const response = await fetch(`${NEXUS_API_BASE_URL}/profile_repositories`, { headers: this.getNexusRequestHeaders(), }); + if (!response.ok) { throw new Error( `Unable to fetch repository: ${response.status}, ${response.statusText}` @@ -444,7 +438,7 @@ export class MavenTarget extends BaseTarget { return repositories[0]; } - private async closeRepository(repositoryId: string): Promise { + public async closeRepository(repositoryId: string): Promise { const response = await fetch(`${NEXUS_API_BASE_URL}/bulk/close`, { headers: this.getNexusRequestHeaders(), method: 'POST', @@ -472,12 +466,18 @@ export class MavenTarget extends BaseTarget { const { type, transitioning } = repository; if (type === 'closed' && !transitioning) { - break; + return true; } + + this.logger.info( + `Nexus repository still not closed. Waiting for ${ + NEXUS_RETRY_DELAY / 1000 + }}s to try again.` + ); } } - private async releaseRepository(repositoryId: string): Promise { + public async releaseRepository(repositoryId: string): Promise { const response = await fetch(`${NEXUS_API_BASE_URL}/bulk/promote`, { headers: this.getNexusRequestHeaders(), method: 'POST', @@ -491,5 +491,16 @@ export class MavenTarget extends BaseTarget { `Unable to release repository: ${response.status}, ${response.statusText}` ); } + + return true; + } + + private getNexusRequestHeaders(): Record { + return { + Accept: 'application/json', + Authorization: `Basic ${Buffer.from( + `${process.env.OSSRH_USERNAME}:${process.env.OSSRH_USERNAME}` + ).toString(`base64`)}`, + }; } } diff --git a/yarn.lock b/yarn.lock index b471b72c2..54d4bc3ca 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4190,6 +4190,11 @@ lodash.merge@^4.6.2: resolved "https://registry.yarnpkg.com/lodash.merge/-/lodash.merge-4.6.2.tgz#558aa53b43b661e1925a0afdfa36a9a1085fe57a" integrity sha512-0KpjqXRVvrYyCsX1swR/XTK0va6VQkQM6MNo7PqW77ByjAhoARA8EfrP1N4+KlKj8YS0ZUCtRT/YUuhyYDujIQ== +lodash.set@^4.3.2: + version "4.3.2" + resolved "https://registry.yarnpkg.com/lodash.set/-/lodash.set-4.3.2.tgz#d8757b1da807dde24816b0d6a84bea1a76230b23" + integrity sha1-2HV7HagH3eJIFrDWqEvqGnYjCyM= + lodash.sortby@^4.7.0: version "4.7.0" resolved "https://registry.yarnpkg.com/lodash.sortby/-/lodash.sortby-4.7.0.tgz#edd14c824e2cc9c1e0b0a1b42bb5210516a42438" @@ -4456,6 +4461,16 @@ nice-try@^1.0.4: resolved "https://registry.yarnpkg.com/nice-try/-/nice-try-1.0.5.tgz#a3378a7696ce7d223e88fc9b764bd7ef1089e366" integrity sha512-1nh45deeb5olNY7eX82BkPO7SSxR5SSYJiPTrTdFUVYwAl8CKMA5N9PjTYkHiRjisVcxcQ1HXdLhx2qxxJzLNQ== +nock@^13.2.4: + version "13.2.4" + resolved "https://registry.yarnpkg.com/nock/-/nock-13.2.4.tgz#43a309d93143ee5cdcca91358614e7bde56d20e1" + integrity sha512-8GPznwxcPNCH/h8B+XZcKjYPXnUV5clOKCjAqyjsiqA++MpNx9E9+t8YPp0MbThO+KauRo7aZJ1WuIZmOrT2Ug== + dependencies: + debug "^4.1.0" + json-stringify-safe "^5.0.1" + lodash.set "^4.3.2" + propagate "^2.0.0" + node-fetch@>=2.6.1, node-fetch@^2.3.0, node-fetch@^2.6.1: version "2.6.1" resolved "https://registry.yarnpkg.com/node-fetch/-/node-fetch-2.6.1.tgz#045bd323631f76ed2e2b55573394416b639a0052" @@ -4822,6 +4837,11 @@ prompts@2.4.1, prompts@^2.0.1: kleur "^3.0.3" sisteransi "^1.0.5" +propagate@^2.0.0: + version "2.0.1" + resolved "https://registry.yarnpkg.com/propagate/-/propagate-2.0.1.tgz#40cdedab18085c792334e64f0ac17256d38f9a45" + integrity sha512-vGrhOavPSTz4QVNuBNdcNXePNdNMaO1xj9yBeH1ScQPjk/rhg9sSlCXPhMkFuaNNW/syTvYqsnbIJxMBfRbbag== + protocols@^1.1.0, protocols@^1.4.0: version "1.4.8" resolved "https://registry.yarnpkg.com/protocols/-/protocols-1.4.8.tgz#48eea2d8f58d9644a4a32caae5d5db290a075ce8" From bb219d634c25c2799a1596bb65fad10ce376d23d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Mon, 14 Feb 2022 13:04:37 +0100 Subject: [PATCH 4/4] Code review feedback --- src/targets/__tests__/maven.test.ts | 10 +++++++--- src/targets/maven.ts | 12 +++++------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/targets/__tests__/maven.test.ts b/src/targets/__tests__/maven.test.ts index 49b0aa074..6ced20cd4 100644 --- a/src/targets/__tests__/maven.test.ts +++ b/src/targets/__tests__/maven.test.ts @@ -424,7 +424,7 @@ describe('getRepository', () => { const mvnTarget = createMavenTarget(); await expect(mvnTarget.getRepository()).rejects.toThrow( - new Error('Unable to fetch repository: 500, Internal Server Error') + new Error('Unable to fetch repositories: 500, Internal Server Error') ); }); }); @@ -457,7 +457,9 @@ describe('closeRepository', () => { const mvnTarget = createMavenTarget(); await expect(mvnTarget.closeRepository(repositoryId)).rejects.toThrow( - new Error('Unable to close repository: 500, Internal Server Error') + new Error( + 'Unable to close repository sentry-java: 500, Internal Server Error' + ) ); }); @@ -563,7 +565,9 @@ describe('releaseRepository', () => { const mvnTarget = createMavenTarget(); await expect(mvnTarget.releaseRepository(repositoryId)).rejects.toThrow( - new Error('Unable to release repository: 500, Internal Server Error') + new Error( + 'Unable to release repository sentry-java: 500, Internal Server Error' + ) ); }); }); diff --git a/src/targets/maven.ts b/src/targets/maven.ts index 222503f39..2a3b839be 100644 --- a/src/targets/maven.ts +++ b/src/targets/maven.ts @@ -398,8 +398,7 @@ export class MavenTarget extends BaseTarget { // retry every so often and query it for the new state of repository. // Based on: https://github.com/vanniktech/gradle-maven-publish-plugin/ implementation. public async closeAndReleaseRepository(): Promise { - const repository = await this.getRepository(); - const { repositoryId, type } = repository; + const { repositoryId, type } = await this.getRepository(); if (type !== 'open') { throw new Error( @@ -418,7 +417,7 @@ export class MavenTarget extends BaseTarget { if (!response.ok) { throw new Error( - `Unable to fetch repository: ${response.status}, ${response.statusText}` + `Unable to fetch repositories: ${response.status}, ${response.statusText}` ); } @@ -449,7 +448,7 @@ export class MavenTarget extends BaseTarget { if (!response.ok) { throw new Error( - `Unable to close repository: ${response.status}, ${response.statusText}` + `Unable to close repository ${repositoryId}: ${response.status}, ${response.statusText}` ); } @@ -462,8 +461,7 @@ export class MavenTarget extends BaseTarget { await sleep(NEXUS_RETRY_DELAY); - const repository = await this.getRepository(); - const { type, transitioning } = repository; + const { type, transitioning } = await this.getRepository(); if (type === 'closed' && !transitioning) { return true; @@ -488,7 +486,7 @@ export class MavenTarget extends BaseTarget { if (!response.ok) { throw new Error( - `Unable to release repository: ${response.status}, ${response.statusText}` + `Unable to release repository ${repositoryId}: ${response.status}, ${response.statusText}` ); }