Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 15, 2025

Fix for Issue #1084: Nested Component Group Delimiter

Plan

  • Understand the XSL issue with nested components in groups
  • Fix MessageSubclass.xsl template for group-delimeter mode
  • Create minimal test XML specification (NestedComponentsTest.xml)
  • Create JUnit test (NestedComponentDelimiterTest.java)
  • Run test to verify the fix
  • Validate that existing tests still pass
  • Address review feedback - use JUnit TemporaryFolder and remove println

Summary

Successfully fixed the issue where MessageCodeGenerator was generating incorrect code for groups whose first element is a nested component (component-of-component).

Changes Made

  1. Fixed MessageSubclass.xsl (line 200): Changed match="group//component" to match="component" in the group-delimeter template. This allows the template to match components at any nesting level during recursion, not just those that are direct descendants of groups in the source XML. This pattern is consistent with the field and group templates in the same mode.

  2. Created test specification (NestedComponentsTest.xml): A minimal FIX dictionary with:

    • Field: Text(58), NoEntries(20001)
    • Component Level2: contains field Text
    • Component Level1: contains component Level2
    • Component NestedTwice: contains a group NoEntries whose first element is component Level1
    • Message TestMessage: references NestedTwice component
  3. Created JUnit test (NestedComponentDelimiterTest.java): Generates code from the test spec and asserts that:

    • The NestedTwice.java component file is generated
    • The NoEntries group class is present
    • The ORDER array is present
    • The constructor contains super(20001, 58, ORDER); (correct)
    • The constructor does NOT contain super(20001, , ORDER); (the bug)
    • Updated to use JUnit's @Rule TemporaryFolder instead of manually managing directories
    • Removed unnecessary println statement

Test Results

✅ All 3 tests pass (2 existing + 1 new):

  • OverwriteTest: 2 tests pass
  • NestedComponentDelimiterTest: 1 test passes

The generated code correctly resolves nested components to find the first field (Text with field number 58) and uses it as the group delimiter.

Original prompt

Summary
Fix MessageSubclass.xsl so MessageCodeGenerator sets the group delimiter correctly when the first element of a group is a component whose first element is another component (nested > 1). Add a regression test using a minimal FIX spec to ensure the generated code uses the proper delimiter and compiles structurally.

Context

  • Repository: quickfix-j/quickfixj
  • Problem: Issue MessageCodeGenerator fails to set group delimiter for nested components #1084 reports generated code like:
    super(20001, , ORDER);
    when generating a group whose first child is a nested component-of-component. Current XSL uses a template match="group//component" in mode group-delimeter, which fails after the first recursion step (the next component node is resolved in the global /fix/components tree and no longer matches group//component). As a result, no delimiter is emitted.

Proposed changes

  1. XSLT fix (MessageSubclass.xsl)
  • Replace the component delimiter template that currently only matches descendants under a group:
    <xsl:template mode="group-delimeter" match="group//component"> ... </xsl:template>
    with a generalized version that matches any component node:
    <xsl:template mode="group-delimeter" match="component"> ... </xsl:template>
  • Keep existing field and group templates unchanged. This allows recursion across arbitrarily deep nested components to reach the first field (or nested group) and output its tag number correctly.
  1. Regression test (quickfixj-codegenerator)
  • Add a minimal FIX specification XML under test resources that defines:
    • fields: Text(58), NoEntries(20001)
    • components: Level2 contains field Text; Level1 contains component Level2; NestedTwice contains a group NoEntries whose first element is component Level1
    • a dummy message that references the NestedTwice component so code-generation includes it
  • Add a JUnit test that runs MessageCodeGenerator on this spec to generate component classes into a temp output directory and asserts that the generated NestedTwice.java group constructor contains:
    super(20001, 58, ORDER);
  • Optionally assert that generated file contains the ORDER array skeleton and the NoEntries group class is present (structural compile checks are covered by string contains; we don’t require javac invocation here, consistent with existing tests in this module).

Files to modify/add

  • Modify: quickfixj-codegenerator/src/main/resources/org/quickfixj/codegenerator/MessageSubclass.xsl
    • Replace the group-delimeter template for components with a version that matches any component
  • Add: quickfixj-codegenerator/src/test/resources/org/quickfixj/codegenerator/NestedComponentsTest.xml
    • Minimal dictionary as described above
  • Add: quickfixj-codegenerator/src/test/java/org/quickfixj/codegenerator/NestedComponentDelimiterTest.java
    • New JUnit test to generate classes from NestedComponentsTest.xml and assert on the generated NestedTwice.java constructor

Acceptance criteria

  • Running the new test passes on Linux/macOS/Windows JDK 8+.
  • When generating a component with a group starting with a nested component-of-component, the constructor is generated with the correct delimiter, e.g. super(20001, 58, ORDER).
  • Existing behavior for groups whose first item is a field or a group is unchanged.

Notes

  • The change is minimal and localized to the XSL template selection; it does not alter other generation templates or message/component field ordering logic.
  • The test mirrors the user-reported scenario but with a self-contained spec so CI can validate future changes.

Linked issue

This pull request was created as a result of the following prompt from Copilot chat.

Summary
Fix MessageSubclass.xsl so MessageCodeGenerator sets the group delimiter correctly when the first element of a group is a component whose first element is another component (nested > 1). Add a regression test using a minimal FIX spec to ensure the generated code uses the proper delimiter and compiles structurally.

Context

  • Repository: quickfix-j/quickfixj
  • Problem: Issue MessageCodeGenerator fails to set group delimiter for nested components #1084 reports generated code like:
    super(20001, , ORDER);
    when generating a group whose first child is a nested component-of-component. Current XSL uses a template match="group//component" in mode group-delimeter, which fails after the first recursion step (the next component node is resolved in the global /fix/components tree and no longer matches group//component). As a result, no delimiter is emitted.

Proposed changes

  1. XSLT fix (MessageSubclass.xsl)
  • Replace the component delimiter template that currently only matches descendants under a group:
    <xsl:template mode="group-delimeter" match="group//component"> ... </xsl:template>
    with a generalized version that matches any component node:
    <xsl:template mode="group-delimeter" match="component"> ... </xsl:template>
  • Keep existing field and group templates unchanged. This allows recursion across arbitrarily deep nested components to reach the first field (or nested group) and output its tag number correctly.
  1. Regression test (quickfixj-codegenerator)
  • Add a minimal FIX specification XML under test resources that defines:
    • fields: Text(58), NoEntries(20001)
    • components: Level2 contains field Text; Level1 contains component Level2; NestedTwice contains a group NoEntries whose first element is component Level1
    • a dummy message that references the NestedTwice component so code-generation includes it
  • Add a JUnit test that runs MessageCodeGenerator on this spec to generate component classes into a temp output directory and asserts that the generated NestedTwice.java group constructor contains:
    super(20001, 58, ORDER);
  • Optionally assert that generated file contains the ORDER array skeleton and the NoEntries group class is present (structural compile checks are covered by string contains; we don’t require javac invocation here, consistent with existing tests in this module).

Files to modify/add

  • Modify: quickfixj-codegenerator/src/main/resources/org/quickfixj/codegenerator/MessageSubclass.xsl
    • Replace the group-delimeter template for components with a version that matches any component
  • Add: quickfixj-codegenerator/src/test/resources/org/quickfixj/codegenerator/NestedComponentsTest.xml
    • Minimal dictionary as described above
  • Add: quickfixj-codegenerator/src/test/java/org/quickfixj/codegenerator/NestedComponentDelimiterTest.java
    • New JUnit test to generate classes from NestedComponentsTest.xml and assert on the generated NestedTwice.java constructor

Acceptance criteria

  • Running the new test passes on Linux/macOS/Windows JDK 8+.
  • When generating a component with a group starting with a nested component-of-component, the constructor is generated with the correct delimiter, e.g. super(20001, 58, ORDER).
  • Existing behavior for groups whose first item is a field or a group is unchanged.

Notes

  • The change is minimal and localized to the XSL template selection; it does not alter other generation templates or message/component field ordering logic.
  • The test mirrors the user-reported scenario but with a self-contained spec so CI can validate future changes.

Linked issue


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix group delimiter in MessageSubclass.xsl for nested components Fix group delimiter resolution for nested components in XSL template Dec 15, 2025
Copilot AI requested a review from chrjohn December 15, 2025 15:27
Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jan 26, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • www.w3.org
    • Triggering command: /opt/hostedtoolcache/CodeQL/2.23.9/x64/codeql/tools/linux64/java/bin/java /opt/hostedtoolcache/CodeQL/2.23.9/x64/codeql/tools/linux64/java/bin/java -jar /opt/hostedtoolcache/CodeQL/2.23.9/x64/codeql/xml/tools/xml-extractor.jar --fileList=/home/REDACTED/work/quickfixj/.codeql-scratch/dbs/java/working/files-to-index17162749862715918584.list --sourceArchiveDir=/home/REDACTED/work/quickfixj/.codeql-scratch/dbs/java/src --outputDir=/home/REDACTED/work/quickfixj/.codeql-scratch/dbs/java/trap/java (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI requested a review from chrjohn January 26, 2026 11:44
@chrjohn chrjohn marked this pull request as ready for review January 27, 2026 09:19
@chrjohn chrjohn added this to the QFJ 3.0.0 milestone Jan 27, 2026
@chrjohn chrjohn merged commit 6208839 into master Jan 27, 2026
29 of 30 checks passed
@chrjohn chrjohn deleted the copilot/fix-group-delimiter-issue branch January 27, 2026 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MessageCodeGenerator fails to set group delimiter for nested components

2 participants