From 2665c3a1e0623839d3ca739b32fc7bd8e76c1e33 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Tue, 14 Feb 2017 16:50:33 +0100 Subject: [PATCH] rethink mam catchup strategies --- .../java/eu/siacs/conversations/Config.java | 4 +- .../conversations/parser/MessageParser.java | 7 ++- .../services/MessageArchiveService.java | 49 +++++++++++++------ .../services/XmppConnectionService.java | 19 ++++--- .../ui/StartConversationActivity.java | 19 ++----- 5 files changed, 58 insertions(+), 40 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/Config.java b/src/main/java/eu/siacs/conversations/Config.java index c1ec2d5dd..8b9a57fcb 100644 --- a/src/main/java/eu/siacs/conversations/Config.java +++ b/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; diff --git a/src/main/java/eu/siacs/conversations/parser/MessageParser.java b/src/main/java/eu/siacs/conversations/parser/MessageParser.java index 86ed28360..fd719da5d 100644 --- a/src/main/java/eu/siacs/conversations/parser/MessageParser.java +++ b/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) { diff --git a/src/main/java/eu/siacs/conversations/services/MessageArchiveService.java b/src/main/java/eu/siacs/conversations/services/MessageArchiveService.java index 8e4b95b73..ab2f59b0b 100644 --- a/src/main/java/eu/siacs/conversations/services/MessageArchiveService.java +++ b/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(); } diff --git a/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java b/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java index 99b075629..03fe14c3f 100644 --- a/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java +++ b/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) { diff --git a/src/main/java/eu/siacs/conversations/ui/StartConversationActivity.java b/src/main/java/eu/siacs/conversations/ui/StartConversationActivity.java index b81d2d979..76fd5e9df 100644 --- a/src/main/java/eu/siacs/conversations/ui/StartConversationActivity.java +++ b/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); }