Skip to content
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

Ensures the fedora id is sent in the body on reindex. #1

Merged
merged 3 commits into from Oct 5, 2021

Conversation

dbernstein
Copy link

Try this. Since reindexing uses a different path, it does not have an Activity Streams compliant json message like we have when receiving messages directly from fcrepo. Therefore I'm using a mustache template to pull in the fedora id on it's way out to the external http service.

@demiankatz
Copy link
Collaborator

@dbernstein, thanks for this! Unfortunately, the reindex.mustache file seems to be missing; can you commit that when time permits? Also, note that I removed the mustache dependency from this module (fcrepo-exts@842d311) because I didn't think we needed it... but now maybe that commit needs to be reverted to bring it back.

@demiankatz
Copy link
Collaborator

Thanks for the progress, @dbernstein. Unfortunately, it's still not working for me. I'm seeing the proper ID in the log message you added (e.g. "sending http://localhost:8080/rest/zzzz2 to http endpoint..."), but my HTTP endpoint receives an empty POST. I've used a browser-based REST plug-in to send POST data to the endpoint manually, so I can confirm that my endpoint is capable of receiving POST data... but it's not getting any from the Camel toolbox. Any ideas?

@demiankatz
Copy link
Collaborator

I should also clarify that the POST is empty whether I do a reindex, or just trigger an update by creating/editing an object in Fedora, so it's not a reindex-specific problem. (I assume that your code change is intended to ALWAYS include a POST body now).

@demiankatz
Copy link
Collaborator

Are you seeing different results on your end, or have you not been able to actually test this yet? Just curious where I should focus my troubleshooting, and knowing that context might help. :-)

@demiankatz
Copy link
Collaborator

In comparing the code here with the Solr delete logic, I am wondering if the absence of a CONTENT_TYPE setting might be the reason the body is not getting set as expected; I'm experimenting with that now...

@demiankatz
Copy link
Collaborator

Yes, that was it!! This patch should fix it (but I don't seem to have permission to push to this branch):

diff --git a/fcrepo-indexing-http/src/main/java/org/fcrepo/camel/indexing/http/HttpRouter.java b/fcrepo-indexing-http/src/main/java/org/fcrepo/camel/indexing/http/HttpRouter.java
index cd40d8fb..642db708 100644
--- a/fcrepo-indexing-http/src/main/java/org/fcrepo/camel/indexing/http/HttpRouter.java
+++ b/fcrepo-indexing-http/src/main/java/org/fcrepo/camel/indexing/http/HttpRouter.java
@@ -28,6 +28,7 @@ import org.fcrepo.camel.processor.EventProcessor;
 import org.slf4j.Logger;
 import org.springframework.beans.factory.annotation.Autowired;
 
+import static org.apache.camel.Exchange.CONTENT_TYPE;
 import static org.apache.camel.Exchange.HTTP_METHOD;
 import static org.slf4j.LoggerFactory.getLogger;
 
@@ -83,6 +84,7 @@ public class HttpRouter extends RouteBuilder {
             .log(LoggingLevel.INFO, LOGGER, "sending ${headers[CamelFcrepoUri]} to http endpoint...")
             .to("mustache:org/fcrepo/camel/indexing/http/reindex.mustache")
             .setHeader(HTTP_METHOD).constant("POST")
+            .setHeader(CONTENT_TYPE).constant("application/json")
             .to(config.getHttpBaseUrl());
     }
 }

@demiankatz
Copy link
Collaborator

Now that the technical issue is resolved, I think the next question is what the JSON message should actually look like... I don't think we really want a reindex.mustache; maybe it's more of an http-update.mustache that is needed, since the same template is currently getting used for all contexts. We also need to be able to differentiate between create/update/delete messages (with reindex acting as update, I would think).

I'm thinking we probably want a message more like:

{
    'type': 'create|update|delete',
    'id': 'http://localhost:8080/rest/record-id',
}

What do you think?

@demiankatz demiankatz merged commit 794a624 into Geoffsc:FCREPO-3753 Oct 5, 2021
@demiankatz
Copy link
Collaborator

I've gone ahead and merged this since it's incremental progress, and I can push the rest of the changes/fixes back on the original PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants