rethink mam catchup strategies

Daniel Gultsch created

Change summary

src/main/java/eu/siacs/conversations/Config.java                         |  4 
src/main/java/eu/siacs/conversations/parser/MessageParser.java           |  7 
src/main/java/eu/siacs/conversations/services/MessageArchiveService.java | 49 
src/main/java/eu/siacs/conversations/services/XmppConnectionService.java | 19 
src/main/java/eu/siacs/conversations/ui/StartConversationActivity.java   | 19 
5 files changed, 58 insertions(+), 40 deletions(-)

Detailed changes

src/main/java/eu/siacs/conversations/Config.java πŸ”—

@@ -106,8 +106,8 @@ public final class Config {
 
 	public static final boolean PARSE_REAL_JID_FROM_MUC_MAM = false; //dangerous if server doesn’t filter
 
-	public static final long MAM_MAX_CATCHUP =  MILLISECONDS_IN_DAY / 2;
-	public static final int MAM_MAX_MESSAGES = 500;
+	public static final long MAM_MAX_CATCHUP =  MILLISECONDS_IN_DAY * 5;
+	public static final int MAM_MAX_MESSAGES = 750;
 
 	public static final long FREQUENT_RESTARTS_DETECTION_WINDOW = 12 * 60 * 60 * 1000; // 10 hours
 	public static final long FREQUENT_RESTARTS_THRESHOLD = 0; // previous value was 16;

src/main/java/eu/siacs/conversations/parser/MessageParser.java πŸ”—

@@ -382,7 +382,7 @@ public class MessageParser extends AbstractParser implements OnMessagePacketRece
 		}
 
 		if ((body != null || pgpEncrypted != null || axolotlEncrypted != null) && !isMucStatusMessage) {
-			Conversation conversation = mXmppConnectionService.findOrCreateConversation(account, counterpart.toBareJid(), isTypeGroupChat, query);
+			Conversation conversation = mXmppConnectionService.findOrCreateConversation(account, counterpart.toBareJid(), isTypeGroupChat, false, query);
 			final boolean conversationMultiMode = conversation.getMode() == Conversation.MODE_MULTI;
 			if (isTypeGroupChat) {
 				if (counterpart.getResourcepart().equals(conversation.getMucOptions().getActualNick())) {
@@ -540,8 +540,11 @@ public class MessageParser extends AbstractParser implements OnMessagePacketRece
 			} else {
 				conversation.add(message);
 			}
+			if (query != null) {
+				query.incrementActualMessageCount();
+			}
 
-			if (query == null || query.getWith() == null) { //either no mam or catchup
+			if (query == null || !query.isCatchup()) { //either no mam or catchup
 				if (status == Message.STATUS_SEND || status == Message.STATUS_SEND_RECEIVED) {
 					mXmppConnectionService.markRead(conversation);
 					if (query == null) {

src/main/java/eu/siacs/conversations/services/MessageArchiveService.java πŸ”—

@@ -108,12 +108,20 @@ public class MessageArchiveService implements OnAdvancedStreamFeaturesLoaded {
 
 	public Query query(Conversation conversation, long start, long end) {
 		synchronized (this.queries) {
-			final Query query = new Query(conversation, start, end,PagingOrder.REVERSE);
+			final Query query;
+			final long startActual = Math.max(start,mXmppConnectionService.getAutomaticMessageDeletionDate());
 			if (start==0) {
+				query = new Query(conversation, startActual, end, false);
 				query.reference = conversation.getFirstMamReference();
-				Log.d(Config.LOGTAG,"setting mam reference");
+			} else {
+				long maxCatchup = Math.max(startActual,System.currentTimeMillis() - Config.MAM_MAX_CATCHUP);
+				if (maxCatchup > startActual) {
+					Query reverseCatchup = new Query(conversation,startActual,maxCatchup,false);
+					this.queries.add(reverseCatchup);
+					this.execute(reverseCatchup);
+				}
+				query = new Query(conversation, maxCatchup, end);
 			}
-			query.start = Math.max(start,mXmppConnectionService.getAutomaticMessageDeletionDate());
 			if (start > end) {
 				return null;
 			}
@@ -220,15 +228,15 @@ public class MessageArchiveService implements OnAdvancedStreamFeaturesLoaded {
 		Element last = set == null ? null : set.findChild("last");
 		Element first = set == null ? null : set.findChild("first");
 		Element relevant = query.getPagingOrder() == PagingOrder.NORMAL ? last : first;
-		boolean abort = (query.getStart() == 0 && query.getTotalCount() >= Config.PAGE_SIZE) || query.getTotalCount() >= Config.MAM_MAX_MESSAGES;
+		boolean abort = (!query.isCatchup() && query.getTotalCount() >= Config.PAGE_SIZE) || query.getTotalCount() >= Config.MAM_MAX_MESSAGES;
 		if (query.getConversation() != null) {
 			query.getConversation().setFirstMamReference(first == null ? null : first.getContent());
 		}
 		if (complete || relevant == null || abort) {
-			final boolean done = (complete || query.getMessageCount() == 0) && query.getStart() <= mXmppConnectionService.getAutomaticMessageDeletionDate();
+			final boolean done = (complete || query.getActualMessageCount() == 0) && !query.isCatchup();
 			this.finalizeQuery(query, done);
-			Log.d(Config.LOGTAG,query.getAccount().getJid().toBareJid()+": finished mam after "+query.getTotalCount()+" messages. messages left="+Boolean.toString(!done));
-			if (query.getWith() == null && query.getMessageCount() > 0) {
+			Log.d(Config.LOGTAG,query.getAccount().getJid().toBareJid()+": finished mam after "+query.getTotalCount()+"("+query.getActualMessageCount()+") messages. messages left="+Boolean.toString(!done));
+			if (query.getWith() == null && query.getActualMessageCount() > 0) {
 				mXmppConnectionService.getNotificationService().finishBacklog(true,query.getAccount());
 			}
 		} else {
@@ -269,7 +277,7 @@ public class MessageArchiveService implements OnAdvancedStreamFeaturesLoaded {
 
 	public class Query {
 		private int totalCount = 0;
-		private int messageCount = 0;
+		private int actualCount = 0;
 		private long start;
 		private long end;
 		private String queryId;
@@ -278,6 +286,7 @@ public class MessageArchiveService implements OnAdvancedStreamFeaturesLoaded {
 		private Conversation conversation;
 		private PagingOrder pagingOrder = PagingOrder.NORMAL;
 		private XmppConnectionService.OnMoreMessagesLoaded callback = null;
+		private boolean catchup = true;
 
 
 		public Query(Conversation conversation, long start, long end) {
@@ -285,9 +294,10 @@ public class MessageArchiveService implements OnAdvancedStreamFeaturesLoaded {
 			this.conversation = conversation;
 		}
 
-		public Query(Conversation conversation, long start, long end, PagingOrder order) {
+		public Query(Conversation conversation, long start, long end, boolean catchup) {
 			this(conversation,start,end);
-			this.pagingOrder = order;
+			this.pagingOrder = catchup ? PagingOrder.NORMAL : PagingOrder.REVERSE;
+			this.catchup = catchup;
 		}
 
 		public Query(Account account, long start, long end) {
@@ -302,7 +312,9 @@ public class MessageArchiveService implements OnAdvancedStreamFeaturesLoaded {
 			query.reference = reference;
 			query.conversation = conversation;
 			query.totalCount = totalCount;
+			query.actualCount = actualCount;
 			query.callback = callback;
+			query.catchup = catchup;
 			return query;
 		}
 
@@ -342,13 +354,17 @@ public class MessageArchiveService implements OnAdvancedStreamFeaturesLoaded {
 			return start;
 		}
 
+		public boolean isCatchup() {
+			return catchup;
+		}
+
 		public void setCallback(XmppConnectionService.OnMoreMessagesLoaded callback) {
 			this.callback = callback;
 		}
 
 		public void callback(boolean done) {
 			if (this.callback != null) {
-				this.callback.onMoreMessagesLoaded(messageCount,conversation);
+				this.callback.onMoreMessagesLoaded(actualCount,conversation);
 				if (done) {
 					this.callback.informUser(R.string.no_more_history_on_server);
 				}
@@ -368,16 +384,19 @@ public class MessageArchiveService implements OnAdvancedStreamFeaturesLoaded {
 		}
 
 		public void incrementMessageCount() {
-			this.messageCount++;
 			this.totalCount++;
 		}
 
+		public void incrementActualMessageCount() {
+			this.actualCount++;
+		}
+
 		public int getTotalCount() {
 			return this.totalCount;
 		}
 
-		public int getMessageCount() {
-			return this.messageCount;
+		public int getActualMessageCount() {
+			return this.actualCount;
 		}
 
 		public boolean validFrom(Jid from) {
@@ -406,6 +425,7 @@ public class MessageArchiveService implements OnAdvancedStreamFeaturesLoaded {
 			builder.append(AbstractGenerator.getTimestamp(this.start));
 			builder.append(", end=");
 			builder.append(AbstractGenerator.getTimestamp(this.end));
+			builder.append(", order="+pagingOrder.toString());
 			if (this.reference!=null) {
 				if (this.pagingOrder == PagingOrder.NORMAL) {
 					builder.append(", after=");
@@ -414,6 +434,7 @@ public class MessageArchiveService implements OnAdvancedStreamFeaturesLoaded {
 				}
 				builder.append(this.reference);
 			}
+			builder.append(", catchup="+Boolean.toString(catchup));
 			return builder.toString();
 		}
 

src/main/java/eu/siacs/conversations/services/XmppConnectionService.java πŸ”—

@@ -1413,10 +1413,8 @@ public class XmppConnectionService extends Service {
 								if (conversation != null) {
 									conversation.setBookmark(bookmark);
 								} else if (bookmark.autojoin() && bookmark.getJid() != null && autojoin) {
-									conversation = findOrCreateConversation(
-											account, bookmark.getJid(), true);
+									conversation = findOrCreateConversation(account, bookmark.getJid(), true, true);
 									conversation.setBookmark(bookmark);
-									joinMuc(conversation);
 								}
 							}
 						}
@@ -1683,11 +1681,15 @@ public class XmppConnectionService extends Service {
 		return null;
 	}
 
-	public Conversation findOrCreateConversation(final Account account, final Jid jid, final boolean muc) {
-		return this.findOrCreateConversation(account, jid, muc, null);
+	public Conversation findOrCreateConversation(Account account, Jid jid, boolean muc) {
+		return this.findOrCreateConversation(account,jid,muc,false);
 	}
 
-	public Conversation findOrCreateConversation(final Account account, final Jid jid, final boolean muc, final MessageArchiveService.Query query) {
+	public Conversation findOrCreateConversation(final Account account, final Jid jid, final boolean muc, final boolean joinAfterCreate) {
+		return this.findOrCreateConversation(account, jid, muc, joinAfterCreate, null);
+	}
+
+	public Conversation findOrCreateConversation(final Account account, final Jid jid, final boolean muc, final boolean joinAfterCreate, final MessageArchiveService.Query query) {
 		synchronized (this.conversations) {
 			Conversation conversation = find(account, jid);
 			if (conversation != null) {
@@ -1746,6 +1748,9 @@ public class XmppConnectionService extends Service {
 						}
 					}
 					checkDeletedFiles(c);
+					if (joinAfterCreate) {
+						joinMuc(c);
+					}
 				}
 			});
 			this.conversations.add(conversation);
@@ -2444,7 +2449,7 @@ public class XmppConnectionService extends Service {
 					return false;
 				}
 				final Jid jid = Jid.fromParts(new BigInteger(64, getRNG()).toString(Character.MAX_RADIX), server, null);
-				final Conversation conversation = findOrCreateConversation(account, jid, true);
+				final Conversation conversation = findOrCreateConversation(account, jid, true, false);
 				joinMuc(conversation, new OnConferenceJoined() {
 					@Override
 					public void onConferenceJoined(final Conversation conversation) {

src/main/java/eu/siacs/conversations/ui/StartConversationActivity.java πŸ”—

@@ -348,11 +348,8 @@ public class StartConversationActivity extends XmppActivity implements OnRosterU
             Toast.makeText(this, R.string.invalid_jid, Toast.LENGTH_SHORT).show();
             return;
         }
-        Conversation conversation = xmppConnectionService.findOrCreateConversation(bookmark.getAccount(), jid, true);
+        Conversation conversation = xmppConnectionService.findOrCreateConversation(bookmark.getAccount(), jid, true, true);
         conversation.setBookmark(bookmark);
-        if (!conversation.getMucOptions().online()) {
-            xmppConnectionService.joinMuc(conversation);
-        }
         if (!bookmark.autojoin() && getPreferences().getBoolean("autojoin", true)) {
             bookmark.setAutojoin(true);
             xmppConnectionService.pushBookmarks(bookmark.getAccount());
@@ -507,23 +504,15 @@ public class StartConversationActivity extends XmppActivity implements OnRosterU
                                 account.getBookmarks().add(bookmark);
                                 xmppConnectionService.pushBookmarks(account);
                                 final Conversation conversation = xmppConnectionService
-                                        .findOrCreateConversation(account,
-                                                conferenceJid, true);
+                                        .findOrCreateConversation(account, conferenceJid, true, true);
                                 conversation.setBookmark(bookmark);
-                                if (!conversation.getMucOptions().online()) {
-                                    xmppConnectionService.joinMuc(conversation);
-                                }
                                 dialog.dismiss();
                                 mCurrentDialog = null;
                                 switchToConversation(conversation);
                             }
                         } else {
                             final Conversation conversation = xmppConnectionService
-                                    .findOrCreateConversation(account,
-                                            conferenceJid, true);
-                            if (!conversation.getMucOptions().online()) {
-                                xmppConnectionService.joinMuc(conversation);
-                            }
+                                    .findOrCreateConversation(account,conferenceJid, true, true);
                             dialog.dismiss();
                             mCurrentDialog = null;
                             switchToConversation(conversation);
@@ -584,7 +573,7 @@ public class StartConversationActivity extends XmppActivity implements OnRosterU
     protected void switchToConversation(Contact contact, String body) {
         Conversation conversation = xmppConnectionService
                 .findOrCreateConversation(contact.getAccount(),
-                        contact.getJid(), false);
+                        contact.getJid(),false);
         switchToConversation(conversation, body, false);
     }