Conversation
WalkthroughThis pull request simplifies exception handling across several modules. Method signatures in service implementations, interfaces, scripts, and repositories have been updated to throw a generic Changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/MetadataBitstreamRestRepository.java (1)
70-71: Changed exception handling to use a generic Exception type.The method signature has been modified to throw a generic
Exceptioninstead of specific exceptions (SQLException,ParserConfigurationException,IOException,SAXException,AuthorizeException, andArchiveException). This simplifies the exception specification but reduces type safety by hiding expected failure cases from callers.While this change aligns with the PR objective to better handle exceptions during file preview generation, consider maintaining specific exception types in public API methods for better caller awareness. Alternatively, document the potential exceptions that might be thrown in the method's JavaDoc.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
dspace-api/src/main/java/org/dspace/content/PreviewContentServiceImpl.java(4 hunks)dspace-api/src/main/java/org/dspace/content/service/PreviewContentService.java(2 hunks)dspace-api/src/main/java/org/dspace/scripts/filepreview/FilePreview.java(2 hunks)dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/MetadataBitstreamRestRepository.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: dspace-dependencies / docker-build (linux/amd64, ubuntu-latest, true)
- GitHub Check: Run Integration Tests
- GitHub Check: Run Unit Tests
🔇 Additional comments (9)
dspace-api/src/main/java/org/dspace/scripts/filepreview/FilePreview.java (2)
102-105: Simplified exception catch block to handle all exceptions.The catch block now handles the generic
Exceptiontype instead of multiple specific exceptions. This change is aligned with the PR objective to catch all exceptions during file preview generation.
117-117: Method signature updated to throw a generic Exception.The method signature has been changed from declaring multiple specific exceptions to a single
Exceptiontype. This simplifies the method signature but reduces the clarity of what exceptions callers should expect.dspace-api/src/main/java/org/dspace/content/service/PreviewContentService.java (2)
118-118: Interface method signature updated to throw a generic Exception.The
getFilePreviewContentmethod now throws a genericExceptioninstead of specific exceptions (SQLException,AuthorizeException,IOException). This change simplifies the interface but reduces type information for implementers and callers.
153-154: Interface method signature updated to throw a generic Exception.The
processInputStreamToFilePreviewmethod now throws a genericExceptioninstead of specific exceptions (SQLException,IOException). This change aligns with the PR's goal of simplifying exception handling.dspace-api/src/main/java/org/dspace/content/PreviewContentServiceImpl.java (5)
154-155: Method signature updated to throw a generic Exception.The
getFilePreviewContentmethod implementation now throws a genericExceptioninstead of specific exceptions, matching the interface change. Additionally, it appears the nested exception handling has been removed, allowing exceptions to propagate up the call stack.
163-163: Removed try-catch block for specific exception.A try-catch block for
IllegalStateExceptionhas been removed, allowing any exceptions fromprocessInputStreamToFilePreviewto propagate to the caller. This change streamlines the error handling flow.
191-192: Method signature updated to throw a generic Exception.The
processInputStreamToFilePreviewmethod implementation now throws a genericExceptionrather than specific exceptions, matching the interface change.
215-216: Removed try-catch blocks around file extraction.Try-catch blocks have been removed around the calls to
extractFileandFileTreeViewGenerator.parse, allowing any exceptions to propagate to the caller. This simplifies the control flow and ensures exceptions are logged at a higher level.
415-415: Method signature updated to throw a generic Exception.The
extractFilemethod now declares that it can throwExceptioninstead of handling exceptions internally. This change allows for better error propagation and more centralized error handling.
…dev/DSpace into ufal/file-preview-better-logs
Merging latest dataquest-dev/dspace:dtq-dev This contains the following commits: Run build action every 4h for every customer/ branch UFAL/Do not use not-existing metadatafield `hasMetadata` in the submission-forms-cz (dataquest-dev#888) UFAL/Created job to generate preview for every item or for a specific one (dataquest-dev#887) UFAL/bitstream preview wrong file name according to it's mimetype (dataquest-dev#890) Fixed typo in the error exception The owning community was null. (dataquest-dev#891) The `+` characted was wrongly encoded in the URL (dataquest-dev#893) Set limit when splitting key/value using `=` (dataquest-dev#894) Ufal/header value could have equals char (dataquest-dev#895) UFAL/File preview - Added the method for extracting the file into try catch block (dataquest-dev#909) UFAL/File preview better logs (dataquest-dev#910) UFAL/File preview - Return empty list if an error has occured (dataquest-dev#915) UFAL/Matomo fix tracking of the statistics (dataquest-dev#912) UFAL/Matomo statistics - Use the bitstream name instead of the UUID in the tracking download url (dataquest-dev#917) UFAL/Matomo bitstream tracker has error when bitstream name was null (dataquest-dev#918) UFAL/Endpoints leaks private information (dataquest-dev#924) UFAL/Fix parts identifiers resolution (dataquest-dev#913) UFAL/Update `clarin-dspace.cfg` - handle.plugin.checknameauthority (dataquest-dev#897) Creating Legal check (dataquest-dev#863) import/comment-license-script (dataquest-dev#882) UFAL/Renamed property dspace.url to dspace.ui.url (dataquest-dev#906)
Problem description
Catch every exception during generating the file preview and log info about the item and the exception.
Summary by CodeRabbit