feat/dispatcher-metadata-destination #127
Conversation
WalkthroughThis PR updates the dispatching mechanism to support dynamic destination resolution using metadata. The modifications include adding a new method for resolving destination binding and updating the dispatch method signature to accept an additional destination parameter. Enhanced error handling is implemented via a new exception constructor and improved metadata parsing logic. A new helper class and record encapsulate metadata-related functionality, while configuration and test files are updated accordingly to support metadata-based dispatching. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant D as DispatcherUseCase
participant M as MetadataHelper
C->>D: processFile(file, useCase)
alt UseCase requires metadata
D->>M: parseMetadataFile(inputStream)
M-->>D: Metadata (jsonNode, indexFields)
D->>D: resolveDestinationBinding(useCase, file) returns destination
end
D->>D: dispatchFile(useCase, file, destination)
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
🚧 Files skipped from review as they are similar to previous changes (10)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
dispatch-service/src/test/java/de/muenchen/oss/swim/dispatcher/application/usecase/DispatcherUseCaseTest.java (2)
54-56: Consider renaming 'METATA_PATH' to 'METADATA_PATH'.
The nameMETATA_PATHappears to be a minor typo. A clearer name can improve readability.- public static final String METATA_PATH = "test/inProcess/path/test.json"; + public static final String METADATA_PATH = "test/inProcess/path/test.json";
131-149: Test coverage for metadata-based destination.
This new test method thoroughly checks the flow of reading metadata and dispatching to a dynamic destination. Consider adding a negative test scenario for missing or incomplete destination data in the metadata to enhance robustness further.dispatch-service/src/main/java/de/muenchen/oss/swim/dispatcher/application/usecase/DispatcherUseCase.java (3)
30-30: Usage ofStrings.isNotBlank.
You are usingorg.apache.logging.log4j.util.Strings. Ensure this is consistent with string utility usage across the codebase to maintain uniformity.
135-135: Assigning default destination from action.
Usingaction.name()as a placeholder ensures a non-null destination. Proceed carefully if future actions have unexpected names.
202-228: Comprehensive destination resolution method.
This method handles metadata loading, parsing, fallback behavior, and exceptions. Consider logging how the fallback destination is chosen if metadata is absent or invalid for traceability.dispatch-service/src/main/java/de/muenchen/oss/swim/dispatcher/domain/helper/MetadataHelper.java (1)
48-66: Consider additional validation for empty arrays and valuesThe method validates the structure well, but consider the following improvements:
- Check if the indexFieldsNode array is empty, as an empty array might not be meaningful
- Consider validating the value similar to how you validate the key, depending on your business requirements
if (indexFieldsNode == null || !indexFieldsNode.isArray()) { throw new MetadataException("Missing or invalid '" + METADATA_INDEX_FIELDS_KEY + "' in metadata JSON"); } +if (indexFieldsNode.isEmpty()) { + throw new MetadataException("Empty '" + METADATA_INDEX_FIELDS_KEY + "' in metadata JSON"); +} final Map<String, String> indexFields = new HashMap<>(); for (final JsonNode indexField : indexFieldsNode) { final String key = indexField.path(METADATA_KEY_KEY).asText(); final String value = indexField.path(METADATA_VALUE_KEY).asText(); - if (!key.isEmpty()) { + if (!key.isEmpty() && !value.isEmpty()) { indexFields.put(key, value); } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
dispatch-service/src/test/resources/files/protocol.csvis excluded by!**/*.csv
📒 Files selected for processing (12)
dispatch-service/src/main/java/de/muenchen/oss/swim/dispatcher/application/usecase/DispatcherUseCase.java(6 hunks)dispatch-service/src/main/java/de/muenchen/oss/swim/dispatcher/configuration/SwimDispatcherProperties.java(1 hunks)dispatch-service/src/main/java/de/muenchen/oss/swim/dispatcher/domain/exception/MetadataException.java(1 hunks)dispatch-service/src/main/java/de/muenchen/oss/swim/dispatcher/domain/helper/MetadataHelper.java(1 hunks)dispatch-service/src/main/java/de/muenchen/oss/swim/dispatcher/domain/model/Metadata.java(1 hunks)dispatch-service/src/main/java/de/muenchen/oss/swim/dispatcher/domain/model/UseCase.java(1 hunks)dispatch-service/src/main/resources/application.yml(1 hunks)dispatch-service/src/test/java/de/muenchen/oss/swim/dispatcher/application/usecase/DispatcherUseCaseTest.java(6 hunks)dispatch-service/src/test/java/de/muenchen/oss/swim/dispatcher/application/usecase/ProtocolProcessingUseCaseTest.java(2 hunks)dispatch-service/src/test/java/de/muenchen/oss/swim/dispatcher/domain/model/FileTest.java(1 hunks)dispatch-service/src/test/resources/application-test.yml(1 hunks)dispatch-service/src/test/resources/files/example-metadata-destination.json(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- dispatch-service/src/test/resources/files/example-metadata-destination.json
- dispatch-service/src/main/java/de/muenchen/oss/swim/dispatcher/domain/model/Metadata.java
- dispatch-service/src/test/java/de/muenchen/oss/swim/dispatcher/domain/model/FileTest.java
- dispatch-service/src/test/java/de/muenchen/oss/swim/dispatcher/application/usecase/ProtocolProcessingUseCaseTest.java
🔇 Additional comments (21)
dispatch-service/src/main/java/de/muenchen/oss/swim/dispatcher/configuration/SwimDispatcherProperties.java (1)
76-81: Property addition looks clean and well-documented.The
metadataDispatchBindingKeyproperty is properly annotated with@NotBlankto ensure it's configured and includes clear Javadoc that explains its purpose and relationship with theUseCase.isDestinationViaMetadata()method.dispatch-service/src/main/java/de/muenchen/oss/swim/dispatcher/domain/model/UseCase.java (1)
39-44: The newdestinationViaMetadataflag is well-documented.Good documentation that explains this flag's relationship with the
SwimDispatcherProperties.getMetadataDispatchBindingKey()method and clarifies the fallback behavior when set to true. The default value offalseis appropriate for backward compatibility.dispatch-service/src/main/resources/application.yml (1)
93-94: New configuration property is properly categorized.The new
metadata-dispatch-binding-keyconfiguration is well-organized with a relevant comment section labeled "# metadata", making it easy to locate related settings in the configuration file.dispatch-service/src/main/java/de/muenchen/oss/swim/dispatcher/domain/exception/MetadataException.java (1)
8-11: Good addition of cause-aware constructor.Adding a constructor that accepts both a message and a cause follows standard Java exception design patterns and improves error reporting by preserving the original exception chain. This will be helpful for debugging and error logging.
dispatch-service/src/test/resources/application-test.yml (1)
16-24: Add tests or validation for the new use-case configuration.
These lines introduce thetest-meta-destuse case withdestination-via-metadata. Ensure this new configuration is tested, especially to confirm that metadata-based dispatching behaves as expected.dispatch-service/src/test/java/de/muenchen/oss/swim/dispatcher/application/usecase/DispatcherUseCaseTest.java (7)
23-23: Import usage check.
ImportingObjectMapperhere seems valid, ensuring JSON parsing is properly handled.
34-34: Import usage check.
ImportingMetadataHelperhere integrates well with metadata parsing in the new tests.
49-49: Clarify test configuration.
IncludingMetadataHelperin the Spring context ensures it's injected and tested end-to-end. This is a good approach for verifying metadata-based dispatch.
117-119: Good use of mocks for metadata existence and URL retrieval.
The test setup adequately mocks file existence and presigned URL generation for metadata. This appears consistent with the new dispatch logic.
123-124: Verification logic appears correct.
Verifying the call todispatchFilewith the correct arguments ensures the underlying logic is functioning as intended.
162-162: Properly testing the missing metadata file scenario.
Returningfalsefor file existence and expectingMetadataExceptionis a valid check for handling absent metadata.
178-178: Ensuring zero calls to the legacy dispatch method signature.
This verification helps confirm that the updated dispatch logic with metadata is used instead of the older signature.dispatch-service/src/main/java/de/muenchen/oss/swim/dispatcher/application/usecase/DispatcherUseCase.java (6)
14-14: EnsureFileSystemAccessExceptionusage is consistent.
This import is suitable for handling I/O-level issues when reading metadata files. Confirm that existing code references it correctly.
17-17: MetadataHelper import usage.
This helper centralizes metadata parsing logic, aligning with the new dynamic destination approach.
20-20: Metadata model import.
ImportingMetadatarecord ensures typed access to metadata fields. No concerns here.
22-23: New I/O imports.
Imports forIOExceptionandInputStreamare valid for reading the metadata stream.
46-46: NewmetadataHelperdependency.
InjectingMetadataHelperfacilitates metadata-based dispatch resolution. Looks correctly integrated.
144-145: Resolving destination from metadata.
Switching to metadata-based dispatch when action isDISPATCHis a clean, centralized approach.dispatch-service/src/main/java/de/muenchen/oss/swim/dispatcher/domain/helper/MetadataHelper.java (3)
32-39: LGTM - Method handles parsing and error handling wellThe
parseMetadataFilemethod is well-structured with good error handling by wrapping IOExceptions in a domain-specific MetadataException. The NotNull annotation ensures proper input validation.
59-60: Be aware of potential null handling in JSON pathsUsing
path()withasText()will convert missing nodes to empty strings rather than null. This is working as intended with your empty check, but it's worth noting this behavior in case you expect different handling for missing fields in the future.
1-67: Overall well-structured component for metadata handlingThe MetadataHelper class is well designed and implemented with:
- Clear separation of concerns
- Good error handling
- Appropriate use of Spring and Lombok annotations
- Well-documented methods with Javadoc
- Constants for reusable string values
This will integrate well with the dispatcher functionality described in the PR objectives.
Description
Implement dispatch destination via metadata file.
Reference
Issues closes #27
Summary by CodeRabbit
New Features
MetadataHelperclass for parsing metadata files and extracting index fields.Metadatato encapsulate related data elements.Bug Fixes
Tests