-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Accept and pass along Fedora base url #65
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #65 +/- ##
============================================
+ Coverage 85.33% 89.76% +4.42%
+ Complexity 98 73 -25
============================================
Files 16 10 -6
Lines 341 293 -48
Branches 1 1
============================================
- Hits 291 263 -28
+ Misses 49 29 -20
Partials 1 1
Continue to review full report at Codecov.
|
@@ -129,11 +132,15 @@ public void configure() { | |||
.setProperty("event").simple("${body}") | |||
.setProperty("uuid").simple("${exchangeProperty.event.object.id.replaceAll(\"urn:uuid:\",\"\")}") | |||
.setProperty("jsonldUrl").simple("${exchangeProperty.event.object.url[2].href}") | |||
.setProperty("fedoraUrl").simple("${exchangeProperty.event.target}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
...dora-event-support/src/main/java/ca/islandora/alpaca/support/event/AS2AttachmentContent.java
Outdated
Show resolved
Hide resolved
islandora-indexing-fcrepo/src/main/java/ca/islandora/alpaca/indexing/fcrepo/FcrepoIndexer.java
Show resolved
Hide resolved
@dannylamb your comments lead me to believe we need to make the differentiation between the "Base URL of a Fedora instance" and a "single Fedora resource URI" clearer or others will get confused. I'm still fighting with this and may end up backing out if these annotations don't help me out. But either way I'll try to make it clearer. |
Okay, I think this is ready. I was able to get it working passing stuff around and still indexing in Fedora and creating derivatives. I updated the instructions at the top to get the various dynamic routes to regenerate after you install the new |
@whikloj |
I'm good with the changes you made @whikloj 👍 |
Clean up PMD ruleset and make changes
do you need someone to test this? |
@elizoller sure, if you want to give it a test that would be great. |
I've managed to get this installed and working with all the other pieces. I'm happy to pull all this work in, but Travis is not happy 😢 |
Also, if you want tests to pass on the main islandora repo, this'll need to get merged beforehand: Islandora/islandora#747 |
I feel like we should consider removing Oracle Java from our testing matrix, but I'll spin up a Travis box and see if this is a simple fix. |
@whikloj I'm fine with sticking to openjdk for testing |
@dannylamb fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
GitHub Issue: Islandora/documentation#926
Related to:
What does this Pull Request do?
Instead of setting the Fedora base url in the microservice configuration we can instead provide it in the message. It is possible that we retain the configuration argument as a fall back, but if you have multiple containers for a multi-site you might want to dynamically change the base url per request.
This work relies on @Natkeeran's work in the above branches to send the event and receive the event.
I went with the header
X-Islandora-Fedora-Endpoint
but this could be a configuration parameter if we like.How should this be tested?
I did not test this with the derivative connectors as that would require a full playbook setup which I did not have.
vagrant ssh
) I did/opt/karaf/data/logs/islandora.log
like:Interested parties
@Islandora/8-x-committers especially @Natkeeran