Fix checkstyle issues, setup thirdPartyAudit checks and enable forbiddenApisMain

client/build.gradle

@@ -23,60 +23,51 @@ group = 'org.elasticsearch.client'
 apply plugin: 'elasticsearch.build'
 
 dependencies {
-  // we use the lucene test-framework here but we are not pulling in ES core or the test framework
+  // TODO once we got rid of the client in the test framework we should use a version variable here
+  compile "org.apache.httpcomponents:httpclient:4.5.2"
+  compile "org.apache.httpcomponents:httpcore:4.4.4"
+  //compile "org.apache.httpcomponents:httpcore-nio:4.4.4"
+  //compile "org.apache.httpcomponents:httpasyncclient:4.1.1"
+  compile "commons-codec:commons-codec:1.9"
+  compile "commons-logging:commons-logging:1.2"
+
+  // we use lucene-test-framework here without pulling in ES core or its own test-framework
   testCompile "com.carrotsearch.randomizedtesting:randomizedtesting-runner:${versions.randomizedrunner}"
   testCompile "junit:junit:${versions.junit}"
   testCompile 'org.hamcrest:hamcrest-all:1.3'
   testCompile "org.apache.lucene:lucene-test-framework:${versions.lucene}"
   testCompile "org.apache.lucene:lucene-core:${versions.lucene}"
   testCompile "org.apache.lucene:lucene-codecs:${versions.lucene}"
-
-
-  // TODO once we got rid of the client in the test framework we should use a version variable here
-  // we use httpclient here since the JDK support has several issue
-  // - httpclient supports basic and digest auth and other schemes
-  // - URLConnection has issues with SSL and not all system patches are available
-  // - URLConnection can't stream data but httpclient can
-  // - URLConnection doesn't expose responsecodes unless it's a 200
-  // - httpclient supports pipelining which we might wanna expose down the road?
-  compile "org.apache.httpcomponents:httpclient:4.5.1"
-  compile "org.apache.httpcomponents:httpcore:4.4.4"
-  compile "org.apache.httpcomponents:httpcore-nio:4.4.4" // currently unused
-  compile "commons-logging:commons-logging:1.2"
-  compile 'org.apache.httpcomponents:httpasyncclient:4.1.1' // currently unused
 }
 
-
 compileJava.options.compilerArgs << '-Xlint:-cast,-rawtypes,-try,-unchecked'
 compileTestJava.options.compilerArgs << '-Xlint:-rawtypes'
 
-// the main files are actually test files, so use the appopriate forbidden api sigs
 forbiddenApisMain {
-  bundledSignatures = ['jdk-unsafe', 'jdk-deprecated']
-  signaturesURLs = [PrecommitTasks.getResource('/forbidden/all-signatures.txt'),
-                    PrecommitTasks.getResource('/forbidden/test-signatures.txt')]
+  //client does not depend on core, so only jdk signatures should be checked
+  signaturesURLs = [PrecommitTasks.getResource('/forbidden/jdk-signatures.txt')]
 }
 
+forbiddenApisMain.enabled=true
+//TODO remove use of sun http server and enable forbidden-apis for tests
 forbiddenApisTest.enabled=false
-forbiddenApisMain.enabled=false
+//TODO add licenses for deps and check out distribution task
+//dependency license are currently checked in distribution
+dependencyLicenses.enabled=false
+//TODO enable jarhell checks
+jarHell.enabled=false
+//NamingConventionCheck is part of test-framework, which we don't want to pull in as it depends on es core
+namingConventions.enabled=false
 
-// dependency license are currently checked in distribution
-dependencyLicenses.enabled = false
-jarHell.enabled = false
-thirdPartyAudit.enabled = false
 thirdPartyAudit.excludes = [
-  // classes are missing
-  'javax.servlet.ServletContextEvent', 
-  'javax.servlet.ServletContextListener', 
-  'org.apache.avalon.framework.logger.Logger', 
-  'org.apache.log.Hierarchy', 
-  'org.apache.log.Logger', 
-   // we intentionally exclude the ant tasks because people were depending on them from their tests!!!!!!!
-  'org.apache.tools.ant.BuildException', 
-  'org.apache.tools.ant.DirectoryScanner', 
-  'org.apache.tools.ant.Task', 
-  'org.apache.tools.ant.types.FileSet', 
-  'org.easymock.EasyMock', 
-  'org.easymock.IArgumentMatcher', 
-  'org.jmock.core.Constraint',
+  //commons-logging optional dependencies
+  'org.apache.avalon.framework.logger.Logger',
+  'org.apache.log.Hierarchy',
+  'org.apache.log.Logger',
+  'org.apache.log4j.Level',
+  'org.apache.log4j.Logger',
+  'org.apache.log4j.Priority',
+  //commons-logging provided dependencies
+  'javax.servlet.ServletContextEvent',
+  'javax.servlet.ServletContextListener'
 ]

client/src/main/java/org/elasticsearch/client/IndexClient.java

@@ -1,14 +1,30 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
 package org.elasticsearch.client;
 
 import java.io.IOException;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
 
-/**
- * Created by simon on 2/16/16.
- */
 public class IndexClient {
 
     private final RestClient client;
@@ -24,7 +40,7 @@ public class IndexClient {
         Objects.requireNonNull(index, "index must not be null");
         Objects.requireNonNull(type, "type must not be null");
         Objects.requireNonNull(id, "id must not be null");
-        String deleteEndpoint = String.format("/%s/%s/%s", index, type, id);
+        String deleteEndpoint = String.format(Locale.ROOT, "/%s/%s/%s", index, type, id);
         client.httpDelete(deleteEndpoint, params == null ? Collections.emptyMap() : params.options);
     }
 
@@ -59,7 +75,4 @@ public class IndexClient {
             options.put("timeout", timeout);
         };
     }
-
-
-
 }

client/src/main/java/org/elasticsearch/client/RestClient.java

@@ -60,7 +60,8 @@ public class RestClient implements Closeable{
     private final Set<HttpHost> blackList = new CopyOnWriteArraySet<>();
 
     public RestClient(HttpHost... hosts) {
-        this("http", HttpClientBuilder.create().setDefaultRequestConfig(RequestConfig.custom().setConnectTimeout(100).build()).build(), hosts);
+        this("http", HttpClientBuilder.create().setDefaultRequestConfig(
+                RequestConfig.custom().setConnectTimeout(100).build()).build(), hosts);
     }
 
     public RestClient(String scheme, CloseableHttpClient client, HttpHost[] hosts) {

client/src/test/java/org/elasticsearch/client/RestClientTests.java

@@ -26,11 +26,10 @@ import org.apache.lucene.util.LuceneTestCase;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.net.InetSocketAddress;
-import java.net.URISyntaxException;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
-import java.util.concurrent.ExecutionException;
+
 public class RestClientTests extends LuceneTestCase {
     //TODO this should be refactored into a base test!!
     HttpServer server;
@@ -96,7 +95,8 @@ public class RestClientTests extends LuceneTestCase {
         HttpHost httpHost = new HttpHost("127.0.0.1", server.getAddress().getPort(), "http");
         try(RestClient client = new RestClient(httpHost)) {
             assertEquals(3, client.fetchNodes(httpHost, true, true, false).size());
-            assertTrue(client.fetchNodes(httpHost, true, true, false).toString(), client.fetchNodes(httpHost, true, true, false).contains(new HttpHost("127.0.0.2", 9200, "http")));
+            assertTrue(client.fetchNodes(httpHost, true, true, false).toString(), client.fetchNodes(httpHost, true, true, false)
+                    .contains(new HttpHost("127.0.0.2", 9200, "http")));
             assertTrue(client.fetchNodes(httpHost, true, true, false).contains(new HttpHost("127.0.0.3", 9200, "http")));
             assertTrue(client.fetchNodes(httpHost, true, true, false).contains(httpHost));
             assertEquals(1, client.fetchNodes(httpHost, true, true, true).size());
@@ -115,7 +115,8 @@ public class RestClientTests extends LuceneTestCase {
                 client.httpGet("/_cat/health", Collections.emptyMap());
                 fail();
             } catch (IOException ex) {
-                assertTrue(ex.getMessage(), ex.getMessage().endsWith("failed: connect timed out") || ex.getMessage().endsWith("failed: Connection refused"));
+                assertTrue(ex.getMessage(), ex.getMessage().endsWith("failed: connect timed out") ||
+                        ex.getMessage().endsWith("failed: Connection refused"));
             }
         }
     }
@@ -132,7 +133,8 @@ public class RestClientTests extends LuceneTestCase {
                 client.httpGet("/_cat/health", Collections.emptyMap());
                 fail();
             } catch (IOException ex) {
-                assertTrue(ex.getMessage(), ex.getMessage().endsWith("failed: connect timed out") || ex.getMessage().endsWith("failed: Connection refused"));
+                assertTrue(ex.getMessage(), ex.getMessage().endsWith("failed: connect timed out") ||
+                        ex.getMessage().endsWith("failed: Connection refused"));
             }
             assertEquals(3, client.getNumHosts());
             assertEquals(3, client.getNumBlacklistedHosts());
@@ -147,6 +149,4 @@ public class RestClientTests extends LuceneTestCase {
             assertEquals(3, num);
         }
     }
-
-
 }