Uh oh!
There was an error while loading. Please reload this page.
- 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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
5e3f420c724b0b0331b917d77effdd51af2a73e4fc2eb247eFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,10 @@ | ||
| package eu.europa.ted.eforms.sdk.component; | ||
| /** | ||
| * Enumeration of component types that can be registered with the SDK component factory. | ||
| * Enumeration of component types that can be registered with the SDK component | ||
| * factory. | ||
| */ | ||
| public enum SdkComponentType { | ||
| FIELD, NODE, CODELIST, EFX_EXPRESSION_TRANSLATOR, EFX_TEMPLATE_TRANSLATOR, SYMBOL_RESOLVER, SCRIPT_GENERATOR, MARKUP_GENERATOR; | ||
| FIELD, NODE, CODELIST, NOTICE_TYPE, EFX_EXPRESSION_TRANSLATOR, EFX_TEMPLATE_TRANSLATOR, EFX_RULES_TRANSLATOR, | ||
| SYMBOL_RESOLVER, SCRIPT_GENERATOR, MARKUP_GENERATOR, VALIDATOR_GENERATOR; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -103,6 +103,6 @@ public int hashCode() { | ||
| @Override | ||
| public String toString() { | ||
| return "SdkField [id=" + id + "]"; | ||
| return id; | ||
| } | ||
| } | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar classes in this folder are described in the README file in the folder. So this new class should also be described in this README, for consistency. ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @copilot please address this remark by adding an adequate reference to this class in the respected README file. |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,75 @@ | ||||||||||||||||||||||||||||||||||||||
| package eu.europa.ted.eforms.sdk.entity; | ||||||||||||||||||||||||||||||||||||||
| import java.util.Objects; | ||||||||||||||||||||||||||||||||||||||
| import com.fasterxml.jackson.databind.JsonNode; | ||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||
| * Represents a notice subtype from the SDK's notice-types.json file. | ||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||
| public abstract class SdkNoticeSubtype implements Comparable<SdkNoticeSubtype> { | ||||||||||||||||||||||||||||||||||||||
| private final String subTypeId; | ||||||||||||||||||||||||||||||||||||||
| private final String documentType; | ||||||||||||||||||||||||||||||||||||||
| private final String type; | ||||||||||||||||||||||||||||||||||||||
| protected SdkNoticeSubtype(String subTypeId, String documentType, String type) { | ||||||||||||||||||||||||||||||||||||||
| this.subTypeId = subTypeId; | ||||||||||||||||||||||||||||||||||||||
| this.documentType = documentType; | ||||||||||||||||||||||||||||||||||||||
| this.type = type; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| protected SdkNoticeSubtype(JsonNode json) { | ||||||||||||||||||||||||||||||||||||||
| this.subTypeId = json.get("subTypeId").asText(null); | ||||||||||||||||||||||||||||||||||||||
| this.documentType = json.get("documentType").asText(null); | ||||||||||||||||||||||||||||||||||||||
| this.type = json.get("type").asText(null); | ||||||||||||||||||||||||||||||||||||||
Comment on lines
+21
to
+23
CopilotAI | ||||||||||||||||||||||||||||||||||||||
| this.subTypeId = json.get("subTypeId").asText(null); | |
| this.documentType = json.get("documentType").asText(null); | |
| this.type = json.get("type").asText(null); | |
| if (json == null) { | |
| thrownewIllegalArgumentException("json must not be null when creating SdkNoticeSubtype"); | |
| } | |
| JsonNodesubTypeIdNode = json.get("subTypeId"); | |
| JsonNodedocumentTypeNode = json.get("documentType"); | |
| JsonNodetypeNode = json.get("type"); | |
| if (subTypeIdNode == null || documentTypeNode == null || typeNode == null) { | |
| thrownewIllegalArgumentException("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); |
CopilotAIDec 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.
CopilotAIDec 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).
Uh oh!
There was an error while loading. Please reload this page.
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar classes in this folder are described in the README file in the folder. So this new class should also be described in this README, for consistency. ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @copilot please address this remark by adding an adequate reference to this class in the respected README file. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| package eu.europa.ted.eforms.sdk.repository; | ||
| import java.nio.file.Path; | ||
| import com.fasterxml.jackson.databind.JsonNode; | ||
| import com.fasterxml.jackson.databind.node.ArrayNode; | ||
| import eu.europa.ted.eforms.sdk.SdkConstants; | ||
| import eu.europa.ted.eforms.sdk.entity.SdkEntityFactory; | ||
| import eu.europa.ted.eforms.sdk.entity.SdkNoticeSubtype; | ||
| /** | ||
| * Repository for SDK notice types loaded from notice-types.json. | ||
| * Maps notice subtype IDs (e.g., "1", "3", "CEI", "E1", "X01") to SdkNoticeSubtype objects. | ||
| */ | ||
| public class SdkNoticeTypeRepository extends MapFromJson<SdkNoticeSubtype> { | ||
| private static final long serialVersionUID = 1L; | ||
| public SdkNoticeTypeRepository(String sdkVersion, Path jsonPath) throws InstantiationException { | ||
| super(sdkVersion, jsonPath); | ||
| } | ||
| @Override | ||
| protected void populateMap(final JsonNode json) throws InstantiationException { | ||
| final ArrayNode noticeSubtypes = (ArrayNode) json.get(SdkConstants.NOTICE_TYPES_JSON_SUBTYPES_KEY); | ||
| for (final JsonNode noticeSubtype : noticeSubtypes) { | ||
| final SdkNoticeSubtype sdkNoticeSubtype = SdkEntityFactory.getSdkNoticeSubtype(sdkVersion, noticeSubtype); | ||
| put(sdkNoticeSubtype.getId(), sdkNoticeSubtype); | ||
| } | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.
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 value name "NOTICE_TYPE" is inconsistent with the corresponding class SdkNoticeSubtype, but it corresponds to the folder named "notice-types" in the SDK.
So I'm not sure which is the best name here.
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.
I am not sure either... We will have a name finalisation debate before we release the SDK. The subtypes are a common naming problem but there are others like for example the fact that we interchangeably use the terms "string" and "text" in the grammar. We will deal with any such remaining naming issues when all else i ready.