Skip to content

Commit

Permalink
core: Move io.grpc to grpc-api
Browse files Browse the repository at this point in the history
io.grpc has fewer dependencies than io.grpc.internal. Moving it to a
separate artifact lets users use the API without bringing in the deps.
If the library has an optional dependency on grpc, that can be quite
convenient.

We now version-pin both grpc-api and grpc-core, since both contain
internal APIs.

I had to change a few tests in grpc-api to avoid FakeClock. Moving
FakeClock to grpc-api was difficult because it uses
io.grpc.internal.TimeProvider, which can't be moved since it is a
production class. Having grpc-api's tests depend on grpc-core's test
classes would be weird and cause a circular dependincy. Having
grpc-api's tests depend on grpc-core is likely possible, but weird and
fairly unnecessary at this point. So instead I rewrote the tests to
avoid FakeClock.

Fixes grpc#1447
  • Loading branch information
ejona86 committed Apr 17, 2019
1 parent f3731ea commit 80c3c99
Show file tree
Hide file tree
Showing 157 changed files with 220 additions and 95 deletions.
4 changes: 2 additions & 2 deletions BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ java_library(
name = "java_grpc_library_deps__do_not_reference",
visibility = ["//visibility:public"],
exports = [
"//core",
"//api",
"//protobuf",
"//stub",
"//stub:javax_annotation",
Expand All @@ -43,7 +43,7 @@ java_library(
name = "java_lite_grpc_library_deps__do_not_reference",
visibility = ["//visibility:public"],
exports = [
"//core",
"//api",
"//protobuf-lite",
"//stub",
"//stub:javax_annotation",
Expand Down
1 change: 1 addition & 0 deletions all/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ buildscript {
}

def subprojects = [
project(':grpc-api'),
project(':grpc-auth'),
project(':grpc-core'),
project(':grpc-context'),
Expand Down
4 changes: 2 additions & 2 deletions alts/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ java_library(
deps = [
":handshaker_java_grpc",
":handshaker_java_proto",
"//core",
"//api",
"//core:internal",
"//netty",
"//stub",
Expand All @@ -35,8 +35,8 @@ java_library(
deps = [
":alts_internal",
":handshaker_java_grpc",
"//api",
"//auth",
"//core",
"//core:internal",
"//netty",
"@com_google_auth_google_auth_library_oauth2_http//jar",
Expand Down
14 changes: 14 additions & 0 deletions api/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
java_library(
name = "api",
srcs = glob([
"src/main/java/**/*.java",
]),
visibility = ["//visibility:public"],
deps = [
"//context",
"@com_google_code_findbugs_jsr305//jar",
"@com_google_guava_failureaccess//jar", # future transitive dep of Guava. See #5214
"@com_google_guava_guava//jar",
"@com_google_j2objc_j2objc_annotations//jar",
],
)
32 changes: 32 additions & 0 deletions api/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
description = 'gRPC: API'

dependencies {
compile project(':grpc-context'),
libraries.errorprone,
libraries.jsr305,
libraries.animalsniffer_annotations
compile (libraries.guava) {
// prefer 2.3.2 from libraries instead of 2.1.3
exclude group: 'com.google.errorprone', module: 'error_prone_annotations'
// prefer 3.0.2 from libraries instead of 3.0.1
exclude group: 'com.google.code.findbugs', module: 'jsr305'
// prefer 1.17 from libraries instead of 1.14
exclude group: 'org.codehaus.mojo', module: 'animal-sniffer-annotations'
}

testCompile project(':grpc-context').sourceSets.test.output,
project(':grpc-testing'),
project(':grpc-grpclb'),
libraries.guava_testlib

jmh project(':grpc-core')

signature "org.codehaus.mojo.signature:java17:1.0@signature"
signature "net.sf.androidscents.signature:android-api-level-14:4.0_r4@signature"
}

javadoc {
// We want io.grpc.Internal, but not io.grpc.Internal*
exclude 'io/grpc/Internal?*.java'
exclude 'io/grpc/internal/**'
}
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,13 @@
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import io.grpc.internal.FakeClock;
import com.google.common.util.concurrent.testing.TestingExecutors;
import io.grpc.internal.NoopServerCall;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import org.junit.Test;
Expand Down Expand Up @@ -213,11 +215,29 @@ public void statusFromCancelled_shouldReturnStatusWithCauseAttached() {

@Test
public void statusFromCancelled_TimeoutExceptionShouldMapToDeadlineExceeded() {
FakeClock fakeClock = new FakeClock();
final long expectedDelay = 100;
final TimeUnit expectedUnit = TimeUnit.SECONDS;
class MockScheduledExecutorService extends ForwardingScheduledExecutorService {
private ScheduledExecutorService delegate = TestingExecutors.noOpScheduledExecutor();
Runnable command;

@Override public ScheduledExecutorService delegate() {
return delegate;
}

@Override public ScheduledFuture<?> schedule(Runnable command, long delay, TimeUnit unit) {
if (delay > unit.convert(expectedDelay, expectedUnit)) {
fail("Delay larger than expected: " + delay + " " + unit);
}
this.command = command;
return super.schedule(command, delay, unit);
}
}

MockScheduledExecutorService executorService = new MockScheduledExecutorService();
Context.CancellableContext cancellableContext = Context.current()
.withDeadlineAfter(100, TimeUnit.NANOSECONDS, fakeClock.getScheduledExecutorService());
fakeClock.forwardTime(System.nanoTime(), TimeUnit.NANOSECONDS);
fakeClock.forwardNanos(100);
.withDeadlineAfter(expectedDelay, expectedUnit, executorService);
executorService.command.run();

assertTrue(cancellableContext.isCancelled());
assertThat(cancellableContext.cancellationCause(), instanceOf(TimeoutException.class));
Expand Down
54 changes: 54 additions & 0 deletions api/src/test/java/io/grpc/ForwardingScheduledExecutorService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* Copyright 2019 The gRPC Authors
*
* 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 io.grpc;

import com.google.common.util.concurrent.ForwardingExecutorService;
import java.util.concurrent.Callable;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;

/**
* Forwards all methods to delegate.
*/
abstract class ForwardingScheduledExecutorService extends ForwardingExecutorService
implements ScheduledExecutorService {
@Override
protected abstract ScheduledExecutorService delegate();

@Override
public <V> ScheduledFuture<V> schedule(Callable<V> callable, long delay, TimeUnit unit) {
return delegate().schedule(callable, delay, unit);
}

@Override
public ScheduledFuture<?> schedule(Runnable command, long delay, TimeUnit unit) {
return delegate().schedule(command, delay, unit);
}

@Override
public ScheduledFuture<?> scheduleAtFixedRate(
Runnable command, long initialDelay, long period, TimeUnit unit) {
return delegate().scheduleAtFixedRate(command, initialDelay, period, unit);
}

@Override
public ScheduledFuture<?> scheduleWithFixedDelay(
Runnable command, long initialDelay, long delay, TimeUnit unit) {
return delegate().scheduleWithFixedDelay(command, initialDelay, delay, unit);
}
}
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,13 @@
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;

import com.google.common.util.concurrent.testing.TestingExecutors;
import io.grpc.SynchronizationContext.ScheduledHandle;
import io.grpc.internal.FakeClock;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
Expand Down Expand Up @@ -231,54 +233,54 @@ public Void answer(InvocationOnMock invocation) {

@Test
public void schedule() {
FakeClock clock = new FakeClock();
MockScheduledExecutorService executorService = new MockScheduledExecutorService();
ScheduledHandle handle =
syncContext.schedule(task1, 110, TimeUnit.NANOSECONDS, clock.getScheduledExecutorService());
assertThat(handle.isPending()).isTrue();
assertThat(clock.runDueTasks()).isEqualTo(0);
assertThat(clock.forwardNanos(109)).isEqualTo(0);
syncContext.schedule(task1, 110, TimeUnit.NANOSECONDS, executorService);
assertThat(executorService.delay)
.isEqualTo(executorService.unit.convert(110, TimeUnit.NANOSECONDS));
assertThat(handle.isPending()).isTrue();
verify(task1, never()).run();

assertThat(clock.forwardNanos(1)).isEqualTo(1);
executorService.command.run();
assertThat(handle.isPending()).isFalse();
verify(task1).run();
}

@Test
public void scheduleDueImmediately() {
FakeClock clock = new FakeClock();
ScheduledHandle handle =
syncContext.schedule(task1, -1, TimeUnit.NANOSECONDS, clock.getScheduledExecutorService());
MockScheduledExecutorService executorService = new MockScheduledExecutorService();
ScheduledHandle handle = syncContext.schedule(task1, -1, TimeUnit.NANOSECONDS, executorService);
assertThat(executorService.delay)
.isEqualTo(executorService.unit.convert(-1, TimeUnit.NANOSECONDS));
verify(task1, never()).run();
assertThat(handle.isPending()).isTrue();

assertThat(clock.runDueTasks()).isEqualTo(1);
executorService.command.run();
assertThat(handle.isPending()).isFalse();
verify(task1).run();
}

@Test
public void scheduleHandle_cancel() {
FakeClock clock = new FakeClock();
MockScheduledExecutorService executorService = new MockScheduledExecutorService();
ScheduledHandle handle =
syncContext.schedule(task1, 110, TimeUnit.NANOSECONDS, clock.getScheduledExecutorService());
assertThat(handle.isPending()).isTrue();
assertThat(clock.runDueTasks()).isEqualTo(0);
syncContext.schedule(task1, 110, TimeUnit.NANOSECONDS, executorService);
assertThat(handle.isPending()).isTrue();
assertThat(executorService.delay)
.isEqualTo(executorService.unit.convert(110, TimeUnit.NANOSECONDS));

handle.cancel();
assertThat(handle.isPending()).isFalse();
syncContext.drain();
assertThat(clock.numPendingTasks()).isEqualTo(0);
assertThat(executorService.future.isCancelled()).isTrue();
verify(task1, never()).run();
}

// Test that a scheduled task is cancelled after the timer has expired on the
// ScheduledExecutorService, but before the task is run.
@Test
public void scheduledHandle_cancelRacesWithTimerExpiration() throws Exception {
FakeClock clock = new FakeClock();
MockScheduledExecutorService executorService = new MockScheduledExecutorService();

final CountDownLatch task1Running = new CountDownLatch(1);
final LinkedBlockingQueue<ScheduledHandle> task2HandleQueue = new LinkedBlockingQueue<>();
Expand Down Expand Up @@ -309,17 +311,17 @@ public void run() {
}
};

ScheduledHandle handle =
syncContext.schedule(task2, 10, TimeUnit.NANOSECONDS, clock.getScheduledExecutorService());
ScheduledHandle handle = syncContext.schedule(task2, 10, TimeUnit.NANOSECONDS, executorService);
// This will execute and block in task1
sideThread.start();

// Make sure task1 is running and blocking the execution
assertThat(task1Running.await(5, TimeUnit.SECONDS)).isTrue();

// Timer expires. task2 will be enqueued, but blocked by task1
assertThat(clock.forwardNanos(10)).isEqualTo(1);
assertThat(clock.numPendingTasks()).isEqualTo(0);
assertThat(executorService.delay)
.isEqualTo(executorService.unit.convert(10, TimeUnit.NANOSECONDS));
executorService.command.run();
assertThat(handle.isPending()).isTrue();

// Enqueue task3 following task2
Expand All @@ -335,6 +337,25 @@ public void run() {
assertThat(handle.isPending()).isFalse();
verify(task2, never()).run();
verify(task3).run();
assertThat(clock.numPendingTasks()).isEqualTo(0);
}

static class MockScheduledExecutorService extends ForwardingScheduledExecutorService {
private ScheduledExecutorService delegate = TestingExecutors.noOpScheduledExecutor();

Runnable command;
long delay;
TimeUnit unit;
ScheduledFuture<?> future;

@Override public ScheduledExecutorService delegate() {
return delegate;
}

@Override public ScheduledFuture<?> schedule(Runnable command, long delay, TimeUnit unit) {
this.command = command;
this.delay = delay;
this.unit = unit;
return future = super.schedule(command, delay, unit);
}
}
}
2 changes: 1 addition & 1 deletion auth/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ java_library(
]),
visibility = ["//visibility:public"],
deps = [
"//core",
"//api",
"@com_google_auth_google_auth_library_credentials//jar",
"@com_google_code_findbugs_jsr305//jar",
"@com_google_guava_guava//jar",
Expand Down
2 changes: 1 addition & 1 deletion auth/build.gradle
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
description = "gRPC: Auth"
dependencies {
compile project(':grpc-core'),
compile project(':grpc-api'),
libraries.google_auth_credentials
testCompile project(':grpc-testing'),
libraries.google_auth_oauth2_http
Expand Down
3 changes: 2 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ subprojects {
"grpc-protobuf-nano"
])) {
asNode().dependencies.'*'.findAll() { dep ->
dep.artifactId.text() == 'grpc-core'
dep.artifactId.text() in ['grpc-api', 'grpc-core']
}.each() { core ->
core.version*.value = "[" + core.version.text() + "]"
}
Expand Down Expand Up @@ -452,6 +452,7 @@ def baselineGrpcVersion = '1.6.1'
def publicApiSubprojects = [
// TODO: uncomment after grpc-alts, grpc-bom artifact is published.
// ':grpc-alts',
//':grpc-api',
':grpc-auth',
//':grpc-bom',
':grpc-context',
Expand Down
Loading

0 comments on commit 80c3c99

Please sign in to comment.