quantranhong1999 commented on code in PR #2884:
URL: https://github.com/apache/james-project/pull/2884#discussion_r2609532816
##########
mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/DeleteMessageListener.java:
##########
@@ -270,11 +283,29 @@ private Mono<Void>
handleMessageDeletion(CassandraMessageId messageId, MailboxId
.then(threadLookupDAO.deleteOneRow(threadId, messageId)));
}
+ private Mono<Void> dispatchMessageContentDeletionEvent(MailboxId
mailboxId, Username owner, MessageRepresentation message) {
+ return
Mono.from(contentDeletionEventBus.dispatch(EventFactory.messageContentDeleted()
+ .randomEventId()
+ .user(owner)
+ .mailboxId(mailboxId)
+ .messageId(message.getMessageId())
+ .size(message.getSize())
+ .instant(message.getInternalDate().toInstant())
+ .hasAttachments(!message.getAttachments().isEmpty())
+ .headerBlobId(message.getHeaderId().asString())
+ .bodyBlobId(message.getBodyId().asString())
+ .build(),
+ ImmutableSet.of()));
+ }
+
private Mono<Void>
handleMessageDeletionAsPartOfMailboxDeletion(CassandraMessageId messageId,
ThreadId threadId, CassandraId excludedId, Username owner) {
return Mono.just(messageId)
.filterWhen(id -> isReferenced(id, excludedId))
.flatMap(id -> readMessage(id)
- .flatMap(message ->
Flux.fromIterable(deletionCallbackList).concatMap(callback ->
callback.forMessage(message, excludedId, owner)).then().thenReturn(message))
+ .flatMap(message -> Flux.fromIterable(deletionCallbackList)
+ .concatMap(callback -> callback.forMessage(message,
excludedId, owner))
+ .then(dispatchMessageContentDeletionEvent(excludedId,
owner, message)
Review Comment:
> I would however propose an optimisation on Rabbit group event bus side:
publis if and only if we have group listener regitered. However if no group
listeners are registered we can skip publishing in the first place.
We needed to register a noop listener on the content deletion listener.
Otherwise, the queue won't get created until there is a listener registered,
and the consumer healthcheck would fail (queue not found). That is kinda
against the idea you proposed...
Proposal:
- Always guice binding for the content deletion event bus
- Implement the messageContentDeleted dispatch as a callback e.g.
`DispatchMessageContentDeletionCallback`, and in James, bind it only when the
vault work queue is enabled.
So we do not waste time deserializing and dispatching the event when we do
not need it.
- In TMail, in the `AiBotModule`, we can bind the
`DispatchMessageContentDeletionCallback` + `RagDeletionListener`. Hopefully,
the `DispatchMessageContentDeletionCallback` can override the existing binding,
if any, from the vault module.
That way, we can have the dispatch work only when needed, regardless of
whether the vault or rag is enabled.
Thoughts?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]