Issue 1827: The wrong URLs are being used to deposit. #1831
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The current behavior is to set the URL to `servicedocument` to perform the testing. This ends up being used by both the `getCollections()` and `deposit()` end points. If the Repository URL is set to say `http://example.com/sword/servicedocument`, then this works for the `getCollections()` and breaks for the `deposit()` end points. If the Repository URL is set to say `http://example.com/sword/deposit`, then this breaks for the `getCollections()` and works for the `deposit()` end points. Both need to work. Change the Repository URL configuration design to instead except a Repository URL of `http://example.com/sword/`. Now, when `getCollections()` end point is called, it appends the `/servicedocument` to that URL. Now, when `deposit()` end point is called, it appends the `/deposit` to that URL. This could be simplified further to append to not have the `/sword` and then append that as needed. However, this change is considered out of scope and is left for future discussion.
rmathew1011
approved these changes
Jun 30, 2023
… behavior. Add a configuration file witht he configuration `vireo.depositor.swordv1.single-url` to designate how to handle the URL. This allows for backwards compatibility in case the new behavior is a problem in some system. The `SWORDv1Depositor` itself is not detectable by Spring and so `@Autowired` does not work. Fix this by adding an `@Bean` in the configuration and calling that loader instead of `new SWORDv1Depositor()` in the system data loader.
rmathew1011
approved these changes
Jul 6, 2023
…tory URL behavior." This reverts commit be86fa3. The reverted commit is not needed now that the problem is identified to be that the repositoryUrl is not being used as original expected.
Partially restore the previous behavior of using the `/servicedocument`. It turns out that while the repository URL gets used, it gets stripped to just the host name and port number during deposit. The swordv1 library ends up adding the deposit location from the service document manifest. Using the Repository URL in the error logs is misleading and confusing. Change the behavior to better describe what the given URL is on error. The sword handles the actual deposit location internally and is not as readily available for use in the error log.
rmathew1011
approved these changes
Jul 10, 2023
jcreel
approved these changes
Jul 10, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
resolves #1827
The current behavior is to set the URL to contain
servicedocument, such ashttp://example.com/sword/servicedocument) to perform the testing.This ends up being used by both the
getCollections()anddeposit()end points.If the Repository URL is set to say
http://example.com/sword/servicedocument, then this works for thegetCollections()but causes problems for thedeposit()end points.The Repository URL ends up getting used in the
deposit()end point but everything except for the host name and port are stripped off.This leads to confusing error messages and reporting that makes it look like the
/depositend point is not being used.This end point ends up being used after all but through the sword client via the service document manifest.
The previous behavior of expecting
http://example.com/swordand appending/servicedocumentand/depositis reverted. The configuration for single url is no longer needed now that the problem has been identified and that commit has also been reverted.