When will this PR be reviewed / merged?

PR is now up to date with master

Rebased to the latest master again, and added a changelog entry.

yes i have stumbled about an issue in converse js (missing body in retraction message) which would also applies to your concerns... i am going to redesign this PR in the next days :)

Okay i have refactored the whole PR... please review it :)

I've got a number of concerns regarding this:

• the placement of the checkbox on the admin console makes me wonder if the chat marker setting applies to both types of messages (one-on-one and MUC).
• We might want something similar for other message stanzas that don't have a body (eg: XEP-0184: Message Delivery Receipts, XEP-0085: Chat State Notifications)
• The code to distill the type of marker from a message seems overly complex. The end result is hardly used.
• The newly added field org.jivesoftware.openfire.archive.ConversationEvent#marker doesn't seem to be used. Does that mean that this functionality is broken in a cluster?
• and, somewhat trivial: the MARKERTYPE enum shouldn't have an all-caps name.
• the second non-null check in if (message.getBody() != null||(message.getBody()==null&&this.conversationManager.isChatmarkerArchivingEnabled())) is unneeded
• the naming of ConversationEvent.chatmarkerMessageReceived suggests that this is an event handler, but it's not. This duplicates a similarly misnamed, pre-existing org.jivesoftware.openfire.archive.ConversationEvent#chatMessageReceived though.
• I'm not often one to comment on whitespace, but the (lack of) indentation of the four lines in this if-statement is very confusing:

if (markertype!=ChatMarker.MARKERTYPE.NONE)
{
    eventsQueue.addChatEvent(conversationManager.getConversationKey(sender, receiver),
    ConversationEvent.chatmarkerMessageReceived(sender, receiver,markertype,
    conversationManager.isMessageArchivingEnabled() ? message.toXML() : null,
    new Date()));
}

My suggestion would be to redesign this functionality, to become something like: "log message stanzas without bodies if it includes metadata for:" (and then offer a checkbox for all types of child elements that we want to support).

review plz :)

@mightymop: One year after your changes, have you received a review from @guusdk?

@mightymop: One year after your changes, have you received a review from @guusdk?

nope

I have rebased this to the current main branch head.

I identified at least one bug, and several inconsistencies. I've committed a number of changes that each address some concerns that I had. The current state of the code is untested. I particularly worry how this behaves in a cluster.

Additional thought: maybe we should support for XEP-0334: Message Processing Hints. Server-side rules are generally good, but they aren't discoverable and may lag behind. If a new XEP is written tomorrow and we do/don't want that in MAM, hints can help with that. On the other hand, I'm wondering if having some clients having their empty-bodied messages stored in MAM, is introducing more confusion than having either all or none of them do that. That probably only applies to MUC archives though.

I'm now noticing more code duplication, in the sense that most of the configuration is duplicated for MUC vs. non-MUC messages. That also makes me wonder if my previous refactoring has taken that into account correctly.

In all, I'm disliking the over-complication of this solution: both in the implementation, as well as the UI options that are presented to the end-user.