Skip to content

Commit 416c5af

Browse files
dshimodshimo
committed
two-pass must be included implemented, unit test
1 parent 8b3cd14 commit 416c5af

3 files changed

Lines changed: 88 additions & 53 deletions

File tree

‎src/liberty/projectDiscovery.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,12 +146,14 @@ async function discoverProjects(
146146
const gradleEntries = allEntries.filter(e => e.type === "gradle");
147147

148148
// ── Phase 2: classify parents ──────────────────────────────────────────
149+
const childParentArtifactIds = mavenUtil.collectChildParentArtifactIds(mavenEntries);
150+
149151
let mavenChildMap: Map<string, string[]> = new Map();
150152
const mavenParentPaths = new Set<string>();
151153
const mavenChildPomPaths = new Set<string>();
152154
for (const entry of mavenEntries) {
153155
if (!entry.xmlString) { continue; }
154-
const validParent = mavenUtil.validParentPom(entry.xmlString);
156+
const validParent = mavenUtil.validParentPom(entry.xmlString, childParentArtifactIds);
155157
console.log(`[discovery] phase2 validParentPom(${entry.path}): isValid=${validParent.isValidBuildFile()} type=${validParent.getProjectType()}`);
156158
if (validParent.isValidBuildFile()) {
157159
const childModules = mavenUtil.findChildMavenModules(entry.xmlString);
@@ -202,7 +204,7 @@ async function discoverProjects(
202204
const isParent = mavenParentPaths.has(entry.path);
203205
const isChild = mavenChildPomPaths.has(entry.path);
204206
if (isParent) {
205-
buildFile = mavenUtil.validParentPom(entry.xmlString);
207+
buildFile = mavenUtil.validParentPom(entry.xmlString, childParentArtifactIds);
206208
} else if (isChild) {
207209
buildFile = new BuildFileImpl(true, LIBERTY_PROJECT_MAVEN);
208210
buildFile.setBuildFilePath(entry.path);

‎src/test/unit/mavenDiscovery.test.ts‎

Lines changed: 62 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,25 @@
11
/*
2-
* Unit tests for Maven multi-module discovery — full truth table coverage.
2+
* Unit tests for Maven multi-module discovery.
3+
* Plain mocha + chai, no VS Code instance required.
34
*
45
* Variables:
5-
* Parent Liberty — parent pom declares the liberty-maven-plugin
6-
* Child Liberty — child pom declares the liberty-maven-plugin
7-
* Parent <modules> — parent pom lists the child in <modules>
8-
* Child <parent> — child pom declares <parent> referencing the parent's artifactId
6+
* PL = Parent is Liberty enabled
7+
* CL = Child is Liberty enabled
8+
* PM = Parent -> Child: Parent lists child in <modules>
9+
* CP = Parent <- Child: Child declares <parent>
10+
*
11+
* Group | PL | CL | PM | CP | Expected
12+
* -------|----|----|----|----|-----------------------------------------
13+
* 1 | Y | Y | Y | Y | Parent aggregator, child actionable
14+
* 2 | Y | N | Y | Y | Parent aggregator, child inherits Liberty
15+
* 3 | N | Y | Y | Y | Parent display-only aggregator, child actionable
16+
* 4 | N | N | Y | Y | Both excluded (no Liberty anywhere)
17+
* 5 | Y | N | Y | N | No link. Parent standalone, child excluded
18+
* 6 | N | Y | N | Y | No link. Child standalone, parent excluded
919
*
10-
* # | Par Lib | Chd Lib | Par <mod> | Chd <par> | Expected behavior
11-
* ----|---------|---------|-----------|-----------|--------------------------------------------------
12-
* 1 | Y | Y | Y | Y | Both valid. Parent aggregator, child actionable
13-
* 2 | Y | Y | Y | N | One-directional. Parent standalone (aggregator, no actions), child standalone
14-
* 3 | Y | Y | N | Y | One-directional. Parent standalone (aggregator, no actions), child standalone
15-
* 4 | Y | Y | N | N | No relationship. Both standalone
16-
* 5 | Y | N | Y | Y | Parent aggregator. Child valid via Liberty inheritance from parent
17-
* 6 | Y | N | Y | N | One-directional. Parent standalone (aggregator, no actions), child excluded
18-
* 7 | Y | N | N | Y | One-directional. Parent standalone (aggregator, no actions), child excluded
19-
* 8 | Y | N | N | N | No relationship. Parent standalone (aggregator, no actions), child excluded
20-
* 9 | N | Y | Y | Y | Parent display-only aggregator. Child actionable
21-
* 10 | N | Y | Y | N | One-directional. Child standalone, parent excluded
22-
* 11 | N | Y | N | Y | One-directional. Child standalone, parent excluded
23-
* 12 | N | Y | N | N | No relationship. Child standalone, parent excluded
24-
* 13 | N | N | Y | Y | No Liberty anywhere. Both excluded
25-
* 14 | N | N | Y | N | No Liberty anywhere. Both excluded
26-
* 15 | N | N | N | Y | No Liberty anywhere. Both excluded
27-
* 16 | N | N | N | N | No Liberty anywhere. Both excluded
2820
*/
2921
import { strict as assert } from "assert";
30-
import { validParentPom, validPom } from "../../util/mavenUtil";
22+
import { validParentPom, validPom, collectChildParentArtifactIds } from "../../util/mavenUtil";
3123
import { LIBERTY_PROJECT_MAVEN } from "../../definitions/constants";
3224

3325
// ── Fixture builders ──────────────────────────────────────────────────────────
@@ -77,19 +69,20 @@ function makeChildPom(hasLiberty: boolean, hasParentRef: boolean): string {
7769
const CHILDREN_MAP_WITH_CHILD = new Map([["my-parent", ["child-module"]]]);
7870
const EMPTY_CHILDREN_MAP = new Map<string, string[]>();
7971

80-
// childParentArtifactIds used by validParentPom — set of parentArtifactIds declared by children
81-
const CHILD_REFERENCES_PARENT = new Set(["my-parent"]);
82-
const NO_CHILD_REFERENCES = new Set<string>();
83-
8472
// ── Group 1: Bidirectional + both Liberty (row 1) ─────────────────────────────
8573
// Parent aggregator, child actionable
8674

8775
describe("row 1 — bidirectional, both have Liberty", () => {
8876
const parentXml = makeParentPom(true, true);
8977
const childXml = makeChildPom(true, true);
78+
const childParentArtifactIds = collectChildParentArtifactIds([{ xmlString: childXml }]);
79+
80+
it("pass 1 collects child's parent reference", () => {
81+
assert.ok(childParentArtifactIds.has("my-parent"));
82+
});
9083

9184
it("parent is valid aggregator", () => {
92-
const result = validParentPom(parentXml, CHILD_REFERENCES_PARENT);
85+
const result = validParentPom(parentXml, childParentArtifactIds);
9386
assert.equal(result.isValidBuildFile(), true);
9487
assert.equal(result.getProjectType(), LIBERTY_PROJECT_MAVEN);
9588
});
@@ -107,9 +100,14 @@ describe("row 1 — bidirectional, both have Liberty", () => {
107100
describe("row 5 — bidirectional, parent has Liberty, child does not", () => {
108101
const parentXml = makeParentPom(true, true);
109102
const childXml = makeChildPom(false, true);
103+
const childParentArtifactIds = collectChildParentArtifactIds([{ xmlString: childXml }]);
104+
105+
it("pass 1 collects child's parent reference", () => {
106+
assert.ok(childParentArtifactIds.has("my-parent"));
107+
});
110108

111109
it("parent is valid aggregator", () => {
112-
const result = validParentPom(parentXml, CHILD_REFERENCES_PARENT);
110+
const result = validParentPom(parentXml, childParentArtifactIds);
113111
assert.equal(result.isValidBuildFile(), true);
114112
assert.equal(result.getProjectType(), LIBERTY_PROJECT_MAVEN);
115113
});
@@ -127,9 +125,14 @@ describe("row 5 — bidirectional, parent has Liberty, child does not", () => {
127125
describe("row 9 — bidirectional, child has Liberty, parent does not", () => {
128126
const parentXml = makeParentPom(false, true);
129127
const childXml = makeChildPom(true, true);
128+
const childParentArtifactIds = collectChildParentArtifactIds([{ xmlString: childXml }]);
129+
130+
it("pass 1 collects child's parent reference", () => {
131+
assert.ok(childParentArtifactIds.has("my-parent"));
132+
});
130133

131134
it("parent is valid aggregator despite no Liberty plugin", () => {
132-
const result = validParentPom(parentXml, CHILD_REFERENCES_PARENT);
135+
const result = validParentPom(parentXml, childParentArtifactIds);
133136
assert.equal(result.isValidBuildFile(), true);
134137
assert.equal(result.getProjectType(), LIBERTY_PROJECT_MAVEN);
135138
});
@@ -142,36 +145,40 @@ describe("row 9 — bidirectional, child has Liberty, parent does not", () => {
142145
});
143146

144147
// ── Group 4: Bidirectional + no Liberty (row 13) ─────────────────────────────
145-
// Both excluded
148+
// Both structurally linked but no Liberty anywhere — both excluded via hasLibertyDescendants
146149

147150
describe("row 13 — bidirectional, neither has Liberty", () => {
148151
const parentXml = makeParentPom(false, true);
149152
const childXml = makeChildPom(false, true);
153+
const childParentArtifactIds = collectChildParentArtifactIds([{ xmlString: childXml }]);
154+
155+
it("pass 1 collects child's parent reference", () => {
156+
assert.ok(childParentArtifactIds.has("my-parent"));
157+
});
150158

151-
it("parent is invalid — no Liberty anywhere", () => {
152-
// childParentArtifactIds is populated but child has no Liberty — parent still invalid
153-
// because bidirectional check qualifies the structure, but hasLibertyDescendants
154-
// will filter it from rootProjects. validParentPom itself returns valid for structure,
155-
// but child validPom returns false — tested together this confirms exclusion.
159+
it("parent passes structural check but has no Liberty descendants — excluded by hasLibertyDescendants in linkProjects", () => {
160+
const parentResult = validParentPom(parentXml, childParentArtifactIds);
161+
assert.equal(parentResult.isValidBuildFile(), true); // structurally valid
156162
const childResult = validPom(childXml, CHILDREN_MAP_WITH_CHILD);
157-
assert.equal(childResult.isValidBuildFile(), true); // child in childrenMap → structurally valid
158-
// parent passes structural check but no Liberty child → excluded by hasLibertyDescendants
159-
const parentResult = validParentPom(parentXml, CHILD_REFERENCES_PARENT);
160-
assert.equal(parentResult.isValidBuildFile(), true); // structurally valid aggregator
161-
// NOTE: exclusion from tree happens in linkProjects via hasLibertyDescendants,
162-
// not at validParentPom level. Both isLibertyEnabled=false → filtered from rootProjects.
163+
assert.equal(childResult.isValidBuildFile(), true); // structurally valid
164+
// both isLibertyEnabled=false → hasLibertyDescendants=false → filtered from rootProjects
163165
});
164166
});
165167

166-
// ── Group 5: One-directional — parent has Liberty, child <parent> missing (row 6) ─
167-
// Parent standalone (aggregator with no Liberty children), child excluded
168+
// ── Group 5: One-directional, parent has Liberty, child <parent> missing (row 6) ─
169+
// Parent standalone (aggregator, no actions), child excluded
168170

169171
describe("row 6 — parent has Liberty + <modules>, child has no <parent> ref", () => {
170172
const parentXml = makeParentPom(true, true);
171173
const childXml = makeChildPom(false, false);
174+
const childParentArtifactIds = collectChildParentArtifactIds([{ xmlString: childXml }]);
172175

173-
it("parent is valid via Liberty plugin (existing path)", () => {
174-
const result = validParentPom(parentXml, NO_CHILD_REFERENCES);
176+
it("pass 1 collects no parent references — link is broken", () => {
177+
assert.equal(childParentArtifactIds.size, 0);
178+
});
179+
180+
it("parent is valid via Liberty plugin (existing path, no bidirectional check needed)", () => {
181+
const result = validParentPom(parentXml, childParentArtifactIds);
175182
assert.equal(result.isValidBuildFile(), true);
176183
assert.equal(result.getProjectType(), LIBERTY_PROJECT_MAVEN);
177184
});
@@ -182,20 +189,24 @@ describe("row 6 — parent has Liberty + <modules>, child has no <parent> ref",
182189
});
183190
});
184191

185-
// ── Group 6: One-directional — child has Liberty, parent <modules> missing (row 11) ─
192+
// ── Group 6: One-directional, child has Liberty, parent <modules> missing (row 11) ─
186193
// Child standalone, parent excluded
187194

188195
describe("row 11 — child has Liberty + <parent>, parent has no <modules>", () => {
189196
const parentXml = makeParentPom(false, false);
190197
const childXml = makeChildPom(true, true);
198+
const childParentArtifactIds = collectChildParentArtifactIds([{ xmlString: childXml }]);
199+
200+
it("pass 1 collects child's parent reference", () => {
201+
assert.ok(childParentArtifactIds.has("my-parent"));
202+
});
191203

192-
it("parent is invalid — no Liberty, no <modules>", () => {
193-
const result = validParentPom(parentXml, CHILD_REFERENCES_PARENT);
204+
it("parent is invalid — no Liberty, no <modules> to satisfy bidirectional check", () => {
205+
const result = validParentPom(parentXml, childParentArtifactIds);
194206
assert.equal(result.isValidBuildFile(), false);
195207
});
196208

197209
it("child is valid standalone Liberty project", () => {
198-
// Not in childrenMap (no structural link), but has Liberty plugin directly
199210
const result = validPom(childXml, EMPTY_CHILDREN_MAP);
200211
assert.equal(result.isValidBuildFile(), true);
201212
});

‎src/util/mavenUtil.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,28 @@ function containerVersion(plugin: any): boolean {
204204
return false;
205205
}
206206

207+
/**
208+
* Pass 1 of the two-pass aggregator detection.
209+
* Iterates all parsed Maven entries and collects every <parent><artifactId> declared.
210+
* The resulting Set is passed to validParentPom() in pass 2 to enable the
211+
* bidirectional aggregator check without O(n²) cross-referencing.
212+
*
213+
* @param entries Array of objects carrying xmlString for each pom.xml
214+
*/
215+
export function collectChildParentArtifactIds(entries: Array<{ xmlString?: string }>): Set<string> {
216+
const parseString = require("xml2js").parseString;
217+
const result = new Set<string>();
218+
for (const entry of entries) {
219+
if (!entry.xmlString) { continue; }
220+
parseString(entry.xmlString, (err: any, parsed: any) => {
221+
if (err || !parsed?.project?.parent) { return; }
222+
const parentArtifactId = parsed.project.parent[0]?.artifactId?.[0];
223+
if (parentArtifactId) { result.add(parentArtifactId); }
224+
});
225+
}
226+
return result;
227+
}
228+
207229
/**
208230
* Interface for Maven project metadata used in multi-module hierarchy
209231
*/

0 commit comments

Comments
 (0)