From 624660c603b44dfde07f1328847186dc36548498 Mon Sep 17 00:00:00 2001 From: Diego Alonso Marquez Palacios Date: Fri, 22 Sep 2023 14:35:01 -0400 Subject: [PATCH 01/22] fix: point to documentation on LRO CancelledException --- .../api/gax/longrunning/OperationTimedPollAlgorithm.java | 4 +++- .../google/showcase/v1beta1/it/ITLongRunningOperation.java | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java b/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java index ec7e842e3d..8605b6da83 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java @@ -77,7 +77,9 @@ public boolean shouldRetry(TimedAttemptSettings nextAttemptSettings) if (super.shouldRetry(nextAttemptSettings)) { return true; } - throw new CancellationException(); + throw new CancellationException( + "The task has been cancelled. Please refer to " + + "https://github.com/googleapis/google-cloud-java#lro-timeouts for more information"); } // Note: if the potential time spent is exactly equal to the totalTimeout, diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java index 5f1e29963d..54cb7a61bf 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java @@ -185,7 +185,8 @@ public void testGRPC_LROUnsuccessfulResponse_exceedsTotalTimeout_throwsDeadlineE .build(); OperationFuture operationFuture = httpjsonClient.waitOperationCallable().futureCall(waitRequest); - assertThrows(CancellationException.class, operationFuture::get); + CancellationException cancellationException = assertThrows(CancellationException.class, operationFuture::get); + assertThat(cancellationException).hasMessageThat().contains("https://github.com/googleapis/google-cloud-java#lro-timeouts"); int attemptCount = operationFuture.getPollingFuture().getAttemptSettings().getAttemptCount(); assertThat(attemptCount).isGreaterThan(1); } finally { From a4483eeb1b0a4d2155bee8fea74ccc32fcb9d522 Mon Sep 17 00:00:00 2001 From: Diego Marquez Date: Fri, 22 Sep 2023 18:53:30 +0000 Subject: [PATCH 02/22] mvn fmt:format --- .../google/showcase/v1beta1/it/ITLongRunningOperation.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java index 54cb7a61bf..e65736a04a 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java @@ -185,8 +185,11 @@ public void testGRPC_LROUnsuccessfulResponse_exceedsTotalTimeout_throwsDeadlineE .build(); OperationFuture operationFuture = httpjsonClient.waitOperationCallable().futureCall(waitRequest); - CancellationException cancellationException = assertThrows(CancellationException.class, operationFuture::get); - assertThat(cancellationException).hasMessageThat().contains("https://github.com/googleapis/google-cloud-java#lro-timeouts"); + CancellationException cancellationException = + assertThrows(CancellationException.class, operationFuture::get); + assertThat(cancellationException) + .hasMessageThat() + .contains("https://github.com/googleapis/google-cloud-java#lro-timeouts"); int attemptCount = operationFuture.getPollingFuture().getAttemptSettings().getAttemptCount(); assertThat(attemptCount).isGreaterThan(1); } finally { From fa781ea430d2f32f56207c7b9c67312170d1c807 Mon Sep 17 00:00:00 2001 From: Diego Alonso Marquez Palacios Date: Tue, 17 Oct 2023 21:43:11 -0400 Subject: [PATCH 03/22] add test for http IT in LRO --- .../google/showcase/v1beta1/it/ITLongRunningOperation.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java index e65736a04a..ccb65a9db4 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java @@ -146,7 +146,11 @@ public void testGRPC_LROUnsuccessfulResponse_exceedsTotalTimeout_throwsDeadlineE .build(); OperationFuture operationFuture = grpcClient.waitOperationCallable().futureCall(waitRequest); - assertThrows(CancellationException.class, operationFuture::get); + CancellationException cancellationException = + assertThrows(CancellationException.class, operationFuture::get); + assertThat(cancellationException) + .hasMessageThat() + .contains("https://github.com/googleapis/google-cloud-java#lro-timeouts"); int attemptCount = operationFuture.getPollingFuture().getAttemptSettings().getAttemptCount(); assertThat(attemptCount).isGreaterThan(1); } finally { From 937e4e6cc16ca53714e370640d86f05ebb6f95ab Mon Sep 17 00:00:00 2001 From: Diego Alonso Marquez Palacios Date: Tue, 17 Oct 2023 21:53:40 -0400 Subject: [PATCH 04/22] format code --- .../google/showcase/v1beta1/it/ITLongRunningOperation.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java index ccb65a9db4..003dd928c3 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java @@ -147,10 +147,10 @@ public void testGRPC_LROUnsuccessfulResponse_exceedsTotalTimeout_throwsDeadlineE OperationFuture operationFuture = grpcClient.waitOperationCallable().futureCall(waitRequest); CancellationException cancellationException = - assertThrows(CancellationException.class, operationFuture::get); + assertThrows(CancellationException.class, operationFuture::get); assertThat(cancellationException) - .hasMessageThat() - .contains("https://github.com/googleapis/google-cloud-java#lro-timeouts"); + .hasMessageThat() + .contains("https://github.com/googleapis/google-cloud-java#lro-timeouts"); int attemptCount = operationFuture.getPollingFuture().getAttemptSettings().getAttemptCount(); assertThat(attemptCount).isGreaterThan(1); } finally { From 9f17544b510210ae33ca1bc545e587924af678b8 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 3 Nov 2023 21:01:51 +0000 Subject: [PATCH 05/22] use final variable on LRO cancellation --- .../api/gax/longrunning/OperationTimedPollAlgorithm.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java b/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java index 8605b6da83..25072d4144 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java @@ -43,6 +43,10 @@ * algorithm cancels polling. */ public class OperationTimedPollAlgorithm extends ExponentialRetryAlgorithm { + + public static final String LRO_TROUBLESHOOTING_LINK = + "https://github.com/googleapis/google-cloud-java#lro-timeouts"; + /** * Creates the polling algorithm, using the default {@code NanoClock} for time computations. * @@ -79,7 +83,7 @@ public boolean shouldRetry(TimedAttemptSettings nextAttemptSettings) } throw new CancellationException( "The task has been cancelled. Please refer to " - + "https://github.com/googleapis/google-cloud-java#lro-timeouts for more information"); + + LRO_TROUBLESHOOTING_LINK + " for more information"); } // Note: if the potential time spent is exactly equal to the totalTimeout, From 27d224038b4b2f6635a98160347ff9eb234936e5 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Sat, 4 Nov 2023 01:54:00 +0000 Subject: [PATCH 06/22] Wrap guava's CancellationException in our own Exception --- .../api/gax/retrying/BasicRetryingFuture.java | 17 +++++++++++++++++ .../v1beta1/it/ITLongRunningOperation.java | 5 +++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/retrying/BasicRetryingFuture.java b/gax-java/gax/src/main/java/com/google/api/gax/retrying/BasicRetryingFuture.java index de7b5b5acb..303ba42c0d 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/retrying/BasicRetryingFuture.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/retrying/BasicRetryingFuture.java @@ -67,6 +67,8 @@ class BasicRetryingFuture extends AbstractFuture private volatile ApiFuture latestCompletedAttemptResult; private volatile ApiFuture attemptResult; + private volatile CancellationException customCancellationException = null; + private static final Logger LOG = Logger.getLogger(BasicRetryingFuture.class.getName()); BasicRetryingFuture( @@ -205,6 +207,9 @@ void handleAttempt(Throwable throwable, ResponseT response) { } catch (CancellationException e) { // A retry algorithm triggered cancellation. tracer.attemptFailedRetriesExhausted(e); + // The exception caught here is coming from gax code. We save it in order to throw + // this exception as the main one, while keeping guava's `Task was cancelled.` as a cause + customCancellationException = e; super.cancel(false); } catch (Exception e) { // Should never happen, but still possible in case of buggy retry algorithm implementation. @@ -264,6 +269,18 @@ private void setAttemptResult(Throwable throwable, ResponseT response, boolean s } } + public ResponseT get() throws InterruptedException, ExecutionException{ + try { + return super.get(); + } catch (CancellationException ex) { + if (customCancellationException instanceof CancellationException) { + customCancellationException.initCause(ex); + throw customCancellationException; + } + throw ex; + } + } + private class CompletionListener implements Runnable { @Override public void run() { diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java index 003dd928c3..47d38095d6 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java @@ -15,6 +15,7 @@ */ package com.google.showcase.v1beta1.it; +import static com.google.api.gax.longrunning.OperationTimedPollAlgorithm.LRO_TROUBLESHOOTING_LINK; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertThrows; @@ -150,7 +151,7 @@ public void testGRPC_LROUnsuccessfulResponse_exceedsTotalTimeout_throwsDeadlineE assertThrows(CancellationException.class, operationFuture::get); assertThat(cancellationException) .hasMessageThat() - .contains("https://github.com/googleapis/google-cloud-java#lro-timeouts"); + .contains(LRO_TROUBLESHOOTING_LINK); int attemptCount = operationFuture.getPollingFuture().getAttemptSettings().getAttemptCount(); assertThat(attemptCount).isGreaterThan(1); } finally { @@ -193,7 +194,7 @@ public void testGRPC_LROUnsuccessfulResponse_exceedsTotalTimeout_throwsDeadlineE assertThrows(CancellationException.class, operationFuture::get); assertThat(cancellationException) .hasMessageThat() - .contains("https://github.com/googleapis/google-cloud-java#lro-timeouts"); + .contains(LRO_TROUBLESHOOTING_LINK); int attemptCount = operationFuture.getPollingFuture().getAttemptSettings().getAttemptCount(); assertThat(attemptCount).isGreaterThan(1); } finally { From 383b42dc27cab0589cf5eef1af8d2fe8a05fc58a Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 10 Nov 2023 18:48:53 +0000 Subject: [PATCH 07/22] reset test and basic retry future --- .../api/gax/retrying/BasicRetryingFuture.java | 17 ----------------- .../v1beta1/it/ITLongRunningOperation.java | 13 ++----------- 2 files changed, 2 insertions(+), 28 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/retrying/BasicRetryingFuture.java b/gax-java/gax/src/main/java/com/google/api/gax/retrying/BasicRetryingFuture.java index 303ba42c0d..de7b5b5acb 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/retrying/BasicRetryingFuture.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/retrying/BasicRetryingFuture.java @@ -67,8 +67,6 @@ class BasicRetryingFuture extends AbstractFuture private volatile ApiFuture latestCompletedAttemptResult; private volatile ApiFuture attemptResult; - private volatile CancellationException customCancellationException = null; - private static final Logger LOG = Logger.getLogger(BasicRetryingFuture.class.getName()); BasicRetryingFuture( @@ -207,9 +205,6 @@ void handleAttempt(Throwable throwable, ResponseT response) { } catch (CancellationException e) { // A retry algorithm triggered cancellation. tracer.attemptFailedRetriesExhausted(e); - // The exception caught here is coming from gax code. We save it in order to throw - // this exception as the main one, while keeping guava's `Task was cancelled.` as a cause - customCancellationException = e; super.cancel(false); } catch (Exception e) { // Should never happen, but still possible in case of buggy retry algorithm implementation. @@ -269,18 +264,6 @@ private void setAttemptResult(Throwable throwable, ResponseT response, boolean s } } - public ResponseT get() throws InterruptedException, ExecutionException{ - try { - return super.get(); - } catch (CancellationException ex) { - if (customCancellationException instanceof CancellationException) { - customCancellationException.initCause(ex); - throw customCancellationException; - } - throw ex; - } - } - private class CompletionListener implements Runnable { @Override public void run() { diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java index 47d38095d6..5f1e29963d 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java @@ -15,7 +15,6 @@ */ package com.google.showcase.v1beta1.it; -import static com.google.api.gax.longrunning.OperationTimedPollAlgorithm.LRO_TROUBLESHOOTING_LINK; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertThrows; @@ -147,11 +146,7 @@ public void testGRPC_LROUnsuccessfulResponse_exceedsTotalTimeout_throwsDeadlineE .build(); OperationFuture operationFuture = grpcClient.waitOperationCallable().futureCall(waitRequest); - CancellationException cancellationException = - assertThrows(CancellationException.class, operationFuture::get); - assertThat(cancellationException) - .hasMessageThat() - .contains(LRO_TROUBLESHOOTING_LINK); + assertThrows(CancellationException.class, operationFuture::get); int attemptCount = operationFuture.getPollingFuture().getAttemptSettings().getAttemptCount(); assertThat(attemptCount).isGreaterThan(1); } finally { @@ -190,11 +185,7 @@ public void testGRPC_LROUnsuccessfulResponse_exceedsTotalTimeout_throwsDeadlineE .build(); OperationFuture operationFuture = httpjsonClient.waitOperationCallable().futureCall(waitRequest); - CancellationException cancellationException = - assertThrows(CancellationException.class, operationFuture::get); - assertThat(cancellationException) - .hasMessageThat() - .contains(LRO_TROUBLESHOOTING_LINK); + assertThrows(CancellationException.class, operationFuture::get); int attemptCount = operationFuture.getPollingFuture().getAttemptSettings().getAttemptCount(); assertThat(attemptCount).isGreaterThan(1); } finally { From a9245bab3aa87c5e3a1d1c264067cddce3159d5d Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 10 Nov 2023 19:52:05 +0000 Subject: [PATCH 08/22] use logging to inform of failed LRO --- .../google/api/gax/grpc/ChannelPoolTest.java | 19 +------------- gax-java/gax/pom.xml | 1 + .../OperationTimedPollAlgorithm.java | 13 +++++++--- .../google/api/gax/util/FakeLogHandler.java | 26 +++++++++++++++++++ .../v1beta1/it/ITLongRunningOperation.java | 14 ++++++++++ 5 files changed, 52 insertions(+), 21 deletions(-) create mode 100644 gax-java/gax/src/test/java/com/google/api/gax/util/FakeLogHandler.java diff --git a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/ChannelPoolTest.java b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/ChannelPoolTest.java index 173528d6e2..6bfabc626c 100644 --- a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/ChannelPoolTest.java +++ b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/ChannelPoolTest.java @@ -43,6 +43,7 @@ import com.google.api.gax.rpc.StreamController; import com.google.api.gax.rpc.UnaryCallSettings; import com.google.api.gax.rpc.UnaryCallable; +import com.google.api.gax.util.FakeLogHandler; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; @@ -718,22 +719,4 @@ public void testDoubleRelease() throws Exception { } } - private static class FakeLogHandler extends Handler { - List records = new ArrayList<>(); - - @Override - public void publish(LogRecord record) { - records.add(record); - } - - @Override - public void flush() {} - - @Override - public void close() throws SecurityException {} - - List getAllMessages() { - return records.stream().map(LogRecord::getMessage).collect(Collectors.toList()); - } - } } diff --git a/gax-java/gax/pom.xml b/gax-java/gax/pom.xml index d60a739519..a62ab75dbc 100644 --- a/gax-java/gax/pom.xml +++ b/gax-java/gax/pom.xml @@ -76,6 +76,7 @@ com/google/api/gax/core/** com/google/api/gax/rpc/testing/** com/google/api/gax/rpc/mtls/** + com/google/api/gax/util/** diff --git a/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java b/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java index 25072d4144..233551ce44 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java @@ -35,7 +35,10 @@ import com.google.api.gax.retrying.ExponentialRetryAlgorithm; import com.google.api.gax.retrying.RetrySettings; import com.google.api.gax.retrying.TimedAttemptSettings; +import com.google.common.annotations.VisibleForTesting; import java.util.concurrent.CancellationException; +import java.util.logging.Level; +import java.util.logging.Logger; /** * Operation timed polling algorithm, which uses exponential backoff factor for determining when the @@ -44,6 +47,8 @@ */ public class OperationTimedPollAlgorithm extends ExponentialRetryAlgorithm { + @VisibleForTesting + public static Logger LOGGER = Logger.getLogger(OperationTimedPollAlgorithm.class.getName()); public static final String LRO_TROUBLESHOOTING_LINK = "https://github.com/googleapis/google-cloud-java#lro-timeouts"; @@ -81,9 +86,11 @@ public boolean shouldRetry(TimedAttemptSettings nextAttemptSettings) if (super.shouldRetry(nextAttemptSettings)) { return true; } - throw new CancellationException( - "The task has been cancelled. Please refer to " - + LRO_TROUBLESHOOTING_LINK + " for more information"); + if (LOGGER.isLoggable(Level.SEVERE)) { + LOGGER.log(Level.SEVERE, "The task has been cancelled. Please refer to " + + LRO_TROUBLESHOOTING_LINK + " for more information"); + } + throw new CancellationException(); } // Note: if the potential time spent is exactly equal to the totalTimeout, diff --git a/gax-java/gax/src/test/java/com/google/api/gax/util/FakeLogHandler.java b/gax-java/gax/src/test/java/com/google/api/gax/util/FakeLogHandler.java new file mode 100644 index 0000000000..ad63c8e6fc --- /dev/null +++ b/gax-java/gax/src/test/java/com/google/api/gax/util/FakeLogHandler.java @@ -0,0 +1,26 @@ +package com.google.api.gax.util; + +import java.util.ArrayList; +import java.util.List; +import java.util.logging.Handler; +import java.util.logging.LogRecord; +import java.util.stream.Collectors; + +public class FakeLogHandler extends Handler { + List records = new ArrayList<>(); + + @Override + public void publish(LogRecord record) { + records.add(record); + } + + @Override + public void flush() {} + + @Override + public void close() throws SecurityException {} + + public List getAllMessages() { + return records.stream().map(LogRecord::getMessage).collect(Collectors.toList()); + } +} diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java index 5f1e29963d..71ab42c356 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java @@ -16,16 +16,20 @@ package com.google.showcase.v1beta1.it; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; import com.google.api.gax.longrunning.OperationFuture; +import com.google.api.gax.longrunning.OperationTimedPollAlgorithm; import com.google.api.gax.retrying.RetrySettings; +import com.google.api.gax.util.FakeLogHandler; import com.google.protobuf.Timestamp; import com.google.showcase.v1beta1.EchoClient; import com.google.showcase.v1beta1.WaitMetadata; import com.google.showcase.v1beta1.WaitRequest; import com.google.showcase.v1beta1.WaitResponse; import com.google.showcase.v1beta1.it.util.TestClientInitializer; +import com.google.showcase.v1beta1.stub.EchoStubSettings; import java.util.concurrent.CancellationException; import java.util.concurrent.TimeUnit; import org.junit.Test; @@ -121,6 +125,8 @@ public void testHttpJson_LROSuccessfulResponse_doesNotExceedTotalTimeout() throw @Test public void testGRPC_LROUnsuccessfulResponse_exceedsTotalTimeout_throwsDeadlineExceededException() throws Exception { + FakeLogHandler logHandler = new FakeLogHandler(); + OperationTimedPollAlgorithm.LOGGER.addHandler(logHandler); RetrySettings initialUnaryRetrySettings = RetrySettings.newBuilder() .setInitialRpcTimeout(Duration.ofMillis(5000L)) @@ -149,6 +155,9 @@ public void testGRPC_LROUnsuccessfulResponse_exceedsTotalTimeout_throwsDeadlineE assertThrows(CancellationException.class, operationFuture::get); int attemptCount = operationFuture.getPollingFuture().getAttemptSettings().getAttemptCount(); assertThat(attemptCount).isGreaterThan(1); + assertEquals(1, logHandler.getAllMessages().size()); + assertThat(logHandler.getAllMessages().get(0)) + .contains(OperationTimedPollAlgorithm.LRO_TROUBLESHOOTING_LINK); } finally { grpcClient.close(); grpcClient.awaitTermination( @@ -160,6 +169,8 @@ public void testGRPC_LROUnsuccessfulResponse_exceedsTotalTimeout_throwsDeadlineE public void testHttpJson_LROUnsuccessfulResponse_exceedsTotalTimeout_throwsDeadlineExceededException() throws Exception { + FakeLogHandler logHandler = new FakeLogHandler(); + OperationTimedPollAlgorithm.LOGGER.addHandler(logHandler); RetrySettings initialUnaryRetrySettings = RetrySettings.newBuilder() .setInitialRpcTimeout(Duration.ofMillis(5000L)) @@ -188,6 +199,9 @@ public void testGRPC_LROUnsuccessfulResponse_exceedsTotalTimeout_throwsDeadlineE assertThrows(CancellationException.class, operationFuture::get); int attemptCount = operationFuture.getPollingFuture().getAttemptSettings().getAttemptCount(); assertThat(attemptCount).isGreaterThan(1); + assertEquals(1, logHandler.getAllMessages().size()); + assertThat(logHandler.getAllMessages().get(0)) + .contains(OperationTimedPollAlgorithm.LRO_TROUBLESHOOTING_LINK); } finally { httpjsonClient.close(); httpjsonClient.awaitTermination( From fa774a2fae3cdcf96ccdb569e5bfc4fdbef14eb2 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 10 Nov 2023 20:17:43 +0000 Subject: [PATCH 09/22] add license and comment to fake log handler --- .../google/api/gax/util/FakeLogHandler.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/gax-java/gax/src/test/java/com/google/api/gax/util/FakeLogHandler.java b/gax-java/gax/src/test/java/com/google/api/gax/util/FakeLogHandler.java index ad63c8e6fc..db4cf54c67 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/util/FakeLogHandler.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/util/FakeLogHandler.java @@ -1,3 +1,18 @@ +/* + * 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 + * + * https://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.api.gax.util; import java.util.ArrayList; @@ -6,6 +21,10 @@ import java.util.logging.LogRecord; import java.util.stream.Collectors; +/* + * Convenience class that stores the log entries produced by any logger + * It can then be inspected - its entries are a list log records + */ public class FakeLogHandler extends Handler { List records = new ArrayList<>(); From f245e1f2e12e91cacdb16c24141f996fab87a4c7 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 10 Nov 2023 20:21:26 +0000 Subject: [PATCH 10/22] format code --- .../java/com/google/api/gax/grpc/ChannelPoolTest.java | 4 ---- .../api/gax/longrunning/OperationTimedPollAlgorithm.java | 8 ++++++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/ChannelPoolTest.java b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/ChannelPoolTest.java index 6bfabc626c..7131873333 100644 --- a/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/ChannelPoolTest.java +++ b/gax-java/gax-grpc/src/test/java/com/google/api/gax/grpc/ChannelPoolTest.java @@ -67,9 +67,6 @@ import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; -import java.util.logging.Handler; -import java.util.logging.LogRecord; -import java.util.stream.Collectors; import org.junit.After; import org.junit.Assert; import org.junit.Test; @@ -718,5 +715,4 @@ public void testDoubleRelease() throws Exception { ChannelPool.LOG.removeHandler(logHandler); } } - } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java b/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java index 233551ce44..cbc137a3bc 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java @@ -49,6 +49,7 @@ public class OperationTimedPollAlgorithm extends ExponentialRetryAlgorithm { @VisibleForTesting public static Logger LOGGER = Logger.getLogger(OperationTimedPollAlgorithm.class.getName()); + public static final String LRO_TROUBLESHOOTING_LINK = "https://github.com/googleapis/google-cloud-java#lro-timeouts"; @@ -87,8 +88,11 @@ public boolean shouldRetry(TimedAttemptSettings nextAttemptSettings) return true; } if (LOGGER.isLoggable(Level.SEVERE)) { - LOGGER.log(Level.SEVERE, "The task has been cancelled. Please refer to " - + LRO_TROUBLESHOOTING_LINK + " for more information"); + LOGGER.log( + Level.SEVERE, + "The task has been cancelled. Please refer to " + + LRO_TROUBLESHOOTING_LINK + + " for more information"); } throw new CancellationException(); } From f4fd1c00f5a95e44f4fc4f22da4a2752bf131f7f Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 10 Nov 2023 20:30:23 +0000 Subject: [PATCH 11/22] fix license --- .../google/api/gax/util/FakeLogHandler.java | 34 +++++++++++++------ 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/gax-java/gax/src/test/java/com/google/api/gax/util/FakeLogHandler.java b/gax-java/gax/src/test/java/com/google/api/gax/util/FakeLogHandler.java index db4cf54c67..9e5dbaf3f6 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/util/FakeLogHandler.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/util/FakeLogHandler.java @@ -1,17 +1,31 @@ /* - * Copyright 2023 Google LLC + * Copyright 2017 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 + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: * - * https://www.apache.org/licenses/LICENSE-2.0 + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google LLC nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. * - * 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. + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ package com.google.api.gax.util; From a899a897dd878b47f5a1f07d075da8b76756c6cb Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Fri, 10 Nov 2023 20:41:19 +0000 Subject: [PATCH 12/22] format souce --- .../com/google/showcase/v1beta1/it/ITLongRunningOperation.java | 1 - 1 file changed, 1 deletion(-) diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java index 71ab42c356..d533de42ff 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java @@ -29,7 +29,6 @@ import com.google.showcase.v1beta1.WaitRequest; import com.google.showcase.v1beta1.WaitResponse; import com.google.showcase.v1beta1.it.util.TestClientInitializer; -import com.google.showcase.v1beta1.stub.EchoStubSettings; import java.util.concurrent.CancellationException; import java.util.concurrent.TimeUnit; import org.junit.Test; From e28c03fc4de3baaf6b252751783f9c15a3839bb0 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Mon, 13 Nov 2023 16:50:14 +0000 Subject: [PATCH 13/22] decrease LRO timeout logging level to WARNING --- .../google/api/gax/longrunning/OperationTimedPollAlgorithm.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java b/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java index cbc137a3bc..3a9f0514de 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java @@ -89,7 +89,7 @@ public boolean shouldRetry(TimedAttemptSettings nextAttemptSettings) } if (LOGGER.isLoggable(Level.SEVERE)) { LOGGER.log( - Level.SEVERE, + Level.WARNING, "The task has been cancelled. Please refer to " + LRO_TROUBLESHOOTING_LINK + " for more information"); From 2328c014dd93ea0c439cf2bacb4c0ae2dd413276 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Mon, 13 Nov 2023 16:51:24 +0000 Subject: [PATCH 14/22] fix logging threshold check --- .../google/api/gax/longrunning/OperationTimedPollAlgorithm.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java b/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java index 3a9f0514de..5aefd3f45c 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java @@ -87,7 +87,7 @@ public boolean shouldRetry(TimedAttemptSettings nextAttemptSettings) if (super.shouldRetry(nextAttemptSettings)) { return true; } - if (LOGGER.isLoggable(Level.SEVERE)) { + if (LOGGER.isLoggable(Level.WARNING)) { LOGGER.log( Level.WARNING, "The task has been cancelled. Please refer to " From 7f0a751816245ee5ee53984d98bce27bb5c7cfc5 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Mon, 13 Nov 2023 18:36:27 +0000 Subject: [PATCH 15/22] use common log handler setup for ITLongRunningOperation --- .../gax/longrunning/OperationTimedPollAlgorithm.java | 1 - .../showcase/v1beta1/it/ITLongRunningOperation.java | 10 ++++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java b/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java index 5aefd3f45c..593c5df369 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java @@ -47,7 +47,6 @@ */ public class OperationTimedPollAlgorithm extends ExponentialRetryAlgorithm { - @VisibleForTesting public static Logger LOGGER = Logger.getLogger(OperationTimedPollAlgorithm.class.getName()); public static final String LRO_TROUBLESHOOTING_LINK = diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java index d533de42ff..aad85e0cc4 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java @@ -31,6 +31,7 @@ import com.google.showcase.v1beta1.it.util.TestClientInitializer; import java.util.concurrent.CancellationException; import java.util.concurrent.TimeUnit; +import org.junit.Before; import org.junit.Test; import org.threeten.bp.Duration; import org.threeten.bp.Instant; @@ -42,6 +43,13 @@ */ public class ITLongRunningOperation { + FakeLogHandler logHandler; + + @Before + public void setUp() { + logHandler = new FakeLogHandler(); + } + @Test public void testGRPC_LROSuccessfulResponse_doesNotExceedTotalTimeout() throws Exception { RetrySettings initialUnaryRetrySettings = @@ -124,7 +132,6 @@ public void testHttpJson_LROSuccessfulResponse_doesNotExceedTotalTimeout() throw @Test public void testGRPC_LROUnsuccessfulResponse_exceedsTotalTimeout_throwsDeadlineExceededException() throws Exception { - FakeLogHandler logHandler = new FakeLogHandler(); OperationTimedPollAlgorithm.LOGGER.addHandler(logHandler); RetrySettings initialUnaryRetrySettings = RetrySettings.newBuilder() @@ -168,7 +175,6 @@ public void testGRPC_LROUnsuccessfulResponse_exceedsTotalTimeout_throwsDeadlineE public void testHttpJson_LROUnsuccessfulResponse_exceedsTotalTimeout_throwsDeadlineExceededException() throws Exception { - FakeLogHandler logHandler = new FakeLogHandler(); OperationTimedPollAlgorithm.LOGGER.addHandler(logHandler); RetrySettings initialUnaryRetrySettings = RetrySettings.newBuilder() From e603ba7c3fd425ecf90147cb5c747f98e5ac1dfc Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Mon, 13 Nov 2023 20:57:51 +0000 Subject: [PATCH 16/22] fix license header year --- .../src/test/java/com/google/api/gax/util/FakeLogHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gax-java/gax/src/test/java/com/google/api/gax/util/FakeLogHandler.java b/gax-java/gax/src/test/java/com/google/api/gax/util/FakeLogHandler.java index 9e5dbaf3f6..f6dbef78ea 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/util/FakeLogHandler.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/util/FakeLogHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 Google LLC + * Copyright 2023 Google LLC * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are From b8ad02782968f8340fab41cf6d9361e2068b6e47 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Mon, 13 Nov 2023 21:08:10 +0000 Subject: [PATCH 17/22] format source --- .../google/api/gax/longrunning/OperationTimedPollAlgorithm.java | 1 - 1 file changed, 1 deletion(-) diff --git a/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java b/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java index 593c5df369..ab08239ce9 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java @@ -35,7 +35,6 @@ import com.google.api.gax.retrying.ExponentialRetryAlgorithm; import com.google.api.gax.retrying.RetrySettings; import com.google.api.gax.retrying.TimedAttemptSettings; -import com.google.common.annotations.VisibleForTesting; import java.util.concurrent.CancellationException; import java.util.logging.Level; import java.util.logging.Logger; From 8f6092599d234cd5b9b7db1a5f4bab0444a3e763 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Mon, 13 Nov 2023 22:40:56 +0000 Subject: [PATCH 18/22] reset lro it --- .../v1beta1/it/ITLongRunningOperation.java | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java index aad85e0cc4..5f1e29963d 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITLongRunningOperation.java @@ -16,13 +16,10 @@ package com.google.showcase.v1beta1.it; import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; import com.google.api.gax.longrunning.OperationFuture; -import com.google.api.gax.longrunning.OperationTimedPollAlgorithm; import com.google.api.gax.retrying.RetrySettings; -import com.google.api.gax.util.FakeLogHandler; import com.google.protobuf.Timestamp; import com.google.showcase.v1beta1.EchoClient; import com.google.showcase.v1beta1.WaitMetadata; @@ -31,7 +28,6 @@ import com.google.showcase.v1beta1.it.util.TestClientInitializer; import java.util.concurrent.CancellationException; import java.util.concurrent.TimeUnit; -import org.junit.Before; import org.junit.Test; import org.threeten.bp.Duration; import org.threeten.bp.Instant; @@ -43,13 +39,6 @@ */ public class ITLongRunningOperation { - FakeLogHandler logHandler; - - @Before - public void setUp() { - logHandler = new FakeLogHandler(); - } - @Test public void testGRPC_LROSuccessfulResponse_doesNotExceedTotalTimeout() throws Exception { RetrySettings initialUnaryRetrySettings = @@ -132,7 +121,6 @@ public void testHttpJson_LROSuccessfulResponse_doesNotExceedTotalTimeout() throw @Test public void testGRPC_LROUnsuccessfulResponse_exceedsTotalTimeout_throwsDeadlineExceededException() throws Exception { - OperationTimedPollAlgorithm.LOGGER.addHandler(logHandler); RetrySettings initialUnaryRetrySettings = RetrySettings.newBuilder() .setInitialRpcTimeout(Duration.ofMillis(5000L)) @@ -161,9 +149,6 @@ public void testGRPC_LROUnsuccessfulResponse_exceedsTotalTimeout_throwsDeadlineE assertThrows(CancellationException.class, operationFuture::get); int attemptCount = operationFuture.getPollingFuture().getAttemptSettings().getAttemptCount(); assertThat(attemptCount).isGreaterThan(1); - assertEquals(1, logHandler.getAllMessages().size()); - assertThat(logHandler.getAllMessages().get(0)) - .contains(OperationTimedPollAlgorithm.LRO_TROUBLESHOOTING_LINK); } finally { grpcClient.close(); grpcClient.awaitTermination( @@ -175,7 +160,6 @@ public void testGRPC_LROUnsuccessfulResponse_exceedsTotalTimeout_throwsDeadlineE public void testHttpJson_LROUnsuccessfulResponse_exceedsTotalTimeout_throwsDeadlineExceededException() throws Exception { - OperationTimedPollAlgorithm.LOGGER.addHandler(logHandler); RetrySettings initialUnaryRetrySettings = RetrySettings.newBuilder() .setInitialRpcTimeout(Duration.ofMillis(5000L)) @@ -204,9 +188,6 @@ public void testGRPC_LROUnsuccessfulResponse_exceedsTotalTimeout_throwsDeadlineE assertThrows(CancellationException.class, operationFuture::get); int attemptCount = operationFuture.getPollingFuture().getAttemptSettings().getAttemptCount(); assertThat(attemptCount).isGreaterThan(1); - assertEquals(1, logHandler.getAllMessages().size()); - assertThat(logHandler.getAllMessages().get(0)) - .contains(OperationTimedPollAlgorithm.LRO_TROUBLESHOOTING_LINK); } finally { httpjsonClient.close(); httpjsonClient.awaitTermination( From 0e5355917167dba22be9da2f48b377e02ba66dc6 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Mon, 13 Nov 2023 23:07:34 +0000 Subject: [PATCH 19/22] change to unit tests --- .../OperationTimedPollAlgorithm.java | 7 +- .../OperationTimedPollAlgorithmTest.java | 122 ++++++++++++++++++ 2 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 gax-java/gax/src/test/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithmTest.java diff --git a/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java b/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java index ab08239ce9..0b2f225dfd 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithm.java @@ -35,6 +35,7 @@ import com.google.api.gax.retrying.ExponentialRetryAlgorithm; import com.google.api.gax.retrying.RetrySettings; import com.google.api.gax.retrying.TimedAttemptSettings; +import com.google.common.annotations.VisibleForTesting; import java.util.concurrent.CancellationException; import java.util.logging.Level; import java.util.logging.Logger; @@ -46,9 +47,11 @@ */ public class OperationTimedPollAlgorithm extends ExponentialRetryAlgorithm { - public static Logger LOGGER = Logger.getLogger(OperationTimedPollAlgorithm.class.getName()); + @VisibleForTesting + static final Logger LOGGER = Logger.getLogger(OperationTimedPollAlgorithm.class.getName()); - public static final String LRO_TROUBLESHOOTING_LINK = + @VisibleForTesting + static final String LRO_TROUBLESHOOTING_LINK = "https://github.com/googleapis/google-cloud-java#lro-timeouts"; /** diff --git a/gax-java/gax/src/test/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithmTest.java b/gax-java/gax/src/test/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithmTest.java new file mode 100644 index 0000000000..103dc63bbb --- /dev/null +++ b/gax-java/gax/src/test/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithmTest.java @@ -0,0 +1,122 @@ +/* + * Copyright 2023 Google LLC + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google LLC nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package com.google.api.gax.longrunning; + +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import com.google.api.gax.core.FakeApiClock; +import com.google.api.gax.retrying.RetrySettings; +import com.google.api.gax.retrying.TimedAttemptSettings; +import com.google.api.gax.util.FakeLogHandler; +import java.util.concurrent.CancellationException; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.threeten.bp.Duration; + +public class OperationTimedPollAlgorithmTest { + + private static final RetrySettings FAST_RETRY_SETTINGS = + RetrySettings.newBuilder() + .setInitialRetryDelay(Duration.ofMillis(1L)) + .setRetryDelayMultiplier(1) + .setMaxRetryDelay(Duration.ofMillis(1L)) + .setInitialRpcTimeout(Duration.ofMillis(1L)) + .setMaxAttempts(0) + .setJittered(false) + .setRpcTimeoutMultiplier(1) + .setMaxRpcTimeout(Duration.ofMillis(1L)) + .setTotalTimeout(Duration.ofMillis(5L)) + .build(); + + private FakeLogHandler logHandler; + + @Before + public void setUp() { + logHandler = new FakeLogHandler(); + OperationTimedPollAlgorithm.LOGGER.addHandler(logHandler); + } + + @After + public void tearDown() { + OperationTimedPollAlgorithm.LOGGER.removeHandler(logHandler); + // redundant null assignment for readability - a new log handler will be used + logHandler = null; + } + + @Test + public void testAlgorithmThatShouldRetry_doesNotLogTimeoutHelpMessage() { + FakeApiClock clock = new FakeApiClock(System.nanoTime()); + OperationTimedPollAlgorithm algorithm = + OperationTimedPollAlgorithm.create(FAST_RETRY_SETTINGS, clock); + TimedAttemptSettings settings = + TimedAttemptSettings.newBuilder() + .setGlobalSettings(FAST_RETRY_SETTINGS) + .setRetryDelay(Duration.ofMillis(1l)) + .setRpcTimeout(Duration.ofMillis(1l)) + .setRandomizedRetryDelay(Duration.ofMillis(1l)) + .setAttemptCount(0) + .setFirstAttemptStartTimeNanos(clock.nanoTime()) + .build(); + try { + algorithm.shouldRetry(settings); + } catch (CancellationException ex) { + fail("Unexpected unsuccessful shouldRetry()"); + } + assertTrue( + logHandler.getAllMessages().stream() + .noneMatch( + entry -> entry.contains(OperationTimedPollAlgorithm.LRO_TROUBLESHOOTING_LINK))); + } + + @Test + public void testAlgorithmThatShouldNotRetry_logsTimeoutHelpMessage() { + FakeApiClock clock = new FakeApiClock(System.nanoTime()); + OperationTimedPollAlgorithm algorithm = + OperationTimedPollAlgorithm.create(FAST_RETRY_SETTINGS, clock); + TimedAttemptSettings settings = + TimedAttemptSettings.newBuilder() + .setGlobalSettings(FAST_RETRY_SETTINGS) + .setRetryDelay(Duration.ofMillis(1l)) + .setRpcTimeout(Duration.ofMillis(1l)) + .setRandomizedRetryDelay(Duration.ofMillis(1l)) + .setAttemptCount(0) + .setFirstAttemptStartTimeNanos(clock.nanoTime()) + .build(); + clock.incrementNanoTime(1 * 1000 * 1000 * 1000); + assertThrows(CancellationException.class, () -> algorithm.shouldRetry(settings)); + assertTrue( + logHandler.getAllMessages().stream() + .anyMatch( + entry -> entry.contains(OperationTimedPollAlgorithm.LRO_TROUBLESHOOTING_LINK))); + } +} From 2c335156ea7ee9a7b470fcfb9c274b74c55bafa3 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Mon, 13 Nov 2023 23:11:45 +0000 Subject: [PATCH 20/22] reset testjar exports --- gax-java/gax/pom.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/gax-java/gax/pom.xml b/gax-java/gax/pom.xml index a62ab75dbc..d60a739519 100644 --- a/gax-java/gax/pom.xml +++ b/gax-java/gax/pom.xml @@ -76,7 +76,6 @@ com/google/api/gax/core/** com/google/api/gax/rpc/testing/** com/google/api/gax/rpc/mtls/** - com/google/api/gax/util/** From 152fc5b8d431c67edf50e3137b253b2dd2309293 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Tue, 14 Nov 2023 16:18:30 +0000 Subject: [PATCH 21/22] simplify unit test --- gax-java/gax/pom.xml | 1 + .../OperationTimedPollAlgorithmTest.java | 38 ++++++++----------- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/gax-java/gax/pom.xml b/gax-java/gax/pom.xml index d60a739519..a62ab75dbc 100644 --- a/gax-java/gax/pom.xml +++ b/gax-java/gax/pom.xml @@ -76,6 +76,7 @@ com/google/api/gax/core/** com/google/api/gax/rpc/testing/** com/google/api/gax/rpc/mtls/** + com/google/api/gax/util/** diff --git a/gax-java/gax/src/test/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithmTest.java b/gax-java/gax/src/test/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithmTest.java index 103dc63bbb..7c9217d29c 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithmTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithmTest.java @@ -57,6 +57,8 @@ public class OperationTimedPollAlgorithmTest { .setMaxRpcTimeout(Duration.ofMillis(1L)) .setTotalTimeout(Duration.ofMillis(5L)) .build(); + private static TimedAttemptSettings timedAttemptSettings; + private static FakeApiClock clock; private FakeLogHandler logHandler; @@ -64,6 +66,16 @@ public class OperationTimedPollAlgorithmTest { public void setUp() { logHandler = new FakeLogHandler(); OperationTimedPollAlgorithm.LOGGER.addHandler(logHandler); + clock = new FakeApiClock(System.nanoTime()); + timedAttemptSettings = + TimedAttemptSettings.newBuilder() + .setGlobalSettings(FAST_RETRY_SETTINGS) + .setRetryDelay(Duration.ofMillis(1l)) + .setRpcTimeout(Duration.ofMillis(1l)) + .setRandomizedRetryDelay(Duration.ofMillis(1l)) + .setAttemptCount(0) + .setFirstAttemptStartTimeNanos(clock.nanoTime()) + .build(); } @After @@ -75,20 +87,10 @@ public void tearDown() { @Test public void testAlgorithmThatShouldRetry_doesNotLogTimeoutHelpMessage() { - FakeApiClock clock = new FakeApiClock(System.nanoTime()); OperationTimedPollAlgorithm algorithm = OperationTimedPollAlgorithm.create(FAST_RETRY_SETTINGS, clock); - TimedAttemptSettings settings = - TimedAttemptSettings.newBuilder() - .setGlobalSettings(FAST_RETRY_SETTINGS) - .setRetryDelay(Duration.ofMillis(1l)) - .setRpcTimeout(Duration.ofMillis(1l)) - .setRandomizedRetryDelay(Duration.ofMillis(1l)) - .setAttemptCount(0) - .setFirstAttemptStartTimeNanos(clock.nanoTime()) - .build(); try { - algorithm.shouldRetry(settings); + algorithm.shouldRetry(timedAttemptSettings); } catch (CancellationException ex) { fail("Unexpected unsuccessful shouldRetry()"); } @@ -100,20 +102,10 @@ public void testAlgorithmThatShouldRetry_doesNotLogTimeoutHelpMessage() { @Test public void testAlgorithmThatShouldNotRetry_logsTimeoutHelpMessage() { - FakeApiClock clock = new FakeApiClock(System.nanoTime()); OperationTimedPollAlgorithm algorithm = OperationTimedPollAlgorithm.create(FAST_RETRY_SETTINGS, clock); - TimedAttemptSettings settings = - TimedAttemptSettings.newBuilder() - .setGlobalSettings(FAST_RETRY_SETTINGS) - .setRetryDelay(Duration.ofMillis(1l)) - .setRpcTimeout(Duration.ofMillis(1l)) - .setRandomizedRetryDelay(Duration.ofMillis(1l)) - .setAttemptCount(0) - .setFirstAttemptStartTimeNanos(clock.nanoTime()) - .build(); - clock.incrementNanoTime(1 * 1000 * 1000 * 1000); - assertThrows(CancellationException.class, () -> algorithm.shouldRetry(settings)); + clock.incrementNanoTime(1 * 1000 * 1000 * 1000); // force rpc timeout + assertThrows(CancellationException.class, () -> algorithm.shouldRetry(timedAttemptSettings)); assertTrue( logHandler.getAllMessages().stream() .anyMatch( From 81a978c1acbefd1a8188325c1a7560b4faceaa04 Mon Sep 17 00:00:00 2001 From: diegomarquezp Date: Tue, 14 Nov 2023 16:45:37 +0000 Subject: [PATCH 22/22] use instance variables in test --- .../api/gax/longrunning/OperationTimedPollAlgorithmTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gax-java/gax/src/test/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithmTest.java b/gax-java/gax/src/test/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithmTest.java index 7c9217d29c..583afcc819 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithmTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/longrunning/OperationTimedPollAlgorithmTest.java @@ -57,8 +57,8 @@ public class OperationTimedPollAlgorithmTest { .setMaxRpcTimeout(Duration.ofMillis(1L)) .setTotalTimeout(Duration.ofMillis(5L)) .build(); - private static TimedAttemptSettings timedAttemptSettings; - private static FakeApiClock clock; + private TimedAttemptSettings timedAttemptSettings; + private FakeApiClock clock; private FakeLogHandler logHandler;