[cristianscheid]

• Resolves: #

Summary

• support new notification payload format on /ocm/notifications
• since OCM 1.5, notification payloads include a senderDomain field:
  https://github.com/cs3org/OCM-API/blob/v1.5.0/IETF-OCM.md#notifications
• RequestHandlerController::receiveNotification() checks whether the payload has a senderDomain field:
  • with senderDomain field: handles it with the new format
  • without senderDomain field: handled as before
• for the new format, request must be signed by senderDomain (checked with IOCMDiscoveryService::getIncomingSignedRequestFromDomain())
• instead of routing to a cloud federation provider (e.g. CloudFederationProviderFiles), an OCMNotificationReceivedEvent is dispatched, which apps can listen to and act on
• listeners can set response that is sent back to the sender with OCMNotificationReceivedEvent::setResult()
• the event carries a ISenderDomainAwareCloudFederationNotification, which extends ICloudFederationNotification with senderDomain, shareType
• to let other servers know this format is supported, new capability is advertised in discovery: notification-sender-domain

TODO

• this is a draft to discuss whether this is a good approach
• if we go with it, refine code, add tests and do manual testing

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)
• Labels added where applicable (ex: bug/enhancement, 3. to review, feature component)
• Milestone added for target branch/version (ex: 32.x for stable32)

AI (if applicable)

• The content of this PR was partly or fully generated using AI

[mickenordin]

Just a few comments

[mickenordin]

I guess this will throw if there is no listener? Perhaps that is not what we want?

[cristianscheid]

with no listener, yes, it would throw, but the exception is handled and the sender of the notification would get a 501 (Http::STATUS_NOT_IMPLEMENTED)

looking at the spec for 501: https://github.com/cs3org/OCM-API/blob/v1.5.0/spec.yaml#L95

you think we should return something else, maybe 201?

[mickenordin]

with no listener, yes, it would throw, but the exception is handled and the sender of the notification would get a 501 (Http::STATUS_NOT_IMPLEMENTED)

looking at the spec for 501: https://github.com/cs3org/OCM-API/blob/v1.5.0/spec.yaml#L95

you think we should return something else, maybe 201?

That is for /shares /notifications is a bit down. Maybe 501 is ok, or alternatively 404: https://github.com/cs3org/OCM-API/blob/277e020f2737998238c5bf68689bd8c2da1ee5f9/spec.yaml#L162

[mickenordin]

Another thing we may want to do, is deprecate notificationRecieved in cloudFederationProvider, and try to have the core implementations stop relying on this:

server/apps/cloud_federation_api/lib/Controller/RequestHandlerController.php

Lines 318 to 319 in ca11c0d

	$provider = $this->cloudFederationProviderManager->getCloudFederationProvider($resourceType);
	$result = $provider->notificationReceived($notificationType, $providerId, $notification);

and instead add listeners.

[cristianscheid]

That is for /shares /notifications is a bit down

yes, sorry linked the wrong line

Maybe 501 is ok, or alternatively 404

between those two I would lean towards 501, what you think?

Another thing we may want to do, is deprecate notificationRecieved in cloudFederationProvider, and try to have the core implementations stop relying on this and instead add listeners.

I wonder whether we should tackle this here, or if it would fit better as a follow up PR

cc @mickenordin