From daee733745583f8d34779e9e947c275da08ea547 Mon Sep 17 00:00:00 2001 From: pandaapo <1052156701@qq.com> Date: Sat, 12 Nov 2022 22:05:42 +0800 Subject: [PATCH 1/9] Fix issue: CallOptions is not thread-safe. --- api/src/main/java/io/grpc/CallOptions.java | 204 ++++++++++++++------- 1 file changed, 137 insertions(+), 67 deletions(-) diff --git a/api/src/main/java/io/grpc/CallOptions.java b/api/src/main/java/io/grpc/CallOptions.java index 5c05d5b7bd7..013903670af 100644 --- a/api/src/main/java/io/grpc/CallOptions.java +++ b/api/src/main/java/io/grpc/CallOptions.java @@ -35,48 +35,123 @@ * *

A field that is not set is {@code null}. */ -@Immutable @CheckReturnValue public final class CallOptions { /** * A blank {@code CallOptions} that all fields are not set. */ - public static final CallOptions DEFAULT = new CallOptions(); + public static final CallOptions DEFAULT = new Builder().build(); - // Although {@code CallOptions} is immutable, its fields are not final, so that we can initialize - // them outside of constructor. Otherwise the constructor will have a potentially long list of - // unnamed arguments, which is undesirable. @Nullable - private Deadline deadline; - + private final Deadline deadline; + @Nullable - private Executor executor; + private final Executor executor; @Nullable - private String authority; + private final String authority; @Nullable - private CallCredentials credentials; + private final CallCredentials credentials; @Nullable - private String compressorName; + private final String compressorName; - private Object[][] customOptions; + private final Object[][] customOptions; - // Unmodifiable list - private List streamTracerFactories = Collections.emptyList(); + private final List streamTracerFactories; /** * Opposite to fail fast. */ @Nullable - private Boolean waitForReady; + private final Boolean waitForReady; @Nullable - private Integer maxInboundMessageSize; + private final Integer maxInboundMessageSize; @Nullable - private Integer maxOutboundMessageSize; + private final Integer maxOutboundMessageSize; + + private CallOptions(Builder builder) { + this.deadline = builder.deadline; + this.executor = builder.executor; + this.authority = builder.authority; + this.credentials = builder.credentials; + this.compressorName = builder.compressorName; + this.customOptions = builder.customOptions; + this.streamTracerFactories = builder.streamTracerFactories; + this.waitForReady = builder.waitForReady; + this.maxInboundMessageSize = builder.maxInboundMessageSize; + this.maxOutboundMessageSize = builder.maxOutboundMessageSize; + } + + private static class Builder { + private Deadline deadline; + private Executor executor; + private String authority; + private CallCredentials credentials; + private String compressorName; + private Object[][] customOptions = new Object[0][2]; + // Unmodifiable list + private List streamTracerFactories = Collections.emptyList(); + private Boolean waitForReady; + private Integer maxInboundMessageSize; + private Integer maxOutboundMessageSize; + + private Builder deadline(Deadline deadline) { + this.deadline = deadline; + return this; + } + + private Builder executor(Executor executor) { + this.executor = executor; + return this; + } + + private Builder authority(String authority) { + this.authority = authority; + return this; + } + + private Builder credentials(CallCredentials credentials) { + this.credentials = credentials; + return this; + } + + private Builder compressorName(String compressorName) { + this.compressorName = compressorName; + return this; + } + + private Builder customOptions(Object[][] customOptions) { + this.customOptions = customOptions; + return this; + } + + private Builder waitForReady(Boolean waitForReady) { + this.waitForReady = waitForReady; + return this; + } + private Builder maxInboundMessageSize(Integer maxInboundMessageSize) { + this.maxInboundMessageSize = maxInboundMessageSize; + return this; + } + + private Builder maxOutboundMessageSize(Integer maxOutboundMessageSize) { + this.maxOutboundMessageSize = maxOutboundMessageSize; + return this; + } + + private Builder streamTracerFactories(List streamTracerFactories) { + this.streamTracerFactories = streamTracerFactories; + return this; + } + + private CallOptions build() { + return new CallOptions(this); + } + } /** * Override the HTTP/2 authority the channel claims to be connecting to. This is not @@ -89,8 +164,7 @@ public final class CallOptions { */ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/1767") public CallOptions withAuthority(@Nullable String authority) { - CallOptions newOptions = new CallOptions(this); - newOptions.authority = authority; + CallOptions newOptions = fullBuild(this).authority(authority).build(); return newOptions; } @@ -98,8 +172,7 @@ public CallOptions withAuthority(@Nullable String authority) { * Returns a new {@code CallOptions} with the given call credentials. */ public CallOptions withCallCredentials(@Nullable CallCredentials credentials) { - CallOptions newOptions = new CallOptions(this); - newOptions.credentials = credentials; + CallOptions newOptions = fullBuild(this).credentials(credentials).build(); return newOptions; } @@ -113,8 +186,7 @@ public CallOptions withCallCredentials(@Nullable CallCredentials credentials) { */ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/1704") public CallOptions withCompression(@Nullable String compressorName) { - CallOptions newOptions = new CallOptions(this); - newOptions.compressorName = compressorName; + CallOptions newOptions = fullBuild(this).compressorName(compressorName).build(); return newOptions; } @@ -127,8 +199,7 @@ public CallOptions withCompression(@Nullable String compressorName) { * @param deadline the deadline or {@code null} for unsetting the deadline. */ public CallOptions withDeadline(@Nullable Deadline deadline) { - CallOptions newOptions = new CallOptions(this); - newOptions.deadline = deadline; + CallOptions newOptions = fullBuild(this).deadline(deadline).build(); return newOptions; } @@ -156,8 +227,7 @@ public Deadline getDeadline() { * fails RPCs without sending them if unable to connect. */ public CallOptions withWaitForReady() { - CallOptions newOptions = new CallOptions(this); - newOptions.waitForReady = Boolean.TRUE; + CallOptions newOptions = fullBuild(this).waitForReady(Boolean.TRUE).build(); return newOptions; } @@ -166,8 +236,7 @@ public CallOptions withWaitForReady() { * This method should be rarely used because the default is without 'wait for ready'. */ public CallOptions withoutWaitForReady() { - CallOptions newOptions = new CallOptions(this); - newOptions.waitForReady = Boolean.FALSE; + CallOptions newOptions = new Builder().waitForReady(Boolean.FALSE).build(); return newOptions; } @@ -208,8 +277,7 @@ public CallCredentials getCredentials() { * executor specified with {@link ManagedChannelBuilder#executor}. */ public CallOptions withExecutor(@Nullable Executor executor) { - CallOptions newOptions = new CallOptions(this); - newOptions.executor = executor; + CallOptions newOptions = fullBuild(this).executor(executor).build(); return newOptions; } @@ -221,12 +289,13 @@ public CallOptions withExecutor(@Nullable Executor executor) { */ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/2861") public CallOptions withStreamTracerFactory(ClientStreamTracer.Factory factory) { - CallOptions newOptions = new CallOptions(this); ArrayList newList = - new ArrayList<>(streamTracerFactories.size() + 1); + new ArrayList<>(streamTracerFactories.size() + 1); newList.addAll(streamTracerFactories); newList.add(factory); - newOptions.streamTracerFactories = Collections.unmodifiableList(newList); + CallOptions newOptions = fullBuild(this) + .streamTracerFactories(Collections.unmodifiableList(newList)) + .build(); return newOptions; } @@ -319,7 +388,7 @@ public CallOptions withOption(Key key, T value) { Preconditions.checkNotNull(key, "key"); Preconditions.checkNotNull(value, "value"); - CallOptions newOptions = new CallOptions(this); + Builder builder = fullBuild(this); int existingIdx = -1; for (int i = 0; i < customOptions.length; i++) { if (key.equals(customOptions[i][0])) { @@ -328,7 +397,9 @@ public CallOptions withOption(Key key, T value) { } } - newOptions.customOptions = new Object[customOptions.length + (existingIdx == -1 ? 1 : 0)][2]; + CallOptions newOptions = builder + .customOptions(new Object[customOptions.length + (existingIdx == -1 ? 1 : 0)][2]) + .build(); System.arraycopy(customOptions, 0, newOptions.customOptions, 0, customOptions.length); if (existingIdx == -1) { @@ -368,10 +439,6 @@ public Executor getExecutor() { return executor; } - private CallOptions() { - customOptions = new Object[0][2]; - } - /** * Returns whether * 'wait for ready' option is enabled for the call. 'Fail fast' is the default option for gRPC @@ -392,8 +459,9 @@ Boolean getWaitForReady() { @ExperimentalApi("https://github.com/grpc/grpc-java/issues/2563") public CallOptions withMaxInboundMessageSize(int maxSize) { checkArgument(maxSize >= 0, "invalid maxsize %s", maxSize); - CallOptions newOptions = new CallOptions(this); - newOptions.maxInboundMessageSize = maxSize; + CallOptions newOptions = fullBuild(this) + .maxInboundMessageSize(maxSize) + .build(); return newOptions; } @@ -403,8 +471,9 @@ public CallOptions withMaxInboundMessageSize(int maxSize) { @ExperimentalApi("https://github.com/grpc/grpc-java/issues/2563") public CallOptions withMaxOutboundMessageSize(int maxSize) { checkArgument(maxSize >= 0, "invalid maxsize %s", maxSize); - CallOptions newOptions = new CallOptions(this); - newOptions.maxOutboundMessageSize = maxSize; + CallOptions newOptions = fullBuild(this) + .maxOutboundMessageSize(maxSize) + .build(); return newOptions; } @@ -427,34 +496,35 @@ public Integer getMaxOutboundMessageSize() { } /** - * Copy constructor. + * Copy CallOptions. */ - private CallOptions(CallOptions other) { - deadline = other.deadline; - authority = other.authority; - credentials = other.credentials; - executor = other.executor; - compressorName = other.compressorName; - customOptions = other.customOptions; - waitForReady = other.waitForReady; - maxInboundMessageSize = other.maxInboundMessageSize; - maxOutboundMessageSize = other.maxOutboundMessageSize; - streamTracerFactories = other.streamTracerFactories; + private Builder fullBuild(CallOptions other) { + return new Builder() + .deadline(other.deadline) + .executor(other.executor) + .authority(other.authority) + .credentials(other.credentials) + .compressorName(other.compressorName) + .customOptions(other.customOptions) + .streamTracerFactories(other.streamTracerFactories) + .waitForReady(other.waitForReady) + .maxInboundMessageSize(other.maxInboundMessageSize) + .maxOutboundMessageSize(other.maxOutboundMessageSize); } @Override public String toString() { return MoreObjects.toStringHelper(this) - .add("deadline", deadline) - .add("authority", authority) - .add("callCredentials", credentials) - .add("executor", executor != null ? executor.getClass() : null) - .add("compressorName", compressorName) - .add("customOptions", Arrays.deepToString(customOptions)) - .add("waitForReady", isWaitForReady()) - .add("maxInboundMessageSize", maxInboundMessageSize) - .add("maxOutboundMessageSize", maxOutboundMessageSize) - .add("streamTracerFactories", streamTracerFactories) - .toString(); + .add("deadline", deadline) + .add("authority", authority) + .add("callCredentials", credentials) + .add("executor", executor != null ? executor.getClass() : null) + .add("compressorName", compressorName) + .add("customOptions", Arrays.deepToString(customOptions)) + .add("waitForReady", isWaitForReady()) + .add("maxInboundMessageSize", maxInboundMessageSize) + .add("maxOutboundMessageSize", maxOutboundMessageSize) + .add("streamTracerFactories", streamTracerFactories) + .toString(); } } From c8fb1de0ee269a9a4ae366e8a29b4b9955cca3ee Mon Sep 17 00:00:00 2001 From: pandaapo <1052156701@qq.com> Date: Sat, 12 Nov 2022 22:33:43 +0800 Subject: [PATCH 2/9] Update copyright in file header and adjust indentation. --- api/src/main/java/io/grpc/CallOptions.java | 26 +++++++++++----------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/api/src/main/java/io/grpc/CallOptions.java b/api/src/main/java/io/grpc/CallOptions.java index 013903670af..8a4adb07f1f 100644 --- a/api/src/main/java/io/grpc/CallOptions.java +++ b/api/src/main/java/io/grpc/CallOptions.java @@ -1,5 +1,5 @@ /* - * Copyright 2015 The gRPC Authors + * Copyright 2015, 2022 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. @@ -290,7 +290,7 @@ public CallOptions withExecutor(@Nullable Executor executor) { @ExperimentalApi("https://github.com/grpc/grpc-java/issues/2861") public CallOptions withStreamTracerFactory(ClientStreamTracer.Factory factory) { ArrayList newList = - new ArrayList<>(streamTracerFactories.size() + 1); + new ArrayList<>(streamTracerFactories.size() + 1); newList.addAll(streamTracerFactories); newList.add(factory); CallOptions newOptions = fullBuild(this) @@ -515,16 +515,16 @@ private Builder fullBuild(CallOptions other) { @Override public String toString() { return MoreObjects.toStringHelper(this) - .add("deadline", deadline) - .add("authority", authority) - .add("callCredentials", credentials) - .add("executor", executor != null ? executor.getClass() : null) - .add("compressorName", compressorName) - .add("customOptions", Arrays.deepToString(customOptions)) - .add("waitForReady", isWaitForReady()) - .add("maxInboundMessageSize", maxInboundMessageSize) - .add("maxOutboundMessageSize", maxOutboundMessageSize) - .add("streamTracerFactories", streamTracerFactories) - .toString(); + .add("deadline", deadline) + .add("authority", authority) + .add("callCredentials", credentials) + .add("executor", executor != null ? executor.getClass() : null) + .add("compressorName", compressorName) + .add("customOptions", Arrays.deepToString(customOptions)) + .add("waitForReady", isWaitForReady()) + .add("maxInboundMessageSize", maxInboundMessageSize) + .add("maxOutboundMessageSize", maxOutboundMessageSize) + .add("streamTracerFactories", streamTracerFactories) + .toString(); } } From 6f8830612e3acf882de5000fc8464bc34b8f947e Mon Sep 17 00:00:00 2001 From: pandaapo <1052156701@qq.com> Date: Sat, 12 Nov 2022 22:49:49 +0800 Subject: [PATCH 3/9] Fix checkstyle error. --- api/src/main/java/io/grpc/CallOptions.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/api/src/main/java/io/grpc/CallOptions.java b/api/src/main/java/io/grpc/CallOptions.java index 8a4adb07f1f..ff3e97b4e57 100644 --- a/api/src/main/java/io/grpc/CallOptions.java +++ b/api/src/main/java/io/grpc/CallOptions.java @@ -16,19 +16,19 @@ package io.grpc; -import static com.google.common.base.Preconditions.checkArgument; - import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; + +import javax.annotation.CheckReturnValue; +import javax.annotation.Nullable; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; -import javax.annotation.CheckReturnValue; -import javax.annotation.Nullable; -import javax.annotation.concurrent.Immutable; + +import static com.google.common.base.Preconditions.checkArgument; /** * The collection of runtime options for a new RPC call. From dea7008c027b936f15bc74b6dd7a8112d43713dc Mon Sep 17 00:00:00 2001 From: pandaapo <1052156701@qq.com> Date: Sat, 12 Nov 2022 22:55:56 +0800 Subject: [PATCH 4/9] Reformat code --- api/src/main/java/io/grpc/CallOptions.java | 32 +++++++++++----------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/api/src/main/java/io/grpc/CallOptions.java b/api/src/main/java/io/grpc/CallOptions.java index ff3e97b4e57..1c9440ef0bc 100644 --- a/api/src/main/java/io/grpc/CallOptions.java +++ b/api/src/main/java/io/grpc/CallOptions.java @@ -16,19 +16,19 @@ package io.grpc; +import static com.google.common.base.Preconditions.checkArgument; + import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; -import javax.annotation.CheckReturnValue; -import javax.annotation.Nullable; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; - -import static com.google.common.base.Preconditions.checkArgument; +import javax.annotation.CheckReturnValue; +import javax.annotation.Nullable; /** * The collection of runtime options for a new RPC call. @@ -290,7 +290,7 @@ public CallOptions withExecutor(@Nullable Executor executor) { @ExperimentalApi("https://github.com/grpc/grpc-java/issues/2861") public CallOptions withStreamTracerFactory(ClientStreamTracer.Factory factory) { ArrayList newList = - new ArrayList<>(streamTracerFactories.size() + 1); + new ArrayList<>(streamTracerFactories.size() + 1); newList.addAll(streamTracerFactories); newList.add(factory); CallOptions newOptions = fullBuild(this) @@ -515,16 +515,16 @@ private Builder fullBuild(CallOptions other) { @Override public String toString() { return MoreObjects.toStringHelper(this) - .add("deadline", deadline) - .add("authority", authority) - .add("callCredentials", credentials) - .add("executor", executor != null ? executor.getClass() : null) - .add("compressorName", compressorName) - .add("customOptions", Arrays.deepToString(customOptions)) - .add("waitForReady", isWaitForReady()) - .add("maxInboundMessageSize", maxInboundMessageSize) - .add("maxOutboundMessageSize", maxOutboundMessageSize) - .add("streamTracerFactories", streamTracerFactories) - .toString(); + .add("deadline", deadline) + .add("authority", authority) + .add("callCredentials", credentials) + .add("executor", executor != null ? executor.getClass() : null) + .add("compressorName", compressorName) + .add("customOptions", Arrays.deepToString(customOptions)) + .add("waitForReady", isWaitForReady()) + .add("maxInboundMessageSize", maxInboundMessageSize) + .add("maxOutboundMessageSize", maxOutboundMessageSize) + .add("streamTracerFactories", streamTracerFactories) + .toString(); } } From 2e62fead71df473fdf3b33a9fb02dba0e374c9de Mon Sep 17 00:00:00 2001 From: pandaapo <1052156701@qq.com> Date: Wed, 16 Nov 2022 11:31:00 +0800 Subject: [PATCH 5/9] Modify as reviews. --- api/src/main/java/io/grpc/CallOptions.java | 69 +++++++++------------- 1 file changed, 27 insertions(+), 42 deletions(-) diff --git a/api/src/main/java/io/grpc/CallOptions.java b/api/src/main/java/io/grpc/CallOptions.java index 1c9440ef0bc..c1046492176 100644 --- a/api/src/main/java/io/grpc/CallOptions.java +++ b/api/src/main/java/io/grpc/CallOptions.java @@ -1,5 +1,5 @@ /* - * Copyright 2015, 2022 The gRPC Authors + * Copyright 2015 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. @@ -20,7 +20,6 @@ import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; - import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -29,12 +28,14 @@ import java.util.concurrent.TimeUnit; import javax.annotation.CheckReturnValue; import javax.annotation.Nullable; +import javax.annotation.concurrent.Immutable; /** * The collection of runtime options for a new RPC call. * *

A field that is not set is {@code null}. */ +@Immutable @CheckReturnValue public final class CallOptions { /** @@ -85,18 +86,18 @@ private CallOptions(Builder builder) { this.maxOutboundMessageSize = builder.maxOutboundMessageSize; } - private static class Builder { - private Deadline deadline; - private Executor executor; - private String authority; - private CallCredentials credentials; - private String compressorName; - private Object[][] customOptions = new Object[0][2]; + static class Builder { + Deadline deadline; + Executor executor; + String authority; + CallCredentials credentials; + String compressorName; + Object[][] customOptions = new Object[0][2]; // Unmodifiable list - private List streamTracerFactories = Collections.emptyList(); - private Boolean waitForReady; - private Integer maxInboundMessageSize; - private Integer maxOutboundMessageSize; + List streamTracerFactories = Collections.emptyList(); + Boolean waitForReady; + Integer maxInboundMessageSize; + Integer maxOutboundMessageSize; private Builder deadline(Deadline deadline) { this.deadline = deadline; @@ -144,7 +145,7 @@ private Builder maxOutboundMessageSize(Integer maxOutboundMessageSize) { } private Builder streamTracerFactories(List streamTracerFactories) { - this.streamTracerFactories = streamTracerFactories; + this.streamTracerFactories = Collections.unmodifiableList(streamTracerFactories); return this; } @@ -164,16 +165,14 @@ private CallOptions build() { */ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/1767") public CallOptions withAuthority(@Nullable String authority) { - CallOptions newOptions = fullBuild(this).authority(authority).build(); - return newOptions; + return toBuilder(this).authority(authority).build(); } /** * Returns a new {@code CallOptions} with the given call credentials. */ public CallOptions withCallCredentials(@Nullable CallCredentials credentials) { - CallOptions newOptions = fullBuild(this).credentials(credentials).build(); - return newOptions; + return toBuilder(this).credentials(credentials).build(); } /** @@ -186,8 +185,7 @@ public CallOptions withCallCredentials(@Nullable CallCredentials credentials) { */ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/1704") public CallOptions withCompression(@Nullable String compressorName) { - CallOptions newOptions = fullBuild(this).compressorName(compressorName).build(); - return newOptions; + return toBuilder(this).compressorName(compressorName).build(); } /** @@ -199,8 +197,7 @@ public CallOptions withCompression(@Nullable String compressorName) { * @param deadline the deadline or {@code null} for unsetting the deadline. */ public CallOptions withDeadline(@Nullable Deadline deadline) { - CallOptions newOptions = fullBuild(this).deadline(deadline).build(); - return newOptions; + return toBuilder(this).deadline(deadline).build(); } /** @@ -227,8 +224,7 @@ public Deadline getDeadline() { * fails RPCs without sending them if unable to connect. */ public CallOptions withWaitForReady() { - CallOptions newOptions = fullBuild(this).waitForReady(Boolean.TRUE).build(); - return newOptions; + return toBuilder(this).waitForReady(Boolean.TRUE).build(); } /** @@ -236,8 +232,7 @@ public CallOptions withWaitForReady() { * This method should be rarely used because the default is without 'wait for ready'. */ public CallOptions withoutWaitForReady() { - CallOptions newOptions = new Builder().waitForReady(Boolean.FALSE).build(); - return newOptions; + return new Builder().waitForReady(Boolean.FALSE).build(); } /** @@ -277,8 +272,7 @@ public CallCredentials getCredentials() { * executor specified with {@link ManagedChannelBuilder#executor}. */ public CallOptions withExecutor(@Nullable Executor executor) { - CallOptions newOptions = fullBuild(this).executor(executor).build(); - return newOptions; + return toBuilder(this).executor(executor).build(); } /** @@ -293,10 +287,7 @@ public CallOptions withStreamTracerFactory(ClientStreamTracer.Factory factory) { new ArrayList<>(streamTracerFactories.size() + 1); newList.addAll(streamTracerFactories); newList.add(factory); - CallOptions newOptions = fullBuild(this) - .streamTracerFactories(Collections.unmodifiableList(newList)) - .build(); - return newOptions; + return toBuilder(this).streamTracerFactories(newList).build(); } /** @@ -388,7 +379,7 @@ public CallOptions withOption(Key key, T value) { Preconditions.checkNotNull(key, "key"); Preconditions.checkNotNull(value, "value"); - Builder builder = fullBuild(this); + Builder builder = toBuilder(this); int existingIdx = -1; for (int i = 0; i < customOptions.length; i++) { if (key.equals(customOptions[i][0])) { @@ -459,10 +450,7 @@ Boolean getWaitForReady() { @ExperimentalApi("https://github.com/grpc/grpc-java/issues/2563") public CallOptions withMaxInboundMessageSize(int maxSize) { checkArgument(maxSize >= 0, "invalid maxsize %s", maxSize); - CallOptions newOptions = fullBuild(this) - .maxInboundMessageSize(maxSize) - .build(); - return newOptions; + return toBuilder(this).maxInboundMessageSize(maxSize).build(); } /** @@ -471,10 +459,7 @@ public CallOptions withMaxInboundMessageSize(int maxSize) { @ExperimentalApi("https://github.com/grpc/grpc-java/issues/2563") public CallOptions withMaxOutboundMessageSize(int maxSize) { checkArgument(maxSize >= 0, "invalid maxsize %s", maxSize); - CallOptions newOptions = fullBuild(this) - .maxOutboundMessageSize(maxSize) - .build(); - return newOptions; + return toBuilder(this).maxOutboundMessageSize(maxSize).build(); } /** @@ -498,7 +483,7 @@ public Integer getMaxOutboundMessageSize() { /** * Copy CallOptions. */ - private Builder fullBuild(CallOptions other) { + private Builder toBuilder(CallOptions other) { return new Builder() .deadline(other.deadline) .executor(other.executor) From a523fdbb0e5d157ff7cff040b1ed603b55a27781 Mon Sep 17 00:00:00 2001 From: pandaapo <1052156701@qq.com> Date: Wed, 16 Nov 2022 13:32:34 +0800 Subject: [PATCH 6/9] Modify as review --- api/src/main/java/io/grpc/CallOptions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/io/grpc/CallOptions.java b/api/src/main/java/io/grpc/CallOptions.java index c1046492176..7b0721c4051 100644 --- a/api/src/main/java/io/grpc/CallOptions.java +++ b/api/src/main/java/io/grpc/CallOptions.java @@ -483,7 +483,7 @@ public Integer getMaxOutboundMessageSize() { /** * Copy CallOptions. */ - private Builder toBuilder(CallOptions other) { + private static Builder toBuilder(CallOptions other) { return new Builder() .deadline(other.deadline) .executor(other.executor) From 131535262756d541a7e1b82053ccdd90525fcf6c Mon Sep 17 00:00:00 2001 From: pandaapo <1052156701@qq.com> Date: Wed, 16 Nov 2022 14:07:17 +0800 Subject: [PATCH 7/9] Modify as review. --- api/src/main/java/io/grpc/CallOptions.java | 118 ++++++++------------- 1 file changed, 44 insertions(+), 74 deletions(-) diff --git a/api/src/main/java/io/grpc/CallOptions.java b/api/src/main/java/io/grpc/CallOptions.java index 7b0721c4051..f3c33377cc9 100644 --- a/api/src/main/java/io/grpc/CallOptions.java +++ b/api/src/main/java/io/grpc/CallOptions.java @@ -99,56 +99,6 @@ static class Builder { Integer maxInboundMessageSize; Integer maxOutboundMessageSize; - private Builder deadline(Deadline deadline) { - this.deadline = deadline; - return this; - } - - private Builder executor(Executor executor) { - this.executor = executor; - return this; - } - - private Builder authority(String authority) { - this.authority = authority; - return this; - } - - private Builder credentials(CallCredentials credentials) { - this.credentials = credentials; - return this; - } - - private Builder compressorName(String compressorName) { - this.compressorName = compressorName; - return this; - } - - private Builder customOptions(Object[][] customOptions) { - this.customOptions = customOptions; - return this; - } - - private Builder waitForReady(Boolean waitForReady) { - this.waitForReady = waitForReady; - return this; - } - - private Builder maxInboundMessageSize(Integer maxInboundMessageSize) { - this.maxInboundMessageSize = maxInboundMessageSize; - return this; - } - - private Builder maxOutboundMessageSize(Integer maxOutboundMessageSize) { - this.maxOutboundMessageSize = maxOutboundMessageSize; - return this; - } - - private Builder streamTracerFactories(List streamTracerFactories) { - this.streamTracerFactories = Collections.unmodifiableList(streamTracerFactories); - return this; - } - private CallOptions build() { return new CallOptions(this); } @@ -165,14 +115,18 @@ private CallOptions build() { */ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/1767") public CallOptions withAuthority(@Nullable String authority) { - return toBuilder(this).authority(authority).build(); + Builder builder = toBuilder(this); + builder.authority = authority; + return builder.build(); } /** * Returns a new {@code CallOptions} with the given call credentials. */ public CallOptions withCallCredentials(@Nullable CallCredentials credentials) { - return toBuilder(this).credentials(credentials).build(); + Builder builder = toBuilder(this); + builder.credentials = credentials; + return builder.build(); } /** @@ -185,7 +139,9 @@ public CallOptions withCallCredentials(@Nullable CallCredentials credentials) { */ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/1704") public CallOptions withCompression(@Nullable String compressorName) { - return toBuilder(this).compressorName(compressorName).build(); + Builder builder = toBuilder(this); + builder.compressorName = compressorName; + return builder.build(); } /** @@ -197,7 +153,9 @@ public CallOptions withCompression(@Nullable String compressorName) { * @param deadline the deadline or {@code null} for unsetting the deadline. */ public CallOptions withDeadline(@Nullable Deadline deadline) { - return toBuilder(this).deadline(deadline).build(); + Builder builder = toBuilder(this); + builder.deadline = deadline; + return builder.build(); } /** @@ -224,7 +182,9 @@ public Deadline getDeadline() { * fails RPCs without sending them if unable to connect. */ public CallOptions withWaitForReady() { - return toBuilder(this).waitForReady(Boolean.TRUE).build(); + Builder builder = toBuilder(this); + builder.waitForReady = Boolean.TRUE; + return builder.build(); } /** @@ -232,7 +192,9 @@ public CallOptions withWaitForReady() { * This method should be rarely used because the default is without 'wait for ready'. */ public CallOptions withoutWaitForReady() { - return new Builder().waitForReady(Boolean.FALSE).build(); + Builder builder = new Builder(); + builder.waitForReady = Boolean.FALSE; + return builder.build(); } /** @@ -272,7 +234,9 @@ public CallCredentials getCredentials() { * executor specified with {@link ManagedChannelBuilder#executor}. */ public CallOptions withExecutor(@Nullable Executor executor) { - return toBuilder(this).executor(executor).build(); + Builder builder = toBuilder(this); + builder.executor = executor; + return builder.build(); } /** @@ -287,7 +251,9 @@ public CallOptions withStreamTracerFactory(ClientStreamTracer.Factory factory) { new ArrayList<>(streamTracerFactories.size() + 1); newList.addAll(streamTracerFactories); newList.add(factory); - return toBuilder(this).streamTracerFactories(newList).build(); + Builder builder = toBuilder(this); + builder.streamTracerFactories = Collections.unmodifiableList(newList); + return builder.build(); } /** @@ -388,9 +354,8 @@ public CallOptions withOption(Key key, T value) { } } - CallOptions newOptions = builder - .customOptions(new Object[customOptions.length + (existingIdx == -1 ? 1 : 0)][2]) - .build(); + builder.customOptions = new Object[customOptions.length + (existingIdx == -1 ? 1 : 0)][2]; + CallOptions newOptions = builder.build(); System.arraycopy(customOptions, 0, newOptions.customOptions, 0, customOptions.length); if (existingIdx == -1) { @@ -450,7 +415,9 @@ Boolean getWaitForReady() { @ExperimentalApi("https://github.com/grpc/grpc-java/issues/2563") public CallOptions withMaxInboundMessageSize(int maxSize) { checkArgument(maxSize >= 0, "invalid maxsize %s", maxSize); - return toBuilder(this).maxInboundMessageSize(maxSize).build(); + Builder builder = toBuilder(this); + builder.maxInboundMessageSize = maxSize; + return builder.build(); } /** @@ -459,7 +426,9 @@ public CallOptions withMaxInboundMessageSize(int maxSize) { @ExperimentalApi("https://github.com/grpc/grpc-java/issues/2563") public CallOptions withMaxOutboundMessageSize(int maxSize) { checkArgument(maxSize >= 0, "invalid maxsize %s", maxSize); - return toBuilder(this).maxOutboundMessageSize(maxSize).build(); + Builder builder = toBuilder(this); + builder.maxOutboundMessageSize = maxSize; + return builder.build(); } /** @@ -484,17 +453,18 @@ public Integer getMaxOutboundMessageSize() { * Copy CallOptions. */ private static Builder toBuilder(CallOptions other) { - return new Builder() - .deadline(other.deadline) - .executor(other.executor) - .authority(other.authority) - .credentials(other.credentials) - .compressorName(other.compressorName) - .customOptions(other.customOptions) - .streamTracerFactories(other.streamTracerFactories) - .waitForReady(other.waitForReady) - .maxInboundMessageSize(other.maxInboundMessageSize) - .maxOutboundMessageSize(other.maxOutboundMessageSize); + Builder builder = new Builder(); + builder.deadline = other.deadline; + builder.executor = other.executor; + builder.authority = other.authority; + builder.credentials = other.credentials; + builder.compressorName = other.compressorName; + builder.customOptions = other.customOptions; + builder.streamTracerFactories = other.streamTracerFactories; + builder.waitForReady = other.waitForReady; + builder.maxInboundMessageSize = other.maxInboundMessageSize; + builder.maxOutboundMessageSize = other.maxOutboundMessageSize; + return builder; } @Override From 98bfa85e2807f7b9369bd6366c1afe878ffca829 Mon Sep 17 00:00:00 2001 From: pandaapo <1052156701@qq.com> Date: Thu, 17 Nov 2022 01:27:02 +0800 Subject: [PATCH 8/9] Modify as review --- api/src/main/java/io/grpc/CallOptions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/io/grpc/CallOptions.java b/api/src/main/java/io/grpc/CallOptions.java index f3c33377cc9..4326e5a61ba 100644 --- a/api/src/main/java/io/grpc/CallOptions.java +++ b/api/src/main/java/io/grpc/CallOptions.java @@ -192,7 +192,7 @@ public CallOptions withWaitForReady() { * This method should be rarely used because the default is without 'wait for ready'. */ public CallOptions withoutWaitForReady() { - Builder builder = new Builder(); + Builder builder = toBuilder(this); builder.waitForReady = Boolean.FALSE; return builder.build(); } From 09731476faeac6792cb3e2716e62710728de2b2e Mon Sep 17 00:00:00 2001 From: pandaapo <1052156701@qq.com> Date: Thu, 17 Nov 2022 07:46:14 +0800 Subject: [PATCH 9/9] Modify as review --- api/src/main/java/io/grpc/CallOptions.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/api/src/main/java/io/grpc/CallOptions.java b/api/src/main/java/io/grpc/CallOptions.java index 4326e5a61ba..16684275dea 100644 --- a/api/src/main/java/io/grpc/CallOptions.java +++ b/api/src/main/java/io/grpc/CallOptions.java @@ -355,18 +355,17 @@ public CallOptions withOption(Key key, T value) { } builder.customOptions = new Object[customOptions.length + (existingIdx == -1 ? 1 : 0)][2]; - CallOptions newOptions = builder.build(); - System.arraycopy(customOptions, 0, newOptions.customOptions, 0, customOptions.length); + System.arraycopy(customOptions, 0, builder.customOptions, 0, customOptions.length); if (existingIdx == -1) { // Add a new option - newOptions.customOptions[customOptions.length] = new Object[] {key, value}; + builder.customOptions[customOptions.length] = new Object[] {key, value}; } else { // Replace an existing option - newOptions.customOptions[existingIdx] = new Object[] {key, value}; + builder.customOptions[existingIdx] = new Object[] {key, value}; } - return newOptions; + return builder.build(); } /**