Next Version (v2.1.0-rc2) #255
Conversation
WalkthroughThis update introduces a significant refactor and expansion of the RO-Crate Java library. It replaces and renames several core reader and writer strategy classes, introduces new utility and interface abstractions, and adds comprehensive example-driven and specification-based test suites. The update also removes deprecated or redundant classes, clarifies exception handling, and enhances testability and maintainability through interface-based test design. Documentation and build scripts are updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CrateWriter
participant WriteZipStrategy
participant WriteZipStreamStrategy
participant OutputStream
participant ZipOutputStream
User->>CrateWriter: save(crate, "path/to/zip")
CrateWriter->>WriteZipStrategy: save(crate, "path/to/zip")
WriteZipStrategy->>OutputStream: open FileOutputStream("path/to/zip")
WriteZipStrategy->>WriteZipStreamStrategy: save(crate, OutputStream)
WriteZipStreamStrategy->>ZipOutputStream: wrap OutputStream
WriteZipStreamStrategy->>ZipOutputStream: write metadata, entities, preview
ZipOutputStream-->>WriteZipStreamStrategy: close
WriteZipStreamStrategy-->>WriteZipStrategy: return
WriteZipStrategy-->>CrateWriter: return
CrateWriter-->>User: return
sequenceDiagram
participant User
participant CrateReader
participant ReadZipStrategy
participant FileSystemUtil
User->>CrateReader: readCrate("path/to/zip")
CrateReader->>ReadZipStrategy: readMetadataJson("path/to/zip")
ReadZipStrategy->>FileSystemUtil: mkdirOrDeleteContent(tempFolder)
ReadZipStrategy->>ReadZipStrategy: extract ZIP to tempFolder
ReadZipStrategy->>ReadZipStrategy: locate metadata JSON (root or subfolder)
ReadZipStrategy-->>CrateReader: return ObjectNode (metadata)
CrateReader-->>User: return RoCrate
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 30th. To opt out, configure ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
…duplicates Fix adding id property duplicates
… the reader better
…OfSpecificationV1p1Test
The example does not have it, so we need to remove it afterwards.
…utable Make specification examples (from readme) executable
Not recursive and limited to 50 folders to avoid load vulnerabilities.
… deprecation linter errors
…pStreamStrategyTest
…view.saveAllToStream
…files-in-zip-streams fix: enforce correct subfolder name for additional files in StaticPreview.saveAllToStream
… Write accordingly. This eases importing reader and writer strategies of the same kind (source/destination type).
…WriteZipStreamStrategy
…rateWriter type parameter
Support .ELN-style crates in all zip readers and writers
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
♻️ Duplicate comments (1)
src/test/java/edu/kit/datamanager/ro_crate/preview/PreviewTest.java (1)
256-262: Repeat of unclosed stream / implicit charsetThe same concerns as above apply here for the CustomPreview variant. Apply the
same fix or factor the logic into a reusable helper.
🧹 Nitpick comments (26)
src/main/java/edu/kit/datamanager/ro_crate/writer/WriteFolderStrategy.java (1)
71-79: Minor naming inconsistency in method name.The method name
savetoFileuses inconsistent camel case naming convention. In Java, the convention is to use camelCase for method names.-private void savetoFile(DataEntity entity, File file) throws IOException { +private void saveToFile(DataEntity entity, File file) throws IOException {Don't forget to update the method call on line 67 as well.
README.md (1)
59-64: Excellent addition of example-driven documentation.Directing users to well-documented examples and tests is a great approach to documentation. The LearnByExampleTest and ExamplesOfSpecificationV1p1Test provide valuable resources for understanding the library.
Minor style issue: In American English, "afterward" is the preferred spelling (without the 's').
🧰 Tools
🪛 LanguageTool
[locale-violation] ~59-~59: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ...ed crate can of course also be modified afterwards. Take a look at our further documentati...(AFTERWARDS_US)
src/test/java/edu/kit/datamanager/ro_crate/reader/ElnFileFormatTest.java (2)
26-27: Fix typo in Javadoc comment.The word "methode" should be "method".
- * An implementation test may use this methode to provide a subset of the + * An implementation test may use this method to provide a subset of the
43-43: Missing URL text in the @see tagThe hyperlink in the Javadoc @see tag is empty.
- * @see <a href="https://github.com/TheELNConsortium/TheELNFileFormat"></a> + * @see <a href="https://github.com/TheELNConsortium/TheELNFileFormat">The ELN File Format</a>src/test/java/edu/kit/datamanager/ro_crate/writer/ZipStreamWriterTest.java (1)
9-11: Enhance class Javadoc comment.The class Javadoc only includes an @author tag but lacks a description of the class purpose and functionality.
/** + * Test class for writing RO-Crates to ZIP streams. + * Implements both CommonWriterTest and ElnFileWriterTest interfaces to + * verify standard and ELN-style ZIP stream writing functionality. + * * @author jejkal */src/main/java/edu/kit/datamanager/ro_crate/preview/CratePreview.java (2)
24-31: Deprecation notice in Javadoc can mis-lead future implementersStating that the default implementation “relies on deprecated methods” is useful now but fragile long-term.
Once the deprecated methods are actually removed this Javadoc will become stale and the interface will break without a clear migration hint.Consider:
- Explicitly marking a removal version (e.g. “will be removed in v3.0”).
- Adding
@implNoteor@todoexplaining the intended future contract.
41-43: Raw-typeCrateWriterleads to unchecked warnings
new CrateWriter<>( … )relies on the diamond operator to infer<?>, effectively using a raw type.
Specify the generic argument (e.g.<Path>or whatever the writer expects) to avoid unchecked‐operation warnings and improve type safety.- new CrateWriter<>(new WriteFolderStrategy().disablePreview()) + new CrateWriter<Path>(new WriteFolderStrategy().disablePreview())src/test/java/edu/kit/datamanager/ro_crate/reader/TestableReaderStrategy.java (2)
14-14: Package-private interface limits reuseThe interface has default (package-private) visibility, which prevents tests located in other packages (e.g.
reader.zip) from implementing it.
Making itpubliccosts nothing and future-proofs the test API:-interface TestableReaderStrategy<SOURCE_T, READER_STRATEGY extends GenericReaderStrategy<SOURCE_T>> { +public interface TestableReaderStrategy<SOURCE_T, + READER_STRATEGY extends GenericReaderStrategy<SOURCE_T>> {
33-39: Clarify behaviour ofnewReaderStrategyWithTmpThe contract does not specify what happens when a strategy doesn’t support a custom temp directory.
Add a note to either:
- Throw
UnsupportedOperationException, or- Fall back to the default tmp dir.
Documenting this avoids inconsistent behaviour across implementations.
src/test/java/edu/kit/datamanager/ro_crate/writer/CommonWriterTest.java (1)
51-52: Unconditional debug output clutters test logs
HelpFunctions.printFileTree(...)is called unconditionally and will spam CI logs.
Wrap these calls in a logger at DEBUG level or guard them with an environment flag so that regular test runs stay quiet.src/test/java/edu/kit/datamanager/ro_crate/writer/TestableWriterStrategy.java (1)
44-84: Comprehensive test structure creation method.The
createManualCrateStructuremethod creates a complete test environment with files and directories needed for testing. It's well-documented and handles the creation of a complex directory structure that tests can validate against.One minor issue: There's a hard-coded reference to
ZipStreamWriterTest.classon line 54 that should ideally be replaced with a more generic reference likethis.getClass()orTestableWriterStrategy.classto avoid coupling to a specific implementation.- InputStream fileJson = ZipStreamWriterTest.class + InputStream fileJson = TestableWriterStrategy.class .getResourceAsStream("/json/crate/fileAndDir.json");src/main/java/edu/kit/datamanager/ro_crate/reader/ReadZipStreamStrategy.java (3)
58-62: Typo in documentation – “Crates” → “Creates”The Javadoc in the no-arg constructor contains a small typo which may trip up IDE quick-docs and automated doc generators.
- * Crates an instance with the default configuration. + * Creates an instance with the default configuration.
119-137: Missing preservation of file attributesThe extraction loop copies file contents but silently drops executable flags, last-modified times, etc.
If downstream tooling expects those attributes (e.g. scripts in a crate) they will be lost. Consider copying basic attributes:Files.setLastModifiedTime(extractedFile.toPath(), FileTime.fromMillis(localFileHeader.getLastModifiedTime()));
ZipInputStreamexposes most metadata throughLocalFileHeader.
140-141:forceDeleteOnExitmay exhaust the JVM’s delete-on-exit queueFor long-running services extracting many crates, the JVM keeps every path until shutdown ⇒ potential memory leak.
Expose an explicitcleanup()method or document that callers should delete the folder when done.src/test/java/edu/kit/datamanager/ro_crate/writer/ElnFileWriterTest.java (1)
69-71: Avoid debug output in unit tests
HelpFunctions.printFileTree(...)writes to STDOUT on every run and clutters CI logs.
Replace with logging at debug level or remove entirely once the test is stable.- HelpFunctions.printFileTree(correctCrate); - HelpFunctions.printFileTree(extractionPath); + // Debug output removed to keep test logs concisesrc/main/java/edu/kit/datamanager/ro_crate/writer/WriteZipStreamStrategy.java (2)
102-110: I/O loop can be simplified & made more efficientWriting the metadata JSON via a 4 kB buffer is fine, but
ZipOutputStreamaccepts byte arrays directly—avoiding the extra loop reduces code and GC pressure for small files.- byte[] buff = new byte[4096]; - int readLen; - zipStream.putNextEntry(zipParameters); - try (InputStream inputStream = new ByteArrayInputStream(str.getBytes(StandardCharsets.UTF_8))) { - while ((readLen = inputStream.read(buff)) != -1) { - zipStream.write(buff, 0, readLen); - } - } + zipStream.putNextEntry(zipParameters); + byte[] data = str.getBytes(StandardCharsets.UTF_8); + zipStream.write(data);
118-149: Temporary preview folder is not deleted on exceptionIf an exception occurs between preview generation and the
try { forceDelete(...) }block, the folder remains on disk.
Wrap creation & deletion intry … finallyto guarantee cleanup:File tmpPreviewFolder = …; try { FileUtils.forceMkdir(tmpPreviewFolder); preview.get().generate(crate, tmpPreviewFolder); … } finally { FileUtils.deleteQuietly(tmpPreviewFolder); }src/test/java/edu/kit/datamanager/ro_crate/preview/PreviewTest.java (1)
173-177: Same try-with-resources issue forZipOutputStreamReplicate the pattern recommended above for
staticPreviewSaveToZipStream.
All three new*ZipStreamtests now share identical boiler-plate; consider
extracting a small helper to reduce duplication.src/test/java/edu/kit/datamanager/ro_crate/reader/CommonReaderTest.java (2)
45-53: Explicit UTF-8 when writing fixture filesSeveral helper tests rely on
Charset.defaultCharset(), which varies between
environments and can break assertions on systems not using UTF-8.-FileUtils.writeStringToFile(csvPath.toFile(), "Dummy content", Charset.defaultCharset()); +FileUtils.writeStringToFile(csvPath.toFile(), "Dummy content", StandardCharsets.UTF_8);Consider replacing all occurrences of
defaultCharset()in this
interface-based test suite withStandardCharsets.UTF_8for deterministic
behaviour.
30-35: Interface naming & capitalisation of test methodsMinor style nit:
- Method names
TestWithFileWithLocation/TestWithFileWithLocationAddEntity
start with a capital letter, differing from the usual camelCase convention
in the rest of the suite and JUnit examples.Renaming is optional but would improve consistency.
src/test/java/edu/kit/datamanager/ro_crate/examples/LearnByExampleTest.java (1)
185-186: PreferStandardCharsets.UTF_8to magic stringsUsing the constant provided by the JDK avoids typos and is slightly faster as it skips a charset lookup.
-FileUtils.write(lovelyFilePointer, "My great experiment 001", "UTF-8"); +FileUtils.write(lovelyFilePointer, "My great experiment 001", StandardCharsets.UTF_8);Remember to add
import java.nio.charset.StandardCharsets;.src/main/java/edu/kit/datamanager/ro_crate/reader/ReadZipStrategy.java (1)
100-106: Extraction folder is scheduled for deletion once per JVM run
FileUtils.forceDeleteOnExit(folder)silently keeps adding shutdown hooks; repeated calls for many crates can exhaust the JVM’s shutdown-hook limit (at most 2147483647 but practically lower for memory).
If the reader is used often, prefer a single, process-wide cleanup strategy (e.g., delete on close, or maintain one shared temp root and register a single shutdown hook).src/test/java/edu/kit/datamanager/ro_crate/examples/ExamplesOfSpecificationV1p1Test.java (2)
110-112: Broken Javadoc link – duplicated “.json” extensionThe anchor path ends with
files-and-folders.json.json, resulting in a 404 in generated Javadoc.-</a> (<a href="src/test/resources/spec-v1.1-example-json-files/files-and-folders.json.json">location in repo</a>) +</a> (<a href="src/test/resources/spec-v1.1-example-json-files/files-and-folders.json">location in repo</a>)
158-160: Minor typo in documentation
"twp FileEntities"→"two FileEntities"
Correcting small typos keeps the example tests tidy and professional.src/main/java/edu/kit/datamanager/ro_crate/entities/AbstractEntity.java (2)
599-615:setAllUnsafelacks null-checks and exposes internal state
setAllUnsafe(ObjectNode properties)directly assigns the incoming node to the builder’s field without:
- Checking for
null– anullargument will trigger an NPE later.- Deep-copying – callers retain a live reference and may inadvertently mutate the builder after the fact.
- public T setAllUnsafe(ObjectNode properties) { - // This will currently only print errors. - AbstractEntity.entityValidation.entityValidation(properties); - this.properties = properties; + public T setAllUnsafe(ObjectNode properties) { + if (properties == null) { + return self(); // nothing to do + } + // Validate, but ignore result – errors are printed inside the validator + AbstractEntity.entityValidation.entityValidation(properties); + + // Keep a defensive copy to avoid outside mutation + this.properties = properties.deepCopy(); this.relatedItems.addAll(JsonUtilFunctions.getIdPropertiesFromJsonNode(properties)); return self(); }
396-404: Minor Javadoc nit – repeated method reference
@deprecatedblock references{@link #setAllIfValid(ObjectNode)}twice; the second occurrence should point to{@link #setAllUnsafe(ObjectNode)}.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jar
📒 Files selected for processing (72)
.github/workflows/gradle.yml(0 hunks)README.md(2 hunks)build.gradle(2 hunks)gradle/wrapper/gradle-wrapper.properties(1 hunks)gradlew(2 hunks)gradlew.bat(1 hunks)src/main/java/edu/kit/datamanager/ro_crate/RoCrate.java(2 hunks)src/main/java/edu/kit/datamanager/ro_crate/context/RoCrateMetadataContext.java(2 hunks)src/main/java/edu/kit/datamanager/ro_crate/entities/AbstractEntity.java(4 hunks)src/main/java/edu/kit/datamanager/ro_crate/entities/contextual/JsonDescriptor.java(2 hunks)src/main/java/edu/kit/datamanager/ro_crate/entities/data/DataEntity.java(0 hunks)src/main/java/edu/kit/datamanager/ro_crate/entities/data/DataSetEntity.java(0 hunks)src/main/java/edu/kit/datamanager/ro_crate/entities/validation/JsonSchemaValidation.java(1 hunks)src/main/java/edu/kit/datamanager/ro_crate/externalproviders/dataentities/ImportFromZenodo.java(1 hunks)src/main/java/edu/kit/datamanager/ro_crate/externalproviders/personprovider/OrcidProvider.java(1 hunks)src/main/java/edu/kit/datamanager/ro_crate/preview/AutomaticPreview.java(3 hunks)src/main/java/edu/kit/datamanager/ro_crate/preview/CratePreview.java(2 hunks)src/main/java/edu/kit/datamanager/ro_crate/preview/CustomPreview.java(3 hunks)src/main/java/edu/kit/datamanager/ro_crate/preview/StaticPreview.java(3 hunks)src/main/java/edu/kit/datamanager/ro_crate/reader/CrateReader.java(7 hunks)src/main/java/edu/kit/datamanager/ro_crate/reader/FolderReader.java(1 hunks)src/main/java/edu/kit/datamanager/ro_crate/reader/GenericReaderStrategy.java(1 hunks)src/main/java/edu/kit/datamanager/ro_crate/reader/ReadFolderStrategy.java(1 hunks)src/main/java/edu/kit/datamanager/ro_crate/reader/ReadZipStrategy.java(4 hunks)src/main/java/edu/kit/datamanager/ro_crate/reader/ReadZipStreamStrategy.java(1 hunks)src/main/java/edu/kit/datamanager/ro_crate/reader/Readers.java(3 hunks)src/main/java/edu/kit/datamanager/ro_crate/reader/ZipReader.java(1 hunks)src/main/java/edu/kit/datamanager/ro_crate/reader/ZipStreamStrategy.java(0 hunks)src/main/java/edu/kit/datamanager/ro_crate/util/FileSystemUtil.java(1 hunks)src/main/java/edu/kit/datamanager/ro_crate/util/ZipStreamUtil.java(1 hunks)src/main/java/edu/kit/datamanager/ro_crate/writer/CrateWriter.java(2 hunks)src/main/java/edu/kit/datamanager/ro_crate/writer/ElnFormatWriter.java(1 hunks)src/main/java/edu/kit/datamanager/ro_crate/writer/FolderStrategy.java(0 hunks)src/main/java/edu/kit/datamanager/ro_crate/writer/FolderWriter.java(1 hunks)src/main/java/edu/kit/datamanager/ro_crate/writer/GenericWriterStrategy.java(1 hunks)src/main/java/edu/kit/datamanager/ro_crate/writer/WriteFolderStrategy.java(1 hunks)src/main/java/edu/kit/datamanager/ro_crate/writer/WriteZipStrategy.java(1 hunks)src/main/java/edu/kit/datamanager/ro_crate/writer/WriteZipStreamStrategy.java(1 hunks)src/main/java/edu/kit/datamanager/ro_crate/writer/Writers.java(3 hunks)src/main/java/edu/kit/datamanager/ro_crate/writer/ZipStrategy.java(0 hunks)src/main/java/edu/kit/datamanager/ro_crate/writer/ZipStreamStrategy.java(0 hunks)src/main/java/edu/kit/datamanager/ro_crate/writer/ZipWriter.java(1 hunks)src/test/java/edu/kit/datamanager/ro_crate/HelpFunctions.java(4 hunks)src/test/java/edu/kit/datamanager/ro_crate/crate/BuilderSpec12Test.java(2 hunks)src/test/java/edu/kit/datamanager/ro_crate/crate/ReadAndWriteTest.java(1 hunks)src/test/java/edu/kit/datamanager/ro_crate/crate/TestRemoveAddContext.java(2 hunks)src/test/java/edu/kit/datamanager/ro_crate/crate/preview/PreviewCrateTest.java(3 hunks)src/test/java/edu/kit/datamanager/ro_crate/crate/realexamples/RealTest.java(2 hunks)src/test/java/edu/kit/datamanager/ro_crate/entities/contextual/ContextualEntityTest.java(2 hunks)src/test/java/edu/kit/datamanager/ro_crate/examples/ExamplesOfSpecificationV1p1Test.java(1 hunks)src/test/java/edu/kit/datamanager/ro_crate/examples/LearnByExampleTest.java(1 hunks)src/test/java/edu/kit/datamanager/ro_crate/preview/PreviewTest.java(4 hunks)src/test/java/edu/kit/datamanager/ro_crate/reader/CommonReaderTest.java(8 hunks)src/test/java/edu/kit/datamanager/ro_crate/reader/ElnFileFormatTest.java(1 hunks)src/test/java/edu/kit/datamanager/ro_crate/reader/FolderReaderTest.java(1 hunks)src/test/java/edu/kit/datamanager/ro_crate/reader/RoCrateReaderSpec12Test.java(2 hunks)src/test/java/edu/kit/datamanager/ro_crate/reader/TestableReaderStrategy.java(1 hunks)src/test/java/edu/kit/datamanager/ro_crate/reader/ZipReaderTest.java(2 hunks)src/test/java/edu/kit/datamanager/ro_crate/reader/ZipStreamReaderTest.java(2 hunks)src/test/java/edu/kit/datamanager/ro_crate/util/FileSystemUtilTest.java(1 hunks)src/test/java/edu/kit/datamanager/ro_crate/writer/CommonWriterTest.java(7 hunks)src/test/java/edu/kit/datamanager/ro_crate/writer/ElnFileWriterTest.java(1 hunks)src/test/java/edu/kit/datamanager/ro_crate/writer/FolderWriterTest.java(1 hunks)src/test/java/edu/kit/datamanager/ro_crate/writer/TestableWriterStrategy.java(1 hunks)src/test/java/edu/kit/datamanager/ro_crate/writer/ZipStreamStrategyTest.java(0 hunks)src/test/java/edu/kit/datamanager/ro_crate/writer/ZipStreamWriterTest.java(1 hunks)src/test/java/edu/kit/datamanager/ro_crate/writer/ZipWriterTest.java(1 hunks)src/test/resources/spec-v1.1-example-json-files/complete-workflow-example.json(1 hunks)src/test/resources/spec-v1.1-example-json-files/file-author-location.json(1 hunks)src/test/resources/spec-v1.1-example-json-files/files-and-folders.json(1 hunks)src/test/resources/spec-v1.1-example-json-files/minimal.json(1 hunks)src/test/resources/spec-v1.1-example-json-files/web-based-data-entities.json(1 hunks)
💤 Files with no reviewable changes (8)
- .github/workflows/gradle.yml
- src/test/java/edu/kit/datamanager/ro_crate/writer/ZipStreamStrategyTest.java
- src/main/java/edu/kit/datamanager/ro_crate/entities/data/DataSetEntity.java
- src/main/java/edu/kit/datamanager/ro_crate/writer/FolderStrategy.java
- src/main/java/edu/kit/datamanager/ro_crate/entities/data/DataEntity.java
- src/main/java/edu/kit/datamanager/ro_crate/writer/ZipStreamStrategy.java
- src/main/java/edu/kit/datamanager/ro_crate/reader/ZipStreamStrategy.java
- src/main/java/edu/kit/datamanager/ro_crate/writer/ZipStrategy.java
🧰 Additional context used
🧬 Code Graph Analysis (15)
src/main/java/edu/kit/datamanager/ro_crate/reader/FolderReader.java (5)
src/main/java/edu/kit/datamanager/ro_crate/writer/FolderWriter.java (1)
Deprecated(10-11)src/main/java/edu/kit/datamanager/ro_crate/reader/ZipReader.java (1)
Deprecated(23-47)src/main/java/edu/kit/datamanager/ro_crate/writer/ZipWriter.java (1)
Deprecated(9-10)src/main/java/edu/kit/datamanager/ro_crate/writer/RoCrateWriter.java (1)
Deprecated(9-15)src/main/java/edu/kit/datamanager/ro_crate/reader/RoCrateReader.java (1)
Deprecated(15-20)
src/main/java/edu/kit/datamanager/ro_crate/reader/ZipReader.java (5)
src/main/java/edu/kit/datamanager/ro_crate/reader/FolderReader.java (1)
Deprecated(11-12)src/main/java/edu/kit/datamanager/ro_crate/writer/FolderWriter.java (1)
Deprecated(10-11)src/main/java/edu/kit/datamanager/ro_crate/writer/ZipWriter.java (1)
Deprecated(9-10)src/main/java/edu/kit/datamanager/ro_crate/writer/RoCrateWriter.java (1)
Deprecated(9-15)src/main/java/edu/kit/datamanager/ro_crate/reader/RoCrateReader.java (1)
Deprecated(15-20)
src/main/java/edu/kit/datamanager/ro_crate/preview/CustomPreview.java (1)
src/main/java/edu/kit/datamanager/ro_crate/util/ZipStreamUtil.java (1)
ZipStreamUtil(14-93)
src/test/java/edu/kit/datamanager/ro_crate/crate/realexamples/RealTest.java (1)
src/main/java/edu/kit/datamanager/ro_crate/reader/Readers.java (1)
Readers(10-77)
src/main/java/edu/kit/datamanager/ro_crate/preview/StaticPreview.java (1)
src/main/java/edu/kit/datamanager/ro_crate/util/ZipStreamUtil.java (1)
ZipStreamUtil(14-93)
src/main/java/edu/kit/datamanager/ro_crate/reader/ReadFolderStrategy.java (1)
src/main/java/edu/kit/datamanager/ro_crate/objectmapper/MyObjectMapper.java (1)
MyObjectMapper(13-27)
src/test/java/edu/kit/datamanager/ro_crate/writer/ZipWriterTest.java (2)
src/main/java/edu/kit/datamanager/ro_crate/RoCrate.java (1)
RoCrate(43-529)src/main/java/edu/kit/datamanager/ro_crate/writer/Writers.java (1)
Writers(9-42)
src/test/java/edu/kit/datamanager/ro_crate/writer/CommonWriterTest.java (1)
src/test/java/edu/kit/datamanager/ro_crate/HelpFunctions.java (1)
HelpFunctions(27-178)
src/test/java/edu/kit/datamanager/ro_crate/util/FileSystemUtilTest.java (1)
src/main/java/edu/kit/datamanager/ro_crate/util/FileSystemUtil.java (1)
FileSystemUtil(11-72)
src/test/java/edu/kit/datamanager/ro_crate/writer/TestableWriterStrategy.java (5)
src/main/java/edu/kit/datamanager/ro_crate/RoCrate.java (1)
RoCrate(43-529)src/main/java/edu/kit/datamanager/ro_crate/entities/data/DataSetEntity.java (1)
DataSetEntity(17-97)src/main/java/edu/kit/datamanager/ro_crate/entities/data/FileEntity.java (1)
FileEntity(9-45)src/main/java/edu/kit/datamanager/ro_crate/preview/AutomaticPreview.java (1)
AutomaticPreview(20-89)src/main/java/edu/kit/datamanager/ro_crate/preview/PreviewGenerator.java (1)
PreviewGenerator(16-85)
src/main/java/edu/kit/datamanager/ro_crate/util/FileSystemUtil.java (1)
src/main/java/edu/kit/datamanager/ro_crate/preview/CustomPreviewModel.java (1)
File(97-125)
src/test/java/edu/kit/datamanager/ro_crate/writer/ZipStreamWriterTest.java (2)
src/main/java/edu/kit/datamanager/ro_crate/RoCrate.java (1)
RoCrate(43-529)src/main/java/edu/kit/datamanager/ro_crate/writer/Writers.java (1)
Writers(9-42)
src/main/java/edu/kit/datamanager/ro_crate/reader/ReadZipStreamStrategy.java (3)
src/main/java/edu/kit/datamanager/ro_crate/entities/contextual/JsonDescriptor.java (1)
JsonDescriptor(14-95)src/main/java/edu/kit/datamanager/ro_crate/objectmapper/MyObjectMapper.java (1)
MyObjectMapper(13-27)src/main/java/edu/kit/datamanager/ro_crate/util/FileSystemUtil.java (1)
FileSystemUtil(11-72)
src/main/java/edu/kit/datamanager/ro_crate/reader/ReadZipStrategy.java (3)
src/main/java/edu/kit/datamanager/ro_crate/entities/contextual/JsonDescriptor.java (1)
JsonDescriptor(14-95)src/main/java/edu/kit/datamanager/ro_crate/objectmapper/MyObjectMapper.java (1)
MyObjectMapper(13-27)src/main/java/edu/kit/datamanager/ro_crate/util/FileSystemUtil.java (1)
FileSystemUtil(11-72)
src/test/java/edu/kit/datamanager/ro_crate/reader/ZipReaderTest.java (2)
src/main/java/edu/kit/datamanager/ro_crate/writer/Writers.java (1)
Writers(9-42)src/main/java/edu/kit/datamanager/ro_crate/reader/Readers.java (1)
Readers(10-77)
🪛 LanguageTool
README.md
[locale-violation] ~59-~59: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ...ed crate can of course also be modified afterwards. Take a look at our further documentati...
(AFTERWARDS_US)
🪛 markdownlint-cli2 (0.17.2)
README.md
41-41: Spaces inside code span elements
null
(MD038, no-space-in-code)
🔇 Additional comments (103)
src/test/java/edu/kit/datamanager/ro_crate/crate/TestRemoveAddContext.java (2)
9-9: Appropriate import addition.The import of
java.io.IOExceptionis correctly added to support the updated method signature.
21-21: Proper exception declaration.The method signature change to include
throws IOExceptionis appropriate sincereadCrate()could encounter I/O errors when reading files from the filesystem. This change improves error propagation and aligns with the broader enhancement of exception handling across the codebase.src/test/java/edu/kit/datamanager/ro_crate/crate/BuilderSpec12Test.java (2)
7-7: Appropriate import addition for exception handling.The addition of the
IOExceptionimport supports the updated method signature that now handles this exception type, aligning with the library's enhanced IO exception handling approach.
41-41: Correctly updated method signature to match refactored reader interfaces.The method signature now properly declares
IOExceptionin addition toURISyntaxException, which aligns with the underlying refactored reader infrastructure. This ensures that IO exceptions from thereadCrate()method on line 43 are properly handled at the test method level.build.gradle (3)
40-42: Library upgrade confirmed.Jackson library has been updated to version 2.19.0 from the previous 2.18.3 version. This update should provide bug fixes and new features available in the latest version.
70-70: Template engine upgrade.JTE template engine has been updated from 3.2.0 to 3.2.1, which should include minor improvements and bug fixes.
74-77: Good practice: Enabling deprecation warnings.Adding
-Xlint:deprecationwill help identify usage of deprecated APIs during compilation. This aligns with the broader refactoring changes in the codebase and will help maintain code quality as deprecated classes are phased out.gradle/wrapper/gradle-wrapper.properties (1)
3-3: Gradle version upgrade.Upgrading from Gradle 8.13 to 8.14 ensures the project stays current with the latest build tool improvements and bug fixes. This change is properly coordinated with the wrapper script changes in gradlew and gradlew.bat.
gradlew.bat (1)
73-77: Wrapper execution mechanism updated.The execution mechanism for the Gradle wrapper has been updated to use the
-jaroption directly rather than setting the class path and specifying the main class. This aligns with the changes in the Unix shell script and the Gradle version upgrade to 8.14.gradlew (2)
117-117: Empty classpath initialization updated.The CLASSPATH variable is now initialized with escaped empty quotes, which is consistent with the changes in the Windows batch script.
215-217: Wrapper execution mechanism updated.The execution mechanism for the Gradle wrapper has been updated to use the
-jaroption directly rather than specifying the main class. This aligns with the changes in the Windows batch script and the Gradle version upgrade to 8.14.src/main/java/edu/kit/datamanager/ro_crate/util/FileSystemUtil.java (1)
44-52: Implementation looks correctThe
ensureTrailingSlashmethod properly handles null/empty inputs and correctly adds a trailing slash when needed.src/test/java/edu/kit/datamanager/ro_crate/util/FileSystemUtilTest.java (2)
11-23: Test coverage for ensureTrailingSlash looks goodThe parameterized test ensures that trailing slashes are properly handled for various input paths.
25-30: Test for null input is handled correctlyThe test properly verifies that null input is returned as null, matching the implementation's behavior.
src/main/java/edu/kit/datamanager/ro_crate/util/ZipStreamUtil.java (1)
14-14: Class renaming is appropriate for clarityThe class has been renamed from
ZipUtiltoZipStreamUtil, which better reflects its purpose of working with ZIP streams. This naming is more consistent with the overall refactoring pattern observed in the codebase.src/main/java/edu/kit/datamanager/ro_crate/reader/ZipReader.java (1)
21-24: Deprecation documentation is clear and appropriateThe updated deprecation notice correctly points users to the new
ReadZipStrategyclass, and the class now extends this new strategy class. This aligns with the broader refactoring effort visible in the codebase, where similar reader/writer classes are being updated with the same pattern.src/main/java/edu/kit/datamanager/ro_crate/writer/ZipWriter.java (1)
7-10: Correctly implemented inheritance update and deprecation noticeThe changes properly update the class to extend the new
WriteZipStrategyinstead of the deprecatedZipStrategy, with a corresponding update to the deprecation notice. This is consistent with the broader refactoring pattern in the codebase where older strategy classes are being replaced with more appropriately named versions.src/main/java/edu/kit/datamanager/ro_crate/reader/FolderReader.java (1)
9-12: Correctly implemented inheritance update and deprecation noticeThe changes properly update the class to extend the new
ReadFolderStrategyinstead of the deprecatedFolderStrategy, with a corresponding update to the deprecation notice. This aligns with the consistent refactoring pattern across the codebase.src/test/java/edu/kit/datamanager/ro_crate/crate/ReadAndWriteTest.java (1)
52-52: Appropriate exception handling updateAdding
throws IOExceptionto the test method correctly aligns with the broader changes in the codebase where reading methods now explicitly propagate IO exceptions. This ensures compatibility with the updated reader classes that now declare IOException on their methods.src/main/java/edu/kit/datamanager/ro_crate/writer/FolderWriter.java (1)
8-11: Correctly implemented inheritance update and deprecation noticeThe changes properly update the class to extend the new
WriteFolderStrategyinstead of the deprecatedFolderStrategy, with a corresponding update to the deprecation notice. This follows the consistent refactoring pattern seen throughout the codebase.src/main/java/edu/kit/datamanager/ro_crate/entities/validation/JsonSchemaValidation.java (1)
63-63: Improved error reporting during validation.Great enhancement to print individual error messages for each validation failure rather than just a generic message. This will make it much easier to debug validation issues.
src/main/java/edu/kit/datamanager/ro_crate/externalproviders/personprovider/OrcidProvider.java (1)
75-75: Updated to use setAllUnsafe for more permissive property setting.This change aligns with the broader codebase update to use
setAllUnsafeinstead ofsetAllwhen populating entity properties from external JSON sources. The unsafe variant allows the object to be populated even if some properties don't pass validation.src/main/java/edu/kit/datamanager/ro_crate/preview/StaticPreview.java (2)
3-3: Updated import to use the new ZipStreamUtil class.This change aligns with the broader refactoring of ZIP handling utilities in the codebase.
67-69: Updated to use ZipStreamUtil methods for ZIP stream operations.The code now uses the newer
ZipStreamUtilclass methods and standardizes the folder name to "ro-crate-preview_files" in the ZIP file, ensuring consistent structure regardless of the source folder name.However, there appears to be an inconsistency between this method and
saveAllToZip. InsaveAllToZip, there's a null check forthis.metadataHtmlbefore adding it to the ZIP file, but this check is missing insaveAllToStream.Could you verify that
this.metadataHtmlis always non-null whensaveAllToStreamis called, or consider adding a null check to maintain consistency with other methods?src/main/java/edu/kit/datamanager/ro_crate/externalproviders/dataentities/ImportFromZenodo.java (1)
115-115: Updated to use setAllUnsafe for more permissive property setting.Similar to the changes in OrcidProvider, this update switches from
setAlltosetAllUnsafewhen building entities from external JSON sources. This change allows the entity builder to set properties even if they don't strictly comply with validation rules, which is appropriate for handling external data that might not perfectly match the internal schema.Also applies to: 120-120
src/main/java/edu/kit/datamanager/ro_crate/entities/contextual/JsonDescriptor.java (2)
16-17: Visibility changes to constants improve extensibility.The increased visibility of these constants (
CONFORMS_TOfrom private to protected andIDfrom protected to public) enables better access from subclasses and external code that needs to reference these values. This change supports enhanced entity construction and validation workflows in the broader refactoring context of the PR.
42-43: Documentation formatting improved.The change from a blank line to a proper paragraph tag
<p>in the Javadoc comment follows better documentation standards.src/main/java/edu/kit/datamanager/ro_crate/writer/Writers.java (3)
22-22: Strategy class naming is now more intuitive.The change from the old strategy class to
WriteFolderStrategyaligns with the broader refactoring of writer strategies in the codebase, providing clearer naming that reflects the purpose of each class.
31-31: Strategy class naming is now more intuitive.The change from the old strategy class to
WriteZipStreamStrategyfollows the same naming pattern, creating a consistent naming scheme across writer strategies.
40-40: Strategy class naming is now more intuitive.The change from the old strategy class to
WriteZipStrategycompletes the consistent renaming of writer strategies.src/main/java/edu/kit/datamanager/ro_crate/preview/AutomaticPreview.java (4)
3-3: Updated import to use renamed utility class.The import change from the old utility class to
ZipStreamUtilis part of a broader refactoring to standardize ZIP stream handling utilities across the codebase.
13-14: Documentation improvement with hyperlink.The Javadoc comment has been enhanced with a proper HTML link tag, which improves documentation by making the reference to the rochtml tool clickable.
69-69: Code simplification in FileWriter instantiation.Removed redundant
new File(...)wrapper around the file path string, which simplifies the code without changing functionality.
75-75: Updated method call to use renamed utility.Changed to use
ZipStreamUtil.addFileToZipStreamto align with the updated utility class name, ensuring consistent ZIP stream handling across the codebase.src/main/java/edu/kit/datamanager/ro_crate/preview/CustomPreview.java (4)
5-5: Updated import to use renamed utility class.The import change from the old utility class to
ZipStreamUtilaligns with similar changes in other preview classes, standardizing ZIP stream handling utilities.
56-56: Simplified type handling in JSON parsing.Removed redundant explicit cast to
JsonNode, which simplifies the code and relies on the method's return type.
199-199: Code simplification in FileWriter instantiation.Removed redundant
new File(...)wrapper around the file path string, similar to the change inAutomaticPreview.
205-205: Updated method call to use renamed utility.Changed to use
ZipStreamUtil.addFileToZipStreamto align with the renamed utility class, ensuring consistent ZIP stream handling across preview implementations.src/main/java/edu/kit/datamanager/ro_crate/reader/Readers.java (1)
22-75: Clean renaming of strategy classes in factory methods.The changes consistently update all references to the old strategy classes (
ZipStreamStrategy,FolderStrategy,ZipStrategy) with their renamed counterparts (ReadZipStreamStrategy,ReadFolderStrategy,ReadZipStrategy). This includes both JavaDoc@seeannotations and actual instantiations in the factory methods.src/main/java/edu/kit/datamanager/ro_crate/RoCrate.java (1)
351-361: Well-implemented identifier property support.The new
addIdentifiermethod adds valuable functionality to the builder, allowing users to assign identifiers (like DOIs) to RO-Crates. The implementation follows good practices by trimming whitespace and maintaining the builder pattern for method chaining. The method is also properly documented with JavaDoc.src/test/java/edu/kit/datamanager/ro_crate/reader/RoCrateReaderSpec12Test.java (1)
6-6: Improved exception handling in test method.The method now properly declares that it throws
IOException, which aligns with exception handling changes in the reader strategy classes. This makes the method's contract more explicit and allows for proper propagation of IO-related exceptions during crate reading.Also applies to: 31-31
src/test/java/edu/kit/datamanager/ro_crate/crate/realexamples/RealTest.java (2)
25-25: Improved imports and class formatting.Added static import for
assertNotNulland updated class declaration to block style, enhancing code readability and consistency.Also applies to: 27-28
32-36: Enhanced test robustness with null checks.The test now directly uses
Readers.newFolderReader()for streamlined crate reading and adds important assertions to verify that both the crate object and the PersonEntity returned fromOrcidProvider.getPersonare not null. These changes improve test reliability by explicitly validating object creation.Also applies to: 49-49
src/main/java/edu/kit/datamanager/ro_crate/writer/GenericWriterStrategy.java (3)
5-5: Added IOException import for checked exception handling.This import supports the new throws declaration in the save method.
11-13: Improved generic type parameter naming.Renaming from
DESTINATIONtoDESTINATION_TYPEfollows Java conventions by clearly indicating this is a type parameter rather than a variable name.
20-20: Enhanced error handling with explicit IOException declaration.Adding
throws IOExceptionto the method signature ensures that implementing classes properly propagate IO errors to callers, forcing them to handle potential failures during crate saving operations.src/main/java/edu/kit/datamanager/ro_crate/context/RoCrateMetadataContext.java (3)
116-119: Improved null-safety with path() instead of get().Using
path("@type")instead ofget("@type")adds robustness by returning aMissingNoderather thannullwhen the property doesn't exist, preventing potentialNullPointerExceptionduring conversion.
180-180: Enhanced resource management with try-with-resources.Using try-with-resources for the HTTP client ensures proper resource cleanup even if an exception occurs during execution, preventing potential resource leaks.
185-185: Simplified error logging with printf.Replacing
System.err.println(String.format(...))with the more conciseSystem.err.printf(...)improves code readability while maintaining the same functionality.src/test/resources/spec-v1.1-example-json-files/files-and-folders.json (1)
1-38: Well-structured RO-Crate example demonstrating file and folder organization.This example JSON-LD file correctly implements the RO-Crate 1.1 specification, showing:
- Proper metadata descriptor with conformance statement
- Root dataset with hasPart relationships to nested elements
- File representation with detailed metadata (name, size, format, description)
- Directory representation as a Dataset with minimal metadata
This addition provides valuable documentation and test fixtures for validating RO-Crate handling capabilities.
src/main/java/edu/kit/datamanager/ro_crate/reader/CrateReader.java (2)
20-20: Added explicit IOException declaration for better error handling.The method signature now properly declares that it throws IOException, requiring callers to handle potential IO failures when reading crates. This improves error propagation throughout the application.
Also applies to: 87-89
125-125: Switched to setAllUnsafe() for improved performance during crate reading.Changed multiple instances of
setAll()tosetAllUnsafe()when building entities. This change is appropriate since:
- The reader is working with JSON that was presumably valid when created
- Skipping validation during reconstruction can improve performance
- The approach is applied consistently across all entity building in the class
This change aligns with the broader pattern of separating validated and unvalidated property setting throughout the codebase.
Also applies to: 139-139, 261-261, 339-339
src/main/java/edu/kit/datamanager/ro_crate/reader/ReadFolderStrategy.java (3)
17-17: Good class renaming for clarity and consistency.The renaming from
FolderStrategytoReadFolderStrategyimproves code clarity by explicitly indicating this class's purpose. This follows the standardized naming convention being applied across reader strategies in this refactoring.
20-20: Improved exception handling approach.Declaring
IOExceptionin the method signature rather than catching it internally is a better practice. This gives callers more control over exception handling and makes the error conditions more visible in the API contract.
24-24: Good use of defensive copying.The implementation correctly uses
deepCopy()to ensure that modifications to the returned ObjectNode won't affect the original parsed tree. This is an important defensive programming technique that preserves immutability.src/test/resources/spec-v1.1-example-json-files/web-based-data-entities.json (1)
1-39: Excellent addition of specification-compliant example JSON.This example clearly demonstrates how to structure a RO-Crate with both local and web-based file entities, providing valuable reference material for testing and documentation. The file correctly follows the RO-Crate 1.1 specification format and includes appropriate metadata properties for each entity.
src/test/resources/spec-v1.1-example-json-files/minimal.json (1)
1-27: Well-structured minimal example for RO-Crate 1.1.This minimal but complete example provides an excellent reference for the essential components of a valid RO-Crate. It clearly demonstrates the required structure including context, metadata file entry, root dataset, and external entity references. This will be valuable for both new users learning the format and for baseline testing of the library functionality.
src/test/java/edu/kit/datamanager/ro_crate/writer/FolderWriterTest.java (2)
14-14: Good architectural shift to interface-based testing.Changing from extending an abstract class to implementing an interface aligns with modern Java practices and provides greater flexibility. This interface-based approach enables better composition and reduces tight coupling between test components.
17-20: Appropriate method visibility for interface compliance.The visibility change from protected to public for these methods appropriately fulfills the interface contract requirements. The implementation logic remains sound, properly leveraging the Writers factory and FileUtils for directory copying.
Also applies to: 23-25
src/test/resources/spec-v1.1-example-json-files/file-author-location.json (1)
1-47: Well-structured RO-Crate 1.1 example fileThis example correctly demonstrates a complete RO-Crate structure with metadata, data entities, and contextual entities. It showcases important RO-Crate features including:
- Proper metadata file declaration with conformance statement
- Root dataset with hasPart relationships to data entities
- Linking data entities to contextual entities (author and location)
- External identifier usage (GeoNames URI for location)
This provides an excellent reference file for testing and documentation.
src/main/java/edu/kit/datamanager/ro_crate/reader/GenericReaderStrategy.java (3)
5-5: Added necessary import for IOExceptionAdding the IOException import supports the updated method signatures that now properly declare IO exceptions.
11-13: Improved generic type parameter namingRenaming the generic type parameter from 'T' to 'SOURCE_TYPE' makes the code more self-documenting by clearly indicating the parameter's purpose.
20-20: Enhanced exception handling with IOException declarationMethods now properly declare that they throw IOException, making potential IO failures explicit in the contract. This change improves error transparency and forces implementers to handle IO exceptions appropriately.
Also applies to: 28-28
src/test/java/edu/kit/datamanager/ro_crate/crate/preview/PreviewCrateTest.java (1)
25-25: Updated test methods to declare IOExceptionTest methods now properly declare that they throw IOException, aligning with the changes in the underlying code. This ensures consistent exception handling throughout the codebase.
Also applies to: 36-36, 50-50
src/main/java/edu/kit/datamanager/ro_crate/writer/CrateWriter.java (4)
7-8: Added necessary import for IOExceptionAdding the IOException import supports the updated method signatures that now properly declare IO exceptions.
13-13: Added Javadoc for generic type parameterAdded documentation for the DESTINATION_TYPE parameter, improving code clarity and documentation.
15-15: Improved generic type parameter namingRenaming from 'DESTINATION' to 'DESTINATION_TYPE' makes the code more self-documenting and consistent with other strategy interfaces (like GenericReaderStrategy with SOURCE_TYPE).
Also applies to: 17-17, 19-19
29-29: Enhanced exception handling with IOException declarationThe save method now properly declares that it throws IOException, making potential IO failures explicit. This change improves error transparency and ensures consistent exception handling across the codebase.
src/main/java/edu/kit/datamanager/ro_crate/writer/WriteFolderStrategy.java (2)
24-41: Implementation looks good with appropriate deprecation.The class correctly implements the
GenericWriterStrategy<String>interface and includes a deprecated method with proper annotations. The inclusion offorRemoval = trueandsince = "2.1.0"provides clear guidance to users about the method's lifecycle.
43-69: Complete implementation with proper resource handling.The save method properly creates the destination directory, handles JSON formatting, manages input streams correctly with proper closing, and processes untracked files and data entities. The method signature correctly declares
IOExceptionfor transparent error handling.src/test/java/edu/kit/datamanager/ro_crate/writer/ZipWriterTest.java (3)
9-11: Good refactoring to use interfaces instead of inheritance.The change from extending a test class to implementing test interfaces follows good object-oriented design principles, promoting composition over inheritance and enabling more flexible test configurations.
14-17: Properly updated method signature.The method signature now correctly throws IOException and uses the Writers utility class, aligning with the interface requirements.
19-29: Added support for ELN-style and subdirectory formats.The implementation of the two new methods adds support for different RO-Crate writing styles, enhancing the flexibility of the library. Each method uses the appropriate configuration for the WriteZipStrategy.
src/main/java/edu/kit/datamanager/ro_crate/writer/WriteZipStrategy.java (3)
16-19: Good implementation of multiple interfaces.The class implements both
GenericWriterStrategy<String>andElnFormatWriter<String>, providing a cohesive API for ZIP-based RO-Crate writing while supporting the ELN format.
21-27: Effective use of delegation pattern.The class appropriately uses delegation to
WriteZipStreamStrategyfor the actual implementation, promoting code reuse and separation of concerns.
29-32:❌ Incorrect review comment
Potential resource leak in FileOutputStream.
The
FileOutputStreamcreated in thesavemethod is not explicitly closed, which could lead to resource leaks if the delegate does not handle closing.
🏁 Script executed:
#!/bin/bash # Check if WriteZipStreamStrategy properly closes the OutputStream rg -A 10 "save\(Crate crate, OutputStream" src/main/java/edu/kit/datamanager/ro_crate/writer/WriteZipStreamStrategy.javaLength of output: 730
No resource leak: ZipOutputStream closes the underlying stream
TheFileOutputStreampassed intodelegate.save(…)is wrapped by aZipOutputStreamin a try-with-resources block inWriteZipStreamStrategy.save(...). Closing theZipOutputStreamautomatically closes its wrappedOutputStream(perFilterOutputStream.close()), so there is no unclosed stream.• WriteZipStrategy.save (src/main/java/edu/kit/datamanager/ro_crate/writer/WriteZipStrategy.java)
• WriteZipStreamStrategy.save (src/main/java/edu/kit/datamanager/ro_crate/writer/WriteZipStreamStrategy.java)Likely an incorrect or invalid review comment.
README.md (3)
15-17: Improved documentation navigation.Adding a direct link to JavaDoc documentation in the main navigation improves the user experience by making API documentation more accessible.
34-34: Better reference to specification examples.Linking to the extracted examples as well-described unit tests provides users with practical, specification-aligned implementation guidance.
41-56: Simplified Quick-start section.The Quick-start section has been appropriately simplified to focus on the core builder pattern without overwhelming users with details. This makes it easier for new users to get started.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
41-41: Spaces inside code span elements
null(MD038, no-space-in-code)
src/test/java/edu/kit/datamanager/ro_crate/reader/ElnFileFormatTest.java (1)
56-72: Well-implemented test with good error handling.The test method is well structured with appropriate error handling and assertions for both successful and expected failure cases. The approach of using a configurable blacklist is flexible and allows different reader implementations to specify their limitations.
src/main/java/edu/kit/datamanager/ro_crate/writer/ElnFormatWriter.java (3)
3-9: Well-documented interface with clear purpose.The interface is well-documented with clear references to the ELN file format. The generic type parameter is properly explained.
11-18: Clear method documentation.The usingElnStyle method is well-documented, clearly explaining its purpose and relationship to the withRootSubdirectory method.
19-26: Good use of default method for alias.The withRootSubdirectory method is implemented as a default method that delegates to usingElnStyle, providing a clearer name for more generic contexts while maintaining the specialized terminology where appropriate.
src/test/resources/spec-v1.1-example-json-files/complete-workflow-example.json (1)
1-113: Well-structured JSON-LD example file.The JSON file is:
- Well-formatted and valid JSON-LD
- Properly references the RO-Crate 1.1 context and specification
- Comprehensively models a computational workflow with inputs, outputs, creator, and license
- Includes proper cross-references between entities via JSON-LD identifiers
- References external ontologies like EDAM for biological data types
This example will be valuable for testing and documentation purposes.
src/test/java/edu/kit/datamanager/ro_crate/writer/CommonWriterTest.java (1)
72-74: Use path segments without trailing “/” for clarityThe assertions resolve
"lots_of_little_files/"(note the trailing slash).
While this works on Unix, the segment actually contains a separator and may behave unexpectedly on Windows path implementations.- Files.isDirectory(extractionPath.resolve("lots_of_little_files/")), + Files.isDirectory(extractionPath.resolve("lots_of_little_files")),Apply the same change to related
resolvecalls.src/test/java/edu/kit/datamanager/ro_crate/reader/FolderReaderTest.java (4)
20-21: Interface implementation aligns with new architecture.The refactoring from extending an abstract class
CrateReaderTest<String, FolderStrategy>to implementing an interfaceCommonReaderTest<String, ReadFolderStrategy>follows good object-oriented design principles by favoring composition over inheritance and using interfaces for common behavior.
23-26: Appropriate exception handling added.Adding the
throws IOExceptiondeclaration to thesaveCratemethod is a good practice as it properly communicates that this operation might fail due to I/O issues, allowing the caller to handle the exception appropriately.
34-38: Clear implementation of strategy creation.The method provides a good implementation of the interface method, with helpful comments explaining why the temporary directory parameter is not supported in this strategy implementation.
42-45: Well-structured reader implementation.The
readCratemethod correctly uses the strategy pattern throughCrateReader<>and passes the appropriate path format to the strategy, maintaining consistency with the rest of the codebase.src/test/java/edu/kit/datamanager/ro_crate/entities/contextual/ContextualEntityTest.java (4)
5-12: Appropriate imports added for new test functionality.The added imports for Jackson classes and assertions are necessary for the new test methods that validate JSON handling functionality.
52-78: Good test for valid property assignment.This test effectively verifies that
setAllIfValid()correctly processes a valid JSON object and produces an entity equivalent to one built with explicit property setters. The use of multiline strings improves readability.
80-113: Well-structured test for invalid property rejection.This test correctly verifies that
setAllIfValid()rejects invalid properties by checking that the resulting entity has an empty property set. The assertions are appropriate for validating the expected behavior.
115-148: Comprehensive test for unsafe property assignment.This test clearly demonstrates that
setAllUnsafe()accepts invalid properties without validation, which is an important distinction from the safersetAllIfValid()method. Having tests for both the safe and unsafe variants ensures complete coverage of the API behavior.src/test/java/edu/kit/datamanager/ro_crate/HelpFunctions.java (6)
7-7: Added required import for JSON formatting.The addition of the Jackson
SerializationFeatureimport is appropriate for the new pretty-printing functionality.
15-16: Added imports for new file operations.The additions of Java NIO imports for
FilesandPathare needed for the new file tree printing functionality.
80-93: Useful JSON pretty-printing utility for debugging.This helper method will improve test debugging by formatting JSON in a readable way. The error handling with
AssertionFailedErrorensures that JSON parsing issues are properly reported as test failures.
95-104: Convenient test utility combining print and assert.This method combines pretty-printing and assertion in a single call, which will reduce boilerplate in tests and standardize error handling for resource validation.
113-120: Good documentation added to existing method.Adding proper JavaDoc to the
compareCrateJsonToFileInResourcesmethod improves code maintainability and developer experience.
158-177: Helpful directory tree visualization for debugging.The
printFileTreemethod provides a useful visualization of directory structures, which will aid in debugging and understanding test results. The implementation usingFiles.walk()and a tree-like format is clear and effective.One minor suggestion: Consider adding a depth limit parameter to prevent overly verbose output for deeply nested directories.
src/test/java/edu/kit/datamanager/ro_crate/writer/TestableWriterStrategy.java (3)
1-30: Well-designed interface for writer strategy testing.This interface establishes a clear contract for testing writer strategies with appropriate documentation. The
saveCratemethod provides a standard way for implementations to define their specific writing behavior.
31-42: Useful utility method for ZIP extraction.The
ensureCrateIsExtractedIndefault method provides a standard way to handle ZIP extraction, promoting code reuse across test implementations. Using try-with-resources ensures proper resource cleanup.
86-120: Well-structured crate builder factory method.The
getCrateWithFileAndDirmethod provides a standard way to create test crates with consistent metadata, which will ensure test consistency across different writer implementations. The builder pattern usage is clear and appropriate.src/test/java/edu/kit/datamanager/ro_crate/reader/ZipReaderTest.java (1)
11-45: Test refactor looks goodThe switch from the old
ZipStrategytoReadZipStrategyis consistent and the additional assertions strengthen the contract of the strategy.
src/main/java/edu/kit/datamanager/ro_crate/util/FileSystemUtil.java
Outdated
Show resolved
Hide resolved
src/test/java/edu/kit/datamanager/ro_crate/reader/ElnFileFormatTest.java
Show resolved
Hide resolved
src/test/java/edu/kit/datamanager/ro_crate/writer/ZipStreamWriterTest.java
Outdated
Show resolved
Hide resolved
src/test/java/edu/kit/datamanager/ro_crate/reader/ZipStreamReaderTest.java
Outdated
Show resolved
Hide resolved
src/test/java/edu/kit/datamanager/ro_crate/examples/LearnByExampleTest.java
Show resolved
Hide resolved
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/AbstractEntity.java
Outdated
Show resolved
Hide resolved
…variable for file listing
…cify null parameter constraints
…l path validation
Let's give the CI a bit more time.
…ream in ZipStreamReaderTest
…operty This never happened in the tests, but may be useful when reading inconsistent crates.
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Documentation
Refactor
Tests
Chores