From 94790f3ef21eb4162a526b1285c91f7c52161e63 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Tue, 1 Dec 2020 14:45:09 +0100 Subject: [PATCH 01/19] Minor tweaks Stopped using deprecated API of PluginManager. Applied various IDE suggestions. --- .../impl/PaginatedMucMessageLuceneQuery.java | 13 +++--- .../archive/xep0313/IQQueryHandler.java | 17 ++++--- .../openfire/archive/Conversation.java | 45 +++++++------------ .../openfire/archive/ConversationEvent.java | 2 +- .../archive/ConversationEventsQueue.java | 6 +-- .../openfire/archive/MonitoringConstants.java | 7 +++ .../cluster/GetConversationCountTask.java | 24 +++++++--- .../archive/cluster/GetConversationTask.java | 22 ++++++--- .../archive/cluster/GetConversationsTask.java | 22 ++++++--- .../cluster/GetConversationsWriteETATask.java | 19 ++++++-- .../cluster/SendConversationEventsTask.java | 23 ++++++---- .../GetGroupConversationTranscript.java | 18 ++++---- .../reporting/stats/GetStatistics.java | 6 +-- 13 files changed, 139 insertions(+), 85 deletions(-) diff --git a/src/java/com/reucon/openfire/plugin/archive/impl/PaginatedMucMessageLuceneQuery.java b/src/java/com/reucon/openfire/plugin/archive/impl/PaginatedMucMessageLuceneQuery.java index ea5211c8d..f3ad7f963 100644 --- a/src/java/com/reucon/openfire/plugin/archive/impl/PaginatedMucMessageLuceneQuery.java +++ b/src/java/com/reucon/openfire/plugin/archive/impl/PaginatedMucMessageLuceneQuery.java @@ -11,6 +11,7 @@ import org.apache.lucene.search.*; import org.jivesoftware.openfire.XMPPServer; import org.jivesoftware.openfire.archive.MonitoringConstants; +import org.jivesoftware.openfire.container.Plugin; import org.jivesoftware.openfire.muc.MUCRoom; import org.jivesoftware.openfire.plugin.MonitoringPlugin; import org.slf4j.Logger; @@ -18,10 +19,7 @@ import org.xmpp.packet.JID; import java.io.IOException; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Date; -import java.util.List; +import java.util.*; public class PaginatedMucMessageLuceneQuery { @@ -44,8 +42,11 @@ public PaginatedMucMessageLuceneQuery( final Date startDate, final Date endDate, protected IndexSearcher getSearcher() throws IOException { - final MonitoringPlugin plugin = (MonitoringPlugin) XMPPServer.getInstance().getPluginManager().getPlugin(MonitoringConstants.NAME); - final MucIndexer mucIndexer = (MucIndexer) plugin.getModule(MucIndexer.class); + final Optional plugin = XMPPServer.getInstance().getPluginManager().getPluginByName(MonitoringConstants.PLUGIN_NAME); + if (!plugin.isPresent()) { + throw new IllegalStateException("Unable to obtain Lucene Index Searcher! The Monitoring plugin does not appear to be loaded on this machine."); + } + final MucIndexer mucIndexer = (MucIndexer) ((MonitoringPlugin)plugin.get()).getModule(MucIndexer.class); final IndexSearcher searcher = mucIndexer.getSearcher(); return searcher; } diff --git a/src/java/com/reucon/openfire/plugin/archive/xep0313/IQQueryHandler.java b/src/java/com/reucon/openfire/plugin/archive/xep0313/IQQueryHandler.java index 9b1af50f1..254d8b3b0 100644 --- a/src/java/com/reucon/openfire/plugin/archive/xep0313/IQQueryHandler.java +++ b/src/java/com/reucon/openfire/plugin/archive/xep0313/IQQueryHandler.java @@ -1,6 +1,7 @@ package com.reucon.openfire.plugin.archive.xep0313; import com.reucon.openfire.plugin.archive.ArchiveProperties; +import com.reucon.openfire.plugin.archive.impl.MucIndexer; import com.reucon.openfire.plugin.archive.model.ArchivedMessage; import com.reucon.openfire.plugin.archive.xep.AbstractIQHandler; import com.reucon.openfire.plugin.archive.xep0059.XmppResultSet; @@ -10,6 +11,7 @@ import org.jivesoftware.openfire.archive.ConversationManager; import org.jivesoftware.openfire.archive.MonitoringConstants; import org.jivesoftware.openfire.auth.UnauthorizedException; +import org.jivesoftware.openfire.container.Plugin; import org.jivesoftware.openfire.disco.ServerFeaturesProvider; import org.jivesoftware.openfire.forward.Forwarded; import org.jivesoftware.openfire.muc.MUCRole; @@ -231,8 +233,11 @@ public IQ handleIQ( final IQ packet ) throws UnauthorizedException { queryRequest = new QueryRequest(packet.getChildElement(), archiveJid); // OF-1200: make sure that data is flushed to the database before retrieving it. - final MonitoringPlugin plugin = (MonitoringPlugin) XMPPServer.getInstance().getPluginManager().getPlugin(MonitoringConstants.NAME); - final ConversationManager conversationManager = (ConversationManager)plugin.getModule( ConversationManager.class); + final Optional plugin = XMPPServer.getInstance().getPluginManager().getPluginByName(MonitoringConstants.PLUGIN_NAME); + if (!plugin.isPresent()) { + throw new IllegalStateException("Unable to handle IQ stanza. The Monitoring plugin does not appear to be loaded on this machine."); + } + final ConversationManager conversationManager = (ConversationManager) ((MonitoringPlugin)plugin.get()).getModule(ConversationManager.class); final Instant targetEndDate = Instant.now(); // TODO or, the timestamp of the element referenced by 'before' from RSM, if that's set. final QueryRequest finalQueryRequest = queryRequest; @@ -328,9 +333,11 @@ private Collection retrieveMessages(QueryRequest queryRequest) String startField = null; String endField = null; String textField = null; - final MonitoringPlugin plugin = (MonitoringPlugin) XMPPServer.getInstance().getPluginManager().getPlugin(MonitoringConstants.NAME); - final ConversationManager conversationManager = (ConversationManager)plugin.getModule( ConversationManager.class); - + final Optional plugin = XMPPServer.getInstance().getPluginManager().getPluginByName(MonitoringConstants.PLUGIN_NAME); + if (!plugin.isPresent()) { + throw new IllegalStateException("Unable to retrieve messages. The Monitoring plugin does not appear to be loaded on this machine."); + } + final ConversationManager conversationManager = (ConversationManager) ((MonitoringPlugin)plugin.get()).getModule(ConversationManager.class); DataForm dataForm = queryRequest.getDataForm(); if(dataForm != null) { diff --git a/src/java/org/jivesoftware/openfire/archive/Conversation.java b/src/java/org/jivesoftware/openfire/archive/Conversation.java index f283fb9c4..fe25c7bb7 100644 --- a/src/java/org/jivesoftware/openfire/archive/Conversation.java +++ b/src/java/org/jivesoftware/openfire/archive/Conversation.java @@ -24,21 +24,14 @@ import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.Comparator; -import java.util.Date; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.concurrent.ConcurrentHashMap; import org.jivesoftware.database.DbConnectionManager; import org.jivesoftware.database.JiveID; import org.jivesoftware.database.SequenceManager; import org.jivesoftware.openfire.XMPPServer; +import org.jivesoftware.openfire.container.Plugin; import org.jivesoftware.openfire.muc.MUCRole; import org.jivesoftware.openfire.muc.MUCRoom; import org.jivesoftware.openfire.plugin.MonitoringPlugin; @@ -47,7 +40,6 @@ import org.jivesoftware.util.JiveGlobals; import org.jivesoftware.util.LocaleUtils; import org.jivesoftware.util.NotFoundException; -import org.jivesoftware.util.StringUtils; import org.jivesoftware.util.cache.ExternalizableUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -121,7 +113,7 @@ public Conversation(ConversationManager conversationManager, Collection use throw new IllegalArgumentException("Illegal number of participants: " + users.size()); } this.conversationManager = conversationManager; - this.participants = new HashMap(2); + this.participants = new HashMap<>(2); // Ensure that we're use the full JID of each participant. for (JID user : users) { UserParticipations userParticipations = new UserParticipations(false); @@ -155,7 +147,7 @@ public Conversation(ConversationManager conversationManager, Collection use */ public Conversation(ConversationManager conversationManager, JID room, boolean external, Date startDate) { this.conversationManager = conversationManager; - this.participants = new ConcurrentHashMap(); + this.participants = new ConcurrentHashMap<>(); // Add list of existing room occupants as participants of this conversation MUCRoom mucRoom = XMPPServer.getInstance().getMultiUserChatManager().getMultiUserChatService(room).getChatRoom(room.getNode()); if (mucRoom != null) { @@ -221,7 +213,7 @@ public JID getRoom() { * @return the two conversation participants. Returned JIDs are full JIDs. */ public Collection getParticipants() { - List users = new ArrayList(); + List users = new ArrayList<>(); for (String key : participants.keySet()) { users.add(new JID(key)); } @@ -294,7 +286,7 @@ public List getMessages() { return Collections.emptyList(); } - List messages = new ArrayList(); + List messages = new ArrayList<>(); Connection con = null; PreparedStatement pstmt = null; ResultSet rs = null; @@ -345,9 +337,9 @@ public List getMessages() { String leftBody; if (anonymous) { joinBody = LocaleUtils.getLocalizedString("muc.conversation.joined.anonymous", MonitoringConstants.NAME, - Arrays.asList(participation.getNickname())); + Collections.singletonList(participation.getNickname())); leftBody = LocaleUtils.getLocalizedString("muc.conversation.left.anonymous", MonitoringConstants.NAME, - Arrays.asList(participation.getNickname())); + Collections.singletonList(participation.getNickname())); } else { joinBody = LocaleUtils.getLocalizedString("muc.conversation.joined", MonitoringConstants.NAME, Arrays.asList(participation.getNickname(), name)); @@ -361,11 +353,7 @@ public List getMessages() { } } // Sort messages by sent date - Collections.sort(messages, new Comparator() { - public int compare(ArchivedMessage o1, ArchivedMessage o2) { - return o1.getSentDate().compareTo(o2.getSentDate()); - } - }); + messages.sort(Comparator.comparing(ArchivedMessage::getSentDate)); } return messages; } @@ -519,8 +507,6 @@ private void insertIntoDb(JID participant, String nickname, long joined) throws pstmt.setString(5, nickname); pstmt.executeUpdate(); pstmt.close(); - } catch (SQLException sqle) { - throw sqle; } finally { DbConnectionManager.closeConnection(con); } @@ -546,7 +532,7 @@ private void loadFromDb() throws NotFoundException { rs.close(); pstmt.close(); - this.participants = new ConcurrentHashMap(); + this.participants = new ConcurrentHashMap<>(); pstmt = con.prepareStatement(LOAD_PARTICIPANTS); pstmt.setLong(1, conversationID); rs = pstmt.executeQuery(); @@ -605,11 +591,14 @@ public void writeExternal(ObjectOutput out) throws IOException { } } - public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException { - MonitoringPlugin plugin = (MonitoringPlugin) XMPPServer.getInstance().getPluginManager().getPlugin(MonitoringConstants.NAME); - conversationManager = (ConversationManager) plugin.getModule(ConversationManager.class); + public void readExternal(ObjectInput in) throws IOException { + final Optional plugin = XMPPServer.getInstance().getPluginManager().getPluginByName(MonitoringConstants.PLUGIN_NAME); + if (!plugin.isPresent()) { + throw new IllegalStateException("Unable to handle IQ stanza. The Monitoring plugin does not appear to be loaded on this machine."); + } + conversationManager = (ConversationManager) ((MonitoringPlugin)plugin.get()).getModule(ConversationManager.class); - this.participants = new ConcurrentHashMap(); + this.participants = new ConcurrentHashMap<>(); conversationID = ExternalizableUtil.getInstance().readLong(in); ExternalizableUtil.getInstance().readExternalizableMap(in, participants, getClass().getClassLoader()); diff --git a/src/java/org/jivesoftware/openfire/archive/ConversationEvent.java b/src/java/org/jivesoftware/openfire/archive/ConversationEvent.java index 824843636..463923e59 100644 --- a/src/java/org/jivesoftware/openfire/archive/ConversationEvent.java +++ b/src/java/org/jivesoftware/openfire/archive/ConversationEvent.java @@ -64,7 +64,7 @@ else if (Type.occupantJoined == type) { } else if (Type.occupantLeft == type) { conversationManager.leftGroupConversation(roomJID, user, date); - // If there are no more occupants then consider the group conversarion over + // If there are no more occupants then consider the group conversation over MUCRoom mucRoom = XMPPServer.getInstance().getMultiUserChatManager().getMultiUserChatService(roomJID).getChatRoom(roomJID.getNode()); if (mucRoom != null && mucRoom.getOccupantsCount() == 0) { conversationManager.roomConversationEnded(roomJID, date); diff --git a/src/java/org/jivesoftware/openfire/archive/ConversationEventsQueue.java b/src/java/org/jivesoftware/openfire/archive/ConversationEventsQueue.java index 426d87ac6..1bc275a79 100644 --- a/src/java/org/jivesoftware/openfire/archive/ConversationEventsQueue.java +++ b/src/java/org/jivesoftware/openfire/archive/ConversationEventsQueue.java @@ -31,12 +31,12 @@ /** * Queue conversation events generated by this JVM and send them to the senior cluster * member every 3 seconds. This is an optimization to reduce traffic between the cluster - * nodes specialy when under heavy conversations load. + * nodes especially when under heavy conversations load. * * @author Gaston Dombiak */ public class ConversationEventsQueue { - private ConversationManager conversationManager; + private final ConversationManager conversationManager; /** * Chat events that are pending to be sent to the senior cluster member. * Key: Conversation Key; Value: List of conversation events. @@ -56,7 +56,7 @@ public ConversationEventsQueue(ConversationManager conversationManager, TaskEngi @Override public void run() { // Move queued events to a temp place - List eventsToSend = new ArrayList(); + List eventsToSend = new ArrayList<>(); synchronized (chatEvents) { for (List list : chatEvents.values()) { // Just send the first and last event if we are not archiving messages diff --git a/src/java/org/jivesoftware/openfire/archive/MonitoringConstants.java b/src/java/org/jivesoftware/openfire/archive/MonitoringConstants.java index 6944ca240..0bb89ec89 100644 --- a/src/java/org/jivesoftware/openfire/archive/MonitoringConstants.java +++ b/src/java/org/jivesoftware/openfire/archive/MonitoringConstants.java @@ -2,6 +2,13 @@ public class MonitoringConstants { + /** + * The canonical name of the plugin (matching its jar file). + */ public static final String NAME = "monitoring"; + /** + * The name as defined in the 'name' element of the plugin.xml file. + */ + public static final String PLUGIN_NAME = "Monitoring Service"; } diff --git a/src/java/org/jivesoftware/openfire/archive/cluster/GetConversationCountTask.java b/src/java/org/jivesoftware/openfire/archive/cluster/GetConversationCountTask.java index 61de7b62b..1fc2e3215 100644 --- a/src/java/org/jivesoftware/openfire/archive/cluster/GetConversationCountTask.java +++ b/src/java/org/jivesoftware/openfire/archive/cluster/GetConversationCountTask.java @@ -19,12 +19,17 @@ import org.jivesoftware.openfire.archive.ConversationManager; import org.jivesoftware.openfire.archive.MonitoringConstants; import org.jivesoftware.openfire.XMPPServer; +import org.jivesoftware.openfire.container.Plugin; import org.jivesoftware.openfire.plugin.MonitoringPlugin; import org.jivesoftware.util.cache.ClusterTask; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import javax.annotation.Nonnull; import java.io.IOException; import java.io.ObjectInput; import java.io.ObjectOutput; +import java.util.Optional; /** * Task that will return the number of current conversations taking place in the senior cluster member. @@ -32,25 +37,32 @@ * * @author Gaston Dombiak */ -public class GetConversationCountTask implements ClusterTask { +public class GetConversationCountTask implements ClusterTask +{ + private static final Logger Log = LoggerFactory.getLogger(GetConversationTask.class); + private int conversationCount; + @Nonnull public Integer getResult() { return conversationCount; } public void run() { - MonitoringPlugin plugin = (MonitoringPlugin) XMPPServer.getInstance().getPluginManager().getPlugin( - MonitoringConstants.NAME); - ConversationManager conversationManager = (ConversationManager)plugin.getModule(ConversationManager.class); + final Optional plugin = XMPPServer.getInstance().getPluginManager().getPluginByName(MonitoringConstants.PLUGIN_NAME); + if (!plugin.isPresent()) { + Log.error("Unable to execute cluster task! The Monitoring plugin does not appear to be loaded on this machine."); + return; + } + final ConversationManager conversationManager = (ConversationManager) ((MonitoringPlugin)plugin.get()).getModule(ConversationManager.class); conversationCount = conversationManager.getConversationCount(); } - public void writeExternal(ObjectOutput out) throws IOException { + public void writeExternal(ObjectOutput out) { // Do nothing } - public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException { + public void readExternal(ObjectInput in) { // Do nothing } } diff --git a/src/java/org/jivesoftware/openfire/archive/cluster/GetConversationTask.java b/src/java/org/jivesoftware/openfire/archive/cluster/GetConversationTask.java index dc4d9807e..9b5b1c450 100644 --- a/src/java/org/jivesoftware/openfire/archive/cluster/GetConversationTask.java +++ b/src/java/org/jivesoftware/openfire/archive/cluster/GetConversationTask.java @@ -20,28 +20,35 @@ import org.jivesoftware.openfire.archive.ConversationManager; import org.jivesoftware.openfire.archive.MonitoringConstants; import org.jivesoftware.openfire.XMPPServer; +import org.jivesoftware.openfire.container.Plugin; import org.jivesoftware.openfire.plugin.MonitoringPlugin; import org.jivesoftware.util.NotFoundException; import org.jivesoftware.util.cache.ClusterTask; import org.jivesoftware.util.cache.ExternalizableUtil; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.io.IOException; import java.io.ObjectInput; import java.io.ObjectOutput; +import java.util.Optional; /** * Task that returns the specified conversation or null if not found. * * @author Gaston Dombiak */ -public class GetConversationTask implements ClusterTask { +public class GetConversationTask implements ClusterTask +{ + private static final Logger Log = LoggerFactory.getLogger(GetConversationTask.class); + private long conversationID; private Conversation conversation; public GetConversationTask() { } - public GetConversationTask(long conversationID) { + public GetConversationTask(final long conversationID) { this.conversationID = conversationID; } @@ -50,9 +57,12 @@ public Conversation getResult() { } public void run() { - MonitoringPlugin plugin = (MonitoringPlugin) XMPPServer.getInstance().getPluginManager().getPlugin( - MonitoringConstants.NAME); - ConversationManager conversationManager = (ConversationManager)plugin.getModule(ConversationManager.class); + final Optional plugin = XMPPServer.getInstance().getPluginManager().getPluginByName(MonitoringConstants.PLUGIN_NAME); + if (!plugin.isPresent()) { + Log.error("Unable to execute cluster task! The Monitoring plugin does not appear to be loaded on this machine."); + return; + } + final ConversationManager conversationManager = (ConversationManager) ((MonitoringPlugin)plugin.get()).getModule(ConversationManager.class); try { conversation = conversationManager.getConversation(conversationID); } catch (NotFoundException e) { @@ -64,7 +74,7 @@ public void writeExternal(ObjectOutput out) throws IOException { ExternalizableUtil.getInstance().writeLong(out, conversationID); } - public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException { + public void readExternal(ObjectInput in) throws IOException { conversationID = ExternalizableUtil.getInstance().readLong(in); } } diff --git a/src/java/org/jivesoftware/openfire/archive/cluster/GetConversationsTask.java b/src/java/org/jivesoftware/openfire/archive/cluster/GetConversationsTask.java index f1b43df5c..e7676a40f 100644 --- a/src/java/org/jivesoftware/openfire/archive/cluster/GetConversationsTask.java +++ b/src/java/org/jivesoftware/openfire/archive/cluster/GetConversationsTask.java @@ -20,13 +20,17 @@ import org.jivesoftware.openfire.archive.ConversationManager; import org.jivesoftware.openfire.archive.MonitoringConstants; import org.jivesoftware.openfire.XMPPServer; +import org.jivesoftware.openfire.container.Plugin; import org.jivesoftware.openfire.plugin.MonitoringPlugin; import org.jivesoftware.util.cache.ClusterTask; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.io.IOException; import java.io.ObjectInput; import java.io.ObjectOutput; import java.util.Collection; +import java.util.Optional; /** * Task that will return current conversations taking place in the senior cluster member. @@ -34,7 +38,10 @@ * * @author Gaston Dombiak */ -public class GetConversationsTask implements ClusterTask> { +public class GetConversationsTask implements ClusterTask> +{ + private static final Logger Log = LoggerFactory.getLogger(GetConversationTask.class); + private Collection conversations; public Collection getResult() { @@ -42,17 +49,20 @@ public Collection getResult() { } public void run() { - MonitoringPlugin plugin = (MonitoringPlugin) XMPPServer.getInstance().getPluginManager().getPlugin( - MonitoringConstants.NAME); - ConversationManager conversationManager = (ConversationManager)plugin.getModule(ConversationManager.class); + final Optional plugin = XMPPServer.getInstance().getPluginManager().getPluginByName(MonitoringConstants.PLUGIN_NAME); + if (!plugin.isPresent()) { + Log.error("Unable to execute cluster task! The Monitoring plugin does not appear to be loaded on this machine."); + return; + } + final ConversationManager conversationManager = (ConversationManager) ((MonitoringPlugin)plugin.get()).getModule(ConversationManager.class); conversations = conversationManager.getConversations(); } - public void writeExternal(ObjectOutput out) throws IOException { + public void writeExternal(ObjectOutput out) { // Do nothing } - public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException { + public void readExternal(ObjectInput in) { // Do nothing } } diff --git a/src/java/org/jivesoftware/openfire/archive/cluster/GetConversationsWriteETATask.java b/src/java/org/jivesoftware/openfire/archive/cluster/GetConversationsWriteETATask.java index 2a837d7f5..9e30a8284 100644 --- a/src/java/org/jivesoftware/openfire/archive/cluster/GetConversationsWriteETATask.java +++ b/src/java/org/jivesoftware/openfire/archive/cluster/GetConversationsWriteETATask.java @@ -19,15 +19,20 @@ import org.jivesoftware.openfire.XMPPServer; import org.jivesoftware.openfire.archive.ConversationManager; import org.jivesoftware.openfire.archive.MonitoringConstants; +import org.jivesoftware.openfire.container.Plugin; import org.jivesoftware.openfire.plugin.MonitoringPlugin; import org.jivesoftware.util.cache.ClusterTask; import org.jivesoftware.util.cache.ExternalizableUtil; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import javax.annotation.Nonnull; import java.io.IOException; import java.io.ObjectInput; import java.io.ObjectOutput; import java.time.Duration; import java.time.Instant; +import java.util.Optional; /** * A task that retrieves a time estimation on the time it takes for conversations (and metadata) to have been written to persistent storage. @@ -36,12 +41,14 @@ */ public class GetConversationsWriteETATask implements ClusterTask { + private static final Logger Log = LoggerFactory.getLogger(GetConversationsWriteETATask.class); + private Instant instant; private Duration result; public GetConversationsWriteETATask() {} - public GetConversationsWriteETATask( Instant instant ) + public GetConversationsWriteETATask( @Nonnull final Instant instant ) { this.instant = instant; } @@ -49,8 +56,12 @@ public GetConversationsWriteETATask( Instant instant ) @Override public void run() { - final MonitoringPlugin plugin = (MonitoringPlugin) XMPPServer.getInstance().getPluginManager().getPlugin( MonitoringConstants.NAME ); - final ConversationManager conversationManager = (ConversationManager) plugin.getModule( ConversationManager.class ); + final Optional plugin = XMPPServer.getInstance().getPluginManager().getPluginByName(MonitoringConstants.PLUGIN_NAME); + if (!plugin.isPresent()) { + Log.error("Unable to execute cluster task! The Monitoring plugin does not appear to be loaded on this machine."); + return; + } + final ConversationManager conversationManager = (ConversationManager) ((MonitoringPlugin)plugin.get()).getModule(ConversationManager.class); result = conversationManager.availabilityETAOnLocalNode( instant ); } @@ -67,7 +78,7 @@ public void writeExternal( ObjectOutput out ) throws IOException } @Override - public void readExternal( ObjectInput in ) throws IOException, ClassNotFoundException + public void readExternal( ObjectInput in ) throws IOException { instant = (Instant) ExternalizableUtil.getInstance().readSerializable( in ); } diff --git a/src/java/org/jivesoftware/openfire/archive/cluster/SendConversationEventsTask.java b/src/java/org/jivesoftware/openfire/archive/cluster/SendConversationEventsTask.java index f5fc94480..4c73f360b 100644 --- a/src/java/org/jivesoftware/openfire/archive/cluster/SendConversationEventsTask.java +++ b/src/java/org/jivesoftware/openfire/archive/cluster/SendConversationEventsTask.java @@ -21,19 +21,23 @@ import java.io.ObjectOutput; import java.util.ArrayList; import java.util.List; +import java.util.Optional; import org.jivesoftware.openfire.XMPPServer; import org.jivesoftware.openfire.archive.ConversationEvent; import org.jivesoftware.openfire.archive.ConversationManager; import org.jivesoftware.openfire.archive.MonitoringConstants; +import org.jivesoftware.openfire.container.Plugin; import org.jivesoftware.openfire.plugin.MonitoringPlugin; import org.jivesoftware.util.cache.ClusterTask; import org.jivesoftware.util.cache.ExternalizableUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import javax.annotation.Nonnull; + /** - * Task that sends cnoversation events to the senior cluster member. + * Task that sends conversation events to the senior cluster member. * * @author Gaston Dombiak */ @@ -49,7 +53,7 @@ public class SendConversationEventsTask implements ClusterTask { public SendConversationEventsTask() { } - public SendConversationEventsTask(List events) { + public SendConversationEventsTask(@Nonnull final List events) { this.events = events; } @@ -58,10 +62,13 @@ public Void getResult() { } public void run() { - MonitoringPlugin plugin = (MonitoringPlugin) XMPPServer.getInstance().getPluginManager().getPlugin( - MonitoringConstants.NAME); - ConversationManager conversationManager = (ConversationManager)plugin.getModule(ConversationManager.class); - for (ConversationEvent event : events) { + final Optional plugin = XMPPServer.getInstance().getPluginManager().getPluginByName(MonitoringConstants.PLUGIN_NAME); + if (!plugin.isPresent()) { + Log.error("Unable to execute cluster task! The Monitoring plugin does not appear to be loaded on this machine."); + return; + } + final ConversationManager conversationManager = (ConversationManager) ((MonitoringPlugin)plugin.get()).getModule(ConversationManager.class); + for (final ConversationEvent event : events) { try { event.run(conversationManager); } catch (Exception e) { @@ -74,8 +81,8 @@ public void writeExternal(ObjectOutput out) throws IOException { ExternalizableUtil.getInstance().writeExternalizableCollection(out, events); } - public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException { - events = new ArrayList(); + public void readExternal(ObjectInput in) throws IOException { + events = new ArrayList<>(); ExternalizableUtil.getInstance().readExternalizableCollection(in, events, getClass().getClassLoader()); } } diff --git a/src/java/org/jivesoftware/openfire/archive/commands/GetGroupConversationTranscript.java b/src/java/org/jivesoftware/openfire/archive/commands/GetGroupConversationTranscript.java index 7c77a7202..36e8286cb 100644 --- a/src/java/org/jivesoftware/openfire/archive/commands/GetGroupConversationTranscript.java +++ b/src/java/org/jivesoftware/openfire/archive/commands/GetGroupConversationTranscript.java @@ -19,10 +19,7 @@ package org.jivesoftware.openfire.archive.commands; import java.io.ByteArrayOutputStream; -import java.util.Arrays; -import java.util.Collection; -import java.util.Date; -import java.util.List; +import java.util.*; import org.dom4j.Element; import org.jivesoftware.openfire.XMPPServer; @@ -35,6 +32,7 @@ import org.jivesoftware.openfire.commands.AdHocCommand; import org.jivesoftware.openfire.commands.SessionData; import org.jivesoftware.openfire.component.InternalComponentManager; +import org.jivesoftware.openfire.container.Plugin; import org.jivesoftware.openfire.plugin.MonitoringPlugin; import org.jivesoftware.util.StringUtils; import org.slf4j.Logger; @@ -98,10 +96,12 @@ protected void addStageInformation(SessionData data, Element command) { public void execute(SessionData data, Element command) { Element note = command.addElement("note"); // Get handle on the Monitoring plugin - MonitoringPlugin plugin = (MonitoringPlugin) XMPPServer.getInstance().getPluginManager() - .getPlugin(MonitoringConstants.NAME); - ConversationManager conversationManager = - (ConversationManager) plugin.getModule(ConversationManager.class); + final Optional plugin = XMPPServer.getInstance().getPluginManager().getPluginByName(MonitoringConstants.PLUGIN_NAME); + if (!plugin.isPresent()) { + Log.error("Unable to execute command! The Monitoring plugin does not appear to be loaded on this machine."); + return; + } + final ConversationManager conversationManager = (ConversationManager) ((MonitoringPlugin)plugin.get()).getModule(ConversationManager.class); if (!conversationManager.isArchivingEnabled()) { note.addAttribute("type", "error"); note.setText("Message archiving is not enabled."); @@ -130,7 +130,7 @@ public void execute(SessionData data, Element command) { boolean includePDF = DataForm.parseBoolean(data.getData().get("includePDF").get(0)); // Get archive searcher module - ArchiveSearcher archiveSearcher = (ArchiveSearcher) plugin.getModule(ArchiveSearcher.class); + final ArchiveSearcher archiveSearcher = (ArchiveSearcher) ((MonitoringPlugin)plugin.get()).getModule(ArchiveSearcher.class); ArchiveSearch search = new ArchiveSearch(); search.setParticipants(participant); diff --git a/src/java/org/jivesoftware/openfire/reporting/stats/GetStatistics.java b/src/java/org/jivesoftware/openfire/reporting/stats/GetStatistics.java index 1eb818ad0..c911cf339 100644 --- a/src/java/org/jivesoftware/openfire/reporting/stats/GetStatistics.java +++ b/src/java/org/jivesoftware/openfire/reporting/stats/GetStatistics.java @@ -46,7 +46,7 @@ public Map getResult() { } public void run() { - samples = new HashMap(); + samples = new HashMap<>(); for (Map.Entry statisticEntry : StatisticsManager.getInstance().getAllStatistics()) { String key = statisticEntry.getKey(); Statistic statistic = statisticEntry.getValue(); @@ -59,11 +59,11 @@ public void run() { } } - public void writeExternal(ObjectOutput out) throws IOException { + public void writeExternal(ObjectOutput out) { // Ignore } - public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException { + public void readExternal(ObjectInput in) { // Ignore } From b6742478460f9cfbb0d736eb6c6b67f4a7f9572b Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Tue, 1 Dec 2020 14:52:12 +0100 Subject: [PATCH 02/19] fixes #137: include stanza in cluster task As the senior node is responsible for processing cluster events related, that node needs to have all available data to operate on. By making available the original stanza, this stanza can be persisted in the database. This in turn prevents an inpresice reproduction from being used when the corresponding message archive is used at some point later in the future. --- changelog.html | 5 +++++ pom.xml | 2 +- .../jivesoftware/openfire/archive/ConversationEvent.java | 8 +++++++- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/changelog.html b/changelog.html index 33c20e1cc..c0014e157 100644 --- a/changelog.html +++ b/changelog.html @@ -44,6 +44,11 @@

Monitoring Plugin Changelog

+

2.2.0 -- (tbd)

+
    +
  • [Issue #137] - MUC messages duplicated as one-on-one messages
    • +
+

2.1.0 -- September 10, 2020

  • [Issue #72] - Return chat message bodies in response to xep-0136 requests.
    • diff --git a/pom.xml b/pom.xml index 8cdd1da6e..d683212b9 100644 --- a/pom.xml +++ b/pom.xml @@ -8,7 +8,7 @@ org.igniterealtime.openfire.plugins monitoring - 2.1.1-SNAPSHOT + 2.2.0-SNAPSHOT Monitoring Plugin Monitors conversations and statistics of the server. diff --git a/src/java/org/jivesoftware/openfire/archive/ConversationEvent.java b/src/java/org/jivesoftware/openfire/archive/ConversationEvent.java index 463923e59..167eeccc2 100644 --- a/src/java/org/jivesoftware/openfire/archive/ConversationEvent.java +++ b/src/java/org/jivesoftware/openfire/archive/ConversationEvent.java @@ -95,7 +95,10 @@ public void writeExternal(ObjectOutput out) throws IOException { if (body != null) { ExternalizableUtil.getInstance().writeSafeUTF(out, body); } - + ExternalizableUtil.getInstance().writeBoolean(out, stanza != null); + if (stanza != null) { + ExternalizableUtil.getInstance().writeSafeUTF(out, stanza); + } ExternalizableUtil.getInstance().writeBoolean(out, roomJID != null); if (roomJID != null) { ExternalizableUtil.getInstance().writeSerializable(out, roomJID); @@ -123,6 +126,9 @@ public void readExternal(ObjectInput in) throws IOException, ClassNotFoundExcept if (ExternalizableUtil.getInstance().readBoolean(in)) { body = ExternalizableUtil.getInstance().readSafeUTF(in); } + if (ExternalizableUtil.getInstance().readBoolean(in)) { + stanza = ExternalizableUtil.getInstance().readSafeUTF(in); + } if (ExternalizableUtil.getInstance().readBoolean(in)) { roomJID = (JID) ExternalizableUtil.getInstance().readSerializable(in); From 05bb1f5d9b266c03125b51294698bfb94c66f1e7 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Tue, 1 Dec 2020 16:18:06 +0100 Subject: [PATCH 03/19] fixes #139: refactoring: reduce complexity by making ArchivedMessage (almost) immutable No functional changes intended. --- .../plugin/archive/ArchiveFactory.java | 7 +-- .../archive/impl/JdbcPersistenceManager.java | 5 +- .../impl/MucMamPersistenceManager.java | 4 +- .../plugin/archive/model/ArchivedMessage.java | 48 ++++--------------- 4 files changed, 16 insertions(+), 48 deletions(-) diff --git a/src/java/com/reucon/openfire/plugin/archive/ArchiveFactory.java b/src/java/com/reucon/openfire/plugin/archive/ArchiveFactory.java index 89b9c31ed..d3f8ad713 100644 --- a/src/java/com/reucon/openfire/plugin/archive/ArchiveFactory.java +++ b/src/java/com/reucon/openfire/plugin/archive/ArchiveFactory.java @@ -22,9 +22,10 @@ public static ArchivedMessage createArchivedMessage(Session session, final String sid = StanzaIDUtil.findFirstUniqueAndStableStanzaID( message, owner.toBareJID() ); final ArchivedMessage archivedMessage; - archivedMessage = new ArchivedMessage(new Date(), direction, message.getType().toString(), with, sid); - archivedMessage.setSubject(message.getSubject()); - archivedMessage.setBody(message.getBody()); + // Use a 'null' value for the numeric database ID, as the resulting object has not yet been stored in the database. + final Long id = null; + + archivedMessage = new ArchivedMessage(id, new Date(), direction, message.getType().toString(), with, sid, message.getBody(), message.toXML()); return archivedMessage; } diff --git a/src/java/com/reucon/openfire/plugin/archive/impl/JdbcPersistenceManager.java b/src/java/com/reucon/openfire/plugin/archive/impl/JdbcPersistenceManager.java index 6c4936760..c556fb2f2 100644 --- a/src/java/com/reucon/openfire/plugin/archive/impl/JdbcPersistenceManager.java +++ b/src/java/com/reucon/openfire/plugin/archive/impl/JdbcPersistenceManager.java @@ -880,10 +880,7 @@ static protected ArchivedMessage asArchivedMessage(JID owner, String fromJID, St direction = Direction.to; with = to; } - final ArchivedMessage archivedMessage = new ArchivedMessage(sentDate, direction, type == null ? null : type.toString(), with, sid); - archivedMessage.setStanza(stanza); - archivedMessage.setBody(body); - archivedMessage.setId(id); + final ArchivedMessage archivedMessage = new ArchivedMessage(id, sentDate, direction, type == null ? null : type.toString(), with, sid, body, stanza); return archivedMessage; } diff --git a/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java b/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java index 071c2c5a3..427704e04 100644 --- a/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java +++ b/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java @@ -297,9 +297,7 @@ static protected ArchivedMessage asArchivedMessage(JID roomJID, String senderJID sid = null; } - final ArchivedMessage archivedMessage = new ArchivedMessage(sentDate, ArchivedMessage.Direction.from, null, null, sid); - archivedMessage.setStanza(stanza); - archivedMessage.setId(id); + final ArchivedMessage archivedMessage = new ArchivedMessage(id, sentDate, ArchivedMessage.Direction.from, null, null, sid, body, stanza); return archivedMessage; } diff --git a/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java b/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java index 3eddc647d..a13513aa0 100644 --- a/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java +++ b/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java @@ -23,33 +23,31 @@ public enum Direction { from } - private Long id; + private final Long id; private final Date time; private final Direction direction; private final String type; - private String subject; - private String body; + private final String body; private Conversation conversation; - private JID with; - private String stanza; - private String stableId; + private final JID with; + private final String stanza; + private final String stableId; - public ArchivedMessage( Date time, Direction direction, String type, JID with, String stableId ) { + public ArchivedMessage( Long id, Date time, Direction direction, String type, JID with, String stableId, String body, String stanza) { + this.id = id; this.time = time; this.direction = direction; this.type = type; this.with = with; this.stableId = stableId; + this.body = body; + this.stanza = stanza; } public Long getId() { return id; } - public void setId(Long id) { - this.id = id; - } - public Date getTime() { return time; } @@ -62,30 +60,14 @@ public String getType() { return type; } - public String getSubject() { - return subject; - } - - public void setSubject(String subject) { - this.subject = subject; - } - public String getBody() { return body; } - public void setBody(String body) { - this.body = body; - } - public String getStanza() { return stanza; } - public void setStanza(String stanza) { - this.stanza = stanza; - } - public Conversation getConversation() { return conversation; } @@ -101,7 +83,7 @@ public void setConversation(Conversation conversation) { * otherwise. */ public boolean isEmpty() { - return subject == null && body == null; + return body == null; } public JID getWith() { @@ -113,16 +95,6 @@ public String getStableId() return stableId; } - public void setStableId( final UUID stableId ) - { - this.stableId = stableId.toString(); - } - - public void setStableId( final String stableId ) - { - this.stableId = stableId; - } - public String toString() { StringBuilder sb = new StringBuilder(); From f30b0fd0c9836e9bc42f783cd16b806d8fbb6cb6 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Tue, 1 Dec 2020 16:35:35 +0100 Subject: [PATCH 04/19] fixes #139: Refactor: remove unused ArchiveManager Reduce code complexity by removing code that is not used. --- .../plugin/archive/ArchiveManager.java | 29 --- .../archive/impl/ArchiveManagerImpl.java | 172 ------------------ .../openfire/plugin/MonitoringPlugin.java | 9 - 3 files changed, 210 deletions(-) delete mode 100644 src/java/com/reucon/openfire/plugin/archive/ArchiveManager.java delete mode 100644 src/java/com/reucon/openfire/plugin/archive/impl/ArchiveManagerImpl.java diff --git a/src/java/com/reucon/openfire/plugin/archive/ArchiveManager.java b/src/java/com/reucon/openfire/plugin/archive/ArchiveManager.java deleted file mode 100644 index f4cef600c..000000000 --- a/src/java/com/reucon/openfire/plugin/archive/ArchiveManager.java +++ /dev/null @@ -1,29 +0,0 @@ -package com.reucon.openfire.plugin.archive; - -import org.jivesoftware.openfire.session.Session; -import org.xmpp.packet.Message; - -/** - * Adds messages to the archive. - */ -public interface ArchiveManager -{ - /** - * Adds a message to the archive. - * - * @param session the session the message was received through. - * @param message the message to archive. - * @param incoming true if this a message received by the server, false if it - * is sent by the server. - */ - void archiveMessage(Session session, Message message, boolean incoming); - - /** - * Sets the conversation timeout.

    - * A new conversation is created if there no messages have been exchanged between two JIDs - * for the given timeout. - * - * @param conversationTimeout the conversation timeout to set in minutes. - */ - void setConversationTimeout(int conversationTimeout); -} diff --git a/src/java/com/reucon/openfire/plugin/archive/impl/ArchiveManagerImpl.java b/src/java/com/reucon/openfire/plugin/archive/impl/ArchiveManagerImpl.java deleted file mode 100644 index 3d971aa06..000000000 --- a/src/java/com/reucon/openfire/plugin/archive/impl/ArchiveManagerImpl.java +++ /dev/null @@ -1,172 +0,0 @@ -package com.reucon.openfire.plugin.archive.impl; - -import com.reucon.openfire.plugin.archive.ArchiveFactory; -import com.reucon.openfire.plugin.archive.ArchiveManager; -import com.reucon.openfire.plugin.archive.IndexManager; -import com.reucon.openfire.plugin.archive.PersistenceManager; -import com.reucon.openfire.plugin.archive.model.ArchivedMessage; -import com.reucon.openfire.plugin.archive.model.Conversation; -import com.reucon.openfire.plugin.archive.model.Participant; -import org.jivesoftware.openfire.XMPPServer; -import org.jivesoftware.openfire.session.Session; -import org.xmpp.packet.JID; -import org.xmpp.packet.Message; - -import java.util.ArrayList; -import java.util.Collection; - -/** - * Default implementation of ArchiveManager. - */ -public class ArchiveManagerImpl implements ArchiveManager -{ - private final PersistenceManager persistenceManager; - private final IndexManager indexManager; - private final Collection activeConversations; - private int conversationTimeout; - - public ArchiveManagerImpl(PersistenceManager persistenceManager, IndexManager indexManager, int conversationTimeout) - { - this.persistenceManager = persistenceManager; - this.indexManager = indexManager; - this.conversationTimeout = conversationTimeout; - - activeConversations = persistenceManager.getActiveConversations(conversationTimeout); - } - - public void archiveMessage(Session session, Message message, boolean incoming) - { - final XMPPServer server = XMPPServer.getInstance(); - final ArchivedMessage.Direction direction; - final ArchivedMessage archivedMessage; - final Conversation conversation; - final JID owner; - final JID with; - - // TODO support groupchat - if (message.getType() != Message.Type.chat && message.getType() != Message.Type.normal) - { - return; - } - - if (server.isLocal(message.getFrom()) && incoming) - { - owner = message.getFrom(); - with = message.getTo(); - // sent by the owner => to - direction = ArchivedMessage.Direction.to; - } - else if (server.isLocal(message.getTo()) && ! incoming) - { - owner = message.getTo(); - with = message.getFrom(); - // received by the owner => from - direction = ArchivedMessage.Direction.from; - } - else - { - return; - } - - archivedMessage = ArchiveFactory.createArchivedMessage(session, message, direction, owner, with); - if (archivedMessage.isEmpty()) - { - return; - } - - conversation = determineConversation(owner, with, message.getSubject(), message.getThread(), archivedMessage); - archivedMessage.setConversation(conversation); - - persistenceManager.createMessage(archivedMessage); - if (indexManager != null) - { - indexManager.indexObject(archivedMessage); - } - } - - public void setConversationTimeout(int conversationTimeout) - { - this.conversationTimeout = conversationTimeout; - } - - private Conversation determineConversation(JID owner, JID with, String subject, String thread, ArchivedMessage archivedMessage) - { - Conversation conversation = null; - Collection staleConversations; - - staleConversations = new ArrayList(); - synchronized (activeConversations) - { - for (Conversation c : activeConversations) - { - if (c.isStale(conversationTimeout)) - { - staleConversations.add(c); - continue; - } - - if (matches(owner, with, thread, c)) - { - conversation = c; - break; - } - } - - activeConversations.removeAll(staleConversations); - - if (conversation == null) - { - final Participant p1; - final Participant p2; - - conversation = new Conversation(archivedMessage.getTime(), owner, with, subject, thread); - persistenceManager.createConversation(conversation); - - p1 = new Participant(archivedMessage.getTime(), owner.asBareJID()); - conversation.addParticipant(p1); - persistenceManager.createParticipant(p1, conversation.getId()); - - p2 = new Participant(archivedMessage.getTime(), with.asBareJID()); - conversation.addParticipant(p2); - persistenceManager.createParticipant(p2, conversation.getId()); - activeConversations.add(conversation); - } - else - { - conversation.setEnd(archivedMessage.getTime()); - persistenceManager.updateConversationEnd(conversation); - } - } - - return conversation; - } - - private boolean matches(JID owner, JID with, String thread, Conversation c) - { - if (! owner.asBareJID().equals(c.getOwnerBareJid())) - { - return false; - } - if (! with.asBareJID().equals(c.getWithBareJid())) - { - return false; - } - - if (thread != null) - { - if (! thread.equals(c.getThread())) - { - return false; - } - } - else - { - if (c.getThread() != null) - { - return false; - } - } - - return true; - } -} diff --git a/src/java/org/jivesoftware/openfire/plugin/MonitoringPlugin.java b/src/java/org/jivesoftware/openfire/plugin/MonitoringPlugin.java index efc403d04..82f463439 100644 --- a/src/java/org/jivesoftware/openfire/plugin/MonitoringPlugin.java +++ b/src/java/org/jivesoftware/openfire/plugin/MonitoringPlugin.java @@ -53,7 +53,6 @@ import org.picocontainer.MutablePicoContainer; import org.picocontainer.defaults.DefaultPicoContainer; -import com.reucon.openfire.plugin.archive.ArchiveManager; import com.reucon.openfire.plugin.archive.ArchiveProperties; import com.reucon.openfire.plugin.archive.IndexManager; import com.reucon.openfire.plugin.archive.PersistenceManager; @@ -93,7 +92,6 @@ public class MonitoringPlugin implements Plugin { private boolean enabled = true; private PersistenceManager persistenceManager; private PersistenceManager mucPersistenceManager; - private ArchiveManager archiveManager; private IndexManager indexManager; private Xep0136Support xep0136Support; private Xep0313Support xep0313Support; @@ -146,10 +144,6 @@ public boolean isEnabled() { return this.enabled; } - public ArchiveManager getArchiveManager() { - return archiveManager; - } - public IndexManager getIndexManager() { return indexManager; } @@ -190,9 +184,6 @@ public void initializePlugin(PluginManager manager, File pluginDirectory) { persistenceManager = new JdbcPersistenceManager(); mucPersistenceManager = new MucMamPersistenceManager(); - archiveManager = new ArchiveManagerImpl(persistenceManager, - indexManager, conversationTimeout); - xep0136Support = new Xep0136Support(XMPPServer.getInstance()); xep0136Support.start(); From deb828388ed1b69190f34314d629187acc5d8dc6 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Tue, 1 Dec 2020 16:55:27 +0100 Subject: [PATCH 05/19] fixes #139: Refactor: remove unused code from PersistenceManager PersistenceManager had a lot of definition that wasn't used. It either had no implementation, or the little implementation that was there, went unreferenced. This commit removes it all, reducing code complexity by a significant factor. It appears to me that all removed code was replaced by code in the implementation of ConversationManager - although I'm not 100% sure. --- .../plugin/archive/PersistenceManager.java | 59 +--- .../archive/impl/JdbcPersistenceManager.java | 271 ++---------------- .../impl/MucMamPersistenceManager.java | 54 +--- 3 files changed, 33 insertions(+), 351 deletions(-) diff --git a/src/java/com/reucon/openfire/plugin/archive/PersistenceManager.java b/src/java/com/reucon/openfire/plugin/archive/PersistenceManager.java index a813ada78..0a8857946 100644 --- a/src/java/com/reucon/openfire/plugin/archive/PersistenceManager.java +++ b/src/java/com/reucon/openfire/plugin/archive/PersistenceManager.java @@ -2,63 +2,18 @@ import com.reucon.openfire.plugin.archive.model.ArchivedMessage; import com.reucon.openfire.plugin.archive.model.Conversation; -import com.reucon.openfire.plugin.archive.model.Participant; import com.reucon.openfire.plugin.archive.xep0059.XmppResultSet; import org.jivesoftware.util.NotFoundException; import org.xmpp.packet.JID; import java.util.Collection; import java.util.Date; -import java.util.List; /** * Manages database persistence. */ public interface PersistenceManager { - /** - * Creates a new archived message. - * - * @param message the message to create. - * @return true on success, false otherwise. - */ - boolean createMessage(ArchivedMessage message); - - /** - * Selects all messages and passes each message to the given callback for processing. - * - * @param callback callback to process messages. - * @return number of messages processed. - */ - int processAllMessages(ArchivedMessageConsumer callback); - - /** - * Creates a new conversation. - * - * @param conversation the conversation to create. - * @return true on success, false otherwise. - */ - boolean createConversation(Conversation conversation); - - /** - * Updates the end time of a conversation. The conversation must be persisted. - * - * @param conversation conversation to update with id and endDate attributes not null. - * @return true on success, false otherwise. - */ - boolean updateConversationEnd(Conversation conversation); - - /** - * Adds a new participant to a conversation. - * - * @param participant the participant to add. - * @param conversationId id of the conversation to add the participant to. - * @return true on success, false otherwise. - */ - boolean createParticipant(Participant participant, Long conversationId); - - List findConversations(String[] participants, Date startDate, Date endDate); - /** * Searches for conversations. * @@ -85,10 +40,6 @@ public interface PersistenceManager */ Collection findMessages( Date startDate, Date endDate, JID owner, JID with, String query, XmppResultSet xmppResultSet, boolean useStableID) throws NotFoundException; - Collection getActiveConversations(int conversationTimeout); - - List getConversations(Collection conversationIds); - /** * Returns the conversation with the given owner, with and start time including participants and messages. * @@ -97,13 +48,5 @@ public interface PersistenceManager * @param start exact start time * @return the matching conversation or null if none matches. */ - Conversation getConversation(JID owner, JID with, Date start); - - /** - * Returns the conversation with the given id including participants and messages. - * - * @param conversationId id of the conversation to retrieve. - * @return the matching conversation or null if none matches. - */ - Conversation getConversation(Long conversationId); + Conversation getConversation(JID owner, JID with, Date start); // TODO move to ConversationManager? } diff --git a/src/java/com/reucon/openfire/plugin/archive/impl/JdbcPersistenceManager.java b/src/java/com/reucon/openfire/plugin/archive/impl/JdbcPersistenceManager.java index c556fb2f2..c6e0488d0 100644 --- a/src/java/com/reucon/openfire/plugin/archive/impl/JdbcPersistenceManager.java +++ b/src/java/com/reucon/openfire/plugin/archive/impl/JdbcPersistenceManager.java @@ -1,37 +1,31 @@ package com.reucon.openfire.plugin.archive.impl; -import java.sql.*; -import java.time.Instant; -import java.time.temporal.ChronoUnit; -import java.util.*; -import java.util.Date; - +import com.reucon.openfire.plugin.archive.PersistenceManager; +import com.reucon.openfire.plugin.archive.model.ArchivedMessage; +import com.reucon.openfire.plugin.archive.model.ArchivedMessage.Direction; +import com.reucon.openfire.plugin.archive.model.Conversation; +import com.reucon.openfire.plugin.archive.model.Participant; +import com.reucon.openfire.plugin.archive.util.StanzaIDUtil; +import com.reucon.openfire.plugin.archive.xep0059.XmppResultSet; import org.dom4j.Document; import org.dom4j.DocumentFactory; import org.dom4j.DocumentHelper; import org.dom4j.Element; import org.jivesoftware.database.DbConnectionManager; import org.jivesoftware.openfire.archive.ConversationManager; -import org.jivesoftware.openfire.cluster.ClusterManager; -import org.jivesoftware.openfire.muc.MUCRoom; -import org.jivesoftware.openfire.reporting.util.TaskEngine; -import com.reucon.openfire.plugin.archive.util.StanzaIDUtil; import org.jivesoftware.util.JiveConstants; import org.jivesoftware.util.JiveGlobals; -import org.jivesoftware.util.NotFoundException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.xmpp.packet.JID; - -import com.reucon.openfire.plugin.archive.ArchivedMessageConsumer; -import com.reucon.openfire.plugin.archive.PersistenceManager; -import com.reucon.openfire.plugin.archive.model.ArchivedMessage; -import com.reucon.openfire.plugin.archive.model.ArchivedMessage.Direction; -import com.reucon.openfire.plugin.archive.model.Conversation; -import com.reucon.openfire.plugin.archive.model.Participant; -import com.reucon.openfire.plugin.archive.xep0059.XmppResultSet; import org.xmpp.packet.Message; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.*; + /** * Manages database persistence. */ @@ -76,102 +70,12 @@ public class JdbcPersistenceManager implements PersistenceManager { public static final String CONVERSATION_WITH_JID = "ofMessageArchive.toJID"; - public static final String SELECT_ACTIVE_CONVERSATIONS = "SELECT DISTINCT ofConversation.conversationID, ofConversation.room, " - + "ofConversation.isExternal, ofConversation.startDate, ofConversation.lastActivity, ofConversation.messageCount, " - + "ofConParticipant.joinedDate, ofConParticipant.leftDate, ofConParticipant.bareJID, ofConParticipant.jidResource, " - + "ofConParticipant.nickname, ofMessageArchive.fromJID, ofMessageArchive.fromJIDResource, ofMessageArchive.toJID, " - + "ofMessageArchive.toJIDResource, ofMessageArchive.sentDate, " - + "ofMessageArchive.body FROM ofConversation " - + "INNER JOIN ofConParticipant ON ofConversation.conversationID = ofConParticipant.conversationID " - + "INNER JOIN ofMessageArchive ON ofConParticipant.conversationID = ofMessageArchive.conversationID " - + "WHERE ofConversation.lastActivity > ?"; - - public static final String SELECT_ACTIVE_CONVERSATIONS_ORACLE = "select SUBSET.conversationID," - + "SUBSET.room," - + "SUBSET.isExternal," - + "SUBSET.startDate," - + "SUBSET.lastActivity," - + "SUBSET.messageCount," - + "SUBSET.joinedDate," - + "SUBSET.leftDate," - + "SUBSET.bareJID," - + "SUBSET.jidResource," - + "SUBSET.nickname," - + "SUBSET.fromJID," - + "SUBSET.fromJIDResource," - + "SUBSET.toJID," - + "SUBSET.toJIDResource," - + "SUBSET.sentDate," - + "MAR.body from (" - + "SELECT DISTINCT ofConversation.conversationID as conversationID," - + "ofConversation.room as room," - + "ofConversation.isExternal as isExternal," - + "ofConversation.startDate as startDate," - + "ofConversation.lastActivity as lastActivity," - + "ofConversation.messageCount as messageCount," - + "ofConParticipant.joinedDate as joinedDate," - + "ofConParticipant.leftDate as leftDate," - + "ofConParticipant.bareJID as bareJID," - + "ofConParticipant.jidResource as jidResource," - + "ofConParticipant.nickname as nickname," - + "ofMessageArchive.fromJID as fromJID," - + "ofMessageArchive.fromJIDResource as fromJIDResource," - + "ofMessageArchive.toJID as toJID," - + "ofMessageArchive.toJIDResource as toJIDResource," - + "ofMessageArchive.sentDate as sentDate," - + "ofMessageArchive.MESSAGEID as msgId " - + "FROM ofConversation " - + "INNER JOIN ofConParticipant ON ofConversation.conversationID = ofConParticipant.conversationID " - + "INNER JOIN ofMessageArchive ON ofConParticipant.conversationID = ofMessageArchive.conversationID " - + "where ofConversation.lastActivity > ? ) SUBSET " - + "INNER JOIN ofMessageArchive MAR ON MAR.conversationID = SUBSET.conversationID " - + "where MAR.MESSAGEID = SUBSET.msgId " - + "and MAR.sentDate = SUBSET.sentDate " - + "and MAR.fromJID = SUBSET.fromJID " - + "and MAR.fromJIDResource = SUBSET.fromJIDResource " - + "and MAR.toJID = SUBSET.toJID " - + "and MAR.toJIDResource = SUBSET.toJIDResource"; - public static final String SELECT_PARTICIPANTS_BY_CONVERSATION = "SELECT DISTINCT ofConversation.conversationID, " + "ofConversation.startDate, ofConversation.lastActivity, ofConParticipant.bareJID FROM ofConversation " + "INNER JOIN ofConParticipant ON ofConversation.conversationID = ofConParticipant.conversationID " + "INNER JOIN ofMessageArchive ON ofConParticipant.conversationID = ofMessageArchive.conversationID " + "WHERE ofConversation.conversationID = ? ORDER BY ofConversation.startDate"; - @Override - public boolean createMessage(ArchivedMessage message) { - /* read only */ - return false; - } - - @Override - public int processAllMessages(ArchivedMessageConsumer callback) { - return 0; - } - - @Override - public boolean createConversation(Conversation conversation) { - /* read only */ - return false; - } - - @Override - public boolean updateConversationEnd(Conversation conversation) { - /* read only */ - return false; - } - - @Override - public boolean createParticipant(Participant participant, Long conversationId) { - return false; - } - - @Override - public List findConversations(String[] participants, Date startDate, Date endDate) { - final List conversations = new ArrayList(); - return conversations; - } - public Date getAuditedStartDate(Date startDate) { long maxRetrievable = JiveGlobals.getIntProperty("conversation.maxRetrievable", ConversationManager.DEFAULT_MAX_RETRIEVABLE) * JiveConstants.DAY; @@ -498,99 +402,8 @@ public Collection findMessages(Date startDate, Date endDate, JI return msgs; } - private boolean isOracleDB() - { - return DbConnectionManager.getDatabaseType() == DbConnectionManager.DatabaseType.oracle; - } - - @Override - public Collection getActiveConversations(int conversationTimeout) { - final Collection conversations; - final long now = System.currentTimeMillis(); - - conversations = new ArrayList<>(); - - Connection con = null; - PreparedStatement pstmt = null; - ResultSet rs = null; - try { - con = DbConnectionManager.getConnection(); - pstmt = con.prepareStatement( isOracleDB() ? SELECT_ACTIVE_CONVERSATIONS_ORACLE : SELECT_ACTIVE_CONVERSATIONS ); - - pstmt.setLong(1, now - conversationTimeout * 60L * 1000L); - rs = pstmt.executeQuery(); - while (rs.next()) { - conversations.add(extractConversation(rs)); - } - } catch (SQLException sqle) { - Log.error("Error selecting conversations", sqle); - } finally { - DbConnectionManager.closeConnection(rs, pstmt, con); - } - - return conversations; - } - - @Override - public List getConversations(Collection conversationIds) { - final List conversations; - final StringBuilder querySB; - - conversations = new ArrayList<>(); - if (conversationIds.isEmpty()) { - return conversations; - } - - querySB = new StringBuilder(SELECT_CONVERSATIONS); - querySB.append(" WHERE "); - querySB.append(CONVERSATION_ID); - querySB.append(" IN ( "); - for (int i = 0; i < conversationIds.size(); i++) { - if (i == 0) { - querySB.append("?"); - } else { - querySB.append(",?"); - } - } - querySB.append(" )"); - querySB.append(SELECT_CONVERSATIONS_GROUP_BY); - querySB.append(" ORDER BY ").append(CONVERSATION_END_TIME); - - Connection con = null; - PreparedStatement pstmt = null; - ResultSet rs = null; - try { - con = DbConnectionManager.getConnection(); - pstmt = con.prepareStatement(querySB.toString()); - - int i = 0; - for (Long id : conversationIds) { - pstmt.setLong(++i, id); - } - rs = pstmt.executeQuery(); - while (rs.next()) { - conversations.add(extractConversation(rs)); - } - } catch (SQLException sqle) { - Log.error("Error selecting conversations", sqle); - } finally { - DbConnectionManager.closeConnection(rs, pstmt, con); - } - - return conversations; - } - @Override public Conversation getConversation(JID owner, JID with, Date start) { - return getConversation(null, owner, with, start); - } - - @Override - public Conversation getConversation(Long conversationId) { - return getConversation(conversationId, null, null, null); - } - - private Conversation getConversation(Long conversationId, JID owner, JID with, Date start) { Conversation conversation = null; StringBuilder querySB; @@ -600,18 +413,14 @@ private Conversation getConversation(Long conversationId, JID owner, JID with, D querySB = new StringBuilder(SELECT_CONVERSATIONS); querySB.append(" WHERE "); - if (conversationId != null) { - querySB.append(CONVERSATION_ID).append(" = ? "); - } else { - querySB.append(CONVERSATION_OWNER_JID).append(" = ?"); - if (with != null) { - querySB.append(" AND "); - querySB.append(CONVERSATION_WITH_JID).append(" = ? "); - } - if (start != null) { - querySB.append(" AND "); - querySB.append(CONVERSATION_START_TIME).append(" = ? "); - } + querySB.append(CONVERSATION_OWNER_JID).append(" = ?"); + if (with != null) { + querySB.append(" AND "); + querySB.append(CONVERSATION_WITH_JID).append(" = ? "); + } + if (start != null) { + querySB.append(" AND "); + querySB.append(CONVERSATION_START_TIME).append(" = ? "); } querySB.append(SELECT_CONVERSATIONS_GROUP_BY); @@ -619,17 +428,12 @@ private Conversation getConversation(Long conversationId, JID owner, JID with, D con = DbConnectionManager.getConnection(); pstmt = con.prepareStatement(querySB.toString()); int i = 1; - - if (conversationId != null) { - pstmt.setLong(1, conversationId); - } else { - pstmt.setString(i++, owner.toString()); - if (with != null) { - pstmt.setString(i++, with.toString()); - } - if (start != null) { - pstmt.setLong(i++, dateToMillis(start)); - } + pstmt.setString(i++, owner.toString()); + if (with != null) { + pstmt.setString(i++, with.toString()); + } + if (start != null) { + pstmt.setLong(i++, dateToMillis(start)); } rs = pstmt.executeQuery(); Log.debug("getConversation: SELECT_CONVERSATIONS: " + pstmt.toString()); @@ -700,27 +504,6 @@ private JID getWithJidConversations(ResultSet rs) throws SQLException { return result == null ? null : new JID(result); } - private static Direction getDirection(ResultSet rs) throws SQLException { - Direction direction = null; - String bareJid = rs.getString("bareJID"); - String fromJid = rs.getString("fromJID"); - String toJid = rs.getString("toJID"); - if (bareJid != null && fromJid != null && toJid != null) { - if (fromJid.contains(bareJid)) { // older versions of the database put the full jid in 'fromJID'. Using 'contains' (instead of 'equals') will also match those. - /* - * if message from me to 'with' then it is to the 'with' participant - */ - direction = Direction.to; - } else { - /* - * if message to me from 'with' then it is from the 'with' participant - */ - direction = Direction.from; - } - } - return direction; - } - private Conversation extractConversation(ResultSet rs) throws SQLException { final Conversation conversation; diff --git a/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java b/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java index 427704e04..e15b64cb0 100644 --- a/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java +++ b/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java @@ -1,10 +1,9 @@ package com.reucon.openfire.plugin.archive.impl; -import com.reucon.openfire.plugin.archive.ArchivedMessageConsumer; import com.reucon.openfire.plugin.archive.PersistenceManager; import com.reucon.openfire.plugin.archive.model.ArchivedMessage; import com.reucon.openfire.plugin.archive.model.Conversation; -import com.reucon.openfire.plugin.archive.model.Participant; +import com.reucon.openfire.plugin.archive.util.StanzaIDUtil; import com.reucon.openfire.plugin.archive.xep0059.XmppResultSet; import org.dom4j.*; import org.jivesoftware.database.DbConnectionManager; @@ -12,7 +11,6 @@ import org.jivesoftware.openfire.muc.MUCRoom; import org.jivesoftware.openfire.muc.MultiUserChatManager; import org.jivesoftware.openfire.muc.MultiUserChatService; -import com.reucon.openfire.plugin.archive.util.StanzaIDUtil; import org.jivesoftware.util.JiveGlobals; import org.jivesoftware.util.NotFoundException; import org.slf4j.Logger; @@ -25,7 +23,10 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.time.Instant; -import java.util.*; +import java.util.Collection; +import java.util.Collections; +import java.util.Date; +import java.util.List; /** * Created by dwd on 25/07/16. @@ -35,36 +36,6 @@ public class MucMamPersistenceManager implements PersistenceManager { protected static final DocumentFactory docFactory = DocumentFactory.getInstance(); private static final int DEFAULT_MAX = 100; - @Override - public boolean createMessage(ArchivedMessage message) { - throw new UnsupportedOperationException("MAM-MUC cannot perform this operation"); - } - - @Override - public int processAllMessages(ArchivedMessageConsumer callback) { - throw new UnsupportedOperationException("MAM-MUC cannot perform this operation"); - } - - @Override - public boolean createConversation(Conversation conversation) { - throw new UnsupportedOperationException("MAM-MUC cannot perform this operation"); - } - - @Override - public boolean updateConversationEnd(Conversation conversation) { - throw new UnsupportedOperationException("MAM-MUC cannot perform this operation"); - } - - @Override - public boolean createParticipant(Participant participant, Long conversationId) { - throw new UnsupportedOperationException("MAM-MUC cannot perform this operation"); - } - - @Override - public List findConversations(String[] participants, Date startDate, Date endDate) { - throw new UnsupportedOperationException("MAM-MUC cannot perform this operation"); - } - @Override public Collection findConversations(Date startDate, Date endDate, JID owner, JID with, XmppResultSet xmppResultSet) { throw new UnsupportedOperationException("MAM-MUC cannot perform this operation"); @@ -385,26 +356,11 @@ static Long getMessageIdForStableId( final MUCRoom room, final String value ) return null; } - @Override - public Collection getActiveConversations(int conversationTimeout) { - throw new UnsupportedOperationException("MAM-MUC cannot perform this operation"); - } - - @Override - public List getConversations(Collection conversationIds) { - throw new UnsupportedOperationException("MAM-MUC cannot perform this operation"); - } - @Override public Conversation getConversation(JID owner, JID with, Date start) { throw new UnsupportedOperationException("MAM-MUC cannot perform this operation"); } - @Override - public Conversation getConversation(Long conversationId) { - throw new UnsupportedOperationException("MAM-MUC cannot perform this operation"); - } - public static Instant getDateOfFirstLog( MUCRoom room ) { Connection connection = null; From 93ec9a4d2db79b6cd3db49822719a29d5f63b5fc Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Tue, 1 Dec 2020 17:30:32 +0100 Subject: [PATCH 06/19] fixes #139: Removing unused code Removing code that is not referenced / used anywhere. This reduces complexity, and improves maintainability. --- .../plugin/archive/ArchiveFactory.java | 32 ------------------- .../plugin/archive/ArchiveProperties.java | 3 -- .../archive/ArchivedMessageConsumer.java | 11 ------- .../openfire/plugin/archive/IndexManager.java | 32 ------------------- .../archive/impl/JdbcPersistenceManager.java | 6 +--- .../plugin/archive/impl/MessageIndexer.java | 1 - .../impl/MucMamPersistenceManager.java | 2 +- .../impl/PaginatedMessageDatabaseQuery.java | 2 -- .../plugin/archive/model/ArchivedMessage.java | 28 +--------------- .../plugin/archive/util/StanzaIDUtil.java | 5 --- .../plugin/archive/xep/AbstractIQHandler.java | 5 --- .../archive/xep0313/IQQueryHandler.java | 2 -- .../plugin/archive/xep0313/Result.java | 2 -- .../archive/xep0313/Xep0313Support.java | 1 - .../archive/xep0313/Xep0313Support1.java | 1 - .../openfire/archive/ArchivedMessage.java | 1 - .../cluster/GetConversationCountTask.java | 1 - .../archive/cluster/GetConversationsTask.java | 1 - .../openfire/plugin/JerseyWrapper.java | 1 - .../openfire/plugin/JerseyWrapperPublic.java | 1 - .../openfire/plugin/MonitoringPlugin.java | 20 ------------ .../openfire/plugin/service/LogAPI.java | 2 -- .../reporting/stats/GetStatistics.java | 1 - .../openfire/reporting/util/TaskEngine.java | 1 - src/web/archive-search.jsp | 2 -- 25 files changed, 3 insertions(+), 161 deletions(-) delete mode 100644 src/java/com/reucon/openfire/plugin/archive/ArchiveFactory.java delete mode 100644 src/java/com/reucon/openfire/plugin/archive/ArchivedMessageConsumer.java delete mode 100644 src/java/com/reucon/openfire/plugin/archive/IndexManager.java diff --git a/src/java/com/reucon/openfire/plugin/archive/ArchiveFactory.java b/src/java/com/reucon/openfire/plugin/archive/ArchiveFactory.java deleted file mode 100644 index d3f8ad713..000000000 --- a/src/java/com/reucon/openfire/plugin/archive/ArchiveFactory.java +++ /dev/null @@ -1,32 +0,0 @@ -package com.reucon.openfire.plugin.archive; - -import java.util.Date; - -import org.jivesoftware.openfire.session.Session; -import com.reucon.openfire.plugin.archive.util.StanzaIDUtil; -import org.xmpp.packet.JID; -import org.xmpp.packet.Message; - -import com.reucon.openfire.plugin.archive.model.ArchivedMessage; - -/** - * Factory to create model objects. - */ -public class ArchiveFactory { - private ArchiveFactory() { - - } - - public static ArchivedMessage createArchivedMessage(Session session, - Message message, ArchivedMessage.Direction direction, JID owner, JID with) { - final String sid = StanzaIDUtil.findFirstUniqueAndStableStanzaID( message, owner.toBareJID() ); - final ArchivedMessage archivedMessage; - - // Use a 'null' value for the numeric database ID, as the resulting object has not yet been stored in the database. - final Long id = null; - - archivedMessage = new ArchivedMessage(id, new Date(), direction, message.getType().toString(), with, sid, message.getBody(), message.toXML()); - - return archivedMessage; - } -} diff --git a/src/java/com/reucon/openfire/plugin/archive/ArchiveProperties.java b/src/java/com/reucon/openfire/plugin/archive/ArchiveProperties.java index c587fc808..5596df2e4 100644 --- a/src/java/com/reucon/openfire/plugin/archive/ArchiveProperties.java +++ b/src/java/com/reucon/openfire/plugin/archive/ArchiveProperties.java @@ -7,8 +7,5 @@ public interface ArchiveProperties { // TODO: change the below to a separate property to allow archiving but disable/enable XEP-0136 String ENABLED = "conversation.metadataArchiving"; - String INDEX_DIR = "archive.indexdir"; - // Unnecessary since Open Archive Archive Manager no longer archives messages - String CONVERSATION_TIMEOUT = "conversation.idleTime"; String FORCE_RSM = "archive.FORCE_RSM"; } diff --git a/src/java/com/reucon/openfire/plugin/archive/ArchivedMessageConsumer.java b/src/java/com/reucon/openfire/plugin/archive/ArchivedMessageConsumer.java deleted file mode 100644 index 099eb169a..000000000 --- a/src/java/com/reucon/openfire/plugin/archive/ArchivedMessageConsumer.java +++ /dev/null @@ -1,11 +0,0 @@ -package com.reucon.openfire.plugin.archive; - -import com.reucon.openfire.plugin.archive.model.ArchivedMessage; - -/** - * Consumes an ArchivedMessage. - */ -public interface ArchivedMessageConsumer -{ - boolean consume(ArchivedMessage message); -} diff --git a/src/java/com/reucon/openfire/plugin/archive/IndexManager.java b/src/java/com/reucon/openfire/plugin/archive/IndexManager.java deleted file mode 100644 index 5566e6f6b..000000000 --- a/src/java/com/reucon/openfire/plugin/archive/IndexManager.java +++ /dev/null @@ -1,32 +0,0 @@ -package com.reucon.openfire.plugin.archive; - -import com.reucon.openfire.plugin.archive.model.Conversation; - -import java.util.Collection; -import java.util.Date; - -/** - * Maintains an index for message retrieval. - */ -public interface IndexManager -{ - /** - * Asynchronously indexes the given object. - * @param object the object to index. - * @return true if successfully queued for indexing, false otherwise. - */ - boolean indexObject(Object object); - - /** - * Rebuilds the index. - * - * @return the number of messages indexed or -1 on error. - */ - int rebuildIndex(); - - Collection searchParticipant(String token); - - Collection findConversations(String[] participants, Date startDate, Date endDate, String keywords); - - void destroy(); -} diff --git a/src/java/com/reucon/openfire/plugin/archive/impl/JdbcPersistenceManager.java b/src/java/com/reucon/openfire/plugin/archive/impl/JdbcPersistenceManager.java index c6e0488d0..3f0a660d1 100644 --- a/src/java/com/reucon/openfire/plugin/archive/impl/JdbcPersistenceManager.java +++ b/src/java/com/reucon/openfire/plugin/archive/impl/JdbcPersistenceManager.java @@ -472,7 +472,6 @@ public Conversation getConversation(JID owner, JID with, Date start) { ArchivedMessage message; message = extractMessage(rs); - message.setConversation(conversation); conversation.addMessage(message); } } catch (SQLException sqle) { @@ -621,7 +620,6 @@ static protected ArchivedMessage asArchivedMessage(JID owner, String fromJID, St stanza = message.toString(); } - Message.Type type; String sid; try { @@ -643,12 +641,10 @@ static protected ArchivedMessage asArchivedMessage(JID owner, String fromJID, St } final Document doc = DocumentHelper.parseText( stanza ); final Message message = new Message( doc.getRootElement() ); - type = message.getType(); sid = StanzaIDUtil.findFirstUniqueAndStableStanzaID( message, owner.toBareJID() ); } catch ( Exception e ) { Log.warn( "An exception occurred while parsing message with ID {}", id, e ); sid = null; - type = null; } final JID from = new JID(fromJID + ( fromJIDResource == null || fromJIDResource.isEmpty() ? "" : "/" + fromJIDResource )); @@ -663,7 +659,7 @@ static protected ArchivedMessage asArchivedMessage(JID owner, String fromJID, St direction = Direction.to; with = to; } - final ArchivedMessage archivedMessage = new ArchivedMessage(id, sentDate, direction, type == null ? null : type.toString(), with, sid, body, stanza); + final ArchivedMessage archivedMessage = new ArchivedMessage(id, sentDate, direction, with, sid, body, stanza); return archivedMessage; } diff --git a/src/java/com/reucon/openfire/plugin/archive/impl/MessageIndexer.java b/src/java/com/reucon/openfire/plugin/archive/impl/MessageIndexer.java index 48f128611..e7738caea 100644 --- a/src/java/com/reucon/openfire/plugin/archive/impl/MessageIndexer.java +++ b/src/java/com/reucon/openfire/plugin/archive/impl/MessageIndexer.java @@ -9,7 +9,6 @@ import org.jivesoftware.openfire.index.LuceneIndexer; import org.jivesoftware.openfire.reporting.util.TaskEngine; import org.jivesoftware.util.JiveGlobals; -import org.jivesoftware.util.StringUtils; import org.xmpp.packet.JID; import java.io.File; diff --git a/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java b/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java index e15b64cb0..267716bb8 100644 --- a/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java +++ b/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java @@ -268,7 +268,7 @@ static protected ArchivedMessage asArchivedMessage(JID roomJID, String senderJID sid = null; } - final ArchivedMessage archivedMessage = new ArchivedMessage(id, sentDate, ArchivedMessage.Direction.from, null, null, sid, body, stanza); + final ArchivedMessage archivedMessage = new ArchivedMessage(id, sentDate, ArchivedMessage.Direction.from, null, sid, body, stanza); return archivedMessage; } diff --git a/src/java/com/reucon/openfire/plugin/archive/impl/PaginatedMessageDatabaseQuery.java b/src/java/com/reucon/openfire/plugin/archive/impl/PaginatedMessageDatabaseQuery.java index cd3fb6505..185281b30 100644 --- a/src/java/com/reucon/openfire/plugin/archive/impl/PaginatedMessageDatabaseQuery.java +++ b/src/java/com/reucon/openfire/plugin/archive/impl/PaginatedMessageDatabaseQuery.java @@ -16,8 +16,6 @@ import com.reucon.openfire.plugin.archive.model.ArchivedMessage; import org.jivesoftware.database.DbConnectionManager; -import com.reucon.openfire.plugin.archive.util.StanzaIDUtil; -import org.jivesoftware.util.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.xmpp.packet.JID; diff --git a/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java b/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java index a13513aa0..1f288c724 100644 --- a/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java +++ b/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java @@ -4,7 +4,6 @@ import org.xmpp.packet.JID; import java.util.Date; -import java.util.UUID; /** * An archived message. @@ -26,18 +25,15 @@ public enum Direction { private final Long id; private final Date time; private final Direction direction; - private final String type; private final String body; - private Conversation conversation; private final JID with; private final String stanza; private final String stableId; - public ArchivedMessage( Long id, Date time, Direction direction, String type, JID with, String stableId, String body, String stanza) { + public ArchivedMessage( Long id, Date time, Direction direction, JID with, String stableId, String body, String stanza) { this.id = id; this.time = time; this.direction = direction; - this.type = type; this.with = with; this.stableId = stableId; this.body = body; @@ -56,10 +52,6 @@ public Direction getDirection() { return direction; } - public String getType() { - return type; - } - public String getBody() { return body; } @@ -68,24 +60,6 @@ public String getStanza() { return stanza; } - public Conversation getConversation() { - return conversation; - } - - public void setConversation(Conversation conversation) { - this.conversation = conversation; - } - - /** - * Checks if this message contains payload that should be archived. - * - * @return true if this message is empty, false - * otherwise. - */ - public boolean isEmpty() { - return body == null; - } - public JID getWith() { return with; } diff --git a/src/java/com/reucon/openfire/plugin/archive/util/StanzaIDUtil.java b/src/java/com/reucon/openfire/plugin/archive/util/StanzaIDUtil.java index 3b5c539fe..2e7de90b8 100644 --- a/src/java/com/reucon/openfire/plugin/archive/util/StanzaIDUtil.java +++ b/src/java/com/reucon/openfire/plugin/archive/util/StanzaIDUtil.java @@ -2,14 +2,9 @@ import org.dom4j.Element; import org.dom4j.QName; -import org.jivesoftware.util.JiveGlobals; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.xmpp.packet.*; -import java.util.Iterator; import java.util.List; -import java.util.UUID; /* * This is a partial copy of the implementation provided in Openfire 4.5.2 by diff --git a/src/java/com/reucon/openfire/plugin/archive/xep/AbstractIQHandler.java b/src/java/com/reucon/openfire/plugin/archive/xep/AbstractIQHandler.java index 5e0e25b95..86ebf5565 100644 --- a/src/java/com/reucon/openfire/plugin/archive/xep/AbstractIQHandler.java +++ b/src/java/com/reucon/openfire/plugin/archive/xep/AbstractIQHandler.java @@ -8,7 +8,6 @@ import org.xmpp.packet.Packet; import org.xmpp.packet.PacketError; -import com.reucon.openfire.plugin.archive.IndexManager; import com.reucon.openfire.plugin.archive.PersistenceManager; /** @@ -31,10 +30,6 @@ protected PersistenceManager getPersistenceManager(JID jid) { return MonitoringPlugin.getInstance().getPersistenceManager(jid); } - protected IndexManager getIndexManager() { - return MonitoringPlugin.getInstance().getIndexManager(); - } - protected IQ error(Packet packet, PacketError.Condition condition) { IQ reply; diff --git a/src/java/com/reucon/openfire/plugin/archive/xep0313/IQQueryHandler.java b/src/java/com/reucon/openfire/plugin/archive/xep0313/IQQueryHandler.java index 254d8b3b0..e96ac943c 100644 --- a/src/java/com/reucon/openfire/plugin/archive/xep0313/IQQueryHandler.java +++ b/src/java/com/reucon/openfire/plugin/archive/xep0313/IQQueryHandler.java @@ -1,7 +1,6 @@ package com.reucon.openfire.plugin.archive.xep0313; import com.reucon.openfire.plugin.archive.ArchiveProperties; -import com.reucon.openfire.plugin.archive.impl.MucIndexer; import com.reucon.openfire.plugin.archive.model.ArchivedMessage; import com.reucon.openfire.plugin.archive.xep.AbstractIQHandler; import com.reucon.openfire.plugin.archive.xep0059.XmppResultSet; @@ -30,7 +29,6 @@ import java.text.ParseException; import java.time.*; -import java.time.Instant; import java.util.*; import java.util.LinkedList; import java.util.concurrent.ExecutorService; diff --git a/src/java/com/reucon/openfire/plugin/archive/xep0313/Result.java b/src/java/com/reucon/openfire/plugin/archive/xep0313/Result.java index 0e2ea19cf..7444da039 100644 --- a/src/java/com/reucon/openfire/plugin/archive/xep0313/Result.java +++ b/src/java/com/reucon/openfire/plugin/archive/xep0313/Result.java @@ -3,8 +3,6 @@ import org.jivesoftware.openfire.forward.Forwarded; import org.xmpp.packet.PacketExtension; -import java.util.Date; - /** * Created by dwd on 26/07/16. */ diff --git a/src/java/com/reucon/openfire/plugin/archive/xep0313/Xep0313Support.java b/src/java/com/reucon/openfire/plugin/archive/xep0313/Xep0313Support.java index 773b0d2f6..cc8f0a9a4 100644 --- a/src/java/com/reucon/openfire/plugin/archive/xep0313/Xep0313Support.java +++ b/src/java/com/reucon/openfire/plugin/archive/xep0313/Xep0313Support.java @@ -3,7 +3,6 @@ import java.util.ArrayList; import org.jivesoftware.openfire.XMPPServer; -import org.jivesoftware.openfire.handler.IQHandler; import com.reucon.openfire.plugin.archive.xep.AbstractXepSupport; diff --git a/src/java/com/reucon/openfire/plugin/archive/xep0313/Xep0313Support1.java b/src/java/com/reucon/openfire/plugin/archive/xep0313/Xep0313Support1.java index ef2cf8fb0..3734413f5 100644 --- a/src/java/com/reucon/openfire/plugin/archive/xep0313/Xep0313Support1.java +++ b/src/java/com/reucon/openfire/plugin/archive/xep0313/Xep0313Support1.java @@ -2,7 +2,6 @@ import com.reucon.openfire.plugin.archive.xep.AbstractXepSupport; import org.jivesoftware.openfire.XMPPServer; -import org.jivesoftware.openfire.handler.IQHandler; import java.util.ArrayList; diff --git a/src/java/org/jivesoftware/openfire/archive/ArchivedMessage.java b/src/java/org/jivesoftware/openfire/archive/ArchivedMessage.java index 298b44797..a9523ab97 100644 --- a/src/java/org/jivesoftware/openfire/archive/ArchivedMessage.java +++ b/src/java/org/jivesoftware/openfire/archive/ArchivedMessage.java @@ -18,7 +18,6 @@ import org.jivesoftware.database.JiveID; import org.jivesoftware.database.SequenceManager; -import org.jivesoftware.util.JiveConstants; import org.xmpp.packet.JID; import java.util.Date; diff --git a/src/java/org/jivesoftware/openfire/archive/cluster/GetConversationCountTask.java b/src/java/org/jivesoftware/openfire/archive/cluster/GetConversationCountTask.java index 1fc2e3215..498d06d5f 100644 --- a/src/java/org/jivesoftware/openfire/archive/cluster/GetConversationCountTask.java +++ b/src/java/org/jivesoftware/openfire/archive/cluster/GetConversationCountTask.java @@ -26,7 +26,6 @@ import org.slf4j.LoggerFactory; import javax.annotation.Nonnull; -import java.io.IOException; import java.io.ObjectInput; import java.io.ObjectOutput; import java.util.Optional; diff --git a/src/java/org/jivesoftware/openfire/archive/cluster/GetConversationsTask.java b/src/java/org/jivesoftware/openfire/archive/cluster/GetConversationsTask.java index e7676a40f..13d2dc611 100644 --- a/src/java/org/jivesoftware/openfire/archive/cluster/GetConversationsTask.java +++ b/src/java/org/jivesoftware/openfire/archive/cluster/GetConversationsTask.java @@ -26,7 +26,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.IOException; import java.io.ObjectInput; import java.io.ObjectOutput; import java.util.Collection; diff --git a/src/java/org/jivesoftware/openfire/plugin/JerseyWrapper.java b/src/java/org/jivesoftware/openfire/plugin/JerseyWrapper.java index 9ba5aa29e..3f917fd3f 100644 --- a/src/java/org/jivesoftware/openfire/plugin/JerseyWrapper.java +++ b/src/java/org/jivesoftware/openfire/plugin/JerseyWrapper.java @@ -6,7 +6,6 @@ import javax.servlet.ServletConfig; import javax.servlet.ServletException; -import org.jivesoftware.openfire.plugin.service.LogAPI; import org.jivesoftware.openfire.plugin.service.MonitoringAPI; import com.sun.jersey.api.core.PackagesResourceConfig; diff --git a/src/java/org/jivesoftware/openfire/plugin/JerseyWrapperPublic.java b/src/java/org/jivesoftware/openfire/plugin/JerseyWrapperPublic.java index 90b140058..3ac8b89e2 100644 --- a/src/java/org/jivesoftware/openfire/plugin/JerseyWrapperPublic.java +++ b/src/java/org/jivesoftware/openfire/plugin/JerseyWrapperPublic.java @@ -3,7 +3,6 @@ import com.sun.jersey.api.core.PackagesResourceConfig; import com.sun.jersey.spi.container.servlet.ServletContainer; import org.jivesoftware.openfire.plugin.service.LogAPI; -import org.jivesoftware.openfire.plugin.service.MonitoringAPI; import javax.servlet.ServletConfig; import javax.servlet.ServletException; diff --git a/src/java/org/jivesoftware/openfire/plugin/MonitoringPlugin.java b/src/java/org/jivesoftware/openfire/plugin/MonitoringPlugin.java index 82f463439..4758df5cb 100644 --- a/src/java/org/jivesoftware/openfire/plugin/MonitoringPlugin.java +++ b/src/java/org/jivesoftware/openfire/plugin/MonitoringPlugin.java @@ -49,12 +49,10 @@ import org.jivesoftware.openfire.reporting.util.TaskEngine; import org.jivesoftware.util.JiveGlobals; import org.jivesoftware.util.JiveProperties; -import org.jivesoftware.util.SystemProperty; import org.picocontainer.MutablePicoContainer; import org.picocontainer.defaults.DefaultPicoContainer; import com.reucon.openfire.plugin.archive.ArchiveProperties; -import com.reucon.openfire.plugin.archive.IndexManager; import com.reucon.openfire.plugin.archive.PersistenceManager; import com.reucon.openfire.plugin.archive.xep0136.Xep0136Support; import com.reucon.openfire.plugin.archive.xep0313.Xep0313Support; @@ -85,14 +83,10 @@ public class MonitoringPlugin implements Plugin { private MutablePicoContainer picoContainer; - private boolean shuttingDown = false; - - private int conversationTimeout; private static MonitoringPlugin instance; private boolean enabled = true; private PersistenceManager persistenceManager; private PersistenceManager mucPersistenceManager; - private IndexManager indexManager; private Xep0136Support xep0136Support; private Xep0313Support xep0313Support; private Xep0313Support1 xep0313Support1; @@ -144,10 +138,6 @@ public boolean isEnabled() { return this.enabled; } - public IndexManager getIndexManager() { - return indexManager; - } - public PersistenceManager getPersistenceManager(JID jid) { Log.debug("Getting PersistenceManager for {}", jid); if (XMPPServer.getInstance().getMultiUserChatManager().getMultiUserChatService(jid) != null) { @@ -172,9 +162,6 @@ public void initializePlugin(PluginManager manager, File pluginDirectory) { Log = LoggerFactory.getLogger(MonitoringPlugin.class); /* Configuration */ - conversationTimeout = JiveGlobals.getIntProperty( - ArchiveProperties.CONVERSATION_TIMEOUT, - DEFAULT_CONVERSATION_TIMEOUT); enabled = JiveGlobals.getBooleanProperty(ArchiveProperties.ENABLED, false); @@ -213,8 +200,6 @@ public boolean accept(File pathname) { "This plugin cannot run next to the Enterprise plugin"); } - shuttingDown = false; - // Make sure that the monitoring folder exists under the home directory File dir = new File(JiveGlobals.getHomeDirectory() + File.separator + MonitoringConstants.NAME); @@ -228,7 +213,6 @@ public boolean accept(File pathname) { } public void destroyPlugin() { - shuttingDown = true; // Issue #114: Shut down the task-engine first, to prevent tasks from being executed in a shutting-down environment. TaskEngine.getInstance().dispose(); @@ -249,10 +233,6 @@ public void destroyPlugin() { instance = null; } - public boolean isShuttingDown() { - return shuttingDown; - } - protected void loadPublicWeb(File pluginDirectory) { diff --git a/src/java/org/jivesoftware/openfire/plugin/service/LogAPI.java b/src/java/org/jivesoftware/openfire/plugin/service/LogAPI.java index 37bbf254d..5f6f25fea 100644 --- a/src/java/org/jivesoftware/openfire/plugin/service/LogAPI.java +++ b/src/java/org/jivesoftware/openfire/plugin/service/LogAPI.java @@ -7,7 +7,6 @@ import org.jivesoftware.openfire.muc.MultiUserChatService; import org.jivesoftware.util.JiveGlobals; import org.jivesoftware.util.StringUtils; -import org.jivesoftware.util.SystemProperty; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.xmpp.packet.JID; @@ -24,7 +23,6 @@ import java.sql.SQLException; import java.time.Instant; import java.time.LocalDate; -import java.time.LocalDateTime; import java.time.ZoneOffset; import java.time.format.DateTimeFormatter; import java.time.format.DateTimeParseException; diff --git a/src/java/org/jivesoftware/openfire/reporting/stats/GetStatistics.java b/src/java/org/jivesoftware/openfire/reporting/stats/GetStatistics.java index c911cf339..a71584e33 100644 --- a/src/java/org/jivesoftware/openfire/reporting/stats/GetStatistics.java +++ b/src/java/org/jivesoftware/openfire/reporting/stats/GetStatistics.java @@ -15,7 +15,6 @@ */ package org.jivesoftware.openfire.reporting.stats; -import java.io.IOException; import java.io.ObjectInput; import java.io.ObjectOutput; import java.util.HashMap; diff --git a/src/java/org/jivesoftware/openfire/reporting/util/TaskEngine.java b/src/java/org/jivesoftware/openfire/reporting/util/TaskEngine.java index e983f31c9..a1f43a8ae 100644 --- a/src/java/org/jivesoftware/openfire/reporting/util/TaskEngine.java +++ b/src/java/org/jivesoftware/openfire/reporting/util/TaskEngine.java @@ -23,7 +23,6 @@ import java.util.*; import java.util.concurrent.*; -import java.util.concurrent.atomic.AtomicInteger; /** * Performs tasks using worker threads. It also allows tasks to be scheduled to be diff --git a/src/web/archive-search.jsp b/src/web/archive-search.jsp index 813e5f2b8..9e9396517 100644 --- a/src/web/archive-search.jsp +++ b/src/web/archive-search.jsp @@ -5,7 +5,6 @@ <%@ page import="org.jivesoftware.openfire.archive.Conversation" %> <%@ page import="org.jivesoftware.openfire.archive.ConversationManager" %> <%@ page import="org.jivesoftware.openfire.XMPPServer" %> -<%@ page import="org.jivesoftware.openfire.user.UserManager" %> <%@ page import="org.jivesoftware.openfire.user.UserNameManager" %> <%@ page import="org.jivesoftware.openfire.user.UserNotFoundException" %> <%@ page import="org.jivesoftware.util.*" %> @@ -66,7 +65,6 @@ } if (submit) { - UserManager userManager = UserManager.getInstance(); ArchiveSearch search = new ArchiveSearch(); JID participant1JID = null; JID participant2JID = null; From cf557a7ca216fd534fb9d8605528722061575c90 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Wed, 2 Dec 2020 09:26:30 +0100 Subject: [PATCH 07/19] fixes #139: Removing unused code Removing code that is not referenced / used anywhere. This reduces complexity, and improves maintainability. --- .../PaginatedMucMessageDatabaseQuery.java | 2 +- .../plugin/archive/model/Preferences.java | 22 -- .../plugin/archive/util/EscapeUtil.java | 50 ---- .../plugin/archive/xep0136/RemoveRequest.java | 78 ------- .../openfire/archive/ArchiveSearch.java | 13 -- .../openfire/archive/ArchiveSearcher.java | 8 +- .../openfire/archive/ConversationEvent.java | 2 +- .../archive/ConversationListener.java | 7 +- .../GetGroupConversationTranscript.java | 216 ------------------ .../openfire/exceptions/ExceptionType.java | 31 --- .../openfire/plugin/MonitoringPlugin.java | 2 - .../openfire/plugin/service/LogAPI.java | 2 +- .../openfire/reporting/graph/GraphEngine.java | 26 +-- .../reporting/graph/GraphServlet.java | 2 +- .../openfire/reporting/stats/StatsAction.java | 2 +- .../openfire/reporting/stats/StatsViewer.java | 6 +- 16 files changed, 21 insertions(+), 448 deletions(-) delete mode 100644 src/java/com/reucon/openfire/plugin/archive/model/Preferences.java delete mode 100644 src/java/com/reucon/openfire/plugin/archive/util/EscapeUtil.java delete mode 100644 src/java/com/reucon/openfire/plugin/archive/xep0136/RemoveRequest.java delete mode 100644 src/java/org/jivesoftware/openfire/archive/commands/GetGroupConversationTranscript.java delete mode 100644 src/java/org/jivesoftware/openfire/exceptions/ExceptionType.java diff --git a/src/java/com/reucon/openfire/plugin/archive/impl/PaginatedMucMessageDatabaseQuery.java b/src/java/com/reucon/openfire/plugin/archive/impl/PaginatedMucMessageDatabaseQuery.java index b042ea08a..7f0717538 100644 --- a/src/java/com/reucon/openfire/plugin/archive/impl/PaginatedMucMessageDatabaseQuery.java +++ b/src/java/com/reucon/openfire/plugin/archive/impl/PaginatedMucMessageDatabaseQuery.java @@ -211,7 +211,7 @@ private String getStatementForSQLServer(final Long after, final Long before, fin { String sql = "SELECT sender, nickname, logTime, subject, body, stanza, messageId "; sql += " FROM ( "; - sql += " SELECT TOP("+String.valueOf(maxResults)+") sender, nickname, logTime, subject, body, stanza, messageId FROM ofMucConversationLog "; + sql += " SELECT TOP("+ maxResults +") sender, nickname, logTime, subject, body, stanza, messageId FROM ofMucConversationLog "; sql += " WHERE messageId IS NOT NULL AND logTime > ? AND logTime <= ? AND roomID = ? AND (nickname IS NOT NULL OR subject IS NOT NULL) "; if ( with != null ) { // XEP-0313 specifies: If (and only if) the supplied JID is a bare JID (i.e. no resource is present), then the server diff --git a/src/java/com/reucon/openfire/plugin/archive/model/Preferences.java b/src/java/com/reucon/openfire/plugin/archive/model/Preferences.java deleted file mode 100644 index 959859d83..000000000 --- a/src/java/com/reucon/openfire/plugin/archive/model/Preferences.java +++ /dev/null @@ -1,22 +0,0 @@ -package com.reucon.openfire.plugin.archive.model; - -import java.util.Map; - -/** - * A user's archiving preferences according to XEP-0136. - */ -public class Preferences -{ - public enum MethodUsage - { - forbid, - concide, - prefer - } - - private String username; - private Map methods; - - - -} diff --git a/src/java/com/reucon/openfire/plugin/archive/util/EscapeUtil.java b/src/java/com/reucon/openfire/plugin/archive/util/EscapeUtil.java deleted file mode 100644 index 0a816faf3..000000000 --- a/src/java/com/reucon/openfire/plugin/archive/util/EscapeUtil.java +++ /dev/null @@ -1,50 +0,0 @@ -package com.reucon.openfire.plugin.archive.util; - -public class EscapeUtil -{ - public static String escapeHtml(String source) - { - - int terminatorIndex; - if (source == null) - { - return null; - } - StringBuffer result = new StringBuffer(source.length() * 2); - for (int i = 0; i < source.length(); i++) - { - int ch = source.charAt(i); - // avoid escaping already escaped characters - if (ch == 38) - { - terminatorIndex = source.indexOf(";", i); - if (terminatorIndex > 0) - { - if (source.substring(i + 1, terminatorIndex).matches("#[0-9]+|lt|gt|amp|quote")) - { - result.append(source.substring(i, terminatorIndex + 1)); - // Skip remaining chars up to (and including) ";" - i = terminatorIndex; - continue; - } - } - } - if (ch == 10) - { - result.append("
    "); - } - else if (ch != 32 && (ch > 122 || ch < 48 || ch == 60 || ch == 62)) - { - result.append("&#"); - result.append(ch); - result.append(";"); - } - else - { - result.append((char) ch); - } - } - return new String(result); - } - -} diff --git a/src/java/com/reucon/openfire/plugin/archive/xep0136/RemoveRequest.java b/src/java/com/reucon/openfire/plugin/archive/xep0136/RemoveRequest.java deleted file mode 100644 index 74c52d1f7..000000000 --- a/src/java/com/reucon/openfire/plugin/archive/xep0136/RemoveRequest.java +++ /dev/null @@ -1,78 +0,0 @@ -package com.reucon.openfire.plugin.archive.xep0136; - -import com.reucon.openfire.plugin.archive.util.XmppDateUtil; -import org.dom4j.Element; - -import java.util.Date; - -/** - * A request to remove one or more collections. - *

    - * To request the removal of a single collection the client sends an empty <remove/> element.
    - * The 'with' (full JID) and 'start' attributes MUST be included to uniquely identify the collection. - *

    - * The client may remove several collections at once.
    - * The 'start' and 'end' elements MAY be specified to indicate a date range.
    - * The 'with' attribute MAY be a full JID, bare JID or domain. - *

    - * If the value of the optional 'open' attribute is set to 'true' then only collections that are currently - * being recorded automatically by the server (see Automated Archiving) are removed. - */ -public class RemoveRequest -{ - private final String with; - private final Date start; - private final Date end; - private final boolean open; - - public RemoveRequest(Element listElement) - { - this.with = listElement.attributeValue("with"); - this.start = XmppDateUtil.parseDate(listElement.attributeValue("start")); - this.end = XmppDateUtil.parseDate(listElement.attributeValue("end")); - this.open = "true".equals(listElement.attributeValue("open")); - } - - /** - * The 'with' attribute MAY be a full JID, bare JID or domain.
    - * If the 'with' attribute is omitted then collections with any JID are removed. - * - * @return the value of the with attribute, may be null. - */ - public String getWith() - { - return with; - } - - /** - * If the start date is before all the collections in the archive then all collections prior - * to the end date are removed. - * - * @return the value of the start attribute, may be null. - */ - public Date getStart() - { - return start; - } - - /** - * If the end date is in the future then then all collections after the start date are removed. - * - * @return the value of the end attribute, may be null. - */ - public Date getEnd() - { - return end; - } - - /** - * If the value of the optional 'open' attribute is set to 'true' then only collections that - * are currently being recorded automatically by the server (see Automated Archiving) are removed. - * - * @return the value of the open attribute or false if not set. - */ - public boolean getOpen() - { - return open; - } -} diff --git a/src/java/org/jivesoftware/openfire/archive/ArchiveSearch.java b/src/java/org/jivesoftware/openfire/archive/ArchiveSearch.java index ce9adeeb1..0515b78d0 100644 --- a/src/java/org/jivesoftware/openfire/archive/ArchiveSearch.java +++ b/src/java/org/jivesoftware/openfire/archive/ArchiveSearch.java @@ -63,19 +63,6 @@ public class ArchiveSearch { private SortOrder sortOrder; private boolean externalWildcardMode; - /** - * Creates a new search on a query string. - * - * @param queryString the query string to use for the search. - * @return an ArchiveSearch instance to search using the specified query string. - */ - public static ArchiveSearch createKeywordSearch(String queryString) { - ArchiveSearch search = new ArchiveSearch(); - search.setQueryString(queryString); - search.setSortField(SortField.relevance); - return search; - } - /** * Constructs a new archive search, sorted on date descending. */ diff --git a/src/java/org/jivesoftware/openfire/archive/ArchiveSearcher.java b/src/java/org/jivesoftware/openfire/archive/ArchiveSearcher.java index ed15b5939..cfac39e79 100644 --- a/src/java/org/jivesoftware/openfire/archive/ArchiveSearcher.java +++ b/src/java/org/jivesoftware/openfire/archive/ArchiveSearcher.java @@ -430,9 +430,7 @@ public Iterator iterator() { public boolean hasNext() { if (nextElement == null) { nextElement = getNextElement(); - if (nextElement == null) { - return false; - } + return nextElement != null; } return true; } @@ -518,9 +516,7 @@ public Iterator iterator() { public boolean hasNext() { if (nextElement == null) { nextElement = getNextElement(); - if (nextElement == null) { - return false; - } + return nextElement != null; } return true; } diff --git a/src/java/org/jivesoftware/openfire/archive/ConversationEvent.java b/src/java/org/jivesoftware/openfire/archive/ConversationEvent.java index 167eeccc2..3f144dcdf 100644 --- a/src/java/org/jivesoftware/openfire/archive/ConversationEvent.java +++ b/src/java/org/jivesoftware/openfire/archive/ConversationEvent.java @@ -201,7 +201,7 @@ public static ConversationEvent roomMessageReceived(JID roomJID, JID user, Strin return event; } - private static enum Type { + private enum Type { /** * Event triggered when a room was destroyed. */ diff --git a/src/java/org/jivesoftware/openfire/archive/ConversationListener.java b/src/java/org/jivesoftware/openfire/archive/ConversationListener.java index 0fdc73085..cabd34287 100644 --- a/src/java/org/jivesoftware/openfire/archive/ConversationListener.java +++ b/src/java/org/jivesoftware/openfire/archive/ConversationListener.java @@ -32,7 +32,7 @@ public interface ConversationListener { * * @param conversation the conversation. */ - public void conversationCreated(Conversation conversation); + void conversationCreated(Conversation conversation); /** * A conversation was updated, which means that a new message was sent between @@ -41,7 +41,7 @@ public interface ConversationListener { * @param conversation the conversation. * @param date the date the conversation was updated. */ - public void conversationUpdated(Conversation conversation, Date date); + void conversationUpdated(Conversation conversation, Date date); /** * A conversation ended due to inactivity or because the maximum conversation time @@ -49,6 +49,5 @@ public interface ConversationListener { * * @param conversation the conversation. */ - public void conversationEnded(Conversation conversation); - + void conversationEnded(Conversation conversation); } diff --git a/src/java/org/jivesoftware/openfire/archive/commands/GetGroupConversationTranscript.java b/src/java/org/jivesoftware/openfire/archive/commands/GetGroupConversationTranscript.java deleted file mode 100644 index 36e8286cb..000000000 --- a/src/java/org/jivesoftware/openfire/archive/commands/GetGroupConversationTranscript.java +++ /dev/null @@ -1,216 +0,0 @@ -/** - - * - * Copyright (C) 2008 Jive Software. All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.jivesoftware.openfire.archive.commands; - -import java.io.ByteArrayOutputStream; -import java.util.*; - -import org.dom4j.Element; -import org.jivesoftware.openfire.XMPPServer; -import org.jivesoftware.openfire.archive.ArchiveSearch; -import org.jivesoftware.openfire.archive.ArchiveSearcher; -import org.jivesoftware.openfire.archive.Conversation; -import org.jivesoftware.openfire.archive.ConversationManager; -import org.jivesoftware.openfire.archive.ConversationUtils; -import org.jivesoftware.openfire.archive.MonitoringConstants; -import org.jivesoftware.openfire.commands.AdHocCommand; -import org.jivesoftware.openfire.commands.SessionData; -import org.jivesoftware.openfire.component.InternalComponentManager; -import org.jivesoftware.openfire.container.Plugin; -import org.jivesoftware.openfire.plugin.MonitoringPlugin; -import org.jivesoftware.util.StringUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.xmpp.component.ComponentManagerFactory; -import org.xmpp.forms.DataForm; -import org.xmpp.forms.FormField; -import org.xmpp.packet.JID; - -/** - * Command that allows to retrieve PDF content of group chat transcripts. - * - * @author Gaston Dombiak - * - * TODO Use i18n - */ -public class GetGroupConversationTranscript extends AdHocCommand { - - private static final Logger Log = LoggerFactory.getLogger(GetGroupConversationTranscript.class); - - @Override - protected void addStageInformation(SessionData data, Element command) { - DataForm form = new DataForm(DataForm.Type.form); - form.setTitle("Requesting PDF of conversation transcript"); - form.addInstruction("Fill out this form to request the conversation transcript in PDF format."); - - FormField field = form.addField(); - field.setType(FormField.Type.hidden); - field.setVariable("FORM_TYPE"); - field.addValue("http://jabber.org/protocol/admin"); - - field = form.addField(); - field.setType(FormField.Type.jid_single); - field.setLabel("JID of the user that participated in the chat"); - field.setVariable("participant"); - field.setRequired(true); - - field = form.addField(); - field.setType(FormField.Type.jid_single); - field.setLabel("JID of the room"); - field.setVariable("room"); - field.setRequired(true); - - field = form.addField(); - field.setType(FormField.Type.text_single); - field.setLabel("Time when the chat took place"); - field.setVariable("time"); - field.setRequired(true); - - field = form.addField(); - field.setType(FormField.Type.boolean_type); - field.setLabel("Include PDF"); - field.setVariable("includePDF"); - field.setRequired(true); - - // Add the form to the command - command.add(form.getElement()); - } - - @Override - public void execute(SessionData data, Element command) { - Element note = command.addElement("note"); - // Get handle on the Monitoring plugin - final Optional plugin = XMPPServer.getInstance().getPluginManager().getPluginByName(MonitoringConstants.PLUGIN_NAME); - if (!plugin.isPresent()) { - Log.error("Unable to execute command! The Monitoring plugin does not appear to be loaded on this machine."); - return; - } - final ConversationManager conversationManager = (ConversationManager) ((MonitoringPlugin)plugin.get()).getModule(ConversationManager.class); - if (!conversationManager.isArchivingEnabled()) { - note.addAttribute("type", "error"); - note.setText("Message archiving is not enabled."); - - DataForm form = new DataForm(DataForm.Type.result); - - FormField field = form.addField(); - field.setType(FormField.Type.hidden); - field.setVariable("FORM_TYPE"); - field.addValue("http://jabber.org/protocol/admin"); - - field = form.addField(); - field.setLabel("Conversation Found?"); - field.setVariable("found"); - field.addValue(false); - // Add form to reply - command.add(form.getElement()); - - return; - } - - try { - JID participant = new JID(data.getData().get("participant").get(0)); - JID room = new JID(data.getData().get("room").get(0)); - Date time = DataForm.parseDate(data.getData().get("time").get(0)); - boolean includePDF = DataForm.parseBoolean(data.getData().get("includePDF").get(0)); - - // Get archive searcher module - final ArchiveSearcher archiveSearcher = (ArchiveSearcher) ((MonitoringPlugin)plugin.get()).getModule(ArchiveSearcher.class); - - ArchiveSearch search = new ArchiveSearch(); - search.setParticipants(participant); - search.setIncludeTimestamp(time); - search.setRoom(room); - - Collection conversations = archiveSearcher.search(search); - - DataForm form = new DataForm(DataForm.Type.result); - - FormField field = form.addField(); - field.setType(FormField.Type.hidden); - field.setVariable("FORM_TYPE"); - field.addValue("http://jabber.org/protocol/admin"); - - field = form.addField(); - field.setLabel("Conversation Found?"); - field.setVariable("found"); - field.addValue(!conversations.isEmpty()); - - if (includePDF) { - ByteArrayOutputStream stream = null; - if (!conversations.isEmpty()) { - stream = new ConversationUtils().getConversationPDF(conversations.iterator().next()); - } - - if (stream != null) { - field = form.addField(); - field.setLabel("PDF"); - field.setVariable("pdf"); - field.addValue(StringUtils.encodeBase64(stream.toByteArray())); - } - } - - // Add form to reply - command.add(form.getElement()); - } - catch (Exception e) { - Log.error("Error occurred while running the command", e); - note.addAttribute("type", "error"); - note.setText("Error while processing the command."); - } - } - - @Override - public String getCode() { - return "http://jivesoftware.com/protocol/workgroup#get-group-conv-transcript"; - } - - @Override - public String getDefaultLabel() { - return "Get Group Conversation Transcript"; - } - - @Override - protected List getActions(SessionData data) { - return Arrays.asList(Action.complete); - } - - @Override - protected Action getExecuteAction(SessionData data) { - return Action.complete; - } - - @Override - public int getMaxStages(SessionData data) { - return 1; - } - - /** - * Returns if the requester can access this command. Admins and components are allowed to - * execute this command. - * - * @param requester the JID of the entity requesting to execute this command. - * @return true if the requester can access this command. - */ - @Override - public boolean hasPermission(JID requester) { - InternalComponentManager componentManager = - (InternalComponentManager) ComponentManagerFactory.getComponentManager(); - return super.hasPermission(requester) || componentManager.hasComponent(requester); - } -} diff --git a/src/java/org/jivesoftware/openfire/exceptions/ExceptionType.java b/src/java/org/jivesoftware/openfire/exceptions/ExceptionType.java deleted file mode 100644 index 9ccbc1e9e..000000000 --- a/src/java/org/jivesoftware/openfire/exceptions/ExceptionType.java +++ /dev/null @@ -1,31 +0,0 @@ -package org.jivesoftware.openfire.exceptions; - -/** - * The Class ExceptionType. - */ -public final class ExceptionType { - - /** The Constant SHARED_GROUP_EXCEPTION. */ - public static final String SHARED_GROUP_EXCEPTION = "SharedGroupException"; - - /** The Constant PROPERTY_NOT_FOUND. */ - public static final String PROPERTY_NOT_FOUND = "PropertyNotFoundException"; - - /** The Constant USER_ALREADY_EXISTS_EXCEPTION. */ - public static final String USER_ALREADY_EXISTS_EXCEPTION = "UserAlreadyExistsException"; - - /** The Constant USER_NOT_FOUND_EXCEPTION. */ - public static final String USER_NOT_FOUND_EXCEPTION = "UserNotFoundException"; - - /** The Constant GROUP_ALREADY_EXISTS. */ - public static final String GROUP_ALREADY_EXISTS = "GroupAlreadyExistsException"; - - /** The Constant GROUP_NOT_FOUND. */ - public static final String GROUP_NOT_FOUND = "GroupNotFoundException"; - - /** - * Instantiates a new exception type. - */ - private ExceptionType() { - } -} diff --git a/src/java/org/jivesoftware/openfire/plugin/MonitoringPlugin.java b/src/java/org/jivesoftware/openfire/plugin/MonitoringPlugin.java index 4758df5cb..336124975 100644 --- a/src/java/org/jivesoftware/openfire/plugin/MonitoringPlugin.java +++ b/src/java/org/jivesoftware/openfire/plugin/MonitoringPlugin.java @@ -67,8 +67,6 @@ */ public class MonitoringPlugin implements Plugin { - private static final int DEFAULT_CONVERSATION_TIMEOUT = 30; // minutes - /** * The context root of the URL under which the public web endpoints are exposed. */ diff --git a/src/java/org/jivesoftware/openfire/plugin/service/LogAPI.java b/src/java/org/jivesoftware/openfire/plugin/service/LogAPI.java index 5f6f25fea..c008b8492 100644 --- a/src/java/org/jivesoftware/openfire/plugin/service/LogAPI.java +++ b/src/java/org/jivesoftware/openfire/plugin/service/LogAPI.java @@ -186,7 +186,7 @@ public static class Message { private String nickname; private String message; - public Message() {}; + public Message() {} public String getTimestamp() { diff --git a/src/java/org/jivesoftware/openfire/reporting/graph/GraphEngine.java b/src/java/org/jivesoftware/openfire/reporting/graph/GraphEngine.java index 2aa6c0e1f..39d562d83 100644 --- a/src/java/org/jivesoftware/openfire/reporting/graph/GraphEngine.java +++ b/src/java/org/jivesoftware/openfire/reporting/graph/GraphEngine.java @@ -131,19 +131,11 @@ public JFreeChart generateChart(String key, int width, int height, String color, } XYDataset data = populateData(key, def, startTime, endTime, dataPoints); - if (data == null) { - return null; - } - JFreeChart chart; - switch(def[0].getStatType()) { - case count: - chart = createTimeBarChart(null, color, def[0].getUnits(), data); - break; - default: - chart = createTimeAreaChart(null, color, def[0].getUnits(), data); + if (def[0].getStatType() == Statistic.Type.count) { + return createTimeBarChart(null, color, def[0].getUnits(), data); + } else { + return createTimeAreaChart(null, color, def[0].getUnits(), data); } - - return chart; } @@ -172,12 +164,10 @@ public byte[] generateSparklinesGraph(String key, int width, int height, String } JFreeChart chart; - switch (def[0].getStatType()) { - case count: - chart = generateSparklineBarGraph(key, color, def, startTime, endTime, dataPoints); - break; - default: - chart = generateSparklineAreaChart(key, color, def, startTime, endTime, dataPoints); + if (def[0].getStatType() == Statistic.Type.count) { + chart = generateSparklineBarGraph(key, color, def, startTime, endTime, dataPoints); + } else { + chart = generateSparklineAreaChart(key, color, def, startTime, endTime, dataPoints); } KeypointPNGEncoderAdapter encoder = new KeypointPNGEncoderAdapter(); diff --git a/src/java/org/jivesoftware/openfire/reporting/graph/GraphServlet.java b/src/java/org/jivesoftware/openfire/reporting/graph/GraphServlet.java index 66bc2fa6e..a17f832c7 100644 --- a/src/java/org/jivesoftware/openfire/reporting/graph/GraphServlet.java +++ b/src/java/org/jivesoftware/openfire/reporting/graph/GraphServlet.java @@ -131,7 +131,7 @@ public int compare(String stat1, String stat2) { } - private void writePDFContent(HttpServletRequest request, HttpServletResponse response, JFreeChart charts[], Statistic[] stats, long starttime, long endtime, int width, int height) + private void writePDFContent(HttpServletRequest request, HttpServletResponse response, JFreeChart[] charts, Statistic[] stats, long starttime, long endtime, int width, int height) throws IOException { diff --git a/src/java/org/jivesoftware/openfire/reporting/stats/StatsAction.java b/src/java/org/jivesoftware/openfire/reporting/stats/StatsAction.java index 1eda3e7ed..91240e19e 100644 --- a/src/java/org/jivesoftware/openfire/reporting/stats/StatsAction.java +++ b/src/java/org/jivesoftware/openfire/reporting/stats/StatsAction.java @@ -119,7 +119,7 @@ public List> getNLatestConversations(int count, long mostRecen } else { Map mCon = new HashMap(); mCon.put("conversationid", con.getConversationID()); - String users[]; + String[] users; int usersIdx = 0; if (con.getRoom() == null) { users = new String[con.getParticipants().size()]; diff --git a/src/java/org/jivesoftware/openfire/reporting/stats/StatsViewer.java b/src/java/org/jivesoftware/openfire/reporting/stats/StatsViewer.java index 797b64d70..3a156a471 100644 --- a/src/java/org/jivesoftware/openfire/reporting/stats/StatsViewer.java +++ b/src/java/org/jivesoftware/openfire/reporting/stats/StatsViewer.java @@ -155,14 +155,14 @@ public interface StatsViewer { * determine the period of time which data should be returned, it also provides a * suggestion on the number of datapoints that should be provided. */ - public enum TimePeriod { + enum TimePeriod { last_hour(3600, 15), last_day(43200, 15); private long timePeriod; private int dataPoints; - private TimePeriod(long timePeriod, int dataPoints) { + TimePeriod(long timePeriod, int dataPoints) { this.timePeriod = timePeriod; this.dataPoints = dataPoints; } @@ -196,7 +196,7 @@ public long getTimePeriod() { /** * A snapshot of a stat in time. */ - public final class StatView { + final class StatView { private long startTime; private long endTime; private double[][] data; From 9834cfc7c572142d4862d921a9f9a181ab5acede Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Wed, 2 Dec 2020 09:59:44 +0100 Subject: [PATCH 08/19] fixes #139: Refactoring: annotating ArchivedMessage This adds documentation that helps define how ArchivedMessage is to be used. --- .../plugin/archive/model/ArchivedMessage.java | 69 ++++++++++++++++++- 1 file changed, 68 insertions(+), 1 deletion(-) diff --git a/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java b/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java index 1f288c724..8eb497705 100644 --- a/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java +++ b/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java @@ -3,12 +3,16 @@ import org.jivesoftware.database.JiveID; import org.xmpp.packet.JID; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import javax.annotation.concurrent.Immutable; import java.util.Date; /** * An archived message. */ @JiveID(601) +@Immutable public class ArchivedMessage { public enum Direction { /** @@ -22,15 +26,28 @@ public enum Direction { from } + @Nullable private final Long id; + + @Nonnull private final Date time; + + @Nonnull private final Direction direction; + + @Nullable private final String body; + + @Nullable private final JID with; + + @Nullable private final String stanza; + + @Nullable private final String stableId; - public ArchivedMessage( Long id, Date time, Direction direction, JID with, String stableId, String body, String stanza) { + public ArchivedMessage(@Nullable final Long id, @Nonnull final Date time, @Nonnull final Direction direction, @Nullable final JID with, @Nullable final String stableId, @Nullable final String body, @Nullable final String stanza) { this.id = id; this.time = time; this.direction = direction; @@ -40,30 +57,80 @@ public ArchivedMessage( Long id, Date time, Direction direction, JID with, Strin this.stanza = stanza; } + /** + * The database identifier used to store this message. Expected to be non-null, unless an instance has not been + * persisted in the database yet. + * + * @return A database identifier + */ + @Nullable public Long getId() { return id; } + /** + * The instant when the message was originally sent. + * + * @return date that the message wos originally sent. + */ + @Nonnull public Date getTime() { return time; } + /** + * Defines if the message was originally sent, or received, by the owner of the archive that this message is part of. + * + * @return indication if message was originally sent or received. + */ + @Nonnull public Direction getDirection() { return direction; } + /** + * The textual content of the message that was sent, if the message had any text. + * + * @return Message content + */ + @Nullable public String getBody() { return body; } + /** + * The XML representation of the XMPP stanza that was used to transmit the message. + * + * Note that older version of the Monitoring plugin did not store the XMPP stanza to the database. As a result, + * messages that were archived by those versions of the plugin do not include a stanza. + * + * @return XMPP packet + */ + @Nullable public String getStanza() { return stanza; } + /** + * The message peer (the 'other side' of the conversation), in respect to the owner of the archive that this message + * is part of. + * + * This value will only have meaning when the original message was sent in a one-to-one conversation. When the + * archived message was originally shared in a group chat, this value can be null. + * + * @return The conversation peer. + */ + @Nullable public JID getWith() { return with; } + /** + * The first stable and unique stanza-id value in the stanza, if the stanza contains such a value. + * + * @return a stable and unique stanza-id value. + */ + @Nullable public String getStableId() { return stableId; From b3fed324aaad06646292e5eb0805340c5e212a5d Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Wed, 2 Dec 2020 13:54:37 +0100 Subject: [PATCH 09/19] fixes #139: Refactor stanza construction When a previously archived message is being retrieved, the optional 'stanza' database value is eventually parsed as XMPP. This commit brings forward this process (to a place immediately after the data is obtained from the database), replacing the existing mechanism that did it just before the message was being routed to the intended recipient. The benefit of this is that the code that it is more fail-fast, and operations that follow the database retrieval can work with a proper stanza, instead of having to work with its String representation. This brings type-safety. Lastly, there's less ambiguity around where parsing occurs (and at what point the parsing can be expected to have been completed). --- .../archive/impl/JdbcPersistenceManager.java | 56 ++++--------- .../impl/MucMamPersistenceManager.java | 37 +++----- .../impl/PaginatedMessageDatabaseQuery.java | 5 +- .../PaginatedMucMessageDatabaseQuery.java | 5 +- .../plugin/archive/model/ArchivedMessage.java | 84 +++++++++++++++++-- .../archive/xep0313/IQQueryHandler.java | 66 ++++++--------- 6 files changed, 143 insertions(+), 110 deletions(-) diff --git a/src/java/com/reucon/openfire/plugin/archive/impl/JdbcPersistenceManager.java b/src/java/com/reucon/openfire/plugin/archive/impl/JdbcPersistenceManager.java index 3f0a660d1..6687c7724 100644 --- a/src/java/com/reucon/openfire/plugin/archive/impl/JdbcPersistenceManager.java +++ b/src/java/com/reucon/openfire/plugin/archive/impl/JdbcPersistenceManager.java @@ -7,10 +7,7 @@ import com.reucon.openfire.plugin.archive.model.Participant; import com.reucon.openfire.plugin.archive.util.StanzaIDUtil; import com.reucon.openfire.plugin.archive.xep0059.XmppResultSet; -import org.dom4j.Document; -import org.dom4j.DocumentFactory; -import org.dom4j.DocumentHelper; -import org.dom4j.Element; +import org.dom4j.*; import org.jivesoftware.database.DbConnectionManager; import org.jivesoftware.openfire.archive.ConversationManager; import org.jivesoftware.util.JiveConstants; @@ -31,7 +28,6 @@ */ public class JdbcPersistenceManager implements PersistenceManager { private static final Logger Log = LoggerFactory.getLogger( JdbcPersistenceManager.class ); - protected static final DocumentFactory docFactory = DocumentFactory.getInstance(); public static final int DEFAULT_MAX = 1000; public static final String SELECT_MESSAGES_BY_CONVERSATION = "SELECT DISTINCT ofConversation.conversationID, ofConversation.room, " @@ -474,7 +470,7 @@ public Conversation getConversation(JID owner, JID with, Date start) { message = extractMessage(rs); conversation.addMessage(message); } - } catch (SQLException sqle) { + } catch (SQLException | DocumentException sqle) { Log.error("Error selecting conversation", sqle); } finally { DbConnectionManager.closeConnection(rs, pstmt, con); @@ -542,7 +538,7 @@ private Collection extractParticipant(ResultSet rs) throws SQLExcep return participants; } - static ArchivedMessage extractMessage(ResultSet rs) throws SQLException { + static ArchivedMessage extractMessage(ResultSet rs) throws SQLException, DocumentException { Date time = millisToDate(rs.getLong("sentDate")); String body = rs.getString("body"); String stanza = rs.getString("stanza"); @@ -604,46 +600,26 @@ public static ArchivedMessage getArchivedMessage( long messageId, JID owner ) } catch (SQLException ex) { Log.warn("SQL failure while trying to get message with ID {} from the archive of {}.", messageId, owner, ex); return null; + } catch (DocumentException ex) { + Log.warn("Failure to parse 'stanza' value as XMPP for the message with ID {} from the archive of {}.", messageId, owner, ex); + return null; } finally { DbConnectionManager.closeConnection(rs, pstmt, connection); } } - static protected ArchivedMessage asArchivedMessage(JID owner, String fromJID, String fromJIDResource, String toJID, String toJIDResource, Date sentDate, String body, String stanza, Long id) - { - if (stanza == null) { - Message message = new Message(); - message.setFrom(fromJID); - message.setTo(toJID); - message.setType(Message.Type.normal); - message.setBody(body); - stanza = message.toString(); - } - + static protected ArchivedMessage asArchivedMessage(JID owner, String fromJID, String fromJIDResource, String toJID, String toJIDResource, Date sentDate, String body, String stanza, Long id) throws DocumentException { String sid; - try - { - if ( !JiveGlobals.getBooleanProperty( "conversation.OF-1804.disable", false ) ) - { - // Prior to OF-1804 (Openfire 4.4.0), the stanza was logged with a formatter applied. - // This causes message formatting to be modified (notably, new lines could be altered). - // This workaround restores the original body text, that was stored in a different column. - final int pos1 = stanza.indexOf( "" ); - final int pos2 = stanza.indexOf( "" ); - - if ( pos1 > -1 && pos2 > -1 ) - { - // Add the body value to a proper XML element, so that the strings get XML encoded (eg: ampersand is escaped). - final Element bodyEl = docFactory.createDocument().addElement("body"); - bodyEl.setText(body); - stanza = stanza.substring( 0, pos1 ) + bodyEl.asXML() + stanza.substring( pos2 + 7 ); - } + if (stanza != null) { + try { + final Document doc = DocumentHelper.parseText(stanza); + final Message message = new Message(doc.getRootElement()); + sid = StanzaIDUtil.findFirstUniqueAndStableStanzaID(message, owner.toBareJID()); + } catch (Exception e) { + Log.warn("An exception occurred while parsing message with ID {}", id, e); + sid = null; } - final Document doc = DocumentHelper.parseText( stanza ); - final Message message = new Message( doc.getRootElement() ); - sid = StanzaIDUtil.findFirstUniqueAndStableStanzaID( message, owner.toBareJID() ); - } catch ( Exception e ) { - Log.warn( "An exception occurred while parsing message with ID {}", id, e ); + } else { sid = null; } diff --git a/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java b/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java index 267716bb8..ce469cd8b 100644 --- a/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java +++ b/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java @@ -217,13 +217,15 @@ public static ArchivedMessage getArchivedMessage( long messageId, MUCRoom room ) } catch (SQLException ex) { Log.warn("SQL failure while trying to get message with ID {} from the archive of MUC room {}.", messageId, room, ex); return null; + } catch (DocumentException ex) { + Log.warn("Failure to parse 'stanza' value as XMPP for the message with ID {} from the archive of MUC room {}.", messageId, room, ex); + return null; } finally { DbConnectionManager.closeConnection(rs, pstmt, connection); } } - static protected ArchivedMessage asArchivedMessage(JID roomJID, String senderJID, String nickname, Date sentDate, String subject, String body, String stanza, long id) - { + static protected ArchivedMessage asArchivedMessage(JID roomJID, String senderJID, String nickname, Date sentDate, String subject, String body, String stanza, long id) throws DocumentException { if (stanza == null) { Message message = new Message(); message.setType(Message.Type.groupchat); @@ -242,29 +244,16 @@ static protected ArchivedMessage asArchivedMessage(JID roomJID, String senderJID } String sid; - try - { - if ( !JiveGlobals.getBooleanProperty( "conversation.OF-1804.disable", false ) ) - { - // Prior to OF-1804 (Openfire 4.4.0), the stanza was logged with a formatter applied. - // This causes message formatting to be modified (notably, new lines could be altered). - // This workaround restores the original body text, that was stored in a different column. - final int pos1 = stanza.indexOf( "" ); - final int pos2 = stanza.indexOf( "" ); - - if ( pos1 > -1 && pos2 > -1 ) - { - // Add the body value to a proper XML element, so that the strings get XML encoded (eg: ampersand is escaped). - final Element bodyEl = docFactory.createDocument().addElement("body"); - bodyEl.setText(body); - stanza = stanza.substring( 0, pos1 ) + bodyEl.asXML() + stanza.substring( pos2 + 7 ); - } + if (stanza != null) { + try { + final Document doc = DocumentHelper.parseText(stanza); + final Message message = new Message(doc.getRootElement()); + sid = StanzaIDUtil.findFirstUniqueAndStableStanzaID(message, roomJID.toBareJID()); + } catch (Exception e) { + Log.warn("An exception occurred while parsing message with ID {}", id, e); + sid = null; } - final Document doc = DocumentHelper.parseText( stanza ); - final Message message = new Message( doc.getRootElement() ); - sid = StanzaIDUtil.findFirstUniqueAndStableStanzaID( message, roomJID.toBareJID() ); - } catch ( Exception e ) { - Log.warn( "An exception occurred while parsing message with ID {}", id, e ); + } else { sid = null; } diff --git a/src/java/com/reucon/openfire/plugin/archive/impl/PaginatedMessageDatabaseQuery.java b/src/java/com/reucon/openfire/plugin/archive/impl/PaginatedMessageDatabaseQuery.java index 185281b30..5ccff30d2 100644 --- a/src/java/com/reucon/openfire/plugin/archive/impl/PaginatedMessageDatabaseQuery.java +++ b/src/java/com/reucon/openfire/plugin/archive/impl/PaginatedMessageDatabaseQuery.java @@ -15,6 +15,7 @@ package com.reucon.openfire.plugin.archive.impl; import com.reucon.openfire.plugin.archive.model.ArchivedMessage; +import org.dom4j.DocumentException; import org.jivesoftware.database.DbConnectionManager; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -134,7 +135,9 @@ protected List getPage( final Long after, final Long before, fi archivedMessages.add(archivedMessage); } } catch (SQLException e) { - Log.error("SQL failure during MAM: ", e); + Log.error("SQL failure during MAM for owner: {}", this.owner, e); + } catch (DocumentException e) { + Log.error("Unable to parse 'stanza' value as valid XMMP for owner {}", this.owner, e); } finally { DbConnectionManager.closeConnection(rs, pstmt, connection); } diff --git a/src/java/com/reucon/openfire/plugin/archive/impl/PaginatedMucMessageDatabaseQuery.java b/src/java/com/reucon/openfire/plugin/archive/impl/PaginatedMucMessageDatabaseQuery.java index 7f0717538..89a91f9f4 100644 --- a/src/java/com/reucon/openfire/plugin/archive/impl/PaginatedMucMessageDatabaseQuery.java +++ b/src/java/com/reucon/openfire/plugin/archive/impl/PaginatedMucMessageDatabaseQuery.java @@ -15,6 +15,7 @@ package com.reucon.openfire.plugin.archive.impl; import com.reucon.openfire.plugin.archive.model.ArchivedMessage; +import org.dom4j.DocumentException; import org.jivesoftware.database.DbConnectionManager; import org.jivesoftware.openfire.muc.MUCRoom; import org.jivesoftware.util.StringUtils; @@ -136,7 +137,9 @@ protected List getPage( final Long after, final Long before, fi msgs.add( MucMamPersistenceManager.asArchivedMessage(room.getJID(), senderJID, nickname, sentDate, subject, body, stanza, id) ); } } catch (SQLException e) { - Log.error("SQL failure during MAM-MUC: ", e); + Log.error("SQL failure during MAM-MUC for room: {}", this.room, e); + } catch (DocumentException e) { + Log.error("Unable to parse 'stanza' value as valid XMMP for MAM-MUC room {}", this.room, e); } finally { DbConnectionManager.closeConnection(rs, pstmt, connection); } diff --git a/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java b/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java index 8eb497705..f6b2f752e 100644 --- a/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java +++ b/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java @@ -1,7 +1,13 @@ package com.reucon.openfire.plugin.archive.model; +import org.dom4j.*; import org.jivesoftware.database.JiveID; +import org.jivesoftware.openfire.XMPPServer; +import org.jivesoftware.util.JiveGlobals; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.xmpp.packet.JID; +import org.xmpp.packet.Message; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -14,6 +20,9 @@ @JiveID(601) @Immutable public class ArchivedMessage { + + public static final Logger Log = LoggerFactory.getLogger( ArchivedMessage.class ); + public enum Direction { /** * A message sent by the owner. @@ -42,19 +51,33 @@ public enum Direction { private final JID with; @Nullable - private final String stanza; + private final Message stanza; @Nullable private final String stableId; - public ArchivedMessage(@Nullable final Long id, @Nonnull final Date time, @Nonnull final Direction direction, @Nullable final JID with, @Nullable final String stableId, @Nullable final String body, @Nullable final String stanza) { + public ArchivedMessage(@Nullable final Long id, @Nonnull final Date time, @Nonnull final Direction direction, @Nullable final JID with, @Nullable final String stableId, @Nullable final String body, @Nullable final String stanza) throws DocumentException { this.id = id; this.time = time; this.direction = direction; this.with = with; this.stableId = stableId; this.body = body; - this.stanza = stanza; + + if ( stanza != null) { + final Document doc = DocumentHelper.parseText( stanza ); + this.stanza = new Message( doc.getRootElement() ); + } else { + this.stanza = null; + } + + if ( this.stanza != null && !JiveGlobals.getBooleanProperty( "conversation.OF-1804.disable", false ) ) + { + // Prior to OF-1804 (Openfire 4.4.0), the stanza was logged with a formatter applied. + // This causes message formatting to be modified (notably, new lines could be altered). + // This workaround restores the original body text, that was stored in a different column. + this.stanza.setBody( body ); + } } /** @@ -107,7 +130,7 @@ public String getBody() { * @return XMPP packet */ @Nullable - public String getStanza() { + public Message getStanza() { return stanza; } @@ -126,7 +149,8 @@ public JID getWith() { } /** - * The first stable and unique stanza-id value in the stanza, if the stanza contains such a value. + * The first stable and unique stanza-id value in the stanza that was set by owner of the message archive, if the + * stanza contains such a value. * * @return a stable and unique stanza-id value. */ @@ -146,4 +170,54 @@ public String toString() { return sb.toString(); } + + /** + * When the archived message does not include the original stanza, this method can be used to recreate a stanza from + * the individual parts that did get persisted. + * + * Note that the result of this method will not include most of the metadata that would have been sent in the original + * stanza. When the original stanza is available, that should be preferred over the result of this method. This + * method explicitly does not return or use a stanza that is available in the archivedMessage argument, assuming + * that the caller has evaluated that, and choose (for whatever reason) to not use that (eg: it might be malformed?) + * + * When the archived message does not contain certain optional data (such as a body), this method cannot recreate a + * message stanza. In such cases, this method returns null. + * + * @param archivedMessage The archived message for which to recreate a stanza. + * @param archiveOwner The owner of the archive from which to recreate a stanza + * @return A recreated stanza. + */ + @Nullable + public static Message recreateStanza( @Nonnull final ArchivedMessage archivedMessage, @Nonnull final JID archiveOwner ) + { + if ( archivedMessage.getBody() == null || archivedMessage.getBody().isEmpty() ) { + Log.trace("Cannot reconstruct stanza for archived message ID {}, as it has no body.", archivedMessage.getId()); + return null; + } + + // Try creating a fake one from the body. + final JID to; + final JID from; + if (archivedMessage.getDirection() == ArchivedMessage.Direction.to) { + // message sent by the archive owner; + to = archivedMessage.getWith(); + from = archiveOwner; + } else { + // message received by the archive owner; + to = archiveOwner; + from = archivedMessage.getWith(); + } + + final boolean isMuc = (to != null && XMPPServer.getInstance().getMultiUserChatManager().getMultiUserChatService( to ) != null) + || (from != null && XMPPServer.getInstance().getMultiUserChatManager().getMultiUserChatService( from ) != null); + + final Message result = new Message(); + result.setFrom(from); + result.setTo(to); + result.setType( isMuc ? Message.Type.groupchat : Message.Type.chat ); + result.setBody(archivedMessage.getBody()); + + Log.trace( "Reconstructed stanza for archived message with ID {} (only a body was stored): {}", archivedMessage.getId(), result ); + return result; + } } diff --git a/src/java/com/reucon/openfire/plugin/archive/xep0313/IQQueryHandler.java b/src/java/com/reucon/openfire/plugin/archive/xep0313/IQQueryHandler.java index e96ac943c..df684f8e9 100644 --- a/src/java/com/reucon/openfire/plugin/archive/xep0313/IQQueryHandler.java +++ b/src/java/com/reucon/openfire/plugin/archive/xep0313/IQQueryHandler.java @@ -514,30 +514,17 @@ private void sendFinalMessage(JID from, final QueryRequest queryRequest) { * @return */ private void sendMessageResult(JID from, QueryRequest queryRequest, ArchivedMessage archivedMessage) { - String stanzaText = archivedMessage.getStanza(); - if(stanzaText == null || stanzaText.equals("")) { - // Try creating a fake one from the body. - if (archivedMessage.getBody() != null && !archivedMessage.getBody().equals("")) { - final JID to; - final JID frm; - if (archivedMessage.getDirection() == ArchivedMessage.Direction.to) { - // message sent by the archive owner; - to = archivedMessage.getWith(); - frm = queryRequest.getArchive(); - } else { - // message received by the archive owner; - to = queryRequest.getArchive(); - frm = archivedMessage.getWith(); - } - final boolean isMuc = (to != null &&XMPPServer.getInstance().getMultiUserChatManager().getMultiUserChatService( to ) != null) - || (from != null && XMPPServer.getInstance().getMultiUserChatManager().getMultiUserChatService( from ) != null); + final Message stanza; + if ( archivedMessage.getStanza() != null ) { + stanza = archivedMessage.getStanza(); + } else { + // Try create a fake on from the body. + stanza = ArchivedMessage.recreateStanza(archivedMessage, queryRequest.getArchive()); + } - stanzaText = String.format("%s", frm, to, isMuc ? "groupchat" : "chat", archivedMessage.getBody()); - Log.trace( "Reconstructed stanza (only a body was stored): {}", stanzaText ); - } else { - // Don't send legacy archived messages (that have no stanza) - return; - } + if (stanza == null) { + Log.debug("Unable to formulate a stanza from archived message with ID {}", archivedMessage.getId()); + return; } final boolean isMuc = XMPPServer.getInstance().getMultiUserChatManager().getMultiUserChatService( queryRequest.getArchive() ) != null; @@ -547,25 +534,26 @@ private void sendMessageResult(JID from, QueryRequest queryRequest, ArchivedMess { messagePacket.setFrom( queryRequest.getArchive().asBareJID() ); } - Forwarded fwd; - - Document stanza; - try { - stanza = DocumentHelper.parseText(stanzaText); - if ( isMuc ) { - // XEP-0313 specifies in section 5.1.2 MUC Archives: When sending out the archives to a requesting client, the forwarded stanza MUST NOT have a 'to' attribute. - final Attribute to = stanza.getRootElement().attribute("to"); - if (to != null) { - stanza.getRootElement().remove(to); - } + + final Element rootElement = stanza.getElement().createCopy(); + if ( isMuc ) { + // XEP-0313 specifies in section 5.1.2 MUC Archives: When sending out the archives to a requesting client, the forwarded stanza MUST NOT have a 'to' attribute. + final Attribute to = rootElement.attribute("to"); + if (to != null) { + rootElement.remove(to); } - fwd = new Forwarded(stanza.getRootElement(), archivedMessage.getTime(), null); - } catch (DocumentException e) { - Log.error("Failed to parse message stanza.", e); - // If we can't parse stanza then we have no message to send to client, abort - return; + } + final Forwarded fwd = new Forwarded(rootElement, archivedMessage.getTime(), null); + + if (archivedMessage.getId() == null) { + // TODO The MAM XEP specifies that there _must_ be an ID value in the result element. Traditionally, this + // code used the database ID. That introduces a weird dependency that a message must have a database ID + // before it can be used in results. In practise, that will probably always be the case, but it is not a + // particularly nice dependency. + throw new IllegalStateException("Unable to use an archived message that has no database ID."); } + // TODO Can/should we use a SSID instead of the database ID for the result 'ID' attribute value? messagePacket.addExtension(new Result(fwd, NAMESPACE, queryRequest.getQueryid(), archivedMessage.getId().toString())); router.route(messagePacket); } From 7160997b196d391c7e0cdd1fa5bd1d25aecb0227 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Wed, 2 Dec 2020 14:10:50 +0100 Subject: [PATCH 10/19] fixes #139: Refactor: centralize logic to determine 'direction' The concept of 'direction' of an ArchivedMessage is, to some, counter-intuitive. The mechanism that determines the direction of a particular stanza should be centralized to reduce confusion. --- .../archive/impl/JdbcPersistenceManager.java | 14 +++----------- .../plugin/archive/model/ArchivedMessage.java | 18 +++++++++++++++++- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/java/com/reucon/openfire/plugin/archive/impl/JdbcPersistenceManager.java b/src/java/com/reucon/openfire/plugin/archive/impl/JdbcPersistenceManager.java index 6687c7724..52df5e8ae 100644 --- a/src/java/com/reucon/openfire/plugin/archive/impl/JdbcPersistenceManager.java +++ b/src/java/com/reucon/openfire/plugin/archive/impl/JdbcPersistenceManager.java @@ -626,17 +626,9 @@ static protected ArchivedMessage asArchivedMessage(JID owner, String fromJID, St final JID from = new JID(fromJID + ( fromJIDResource == null || fromJIDResource.isEmpty() ? "" : "/" + fromJIDResource )); final JID to = new JID(toJID + ( toJIDResource == null || toJIDResource.isEmpty() ? "" : "/" + toJIDResource )); - final ArchivedMessage.Direction direction; - final JID with; - if (owner.asBareJID().equals(to.asBareJID())) { - direction = Direction.from; - with = from; - } else { - direction = Direction.to; - with = to; - } - final ArchivedMessage archivedMessage = new ArchivedMessage(id, sentDate, direction, with, sid, body, stanza); - return archivedMessage; + final ArchivedMessage.Direction direction = ArchivedMessage.Direction.getDirection(owner, to); + final JID with = direction == Direction.from ? from : to; + return new ArchivedMessage(id, sentDate, direction, with, sid, body, stanza); } private static Long dateToMillis(Date date) { diff --git a/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java b/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java index f6b2f752e..8f3a9f67c 100644 --- a/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java +++ b/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java @@ -32,7 +32,23 @@ public enum Direction { /** * A message received by the owner. */ - from + from; + + /** + * Returns a direction instance for a particular stanza, based on the owner of the archive that the stanza is + * in, and the addressee ('to') of the stanza. + * + * @param owner The owner of the archive that a message is in. + * @param addressee The addressee of the stanza. + * @return The direction of the stanza. + */ + public static Direction getDirection(@Nonnull final JID owner, @Nonnull final JID addressee) { + if (owner.asBareJID().equals(addressee.asBareJID())) { + return Direction.from; + } else { + return Direction.to; + } + } } @Nullable From 9c0934632655f56af0323bdaaf9c8e96421ee24e Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Wed, 2 Dec 2020 16:43:39 +0100 Subject: [PATCH 11/19] fixes #138: Process stanza in ConversationEvent When in 2015 the XEP-0313 implementation was added to the code, this API of ConversationManager got expanded to take in the complete 'stanza' element. The ConversationEvent did not have that, which is why an empty string was used instead. Later (in 2016), the stanza was made available in ConversationEvent, but that empty string never got replaced. This commit fixes that. --- .../org/jivesoftware/openfire/archive/ConversationEvent.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/java/org/jivesoftware/openfire/archive/ConversationEvent.java b/src/java/org/jivesoftware/openfire/archive/ConversationEvent.java index 3f144dcdf..d35c7130d 100644 --- a/src/java/org/jivesoftware/openfire/archive/ConversationEvent.java +++ b/src/java/org/jivesoftware/openfire/archive/ConversationEvent.java @@ -54,7 +54,7 @@ public ConversationEvent() { public void run(ConversationManager conversationManager) { if (Type.chatMessageReceived == type) { - conversationManager.processMessage(sender, receiver, body, "", date); + conversationManager.processMessage(sender, receiver, body, stanza, date); } else if (Type.roomDestroyed == type) { conversationManager.roomConversationEnded(roomJID, date); From bca04d30f26e37b0cf0e80e47c5002a812631442 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Wed, 2 Dec 2020 17:13:48 +0100 Subject: [PATCH 12/19] fixes #139: refactoring: repurposing 'with' in MUC context When processing an archived message in MAM context, using the 'with' attribute (that would otherwise go unused) to reflect the nickname of the sender of the message allows us to avoid unneeded stanza parsing. --- .../impl/MucMamPersistenceManager.java | 24 +++++++------------ .../plugin/archive/model/ArchivedMessage.java | 14 +++++------ .../archive/xep0136/IQRetrieveHandler.java | 4 +--- 3 files changed, 16 insertions(+), 26 deletions(-) diff --git a/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java b/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java index ce469cd8b..e59658ff1 100644 --- a/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java +++ b/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java @@ -226,21 +226,13 @@ public static ArchivedMessage getArchivedMessage( long messageId, MUCRoom room ) } static protected ArchivedMessage asArchivedMessage(JID roomJID, String senderJID, String nickname, Date sentDate, String subject, String body, String stanza, long id) throws DocumentException { - if (stanza == null) { - Message message = new Message(); - message.setType(Message.Type.groupchat); - message.setSubject(subject); - message.setBody(body); - // Set the sender of the message - if (nickname != null && nickname.trim().length() > 0) { - // Recreate the sender address based on the nickname and room's JID - message.setFrom(new JID(roomJID.getNode(), roomJID.getDomain(), nickname, true)); - } - else { - // Set the room as the sender of the message - message.setFrom(roomJID); - } - stanza = message.toString(); + final JID with; + if (nickname != null && nickname.trim().length() > 0) { + // Recreate the sender address based on the nickname and room's JID + with = new JID(roomJID.getNode(), roomJID.getDomain(), nickname, true); + } + else { + with = roomJID; } String sid; @@ -257,7 +249,7 @@ static protected ArchivedMessage asArchivedMessage(JID roomJID, String senderJID sid = null; } - final ArchivedMessage archivedMessage = new ArchivedMessage(id, sentDate, ArchivedMessage.Direction.from, null, sid, body, stanza); + final ArchivedMessage archivedMessage = new ArchivedMessage(id, sentDate, ArchivedMessage.Direction.from, with, sid, body, stanza); return archivedMessage; } diff --git a/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java b/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java index 8f3a9f67c..38d135f9b 100644 --- a/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java +++ b/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java @@ -63,7 +63,7 @@ public static Direction getDirection(@Nonnull final JID owner, @Nonnull final JI @Nullable private final String body; - @Nullable + @Nonnull private final JID with; @Nullable @@ -72,7 +72,7 @@ public static Direction getDirection(@Nonnull final JID owner, @Nonnull final JI @Nullable private final String stableId; - public ArchivedMessage(@Nullable final Long id, @Nonnull final Date time, @Nonnull final Direction direction, @Nullable final JID with, @Nullable final String stableId, @Nullable final String body, @Nullable final String stanza) throws DocumentException { + public ArchivedMessage(@Nullable final Long id, @Nonnull final Date time, @Nonnull final Direction direction, @Nonnull final JID with, @Nullable final String stableId, @Nullable final String body, @Nullable final String stanza) throws DocumentException { this.id = id; this.time = time; this.direction = direction; @@ -154,12 +154,12 @@ public Message getStanza() { * The message peer (the 'other side' of the conversation), in respect to the owner of the archive that this message * is part of. * - * This value will only have meaning when the original message was sent in a one-to-one conversation. When the - * archived message was originally shared in a group chat, this value can be null. + * When the archived message was originally shared in a group chat, this value will reference the in-room address + * of the participant that sent the message (eg: room@service/nickname). * * @return The conversation peer. */ - @Nullable + @Nonnull public JID getWith() { return with; } @@ -224,8 +224,8 @@ public static Message recreateStanza( @Nonnull final ArchivedMessage archivedMes from = archivedMessage.getWith(); } - final boolean isMuc = (to != null && XMPPServer.getInstance().getMultiUserChatManager().getMultiUserChatService( to ) != null) - || (from != null && XMPPServer.getInstance().getMultiUserChatManager().getMultiUserChatService( from ) != null); + final boolean isMuc = XMPPServer.getInstance().getMultiUserChatManager().getMultiUserChatService( to ) != null + || XMPPServer.getInstance().getMultiUserChatManager().getMultiUserChatService( from ) != null; final Message result = new Message(); result.setFrom(from); diff --git a/src/java/com/reucon/openfire/plugin/archive/xep0136/IQRetrieveHandler.java b/src/java/com/reucon/openfire/plugin/archive/xep0136/IQRetrieveHandler.java index b8fb22d01..c3b5b983c 100644 --- a/src/java/com/reucon/openfire/plugin/archive/xep0136/IQRetrieveHandler.java +++ b/src/java/com/reucon/openfire/plugin/archive/xep0136/IQRetrieveHandler.java @@ -122,9 +122,7 @@ private Element addMessageElement(Element parentElement, messageElement = parentElement.addElement(message.getDirection() .toString()); messageElement.addAttribute("secs", Long.toString(secs)); - if (message.getWith() != null) { - messageElement.addAttribute("jid", message.getWith().toBareJID()); - } + messageElement.addAttribute("jid", message.getWith().toBareJID()); messageElement.addElement("body").setText(message.getBody()); return messageElement; From 2bfc5a5ef92c2f0cdb8f4738fbef98c582afb0fa Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Wed, 2 Dec 2020 17:22:50 +0100 Subject: [PATCH 13/19] fixes #139: Reduce code duplication for Stable/Unique stanza IDs Code duplication (and thus complexity) is reduced by centralizing where an appropriate stanza ID is obtained. --- .../archive/impl/JdbcPersistenceManager.java | 20 +++--------------- .../impl/MucMamPersistenceManager.java | 21 +++---------------- .../plugin/archive/model/ArchivedMessage.java | 21 ++++++++++++------- 3 files changed, 19 insertions(+), 43 deletions(-) diff --git a/src/java/com/reucon/openfire/plugin/archive/impl/JdbcPersistenceManager.java b/src/java/com/reucon/openfire/plugin/archive/impl/JdbcPersistenceManager.java index 52df5e8ae..33a7275a4 100644 --- a/src/java/com/reucon/openfire/plugin/archive/impl/JdbcPersistenceManager.java +++ b/src/java/com/reucon/openfire/plugin/archive/impl/JdbcPersistenceManager.java @@ -353,14 +353,14 @@ public Collection findMessages(Date startDate, Date endDate, JI final String first; final String last; if ( useStableID ) { - final String firstSid = firstMessage.getStableId(); + final String firstSid = firstMessage.getStableId(owner); if ( firstSid != null && !firstSid.isEmpty() ) { first = firstSid; } else { // Issue #98: Fall-back to using the database-identifier. Although not a stable-id, it at least gives the client the option to paginate. first = firstMessage.getId().toString(); } - final String lastSid = lastMessage.getStableId(); + final String lastSid = lastMessage.getStableId(owner); if ( lastSid != null && !lastSid.isEmpty()) { last = lastSid; } else { @@ -609,26 +609,12 @@ public static ArchivedMessage getArchivedMessage( long messageId, JID owner ) } static protected ArchivedMessage asArchivedMessage(JID owner, String fromJID, String fromJIDResource, String toJID, String toJIDResource, Date sentDate, String body, String stanza, Long id) throws DocumentException { - String sid; - if (stanza != null) { - try { - final Document doc = DocumentHelper.parseText(stanza); - final Message message = new Message(doc.getRootElement()); - sid = StanzaIDUtil.findFirstUniqueAndStableStanzaID(message, owner.toBareJID()); - } catch (Exception e) { - Log.warn("An exception occurred while parsing message with ID {}", id, e); - sid = null; - } - } else { - sid = null; - } - final JID from = new JID(fromJID + ( fromJIDResource == null || fromJIDResource.isEmpty() ? "" : "/" + fromJIDResource )); final JID to = new JID(toJID + ( toJIDResource == null || toJIDResource.isEmpty() ? "" : "/" + toJIDResource )); final ArchivedMessage.Direction direction = ArchivedMessage.Direction.getDirection(owner, to); final JID with = direction == Direction.from ? from : to; - return new ArchivedMessage(id, sentDate, direction, with, sid, body, stanza); + return new ArchivedMessage(id, sentDate, direction, with, body, stanza); } private static Long dateToMillis(Date date) { diff --git a/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java b/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java index e59658ff1..951f22bd1 100644 --- a/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java +++ b/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java @@ -100,14 +100,14 @@ public Collection findMessages(Date startDate, Date endDate, JI final String first; final String last; if ( useStableID ) { - final String firstSid = firstMessage.getStableId(); + final String firstSid = firstMessage.getStableId(owner); if ( firstSid != null && !firstSid.isEmpty() ) { first = firstSid; } else { // Issue #98: Fall-back to using the database-identifier. Although not a stable-id, it at least gives the client the option to paginate. first = firstMessage.getId().toString(); } - final String lastSid = lastMessage.getStableId(); + final String lastSid = lastMessage.getStableId(owner); if ( lastSid != null && !lastSid.isEmpty()) { last = lastSid; } else { @@ -235,22 +235,7 @@ static protected ArchivedMessage asArchivedMessage(JID roomJID, String senderJID with = roomJID; } - String sid; - if (stanza != null) { - try { - final Document doc = DocumentHelper.parseText(stanza); - final Message message = new Message(doc.getRootElement()); - sid = StanzaIDUtil.findFirstUniqueAndStableStanzaID(message, roomJID.toBareJID()); - } catch (Exception e) { - Log.warn("An exception occurred while parsing message with ID {}", id, e); - sid = null; - } - } else { - sid = null; - } - - final ArchivedMessage archivedMessage = new ArchivedMessage(id, sentDate, ArchivedMessage.Direction.from, with, sid, body, stanza); - return archivedMessage; + return new ArchivedMessage(id, sentDate, ArchivedMessage.Direction.from, with, body, stanza); } static Long parseIdentifier( String value, MUCRoom room, boolean useStableID ) diff --git a/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java b/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java index 38d135f9b..1b0f0bb17 100644 --- a/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java +++ b/src/java/com/reucon/openfire/plugin/archive/model/ArchivedMessage.java @@ -1,5 +1,6 @@ package com.reucon.openfire.plugin.archive.model; +import com.reucon.openfire.plugin.archive.util.StanzaIDUtil; import org.dom4j.*; import org.jivesoftware.database.JiveID; import org.jivesoftware.openfire.XMPPServer; @@ -69,15 +70,11 @@ public static Direction getDirection(@Nonnull final JID owner, @Nonnull final JI @Nullable private final Message stanza; - @Nullable - private final String stableId; - - public ArchivedMessage(@Nullable final Long id, @Nonnull final Date time, @Nonnull final Direction direction, @Nonnull final JID with, @Nullable final String stableId, @Nullable final String body, @Nullable final String stanza) throws DocumentException { + public ArchivedMessage(@Nullable final Long id, @Nonnull final Date time, @Nonnull final Direction direction, @Nonnull final JID with, @Nullable final String body, @Nullable final String stanza) throws DocumentException { this.id = id; this.time = time; this.direction = direction; this.with = with; - this.stableId = stableId; this.body = body; if ( stanza != null) { @@ -171,16 +168,24 @@ public JID getWith() { * @return a stable and unique stanza-id value. */ @Nullable - public String getStableId() + public String getStableId(final JID owner) { - return stableId; + if (this.stanza == null) { + return null; + } + + try { + return StanzaIDUtil.findFirstUniqueAndStableStanzaID(this.stanza, owner.toBareJID()); + } catch (Exception e) { + Log.warn("An exception occurred while parsing message with ID {}", id, e); + return null; + } } public String toString() { StringBuilder sb = new StringBuilder(); sb.append("ArchivedMessage[id=").append(id).append(","); - sb.append("stableId=").append(stableId).append(","); sb.append("time=").append(time).append(","); sb.append("direction=").append(direction).append("]"); From 36f58d58337469ac17f80f518e15c4ee4d09de2a Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Wed, 2 Dec 2020 17:38:59 +0100 Subject: [PATCH 14/19] Add recent changes to changelog --- changelog.html | 2 ++ 1 file changed, 2 insertions(+) diff --git a/changelog.html b/changelog.html index c0014e157..00e0aa783 100644 --- a/changelog.html +++ b/changelog.html @@ -47,6 +47,8 @@

    2.2.0 -- (tbd)

    • [Issue #137] - MUC messages duplicated as one-on-one messages
      • +
    • [Issue #138] - Stanzas not always stored for one-to-one messages whilst clustered
      • +
    • [Issue #139] - Reduce code complexity

    2.1.0 -- September 10, 2020

    From 4a371141c979ec575262202c4542802533657373 Mon Sep 17 00:00:00 2001 From: Dan Caseley Date: Thu, 3 Dec 2020 16:05:08 +0000 Subject: [PATCH 15/19] fixes #138: Actually add the 1-to-1 stanzas to the ConversationEvent --- .../openfire/archive/ArchiveInterceptor.java | 1 + .../openfire/archive/ConversationEvent.java | 3 ++- .../archive/GroupConversationInterceptor.java | 21 ++++++++++--------- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/java/org/jivesoftware/openfire/archive/ArchiveInterceptor.java b/src/java/org/jivesoftware/openfire/archive/ArchiveInterceptor.java index 24a506b1f..c199fe17b 100644 --- a/src/java/org/jivesoftware/openfire/archive/ArchiveInterceptor.java +++ b/src/java/org/jivesoftware/openfire/archive/ArchiveInterceptor.java @@ -78,6 +78,7 @@ public void interceptPacket(Packet packet, Session session, boolean incoming, bo eventsQueue.addChatEvent(conversationManager.getConversationKey(sender, receiver), ConversationEvent.chatMessageReceived(sender, receiver, conversationManager.isMessageArchivingEnabled() ? message.getBody() : null, + conversationManager.isMessageArchivingEnabled() ? message.toXML() : null, new Date())); } } diff --git a/src/java/org/jivesoftware/openfire/archive/ConversationEvent.java b/src/java/org/jivesoftware/openfire/archive/ConversationEvent.java index d35c7130d..3f762986c 100644 --- a/src/java/org/jivesoftware/openfire/archive/ConversationEvent.java +++ b/src/java/org/jivesoftware/openfire/archive/ConversationEvent.java @@ -141,12 +141,13 @@ public void readExternal(ObjectInput in) throws IOException, ClassNotFoundExcept } } - public static ConversationEvent chatMessageReceived(JID sender, JID receiver, String body, Date date) { + public static ConversationEvent chatMessageReceived(JID sender, JID receiver, String body, String stanza, Date date) { ConversationEvent event = new ConversationEvent(); event.type = Type.chatMessageReceived; event.sender = sender; event.receiver = receiver; event.body = body; + event.stanza = stanza; event.date = date; return event; } diff --git a/src/java/org/jivesoftware/openfire/archive/GroupConversationInterceptor.java b/src/java/org/jivesoftware/openfire/archive/GroupConversationInterceptor.java index 6dcea8622..54d01f1a2 100644 --- a/src/java/org/jivesoftware/openfire/archive/GroupConversationInterceptor.java +++ b/src/java/org/jivesoftware/openfire/archive/GroupConversationInterceptor.java @@ -124,17 +124,18 @@ public void messageReceived(JID roomJID, JID user, String nickname, Message mess public void privateMessageRecieved(JID toJID, JID fromJID, Message message) { if(message.getBody() != null) { - if (ClusterManager.isSeniorClusterMember()) { - conversationManager.processMessage(fromJID, toJID, message.getBody(), message.toXML(), new Date()); - } - else { - ConversationEventsQueue eventsQueue = conversationManager.getConversationEventsQueue(); - eventsQueue.addChatEvent(conversationManager.getConversationKey(fromJID, toJID), - ConversationEvent.chatMessageReceived(toJID, fromJID, - conversationManager.isMessageArchivingEnabled() ? message.getBody() : null, - new Date())); + if (ClusterManager.isSeniorClusterMember()) { + conversationManager.processMessage(fromJID, toJID, message.getBody(), message.toXML(), new Date()); + } + else { + ConversationEventsQueue eventsQueue = conversationManager.getConversationEventsQueue(); + eventsQueue.addChatEvent(conversationManager.getConversationKey(fromJID, toJID), + ConversationEvent.chatMessageReceived(toJID, fromJID, + conversationManager.isMessageArchivingEnabled() ? message.getBody() : null, + conversationManager.isMessageArchivingEnabled() ? message.toXML() : null, + new Date())); } - } + } } public void roomSubjectChanged(JID roomJID, JID user, String newSubject) { From 12471461365bef4ad0426a265c73a3befd86d53e Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Fri, 4 Dec 2020 11:12:09 +0100 Subject: [PATCH 16/19] OF-2157 / #137: Ensure correct ordering of messages in cluster It is important to assign a message ID, which is used for ordering messages in a conversation, soon after the message arrived, as opposed to just before the message gets written to the database. In the latter scenario, the message ID values might no longer reflect the order of the messages in a conversation, as database writes are batched up together for performance reasons. Using these batches won't affect the database-insertion order (as compared to the order of messages in the conversation) on a single Openfire server, but when running in a cluster, these batches do have a good chance to mess up the order of things. --- changelog.html | 1 + .../impl/MucMamPersistenceManager.java | 12 ++++-- .../PaginatedMucMessageDatabaseQuery.java | 3 ++ .../openfire/archive/ArchivedMessage.java | 41 ++++++++++++++----- .../openfire/archive/ConversationManager.java | 4 +- 5 files changed, 43 insertions(+), 18 deletions(-) diff --git a/changelog.html b/changelog.html index 00e0aa783..e86954346 100644 --- a/changelog.html +++ b/changelog.html @@ -46,6 +46,7 @@

    2.2.0 -- (tbd)

      +
    • [OF-2157] - SequenceManager generated IDs are unreliable whilst clustering.
    • [Issue #137] - MUC messages duplicated as one-on-one messages
    • [Issue #138] - Stanzas not always stored for one-to-one messages whilst clustered
    • [Issue #139] - Reduce code complexity
      • diff --git a/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java b/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java index 951f22bd1..3396ab8a4 100644 --- a/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java +++ b/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java @@ -5,13 +5,14 @@ import com.reucon.openfire.plugin.archive.model.Conversation; import com.reucon.openfire.plugin.archive.util.StanzaIDUtil; import com.reucon.openfire.plugin.archive.xep0059.XmppResultSet; -import org.dom4j.*; +import org.dom4j.Document; +import org.dom4j.DocumentException; +import org.dom4j.DocumentHelper; import org.jivesoftware.database.DbConnectionManager; import org.jivesoftware.openfire.XMPPServer; import org.jivesoftware.openfire.muc.MUCRoom; import org.jivesoftware.openfire.muc.MultiUserChatManager; import org.jivesoftware.openfire.muc.MultiUserChatService; -import org.jivesoftware.util.JiveGlobals; import org.jivesoftware.util.NotFoundException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -29,11 +30,15 @@ import java.util.List; /** + * A persistence provider that facilitates the implementation of Message Archive Management (XEP-0313) for MUC rooms. + * + * Note that this implementation primarily makes use of the database tables that are provided by Openfire (core), and + * not of the database tables that are provided by the Monitoring plugin. + * * Created by dwd on 25/07/16. */ public class MucMamPersistenceManager implements PersistenceManager { private final static Logger Log = LoggerFactory.getLogger( MucMamPersistenceManager.class ); - protected static final DocumentFactory docFactory = DocumentFactory.getInstance(); private static final int DEFAULT_MAX = 100; @Override @@ -355,7 +360,6 @@ public static Instant getDateOfLastLog( MUCRoom room ) PreparedStatement pstmt = null; ResultSet rs = null; - int totalCount = 0; try { connection = DbConnectionManager.getConnection(); pstmt = connection.prepareStatement( "SELECT MAX(logTime) FROM ofMucConversationLog WHERE roomid = ?"); diff --git a/src/java/com/reucon/openfire/plugin/archive/impl/PaginatedMucMessageDatabaseQuery.java b/src/java/com/reucon/openfire/plugin/archive/impl/PaginatedMucMessageDatabaseQuery.java index 89a91f9f4..1debc2d2c 100644 --- a/src/java/com/reucon/openfire/plugin/archive/impl/PaginatedMucMessageDatabaseQuery.java +++ b/src/java/com/reucon/openfire/plugin/archive/impl/PaginatedMucMessageDatabaseQuery.java @@ -35,6 +35,9 @@ /** * Encapsulates responsibility of creating a database query that retrieves a specific subset (page) of archived messages related to a MUC room. * + * Note that this implementation primarily makes use of the database tables that are provided by Openfire (core), and + * not of the database tables that are provided by the Monitoring plugin. + * * @author Guus der Kinderen, guus.der.kinderen@gmail.com */ public class PaginatedMucMessageDatabaseQuery diff --git a/src/java/org/jivesoftware/openfire/archive/ArchivedMessage.java b/src/java/org/jivesoftware/openfire/archive/ArchivedMessage.java index a9523ab97..651050801 100644 --- a/src/java/org/jivesoftware/openfire/archive/ArchivedMessage.java +++ b/src/java/org/jivesoftware/openfire/archive/ArchivedMessage.java @@ -35,13 +35,14 @@ public class ArchivedMessage { new SequenceManager(604, 50); } - private long conversationID; - private JID fromJID; - private JID toJID; - private Date sentDate; - private String body; - private String stanza; - private boolean roomEvent; + private final long conversationID; + private final JID fromJID; + private final JID toJID; + private final Date sentDate; + private final String body; + private final String stanza; + private final boolean roomEvent; + private final long id; /** * Creates a new archived message. @@ -54,6 +55,17 @@ public class ArchivedMessage { * @param roomEvent true if the message belongs to a room event. Eg. User joined room. */ public ArchivedMessage(long conversationID, JID fromJID, JID toJID, Date sentDate, String body, boolean roomEvent) { + this(conversationID, fromJID, toJID, sentDate, body, null, roomEvent); + } + + public ArchivedMessage(long conversationID, JID fromJID, JID toJID, Date sentDate, String body, String stanza, boolean roomEvent) { + // OF-2157: It is important to assign a message ID, which is used for ordering messages in a conversation, soon + // after the message arrived, as opposed to just before the message gets written to the database. In the latter + // scenario, the message ID values might no longer reflect the order of the messages in a conversation, as + // database writes are batched up together for performance reasons. Using these batches won't affect the + // database-insertion order (as compared to the order of messages in the conversation) on a single Openfire + // server, but when running in a cluster, these batches do have a good chance to mess up the order of things. + this.id = SequenceManager.nextID(this); this.conversationID = conversationID; // Convert both JID's to bare JID's so that we don't store resource information. this.fromJID = fromJID; @@ -61,10 +73,6 @@ public ArchivedMessage(long conversationID, JID fromJID, JID toJID, Date sentDat this.sentDate = sentDate; this.body = body; this.roomEvent = roomEvent; - } - - public ArchivedMessage(long conversationID, JID fromJID, JID toJID, Date sentDate, String body, String stanza, boolean roomEvent) { - this(conversationID, fromJID, toJID, sentDate, body, roomEvent); this.stanza = stanza; } @@ -77,6 +85,17 @@ public long getConversationID() { return conversationID; } + /** + * The ID that the message is associated with. + * + * This value is used to order messages in a conversation. + * + * @return the conversation ID. + */ + public long getID() { + return id; + } + /** * The JID of the user that sent the message. * diff --git a/src/java/org/jivesoftware/openfire/archive/ConversationManager.java b/src/java/org/jivesoftware/openfire/archive/ConversationManager.java index da3141652..67e89d3f6 100644 --- a/src/java/org/jivesoftware/openfire/archive/ConversationManager.java +++ b/src/java/org/jivesoftware/openfire/archive/ConversationManager.java @@ -1163,9 +1163,7 @@ protected void store( List workQueue ) for ( final ArchivedMessage work : workQueue ) { - long id = SequenceManager.nextID(work); - - pstmt.setLong(1, id); + pstmt.setLong(1, work.getID()); pstmt.setLong(2, work.getConversationID()); pstmt.setString(3, work.getFromJID().toBareJID()); pstmt.setString(4, work.getFromJID().getResource()); From f98ea09c8d5ae791e62a76a7ac08be60f3c660ba Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Fri, 4 Dec 2020 11:56:59 +0100 Subject: [PATCH 17/19] fixes #141: Make Archiver config configurable These are the new properties and their defaults: conversation.archiver.conversation.max-work-queue-size default: 500 conversation.archiver.conversation.max-purge-interval default: 1000 (one second) conversation.archiver.conversation.grace-period default: 50 conversation.archiver.message.max-work-queue-size default: 500 conversation.archiver.message.max-purge-interval default: 1000 (one second) conversation.archiver.message.grace-period default: 50 conversation.archiver.participant.max-work-queue-size default: 500 conversation.archiver.participant.max-purge-interval default: 1000 (one second) conversation.archiver.participant.grace-period default: 50 --- changelog.html | 1 + .../openfire/archive/ConversationManager.java | 20 +++++++++++++++---- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/changelog.html b/changelog.html index e86954346..467fff9a9 100644 --- a/changelog.html +++ b/changelog.html @@ -50,6 +50,7 @@

    • [Issue #137] - MUC messages duplicated as one-on-one messages
    • [Issue #138] - Stanzas not always stored for one-to-one messages whilst clustered
    • [Issue #139] - Reduce code complexity
      • +
    • [Issue #141] - Make Archiver configuration configurable

    2.1.0 -- September 10, 2020

    diff --git a/src/java/org/jivesoftware/openfire/archive/ConversationManager.java b/src/java/org/jivesoftware/openfire/archive/ConversationManager.java index 67e89d3f6..1d4c2ff9f 100644 --- a/src/java/org/jivesoftware/openfire/archive/ConversationManager.java +++ b/src/java/org/jivesoftware/openfire/archive/ConversationManager.java @@ -1086,7 +1086,11 @@ private static class ConversationArchivingRunnable extends Archiver workQueue ) @@ -1138,11 +1142,15 @@ protected void store( List workQueue ) /** * Stores Messages in the database. */ - private class MessageArchivingRunnable extends Archiver + private static class MessageArchivingRunnable extends Archiver { MessageArchivingRunnable( String id ) { - super( id, 500, Duration.ofSeconds( 1 ), Duration.ofMillis( 50 ) ); // TODO make values configurable. + super( id, + JiveGlobals.getIntProperty("conversation.archiver.message.max-work-queue-size", 500), + Duration.ofMillis( JiveGlobals.getLongProperty("conversation.archiver.message.max-purge-interval", 1000)), + Duration.ofMillis( JiveGlobals.getLongProperty("conversation.archiver.message.grace-period", 50)) + ); } @Override @@ -1206,7 +1214,11 @@ private static class ParticipantArchivingRunnable extends Archiver workQueue ) From 250b6e93c466f3f2eb848fb18c212b24cf22fda2 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Fri, 4 Dec 2020 13:04:56 +0100 Subject: [PATCH 18/19] fixes #19: Adding issue 19 to changelog (duplicate of #137) With issue #137 fixed, issue #19 can be closed. --- changelog.html | 1 + 1 file changed, 1 insertion(+) diff --git a/changelog.html b/changelog.html index 467fff9a9..a34e28f7c 100644 --- a/changelog.html +++ b/changelog.html @@ -47,6 +47,7 @@

    2.2.0 -- (tbd)

    • [OF-2157] - SequenceManager generated IDs are unreliable whilst clustering.
      • +
    • [Issue #19] - Monitoring / Archive plugin fails to reconstruct archived stanza
    • [Issue #137] - MUC messages duplicated as one-on-one messages
    • [Issue #138] - Stanzas not always stored for one-to-one messages whilst clustered
    • [Issue #139] - Reduce code complexity
      • From 5822cda2d725c4be2771812066d72f56d78adcb9 Mon Sep 17 00:00:00 2001 From: Dan Caseley Date: Fri, 4 Dec 2020 22:12:18 +0000 Subject: [PATCH 19/19] fixes #139: Fix MAM queries using before/after the result ID The implementation looks for a stanza-id and falls back to a database ID. The XEP-0313 says a client shouldn't rely on the stanza-id as the UID of a message in the archive, but OF is trending towards making that reliable. In the meantime, the result ID is still the database ID. Without this change, searches for the database ID is searched as freetext within the stanza. As a short number in amongst many GUIDs, false positives are more likely than not. This change adds to the defense against false positives and ensures that a search for a stanza-id is more thoroughly checked to be the one we were looking for. --- .../openfire/plugin/archive/impl/MucMamPersistenceManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java b/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java index 3396ab8a4..e4dd26a69 100644 --- a/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java +++ b/src/java/com/reucon/openfire/plugin/archive/impl/MucMamPersistenceManager.java @@ -303,7 +303,7 @@ static Long getMessageIdForStableId( final MUCRoom room, final String value ) final Document doc = DocumentHelper.parseText( stanza ); final Message message = new Message( doc.getRootElement() ); final String sid = StanzaIDUtil.findFirstUniqueAndStableStanzaID( message, room.getJID().toBareJID() ); - if ( sid != null ) { + if ( sid != null && sid.equals(value)) { Log.debug( "Found stable/unique stanza ID {} in message with ID {}.", value, messageId ); return messageId; }