Skip to content

Conversation

@Pfeil
Copy link
Member

@Pfeil Pfeil commented May 27, 2025

Summary by CodeRabbit

  • New Features

    • Enhanced validation of entities before adding them to RO-Crate packages, including support for prefixed types.
    • Improved context management with new options to add, retrieve, and remove context entries and URLs.
    • Ability to generate JSON-LD representations of context for better interoperability.
    • Added provenance management to track creation and update actions with versioned metadata in RO-Crates.
    • Introduced marking of crates as imported to distinguish imported from newly created crates.
    • Added version information provisioning via classpath properties and integrated provenance handling in crate saving.
    • New utility for querying RO-Crate graph entities by ID or type.
    • Added comprehensive tests for provenance management and metadata generation in RO-Crates.
    • Added new Gradle tasks to generate version properties files for build and test resources.
    • Introduced a Gradle task to update the CITATION.cff file with current version and release date during release.
  • Bug Fixes

    • Corrected URLs in test resources to ensure accurate validation scenarios.
  • Documentation

    • Expanded JavaDoc comments for constructors and methods to clarify context handling and usage.
  • Chores

    • Updated Gradle wrapper and dependencies for improved stability and compatibility.
    • Renamed and updated test classes for clarity and better coverage of new validation logic.
    • Modified test crate saving to explicitly disable automatic provenance where appropriate.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented May 27, 2025

Walkthrough

The changes update dependency versions and Gradle wrapper, add version property generation tasks, introduce provenance management with versioned metadata tracking, and extend the RoCrate and Crate interfaces to mark crates as imported. The RoCrateMetadataContext class gains comprehensive context manipulation and validation methods. Multiple test classes are added or updated to validate provenance handling, context validation, and crate saving behavior with provenance control.

Changes

File(s) Change Summary
build.gradle, gradle/wrapper/gradle-wrapper.properties Updated JUnit BOM to 5.13.0, json-schema-validator to 1.5.7, Gradle wrapper to 8.14.2; added tasks generateVersionProps and generateVersionPropsTest to create version.properties files; configured task dependencies; applied updateCff.gradle script.
gradle/updateCff.gradle Added new Gradle task updateCff to update version and release date in CITATION.cff during release process.
src/main/java/edu/kit/datamanager/ro_crate/Crate.java, RoCrate.java Added markAsImported() and isImported() methods to mark crates as imported; added isImported field and entity validation calls before adding entities in RoCrateBuilder.
src/main/java/edu/kit/datamanager/ro_crate/context/RoCrateMetadataContext.java Added detailed JavaDoc; added methods for context JSON serialization, adding/removing URL and key-value pairs; improved entity validation with prefix-aware checks.
src/main/java/edu/kit/datamanager/ro_crate/entities/AbstractEntity.java Added method to get @id property; improved optional handling in builder; minor Javadoc formatting updates.
src/main/java/edu/kit/datamanager/ro_crate/preview/CratePreview.java Modified default generate method to disable automatic provenance by passing null before saving crate.
src/main/java/edu/kit/datamanager/ro_crate/reader/CrateReader.java Modified readCrate to mark returned crate as imported.
src/main/java/edu/kit/datamanager/ro_crate/util/VersionProvider.java Added new interface VersionProvider with method getVersion().
src/main/java/edu/kit/datamanager/ro_crate/util/ClasspathPropertiesVersionProvider.java Added implementation of VersionProvider reading version from classpath version.properties.
src/main/java/edu/kit/datamanager/ro_crate/util/Graph.java Added utility class with static methods to find entities by ID or type in RO-Crate JSON graphs.
src/main/java/edu/kit/datamanager/ro_crate/writer/CrateWriter.java Added provenanceManager field and fluent setter withAutomaticProvenance(); modified save to add provenance info conditionally.
src/main/java/edu/kit/datamanager/ro_crate/writer/ProvenanceManager.java Added new class managing provenance metadata entities and actions, versioned by ro-crate-java version, adding create/update provenance info to crates.
src/test/java/edu/kit/datamanager/ro_crate/context/RoCrateMetadataContextTest.java Renamed class; fixed test data URLs; added tests for entity validation with prefixed types; minor formatting fixes.
src/test/java/edu/kit/datamanager/ro_crate/reader/CommonReaderTest.java Updated tests to disable automatic provenance before saving crates by invoking withAutomaticProvenance(null).
src/test/java/edu/kit/datamanager/ro_crate/reader/FolderReaderTest.java Modified saveCrate to disable automatic provenance before saving.
src/test/java/edu/kit/datamanager/ro_crate/reader/ZipReaderTest.java Modified saveCrate to disable automatic provenance before saving.
src/test/java/edu/kit/datamanager/ro_crate/reader/ZipStreamReaderTest.java Modified saveCrate to disable automatic provenance before saving.
src/test/java/edu/kit/datamanager/ro_crate/writer/FolderWriterTest.java Modified saveCrate to disable automatic provenance before saving.
src/test/java/edu/kit/datamanager/ro_crate/writer/ZipStreamWriterTest.java Modified save methods to disable automatic provenance before saving.
src/test/java/edu/kit/datamanager/ro_crate/writer/ZipWriterTest.java Modified save methods to disable automatic provenance before saving.
src/test/java/edu/kit/datamanager/ro_crate/writer/ProvenanceManagerTest.java Added new test class verifying provenance manager behavior: creation, reuse, and metadata preservation of provenance entities and actions.
src/test/java/edu/kit/datamanager/ro_crate/writer/RoCrateMetadataGenerationTest.java Added extensive tests for provenance metadata generation, validation, accumulation, and preservation in RO-Crate metadata files.
src/test/java/edu/kit/datamanager/ro_crate/writer/RoCrateWriterSpec12Test.java Added disabling of automatic provenance before saving crate.

Sequence Diagram(s)

sequenceDiagram
    participant CrateReader
    participant RoCrate
    participant Crate

    CrateReader->>RoCrate: rebuildCrate(...)
    RoCrate->>RoCrate: markAsImported()
    RoCrate-->>CrateReader: return marked crate
Loading
sequenceDiagram
    participant CrateWriter
    participant ProvenanceManager
    participant Crate

    CrateWriter->>Crate: validate()
    CrateWriter->>ProvenanceManager: addProvenanceInformation(crate)
    ProvenanceManager->>Crate: add or update provenance entities and actions
    CrateWriter->>CrateWriter: delegate save operation
Loading
sequenceDiagram
    participant RoCrateBuilder
    participant RoCrateMetadataContext

    RoCrateBuilder->>RoCrateMetadataContext: checkEntity(entity)
    RoCrateMetadataContext-->>RoCrateBuilder: validation result
    alt valid
        RoCrateBuilder->>RoCrateBuilder: add entity to payload
    else invalid
        RoCrateBuilder->>User: print error, do not add entity
    end
Loading

Poem

🐇 Hopping through crates with care,
Provenance tracked with flair.
Versions marked, contexts checked,
Metadata neatly decked.
Tests abound, dependencies new,
Our ro-crate hops fresh and true!
🌿📦✨


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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@coveralls
Copy link

coveralls commented May 27, 2025

Pull Request Test Coverage Report for Build #478

Details

  • 90 of 93 (96.77%) changed or added relevant lines in 9 files are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.09%) to 90.804%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/main/java/edu/kit/datamanager/ro_crate/util/ClasspathPropertiesVersionProvider.java 13 16 81.25%
Files with Coverage Reduction New Missed Lines %
src/main/java/edu/kit/datamanager/ro_crate/context/RoCrateMetadataContext.java 3 94.17%
Totals Coverage Status
Change from base Build #445: 0.09%
Covered Lines: 2044
Relevant Lines: 2251

💛 - Coveralls

@Pfeil Pfeil marked this pull request as ready for review June 2, 2025 15:33
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🔭 Outside diff range comments (1)
src/main/java/edu/kit/datamanager/ro_crate/context/RoCrateMetadataContext.java (1)

136-203: 🛠️ Refactor suggestion

Consider improving error handling mechanism.

The validation logic is comprehensive and handles prefixes well. However, printing errors to System.err is not ideal for a library. Consider using a logging framework or collecting errors for the caller to handle.

Consider refactoring the error handling:

-        System.err.println("type " + s + " is missing from the context!");
+        // Consider using SLF4J logger instead
+        // logger.error("type {} is missing from the context!", s);
         return false;
-        System.err.println("attribute name " + s + " is missing from context;");
+        // Consider using SLF4J logger instead
+        // logger.error("attribute name {} is missing from context", s);
         return false;

Alternatively, consider collecting validation errors and returning them to the caller for better error handling flexibility.

🧹 Nitpick comments (1)
src/main/java/edu/kit/datamanager/ro_crate/context/RoCrateMetadataContext.java (1)

263-271: Consider simplifying the getValueOf implementation.

The current implementation uses Optional but then falls back to the same map lookup. Since contextMap should contain all entries from other, this might be redundant.

Consider simplifying:

-  public String getValueOf(String key) {
-    return Optional.ofNullable(this.contextMap.get(key))
-            .orElseGet(() -> this.other.get(key));
-  }
+  public String getValueOf(String key) {
+    return this.contextMap.get(key);
+  }

If there's a specific reason for checking both maps, please add a comment explaining the logic.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7254152 and bd8aba4.

📒 Files selected for processing (5)
  • build.gradle (2 hunks)
  • gradle/wrapper/gradle-wrapper.properties (1 hunks)
  • src/main/java/edu/kit/datamanager/ro_crate/RoCrate.java (1 hunks)
  • src/main/java/edu/kit/datamanager/ro_crate/context/RoCrateMetadataContext.java (11 hunks)
  • src/test/java/edu/kit/datamanager/ro_crate/context/RoCrateMetadataContextTest.java (5 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/test/java/edu/kit/datamanager/ro_crate/context/RoCrateMetadataContextTest.java (2)
src/main/java/edu/kit/datamanager/ro_crate/RoCrate.java (1)
  • RoCrate (43-531)
src/test/java/edu/kit/datamanager/ro_crate/HelpFunctions.java (1)
  • HelpFunctions (27-178)
🔇 Additional comments (14)
gradle/wrapper/gradle-wrapper.properties (1)

3-3: LGTM!

Gradle wrapper patch version update from 8.14 to 8.14.1 is appropriate for bug fixes and improvements.

src/main/java/edu/kit/datamanager/ro_crate/context/RoCrateMetadataContext.java (7)

15-15: LGTM!

The Function import is correctly added to support the new validation logic.


42-43: Excellent documentation improvements!

The clarifications about the default context not being automatically included are crucial for API users to understand the behavior of these constructors.

Also applies to: 50-52, 62-63


91-134: Well-implemented context serialization method!

The method correctly handles different context structures and provides clear documentation with examples. The conditional logic for single vs. multiple context entries is appropriate for JSON-LD standards.


205-212: LGTM!

Good documentation addition for the existing method.


273-295: LGTM!

The getKeys() and getPairs() methods provide clean, immutable views of the context data with appropriate documentation.


306-312: Verify the impact of URL deletion on context entries.

The deleteUrlFromContext method removes the URL but doesn't remove the corresponding entries from contextMap that were added when the URL was processed. This could leave orphaned context entries.

Consider whether entries added from the deleted URL should also be removed from the context maps, or document this behavior if it's intentional.


251-261: LGTM!

The addToContext method correctly maintains consistency between both internal maps.

src/main/java/edu/kit/datamanager/ro_crate/RoCrate.java (1)

385-385: Same validation concern as previous method.

Good addition of validation before adding contextual entities. Please refer to the previous comment about verifying the checkEntity error handling behavior.

src/test/java/edu/kit/datamanager/ro_crate/context/RoCrateMetadataContextTest.java (5)

9-9: LGTM!

Import addition is necessary for the new test functionality.


24-24: LGTM!

Class rename improves clarity and follows standard test naming conventions.


36-36: LGTM!

Resource stream update is necessary after the class rename.


102-103: LGTM!

Good catch fixing the typos in the test URLs.


290-305: Excellent test coverage for prefixed type validation.

This test comprehensively validates the new prefixed type functionality. It properly tests the integration between RoCrate and RoCrateMetadataContext with a realistic scenario using "rdfs:Property".

The test structure is good:

  1. Sets up context with prefix mapping
  2. Creates entity with prefixed type
  3. Adds to crate (triggering validation)
  4. Explicitly validates the entity

Note: The test's behavior depends on how checkEntity handles validation failures in the addDataEntity method (see previous comments about error handling).

Pfeil added 17 commits June 4, 2025 16:48
…eval and refactor ProvenanceManager to use it
…anager instance

This allows disabling provenance management with null, but also enable to implement derived or completely new implementations.
…-crates-about-the-tool-which-created-it

265 store provenance in crates about the tool which created it
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (9)
src/main/java/edu/kit/datamanager/ro_crate/util/Graph.java (4)

11-11: Fix documentation reference.

The {@see JsonUtilFunctions} reference appears to be incorrect. Use {@link JsonUtilFunctions} if the class exists, or remove the reference if it doesn't.

- * {@see JsonUtilFunctions}.
+ * {@link JsonUtilFunctions}.

26-33: Consider using Stream API for consistency.

For consistency with the other methods, consider refactoring this method to use Stream API instead of traditional iteration.

 public static JsonNode findEntityById(JsonNode graph, String id) {
-    for (JsonNode entity : graph) {
-        if (entity.has("@id") && entity.get("@id").asText().equals(id)) {
-            return entity;
-        }
-    }
-    return null;
+    return StreamSupport.stream(graph.spliterator(), false)
+            .filter(entity -> entity.has("@id") && entity.get("@id").asText().equals(id))
+            .findFirst()
+            .orElse(null);
 }

42-47: Add null safety check.

Consider adding a null safety check similar to the findEntityById method to ensure robust handling of entities without the @type field.

 public static JsonNode findEntityByType(JsonNode graph, String type) {
     return StreamSupport.stream(graph.spliterator(), false)
-            .filter(entity -> entity.path("@type").asText().equals(type))
+            .filter(entity -> entity.has("@type") && entity.get("@type").asText().equals(type))
             .findFirst()
             .orElse(null);
 }

56-60: Apply same null safety improvement.

Apply the same null safety improvement as suggested for findEntityByType.

 public static JsonNode[] findEntitiesByType(JsonNode graph, String type) {
     return StreamSupport.stream(graph.spliterator(), false)
-            .filter(entity -> entity.path("@type").asText().equals(type))
+            .filter(entity -> entity.has("@type") && entity.get("@type").asText().equals(type))
             .toArray(JsonNode[]::new);
 }
src/main/java/edu/kit/datamanager/ro_crate/writer/ProvenanceManager.java (2)

105-112: Extract action types as constants.

Consider extracting the action type strings as constants to improve maintainability and reduce the risk of typos.

+    private static final String CREATE_ACTION_TYPE = "CreateAction";
+    private static final String UPDATE_ACTION_TYPE = "UpdateAction";
+    
     protected ContextualEntity buildNewActionEntity(boolean isFirstWrite, String libraryId) {
         return new ContextualEntityBuilder()
-                .addType(isFirstWrite ? "CreateAction" : "UpdateAction")
+                .addType(isFirstWrite ? CREATE_ACTION_TYPE : UPDATE_ACTION_TYPE)
                 .addIdProperty("result", "./")
                 .addProperty("startTime", Instant.now().toString())
                 .addIdProperty("agent", libraryId)
                 .build();
     }

114-137: Consider extracting entity properties as constants.

The hardcoded metadata properties could be extracted as constants for better maintainability.

+    private static final String RO_CRATE_JAVA_NAME = "ro-crate-java";
+    private static final String RO_CRATE_JAVA_URL = "https://github.com/kit-data-manager/ro-crate-java";
+    private static final String RO_CRATE_JAVA_LICENSE = "Apache-2.0";
+    private static final String RO_CRATE_JAVA_DESCRIPTION = "A Java library for creating and manipulating RO-Crates";
+    private static final String SOFTWARE_APPLICATION_TYPE = "SoftwareApplication";

     protected ContextualEntity buildRoCrateJavaEntity(
             Crate crate,
             String newActionId,
             String libraryId
     ) {
         String version = this.versionProvider.getVersion();
         ContextualEntity self = crate.getAllContextualEntities().stream()
                 .filter(contextualEntity -> libraryId.equals(contextualEntity.getId()))
                 .findFirst()
                 .orElseGet(() -> new ContextualEntityBuilder()
                         .setId(libraryId)
-                        .addType("SoftwareApplication")
-                        .addProperty("name", "ro-crate-java")
-                        .addProperty("url", "https://github.com/kit-data-manager/ro-crate-java")
+                        .addType(SOFTWARE_APPLICATION_TYPE)
+                        .addProperty("name", RO_CRATE_JAVA_NAME)
+                        .addProperty("url", RO_CRATE_JAVA_URL)
                         .addProperty("version", version)
                         .addProperty("softwareVersion", version)
-                        .addProperty("license", "Apache-2.0")
-                        .addProperty("description", "A Java library for creating and manipulating RO-Crates")
+                        .addProperty("license", RO_CRATE_JAVA_LICENSE)
+                        .addProperty("description", RO_CRATE_JAVA_DESCRIPTION)
                         .addIdProperty("Action", newActionId)
                         .build()
                 );
src/test/java/edu/kit/datamanager/ro_crate/writer/RoCrateMetadataGenerationTest.java (3)

240-241: Extract regex pattern as a constant.

The semantic versioning regex pattern should be extracted as a class constant for reusability and maintainability.

+    private static final String SEMVER_PATTERN = 
+            "(?i)^\\d+\\.\\d+\\.\\d+(?:-(?:rc\\d+|alpha|beta|snapshot)(?:\\.\\d+)?)?(?:\\+[0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*)?$";
+
     @Test
     void should_HaveValidVersionFormat_When_WritingCrate(@TempDir Path tempDir) throws IOException {
         // ... existing code ...
-        String semverPattern = "(?i)^\\d+\\.\\d+\\.\\d+(?:-(?:rc\\d+|alpha|beta|snapshot)(?:\\.\\d+)?)?(?:\\+[0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*)?$";
         
-        assertTrue(version.matches(semverPattern),
+        assertTrue(version.matches(SEMVER_PATTERN),

144-212: Consider breaking down this test method.

This test method is quite long and tests multiple aspects. Consider breaking it into smaller, more focused test methods for better maintainability and clarity.

This method could be split into:

  • should_CreateOneCreateAction_When_WritingCrateFirstTime
  • should_CreateUpdateActions_When_WritingCrateMultipleTimes
  • should_MaintainChronologicalOrder_When_WritingMultipleTimes

233-234: Review necessity of @SuppressWarnings annotation.

The @SuppressWarnings("DataFlowIssue") annotation suggests potential null pointer concerns. Consider adding explicit null checks instead of suppressing warnings.

         // Version format validation
-        @SuppressWarnings("DataFlowIssue")
-        String version = roCrateJavaEntity.get("version").asText();
+        JsonNode versionNode = roCrateJavaEntity.get("version");
+        assertNotNull(versionNode, "Version property should not be null");
+        String version = versionNode.asText();
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bd8aba4 and 7905f12.

📒 Files selected for processing (22)
  • CITATION.cff (1 hunks)
  • build.gradle (4 hunks)
  • src/main/java/edu/kit/datamanager/ro_crate/Crate.java (1 hunks)
  • src/main/java/edu/kit/datamanager/ro_crate/RoCrate.java (2 hunks)
  • src/main/java/edu/kit/datamanager/ro_crate/entities/AbstractEntity.java (6 hunks)
  • src/main/java/edu/kit/datamanager/ro_crate/preview/CratePreview.java (1 hunks)
  • src/main/java/edu/kit/datamanager/ro_crate/reader/CrateReader.java (1 hunks)
  • src/main/java/edu/kit/datamanager/ro_crate/util/ClasspathPropertiesVersionProvider.java (1 hunks)
  • src/main/java/edu/kit/datamanager/ro_crate/util/Graph.java (1 hunks)
  • src/main/java/edu/kit/datamanager/ro_crate/util/VersionProvider.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/ProvenanceManager.java (1 hunks)
  • src/test/java/edu/kit/datamanager/ro_crate/reader/CommonReaderTest.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/ZipReaderTest.java (1 hunks)
  • src/test/java/edu/kit/datamanager/ro_crate/reader/ZipStreamReaderTest.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/ProvenanceManagerTest.java (1 hunks)
  • src/test/java/edu/kit/datamanager/ro_crate/writer/RoCrateMetadataGenerationTest.java (1 hunks)
  • src/test/java/edu/kit/datamanager/ro_crate/writer/RoCrateWriterSpec12Test.java (1 hunks)
  • src/test/java/edu/kit/datamanager/ro_crate/writer/ZipStreamWriterTest.java (2 hunks)
  • src/test/java/edu/kit/datamanager/ro_crate/writer/ZipWriterTest.java (1 hunks)
✅ Files skipped from review due to trivial changes (4)
  • src/main/java/edu/kit/datamanager/ro_crate/util/VersionProvider.java
  • CITATION.cff
  • src/test/java/edu/kit/datamanager/ro_crate/writer/ZipWriterTest.java
  • src/test/java/edu/kit/datamanager/ro_crate/writer/ZipStreamWriterTest.java
🚧 Files skipped from review as they are similar to previous changes (2)
  • build.gradle
  • src/main/java/edu/kit/datamanager/ro_crate/RoCrate.java
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/test/java/edu/kit/datamanager/ro_crate/reader/ZipReaderTest.java (1)
src/main/java/edu/kit/datamanager/ro_crate/writer/Writers.java (1)
  • Writers (9-42)
src/test/java/edu/kit/datamanager/ro_crate/reader/FolderReaderTest.java (1)
src/main/java/edu/kit/datamanager/ro_crate/writer/Writers.java (1)
  • Writers (9-42)
src/main/java/edu/kit/datamanager/ro_crate/entities/AbstractEntity.java (1)
src/main/java/edu/kit/datamanager/ro_crate/special/IdentifierUtils.java (1)
  • IdentifierUtils (23-158)
src/test/java/edu/kit/datamanager/ro_crate/reader/ZipStreamReaderTest.java (1)
src/main/java/edu/kit/datamanager/ro_crate/writer/Writers.java (1)
  • Writers (9-42)
⏰ Context from checks skipped due to timeout of 90000ms (11)
  • GitHub Check: build (macos-latest, 17)
  • GitHub Check: build (windows-latest, 21)
  • GitHub Check: build (macos-latest, 21)
  • GitHub Check: build (windows-latest, 17)
  • GitHub Check: build (ubuntu-latest, 21)
  • GitHub Check: Analyze (java)
  • GitHub Check: build (ubuntu-latest, 17)
  • GitHub Check: build (macos-latest, 17)
  • GitHub Check: build (windows-latest, 17)
  • GitHub Check: build (windows-latest, 21)
  • GitHub Check: build (ubuntu-latest, 21)
🔇 Additional comments (18)
src/test/java/edu/kit/datamanager/ro_crate/writer/FolderWriterTest.java (1)

19-19: LGTM: Appropriate test isolation by disabling automatic provenance.

This change ensures test predictability by explicitly disabling automatic provenance metadata addition during crate writing, which is the correct approach for maintaining test isolation.

src/test/java/edu/kit/datamanager/ro_crate/writer/RoCrateWriterSpec12Test.java (1)

33-33: LGTM: Essential for test correctness.

This change is crucial for the writeDoesNotModifyTest since automatic provenance addition would invalidate the test's premise that writing doesn't modify the original crate. Disabling provenance ensures the test accurately validates the write operation's non-invasive behavior.

src/test/java/edu/kit/datamanager/ro_crate/reader/FolderReaderTest.java (1)

24-26: LGTM: Ensures clean test data for reader tests.

The addition of .withAutomaticProvenance(null) maintains test data integrity by preventing automatic metadata addition during the save operation used for test setup, ensuring that reader tests work with clean, predictable data.

src/test/java/edu/kit/datamanager/ro_crate/reader/ZipReaderTest.java (1)

17-19: LGTM: Consistent provenance handling across writer types.

The addition of .withAutomaticProvenance(null) maintains consistency with other test files while ensuring that zip-based tests also have predictable behavior without automatic provenance metadata addition.

src/main/java/edu/kit/datamanager/ro_crate/reader/CrateReader.java (1)

100-100: LGTM: Correctly marks crates as imported when read from external sources.

This change appropriately integrates with the new provenance management system by marking crates as imported upon reading, which enables proper provenance handling downstream.

src/test/java/edu/kit/datamanager/ro_crate/reader/ZipStreamReaderTest.java (1)

36-38: LGTM: Proper test isolation by disabling automatic provenance.

Explicitly disabling automatic provenance in tests ensures consistent behavior and prevents automatic metadata generation from interfering with test assertions.

src/test/java/edu/kit/datamanager/ro_crate/reader/CommonReaderTest.java (1)

130-135: LGTM: Maintains test correctness by disabling automatic provenance.

Disabling automatic provenance is essential here to ensure fair comparison between raw and imported crates, as automatic metadata addition would introduce differences that would break the test logic.

src/main/java/edu/kit/datamanager/ro_crate/preview/CratePreview.java (1)

43-44: LGTM: Appropriate separation of concerns for provenance handling.

The assumption that the caller has already handled provenance is reasonable for preview generation, and the explanatory comment clearly documents this design decision.

src/main/java/edu/kit/datamanager/ro_crate/Crate.java (1)

25-45: LGTM! Well-designed interface additions for crate state tracking.

The new methods provide a clean way to distinguish between imported and newly created crates, which is essential for proper provenance handling. The fluent interface pattern and comprehensive Javadoc documentation are excellent.

src/main/java/edu/kit/datamanager/ro_crate/writer/CrateWriter.java (2)

18-18: LGTM! Good initialization of provenance manager.

The default initialization provides sensible behavior while allowing customization through the fluent interface.


29-39: Well-designed fluent interface for provenance configuration.

The method provides clear control over provenance management with good documentation explaining the null behavior for disabling provenance.

src/main/java/edu/kit/datamanager/ro_crate/entities/AbstractEntity.java (4)

115-126: Useful utility method for extracting ID properties.

This method provides a clean way to extract @id values from properties, which is commonly needed when working with RO-Crate entities.


257-257: Minor style improvement removing unnecessary braces.

Clean simplification of the lambda expression while maintaining readability.


425-425: Excellent safety improvement using orElse() instead of get().

This change prevents potential NoSuchElementException if the encoding fails and returns an empty Optional. Using orElse(this.id) maintains the current ID value as a fallback, which is the correct behavior.


370-370: Good Javadoc formatting improvements.

The consistent use of <p> tags improves documentation rendering and readability.

Also applies to: 462-462, 522-522

src/test/java/edu/kit/datamanager/ro_crate/writer/ProvenanceManagerTest.java (1)

1-182: Excellent test coverage and structure.

This test class demonstrates excellent testing practices:

  • Clear test naming with should/when structure
  • Comprehensive coverage of provenance scenarios
  • Good use of Given/When/Then pattern
  • Descriptive assertion messages
  • Proper verification of entity relationships and properties

The tests effectively validate the core functionality of the ProvenanceManager class.

src/main/java/edu/kit/datamanager/ro_crate/writer/ProvenanceManager.java (1)

22-38: Well-designed IdPrefix record.

The IdPrefix record is a clever design that provides type safety and prevents accidental misuse of partial IDs. The withSuffix method and toString override are well-implemented.

src/test/java/edu/kit/datamanager/ro_crate/writer/RoCrateMetadataGenerationTest.java (1)

1-406: Excellent comprehensive test coverage.

This test class provides outstanding integration test coverage for the metadata generation functionality:

  • Thorough validation of provenance entity creation and relationships
  • Proper file I/O testing with temporary directories
  • Good use of JSON parsing and validation
  • Comprehensive edge case coverage
  • Excellent debugging support with pretty printing

The tests effectively validate the integration between multiple components of the provenance management system.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
gradle/updateCff.gradle (3)

40-42: Guard against missing input file.
Reading cffFile.text without verifying existence can fail the build unexpectedly. Add a check to throw a clear error or initialize the file if it’s missing.

     def cffFile = file('CITATION.cff')
+    if (!cffFile.exists()) {
+        throw new GradleException("CITATION.cff not found at ${cffFile.absolutePath}")
+    }
     def content = cffFile.text

44-48: Refactor version update logic for clarity.
Instead of separate contains + replaceAll, leverage a regex match to detect existing entries and minimize duplication.

-        if (content.contains('version:')) {
-            content = content.replaceAll(/(?m)^version:\s*.+$/, "version: ${version}")
-        } else {
-            content = content + "\nversion: ${version}"
-        }
+        def versionPattern = /(?m)^version:\s*.+$/
+        if (content =~ versionPattern) {
+            content = content.replaceAll(versionPattern, "version: ${version}")
+        } else {
+            content += "\nversion: ${version}"
+        }

50-55: Apply the same consolidation to date-released.
DRY up the code by reusing the pattern-based approach for updating or inserting the release date.

-        if (content.contains('date-released:')) {
-            content = content.replaceAll(/(?m)^date-released:\s*.+$/, "date-released: ${today}")
-        } else {
-            content = content + "\ndate-released: ${today}"
-        }
+        def datePattern = /(?m)^date-released:\s*.+$/
+        if (content =~ datePattern) {
+            content = content.replaceAll(datePattern, "date-released: ${today}")
+        } else {
+            content += "\ndate-released: ${today}"
+        }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5f172da and 88fc710.

📒 Files selected for processing (3)
  • build.gradle (5 hunks)
  • gradle/profile-release.gradle (1 hunks)
  • gradle/updateCff.gradle (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • gradle/profile-release.gradle
🚧 Files skipped from review as they are similar to previous changes (1)
  • build.gradle
⏰ Context from checks skipped due to timeout of 90000ms (13)
  • GitHub Check: build (windows-latest, 21)
  • GitHub Check: build (windows-latest, 17)
  • GitHub Check: build (macos-latest, 17)
  • GitHub Check: build (macos-latest, 21)
  • GitHub Check: build (ubuntu-latest, 21)
  • GitHub Check: build (ubuntu-latest, 17)
  • GitHub Check: Analyze (java)
  • GitHub Check: build (windows-latest, 21)
  • GitHub Check: build (windows-latest, 17)
  • GitHub Check: build (ubuntu-latest, 17)
  • GitHub Check: build (macos-latest, 21)
  • GitHub Check: build (ubuntu-latest, 21)
  • GitHub Check: build (macos-latest, 17)
🔇 Additional comments (3)
gradle/updateCff.gradle (3)

1-2: No import issues.
The LocalDate and DateTimeFormatter imports are correct and required.


63-65: Task ordering aligns with release workflow.
Using mustRunAfter(confirmReleaseVersion) ensures updateCff runs at the correct point.


68-70: Dependency integration is correct.
Adding dependsOn('updateCff') to checkSnapshotDependencies properly hooks the task into the release chain.

Comment on lines +30 to +35
tasks.register('updateCff') {
group = 'release'
description = 'Updates the version in CITATION.cff file'

outputs.file("CITATION.cff")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Declare task inputs for up-to-date checks.
Gradle tasks should declare both inputs and outputs to support incremental builds. Add inputs.file("CITATION.cff") alongside the existing output.

 tasks.register('updateCff') {
     group = 'release'
     description = 'Updates the version in CITATION.cff file'
-    outputs.file("CITATION.cff")
+    inputs.file("CITATION.cff")
+    outputs.file("CITATION.cff")
     ...
 }
📝 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.

Suggested change
tasks.register('updateCff') {
group = 'release'
description = 'Updates the version in CITATION.cff file'
outputs.file("CITATION.cff")
tasks.register('updateCff') {
group = 'release'
description = 'Updates the version in CITATION.cff file'
inputs.file("CITATION.cff")
outputs.file("CITATION.cff")
...
}
🤖 Prompt for AI Agents
In gradle/updateCff.gradle around lines 30 to 35, the 'updateCff' task declares
outputs but lacks declared inputs, which is necessary for Gradle's incremental
build checks. Add an inputs declaration by including inputs.file("CITATION.cff")
within the task configuration to properly specify the input file for up-to-date
checks.

@Pfeil Pfeil self-assigned this Jun 6, 2025
@Pfeil Pfeil merged commit 0ca5d73 into main Jun 6, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants