Code style cleanup (#571)

* Code clarity fixes from import * Java suggestion: UnknownHostException is subtype of IOException * Java suggestion: params is only assigned during initialization * Java suggestion: simpler equivalent code * Java suggestion: empty array is faster than presized array * Java suggestion: JDK case-related methods depend dangerously on implicit default locale * Java suggestion: method list type can be inferred * Java suggestion: remove redundant toString() call on string * Java suggestion: use immutable collections API instead of singleton collection APIs * Java suggestion: make final tracer/idGenerator only assigned during initialization * Java suggestion: make final tracer only assigned during initialization * Java style: group overloads together * Update maven-checkstyle-plugin for Java 7 syntax
googleapis · Jan 10, 2019 · c8c44d9 · c8c44d9
1 parent 132edb7
commit c8c44d9
Show file tree

Hide file tree

Showing 12 changed files with 78 additions and 57 deletions.
diff --git a/...ent-apache-legacy/src/main/java/com/google/api/client/http/apache/ApacheHttpResponse.java b/...ent-apache-legacy/src/main/java/com/google/api/client/http/apache/ApacheHttpResponse.java
@@ -89,10 +89,6 @@ public String getStatusLine() {
     return statusLine == null ? null : statusLine.toString();
   }
 
-  public String getHeaderValue(String name) {
-    return response.getLastHeader(name).getValue();
-  }
-
   @Override
   public int getHeaderCount() {
     return allHeaders.length;
@@ -103,6 +99,10 @@ public String getHeaderName(int index) {
     return allHeaders[index].getName();
   }
 
+  public String getHeaderValue(String name) {
+    return response.getLastHeader(name).getValue();
+  }
+
   @Override
   public String getHeaderValue(int index) {
     return allHeaders[index].getValue();

diff --git a/...nt-apache-legacy/src/main/java/com/google/api/client/http/apache/ApacheHttpTransport.java b/...nt-apache-legacy/src/main/java/com/google/api/client/http/apache/ApacheHttpTransport.java
@@ -131,23 +131,34 @@ public ApacheHttpTransport(HttpClient httpClient) {
     params.setBooleanParameter(ClientPNames.HANDLE_REDIRECTS, false);
   }
 
+  /** Returns a new instance of the default HTTP parameters we use. */
+  static HttpParams newDefaultHttpParams() {
+    HttpParams params = new BasicHttpParams();
+    // Turn off stale checking. Our connections break all the time anyway,
+    // and it's not worth it to pay the penalty of checking every time.
+    HttpConnectionParams.setStaleCheckingEnabled(params, false);
+    HttpConnectionParams.setSocketBufferSize(params, 8192);
+    ConnManagerParams.setMaxTotalConnections(params, 200);
+    ConnManagerParams.setMaxConnectionsPerRoute(params, new ConnPerRouteBean(20));
+    return params;
+  }
+
   /**
-   * Creates a new instance of the Apache HTTP client that is used by the
-   * {@link #ApacheHttpTransport()} constructor.
+   * Creates a new instance of the Apache HTTP client that is used by the {@link
+   * #ApacheHttpTransport()} constructor.
+   *
+   * <p>Use this constructor if you want to customize the default Apache HTTP client. Settings:
    *
-   * <p>
-   * Use this constructor if you want to customize the default Apache HTTP client. Settings:
-   * </p>
    * <ul>
-   * <li>The client connection manager is set to {@link ThreadSafeClientConnManager}.</li>
-   * <li>The socket buffer size is set to 8192 using
-   * {@link HttpConnectionParams#setSocketBufferSize}.</li>
-   * <li><The retry mechanism is turned off by setting
-   * {@code new DefaultHttpRequestRetryHandler(0, false)}.</li>
-   * <li>The route planner uses {@link ProxySelectorRoutePlanner} with
-   * {@link ProxySelector#getDefault()}, which uses the proxy settings from <a
-   * href="http://docs.oracle.com/javase/7/docs/api/java/net/doc-files/net-properties.html">system
-   * properties</a>.</li>
+   *   <li>The client connection manager is set to {@link ThreadSafeClientConnManager}.</li>
+   *   <li>The socket buffer size is set to 8192 using {@link
+   *       HttpConnectionParams#setSocketBufferSize}.
+   *   <li>The retry mechanism is turned off by setting {@code new
+   *       DefaultHttpRequestRetryHandler(0, false)}.
+   *   <li>The route planner uses {@link ProxySelectorRoutePlanner} with {@link
+   *       ProxySelector#getDefault()}, which uses the proxy settings from <a
+   *       href="http://docs.oracle.com/javase/7/docs/api/java/net/doc-files/net-properties.html">system
+   *       properties</a>.
    * </ul>
    *
    * @return new instance of the Apache HTTP client
@@ -158,18 +169,6 @@ public static DefaultHttpClient newDefaultHttpClient() {
         SSLSocketFactory.getSocketFactory(), newDefaultHttpParams(), ProxySelector.getDefault());
   }
 
-  /** Returns a new instance of the default HTTP parameters we use. */
-  static HttpParams newDefaultHttpParams() {
-    HttpParams params = new BasicHttpParams();
-    // Turn off stale checking. Our connections break all the time anyway,
-    // and it's not worth it to pay the penalty of checking every time.
-    HttpConnectionParams.setStaleCheckingEnabled(params, false);
-    HttpConnectionParams.setSocketBufferSize(params, 8192);
-    ConnManagerParams.setMaxTotalConnections(params, 200);
-    ConnManagerParams.setMaxConnectionsPerRoute(params, new ConnPerRouteBean(20));
-    return params;
-  }
-
   /**
    * Creates a new instance of the Apache HTTP client that is used by the
    * {@link #ApacheHttpTransport()} constructor.
@@ -258,7 +257,7 @@ public static final class Builder {
     private SSLSocketFactory socketFactory = SSLSocketFactory.getSocketFactory();
 
     /** HTTP parameters. */
-    private HttpParams params = newDefaultHttpParams();
+    private final HttpParams params = newDefaultHttpParams();
 
     /**
      * HTTP proxy selector to use {@link ProxySelectorRoutePlanner} or {@code null} for

diff --git a/...p-client-apache-legacy/src/main/java/com/google/api/client/http/apache/ContentEntity.java b/...p-client-apache-legacy/src/main/java/com/google/api/client/http/apache/ContentEntity.java
@@ -41,22 +41,27 @@ final class ContentEntity extends AbstractHttpEntity {
     this.streamingContent = Preconditions.checkNotNull(streamingContent);
   }
 
+  @Override
   public InputStream getContent() {
     throw new UnsupportedOperationException();
   }
 
+  @Override
   public long getContentLength() {
     return contentLength;
   }
 
+  @Override
   public boolean isRepeatable() {
     return false;
   }
 
+  @Override
   public boolean isStreaming() {
     return true;
   }
 
+  @Override
   public void writeTo(OutputStream out) throws IOException {
     if (contentLength != 0) {
       streamingContent.writeTo(out);

diff --git a/...che-legacy/src/main/java/com/google/api/client/http/apache/SSLSocketFactoryExtension.java b/...che-legacy/src/main/java/com/google/api/client/http/apache/SSLSocketFactoryExtension.java
@@ -16,7 +16,6 @@
 
 import java.io.IOException;
 import java.net.Socket;
-import java.net.UnknownHostException;
 import java.security.KeyManagementException;
 import java.security.KeyStore;
 import java.security.KeyStoreException;
@@ -53,7 +52,7 @@ public Socket createSocket() throws IOException {
 
   @Override
   public Socket createSocket(Socket socket, String host, int port, boolean autoClose)
-      throws IOException, UnknownHostException {
+      throws IOException {
     SSLSocket sslSocket = (SSLSocket) socketFactory.createSocket(socket, host, port, autoClose);
     getHostnameVerifier().verify(host, sslSocket);
     return sslSocket;

diff --git a/google-http-client-apache/src/main/java/com/google/api/client/http/apache/ContentEntity.java b/google-http-client-apache/src/main/java/com/google/api/client/http/apache/ContentEntity.java
@@ -41,22 +41,27 @@ final class ContentEntity extends AbstractHttpEntity {
     this.streamingContent = Preconditions.checkNotNull(streamingContent);
   }
 
+  @Override
   public InputStream getContent() {
     throw new UnsupportedOperationException();
   }
 
+  @Override
   public long getContentLength() {
     return contentLength;
   }
 
+  @Override
   public boolean isRepeatable() {
     return false;
   }
 
+  @Override
   public boolean isStreaming() {
     return true;
   }
 
+  @Override
   public void writeTo(OutputStream out) throws IOException {
     if (contentLength != 0) {
       streamingContent.writeTo(out);

diff --git a/...ient/src/main/java/com/google/api/client/http/HttpBackOffUnsuccessfulResponseHandler.java b/...ient/src/main/java/com/google/api/client/http/HttpBackOffUnsuccessfulResponseHandler.java
@@ -19,7 +19,6 @@
 import com.google.api.client.util.Beta;
 import com.google.api.client.util.Preconditions;
 import com.google.api.client.util.Sleeper;
-
 import java.io.IOException;
 
 /**
@@ -131,13 +130,12 @@ public HttpBackOffUnsuccessfulResponseHandler setSleeper(Sleeper sleeper) {
   /**
    * {@inheritDoc}
    *
-   * <p>
-   * Handles the request with {@link BackOff}. That means that if back-off is required a call to
+   * <p>Handles the request with {@link BackOff}. That means that if back-off is required a call to
    * {@link Sleeper#sleep(long)} will be made.
-   * </p>
    */
-  public boolean handleResponse(
-      HttpRequest request, HttpResponse response, boolean supportsRetry) throws IOException {
+  @Override
+  public boolean handleResponse(HttpRequest request, HttpResponse response, boolean supportsRetry)
+      throws IOException {
     if (!supportsRetry) {
       return false;
     }

diff --git a/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java b/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java
@@ -222,7 +222,7 @@ static String executeAndGetValueOfSomeCustomHeader(HttpRequest request) {
   private Sleeper sleeper = Sleeper.DEFAULT;
 
   /** OpenCensus tracing component. */
-  private Tracer tracer = OpenCensusUtils.getTracer();
+  private final Tracer tracer = OpenCensusUtils.getTracer();
 
   /**
    * @param transport HTTP transport

diff --git a/google-http-client/src/main/java/com/google/api/client/http/OpenCensusUtils.java b/google-http-client/src/main/java/com/google/api/client/http/OpenCensusUtils.java
@@ -18,6 +18,7 @@
 import com.google.api.client.util.Preconditions;
 import com.google.common.annotations.VisibleForTesting;
 
+import com.google.common.collect.ImmutableList;
 import io.opencensus.contrib.http.util.HttpPropagationUtil;
 import io.opencensus.trace.BlankSpan;
 import io.opencensus.trace.EndSpanOptions;
@@ -29,7 +30,6 @@
 import io.opencensus.trace.Tracing;
 import io.opencensus.trace.propagation.TextFormat;
 
-import java.util.Collections;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -57,12 +57,12 @@ public class OpenCensusUtils {
    * OpenCensus tracing component. When no OpenCensus implementation is provided, it will return a
    * no-op tracer.
    */
-  private static Tracer tracer = Tracing.getTracer();
+  private static final Tracer tracer = Tracing.getTracer();
 
   /**
    * Sequence id generator for message event.
    */
-  private static AtomicLong idGenerator = new AtomicLong();
+  private static final AtomicLong idGenerator = new AtomicLong();
 
   /**
    * Whether spans should be recorded locally. Defaults to true.
@@ -253,8 +253,9 @@ public void put(HttpHeaders carrier, String key, String value) {
     }
 
     try {
-      Tracing.getExportComponent().getSampledSpanStore().registerSpanNamesForCollection(
-          Collections.<String>singletonList(SPAN_NAME_HTTP_REQUEST_EXECUTE));
+      Tracing.getExportComponent()
+          .getSampledSpanStore()
+          .registerSpanNamesForCollection(ImmutableList.of(SPAN_NAME_HTTP_REQUEST_EXECUTE));
     } catch (Exception e) {
       logger.log(
           Level.WARNING, "Cannot register default OpenCensus span names for collection.", e);

diff --git a/google-http-client/src/main/java/com/google/api/client/http/UriTemplate.java b/google-http-client/src/main/java/com/google/api/client/http/UriTemplate.java
@@ -360,7 +360,7 @@ public static String expand(String pathUri, Object parameters,
 
   private static String getSimpleValue(String name, String value, CompositeOutput compositeOutput) {
     if (compositeOutput.requiresVarAssignment()) {
-      return String.format("%s=%s", name, compositeOutput.getEncodedValue(value.toString()));
+      return String.format("%s=%s", name, compositeOutput.getEncodedValue(value));
     }
     return compositeOutput.getEncodedValue(value);
   }

diff --git a/google-http-client/src/main/java/com/google/api/client/util/FieldInfo.java b/google-http-client/src/main/java/com/google/api/client/util/FieldInfo.java
@@ -14,6 +14,7 @@
 
 package com.google.api.client.util;
 
+import com.google.common.base.Ascii;
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
@@ -141,14 +142,14 @@ public static FieldInfo of(Field field) {
    * Creates list of setter methods for a field only in declaring class.
    */
   private Method[] settersMethodForField(Field field) {
-    List<Method> methods = new ArrayList<Method>();
+    List<Method> methods = new ArrayList<>();
     for (Method method : field.getDeclaringClass().getDeclaredMethods()) {
-      if (method.getName().toLowerCase().equals("set" + field.getName().toLowerCase())
+      if (Ascii.toLowerCase(method.getName()).equals("set" + field.getName().toLowerCase())
           && method.getParameterTypes().length == 1) {
         methods.add(method);
       }
     }
-    return methods.toArray(new Method[methods.size()]);
+    return methods.toArray(new Method[0]);
   }
 
   /**
@@ -231,9 +232,7 @@ public void setValue(Object obj, Object value) {
           try {
             method.invoke(obj, value);
             return;
-          } catch (IllegalAccessException e) {
-            // try to set field directly
-          } catch (InvocationTargetException e) {
+          } catch (IllegalAccessException | InvocationTargetException e) {
             // try to set field directly
           }
         }

diff --git a/pom.xml b/pom.xml
@@ -361,8 +361,9 @@
           </configuration>
         </plugin>
         <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
           <artifactId>maven-checkstyle-plugin</artifactId>
-          <version>2.6</version>
+          <version>3.0.0</version>
         </plugin>
         <plugin>
           <groupId>org.codehaus.mojo</groupId>
@@ -482,9 +483,9 @@
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-checkstyle-plugin</artifactId>
         <configuration>
-          <configLocation>checkstyle.xml</configLocation>
+          <configLocation>${project.root-directory}/checkstyle.xml</configLocation>
           <consoleOutput>true</consoleOutput>
-          <suppressionsLocation>${basedir}/../checkstyle-suppressions.xml</suppressionsLocation>
+          <suppressionsLocation>${project.root-directory}/checkstyle-suppressions.xml</suppressionsLocation>
         </configuration>
         <executions>
           <execution>
@@ -576,6 +577,7 @@
     <project.datanucleus-rdbms.version>3.2.1</project.datanucleus-rdbms.version>
     <project.datanucleus-maven-plugin.version>4.0.3</project.datanucleus-maven-plugin.version>
     <project.opencensus.version>0.18.0</project.opencensus.version>
+    <project.root-directory>..</project.root-directory>
   </properties>
 
   <profiles>
@@ -612,5 +614,17 @@
         </plugins>
       </build>
     </profile>
+    <!-- set project.root-directory property based on where we are -->
+    <profile>
+      <id>root-directory</id>
+      <activation>
+        <file>
+          <exists>checkstyle-suppressions.xml</exists>
+        </file>
+      </activation>
+      <properties>
+        <project.root-directory>.</project.root-directory>
+      </properties>
+    </profile>
   </profiles>
 </project>
diff --git a/samples/dailymotion-simple-cmdline-sample/pom.xml b/samples/dailymotion-simple-cmdline-sample/pom.xml
@@ -35,11 +35,12 @@
       </plugin>
       <plugin>
         <artifactId>maven-checkstyle-plugin</artifactId>
-        <version>2.6</version>
+        <version>3.0.0</version>
         <configuration>
-          <configLocation>../checkstyle.xml</configLocation>
+          <configLocation>../../checkstyle.xml</configLocation>
           <consoleOutput>true</consoleOutput>
           <failOnViolation>false</failOnViolation>
+          <suppressionsLocation>../../checkstyle-suppressions.xml</suppressionsLocation>
         </configuration>
         <executions>
           <execution>