refactored disco cache. avoid making duplicate call. check hash

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 9ee221cc..3d32a2cb 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 4abfc801..356b34e5 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<Pair<String, String>> 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 d31bb69d..69cde832 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 9d77efab..c50640e1 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 5a25b283..2e706ff7 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 43ca5a54..6735b9cc 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<Pair<String,String>,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<String,String> 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<String,String> 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);