chore: make FirestoreException factory methods public to allow constr…

…uction for testing (#477) * Expose each factory method at @BetaApi level ensuring no guarantee about api surface * Expose getStatus() at @BetaApi up from package private @internalapi
googleapis · Jan 15, 2021 · 23d5415 · 23d5415
1 parent d17e17c
commit 23d5415
Show file tree

Hide file tree

Showing 15 changed files with 51 additions and 39 deletions.
diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/BulkCommitBatch.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/BulkCommitBatch.java
@@ -117,7 +117,7 @@ public List<BatchWriteResult> apply(BatchWriteResponse batchWriteResponse) {
               if (code == Status.OK) {
                 updateTime = Timestamp.fromProto(writeResult.getUpdateTime());
               } else {
-                exception = FirestoreException.serverRejected(code, status.getMessage());
+                exception = FirestoreException.forServerRejection(code, status.getMessage());
               }
               result.add(new BatchWriteResult(updateTime, exception));
             }

diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/BulkWriterOptions.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/BulkWriterOptions.java
@@ -130,24 +130,24 @@ BulkWriterOptions build() {
       Double maxRate = options.getMaxOpsPerSecond();
 
       if (initialRate != null && initialRate < 1) {
-        throw FirestoreException.invalidState(
+        throw FirestoreException.forInvalidArgument(
             "Value for argument 'initialOpsPerSecond' must be greater than 1, but was: "
                 + initialRate.intValue());
       }
 
       if (maxRate != null && maxRate < 1) {
-        throw FirestoreException.invalidState(
+        throw FirestoreException.forInvalidArgument(
             "Value for argument 'maxOpsPerSecond' must be greater than 1, but was: "
                 + maxRate.intValue());
       }
 
       if (maxRate != null && initialRate != null && initialRate > maxRate) {
-        throw FirestoreException.invalidState(
+        throw FirestoreException.forInvalidArgument(
             "'maxOpsPerSecond' cannot be less than 'initialOpsPerSecond'.");
       }
 
       if (!options.getThrottlingEnabled() && (maxRate != null || initialRate != null)) {
-        throw FirestoreException.invalidState(
+        throw FirestoreException.forInvalidArgument(
             "Cannot set 'initialOpsPerSecond' or 'maxOpsPerSecond' when 'throttlingEnabled' is set to false.");
       }
       return options;

diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/CollectionGroup.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/CollectionGroup.java
@@ -82,7 +82,7 @@ public void getPartitions(
                     request.build(), rpcContext.getClient().partitionQueryPagedCallable()));
       } catch (ApiException exception) {
         span.setStatus(Status.UNKNOWN.withDescription(exception.getMessage()));
-        throw FirestoreException.apiException(exception);
+        throw FirestoreException.forApiException(exception);
       } finally {
         span.end(TraceUtil.END_SPAN_OPTIONS);
       }

diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/CollectionReference.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/CollectionReference.java
@@ -149,7 +149,7 @@ public Iterable<DocumentReference> listDocuments() {
       response = ApiExceptions.callAndTranslateApiException(future);
     } catch (ApiException exception) {
       span.setStatus(Status.UNKNOWN.withDescription(exception.getMessage()));
-      throw FirestoreException.apiException(exception);
+      throw FirestoreException.forApiException(exception);
     } finally {
       span.end(TraceUtil.END_SPAN_OPTIONS);
     }
@@ -217,7 +217,8 @@ public DocumentReference apply(WriteResult writeResult) {
   public ApiFuture<DocumentReference> add(Object pojo) {
     Object converted = CustomClassMapper.convertToPlainJavaTypes(pojo);
     if (!(converted instanceof Map)) {
-      throw FirestoreException.invalidState("Can't set a document's data to an array or primitive");
+      throw FirestoreException.forInvalidArgument(
+          "Can't set a document's data to an array or primitive");
     }
     return add((Map<String, Object>) converted);
   }

diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/DocumentReference.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/DocumentReference.java
@@ -393,7 +393,7 @@ public Iterable<CollectionReference> listCollections() {
                   request.build(), rpcContext.getClient().listCollectionIdsPagedCallable()));
     } catch (ApiException exception) {
       span.setStatus(Status.UNKNOWN.withDescription(exception.getMessage()));
-      throw FirestoreException.apiException(exception);
+      throw FirestoreException.forApiException(exception);
     } finally {
       span.end(TraceUtil.END_SPAN_OPTIONS);
     }

diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/DocumentTransform.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/DocumentTransform.java
@@ -75,7 +75,7 @@ private static SortedMap<FieldPath, FieldTransform> extractFromMap(
             transforms.put(path, fieldValue.toProto(path));
           }
         } else {
-          throw FirestoreException.invalidState(
+          throw FirestoreException.forInvalidArgument(
               fieldValue.getMethodName() + " is not supported inside of an array.");
         }
       } else if (value instanceof Map) {
@@ -94,7 +94,7 @@ private static void validateArray(List<Object> values, FieldPath path) {
       Object value = values.get(i);
       path = path.append(FieldPath.of(Integer.toString(i)));
       if (value instanceof FieldValue) {
-        throw FirestoreException.invalidState(
+        throw FirestoreException.forInvalidArgument(
             ((FieldValue) value).getMethodName() + " is not supported inside of an array.");
       } else if (value instanceof Map) {
         extractFromMap((Map<String, Object>) value, path, false);

diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreException.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreException.java
@@ -16,7 +16,7 @@
 
 package com.google.cloud.firestore;
 
-import com.google.api.core.InternalApi;
+import com.google.api.core.BetaApi;
 import com.google.api.gax.rpc.ApiException;
 import com.google.cloud.grpc.BaseGrpcServiceException;
 import io.grpc.Status;
@@ -55,7 +55,8 @@ private FirestoreException(IOException exception, boolean retryable) {
    *
    * @return The FirestoreException
    */
-  static FirestoreException invalidState(String message, Object... params) {
+  @BetaApi
+  public static FirestoreException forInvalidArgument(String message, Object... params) {
     return new FirestoreException(String.format(message, params), Status.INVALID_ARGUMENT);
   }
 
@@ -65,8 +66,10 @@ static FirestoreException invalidState(String message, Object... params) {
    *
    * @return The FirestoreException
    */
-  static FirestoreException serverRejected(Status status, String message, Object... params) {
-    return serverRejected(status, null, message, params);
+  @BetaApi
+  public static FirestoreException forServerRejection(
+      Status status, String message, Object... params) {
+    return forServerRejection(status, null, message, params);
   }
 
   /**
@@ -75,7 +78,8 @@ static FirestoreException serverRejected(Status status, String message, Object..
    *
    * @return The FirestoreException
    */
-  static FirestoreException serverRejected(
+  @BetaApi
+  public static FirestoreException forServerRejection(
       Status status, @Nullable Throwable cause, String message, Object... params) {
     return new FirestoreException(String.format(message, params), status, cause);
   }
@@ -85,7 +89,8 @@ static FirestoreException serverRejected(
    *
    * @return The FirestoreException
    */
-  static FirestoreException networkException(IOException exception, boolean retryable) {
+  @BetaApi
+  public static FirestoreException forIOException(IOException exception, boolean retryable) {
     return new FirestoreException(exception, retryable);
   }
 
@@ -94,7 +99,8 @@ static FirestoreException networkException(IOException exception, boolean retrya
    *
    * @return The FirestoreException
    */
-  static FirestoreException apiException(ApiException exception) {
+  @BetaApi
+  public static FirestoreException forApiException(ApiException exception) {
     return new FirestoreException(exception.getMessage(), exception);
   }
 
@@ -103,13 +109,14 @@ static FirestoreException apiException(ApiException exception) {
    *
    * @return The FirestoreException
    */
-  static FirestoreException apiException(ApiException exception, String message) {
+  @BetaApi
+  public static FirestoreException forApiException(ApiException exception, String message) {
     return new FirestoreException(message, exception);
   }
 
-  @InternalApi
+  @BetaApi
   @Nullable
-  Status getStatus() {
+  public Status getStatus() {
     return status;
   }
 }
diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreOptions.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreOptions.java
@@ -88,7 +88,7 @@ public FirestoreRpc create(@Nonnull FirestoreOptions options) {
       try {
         return new GrpcFirestoreRpc(options);
       } catch (IOException e) {
-        throw FirestoreException.networkException(e, false);
+        throw FirestoreException.forIOException(e, false);
       }
     }
   }

diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Query.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Query.java
@@ -406,7 +406,7 @@ private Cursor createCursor(List<FieldOrder> order, Object[] fieldValues, boolea
       Value encodedValue = encodeValue(fieldReference, sanitizedValue);
 
       if (encodedValue == null) {
-        throw FirestoreException.invalidState(
+        throw FirestoreException.forInvalidArgument(
             "Cannot use FieldValue.delete() or FieldValue.serverTimestamp() in a query boundary");
       }
       result.addValues(encodedValue);
@@ -1434,7 +1434,7 @@ private Value encodeValue(FieldPath fieldPath, Object value) {
     Value encodedValue =
         UserDataConverter.encodeValue(fieldPath, sanitizedObject, UserDataConverter.ARGUMENT);
     if (encodedValue == null) {
-      throw FirestoreException.invalidState(
+      throw FirestoreException.forInvalidArgument(
           "Cannot use Firestore sentinels in FieldFilter or cursors");
     }
     return encodedValue;

diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/TransactionRunner.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/TransactionRunner.java
@@ -236,14 +236,14 @@ public ApiFuture<T> apply(Throwable throwable) {
         } else {
           span.setStatus(TOO_MANY_RETRIES_STATUS);
           final FirestoreException firestoreException =
-              FirestoreException.apiException(
+              FirestoreException.forApiException(
                   apiException, "Transaction was cancelled because of too many retries.");
           return rollbackAndReject(firestoreException);
         }
       } else {
         span.setStatus(TraceUtil.statusFromApiException(apiException));
         final FirestoreException firestoreException =
-            FirestoreException.apiException(
+            FirestoreException.forApiException(
                 apiException, "Transaction failed with non-retryable error");
         return rollbackAndReject(firestoreException);
       }

diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/UpdateBuilder.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/UpdateBuilder.java
@@ -172,7 +172,8 @@ private void verifyNotCommitted() {
   public T create(@Nonnull DocumentReference documentReference, @Nonnull Object pojo) {
     Object data = CustomClassMapper.convertToPlainJavaTypes(pojo);
     if (!(data instanceof Map)) {
-      throw FirestoreException.invalidState("Can't set a document's data to an array or primitive");
+      throw FirestoreException.forInvalidArgument(
+          "Can't set a document's data to an array or primitive");
     }
     return performCreate(documentReference, (Map<String, Object>) data);
   }

diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/UserDataConverter.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/UserDataConverter.java
@@ -181,7 +181,8 @@ static Value encodeValue(
       }
     }
 
-    throw FirestoreException.invalidState("Cannot convert %s to Firestore Value", sanitizedObject);
+    throw FirestoreException.forInvalidArgument(
+        "Cannot convert %s to Firestore Value", sanitizedObject);
   }
 
   static Object decodeValue(FirestoreRpcContext<?> rpcContext, Value v) {
@@ -222,7 +223,8 @@ static Object decodeValue(FirestoreRpcContext<?> rpcContext, Value v) {
         }
         return outputMap;
       default:
-        throw FirestoreException.invalidState(String.format("Unknown Value Type: %s", typeCase));
+        throw FirestoreException.forInvalidArgument(
+            String.format("Unknown Value Type: %s", typeCase));
     }
   }
 }
diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Watch.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Watch.java
@@ -184,7 +184,7 @@ public synchronized void onNext(ListenResponse listenResponse) {
             break;
           case ADD:
             if (WATCH_TARGET_ID != change.getTargetIds(0)) {
-              closeStream(FirestoreException.invalidState("Target ID must be 0x01"));
+              closeStream(FirestoreException.forInvalidArgument("Target ID must be 0x01"));
             }
             break;
           case REMOVE:
@@ -193,7 +193,7 @@ public synchronized void onNext(ListenResponse listenResponse) {
                     ? Status.fromCodeValue(change.getCause().getCode())
                     : Status.CANCELLED;
             closeStream(
-                FirestoreException.serverRejected(
+                FirestoreException.forServerRejection(
                     status, "Backend ended Listen stream: " + change.getCause().getMessage()));
             break;
           case CURRENT:
@@ -204,7 +204,7 @@ public synchronized void onNext(ListenResponse listenResponse) {
             break;
           default:
             closeStream(
-                FirestoreException.invalidState(
+                FirestoreException.forInvalidArgument(
                     "Encountered invalid target change type: " + change.getTargetChangeType()));
         }
 
@@ -247,7 +247,8 @@ && affectsTarget(change.getTargetIdsList(), WATCH_TARGET_ID)) {
         }
         break;
       default:
-        closeStream(FirestoreException.invalidState("Encountered invalid listen response type"));
+        closeStream(
+            FirestoreException.forInvalidArgument("Encountered invalid listen response type"));
         break;
     }
   }
@@ -340,7 +341,7 @@ public void run() {
                   null,
                   throwable instanceof FirestoreException
                       ? (FirestoreException) throwable
-                      : FirestoreException.apiException(
+                      : FirestoreException.forApiException(
                           new ApiException(
                               throwable,
                               GrpcStatusCode.of(getStatus(throwable).getCode()),

diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/BulkWriterTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/BulkWriterTest.java
@@ -997,7 +997,7 @@ public void closeSucceedsEvenIfBulkCommitFails() throws Exception {
   public void individualWritesErrorIfBulkCommitFails() throws Exception {
     doReturn(
             ApiFutures.immediateFailedFuture(
-                FirestoreException.serverRejected(
+                FirestoreException.forServerRejection(
                     Status.DEADLINE_EXCEEDED, "Mock batchWrite failed in test")))
         .when(firestoreMock)
         .sendRequest(
@@ -1049,7 +1049,7 @@ public void individualWritesErrorIfBulkCommitFailsWithNonFirestoreException() th
   @Test
   public void retriesWritesWhenBatchWriteFailsWithRetryableError() throws Exception {
     FirestoreException retryableError =
-        FirestoreException.serverRejected(
+        FirestoreException.forServerRejection(
             Status.fromCode(Status.Code.ABORTED), "Mock batchWrite failed in test");
 
     ApiFuture<Void> errorFuture = ApiFutures.immediateFailedFuture(retryableError);
@@ -1077,7 +1077,7 @@ public void failsWritesAfterAllRetryAttemptsFail() throws Exception {
               public ApiFuture<Object> answer(InvocationOnMock mock) {
                 retryAttempts[0]++;
                 return ApiFutures.immediateFailedFuture(
-                    FirestoreException.serverRejected(
+                    FirestoreException.forServerRejection(
                         Status.fromCode(Status.Code.ABORTED), "Mock batchWrite failed in test"));
               }
             })

diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/QueryTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/QueryTest.java
@@ -938,7 +938,7 @@ public RunQueryResponse answer(InvocationOnMock invocation) throws Throwable {
                 if (returnError[0]) {
                   returnError[0] = false;
                   return queryResponse(
-                          FirestoreException.serverRejected(
+                          FirestoreException.forServerRejection(
                               Status.DEADLINE_EXCEEDED, "Simulated test failure"),
                           DOCUMENT_NAME + "1",
                           DOCUMENT_NAME + "2")
@@ -998,7 +998,7 @@ public void onCompleted() {
   public void doesNotRetryAfterNonRetryableError() throws Exception {
     doAnswer(
             queryResponse(
-                FirestoreException.serverRejected(
+                FirestoreException.forServerRejection(
                     Status.PERMISSION_DENIED, "Simulated test failure"),
                 DOCUMENT_NAME + "1",
                 DOCUMENT_NAME + "2"))