Include XML parsing example in the Phone Bill archetype.

[DavidWhitlock]

No description provided.

[Copilot]

Pull request overview

This PR adds XML parsing capabilities to the Phone Bill archetype by introducing example XML files, a helper class for XML processing, and corresponding tests that demonstrate DTD validation.

• Adds PhoneBillXmlHelper class that extends ProjectXmlHelper for XML parsing with DTD validation
• Includes valid and invalid XML test resources to demonstrate parsing behavior
• Adds comprehensive test coverage in PhoneBillXmlHelperTest for both valid and invalid XML scenarios

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
valid-artifactId.xml	Provides a valid XML example with two phone call records for testing successful parsing
invalid-artifactId.xml	Provides an invalid XML example (missing callee element) for testing validation error handling
package.html (test)	Adds javadoc description for test package
PhoneBillXmlHelperTest.java	Implements test cases for validating XML parsing of both valid and invalid documents
package.html (main)	Adds javadoc description for main application package
TextDumper.java	Removes unused imports to clean up code
PhoneBillXmlHelper.java	Implements XML helper class for Phone Bill DTD validation and entity resolution
archetype-metadata.xml	Updates archetype configuration to include XML files in test resources

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The DTD filename is hardcoded as "phonebill.dtd", but the XML files reference "${artifactId}.dtd" (line 7 in both XML files). This mismatch will cause the DTD resolution to fail. The filename should be a template variable that matches the XML reference, or the XML files should use the hardcoded filename.

Suggested change

	super(PUBLIC_ID, "phonebill.dtd");
	super(PUBLIC_ID, "${artifactId}.dtd");

[DavidWhitlock]

Good catch. At one point I had the name of the the DTD as ${artifactId}.dtd, but it caused the integration tests to fail. The integration tests use artifacts names like "basic" and "javadoc". And, clearly, there isn't a "basic.dtd" file.

So, this doesn't seem like the right change to me.

You make a good point about the names of the testing .xml files having $artifactId in them.

I think those work out okay because they are also referenced by $artifactId in the unit testing code.

[Copilot]

The test method name "cantParseInvalidXmlFile" uses informal contraction "cant". For consistency with standard Java naming conventions, this should be "cannotParseInvalidXmlFile" or "throwsExceptionWhenParsingInvalidXmlFile".

Suggested change

	void cantParseInvalidXmlFile() throws ParserConfigurationException {
	void cannotParseInvalidXmlFile() throws ParserConfigurationException {

[DavidWhitlock]

Oh! I like that suggestion. But there are several places in the code base that have a cantParseInvalidXmlFile() method.

@copilot, please create a new PR that changes the name of the cantParseInvalidXmlFile() method to throwsExceptionWhenParsingInvalidXmlFile() across several occurrences: PhoneBillXmlHelperTest, AirlineXmlHelperTest, and AppointmentBookXmlHelperTest.

[Copilot]

The DocumentBuilderFactory and DocumentBuilder setup code (lines 24-31 and 41-48) is duplicated between the two test methods. Consider extracting this into a shared setup method (e.g., using @beforeeach or a private helper method) to improve maintainability.

Suggested change

	@Test
	void canParseValidXmlFile() throws ParserConfigurationException, IOException, SAXException {
	PhoneBillXmlHelper helper = new PhoneBillXmlHelper();
	DocumentBuilderFactory factory =
	DocumentBuilderFactory.newInstance();
	factory.setValidating(true);
	DocumentBuilder builder =
	factory.newDocumentBuilder();
	builder.setErrorHandler(helper);
	builder.setEntityResolver(helper);
	builder.parse(this.getClass().getResourceAsStream("valid-${artifactId}.xml"));
	}
	@Test
	void cantParseInvalidXmlFile() throws ParserConfigurationException {
	PhoneBillXmlHelper helper = new PhoneBillXmlHelper();
	DocumentBuilderFactory factory =
	DocumentBuilderFactory.newInstance();
	factory.setValidating(true);
	DocumentBuilder builder =
	factory.newDocumentBuilder();
	builder.setErrorHandler(helper);
	builder.setEntityResolver(helper);
	/**
	* Helper method to create and configure a DocumentBuilder with validation,
	* error handler, and entity resolver set to the provided helper.
	*/
	private DocumentBuilder createConfiguredBuilder(PhoneBillXmlHelper helper)
	throws ParserConfigurationException {
	DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
	factory.setValidating(true);
	DocumentBuilder builder = factory.newDocumentBuilder();
	builder.setErrorHandler(helper);
	builder.setEntityResolver(helper);
	return builder;
	}
	@Test
	void canParseValidXmlFile() throws ParserConfigurationException, IOException, SAXException {
	PhoneBillXmlHelper helper = new PhoneBillXmlHelper();
	DocumentBuilder builder = createConfiguredBuilder(helper);
	builder.parse(this.getClass().getResourceAsStream("valid-${artifactId}.xml"));
	}
	@Test
	void cantParseInvalidXmlFile() throws ParserConfigurationException {
	PhoneBillXmlHelper helper = new PhoneBillXmlHelper();
	DocumentBuilder builder = createConfiguredBuilder(helper);

[DavidWhitlock]

Ooh. Again, this is a great suggestion. But the like the previous suggested change, it ought to be made in several places in the codebase. (Admittedly, this pattern of creating DocumentBuilders is duplicated in the codebase.)

@copilot , please create a separate PR that refactors this duplicate code in the PhoneBillXmlHelperTest, AppointmentBookXmlHelperTest, and AirlineXmlHelperTest. Be sure to get the occurrences in both the "original" Maven projects and the "archetype" Maven projects.

[Copilot]

[nitpick] There are two consecutive blank lines after the helper instantiation. This appears inconsistent with typical Java formatting conventions which use a single blank line to separate logical blocks.

[DavidWhitlock]

@copilot open a new pull request to apply changes based on this feedback

[Copilot]

[nitpick] There are two consecutive blank lines after the helper instantiation. This appears inconsistent with typical Java formatting conventions which use a single blank line to separate logical blocks.

[Copilot]

@DavidWhitlock I've opened a new pull request, #524, to work on those changes. Once the pull request is ready, I'll request review from you.