Next Version (2.1.0-rc4) #279
Conversation
WalkthroughAdds a hierarchy-recognition feature that builds hasPart/isPartOf relationships from path-like data-entity IDs, introduces DataSetEntity hasPart tracking and parent-aware data-entity addition APIs, updates reader/utilities and payload tracking, upgrades build/workflows, and adds tests for hierarchy and hasPart behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RoCrate
participant HierarchyRecognition
participant FileSystemUtil
participant DataSetEntity
participant Result
User->>RoCrate: createDataEntityFileStructure(config?)
RoCrate->>HierarchyRecognition: new(crate, config)
RoCrate->>HierarchyRecognition: buildHierarchy()
rect rgb(220,240,230)
Note over HierarchyRecognition,FileSystemUtil: Collect & filter file-path entities
HierarchyRecognition->>FileSystemUtil: isFilePath(id)
FileSystemUtil-->>HierarchyRecognition: boolean
end
rect rgb(240,230,240)
Note over HierarchyRecognition: Validate hierarchy (no File contains children)
HierarchyRecognition->>HierarchyRecognition: validateHierarchy()
end
rect rgb(235,235,200)
Note over HierarchyRecognition,DataSetEntity: Optionally create missing folders and build relations
HierarchyRecognition->>HierarchyRecognition: createMissingIntermediateEntities()
HierarchyRecognition->>DataSetEntity: create folder entities (if configured)
HierarchyRecognition->>HierarchyRecognition: buildHierarchyRelationships()
HierarchyRecognition->>Result: addProcessedRelationship(from,to)
end
HierarchyRecognition-->>RoCrate: HierarchyRecognitionResult
RoCrate-->>User: result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Pull Request Test Coverage Report for Build #576Details
💛 - Coveralls |
Previously, Dataset entities were just deserialized into DataEntity instances, making casting impossible. Now we can cast them or retrieve them, and make advantage of their specializations.
…der version and the crate version.
We still use it internally, but I plan to replace it at some point in future. We should be able to generate at least basically valid entities from external providers. And if not we can still work around this with a custom, private subclass of RoCrate.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/main/java/edu/kit/datamanager/ro_crate/util/FileSystemUtil.java (1)
36-61: Consider normalizing parent path for consistency.The method may return
"."for some inputs (e.g.,"./a"returns"."as parent) while returning"./"for root cases. In RO-Crate context where"./"is the conventional representation of the root directory, this inconsistency could cause issues when comparing or using parent paths.Additionally, consider these edge cases:
- Multiple consecutive slashes (e.g.,
"a//b") are not normalized- Absolute paths like
"/a/b"return"./"as the ultimate parent rather than"/"If RO-Crate conventions require
"./"for the root directory, apply this diff to ensure consistency:String parentPath = normalizedPath.substring(0, lastSlash); // If parent is empty, it's root - if (parentPath.isEmpty()) { + if (parentPath.isEmpty() || parentPath.equals(".")) { return "./"; } // For validation, we need to check both with and without trailing slash // since files don't have trailing slash but folders do return parentPath;src/main/java/edu/kit/datamanager/ro_crate/hierarchy/HierarchyRecognitionResult.java (1)
1-51: Consider defensive copies for record components.The record uses mutable collections (Set, Map, List) and provides mutation methods (
addSkippedEntity,addError, etc.). While this works, it deviates from the immutability typically expected of records. Consider whether the components should be wrapped in unmodifiable collections when accessed, or document that this record uses a mutable-builder pattern.Otherwise, the implementation is clean and provides a clear API for hierarchy recognition results.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
.github/workflows/codeql-analysis.yml(3 hunks).github/workflows/gradle.yml(1 hunks).github/workflows/publishRelease.yml(1 hunks).gitignore(1 hunks)build.gradle(4 hunks)gradle/wrapper/gradle-wrapper.properties(1 hunks)src/main/java/edu/kit/datamanager/ro_crate/Crate.java(4 hunks)src/main/java/edu/kit/datamanager/ro_crate/RoCrate.java(19 hunks)src/main/java/edu/kit/datamanager/ro_crate/entities/AbstractEntity.java(3 hunks)src/main/java/edu/kit/datamanager/ro_crate/entities/data/DataSetEntity.java(6 hunks)src/main/java/edu/kit/datamanager/ro_crate/hierarchy/HierarchyRecognition.java(1 hunks)src/main/java/edu/kit/datamanager/ro_crate/hierarchy/HierarchyRecognitionConfig.java(1 hunks)src/main/java/edu/kit/datamanager/ro_crate/hierarchy/HierarchyRecognitionResult.java(1 hunks)src/main/java/edu/kit/datamanager/ro_crate/hierarchy/package-info.java(1 hunks)src/main/java/edu/kit/datamanager/ro_crate/payload/RoCratePayload.java(1 hunks)src/main/java/edu/kit/datamanager/ro_crate/reader/CrateReader.java(2 hunks)src/main/java/edu/kit/datamanager/ro_crate/util/FileSystemUtil.java(1 hunks)src/test/java/edu/kit/datamanager/ro_crate/crate/HasPartTest.java(1 hunks)src/test/java/edu/kit/datamanager/ro_crate/entities/data/DataSetEntityTest.java(1 hunks)src/test/java/edu/kit/datamanager/ro_crate/entities/data/RootDataEntityTest.java(1 hunks)src/test/java/edu/kit/datamanager/ro_crate/hierarchy/HierarchyRecognitionTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-29T16:54:57.200Z
Learnt from: Pfeil
Repo: kit-data-manager/ro-crate-java PR: 204
File: gradlew:226-250
Timestamp: 2024-11-29T16:54:57.200Z
Learning: In this project, `gradlew` scripts are considered external dependencies and should not be modified during code reviews.
Applied to files:
gradle/wrapper/gradle-wrapper.properties
🧬 Code graph analysis (8)
src/main/java/edu/kit/datamanager/ro_crate/Crate.java (3)
src/main/java/edu/kit/datamanager/ro_crate/entities/AbstractEntity.java (1)
AbstractEntity(30-648)src/main/java/edu/kit/datamanager/ro_crate/entities/data/DataEntity.java (1)
DataEntity(20-177)src/main/java/edu/kit/datamanager/ro_crate/entities/data/DataSetEntity.java (1)
DataSetEntity(20-140)
src/main/java/edu/kit/datamanager/ro_crate/entities/data/DataSetEntity.java (1)
src/main/java/edu/kit/datamanager/ro_crate/entities/serializers/HasPartSerializer.java (1)
HasPartSerializer(17-48)
src/main/java/edu/kit/datamanager/ro_crate/hierarchy/HierarchyRecognitionResult.java (2)
src/main/java/edu/kit/datamanager/ro_crate/entities/data/DataEntity.java (1)
DataEntity(20-177)src/main/java/edu/kit/datamanager/ro_crate/entities/data/DataSetEntity.java (1)
DataSetEntity(20-140)
src/test/java/edu/kit/datamanager/ro_crate/hierarchy/HierarchyRecognitionTest.java (3)
src/main/java/edu/kit/datamanager/ro_crate/RoCrate.java (1)
RoCrate(44-693)src/main/java/edu/kit/datamanager/ro_crate/entities/data/DataEntity.java (1)
DataEntity(20-177)src/main/java/edu/kit/datamanager/ro_crate/entities/data/DataSetEntity.java (1)
DataSetEntity(20-140)
src/main/java/edu/kit/datamanager/ro_crate/hierarchy/HierarchyRecognition.java (3)
src/main/java/edu/kit/datamanager/ro_crate/entities/data/DataEntity.java (1)
DataEntity(20-177)src/main/java/edu/kit/datamanager/ro_crate/entities/data/DataSetEntity.java (1)
DataSetEntity(20-140)src/main/java/edu/kit/datamanager/ro_crate/util/FileSystemUtil.java (1)
FileSystemUtil(10-122)
src/main/java/edu/kit/datamanager/ro_crate/RoCrate.java (3)
src/main/java/edu/kit/datamanager/ro_crate/hierarchy/HierarchyRecognition.java (1)
HierarchyRecognition(13-208)src/main/java/edu/kit/datamanager/ro_crate/entities/data/DataSetEntity.java (1)
DataSetEntity(20-140)src/main/java/edu/kit/datamanager/ro_crate/special/JsonUtilFunctions.java (1)
JsonUtilFunctions(15-143)
src/test/java/edu/kit/datamanager/ro_crate/crate/HasPartTest.java (5)
src/main/java/edu/kit/datamanager/ro_crate/RoCrate.java (1)
RoCrate(44-693)src/main/java/edu/kit/datamanager/ro_crate/entities/data/DataSetEntity.java (1)
DataSetEntity(20-140)src/main/java/edu/kit/datamanager/ro_crate/entities/data/FileEntity.java (1)
FileEntity(9-45)src/main/java/edu/kit/datamanager/ro_crate/reader/Readers.java (1)
Readers(10-77)src/main/java/edu/kit/datamanager/ro_crate/writer/Writers.java (1)
Writers(9-42)
src/main/java/edu/kit/datamanager/ro_crate/reader/CrateReader.java (1)
src/main/java/edu/kit/datamanager/ro_crate/entities/data/DataSetEntity.java (1)
DataSetEntity(20-140)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build (windows-latest, 21)
- GitHub Check: build (windows-latest, 17)
- GitHub Check: build (windows-latest, 17)
- GitHub Check: build (windows-latest, 21)
🔇 Additional comments (22)
.gitignore (1)
1-4: Good addition for project consistency.Adding Eclipse IDE ignore patterns (.classpath, .project, .settings, .tmp) alongside existing VSCode and IntelliJ patterns improves the .gitignore to support developers across different IDEs. The placement before the VSCode section is appropriate.
src/main/java/edu/kit/datamanager/ro_crate/hierarchy/package-info.java (1)
1-9: HierarchyRecognition class verified.The
HierarchyRecognitionclass exists in the hierarchy package, confirming the{@link}reference in the package-info.java documentation will resolve correctly. The documentation is clear, well-structured, and follows Java conventions. No issues found..github/workflows/gradle.yml (1)
28-28: GitHub Actions updated to v5.All workflow actions have been updated to their v5 releases, which are the current stable versions. These are straightforward maintenance updates with no behavioral changes.
Also applies to: 30-30, 37-37, 42-42
gradle/wrapper/gradle-wrapper.properties (1)
3-3: Gradle patch version bump (8.14.2 → 8.14.3).Minor patch version update with no breaking changes expected. This aligns with routine maintenance and the Java 17 baseline configuration.
.github/workflows/publishRelease.yml (1)
11-11: GitHub Actions updated to v5.Consistent update with gradle.yml workflow. Actions are updated to current stable v5 releases.
Also applies to: 13-13
.github/workflows/codeql-analysis.yml (1)
41-41: GitHub Actions updated to v5 and CodeQL actions to v4.Routine workflow maintenance updates. CodeQL analysis remains functionally equivalent with the newer action versions.
Also applies to: 45-45, 59-59, 72-72
build.gradle (5)
27-30: Java 17 baseline configuration added.The explicit
java { sourceCompatibility = VERSION_17; targetCompatibility = VERSION_17 }configuration properly establishes Java 17 as the minimum target. Verify all transitive dependencies support Java 17, particularly newer versions like Jackson 2.20.1 and JUnit 6.0.1.
16-16: Maven publish plugin major version bump (8.13.1 → 9.0.0).The freefair maven-publish-java plugin has been upgraded to a major version. Confirm:
- The publishing configuration in
gradle/profile-release.gradleremains compatible with v9.0.0.- The publishToSonatype and closeAndReleaseSonatypeStagingRepository tasks (used in publishRelease.yml line 18) work as expected.
19-19: Group identifier format updated to 'edu.kit.datamanager'.The explicit group format change appears administrative. Confirm this matches the publishing configuration and existing artifact coordinates for Maven Central.
43-43: Routine minor/patch dependency updates.Jackson (2.20.1), commons-io (2.21.0), json-compare (7.1), commons-validator (1.10.0), titanium-json-ld (1.7.0), json-schema-validator (1.5.9), and JaCoCo (0.8.14) have all received minor or patch version updates. These are standard maintenance upgrades with no expected breaking changes.
Also applies to: 57-57, 61-61, 63-63, 67-67, 69-69, 194-194
48-48: Based on the comprehensive verification I've conducted, I can now provide a definitive assessment of the JUnit upgrade. The test suite appears to be compatible with JUnit 6.0.1. Here's what I verified:The migration from JUnit 5 to JUnit 6 stands out for its smooth integration, with annotations, extensions, and philosophy remaining intact.
Key findings:
Test API Compatibility: All test annotations found in the codebase (@test, @ParameterizedTest, @beforeeach, @tempdir, assertions from org.junit.jupiter.api.Assertions) remain fully compatible with JUnit 6.0.1 without code changes required.
Java Version Requirement: JUnit 6 requires Java 17 or higher at runtime. The project must ensure Java 17+ is configured as the build target.
CSV Parsing: Although overall behavior remains largely compatible with JUnit 5, there are notable changes: the lineSeparator attribute in @CsvFileSource has been removed, and extra characters after a closing quote are no longer allowed in @CsvSource and @CsvFileSource. The test suite uses parameterized tests extensively, but I found no evidence of edge cases that would trigger these stricter parsing rules.
Removed APIs: Removed modules (junit-platform-runner, junit-platform-jfr) and deprecated reflection utilities are not used in the provided test files.
Build Configuration: The use of
testImplementation(platform("org.junit:junit-bom:6.0.1"))in build.gradle is the correct pattern for managing JUnit dependency versions.Recommended verification steps:
- Confirm the project's Java version target is 17 or higher
- Run the full test suite to ensure all tests pass
- Validate any CSV-driven tests don't use edge cases (e.g., malformed CSV syntax that was previously lenient)
src/main/java/edu/kit/datamanager/ro_crate/entities/AbstractEntity.java (2)
52-56: LGTM! Documentation improvement.The Javadoc clarifies the purpose of the
linkedTofield for faster entity removal.
632-638: LGTM! Type synchronization from JSON.The logic correctly extracts
@typevalues from JSON properties and synchronizes them into the internal type set, handling both array and single-string cases.src/main/java/edu/kit/datamanager/ro_crate/reader/CrateReader.java (1)
118-161: LGTM! Dataset entity recognition implemented correctly.The refactored logic correctly distinguishes between Dataset and regular data entities, instantiating the appropriate class (DataSetEntity vs DataEntity) based on the
@typeproperty. File association is consistently handled through the builder pattern.src/main/java/edu/kit/datamanager/ro_crate/entities/data/DataSetEntity.java (4)
24-30: LGTM! Documentation added.The Javadoc clearly explains the purpose and serialization behavior of the
hasPartfield.
42-44: LGTM! Input validation added.Filtering blank strings from
hasPartensures data quality.
53-55: LGTM! Defensive null/empty check added.The validation prevents null or empty IDs from being added to
hasPart.
58-78: LGTM! API improvement with proper deprecation.The new
hasPart(String)method follows better naming conventions, and the oldhasInHasPart(String)is properly deprecated withforRemoval = true.src/main/java/edu/kit/datamanager/ro_crate/hierarchy/HierarchyRecognitionConfig.java (1)
1-43: LGTM! Clean configuration record with fluent API.The record-based configuration provides a clear, immutable API for hierarchy recognition settings. The no-arg constructor with sensible defaults (all false) and fluent builder methods follow best practices.
src/test/java/edu/kit/datamanager/ro_crate/entities/data/DataSetEntityTest.java (1)
75-75: LGTM! Test updated for new API.The test correctly uses the new
hasPart(id)method instead of the deprecatedhasInHasPart(id).src/test/java/edu/kit/datamanager/ro_crate/entities/data/RootDataEntityTest.java (1)
49-50: LGTM! Tests updated for new API.The tests correctly use the new
hasPart(id)method instead of the deprecatedhasInHasPart(id).src/main/java/edu/kit/datamanager/ro_crate/payload/RoCratePayload.java (1)
24-28: LGTM! Documentation added for associatedItems field.The Javadoc clearly explains the purpose of the
associatedItemsmap for optimizing entity removal operations.
src/main/java/edu/kit/datamanager/ro_crate/entities/AbstractEntity.java
Outdated
Show resolved
Hide resolved
src/main/java/edu/kit/datamanager/ro_crate/entities/data/DataSetEntity.java
Show resolved
Hide resolved
src/main/java/edu/kit/datamanager/ro_crate/hierarchy/HierarchyRecognition.java
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Summary by CodeRabbit