Skip to content
This repository has been archived by the owner on Sep 26, 2023. It is now read-only.

Commit

Permalink
fix: Make watchdog @BetaApi and watchdog provider implementations `…
Browse files Browse the repository at this point in the history
…@InternalApi` (#1119)

This is to address #829.

Also add grpclb lib to bazel dependencies, as its absence was failing some of the tests well executed from bazel.

This  PR basically repeats #881, with few more things in it (like making watchog final, as requested in the review there)
  • Loading branch information
vam-google committed Jun 5, 2020
1 parent bbd3960 commit 593924b
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 8 deletions.
1 change: 1 addition & 0 deletions gax-grpc/BUILD.bazel
Expand Up @@ -24,6 +24,7 @@ _COMPILE_DEPS = [
"@com_google_http_client_google_http_client//jar",
"@io_grpc_grpc_java//context:context",
"@io_grpc_grpc_netty_shaded//jar",
"@io_grpc_grpc_grpclb//jar",
"@io_grpc_grpc_java//alts:alts",
"@io_netty_netty_tcnative_boringssl_static//jar",
"@javax_annotation_javax_annotation_api//jar",
Expand Down
Expand Up @@ -31,13 +31,22 @@

import com.google.api.core.ApiClock;
import com.google.api.core.BetaApi;
import com.google.api.core.InternalApi;
import java.util.concurrent.ScheduledExecutorService;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.threeten.bp.Duration;

/**
* A watchdog provider which always returns the same watchdog instance provided to the provider
* during construction.
*
* <p>This is the internal class and is public only for technical reasons. It may change any time
* without notice, please do not depend on it explicitly.
*/
@BetaApi("The surface for streaming is not stable yet and may change in the future.")
public class FixedWatchdogProvider implements WatchdogProvider {
@InternalApi
public final class FixedWatchdogProvider implements WatchdogProvider {
@Nullable private final Watchdog watchdog;

public static WatchdogProvider create(Watchdog watchdog) {
Expand Down
Expand Up @@ -31,13 +31,21 @@

import com.google.api.core.ApiClock;
import com.google.api.core.BetaApi;
import com.google.api.core.InternalApi;
import com.google.common.base.Preconditions;
import java.util.concurrent.ScheduledExecutorService;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.threeten.bp.Duration;

/**
* A watchdog provider which instantiates a new provider on every request.
*
* <p>This is the internal class and is public only for technical reasons. It may change any time
* without notice, please do not depend on it explicitly.
*/
@BetaApi("The surface for streaming is not stable yet and may change in the future.")
@InternalApi
public final class InstantiatingWatchdogProvider implements WatchdogProvider {
@Nullable private final ApiClock clock;
@Nullable private final ScheduledExecutorService executor;
Expand Down
6 changes: 3 additions & 3 deletions gax/src/main/java/com/google/api/gax/rpc/Watchdog.java
Expand Up @@ -30,7 +30,7 @@
package com.google.api.gax.rpc;

import com.google.api.core.ApiClock;
import com.google.api.core.InternalApi;
import com.google.api.core.BetaApi;
import com.google.api.gax.core.BackgroundResource;
import com.google.common.base.Preconditions;
import java.util.Iterator;
Expand Down Expand Up @@ -59,8 +59,8 @@
* had no outstanding demand. Duration.ZERO disables the timeout.
* </ul>
*/
@InternalApi
public class Watchdog implements Runnable, BackgroundResource {
@BetaApi
public final class Watchdog implements Runnable, BackgroundResource {
// Dummy value to convert the ConcurrentHashMap into a Set
private static Object PRESENT = new Object();
private final ConcurrentHashMap<WatchdogStream, Object> openStreams = new ConcurrentHashMap<>();
Expand Down
Expand Up @@ -230,7 +230,11 @@ private void runTest(
null);
Credentials credentials = Mockito.mock(Credentials.class);
ApiClock clock = Mockito.mock(ApiClock.class);
Watchdog watchdog = Mockito.mock(Watchdog.class);
Watchdog watchdog =
Watchdog.create(
Mockito.mock(ApiClock.class),
Duration.ZERO,
Mockito.mock(ScheduledExecutorService.class));
Duration watchdogCheckInterval = Duration.ofSeconds(11);

builder.setExecutorProvider(executorProvider);
Expand Down
Expand Up @@ -142,7 +142,11 @@ public void testBuilderFromClientContext() throws Exception {
ApiClock clock = Mockito.mock(ApiClock.class);
ApiCallContext callContext = FakeCallContext.createDefault();
Map<String, String> headers = Collections.singletonMap("spiffykey", "spiffyvalue");
Watchdog watchdog = Mockito.mock(Watchdog.class);
Watchdog watchdog =
Watchdog.create(
Mockito.mock(ApiClock.class),
Duration.ZERO,
Mockito.mock(ScheduledExecutorService.class));
Duration watchdogCheckInterval = Duration.ofSeconds(12);

ClientContext clientContext =
Expand Down
Expand Up @@ -49,14 +49,24 @@ public void testNull() {

@Test
public void testSameInstance() {
Watchdog watchdog = Mockito.mock(Watchdog.class);
Watchdog watchdog =
Watchdog.create(
Mockito.mock(ApiClock.class),
Duration.ZERO,
Mockito.mock(ScheduledExecutorService.class));

WatchdogProvider provider = FixedWatchdogProvider.create(watchdog);
assertThat(provider.getWatchdog()).isSameInstanceAs(watchdog);
}

@Test
public void testNoModifications() {
WatchdogProvider provider = FixedWatchdogProvider.create(Mockito.mock(Watchdog.class));
Watchdog watchdog =
Watchdog.create(
Mockito.mock(ApiClock.class),
Duration.ZERO,
Mockito.mock(ScheduledExecutorService.class));
WatchdogProvider provider = FixedWatchdogProvider.create(watchdog);

assertThat(provider.needsCheckInterval()).isFalse();
assertThat(provider.needsClock()).isFalse();
Expand Down
8 changes: 8 additions & 0 deletions repositories.bzl
Expand Up @@ -79,6 +79,14 @@ def com_google_api_gax_java_repositories():
licenses = ["notice", "reciprocal"],
)

_maybe(
jvm_maven_import_external,
name = "io_grpc_grpc_grpclb",
artifact = "io.grpc:grpc-grpclb:%s" % PROPERTIES["version.io_grpc"],
server_urls = ["https://repo.maven.apache.org/maven2/", "http://repo1.maven.org/maven2/"],
licenses = ["notice", "reciprocal"],
)

_maybe(
jvm_maven_import_external,
name = "google_java_format_all_deps",
Expand Down

0 comments on commit 593924b

Please sign in to comment.