From d771cb0a3d4eb04245b781edef23781f14aa1f25 Mon Sep 17 00:00:00 2001 From: iker barriocanal <32816711+iker-barriocanal@users.noreply.github.com> Date: Fri, 23 Jul 2021 00:54:13 +0200 Subject: [PATCH 1/8] Make Android config optional --- src/targets/__tests__/maven.test.ts | 45 +++++++++++++++++++++++++---- src/targets/maven.ts | 36 +++++++++++++++++------ 2 files changed, 67 insertions(+), 14 deletions(-) diff --git a/src/targets/__tests__/maven.test.ts b/src/targets/__tests__/maven.test.ts index 706aeae4e..647464c51 100644 --- a/src/targets/__tests__/maven.test.ts +++ b/src/targets/__tests__/maven.test.ts @@ -46,7 +46,7 @@ function removeTargetSecretsFromEnv(): void { } } -function getTargetOptions() { +function getAllTargetOptions() { return { OSSRH_USERNAME: DEFAULT_OPTION_VALUE, OSSRH_PASSWORD: DEFAULT_OPTION_VALUE, @@ -63,10 +63,24 @@ function getTargetOptions() { }; } +function getRequiredTargetOptions() { + return { + 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, + mavenRepoUrl: DEFAULT_OPTION_VALUE, + }; +} + function createMavenTarget( targetOptions?: Record ): MavenTarget { - const finalOptions = targetOptions ? targetOptions : getTargetOptions(); + const finalOptions = targetOptions + ? targetOptions + : getRequiredTargetOptions(); const mergedOptions = { name: 'maven', ...finalOptions, @@ -77,9 +91,21 @@ function createMavenTarget( describe('Maven target configuration', () => { beforeEach(() => removeTargetSecretsFromEnv()); - test('with options', () => { + test('with only required options', () => { + setTargetSecretsInEnv(); + const mvnTarget = createMavenTarget(getRequiredTargetOptions()); + targetOptions.map(secret => + expect(mvnTarget.config).toEqual( + expect.objectContaining({ + [secret]: DEFAULT_OPTION_VALUE, + }) + ) + ); + }); + + test('with all options', () => { setTargetSecretsInEnv(); - const mvnTarget = createMavenTarget(getTargetOptions()); + const mvnTarget = createMavenTarget(getAllTargetOptions()); targetOptions.map(secret => expect(mvnTarget.config).toEqual( expect.objectContaining({ @@ -87,9 +113,18 @@ describe('Maven target configuration', () => { }) ) ); + expect(mvnTarget.config.android.distDirRegex).toStrictEqual( + expect.any(String) + ); + expect(mvnTarget.config.android.fileReplaceeRegex).toStrictEqual( + expect.any(String) + ); + expect(mvnTarget.config.android.fileReplacerStr).toStrictEqual( + expect.any(String) + ); }); - test('without options', () => + test('without any options', () => expect(createMavenTarget).toThrowErrorMatchingInlineSnapshot( `"Required value(s) OSSRH_USERNAME not found in configuration files or the environment. See the documentation for more details."` )); diff --git a/src/targets/maven.ts b/src/targets/maven.ts index 4d8213084..155224370 100644 --- a/src/targets/maven.ts +++ b/src/targets/maven.ts @@ -51,7 +51,7 @@ type TargetSettingType = SecretsType | OptionsType; * Config options for the "maven" target. */ export type MavenTargetConfig = Record & - AndroidFields; + Partial; type PartialTargetConfig = { name: string; value: string | undefined }[]; @@ -81,10 +81,24 @@ export class MavenTarget extends BaseTarget { * @returns the maven config for this target. */ private getMavenConfig(): MavenTargetConfig { + const targetSecrets = this.getTargetSecrets(); + const outerTargetSettings = this.getOuterTargetSettings(); + + if (this.config.android) { + const androidSettings = this.getAndroidSettings(); + return { + ...targetSecrets, + ...outerTargetSettings, + ...androidSettings, + }; + } + + this.logger.warn( + 'You may need the Android configuration and it was not found in the configuration file.' + ); return { ...this.getTargetSecrets(), ...this.getOuterTargetSettings(), - ...this.getAndroidSettings(), }; } @@ -401,14 +415,18 @@ export class MavenTarget extends BaseTarget { private getTargetFilename(distDir: string): string { const moduleName = parse(distDir).base; - const isAndroidDistDir = this.mavenConfig.android.distDirRegex.test( - moduleName - ); - return isAndroidDistDir - ? moduleName.replace( + if (this.mavenConfig.android) { + const isAndroidDistDir = this.mavenConfig.android.distDirRegex.test( + moduleName + ); + if (isAndroidDistDir) { + return moduleName.replace( this.mavenConfig.android.fileReplaceeRegex, this.mavenConfig.android.fileReplacerStr - ) - : `${moduleName}.jar`; + ); + } + } + + return `${moduleName}.jar`; } } From 5bb6004085d6bbb496517f60ae93a5811ed9ec66 Mon Sep 17 00:00:00 2001 From: iker barriocanal <32816711+iker-barriocanal@users.noreply.github.com> Date: Fri, 23 Jul 2021 01:00:40 +0200 Subject: [PATCH 2/8] Update README --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 3811a2934..0ae6dbe94 100644 --- a/README.md +++ b/README.md @@ -1010,12 +1010,12 @@ PGP signs and publishes packages to Maven Central. | `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` | Structure containing the data available below. | +| `android` | **optional** Structure containing the data available below. | -The `android` structure contains the following options: +The `android` structure is optional, but not including it when required may result in failing the release or even releasing wrong files. It contains the following options: - `distDirRegex`: pattern of distribution directory names. -- `fileReplaceeRegex` :pattern of substring of distribution module names to be replaced to get the Android distribution file. +- `fileReplaceeRegex`: pattern of substring of distribution module names to be replaced to get the Android distribution file. - `fileReplacerStr`: string to be replaced in the module names to get the Android distribution file. **Example** From e4dc758b79cb9ce6b05ecfca9a80037036f4f3c9 Mon Sep 17 00:00:00 2001 From: iker barriocanal <32816711+iker-barriocanal@users.noreply.github.com> Date: Fri, 23 Jul 2021 01:05:11 +0200 Subject: [PATCH 3/8] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b1617b22..224aba752 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - feat(maven): Add maven target to deploy to Maven Central (#258) - feat(symbol-collector): Add symbol-collector target (#266) - ref(maven): Support BOM files in `maven` target (#270) +- ref(maven): Optional `android` config (#271) ## 0.24.4 From b95af385b7d62b1fbc2bcd6ab9a2ebdd2c5bfb48 Mon Sep 17 00:00:00 2001 From: iker barriocanal <32816711+iker-barriocanal@users.noreply.github.com> Date: Tue, 17 Aug 2021 10:53:31 +0200 Subject: [PATCH 4/8] Make latest changelog entries `feat` --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 224aba752..3f0c495b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,8 @@ - feat(maven): Add maven target to deploy to Maven Central (#258) - feat(symbol-collector): Add symbol-collector target (#266) -- ref(maven): Support BOM files in `maven` target (#270) -- ref(maven): Optional `android` config (#271) +- feat(maven): Support BOM files in `maven` target (#270) +- feat(maven): Optional `android` config (#271) ## 0.24.4 From c0691ef6c134333217608a32a6af8b048bb092e8 Mon Sep 17 00:00:00 2001 From: iker barriocanal <32816711+iker-barriocanal@users.noreply.github.com> Date: Wed, 18 Aug 2021 15:02:53 +0200 Subject: [PATCH 5/8] Set required android config This patch requires an Android config. Should no android config be provided, set `android: false` --- src/targets/maven.ts | 39 +++++++++++++++++---------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/src/targets/maven.ts b/src/targets/maven.ts index 155224370..aeed9113e 100644 --- a/src/targets/maven.ts +++ b/src/targets/maven.ts @@ -38,11 +38,13 @@ export const targetOptions = [ type OptionsType = typeof targetOptions[number]; type AndroidFields = { - android: { - distDirRegex: RegExp; - fileReplaceeRegex: RegExp; - fileReplacerStr: string; - }; + android: + | false + | { + distDirRegex: RegExp; + fileReplaceeRegex: RegExp; + fileReplacerStr: string; + }; }; type TargetSettingType = SecretsType | OptionsType; @@ -51,7 +53,7 @@ type TargetSettingType = SecretsType | OptionsType; * Config options for the "maven" target. */ export type MavenTargetConfig = Record & - Partial; + AndroidFields; type PartialTargetConfig = { name: string; value: string | undefined }[]; @@ -81,24 +83,10 @@ export class MavenTarget extends BaseTarget { * @returns the maven config for this target. */ private getMavenConfig(): MavenTargetConfig { - const targetSecrets = this.getTargetSecrets(); - const outerTargetSettings = this.getOuterTargetSettings(); - - if (this.config.android) { - const androidSettings = this.getAndroidSettings(); - return { - ...targetSecrets, - ...outerTargetSettings, - ...androidSettings, - }; - } - - this.logger.warn( - 'You may need the Android configuration and it was not found in the configuration file.' - ); return { ...this.getTargetSecrets(), ...this.getOuterTargetSettings(), + ...this.getAndroidSettings(), }; } @@ -139,7 +127,14 @@ export class MavenTarget extends BaseTarget { } private getAndroidSettings(): AndroidFields { + if (this.config.android === false) { + return { + android: false, + }; + } + if ( + !this.config.android || !this.config.android.distDirRegex || !this.config.android.fileReplaceeRegex || !this.config.android.fileReplacerStr @@ -415,7 +410,7 @@ export class MavenTarget extends BaseTarget { private getTargetFilename(distDir: string): string { const moduleName = parse(distDir).base; - if (this.mavenConfig.android) { + if (this.mavenConfig.android !== false) { const isAndroidDistDir = this.mavenConfig.android.distDirRegex.test( moduleName ); From d5c09cabd99d85da332aeb149e13791274fbdfbc Mon Sep 17 00:00:00 2001 From: iker barriocanal <32816711+iker-barriocanal@users.noreply.github.com> Date: Thu, 19 Aug 2021 10:18:15 +0200 Subject: [PATCH 6/8] Update tests --- src/targets/__tests__/maven.test.ts | 106 ++++++++++++++++++++++------ src/targets/maven.ts | 2 +- 2 files changed, 87 insertions(+), 21 deletions(-) diff --git a/src/targets/__tests__/maven.test.ts b/src/targets/__tests__/maven.test.ts index 647464c51..d0f6fa717 100644 --- a/src/targets/__tests__/maven.test.ts +++ b/src/targets/__tests__/maven.test.ts @@ -46,7 +46,7 @@ function removeTargetSecretsFromEnv(): void { } } -function getAllTargetOptions() { +function getFullTargetConfig(): any { return { OSSRH_USERNAME: DEFAULT_OPTION_VALUE, OSSRH_PASSWORD: DEFAULT_OPTION_VALUE, @@ -63,7 +63,7 @@ function getAllTargetOptions() { }; } -function getRequiredTargetOptions() { +function getRequiredTargetConfig(): any { return { OSSRH_USERNAME: DEFAULT_OPTION_VALUE, OSSRH_PASSWORD: DEFAULT_OPTION_VALUE, @@ -72,28 +72,99 @@ function getRequiredTargetOptions() { mavenSettingsPath: DEFAULT_OPTION_VALUE, mavenRepoId: DEFAULT_OPTION_VALUE, mavenRepoUrl: DEFAULT_OPTION_VALUE, + android: false, }; } function createMavenTarget( - targetOptions?: Record + targetConfig?: Record ): MavenTarget { - const finalOptions = targetOptions - ? targetOptions - : getRequiredTargetOptions(); - const mergedOptions = { + const finalConfig = targetConfig ? targetConfig : getRequiredTargetConfig(); + const mergedConfig = { name: 'maven', - ...finalOptions, + ...finalConfig, }; - return new MavenTarget(mergedOptions, new NoneArtifactProvider()); + return new MavenTarget(mergedConfig, new NoneArtifactProvider()); } describe('Maven target configuration', () => { - beforeEach(() => removeTargetSecretsFromEnv()); + beforeEach(() => setTargetSecretsInEnv()); - test('with only required options', () => { - setTargetSecretsInEnv(); - const mvnTarget = createMavenTarget(getRequiredTargetOptions()); + afterEach(() => removeTargetSecretsFromEnv()); + + test('no env vars and no options', () => { + removeTargetSecretsFromEnv(); + expect(createMavenTarget).toThrowErrorMatchingInlineSnapshot( + `"Required value(s) OSSRH_USERNAME not found in configuration files or the environment. See the documentation for more details."` + ); + }); + + test('env vars without options', () => { + expect(createMavenTarget.bind(this, {})).toThrowErrorMatchingInlineSnapshot( + `"Required configuration gradleCliPath not found in configuration file. See the documentation for more details."` + ); + }); + + test('no android config', () => { + const config = getRequiredTargetConfig(); + delete config.android; + expect( + createMavenTarget.bind(this, config) + ).toThrowErrorMatchingInlineSnapshot( + `"Required Android configuration is incorrect or was not found in the configuration file. See the documentation for more details."` + ); + }); + + test('incorrect one-line android config', () => { + const config = getRequiredTargetConfig(); + config.android = 'yes'; + expect( + createMavenTarget.bind(this, config) + ).toThrowErrorMatchingInlineSnapshot( + `"Required Android configuration is incorrect or was not found in the configuration file. See the documentation for more details."` + ); + }); + + test('correct one-line android config', () => { + const config = getRequiredTargetConfig(); + const mvnTarget = createMavenTarget(config); + expect(mvnTarget.mavenConfig.android).toStrictEqual(config.android); + }); + + test('incorrect object android config, missing prop', () => { + const config = getFullTargetConfig(); + delete config.android.distDirRegex; + expect( + createMavenTarget.bind(this, config) + ).toThrowErrorMatchingInlineSnapshot( + `"Required Android configuration is incorrect or was not found in the configuration file. See the documentation for more details."` + ); + }); + + test('incorrect object android config, replaced prop', () => { + const config = getFullTargetConfig(); + delete config.android.distDirRegex; + config.android.anotherParam = 'unused'; + expect( + createMavenTarget.bind(this, config) + ).toThrowErrorMatchingInlineSnapshot( + `"Required Android configuration is incorrect or was not found in the configuration file. See the documentation for more details."` + ); + }); + + test('correct object android config, with additional props', () => { + const config = getFullTargetConfig(); + config.android.additionalProp = 'not relevant'; + const mvnTarget = createMavenTarget(config); + const androidConfig: any = mvnTarget.mavenConfig.android; + expect(androidConfig.distDirRegex).toBeDefined(); + expect(androidConfig.fileReplaceeRegex).toBeDefined(); + expect(androidConfig.fileReplacerStr).toBeDefined(); + expect(androidConfig.additionalProp).not.toBeDefined(); + }); + + test('minimum required options', () => { + const mvnTarget = createMavenTarget(getRequiredTargetConfig()); targetOptions.map(secret => expect(mvnTarget.config).toEqual( expect.objectContaining({ @@ -103,9 +174,9 @@ describe('Maven target configuration', () => { ); }); - test('with all options', () => { + test('full target options', () => { setTargetSecretsInEnv(); - const mvnTarget = createMavenTarget(getAllTargetOptions()); + const mvnTarget = createMavenTarget(getFullTargetConfig()); targetOptions.map(secret => expect(mvnTarget.config).toEqual( expect.objectContaining({ @@ -123,11 +194,6 @@ describe('Maven target configuration', () => { expect.any(String) ); }); - - test('without any options', () => - expect(createMavenTarget).toThrowErrorMatchingInlineSnapshot( - `"Required value(s) OSSRH_USERNAME not found in configuration files or the environment. See the documentation for more details."` - )); }); describe('publish', () => { diff --git a/src/targets/maven.ts b/src/targets/maven.ts index aeed9113e..87b65723c 100644 --- a/src/targets/maven.ts +++ b/src/targets/maven.ts @@ -140,7 +140,7 @@ export class MavenTarget extends BaseTarget { !this.config.android.fileReplacerStr ) { throw new ConfigurationError( - 'Required Android configuration not found in configuration file. ' + + 'Required Android configuration is incorrect or was not found in the configuration file. ' + 'See the documentation for more details.' ); } From 2be40a788f72c9b426a810edbfa608af3a3f0e43 Mon Sep 17 00:00:00 2001 From: iker barriocanal <32816711+iker-barriocanal@users.noreply.github.com> Date: Thu, 19 Aug 2021 11:11:00 +0200 Subject: [PATCH 7/8] Update docs --- README.md | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 0ae6dbe94..f9cb56fec 100644 --- a/README.md +++ b/README.md @@ -1010,15 +1010,29 @@ PGP signs and publishes packages to Maven Central. | `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` | **optional** Structure containing the data available below. | +| `android` | Android configuration, see below. | -The `android` structure is optional, but not including it when required may result in failing the release or even releasing wrong files. It contains the following options: +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: - `distDirRegex`: pattern of distribution directory names. - `fileReplaceeRegex`: pattern of substring of distribution module names to be replaced to get the Android distribution file. - `fileReplacerStr`: string to be replaced in the module names to get the Android distribution file. -**Example** +**Example (without Android config)** + +```yaml +targets: + - name: maven + gradleCliPath: ./gradlew + mavenCliPath: scripts/mvnw.cmd + mavenSettingsPath: scripts/settings.xml + mavenRepoId: ossrh + mavenRepoUrl: https://oss.sonatype.org/service/local/staging/deploy/maven2/ + android: false +``` + +**Example (with Android config)** ```yaml targets: From 960a4150afea48c9ccfb9a0ca012fc5f3f8cdfff Mon Sep 17 00:00:00 2001 From: iker barriocanal <32816711+iker-barriocanal@users.noreply.github.com> Date: Fri, 20 Aug 2021 15:02:54 +0200 Subject: [PATCH 8/8] feedback --- src/targets/__tests__/maven.test.ts | 42 +++++++++-------------------- src/targets/maven.ts | 11 +++++--- 2 files changed, 21 insertions(+), 32 deletions(-) diff --git a/src/targets/__tests__/maven.test.ts b/src/targets/__tests__/maven.test.ts index 1613c7f6b..fe14509cc 100644 --- a/src/targets/__tests__/maven.test.ts +++ b/src/targets/__tests__/maven.test.ts @@ -98,7 +98,7 @@ describe('Maven target configuration', () => { }); test('env vars without options', () => { - expect(createMavenTarget.bind(this, {})).toThrowErrorMatchingInlineSnapshot( + expect(() => createMavenTarget({})).toThrowErrorMatchingInlineSnapshot( `"Required configuration gradleCliPath not found in configuration file. See the documentation for more details."` ); }); @@ -106,20 +106,16 @@ describe('Maven target configuration', () => { test('no android config', () => { const config = getRequiredTargetConfig(); delete config.android; - expect( - createMavenTarget.bind(this, config) - ).toThrowErrorMatchingInlineSnapshot( - `"Required Android configuration is incorrect or was not found in the configuration file. See the documentation for more details."` + expect(() => createMavenTarget(config)).toThrowErrorMatchingInlineSnapshot( + `"Required Android configuration was not found in the configuration file. See the documentation for more details"` ); }); test('incorrect one-line android config', () => { const config = getRequiredTargetConfig(); config.android = 'yes'; - expect( - createMavenTarget.bind(this, config) - ).toThrowErrorMatchingInlineSnapshot( - `"Required Android configuration is incorrect or was not found in the configuration file. See the documentation for more details."` + expect(() => createMavenTarget(config)).toThrowErrorMatchingInlineSnapshot( + `"Required Android configuration is incorrect. See the documentation for more details."` ); }); @@ -132,10 +128,8 @@ describe('Maven target configuration', () => { test('incorrect object android config, missing prop', () => { const config = getFullTargetConfig(); delete config.android.distDirRegex; - expect( - createMavenTarget.bind(this, config) - ).toThrowErrorMatchingInlineSnapshot( - `"Required Android configuration is incorrect or was not found in the configuration file. See the documentation for more details."` + expect(() => createMavenTarget(config)).toThrowErrorMatchingInlineSnapshot( + `"Required Android configuration is incorrect. See the documentation for more details."` ); }); @@ -143,10 +137,8 @@ describe('Maven target configuration', () => { const config = getFullTargetConfig(); delete config.android.distDirRegex; config.android.anotherParam = 'unused'; - expect( - createMavenTarget.bind(this, config) - ).toThrowErrorMatchingInlineSnapshot( - `"Required Android configuration is incorrect or was not found in the configuration file. See the documentation for more details."` + expect(() => createMavenTarget(config)).toThrowErrorMatchingInlineSnapshot( + `"Required Android configuration is incorrect. See the documentation for more details."` ); }); @@ -155,9 +147,7 @@ describe('Maven target configuration', () => { config.android.additionalProp = 'not relevant'; const mvnTarget = createMavenTarget(config); const androidConfig: any = mvnTarget.mavenConfig.android; - expect(androidConfig.distDirRegex).toBeDefined(); - expect(androidConfig.fileReplaceeRegex).toBeDefined(); - expect(androidConfig.fileReplacerStr).toBeDefined(); + expect(config.android).toMatchObject(androidConfig); expect(androidConfig.additionalProp).not.toBeDefined(); }); @@ -182,15 +172,9 @@ describe('Maven target configuration', () => { }) ) ); - expect(mvnTarget.config.android.distDirRegex).toStrictEqual( - expect.any(String) - ); - expect(mvnTarget.config.android.fileReplaceeRegex).toStrictEqual( - expect.any(String) - ); - expect(mvnTarget.config.android.fileReplacerStr).toStrictEqual( - expect.any(String) - ); + expect(typeof mvnTarget.config.android.distDirRegex).toBe('string'); + expect(typeof mvnTarget.config.android.fileReplaceeRegex).toBe('string'); + expect(typeof mvnTarget.config.android.fileReplacerStr).toBe('string'); }); }); diff --git a/src/targets/maven.ts b/src/targets/maven.ts index 98f328b1a..27d5ed5fc 100644 --- a/src/targets/maven.ts +++ b/src/targets/maven.ts @@ -133,15 +133,20 @@ export class MavenTarget extends BaseTarget { }; } + if (!this.config.android) { + throw new ConfigurationError( + 'Required Android configuration was not found in the configuration file. ' + + 'See the documentation for more details' + ); + } + if ( - !this.config.android || !this.config.android.distDirRegex || !this.config.android.fileReplaceeRegex || !this.config.android.fileReplacerStr ) { throw new ConfigurationError( - 'Required Android configuration is incorrect or was not found in the configuration file. ' + - 'See the documentation for more details.' + 'Required Android configuration is incorrect. See the documentation for more details.' ); }