Fix bug in child pom versions not being updated

Author: ryanjbaxter

.github/actions/update-project-versions/__tests__/index.test.js

@@ -446,6 +446,56 @@ describe('updatePomFile', () => {
     expect(parentBlock).toContain('<version>4.1.1</version>');
   });
 
+  it('updates parent version in a deep sub-module whose parent is an intermediate aggregate pom', () => {
+    // Regression test for spring-cloud-function-adapters:
+    // spring-cloud-function-adapter-aws has parent spring-cloud-function-adapter-parent,
+    // which is itself a child of the root spring-cloud-function-parent.
+    // Without internalArtifactIds the intermediate parent name does not match the root,
+    // so the deep sub-module's parent version was left unchanged.
+    const srcDir = fixturePath('maven-multi-level');
+    const destRoot = path.join(tmpDir, 'root');
+    const destAdapters = path.join(destRoot, 'spring-cloud-function-adapters');
+    const destAws = path.join(destAdapters, 'spring-cloud-function-adapter-aws');
+    fs.mkdirSync(destRoot, { recursive: true });
+    fs.mkdirSync(destAdapters, { recursive: true });
+    fs.mkdirSync(destAws, { recursive: true });
+
+    fs.copyFileSync(path.join(srcDir, 'pom.xml'), path.join(destRoot, 'pom.xml'));
+    fs.copyFileSync(path.join(srcDir, 'spring-cloud-function-adapters', 'pom.xml'), path.join(destAdapters, 'pom.xml'));
+    fs.copyFileSync(path.join(srcDir, 'spring-cloud-function-adapters', 'spring-cloud-function-adapter-aws', 'pom.xml'), path.join(destAws, 'pom.xml'));
+
+    const rootPom     = path.join(destRoot, 'pom.xml');
+    const adaptersPom = path.join(destAdapters, 'pom.xml');
+    const awsPom      = path.join(destAws, 'pom.xml');
+
+    const internalIds = new Set(['spring-cloud-function-parent', 'spring-cloud-function-adapter-parent', 'spring-cloud-function-adapter-aws']);
+    const versionMap  = { 'spring-cloud-build': '4.2.1', 'spring-boot': '3.4.2' };
+    const currentRoot = '5.0.3';
+    const newVersion  = '5.0.3.1-SNAPSHOT';
+
+    updatePomFile(rootPom,     true,  newVersion, versionMap, currentRoot, 'spring-cloud-function-parent', internalIds);
+    updatePomFile(adaptersPom, false, newVersion, versionMap, currentRoot, 'spring-cloud-function-parent', internalIds);
+    updatePomFile(awsPom,      false, newVersion, versionMap, currentRoot, 'spring-cloud-function-parent', internalIds);
+
+    // Root: own version updated
+    const rootContent = fs.readFileSync(rootPom, 'utf-8');
+    expect(rootContent).toMatch(/<artifactId>spring-cloud-function-parent<\/artifactId>\s*<version>5\.0\.3\.1-SNAPSHOT<\/version>/);
+    // Root: spring-cloud-build parent version updated from versions map
+    expect(rootContent).toMatch(/<parent>[\s\S]*?<version>4\.2\.1<\/version>[\s\S]*?<\/parent>/);
+
+    // Intermediate aggregate: no own <version>, but parent version updated
+    const adaptersContent = fs.readFileSync(adaptersPom, 'utf-8');
+    const adaptersParent = adaptersContent.match(/<parent>[\s\S]*?<\/parent>/)[0];
+    expect(adaptersParent).toContain('<version>5.0.3.1-SNAPSHOT</version>');
+
+    // Deep sub-module: parent (spring-cloud-function-adapter-parent) version updated
+    const awsContent = fs.readFileSync(awsPom, 'utf-8');
+    const awsParent = awsContent.match(/<parent>[\s\S]*?<\/parent>/)[0];
+    expect(awsParent).toContain('<version>5.0.3.1-SNAPSHOT</version>');
+    // spring-boot.version property also updated
+    expect(awsContent).toContain('<spring-boot.version>3.4.2</spring-boot.version>');
+  });
+
 });
 
 // ── isChildOfRoot ─────────────────────────────────────────────────────────────
@@ -488,6 +538,24 @@ describe('isChildOfRoot', () => {
   it('returns false with no parent element', () => {
     expect(isChildOfRoot({}, {}, 'spring-cloud-config')).toBe(false);
   });
+
+  it('returns true when parent is an intermediate project pom in internalArtifactIds', () => {
+    // Mirrors spring-cloud-function-adapters case: the leaf adapter has
+    // spring-cloud-function-adapter-parent (not the root) as its parent.
+    const internal = new Set(['spring-cloud-function-parent', 'spring-cloud-function-adapter-parent']);
+    expect(isChildOfRoot(makeProject('spring-cloud-function-adapter-parent'), {}, null, internal)).toBe(true);
+  });
+
+  it('returns false when parent is external even though versions map is empty (internalArtifactIds provided)', () => {
+    const internal = new Set(['spring-cloud-function-parent', 'spring-cloud-function-adapter-parent']);
+    expect(isChildOfRoot(makeProject('spring-boot-dependencies'), {}, null, internal)).toBe(false);
+  });
+
+  it('internalArtifactIds takes precedence over rootArtifactId', () => {
+    // rootArtifactId alone would return false for adapter-parent, but internalArtifactIds knows it's internal
+    const internal = new Set(['spring-cloud-function-parent', 'spring-cloud-function-adapter-parent']);
+    expect(isChildOfRoot(makeProject('spring-cloud-function-adapter-parent'), {}, 'spring-cloud-function-parent', internal)).toBe(true);
+  });
 });
 
 // ── findFiles ──────────────────────────────────────────────────────────────────

.github/actions/update-project-versions/dist/index.js

@@ -28320,6 +28320,19 @@ async function run() {
           : null;
       }
 
+      // Pre-scan all pom files to collect every artifactId that belongs to this
+      // project. This lets isChildOfRoot recognise intermediate aggregate parent poms
+      // (e.g. spring-cloud-function-adapter-parent inside spring-cloud-function-adapters/)
+      // so that their child poms have their parent version updated correctly.
+      const internalArtifactIds = new Set();
+      if (rootArtifactId) internalArtifactIds.add(rootArtifactId);
+      const pomParser = new XMLParser({ ignoreAttributes: false });
+      for (const file of pomFiles) {
+        const parsed = pomParser.parse(fs.readFileSync(file, 'utf-8'));
+        const aid = parsed?.project?.artifactId;
+        if (aid) internalArtifactIds.add(String(aid));
+      }
+
       for (const file of pomFiles) {
         const isRoot = path.resolve(file) === path.resolve(rootPom);
         const { changed, updatedProperties } = updatePomFile(
@@ -28328,7 +28341,8 @@ async function run() {
           projectVersion,
           versions,
           currentRootVersion,
-          rootArtifactId
+          rootArtifactId,
+          internalArtifactIds
         );
         if (changed) {
           core.info(`Updated ${path.relative(directory, file)}: ${updatedProperties.join(', ')}`);
@@ -28516,8 +28530,10 @@ function detectProjectName(directory) {
  * @param {Record<string, string>} versions
  * @param {string|null} currentRootVersion - the root pom's version before any edits;
  *   non-root poms whose own <version> equals this value will also have it updated
+ * @param {Set<string>|null} internalArtifactIds - set of all artifactIds found in
+ *   this project's pom files; used to recognise intermediate parent poms as internal
  */
-function updatePomFile(filePath, isRoot, projectVersion, versions, currentRootVersion = null, rootArtifactId = null) {
+function updatePomFile(filePath, isRoot, projectVersion, versions, currentRootVersion = null, rootArtifactId = null, internalArtifactIds = null) {
   const content = fs.readFileSync(filePath, 'utf-8');
   let updated = content;
   const updatedProperties = [];
@@ -28565,7 +28581,7 @@ function updatePomFile(filePath, isRoot, projectVersion, versions, currentRootVe
         ? versions[parentArtifactId]
         : versions[parentName] !== undefined
         ? versions[parentName]
-      : isChildOfRoot(project, versions, rootArtifactId)
+      : isChildOfRoot(project, versions, rootArtifactId, internalArtifactIds)
       ? projectVersion
         : null;
 
@@ -28815,20 +28831,26 @@ function artifactIdToProjectName(artifactId) {
 }
 
 /**
- * Returns true when a child pom's parent is the root project of this repo
+ * Returns true when a child pom's parent is part of this project
  * (i.e. not an external parent like spring-boot-starter-parent or
  * spring-cloud-dependencies-parent).
  *
- * When rootArtifactId is supplied (read from the root pom at runtime) the parent
- * must both (a) be absent from the external versions map AND (b) match the root
- * artifactId or its stripped form. This prevents external parents that happen to
- * be absent from the versions map (e.g. spring-boot-starter-parent when versions
- * is empty) from being incorrectly stamped with the project version.
+ * When internalArtifactIds is supplied (a Set of every artifactId found in the
+ * project's pom files) the parent must be a member of that set. This correctly
+ * handles multi-level module hierarchies where an intermediate aggregate pom
+ * (e.g. spring-cloud-function-adapter-parent) is itself a parent of deeper
+ * sub-modules — those sub-modules would be missed when only comparing against
+ * the root artifactId.
  *
- * When rootArtifactId is null (e.g. in unit tests that don't supply it) the
- * function falls back to the looser heuristic of "not in versions map".
+ * When only rootArtifactId is supplied the parent must match the root artifactId
+ * or its stripped form. This prevents external parents absent from the versions
+ * map (e.g. spring-boot-starter-parent when versions is empty) from being
+ * incorrectly stamped with the project version.
+ *
+ * When neither is supplied the function falls back to the looser heuristic of
+ * "not in versions map" (used only in unit tests that don't supply either).
  */
-function isChildOfRoot(project, versions, rootArtifactId = null) {
+function isChildOfRoot(project, versions, rootArtifactId = null, internalArtifactIds = null) {
   const parentArtifactId = project?.parent?.artifactId;
   if (!parentArtifactId) return false;
   const parentName = artifactIdToProjectName(parentArtifactId);
@@ -28837,6 +28859,10 @@ function isChildOfRoot(project, versions, rootArtifactId = null) {
     return false;
   }
 
+  if (internalArtifactIds !== null) {
+    return internalArtifactIds.has(parentArtifactId) || internalArtifactIds.has(parentName);
+  }
+
   if (rootArtifactId !== null) {
     const rootName = artifactIdToProjectName(rootArtifactId);
     return (

.github/actions/update-project-versions/src/index.js

@@ -95,6 +95,19 @@ async function run() {
           : null;
       }
 
+      // Pre-scan all pom files to collect every artifactId that belongs to this
+      // project. This lets isChildOfRoot recognise intermediate aggregate parent poms
+      // (e.g. spring-cloud-function-adapter-parent inside spring-cloud-function-adapters/)
+      // so that their child poms have their parent version updated correctly.
+      const internalArtifactIds = new Set();
+      if (rootArtifactId) internalArtifactIds.add(rootArtifactId);
+      const pomParser = new XMLParser({ ignoreAttributes: false });
+      for (const file of pomFiles) {
+        const parsed = pomParser.parse(fs.readFileSync(file, 'utf-8'));
+        const aid = parsed?.project?.artifactId;
+        if (aid) internalArtifactIds.add(String(aid));
+      }
+
       for (const file of pomFiles) {
         const isRoot = path.resolve(file) === path.resolve(rootPom);
         const { changed, updatedProperties } = updatePomFile(
@@ -103,7 +116,8 @@ async function run() {
           projectVersion,
           versions,
           currentRootVersion,
-          rootArtifactId
+          rootArtifactId,
+          internalArtifactIds
         );
         if (changed) {
           core.info(`Updated ${path.relative(directory, file)}: ${updatedProperties.join(', ')}`);
@@ -291,8 +305,10 @@ function detectProjectName(directory) {
  * @param {Record<string, string>} versions
  * @param {string|null} currentRootVersion - the root pom's version before any edits;
  *   non-root poms whose own <version> equals this value will also have it updated
+ * @param {Set<string>|null} internalArtifactIds - set of all artifactIds found in
+ *   this project's pom files; used to recognise intermediate parent poms as internal
  */
-function updatePomFile(filePath, isRoot, projectVersion, versions, currentRootVersion = null, rootArtifactId = null) {
+function updatePomFile(filePath, isRoot, projectVersion, versions, currentRootVersion = null, rootArtifactId = null, internalArtifactIds = null) {
   const content = fs.readFileSync(filePath, 'utf-8');
   let updated = content;
   const updatedProperties = [];
@@ -340,7 +356,7 @@ function updatePomFile(filePath, isRoot, projectVersion, versions, currentRootVe
         ? versions[parentArtifactId]
         : versions[parentName] !== undefined
         ? versions[parentName]
-      : isChildOfRoot(project, versions, rootArtifactId)
+      : isChildOfRoot(project, versions, rootArtifactId, internalArtifactIds)
       ? projectVersion
         : null;
 
@@ -590,20 +606,26 @@ function artifactIdToProjectName(artifactId) {
 }
 
 /**
- * Returns true when a child pom's parent is the root project of this repo
+ * Returns true when a child pom's parent is part of this project
  * (i.e. not an external parent like spring-boot-starter-parent or
  * spring-cloud-dependencies-parent).
  *
- * When rootArtifactId is supplied (read from the root pom at runtime) the parent
- * must both (a) be absent from the external versions map AND (b) match the root
- * artifactId or its stripped form. This prevents external parents that happen to
- * be absent from the versions map (e.g. spring-boot-starter-parent when versions
- * is empty) from being incorrectly stamped with the project version.
+ * When internalArtifactIds is supplied (a Set of every artifactId found in the
+ * project's pom files) the parent must be a member of that set. This correctly
+ * handles multi-level module hierarchies where an intermediate aggregate pom
+ * (e.g. spring-cloud-function-adapter-parent) is itself a parent of deeper
+ * sub-modules — those sub-modules would be missed when only comparing against
+ * the root artifactId.
  *
- * When rootArtifactId is null (e.g. in unit tests that don't supply it) the
- * function falls back to the looser heuristic of "not in versions map".
+ * When only rootArtifactId is supplied the parent must match the root artifactId
+ * or its stripped form. This prevents external parents absent from the versions
+ * map (e.g. spring-boot-starter-parent when versions is empty) from being
+ * incorrectly stamped with the project version.
+ *
+ * When neither is supplied the function falls back to the looser heuristic of
+ * "not in versions map" (used only in unit tests that don't supply either).
  */
-function isChildOfRoot(project, versions, rootArtifactId = null) {
+function isChildOfRoot(project, versions, rootArtifactId = null, internalArtifactIds = null) {
   const parentArtifactId = project?.parent?.artifactId;
   if (!parentArtifactId) return false;
   const parentName = artifactIdToProjectName(parentArtifactId);
@@ -612,6 +634,10 @@ function isChildOfRoot(project, versions, rootArtifactId = null) {
     return false;
   }
 
+  if (internalArtifactIds !== null) {
+    return internalArtifactIds.has(parentArtifactId) || internalArtifactIds.has(parentName);
+  }
+
   if (rootArtifactId !== null) {
     const rootName = artifactIdToProjectName(rootArtifactId);
     return (