fix(maven-workspace): SNAPSHOT versions should not be included in the…

… manifest (googleapis#1727)

* test: failing test for updating non-release versions in manifest

* fix: SNAPSHOT versions should not be included in the manifest

src/plugins/maven-workspace.ts

@@ -248,7 +248,10 @@ export class MavenWorkspace extends WorkspacePlugin<MavenArtifact> {
           this.logger.warn(`${pomUpdate.path} does not have cached contents`);
         }
       }
-      if (candidate.pullRequest.version) {
+      if (
+        candidate.pullRequest.version &&
+        this.isReleaseVersion(candidate.pullRequest.version)
+      ) {
         updatedPathVersions.set(candidate.path, candidate.pullRequest.version);
       }
     }
@@ -269,7 +272,9 @@ export class MavenWorkspace extends WorkspacePlugin<MavenArtifact> {
         } else {
           this.logger.debug(`version: ${version} forced bump`);
           updatedVersions.set(packageName, version);
-          updatedPathVersions.set(this.pathFromPackage(pkg), version);
+          if (this.isReleaseVersion(version)) {
+            updatedPathVersions.set(this.pathFromPackage(pkg), version);
+          }
         }
       }
     }
@@ -310,6 +315,16 @@ export class MavenWorkspace extends WorkspacePlugin<MavenArtifact> {
     return graph;
   }
 
+  /**
+   * Given a release version, determine if we should bump the manifest
+   * version as well. For maven artifacts, SNAPSHOT versions are not
+   * considered releases.
+   * @param {Version} version The release version
+   */
+  protected isReleaseVersion(version: Version): boolean {
+    return !version.preRelease?.includes('SNAPSHOT');
+  }
+
   protected bumpVersion(artifact: MavenArtifact): Version {
     const strategy = new JavaSnapshot(new AlwaysBumpPatch());
     return strategy.bump(Version.parse(artifact.version), [FAKE_COMMIT]);

src/plugins/workspace.ts

@@ -266,7 +266,9 @@ export abstract class WorkspacePlugin<T> extends ManifestPlugin {
         const version = this.bumpVersion(pkg);
         this.logger.debug(`version: ${version} forced bump`);
         updatedVersions.set(packageName, version);
-        updatedPathVersions.set(this.pathFromPackage(pkg), version);
+        if (this.isReleaseVersion(version)) {
+          updatedPathVersions.set(this.pathFromPackage(pkg), version);
+        }
       }
     }
     return {
@@ -275,6 +277,15 @@ export abstract class WorkspacePlugin<T> extends ManifestPlugin {
     };
   }
 
+  /**
+   * Given a release version, determine if we should bump the manifest
+   * version as well.
+   * @param {Version} _version The release version
+   */
+  protected isReleaseVersion(_version: Version): boolean {
+    return true;
+  }
+
   /**
    * Given a package, return the new bumped version after updating
    * the dependency.

test/plugins/maven-workspace.ts

@@ -31,6 +31,7 @@ import {Update} from '../../src/update';
 import {Version} from '../../src/version';
 import {PomXml} from '../../src/updaters/java/pom-xml';
 import {RawContent} from '../../src/updaters/raw-content';
+import {ReleasePleaseManifest} from '../../src/updaters/release-please-manifest';
 
 const sandbox = sinon.createSandbox();
 const fixturesPath = './test/fixtures/plugins/maven-workspace';
@@ -403,6 +404,100 @@ describe('MavenWorkspace plugin', () => {
       );
       safeSnapshot(await renderUpdate(github, bomUpdate));
     });
+    it('skips updating manifest with snapshot versions', async () => {
+      plugin = new MavenWorkspace(github, 'main', {
+        bom: {
+          releaseType: 'maven',
+        },
+        maven1: {
+          releaseType: 'maven',
+        },
+        maven2: {
+          releaseType: 'maven',
+        },
+        maven3: {
+          releaseType: 'maven',
+        },
+        maven4: {
+          releaseType: 'maven',
+        },
+      });
+      sandbox
+        .stub(github, 'findFilesByFilenameAndRef')
+        .withArgs('pom.xml', 'main')
+        .resolves([
+          'maven1/pom.xml',
+          'maven2/pom.xml',
+          'maven3/pom.xml',
+          'maven4/pom.xml',
+        ]);
+      const candidates: CandidateReleasePullRequest[] = [
+        buildMockCandidatePullRequest('maven1', 'maven', '1.1.2-SNAPSHOT', {
+          component: 'maven1',
+          updates: [
+            buildMockPackageUpdate(
+              'maven1/pom.xml',
+              'maven1/pom.xml',
+              '1.1.2-SNAPSHOT'
+            ),
+          ],
+        }),
+        buildMockCandidatePullRequest('maven2', 'maven', '2.2.3-SNAPSHOT', {
+          component: 'maven2',
+          updates: [
+            buildMockPackageUpdate(
+              'maven2/pom.xml',
+              'maven2/pom.xml',
+              '2.2.3-SNAPSHOT'
+            ),
+          ],
+        }),
+        buildMockCandidatePullRequest('maven3', 'maven', '3.3.4-SNAPSHOT', {
+          component: 'maven3',
+          updates: [
+            buildMockPackageUpdate(
+              'maven3/pom.xml',
+              'maven3/pom.xml',
+              '3.3.4-SNAPSHOT'
+            ),
+          ],
+        }),
+        buildMockCandidatePullRequest('maven4', 'maven', '4.4.5-SNAPSHOT', {
+          component: 'maven4',
+          updates: [
+            buildMockPackageUpdate(
+              'maven4/pom.xml',
+              'maven4/pom.xml',
+              '4.4.5-SNAPSHOT'
+            ),
+          ],
+        }),
+      ];
+      stubFilesFromFixtures({
+        sandbox,
+        github,
+        fixturePath: fixturesPath,
+        files: [
+          'maven1/pom.xml',
+          'maven2/pom.xml',
+          'maven3/pom.xml',
+          'maven4/pom.xml',
+        ],
+        flatten: false,
+        targetBranch: 'main',
+      });
+      const newCandidates = await plugin.run(candidates);
+      expect(newCandidates).length(1);
+      const update = assertHasUpdate(
+        newCandidates[0].pullRequest.updates,
+        '.release-please-manifest.json',
+        ReleasePleaseManifest
+      );
+      const updater = update.updater as ReleasePleaseManifest;
+      for (const [_, version] of updater.versionsMap!) {
+        expect(version.toString()).to.not.include('SNAPSHOT');
+      }
+    });
   });
 });