From f0798216d568bca30051ba5392263da31e78eb98 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Wed, 3 Feb 2016 10:40:02 +0100 Subject: [PATCH] refactored disco cache. avoid making duplicate call. check hash --- .../java/eu/siacs/conversations/Config.java | 2 + .../siacs/conversations/entities/Account.java | 3 + .../conversations/entities/Presence.java | 42 ++++++++++--- .../entities/ServiceDiscoveryResult.java | 20 ++++--- .../conversations/parser/PresenceParser.java | 33 +++-------- .../services/XmppConnectionService.java | 59 ++++++++++++++++++- 6 files changed, 116 insertions(+), 43 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/Config.java b/src/main/java/eu/siacs/conversations/Config.java index 9ee221cc3..3d32a2cb3 100644 --- a/src/main/java/eu/siacs/conversations/Config.java +++ b/src/main/java/eu/siacs/conversations/Config.java @@ -62,6 +62,8 @@ public final class Config { public static final boolean IGNORE_ID_REWRITE_IN_MUC = true; + public static final boolean REQUEST_DISCO = true; + public static final long MILLISECONDS_IN_DAY = 24 * 60 * 60 * 1000; public static final long MAM_MAX_CATCHUP = MILLISECONDS_IN_DAY / 2; public static final int MAM_MAX_MESSAGES = 500; diff --git a/src/main/java/eu/siacs/conversations/entities/Account.java b/src/main/java/eu/siacs/conversations/entities/Account.java index 4abfc801a..356b34e57 100644 --- a/src/main/java/eu/siacs/conversations/entities/Account.java +++ b/src/main/java/eu/siacs/conversations/entities/Account.java @@ -3,6 +3,7 @@ package eu.siacs.conversations.entities; import android.content.ContentValues; import android.database.Cursor; import android.os.SystemClock; +import android.util.Pair; import eu.siacs.conversations.crypto.PgpDecryptionService; import net.java.otr4j.crypto.OtrCryptoEngineImpl; @@ -14,6 +15,7 @@ import org.json.JSONObject; import java.security.PublicKey; import java.security.interfaces.DSAPublicKey; import java.util.Collection; +import java.util.HashSet; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.CopyOnWriteArraySet; @@ -48,6 +50,7 @@ public class Account extends AbstractEntity { public static final int OPTION_DISABLED = 1; public static final int OPTION_REGISTER = 2; public static final int OPTION_USECOMPRESSION = 3; + public final HashSet> inProgressDiscoFetches = new HashSet<>(); public boolean httpUploadAvailable() { return xmppConnection != null && xmppConnection.getFeatures().httpUpload(); diff --git a/src/main/java/eu/siacs/conversations/entities/Presence.java b/src/main/java/eu/siacs/conversations/entities/Presence.java index d31bb69d5..69cde8327 100644 --- a/src/main/java/eu/siacs/conversations/entities/Presence.java +++ b/src/main/java/eu/siacs/conversations/entities/Presence.java @@ -22,23 +22,31 @@ public class Presence implements Comparable { } protected final Status status; - protected final ServiceDiscoveryResult disco; + protected ServiceDiscoveryResult disco; + protected final String ver; + protected final String hash; - public Presence(Element show, ServiceDiscoveryResult disco) { - this.disco = disco; + private Presence(Status status, String ver, String hash) { + this.status = status; + this.ver = ver; + this.hash = hash; + } + public static Presence parse(Element show, Element caps) { + final String hash = caps == null ? null : caps.getAttribute("hash"); + final String ver = caps == null ? null : caps.getAttribute("ver"); if ((show == null) || (show.getContent() == null)) { - this.status = Status.ONLINE; + return new Presence(Status.ONLINE, ver, hash); } else if (show.getContent().equals("away")) { - this.status = Status.AWAY; + return new Presence(Status.AWAY, ver, hash); } else if (show.getContent().equals("xa")) { - this.status = Status.XA; + return new Presence(Status.XA, ver, hash); } else if (show.getContent().equals("chat")) { - this.status = Status.CHAT; + return new Presence(Status.CHAT, ver, hash); } else if (show.getContent().equals("dnd")) { - this.status = Status.DND; + return new Presence(Status.DND, ver, hash); } else { - this.status = Status.OFFLINE; + return new Presence(Status.OFFLINE, ver, hash); } } @@ -49,4 +57,20 @@ public class Presence implements Comparable { public Status getStatus() { return this.status; } + + public boolean hasCaps() { + return ver != null && hash != null; + } + + public String getVer() { + return this.ver; + } + + public String getHash() { + return this.hash; + } + + public void setServiceDiscoveryResult(ServiceDiscoveryResult disco) { + this.disco = disco; + } } diff --git a/src/main/java/eu/siacs/conversations/entities/ServiceDiscoveryResult.java b/src/main/java/eu/siacs/conversations/entities/ServiceDiscoveryResult.java index 9d77efab8..c50640e1f 100644 --- a/src/main/java/eu/siacs/conversations/entities/ServiceDiscoveryResult.java +++ b/src/main/java/eu/siacs/conversations/entities/ServiceDiscoveryResult.java @@ -128,7 +128,6 @@ public class ServiceDiscoveryResult { } } } - this.ver = this.mkCapHash(); } @@ -139,16 +138,23 @@ public class ServiceDiscoveryResult { this.ver = ver; JSONArray identities = o.optJSONArray("identities"); - for(int i = 0; i < identities.length(); i++) { - this.identities.add(new Identity(identities.getJSONObject(i))); + if (identities != null) { + for (int i = 0; i < identities.length(); i++) { + this.identities.add(new Identity(identities.getJSONObject(i))); + } } - JSONArray features = o.optJSONArray("features"); - for(int i = 0; i < features.length(); i++) { - this.features.add(features.getString(i)); + if (features != null) { + for (int i = 0; i < features.length(); i++) { + this.features.add(features.getString(i)); + } } } + public String getVer() { + return new String(Base64.encode(this.ver, Base64.DEFAULT)).trim(); + } + public ServiceDiscoveryResult(Cursor cursor) throws JSONException { this( cursor.getString(cursor.getColumnIndex(HASH)), @@ -235,7 +241,7 @@ public class ServiceDiscoveryResult { public ContentValues getContentValues() { final ContentValues values = new ContentValues(); values.put(HASH, this.hash); - values.put(VER, new String(Base64.encode(this.ver, Base64.DEFAULT)).trim()); + values.put(VER, getVer()); values.put(RESULT, this.toJSON().toString()); return values; } diff --git a/src/main/java/eu/siacs/conversations/parser/PresenceParser.java b/src/main/java/eu/siacs/conversations/parser/PresenceParser.java index 5a25b2835..2e706ff79 100644 --- a/src/main/java/eu/siacs/conversations/parser/PresenceParser.java +++ b/src/main/java/eu/siacs/conversations/parser/PresenceParser.java @@ -170,7 +170,7 @@ public class PresenceParser extends AbstractParser implements final String type = packet.getAttribute("type"); final Contact contact = account.getRoster().getContact(from); if (type == null) { - final String presence = from.isBareJid() ? "" : from.getResourcepart(); + final String resource = from.isBareJid() ? "" : from.getResourcepart(); contact.setPresenceName(packet.findChildContent("nick", "http://jabber.org/protocol/nick")); Avatar avatar = Avatar.parsePresence(packet.findChild("x", "vcard-temp:x:update")); if (avatar != null && !contact.isSelf()) { @@ -187,31 +187,12 @@ public class PresenceParser extends AbstractParser implements } int sizeBefore = contact.getPresences().size(); - ServiceDiscoveryResult disco = null; - Element caps = packet.findChild("c", "http://jabber.org/protocol/caps"); - - if (caps != null) { - disco = mXmppConnectionService.databaseBackend. - findDiscoveryResult(caps.getAttribute("hash"), caps.getAttribute("ver")); - } - - if (disco != null || caps == null) { - contact.updatePresence(presence, new Presence(packet.findChild("show"), disco)); - } else { - IqPacket request = new IqPacket(IqPacket.TYPE.GET); - request.setTo(from); - request.query("http://jabber.org/protocol/disco#info"); - - mXmppConnectionService.sendIqPacket(account, request, new OnIqPacketReceived() { - @Override - public void onIqPacketReceived(Account account, IqPacket discoPacket) { - if (discoPacket.getType() == IqPacket.TYPE.RESULT) { - ServiceDiscoveryResult disco = new ServiceDiscoveryResult(discoPacket); - contact.updatePresence(presence, new Presence(packet.findChild("show"), disco)); - mXmppConnectionService.databaseBackend.insertDiscoveryResult(disco); - } - } - }); + final Element show = packet.findChild("show"); + final Element caps = packet.findChild("c", "http://jabber.org/protocol/caps"); + final Presence presence = Presence.parse(show, caps); + contact.updatePresence(resource, presence); + if (presence.hasCaps() && Config.REQUEST_DISCO) { + mXmppConnectionService.fetchCaps(account, from, presence); } PgpEngine pgp = mXmppConnectionService.getPgpEngine(); diff --git a/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java b/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java index 43ca5a545..6735b9cc1 100644 --- a/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java +++ b/src/main/java/eu/siacs/conversations/services/XmppConnectionService.java @@ -74,6 +74,8 @@ import eu.siacs.conversations.entities.MucOptions; import eu.siacs.conversations.entities.MucOptions.OnRenameListener; import eu.siacs.conversations.entities.Presence; import eu.siacs.conversations.entities.Presences; +import eu.siacs.conversations.entities.Roster; +import eu.siacs.conversations.entities.ServiceDiscoveryResult; import eu.siacs.conversations.entities.Transferable; import eu.siacs.conversations.entities.TransferablePlaceholder; import eu.siacs.conversations.generator.IqGenerator; @@ -245,6 +247,7 @@ public class XmppConnectionService extends Service implements OnPhoneContactsLoa private OnKeyStatusUpdated mOnKeyStatusUpdated = null; private int keyStatusUpdatedListenerCount = 0; private SecureRandom mRandom; + private LruCache,ServiceDiscoveryResult> discoCache = new LruCache<>(20); private final OnBindListener mOnBindListener = new OnBindListener() { @Override @@ -2956,13 +2959,67 @@ public class XmppConnectionService extends Service implements OnPhoneContactsLoa @Override public void onIqPacketReceived(Account account, IqPacket packet) { if (packet.getType() == IqPacket.TYPE.ERROR) { - Log.d(Config.LOGTAG,account.getJid().toBareJid()+": could not publish nick"); + Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": could not publish nick"); } } }); } } + private ServiceDiscoveryResult getCachedServiceDiscoveryResult(Pair key) { + ServiceDiscoveryResult result = discoCache.get(key); + if (result != null) { + return result; + } else { + result = databaseBackend.findDiscoveryResult(key.first, key.second); + if (result != null) { + discoCache.put(key, result); + } + return result; + } + } + + public void fetchCaps(Account account, final Jid jid, final Presence presence) { + final Pair key = new Pair<>(presence.getHash(), presence.getVer()); + ServiceDiscoveryResult disco = getCachedServiceDiscoveryResult(key); + if (disco != null) { + presence.setServiceDiscoveryResult(disco); + } else { + if (!account.inProgressDiscoFetches.contains(key)) { + account.inProgressDiscoFetches.add(key); + IqPacket request = new IqPacket(IqPacket.TYPE.GET); + request.setTo(jid); + request.query("http://jabber.org/protocol/disco#info"); + Log.d(Config.LOGTAG,account.getJid().toBareJid()+": making disco request for "+key.second+" to "+jid); + sendIqPacket(account, request, new OnIqPacketReceived() { + @Override + public void onIqPacketReceived(Account account, IqPacket discoPacket) { + if (discoPacket.getType() == IqPacket.TYPE.RESULT) { + ServiceDiscoveryResult disco = new ServiceDiscoveryResult(discoPacket); + if (presence.getVer().equals(disco.getVer())) { + databaseBackend.insertDiscoveryResult(disco); + injectServiceDiscorveryResult(account.getRoster(), presence.getHash(), presence.getVer(), disco); + } else { + Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": mismatch in caps for contact " + jid+" "+presence.getVer()+" vs "+disco.getVer()); + } + } + account.inProgressDiscoFetches.remove(key); + } + }); + } + } + } + + private void injectServiceDiscorveryResult(Roster roster, String hash, String ver, ServiceDiscoveryResult disco) { + for(Contact contact : roster.getContacts()) { + for(Presence presence : contact.getPresences().getPresences().values()) { + if (hash.equals(presence.getHash()) && ver.equals(presence.getVer())) { + presence.setServiceDiscoveryResult(disco); + } + } + } + } + public interface OnAccountCreated { void onAccountCreated(Account account);