From e17ba348adcd524c7c9714e7b06696fa5323ec54 Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Sat, 4 Nov 2023 18:31:59 +0000 Subject: [PATCH 01/26] Create basic structure of GrpcDatastoreRpc and using it in DatastoreOptions --- google-cloud-datastore/pom.xml | 4 + .../cloud/datastore/DatastoreOptions.java | 24 ++++- .../datastore/spi/v1/GrpcDatastoreRpc.java | 93 +++++++++++++++++++ 3 files changed, 116 insertions(+), 5 deletions(-) create mode 100644 google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java diff --git a/google-cloud-datastore/pom.xml b/google-cloud-datastore/pom.xml index fc1e45ef1..59b4b30ed 100644 --- a/google-cloud-datastore/pom.xml +++ b/google-cloud-datastore/pom.xml @@ -30,6 +30,10 @@ com.google.cloud google-cloud-core-http + + com.google.cloud + google-cloud-core-grpc + com.google.api.grpc proto-google-cloud-datastore-v1 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 d4f3be3c2..14077a5aa 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 @@ -16,8 +16,6 @@ package com.google.cloud.datastore; -import static com.google.cloud.datastore.Validator.validateNamespace; - import com.google.api.core.BetaApi; import com.google.cloud.ServiceDefaults; import com.google.cloud.ServiceOptions; @@ -25,14 +23,19 @@ 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.spi.v1.HttpDatastoreRpc; +import com.google.cloud.datastore.spi.v1.GrpcDatastoreRpc; +import com.google.cloud.datastore.v1.DatastoreSettings; import com.google.cloud.http.HttpTransportOptions; import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableSet; + +import java.io.IOException; import java.lang.reflect.Method; import java.util.Objects; import java.util.Set; +import static com.google.cloud.datastore.Validator.validateNamespace; + public class DatastoreOptions extends ServiceOptions { private static final long serialVersionUID = -1018382430058137336L; @@ -60,7 +63,11 @@ public static class DefaultDatastoreRpcFactory implements DatastoreRpcFactory { @Override public ServiceRpc create(DatastoreOptions options) { - return new HttpDatastoreRpc(options); + try { + return new GrpcDatastoreRpc(options); + } catch (IOException e) { + throw new RuntimeException(e); + } } } @@ -116,7 +123,7 @@ protected String getDefaultHost() { System.getProperty( com.google.datastore.v1.client.DatastoreHelper.LOCAL_HOST_ENV_VAR, System.getenv(com.google.datastore.v1.client.DatastoreHelper.LOCAL_HOST_ENV_VAR)); - return host != null ? host : com.google.datastore.v1.client.DatastoreFactory.DEFAULT_HOST; + return host != null ? host : DatastoreDefaults.INSTANCE.getHost(); } @Override @@ -130,6 +137,13 @@ protected String getDefaultProject() { private static class DatastoreDefaults implements ServiceDefaults { + private static final DatastoreDefaults INSTANCE = new DatastoreDefaults(); + private final String HOST = DatastoreSettings.getDefaultEndpoint(); + + String getHost() { + return HOST; + } + @Override public DatastoreFactory getDefaultServiceFactory() { return DefaultDatastoreFactory.INSTANCE; diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java new file mode 100644 index 000000000..d006cc964 --- /dev/null +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java @@ -0,0 +1,93 @@ +package com.google.cloud.datastore.spi.v1; + +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.GrpcDatastoreStub; +import com.google.cloud.grpc.GrpcTransportOptions; +import com.google.datastore.v1.AllocateIdsRequest; +import com.google.datastore.v1.AllocateIdsResponse; +import com.google.datastore.v1.BeginTransactionRequest; +import com.google.datastore.v1.BeginTransactionResponse; +import com.google.datastore.v1.CommitRequest; +import com.google.datastore.v1.CommitResponse; +import com.google.datastore.v1.LookupRequest; +import com.google.datastore.v1.LookupResponse; +import com.google.datastore.v1.ReserveIdsRequest; +import com.google.datastore.v1.ReserveIdsResponse; +import com.google.datastore.v1.RollbackRequest; +import com.google.datastore.v1.RollbackResponse; +import com.google.datastore.v1.RunAggregationQueryRequest; +import com.google.datastore.v1.RunAggregationQueryResponse; +import com.google.datastore.v1.RunQueryRequest; +import com.google.datastore.v1.RunQueryResponse; + +import java.io.IOException; + +//TODO(gapic_upgrade): Make it implement AutoCloseable +public class GrpcDatastoreRpc implements DatastoreRpc{ + + private final GrpcDatastoreStub datastoreStub; + + public GrpcDatastoreRpc(DatastoreOptions datastoreOptions) throws IOException { + + try { + DatastoreSettings datastoreSettings = DatastoreSettings.newBuilder() + .setCredentialsProvider(GrpcTransportOptions.setUpCredentialsProvider(datastoreOptions)) + .setTransportChannelProvider(GrpcTransportOptions.setUpChannelProvider(DatastoreSettings.defaultGrpcTransportProviderBuilder(), datastoreOptions)) + //TODO(gapic_upgrade): setup internal header provider + .build(); + + ClientContext clientContext = ClientContext.create(datastoreSettings); + + //TODO(gapic_upgrade): retry settings + + DatastoreStubSettings datastoreStubSettings = DatastoreStubSettings.newBuilder(clientContext).build(); + datastoreStub = GrpcDatastoreStub.create(datastoreStubSettings); + } catch (IOException e) { + throw new IOException(e); + } + } + + @Override + public AllocateIdsResponse allocateIds(AllocateIdsRequest request) { + return datastoreStub.allocateIdsCallable().call(request); + } + + @Override + public BeginTransactionResponse beginTransaction(BeginTransactionRequest request) throws DatastoreException { + return datastoreStub.beginTransactionCallable().call(request); + } + + @Override + public CommitResponse commit(CommitRequest request) { + return datastoreStub.commitCallable().call(request); + } + + @Override + public LookupResponse lookup(LookupRequest request) { + return datastoreStub.lookupCallable().call(request); + } + + @Override + public ReserveIdsResponse reserveIds(ReserveIdsRequest request) { + return datastoreStub.reserveIdsCallable().call(request); + } + + @Override + public RollbackResponse rollback(RollbackRequest request) { + return datastoreStub.rollbackCallable().call(request); + } + + @Override + public RunQueryResponse runQuery(RunQueryRequest request) { + return datastoreStub.runQueryCallable().call(request); + } + + @Override + public RunAggregationQueryResponse runAggregationQuery(RunAggregationQueryRequest request) { + return datastoreStub.runAggregationQueryCallable().call(request); + } +} From f9e455e7aa5db8c2b251e814170b4fa5406cfd17 Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Mon, 6 Nov 2023 08:32:56 +0000 Subject: [PATCH 02/26] applying unary settings to all the unary methods --- .../cloud/datastore/spi/v1/GrpcDatastoreRpc.java | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java index d006cc964..7d64a90e2 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java @@ -1,6 +1,8 @@ package com.google.cloud.datastore.spi.v1; +import com.google.api.core.ApiFunction; import com.google.api.gax.rpc.ClientContext; +import com.google.api.gax.rpc.UnaryCallSettings; import com.google.cloud.datastore.DatastoreException; import com.google.cloud.datastore.DatastoreOptions; import com.google.cloud.datastore.v1.DatastoreSettings; @@ -42,9 +44,14 @@ public GrpcDatastoreRpc(DatastoreOptions datastoreOptions) throws IOException { ClientContext clientContext = ClientContext.create(datastoreSettings); - //TODO(gapic_upgrade): retry settings - - DatastoreStubSettings datastoreStubSettings = DatastoreStubSettings.newBuilder(clientContext).build(); + ApiFunction, Void> retrySettingsSetter = + builder -> { + builder.setRetrySettings(datastoreOptions.getRetrySettings()); + return null; + }; + DatastoreStubSettings datastoreStubSettings = DatastoreStubSettings.newBuilder(clientContext) + .applyToAllUnaryMethods(retrySettingsSetter) + .build(); datastoreStub = GrpcDatastoreStub.create(datastoreStubSettings); } catch (IOException e) { throw new IOException(e); From 231deef32b5ad3831201cbc6f8d0d36aac37fb3b Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Mon, 6 Nov 2023 09:25:49 +0000 Subject: [PATCH 03/26] Configuring header provider for GrpcDatastoreRpc --- .../datastore/spi/v1/GrpcDatastoreRpc.java | 53 ++++++++++++++++--- 1 file changed, 45 insertions(+), 8 deletions(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java index 7d64a90e2..2dbf03bb7 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java @@ -1,14 +1,19 @@ package com.google.cloud.datastore.spi.v1; import com.google.api.core.ApiFunction; +import com.google.api.gax.core.GaxProperties; import com.google.api.gax.rpc.ClientContext; +import com.google.api.gax.rpc.HeaderProvider; +import com.google.api.gax.rpc.NoHeaderProvider; import com.google.api.gax.rpc.UnaryCallSettings; +import com.google.cloud.ServiceOptions; 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.GrpcDatastoreStub; import com.google.cloud.grpc.GrpcTransportOptions; +import com.google.common.base.Strings; import com.google.datastore.v1.AllocateIdsRequest; import com.google.datastore.v1.AllocateIdsResponse; import com.google.datastore.v1.BeginTransactionRequest; @@ -29,20 +34,27 @@ import java.io.IOException; //TODO(gapic_upgrade): Make it implement AutoCloseable -public class GrpcDatastoreRpc implements DatastoreRpc{ +public class GrpcDatastoreRpc implements DatastoreRpc { private final GrpcDatastoreStub datastoreStub; public GrpcDatastoreRpc(DatastoreOptions datastoreOptions) throws IOException { try { - DatastoreSettings datastoreSettings = DatastoreSettings.newBuilder() - .setCredentialsProvider(GrpcTransportOptions.setUpCredentialsProvider(datastoreOptions)) - .setTransportChannelProvider(GrpcTransportOptions.setUpChannelProvider(DatastoreSettings.defaultGrpcTransportProviderBuilder(), datastoreOptions)) - //TODO(gapic_upgrade): setup internal header provider - .build(); - - ClientContext clientContext = ClientContext.create(datastoreSettings); + HeaderProvider internalHeaderProvider = + DatastoreSettings.defaultApiClientHeaderProviderBuilder() + .setClientLibToken( + ServiceOptions.getGoogApiClientLibName(), + GaxProperties.getLibraryVersion(datastoreOptions.getClass())) + .setResourceToken(getResourceToken(datastoreOptions)) + .build(); + + DatastoreSettingsBuilder settingsBuilder = new DatastoreSettingsBuilder(DatastoreSettings.newBuilder().build()); + settingsBuilder.setCredentialsProvider(GrpcTransportOptions.setUpCredentialsProvider(datastoreOptions)); + settingsBuilder.setTransportChannelProvider(GrpcTransportOptions.setUpChannelProvider(DatastoreSettings.defaultGrpcTransportProviderBuilder(), datastoreOptions)); + settingsBuilder.setInternalHeaderProvider(internalHeaderProvider); + settingsBuilder.setHeaderProvider(datastoreOptions.getMergedHeaderProvider(new NoHeaderProvider())); + ClientContext clientContext = ClientContext.create(settingsBuilder.build()); ApiFunction, Void> retrySettingsSetter = builder -> { @@ -97,4 +109,29 @@ public RunQueryResponse runQuery(RunQueryRequest request) { public RunAggregationQueryResponse runAggregationQuery(RunAggregationQueryRequest request) { return datastoreStub.runAggregationQueryCallable().call(request); } + + private static String getResourceToken(DatastoreOptions datastoreOptions) { + StringBuilder builder = new StringBuilder("project_id="); + builder.append(datastoreOptions.getProjectId()); + if (!Strings.isNullOrEmpty(datastoreOptions.getDatabaseId())) { + builder.append("&database_id="); + builder.append(datastoreOptions.getDatabaseId()); + } + return builder.toString(); + } + + // This class is needed solely to get access to protected method setInternalHeaderProvider() + private static class DatastoreSettingsBuilder extends DatastoreSettings.Builder { + + private DatastoreSettingsBuilder(DatastoreSettings settings) { + super(settings); + } + + @Override + protected DatastoreSettings.Builder setInternalHeaderProvider( + HeaderProvider internalHeaderProvider) { + return super.setInternalHeaderProvider(internalHeaderProvider); + } + + } } From 806f997503ce4a9eb8d10cf817ff2ede3ec9e7bd Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Thu, 9 Nov 2023 08:52:40 +0000 Subject: [PATCH 04/26] fixing emulator tests to be able to run successfully with grpc now --- .../datastore/spi/v1/GrpcDatastoreRpc.java | 65 ++++++++++++++----- .../google/cloud/datastore/DatastoreTest.java | 60 +++++++++-------- 2 files changed, 83 insertions(+), 42 deletions(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java index 2dbf03bb7..abc22c40f 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java @@ -2,10 +2,14 @@ import com.google.api.core.ApiFunction; import com.google.api.gax.core.GaxProperties; +import com.google.api.gax.grpc.GrpcCallContext; +import com.google.api.gax.grpc.GrpcTransportChannel; import com.google.api.gax.rpc.ClientContext; import com.google.api.gax.rpc.HeaderProvider; import com.google.api.gax.rpc.NoHeaderProvider; +import com.google.api.gax.rpc.TransportChannel; import com.google.api.gax.rpc.UnaryCallSettings; +import com.google.cloud.NoCredentials; import com.google.cloud.ServiceOptions; import com.google.cloud.datastore.DatastoreException; import com.google.cloud.datastore.DatastoreOptions; @@ -30,8 +34,12 @@ import com.google.datastore.v1.RunAggregationQueryResponse; import com.google.datastore.v1.RunQueryRequest; import com.google.datastore.v1.RunQueryResponse; +import io.grpc.CallOptions; +import io.grpc.ManagedChannel; +import io.grpc.ManagedChannelBuilder; import java.io.IOException; +import java.util.Collections; //TODO(gapic_upgrade): Make it implement AutoCloseable public class GrpcDatastoreRpc implements DatastoreRpc { @@ -41,21 +49,9 @@ public class GrpcDatastoreRpc implements DatastoreRpc { public GrpcDatastoreRpc(DatastoreOptions datastoreOptions) throws IOException { try { - HeaderProvider internalHeaderProvider = - DatastoreSettings.defaultApiClientHeaderProviderBuilder() - .setClientLibToken( - ServiceOptions.getGoogApiClientLibName(), - GaxProperties.getLibraryVersion(datastoreOptions.getClass())) - .setResourceToken(getResourceToken(datastoreOptions)) - .build(); - - DatastoreSettingsBuilder settingsBuilder = new DatastoreSettingsBuilder(DatastoreSettings.newBuilder().build()); - settingsBuilder.setCredentialsProvider(GrpcTransportOptions.setUpCredentialsProvider(datastoreOptions)); - settingsBuilder.setTransportChannelProvider(GrpcTransportOptions.setUpChannelProvider(DatastoreSettings.defaultGrpcTransportProviderBuilder(), datastoreOptions)); - settingsBuilder.setInternalHeaderProvider(internalHeaderProvider); - settingsBuilder.setHeaderProvider(datastoreOptions.getMergedHeaderProvider(new NoHeaderProvider())); - ClientContext clientContext = ClientContext.create(settingsBuilder.build()); - + ClientContext clientContext = isEmulator(datastoreOptions) ? + getClientContextForEmulator(datastoreOptions) : + getClientContext(datastoreOptions); ApiFunction, Void> retrySettingsSetter = builder -> { builder.setRetrySettings(datastoreOptions.getRetrySettings()); @@ -110,7 +106,44 @@ public RunAggregationQueryResponse runAggregationQuery(RunAggregationQueryReques return datastoreStub.runAggregationQueryCallable().call(request); } - private static String getResourceToken(DatastoreOptions datastoreOptions) { + private boolean isEmulator(DatastoreOptions datastoreOptions) { + return datastoreOptions.getHost().contains("localhost") + || NoCredentials.getInstance().equals(datastoreOptions.getCredentials()); + } + + private ClientContext getClientContextForEmulator(DatastoreOptions datastoreOptions) throws IOException { + ManagedChannel managedChannel = + ManagedChannelBuilder.forTarget(datastoreOptions.getHost()) + .usePlaintext() + .build(); + TransportChannel transportChannel = GrpcTransportChannel.create(managedChannel); + return ClientContext.newBuilder() + .setCredentials(null) + .setTransportChannel(transportChannel) + .setDefaultCallContext(GrpcCallContext.of(managedChannel, CallOptions.DEFAULT)) + .setBackgroundResources(Collections.singletonList(transportChannel)) + .build(); + } + + private ClientContext getClientContext(DatastoreOptions datastoreOptions) throws IOException { + HeaderProvider internalHeaderProvider = + DatastoreSettings.defaultApiClientHeaderProviderBuilder() + .setClientLibToken( + ServiceOptions.getGoogApiClientLibName(), + GaxProperties.getLibraryVersion(datastoreOptions.getClass())) + .setResourceToken(getResourceToken(datastoreOptions)) + .build(); + + DatastoreSettingsBuilder settingsBuilder = new DatastoreSettingsBuilder(DatastoreSettings.newBuilder().build()); + settingsBuilder.setCredentialsProvider(GrpcTransportOptions.setUpCredentialsProvider(datastoreOptions)); + settingsBuilder.setTransportChannelProvider(GrpcTransportOptions.setUpChannelProvider(DatastoreSettings.defaultGrpcTransportProviderBuilder(), datastoreOptions)); + settingsBuilder.setInternalHeaderProvider(internalHeaderProvider); + settingsBuilder.setHeaderProvider(datastoreOptions.getMergedHeaderProvider(new NoHeaderProvider())); + ClientContext clientContext = ClientContext.create(settingsBuilder.build()); + return clientContext; + } + + private String getResourceToken(DatastoreOptions datastoreOptions) { StringBuilder builder = new StringBuilder("project_id="); builder.append(datastoreOptions.getProjectId()); if (!Strings.isNullOrEmpty(datastoreOptions.getDatabaseId())) { 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 cd768f986..ed4dd8dda 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,23 +16,6 @@ package com.google.cloud.datastore; -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; -import static com.google.common.collect.Iterables.getOnlyElement; -import static com.google.common.truth.Truth.assertThat; -import static org.easymock.EasyMock.createStrictMock; -import static org.easymock.EasyMock.replay; -import static org.easymock.EasyMock.verify; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - import com.google.cloud.ServiceOptions; import com.google.cloud.Timestamp; import com.google.cloud.datastore.Query.ResultType; @@ -67,6 +50,17 @@ import com.google.datastore.v1.RunQueryResponse; import com.google.datastore.v1.TransactionOptions; import com.google.protobuf.ByteString; +import org.easymock.EasyMock; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Ignore; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.threeten.bp.Duration; + import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; @@ -79,15 +73,23 @@ import java.util.Set; import java.util.concurrent.TimeoutException; import java.util.function.Predicate; -import org.easymock.EasyMock; -import org.junit.AfterClass; -import org.junit.Assert; -import org.junit.Before; -import org.junit.BeforeClass; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; -import org.threeten.bp.Duration; + +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; +import static com.google.common.collect.Iterables.getOnlyElement; +import static com.google.common.truth.Truth.assertThat; +import static org.easymock.EasyMock.createStrictMock; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.verify; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; @RunWith(JUnit4.class) public class DatastoreTest { @@ -231,6 +233,8 @@ public void testNewTransactionCommit() { verifyNotUsable(transaction); } + //TODO(gapic_upgrade): Remove the @ignore annotation + @Ignore("This should be fixed with actionable error implementation") @Test public void testTransactionWithRead() { Transaction transaction = datastore.newTransaction(); @@ -252,6 +256,8 @@ public void testTransactionWithRead() { } } + //TODO(gapic_upgrade): Remove the @ignore annotation + @Ignore("This should be fixed with actionable error implementation") @Test public void testTransactionWithQuery() { Query query = @@ -648,6 +654,7 @@ private List buildResponsesForQueryPagination() { List responses = new ArrayList<>(); RecordQuery query = Query.newKeyQueryBuilder().build(); RunQueryRequest.Builder requestPb = RunQueryRequest.newBuilder(); + requestPb.setProjectId(PROJECT_ID); query.populatePb(requestPb); QueryResultBatch queryResultBatchPb = RunQueryResponse.newBuilder() @@ -757,6 +764,7 @@ private List buildResponsesForQueryPaginationWithLimit() { List responses = new ArrayList<>(); RecordQuery query = Query.newEntityQueryBuilder().build(); RunQueryRequest.Builder requestPb = RunQueryRequest.newBuilder(); + requestPb.setProjectId(PROJECT_ID); query.populatePb(requestPb); QueryResultBatch queryResultBatchPb = RunQueryResponse.newBuilder() From 0c8704c4652e3cdebb2ecdeadc3ec3fee460c712 Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Thu, 9 Nov 2023 10:12:12 +0000 Subject: [PATCH 05/26] ignoring one more test which will be fixed in actionable error implementation --- .../cloud/datastore/it/ITDatastoreTest.java | 44 ++++++++++--------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreTest.java index 7c68ffe32..033748a90 100644 --- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreTest.java +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreTest.java @@ -16,18 +16,6 @@ package com.google.cloud.datastore.it; -import static com.google.cloud.datastore.aggregation.Aggregation.count; -import static com.google.common.collect.Iterables.getOnlyElement; -import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - import com.google.cloud.Timestamp; import com.google.cloud.Tuple; import com.google.cloud.datastore.AggregationQuery; @@ -71,6 +59,16 @@ import com.google.common.collect.ImmutableList; import com.google.datastore.v1.TransactionOptions; import com.google.datastore.v1.TransactionOptions.ReadOnly; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.Ignore; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.Timeout; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -83,14 +81,18 @@ import java.util.concurrent.Future; import java.util.function.BiConsumer; import java.util.function.Consumer; -import org.junit.After; -import org.junit.AfterClass; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.Timeout; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; + +import static com.google.cloud.datastore.aggregation.Aggregation.count; +import static com.google.common.collect.Iterables.getOnlyElement; +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; @RunWith(Parameterized.class) public class ITDatastoreTest { @@ -1391,6 +1393,8 @@ public Integer run(DatastoreReaderWriter transaction) { } } + //TODO(gapic_upgrade): Remove the @ignore annotation + @Ignore("This should be fixed with actionable error implementation") @Test public void testRunInTransactionReadWrite() { From d6ed23133cbc0c9b97d271d568f2543c907b889a Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Thu, 9 Nov 2023 10:25:34 +0000 Subject: [PATCH 06/26] Making HttpDatastoreRpc completely unused --- .../src/main/java/com/google/cloud/datastore/TraceUtil.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/TraceUtil.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/TraceUtil.java index 57525d15d..876a871a2 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/TraceUtil.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/TraceUtil.java @@ -16,14 +16,14 @@ package com.google.cloud.datastore; -import com.google.cloud.datastore.spi.v1.HttpDatastoreRpc; +import com.google.cloud.datastore.spi.v1.DatastoreRpc; import io.opencensus.trace.EndSpanOptions; import io.opencensus.trace.Span; import io.opencensus.trace.Tracer; import io.opencensus.trace.Tracing; /** - * Helper class for tracing utility. It is used for instrumenting {@link HttpDatastoreRpc} with + * Helper class for tracing utility. It is used for instrumenting {@link DatastoreRpc} with * OpenCensus APIs. * *

TraceUtil instances are created by the {@link TraceUtil#getInstance()} method. From e257a91e97dbaab9d86ad6f2a0fec5d7a5ab2a74 Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Thu, 16 Nov 2023 17:50:29 +0000 Subject: [PATCH 07/26] Making GrpcDatastoreRpc implement AutoCloseable --- .../datastore/spi/v1/GrpcDatastoreRpc.java | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java index abc22c40f..3662365e2 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java @@ -1,6 +1,7 @@ package com.google.cloud.datastore.spi.v1; import com.google.api.core.ApiFunction; +import com.google.api.gax.core.BackgroundResource; import com.google.api.gax.core.GaxProperties; import com.google.api.gax.grpc.GrpcCallContext; import com.google.api.gax.grpc.GrpcTransportChannel; @@ -41,15 +42,19 @@ import java.io.IOException; import java.util.Collections; +import static java.util.concurrent.TimeUnit.SECONDS; + //TODO(gapic_upgrade): Make it implement AutoCloseable -public class GrpcDatastoreRpc implements DatastoreRpc { +public class GrpcDatastoreRpc implements AutoCloseable, DatastoreRpc { private final GrpcDatastoreStub datastoreStub; + private final ClientContext clientContext; + private boolean closed; public GrpcDatastoreRpc(DatastoreOptions datastoreOptions) throws IOException { try { - ClientContext clientContext = isEmulator(datastoreOptions) ? + clientContext = isEmulator(datastoreOptions) ? getClientContextForEmulator(datastoreOptions) : getClientContext(datastoreOptions); ApiFunction, Void> retrySettingsSetter = @@ -66,6 +71,20 @@ public GrpcDatastoreRpc(DatastoreOptions datastoreOptions) throws IOException { } } + @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); + } + } + @Override public AllocateIdsResponse allocateIds(AllocateIdsRequest request) { return datastoreStub.allocateIdsCallable().call(request); From 29841468d8c1f24c6f654da635c4830f9c5b3834 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Thu, 16 Nov 2023 18:31:34 +0000 Subject: [PATCH 08/26] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20?= =?UTF-8?q?post-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 --- .../cloud/datastore/DatastoreOptions.java | 5 +- .../datastore/spi/v1/GrpcDatastoreRpc.java | 47 ++++++++------- .../google/cloud/datastore/DatastoreTest.java | 59 +++++++++---------- .../cloud/datastore/it/ITDatastoreTest.java | 45 +++++++------- 4 files changed, 79 insertions(+), 77 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 14077a5aa..e0afba824 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 @@ -16,6 +16,8 @@ package com.google.cloud.datastore; +import static com.google.cloud.datastore.Validator.validateNamespace; + import com.google.api.core.BetaApi; import com.google.cloud.ServiceDefaults; import com.google.cloud.ServiceOptions; @@ -28,14 +30,11 @@ import com.google.cloud.http.HttpTransportOptions; import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableSet; - import java.io.IOException; import java.lang.reflect.Method; import java.util.Objects; import java.util.Set; -import static com.google.cloud.datastore.Validator.validateNamespace; - public class DatastoreOptions extends ServiceOptions { private static final long serialVersionUID = -1018382430058137336L; diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java index 3662365e2..3ceb67624 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java @@ -1,5 +1,7 @@ package com.google.cloud.datastore.spi.v1; +import static java.util.concurrent.TimeUnit.SECONDS; + import com.google.api.core.ApiFunction; import com.google.api.gax.core.BackgroundResource; import com.google.api.gax.core.GaxProperties; @@ -38,13 +40,10 @@ import io.grpc.CallOptions; import io.grpc.ManagedChannel; import io.grpc.ManagedChannelBuilder; - import java.io.IOException; import java.util.Collections; -import static java.util.concurrent.TimeUnit.SECONDS; - -//TODO(gapic_upgrade): Make it implement AutoCloseable +// TODO(gapic_upgrade): Make it implement AutoCloseable public class GrpcDatastoreRpc implements AutoCloseable, DatastoreRpc { private final GrpcDatastoreStub datastoreStub; @@ -54,17 +53,19 @@ public class GrpcDatastoreRpc implements AutoCloseable, DatastoreRpc { public GrpcDatastoreRpc(DatastoreOptions datastoreOptions) throws IOException { try { - clientContext = isEmulator(datastoreOptions) ? - getClientContextForEmulator(datastoreOptions) : - getClientContext(datastoreOptions); + clientContext = + isEmulator(datastoreOptions) + ? getClientContextForEmulator(datastoreOptions) + : getClientContext(datastoreOptions); ApiFunction, Void> retrySettingsSetter = builder -> { builder.setRetrySettings(datastoreOptions.getRetrySettings()); return null; }; - DatastoreStubSettings datastoreStubSettings = DatastoreStubSettings.newBuilder(clientContext) - .applyToAllUnaryMethods(retrySettingsSetter) - .build(); + DatastoreStubSettings datastoreStubSettings = + DatastoreStubSettings.newBuilder(clientContext) + .applyToAllUnaryMethods(retrySettingsSetter) + .build(); datastoreStub = GrpcDatastoreStub.create(datastoreStubSettings); } catch (IOException e) { throw new IOException(e); @@ -73,7 +74,7 @@ public GrpcDatastoreRpc(DatastoreOptions datastoreOptions) throws IOException { @Override public void close() throws Exception { - if(!closed) { + if (!closed) { datastoreStub.close(); for (BackgroundResource resource : clientContext.getBackgroundResources()) { resource.close(); @@ -91,7 +92,8 @@ public AllocateIdsResponse allocateIds(AllocateIdsRequest request) { } @Override - public BeginTransactionResponse beginTransaction(BeginTransactionRequest request) throws DatastoreException { + public BeginTransactionResponse beginTransaction(BeginTransactionRequest request) + throws DatastoreException { return datastoreStub.beginTransactionCallable().call(request); } @@ -130,11 +132,10 @@ private boolean isEmulator(DatastoreOptions datastoreOptions) { || NoCredentials.getInstance().equals(datastoreOptions.getCredentials()); } - private ClientContext getClientContextForEmulator(DatastoreOptions datastoreOptions) throws IOException { + private ClientContext getClientContextForEmulator(DatastoreOptions datastoreOptions) + throws IOException { ManagedChannel managedChannel = - ManagedChannelBuilder.forTarget(datastoreOptions.getHost()) - .usePlaintext() - .build(); + ManagedChannelBuilder.forTarget(datastoreOptions.getHost()).usePlaintext().build(); TransportChannel transportChannel = GrpcTransportChannel.create(managedChannel); return ClientContext.newBuilder() .setCredentials(null) @@ -153,11 +154,16 @@ private ClientContext getClientContext(DatastoreOptions datastoreOptions) throws .setResourceToken(getResourceToken(datastoreOptions)) .build(); - DatastoreSettingsBuilder settingsBuilder = new DatastoreSettingsBuilder(DatastoreSettings.newBuilder().build()); - settingsBuilder.setCredentialsProvider(GrpcTransportOptions.setUpCredentialsProvider(datastoreOptions)); - settingsBuilder.setTransportChannelProvider(GrpcTransportOptions.setUpChannelProvider(DatastoreSettings.defaultGrpcTransportProviderBuilder(), datastoreOptions)); + DatastoreSettingsBuilder settingsBuilder = + new DatastoreSettingsBuilder(DatastoreSettings.newBuilder().build()); + settingsBuilder.setCredentialsProvider( + GrpcTransportOptions.setUpCredentialsProvider(datastoreOptions)); + settingsBuilder.setTransportChannelProvider( + GrpcTransportOptions.setUpChannelProvider( + DatastoreSettings.defaultGrpcTransportProviderBuilder(), datastoreOptions)); settingsBuilder.setInternalHeaderProvider(internalHeaderProvider); - settingsBuilder.setHeaderProvider(datastoreOptions.getMergedHeaderProvider(new NoHeaderProvider())); + settingsBuilder.setHeaderProvider( + datastoreOptions.getMergedHeaderProvider(new NoHeaderProvider())); ClientContext clientContext = ClientContext.create(settingsBuilder.build()); return clientContext; } @@ -184,6 +190,5 @@ protected DatastoreSettings.Builder setInternalHeaderProvider( HeaderProvider internalHeaderProvider) { return super.setInternalHeaderProvider(internalHeaderProvider); } - } } 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 ed4dd8dda..e7007ca92 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,6 +16,23 @@ package com.google.cloud.datastore; +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; +import static com.google.common.collect.Iterables.getOnlyElement; +import static com.google.common.truth.Truth.assertThat; +import static org.easymock.EasyMock.createStrictMock; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.verify; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + import com.google.cloud.ServiceOptions; import com.google.cloud.Timestamp; import com.google.cloud.datastore.Query.ResultType; @@ -50,17 +67,6 @@ import com.google.datastore.v1.RunQueryResponse; import com.google.datastore.v1.TransactionOptions; import com.google.protobuf.ByteString; -import org.easymock.EasyMock; -import org.junit.AfterClass; -import org.junit.Assert; -import org.junit.Before; -import org.junit.BeforeClass; -import org.junit.Ignore; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; -import org.threeten.bp.Duration; - import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; @@ -73,23 +79,16 @@ import java.util.Set; import java.util.concurrent.TimeoutException; import java.util.function.Predicate; - -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; -import static com.google.common.collect.Iterables.getOnlyElement; -import static com.google.common.truth.Truth.assertThat; -import static org.easymock.EasyMock.createStrictMock; -import static org.easymock.EasyMock.replay; -import static org.easymock.EasyMock.verify; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; +import org.easymock.EasyMock; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Ignore; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.threeten.bp.Duration; @RunWith(JUnit4.class) public class DatastoreTest { @@ -233,7 +232,7 @@ public void testNewTransactionCommit() { verifyNotUsable(transaction); } - //TODO(gapic_upgrade): Remove the @ignore annotation + // TODO(gapic_upgrade): Remove the @ignore annotation @Ignore("This should be fixed with actionable error implementation") @Test public void testTransactionWithRead() { @@ -256,7 +255,7 @@ public void testTransactionWithRead() { } } - //TODO(gapic_upgrade): Remove the @ignore annotation + // TODO(gapic_upgrade): Remove the @ignore annotation @Ignore("This should be fixed with actionable error implementation") @Test public void testTransactionWithQuery() { diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreTest.java index 033748a90..d1e7646d2 100644 --- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreTest.java +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreTest.java @@ -16,6 +16,18 @@ package com.google.cloud.datastore.it; +import static com.google.cloud.datastore.aggregation.Aggregation.count; +import static com.google.common.collect.Iterables.getOnlyElement; +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + import com.google.cloud.Timestamp; import com.google.cloud.Tuple; import com.google.cloud.datastore.AggregationQuery; @@ -59,16 +71,6 @@ import com.google.common.collect.ImmutableList; import com.google.datastore.v1.TransactionOptions; import com.google.datastore.v1.TransactionOptions.ReadOnly; -import org.junit.After; -import org.junit.AfterClass; -import org.junit.Before; -import org.junit.Ignore; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.Timeout; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; - import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -81,18 +83,15 @@ import java.util.concurrent.Future; import java.util.function.BiConsumer; import java.util.function.Consumer; - -import static com.google.cloud.datastore.aggregation.Aggregation.count; -import static com.google.common.collect.Iterables.getOnlyElement; -import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.Ignore; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.Timeout; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; @RunWith(Parameterized.class) public class ITDatastoreTest { @@ -1393,7 +1392,7 @@ public Integer run(DatastoreReaderWriter transaction) { } } - //TODO(gapic_upgrade): Remove the @ignore annotation + // TODO(gapic_upgrade): Remove the @ignore annotation @Ignore("This should be fixed with actionable error implementation") @Test public void testRunInTransactionReadWrite() { From acfbaef6a505227e0fa1f9057a10aedd1d2f8e46 Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Fri, 17 Nov 2023 06:15:04 +0000 Subject: [PATCH 09/26] incorporating feedbacks --- .../cloud/datastore/DatastoreOptions.java | 9 +-------- .../datastore/spi/v1/GrpcDatastoreRpc.java | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 9 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 e0afba824..1eb7f5105 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 @@ -122,7 +122,7 @@ protected String getDefaultHost() { System.getProperty( com.google.datastore.v1.client.DatastoreHelper.LOCAL_HOST_ENV_VAR, System.getenv(com.google.datastore.v1.client.DatastoreHelper.LOCAL_HOST_ENV_VAR)); - return host != null ? host : DatastoreDefaults.INSTANCE.getHost(); + return host != null ? host : DatastoreSettings.getDefaultEndpoint(); } @Override @@ -136,13 +136,6 @@ protected String getDefaultProject() { private static class DatastoreDefaults implements ServiceDefaults { - private static final DatastoreDefaults INSTANCE = new DatastoreDefaults(); - private final String HOST = DatastoreSettings.getDefaultEndpoint(); - - String getHost() { - return HOST; - } - @Override public DatastoreFactory getDefaultServiceFactory() { return DefaultDatastoreFactory.INSTANCE; diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java index 3ceb67624..cc1e411af 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java @@ -1,8 +1,25 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package com.google.cloud.datastore.spi.v1; import static java.util.concurrent.TimeUnit.SECONDS; import com.google.api.core.ApiFunction; +import com.google.api.core.InternalApi; import com.google.api.gax.core.BackgroundResource; import com.google.api.gax.core.GaxProperties; import com.google.api.gax.grpc.GrpcCallContext; @@ -43,7 +60,7 @@ import java.io.IOException; import java.util.Collections; -// TODO(gapic_upgrade): Make it implement AutoCloseable +@InternalApi public class GrpcDatastoreRpc implements AutoCloseable, DatastoreRpc { private final GrpcDatastoreStub datastoreStub; From 60ee45d27412aab8140fc7065010633cf59a0ae8 Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Sat, 18 Nov 2023 03:16:03 +0000 Subject: [PATCH 10/26] pinging emulator after each test for debugging --- .../testing/LocalDatastoreHelper.java | 24 +++++++++++++++++++ .../google/cloud/datastore/DatastoreTest.java | 1 + 2 files changed, 25 insertions(+) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java index 26b892186..01e334aed 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java @@ -25,6 +25,10 @@ import com.google.cloud.testing.BaseEmulatorHelper; import com.google.common.collect.ImmutableList; import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.io.OutputStream; +import java.net.HttpURLConnection; import java.net.MalformedURLException; import java.net.URL; import java.nio.file.FileVisitResult; @@ -38,6 +42,8 @@ import java.util.UUID; import java.util.concurrent.TimeoutException; import java.util.logging.Logger; + +import com.google.common.io.CharStreams; import org.threeten.bp.Duration; /** @@ -95,6 +101,24 @@ public class LocalDatastoreHelper extends BaseEmulatorHelper { .setCredentials(NoCredentials.getInstance()) .setRetrySettings(ServiceOptions.getNoRetrySettings()); + String sendGetRequest(String request) throws IOException { + URL url = new URL("http", DEFAULT_HOST, this.getPort(), request); + HttpURLConnection con = (HttpURLConnection) url.openConnection(); + con.setRequestMethod("GET"); + + InputStream in = con.getInputStream(); + String response = CharStreams.toString(new InputStreamReader(con.getInputStream())); + in.close(); + return response; + } + public String checkHealth() { + try { + return sendGetRequest("/"); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + /** A builder for {@code LocalDatastoreHelper} objects. */ public static class Builder { private double consistency; 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 e7007ca92..da4958d9f 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 @@ -175,6 +175,7 @@ public static void beforeClass() throws IOException, InterruptedException { @Before public void setUp() { + System.out.println(helper.checkHealth());; rpcFactoryMock = EasyMock.createStrictMock(DatastoreRpcFactory.class); rpcMock = EasyMock.createStrictMock(DatastoreRpc.class); rpcMockOptions = From d42e3b995fe56ded8c5f080a262e8ea02c00c9d9 Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Sat, 18 Nov 2023 03:19:54 +0000 Subject: [PATCH 11/26] Revert "pinging emulator after each test for debugging" This reverts commit 60ee45d27412aab8140fc7065010633cf59a0ae8. --- .../testing/LocalDatastoreHelper.java | 24 ------------------- .../google/cloud/datastore/DatastoreTest.java | 1 - 2 files changed, 25 deletions(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java index 01e334aed..26b892186 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java @@ -25,10 +25,6 @@ import com.google.cloud.testing.BaseEmulatorHelper; import com.google.common.collect.ImmutableList; import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; -import java.io.OutputStream; -import java.net.HttpURLConnection; import java.net.MalformedURLException; import java.net.URL; import java.nio.file.FileVisitResult; @@ -42,8 +38,6 @@ import java.util.UUID; import java.util.concurrent.TimeoutException; import java.util.logging.Logger; - -import com.google.common.io.CharStreams; import org.threeten.bp.Duration; /** @@ -101,24 +95,6 @@ public class LocalDatastoreHelper extends BaseEmulatorHelper { .setCredentials(NoCredentials.getInstance()) .setRetrySettings(ServiceOptions.getNoRetrySettings()); - String sendGetRequest(String request) throws IOException { - URL url = new URL("http", DEFAULT_HOST, this.getPort(), request); - HttpURLConnection con = (HttpURLConnection) url.openConnection(); - con.setRequestMethod("GET"); - - InputStream in = con.getInputStream(); - String response = CharStreams.toString(new InputStreamReader(con.getInputStream())); - in.close(); - return response; - } - public String checkHealth() { - try { - return sendGetRequest("/"); - } catch (IOException e) { - throw new RuntimeException(e); - } - } - /** A builder for {@code LocalDatastoreHelper} objects. */ public static class Builder { private double consistency; 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 da4958d9f..e7007ca92 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 @@ -175,7 +175,6 @@ public static void beforeClass() throws IOException, InterruptedException { @Before public void setUp() { - System.out.println(helper.checkHealth());; rpcFactoryMock = EasyMock.createStrictMock(DatastoreRpcFactory.class); rpcMock = EasyMock.createStrictMock(DatastoreRpc.class); rpcMockOptions = From 1d9d7e6c270e8cf14438ae3ad660798773e5ba48 Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Sat, 18 Nov 2023 03:30:36 +0000 Subject: [PATCH 12/26] Reapply "pinging emulator after each test for debugging" This reverts commit d42e3b995fe56ded8c5f080a262e8ea02c00c9d9. --- .../testing/LocalDatastoreHelper.java | 24 +++++++++++++++++++ .../google/cloud/datastore/DatastoreTest.java | 1 + 2 files changed, 25 insertions(+) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java index 26b892186..01e334aed 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java @@ -25,6 +25,10 @@ import com.google.cloud.testing.BaseEmulatorHelper; import com.google.common.collect.ImmutableList; import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.io.OutputStream; +import java.net.HttpURLConnection; import java.net.MalformedURLException; import java.net.URL; import java.nio.file.FileVisitResult; @@ -38,6 +42,8 @@ import java.util.UUID; import java.util.concurrent.TimeoutException; import java.util.logging.Logger; + +import com.google.common.io.CharStreams; import org.threeten.bp.Duration; /** @@ -95,6 +101,24 @@ public class LocalDatastoreHelper extends BaseEmulatorHelper { .setCredentials(NoCredentials.getInstance()) .setRetrySettings(ServiceOptions.getNoRetrySettings()); + String sendGetRequest(String request) throws IOException { + URL url = new URL("http", DEFAULT_HOST, this.getPort(), request); + HttpURLConnection con = (HttpURLConnection) url.openConnection(); + con.setRequestMethod("GET"); + + InputStream in = con.getInputStream(); + String response = CharStreams.toString(new InputStreamReader(con.getInputStream())); + in.close(); + return response; + } + public String checkHealth() { + try { + return sendGetRequest("/"); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + /** A builder for {@code LocalDatastoreHelper} objects. */ public static class Builder { private double consistency; 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 e7007ca92..da4958d9f 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 @@ -175,6 +175,7 @@ public static void beforeClass() throws IOException, InterruptedException { @Before public void setUp() { + System.out.println(helper.checkHealth());; rpcFactoryMock = EasyMock.createStrictMock(DatastoreRpcFactory.class); rpcMock = EasyMock.createStrictMock(DatastoreRpc.class); rpcMockOptions = From d7b652ec24b9cf4d84b81045acd8c7e140e8bc54 Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Sat, 18 Nov 2023 03:54:48 +0000 Subject: [PATCH 13/26] more debugging --- .../cloud/datastore/testing/LocalDatastoreHelper.java | 10 ++++++++++ .../java/com/google/cloud/datastore/DatastoreTest.java | 8 +++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java index 01e334aed..368a0a5a1 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java @@ -102,6 +102,7 @@ public class LocalDatastoreHelper extends BaseEmulatorHelper { .setRetrySettings(ServiceOptions.getNoRetrySettings()); String sendGetRequest(String request) throws IOException { + URL url = new URL("http", DEFAULT_HOST, this.getPort(), request); HttpURLConnection con = (HttpURLConnection) url.openConnection(); con.setRequestMethod("GET"); @@ -111,6 +112,15 @@ String sendGetRequest(String request) throws IOException { in.close(); return response; } + + public void checkProcessStatus() { + for (EmulatorRunner emulatorRunner : emulatorRunners) { + if (emulatorRunner.getProcess() != null && !emulatorRunner.getProcess().isAlive()) { + Process process = emulatorRunner.getProcess(); + System.out.println("Exit code: " + process.exitValue()); + } + } + } public String checkHealth() { try { return sendGetRequest("/"); 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 da4958d9f..b6821d07f 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 @@ -80,6 +80,7 @@ import java.util.concurrent.TimeoutException; import java.util.function.Predicate; import org.easymock.EasyMock; +import org.junit.After; import org.junit.AfterClass; import org.junit.Assert; import org.junit.Before; @@ -175,7 +176,7 @@ public static void beforeClass() throws IOException, InterruptedException { @Before public void setUp() { - System.out.println(helper.checkHealth());; + helper.checkProcessStatus(); rpcFactoryMock = EasyMock.createStrictMock(DatastoreRpcFactory.class); rpcMock = EasyMock.createStrictMock(DatastoreRpc.class); rpcMockOptions = @@ -191,6 +192,11 @@ public void setUp() { datastore.add(ENTITY1, ENTITY2); } + @After + public void tearDown() throws Exception { + helper.checkProcessStatus(); + } + @AfterClass public static void afterClass() throws IOException, InterruptedException, TimeoutException { helper.stop(Duration.ofMinutes(1)); From 9827fb93c5222caa4968277e309d28445577b94e Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Sat, 18 Nov 2023 18:11:35 +0000 Subject: [PATCH 14/26] Constant ping to avoid flaky behaviour of /shutdown endpoint --- .../datastore/testing/LocalDatastoreHelper.java | 12 +----------- .../com/google/cloud/datastore/DatastoreTest.java | 4 ++-- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java index 368a0a5a1..e4af7d8b5 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java @@ -24,10 +24,10 @@ import com.google.cloud.datastore.DatastoreOptions; import com.google.cloud.testing.BaseEmulatorHelper; import com.google.common.collect.ImmutableList; +import com.google.common.io.CharStreams; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; -import java.io.OutputStream; import java.net.HttpURLConnection; import java.net.MalformedURLException; import java.net.URL; @@ -42,8 +42,6 @@ import java.util.UUID; import java.util.concurrent.TimeoutException; import java.util.logging.Logger; - -import com.google.common.io.CharStreams; import org.threeten.bp.Duration; /** @@ -113,14 +111,6 @@ String sendGetRequest(String request) throws IOException { return response; } - public void checkProcessStatus() { - for (EmulatorRunner emulatorRunner : emulatorRunners) { - if (emulatorRunner.getProcess() != null && !emulatorRunner.getProcess().isAlive()) { - Process process = emulatorRunner.getProcess(); - System.out.println("Exit code: " + process.exitValue()); - } - } - } public String checkHealth() { try { return sendGetRequest("/"); 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 b6821d07f..6b778c6c2 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 @@ -176,7 +176,6 @@ public static void beforeClass() throws IOException, InterruptedException { @Before public void setUp() { - helper.checkProcessStatus(); rpcFactoryMock = EasyMock.createStrictMock(DatastoreRpcFactory.class); rpcMock = EasyMock.createStrictMock(DatastoreRpc.class); rpcMockOptions = @@ -194,7 +193,8 @@ public void setUp() { @After public void tearDown() throws Exception { - helper.checkProcessStatus(); + // TODO(gapic_upgrade): Constant ping: temporarily addressing the connection refused error + helper.checkHealth(); } @AfterClass From 38725f0baafff0e19f290beebb9c951e1432a377 Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Sat, 18 Nov 2023 18:19:46 +0000 Subject: [PATCH 15/26] fixing test --- .../test/java/com/google/cloud/datastore/DatastoreTest.java | 2 +- .../test/java/com/google/datastore/snippets/ConceptsTest.java | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) 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 6b778c6c2..1d2d2579f 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 @@ -193,7 +193,7 @@ public void setUp() { @After public void tearDown() throws Exception { - // TODO(gapic_upgrade): Constant ping: temporarily addressing the connection refused error + // TODO(gapic_upgrade): Constant ping: temporarily addressing the flaky connection refused error helper.checkHealth(); } diff --git a/samples/snippets/src/test/java/com/google/datastore/snippets/ConceptsTest.java b/samples/snippets/src/test/java/com/google/datastore/snippets/ConceptsTest.java index 1397728ba..6fc52aa2a 100644 --- a/samples/snippets/src/test/java/com/google/datastore/snippets/ConceptsTest.java +++ b/samples/snippets/src/test/java/com/google/datastore/snippets/ConceptsTest.java @@ -137,6 +137,9 @@ public void tearDown() throws Exception { KeyQuery taskQuery = Query.newKeyQueryBuilder().setKind("Task").build(); Key[] taskKeysToDelete = Iterators.toArray(datastoreRealBackend.run(taskQuery), Key.class); datastoreRealBackend.delete(taskKeysToDelete); + + // TODO(gapic_upgrade): Constant ping: temporarily addressing the flaky connection refused error + HELPER.checkHealth(); } /** From b15a9a9fde14b1dd083833faa81bf298225a1561 Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Mon, 20 Nov 2023 06:16:29 +0000 Subject: [PATCH 16/26] checking if emulator is running before sending a shutdown command --- .../cloud/datastore/testing/LocalDatastoreHelper.java | 2 ++ .../test/java/com/google/cloud/datastore/DatastoreTest.java | 6 ------ 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java index e4af7d8b5..063d34e65 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java @@ -344,6 +344,8 @@ public void reset() throws IOException { */ @Override public void stop(Duration timeout) throws IOException, InterruptedException, TimeoutException { + // TODO(gapic_upgrade): Temporarily addressing the flaky connection refused error + checkHealth(); sendPostRequest("/shutdown"); waitForProcess(timeout); deleteRecursively(gcdPath); 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 1d2d2579f..f22edd838 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 @@ -191,12 +191,6 @@ public void setUp() { datastore.add(ENTITY1, ENTITY2); } - @After - public void tearDown() throws Exception { - // TODO(gapic_upgrade): Constant ping: temporarily addressing the flaky connection refused error - helper.checkHealth(); - } - @AfterClass public static void afterClass() throws IOException, InterruptedException, TimeoutException { helper.stop(Duration.ofMinutes(1)); From ec3888509d8d7ce0fe71080c85e3a3986bf059eb Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Mon, 20 Nov 2023 06:22:04 +0000 Subject: [PATCH 17/26] fix lint --- .../test/java/com/google/cloud/datastore/DatastoreTest.java | 1 - .../test/java/com/google/datastore/snippets/ConceptsTest.java | 3 --- 2 files changed, 4 deletions(-) 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 f22edd838..e7007ca92 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 @@ -80,7 +80,6 @@ import java.util.concurrent.TimeoutException; import java.util.function.Predicate; import org.easymock.EasyMock; -import org.junit.After; import org.junit.AfterClass; import org.junit.Assert; import org.junit.Before; diff --git a/samples/snippets/src/test/java/com/google/datastore/snippets/ConceptsTest.java b/samples/snippets/src/test/java/com/google/datastore/snippets/ConceptsTest.java index 6fc52aa2a..1397728ba 100644 --- a/samples/snippets/src/test/java/com/google/datastore/snippets/ConceptsTest.java +++ b/samples/snippets/src/test/java/com/google/datastore/snippets/ConceptsTest.java @@ -137,9 +137,6 @@ public void tearDown() throws Exception { KeyQuery taskQuery = Query.newKeyQueryBuilder().setKind("Task").build(); Key[] taskKeysToDelete = Iterators.toArray(datastoreRealBackend.run(taskQuery), Key.class); datastoreRealBackend.delete(taskKeysToDelete); - - // TODO(gapic_upgrade): Constant ping: temporarily addressing the flaky connection refused error - HELPER.checkHealth(); } /** From 7bd2c5546da5df174a3cb94443806ef83a451518 Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Wed, 22 Nov 2023 10:48:34 +0000 Subject: [PATCH 18/26] implement helper method for localhost --- .../cloud/datastore/DatastoreUtils.java | 34 +++++++++++++++++++ .../datastore/spi/v1/GrpcDatastoreRpc.java | 9 +++-- .../cloud/datastore/DatastoreUtilsTest.java | 26 ++++++++++++++ 3 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreUtils.java create mode 100644 google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreUtilsTest.java diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreUtils.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreUtils.java new file mode 100644 index 000000000..d952b50f9 --- /dev/null +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreUtils.java @@ -0,0 +1,34 @@ +package com.google.cloud.datastore; + +import com.google.api.core.InternalApi; +import com.google.common.base.Strings; +import java.net.InetAddress; +import java.net.URL; + +@InternalApi +public class DatastoreUtils { + + public static boolean isLocalHost(String host) { + if (Strings.isNullOrEmpty(host)) { + return false; + } + try { + String normalizedHost = "http://" + removeScheme(host); + InetAddress hostAddr = InetAddress.getByName(new URL(normalizedHost).getHost()); + return hostAddr.isAnyLocalAddress() || hostAddr.isLoopbackAddress(); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + + public 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; + } +} diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java index cc1e411af..2035e0cc2 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java @@ -16,6 +16,7 @@ package com.google.cloud.datastore.spi.v1; +import static com.google.cloud.datastore.DatastoreUtils.removeScheme; import static java.util.concurrent.TimeUnit.SECONDS; import com.google.api.core.ApiFunction; @@ -33,6 +34,7 @@ import com.google.cloud.ServiceOptions; import com.google.cloud.datastore.DatastoreException; import com.google.cloud.datastore.DatastoreOptions; +import com.google.cloud.datastore.DatastoreUtils; import com.google.cloud.datastore.v1.DatastoreSettings; import com.google.cloud.datastore.v1.stub.DatastoreStubSettings; import com.google.cloud.datastore.v1.stub.GrpcDatastoreStub; @@ -145,14 +147,17 @@ public RunAggregationQueryResponse runAggregationQuery(RunAggregationQueryReques } private boolean isEmulator(DatastoreOptions datastoreOptions) { - return datastoreOptions.getHost().contains("localhost") + return DatastoreUtils.isLocalHost(datastoreOptions.getHost()) || NoCredentials.getInstance().equals(datastoreOptions.getCredentials()); } private ClientContext getClientContextForEmulator(DatastoreOptions datastoreOptions) throws IOException { + // TODO(gapic_upgrade): ensure there is no scheme in host (HttpDatastoreRpc) ManagedChannel managedChannel = - ManagedChannelBuilder.forTarget(datastoreOptions.getHost()).usePlaintext().build(); + ManagedChannelBuilder.forTarget(removeScheme(datastoreOptions.getHost())) + .usePlaintext() + .build(); TransportChannel transportChannel = GrpcTransportChannel.create(managedChannel); return ClientContext.newBuilder() .setCredentials(null) diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreUtilsTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreUtilsTest.java new file mode 100644 index 000000000..33030f3ff --- /dev/null +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreUtilsTest.java @@ -0,0 +1,26 @@ +package com.google.cloud.datastore; + +import static com.google.cloud.datastore.DatastoreUtils.isLocalHost; +import static com.google.cloud.datastore.DatastoreUtils.removeScheme; +import static com.google.common.truth.Truth.assertThat; + +import org.junit.Test; + +public class DatastoreUtilsTest { + + @Test + public void testIsLocalHost() { + assertThat(isLocalHost(null)).isFalse(); + assertThat(isLocalHost("")).isFalse(); + assertThat(isLocalHost("http://localhost:9090")).isTrue(); + assertThat(isLocalHost("https://localhost:9090")).isTrue(); + assertThat(isLocalHost("10.10.10.10:9090")).isFalse(); + } + + @Test + public void testRemoveScheme() { + assertThat(removeScheme("http://localhost:9090")).isEqualTo("localhost:9090"); + assertThat(removeScheme("https://localhost:9090")).isEqualTo("localhost:9090"); + assertThat(removeScheme("https://localhost:9090")).isEqualTo("localhost:9090"); + } +} From 0517cb6d5da74814e6adbcbce8a22743d10c5f47 Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Wed, 22 Nov 2023 14:24:53 +0000 Subject: [PATCH 19/26] fix header lint --- .../google/cloud/datastore/DatastoreUtils.java | 16 ++++++++++++++++ .../cloud/datastore/DatastoreUtilsTest.java | 16 ++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreUtils.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreUtils.java index d952b50f9..ae1c7e07d 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreUtils.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreUtils.java @@ -1,3 +1,19 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package com.google.cloud.datastore; import com.google.api.core.InternalApi; diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreUtilsTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreUtilsTest.java index 33030f3ff..9a5855d30 100644 --- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreUtilsTest.java +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreUtilsTest.java @@ -1,3 +1,19 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package com.google.cloud.datastore; import static com.google.cloud.datastore.DatastoreUtils.isLocalHost; From 7f4ce8d925484e30e7db85f82e8f6d11eec1500b Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Wed, 22 Nov 2023 15:55:57 +0000 Subject: [PATCH 20/26] moving emulator health check to src/test --- .../testing/LocalDatastoreHelper.java | 21 -------- .../google/cloud/datastore/DatastoreTest.java | 2 + .../google/cloud/datastore/EmulatorUtils.java | 49 +++++++++++++++++++ 3 files changed, 51 insertions(+), 21 deletions(-) create mode 100644 google-cloud-datastore/src/test/java/com/google/cloud/datastore/EmulatorUtils.java diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java index 063d34e65..640e801db 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java @@ -99,25 +99,6 @@ public class LocalDatastoreHelper extends BaseEmulatorHelper { .setCredentials(NoCredentials.getInstance()) .setRetrySettings(ServiceOptions.getNoRetrySettings()); - String sendGetRequest(String request) throws IOException { - - URL url = new URL("http", DEFAULT_HOST, this.getPort(), request); - HttpURLConnection con = (HttpURLConnection) url.openConnection(); - con.setRequestMethod("GET"); - - InputStream in = con.getInputStream(); - String response = CharStreams.toString(new InputStreamReader(con.getInputStream())); - in.close(); - return response; - } - - public String checkHealth() { - try { - return sendGetRequest("/"); - } catch (IOException e) { - throw new RuntimeException(e); - } - } /** A builder for {@code LocalDatastoreHelper} objects. */ public static class Builder { @@ -344,8 +325,6 @@ public void reset() throws IOException { */ @Override public void stop(Duration timeout) throws IOException, InterruptedException, TimeoutException { - // TODO(gapic_upgrade): Temporarily addressing the flaky connection refused error - checkHealth(); sendPostRequest("/shutdown"); waitForProcess(timeout); deleteRecursively(gcdPath); 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 e7007ca92..8766f0c1a 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 @@ -192,6 +192,8 @@ public void setUp() { @AfterClass public static void afterClass() throws IOException, InterruptedException, TimeoutException { + // TODO(gapic_upgrade): Temporarily addressing the flaky connection refused error + EmulatorUtils.checkHealth(helper.getPort()); helper.stop(Duration.ofMinutes(1)); } diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/EmulatorUtils.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/EmulatorUtils.java new file mode 100644 index 000000000..0d3f8599f --- /dev/null +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/EmulatorUtils.java @@ -0,0 +1,49 @@ +/* + * Copyright 2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.datastore; + +import com.google.common.io.CharStreams; +import org.easymock.EasyMock; +import org.easymock.IArgumentMatcher; + +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.net.HttpURLConnection; +import java.net.URL; +import java.util.function.Predicate; + +public class EmulatorUtils { + static String sendGetRequest(int port, String request) throws IOException { + + URL url = new URL("http", "localhost", port, request); + HttpURLConnection con = (HttpURLConnection) url.openConnection(); + con.setRequestMethod("GET"); + + InputStream in = con.getInputStream(); + String response = CharStreams.toString(new InputStreamReader(con.getInputStream())); + in.close(); + return response; + } + + public static String checkHealth(int port) { + try { + return sendGetRequest(port, "/"); + } catch (IOException e) { + throw new RuntimeException(e); + } + } +} From ef5f00215847ab94f34d18714196976378154f19 Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Wed, 22 Nov 2023 16:01:07 +0000 Subject: [PATCH 21/26] fix lint --- .../google/cloud/datastore/testing/LocalDatastoreHelper.java | 5 ----- .../test/java/com/google/cloud/datastore/EmulatorUtils.java | 4 ---- 2 files changed, 9 deletions(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java index 640e801db..26b892186 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java @@ -24,11 +24,7 @@ import com.google.cloud.datastore.DatastoreOptions; import com.google.cloud.testing.BaseEmulatorHelper; import com.google.common.collect.ImmutableList; -import com.google.common.io.CharStreams; import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; -import java.net.HttpURLConnection; import java.net.MalformedURLException; import java.net.URL; import java.nio.file.FileVisitResult; @@ -99,7 +95,6 @@ public class LocalDatastoreHelper extends BaseEmulatorHelper { .setCredentials(NoCredentials.getInstance()) .setRetrySettings(ServiceOptions.getNoRetrySettings()); - /** A builder for {@code LocalDatastoreHelper} objects. */ public static class Builder { private double consistency; diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/EmulatorUtils.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/EmulatorUtils.java index 0d3f8599f..45d443301 100644 --- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/EmulatorUtils.java +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/EmulatorUtils.java @@ -16,15 +16,11 @@ package com.google.cloud.datastore; import com.google.common.io.CharStreams; -import org.easymock.EasyMock; -import org.easymock.IArgumentMatcher; - import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.net.HttpURLConnection; import java.net.URL; -import java.util.function.Predicate; public class EmulatorUtils { static String sendGetRequest(int port, String request) throws IOException { From 9b43798422dc0627b1b2bc6dea8b9f4169682292 Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Wed, 22 Nov 2023 17:05:13 +0000 Subject: [PATCH 22/26] adding no extra headers --- .../datastore/spi/v1/GrpcDatastoreRpc.java | 46 +------------------ 1 file changed, 2 insertions(+), 44 deletions(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java index 2035e0cc2..1f3b7817c 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java @@ -22,16 +22,12 @@ import com.google.api.core.ApiFunction; import com.google.api.core.InternalApi; import com.google.api.gax.core.BackgroundResource; -import com.google.api.gax.core.GaxProperties; import com.google.api.gax.grpc.GrpcCallContext; import com.google.api.gax.grpc.GrpcTransportChannel; import com.google.api.gax.rpc.ClientContext; -import com.google.api.gax.rpc.HeaderProvider; -import com.google.api.gax.rpc.NoHeaderProvider; import com.google.api.gax.rpc.TransportChannel; import com.google.api.gax.rpc.UnaryCallSettings; import com.google.cloud.NoCredentials; -import com.google.cloud.ServiceOptions; import com.google.cloud.datastore.DatastoreException; import com.google.cloud.datastore.DatastoreOptions; import com.google.cloud.datastore.DatastoreUtils; @@ -39,7 +35,6 @@ import com.google.cloud.datastore.v1.stub.DatastoreStubSettings; import com.google.cloud.datastore.v1.stub.GrpcDatastoreStub; import com.google.cloud.grpc.GrpcTransportOptions; -import com.google.common.base.Strings; import com.google.datastore.v1.AllocateIdsRequest; import com.google.datastore.v1.AllocateIdsResponse; import com.google.datastore.v1.BeginTransactionRequest; @@ -168,49 +163,12 @@ private ClientContext getClientContextForEmulator(DatastoreOptions datastoreOpti } private ClientContext getClientContext(DatastoreOptions datastoreOptions) throws IOException { - HeaderProvider internalHeaderProvider = - DatastoreSettings.defaultApiClientHeaderProviderBuilder() - .setClientLibToken( - ServiceOptions.getGoogApiClientLibName(), - GaxProperties.getLibraryVersion(datastoreOptions.getClass())) - .setResourceToken(getResourceToken(datastoreOptions)) - .build(); - - DatastoreSettingsBuilder settingsBuilder = - new DatastoreSettingsBuilder(DatastoreSettings.newBuilder().build()); + DatastoreSettings.Builder settingsBuilder = DatastoreSettings.newBuilder(); settingsBuilder.setCredentialsProvider( GrpcTransportOptions.setUpCredentialsProvider(datastoreOptions)); settingsBuilder.setTransportChannelProvider( GrpcTransportOptions.setUpChannelProvider( DatastoreSettings.defaultGrpcTransportProviderBuilder(), datastoreOptions)); - settingsBuilder.setInternalHeaderProvider(internalHeaderProvider); - settingsBuilder.setHeaderProvider( - datastoreOptions.getMergedHeaderProvider(new NoHeaderProvider())); - ClientContext clientContext = ClientContext.create(settingsBuilder.build()); - return clientContext; - } - - private String getResourceToken(DatastoreOptions datastoreOptions) { - StringBuilder builder = new StringBuilder("project_id="); - builder.append(datastoreOptions.getProjectId()); - if (!Strings.isNullOrEmpty(datastoreOptions.getDatabaseId())) { - builder.append("&database_id="); - builder.append(datastoreOptions.getDatabaseId()); - } - return builder.toString(); - } - - // This class is needed solely to get access to protected method setInternalHeaderProvider() - private static class DatastoreSettingsBuilder extends DatastoreSettings.Builder { - - private DatastoreSettingsBuilder(DatastoreSettings settings) { - super(settings); - } - - @Override - protected DatastoreSettings.Builder setInternalHeaderProvider( - HeaderProvider internalHeaderProvider) { - return super.setInternalHeaderProvider(internalHeaderProvider); - } + return ClientContext.create(settingsBuilder.build()); } } From cb80dd12f89479f577c3a29711759ef3a6538d6b Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Thu, 23 Nov 2023 06:14:49 +0000 Subject: [PATCH 23/26] minor cleanup --- .../com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java index 1f3b7817c..b3f0811fe 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java @@ -16,6 +16,7 @@ package com.google.cloud.datastore.spi.v1; +import static com.google.cloud.datastore.DatastoreUtils.isLocalHost; import static com.google.cloud.datastore.DatastoreUtils.removeScheme; import static java.util.concurrent.TimeUnit.SECONDS; @@ -142,13 +143,12 @@ public RunAggregationQueryResponse runAggregationQuery(RunAggregationQueryReques } private boolean isEmulator(DatastoreOptions datastoreOptions) { - return DatastoreUtils.isLocalHost(datastoreOptions.getHost()) + return isLocalHost(datastoreOptions.getHost()) || NoCredentials.getInstance().equals(datastoreOptions.getCredentials()); } private ClientContext getClientContextForEmulator(DatastoreOptions datastoreOptions) throws IOException { - // TODO(gapic_upgrade): ensure there is no scheme in host (HttpDatastoreRpc) ManagedChannel managedChannel = ManagedChannelBuilder.forTarget(removeScheme(datastoreOptions.getHost())) .usePlaintext() From c86a702d7d3d550ece72f13bb0a8fb061b42184c Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Mon, 27 Nov 2023 16:00:48 +0000 Subject: [PATCH 24/26] using mutlipleAttemptsRule in DatastoreTest --- .../google/cloud/datastore/DatastoreTest.java | 7 ++- .../google/cloud/datastore/EmulatorUtils.java | 45 ------------------- .../datastore/it/MultipleAttemptsRule.java | 2 +- 3 files changed, 6 insertions(+), 48 deletions(-) delete mode 100644 google-cloud-datastore/src/test/java/com/google/cloud/datastore/EmulatorUtils.java 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 8766f0c1a..d844bf36e 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 @@ -38,6 +38,7 @@ import com.google.cloud.datastore.Query.ResultType; import com.google.cloud.datastore.StructuredQuery.OrderBy; import com.google.cloud.datastore.StructuredQuery.PropertyFilter; +import com.google.cloud.datastore.it.MultipleAttemptsRule; import com.google.cloud.datastore.spi.DatastoreRpcFactory; import com.google.cloud.datastore.spi.v1.DatastoreRpc; import com.google.cloud.datastore.testing.LocalDatastoreHelper; @@ -85,6 +86,7 @@ import org.junit.Before; import org.junit.BeforeClass; import org.junit.Ignore; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -92,6 +94,9 @@ @RunWith(JUnit4.class) public class DatastoreTest { + private static final int NUMBER_OF_ATTEMPTS = 5; + @Rule + public MultipleAttemptsRule rr = new MultipleAttemptsRule(NUMBER_OF_ATTEMPTS, 10); private static LocalDatastoreHelper helper = LocalDatastoreHelper.create(1.0); private static final DatastoreOptions options = helper.getOptions(); @@ -192,8 +197,6 @@ public void setUp() { @AfterClass public static void afterClass() throws IOException, InterruptedException, TimeoutException { - // TODO(gapic_upgrade): Temporarily addressing the flaky connection refused error - EmulatorUtils.checkHealth(helper.getPort()); helper.stop(Duration.ofMinutes(1)); } diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/EmulatorUtils.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/EmulatorUtils.java deleted file mode 100644 index 45d443301..000000000 --- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/EmulatorUtils.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright 2023 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.google.cloud.datastore; - -import com.google.common.io.CharStreams; -import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; -import java.net.HttpURLConnection; -import java.net.URL; - -public class EmulatorUtils { - static String sendGetRequest(int port, String request) throws IOException { - - URL url = new URL("http", "localhost", port, request); - HttpURLConnection con = (HttpURLConnection) url.openConnection(); - con.setRequestMethod("GET"); - - InputStream in = con.getInputStream(); - String response = CharStreams.toString(new InputStreamReader(con.getInputStream())); - in.close(); - return response; - } - - public static String checkHealth(int port) { - try { - return sendGetRequest(port, "/"); - } catch (IOException e) { - throw new RuntimeException(e); - } - } -} diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/MultipleAttemptsRule.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/MultipleAttemptsRule.java index 8472f3131..ce9a226a6 100644 --- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/MultipleAttemptsRule.java +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/MultipleAttemptsRule.java @@ -37,7 +37,7 @@ public final class MultipleAttemptsRule implements TestRule { this(attemptCount, 1000L); } - MultipleAttemptsRule(int attemptCount, long initialBackoffMillis) { + public MultipleAttemptsRule(int attemptCount, long initialBackoffMillis) { checkState(attemptCount > 0, "attemptCount must be > 0"); checkState(initialBackoffMillis > 0, "initialBackoffMillis must be > 0"); this.initialBackoffMillis = initialBackoffMillis; From 1351c9733d65d53b9472ca4bd2f28f00b7042f91 Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Mon, 27 Nov 2023 16:03:27 +0000 Subject: [PATCH 25/26] Revert "adding no extra headers" This reverts commit 9b43798422dc0627b1b2bc6dea8b9f4169682292. --- .../datastore/spi/v1/GrpcDatastoreRpc.java | 46 ++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java index b3f0811fe..aaf86aca0 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java @@ -23,12 +23,16 @@ import com.google.api.core.ApiFunction; import com.google.api.core.InternalApi; import com.google.api.gax.core.BackgroundResource; +import com.google.api.gax.core.GaxProperties; import com.google.api.gax.grpc.GrpcCallContext; import com.google.api.gax.grpc.GrpcTransportChannel; import com.google.api.gax.rpc.ClientContext; +import com.google.api.gax.rpc.HeaderProvider; +import com.google.api.gax.rpc.NoHeaderProvider; import com.google.api.gax.rpc.TransportChannel; import com.google.api.gax.rpc.UnaryCallSettings; import com.google.cloud.NoCredentials; +import com.google.cloud.ServiceOptions; import com.google.cloud.datastore.DatastoreException; import com.google.cloud.datastore.DatastoreOptions; import com.google.cloud.datastore.DatastoreUtils; @@ -36,6 +40,7 @@ import com.google.cloud.datastore.v1.stub.DatastoreStubSettings; import com.google.cloud.datastore.v1.stub.GrpcDatastoreStub; import com.google.cloud.grpc.GrpcTransportOptions; +import com.google.common.base.Strings; import com.google.datastore.v1.AllocateIdsRequest; import com.google.datastore.v1.AllocateIdsResponse; import com.google.datastore.v1.BeginTransactionRequest; @@ -163,12 +168,49 @@ private ClientContext getClientContextForEmulator(DatastoreOptions datastoreOpti } private ClientContext getClientContext(DatastoreOptions datastoreOptions) throws IOException { - DatastoreSettings.Builder settingsBuilder = DatastoreSettings.newBuilder(); + HeaderProvider internalHeaderProvider = + DatastoreSettings.defaultApiClientHeaderProviderBuilder() + .setClientLibToken( + ServiceOptions.getGoogApiClientLibName(), + GaxProperties.getLibraryVersion(datastoreOptions.getClass())) + .setResourceToken(getResourceToken(datastoreOptions)) + .build(); + + DatastoreSettingsBuilder settingsBuilder = + new DatastoreSettingsBuilder(DatastoreSettings.newBuilder().build()); settingsBuilder.setCredentialsProvider( GrpcTransportOptions.setUpCredentialsProvider(datastoreOptions)); settingsBuilder.setTransportChannelProvider( GrpcTransportOptions.setUpChannelProvider( DatastoreSettings.defaultGrpcTransportProviderBuilder(), datastoreOptions)); - return ClientContext.create(settingsBuilder.build()); + settingsBuilder.setInternalHeaderProvider(internalHeaderProvider); + settingsBuilder.setHeaderProvider( + datastoreOptions.getMergedHeaderProvider(new NoHeaderProvider())); + ClientContext clientContext = ClientContext.create(settingsBuilder.build()); + return clientContext; + } + + private String getResourceToken(DatastoreOptions datastoreOptions) { + StringBuilder builder = new StringBuilder("project_id="); + builder.append(datastoreOptions.getProjectId()); + if (!Strings.isNullOrEmpty(datastoreOptions.getDatabaseId())) { + builder.append("&database_id="); + builder.append(datastoreOptions.getDatabaseId()); + } + return builder.toString(); + } + + // This class is needed solely to get access to protected method setInternalHeaderProvider() + private static class DatastoreSettingsBuilder extends DatastoreSettings.Builder { + + private DatastoreSettingsBuilder(DatastoreSettings settings) { + super(settings); + } + + @Override + protected DatastoreSettings.Builder setInternalHeaderProvider( + HeaderProvider internalHeaderProvider) { + return super.setInternalHeaderProvider(internalHeaderProvider); + } } } From e5b256586e16a1da868f569dbf87804537a588c4 Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Mon, 27 Nov 2023 16:09:19 +0000 Subject: [PATCH 26/26] using classRule --- .../google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java | 1 - .../java/com/google/cloud/datastore/DatastoreTest.java | 7 ++++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java index aaf86aca0..fe2b2f27b 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java @@ -35,7 +35,6 @@ import com.google.cloud.ServiceOptions; import com.google.cloud.datastore.DatastoreException; import com.google.cloud.datastore.DatastoreOptions; -import com.google.cloud.datastore.DatastoreUtils; import com.google.cloud.datastore.v1.DatastoreSettings; import com.google.cloud.datastore.v1.stub.DatastoreStubSettings; import com.google.cloud.datastore.v1.stub.GrpcDatastoreStub; 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 d844bf36e..b1b36569a 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 @@ -85,8 +85,8 @@ import org.junit.Assert; import org.junit.Before; import org.junit.BeforeClass; +import org.junit.ClassRule; import org.junit.Ignore; -import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -95,8 +95,9 @@ @RunWith(JUnit4.class) public class DatastoreTest { private static final int NUMBER_OF_ATTEMPTS = 5; - @Rule - public MultipleAttemptsRule rr = new MultipleAttemptsRule(NUMBER_OF_ATTEMPTS, 10); + + @ClassRule + public static MultipleAttemptsRule rr = new MultipleAttemptsRule(NUMBER_OF_ATTEMPTS, 10); private static LocalDatastoreHelper helper = LocalDatastoreHelper.create(1.0); private static final DatastoreOptions options = helper.getOptions();