fix: Apply Google Java Code Clarity suggestions (#1044)

* fix: Apply Google Java Code Clarity suggestions

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: Ben Creech <[email protected]>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

Author: 3 people

google-cloud-logging/clirr-ignored-differences.xml

@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<differences>
+  <difference>
+    <differenceType>3003</differenceType>
+    <className>com/google/cloud/logging/Instrumentation</className>
+  </difference>
+  <difference>
+    <differenceType>7009</differenceType>
+    <className>com/google/cloud/logging/Instrumentation</className>
+    <method>Instrumentation()</method>
+  </difference>
+</differences>

google-cloud-logging/pom.xml

@@ -127,6 +127,11 @@
       <type>test-jar</type>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>com.google.guava</groupId>
+      <artifactId>guava-testlib</artifactId>
+      <scope>test</scope>
+    </dependency>
 
     <dependency>
       <groupId>com.google.api.grpc</groupId>

google-cloud-logging/src/main/java/com/google/cloud/logging/Context.java

@@ -17,8 +17,12 @@
 package com.google.cloud.logging;
 
 import com.google.cloud.logging.HttpRequest.RequestMethod;
+import com.google.common.base.Ascii;
 import com.google.common.base.MoreObjects;
+import com.google.common.base.Splitter;
+import com.google.common.collect.Iterables;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
+import java.util.List;
 import java.util.Objects;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -125,7 +129,7 @@ public Builder setSpanId(String spanId) {
     @CanIgnoreReturnValue
     public Builder loadCloudTraceContext(String cloudTrace) {
       if (cloudTrace != null) {
-        cloudTrace = cloudTrace.split(";")[0];
+        cloudTrace = Iterables.get(Splitter.on(';').split(cloudTrace), 0);
         int split = cloudTrace.indexOf('/');
         if (split >= 0) {
           String traceId = cloudTrace.substring(0, split);
@@ -157,16 +161,16 @@ public Builder loadCloudTraceContext(String cloudTrace) {
      *     the format version is not supported.
      */
     @CanIgnoreReturnValue
-    public Builder loadW3CTraceParentContext(String traceParent) throws IllegalArgumentException {
+    public Builder loadW3CTraceParentContext(String traceParent) {
       if (traceParent != null) {
-        Matcher validator = W3C_TRACE_CONTEXT_FORMAT.matcher(traceParent.toLowerCase());
+        Matcher validator = W3C_TRACE_CONTEXT_FORMAT.matcher(Ascii.toLowerCase(traceParent));
         if (!validator.matches()) {
           throw new IllegalArgumentException(
               "Invalid format of the header value. The value does not match W3C Trace Context version \"00\"");
         }
-        String[] fields = traceParent.split("-");
-        setTraceId(fields[1]);
-        setSpanId(fields[2]);
+        List<String> fields = Splitter.on('-').splitToList(traceParent);
+        setTraceId(fields.get(1));
+        setSpanId(fields.get(2));
         // fields[3] contains flag(s)
       }
       return this;

google-cloud-logging/src/main/java/com/google/cloud/logging/Exclusion.java

@@ -23,7 +23,6 @@
 import com.google.logging.v2.LogExclusion;
 import com.google.protobuf.Timestamp;
 import java.util.Objects;
-import org.jspecify.nullness.Nullable;
 
 /**
  * Specifies a set of log entries that are not to be stored in Logging. If your GCP resource
@@ -35,27 +34,21 @@
 public class Exclusion {
 
   static final Function<LogExclusion, Exclusion> FROM_PROTOBUF_FUNCTION =
-      new Function<LogExclusion, Exclusion>() {
-        @Override
-        public @Nullable Exclusion apply(LogExclusion exclusionPb) {
-          return exclusionPb != null ? Exclusion.fromProtobuf(exclusionPb) : null;
-        }
+      (LogExclusion exclusionPb) -> {
+        return exclusionPb != null ? Exclusion.fromProtobuf(exclusionPb) : null;
       };
 
   static final Function<Exclusion, LogExclusion> TO_PROTOBUF_FUNCTION =
-      new Function<Exclusion, LogExclusion>() {
-        @Override
-        public @Nullable LogExclusion apply(Exclusion exclusion) {
-          return exclusion != null ? exclusion.toProtobuf() : null;
-        }
+      (Exclusion exclusion) -> {
+        return exclusion != null ? exclusion.toProtobuf() : null;
       };
 
-  private String name;
-  private String description;
-  private String filter;
-  private boolean disabled;
-  private Timestamp createTime;
-  private Timestamp updateTime;
+  private final String name;
+  private final String description;
+  private final String filter;
+  private final boolean disabled;
+  private final Timestamp createTime;
+  private final Timestamp updateTime;
 
   /** A builder for {@code Exclusion} objects. */
   public static class Builder {
@@ -165,7 +158,7 @@ public boolean equals(Object o) {
     if (this == o) {
       return true;
     }
-    if (o == null || getClass() != o.getClass()) {
+    if (!(o instanceof Exclusion)) {
       return false;
     }
     Exclusion exclusion = (Exclusion) o;
@@ -253,7 +246,7 @@ LogExclusion toProtobuf() {
   static Exclusion fromProtobuf(LogExclusion exclusionPb) {
     Exclusion.Builder builder = newBuilder(exclusionPb.getName(), exclusionPb.getFilter());
     builder.setDisabled(exclusionPb.getDisabled());
-    if (!exclusionPb.getDescription().equals("")) {
+    if (!exclusionPb.getDescription().isEmpty()) {
       builder.setDescription(exclusionPb.getDescription());
     }
     if (exclusionPb.hasCreateTime()) {

google-cloud-logging/src/main/java/com/google/cloud/logging/HttpRequest.java

@@ -20,7 +20,6 @@
 import com.google.cloud.StringEnumType;
 import com.google.cloud.StringEnumValue;
 import com.google.common.base.MoreObjects;
-import com.google.common.base.Strings;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import java.io.Serializable;
 import java.util.Objects;
@@ -71,7 +70,7 @@ public RequestMethod apply(String constant) {
         };
 
     private static final StringEnumType<RequestMethod> type =
-        new StringEnumType<RequestMethod>(RequestMethod.class, CONSTRUCTOR);
+        new StringEnumType<>(RequestMethod.class, CONSTRUCTOR);
 
     public static final RequestMethod GET = type.createAndRegister("GET");
     public static final RequestMethod HEAD = type.createAndRegister("HEAD");
@@ -527,10 +526,10 @@ public static Builder newBuilder() {
 
   static HttpRequest fromPb(com.google.logging.type.HttpRequest requestPb) {
     Builder builder = newBuilder();
-    if (!Strings.isNullOrEmpty(requestPb.getRequestMethod())) {
+    if (!requestPb.getRequestMethod().isEmpty()) {
       builder.setRequestMethod(RequestMethod.valueOf(requestPb.getRequestMethod()));
     }
-    if (!Strings.isNullOrEmpty(requestPb.getRequestUrl())) {
+    if (!requestPb.getRequestUrl().isEmpty()) {
       builder.setRequestUrl(requestPb.getRequestUrl());
     }
     if (requestPb.getRequestSize() != 0L) {
@@ -542,16 +541,16 @@ static HttpRequest fromPb(com.google.logging.type.HttpRequest requestPb) {
     if (requestPb.getResponseSize() != 0L) {
       builder.setResponseSize(requestPb.getResponseSize());
     }
-    if (!Strings.isNullOrEmpty(requestPb.getUserAgent())) {
+    if (!requestPb.getUserAgent().isEmpty()) {
       builder.setUserAgent(requestPb.getUserAgent());
     }
-    if (!Strings.isNullOrEmpty(requestPb.getServerIp())) {
+    if (!requestPb.getServerIp().isEmpty()) {
       builder.setServerIp(requestPb.getServerIp());
     }
-    if (!Strings.isNullOrEmpty(requestPb.getRemoteIp())) {
+    if (!requestPb.getRemoteIp().isEmpty()) {
       builder.setRemoteIp(requestPb.getRemoteIp());
     }
-    if (!Strings.isNullOrEmpty(requestPb.getReferer())) {
+    if (!requestPb.getReferer().isEmpty()) {
       builder.setReferer(requestPb.getReferer());
     }
     builder.setCacheLookup(requestPb.getCacheLookup());

google-cloud-logging/src/main/java/com/google/cloud/logging/Instrumentation.java

@@ -21,18 +21,17 @@
 import com.google.cloud.Tuple;
 import com.google.cloud.logging.Logging.WriteOption;
 import com.google.cloud.logging.Payload.JsonPayload;
-import com.google.cloud.logging.Payload.Type;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.protobuf.ListValue;
 import com.google.protobuf.Struct;
 import com.google.protobuf.Value;
 import java.util.ArrayList;
-import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import org.jspecify.nullness.Nullable;
 
-public class Instrumentation {
+public final class Instrumentation {
   public static final String DIAGNOSTIC_INFO_KEY = "logging.googleapis.com/diagnostic";
   public static final String INSTRUMENTATION_SOURCE_KEY = "instrumentation_source";
   public static final String INSTRUMENTATION_NAME_KEY = "name";
@@ -43,14 +42,14 @@ public class Instrumentation {
   public static final int MAX_DIAGNOSTIC_VALUE_LENGTH = 14;
   public static final int MAX_DIAGNOSTIC_ENTIES = 3;
   private static boolean instrumentationAdded = false;
-  private static Object instrumentationLock = new Object();
+  private static final Object instrumentationLock = new Object();
 
   /**
    * Populates entries with instrumentation info which is added in separate log entry
    *
-   * @param logEntries {Iterable<LogEntry>} The list of entries to be populated
-   * @return {Tuple<Boolean, Iterable<LogEntry>>} containing a flag if instrumentation info was
-   *     added or not and a modified list of log entries
+   * @param logEntries {@code Iterable<LogEntry>} The list of entries to be populated
+   * @return {@code Tuple<Boolean, Iterable<LogEntry>>} containing a flag if instrumentation info
+   *     was added or not and a modified list of log entries
    */
   public static Tuple<Boolean, Iterable<LogEntry>> populateInstrumentationInfo(
       Iterable<LogEntry> logEntries) {
@@ -61,7 +60,7 @@ public static Tuple<Boolean, Iterable<LogEntry>> populateInstrumentationInfo(
     for (LogEntry logEntry : logEntries) {
       // Check if LogEntry has a proper payload and also contains a diagnostic entry
       if (!isWritten
-          && logEntry.getPayload().getType() == Type.JSON
+          && logEntry.getPayload().getType() == Payload.Type.JSON
           && logEntry
               .<Payload.JsonPayload>getPayload()
               .getData()
@@ -77,7 +76,7 @@ public static Tuple<Boolean, Iterable<LogEntry>> populateInstrumentationInfo(
                   .getListValue();
           entries.add(createDiagnosticEntry(null, null, infoList));
           isWritten = true;
-        } catch (Exception ex) {
+        } catch (RuntimeException ex) {
           System.err.println("ERROR: unexpected exception in populateInstrumentationInfo: " + ex);
         }
       } else {
@@ -99,8 +98,8 @@ public static Tuple<Boolean, Iterable<LogEntry>> populateInstrumentationInfo(
    */
   public static WriteOption @Nullable [] addPartialSuccessOption(WriteOption[] options) {
     if (options == null) return options;
-    List<WriteOption> writeOptions = new ArrayList<WriteOption>();
-    writeOptions.addAll(Arrays.asList(options));
+    List<WriteOption> writeOptions = new ArrayList<>();
+    Collections.addAll(writeOptions, options);
     // Make sure we remove all partial success flags if any exist
     writeOptions.removeIf(
         option -> option.getOptionType() == WriteOption.OptionType.PARTIAL_SUCCESS);
@@ -133,18 +132,16 @@ private static LogEntry createDiagnosticEntry(
                             generateLibrariesList(libraryName, libraryVersion, existingLibraryList))
                         .build()))
             .build();
-    LogEntry entry =
-        LogEntry.newBuilder(
-                JsonPayload.of(
-                    Struct.newBuilder()
-                        .putAllFields(
-                            ImmutableMap.of(
-                                DIAGNOSTIC_INFO_KEY,
-                                Value.newBuilder().setStructValue(instrumentation).build()))
-                        .build()))
-            .setLogName(INSTRUMENTATION_LOG_NAME)
-            .build();
-    return entry;
+    return LogEntry.newBuilder(
+            JsonPayload.of(
+                Struct.newBuilder()
+                    .putAllFields(
+                        ImmutableMap.of(
+                            DIAGNOSTIC_INFO_KEY,
+                            Value.newBuilder().setStructValue(instrumentation).build()))
+                    .build()))
+        .setLogName(INSTRUMENTATION_LOG_NAME)
+        .build();
   }
 
   private static ListValue generateLibrariesList(
@@ -171,7 +168,7 @@ private static ListValue generateLibrariesList(
             libraryList.addValues(
                 Value.newBuilder().setStructValue(createInfoStruct(name, version)).build());
             if (libraryList.getValuesCount() == MAX_DIAGNOSTIC_ENTIES) break;
-          } catch (Exception ex) {
+          } catch (RuntimeException ex) {
           }
         }
       }
@@ -222,4 +219,6 @@ private static String truncateValue(String value) {
     if (Strings.isNullOrEmpty(value) || value.length() < MAX_DIAGNOSTIC_VALUE_LENGTH) return value;
     return value.substring(0, MAX_DIAGNOSTIC_VALUE_LENGTH) + "*";
   }
+
+  private Instrumentation() {}
 }