UFAL/Embargo info check - #1033
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a new health check class Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Healthcheck Admin
participant Healthcheck as Healthcheck System
participant EmbargoCheck as EmbargoInfoCheck
participant DSpace as DSpace Repository
Admin->>Healthcheck: Run "Embargo check"
Healthcheck->>EmbargoCheck: invoke run(ReportInfo)
Note over EmbargoCheck,DSpace: initialize Context
EmbargoCheck->>DSpace: list Items / Collections / Communities
loop per Item
EmbargoCheck->>DSpace: fetch Bundles & Bitstreams
EmbargoCheck->>DSpace: fetch ResourcePolicies
EmbargoCheck->>EmbargoCheck: filter policies with start/end dates
EmbargoCheck->>EmbargoCheck: add EmbargoInfo entries
end
loop per Collection/Community
EmbargoCheck->>DSpace: fetch ResourcePolicies
EmbargoCheck->>EmbargoCheck: filter & add EmbargoInfo entries
end
EmbargoCheck->>EmbargoCheck: build formatted report (truncate >50 per type)
EmbargoCheck->>Healthcheck: return report string
Note over EmbargoCheck: close Context
Healthcheck->>Admin: present report
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
dspace-api/src/main/java/org/dspace/health/EmbargoInfoCheck.java (1)
74-76: Consider more specific exception handling.While catching all exceptions prevents the health check from failing completely, it might mask specific issues that could be valuable for debugging.
Consider handling specific exceptions separately:
- } catch (Exception e) { - throw new RuntimeException("Error while processing embargo check", e); + } catch (SQLException e) { + throw new RuntimeException("Database error while processing embargo check", e); + } catch (AuthorizeException e) { + throw new RuntimeException("Authorization error while processing embargo check", e); + } catch (Exception e) { + throw new RuntimeException("Unexpected error while processing embargo check", e);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
dspace-api/src/main/java/org/dspace/health/EmbargoInfoCheck.java(1 hunks)dspace/config/modules/healthcheck.cfg(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: dspace / docker-build (linux/amd64, ubuntu-latest, true)
- GitHub Check: dspace-cli / docker-build (linux/amd64, ubuntu-latest, true)
- GitHub Check: dspace-test / docker-build (linux/amd64, ubuntu-latest, true)
- GitHub Check: Run Integration Tests
- GitHub Check: Run Unit Tests
🔇 Additional comments (10)
dspace/config/modules/healthcheck.cfg (2)
10-11: LGTM! Configuration properly integrates the new embargo check.The addition to the healthcheck.checks list follows the correct format and integrates well with existing checks.
17-17: Plugin registration follows proper naming convention.The plugin declaration correctly maps the EmbargoInfoCheck class to the "Embargo check" label, maintaining consistency with other health check plugins.
dspace-api/src/main/java/org/dspace/health/EmbargoInfoCheck.java (8)
1-41: Well-structured class with appropriate imports and fields.The class follows good practices with:
- Proper package declaration and imports
- Clear documentation explaining the check's purpose
- Appropriate field declarations with meaningful names
- Reasonable display threshold constant
52-64: Efficient iteration over items and nested objects.The logic correctly processes all items and their associated bundles and bitstreams, collecting embargo information at each level. The nesting structure appropriately reflects the DSpace object hierarchy.
66-72: Consistent processing of collections and communities.The iteration over collections and communities follows the same pattern as items, maintaining consistency in the embargo info collection approach.
78-89: Clear and comprehensive reporting structure.The report generation provides both detailed listings and summary counts, making it easy to understand the embargo status across different object types. The distinction between objects that include parent references (bitstreams, bundles) and those that don't is appropriate.
98-105: Efficient embargo detection logic.The method correctly identifies resource policies with start or end dates, which indicates embargo restrictions. The logic is straightforward and handles both embargo start and end scenarios.
107-134: Well-formatted report output with appropriate limits.The appendReport method creates clear, tabular output with:
- Proper column headers and formatting
- Configurable display limits to prevent overwhelming output
- Consistent handling of optional parent item references
The use of
String.repeat()for the separator line is clean and readable.
136-148: Simple and effective data structure.The EmbargoInfo inner class is well-designed with:
- Appropriate fields for capturing embargo information
- Clean constructor that initializes all fields
- Package-private accessibility which is suitable for this use case
118-118: Java 11 minimum confirmed; String.repeat() compatibility OK
- pom.xml (line 21) defines
<java.version>11>.String.repeat()was introduced in Java 11, matching the project’s minimum version.No further changes needed.
There was a problem hiding this comment.
Pull Request Overview
This pull request adds a new health check feature that provides detailed reporting on embargo information across different DSpace objects. The embargo check identifies items, bitstreams, bundles, communities, and collections that have start or end dates defined in their resource policies.
- Implemented a new
EmbargoInfoCheckclass that scans all DSpace objects for embargo-related resource policies - Added the new embargo check to the health check configuration to make it available in the system
- Provides tabular output showing embargo details with counts and object UUIDs
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| dspace/config/modules/healthcheck.cfg | Adds the new embargo check to the list of available health checks and plugin configuration |
| dspace-api/src/main/java/org/dspace/health/EmbargoInfoCheck.java | Implements the embargo information health check that scans DSpace objects for embargo policies |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
dspace-api/src/test/java/org/dspace/identifier/VersionedHandleIdentifierProviderIT.java (1)
69-83: Add a descriptive reason to the @ignore and track itWe currently have 73 ignored tests across the codebase—blanket ignores without a clear link to an issue or feature flag make it hard to know when and why to re-enable them. For this test, please:
- Replace the inline comment with a formal reason in the annotation, pointing to a tracking ticket or PR.
- Optionally, guard the test at runtime using JUnit’s
Assumeso it only runs when versioning is enabled.- Open a follow-up issue (e.g., DS-XXXX) to revisit and re-enable this test once the provider is supported.
Locations to update:
- dspace-api/src/test/java/org/dspace/identifier/VersionedHandleIdentifierProviderIT.java:69
Minimal change:
- @Ignore // This test is ignored because it is not applicable to the current version of DSpace. + @Ignore("VersionedHandleIdentifierProvider not active under current configuration; see DS-XXXX")Optional conditional skip (add import near other imports):
import static org.junit.Assume.assumeTrue;Inside the test method:
assumeTrue("Skip when identifier.versioning.enabled==false", DSpaceServicesFactory.getInstance() .getConfigurationService() .getBooleanProperty("identifier.versioning.enabled", false));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
dspace-api/src/test/java/org/dspace/identifier/VersionedHandleIdentifierProviderIT.java(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: dspace / docker-build (linux/amd64, ubuntu-latest, true)
- GitHub Check: dspace-test / docker-build (linux/amd64, ubuntu-latest, true)
- GitHub Check: dspace-cli / docker-build (linux/amd64, ubuntu-latest, true)
- GitHub Check: Run Integration Tests
- GitHub Check: Run Unit Tests
🔇 Additional comments (1)
dspace-api/src/test/java/org/dspace/identifier/VersionedHandleIdentifierProviderIT.java (1)
27-27: Confirmed JUnit 4–only usage—no mixed JUnit 5 annotations detected
I ran the provided grep across the repository and found no references toorg.junit.jupiter(JUnit 5) while JUnit 4 annotations (@Test,@Before,@Ignore) are used consistently—including in VersionedHandleIdentifierProviderIT.java. It’s safe to keep usingorg.junit.Ignorehere.
Added embargo check to health report.
This is the output
Summary by CodeRabbit
New Features
Chores
Tests