diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java index 1c6edb39944b..48d4adbf065f 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java @@ -19,7 +19,6 @@ import static com.google.cloud.datastore.Validator.validateNamespace; import com.google.api.core.BetaApi; -import com.google.api.core.ObsoleteApi; import com.google.api.gax.grpc.ChannelPoolSettings; import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; import com.google.api.gax.rpc.TransportChannelProvider; @@ -35,11 +34,13 @@ import com.google.cloud.grpc.GrpcTransportOptions; import com.google.cloud.http.HttpTransportOptions; import com.google.common.base.MoreObjects; +import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableSet; import java.io.IOException; import java.lang.reflect.Method; import java.util.Objects; import java.util.Set; +import java.util.logging.Logger; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -47,6 +48,7 @@ public class DatastoreOptions extends ServiceOptions SCOPES = ImmutableSet.of(DATASTORE_SCOPE); private static final String DEFAULT_DATABASE_ID = ""; @@ -72,9 +74,7 @@ public class DatastoreOptions extends ServiceOptionsSets the transport to gRPC. Note this functionality is experimental and subject to change. */ + @Deprecated @BetaApi public Builder setTransportOptions(GrpcTransportOptions transportOptions) { - this.transportOptions = transportOptions; - return super.setTransportOptions(transportOptions); + return setTransportOptions((TransportOptions) transportOptions); } @Override @@ -185,10 +214,28 @@ public Builder setChannelProvider(TransportChannelProvider channelProvider) { return this; } + /** + * Builds the {@link DatastoreOptions} instance. + * + *

If the host is not explicitly set, it defaults to the transport-specific default host: + * + *

+ * + * @return the {@link DatastoreOptions} instance + */ @Override public DatastoreOptions build() { - if (this.host == null && this.transportOptions instanceof GrpcTransportOptions) { - this.setHost(DatastoreSettings.getDefaultEndpoint()); + if (this.host == null) { + // Use whatever host value the user passes in, otherwise use the transport specific default + // host values + 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); + } } return new DatastoreOptions(this); } @@ -218,15 +265,6 @@ public Builder setOpenTelemetryOptions( } } - private static TransportChannelProvider validateChannelProvider( - TransportChannelProvider channelProvider) { - if (channelProvider != null && !(channelProvider instanceof InstantiatingGrpcChannelProvider)) { - throw new IllegalArgumentException( - "Only GRPC channels are allowed for " + API_SHORT_NAME + "."); - } - return channelProvider; - } - private DatastoreOptions(Builder builder) { super(DatastoreFactory.class, DatastoreRpcFactory.class, builder, new DatastoreDefaults()); @@ -239,9 +277,9 @@ private DatastoreOptions(Builder builder) { namespace = MoreObjects.firstNonNull(builder.namespace, defaultNamespace()); databaseId = MoreObjects.firstNonNull(builder.databaseId, DEFAULT_DATABASE_ID); + // ChannelProvider is used by GAX but HttpJson does not use it so we safely ignore it. if (getTransportOptions() instanceof HttpTransportOptions && builder.channelProvider != null) { - throw new IllegalArgumentException( - "Only gRPC transport allows setting of channel provider or credentials provider"); + logger.warning("Channel provider is ignored for HttpJson transport."); } else if (getTransportOptions() instanceof GrpcTransportOptions) { if (builder.channelProvider == null) { // Set the default gRPC connection pool to be configured with a minimum of 1 channel. @@ -310,8 +348,8 @@ public TransportOptions getDefaultTransportOptions() { return TRANSPORT_OPTIONS; } - public static HttpTransportOptions.Builder getDefaultTransportOptionsBuilder() { - return HttpTransportOptions.newBuilder(); + public static GrpcTransportOptions.Builder getDefaultTransportOptionsBuilder() { + return GrpcTransportOptions.newBuilder(); } } diff --git a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java index e1e74dba60fa..427882fd86e7 100644 --- a/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java +++ b/java-datastore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java @@ -22,6 +22,7 @@ import com.google.api.core.InternalApi; import com.google.cloud.NoCredentials; import com.google.cloud.ServiceOptions; +import com.google.cloud.TransportOptions; import com.google.cloud.datastore.DatastoreOptions; import com.google.cloud.grpc.GrpcTransportOptions; import com.google.cloud.testing.BaseEmulatorHelper; @@ -242,11 +243,14 @@ public DatastoreOptions getOptions(String namespace) { } /** - * Returns a {@link DatastoreOptions} instance that sets the host to use the Datastore emulator on - * localhost. The transportOptions is set to {@code grpcTransportOptions}. + * Prefer to set the TransportOptions via the Options Builder. + * + *

Returns a {@link DatastoreOptions} instance that sets the host to use the Datastore emulator + * on localhost. The transportOptions is set to {@code grpcTransportOptions}. */ + @Deprecated public DatastoreOptions getGrpcTransportOptions(GrpcTransportOptions grpcTransportOptions) { - return optionsBuilder.setTransportOptions(grpcTransportOptions).build(); + return optionsBuilder.setTransportOptions((TransportOptions) grpcTransportOptions).build(); } public DatastoreOptions.Builder setNamespace(String namespace) { diff --git a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreImplMetricsTest.java b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreImplMetricsTest.java index 16c27dac97fa..db384255cd94 100644 --- a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreImplMetricsTest.java +++ b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreImplMetricsTest.java @@ -21,6 +21,7 @@ import com.google.api.gax.rpc.StatusCode; import com.google.cloud.NoCredentials; import com.google.cloud.ServiceOptions; +import com.google.cloud.TransportOptions; import com.google.cloud.datastore.spi.DatastoreRpcFactory; import com.google.cloud.datastore.spi.v1.DatastoreRpc; import com.google.cloud.datastore.telemetry.DatastoreMetricsRecorder; @@ -115,7 +116,7 @@ public void setUp() { .build()); if (TelemetryConstants.Transport.GRPC.equals(transport)) { - builder.setTransportOptions(GrpcTransportOptions.newBuilder().build()); + builder.setTransportOptions((TransportOptions) GrpcTransportOptions.newBuilder().build()); } else { builder.setTransportOptions(HttpTransportOptions.newBuilder().build()); } diff --git a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java index e3c55b349c49..bff0fdfcfc95 100644 --- a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java +++ b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java @@ -27,6 +27,7 @@ import com.google.api.gax.grpc.ChannelPoolSettings; import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; import com.google.cloud.NoCredentials; +import com.google.cloud.TransportOptions; import com.google.cloud.datastore.spi.DatastoreRpcFactory; import com.google.cloud.datastore.spi.v1.DatastoreRpc; import com.google.cloud.datastore.v1.DatastoreSettings; @@ -34,7 +35,6 @@ import com.google.cloud.http.HttpTransportOptions; import com.google.datastore.v1.client.DatastoreFactory; import org.easymock.EasyMock; -import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -217,7 +217,7 @@ public void testGrpcDefaultChannelConfigurations() { .setServiceRpcFactory(datastoreRpcFactory) .setProjectId(PROJECT_ID) .setDatabaseId(DATABASE_ID) - .setTransportOptions(GrpcTransportOptions.newBuilder().build()) + .setTransportOptions((TransportOptions) GrpcTransportOptions.newBuilder().build()) .setCredentials(NoCredentials.getInstance()) .setHost("http://localhost:" + PORT) .build(); @@ -250,7 +250,7 @@ public void testCustomChannelAndCredentials() { .setServiceRpcFactory(datastoreRpcFactory) .setProjectId(PROJECT_ID) .setDatabaseId(DATABASE_ID) - .setTransportOptions(GrpcTransportOptions.newBuilder().build()) + .setTransportOptions((TransportOptions) GrpcTransportOptions.newBuilder().build()) .setChannelProvider(channelProvider) .setCredentials(NoCredentials.getInstance()) .setHost("http://localhost:" + PORT) @@ -259,35 +259,23 @@ public void testCustomChannelAndCredentials() { } @Test - public void testInvalidConfigForHttp() { - DatastoreOptions.Builder options = + public void testTransport() { + // default grpc transport + assertThat(options.build().getTransportOptions()).isInstanceOf(GrpcTransportOptions.class); + + // custom http transport + DatastoreOptions httpTransportOptions = DatastoreOptions.newBuilder() - .setServiceRpcFactory(datastoreRpcFactory) - .setProjectId(PROJECT_ID) - .setDatabaseId(DATABASE_ID) .setTransportOptions(HttpTransportOptions.newBuilder().build()) - .setChannelProvider( - DatastoreSettings.defaultGrpcTransportProviderBuilder() - .setChannelPoolSettings( - ChannelPoolSettings.builder() - .setInitialChannelCount(10) - .setMaxChannelCount(20) - .build()) - .build()) + .setProjectId(PROJECT_ID) .setCredentials(NoCredentials.getInstance()) - .setHost("http://localhost:" + PORT); - Assert.assertThrows(IllegalArgumentException.class, options::build); - } - - @Test - public void testTransport() { - // default http transport - assertThat(options.build().getTransportOptions()).isInstanceOf(HttpTransportOptions.class); + .build(); + assertThat(httpTransportOptions.getTransportOptions()).isInstanceOf(HttpTransportOptions.class); // custom grpc transport DatastoreOptions grpcTransportOptions = DatastoreOptions.newBuilder() - .setTransportOptions(GrpcTransportOptions.newBuilder().build()) + .setTransportOptions((TransportOptions) GrpcTransportOptions.newBuilder().build()) .setProjectId(PROJECT_ID) .setCredentials(NoCredentials.getInstance()) .build(); @@ -300,7 +288,7 @@ public void testTransport() { public void testHostWithGrpcAndHttp() { DatastoreOptions grpcTransportOptions = DatastoreOptions.newBuilder() - .setTransportOptions(GrpcTransportOptions.newBuilder().build()) + .setTransportOptions((TransportOptions) GrpcTransportOptions.newBuilder().build()) .setProjectId(PROJECT_ID) .setCredentials(NoCredentials.getInstance()) .build(); @@ -310,15 +298,23 @@ public void testHostWithGrpcAndHttp() { String customHost = "http://localhost:" + PORT; DatastoreOptions grpcTransportOptionsCustomHost = DatastoreOptions.newBuilder() - .setTransportOptions(GrpcTransportOptions.newBuilder().build()) + .setTransportOptions((TransportOptions) GrpcTransportOptions.newBuilder().build()) .setHost(customHost) .setProjectId(PROJECT_ID) .setCredentials(NoCredentials.getInstance()) .build(); assertThat(grpcTransportOptionsCustomHost.getHost()).isEqualTo(customHost); + DatastoreOptions defaultTransportOptions = + DatastoreOptions.newBuilder() + .setProjectId(PROJECT_ID) + .setCredentials(NoCredentials.getInstance()) + .build(); + assertThat(defaultTransportOptions.getHost()).isEqualTo(DatastoreSettings.getDefaultEndpoint()); + DatastoreOptions httpTransportOptions = DatastoreOptions.newBuilder() + .setTransportOptions(HttpTransportOptions.newBuilder().build()) .setProjectId(PROJECT_ID) .setCredentials(NoCredentials.getInstance()) .build(); @@ -326,6 +322,7 @@ public void testHostWithGrpcAndHttp() { DatastoreOptions httpTransportOptionsCustomHost = DatastoreOptions.newBuilder() + .setTransportOptions(HttpTransportOptions.newBuilder().build()) .setHost(customHost) .setProjectId(PROJECT_ID) .setCredentials(NoCredentials.getInstance()) diff --git a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreTestGrpc.java b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreTestGrpc.java index cb242f2af84a..3830a17dc2aa 100644 --- a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreTestGrpc.java +++ b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreTestGrpc.java @@ -17,7 +17,6 @@ package com.google.cloud.datastore; import com.google.cloud.datastore.testing.LocalDatastoreHelper; -import com.google.cloud.grpc.GrpcTransportOptions; import com.google.common.truth.Truth; import java.io.IOException; import java.time.Duration; @@ -32,8 +31,7 @@ public class DatastoreTestGrpc extends AbstractDatastoreTest { private static final LocalDatastoreHelper helper = LocalDatastoreHelper.create(1.0, 9090); - private static DatastoreOptions options = - helper.getGrpcTransportOptions(GrpcTransportOptions.newBuilder().build()); + private static DatastoreOptions options = helper.getOptions(); private static Datastore datastore = options.getService(); public DatastoreTestGrpc(DatastoreOptions options, Datastore datastore) { @@ -48,7 +46,7 @@ public static Iterable data() { @BeforeClass public static void beforeClass() throws IOException, InterruptedException { helper.start(); - options = helper.getGrpcTransportOptions(GrpcTransportOptions.newBuilder().build()); + options = helper.getOptions(); datastore = options.getService(); } diff --git a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreTestHttp.java b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreTestHttp.java index a73cae8e4e5d..98813442c743 100644 --- a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreTestHttp.java +++ b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreTestHttp.java @@ -17,7 +17,7 @@ package com.google.cloud.datastore; import com.google.cloud.datastore.testing.LocalDatastoreHelper; -import com.google.cloud.grpc.GrpcTransportOptions; +import com.google.cloud.http.HttpTransportOptions; import java.io.IOException; import java.time.Duration; import java.util.Arrays; @@ -46,7 +46,10 @@ public static Iterable data() { @BeforeClass public static void beforeClass() throws IOException, InterruptedException { helper.start(); - options = helper.getGrpcTransportOptions(GrpcTransportOptions.newBuilder().build()); + options = + helper.getOptions().toBuilder() + .setTransportOptions(HttpTransportOptions.newBuilder().build()) + .build(); datastore = options.getService(); } diff --git a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreClientSideMetrics.java b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreClientSideMetrics.java index 2cfd1036ec6d..7303bf1e6de2 100644 --- a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreClientSideMetrics.java +++ b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreClientSideMetrics.java @@ -28,7 +28,6 @@ import com.google.cloud.datastore.Key; import com.google.cloud.datastore.telemetry.TelemetryConstants; import com.google.cloud.grpc.GrpcTransportOptions; -import com.google.cloud.http.HttpTransportOptions; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.sdk.OpenTelemetrySdk; import io.opentelemetry.sdk.metrics.SdkMeterProvider; @@ -124,9 +123,9 @@ public void setUp() { .build()); if (transportOptions instanceof GrpcTransportOptions) { - builder.setTransportOptions((GrpcTransportOptions) transportOptions); + builder.setTransportOptions(transportOptions); } else { - builder.setTransportOptions((HttpTransportOptions) transportOptions); + builder.setTransportOptions(transportOptions); } datastore = builder.build().getService(); diff --git a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/testing/ITLocalDatastoreHelperTest.java b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/testing/ITLocalDatastoreHelperTest.java index 3b07254d72e8..a4d0e1178cb4 100644 --- a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/testing/ITLocalDatastoreHelperTest.java +++ b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/testing/ITLocalDatastoreHelperTest.java @@ -24,7 +24,6 @@ import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; -import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; import com.google.cloud.NoCredentials; import com.google.cloud.datastore.Datastore; import com.google.cloud.datastore.DatastoreException; @@ -180,17 +179,18 @@ public void testOptions() { public void testDefaultHttpTransportOptions() { LocalDatastoreHelper helper = LocalDatastoreHelper.create(); DatastoreOptions options = helper.getOptions(); - assertThat(options.getTransportOptions()).isInstanceOf(HttpTransportOptions.class); + assertThat(options.getTransportOptions()).isInstanceOf(GrpcTransportOptions.class); } @Test - public void testSetGrpcTransportOptions() { + public void testSetHttpTransportOptions() { LocalDatastoreHelper helper = LocalDatastoreHelper.create(); DatastoreOptions options = - helper.getGrpcTransportOptions(GrpcTransportOptions.newBuilder().build()); - assertThat(options.getTransportOptions()).isInstanceOf(GrpcTransportOptions.class); - assertThat(options.getTransportChannelProvider()) - .isInstanceOf(InstantiatingGrpcChannelProvider.class); + helper.getOptions().toBuilder() + .setTransportOptions(HttpTransportOptions.newBuilder().build()) + .build(); + assertThat(options.getTransportOptions()).isInstanceOf(HttpTransportOptions.class); + assertNull(options.getTransportChannelProvider()); } @Test diff --git a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/testing/RemoteDatastoreHelper.java b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/testing/RemoteDatastoreHelper.java index 412af667a7cb..6fbf4d3a7913 100644 --- a/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/testing/RemoteDatastoreHelper.java +++ b/java-datastore/google-cloud-datastore/src/test/java/com/google/cloud/datastore/testing/RemoteDatastoreHelper.java @@ -114,8 +114,7 @@ public static RemoteDatastoreHelper create( .setNamespace(UUID.randomUUID().toString()) .setRetrySettings(retrySettings()); if (transportOptions instanceof GrpcTransportOptions) { - datastoreOptionBuilder = - datastoreOptionBuilder.setTransportOptions((GrpcTransportOptions) transportOptions); + datastoreOptionBuilder = datastoreOptionBuilder.setTransportOptions(transportOptions); } else { datastoreOptionBuilder = datastoreOptionBuilder.setTransportOptions(transportOptions); }