De-duplicate results from Identity stores in Directory searches

- Switch to HttpClient for remote fetcher
- Don't fail for remote binding on matrix.org

docs/stores/sql.md

@@ -44,7 +44,7 @@ Example: `/path/to/sqlite/file.db`
 
 #### Others
 ```yaml
-sql.connection: //<HOST[:PORT]/DB?username=USER&password=PASS
+sql.connection: //<HOST[:PORT]/DB?user=USER&password=PASS
 ```
 Set the connection info for the database by replacing the following values:
 - `HOST`: Hostname of the SQL server

docs/stores/synapse.md

@@ -35,7 +35,7 @@ Example: `/path/to/synapse/sqliteFile.db`
 
 ### PostgreSQL
 ```yaml
-synapseSql.connection: //<HOST[:PORT]/DB?username=USER&password=PASS
+synapseSql.connection: //<HOST[:PORT]/DB?user=USER&password=PASS
 ```
 Set the connection info for the database by replacing the following values:
 - `HOST`: Hostname of the SQL server

src/main/java/io/kamax/mxisd/controller/directory/v1/io/UserDirectorySearchResult.java

@@ -20,8 +20,8 @@
 
 package io.kamax.mxisd.controller.directory.v1.io;
 
-import java.util.ArrayList;
-import java.util.List;
+import java.util.HashSet;
+import java.util.Set;
 
 public class UserDirectorySearchResult {
 
@@ -55,10 +55,31 @@ public class UserDirectorySearchResult {
             this.userId = userId;
         }
 
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) return true;
+            if (o == null || getClass() != o.getClass()) return false;
+
+            Result result = (Result) o;
+
+            if (displayName != null ? !displayName.equals(result.displayName) : result.displayName != null)
+                return false;
+            if (avatarUrl != null ? !avatarUrl.equals(result.avatarUrl) : result.avatarUrl != null) return false;
+            return userId.equals(result.userId);
+        }
+
+        @Override
+        public int hashCode() {
+            int result = displayName != null ? displayName.hashCode() : 0;
+            result = 31 * result + (avatarUrl != null ? avatarUrl.hashCode() : 0);
+            result = 31 * result + userId.hashCode();
+            return result;
+        }
+
     }
 
     private boolean limited;
-    private List<Result> results = new ArrayList<>();
+    private Set<Result> results = new HashSet<>();
 
     public boolean isLimited() {
         return limited;
@@ -68,11 +89,11 @@ public class UserDirectorySearchResult {
         this.limited = limited;
     }
 
-    public List<Result> getResults() {
+    public Set<Result> getResults() {
         return results;
     }
 
-    public void setResults(List<Result> results) {
+    public void setResults(Set<Result> results) {
         this.results = results;
     }
 

src/main/java/io/kamax/mxisd/lookup/provider/RemoteIdentityServerFetcher.java

@@ -23,6 +23,7 @@ package io.kamax.mxisd.lookup.provider;
 import com.google.gson.Gson;
 import com.google.gson.JsonObject;
 import com.google.gson.JsonParseException;
+import io.kamax.matrix.json.GsonUtil;
 import io.kamax.mxisd.controller.identity.v1.ClientBulkLookupRequest;
 import io.kamax.mxisd.exception.InvalidResponseJsonException;
 import io.kamax.mxisd.lookup.SingleLookupReply;
@@ -33,18 +34,20 @@ import io.kamax.mxisd.matrix.IdentityServerUtils;
 import io.kamax.mxisd.util.GsonParser;
 import io.kamax.mxisd.util.RestClientUtils;
 import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.methods.HttpPost;
+import org.apache.http.client.utils.URIBuilder;
 import org.apache.http.impl.client.CloseableHttpClient;
-import org.apache.http.impl.client.HttpClients;
+import org.apache.http.util.EntityUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Lazy;
 import org.springframework.context.annotation.Scope;
 import org.springframework.stereotype.Component;
 
 import java.io.IOException;
-import java.net.HttpURLConnection;
-import java.net.URL;
+import java.net.URISyntaxException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
@@ -59,6 +62,9 @@ public class RemoteIdentityServerFetcher implements IRemoteIdentityServerFetcher
     private Gson gson = new Gson();
     private GsonParser parser = new GsonParser(gson);
 
+    @Autowired
+    private CloseableHttpClient client;
+
     @Override
     public boolean isUsable(String remote) {
         return IdentityServerUtils.isUsable(remote);
@@ -69,24 +75,40 @@ public class RemoteIdentityServerFetcher implements IRemoteIdentityServerFetcher
         log.info("Looking up {} 3PID {} using {}", request.getType(), request.getThreePid(), remote);
 
         try {
-            HttpURLConnection rootSrvConn = (HttpURLConnection) new URL(
-                    remote + "/_matrix/identity/api/v1/lookup?medium=" + request.getType() + "&address=" + request.getThreePid()
-            ).openConnection();
-            JsonObject obj = parser.parse(rootSrvConn.getInputStream());
-            if (obj.has("address")) {
-                log.info("Found 3PID mapping: {}", gson.toJson(obj));
+            URIBuilder b = new URIBuilder(remote);
+            b.setPath("/_matrix/identity/api/v1/lookup");
+            b.addParameter("medium", request.getType());
+            b.addParameter("address", request.getThreePid());
+            HttpGet req = new HttpGet(b.build());
 
-                return Optional.of(SingleLookupReply.fromRecursive(request, gson.toJson(obj)));
+            try (CloseableHttpResponse res = client.execute(req)) {
+                int statusCode = res.getStatusLine().getStatusCode();
+                String body = EntityUtils.toString(res.getEntity());
+
+                if (statusCode != 200) {
+                    log.warn("Remote returned status code {}", statusCode);
+                    log.warn("Body: {}", body);
+                    return Optional.empty();
+                }
+
+                JsonObject obj = GsonUtil.parseObj(body);
+                if (obj.has("address")) {
+                    log.debug("Found 3PID mapping: {}", gson.toJson(obj));
+                    return Optional.of(SingleLookupReply.fromRecursive(request, gson.toJson(obj)));
+                }
+
+                log.info("Empty 3PID mapping from {}", remote);
+                return Optional.empty();
             }
-
-            log.info("Empty 3PID mapping from {}", remote);
-            return Optional.empty();
         } catch (IOException e) {
             log.warn("Error looking up 3PID mapping {}: {}", request.getThreePid(), e.getMessage());
             return Optional.empty();
         } catch (JsonParseException e) {
             log.warn("Invalid JSON answer from {}", remote);
             return Optional.empty();
+        } catch (URISyntaxException e) {
+            log.warn("Invalid remote address: {}", e.getMessage(), e);
+            return Optional.empty();
         }
     }
 
@@ -98,12 +120,15 @@ public class RemoteIdentityServerFetcher implements IRemoteIdentityServerFetcher
         mappingRequest.setMappings(mappings);
 
         String url = remote + "/_matrix/identity/api/v1/bulk_lookup";
-        CloseableHttpClient client = HttpClients.createDefault();
         try {
             HttpPost request = RestClientUtils.post(url, mappingRequest);
             try (CloseableHttpResponse response = client.execute(request)) {
-                if (response.getStatusLine().getStatusCode() != 200) {
-                    log.info("Could not perform lookup at {} due to HTTP return code: {}", url, response.getStatusLine().getStatusCode());
+                int statusCode = response.getStatusLine().getStatusCode();
+                String body = EntityUtils.toString(response.getEntity());
+
+                if (statusCode != 200) {
+                    log.warn("Could not perform lookup at {} due to HTTP return code: {}", url, statusCode);
+                    log.warn("Body: {}", body);
                     return mappingsFound;
                 }
 

src/main/java/io/kamax/mxisd/session/SessionMananger.java

@@ -277,8 +277,7 @@ public class SessionMananger {
         }
 
         String is = servers.get(0);
-        String url = IdentityServerUtils.findIsUrlForDomain(is)
-                .orElseThrow(() -> new InternalServerError(is + " could not be resolved to an Identity server"));
+        String url = IdentityServerUtils.findIsUrlForDomain(is).orElse(is);
         log.info("Will use IS endpoint {}", url);
 
         String remoteSecret = session.isRemote() ? session.getRemoteSecret() : RandomStringUtils.randomAlphanumeric(16);

src/main/java/io/kamax/mxisd/spring/CloseableHttpClientFactory.java

@@ -30,7 +30,11 @@ public class CloseableHttpClientFactory {
 
     @Bean
     public CloseableHttpClient getClient() {
-        return HttpClients.custom().setUserAgent("mxisd").build();
+        return HttpClients.custom()
+                .setUserAgent("mxisd")
+                .setMaxConnPerRoute(Integer.MAX_VALUE)
+                .setMaxConnTotal(Integer.MAX_VALUE)
+                .build();
     }
 
 }

src/main/java/io/kamax/mxisd/spring/CryptoFactory.java

@@ -24,7 +24,7 @@ import io.kamax.matrix.crypto.KeyFileStore;
 import io.kamax.matrix.crypto.KeyManager;
 import io.kamax.matrix.crypto.SignatureManager;
 import io.kamax.mxisd.config.KeyConfig;
-import io.kamax.mxisd.config.MatrixConfig;
+import io.kamax.mxisd.config.ServerConfig;
 import org.apache.commons.io.FileUtils;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
@@ -50,8 +50,8 @@ public class CryptoFactory {
     }
 
     @Bean
-    public SignatureManager getSignatureManager(KeyManager keyMgr, MatrixConfig mxCfg) {
-        return new SignatureManager(keyMgr, mxCfg.getDomain());
+    public SignatureManager getSignatureManager(KeyManager keyMgr, ServerConfig cfg) {
+        return new SignatureManager(keyMgr, cfg.getName());
     }
 
 }

src/test/java/io/kamax/mxisd/backend/rest/RestDirectoryProviderTest.java

@@ -33,8 +33,7 @@ import org.junit.Test;
 import java.nio.charset.StandardCharsets;
 
 import static com.github.tomakehurst.wiremock.client.WireMock.*;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.*;
 
 public class RestDirectoryProviderTest {
 
@@ -89,8 +88,8 @@ public class RestDirectoryProviderTest {
 
         UserDirectorySearchResult result = p.searchByDisplayName(byNameSearch);
         assertTrue(!result.isLimited());
-        assertTrue(result.getResults().size() == 1);
-        UserDirectorySearchResult.Result entry = result.getResults().get(0);
+        assertEquals(1, result.getResults().size());
+        UserDirectorySearchResult.Result entry = result.getResults().iterator().next();
         assertNotNull(entry);
         assertTrue(StringUtils.equals(byNameAvatar, entry.getAvatarUrl()));
         assertTrue(StringUtils.equals(byNameDisplay, entry.getDisplayName()));
@@ -132,8 +131,8 @@ public class RestDirectoryProviderTest {
 
         UserDirectorySearchResult result = p.searchBy3pid(byThreepidSearch);
         assertTrue(!result.isLimited());
-        assertTrue(result.getResults().size() == 1);
-        UserDirectorySearchResult.Result entry = result.getResults().get(0);
+        assertEquals(1, result.getResults().size());
+        UserDirectorySearchResult.Result entry = result.getResults().iterator().next();
         assertNotNull(entry);
         assertTrue(StringUtils.equals(byThreepidAvatar, entry.getAvatarUrl()));
         assertTrue(StringUtils.equals(byThreepidDisplay, entry.getDisplayName()));