From 8585c4ce138c3c7b7606c87343dab2a036b0ac01 Mon Sep 17 00:00:00 2001 From: kolea2 Date: Thu, 18 Apr 2024 10:53:03 -0400 Subject: [PATCH 1/7] feat: allow opt-in for gRPC, have datastore-v1-proto-client as default transport --- README.md | 8 +- google-cloud-datastore/pom.xml | 8 + .../cloud/datastore/DatastoreOptions.java | 28 +-- .../datastore/spi/v1/HttpDatastoreRpc.java | 204 +++++++++++++----- .../testing/RemoteDatastoreHelper.java | 17 +- .../cloud/datastore/DatastoreOptionsTest.java | 25 +-- .../google/cloud/datastore/DatastoreTest.java | 21 +- .../datastore/it/AbstractITDatastoreTest.java | 11 +- .../datastore/it/ITDatastoreTestGrpc.java | 9 +- .../datastore/it/ITDatastoreTestHttp.java | 9 +- 10 files changed, 215 insertions(+), 125 deletions(-) diff --git a/README.md b/README.md index c582f2b63..dcb40a9cc 100644 --- a/README.md +++ b/README.md @@ -50,20 +50,20 @@ If you are using Maven without the BOM, add this to your dependencies: If you are using Gradle 5.x or later, add this to your dependencies: ```Groovy -implementation platform('com.google.cloud:libraries-bom:26.34.0') +implementation platform('com.google.cloud:libraries-bom:26.37.0') implementation 'com.google.cloud:google-cloud-datastore' ``` If you are using Gradle without BOM, add this to your dependencies: ```Groovy -implementation 'com.google.cloud:google-cloud-datastore:2.18.5' +implementation 'com.google.cloud:google-cloud-datastore:2.19.0' ``` If you are using SBT, add this to your dependencies: ```Scala -libraryDependencies += "com.google.cloud" % "google-cloud-datastore" % "2.18.5" +libraryDependencies += "com.google.cloud" % "google-cloud-datastore" % "2.19.0" ``` @@ -380,7 +380,7 @@ Java is a registered trademark of Oracle and/or its affiliates. [kokoro-badge-link-5]: http://storage.googleapis.com/cloud-devrel-public/java/badges/java-datastore/java11.html [stability-image]: https://img.shields.io/badge/stability-stable-green [maven-version-image]: https://img.shields.io/maven-central/v/com.google.cloud/google-cloud-datastore.svg -[maven-version-link]: https://central.sonatype.com/artifact/com.google.cloud/google-cloud-datastore/2.18.5 +[maven-version-link]: https://central.sonatype.com/artifact/com.google.cloud/google-cloud-datastore/2.19.0 [authentication]: https://github.com/googleapis/google-cloud-java#authentication [auth-scopes]: https://developers.google.com/identity/protocols/oauth2/scopes [predefined-iam-roles]: https://cloud.google.com/iam/docs/understanding-roles#predefined_roles diff --git a/google-cloud-datastore/pom.xml b/google-cloud-datastore/pom.xml index e07c6912d..45ffa5998 100644 --- a/google-cloud-datastore/pom.xml +++ b/google-cloud-datastore/pom.xml @@ -42,6 +42,10 @@ com.google.api.grpc proto-google-cloud-datastore-admin-v1 + + com.google.cloud.datastore + datastore-v1-proto-client + io.grpc grpc-api @@ -102,6 +106,10 @@ com.google.oauth-client google-oauth-client + + com.google.auth + google-auth-library-oauth2-http + io.opencensus opencensus-api diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java index c852fdb7f..aa29b6624 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java @@ -18,6 +18,7 @@ import static com.google.cloud.datastore.Validator.validateNamespace; +import com.google.api.core.BetaApi; import com.google.api.gax.core.CredentialsProvider; import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; import com.google.api.gax.rpc.TransportChannelProvider; @@ -74,15 +75,8 @@ public ServiceRpc create(DatastoreOptions options) { try { if (options.getTransportOptions() instanceof GrpcTransportOptions) { return new GrpcDatastoreRpc(options); - } else if (options.getTransportOptions() instanceof HttpTransportOptions) { - // todo see if we can remove this check - if (DatastoreUtils.isEmulator(options)) { - throw new IllegalArgumentException("Only GRPC channels are allowed for emulator."); - } - return new HttpDatastoreRpc(options); } else { - throw new IllegalArgumentException( - "unknown transport type: " + options.getTransportOptions()); + return new HttpDatastoreRpc(options); } } catch (IOException e) { throw new RuntimeException(e); @@ -109,6 +103,15 @@ private Builder(DatastoreOptions options) { @Override public Builder setTransportOptions(TransportOptions transportOptions) { + if (!(transportOptions instanceof HttpTransportOptions)) { + throw new IllegalArgumentException( + "Only http transport is allowed for " + API_SHORT_NAME + "."); + } + return super.setTransportOptions(transportOptions); + } + + @BetaApi + public Builder setTransportOptions(GrpcTransportOptions transportOptions) { return super.setTransportOptions(transportOptions); } @@ -118,6 +121,7 @@ public Builder setTransportOptions(TransportOptions transportOptions) { * @param channelProvider A InstantiatingGrpcChannelProvider object that defines the transport * provider for this client. */ + @BetaApi public Builder setChannelProvider(TransportChannelProvider channelProvider) { this.channelProvider = validateChannelProvider(channelProvider); return this; @@ -129,6 +133,7 @@ public Builder setChannelProvider(TransportChannelProvider channelProvider) { * @param credentialsProvider A CredentialsProvider object that defines the credential provider * for this client. */ + @BetaApi public Builder setCredentialsProvider(CredentialsProvider credentialsProvider) { this.credentialsProvider = credentialsProvider; return this; @@ -153,7 +158,7 @@ public Builder setDatabaseId(String databaseId) { private static TransportChannelProvider validateChannelProvider( TransportChannelProvider channelProvider) { - if (!(channelProvider instanceof InstantiatingGrpcChannelProvider)) { + if (channelProvider != null && !(channelProvider instanceof InstantiatingGrpcChannelProvider)) { throw new IllegalArgumentException( "Only GRPC channels are allowed for " + API_SHORT_NAME + "."); } @@ -165,7 +170,6 @@ private DatastoreOptions(Builder builder) { namespace = MoreObjects.firstNonNull(builder.namespace, defaultNamespace()); databaseId = MoreObjects.firstNonNull(builder.databaseId, DEFAULT_DATABASE_ID); - // todo see if we can update this if (getTransportOptions() instanceof HttpTransportOptions && (builder.channelProvider != null || builder.credentialsProvider != null)) { throw new IllegalArgumentException( @@ -222,8 +226,8 @@ public TransportOptions getDefaultTransportOptions() { return TRANSPORT_OPTIONS; } - public static GrpcTransportOptions.Builder getDefaultTransportOptionsBuilder() { - return GrpcTransportOptions.newBuilder(); + public static HttpTransportOptions.Builder getDefaultTransportOptionsBuilder() { + return HttpTransportOptions.newBuilder(); } } diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/HttpDatastoreRpc.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/HttpDatastoreRpc.java index f5a6ebb42..67e7efba8 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/HttpDatastoreRpc.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/HttpDatastoreRpc.java @@ -16,17 +16,15 @@ package com.google.cloud.datastore.spi.v1; -import static com.google.cloud.datastore.DatastoreUtils.isLocalHost; -import static com.google.cloud.datastore.spi.v1.RpcUtils.retrySettingSetter; -import static java.util.concurrent.TimeUnit.SECONDS; - +import com.google.api.client.http.HttpRequest; +import com.google.api.client.http.HttpRequestInitializer; +import com.google.api.client.http.HttpTransport; import com.google.api.core.InternalApi; -import com.google.api.gax.core.BackgroundResource; -import com.google.api.gax.rpc.ClientContext; +import com.google.cloud.datastore.DatastoreException; import com.google.cloud.datastore.DatastoreOptions; -import com.google.cloud.datastore.v1.DatastoreSettings; -import com.google.cloud.datastore.v1.stub.DatastoreStubSettings; -import com.google.cloud.datastore.v1.stub.HttpJsonDatastoreStub; +import com.google.cloud.datastore.TraceUtil; +import com.google.cloud.http.CensusHttpModule; +import com.google.cloud.http.HttpTransportOptions; import com.google.datastore.v1.AllocateIdsRequest; import com.google.datastore.v1.AllocateIdsResponse; import com.google.datastore.v1.BeginTransactionRequest; @@ -43,105 +41,193 @@ import com.google.datastore.v1.RunAggregationQueryResponse; import com.google.datastore.v1.RunQueryRequest; import com.google.datastore.v1.RunQueryResponse; +import com.google.datastore.v1.client.DatastoreFactory; import java.io.IOException; +import java.net.InetAddress; +import java.net.URL; @InternalApi public class HttpDatastoreRpc implements DatastoreRpc { - private final ClientContext clientContext; - private final HttpJsonDatastoreStub datastoreStub; + private final com.google.datastore.v1.client.Datastore client; + + public HttpDatastoreRpc(DatastoreOptions options) { + HttpTransportOptions httpTransportOptions = + (HttpTransportOptions) options.getTransportOptions(); + HttpTransport transport = httpTransportOptions.getHttpTransportFactory().create(); + com.google.datastore.v1.client.DatastoreOptions.Builder clientBuilder = + new com.google.datastore.v1.client.DatastoreOptions.Builder() + .projectId(options.getProjectId()) + .initializer(getHttpRequestInitializer(options, httpTransportOptions)) + .transport(transport); + String normalizedHost = options.getHost() != null ? options.getHost().toLowerCase() : ""; + + // the default gRPC host was set, reset to the default HTTP host + if (normalizedHost.equals("datastore.googleapis.com:443")) { + normalizedHost = DatastoreFactory.DEFAULT_HOST; + } - private boolean closed; + if (isLocalHost(normalizedHost)) { + clientBuilder = clientBuilder.localHost(removeScheme(normalizedHost)); + } else if (!removeScheme(com.google.datastore.v1.client.DatastoreFactory.DEFAULT_HOST) + .equals(removeScheme(normalizedHost)) + && !normalizedHost.isEmpty()) { + String fullUrl = normalizedHost; + if (fullUrl.charAt(fullUrl.length() - 1) != '/') { + fullUrl = fullUrl + '/'; + } + fullUrl = + fullUrl + + com.google.datastore.v1.client.DatastoreFactory.VERSION + + "/projects/" + + options.getProjectId(); + clientBuilder = clientBuilder.projectId(null).projectEndpoint(fullUrl); + } + client = com.google.datastore.v1.client.DatastoreFactory.get().create(clientBuilder.build()); + } - public HttpDatastoreRpc(DatastoreOptions datastoreOptions) throws IOException { - DatastoreSettings datastoreSettings = - new DatastoreSettingsBuilder(DatastoreSettings.newBuilder().build()) - .setInternalHeaderProvider( - DatastoreStubSettings.defaultHttpJsonApiClientHeaderProviderBuilder().build()) - .setTransportChannelProvider( - DatastoreStubSettings.defaultHttpJsonTransportProviderBuilder().build()) - .setEndpoint(getHost(datastoreOptions)) - .build(); + private HttpRequestInitializer getHttpRequestInitializer( + final DatastoreOptions options, HttpTransportOptions httpTransportOptions) { + // Open Census initialization + CensusHttpModule censusHttpModule = + new CensusHttpModule(TraceUtil.getInstance().getTracer(), true); + final HttpRequestInitializer censusHttpModuleHttpRequestInitializer = + censusHttpModule.getHttpRequestInitializer( + httpTransportOptions.getHttpRequestInitializer(options)); + + final String applicationName = options.getApplicationName(); + return new HttpRequestInitializer() { + @Override + public void initialize(HttpRequest httpRequest) throws IOException { + censusHttpModuleHttpRequestInitializer.initialize(httpRequest); + httpRequest.getHeaders().setUserAgent(applicationName); + } + }; + } - clientContext = ClientContext.create(datastoreSettings); + private static boolean isLocalHost(String host) { + if (!host.isEmpty()) { + try { + String normalizedHost = "http://" + removeScheme(host); + InetAddress hostAddr = InetAddress.getByName(new URL(normalizedHost).getHost()); + return hostAddr.isAnyLocalAddress() || hostAddr.isLoopbackAddress(); + } catch (Exception e) { + // ignore + } + } + return false; + } - DatastoreStubSettings datastoreStubSettings = - DatastoreStubSettings.newBuilder(clientContext) - .applyToAllUnaryMethods(retrySettingSetter(datastoreOptions)) - .build(); + private static String removeScheme(String url) { + if (url != null) { + if (url.startsWith("https://")) { + return url.substring("https://".length()); + } else if (url.startsWith("http://")) { + return url.substring("http://".length()); + } + } + return url; + } + + private static DatastoreException translate( + com.google.datastore.v1.client.DatastoreException exception) { + return translate(exception, true); + } - datastoreStub = HttpJsonDatastoreStub.create(datastoreStubSettings); + private static DatastoreException translate( + com.google.datastore.v1.client.DatastoreException exception, boolean idempotent) { + String reason = ""; + if (exception.getCode() != null) { + reason = exception.getCode().name(); + } + if (reason.isEmpty()) { + if (exception.getCause() instanceof IOException) { + return new DatastoreException((IOException) exception.getCause()); + } + } + return new DatastoreException( + exception.getCode().getNumber(), exception.getMessage(), reason, idempotent, exception); } @Override public AllocateIdsResponse allocateIds(AllocateIdsRequest request) { - return this.datastoreStub.allocateIdsCallable().call(request); + try { + return client.allocateIds(request); + } catch (com.google.datastore.v1.client.DatastoreException ex) { + throw translate(ex); + } } @Override public BeginTransactionResponse beginTransaction(BeginTransactionRequest request) { - return this.datastoreStub.beginTransactionCallable().call(request); + try { + return client.beginTransaction(request); + } catch (com.google.datastore.v1.client.DatastoreException ex) { + throw translate(ex); + } } @Override public CommitResponse commit(CommitRequest request) { - return this.datastoreStub.commitCallable().call(request); + try { + return client.commit(request); + } catch (com.google.datastore.v1.client.DatastoreException ex) { + throw translate(ex, request.getMode() == CommitRequest.Mode.NON_TRANSACTIONAL); + } } @Override public LookupResponse lookup(LookupRequest request) { - return this.datastoreStub.lookupCallable().call(request); + try { + return client.lookup(request); + } catch (com.google.datastore.v1.client.DatastoreException ex) { + throw translate(ex); + } } @Override public ReserveIdsResponse reserveIds(ReserveIdsRequest request) { - return this.datastoreStub.reserveIdsCallable().call(request); + try { + return client.reserveIds(request); + } catch (com.google.datastore.v1.client.DatastoreException ex) { + throw translate(ex); + } } @Override public RollbackResponse rollback(RollbackRequest request) { - return this.datastoreStub.rollbackCallable().call(request); + try { + return client.rollback(request); + } catch (com.google.datastore.v1.client.DatastoreException ex) { + throw translate(ex); + } } @Override public RunQueryResponse runQuery(RunQueryRequest request) { - return this.datastoreStub.runQueryCallable().call(request); + try { + return client.runQuery(request); + } catch (com.google.datastore.v1.client.DatastoreException ex) { + throw translate(ex); + } } @Override public RunAggregationQueryResponse runAggregationQuery(RunAggregationQueryRequest request) { - return this.datastoreStub.runAggregationQueryCallable().call(request); + try { + return client.runAggregationQuery(request); + } catch (com.google.datastore.v1.client.DatastoreException ex) { + throw translate(ex); + } } @Override public void close() throws Exception { - if (!closed) { - datastoreStub.close(); - for (BackgroundResource resource : clientContext.getBackgroundResources()) { - resource.close(); - } - closed = true; - } - for (BackgroundResource resource : clientContext.getBackgroundResources()) { - resource.awaitTermination(1, SECONDS); - } + throw new UnsupportedOperationException("Not implemented."); } @Override public boolean isClosed() { - return closed && datastoreStub.isShutdown(); - } - - /** - * Prefixing it with http scheme when host is localhost, otherwise {@link - * com.google.api.gax.httpjson.HttpRequestRunnable#normalizeEndpoint(String)} will prefix it with - * https. - */ - private String getHost(DatastoreOptions options) { - String host = options.getHost(); - if (isLocalHost(host) && !host.contains("://")) { - return "http://" + host; - } - return host; + throw new UnsupportedOperationException("Not implemented."); } } diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/RemoteDatastoreHelper.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/RemoteDatastoreHelper.java index 329b20dca..1471328ff 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/RemoteDatastoreHelper.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/RemoteDatastoreHelper.java @@ -25,6 +25,7 @@ import com.google.cloud.datastore.Query; import com.google.cloud.datastore.QueryResults; import com.google.cloud.datastore.StructuredQuery; +import com.google.cloud.grpc.GrpcTransportOptions; import com.google.cloud.http.HttpTransportOptions; import java.util.UUID; import org.threeten.bp.Duration; @@ -75,11 +76,11 @@ public void deleteNamespace() { /** Creates a {@code RemoteStorageHelper} object. */ public static RemoteDatastoreHelper create() { - return create("", DatastoreOptions.getDefaultGrpcTransportOptions()); + return create("", DatastoreOptions.getDefaultHttpTransportOptions()); } public static RemoteDatastoreHelper create(String databaseId) { - return create(databaseId, DatastoreOptions.getDefaultGrpcTransportOptions()); + return create(databaseId, DatastoreOptions.getDefaultHttpTransportOptions()); } public static RemoteDatastoreHelper create(TransportOptions transportOptions) { @@ -88,13 +89,17 @@ public static RemoteDatastoreHelper create(TransportOptions transportOptions) { /** Creates a {@code RemoteStorageHelper} object. */ public static RemoteDatastoreHelper create(String databaseId, TransportOptions transportOptions) { - DatastoreOptions datastoreOption = + DatastoreOptions.Builder builder = DatastoreOptions.newBuilder() .setDatabaseId(databaseId) .setNamespace(UUID.randomUUID().toString()) - .setRetrySettings(retrySettings()) - .setTransportOptions(transportOptions) - .build(); + .setRetrySettings(retrySettings()); + if (transportOptions instanceof GrpcTransportOptions) { + builder = builder.setTransportOptions((GrpcTransportOptions) transportOptions); + } else { + builder = builder.setTransportOptions(transportOptions); + } + DatastoreOptions datastoreOption = builder.build(); return new RemoteDatastoreHelper(datastoreOption); } diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java index 098ae3427..908068e7c 100644 --- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java @@ -105,6 +105,7 @@ public void testCustomChannelAndCredentials() { .setServiceRpcFactory(datastoreRpcFactory) .setProjectId(PROJECT_ID) .setDatabaseId(DATABASE_ID) + .setTransportOptions(GrpcTransportOptions.newBuilder().build()) .setChannelProvider(channelProvider) .setCredentialsProvider(noCredentialsProvider) .setHost("http://localhost:" + PORT) @@ -136,26 +137,21 @@ public void testInvalidConfigForHttp() { @Test public void testTransport() { - // default grpc transport - assertThat(options.build().getTransportOptions()).isInstanceOf(GrpcTransportOptions.class); - - // custom http transport - DatastoreOptions httpDatastoreOptions = - DatastoreOptions.newBuilder() - .setTransportOptions(HttpTransportOptions.newBuilder().build()) - .setProjectId(PROJECT_ID) - .build(); - assertThat(httpDatastoreOptions.getTransportOptions()).isInstanceOf(HttpTransportOptions.class); - assertThat(httpDatastoreOptions.getCredentialsProvider()).isNull(); - assertThat(httpDatastoreOptions.getTransportChannelProvider()).isNull(); + // default http transport + assertThat(options.build().getTransportOptions()).isInstanceOf(HttpTransportOptions.class); // custom grpc transport - DatastoreOptions grpcDatastoreOptions = + DatastoreOptions grpcTransportOptions = DatastoreOptions.newBuilder() .setTransportOptions(GrpcTransportOptions.newBuilder().build()) .setProjectId(PROJECT_ID) + .setCredentialsProvider(NoCredentialsProvider.create()) .build(); - assertThat(grpcDatastoreOptions.getTransportOptions()).isInstanceOf(GrpcTransportOptions.class); + assertThat(grpcTransportOptions.getTransportOptions()).isInstanceOf(GrpcTransportOptions.class); + assertThat(grpcTransportOptions.getCredentialsProvider()) + .isInstanceOf(NoCredentialsProvider.class); + assertThat(grpcTransportOptions.getTransportChannelProvider()) + .isInstanceOf(InstantiatingGrpcChannelProvider.class); } @Test @@ -164,6 +160,7 @@ public void testToBuilder() { DatastoreOptions copy = original.toBuilder().build(); assertEquals(original.getProjectId(), copy.getProjectId()); assertEquals(original.getNamespace(), copy.getNamespace()); + assertEquals(original.getDatabaseId(), copy.getDatabaseId()); assertEquals(original.getHost(), copy.getHost()); assertEquals(original.getRetrySettings(), copy.getRetrySettings()); assertEquals(original.getCredentials(), copy.getCredentials()); diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreTest.java index efde25e61..184712443 100644 --- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreTest.java +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreTest.java @@ -16,7 +16,6 @@ package com.google.cloud.datastore; -import static com.google.api.gax.rpc.StatusCode.Code.ABORTED; import static com.google.cloud.datastore.ProtoTestData.intValue; import static com.google.cloud.datastore.TestUtils.matches; import static com.google.cloud.datastore.aggregation.Aggregation.count; @@ -31,7 +30,6 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -252,7 +250,7 @@ public void testTransactionWithRead() { transaction.commit(); fail("Expecting a failure"); } catch (DatastoreException expected) { - assertEquals(ABORTED.getHttpStatusCode(), expected.getCode()); + assertEquals("ABORTED", expected.getReason()); } } @@ -281,7 +279,7 @@ public void testTransactionWithQuery() { transaction.commit(); fail("Expecting a failure"); } catch (DatastoreException expected) { - assertEquals(ABORTED.getHttpStatusCode(), expected.getCode()); + assertEquals("ABORTED", expected.getReason()); } } @@ -1383,21 +1381,6 @@ public void testDatabaseIdKeyFactory() { checkKeyProperties(incompleteKey); } - @Test - public void testDatastoreClose() throws Exception { - Datastore datastore = options.toBuilder().build().getService(); - Entity entity = datastore.get(KEY3); - assertNull(entity); - - datastore.close(); - assertTrue(datastore.isClosed()); - - assertThrows( - "io.grpc.StatusRuntimeException: UNAVAILABLE: Channel shutdown invoked", - DatastoreException.class, - () -> datastore.get(KEY3)); - } - private void checkKeyProperties(BaseKey key) { assertEquals(options.getDatabaseId(), key.getDatabaseId()); assertEquals(options.getProjectId(), key.getProjectId()); diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/AbstractITDatastoreTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/AbstractITDatastoreTest.java index 357471abc..7c105672c 100644 --- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/AbstractITDatastoreTest.java +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/AbstractITDatastoreTest.java @@ -70,6 +70,7 @@ import com.google.cloud.datastore.TimestampValue; import com.google.cloud.datastore.Transaction; import com.google.cloud.datastore.ValueType; +import com.google.cloud.grpc.GrpcTransportOptions; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.datastore.v1.TransactionOptions; @@ -1449,9 +1450,13 @@ public Integer run(DatastoreReaderWriter transaction) { datastore.runInTransaction(callable2, readOnlyOptions); fail("Expecting a failure"); } catch (DatastoreException expected) { - assertEquals( - INVALID_ARGUMENT.getHttpStatusCode(), - ((DatastoreException) expected.getCause()).getCode()); + if (datastore.getOptions().getTransportOptions() instanceof GrpcTransportOptions) { + assertEquals( + INVALID_ARGUMENT.getHttpStatusCode(), + ((DatastoreException) expected.getCause()).getCode()); + } else { + assertEquals(3, ((DatastoreException) expected.getCause()).getCode()); + } } } diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreTestGrpc.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreTestGrpc.java index d9acbd088..7bb809997 100644 --- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreTestGrpc.java +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreTestGrpc.java @@ -18,6 +18,8 @@ import com.google.cloud.datastore.Datastore; import com.google.cloud.datastore.DatastoreOptions; import com.google.cloud.datastore.testing.RemoteDatastoreHelper; +import com.google.cloud.grpc.GrpcTransportOptions; +import com.google.common.truth.Truth; import java.util.Arrays; import org.junit.AfterClass; import org.junit.runner.RunWith; @@ -26,13 +28,14 @@ @RunWith(Parameterized.class) public class ITDatastoreTestGrpc extends AbstractITDatastoreTest { // setup for default db, grpc transport - protected static final RemoteDatastoreHelper HELPER_DEFAULT_GRPC = RemoteDatastoreHelper.create(); + protected static final RemoteDatastoreHelper HELPER_DEFAULT_GRPC = + RemoteDatastoreHelper.create(GrpcTransportOptions.newBuilder().build()); private static final DatastoreOptions OPTIONS_DEFAULT_GRPC = HELPER_DEFAULT_GRPC.getOptions(); private static final Datastore DATASTORE_DEFAULT_GRPC = OPTIONS_DEFAULT_GRPC.getService(); // setup for custom db, grpc transport private static final RemoteDatastoreHelper HELPER_CUSTOM_DB_GRPC = - RemoteDatastoreHelper.create(CUSTOM_DB_ID); + RemoteDatastoreHelper.create(CUSTOM_DB_ID, GrpcTransportOptions.newBuilder().build()); private static final DatastoreOptions OPTIONS_CUSTOM_DB_GRPC = HELPER_CUSTOM_DB_GRPC.getOptions(); private static final Datastore DATASTORE_CUSTOM_DB_GRPC = OPTIONS_CUSTOM_DB_GRPC.getService(); @@ -55,5 +58,7 @@ public static void afterClass() throws Exception { HELPER_CUSTOM_DB_GRPC.deleteNamespace(); DATASTORE_DEFAULT_GRPC.close(); DATASTORE_CUSTOM_DB_GRPC.close(); + Truth.assertThat(DATASTORE_DEFAULT_GRPC.isClosed()).isTrue(); + Truth.assertThat(DATASTORE_CUSTOM_DB_GRPC.isClosed()).isTrue(); } } diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreTestHttp.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreTestHttp.java index 0a453bbb4..3546bfcaf 100644 --- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreTestHttp.java +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreTestHttp.java @@ -18,7 +18,6 @@ import com.google.cloud.datastore.Datastore; import com.google.cloud.datastore.DatastoreOptions; import com.google.cloud.datastore.testing.RemoteDatastoreHelper; -import com.google.cloud.http.HttpTransportOptions; import java.util.Arrays; import org.junit.AfterClass; import org.junit.runner.RunWith; @@ -27,14 +26,14 @@ @RunWith(Parameterized.class) public class ITDatastoreTestHttp extends AbstractITDatastoreTest { // setup for default db, http transport - private static final RemoteDatastoreHelper HELPER_DEFAULT_HTTP = - RemoteDatastoreHelper.create(HttpTransportOptions.newBuilder().build()); + private static final RemoteDatastoreHelper HELPER_DEFAULT_HTTP = RemoteDatastoreHelper.create(); + private static final DatastoreOptions OPTIONS_DEFAULT_HTTP = HELPER_DEFAULT_HTTP.getOptions(); private static final Datastore DATASTORE_DEFAULT_HTTP = OPTIONS_DEFAULT_HTTP.getService(); // setup for custom db, http transport private static final RemoteDatastoreHelper HELPER_CUSTOM_DB_HTTP = - RemoteDatastoreHelper.create(CUSTOM_DB_ID, HttpTransportOptions.newBuilder().build()); + RemoteDatastoreHelper.create(CUSTOM_DB_ID); private static final DatastoreOptions OPTIONS_CUSTOM_DB_HTTP = HELPER_CUSTOM_DB_HTTP.getOptions(); private static final Datastore DATASTORE_CUSTOM_DB_HTTP = OPTIONS_CUSTOM_DB_HTTP.getService(); @@ -55,7 +54,5 @@ public static Iterable data() { public static void afterClass() throws Exception { HELPER_DEFAULT_HTTP.deleteNamespace(); HELPER_CUSTOM_DB_HTTP.deleteNamespace(); - DATASTORE_DEFAULT_HTTP.close(); - DATASTORE_CUSTOM_DB_HTTP.close(); } } From 2d1c6ad1937105148af40c897cdbb0c8f575f866 Mon Sep 17 00:00:00 2001 From: kolea2 <45548808+kolea2@users.noreply.github.com> Date: Mon, 25 Mar 2024 11:56:16 -0400 Subject: [PATCH 2/7] chore: update concepts test (#1382) Fixes #1379 (cherry picked from commit 9818aeec24eb6da9d4726358b03bb6a5ca384f25) --- .../google/cloud/datastore/it/ITDatastoreConceptsTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreConceptsTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreConceptsTest.java index f61db4f48..65ee3a78d 100644 --- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreConceptsTest.java +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreConceptsTest.java @@ -575,7 +575,7 @@ public void testInequalityRange() { } @Test - public void testInequalityInvalid() { + public void testInequalityValid() { Query query = Query.newEntityQueryBuilder() .setKind(TASK_CONCEPTS) @@ -583,7 +583,7 @@ public void testInequalityInvalid() { CompositeFilter.and( PropertyFilter.gt("created", startDate), PropertyFilter.gt("priority", 3))) .build(); - assertInvalidQuery(query); + assertValidQuery(query); } @Test From 431e28be85e30fed1c7933cc7db79fd64612e932 Mon Sep 17 00:00:00 2001 From: kolea2 Date: Fri, 19 Apr 2024 08:22:49 -0400 Subject: [PATCH 3/7] fix host setting logic --- .../cloud/datastore/DatastoreOptions.java | 16 +++++++- .../datastore/spi/v1/HttpDatastoreRpc.java | 6 --- .../cloud/datastore/DatastoreOptionsTest.java | 39 +++++++++++++++++++ 3 files changed, 54 insertions(+), 7 deletions(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java index aa29b6624..196eb8880 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java @@ -31,6 +31,7 @@ import com.google.cloud.datastore.spi.v1.GrpcDatastoreRpc; import com.google.cloud.datastore.spi.v1.HttpDatastoreRpc; import com.google.cloud.datastore.v1.DatastoreSettings; +import com.google.cloud.datastore.v1.stub.DatastoreStubSettings; import com.google.cloud.grpc.GrpcTransportOptions; import com.google.cloud.http.HttpTransportOptions; import com.google.common.base.MoreObjects; @@ -90,6 +91,8 @@ public static class Builder extends ServiceOptions.Builder Date: Thu, 18 Apr 2024 15:40:53 -0400 Subject: [PATCH 4/7] fix test --- .../google/cloud/datastore/it/ITDatastoreConceptsTest.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreConceptsTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreConceptsTest.java index 65ee3a78d..90b324c99 100644 --- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreConceptsTest.java +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreConceptsTest.java @@ -132,11 +132,6 @@ public void tearDown() { datastore.delete(taskKeysToDelete); } - @AfterClass - public static void afterClass() throws Exception { - datastore.close(); - } - private void assertValidKey(Key taskKey) { datastore.put(Entity.newBuilder(taskKey, TEST_FULL_ENTITY).build()); } From 295501de83a1bc22b387e074aed0d2b88c56b2e6 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Fri, 19 Apr 2024 17:49:58 +0000 Subject: [PATCH 5/7] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20po?= =?UTF-8?q?st-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- .../com/google/cloud/datastore/it/ITDatastoreConceptsTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreConceptsTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreConceptsTest.java index 90b324c99..1d6b2f838 100644 --- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreConceptsTest.java +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreConceptsTest.java @@ -67,7 +67,6 @@ import java.util.Map; import java.util.concurrent.TimeUnit; import org.junit.After; -import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; From b2ac4f27acbcd9d6b62879b9f742d8ce4c6d9c42 Mon Sep 17 00:00:00 2001 From: kolea2 Date: Mon, 22 Apr 2024 10:22:27 -0400 Subject: [PATCH 6/7] update test --- .../java/com/google/cloud/datastore/DatastoreOptions.java | 2 +- .../java/com/google/cloud/datastore/DatastoreOptionsTest.java | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java index 196eb8880..dd988add0 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java @@ -153,7 +153,7 @@ public Builder setCredentialsProvider(CredentialsProvider credentialsProvider) { @Override public DatastoreOptions build() { if (this.host == null && this.transportOptions instanceof GrpcTransportOptions) { - this.setHost(DatastoreStubSettings.getDefaultEndpoint()); + this.setHost(DatastoreSettings.getDefaultEndpoint()); } return new DatastoreOptions(this); } diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java index 74e47b73b..f43c0c670 100644 --- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java @@ -165,7 +165,9 @@ public void testHostWithGrpcAndHttp() { .setCredentialsProvider(NoCredentialsProvider.create()) .build(); assertThat(grpcTransportOptions.getHost()) - .isEqualTo(DatastoreStubSettings.getDefaultEndpoint()); + .isEqualTo(DatastoreSettings.getDefaultEndpoint()); + assertThat(grpcTransportOptions.getHost()) + .isEqualTo("datastore.googleapis.com:443"); String customHost = "http://localhost:" + PORT; DatastoreOptions grpcTransportOptionsCustomHost = From ea6b223a2f20a7c38dff5e515cad0437e00697cb Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Mon, 22 Apr 2024 14:46:20 +0000 Subject: [PATCH 7/7] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20po?= =?UTF-8?q?st-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- .../java/com/google/cloud/datastore/DatastoreOptions.java | 1 - .../com/google/cloud/datastore/DatastoreOptionsTest.java | 7 ++----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java index dd988add0..7e5192b52 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java @@ -31,7 +31,6 @@ import com.google.cloud.datastore.spi.v1.GrpcDatastoreRpc; import com.google.cloud.datastore.spi.v1.HttpDatastoreRpc; import com.google.cloud.datastore.v1.DatastoreSettings; -import com.google.cloud.datastore.v1.stub.DatastoreStubSettings; import com.google.cloud.grpc.GrpcTransportOptions; import com.google.cloud.http.HttpTransportOptions; import com.google.common.base.MoreObjects; diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java index f43c0c670..e3f48ecc9 100644 --- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java @@ -29,7 +29,6 @@ import com.google.cloud.datastore.spi.DatastoreRpcFactory; import com.google.cloud.datastore.spi.v1.DatastoreRpc; import com.google.cloud.datastore.v1.DatastoreSettings; -import com.google.cloud.datastore.v1.stub.DatastoreStubSettings; import com.google.cloud.grpc.GrpcTransportOptions; import com.google.cloud.http.HttpTransportOptions; import com.google.datastore.v1.client.DatastoreFactory; @@ -164,10 +163,8 @@ public void testHostWithGrpcAndHttp() { .setProjectId(PROJECT_ID) .setCredentialsProvider(NoCredentialsProvider.create()) .build(); - assertThat(grpcTransportOptions.getHost()) - .isEqualTo(DatastoreSettings.getDefaultEndpoint()); - assertThat(grpcTransportOptions.getHost()) - .isEqualTo("datastore.googleapis.com:443"); + assertThat(grpcTransportOptions.getHost()).isEqualTo(DatastoreSettings.getDefaultEndpoint()); + assertThat(grpcTransportOptions.getHost()).isEqualTo("datastore.googleapis.com:443"); String customHost = "http://localhost:" + PORT; DatastoreOptions grpcTransportOptionsCustomHost =