From e968c36a8c148cefe99067f08659600309c45188 Mon Sep 17 00:00:00 2001 From: Nick Birnie Date: Mon, 5 Oct 2020 17:00:21 +0100 Subject: [PATCH 1/3] Transform exceptions on server streaming calls. Currently we do not generate GoogleAdsException instances for server streaming callables (see #332) Change-Id: Ia8339f1ee482ab7f02085f5c47f90ab1cc0aee5b --- google-ads/pom.xml | 4 +- .../lib/GrpcGoogleAdsCallableFactory.java | 52 +++-- .../callables/ExceptionTransformation.java | 26 +++ ...onTransformingServerStreamingCallable.java | 80 ++++++++ .../ExceptionTransformingUnaryCallable.java} | 19 +- .../GoogleAdsExceptionTransformation.java | 22 ++- .../GoogleAdsExceptionTransformationTest.java | 1 + .../lib/callables/ExceptionCase.java | 179 ++++++++++++++++++ ...ansformingServerStreamingCallableTest.java | 93 +++++++++ ...xceptionTransformingUnaryCallableTest.java | 84 ++++++++ 10 files changed, 526 insertions(+), 34 deletions(-) create mode 100644 google-ads/src/main/java/com/google/ads/googleads/lib/callables/ExceptionTransformation.java create mode 100644 google-ads/src/main/java/com/google/ads/googleads/lib/callables/ExceptionTransformingServerStreamingCallable.java rename google-ads/src/main/java/com/google/ads/googleads/lib/{ExceptionTransformingCallable.java => callables/ExceptionTransformingUnaryCallable.java} (83%) rename google-ads/src/main/java/com/google/ads/googleads/lib/{ => callables}/GoogleAdsExceptionTransformation.java (62%) create mode 100644 google-ads/src/test/java/com/google/ads/googleads/lib/callables/ExceptionCase.java create mode 100644 google-ads/src/test/java/com/google/ads/googleads/lib/callables/ExceptionTransformingServerStreamingCallableTest.java create mode 100644 google-ads/src/test/java/com/google/ads/googleads/lib/callables/ExceptionTransformingUnaryCallableTest.java diff --git a/google-ads/pom.xml b/google-ads/pom.xml index 2f6be1c041..f87a405f2f 100644 --- a/google-ads/pom.xml +++ b/google-ads/pom.xml @@ -92,8 +92,8 @@ org.mockito - mockito-all - 1.9.5 + mockito-core + 3.1.0 test diff --git a/google-ads/src/main/java/com/google/ads/googleads/lib/GrpcGoogleAdsCallableFactory.java b/google-ads/src/main/java/com/google/ads/googleads/lib/GrpcGoogleAdsCallableFactory.java index ced2c9cbc7..4aa4b6f2c4 100644 --- a/google-ads/src/main/java/com/google/ads/googleads/lib/GrpcGoogleAdsCallableFactory.java +++ b/google-ads/src/main/java/com/google/ads/googleads/lib/GrpcGoogleAdsCallableFactory.java @@ -15,9 +15,10 @@ */ package com.google.ads.googleads.lib; -import com.google.ads.googleads.lib.ExceptionTransformingCallable; -import com.google.ads.googleads.lib.ExceptionTransformingCallable.ExceptionTransformation; -import com.google.ads.googleads.lib.GoogleAdsExceptionTransformation; +import com.google.ads.googleads.lib.callables.ExceptionTransformation; +import com.google.ads.googleads.lib.callables.ExceptionTransformingServerStreamingCallable; +import com.google.ads.googleads.lib.callables.ExceptionTransformingUnaryCallable; +import com.google.ads.googleads.lib.callables.GoogleAdsExceptionTransformation; import com.google.api.gax.grpc.GrpcCallSettings; import com.google.api.gax.grpc.GrpcCallableFactory; import com.google.api.gax.grpc.GrpcStubCallableFactory; @@ -37,42 +38,61 @@ import com.google.longrunning.Operation; import com.google.longrunning.stub.OperationsStub; +/** + * Defines the factory used to create instances for all Google Ads services. + * + *

Used in place of the default generated code to override the exceptions generated to throw + * GoogleAdsException instead of ApiException. + */ public class GrpcGoogleAdsCallableFactory implements GrpcStubCallableFactory { - private static final ExceptionTransformation googleAdsExceptionTransformation = new GoogleAdsExceptionTransformation(); + private static final ExceptionTransformation googleAdsExceptionTransformation = + new GoogleAdsExceptionTransformation(); - public static UnaryCallable createBaseUnaryCallable(GrpcCallSettings grpcCallSettings, UnaryCallSettings callSettings, ClientContext clientContext) { - UnaryCallable callable = GrpcCallableFactory.createBaseUnaryCallable(grpcCallSettings, callSettings, clientContext); - return new ExceptionTransformingCallable<>(callable, googleAdsExceptionTransformation); + public static UnaryCallable createBaseUnaryCallable( + GrpcCallSettings grpcCallSettings, + UnaryCallSettings callSettings, + ClientContext clientContext) { + UnaryCallable callable = + GrpcCallableFactory.createBaseUnaryCallable(grpcCallSettings, callSettings, clientContext); + return new ExceptionTransformingUnaryCallable<>(callable, googleAdsExceptionTransformation); } + @Override public UnaryCallable createUnaryCallable( GrpcCallSettings grpcCallSettings, UnaryCallSettings callSettings, ClientContext clientContext) { - UnaryCallable callable = createBaseUnaryCallable(grpcCallSettings, callSettings, clientContext); + UnaryCallable callable = + createBaseUnaryCallable(grpcCallSettings, callSettings, clientContext); return callable.withDefaultCallContext(clientContext.getDefaultCallContext()); } + @Override public UnaryCallable createPagedCallable( GrpcCallSettings grpcCallSettings, PagedCallSettings pagedCallSettings, ClientContext clientContext) { - UnaryCallable innerCallable = createBaseUnaryCallable(grpcCallSettings, pagedCallSettings, clientContext); - UnaryCallable pagedCallable = Callables.paged(innerCallable, pagedCallSettings); + UnaryCallable innerCallable = + createBaseUnaryCallable(grpcCallSettings, pagedCallSettings, clientContext); + UnaryCallable pagedCallable = + Callables.paged(innerCallable, pagedCallSettings); return pagedCallable.withDefaultCallContext(clientContext.getDefaultCallContext()); } + @Override public UnaryCallable createBatchingCallable( GrpcCallSettings grpcCallSettings, BatchingCallSettings batchingCallSettings, ClientContext clientContext) { - UnaryCallable callable = createBaseUnaryCallable(grpcCallSettings, batchingCallSettings, clientContext); + UnaryCallable callable = + createBaseUnaryCallable(grpcCallSettings, batchingCallSettings, clientContext); callable = Callables.batching(callable, batchingCallSettings, clientContext); return callable.withDefaultCallContext(clientContext.getDefaultCallContext()); } + @Override public OperationCallable createOperationCallable( GrpcCallSettings grpcCallSettings, @@ -83,6 +103,7 @@ OperationCallable createOperationCallable( grpcCallSettings, operationCallSettings, clientContext, operationsStub); } + @Override public BidiStreamingCallable createBidiStreamingCallable( GrpcCallSettings grpcCallSettings, @@ -92,15 +113,20 @@ BidiStreamingCallable createBidiStreamingCallable( grpcCallSettings, streamingCallSettings, clientContext); } + @Override public ServerStreamingCallable createServerStreamingCallable( GrpcCallSettings grpcCallSettings, ServerStreamingCallSettings streamingCallSettings, ClientContext clientContext) { - return GrpcCallableFactory.createServerStreamingCallable( - grpcCallSettings, streamingCallSettings, clientContext); + ServerStreamingCallable defaultCallable = + GrpcCallableFactory.createServerStreamingCallable( + grpcCallSettings, streamingCallSettings, clientContext); + return new ExceptionTransformingServerStreamingCallable( + defaultCallable, new GoogleAdsExceptionTransformation()); } + @Override public ClientStreamingCallable createClientStreamingCallable( GrpcCallSettings grpcCallSettings, diff --git a/google-ads/src/main/java/com/google/ads/googleads/lib/callables/ExceptionTransformation.java b/google-ads/src/main/java/com/google/ads/googleads/lib/callables/ExceptionTransformation.java new file mode 100644 index 0000000000..4788cdbf03 --- /dev/null +++ b/google-ads/src/main/java/com/google/ads/googleads/lib/callables/ExceptionTransformation.java @@ -0,0 +1,26 @@ +// Copyright 2020 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 +// +// 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 com.google.ads.googleads.lib.callables; + +/** Represents a transformation between {@link Throwable}s. */ +public interface ExceptionTransformation { + + /** + * Transforms an input throwable to an output throwable. + * + *

If no transformation is applied this must return the input throwable. + */ + Throwable transform(Throwable throwable); +} diff --git a/google-ads/src/main/java/com/google/ads/googleads/lib/callables/ExceptionTransformingServerStreamingCallable.java b/google-ads/src/main/java/com/google/ads/googleads/lib/callables/ExceptionTransformingServerStreamingCallable.java new file mode 100644 index 0000000000..5042549edc --- /dev/null +++ b/google-ads/src/main/java/com/google/ads/googleads/lib/callables/ExceptionTransformingServerStreamingCallable.java @@ -0,0 +1,80 @@ +// Copyright 2020 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 +// +// 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 com.google.ads.googleads.lib.callables; + +import com.google.api.gax.rpc.ApiCallContext; +import com.google.api.gax.rpc.ApiException; +import com.google.api.gax.rpc.ResponseObserver; +import com.google.api.gax.rpc.ServerStreamingCallable; +import com.google.api.gax.rpc.StreamController; + +/** + * Wrapper around a {@link ServerStreamingCallable} which invokes an {@link ExceptionTransformation} + * for {@link Throwable}s which occur on the stream. + * + *

NOTE: This class could be pushed into the gax library, as it is not specific to the Google Ads + * API. + */ +public class ExceptionTransformingServerStreamingCallable + extends ServerStreamingCallable { + + private final ServerStreamingCallable innerCallable; + private final ExceptionTransformation exceptionTransformation; + + public ExceptionTransformingServerStreamingCallable( + ServerStreamingCallable innerCallable, + ExceptionTransformation exceptionTransformation) { + this.innerCallable = innerCallable; + this.exceptionTransformation = exceptionTransformation; + } + + @Override + public void call( + RequestT request, ResponseObserver responseObserver, ApiCallContext context) { + innerCallable.call(request, new ExceptionTransformingStreamObserver(responseObserver), context); + } + + private class ExceptionTransformingStreamObserver implements ResponseObserver { + + private final ResponseObserver innerObserver; + + public ExceptionTransformingStreamObserver(ResponseObserver innerObserver) { + this.innerObserver = innerObserver; + } + + @Override + public void onStart(StreamController controller) { + innerObserver.onStart(controller); + } + + @Override + public void onResponse(ResponseT response) { + innerObserver.onResponse(response); + } + + @Override + public void onError(Throwable t) { + if (t instanceof ApiException) { + t = exceptionTransformation.transform(t); + } + innerObserver.onError(t); + } + + @Override + public void onComplete() { + innerObserver.onComplete(); + } + } +} diff --git a/google-ads/src/main/java/com/google/ads/googleads/lib/ExceptionTransformingCallable.java b/google-ads/src/main/java/com/google/ads/googleads/lib/callables/ExceptionTransformingUnaryCallable.java similarity index 83% rename from google-ads/src/main/java/com/google/ads/googleads/lib/ExceptionTransformingCallable.java rename to google-ads/src/main/java/com/google/ads/googleads/lib/callables/ExceptionTransformingUnaryCallable.java index 95f6b8394e..24fe45c591 100644 --- a/google-ads/src/main/java/com/google/ads/googleads/lib/ExceptionTransformingCallable.java +++ b/google-ads/src/main/java/com/google/ads/googleads/lib/callables/ExceptionTransformingUnaryCallable.java @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package com.google.ads.googleads.lib; +package com.google.ads.googleads.lib.callables; import com.google.api.core.AbstractApiFuture; import com.google.api.core.ApiFuture; @@ -26,20 +26,19 @@ import java.util.concurrent.CancellationException; /** - * NOTE: This class could be pushed into the gax library, as it is not specific to the Google Ads + * Wrapper around a {@link UnaryCallable} which invokes an {@link ExceptionTransformation} for + * {@link Throwable}s which occur on the stream. + * + *

NOTE: This class could be pushed into the gax library, as it is not specific to the Google Ads * API. */ -public class ExceptionTransformingCallable +public class ExceptionTransformingUnaryCallable extends UnaryCallable { - public interface ExceptionTransformation { - Throwable transform(ApiException throwable); - } - private final UnaryCallable callable; private final ExceptionTransformation transformation; - public ExceptionTransformingCallable( + public ExceptionTransformingUnaryCallable( UnaryCallable callable, ExceptionTransformation transformation) { this.callable = Preconditions.checkNotNull(callable); this.transformation = transformation; @@ -80,7 +79,9 @@ public void onFailure(Throwable throwable) { if (throwable instanceof CancellationException && cancelled) { // this just circled around, so ignore. } else if (throwable instanceof ApiException) { - setException(transformation.transform((ApiException) throwable)); + setException(transformation.transform(throwable)); + } else { + setException(throwable); } } } diff --git a/google-ads/src/main/java/com/google/ads/googleads/lib/GoogleAdsExceptionTransformation.java b/google-ads/src/main/java/com/google/ads/googleads/lib/callables/GoogleAdsExceptionTransformation.java similarity index 62% rename from google-ads/src/main/java/com/google/ads/googleads/lib/GoogleAdsExceptionTransformation.java rename to google-ads/src/main/java/com/google/ads/googleads/lib/callables/GoogleAdsExceptionTransformation.java index b596230ab5..cdf163b697 100644 --- a/google-ads/src/main/java/com/google/ads/googleads/lib/GoogleAdsExceptionTransformation.java +++ b/google-ads/src/main/java/com/google/ads/googleads/lib/callables/GoogleAdsExceptionTransformation.java @@ -12,8 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -package com.google.ads.googleads.lib; +package com.google.ads.googleads.lib.callables; +import com.google.ads.googleads.lib.BaseGoogleAdsException; import com.google.ads.googleads.lib.catalog.GeneratedCatalog; import com.google.ads.googleads.lib.catalog.Version; import com.google.api.gax.rpc.ApiException; @@ -23,20 +24,21 @@ * Transforms an ApiException into a GoogleAdsException whenever a binary GoogleAdsFailure message * was sent in the RPC trailers. */ -public class GoogleAdsExceptionTransformation - implements ExceptionTransformingCallable.ExceptionTransformation { +public class GoogleAdsExceptionTransformation implements ExceptionTransformation { private static final GeneratedCatalog catalog = GeneratedCatalog.getDefault(); @Override - public Throwable transform(ApiException apiException) { - for (Version version : catalog.getSupportedVersions()) { - Optional result = - version.getExceptionFactory().createGoogleAdsException(apiException); - if (result.isPresent()) { - return result.get(); + public Throwable transform(Throwable input) { + if (input.getClass().isAssignableFrom(ApiException.class)) { + for (Version version : catalog.getSupportedVersions()) { + Optional result = + version.getExceptionFactory().createGoogleAdsException((ApiException) input); + if (result.isPresent()) { + return result.get(); + } } } - return apiException; + return input; } } diff --git a/google-ads/src/test/java/com/google/ads/googleads/lib/GoogleAdsExceptionTransformationTest.java b/google-ads/src/test/java/com/google/ads/googleads/lib/GoogleAdsExceptionTransformationTest.java index 271353d00e..4eaca162d1 100644 --- a/google-ads/src/test/java/com/google/ads/googleads/lib/GoogleAdsExceptionTransformationTest.java +++ b/google-ads/src/test/java/com/google/ads/googleads/lib/GoogleAdsExceptionTransformationTest.java @@ -16,6 +16,7 @@ import static org.junit.Assert.assertEquals; +import com.google.ads.googleads.lib.callables.GoogleAdsExceptionTransformation; import com.google.ads.googleads.lib.catalog.ApiCatalog; import com.google.ads.googleads.lib.catalog.Version; import com.google.api.gax.grpc.GrpcStatusCode; diff --git a/google-ads/src/test/java/com/google/ads/googleads/lib/callables/ExceptionCase.java b/google-ads/src/test/java/com/google/ads/googleads/lib/callables/ExceptionCase.java new file mode 100644 index 0000000000..40344490f7 --- /dev/null +++ b/google-ads/src/test/java/com/google/ads/googleads/lib/callables/ExceptionCase.java @@ -0,0 +1,179 @@ +// Copyright 2020 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 +// +// 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 com.google.ads.googleads.lib.callables; + +import com.google.ads.googleads.lib.catalog.GeneratedCatalog; +import com.google.ads.googleads.v5.errors.GoogleAdsError; +import com.google.ads.googleads.v5.errors.GoogleAdsException; +import com.google.ads.googleads.v5.errors.GoogleAdsFailure; +import com.google.api.gax.grpc.GrpcStatusCode; +import com.google.api.gax.rpc.ApiException; +import com.google.common.collect.ImmutableList; +import io.grpc.Metadata; +import io.grpc.Status; +import io.grpc.Status.Code; +import io.grpc.StatusException; +import java.util.List; +import java.util.Objects; +import java.util.stream.Collectors; +import org.mockito.ArgumentMatcher; + +/** Abstracts a test case for exception transformation. */ +public abstract class ExceptionCase implements ArgumentMatcher { + + /** Gets all test cases. */ + public static List getCases() { + return ImmutableList.of( + getGoogleAdsFailureCase(), + getPlainApiExceptionCase(), + getPlainRuntimeExceptionCase(), + getUnparsableMetadataCase()); + } + + /** Gets all test cases in a format expected by JUnit parameterized tests. */ + static List getCasesForParameterized() { + return getCases().stream().map(c -> new Object[] {c.getName(), c}).collect(Collectors.toList()); + } + + /** A short human readable name. */ + abstract String getName(); + + /** Throwable that should be used for testing. */ + abstract Throwable getInputThrowable(); + + /** + * Ensures that the resulting throwable matches expectations. + * + *

Handles most cases where the exceptions can be directly compared with Objects.equals(). + */ + @Override + public boolean matches(Throwable t) { + return Objects.equals(t, getInputThrowable()); + } + + /** Vanilla failure case with a GoogleAdsException. */ + private static ExceptionCase getGoogleAdsFailureCase() { + // Creates a simple GoogleAdsFailure with enough content not to be equal to empty protobuf + // message. + GoogleAdsFailure originalFailure = + GoogleAdsFailure.newBuilder().addErrors(GoogleAdsError.getDefaultInstance()).build(); + + // Creates a GoogleAdsFailure embedded in trailers to match how the API sends back error + // details. + Metadata trailers = new Metadata(); + trailers.put( + GeneratedCatalog.getDefault().getLatestVersion().getExceptionFactory().getTrailerKey(), + originalFailure.toByteArray()); + + // Creates an ApiException with the GoogleAdsFailure trailers, as would be generated by GAX in + // response to an API call that failed on the ads backend. + ApiException originalException = + new ApiException( + new StatusException(Status.INVALID_ARGUMENT, trailers), + GrpcStatusCode.of(Code.INVALID_ARGUMENT), + false); + + return new ExceptionCase() { + + @Override + String getName() { + return "Unpack GoogleAdsFailure"; + } + + @Override + Throwable getInputThrowable() { + return originalException; + } + + @Override + public boolean matches(Throwable t) { + if (t == null) { + return false; + } + if (!t.getClass().isAssignableFrom(GoogleAdsException.class)) { + return false; + } + GoogleAdsException ex = (GoogleAdsException) t; + return Objects.equals(ex.getGoogleAdsFailure(), originalFailure); + } + }; + } + + /** Case where ApiException doesn't contain GoogleAdsFailure. */ + private static ExceptionCase getPlainApiExceptionCase() { + ApiException originalException = + new ApiException(null, GrpcStatusCode.of(Code.INVALID_ARGUMENT), false); + + return new ExceptionCase() { + @Override + String getName() { + return "Plain ApiException passes through"; + } + + @Override + Throwable getInputThrowable() { + return originalException; + } + }; + } + + /** Case where a generic RuntimeException occurs. */ + private static ExceptionCase getPlainRuntimeExceptionCase() { + RuntimeException originalException = new RuntimeException(); + return new ExceptionCase() { + + @Override + String getName() { + return "Plan RuntimeException passes through"; + } + + @Override + Throwable getInputThrowable() { + return originalException; + } + }; + } + + /** Case where a GoogleAdsFailure appears with unparsable Metadata. */ + private static ExceptionCase getUnparsableMetadataCase() { + // Creates a GoogleAdsFailure embedded in trailers to match how the API sends back error + // details. + Metadata trailers = new Metadata(); + trailers.put( + GeneratedCatalog.getDefault().getLatestVersion().getExceptionFactory().getTrailerKey(), + new byte[] {0x1, 0x2, 0x3}); + + // Creates an ApiException with the GoogleAdsFailure trailers, as would be generated by GAX in + // response to an API call that failed on the ads backend. + ApiException originalException = + new ApiException( + new StatusException(Status.INVALID_ARGUMENT, trailers), + GrpcStatusCode.of(Code.INVALID_ARGUMENT), + false); + + return new ExceptionCase() { + + @Override + String getName() { + return "Unparsable GoogleAdsFailure"; + } + + @Override + Throwable getInputThrowable() { + return originalException; + } + }; + } +} diff --git a/google-ads/src/test/java/com/google/ads/googleads/lib/callables/ExceptionTransformingServerStreamingCallableTest.java b/google-ads/src/test/java/com/google/ads/googleads/lib/callables/ExceptionTransformingServerStreamingCallableTest.java new file mode 100644 index 0000000000..56144cd2df --- /dev/null +++ b/google-ads/src/test/java/com/google/ads/googleads/lib/callables/ExceptionTransformingServerStreamingCallableTest.java @@ -0,0 +1,93 @@ +// Copyright 2020 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 +// +// 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 com.google.ads.googleads.lib.callables; + +import static org.mockito.Matchers.argThat; +import static org.mockito.Mockito.verify; + +import com.google.api.gax.grpc.GrpcCallContext; +import com.google.api.gax.rpc.ApiCallContext; +import com.google.api.gax.rpc.ResponseObserver; +import com.google.api.gax.rpc.ServerStreamingCallable; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; + +@RunWith(Parameterized.class) +public class ExceptionTransformingServerStreamingCallableTest { + private static final Object request = "test"; + private static final ApiCallContext context = GrpcCallContext.createDefault(); + + private final ExceptionCase exceptionCase; + private MockServerStreamingCallable innerCallable; + private ExceptionTransformingServerStreamingCallable serverStreamingCallable; + + @Rule public MockitoRule mockitoRule = MockitoJUnit.rule(); + @Mock public ResponseObserver innerObserver; + + @Parameters(name = "{0}") + public static Iterable params() { + return ExceptionCase.getCasesForParameterized(); + } + + public ExceptionTransformingServerStreamingCallableTest( + String name, ExceptionCase exceptionCase) { + this.exceptionCase = exceptionCase; + } + + @Before + public void setup() { + // Sets up mock callable to abstract away gRPC. + this.innerCallable = new MockServerStreamingCallable(); + // Creates the instances to be tested. + this.serverStreamingCallable = + new ExceptionTransformingServerStreamingCallable( + innerCallable, new GoogleAdsExceptionTransformation()); + } + + @Test + public void simulatedException_matchesExpectations() { + // Starts the call. + serverStreamingCallable.call(request, innerObserver, context); + // Simulates the call failing due to an ads backend error. + innerCallable.setException(exceptionCase.getInputThrowable()); + // Ensures that our GoogleAdsFailure was passed to the ResponseObserver. + verify(innerObserver).onError(argThat(exceptionCase)); + } + + /** Mocks out gRPC so we can artificially generate RPC failures. */ + private static class MockServerStreamingCallable + extends ServerStreamingCallable { + + private ResponseObserver responseObserver; + + @Override + public void call( + RequestT request, ResponseObserver responseObserver, ApiCallContext context) { + this.responseObserver = responseObserver; + } + + /** Injects an exception to the call. */ + void setException(Throwable t) { + responseObserver.onError(t); + } + } +} diff --git a/google-ads/src/test/java/com/google/ads/googleads/lib/callables/ExceptionTransformingUnaryCallableTest.java b/google-ads/src/test/java/com/google/ads/googleads/lib/callables/ExceptionTransformingUnaryCallableTest.java new file mode 100644 index 0000000000..d7ef2eb59a --- /dev/null +++ b/google-ads/src/test/java/com/google/ads/googleads/lib/callables/ExceptionTransformingUnaryCallableTest.java @@ -0,0 +1,84 @@ +// Copyright 2020 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 +// +// 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 com.google.ads.googleads.lib.callables; + +import static org.junit.Assert.fail; +import static org.mockito.Mockito.when; + +import com.google.api.core.ApiFuture; +import com.google.api.core.SettableApiFuture; +import com.google.api.gax.grpc.GrpcCallContext; +import com.google.api.gax.rpc.ApiCallContext; +import com.google.api.gax.rpc.UnaryCallable; +import java.util.concurrent.ExecutionException; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; + +@RunWith(Parameterized.class) +public class ExceptionTransformingUnaryCallableTest { + private static final Object request = "test"; + private static final ApiCallContext callContext = GrpcCallContext.createDefault(); + + private final ExceptionCase exceptionCase; + private ExceptionTransformingUnaryCallable callable; + private SettableApiFuture innerFuture; + + @Rule public MockitoRule mockito = MockitoJUnit.rule(); + @Mock public UnaryCallable mockCallable; + + @Parameters(name = "{0}") + public static Iterable params() { + return ExceptionCase.getCasesForParameterized(); + } + + public ExceptionTransformingUnaryCallableTest(String name, ExceptionCase exceptionCase) { + this.exceptionCase = exceptionCase; + } + + @Before + public void setup() { + // Sets up the class we're going to test. + callable = + new ExceptionTransformingUnaryCallable( + mockCallable, new GoogleAdsExceptionTransformation()); + + // Mocks out the gRPC callable implementation. + innerFuture = SettableApiFuture.create(); + when(mockCallable.futureCall(request, callContext)).thenReturn(innerFuture); + } + + @Test + public void simulatedException_matchesExpectations() throws InterruptedException { + // Invokes the async callable. + ApiFuture actualFuture = callable.futureCall(request, GrpcCallContext.createDefault()); + // Simulates a failure of the underlying RPC. + innerFuture.setException(exceptionCase.getInputThrowable()); + try { + // Attempts to get the RPC result. + actualFuture.get(); + // Expects this to fail. + fail(); + } catch (ExecutionException ex) { + exceptionCase.matches(ex.getCause()); + } + } +} From 15323d367f9d2f18e9063094bb4a5f3a8773d8ac Mon Sep 17 00:00:00 2001 From: Nick Birnie Date: Wed, 7 Oct 2020 17:45:18 +0100 Subject: [PATCH 2/3] Address review comments Change-Id: I6993862c9427e6678b1b3ab00a041cbea5383fd7 --- google-ads/pom.xml | 4 +- .../GoogleAdsExceptionTransformation.java | 2 +- .../googleads/lib/GoogleAdsClientTest.java | 125 ++++++++---------- .../lib/callables/ExceptionCase.java | 14 +- ...ansformingServerStreamingCallableTest.java | 6 +- ...xceptionTransformingUnaryCallableTest.java | 6 +- 6 files changed, 69 insertions(+), 88 deletions(-) diff --git a/google-ads/pom.xml b/google-ads/pom.xml index f87a405f2f..2f6be1c041 100644 --- a/google-ads/pom.xml +++ b/google-ads/pom.xml @@ -92,8 +92,8 @@ org.mockito - mockito-core - 3.1.0 + mockito-all + 1.9.5 test diff --git a/google-ads/src/main/java/com/google/ads/googleads/lib/callables/GoogleAdsExceptionTransformation.java b/google-ads/src/main/java/com/google/ads/googleads/lib/callables/GoogleAdsExceptionTransformation.java index cdf163b697..9ca152c30d 100644 --- a/google-ads/src/main/java/com/google/ads/googleads/lib/callables/GoogleAdsExceptionTransformation.java +++ b/google-ads/src/main/java/com/google/ads/googleads/lib/callables/GoogleAdsExceptionTransformation.java @@ -30,7 +30,7 @@ public class GoogleAdsExceptionTransformation implements ExceptionTransformation @Override public Throwable transform(Throwable input) { - if (input.getClass().isAssignableFrom(ApiException.class)) { + if (ApiException.class.isAssignableFrom(input.getClass())) { for (Version version : catalog.getSupportedVersions()) { Optional result = version.getExceptionFactory().createGoogleAdsException((ApiException) input); diff --git a/google-ads/src/test/java/com/google/ads/googleads/lib/GoogleAdsClientTest.java b/google-ads/src/test/java/com/google/ads/googleads/lib/GoogleAdsClientTest.java index dcff1ff03d..a4435ce1de 100644 --- a/google-ads/src/test/java/com/google/ads/googleads/lib/GoogleAdsClientTest.java +++ b/google-ads/src/test/java/com/google/ads/googleads/lib/GoogleAdsClientTest.java @@ -32,6 +32,7 @@ import com.google.ads.googleads.v5.services.GoogleAdsServiceClient; import com.google.ads.googleads.v5.services.MockGoogleAdsService; import com.google.ads.googleads.v5.services.SearchGoogleAdsResponse; +import com.google.ads.googleads.v5.services.SearchGoogleAdsStreamRequest; import com.google.api.gax.grpc.GaxGrpcProperties; import com.google.api.gax.grpc.GrpcStatusCode; import com.google.api.gax.grpc.testing.LocalChannelProvider; @@ -69,9 +70,7 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; -/** - * Tests for {@link GoogleAdsClient}. - */ +/** Tests for {@link GoogleAdsClient}. */ @RunWith(Parameterized.class) public class GoogleAdsClientTest { @@ -84,12 +83,9 @@ public class GoogleAdsClientTest { private static final MockGoogleAdsService mockService = new MockGoogleAdsService(); private static final MockServiceHelper mockServiceHelper = new MockServiceHelper("fake-address", mockService); - @Rule - public TemporaryFolder folder = new TemporaryFolder(); - @Rule - public ExpectedException thrown = ExpectedException.none(); - @Mock - private ScheduledExecutorService executor; + @Rule public TemporaryFolder folder = new TemporaryFolder(); + @Rule public ExpectedException thrown = ExpectedException.none(); + @Mock private ScheduledExecutorService executor; private Credentials fakeCredentials = new FakeCredential(); private LocalChannelProvider localChannelProvider; private Properties testProperties; @@ -101,7 +97,7 @@ public GoogleAdsClientTest(boolean enabledGeneratedCatalog) { @Parameterized.Parameters public static List parameters() { - return Arrays.asList(new Object[]{Boolean.FALSE}, new Object[]{Boolean.TRUE}); + return Arrays.asList(new Object[] {Boolean.FALSE}, new Object[] {Boolean.TRUE}); } @BeforeClass @@ -131,9 +127,7 @@ public void setUp() { localChannelProvider = mockServiceHelper.createChannelProvider(); } - /** - * Verifies that all Factory methods are implemented and operational in GoogleAdsClient. - */ + /** Verifies that all Factory methods are implemented and operational in GoogleAdsClient. */ @Test public void getServiceClient_creationSucceeds() { Stream.of( @@ -151,9 +145,7 @@ public void getServiceClient_creationSucceeds() { }); } - /** - * Verifies reading config from a Java properties file - */ + /** Verifies reading config from a Java properties file */ @Test public void buildFromPropertiesFile_readsFromPropertiesFile() throws IOException { File propertiesFile = folder.newFile("ads.properties"); @@ -166,9 +158,7 @@ public void buildFromPropertiesFile_readsFromPropertiesFile() throws IOException assertGoogleAdsClient(client); } - /** - * Tests building a client from a properties file. - */ + /** Tests building a client from a properties file. */ @Test public void buildFromPropertiesFile_readsAllProperties() throws IOException { // Create a properties file in the temporary folder. @@ -206,9 +196,7 @@ public void testBuildFromPropertiesFile_withoutLoginCustomerId() throws IOExcept assertGoogleAdsClient(client, null, enabledGeneratedCatalog); } - /** - * Tests that an exception is thrown for a nonexistant properties file. - */ + /** Tests that an exception is thrown for a nonexistant properties file. */ @Test public void buildFromPropertiesFile_invalidFilePath_throwsException() throws IOException { File nonExistentFile = new File(folder.getRoot(), "I_dont_exist.properties"); @@ -241,9 +229,7 @@ public void setLoginCustomerId_canClearOnceSet() { assertNull("Unable to clear loginCustomerId", client.getLoginCustomerId()); } - /** - * Tests building a client without the use of a properties file. - */ + /** Tests building a client without the use of a properties file. */ @Test public void buildWithoutPropertiesFile_supportsAllFields() throws IOException { Credentials credentials = @@ -263,9 +249,7 @@ public void buildWithoutPropertiesFile_supportsAllFields() throws IOException { assertGoogleAdsClient(client); } - /** - * Verifies that builder supports nullable loginCustomerId. - */ + /** Verifies that builder supports nullable loginCustomerId. */ @Test public void build_loginCustomerId_allowsNullable() { Credentials credentials = @@ -283,9 +267,7 @@ public void build_loginCustomerId_allowsNullable() { assertNull("invalid login-customer-id", client.getLoginCustomerId()); } - /** - * Verifies that builder does not require enableGeneratedCatalog to be set explicitly. - */ + /** Verifies that builder does not require enableGeneratedCatalog to be set explicitly. */ @Test public void build_enableGeneratedCatalog_not_required() throws IOException { Credentials credentials = @@ -303,9 +285,7 @@ public void build_enableGeneratedCatalog_not_required() throws IOException { assertGoogleAdsClient(client, LOGIN_CUSTOMER_ID, true); } - /** - * Verifies that loginCustomerId is not required. - */ + /** Verifies that loginCustomerId is not required. */ @Test public void buildFromProperties_loginCustomerId_isOptional() { testProperties.remove(ConfigPropertyKey.LOGIN_CUSTOMER_ID.getPropertyKey()); @@ -313,9 +293,7 @@ public void buildFromProperties_loginCustomerId_isOptional() { assertNull(client.getLoginCustomerId()); } - /** - * Verifies that enableGeneratedCatalog is not required and defaults to false. - */ + /** Verifies that enableGeneratedCatalog is not required and defaults to false. */ @Test public void buildFromProperties_enableGeneratedCatalog_isOptional() { testProperties.remove(ConfigPropertyKey.ENABLE_GENERATED_CATALOG.getPropertyKey()); @@ -324,8 +302,7 @@ public void buildFromProperties_enableGeneratedCatalog_isOptional() { } /** - * Verifies that the internal headers for the API client versions (gax, grpc, java) etc. are - * sent. + * Verifies that the internal headers for the API client versions (gax, grpc, java) etc. are sent. */ @Test public void x_goog_api_client_header_isSent() { @@ -350,9 +327,7 @@ public void x_goog_api_client_header_isSent() { GaxGrpcProperties.getDefaultApiClientHeaderPattern())); } - /** - * Verifies that headers include loginCustomerId if present. - */ + /** Verifies that headers include loginCustomerId if present. */ @Test public void loginCustomerId_sentIfSpecified() { GoogleAdsClient client = @@ -375,9 +350,7 @@ public void loginCustomerId_sentIfSpecified() { "login-customer-id", Pattern.compile(String.valueOf(LOGIN_CUSTOMER_ID)))); } - /** - * Verifies that headers does not include loginCustomerId if not specified. - */ + /** Verifies that headers does not include loginCustomerId if not specified. */ @Test public void loginCustomerId_notSentIfExcluded() { GoogleAdsClient client = @@ -397,9 +370,7 @@ public void loginCustomerId_notSentIfExcluded() { localChannelProvider.isHeaderSent("login-customer-id", Pattern.compile(".*"))); } - /** - * Verifies that headers include linkedCustomerId if present. - */ + /** Verifies that headers include linkedCustomerId if present. */ @Test public void linkedCustomerId_sentIfSpecified() { GoogleAdsClient client = @@ -422,11 +393,9 @@ public void linkedCustomerId_sentIfSpecified() { "linked-customer-id", Pattern.compile(String.valueOf(LINKED_CUSTOMER_ID)))); } - /** - * Verifies that the exception transformation behaviour is working for a test example. - */ + /** Verifies that the exception transformation behaviour is working for a test example. */ @Test - public void exceptionTransformedToGoogleAdsException() { + public void unaryCallable_exceptionTransformedToGoogleAdsException() { GoogleAdsClient client = GoogleAdsClient.newBuilder() .setCredentials(fakeCredentials) @@ -450,9 +419,35 @@ public void exceptionTransformedToGoogleAdsException() { } } - /** - * Ensure that can set endpoint on default transport channel. - */ + /** Verifies that the exception transformation behaviour is working for a test example. */ + @Test + public void streamingCallable_exceptionTransformedToGoogleAdsException() { + GoogleAdsClient client = + GoogleAdsClient.newBuilder() + .setCredentials(fakeCredentials) + .setDeveloperToken(DEVELOPER_TOKEN) + .setTransportChannelProvider(localChannelProvider) + .setEnableGeneratedCatalog(enabledGeneratedCatalog) + .build(); + Metadata.Key trailerKey = + ApiCatalog.getDefault().getLatestVersion().getExceptionFactory().getTrailerKey(); + Metadata trailers = new Metadata(); + GoogleAdsFailure.Builder failure = GoogleAdsFailure.newBuilder(); + failure.addErrors(GoogleAdsError.newBuilder().setMessage("Test error message")); + trailers.put(trailerKey, failure.build().toByteArray()); + StatusException rootCause = new StatusException(Status.UNKNOWN, trailers); + mockService.addException(new ApiException(rootCause, GrpcStatusCode.of(Code.UNKNOWN), false)); + try (GoogleAdsServiceClient googleAdsServiceClient = + client.getLatestVersion().createGoogleAdsServiceClient()) { + googleAdsServiceClient + .searchStreamCallable() + .call(SearchGoogleAdsStreamRequest.getDefaultInstance()); + } catch (GoogleAdsException ex) { + // Expected + } + } + + /** Ensure that can set endpoint on default transport channel. */ @Test public void transportChannelProvider_defaultRequiresEndpoint() { assertTrue( @@ -460,9 +455,7 @@ public void transportChannelProvider_defaultRequiresEndpoint() { GoogleAdsClient.newBuilder().getTransportChannelProvider().needsEndpoint()); } - /** - * Ensure that hashCode doesn't collide for a test instance. - */ + /** Ensure that hashCode doesn't collide for a test instance. */ @Test public void hashCode_doesNotCollide() { GoogleAdsClient clientA = @@ -484,9 +477,7 @@ public void hashCode_doesNotCollide() { clientB.hashCode()); } - /** - * Ensures that equals is true for A == B. - */ + /** Ensures that equals is true for A == B. */ @Test public void equals_equalIfSameInstance() { GoogleAdsClient client = @@ -498,9 +489,7 @@ public void equals_equalIfSameInstance() { assertEquals("same instance should be equal", client, client); } - /** - * Ensures that equals is false for A != B, with same config. - */ + /** Ensures that equals is false for A != B, with same config. */ @Test public void equals_notEqualIfDifferentInstance() { GoogleAdsClient clientA = @@ -518,9 +507,7 @@ public void equals_notEqualIfDifferentInstance() { assertNotEquals("different instances should not be equal", clientA, clientB); } - /** - * Ensures that toString returns a nonnull value with length() > 0. - */ + /** Ensures that toString returns a nonnull value with length() > 0. */ @Test public void toString_returnsNotNull() { GoogleAdsClient client = @@ -534,9 +521,7 @@ public void toString_returnsNotNull() { assertFalse("toString should return a non-empty string", toString.isEmpty()); } - /** - * Creates an GoogleAdsClient using mock credentials. - */ + /** Creates an GoogleAdsClient using mock credentials. */ private GoogleAdsClient createTestGoogleAdsClient() { return GoogleAdsClient.newBuilder() .setCredentials(fakeCredentials) diff --git a/google-ads/src/test/java/com/google/ads/googleads/lib/callables/ExceptionCase.java b/google-ads/src/test/java/com/google/ads/googleads/lib/callables/ExceptionCase.java index 40344490f7..c257b0d0ab 100644 --- a/google-ads/src/test/java/com/google/ads/googleads/lib/callables/ExceptionCase.java +++ b/google-ads/src/test/java/com/google/ads/googleads/lib/callables/ExceptionCase.java @@ -31,7 +31,7 @@ import org.mockito.ArgumentMatcher; /** Abstracts a test case for exception transformation. */ -public abstract class ExceptionCase implements ArgumentMatcher { +public abstract class ExceptionCase extends ArgumentMatcher { /** Gets all test cases. */ public static List getCases() { @@ -59,8 +59,8 @@ static List getCasesForParameterized() { *

Handles most cases where the exceptions can be directly compared with Objects.equals(). */ @Override - public boolean matches(Throwable t) { - return Objects.equals(t, getInputThrowable()); + public boolean matches(Object actual) { + return Objects.equals(actual, getInputThrowable()); } /** Vanilla failure case with a GoogleAdsException. */ @@ -98,14 +98,14 @@ Throwable getInputThrowable() { } @Override - public boolean matches(Throwable t) { - if (t == null) { + public boolean matches(Object actual) { + if (actual == null) { return false; } - if (!t.getClass().isAssignableFrom(GoogleAdsException.class)) { + if (!actual.getClass().isAssignableFrom(GoogleAdsException.class)) { return false; } - GoogleAdsException ex = (GoogleAdsException) t; + GoogleAdsException ex = (GoogleAdsException) actual; return Objects.equals(ex.getGoogleAdsFailure(), originalFailure); } }; diff --git a/google-ads/src/test/java/com/google/ads/googleads/lib/callables/ExceptionTransformingServerStreamingCallableTest.java b/google-ads/src/test/java/com/google/ads/googleads/lib/callables/ExceptionTransformingServerStreamingCallableTest.java index 56144cd2df..a55339cb83 100644 --- a/google-ads/src/test/java/com/google/ads/googleads/lib/callables/ExceptionTransformingServerStreamingCallableTest.java +++ b/google-ads/src/test/java/com/google/ads/googleads/lib/callables/ExceptionTransformingServerStreamingCallableTest.java @@ -22,14 +22,12 @@ import com.google.api.gax.rpc.ResponseObserver; import com.google.api.gax.rpc.ServerStreamingCallable; import org.junit.Before; -import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameters; import org.mockito.Mock; -import org.mockito.junit.MockitoJUnit; -import org.mockito.junit.MockitoRule; +import org.mockito.MockitoAnnotations; @RunWith(Parameterized.class) public class ExceptionTransformingServerStreamingCallableTest { @@ -40,7 +38,6 @@ public class ExceptionTransformingServerStreamingCallableTest { private MockServerStreamingCallable innerCallable; private ExceptionTransformingServerStreamingCallable serverStreamingCallable; - @Rule public MockitoRule mockitoRule = MockitoJUnit.rule(); @Mock public ResponseObserver innerObserver; @Parameters(name = "{0}") @@ -55,6 +52,7 @@ public ExceptionTransformingServerStreamingCallableTest( @Before public void setup() { + MockitoAnnotations.initMocks(this); // Sets up mock callable to abstract away gRPC. this.innerCallable = new MockServerStreamingCallable(); // Creates the instances to be tested. diff --git a/google-ads/src/test/java/com/google/ads/googleads/lib/callables/ExceptionTransformingUnaryCallableTest.java b/google-ads/src/test/java/com/google/ads/googleads/lib/callables/ExceptionTransformingUnaryCallableTest.java index d7ef2eb59a..bb9cd6e4ad 100644 --- a/google-ads/src/test/java/com/google/ads/googleads/lib/callables/ExceptionTransformingUnaryCallableTest.java +++ b/google-ads/src/test/java/com/google/ads/googleads/lib/callables/ExceptionTransformingUnaryCallableTest.java @@ -24,14 +24,12 @@ import com.google.api.gax.rpc.UnaryCallable; import java.util.concurrent.ExecutionException; import org.junit.Before; -import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameters; import org.mockito.Mock; -import org.mockito.junit.MockitoJUnit; -import org.mockito.junit.MockitoRule; +import org.mockito.MockitoAnnotations; @RunWith(Parameterized.class) public class ExceptionTransformingUnaryCallableTest { @@ -42,7 +40,6 @@ public class ExceptionTransformingUnaryCallableTest { private ExceptionTransformingUnaryCallable callable; private SettableApiFuture innerFuture; - @Rule public MockitoRule mockito = MockitoJUnit.rule(); @Mock public UnaryCallable mockCallable; @Parameters(name = "{0}") @@ -56,6 +53,7 @@ public ExceptionTransformingUnaryCallableTest(String name, ExceptionCase excepti @Before public void setup() { + MockitoAnnotations.initMocks(this); // Sets up the class we're going to test. callable = new ExceptionTransformingUnaryCallable( From 0b3d530f8275344edddc671247c0c98f0ac82cbe Mon Sep 17 00:00:00 2001 From: Nick Birnie Date: Thu, 8 Oct 2020 13:55:10 +0100 Subject: [PATCH 3/3] Address review comments Change-Id: If3fef16448c9f6497dbe0a8ae97946d92a47adbb --- ...onTransformingServerStreamingCallable.java | 1 + .../ExceptionTransformingUnaryCallable.java | 6 ++---- .../googleads/lib/GoogleAdsClientTest.java | 19 +++++++++++++++---- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/google-ads/src/main/java/com/google/ads/googleads/lib/callables/ExceptionTransformingServerStreamingCallable.java b/google-ads/src/main/java/com/google/ads/googleads/lib/callables/ExceptionTransformingServerStreamingCallable.java index 5042549edc..5ff36d0f07 100644 --- a/google-ads/src/main/java/com/google/ads/googleads/lib/callables/ExceptionTransformingServerStreamingCallable.java +++ b/google-ads/src/main/java/com/google/ads/googleads/lib/callables/ExceptionTransformingServerStreamingCallable.java @@ -46,6 +46,7 @@ public void call( innerCallable.call(request, new ExceptionTransformingStreamObserver(responseObserver), context); } + /** Provides a mechanism for transforming any exceptions which occur on the stream. */ private class ExceptionTransformingStreamObserver implements ResponseObserver { private final ResponseObserver innerObserver; diff --git a/google-ads/src/main/java/com/google/ads/googleads/lib/callables/ExceptionTransformingUnaryCallable.java b/google-ads/src/main/java/com/google/ads/googleads/lib/callables/ExceptionTransformingUnaryCallable.java index 24fe45c591..c13e65baaa 100644 --- a/google-ads/src/main/java/com/google/ads/googleads/lib/callables/ExceptionTransformingUnaryCallable.java +++ b/google-ads/src/main/java/com/google/ads/googleads/lib/callables/ExceptionTransformingUnaryCallable.java @@ -20,7 +20,6 @@ import com.google.api.core.ApiFutures; import com.google.api.gax.grpc.GrpcCallContext; import com.google.api.gax.rpc.ApiCallContext; -import com.google.api.gax.rpc.ApiException; import com.google.api.gax.rpc.UnaryCallable; import com.google.common.base.Preconditions; import java.util.concurrent.CancellationException; @@ -54,6 +53,7 @@ public ApiFuture futureCall(RequestT request, ApiCallContext inputCon return transformingFuture; } + /** Provides a mechanism to transform exceptions which occur on the inner callable. */ private class ExceptionTransformingFuture extends AbstractApiFuture implements ApiFutureCallback { private ApiFuture innerCallFuture; @@ -78,10 +78,8 @@ public void onSuccess(ResponseT r) { public void onFailure(Throwable throwable) { if (throwable instanceof CancellationException && cancelled) { // this just circled around, so ignore. - } else if (throwable instanceof ApiException) { - setException(transformation.transform(throwable)); } else { - setException(throwable); + setException(transformation.transform(throwable)); } } } diff --git a/google-ads/src/test/java/com/google/ads/googleads/lib/GoogleAdsClientTest.java b/google-ads/src/test/java/com/google/ads/googleads/lib/GoogleAdsClientTest.java index a4435ce1de..ad5a8853aa 100644 --- a/google-ads/src/test/java/com/google/ads/googleads/lib/GoogleAdsClientTest.java +++ b/google-ads/src/test/java/com/google/ads/googleads/lib/GoogleAdsClientTest.java @@ -29,10 +29,13 @@ import com.google.ads.googleads.v5.errors.GoogleAdsError; import com.google.ads.googleads.v5.errors.GoogleAdsException; import com.google.ads.googleads.v5.errors.GoogleAdsFailure; +import com.google.ads.googleads.v5.services.GoogleAdsRow; import com.google.ads.googleads.v5.services.GoogleAdsServiceClient; +import com.google.ads.googleads.v5.services.GoogleAdsServiceClient.SearchPagedResponse; import com.google.ads.googleads.v5.services.MockGoogleAdsService; import com.google.ads.googleads.v5.services.SearchGoogleAdsResponse; import com.google.ads.googleads.v5.services.SearchGoogleAdsStreamRequest; +import com.google.ads.googleads.v5.services.SearchGoogleAdsStreamResponse; import com.google.api.gax.grpc.GaxGrpcProperties; import com.google.api.gax.grpc.GrpcStatusCode; import com.google.api.gax.grpc.testing.LocalChannelProvider; @@ -413,7 +416,11 @@ public void unaryCallable_exceptionTransformedToGoogleAdsException() { mockService.addException(new ApiException(rootCause, GrpcStatusCode.of(Code.UNKNOWN), false)); try (GoogleAdsServiceClient googleAdsServiceClient = client.getLatestVersion().createGoogleAdsServiceClient()) { - googleAdsServiceClient.search("123", "select blah"); + SearchPagedResponse response = googleAdsServiceClient.search("123", "select blah"); + for (GoogleAdsRow row : response.iterateAll()) { + // Attempt to process the rows. + } + fail(); } catch (GoogleAdsException ex) { // Expected } @@ -439,9 +446,13 @@ public void streamingCallable_exceptionTransformedToGoogleAdsException() { mockService.addException(new ApiException(rootCause, GrpcStatusCode.of(Code.UNKNOWN), false)); try (GoogleAdsServiceClient googleAdsServiceClient = client.getLatestVersion().createGoogleAdsServiceClient()) { - googleAdsServiceClient - .searchStreamCallable() - .call(SearchGoogleAdsStreamRequest.getDefaultInstance()); + for (SearchGoogleAdsStreamResponse row : + googleAdsServiceClient + .searchStreamCallable() + .call(SearchGoogleAdsStreamRequest.getDefaultInstance())) { + // Attempt to process the stream. + } + fail(); } catch (GoogleAdsException ex) { // Expected }