Tedefo 4805 efx rules translator#129
Conversation
Infrastructure: - Add EfxRulesTranslator interface with translateRules methods - Add factory methods to EfxTranslatorFactory - Add convenience methods to EfxTranslator - Update eforms-core dependency to 1.6.0-SNAPSHOT Data Model: - Create Schematron model classes (SchematronSchema, SchematronPattern, SchematronRule, SchematronAssert, SchematronLet, SchematronPhase, SchematronFile) - Add Freemarker templates for Schematron XML generation (complete-validation.ftl, pattern.ftl, rule.ftl, assert.ftl, let.ftl) Components: - Create SchematronMarkupGenerator with Freemarker integration - Create EfxRulesTranslatorV2 skeleton extending EfxExpressionTranslatorV2 - Add Freemarker 2.3.31 dependency to pom.xml Status: Foundation complete, listener methods TODO
There was a problem hiding this comment.
Pull request overview
This PR implements a transpiler that generates Schematron validation files from EFX (eForms Expression) rules files. The PR adds extensive test resources covering the transpilation functionality.
Key Changes
- Added comprehensive test fixtures for EFX rules translator
- Implemented support for various EFX language features: variables, WHEN clauses, stages, notice types
- Generated both static and dynamic Schematron output files
- Added field definitions supporting test scenarios including fields with parentheses in IDs
Reviewed changes
Copilot reviewed 234 out of 234 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| fields-sdk2.json | Added test field definition with parentheses in ID (BT-00(a)-Text) |
| testWithClause_VariablePositioning/* | Test files for variable declaration positioning at pattern and rule levels |
| testWithClause_RootContextShortcut/* | Test files for root context syntax (WITH /) |
| testWithClause_ContextVariable/* | Test files for context variable syntax |
| testWhen_WithOtherwise/* | Test files for WHEN/OTHERWISE conditional logic |
| testWhen_SimpleCondition/* | Test files for simple WHEN conditions |
| testVariable_StageLevel/* | Test files for stage-level variable scoping |
| testVariable_Global_AppearsBeforeIncludes/* | Test files for global variable positioning |
| testStage_Multiple_GeneratesSeparatePatterns/* | Test files for multiple stage handling |
| testOutput_FromSampleRulesFile/* | Comprehensive sample rules test |
| testOutput_ComprehensiveMixedRules/* | Test files for mixed ASSERT/REPORT rules |
| testInClause_SpecificNoticeTypes/* | Test files for specific notice type filtering |
| testInClause_PhaseGeneration/* | Test files for phase generation logic |
| testInClause_NoticeTypeRange/* | Test files for notice type range expansion |
| testInClause_AllNoticeTypes/* | Test files for wildcard notice types |
| testForClause_NodeReference/* | Test files for node references in FOR clauses |
| testDiagnostics_SanitizesParentheses/* | Test files for parentheses sanitization in diagnostics |
| testDiagnostics_NoDuplicateEntries/* | Test files for duplicate diagnostic prevention |
| testDiagnostics_MultipleFields/* | Test files for multiple field diagnostics |
| testAssertAndReport_Simple/* | Basic ASSERT and REPORT test files |
After thoroughly reviewing all the test resource files, I found no issues to report. The files are well-structured test fixtures that follow consistent patterns and demonstrate comprehensive coverage of the transpiler functionality. All XML is properly formatted, test scenarios are well-documented with comments, and the expected outputs are consistent with the input specifications.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…chematronMarkupGenerator to SchematronGenerator
The "schematrons.json" were indicated as different when running on Windows. Stripping the trailing whitespace avoids this problem.
...x/sdk2/EfxRulesTranslatorV2Test/testAssertAndReport_Simple/dynamic/validation-stage-1a-1.sch
Outdated
Show resolved
Hide resolved
...eu/europa/ted/efx/sdk2/EfxRulesTranslatorV2Test/testDiagnostics_NoDuplicateEntries/input.efx
Show resolved
Hide resolved
.../efx/sdk2/EfxRulesTranslatorV2Test/testWhen_SimpleCondition/static/validation-stage-1a-1.sch
Outdated
Show resolved
Hide resolved
| <ns prefix="fn" uri="http://www.w3.org/2005/xpath-functions" /> | ||
|
|
||
| <let name="sdkVersion" value=""2.0.0""/> | ||
| <let name="noticeType" value=""16""/> |
There was a problem hiding this comment.
The value seems incorrect, I don't think it should have "
There was a problem hiding this comment.
The " entities are actually necessary and correct here.
EFX supports both single and double-quoted string literals:
LET text : $foo = "it's a test";LET text : $bar = 'say "hello"';
We pass through the quote type from the EFX source to XPath to handle strings containing the opposite quote character. If we blindly converted all to single quotes, strings like "it's a test" would break because the internal single quote would terminate the string.
The XML entities (") are just how XML escapes double quotes in attribute values - the XPath processor sees the correct string literal with its original quotes.
| <rule context="/*/PathNode/TextField"> | ||
| <report id="R-X3F-N8W" role="info" test="(not(./normalize-space(text()) = 'restricted')) or (../NumberField/number() > 0)">rule|text|R-X3F-N8W</report> | ||
| <assert id="R-H9T-V5L" role="warning" test="(not(./normalize-space(text()) = 'negotiated')) or (../IndicatorField)">rule|text|R-H9T-V5L</assert> | ||
| <report id="R-B6J-C4R" role="info" test="(not(not(./normalize-space(text()) = 'open') and not(./normalize-space(text()) = 'restricted') and not(./normalize-space(text()) = 'negotiated'))) or (./normalize-space(text()) != '')">rule|text|R-B6J-C4R</report> |
There was a problem hiding this comment.
I would not necessarily expect the condition on 'open' to be here: it does not apply to subtype 2, so having it in the "report" element corresponding to the OTHERWISE might not be expected.
It can stay like it is, but it will need to be clearly indicated for the documentation on "OTHERWISE".
There was a problem hiding this comment.
You're right - this is a subtlety in how OTHERWISE works.
The OTHERWISE condition includes ALL preceding WHEN conditions from the same WITH block, regardless of their IN clauses. In this case:
WHEN text == 'open' FOR IN 1WHEN text == 'restricted' FOR IN 1, 2WHEN text == 'negotiated' FOR IN 2OTHERWISE REPORT ... FOR IN 2
The OTHERWISE for notice type 2 checks: NOT ('open' OR 'restricted' OR 'negotiated') even though the 'open' condition only applies to type 1.
This is safe but not optimal - it includes a redundant check. We can document this behavior clearly in the OTHERWISE documentation to explain that it combines all WHEN conditions from the WITH block, not just those that apply to the same notice types.
The alternative would be to filter the inverted conditions by notice type, but that would complicate the logic and the current behavior is correct (just not minimal).
Also, the user needs to create a rule that makes sense. If the user adds an otherwise clause which contradicts notice filtering, then the input may not make sense. We can add an error for this case later, but I found it an overkill since we do not yet use this feature at all. In any case OTHERWISE should run ONLY IF none of the WHEN conditions was met regardless of notice filtering.
There was a problem hiding this comment.
OK, this can stay as it is.
I'm not marking this as resolved, just to keep the explanations visible by default.
src/main/java/eu/europa/ted/efx/interfaces/ValidatorGenerator.java
Outdated
Show resolved
Hide resolved
The method transforms an in-memory model to strings with no I/O operations implied by the signature. IOException is an implementation detail of Freemarker template loading, which is now handled internally by wrapping in RuntimeException.
Stream.toList() was only added in JDK 16, so instead use "collect(Collectors.toList())"
This adds a transpiler to generate Schematron from EFX rules files.