Rejecting empty Document Ids

schmidt-sebastian · schmidt-sebastian · commit 8dad46922b01 · 2017-10-27T16:16:25.000-07:00
diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/BasePath.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/BasePath.java
@@ -16,6 +16,7 @@
 
 package com.google.cloud.firestore;
 
+import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import javax.annotation.Nullable;
 
@@ -52,6 +53,7 @@ B getParent() {
    * @param path A relative path
    */
   B append(String path) {
+    Preconditions.checkArgument(path != null && !path.isEmpty(), "'path' must be a non-empty String" );
     ImmutableList.Builder<String> components = ImmutableList.builder();
     components.addAll(this.getSegments());
     components.add(splitChildPath(path));
diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/CollectionReferenceTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/CollectionReferenceTest.java
@@ -19,10 +19,12 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 import static org.mockito.Mockito.doReturn;
 
 import com.google.api.core.ApiFuture;
 import com.google.api.gax.rpc.UnaryCallable;
+import com.google.cloud.firestore.Transaction.Function;
 import com.google.cloud.firestore.spi.v1beta1.FirestoreRpc;
 import com.google.firestore.v1beta1.CommitRequest;
 import com.google.firestore.v1beta1.CommitResponse;
@@ -68,6 +70,13 @@ public void getDocument() {
     assertEquals("doc", documentReference.getId());
     documentReference = collectionReference.document();
     assertEquals(20, documentReference.getId().length());
+
+    try {
+      collectionReference.document("");
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertTrue(e.getMessage().endsWith("'path' must be a non-empty String"));
+    }
   }
 
   @Test
diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITSystemTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITSystemTest.java
@@ -423,6 +423,7 @@ public String updateCallback(Transaction transaction) {
                 }
               })
           .get();
+      fail();
     } catch (Exception e) {
       assertTrue(e.getMessage().endsWith("User exception"));
     }

Original file line number	Diff line number	Diff line change
`@@ -423,6 +423,7 @@ public String updateCallback(Transaction transaction) {`
`423`	`423`	`}`
`424`	`424`	`})`
`425`	`425`	`.get();`
	`426`	`+ fail();`
`426`	`427`	`} catch (Exception e) {`
`427`	`428`	`assertTrue(e.getMessage().endsWith("User exception"));`
`428`	`429`	`}`