-
Notifications
You must be signed in to change notification settings - Fork 1
Tedefo 4805 efx rules translator #40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
- Add EFX_RULES_TRANSLATOR to SdkComponentType enum - Required for Schematron generation from EFX Rules
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds support for notice type/subtype handling and introduces new component types (EFX_RULES_TRANSLATOR and VALIDATOR_GENERATOR) to the SDK component factory. The primary additions include a new entity class for notice subtypes and a repository to manage them.
Key changes:
- Introduces
SdkNoticeSubtypeentity class to represent notice subtypes from the SDK's notice-types.json file - Adds
SdkNoticeTypeRepositoryto load and manage notice subtype data - Extends component type enumeration to support EFX rules translator and validator generator functionality
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| SdkNoticeSubtype.java | New abstract entity class representing notice subtypes with fields for subTypeId, documentType, and type |
| SdkNoticeTypeRepository.java | New repository class extending MapFromJson to load notice subtypes from JSON |
| SdkEntityFactory.java | Adds factory method getSdkNoticeType to instantiate SdkNoticeSubtype implementations |
| SdkComponentType.java | Extends enum with NOTICE_TYPE, EFX_RULES_TRANSLATOR, and VALIDATOR_GENERATOR component types |
| SdkConstants.java | Adds NOTICE_TYPES_JSON_SUBTYPES_KEY constant for JSON parsing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main/java/eu/europa/ted/eforms/sdk/entity/SdkNoticeSubtype.java
Outdated
Show resolved
Hide resolved
src/main/java/eu/europa/ted/eforms/sdk/entity/SdkNoticeSubtype.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main/java/eu/europa/ted/eforms/sdk/repository/SdkNoticeTypeRepository.java
Outdated
Show resolved
Hide resolved
src/main/java/eu/europa/ted/eforms/sdk/entity/SdkEntityFactory.java
Outdated
Show resolved
Hide resolved
src/main/java/eu/europa/ted/eforms/sdk/repository/SdkNoticeTypeRepository.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public int compareTo(SdkNoticeSubtype o) { | ||
| return this.subTypeId.compareTo(o.subTypeId); | ||
| } |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The compareTo method can throw a NullPointerException if subTypeId is null. The JsonNode constructor (line 21) uses asText(null) which can return null. Consider adding null safety checks or using Objects.compare with a null-safe comparator, similar to how SdkCodelist handles this (see SdkCodelist.java:67-69).
| public String getId() { | ||
| return subTypeId; | ||
| } |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The getId() method returns subTypeId which can be null (set via asText(null) in the JsonNode constructor at line 21). This could cause NullPointerException in callers who expect a non-null value, such as in SdkNoticeTypeRepository.java:26 where it's used as a map key, and in the compareTo method. Consider validating that critical fields like subTypeId are non-null after construction, or document that null is a valid return value.
| this.subTypeId = json.get("subTypeId").asText(null); | ||
| this.documentType = json.get("documentType").asText(null); | ||
| this.type = json.get("type").asText(null); |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JsonNode constructor does not validate that required JSON fields (subTypeId, documentType, type) exist before accessing them. If json.get("subTypeId") returns null, calling asText(null) on it will throw a NullPointerException. Consider adding null checks or using safer accessor methods like json.has("subTypeId") before accessing, consistent with how SdkNode handles optional fields (see SdkNode.java:27).
| this.subTypeId = json.get("subTypeId").asText(null); | |
| this.documentType = json.get("documentType").asText(null); | |
| this.type = json.get("type").asText(null); | |
| if (json == null) { | |
| throw new IllegalArgumentException("json must not be null when creating SdkNoticeSubtype"); | |
| } | |
| JsonNode subTypeIdNode = json.get("subTypeId"); | |
| JsonNode documentTypeNode = json.get("documentType"); | |
| JsonNode typeNode = json.get("type"); | |
| if (subTypeIdNode == null || documentTypeNode == null || typeNode == null) { | |
| throw new IllegalArgumentException("Missing required fields (subTypeId, documentType, type) in notice subtype JSON: " + json); | |
| } | |
| this.subTypeId = subTypeIdNode.asText(null); | |
| this.documentType = documentTypeNode.asText(null); | |
| this.type = typeNode.asText(null); |
No description provided.