nit0906 commented on a change in pull request #431:
URL: https://github.com/apache/jackrabbit-oak/pull/431#discussion_r769513816



##########
File path: 
oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticConnectionRule.java
##########
@@ -126,61 +124,56 @@ private void 
downloadSimilaritySearchPluginIfNotExists(String localPluginPath, S
                     if (!pluginFile.delete()) {
                         deleteString = "Could not delete downloaded plugin 
file.";
                     }
-                    throw new RuntimeException("Plugin digest unequal. Found " 
+ result.toString() + ". Expected " + PLUGIN_DIGEST + ". " + deleteString);
+                    throw new RuntimeException("Plugin digest unequal. Found " 
+ result + ". Expected " + PLUGIN_DIGEST + ". " + deleteString);
                 }
-            } catch (IOException|NoSuchAlgorithmException e) {
+            } catch (IOException | NoSuchAlgorithmException e) {
                 throw new RuntimeException("Could not download similarity 
search plugin", e);
             }
         }
     }
 
     public ElasticConnection getElasticConnectionFromString() {
-        if (elasticConnection == null) {
-            try {
-                URI uri = new URI(elasticConnectionString);
-                String host = uri.getHost();
-                String scheme = uri.getScheme();
-                int port = uri.getPort();
-                String query = uri.getQuery();
-
-                String api_key = null;
-                String api_secret = null;
-                if (query != null) {
-                    api_key = query.split(",")[0].split("=")[1];
-                    api_secret = query.split(",")[1].split("=")[1];
-                }
-                elasticConnection = ElasticConnection.newBuilder()
-                        .withIndexPrefix(INDEX_PREFIX + 
System.currentTimeMillis())
-                        .withConnectionParameters(scheme, host, port)
-                        .withApiKeys(api_key, api_secret)
-                        .build();
-            } catch (URISyntaxException e) {
-                return null;
+        try {
+            URI uri = new URI(elasticConnectionString);
+            String host = uri.getHost();
+            String scheme = uri.getScheme();
+            int port = uri.getPort();
+            String query = uri.getQuery();
+
+            String api_key = null;
+            String api_secret = null;
+            if (query != null) {
+                api_key = query.split(",")[0].split("=")[1];
+                api_secret = query.split(",")[1].split("=")[1];
             }
+            return ElasticConnection.newBuilder()
+                    .withIndexPrefix(INDEX_PREFIX + System.currentTimeMillis())
+                    .withConnectionParameters(scheme, host, port)
+                    .withApiKeys(api_key, api_secret)
+                    .build();
+        } catch (URISyntaxException e) {
+            return null;
         }
-        return elasticConnection;
     }
 
     public ElasticConnection getElasticConnectionForDocker() {
-        if (elasticConnection == null) {
-            elasticConnection = ElasticConnection.newBuilder()
-                    .withIndexPrefix(INDEX_PREFIX + System.currentTimeMillis())
-                    .withConnectionParameters(ElasticConnection.DEFAULT_SCHEME,
-                            elastic.getContainerIpAddress(),
-                            
elastic.getMappedPort(ElasticConnection.DEFAULT_PORT))
-                    .withApiKeys(null, null)
-                    .build();
-        }
-        return elasticConnection;
+        return getElasticConnectionForDocker(elastic.getContainerIpAddress(),
+                elastic.getMappedPort(ElasticConnection.DEFAULT_PORT));
+    }
+
+    public ElasticConnection getElasticConnectionForDocker(String 
containerIpAddress, int port) {
+        return ElasticConnection.newBuilder()
+                .withIndexPrefix(INDEX_PREFIX + System.currentTimeMillis())
+                .withConnectionParameters(ElasticConnection.DEFAULT_SCHEME,
+                        containerIpAddress, port)
+                .withApiKeys(null, null)
+                .build();
     }
 
-    public void closeElasticConnection() throws IOException {
+    public void closeElasticConnection(ElasticConnection elasticConnection) 
throws IOException {

Review comment:
       @fabriziofortino 
   I think not having a cleanup method that closes ElasticConnection and 
deletes test indexes might create problems if tests are run on a non docker 
bases elastic server - I am not sure if we would really need to do that. 
   But maybe we can add TODO statement there in case it's needed in future.
   
   Rest all looks good to me.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to