From e68e4004e430cab5d190b0143f8fb7ae4b85b42c Mon Sep 17 00:00:00 2001 From: Chengyuan Zhang Date: Fri, 16 Aug 2019 01:05:32 -0700 Subject: [PATCH] cronet: move call options APIs into an internal accessor class (#6073) * cronet: move call options APIs into an internal accessor class * Removed deprecated call option key. --- .../io/grpc/cronet/CronetCallOptions.java | 19 ++-------- .../io/grpc/cronet/CronetClientStream.java | 35 ++++++++++++++++-- .../cronet/InternalCronetCallOptions.java | 36 +++++++++++++++++++ .../grpc/cronet/CronetClientStreamTest.java | 2 +- 4 files changed, 72 insertions(+), 20 deletions(-) create mode 100644 cronet/src/main/java/io/grpc/cronet/InternalCronetCallOptions.java diff --git a/cronet/src/main/java/io/grpc/cronet/CronetCallOptions.java b/cronet/src/main/java/io/grpc/cronet/CronetCallOptions.java index d361d0b4d71..42cdb72bbdb 100644 --- a/cronet/src/main/java/io/grpc/cronet/CronetCallOptions.java +++ b/cronet/src/main/java/io/grpc/cronet/CronetCallOptions.java @@ -17,9 +17,6 @@ package io.grpc.cronet; import io.grpc.CallOptions; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; /** Call options for use with the Cronet transport. */ public final class CronetCallOptions { @@ -36,7 +33,7 @@ private CronetCallOptions() {} */ @Deprecated public static final CallOptions.Key CRONET_ANNOTATION_KEY = - CallOptions.Key.create("cronet-annotation"); + CronetClientStream.CRONET_ANNOTATION_KEY; /** * Returns a copy of {@code callOptions} with {@code annotation} included as one of the Cronet @@ -48,18 +45,6 @@ private CronetCallOptions() {} * @param annotation the object to attach to the Cronet stream */ public static CallOptions withAnnotation(CallOptions callOptions, Object annotation) { - Collection existingAnnotations = callOptions.getOption(CRONET_ANNOTATIONS_KEY); - ArrayList newAnnotations; - if (existingAnnotations == null) { - newAnnotations = new ArrayList<>(); - } else { - newAnnotations = new ArrayList<>(existingAnnotations); - } - newAnnotations.add(annotation); - return callOptions.withOption( - CronetCallOptions.CRONET_ANNOTATIONS_KEY, Collections.unmodifiableList(newAnnotations)); + return CronetClientStream.withAnnotation(callOptions, annotation); } - - static final CallOptions.Key> CRONET_ANNOTATIONS_KEY = - CallOptions.Key.create("cronet-annotations"); } diff --git a/cronet/src/main/java/io/grpc/cronet/CronetClientStream.java b/cronet/src/main/java/io/grpc/cronet/CronetClientStream.java index 4edee96d0f1..72bd32810a3 100644 --- a/cronet/src/main/java/io/grpc/cronet/CronetClientStream.java +++ b/cronet/src/main/java/io/grpc/cronet/CronetClientStream.java @@ -44,6 +44,7 @@ import java.nio.charset.Charset; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -63,6 +64,14 @@ class CronetClientStream extends AbstractClientStream { private static final int READ_BUFFER_CAPACITY = 4 * 1024; private static final ByteBuffer EMPTY_BUFFER = ByteBuffer.allocateDirect(0); private static final String LOG_TAG = "grpc-java-cronet"; + + @Deprecated + static final CallOptions.Key CRONET_ANNOTATION_KEY = + CallOptions.Key.create("cronet-annotation"); + + static final CallOptions.Key> CRONET_ANNOTATIONS_KEY = + CallOptions.Key.create("cronet-annotations"); + private final String url; private final String userAgent; private final StatsTraceContext statsTraceCtx; @@ -107,8 +116,8 @@ class CronetClientStream extends AbstractClientStream { this.idempotent = method.isIdempotent() || alwaysUsePut; // Only delay flushing header for unary rpcs. this.delayRequestHeader = (method.getType() == MethodDescriptor.MethodType.UNARY); - this.annotation = callOptions.getOption(CronetCallOptions.CRONET_ANNOTATION_KEY); - this.annotations = callOptions.getOption(CronetCallOptions.CRONET_ANNOTATIONS_KEY); + this.annotation = callOptions.getOption(CRONET_ANNOTATION_KEY); + this.annotations = callOptions.getOption(CRONET_ANNOTATIONS_KEY); this.state = new TransportState(maxMessageSize, statsTraceCtx, lock, transportTracer); } @@ -127,6 +136,28 @@ public void setAuthority(String authority) { throw new UnsupportedOperationException("Cronet does not support overriding authority"); } + /** + * Returns a copy of {@code callOptions} with {@code annotation} included as one of the Cronet + * annotation objects. When an RPC is made using a {@link CallOptions} instance returned by this + * method, the annotation objects will be attached to the underlying Cronet bidirectional stream. + * When the stream finishes, the user can retrieve the annotation objects via {@link + * org.chromium.net.RequestFinishedInfo.Listener}. + * + * @param annotation the object to attach to the Cronet stream + */ + static CallOptions withAnnotation(CallOptions callOptions, Object annotation) { + Collection existingAnnotations = callOptions.getOption(CRONET_ANNOTATIONS_KEY); + ArrayList newAnnotations; + if (existingAnnotations == null) { + newAnnotations = new ArrayList<>(); + } else { + newAnnotations = new ArrayList<>(existingAnnotations); + } + newAnnotations.add(annotation); + return callOptions.withOption( + CRONET_ANNOTATIONS_KEY, Collections.unmodifiableList(newAnnotations)); + } + class Sink implements AbstractClientStream.Sink { @Override public void writeHeaders(Metadata metadata, byte[] payload) { diff --git a/cronet/src/main/java/io/grpc/cronet/InternalCronetCallOptions.java b/cronet/src/main/java/io/grpc/cronet/InternalCronetCallOptions.java new file mode 100644 index 00000000000..321354f1a92 --- /dev/null +++ b/cronet/src/main/java/io/grpc/cronet/InternalCronetCallOptions.java @@ -0,0 +1,36 @@ +/* + * 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.cronet; + +import io.grpc.CallOptions; +import io.grpc.Internal; + +/** + * Internal accessor class for call options using with the Cronet transport. This is intended for + * usage internal to the gRPC team. If you *really* think you need to use this, contact the gRPC + * team first. + */ +@Internal +public final class InternalCronetCallOptions { + + // Prevent instantiation + private InternalCronetCallOptions() {} + + public static CallOptions withAnnotation(CallOptions callOptions, Object annotation) { + return CronetClientStream.withAnnotation(callOptions, annotation); + } +} diff --git a/cronet/src/test/java/io/grpc/cronet/CronetClientStreamTest.java b/cronet/src/test/java/io/grpc/cronet/CronetClientStreamTest.java index a51fb3c8ffe..e5ed00a1667 100644 --- a/cronet/src/test/java/io/grpc/cronet/CronetClientStreamTest.java +++ b/cronet/src/test/java/io/grpc/cronet/CronetClientStreamTest.java @@ -587,7 +587,7 @@ public void addCronetRequestAnnotation_deprecated() { false /* alwaysUsePut */, method, StatsTraceContext.NOOP, - CallOptions.DEFAULT.withOption(CronetCallOptions.CRONET_ANNOTATION_KEY, annotation), + CallOptions.DEFAULT.withOption(CronetClientStream.CRONET_ANNOTATION_KEY, annotation), transportTracer); callback.setStream(stream); when(factory.newBidirectionalStreamBuilder(