refactor(datastore): remove redundant null check for executionOptions in DatastoreImpl

Author: rahulmane-goog

java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreExecutionOptions.java

@@ -18,23 +18,24 @@
 
 import com.google.api.core.BetaApi;
 import com.google.cloud.datastore.models.ExplainOptions;
+import com.google.cloud.datastore.models.RequestOptions;
 import com.google.common.base.Objects;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
-import com.google.datastore.v1.RequestOptions;
 import java.util.Collections;
 import java.util.List;
-import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
+import org.jspecify.annotations.NullMarked;
+import org.jspecify.annotations.Nullable;
 
 /**
  * Class representing options for query execution in Google Cloud Datastore. Combines {@link
  * ExplainOptions}, {@link RequestOptions}, and {@link ReadOption}s.
  */
 @BetaApi
+@NullMarked
 public class DatastoreExecutionOptions {
 
-  private final ExplainOptions explainOptions;
+  private final @Nullable ExplainOptions explainOptions;
   private final RequestOptions requestOptions;
   private final List<ReadOption> readOptions;
 
@@ -44,17 +45,14 @@ private DatastoreExecutionOptions(Builder builder) {
     this.readOptions = ImmutableList.copyOf(builder.readOptions);
   }
 
-  @Nullable
-  public ExplainOptions getExplainOptions() {
+  public @Nullable ExplainOptions getExplainOptions() {
     return explainOptions;
   }
 
-  @Nullable
   public RequestOptions getRequestOptions() {
     return requestOptions;
   }
 
-  @Nonnull
   public List<ReadOption> getReadOptions() {
     return readOptions;
   }
@@ -89,8 +87,8 @@ public static DatastoreExecutionOptions getDefaultInstance() {
 
   /** Builder for {@link DatastoreExecutionOptions}. */
   public static class Builder {
-    private ExplainOptions explainOptions;
-    private RequestOptions requestOptions;
+    private @Nullable ExplainOptions explainOptions;
+    private RequestOptions requestOptions = RequestOptions.getDefaultInstance();
     private List<ReadOption> readOptions = Collections.emptyList();
 
     private Builder() {}
@@ -106,12 +104,13 @@ public Builder setExplainOptions(@Nullable ExplainOptions explainOptions) {
       return this;
     }
 
-    public Builder setRequestOptions(@Nullable RequestOptions requestOptions) {
+    public Builder setRequestOptions(RequestOptions requestOptions) {
+      Preconditions.checkNotNull(requestOptions, "requestOptions cannot be null");
       this.requestOptions = requestOptions;
       return this;
     }
 
-    public Builder setReadOptions(@Nonnull List<ReadOption> readOptions) {
+    public Builder setReadOptions(List<ReadOption> readOptions) {
       Preconditions.checkNotNull(readOptions, "readOptions cannot be null");
       this.readOptions = readOptions;
       return this;

java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java

@@ -16,6 +16,7 @@
 
 package com.google.cloud.datastore;
 
+import static com.google.cloud.datastore.RequestOptionsHelper.createRequestOptions;
 import static com.google.cloud.datastore.telemetry.TelemetryConstants.ATTRIBUTES_KEY_DEFERRED;
 import static com.google.cloud.datastore.telemetry.TelemetryConstants.ATTRIBUTES_KEY_DOCUMENT_COUNT;
 import static com.google.cloud.datastore.telemetry.TelemetryConstants.ATTRIBUTES_KEY_MISSING;
@@ -111,7 +112,7 @@ final class DatastoreImpl extends BaseService<DatastoreOptions> implements Datas
   private static final ExceptionHandler TRANSACTION_OPERATION_EXCEPTION_HANDLER =
       TransactionOperationExceptionHandler.build();
 
-  private final com.google.cloud.datastore.telemetry.TraceUtil otelTraceUtil =
+  private final TraceUtil otelTraceUtil =
       getOptions().getTraceUtil();
   private final DatastoreMetricsRecorder metricsRecorder;
   private final OpenTelemetry builtInOpenTelemetry;
@@ -180,7 +181,7 @@ static class TracedReadWriteTransactionCallable<T> implements Callable<T> {
 
     TracedReadWriteTransactionCallable(
         ReadWriteTransactionCallable<T> delegate,
-        @Nullable com.google.cloud.datastore.telemetry.TraceUtil.Span parentSpan) {
+        @Nullable TraceUtil.Span parentSpan) {
       this.delegate = delegate;
       this.parentSpan = parentSpan;
     }
@@ -362,13 +363,16 @@ public <T> QueryResults<T> run(
   @Override
   @BetaApi
   public <T> QueryResults<T> run(Query<T> query, DatastoreExecutionOptions executionOptions) {
+    Preconditions.checkNotNull(executionOptions, "executionOptions cannot be null");
     com.google.cloud.datastore.models.ExplainOptions explainOptions =
         executionOptions.getExplainOptions();
     return run(
         toReadOptionsPb(executionOptions.getReadOptions().toArray(new ReadOption[0])),
         query,
         explainOptions != null ? explainOptions.toPb() : null,
-        executionOptions.getRequestOptions());
+        executionOptions.getRequestOptions() != null
+            ? executionOptions.getRequestOptions().toPb()
+            : null);
   }
 
   @SuppressWarnings("unchecked")
@@ -435,7 +439,7 @@ RunQueryResponse runQuery(final RunQueryRequest requestPb) {
     return runWithObservability(
         () -> {
           RunQueryResponse response = datastoreRpc.runQuery(requestPb);
-          com.google.cloud.datastore.telemetry.TraceUtil.Span span = otelTraceUtil.getCurrentSpan();
+          TraceUtil.Span span = otelTraceUtil.getCurrentSpan();
           if (span != null) {
             span.addEvent(
                 spanName + " complete.",
@@ -488,7 +492,6 @@ public List<Key> allocateId(IncompleteKey... keys) {
   @Override
   @BetaApi
   public List<Key> allocateId(List<IncompleteKey> keys, DatastoreExecutionOptions executionOptions) {
-    Preconditions.checkNotNull(executionOptions, "executionOptions cannot be null");
     Preconditions.checkArgument(
         verifyIncompleteKeyType(keys), "keys must be IncompleteKey instances");
     if (keys.isEmpty()) {
@@ -501,7 +504,7 @@ public List<Key> allocateId(List<IncompleteKey> keys, DatastoreExecutionOptions
     requestPb.setProjectId(getOptions().getProjectId());
     requestPb.setDatabaseId(getOptions().getDatabaseId());
     requestPb.setRequestOptions(
-        RequestOptionsHelper.createRequestOptions(getOptions(), executionOptions));
+        createRequestOptions(getOptions(), executionOptions));
     AllocateIdsResponse responsePb = allocateIds(requestPb.build());
     ImmutableList.Builder<Key> keyList = ImmutableList.builder();
     for (com.google.datastore.v1.Key keyPb : responsePb.getKeysList()) {
@@ -546,7 +549,6 @@ public List<Entity> add(FullEntity<?>... entities) {
   @Override
   @BetaApi
   public List<Entity> add(List<FullEntity<?>> entities, DatastoreExecutionOptions executionOptions) {
-    Preconditions.checkNotNull(executionOptions, "executionOptions cannot be null");
     if (entities.isEmpty()) {
       return Collections.emptyList();
     }
@@ -663,7 +665,7 @@ Iterator<Entity> get(
     requestPb.setProjectId(getOptions().getProjectId());
     requestPb.setDatabaseId(getOptions().getDatabaseId());
     requestPb.setRequestOptions(
-        RequestOptionsHelper.createRequestOptions(getOptions(), executionOptions));
+        createRequestOptions(getOptions(), executionOptions));
     return new ResultsIterator(requestPb);
   }
 
@@ -707,7 +709,7 @@ LookupResponse lookup(final LookupRequest requestPb) {
     return runWithObservability(
         () -> {
           LookupResponse response = datastoreRpc.lookup(requestPb);
-          com.google.cloud.datastore.telemetry.TraceUtil.Span span = otelTraceUtil.getCurrentSpan();
+          TraceUtil.Span span = otelTraceUtil.getCurrentSpan();
           if (span != null) {
             span.addEvent(
                 spanName + " complete.",
@@ -738,15 +740,14 @@ public List<Key> reserveIds(Key... keys) {
   @Override
   @BetaApi
   public List<Key> reserveIds(List<Key> keys, DatastoreExecutionOptions executionOptions) {
-    Preconditions.checkNotNull(executionOptions, "executionOptions cannot be null");
     ReserveIdsRequest.Builder requestPb = ReserveIdsRequest.newBuilder();
     for (Key key : keys) {
       requestPb.addKeys(key.toPb());
     }
     requestPb.setProjectId(getOptions().getProjectId());
     requestPb.setDatabaseId(getOptions().getDatabaseId());
     requestPb.setRequestOptions(
-        RequestOptionsHelper.createRequestOptions(getOptions(), executionOptions));
+        createRequestOptions(getOptions(), executionOptions));
     ReserveIdsResponse responsePb = reserveIds(requestPb.build());
     ImmutableList.Builder<Key> keyList = ImmutableList.builder();
     if (responsePb.isInitialized()) {
@@ -773,7 +774,6 @@ public void update(Entity... entities) {
   @Override
   @BetaApi
   public void update(List<Entity> entities, DatastoreExecutionOptions executionOptions) {
-    Preconditions.checkNotNull(executionOptions, "executionOptions cannot be null");
     if (!entities.isEmpty()) {
       ImmutableList.Builder<Mutation> mutationsPb = ImmutableList.builder();
       Map<Key, Entity> dedupEntities = new LinkedHashMap<>();
@@ -801,7 +801,6 @@ public List<Entity> put(FullEntity<?>... entities) {
   @Override
   @BetaApi
   public List<Entity> put(List<FullEntity<?>> entities, DatastoreExecutionOptions executionOptions) {
-    Preconditions.checkNotNull(executionOptions, "executionOptions cannot be null");
     if (entities.isEmpty()) {
       return Collections.emptyList();
     }
@@ -842,7 +841,6 @@ public void delete(Key... keys) {
   @Override
   @BetaApi
   public void delete(List<Key> keys, DatastoreExecutionOptions executionOptions) {
-    Preconditions.checkNotNull(executionOptions, "executionOptions cannot be null");
     if (!keys.isEmpty()) {
       ImmutableList.Builder<Mutation> mutationsPb = ImmutableList.builder();
       Set<Key> dedupKeys = new LinkedHashSet<>(keys);
@@ -858,20 +856,15 @@ public KeyFactory newKeyFactory() {
     return DatastoreHelper.newKeyFactory(getOptions());
   }
 
-  private CommitResponse commitMutation(ImmutableList<Mutation> mutationsPb) {
-    return commitMutation(mutationsPb, DatastoreExecutionOptions.getDefaultInstance());
-  }
-
   private CommitResponse commitMutation(
       ImmutableList<Mutation> mutationsPb, DatastoreExecutionOptions executionOptions) {
     CommitRequest.Builder requestPb =
         CommitRequest.newBuilder()
             .setMode(CommitRequest.Mode.NON_TRANSACTIONAL)
             .setProjectId(getOptions().getProjectId())
             .setDatabaseId(getOptions().getDatabaseId())
-            .addAllMutations(mutationsPb);
-    requestPb.setRequestOptions(
-        RequestOptionsHelper.createRequestOptions(getOptions(), executionOptions));
+            .addAllMutations(mutationsPb)
+            .setRequestOptions(createRequestOptions(getOptions(), executionOptions));
     return commit(requestPb.build());
   }
 
@@ -883,7 +876,7 @@ CommitResponse commit(final CommitRequest requestPb) {
     return runWithObservability(
         () -> {
           CommitResponse response = datastoreRpc.commit(requestPb);
-          com.google.cloud.datastore.telemetry.TraceUtil.Span span = otelTraceUtil.getCurrentSpan();
+          TraceUtil.Span span = otelTraceUtil.getCurrentSpan();
           if (span != null) {
             span.addEvent(
                 spanName + " complete.",
@@ -925,16 +918,15 @@ void rollbackTransaction(ByteString transaction, DatastoreExecutionOptions execu
     requestPb.setTransaction(transaction);
     requestPb.setProjectId(getOptions().getProjectId());
     requestPb.setDatabaseId(getOptions().getDatabaseId());
-    requestPb.setRequestOptions(
-        RequestOptionsHelper.createRequestOptions(getOptions(), executionOptions));
+    requestPb.setRequestOptions(createRequestOptions(getOptions(), executionOptions));
     rollback(requestPb.build());
   }
 
   void rollback(final RollbackRequest requestPb) {
     runWithObservability(
         () -> {
           datastoreRpc.rollback(requestPb);
-          com.google.cloud.datastore.telemetry.TraceUtil.Span span = otelTraceUtil.getCurrentSpan();
+          TraceUtil.Span span = otelTraceUtil.getCurrentSpan();
           if (span != null) {
             span.addEvent(
                 SPAN_NAME_ROLLBACK,
@@ -954,7 +946,7 @@ private <T> T runWithObservability(
       String methodName,
       String spanName,
       ResultRetryAlgorithm<?> exceptionHandler) {
-    com.google.cloud.datastore.telemetry.TraceUtil.Span span = otelTraceUtil.startSpan(spanName);
+    TraceUtil.Span span = otelTraceUtil.startSpan(spanName);
 
     Stopwatch operationStopwatch = Stopwatch.createStarted();
     String operationStatus = StatusCode.Code.OK.toString();

java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java

@@ -17,6 +17,7 @@
 package com.google.cloud.datastore;
 
 import static com.google.cloud.datastore.Validator.validateNamespace;
+import static com.google.datastore.v1.client.DatastoreFactory.DEFAULT_HOST;
 
 import com.google.api.core.BetaApi;
 import com.google.api.gax.grpc.ChannelPoolSettings;
@@ -240,7 +241,7 @@ public DatastoreOptions build() {
         if (this.transportOptions instanceof GrpcTransportOptions) {
           this.setHost(DatastoreSettings.getDefaultEndpoint());
         } else if (this.transportOptions instanceof HttpTransportOptions) {
-          this.setHost(com.google.datastore.v1.client.DatastoreFactory.DEFAULT_HOST);
+          this.setHost(DEFAULT_HOST);
         }
       }
       return new DatastoreOptions(this);
@@ -260,6 +261,9 @@ public Builder setDatabaseId(String databaseId) {
     /**
      * Sets the request tags to be associated with all requests sent by this client.
      *
+     * <p>These instance-level tags will be merged with any request-level options or tags specified
+     * via {@link DatastoreExecutionOptions} before sending a request.
+     *
      * @param requestTags the list of request tags to set
      * @return the builder object
      */
@@ -340,7 +344,7 @@ public TransportChannelProvider getTransportChannelProvider() {
   @Override
   protected String getDefaultHost() {
     String host = System.getProperty(LOCAL_HOST_ENV_VAR, System.getenv(LOCAL_HOST_ENV_VAR));
-    return host != null ? host : com.google.datastore.v1.client.DatastoreFactory.DEFAULT_HOST;
+    return host != null ? host : DEFAULT_HOST;
   }
 
   @Override
@@ -393,6 +397,9 @@ public String getDatabaseId() {
   /**
    * Returns the request tags to be associated with all requests sent by this client.
    *
+   * <p>These instance-level tags are merged with any request-level options or tags passed via {@link
+   * DatastoreExecutionOptions} before sending a request.
+   *
    * @return the request tags
    */
   public List<String> getRequestTags() {

java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/QueryResultsImpl.java

@@ -16,6 +16,7 @@
 
 package com.google.cloud.datastore;
 
+import static com.google.cloud.datastore.RequestOptionsHelper.createRequestOptions;
 import com.google.api.core.BetaApi;
 import com.google.cloud.datastore.Query.ResultType;
 import com.google.cloud.datastore.models.ExplainMetrics;
@@ -130,10 +131,8 @@ private void sendRequest() {
     if (explainOptions != null) {
       requestPb.setExplainOptions(explainOptions);
     }
-    if (requestOptions != null) {
-      requestPb.setRequestOptions(
-          RequestOptionsHelper.createRequestOptions(datastore.getOptions(), requestOptions));
-    }
+    requestPb.setRequestOptions(
+        createRequestOptions(datastore.getOptions(), requestOptions));
     query.populatePb(requestPb);
     runQueryResponsePb = datastore.runQuery(requestPb.build());
     mostRecentQueryPb = requestPb.getQuery();

java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/ReadOption.java

@@ -25,7 +25,6 @@
 import com.google.datastore.v1.RequestOptions;
 import com.google.protobuf.ByteString;
 import java.io.Serializable;
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
@@ -183,18 +182,20 @@ public RequestOptions getRequestOptions() {
     public static <Q extends Query<?>> QueryConfig<Q> createWithDatastoreExecutionOptions(
         Q query, DatastoreExecutionOptions executionOptions) {
       Preconditions.checkNotNull(query, "query cannot be null");
-      if (executionOptions != null) {
-        ExplainOptions explainOptions =
-            executionOptions.getExplainOptions() != null
-                ? executionOptions.getExplainOptions().toPb()
-                : null;
-        return new QueryConfig<>(
-            query,
-            explainOptions,
-            executionOptions.getReadOptions(),
-            executionOptions.getRequestOptions());
-      }
-      return new QueryConfig<>(query, null, Collections.emptyList(), null);
+      Preconditions.checkNotNull(executionOptions, "executionOptions cannot be null");
+      ExplainOptions explainOptions =
+          executionOptions.getExplainOptions() != null
+              ? executionOptions.getExplainOptions().toPb()
+              : null;
+      RequestOptions requestOptions =
+          executionOptions.getRequestOptions() != null
+              ? executionOptions.getRequestOptions().toPb()
+              : null;
+      return new QueryConfig<>(
+          query,
+          explainOptions,
+          executionOptions.getReadOptions(),
+          requestOptions);
     }
   }
 }