Skip to content

refactor: Swap usage of HttpDatastoreRpc with GrpcDatastoreRpc #1240

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
e17ba34
Create basic structure of GrpcDatastoreRpc and using it in DatastoreO…
jainsahab Nov 4, 2023
f9e455e
applying unary settings to all the unary methods
jainsahab Nov 6, 2023
231deef
Configuring header provider for GrpcDatastoreRpc
jainsahab Nov 6, 2023
806f997
fixing emulator tests to be able to run successfully with grpc now
jainsahab Nov 9, 2023
0c8704c
ignoring one more test which will be fixed in actionable error implem…
jainsahab Nov 9, 2023
d6ed231
Making HttpDatastoreRpc completely unused
jainsahab Nov 9, 2023
e257a91
Making GrpcDatastoreRpc implement AutoCloseable
jainsahab Nov 16, 2023
2984146
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Nov 16, 2023
acfbaef
incorporating feedbacks
jainsahab Nov 17, 2023
60ee45d
pinging emulator after each test for debugging
jainsahab Nov 18, 2023
d42e3b9
Revert "pinging emulator after each test for debugging"
jainsahab Nov 18, 2023
1d9d7e6
Reapply "pinging emulator after each test for debugging"
jainsahab Nov 18, 2023
d7b652e
more debugging
jainsahab Nov 18, 2023
9827fb9
Constant ping to avoid flaky behaviour of /shutdown endpoint
jainsahab Nov 18, 2023
38725f0
fixing test
jainsahab Nov 18, 2023
b15a9a9
checking if emulator is running before sending a shutdown command
jainsahab Nov 20, 2023
ec38885
fix lint
jainsahab Nov 20, 2023
7bd2c55
implement helper method for localhost
jainsahab Nov 22, 2023
0517cb6
fix header lint
jainsahab Nov 22, 2023
7f4ce8d
moving emulator health check to src/test
jainsahab Nov 22, 2023
ef5f002
fix lint
jainsahab Nov 22, 2023
9b43798
adding no extra headers
jainsahab Nov 22, 2023
cb80dd1
minor cleanup
jainsahab Nov 23, 2023
c86a702
using mutlipleAttemptsRule in DatastoreTest
jainsahab Nov 27, 2023
1351c97
Revert "adding no extra headers"
jainsahab Nov 27, 2023
e5b2565
using classRule
jainsahab Nov 27, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions google-cloud-datastore/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@
<groupId>com.google.cloud</groupId>
<artifactId>google-cloud-core-http</artifactId>
</dependency>
<dependency>
<groupId>com.google.cloud</groupId>
<artifactId>google-cloud-core-grpc</artifactId>
</dependency>
<dependency>
<groupId>com.google.api.grpc</groupId>
<artifactId>proto-google-cloud-datastore-v1</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@
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;
Expand Down Expand Up @@ -60,7 +62,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);
}
}
}

Expand Down Expand Up @@ -116,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 : com.google.datastore.v1.client.DatastoreFactory.DEFAULT_HOST;
return host != null ? host : DatastoreSettings.getDefaultEndpoint();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* 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;
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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
* <p>TraceUtil instances are created by the {@link TraceUtil#getInstance()} method.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
/*
* 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 com.google.cloud.datastore.DatastoreUtils.removeScheme;
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.grpc.GrpcCallContext;
import com.google.api.gax.grpc.GrpcTransportChannel;
import com.google.api.gax.rpc.ClientContext;
import com.google.api.gax.rpc.TransportChannel;
import com.google.api.gax.rpc.UnaryCallSettings;
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;
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 io.grpc.CallOptions;
import io.grpc.ManagedChannel;
import io.grpc.ManagedChannelBuilder;
import java.io.IOException;
import java.util.Collections;

@InternalApi
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 =
isEmulator(datastoreOptions)
? getClientContextForEmulator(datastoreOptions)
: getClientContext(datastoreOptions);
ApiFunction<UnaryCallSettings.Builder<?, ?>, 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);
}
}

@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);
}

@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);
}

private boolean isEmulator(DatastoreOptions datastoreOptions) {
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(removeScheme(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 {
DatastoreSettings.Builder settingsBuilder = DatastoreSettings.newBuilder();
settingsBuilder.setCredentialsProvider(
GrpcTransportOptions.setUpCredentialsProvider(datastoreOptions));
settingsBuilder.setTransportChannelProvider(
GrpcTransportOptions.setUpChannelProvider(
DatastoreSettings.defaultGrpcTransportProviderBuilder(), datastoreOptions));
return ClientContext.create(settingsBuilder.build());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
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;
Expand Down Expand Up @@ -191,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());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apologies, I know we had some discussion around this but on second (third? :)) thought, why not just add a MultipleAttemptsRule to DatastoreTest so the test will retry on flakes? example: https://github.com/googleapis/java-datastore/blob/main/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreTest.java#L143

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice idea. let me try it out quickly.

helper.stop(Duration.ofMinutes(1));
}

Expand Down Expand Up @@ -231,6 +234,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();
Expand All @@ -252,6 +257,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<Entity> query =
Expand Down Expand Up @@ -648,6 +655,7 @@ private List<RunQueryResponse> buildResponsesForQueryPagination() {
List<RunQueryResponse> responses = new ArrayList<>();
RecordQuery<Key> query = Query.newKeyQueryBuilder().build();
RunQueryRequest.Builder requestPb = RunQueryRequest.newBuilder();
requestPb.setProjectId(PROJECT_ID);
query.populatePb(requestPb);
QueryResultBatch queryResultBatchPb =
RunQueryResponse.newBuilder()
Expand Down Expand Up @@ -757,6 +765,7 @@ private List<RunQueryResponse> buildResponsesForQueryPaginationWithLimit() {
List<RunQueryResponse> responses = new ArrayList<>();
RecordQuery<Entity> query = Query.newEntityQueryBuilder().build();
RunQueryRequest.Builder requestPb = RunQueryRequest.newBuilder();
requestPb.setProjectId(PROJECT_ID);
query.populatePb(requestPb);
QueryResultBatch queryResultBatchPb =
RunQueryResponse.newBuilder()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* 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;
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");
}
}
Loading