From e17ba348adcd524c7c9714e7b06696fa5323ec54 Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Sat, 4 Nov 2023 18:31:59 +0000 Subject: [PATCH 01/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] =?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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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/32] 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 2b7b85582221655ebf4f03e702f9da10c99a56be Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Fri, 24 Nov 2023 16:40:33 +0000 Subject: [PATCH 24/32] Making DatastoreException extend BaseGrpcServiceException --- .../cloud/datastore/DatastoreException.java | 21 ++++++++++++++----- .../datastore/spi/v1/GrpcDatastoreRpc.java | 1 - .../datastore/DatastoreExceptionTest.java | 7 +++++++ 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java index 512d0a3dc..796c3ea8a 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java @@ -16,9 +16,11 @@ package com.google.cloud.datastore; +import static com.google.cloud.BaseServiceException.isRetryable; + import com.google.cloud.BaseServiceException; import com.google.cloud.RetryHelper.RetryHelperException; -import com.google.cloud.http.BaseHttpServiceException; +import com.google.cloud.grpc.BaseGrpcServiceException; import com.google.common.collect.ImmutableSet; import java.io.IOException; import java.util.Set; @@ -29,7 +31,7 @@ * @see Google Cloud * Datastore error codes */ -public final class DatastoreException extends BaseHttpServiceException { +public final class DatastoreException extends BaseGrpcServiceException { // see https://cloud.google.com/datastore/docs/concepts/errors#Error_Codes" private static final Set RETRYABLE_ERRORS = @@ -38,22 +40,31 @@ public final class DatastoreException extends BaseHttpServiceException { new Error(4, "DEADLINE_EXCEEDED", false), new Error(14, "UNAVAILABLE", true)); private static final long serialVersionUID = 2663750991205874435L; + private String reason; public DatastoreException(int code, String message, String reason) { this(code, message, reason, true, null); + this.reason = reason; } public DatastoreException(int code, String message, String reason, Throwable cause) { - super(code, message, reason, true, RETRYABLE_ERRORS, cause); + super(message, cause, code, isRetryable(code, reason, true, RETRYABLE_ERRORS)); + this.reason = reason; } public DatastoreException( int code, String message, String reason, boolean idempotent, Throwable cause) { - super(code, message, reason, idempotent, RETRYABLE_ERRORS, cause); + super(message, cause, code, isRetryable(code, reason, idempotent, RETRYABLE_ERRORS)); + this.reason = reason; } public DatastoreException(IOException exception) { - super(exception, true, RETRYABLE_ERRORS); + super(exception, true); + } + + @Override + public String getReason() { + return this.reason != null ? this.reason : super.getReason(); } /** 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..4f0ebd835 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 @@ -31,7 +31,6 @@ import com.google.cloud.NoCredentials; 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/DatastoreExceptionTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreExceptionTest.java index 12b99c966..141dff03d 100644 --- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreExceptionTest.java +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreExceptionTest.java @@ -76,6 +76,13 @@ public void testDatastoreException() { assertEquals("message", exception.getMessage()); assertFalse(exception.isRetryable()); assertSame(cause, exception.getCause()); + + exception = new DatastoreException(2, "message", "INTERNAL", true, cause); + assertEquals(2, exception.getCode()); + assertEquals("INTERNAL", exception.getReason()); + assertEquals("message", exception.getMessage()); + assertFalse(exception.isRetryable()); + assertSame(cause, exception.getCause()); } @Test From 7aaa5ed5b55219292c755e8576d4175e2b6a5a09 Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Sat, 25 Nov 2023 14:14:06 +0000 Subject: [PATCH 25/32] Creating DatastoreException through ApiException and exposing methods to extract out domain, reason and metadata --- .../cloud/datastore/DatastoreException.java | 57 +++++++++++++++++++ .../datastore/DatastoreExceptionTest.java | 54 ++++++++++++++++++ 2 files changed, 111 insertions(+) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java index 796c3ea8a..c1f643852 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java @@ -18,11 +18,14 @@ import static com.google.cloud.BaseServiceException.isRetryable; +import com.google.api.gax.rpc.ApiException; +import com.google.api.gax.rpc.ErrorDetails; import com.google.cloud.BaseServiceException; import com.google.cloud.RetryHelper.RetryHelperException; import com.google.cloud.grpc.BaseGrpcServiceException; import com.google.common.collect.ImmutableSet; import java.io.IOException; +import java.util.Map; import java.util.Set; /** @@ -41,6 +44,7 @@ public final class DatastoreException extends BaseGrpcServiceException { new Error(14, "UNAVAILABLE", true)); private static final long serialVersionUID = 2663750991205874435L; private String reason; + private ApiException apiException; public DatastoreException(int code, String message, String reason) { this(code, message, reason, true, null); @@ -62,6 +66,59 @@ public DatastoreException(IOException exception) { super(exception, true); } + public DatastoreException(ApiException apiException) { + super(apiException); + this.apiException = apiException; + } + + /** + * Checks the underlying reason of the exception and if it's {@link ApiException} then return the + * specific domain otherwise null. + * + * @see Domain + * @return the logical grouping to which the "reason" belongs. + */ + public String getDomain() { + if (this.apiException != null) { + return this.apiException.getDomain(); + } + return null; + } + + /** + * Checks the underlying reason of the exception and if it's {@link ApiException} then return a + * map of key-value pairs otherwise null. + * + * @see Metadata + * @return the map of additional structured details about an error. + */ + public Map getMetadata() { + if (this.apiException != null) { + return this.apiException.getMetadata(); + } + return null; + } + + /** + * Checks the underlying reason of the exception and if it's {@link ApiException} then return the + * ErrorDetails otherwise null. + * + * @see Status + * @see Error + * Details + * @return An object containing getters for structured objects from error_details.proto. + */ + public ErrorDetails getErrorDetails() { + if (this.apiException != null) { + return this.apiException.getErrorDetails(); + } + return null; + } + @Override public String getReason() { return this.reason != null ? this.reason : super.getReason(); diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreExceptionTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreExceptionTest.java index 141dff03d..10039f86e 100644 --- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreExceptionTest.java +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreExceptionTest.java @@ -16,6 +16,9 @@ package com.google.cloud.datastore; +import static com.google.common.truth.Truth.assertThat; +import static com.google.rpc.Code.FAILED_PRECONDITION; +import static java.util.Collections.singletonMap; import static org.easymock.EasyMock.createMock; import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.replay; @@ -27,8 +30,16 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import com.google.api.gax.grpc.GrpcStatusCode; +import com.google.api.gax.rpc.ApiException; +import com.google.api.gax.rpc.ApiExceptionFactory; +import com.google.api.gax.rpc.ErrorDetails; +import com.google.api.gax.rpc.StatusCode; import com.google.cloud.BaseServiceException; import com.google.cloud.RetryHelper; +import com.google.protobuf.Any; +import com.google.rpc.ErrorInfo; +import io.grpc.Status; import java.io.IOException; import java.net.SocketTimeoutException; import org.junit.Test; @@ -83,6 +94,27 @@ public void testDatastoreException() { assertEquals("message", exception.getMessage()); assertFalse(exception.isRetryable()); assertSame(cause, exception.getCause()); + + ApiException apiException = + new ApiException("message", cause, GrpcStatusCode.of(Status.Code.NOT_FOUND), true); + exception = new DatastoreException(apiException); + assertEquals(404, exception.getCode()); + assertEquals("NOT_FOUND", exception.getReason()); + assertEquals("message", exception.getMessage()); + assertTrue(exception.isRetryable()); + assertSame(apiException, exception.getCause()); + } + + @Test + public void testApiException() { + ApiException apiException = createApiException(); + DatastoreException datastoreException = new DatastoreException(apiException); + + assertThat(datastoreException.getReason()).isEqualTo("FAILED_PRECONDITION"); + assertThat(datastoreException.getDomain()).isEqualTo("datastore.googleapis.com"); + assertThat(datastoreException.getMetadata()) + .isEqualTo(singletonMap("missing_indexes_url", "__some__url__")); + assertThat(datastoreException.getErrorDetails()).isEqualTo(apiException.getErrorDetails()); } @Test @@ -128,4 +160,26 @@ public void testThrowInvalidRequest() { assertEquals("message a 1", ex.getMessage()); } } + + private ApiException createApiException() { + // Simulating google.rpc.Status with an ErrorInfo + ErrorInfo errorInfo = + ErrorInfo.newBuilder() + .setDomain("datastore.googleapis.com") + .setReason("MISSING_INDEXES") + .putMetadata("missing_indexes_url", "__some__url__") + .build(); + com.google.rpc.Status status = + com.google.rpc.Status.newBuilder() + .setCode(FAILED_PRECONDITION.getNumber()) + .setMessage("The query requires indexes.") + .addDetails(Any.pack(errorInfo)) + .build(); + + // Using Gax to convert to ApiException + StatusCode statusCode = GrpcStatusCode.of(Status.fromCodeValue(status.getCode()).getCode()); + ErrorDetails errorDetails = + ErrorDetails.builder().setRawErrorMessages(status.getDetailsList()).build(); + return ApiExceptionFactory.createException(null, statusCode, true, errorDetails); + } } From 44d0d4991d04c53fb1c22f11f91618592e68aed1 Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Sat, 25 Nov 2023 17:39:34 +0000 Subject: [PATCH 26/32] Wrapping ApiException in DatastoreException before throwing it --- .../cloud/datastore/DatastoreException.java | 3 + .../datastore/DatastoreExceptionTest.java | 61 +++++++++++-------- 2 files changed, 40 insertions(+), 24 deletions(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java index c1f643852..0e7523187 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java @@ -132,6 +132,9 @@ public String getReason() { */ static DatastoreException translateAndThrow(RetryHelperException ex) { BaseServiceException.translate(ex); + if (ex.getCause() instanceof ApiException) { + throw new DatastoreException((ApiException) ex.getCause()); + } throw new DatastoreException(UNKNOWN_CODE, ex.getMessage(), null, ex.getCause()); } diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreExceptionTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreExceptionTest.java index 10039f86e..84dfdb240 100644 --- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreExceptionTest.java +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreExceptionTest.java @@ -27,6 +27,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -120,34 +121,46 @@ public void testApiException() { @Test public void testTranslateAndThrow() { Exception cause = new DatastoreException(14, "message", "UNAVAILABLE"); - RetryHelper.RetryHelperException exceptionMock = + final RetryHelper.RetryHelperException exceptionMock = createMock(RetryHelper.RetryHelperException.class); expect(exceptionMock.getCause()).andReturn(cause).times(2); replay(exceptionMock); - try { - DatastoreException.translateAndThrow(exceptionMock); - } catch (BaseServiceException ex) { - assertEquals(14, ex.getCode()); - assertEquals("message", ex.getMessage()); - assertTrue(ex.isRetryable()); - } finally { - verify(exceptionMock); - } + BaseServiceException ex = + assertThrows( + BaseServiceException.class, () -> DatastoreException.translateAndThrow(exceptionMock)); + assertEquals(14, ex.getCode()); + assertEquals("message", ex.getMessage()); + assertTrue(ex.isRetryable()); + verify(exceptionMock); + + cause = createApiException(); + final RetryHelper.RetryHelperException exceptionMock2 = + createMock(RetryHelper.RetryHelperException.class); + expect(exceptionMock2.getCause()).andReturn(cause).times(3); + replay(exceptionMock2); + DatastoreException ex2 = + assertThrows( + DatastoreException.class, () -> DatastoreException.translateAndThrow(exceptionMock2)); + assertThat(ex2.getReason()).isEqualTo("FAILED_PRECONDITION"); + assertThat(ex2.getDomain()).isEqualTo("datastore.googleapis.com"); + assertThat(ex2.getMetadata()).isEqualTo(singletonMap("missing_indexes_url", "__some__url__")); + assertThat(ex2.getErrorDetails()).isEqualTo(((ApiException) cause).getErrorDetails()); + verify(exceptionMock2); + cause = new IllegalArgumentException("message"); - exceptionMock = createMock(RetryHelper.RetryHelperException.class); - expect(exceptionMock.getMessage()).andReturn("message").times(1); - expect(exceptionMock.getCause()).andReturn(cause).times(2); - replay(exceptionMock); - try { - DatastoreException.translateAndThrow(exceptionMock); - } catch (BaseServiceException ex) { - assertEquals(DatastoreException.UNKNOWN_CODE, ex.getCode()); - assertEquals("message", ex.getMessage()); - assertFalse(ex.isRetryable()); - assertSame(cause, ex.getCause()); - } finally { - verify(exceptionMock); - } + final RetryHelper.RetryHelperException exceptionMock3 = + createMock(RetryHelper.RetryHelperException.class); + expect(exceptionMock3.getMessage()).andReturn("message").times(1); + expect(exceptionMock3.getCause()).andReturn(cause).times(3); + replay(exceptionMock3); + BaseServiceException ex3 = + assertThrows( + BaseServiceException.class, () -> DatastoreException.translateAndThrow(exceptionMock3)); + assertEquals(DatastoreException.UNKNOWN_CODE, ex3.getCode()); + assertEquals("message", ex3.getMessage()); + assertFalse(ex3.isRetryable()); + assertSame(cause, ex3.getCause()); + verify(exceptionMock3); } @Test From a5140dbd076cba6e23d77d205cf3b26d000ba475 Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Sat, 25 Nov 2023 18:09:35 +0000 Subject: [PATCH 27/32] adding clirr configuration --- google-cloud-datastore/clirr-ignored-differences.xml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/google-cloud-datastore/clirr-ignored-differences.xml b/google-cloud-datastore/clirr-ignored-differences.xml index 018afb17e..ec60567ec 100644 --- a/google-cloud-datastore/clirr-ignored-differences.xml +++ b/google-cloud-datastore/clirr-ignored-differences.xml @@ -26,4 +26,14 @@ com.google.cloud.datastore.AggregationResults runAggregation(com.google.cloud.datastore.AggregationQuery) 7012 + + com/google/cloud/datastore/DatastoreException + 5000 + com/google/cloud/grpc/BaseGrpcServiceException + + + com/google/cloud/datastore/DatastoreException + 5001 + com/google/cloud/http/BaseHttpServiceException + From c01276749958bda3b67f7f07fc6f177323b8efc5 Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Mon, 27 Nov 2023 16:34:10 +0000 Subject: [PATCH 28/32] javadoc for getReason --- .../cloud/datastore/DatastoreException.java | 13 ++++++++++++- .../cloud/datastore/DatastoreExceptionTest.java | 15 +++++++-------- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java index 0e7523187..5d01c3b9d 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java @@ -119,9 +119,20 @@ public ErrorDetails getErrorDetails() { return null; } + /** + * Checks the underlying reason of the exception and if it's {@link ApiException} then return the + * reason otherwise null/custom reason. + * + * @see Reason + * @return the reason of an error. + */ @Override public String getReason() { - return this.reason != null ? this.reason : super.getReason(); + if (this.apiException != null) { + return this.apiException.getReason(); + } + return this.reason; } /** diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreExceptionTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreExceptionTest.java index 84dfdb240..33f5ebb9c 100644 --- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreExceptionTest.java +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreExceptionTest.java @@ -96,13 +96,12 @@ public void testDatastoreException() { assertFalse(exception.isRetryable()); assertSame(cause, exception.getCause()); - ApiException apiException = - new ApiException("message", cause, GrpcStatusCode.of(Status.Code.NOT_FOUND), true); + ApiException apiException = createApiException(); exception = new DatastoreException(apiException); - assertEquals(404, exception.getCode()); - assertEquals("NOT_FOUND", exception.getReason()); - assertEquals("message", exception.getMessage()); - assertTrue(exception.isRetryable()); + assertEquals(400, exception.getCode()); + assertEquals("MISSING_INDEXES", exception.getReason()); + assertThat(exception.getMetadata()) + .isEqualTo(singletonMap("missing_indexes_url", "__some__url__")); assertSame(apiException, exception.getCause()); } @@ -111,7 +110,7 @@ public void testApiException() { ApiException apiException = createApiException(); DatastoreException datastoreException = new DatastoreException(apiException); - assertThat(datastoreException.getReason()).isEqualTo("FAILED_PRECONDITION"); + assertThat(datastoreException.getReason()).isEqualTo("MISSING_INDEXES"); assertThat(datastoreException.getDomain()).isEqualTo("datastore.googleapis.com"); assertThat(datastoreException.getMetadata()) .isEqualTo(singletonMap("missing_indexes_url", "__some__url__")); @@ -141,7 +140,7 @@ public void testTranslateAndThrow() { DatastoreException ex2 = assertThrows( DatastoreException.class, () -> DatastoreException.translateAndThrow(exceptionMock2)); - assertThat(ex2.getReason()).isEqualTo("FAILED_PRECONDITION"); + assertThat(ex2.getReason()).isEqualTo("MISSING_INDEXES"); assertThat(ex2.getDomain()).isEqualTo("datastore.googleapis.com"); assertThat(ex2.getMetadata()).isEqualTo(singletonMap("missing_indexes_url", "__some__url__")); assertThat(ex2.getErrorDetails()).isEqualTo(((ApiException) cause).getErrorDetails()); From 37568b981a41cc3f2c17f6e767adc51f33aee091 Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Mon, 27 Nov 2023 17:08:13 +0000 Subject: [PATCH 29/32] deleting unused file --- .../google/cloud/datastore/EmulatorUtils.java | 45 ------------------- 1 file changed, 45 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/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); - } - } -} From 6537bcae4033bf38270d367fcdd68c9947a29034 Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Tue, 28 Nov 2023 08:49:20 +0000 Subject: [PATCH 30/32] enabling tests which were ignore for actionable error implementation --- .../java/com/google/cloud/datastore/DatastoreTest.java | 9 +++------ .../com/google/cloud/datastore/it/ITDatastoreTest.java | 7 ++++--- 2 files changed, 7 insertions(+), 9 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 b1b36569a..42352b94a 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,7 @@ package com.google.cloud.datastore; +import static com.google.api.gax.rpc.StatusCode.Code.ABORTED; import static com.google.cloud.datastore.ProtoTestData.intValue; import static com.google.cloud.datastore.TestUtils.matches; import static com.google.cloud.datastore.aggregation.Aggregation.count; @@ -238,8 +239,6 @@ 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(); @@ -257,12 +256,10 @@ public void testTransactionWithRead() { transaction.commit(); fail("Expecting a failure"); } catch (DatastoreException expected) { - assertEquals("ABORTED", expected.getReason()); + assertEquals(ABORTED.getHttpStatusCode(), expected.getCode()); } } - // TODO(gapic_upgrade): Remove the @ignore annotation - @Ignore("This should be fixed with actionable error implementation") @Test public void testTransactionWithQuery() { Query query = @@ -288,7 +285,7 @@ public void testTransactionWithQuery() { transaction.commit(); fail("Expecting a failure"); } catch (DatastoreException expected) { - assertEquals("ABORTED", expected.getReason()); + assertEquals(ABORTED.getHttpStatusCode(), expected.getCode()); } } 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 d1e7646d2..7feb0c4c8 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,7 @@ package com.google.cloud.datastore.it; +import static com.google.api.gax.rpc.StatusCode.Code.INVALID_ARGUMENT; 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; @@ -28,6 +29,8 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import com.google.api.gax.grpc.GrpcStatusCode; +import com.google.api.gax.rpc.StatusCode; import com.google.cloud.Timestamp; import com.google.cloud.Tuple; import com.google.cloud.datastore.AggregationQuery; @@ -1392,8 +1395,6 @@ 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() { @@ -1444,7 +1445,7 @@ public Integer run(DatastoreReaderWriter transaction) { datastore.runInTransaction(callable2, readOnlyOptions); fail("Expecting a failure"); } catch (DatastoreException expected) { - assertEquals(3, ((DatastoreException) expected.getCause()).getCode()); + assertEquals(INVALID_ARGUMENT.getHttpStatusCode(), ((DatastoreException) expected.getCause()).getCode()); } } From 37466bc416444ef3cadf93e1ad9f6ead08ae0e8d Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Tue, 28 Nov 2023 08:51:39 +0000 Subject: [PATCH 31/32] added clirr comments --- google-cloud-datastore/clirr-ignored-differences.xml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/google-cloud-datastore/clirr-ignored-differences.xml b/google-cloud-datastore/clirr-ignored-differences.xml index ec60567ec..62459c953 100644 --- a/google-cloud-datastore/clirr-ignored-differences.xml +++ b/google-cloud-datastore/clirr-ignored-differences.xml @@ -27,11 +27,13 @@ 7012 + com/google/cloud/datastore/DatastoreException 5000 com/google/cloud/grpc/BaseGrpcServiceException + com/google/cloud/datastore/DatastoreException 5001 com/google/cloud/http/BaseHttpServiceException From a893a14695d423000ac142bb7fe426184ccd38dc Mon Sep 17 00:00:00 2001 From: Prateek Jain Date: Tue, 28 Nov 2023 08:52:18 +0000 Subject: [PATCH 32/32] fix lint --- .../java/com/google/cloud/datastore/DatastoreTest.java | 1 - .../com/google/cloud/datastore/it/ITDatastoreTest.java | 7 +++---- 2 files changed, 3 insertions(+), 5 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 42352b94a..5f42e2aeb 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 @@ -87,7 +87,6 @@ import org.junit.Before; import org.junit.BeforeClass; import org.junit.ClassRule; -import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; 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 7feb0c4c8..1e931dfc4 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 @@ -29,8 +29,6 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import com.google.api.gax.grpc.GrpcStatusCode; -import com.google.api.gax.rpc.StatusCode; import com.google.cloud.Timestamp; import com.google.cloud.Tuple; import com.google.cloud.datastore.AggregationQuery; @@ -89,7 +87,6 @@ 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; @@ -1445,7 +1442,9 @@ public Integer run(DatastoreReaderWriter transaction) { datastore.runInTransaction(callable2, readOnlyOptions); fail("Expecting a failure"); } catch (DatastoreException expected) { - assertEquals(INVALID_ARGUMENT.getHttpStatusCode(), ((DatastoreException) expected.getCause()).getCode()); + assertEquals( + INVALID_ARGUMENT.getHttpStatusCode(), + ((DatastoreException) expected.getCause()).getCode()); } }