Conversation
Fix bugs: silent ignore of unknown labels, parseOBJEKT dimension ID, trailing empty quoted string in splitLine. Writer fixes: ORGNR null guard, skip default/forbidden DIM records, append quantity in period values, add writeBKOD, GEN date null fallback, 2-decimal amount truncation. Reader soft validation warnings for mandatory fields, forbidden records, period format, account numbers, amount decimals, voucher ordering, and ORGNR format. CRC32 now strips quotes/spaces per spec. SIE5 opt-in strict validation for fiscal years, currency, journal entry ordering, quantity/amount sign, foreign currency sign, and balance completeness. Added getActiveLedgerEntries() convenience. 26 new tests (158 total, all passing). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This pull request enhances SIE specification compliance across both SIE 4 (text format) and SIE 5 (XML format) implementations, addressing numerous parsing issues, validation gaps, and adding digital signature support for SIE 5 documents.
Changes:
- Fixed multiple SIE 4 spec compliance issues including unknown label handling, RTRANS/mirror-TRANS processing, ORGNR format validation, CRC calculation, and trailing empty string parsing
- Added comprehensive SIE 5 XMLDSig signature writing and verification with configurable validation options
- Implemented strict validation mode for SIE 5 with fiscal year, currency, journal entry ordering, and sign consistency checks
- Added detailed spec compliance report documenting historical and current compliance status
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/alipsa/sieparser/sie5/TestSigningCredentials.java | Test helper providing RSA key pair and X.509 certificate for signature testing |
| src/test/java/alipsa/sieparser/sie5/Sie5DocumentWriterTest.java | Tests for signature generation and unsigned document rejection |
| src/test/java/alipsa/sieparser/sie5/Sie5DocumentReaderTest.java | Tests for signature validation, strict validation rules, and overstrike filtering |
| src/test/java/alipsa/sieparser/SieDocumentWriterTest.java | Comprehensive tests for KSUMMA, ORGNR, BKOD, quantity, dimension handling, and amount precision |
| src/test/java/alipsa/sieparser/SieDocumentReaderTest.java | Tests for RTRANS mirror handling, unknown labels, validation warnings, and format checks |
| src/test/java/alipsa/sieparser/SieDataItemTest.java | Test for trailing empty quoted string preservation |
| src/main/java/alipsa/sieparser/sie5/Sie5SigningCredentials.java | New class encapsulating private key and certificate for XMLDSig signing |
| src/main/java/alipsa/sieparser/sie5/Sie5Entry.java | Added anyElements support for capturing XMLDSig signatures in entry documents |
| src/main/java/alipsa/sieparser/sie5/Sie5DocumentWriter.java | Implements XMLDSig enveloped signature generation with configurable requirements |
| src/main/java/alipsa/sieparser/sie5/Sie5DocumentReader.java | Implements signature verification, XXE protection, and strict SIE 5 validation rules |
| src/main/java/alipsa/sieparser/sie5/Sie5Document.java | Added anyElements support for capturing XMLDSig signatures in full documents |
| src/main/java/alipsa/sieparser/sie5/JournalEntry.java | Added getActiveLedgerEntries() method to filter out overstriken entries |
| src/main/java/alipsa/sieparser/SieDocumentWriter.java | Fixed CRC handling, ORGNR null check, BKOD output, dimension filtering, quantity preservation, and date defaults |
| src/main/java/alipsa/sieparser/SieDocumentReader.java | Added RTRANS mirror detection, ORGNR format validation, mandatory field warnings, forbidden record checks, and voucher ordering validation |
| src/main/java/alipsa/sieparser/SieDataItem.java | Fixed splitLine to preserve trailing empty quoted strings |
| src/main/java/alipsa/sieparser/SieCRC32.java | Enhanced to strip quotes and whitespace from object lists per spec |
| spec-compliance-report.md | New comprehensive documentation of SIE 4 and SIE 5 compliance issues and their status |
| README.md | Updated with spec compliance section and formatting improvements |
| build.gradle | Changed jakarta.xml.bind from api to implementation scope |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… reset
- Update spec-compliance-report.md to mark all 25 issues as [FIXED]
- Fix README grammar ("has" -> "have") and reconcile compliance claim
- Add null guard in JournalEntry.getActiveLedgerEntries()
- Clear validationWarnings at start of each Sie5DocumentReader.readDocument()
- Document why secureValidation is FALSE (SHA-1 compat with SIE 5 spec)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- CRC32: only strip quotes/whitespace from object-list fields (containing braces), leave regular string fields unchanged to preserve spaces in e.g. company names - Sie5DocumentReader: clear validationWarnings at start of readEntry() - Sie5Document/Sie5Entry: null guard in getSignatures() for anyElements - SieDocumentReader: parseVER falls back to LocalDate.now() when voucher date is missing (prevents downstream NPEs when throwErrors=false) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- rowDataWithoutTag: use Character.isWhitespace instead of only space,
so tab-separated tokens are also handled correctly for RTRANS mirror
detection
- readDocument: reset all per-parse state (sieDocument, CRC, validation
warnings, seenRecordTypes, sieTypSeen, formatSeen, voucher tracking)
so the reader instance can be safely reused across multiple files
- ORGNR: tighten regex to ^\\d{6}-\\d{4}$ matching the canonical Swedish
organisationsnummer format
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- sieAmount: use amount.scale() instead of stripTrailingZeros().scale() to normalize trailing-zero cases like 1.200 - Sie5DocumentWriter.write(doc,fileName): catch SieException too so filename context is preserved on JAXB/signing failures - TestSigningCredentials: replace static PEM with ephemeral keypair generated at test runtime via BouncyCastle (added as test dependency) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- validateJournals: null-check journal.getJournalEntries() - validateLedgerEntries: null-check entry.getLedgerEntries() Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…or type 1 - Sie5DocumentWriter: throw immediately when requireSignatureForFullDocuments is true but no signing credentials are configured, since existing signatures cannot survive JAXB re-marshalling - SieDocumentWriter: skip #OMFATTN for SIE type 1, matching the reader's forbidden-record validation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request focuses on improving SIE specification compliance and validation in the SIEParser library, addressing several issues related to record handling, data validation, and CRC calculation. The changes include both code fixes and documentation updates, as well as the addition of a detailed spec compliance report.
Key improvements and fixes:
SIE 4 Compliance and Validation
#ORGNRformat: Now checks for the required hyphen and adds a warning if the format is incorrect.#RTRANS/mirror-#TRANShandling is now compliant: The parser ignores the mirror#TRANSline following a#RTRANSas per the spec. [1] [2]allowUnderDimensionsis nowtrueby default, aligning with common usage.Data Parsing and CRC Calculation
splitLine, fixing a data loss bug.Documentation and Reporting
spec-compliance-report.mddocumenting current and past compliance issues, their severity, and status.These changes collectively bring the SIEParser closer to full SIE 4 and SIE 5 compliance, improve interoperability, and provide better feedback to users and developers about file validity and spec adherence.