Skip to content

Add validation and fail early if a participant identifier is illegal - #67

Open
bmontuelle wants to merge 3 commits into
OxalisCommunity:masterfrom
bmontuelle:validate-participant-identifiers
Open

Add validation and fail early if a participant identifier is illegal#67
bmontuelle wants to merge 3 commits into
OxalisCommunity:masterfrom
bmontuelle:validate-participant-identifiers

Conversation

@bmontuelle

@bmontuelle bmontuelle commented May 29, 2026

Copy link
Copy Markdown

Pull Request Description

Validating participant ids received from SBDH so their length and other caracteristics are enforced before attempting to process the message any further.
This change is motivated because we received obviously invalid receiver participant Id that were causing failure in persistence for oxalis-ng-statistics, i.e. Data too long for column 'receiver'

Type of Pull Request

  • New feature/Enhancement - non-breaking change which adds functionality
  • Bug fix
  • Breaking change (Require Major version change?)

Type of Change

  • OpenPeppol eDEC Specifications
  • Peppol AS4 Profile specification
  • Peppol Business Envelope specification
  • Peppol Policies specification
  • Peppol eDEC Code Lists specification
  • OpenPeppol Spring/Fall release
  • Oxalis software internal change or enhancement
  • General change

Pull Request Checklist:

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas. But did not add unnecessary annotation/comment say @author name etc
  • I have checked my code for variable and method name and corrected grammar/spelling mistakes if any
  • I have made corresponding changes to the documentation where needed
  • My changes generate no new/additional warnings
  • My change is not breaking or creating conflict with associated dependencies
  • I have performed a self-review of my own code
  • I ran mvn clean install before commit and all tests run successfully
  • I conducted basic QA to assure all features are working fine
  • My pull request generate no conflicts with master branch
  • I requested code review from other team members

@jvanbrakel

Copy link
Copy Markdown

I’m not in a position to judge whether this change is technically necessary, but I did notice a potential functional concern.

The validator limits the identifier to 28 characters, while Peppol recently increased the maximum length (up to 130 characters).
Peppol CNAME to NAPTR Migration Process:

2.1 Changes to Peppol Policy for use of Identifiers
2.1.1 Changes to POLICY 1
The maximum length of Participant Identifiers and Party Identifiers was increased from 50 to 130 characters.

This could lead to rejecting valid identifiers.

Should this be aligned with the updated Peppol limits?

@bmontuelle
bmontuelle force-pushed the validate-participant-identifiers branch from 6cb05b2 to 18e571c Compare May 29, 2026 09:20
@bmontuelle

Copy link
Copy Markdown
Author

@jvanbrakel good catch, I updated the code so it reflects the maximum length change in participant identifiers.

@aaron-kumar

Copy link
Copy Markdown
Member

Thanks for contribution @bmontuelle . I have suggestions for improvements:

  • Reuse existing vefa-peppol identifier parsing/validation instead of introducing a separate regex if possible.
  • Rename the validator (or clearly document that it performs only basic syntax validation).
  • Correct the Javadoc to avoid claiming ISO 6523 compliance when only the prefix/length syntax is checked.
  • Centralize header validation to avoid duplicating validation logic in multiple locations.

Additionally validateAndWarn() is somewhat misleading because validation failures are immediately treated as fatal by the caller (PeppolParsingException / OxalisContentException). The method name suggests a non-fatal validation that only logs a warning, whereas callers actually abort processing.

@bmontuelle
bmontuelle force-pushed the validate-participant-identifiers branch from 18e571c to e9ff3cb Compare July 27, 2026 09:41
@bmontuelle

bmontuelle commented Jul 27, 2026

Copy link
Copy Markdown
Author

@aaron-kumar I updated to use vefa peppol-icd to validate ICDs against the official icd list and adress other feedback provided about thins PR.
But to do this I had to migrate the deprecated 9908 Norwegian scheme to its current 0192 successor in WellKnownParticipant and peppol-bis-invoice-sbdh.xml (same org numbers, valid ICD). Not really sure this is a good thing to do, as the 9908 scheme was still in use for the tests at least, perhaps for a good reason.
What do you think of this ?

@vrbyjimmy

vrbyjimmy commented Aug 1, 2026

Copy link
Copy Markdown

Hello @bmontuelle and @aaron-kumar,

if you consider merging this, could you at least introduce some setting to disable the validation entirely? This change with hard failure everytime could potentially cause a breaking change especially when new jurisdiction is adopting peppol and a new schema code is introduced (for example Slovakia 0245 and Oman 0248 latest). This would lead to companies using oxalis-ng to be unable to send anything and to for example pass the required testbed in the early stages of the peppol adoption. Not everytime the required code lists changes and release of oxalis-ng are in sync with what is already published on testbed and processed by local peppol authority.

Thanks

@bmontuelle

bmontuelle commented Aug 3, 2026

Copy link
Copy Markdown
Author

Hi @vrbyjimmy this is a very serious concern, I didnt knew that participant countries were adding ICP prefixes, and using the vefa Peppol Icd static list for a hard validation would then require a dependency upgrade and rollout through an oxalis-ng release to have the new prefixes. vefa-peppol v4.5.0 Already covers SK_DIC("SK:DIC", "0245", ...) and OM_VAT("OM:VAT", "0248", ...) but this could be a concern in the future.
I'm working on an update to this PR so that validation will behave like this :

  • Hard block on formatting issue, max lenght and colon-separated ICD upfront
  • Log a warning if the ICD code is new and unknown to the vefa-peppol library
  • Option to disable validation entirely

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.

4 participants