-
Notifications
You must be signed in to change notification settings - Fork 529
IQSS/8914 COAR compliant LDN messaging #10490
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
base: develop
Are you sure you want to change the base?
IQSS/8914 COAR compliant LDN messaging #10490
Conversation
GDCC/8914-COAR-compliant_messaging
|
Note: A v.1.0.0 version of the COAR Notify specification has just been released - see https://coar-notify.net/2024/significant-revision-to-specification/. This PR has not yet been checked to make sure it is consistent with the changes in this release. |
pdurbin
left a comment
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.
API tests are passing. The code looks good. I flagged a few issues with docs. Merge conflicts need to be resolved.
GDCC/8914-COAR-compliant_messaging
Co-authored-by: Philip Durbin <[email protected]>
pdurbin
left a comment
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.
Thanks for making the changes I suggested. Approved.
GDCC/8914-COAR-compliant_messaging_orig
|
@qqmyers Please resolve the conflicts |
GDCC/8914-COAR-compliant_messaging_orig
| String message = createRelationshipAnnouncementMessage(citingResourceId, datasetPid, relationship); | ||
|
|
||
| // Send the message to the LDN inbox | ||
| Response response = UtilIT.sendMessageToLDNInbox(message); |
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.
I'm running this locally with docker
message =
{
"@context": [
"https://www.w3.org/ns/activitystreams",
"https://coar-notify.net"
],
"id": "urn:uuid:02d60b78-bec4-427f-af50-c0005a00da94",
"actor": {
"id": "https://10.0.0.235",
"name": "LDN IT Tester",
"type": "Service"
},
"context": {
"id": "doi:10.5072/FK2/YJ6NCB"
},
"object": {
"as:object": "doi:10.5072/FK2/YJ6NCB",
"as:relationship": "https://purl.org/datacite/ontology#Cites",
"as:subject": "https://doi.org/10.1234/example-publication",
"id": "urn:uuid:98cbc096-d8c4-4b7e-bdf6-ea137f4127f6",
"type": "Relationship"
},
"origin": {
"id": "https://10.0.0.235",
"inbox": "https://10.0.0.235/api/inbox",
"type": "Service"
},
"target": {
"id": "http://localhost:8080",
"inbox": "http://localhost:8080/api/inbox",
"type": "Service"
},
"type": [
"Announce",
"coar-notify:RelationshipAction"
]
}
The response is expecting a 200 but I'm getting
{
"status": "ERROR",
"code": 403,
"message": "Not authorized to access this object via this API endpoint. Please check your code for typos, or consult our API guide at http://guides.dataverse.org.",
"requestUrl": "http://localhost:8080/api/v1/inbox/",
"requestMethod": "POST"
}
|
I'm also seeing a lot of these in the log 6.2025.3|edu.harvard.iq.dataverse.api.LDNInbox|_ThreadID=240;_ThreadName=http-thread-pool::http-listener-1(7);_TimeMillis=1763736091927;_LevelValue=1000;| Ignoring message from IP address: 178.128.69.202|#] |
|
LDNInboxIT is not included in tests/integration-tests.txt which explains why there were no failures in the pipeline tests |
| jsonld = JSONLDUtil.decontextualizeJsonLD(body); | ||
| try { | ||
| IpAddress origin = new DataverseRequest(null, httpRequest).getSourceAddress(); | ||
| String allowedIPs = JvmSettings.LINKEDDATANOTIFICATION_ALLOWED_HOSTS.lookupOptional().orElse(""); |
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.
Should this default to empty string or wildcard ?
String allowedIPs = JvmSettings.LINKEDDATANOTIFICATION_ALLOWED_HOSTS.lookupOptional().orElse("*");
For some reason in the test it's always returning an empty string
even when I added @JvmSetting(key = JvmSettings.LINKEDDATANOTIFICATION_ALLOWED_HOSTS, value = "178.128.69.202") to the test it still returns empty string
logger.severe(">>>> allowedIPs " + allowedIPs);
logger.severe(">>>> origin " + origin.toString());
log:
[#|2025-11-21T16:20:02.740+0000|SEVERE|Payara 6.2025.3|edu.harvard.iq.dataverse.api.LDNInbox|_ThreadID=111;_ThreadName=http-thread-pool::http-listener-1(5);_TimeMillis=1763742002740;_LevelValue=1000;|
allowedIPs |#]
[#|2025-11-21T16:20:02.741+0000|SEVERE|Payara 6.2025.3|edu.harvard.iq.dataverse.api.LDNInbox|_ThreadID=111;_ThreadName=http-thread-pool::http-listener-1(5);_TimeMillis=1763742002741;_LevelValue=1000;|
origin 178.128.69.202|#]
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.
got >>>> allowedIPs *|#] when I added -Ddataverse.ldn.allowed-hosts=* to docker-compose-dev.yml
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.
The default should stay "" so that people who don't enable this reject all messages. I've ~fixed the tests by not trying to use @JvmSettings in IT tests (which doesn't work) and adding unit tests that call the api method directly.
What this PR does / why we need it: This PR updates our COAR LDN messaging to be compliant with the final specification and improves/updates the code in various ways now that several sites will be deploying it.
Which issue(s) this PR closes:
Special notes for your reviewer: In testing, I found that the TDL dev server was failing to show the notifications page due to the JSONUtil.getJsonObject(String) method being overloaded by the JSONUtil.getJsonObject(InputStream) method - the log was showing a problem converting the input string to an InputStream. The PR includes a change to make the stream version getJsonObjectFromInputStream() to avoid this.
Suggestions on how to test this: This has been tested against the Harvard DASH test repository and with TDL's DSpace test instance. The code includes IT tests related to receiving and parsing messages. It is possible to manually replicate those tests by sending the appropriate JSON to the /api/inbox endpoint.
Turning on fine logging for the various classes (LDN Inbox, COARNotificationRelationshipAnnouncement, COARNotificationRelationshipAnnouncementStep) will show what messages are being received or sent in the server.log
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Is there a release notes update needed for this change?: included
Additional documentation: included