[NAE-2119] Fix menuItem #315
Conversation
- Added transformation methods `transformCaseFieldField` and `transformFilterFieldField` to `ElasticCaseMappingService`. - Introduced new classes `CaseField` and `FilterField` in both `elastic` and `spring.elastic` domains. - Updated `I18nField` to include `translations` using a `Map<String, String>` and annotated it appropriately for ElasticSearch. - Enhanced `GetDataEvent` to handle null values safely in `getMessage`. - Modified `User` domain to handle `tel` attribute when `attributes` is null. - Fixed formatting of multiple methods for better readability. - Updated `docker-compose.yml` to correct misplaced `networks` and `volumes` definition.
…alization - Introduced a `getValue` method to multiple field classes (`MapField`, `DataField`, `UserField`, `CaseField`, `DateField`, and others) to standardize data retrieval from field objects. - Created a new `ImmediateField` class for representing immediate data fields in `ElasticCase`. - Updated `ElasticCase` to include handling for `immediateData` with proper mapping from `ImmediateField`. - Refactored imports across the affected classes to use `java.util.*` and streamline code. - Enhanced `FileField` with support for file paths and proper serialization via `getValue`. - Updated various field structure definitions to ensure clarity and consistency in field data management. This change improves maintainability and provides better support for advanced data retrieval and serialization of field objects.
- Corrects the condition to check if the user is null before accessing its realmId. Prevents potential NullPointerException when the user object is not initialized.
## Walkthrough
This update introduces new domain classes for case and filter fields with allowed network support, enhances data field value extraction methods, and adds translation mapping in I18n fields. It modifies ElasticCase to include immediate data, adjusts null-check logic in task user realm assignment, and improves null safety in user telephone handling. Minor YAML formatting and method signature refactorings are also present.
## Changes
| File(s) | Change Summary |
|------------------------------------------------------------------------------------------------------------|---------------|
| application-engine/docker-compose.yml | Moved `networks` and `volumes` sections below the commented kibana service; no functional change. |
| application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy | Capitalizes the `type` parameter in `createFilter` method for consistency. |
| application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java | Adds transformation for new `CaseField` and `FilterField` types; updates `transformI18nField` to include translations; refactors method signatures for readability. |
| application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/Task.java | Alters null-check logic for assigning `userRealmId` in constructor. |
| nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/domain/User.java | Adds null checks and map initialization in `getTelNumber()` and `setTelNumber()`. |
| nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/BooleanField.java | Adds override: `getValue()` returns `booleanValue`. |
| nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ButtonField.java | Adds override: `getValue()` returns `buttonValue`. |
| nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/CaseField.java | Introduces abstract class `CaseField` with allowed nets and value extraction logic. |
| nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DataField.java | Adds `getValue()` method returning first fulltext value. |
| nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DateField.java | Adds override: `getValue()` returns `dateValue`. |
| nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ElasticCase.java | Adds `immediateData` field; updates constructor and `update()` method to handle it. |
| nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FieldWithAllowedNetsField.java | Introduces abstract base class for fields with allowed networks. |
| nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FileField.java | Adds `filePath` field; implements `getValue()` to return file value(s) with path. |
| nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FilterField.java | Introduces abstract class `FilterField` with allowed nets and metadata. |
| nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/I18nField.java | Adds `translations` field; updates constructor; implements `getValue()` returning `I18nString`. |
| nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/MapField.java | Adds `getValue()` method for key extraction logic. |
| nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/NumberField.java | Adds `getValue()` method returning `numberValue`. |
| nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/UserField.java | Adds `getValue()` method returning user value(s) as objects. |
| nae-object-library/src/main/java/com/netgrif/application/engine/objects/event/events/data/GetDataEvent.java | Adds null safety in `getMessage()`; minor formatting. |
| nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/ImmediateField.java | Introduces `ImmediateField` class for immediate data mapping. |
| nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/CaseField.java | Adds `CaseField` class extending new elastic domain field; exposes allowed nets for Elasticsearch. |
| nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/FilterField.java | Adds `FilterField` class extending new elastic domain field; exposes allowed nets and filter metadata for Elasticsearch. |
| nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/I18nField.java | Updates constructor to include translations; adds getter for translations, annotated for Elasticsearch. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant WorkflowDomain
participant ElasticCaseMappingService
participant ElasticDomain
WorkflowDomain->>ElasticCaseMappingService: transformDataField(dataField, netField)
alt netField is CaseField
ElasticCaseMappingService->>ElasticDomain: transformCaseFieldField(...)
ElasticDomain-->>ElasticCaseMappingService: CaseField instance with allowedNets
else netField is FilterField
ElasticCaseMappingService->>ElasticDomain: transformFilterFieldField(...)
ElasticDomain-->>ElasticCaseMappingService: FilterField instance with allowedNets and metadata
else netField is I18nField
ElasticCaseMappingService->>ElasticDomain: transformI18nField(...)
ElasticDomain-->>ElasticCaseMappingService: I18nField with translations
end
ElasticCaseMappingService-->>WorkflowDomain: Optional<DataField>Possibly related PRs
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🔭 Outside diff range comments (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FileField.java (1)
35-45:⚠️ Potential issueFix missing
filePathinitialization in array constructor.The constructor for multiple
FileFieldValueobjects doesn't initialize thefilePatharray, but thegetValue()method assumes it exists and tries to accessfilePath[i]. This will cause aNullPointerExceptionat runtime.Apply this fix to initialize the
filePatharray:public FileField(FileFieldValue[] values) { super(new String[values.length]); + this.filePath = new String[values.length]; this.fileNameValue = new String[values.length]; this.fileExtensionValue = new String[values.length]; for (int i = 0; i < values.length; i++) { FileNameAndExtension extracted = this.extractFileExtensionFromName(values[i].getName()); this.fileNameValue[i] = extracted.name; this.fileExtensionValue[i] = extracted.extension; super.fulltextValue[i] = values[i].getName(); + this.filePath[i] = values[i].getPath(); } }
🧹 Nitpick comments (6)
application-engine/docker-compose.yml (1)
70-70: Add missing newline at end of file.YAMLLint reports a missing newline at EOF. Please append a trailing blank line to satisfy the
new-line-at-end-of-filerule.Apply this diff:
--- a/application-engine/docker-compose.yml +++ b/application-engine/docker-compose.yml @@ -70 +71 @@ driver: local +🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 70-70: no new line character at the end of file
(new-line-at-end-of-file)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/NumberField.java (1)
20-22: Add missing @OverRide annotation.The
getValue()method overrides the parent class method but lacks the@Overrideannotation, which helps catch signature mismatches at compile time.+@Override public Object getValue() { return numberValue; }application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy (1)
3-3: Unused or unsupported import ofcom.mchange.v2.lang.StringUtils.The imported
StringUtilsis neither used in this class nor guaranteed on the classpath (no new dependencies were introduced). Remove this import to avoid compilation issues or confusion.nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/MapField.java (1)
36-43: Consider return type consistency for the getValue() method.The method logic is correct, but returning different types (
StringvsLinkedHashSet<String>) based on array size could lead to casting issues for consumers. Consider documenting this behavior clearly or providing type-safe alternatives.However, this appears to be part of a broader standardization effort across field types, so the current implementation may be intentional for flexibility.
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/UserField.java (1)
3-3: Remove unused import.
FileFieldValueis imported but not used in this class.-import com.netgrif.application.engine.objects.petrinet.domain.dataset.FileFieldValue;nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FilterField.java (1)
22-25: Remove redundantgetValue()method override.The overridden
getValue()method simply callssuper.getValue()without adding any functionality. This override is unnecessary and can be removed to reduce code complexity.- @Override - public Object getValue() { - return super.getValue(); - }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
application-engine/docker-compose.yml(1 hunks)application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java(10 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/Task.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/domain/User.java(2 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/BooleanField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ButtonField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/CaseField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DataField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DateField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ElasticCase.java(5 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FieldWithAllowedNetsField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FileField.java(2 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FilterField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/I18nField.java(2 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/MapField.java(2 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/NumberField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/UserField.java(2 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/event/events/data/GetDataEvent.java(2 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/ImmediateField.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/CaseField.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/FilterField.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/I18nField.java(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/ImmediateField.java (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ElasticCase.java (1)
Data(24-141)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/CaseField.java (3)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DataField.java (1)
Data(10-29)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FieldWithAllowedNetsField.java (1)
Data(10-26)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/CaseField.java (1)
NoArgsConstructor(8-26)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/CaseField.java (2)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/FilterField.java (1)
NoArgsConstructor(10-32)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/I18nField.java (1)
NoArgsConstructor(12-34)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FieldWithAllowedNetsField.java (2)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DataField.java (1)
Data(10-29)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/CaseField.java (1)
Data(10-23)
🪛 YAMLlint (1.37.1)
application-engine/docker-compose.yml
[error] 70-70: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (24)
application-engine/docker-compose.yml (1)
63-70: Approve repositioning of networks and volumes sections.Moving the
networksandvolumesblocks to the bottom of the file follows common Docker Compose layout conventions and has no functional impact.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 70-70: no new line character at the end of file
(new-line-at-end-of-file)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ButtonField.java (1)
20-23: LGTM! Proper method override implementation.The
getValue()method is correctly implemented with the@Overrideannotation and follows the established pattern for field value access.nae-object-library/src/main/java/com/netgrif/application/engine/objects/event/events/data/GetDataEvent.java (1)
31-31: Excellent null safety improvement.The added null check prevents potential
NullPointerExceptionand provides clear feedback when the message is null. This follows good defensive programming practices.nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DateField.java (1)
26-30: LGTM! Clean implementation of the getValue() standardization.The override correctly returns the
dateValuefield, aligning with the broader effort to standardizegetValue()methods across field types. The implementation is simple, direct, and follows the expected pattern.application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/Task.java (1)
89-89: LGTM! Improved null safety and consistency.The change correctly applies the same null-check pattern used for
userIdon line 88, preventing potentialNullPointerExceptionwhentask.getUser()returns null. This ensures consistent null handling across both user-related field assignments.nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/MapField.java (1)
7-8: LGTM! Proper import consolidation.The import statements are appropriately consolidated to support the new
getValue()method implementation.nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/BooleanField.java (1)
20-23: LGTM! Consistent getValue() implementation.The method correctly overrides the parent's
getValue()method to return the specificbooleanValuefield. This aligns with the standardization effort mentioned in the AI summary across multiple field types.nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/domain/User.java (1)
344-346: LGTM! Good null safety improvement.The early null check prevents
NullPointerExceptionand follows good defensive programming practices.nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/I18nField.java (1)
18-25: LGTM! Clean implementation of translations support.The addition of the
translationsfield and updated constructor properly extends the I18nField functionality.nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/ImmediateField.java (1)
1-27: LGTM! Well-implemented data class.The
ImmediateFieldclass is cleanly designed with:
- Proper
Serializableimplementation with explicitserialVersionUID- Clear constructor that extracts relevant field metadata
- Focused responsibility as a lightweight field representation
The implementation follows best practices and integrates well with the broader elastic domain model enhancements.
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ElasticCase.java (3)
64-64: Good addition of immediateData field.The new
immediateDatafield properly extends the ElasticCase model to include immediate field metadata.
118-118: Clean implementation of immediateData initialization.The constructor properly maps from the Case object's immediate data to
ImmediateFieldobjects using an efficient stream approach.
139-139: Consistent update pattern for immediateData.The update method correctly assigns the
immediateDatafield, maintaining consistency with other field updates in the method.nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/I18nField.java (2)
15-17: LGTM! Constructor update aligns with domain class changes.The updated constructor properly accepts the
translationsparameter and passes it to the superclass, maintaining consistency with the domain layer changes.
30-33: Good use of@Field(type = Flattened)for the translations map.The
Flattenedfield type is the appropriate choice forMap<String, String>in Elasticsearch, allowing efficient storage and querying of key-value pairs.nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/CaseField.java (2)
15-17: Constructor implementation looks correct.The constructor properly delegates to the superclass with both parameters, maintaining the field hierarchy pattern.
19-22:Details
✅ Verification successful
Verify the getValue() return type change.
The getValue() method returns a mutable ArrayList instead of the parent's single string value. While this makes sense for a field that may contain multiple case references, ensure that consuming code expects a List rather than a String.
🏁 Script executed:
#!/bin/bash # Description: Check how CaseField.getValue() is used in the codebase # Expected: Find usages that expect List return type # Search for CaseField getValue() usage rg -A 3 -B 1 "\.getValue\(\)" --type java | rg -A 5 -B 5 "CaseField" # Search for any casting or type assumptions ast-grep --pattern 'CaseField $_ = $$$; $_.getValue()'Length of output: 2715
CaseField.getValue() return type validated – no further action needed
In
ElasticCaseMappingService.transformCaseFieldField, the callString[] referencedCases = ((List<String>) dataField.getValue()).toArray(new String[0]);already casts the returned value to
List<String>. No other usages assume a singleString.
All existing consumers expect a list, so the change is safe.nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/CaseField.java (2)
11-13: Constructor follows the correct pattern.The constructor properly delegates to the superclass, maintaining consistency with other Spring adapter implementations.
15-24: Elasticsearch field mappings are correctly implemented.Both
getFulltextValue()andgetAllowedNets()are properly annotated with@Field(type = Text), which is appropriate for string arrays in Elasticsearch indexing. This follows the same pattern as other adapter classes likeFilterFieldandI18nField.nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/FilterField.java (2)
13-15: Constructor implementation is correct.The constructor properly accepts all required parameters and delegates to the superclass, following the established pattern for Spring adapter classes.
17-31: Elasticsearch field mappings are appropriately configured.The field annotations are correctly chosen:
@Field(type = Text)for string arrays (fulltextValueandallowedNets)@Field(type = Flattened)for the metadata map, which is ideal for arbitrary key-value structures in ElasticsearchThis follows the same pattern as other adapter classes in the codebase.
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java (3)
82-85: New field type support added correctly.The additions for
CaseFieldandFilterFieldtype detection follow the established pattern in the transformation switch logic.
115-115: I18nField transformation enhancement is appropriate.Adding the translations map to the I18nField constructor provides richer data for Elasticsearch indexing, which aligns with the Spring adapter's support for translation mapping.
94-95: Code formatting improvements enhance readability.The method signature formatting changes improve code readability by properly breaking long parameter lists across multiple lines, following Java style guidelines.
Also applies to: 109-111, 130-131, 137-138, 145-146, 167-168, 183-184, 207-208, 215-216, 220-221, 228-229, 255-257, 272-273, 286-287, 299-300, 304-305, 312-313, 317-318, 322-323
...t-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DataField.java
Show resolved
Hide resolved
...vy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy
Outdated
Show resolved
Hide resolved
nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/domain/User.java
Show resolved
Hide resolved
...t-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/UserField.java
Show resolved
Hide resolved
...t-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/I18nField.java
Show resolved
Hide resolved
...in/java/com/netgrif/application/engine/objects/elastic/domain/FieldWithAllowedNetsField.java
Show resolved
Hide resolved
...t-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FileField.java
Show resolved
Hide resolved
| protected Optional<DataField> transformFilterFieldField(com.netgrif.application.engine.objects.workflow.domain.DataField dataField, com.netgrif.application.engine.objects.petrinet.domain.dataset.FilterField netField) { | ||
| String[] allowedNets = dataField.getAllowedNets().toArray(new String[0]); | ||
| Map<String, Object> filterMetadata = dataField.getFilterMetadata(); | ||
| return Optional.of(new com.netgrif.application.engine.adapter.spring.elastic.domain.FilterField(dataField.getValue().toString(),allowedNets, filterMetadata)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for FilterField transformation.
The method assumes getAllowedNets() and getFilterMetadata() methods exist and that the value can be safely converted to string. Add proper null checks and error handling.
protected Optional<DataField> transformFilterFieldField(com.netgrif.application.engine.objects.workflow.domain.DataField dataField, com.netgrif.application.engine.objects.petrinet.domain.dataset.FilterField netField) {
+ if (dataField.getValue() == null || dataField.getAllowedNets() == null) {
+ return Optional.empty();
+ }
String[] allowedNets = dataField.getAllowedNets().toArray(new String[0]);
Map<String, Object> filterMetadata = dataField.getFilterMetadata();
+ if (filterMetadata == null) {
+ filterMetadata = new HashMap<>();
+ }
return Optional.of(new com.netgrif.application.engine.adapter.spring.elastic.domain.FilterField(dataField.getValue().toString(),allowedNets, filterMetadata));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| protected Optional<DataField> transformFilterFieldField(com.netgrif.application.engine.objects.workflow.domain.DataField dataField, com.netgrif.application.engine.objects.petrinet.domain.dataset.FilterField netField) { | |
| String[] allowedNets = dataField.getAllowedNets().toArray(new String[0]); | |
| Map<String, Object> filterMetadata = dataField.getFilterMetadata(); | |
| return Optional.of(new com.netgrif.application.engine.adapter.spring.elastic.domain.FilterField(dataField.getValue().toString(),allowedNets, filterMetadata)); | |
| } | |
| protected Optional<DataField> transformFilterFieldField(com.netgrif.application.engine.objects.workflow.domain.DataField dataField, com.netgrif.application.engine.objects.petrinet.domain.dataset.FilterField netField) { | |
| if (dataField.getValue() == null || dataField.getAllowedNets() == null) { | |
| return Optional.empty(); | |
| } | |
| String[] allowedNets = dataField.getAllowedNets().toArray(new String[0]); | |
| Map<String, Object> filterMetadata = dataField.getFilterMetadata(); | |
| if (filterMetadata == null) { | |
| filterMetadata = new HashMap<>(); | |
| } | |
| return Optional.of(new com.netgrif.application.engine.adapter.spring.elastic.domain.FilterField( | |
| dataField.getValue().toString(), | |
| allowedNets, | |
| filterMetadata | |
| )); | |
| } |
🤖 Prompt for AI Agents
In
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java
around lines 124 to 128, the method transformFilterFieldField assumes
getAllowedNets(), getFilterMetadata(), and dataField.getValue() are non-null and
safely convertible to string. Add null checks for dataField, its allowedNets,
filterMetadata, and value before using them. If any are null, handle gracefully
by returning Optional.empty() or a default value. Also, wrap the conversion to
string in a try-catch block to handle potential exceptions and avoid runtime
errors.
| protected Optional<DataField> transformCaseFieldField(com.netgrif.application.engine.objects.workflow.domain.DataField dataField, com.netgrif.application.engine.objects.petrinet.domain.dataset.CaseField netField) { | ||
| String[] allowedNets = dataField.getAllowedNets().toArray(new String[0]); | ||
| String[] referencedCases = ((List<String>) dataField.getValue()).toArray(new String[0]); | ||
| return Optional.of(new com.netgrif.application.engine.adapter.spring.elastic.domain.CaseField(referencedCases,allowedNets)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add type safety checks for CaseField transformation.
The method assumes dataField.getValue() returns List<String> and that getAllowedNets() exists on the workflow DataField. Add null checks and type verification to prevent runtime exceptions.
protected Optional<DataField> transformCaseFieldField(com.netgrif.application.engine.objects.workflow.domain.DataField dataField, com.netgrif.application.engine.objects.petrinet.domain.dataset.CaseField netField) {
+ if (dataField.getValue() == null || dataField.getAllowedNets() == null) {
+ return Optional.empty();
+ }
+ if (!(dataField.getValue() instanceof List)) {
+ log.error("CaseField value is not a List. Found: " + dataField.getValue().getClass().getCanonicalName());
+ return Optional.empty();
+ }
String[] allowedNets = dataField.getAllowedNets().toArray(new String[0]);
- String[] referencedCases = ((List<String>) dataField.getValue()).toArray(new String[0]);
+ List<?> valueList = (List<?>) dataField.getValue();
+ String[] referencedCases = valueList.stream()
+ .filter(Objects::nonNull)
+ .map(Object::toString)
+ .toArray(String[]::new);
return Optional.of(new com.netgrif.application.engine.adapter.spring.elastic.domain.CaseField(referencedCases,allowedNets));
}🤖 Prompt for AI Agents
In
application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java
around lines 118 to 122, the method transformCaseFieldField assumes
dataField.getValue() returns a List<String> and that getAllowedNets() is
non-null, which can cause runtime exceptions. Add null checks to verify
dataField.getValue() is not null and is an instance of List<?> before casting,
and ensure getAllowedNets() is not null before converting to array. If checks
fail, handle gracefully by returning Optional.empty() or an appropriate
fallback.
.../src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FileField.java (1)
35-45:⚠️ Potential issueCritical: Missing filePath initialization in array constructor.
The array constructor doesn't initialize or populate the
filePatharray, but thegetValue()method expects it to be available when handling multiple files. This will cause aNullPointerExceptionwhengetValue()tries to accessfilePath[i].Apply this fix to initialize the filePath array:
public FileField(FileFieldValue[] values) { super(new String[values.length]); + this.filePath = new String[values.length]; this.fileNameValue = new String[values.length]; this.fileExtensionValue = new String[values.length]; for (int i = 0; i < values.length; i++) { FileNameAndExtension extracted = this.extractFileExtensionFromName(values[i].getName()); this.fileNameValue[i] = extracted.name; this.fileExtensionValue[i] = extracted.extension; + this.filePath[i] = values[i].getPath(); super.fulltextValue[i] = values[i].getName(); } }
🧹 Nitpick comments (4)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/MapField.java (2)
7-8: Improve import specificity and remove unused imports.The wildcard import
java.util.*should be replaced with specific imports for better code clarity. Additionally,java.util.stream.Collectorsis imported but not used anywhere in this file.-import java.util.*; -import java.util.stream.Collectors; +import java.util.Arrays; +import java.util.LinkedHashSet;
36-43: Add null safety for array element access.The method logic is sound, but there's a potential null safety issue when accessing
keyValue[0]without checking if the array element itself is null.public Object getValue() { - if (keyValue != null && keyValue.length == 1) { - return keyValue[0]; + if (keyValue != null && keyValue.length == 1 && keyValue[0] != null) { + return keyValue[0]; } else if (keyValue != null && keyValue.length > 1) { return new LinkedHashSet<>(Arrays.asList(keyValue)); } return null; }application-engine/docker-compose.yml (1)
70-70: Add missing newline at end of file.YAML files should end with a newline character for proper formatting standards compliance.
volumes: minio_data: - driver: local + driver: local +🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 70-70: no new line character at the end of file
(new-line-at-end-of-file)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/UserField.java (1)
3-3: Remove unused import.The
FileFieldValueimport is not used in this file.-import com.netgrif.application.engine.objects.petrinet.domain.dataset.FileFieldValue;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
application-engine/docker-compose.yml(1 hunks)application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy(2 hunks)application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java(10 hunks)application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/Task.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/domain/User.java(2 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/BooleanField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ButtonField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/CaseField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DataField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DateField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ElasticCase.java(5 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FieldWithAllowedNetsField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FileField.java(2 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FilterField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/I18nField.java(2 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/MapField.java(2 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/NumberField.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/UserField.java(2 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/event/events/data/GetDataEvent.java(2 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/ImmediateField.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/CaseField.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/FilterField.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/I18nField.java(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FieldWithAllowedNetsField.java (2)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/CaseField.java (1)
Data(10-23)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DataField.java (1)
Data(10-29)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/FilterField.java (2)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/CaseField.java (1)
NoArgsConstructor(8-26)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/I18nField.java (1)
NoArgsConstructor(12-34)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/UserField.java (2)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FileField.java (2)
AllArgsConstructor(65-69)Data(13-70)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DataField.java (1)
Data(10-29)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/CaseField.java (3)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DataField.java (1)
Data(10-29)nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FieldWithAllowedNetsField.java (1)
Data(10-26)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/CaseField.java (1)
NoArgsConstructor(8-26)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/I18nField.java (2)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/CaseField.java (1)
NoArgsConstructor(8-26)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/FilterField.java (1)
NoArgsConstructor(10-32)
🪛 YAMLlint (1.37.1)
application-engine/docker-compose.yml
[error] 70-70: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (22)
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: Test
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
🔇 Additional comments (28)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/NumberField.java (1)
20-22: LGTM!The implementation is clean and follows the consistent
getValue()pattern established across field classes.nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ButtonField.java (1)
20-23: LGTM!The implementation correctly overrides the parent class method and follows the established pattern for field value access.
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DateField.java (1)
27-30: LGTM! Consistent value accessor implementation.The
getValue()method correctly returns thedateValuefield, providing standardized access to the internal date representation. This aligns well with the broader refactoring to unify value retrieval across different field types.nae-object-library/src/main/java/com/netgrif/application/engine/objects/event/events/data/GetDataEvent.java (1)
30-32: Excellent null safety improvement!The addition of null checking prevents potential
NullPointerExceptionwheneventOutcome.getMessage()returns null. Using a ternary operator to return "NULL" string is a clean defensive programming approach.nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/BooleanField.java (1)
20-23: LGTM! Consistent with standardization pattern.The
getValue()method correctly returns thebooleanValuefield, maintaining consistency with the value accessor pattern being implemented across all field types in the elastic domain model.application-engine/src/main/java/com/netgrif/application/engine/workflow/web/responsebodies/Task.java (1)
89-89: LGTM! Improved null safety and consistency.The change correctly aligns the null-check logic with the pattern used for
userIdon line 88, ensuring we check for the user object's existence before accessing its properties. This prevents potentialNullPointerExceptionwhen callinggetRealmId()on a null user object.application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy (1)
1604-1604:Details
✅ Verification successful
Good change for type consistency.
Capitalizing the
typeparameter ensures consistent handling with the newFilterFieldclass processing. This aligns well with the enhanced metadata support mentioned in the summary.Please verify that existing callers of this method are compatible with the capitalized type values, as this could be a breaking change if downstream code expects the original case.
🏁 Script executed:
#!/bin/bash # Description: Check for potential breaking changes by finding all callers of createFilter method # Expected: Find all usages and verify they handle capitalized type values correctly echo "Searching for createFilter method calls..." rg -A 3 -B 1 "createFilter\s*\(" --type groovy echo -e "\nSearching for specific type values that might be affected..." rg -A 2 -B 2 "(\"case\"|\"task\"|'case'|'task')" --type groovyLength of output: 28811
🏁 Script executed:
#!/bin/bash # Print the createFilter method definition with context rg -n "Case createFilter" -A 20 -B 5 application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovyLength of output: 8013
🏁 Script executed:
#!/bin/bash # Locate DefaultFiltersRunner constants for FILTER_TYPE_CASE and FILTER_TYPE_TASK rg -n "FILTER_TYPE_CASE" -A 3 -B 3 --type groovy rg -n "FILTER_TYPE_TASK" -A 3 -B 3 --type groovyLength of output: 21825
🏁 Script executed:
#!/bin/bash # Locate the DefaultFiltersRunner class file FILTER_RUNNER_FILE=$(rg -l "class DefaultFiltersRunner" --type groovy) # Print the file path echo "Found DefaultFiltersRunner at: $FILTER_RUNNER_FILE" # Show the definitions of FILTER_TYPE_CASE and FILTER_TYPE_TASK rg -n "FILTER_TYPE_CASE" -A1 -B1 "$FILTER_RUNNER_FILE" rg -n "FILTER_TYPE_TASK" -A1 -B1 "$FILTER_RUNNER_FILE"Length of output: 300
🏁 Script executed:
#!/bin/bash # Search for DefaultFiltersRunner definition across groovy and java sources rg -n "class DefaultFiltersRunner" -n application-engine/src rg -n "interface DefaultFiltersRunner" -n application-engine/src rg -l "DefaultFiltersRunner" -n application-engine/srcLength of output: 1025
No breaking changes to
createFiltertype handlingAll existing callers pass in the official
DefaultFiltersRunner.FILTER_TYPE_CASEand…_TASKconstants (whose values are already “Case” and “Task”), so capitalizing the incomingtypeproduces the same strings. No downstream code relies on lowercase literals for filter types.• Named variants (
createCaseFilter,createTaskFilter) use constants
• DeprecatedcreateFilterInMenuoverloads forward constants
• Test code referencing"case"/"task"pertains to transition IDs, not filter typesnae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/domain/User.java (1)
344-346: LGTM! Good null safety improvement.The null check prevents potential NullPointerException when accessing the attributes map.
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/CaseField.java (1)
19-22: Intentional behavior change in getValue() method.The override changes the return type from String (first element from DataField) to List (all elements). This provides access to all full-text values rather than just the first one, which aligns with the case field's purpose.
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FilterField.java (2)
17-20: Good null safety implementation.The constructor properly handles null filterMetadata by initializing it with an empty HashMap, preventing potential issues downstream.
22-25: Consistent getValue() behavior.The delegation to super.getValue() maintains the inherited behavior of returning the first full-text value element, which is appropriate for filter fields.
nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/dataset/ImmediateField.java (2)
22-26: Clean copy constructor implementation.The constructor efficiently copies the essential properties (stringId, name, type) from the Field object, providing a clean way to create immediate field representations.
12-13: Proper serialization support.The serialVersionUID is correctly defined for serialization compatibility.
nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FileField.java (1)
47-55: Good implementation of getValue() method.The method correctly handles both single and multiple file scenarios, returning appropriate
FileFieldValueobjects. The logic is sound and the return types are consistent.nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/I18nField.java (1)
18-30: Well-implemented internationalization enhancement.The addition of the translations map and the getValue() method override is consistent and properly implemented. The constructor correctly initializes all fields, and the getValue() method returns a well-structured I18nString object.
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/CaseField.java (1)
1-26: Consistent Spring adapter implementation.The new CaseField adapter class follows the established pattern correctly. The constructor delegation, field annotations, and getter methods are properly implemented and consistent with other adapter classes in the same package.
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/I18nField.java (1)
15-33: Consistent adapter enhancement for translation support.The constructor update and new getTranslations() method with
@Field(type = Flattened)annotation properly expose the translations map for Elasticsearch indexing. This aligns well with the domain class changes and follows the established adapter pattern.nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/FieldWithAllowedNetsField.java (1)
17-25: Constructor implementations look correct.Both constructors properly delegate to the parent class and initialize the
allowedNetsfield appropriately.nae-object-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/ElasticCase.java (4)
8-8: Import addition looks good.The import for
ImmediateFieldis correctly added to support the new functionality.
64-64: Field addition is well-structured.The
immediateDatafield follows the existing field patterns in the class and is properly typed.
118-118: Constructor initialization is implemented correctly.The stream mapping from
useCase.getImmediateData()to create newImmediateFieldinstances is appropriate and follows functional programming best practices.
139-139: Update method modification is correct.Direct assignment of
immediateDatain the update method is consistent with how other fields are handled.nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/elastic/domain/FilterField.java (1)
10-32: FilterField adapter implementation follows established patterns correctly.The class properly extends the domain
FilterField, delegates constructor calls to the superclass, and applies appropriate Elasticsearch@Fieldannotations:
getFulltextValue()andgetAllowedNets()useTexttype for searchable contentgetFilterMetadata()usesFlattenedtype for complex object mappingThis implementation is consistent with other adapter classes like
CaseFieldandI18nField.application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticCaseMappingService.java (5)
82-85: New field type transformations are properly integrated.The addition of
CaseFieldandFilterFieldtransformations in the conditional chain follows the established pattern and maintains the proper order of type checking.
118-122: CaseField transformation method is implemented correctly.The method properly extracts
allowedNetsandreferencedCasesfrom the data field and creates the appropriate elastic domain object.
124-128: FilterField transformation method is implemented correctly.The method appropriately extracts
allowedNetsandfilterMetadatafrom the data field and creates the corresponding elastic domain object.
115-115: Enhanced I18nField transformation with translations mapping.The addition of the full translations map to the
I18nFieldconstructor enhances the searchable content and is a valuable improvement over the previous implementation that only included keys and values.
94-95: Method signature formatting improvements enhance readability.The reformatting of method signatures across multiple methods improves code readability without altering functionality. This is a positive maintenance change.
Also applies to: 109-111, 130-131, 137-138, 145-146, 167-168, 183-184, 207-208, 215-216, 220-221, 228-229, 255-257, 272-273, 286-287, 299-300, 304-305, 312-313, 317-318, 322-323
...t-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/DataField.java
Show resolved
Hide resolved
...t-library/src/main/java/com/netgrif/application/engine/objects/elastic/domain/UserField.java
Show resolved
Hide resolved
...vy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy
Outdated
Show resolved
Hide resolved
nae-object-library/src/main/java/com/netgrif/application/engine/objects/auth/domain/User.java
Show resolved
Hide resolved
...in/java/com/netgrif/application/engine/objects/elastic/domain/FieldWithAllowedNetsField.java
Show resolved
Hide resolved
Adjusted null and boundary checks in `getValue` methods to prevent runtime exceptions and ensure data integrity. Added validation for filter type in `ActionDelegate` to avoid illegal arguments and improved string handling for robustness.
Description
Implements NAE-2119
Dependencies
No new dependencies were introduced
Third party dependencies
No new dependencies were introduced
Blocking Pull requests
There are no dependencies on other PR
How Has Been This Tested?
This was tested manually and with unit tests.
Test Configuration
Checklist:
Summary by CodeRabbit