From 0b103de4b7c1dc72fe16189dd4fb955d44fb55ef Mon Sep 17 00:00:00 2001 From: Hui Wu Date: Tue, 11 Aug 2026 14:55:02 +0000 Subject: [PATCH 01/18] feat(firestore): add DML stages (Insert, Upsert) and atomic execution option to Java SDK pipelines - Implement Insert.java and Upsert.java stages in com.google.cloud.firestore.pipeline.stages - Add insert() and upsert() builder methods to Pipeline.java - Add withAtomic() option to PipelineExecuteOptions.java and wire proto request (newTransaction + autoCommitTransaction) - Add proto serialization unit tests in PipelineProtoTest.java - Add system integration test suite in ITPipelineTest.java covering insert, upsert, atomic execution, and transaction execution BUG=b/545136773 TAG=agy --- .../com/google/cloud/firestore/Pipeline.java | 45 +++++- .../firestore/pipeline/stages/Insert.java | 79 ++++++++++ .../stages/PipelineExecuteOptions.java | 9 ++ .../firestore/pipeline/stages/Upsert.java | 98 +++++++++++++ .../cloud/firestore/PipelineProtoTest.java | 137 ++++++++++++++++++ .../cloud/firestore/it/ITPipelineTest.java | 100 +++++++++++++ 6 files changed, 462 insertions(+), 6 deletions(-) create mode 100644 java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java create mode 100644 java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java index c49ee0ad2f9a..5c9b84ab7a2a 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java @@ -47,6 +47,7 @@ import com.google.cloud.firestore.pipeline.stages.Distinct; import com.google.cloud.firestore.pipeline.stages.FindNearest; import com.google.cloud.firestore.pipeline.stages.FindNearestOptions; +import com.google.cloud.firestore.pipeline.stages.Insert; import com.google.cloud.firestore.pipeline.stages.Limit; import com.google.cloud.firestore.pipeline.stages.Offset; import com.google.cloud.firestore.pipeline.stages.PipelineExecuteOptions; @@ -63,6 +64,7 @@ import com.google.cloud.firestore.pipeline.stages.Unnest; import com.google.cloud.firestore.pipeline.stages.UnnestOptions; import com.google.cloud.firestore.pipeline.stages.Update; +import com.google.cloud.firestore.pipeline.stages.Upsert; import com.google.cloud.firestore.pipeline.stages.Where; import com.google.cloud.firestore.telemetry.MetricsUtil.MetricsContext; import com.google.cloud.firestore.telemetry.TelemetryConstants; @@ -76,6 +78,7 @@ import com.google.firestore.v1.ExecutePipelineRequest; import com.google.firestore.v1.ExecutePipelineResponse; import com.google.firestore.v1.StructuredPipeline; +import com.google.firestore.v1.TransactionOptions; import com.google.firestore.v1.Value; import com.google.protobuf.ByteString; import java.util.ArrayList; @@ -1306,6 +1309,21 @@ public Pipeline update(Update update) { return append(update); } + @BetaApi + public Pipeline insert(Insert insert) { + return append(insert); + } + + @BetaApi + public Pipeline upsert(Upsert upsert) { + return append(upsert); + } + + @BetaApi + public Pipeline upsert(Selectable... transformedFields) { + return append(new Upsert(transformedFields)); + } + /** * Performs an insert operation using documents from previous stages. Adds a generic stage to the * pipeline. @@ -1509,15 +1527,13 @@ public void onError(Throwable t) { } } - void executeInternal( + ExecutePipelineRequest toExecutePipelineRequest( @Nonnull PipelineExecuteOptions options, @Nullable final ByteString transactionId, - @Nullable com.google.protobuf.Timestamp readTime, - PipelineResultObserver observer, - MetricsContext metricsContext) { + @Nullable com.google.protobuf.Timestamp readTime) { ExecutePipelineRequest.Builder request = ExecutePipelineRequest.newBuilder() - .setDatabase(rpcContext.getDatabaseName()) + .setDatabase(rpcContext != null ? rpcContext.getDatabaseName() : "") .setStructuredPipeline( StructuredPipeline.newBuilder() .setPipeline(toProto()) @@ -1526,14 +1542,31 @@ void executeInternal( if (transactionId != null) { request.setTransaction(transactionId); + } else if (options.isAtomic()) { + request.setNewTransaction( + TransactionOptions.newBuilder() + .setReadWrite(TransactionOptions.ReadWrite.getDefaultInstance()) + .build()); + request.setAutoCommitTransaction(true); } if (readTime != null) { request.setReadTime(readTime); } + return request.build(); + } + + void executeInternal( + @Nonnull PipelineExecuteOptions options, + @Nullable final ByteString transactionId, + @Nullable com.google.protobuf.Timestamp readTime, + PipelineResultObserver observer, + MetricsContext metricsContext) { + ExecutePipelineRequest request = toExecutePipelineRequest(options, transactionId, readTime); + pipelineInternalStream( - request.build(), + request, new PipelineResultObserver() { @Override public void onCompleted() { diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java new file mode 100644 index 000000000000..246720c0b36c --- /dev/null +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java @@ -0,0 +1,79 @@ +/* + * Copyright 2026 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.cloud.firestore.pipeline.stages; + +import com.google.api.core.BetaApi; +import com.google.api.core.InternalApi; +import com.google.cloud.firestore.PipelineUtils; +import com.google.cloud.firestore.pipeline.expressions.Expression; +import com.google.common.collect.ImmutableMap; +import com.google.firestore.v1.Value; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Map; +import javax.annotation.Nullable; + +@InternalApi +public final class Insert extends Stage { + + @Nullable private final String collectionPath; + @Nullable private final Expression documentIdExpr; + + private Insert( + @Nullable String collectionPath, + @Nullable Expression documentIdExpr, + InternalOptions options) { + super("insert", buildOptions(collectionPath, documentIdExpr, options)); + this.collectionPath = collectionPath; + this.documentIdExpr = documentIdExpr; + } + + @BetaApi + public Insert() { + this(null, null, InternalOptions.EMPTY); + } + + @BetaApi + public Insert withCollection(String collectionPath) { + return new Insert(collectionPath, this.documentIdExpr, this.options); + } + + @BetaApi + public Insert withDocumentId(Expression documentIdExpr) { + return new Insert(this.collectionPath, documentIdExpr, this.options); + } + + private static InternalOptions buildOptions( + @Nullable String collectionPath, + @Nullable Expression documentIdExpr, + InternalOptions baseOptions) { + Map optsMap = new HashMap<>(baseOptions.options); + if (collectionPath != null) { + String path = collectionPath.startsWith("/") ? collectionPath : "/" + collectionPath; + optsMap.put("collection", Value.newBuilder().setReferenceValue(path).build()); + } + if (documentIdExpr != null) { + optsMap.put("document_id", PipelineUtils.encodeValue(documentIdExpr)); + } + return new InternalOptions(ImmutableMap.copyOf(optsMap)); + } + + @Override + Iterable toStageArgs() { + return new ArrayList<>(); + } +} diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/PipelineExecuteOptions.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/PipelineExecuteOptions.java index 754215e675f2..ef3630251240 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/PipelineExecuteOptions.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/PipelineExecuteOptions.java @@ -38,4 +38,13 @@ public PipelineExecuteOptions withExplainOptions(ExplainOptions options) { public PipelineExecuteOptions withIndexMode(String indexMode) { return with("index_mode", indexMode); } + + public PipelineExecuteOptions withAtomic(boolean atomic) { + return with("atomic", atomic); + } + + public boolean isAtomic() { + return options.options.containsKey("atomic") + && options.options.get("atomic").getBooleanValue(); + } } diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java new file mode 100644 index 000000000000..e74e44b2ce30 --- /dev/null +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java @@ -0,0 +1,98 @@ +/* + * Copyright 2026 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.cloud.firestore.pipeline.stages; + +import com.google.api.core.BetaApi; +import com.google.api.core.InternalApi; +import com.google.cloud.firestore.PipelineUtils; +import com.google.cloud.firestore.pipeline.expressions.Expression; +import com.google.cloud.firestore.pipeline.expressions.Selectable; +import com.google.common.collect.ImmutableMap; +import com.google.firestore.v1.Value; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; + +@InternalApi +public final class Upsert extends Stage { + + @Nullable private final Selectable[] transformedFields; + @Nullable private final String collectionPath; + @Nullable private final Expression documentIdExpr; + + private Upsert( + @Nullable Selectable[] transformedFields, + @Nullable String collectionPath, + @Nullable Expression documentIdExpr, + InternalOptions options) { + super("upsert", buildOptions(collectionPath, documentIdExpr, options)); + this.transformedFields = transformedFields; + this.collectionPath = collectionPath; + this.documentIdExpr = documentIdExpr; + } + + @BetaApi + public Upsert() { + this(null, null, null, InternalOptions.EMPTY); + } + + @BetaApi + public Upsert(Selectable... transformedFields) { + this(transformedFields, null, null, InternalOptions.EMPTY); + } + + @BetaApi + public Upsert withCollection(String collectionPath) { + return new Upsert(this.transformedFields, collectionPath, this.documentIdExpr, this.options); + } + + @BetaApi + public Upsert withDocumentId(Expression documentIdExpr) { + return new Upsert(this.transformedFields, this.collectionPath, documentIdExpr, this.options); + } + + private static InternalOptions buildOptions( + @Nullable String collectionPath, + @Nullable Expression documentIdExpr, + InternalOptions baseOptions) { + Map optsMap = new HashMap<>(baseOptions.options); + if (collectionPath != null) { + String path = collectionPath.startsWith("/") ? collectionPath : "/" + collectionPath; + optsMap.put("collection", Value.newBuilder().setReferenceValue(path).build()); + } + if (documentIdExpr != null) { + optsMap.put("document_id", PipelineUtils.encodeValue(documentIdExpr)); + } + return new InternalOptions(ImmutableMap.copyOf(optsMap)); + } + + @Override + Iterable toStageArgs() { + List args = new ArrayList<>(); + if (transformedFields != null && transformedFields.length > 0) { + Map map = PipelineUtils.selectablesToMap(transformedFields); + Map encodedMap = new HashMap<>(); + for (Map.Entry entry : map.entrySet()) { + encodedMap.put(entry.getKey(), PipelineUtils.encodeValue(entry.getValue())); + } + args.add(PipelineUtils.encodeValue(encodedMap)); + } + return args; + } +} diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java index f62e4ef83f63..34e2c5d72b3a 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java @@ -20,7 +20,9 @@ import static com.google.cloud.firestore.pipeline.expressions.Expression.field; import static com.google.common.truth.Truth.assertThat; +import com.google.cloud.firestore.pipeline.stages.PipelineExecuteOptions; import com.google.cloud.firestore.pipeline.stages.Search; +import com.google.firestore.v1.ExecutePipelineRequest; import com.google.firestore.v1.Pipeline.Stage; import com.google.firestore.v1.Value; import org.junit.Test; @@ -103,4 +105,139 @@ public void testSearchStageProtoEncoding() { Value addFields = optionsMap.get("add_fields"); assertThat(addFields.getMapValue().getFieldsMap().get("bar").getBooleanValue()).isTrue(); } + + @Test + public void testInsertStageProtoEncoding() { + FirestoreOptions options = + FirestoreOptions.newBuilder() + .setProjectId("new-project") + .setDatabaseId("(default)") + .build(); + Firestore firestore = options.getService(); + + java.util.Map data = new java.util.HashMap<>(); + data.put("title", "Test Book"); + + Pipeline pipeline = + firestore + .pipeline() + .literals(data) + .insert( + new com.google.cloud.firestore.pipeline.stages.Insert() + .withCollection("books") + .withDocumentId(constant("book1"))); + + com.google.firestore.v1.Pipeline protoPipeline = pipeline.toProto(); + assertThat(protoPipeline.getStagesCount()).isEqualTo(2); + + Stage insertStage = protoPipeline.getStages(1); + assertThat(insertStage.getName()).isEqualTo("insert"); + assertThat(insertStage.getArgsCount()).isEqualTo(0); + + java.util.Map optionsMap = insertStage.getOptionsMap(); + assertThat(optionsMap.get("collection").getReferenceValue()).isEqualTo("/books"); + assertThat(optionsMap.get("document_id").getStringValue()).isEqualTo("book1"); + } + + @Test + public void testUpsertStageProtoEncoding() { + FirestoreOptions options = + FirestoreOptions.newBuilder() + .setProjectId("new-project") + .setDatabaseId("(default)") + .build(); + Firestore firestore = options.getService(); + + java.util.Map data = new java.util.HashMap<>(); + data.put("title", "Upsert Book"); + data.put("count", 1); + + Pipeline pipeline = + firestore + .pipeline() + .literals(data) + .upsert( + new com.google.cloud.firestore.pipeline.stages.Upsert( + com.google.cloud.firestore.pipeline.expressions.Expression.add( + field("count"), constant(1)) + .as("count")) + .withCollection("books") + .withDocumentId(constant("book1"))); + + com.google.firestore.v1.Pipeline protoPipeline = pipeline.toProto(); + assertThat(protoPipeline.getStagesCount()).isEqualTo(2); + + Stage upsertStage = protoPipeline.getStages(1); + assertThat(upsertStage.getName()).isEqualTo("upsert"); + assertThat(upsertStage.getArgsCount()).isEqualTo(1); + assertThat(upsertStage.getArgs(0).getMapValue().getFieldsMap()).containsKey("count"); + + java.util.Map optionsMap = upsertStage.getOptionsMap(); + assertThat(optionsMap.get("collection").getReferenceValue()).isEqualTo("/books"); + assertThat(optionsMap.get("document_id").getStringValue()).isEqualTo("book1"); + } + + @Test + public void testAtomicExecutionOptionsConfigureNewTransactionAndAutoCommitTransaction() { + FirestoreOptions options = + FirestoreOptions.newBuilder() + .setProjectId("new-project") + .setDatabaseId("(default)") + .build(); + Firestore firestore = options.getService(); + + java.util.Map data = new java.util.HashMap<>(); + data.put("title", "Atomic Book"); + + Pipeline pipeline = + firestore + .pipeline() + .literals(data) + .insert( + new com.google.cloud.firestore.pipeline.stages.Insert() + .withCollection("books") + .withDocumentId(constant("book1"))); + + PipelineExecuteOptions executeOptions = new PipelineExecuteOptions().withAtomic(true); + ExecutePipelineRequest request = + pipeline.toExecutePipelineRequest(executeOptions, null, null); + + assertThat(request.hasNewTransaction()).isTrue(); + assertThat(request.getNewTransaction().hasReadWrite()).isTrue(); + assertThat(request.getAutoCommitTransaction()).isTrue(); + } + + @Test + public void testNonAtomicExecutionOptionsDoNotConfigureNewTransactionOrAutoCommitTransaction() { + FirestoreOptions options = + FirestoreOptions.newBuilder() + .setProjectId("new-project") + .setDatabaseId("(default)") + .build(); + Firestore firestore = options.getService(); + + java.util.Map data = new java.util.HashMap<>(); + data.put("title", "Non-Atomic Book"); + + Pipeline pipeline = + firestore + .pipeline() + .literals(data) + .insert( + new com.google.cloud.firestore.pipeline.stages.Insert() + .withCollection("books") + .withDocumentId(constant("book1"))); + + PipelineExecuteOptions executeOptionsDisabled = new PipelineExecuteOptions().withAtomic(false); + ExecutePipelineRequest requestDisabled = + pipeline.toExecutePipelineRequest(executeOptionsDisabled, null, null); + assertThat(requestDisabled.hasNewTransaction()).isFalse(); + assertThat(requestDisabled.getAutoCommitTransaction()).isFalse(); + + PipelineExecuteOptions executeOptionsDefault = new PipelineExecuteOptions(); + ExecutePipelineRequest requestDefault = + pipeline.toExecutePipelineRequest(executeOptionsDefault, null, null); + assertThat(requestDefault.hasNewTransaction()).isFalse(); + assertThat(requestDefault.getAutoCommitTransaction()).isFalse(); + } } diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java index 3af75da42fe7..bbae79fe5cd0 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java @@ -137,11 +137,13 @@ import com.google.cloud.firestore.pipeline.stages.ExplainOptions; import com.google.cloud.firestore.pipeline.stages.FindNearest; import com.google.cloud.firestore.pipeline.stages.FindNearestOptions; +import com.google.cloud.firestore.pipeline.stages.Insert; import com.google.cloud.firestore.pipeline.stages.PipelineExecuteOptions; import com.google.cloud.firestore.pipeline.stages.RawOptions; import com.google.cloud.firestore.pipeline.stages.RawStage; import com.google.cloud.firestore.pipeline.stages.Sample; import com.google.cloud.firestore.pipeline.stages.UnnestOptions; +import com.google.cloud.firestore.pipeline.stages.Upsert; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; @@ -4765,4 +4767,102 @@ public void testLiteralsWithExpressions() throws Exception { assertThat(results.get(0).getData().get("base")).isEqualTo(10L); assertThat(results.get(0).getData().get("doubled")).isEqualTo(20L); } + + @Test + public void testInsertStage() throws Exception { + CollectionReference dmlCol = firestore.collection(LocalFirestoreHelper.autoId()); + java.util.Map data = new java.util.HashMap<>(); + data.put("title", "New Book"); + data.put("author", "Author 1"); + + List results = + firestore + .pipeline() + .literals(data) + .insert(new Insert().withCollection(dmlCol.getPath()).withDocumentId(constant("newBook_insert_1"))) + .execute() + .get() + .getResults(); + + assertThat(results).hasSize(1); + assertThat(results.get(0).getData().get("documents_modified")).isEqualTo(1L); + + DocumentSnapshot snap = dmlCol.document("newBook_insert_1").get().get(); + assertThat(snap.exists()).isTrue(); + assertThat(snap.get("title")).isEqualTo("New Book"); + } + + @Test + public void testUpsertStageWithTransforms() throws Exception { + CollectionReference dmlCol = firestore.collection(LocalFirestoreHelper.autoId()); + java.util.Map data = new java.util.HashMap<>(); + data.put("title", "Upserted Book"); + data.put("count", 1); + + List results = + firestore + .pipeline() + .literals(data) + .upsert( + new Upsert(add(field("count"), constant(1)).as("count")) + .withCollection(dmlCol.getPath()) + .withDocumentId(constant("upsertBook_1"))) + .execute(new PipelineExecuteOptions().withAtomic(true)) + .get() + .getResults(); + + assertThat(results).hasSize(1); + assertThat(results.get(0).getData().get("documents_modified")).isEqualTo(1L); + + DocumentSnapshot snap = dmlCol.document("upsertBook_1").get().get(); + assertThat(snap.exists()).isTrue(); + assertThat(snap.get("title")).isEqualTo("Upserted Book"); + } + + @Test + public void testDMLWithAtomicOption() throws Exception { + CollectionReference dmlCol = firestore.collection(LocalFirestoreHelper.autoId()); + java.util.Map data = new java.util.HashMap<>(); + data.put("title", "Atomic Book"); + + List results = + firestore + .pipeline() + .literals(data) + .insert(new Insert().withCollection(dmlCol.getPath()).withDocumentId(constant("atomicBook_1"))) + .execute(new PipelineExecuteOptions().withAtomic(true)) + .get() + .getResults(); + + assertThat(results).hasSize(1); + assertThat(results.get(0).getData().get("documents_modified")).isEqualTo(1L); + + DocumentSnapshot snap = dmlCol.document("atomicBook_1").get().get(); + assertThat(snap.exists()).isTrue(); + } + + @Test + public void testInsertUpsertStagesInsideTransaction() throws Exception { + CollectionReference dmlCol = firestore.collection(LocalFirestoreHelper.autoId()); + firestore + .runTransaction( + transaction -> { + java.util.Map data = new java.util.HashMap<>(); + data.put("title", "Tx Book"); + Pipeline insertPpl = + firestore + .pipeline() + .literals(data) + .insert(new Insert().withCollection(dmlCol.getPath()).withDocumentId(constant("txBook_1"))); + List res = transaction.execute(insertPpl).get().getResults(); + assertThat(res).hasSize(1); + assertThat(res.get(0).getData().get("documents_modified")).isEqualTo(1L); + return null; + }) + .get(); + + DocumentSnapshot snap = dmlCol.document("txBook_1").get().get(); + assertThat(snap.exists()).isTrue(); + assertThat(snap.get("title")).isEqualTo("Tx Book"); + } } From bef876e75fa63ebc5ad3838267c337b1ce94f6e3 Mon Sep 17 00:00:00 2001 From: wuandy Date: Tue, 8 Sep 2026 19:27:21 +0000 Subject: [PATCH 02/18] refactor(firestore): add withDocumentIdExpression and rename documentIdExpr parameter --- .../firestore/pipeline/stages/Insert.java | 25 ++++++++----- .../firestore/pipeline/stages/Upsert.java | 25 ++++++++----- .../cloud/firestore/PipelineProtoTest.java | 37 +++++++++++++++++-- .../cloud/firestore/it/ITPipelineTest.java | 8 ++-- 4 files changed, 67 insertions(+), 28 deletions(-) diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java index 246720c0b36c..bf8602ade2a9 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java @@ -31,15 +31,15 @@ public final class Insert extends Stage { @Nullable private final String collectionPath; - @Nullable private final Expression documentIdExpr; + @Nullable private final Expression documentIdExpression; private Insert( @Nullable String collectionPath, - @Nullable Expression documentIdExpr, + @Nullable Expression documentIdExpression, InternalOptions options) { - super("insert", buildOptions(collectionPath, documentIdExpr, options)); + super("insert", buildOptions(collectionPath, documentIdExpression, options)); this.collectionPath = collectionPath; - this.documentIdExpr = documentIdExpr; + this.documentIdExpression = documentIdExpression; } @BetaApi @@ -49,25 +49,30 @@ public Insert() { @BetaApi public Insert withCollection(String collectionPath) { - return new Insert(collectionPath, this.documentIdExpr, this.options); + return new Insert(collectionPath, this.documentIdExpression, this.options); } @BetaApi - public Insert withDocumentId(Expression documentIdExpr) { - return new Insert(this.collectionPath, documentIdExpr, this.options); + public Insert withDocumentIdExpression(Expression documentIdExpression) { + return new Insert(this.collectionPath, documentIdExpression, this.options); + } + + @BetaApi + public Insert withDocumentId(Expression documentIdExpression) { + return withDocumentIdExpression(documentIdExpression); } private static InternalOptions buildOptions( @Nullable String collectionPath, - @Nullable Expression documentIdExpr, + @Nullable Expression documentIdExpression, InternalOptions baseOptions) { Map optsMap = new HashMap<>(baseOptions.options); if (collectionPath != null) { String path = collectionPath.startsWith("/") ? collectionPath : "/" + collectionPath; optsMap.put("collection", Value.newBuilder().setReferenceValue(path).build()); } - if (documentIdExpr != null) { - optsMap.put("document_id", PipelineUtils.encodeValue(documentIdExpr)); + if (documentIdExpression != null) { + optsMap.put("document_id", PipelineUtils.encodeValue(documentIdExpression)); } return new InternalOptions(ImmutableMap.copyOf(optsMap)); } diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java index e74e44b2ce30..194f938eb281 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java @@ -34,17 +34,17 @@ public final class Upsert extends Stage { @Nullable private final Selectable[] transformedFields; @Nullable private final String collectionPath; - @Nullable private final Expression documentIdExpr; + @Nullable private final Expression documentIdExpression; private Upsert( @Nullable Selectable[] transformedFields, @Nullable String collectionPath, - @Nullable Expression documentIdExpr, + @Nullable Expression documentIdExpression, InternalOptions options) { - super("upsert", buildOptions(collectionPath, documentIdExpr, options)); + super("upsert", buildOptions(collectionPath, documentIdExpression, options)); this.transformedFields = transformedFields; this.collectionPath = collectionPath; - this.documentIdExpr = documentIdExpr; + this.documentIdExpression = documentIdExpression; } @BetaApi @@ -59,25 +59,30 @@ public Upsert(Selectable... transformedFields) { @BetaApi public Upsert withCollection(String collectionPath) { - return new Upsert(this.transformedFields, collectionPath, this.documentIdExpr, this.options); + return new Upsert(this.transformedFields, collectionPath, this.documentIdExpression, this.options); } @BetaApi - public Upsert withDocumentId(Expression documentIdExpr) { - return new Upsert(this.transformedFields, this.collectionPath, documentIdExpr, this.options); + public Upsert withDocumentIdExpression(Expression documentIdExpression) { + return new Upsert(this.transformedFields, this.collectionPath, documentIdExpression, this.options); + } + + @BetaApi + public Upsert withDocumentId(Expression documentIdExpression) { + return withDocumentIdExpression(documentIdExpression); } private static InternalOptions buildOptions( @Nullable String collectionPath, - @Nullable Expression documentIdExpr, + @Nullable Expression documentIdExpression, InternalOptions baseOptions) { Map optsMap = new HashMap<>(baseOptions.options); if (collectionPath != null) { String path = collectionPath.startsWith("/") ? collectionPath : "/" + collectionPath; optsMap.put("collection", Value.newBuilder().setReferenceValue(path).build()); } - if (documentIdExpr != null) { - optsMap.put("document_id", PipelineUtils.encodeValue(documentIdExpr)); + if (documentIdExpression != null) { + optsMap.put("document_id", PipelineUtils.encodeValue(documentIdExpression)); } return new InternalOptions(ImmutableMap.copyOf(optsMap)); } diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java index 34e2c5d72b3a..7de846ee1017 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java @@ -125,7 +125,7 @@ public void testInsertStageProtoEncoding() { .insert( new com.google.cloud.firestore.pipeline.stages.Insert() .withCollection("books") - .withDocumentId(constant("book1"))); + .withDocumentIdExpression(constant("book1"))); com.google.firestore.v1.Pipeline protoPipeline = pipeline.toProto(); assertThat(protoPipeline.getStagesCount()).isEqualTo(2); @@ -137,6 +137,19 @@ public void testInsertStageProtoEncoding() { java.util.Map optionsMap = insertStage.getOptionsMap(); assertThat(optionsMap.get("collection").getReferenceValue()).isEqualTo("/books"); assertThat(optionsMap.get("document_id").getStringValue()).isEqualTo("book1"); + + // Test backward compatibility with withDocumentId + Pipeline pipelineDeprecated = + firestore + .pipeline() + .literals(data) + .insert( + new com.google.cloud.firestore.pipeline.stages.Insert() + .withCollection("books") + .withDocumentId(constant("book1"))); + Stage insertStageDeprecated = pipelineDeprecated.toProto().getStages(1); + assertThat(insertStageDeprecated.getOptionsMap().get("document_id").getStringValue()) + .isEqualTo("book1"); } @Test @@ -162,7 +175,7 @@ public void testUpsertStageProtoEncoding() { field("count"), constant(1)) .as("count")) .withCollection("books") - .withDocumentId(constant("book1"))); + .withDocumentIdExpression(constant("book1"))); com.google.firestore.v1.Pipeline protoPipeline = pipeline.toProto(); assertThat(protoPipeline.getStagesCount()).isEqualTo(2); @@ -175,6 +188,22 @@ public void testUpsertStageProtoEncoding() { java.util.Map optionsMap = upsertStage.getOptionsMap(); assertThat(optionsMap.get("collection").getReferenceValue()).isEqualTo("/books"); assertThat(optionsMap.get("document_id").getStringValue()).isEqualTo("book1"); + + // Test backward compatibility with withDocumentId + Pipeline pipelineDeprecated = + firestore + .pipeline() + .literals(data) + .upsert( + new com.google.cloud.firestore.pipeline.stages.Upsert( + com.google.cloud.firestore.pipeline.expressions.Expression.add( + field("count"), constant(1)) + .as("count")) + .withCollection("books") + .withDocumentId(constant("book1"))); + Stage upsertStageDeprecated = pipelineDeprecated.toProto().getStages(1); + assertThat(upsertStageDeprecated.getOptionsMap().get("document_id").getStringValue()) + .isEqualTo("book1"); } @Test @@ -196,7 +225,7 @@ public void testAtomicExecutionOptionsConfigureNewTransactionAndAutoCommitTransa .insert( new com.google.cloud.firestore.pipeline.stages.Insert() .withCollection("books") - .withDocumentId(constant("book1"))); + .withDocumentIdExpression(constant("book1"))); PipelineExecuteOptions executeOptions = new PipelineExecuteOptions().withAtomic(true); ExecutePipelineRequest request = @@ -226,7 +255,7 @@ public void testNonAtomicExecutionOptionsDoNotConfigureNewTransactionOrAutoCommi .insert( new com.google.cloud.firestore.pipeline.stages.Insert() .withCollection("books") - .withDocumentId(constant("book1"))); + .withDocumentIdExpression(constant("book1"))); PipelineExecuteOptions executeOptionsDisabled = new PipelineExecuteOptions().withAtomic(false); ExecutePipelineRequest requestDisabled = diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java index bbae79fe5cd0..fc90e3df74dd 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java @@ -4779,7 +4779,7 @@ public void testInsertStage() throws Exception { firestore .pipeline() .literals(data) - .insert(new Insert().withCollection(dmlCol.getPath()).withDocumentId(constant("newBook_insert_1"))) + .insert(new Insert().withCollection(dmlCol.getPath()).withDocumentIdExpression(constant("newBook_insert_1"))) .execute() .get() .getResults(); @@ -4806,7 +4806,7 @@ public void testUpsertStageWithTransforms() throws Exception { .upsert( new Upsert(add(field("count"), constant(1)).as("count")) .withCollection(dmlCol.getPath()) - .withDocumentId(constant("upsertBook_1"))) + .withDocumentIdExpression(constant("upsertBook_1"))) .execute(new PipelineExecuteOptions().withAtomic(true)) .get() .getResults(); @@ -4829,7 +4829,7 @@ public void testDMLWithAtomicOption() throws Exception { firestore .pipeline() .literals(data) - .insert(new Insert().withCollection(dmlCol.getPath()).withDocumentId(constant("atomicBook_1"))) + .insert(new Insert().withCollection(dmlCol.getPath()).withDocumentIdExpression(constant("atomicBook_1"))) .execute(new PipelineExecuteOptions().withAtomic(true)) .get() .getResults(); @@ -4853,7 +4853,7 @@ public void testInsertUpsertStagesInsideTransaction() throws Exception { firestore .pipeline() .literals(data) - .insert(new Insert().withCollection(dmlCol.getPath()).withDocumentId(constant("txBook_1"))); + .insert(new Insert().withCollection(dmlCol.getPath()).withDocumentIdExpression(constant("txBook_1"))); List res = transaction.execute(insertPpl).get().getResults(); assertThat(res).hasSize(1); assertThat(res.get(0).getData().get("documents_modified")).isEqualTo(1L); From 3bf07e39e491735f3063865e11c078bde5620a72 Mon Sep 17 00:00:00 2001 From: wuandy Date: Tue, 8 Sep 2026 20:15:25 +0000 Subject: [PATCH 03/18] refactor(firestore): add withAdditionalFields and rename transformedFields in Upsert --- .../com/google/cloud/firestore/Pipeline.java | 4 +- .../firestore/pipeline/stages/Upsert.java | 56 +++++++++++++++---- .../cloud/firestore/PipelineProtoTest.java | 37 +++++++++++- 3 files changed, 84 insertions(+), 13 deletions(-) diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java index 5c9b84ab7a2a..d959560eac86 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java @@ -1320,8 +1320,8 @@ public Pipeline upsert(Upsert upsert) { } @BetaApi - public Pipeline upsert(Selectable... transformedFields) { - return append(new Upsert(transformedFields)); + public Pipeline upsert(Selectable... additionalFields) { + return append(new Upsert(additionalFields)); } /** diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java index 194f938eb281..48eb95f1f30c 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java @@ -21,6 +21,7 @@ import com.google.cloud.firestore.PipelineUtils; import com.google.cloud.firestore.pipeline.expressions.Expression; import com.google.cloud.firestore.pipeline.expressions.Selectable; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.firestore.v1.Value; import java.util.ArrayList; @@ -32,39 +33,73 @@ @InternalApi public final class Upsert extends Stage { - @Nullable private final Selectable[] transformedFields; + @Nullable private final ImmutableList additionalFields; @Nullable private final String collectionPath; @Nullable private final Expression documentIdExpression; private Upsert( - @Nullable Selectable[] transformedFields, + @Nullable ImmutableList additionalFields, @Nullable String collectionPath, @Nullable Expression documentIdExpression, InternalOptions options) { super("upsert", buildOptions(collectionPath, documentIdExpression, options)); - this.transformedFields = transformedFields; + this.additionalFields = additionalFields; this.collectionPath = collectionPath; this.documentIdExpression = documentIdExpression; } @BetaApi public Upsert() { - this(null, null, null, InternalOptions.EMPTY); + this((ImmutableList) null, null, null, InternalOptions.EMPTY); } @BetaApi - public Upsert(Selectable... transformedFields) { - this(transformedFields, null, null, InternalOptions.EMPTY); + public Upsert(Selectable... additionalFields) { + this( + additionalFields != null ? ImmutableList.copyOf(additionalFields) : null, + null, + null, + InternalOptions.EMPTY); + } + + @BetaApi + public Upsert withAdditionalFields(Selectable... additionalFields) { + return new Upsert( + ImmutableList.copyOf(additionalFields), + this.collectionPath, + this.documentIdExpression, + this.options); + } + + @BetaApi + public Upsert withAdditionalFields(List additionalFields) { + return new Upsert( + ImmutableList.copyOf(additionalFields), + this.collectionPath, + this.documentIdExpression, + this.options); + } + + @BetaApi + public Upsert withTransformedFields(Selectable... transformedFields) { + return withAdditionalFields(transformedFields); + } + + @BetaApi + public Upsert withTransformedFields(List transformedFields) { + return withAdditionalFields(transformedFields); } @BetaApi public Upsert withCollection(String collectionPath) { - return new Upsert(this.transformedFields, collectionPath, this.documentIdExpression, this.options); + return new Upsert( + this.additionalFields, collectionPath, this.documentIdExpression, this.options); } @BetaApi public Upsert withDocumentIdExpression(Expression documentIdExpression) { - return new Upsert(this.transformedFields, this.collectionPath, documentIdExpression, this.options); + return new Upsert( + this.additionalFields, this.collectionPath, documentIdExpression, this.options); } @BetaApi @@ -90,8 +125,9 @@ private static InternalOptions buildOptions( @Override Iterable toStageArgs() { List args = new ArrayList<>(); - if (transformedFields != null && transformedFields.length > 0) { - Map map = PipelineUtils.selectablesToMap(transformedFields); + if (additionalFields != null && !additionalFields.isEmpty()) { + Map map = + PipelineUtils.selectablesToMap(additionalFields.toArray(new Selectable[0])); Map encodedMap = new HashMap<>(); for (Map.Entry entry : map.entrySet()) { encodedMap.put(entry.getKey(), PipelineUtils.encodeValue(entry.getValue())); diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java index 7de846ee1017..a19778161573 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java @@ -170,7 +170,8 @@ public void testUpsertStageProtoEncoding() { .pipeline() .literals(data) .upsert( - new com.google.cloud.firestore.pipeline.stages.Upsert( + new com.google.cloud.firestore.pipeline.stages.Upsert() + .withAdditionalFields( com.google.cloud.firestore.pipeline.expressions.Expression.add( field("count"), constant(1)) .as("count")) @@ -189,6 +190,40 @@ public void testUpsertStageProtoEncoding() { assertThat(optionsMap.get("collection").getReferenceValue()).isEqualTo("/books"); assertThat(optionsMap.get("document_id").getStringValue()).isEqualTo("book1"); + // Test withAdditionalFields with List + Pipeline pipelineWithList = + firestore + .pipeline() + .literals(data) + .upsert( + new com.google.cloud.firestore.pipeline.stages.Upsert() + .withAdditionalFields( + java.util.Collections.singletonList( + com.google.cloud.firestore.pipeline.expressions.Expression.add( + field("count"), constant(1)) + .as("count"))) + .withCollection("books") + .withDocumentIdExpression(constant("book1"))); + Stage upsertStageWithList = pipelineWithList.toProto().getStages(1); + assertThat(upsertStageWithList.getArgs(0).getMapValue().getFieldsMap()).containsKey("count"); + + // Test backward compatibility with withTransformedFields + Pipeline pipelineWithTransformedFields = + firestore + .pipeline() + .literals(data) + .upsert( + new com.google.cloud.firestore.pipeline.stages.Upsert() + .withTransformedFields( + com.google.cloud.firestore.pipeline.expressions.Expression.add( + field("count"), constant(1)) + .as("count")) + .withCollection("books") + .withDocumentIdExpression(constant("book1"))); + Stage upsertStageWithTransformedFields = pipelineWithTransformedFields.toProto().getStages(1); + assertThat(upsertStageWithTransformedFields.getArgs(0).getMapValue().getFieldsMap()) + .containsKey("count"); + // Test backward compatibility with withDocumentId Pipeline pipelineDeprecated = firestore From 194fec0658f8a4a3e0a35a6f64ed03dc02f682b0 Mon Sep 17 00:00:00 2001 From: wuandy Date: Fri, 2 Oct 2026 14:07:30 +0000 Subject: [PATCH 04/18] test(firestore): expand pipeline DML unit and integration test coverage to parity --- .../com/google/cloud/firestore/Pipeline.java | 6 +- .../stages/PipelineExecuteOptions.java | 15 +- .../firestore/pipeline/stages/Upsert.java | 2 + .../cloud/firestore/PipelineProtoTest.java | 165 ++++++++- .../cloud/firestore/it/ITPipelineTest.java | 326 +++++++++++++++++- 5 files changed, 503 insertions(+), 11 deletions(-) diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java index d959560eac86..124605dd34c4 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java @@ -1531,13 +1531,17 @@ ExecutePipelineRequest toExecutePipelineRequest( @Nonnull PipelineExecuteOptions options, @Nullable final ByteString transactionId, @Nullable com.google.protobuf.Timestamp readTime) { + java.util.Map optionsMap = + new java.util.HashMap<>(StageUtils.toMap(options)); + optionsMap.remove("atomic"); + ExecutePipelineRequest.Builder request = ExecutePipelineRequest.newBuilder() .setDatabase(rpcContext != null ? rpcContext.getDatabaseName() : "") .setStructuredPipeline( StructuredPipeline.newBuilder() .setPipeline(toProto()) - .putAllOptions(StageUtils.toMap(options)) + .putAllOptions(optionsMap) .build()); if (transactionId != null) { diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/PipelineExecuteOptions.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/PipelineExecuteOptions.java index ef3630251240..edb84515e043 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/PipelineExecuteOptions.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/PipelineExecuteOptions.java @@ -17,18 +17,24 @@ package com.google.cloud.firestore.pipeline.stages; public final class PipelineExecuteOptions extends AbstractOptions { + private final boolean atomic; public PipelineExecuteOptions() { - super(InternalOptions.EMPTY); + this(InternalOptions.EMPTY, false); } PipelineExecuteOptions(InternalOptions options) { + this(options, false); + } + + PipelineExecuteOptions(InternalOptions options, boolean atomic) { super(options); + this.atomic = atomic; } @Override PipelineExecuteOptions self(InternalOptions options) { - return new PipelineExecuteOptions(options); + return new PipelineExecuteOptions(options, this.atomic); } public PipelineExecuteOptions withExplainOptions(ExplainOptions options) { @@ -40,11 +46,10 @@ public PipelineExecuteOptions withIndexMode(String indexMode) { } public PipelineExecuteOptions withAtomic(boolean atomic) { - return with("atomic", atomic); + return new PipelineExecuteOptions(this.options, atomic); } public boolean isAtomic() { - return options.options.containsKey("atomic") - && options.options.get("atomic").getBooleanValue(); + return this.atomic; } } diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java index 48eb95f1f30c..3a2264027d6c 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java @@ -133,6 +133,8 @@ Iterable toStageArgs() { encodedMap.put(entry.getKey(), PipelineUtils.encodeValue(entry.getValue())); } args.add(PipelineUtils.encodeValue(encodedMap)); + } else { + args.add(PipelineUtils.encodeValue(new HashMap())); } return args; } diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java index a19778161573..6508b0a63346 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java @@ -16,12 +16,15 @@ package com.google.cloud.firestore; +import static com.google.cloud.firestore.pipeline.expressions.Expression.add; import static com.google.cloud.firestore.pipeline.expressions.Expression.constant; import static com.google.cloud.firestore.pipeline.expressions.Expression.field; import static com.google.common.truth.Truth.assertThat; +import com.google.cloud.firestore.pipeline.stages.Insert; import com.google.cloud.firestore.pipeline.stages.PipelineExecuteOptions; import com.google.cloud.firestore.pipeline.stages.Search; +import com.google.cloud.firestore.pipeline.stages.Upsert; import com.google.firestore.v1.ExecutePipelineRequest; import com.google.firestore.v1.Pipeline.Stage; import com.google.firestore.v1.Value; @@ -263,12 +266,12 @@ public void testAtomicExecutionOptionsConfigureNewTransactionAndAutoCommitTransa .withDocumentIdExpression(constant("book1"))); PipelineExecuteOptions executeOptions = new PipelineExecuteOptions().withAtomic(true); - ExecutePipelineRequest request = - pipeline.toExecutePipelineRequest(executeOptions, null, null); + ExecutePipelineRequest request = pipeline.toExecutePipelineRequest(executeOptions, null, null); assertThat(request.hasNewTransaction()).isTrue(); assertThat(request.getNewTransaction().hasReadWrite()).isTrue(); assertThat(request.getAutoCommitTransaction()).isTrue(); + assertThat(request.getStructuredPipeline().getOptionsMap()).doesNotContainKey("atomic"); } @Test @@ -297,11 +300,169 @@ public void testNonAtomicExecutionOptionsDoNotConfigureNewTransactionOrAutoCommi pipeline.toExecutePipelineRequest(executeOptionsDisabled, null, null); assertThat(requestDisabled.hasNewTransaction()).isFalse(); assertThat(requestDisabled.getAutoCommitTransaction()).isFalse(); + assertThat(requestDisabled.getStructuredPipeline().getOptionsMap()).doesNotContainKey("atomic"); PipelineExecuteOptions executeOptionsDefault = new PipelineExecuteOptions(); ExecutePipelineRequest requestDefault = pipeline.toExecutePipelineRequest(executeOptionsDefault, null, null); assertThat(requestDefault.hasNewTransaction()).isFalse(); assertThat(requestDefault.getAutoCommitTransaction()).isFalse(); + assertThat(requestDefault.getStructuredPipeline().getOptionsMap()).doesNotContainKey("atomic"); + } + + @Test + public void testDeleteStageProtoEncoding() { + FirestoreOptions options = + FirestoreOptions.newBuilder() + .setProjectId("new-project") + .setDatabaseId("(default)") + .build(); + Firestore firestore = options.getService(); + + Pipeline pipeline = firestore.pipeline().collection("books").delete(); + + com.google.firestore.v1.Pipeline protoPipeline = pipeline.toProto(); + assertThat(protoPipeline.getStagesCount()).isEqualTo(2); + + Stage deleteStage = protoPipeline.getStages(1); + assertThat(deleteStage.getName()).isEqualTo("delete"); + assertThat(deleteStage.getArgsCount()).isEqualTo(0); + assertThat(deleteStage.getOptionsCount()).isEqualTo(0); + } + + @Test + public void testUpdateStageProtoEncoding() { + FirestoreOptions options = + FirestoreOptions.newBuilder() + .setProjectId("new-project") + .setDatabaseId("(default)") + .build(); + Firestore firestore = options.getService(); + + // 0-arg update() produces 1 empty MapValue arg and 0 options + Pipeline pipeline0 = firestore.pipeline().collection("books").update(); + com.google.firestore.v1.Pipeline protoPipeline0 = pipeline0.toProto(); + assertThat(protoPipeline0.getStagesCount()).isEqualTo(2); + + Stage updateStage0 = protoPipeline0.getStages(1); + assertThat(updateStage0.getName()).isEqualTo("update"); + assertThat(updateStage0.getArgsCount()).isEqualTo(1); + assertThat(updateStage0.getArgs(0).hasMapValue()).isTrue(); + assertThat(updateStage0.getArgs(0).getMapValue().getFieldsCount()).isEqualTo(0); + assertThat(updateStage0.getOptionsCount()).isEqualTo(0); + + // update with vararg Selectables produces 1 MapValue arg with field mappings and 0 options + Pipeline pipeline = + firestore + .pipeline() + .collection("books") + .update(constant("Updated").as("status"), add(field("count"), constant(1)).as("count")); + com.google.firestore.v1.Pipeline protoPipeline = pipeline.toProto(); + assertThat(protoPipeline.getStagesCount()).isEqualTo(2); + + Stage updateStage = protoPipeline.getStages(1); + assertThat(updateStage.getName()).isEqualTo("update"); + assertThat(updateStage.getArgsCount()).isEqualTo(1); + assertThat(updateStage.getArgs(0).hasMapValue()).isTrue(); + java.util.Map fieldsMap = updateStage.getArgs(0).getMapValue().getFieldsMap(); + assertThat(fieldsMap).containsKey("status"); + assertThat(fieldsMap.get("status").getStringValue()).isEqualTo("Updated"); + assertThat(fieldsMap).containsKey("count"); + assertThat(updateStage.getOptionsCount()).isEqualTo(0); + } + + @Test + public void testInsertStageWithoutDocumentIdProtoEncoding() { + FirestoreOptions options = + FirestoreOptions.newBuilder() + .setProjectId("new-project") + .setDatabaseId("(default)") + .build(); + Firestore firestore = options.getService(); + + java.util.Map data = new java.util.HashMap<>(); + data.put("title", "Auto ID Book"); + + Pipeline pipeline = + firestore.pipeline().literals(data).insert(new Insert().withCollection("books")); + + com.google.firestore.v1.Pipeline protoPipeline = pipeline.toProto(); + assertThat(protoPipeline.getStagesCount()).isEqualTo(2); + + Stage insertStage = protoPipeline.getStages(1); + assertThat(insertStage.getName()).isEqualTo("insert"); + assertThat(insertStage.getArgsCount()).isEqualTo(0); + + java.util.Map optionsMap = insertStage.getOptionsMap(); + assertThat(optionsMap.get("collection").getReferenceValue()).isEqualTo("/books"); + assertThat(optionsMap).doesNotContainKey("document_id"); + } + + @Test + public void testUpsertStageWithoutAdditionalFieldsProtoEncoding() { + FirestoreOptions options = + FirestoreOptions.newBuilder() + .setProjectId("new-project") + .setDatabaseId("(default)") + .build(); + Firestore firestore = options.getService(); + + java.util.Map data = new java.util.HashMap<>(); + data.put("title", "Upsert Book"); + + Pipeline pipeline = + firestore + .pipeline() + .literals(data) + .upsert( + new Upsert().withCollection("books").withDocumentIdExpression(constant("book1"))); + + com.google.firestore.v1.Pipeline protoPipeline = pipeline.toProto(); + assertThat(protoPipeline.getStagesCount()).isEqualTo(2); + + Stage upsertStage = protoPipeline.getStages(1); + assertThat(upsertStage.getName()).isEqualTo("upsert"); + assertThat(upsertStage.getArgsCount()).isEqualTo(1); + assertThat(upsertStage.getArgs(0).hasMapValue()).isTrue(); + assertThat(upsertStage.getArgs(0).getMapValue().getFieldsCount()).isEqualTo(0); + + java.util.Map optionsMap = upsertStage.getOptionsMap(); + assertThat(optionsMap.get("collection").getReferenceValue()).isEqualTo("/books"); + assertThat(optionsMap.get("document_id").getStringValue()).isEqualTo("book1"); + } + + @Test + public void testLiteralsStageProtoEncoding() { + FirestoreOptions options = + FirestoreOptions.newBuilder() + .setProjectId("new-project") + .setDatabaseId("(default)") + .build(); + Firestore firestore = options.getService(); + + java.util.Map doc1 = new java.util.HashMap<>(); + doc1.put("title", "Book 1"); + doc1.put("author", "Author 1"); + + java.util.Map doc2 = new java.util.HashMap<>(); + doc2.put("title", "Book 2"); + doc2.put("calc", constant(42)); + + Pipeline pipeline = firestore.pipeline().literals(doc1, doc2); + com.google.firestore.v1.Pipeline protoPipeline = pipeline.toProto(); + assertThat(protoPipeline.getStagesCount()).isEqualTo(1); + + Stage literalsStage = protoPipeline.getStages(0); + assertThat(literalsStage.getName()).isEqualTo("literals"); + assertThat(literalsStage.getArgsCount()).isEqualTo(2); + assertThat(literalsStage.getOptionsCount()).isEqualTo(0); + + java.util.Map doc1Fields = literalsStage.getArgs(0).getMapValue().getFieldsMap(); + assertThat(doc1Fields.get("title").getStringValue()).isEqualTo("Book 1"); + assertThat(doc1Fields.get("author").getStringValue()).isEqualTo("Author 1"); + + java.util.Map doc2Fields = literalsStage.getArgs(1).getMapValue().getFieldsMap(); + assertThat(doc2Fields.get("title").getStringValue()).isEqualTo("Book 2"); + assertThat(doc2Fields.get("calc").getIntegerValue()).isEqualTo(42L); } } diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java index fc90e3df74dd..b36658f155fc 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java @@ -126,6 +126,7 @@ import com.google.cloud.firestore.LocalFirestoreHelper; import com.google.cloud.firestore.Pipeline; import com.google.cloud.firestore.PipelineResult; +import com.google.cloud.firestore.QuerySnapshot; import com.google.cloud.firestore.pipeline.expressions.AggregateFunction; import com.google.cloud.firestore.pipeline.expressions.Expression; import com.google.cloud.firestore.pipeline.expressions.Field; @@ -4779,7 +4780,10 @@ public void testInsertStage() throws Exception { firestore .pipeline() .literals(data) - .insert(new Insert().withCollection(dmlCol.getPath()).withDocumentIdExpression(constant("newBook_insert_1"))) + .insert( + new Insert() + .withCollection(dmlCol.getPath()) + .withDocumentIdExpression(constant("newBook_insert_1"))) .execute() .get() .getResults(); @@ -4829,7 +4833,10 @@ public void testDMLWithAtomicOption() throws Exception { firestore .pipeline() .literals(data) - .insert(new Insert().withCollection(dmlCol.getPath()).withDocumentIdExpression(constant("atomicBook_1"))) + .insert( + new Insert() + .withCollection(dmlCol.getPath()) + .withDocumentIdExpression(constant("atomicBook_1"))) .execute(new PipelineExecuteOptions().withAtomic(true)) .get() .getResults(); @@ -4853,7 +4860,10 @@ public void testInsertUpsertStagesInsideTransaction() throws Exception { firestore .pipeline() .literals(data) - .insert(new Insert().withCollection(dmlCol.getPath()).withDocumentIdExpression(constant("txBook_1"))); + .insert( + new Insert() + .withCollection(dmlCol.getPath()) + .withDocumentIdExpression(constant("txBook_1"))); List res = transaction.execute(insertPpl).get().getResults(); assertThat(res).hasSize(1); assertThat(res.get(0).getData().get("documents_modified")).isEqualTo(1L); @@ -4865,4 +4875,314 @@ public void testInsertUpsertStagesInsideTransaction() throws Exception { assertThat(snap.exists()).isTrue(); assertThat(snap.get("title")).isEqualTo("Tx Book"); } + + @Test + public void testDeleteNonExistingDocument() throws Exception { + CollectionReference dmlCol = firestore.collection(LocalFirestoreHelper.autoId()); + List results = + firestore + .pipeline() + .collection(dmlCol.getPath()) + .where(equal(field("__name__").documentId(), constant("nonExistingId_999"))) + .delete() + .execute() + .get() + .getResults(); + + assertThat(results).hasSize(1); + assertThat(results.get(0).getData().get("documents_modified")).isEqualTo(0L); + } + + @Test + public void testDeleteWithAtomicTrue() throws Exception { + CollectionReference dmlCol = testCollectionWithDocs(bookDocs); + List results = + firestore + .pipeline() + .collection(dmlCol.getPath()) + .where(equal(field("__name__").documentId(), constant("book1"))) + .delete() + .execute(new PipelineExecuteOptions().withAtomic(true)) + .get() + .getResults(); + + assertThat(results).hasSize(1); + assertThat(results.get(0).getData().get("documents_modified")).isEqualTo(1L); + assertThat(dmlCol.document("book1").get().get().exists()).isFalse(); + } + + @Test + public void testDeleteWithAtomicFalse() throws Exception { + CollectionReference dmlCol = testCollectionWithDocs(bookDocs); + List results = + firestore + .pipeline() + .collection(dmlCol.getPath()) + .where(equal(field("__name__").documentId(), constant("book2"))) + .delete() + .execute(new PipelineExecuteOptions().withAtomic(false)) + .get() + .getResults(); + + assertThat(results).hasSize(1); + assertThat(results.get(0).getData().get("documents_modified")).isEqualTo(1L); + assertThat(dmlCol.document("book2").get().get().exists()).isFalse(); + } + + @Test + public void testDeleteStageInsideTransaction() throws Exception { + CollectionReference dmlCol = testCollectionWithDocs(bookDocs); + firestore + .runTransaction( + transaction -> { + Pipeline deletePpl = + firestore + .pipeline() + .collection(dmlCol.getPath()) + .where(equal(field("__name__").documentId(), constant("book1"))) + .delete(); + List res = transaction.execute(deletePpl).get().getResults(); + assertThat(res).hasSize(1); + assertThat(res.get(0).getData().get("documents_modified")).isEqualTo(1L); + return null; + }) + .get(); + + assertThat(dmlCol.document("book1").get().get().exists()).isFalse(); + } + + @Test + public void testUpdateSingleDocWithAddFields() throws Exception { + CollectionReference dmlCol = testCollectionWithDocs(bookDocs); + List results = + firestore + .pipeline() + .collection(dmlCol.getPath()) + .where(equal(field("__name__").documentId(), constant("book3"))) + .addFields(field("__name__").documentId().as("id")) + .update(constant("baz").as("foo")) + .execute() + .get() + .getResults(); + + assertThat(results).hasSize(1); + assertThat(results.get(0).getData().get("documents_modified")).isEqualTo(1L); + DocumentSnapshot snap = dmlCol.document("book3").get().get(); + assertThat(snap.get("foo")).isEqualTo("baz"); + assertThat(snap.get("id")).isEqualTo("book3"); + } + + @Test + public void testUpdateWithMultipleVarargExpressions() throws Exception { + CollectionReference dmlCol = testCollectionWithDocs(bookDocs); + List results = + firestore + .pipeline() + .collection(dmlCol.getPath()) + .where(equal(field("__name__").documentId(), constant("book1"))) + .update(constant("UpdatedMulti").as("status"), constant(99L).as("newField")) + .execute() + .get() + .getResults(); + + assertThat(results).hasSize(1); + assertThat(results.get(0).getData().get("documents_modified")).isEqualTo(1L); + DocumentSnapshot snap = dmlCol.document("book1").get().get(); + assertThat(snap.get("status")).isEqualTo("UpdatedMulti"); + assertThat(snap.get("newField")).isEqualTo(99L); + } + + @Test + public void testUpdateAtomically() throws Exception { + CollectionReference dmlCol = testCollectionWithDocs(bookDocs); + List results = + firestore + .pipeline() + .collection(dmlCol.getPath()) + .where(equal(field("__name__").documentId(), constant("book1"))) + .update(constant("AtomicUpdate").as("status")) + .execute(new PipelineExecuteOptions().withAtomic(true)) + .get() + .getResults(); + + assertThat(results).hasSize(1); + assertThat(results.get(0).getData().get("documents_modified")).isEqualTo(1L); + DocumentSnapshot snap = dmlCol.document("book1").get().get(); + assertThat(snap.get("status")).isEqualTo("AtomicUpdate"); + } + + @Test + public void testUpdateStageInsideTransaction() throws Exception { + CollectionReference dmlCol = testCollectionWithDocs(bookDocs); + firestore + .runTransaction( + transaction -> { + Pipeline updatePpl = + firestore + .pipeline() + .collection(dmlCol.getPath()) + .where(equal(field("__name__").documentId(), constant("book1"))) + .update(constant("TxUpdate").as("status")); + List res = transaction.execute(updatePpl).get().getResults(); + assertThat(res).hasSize(1); + assertThat(res.get(0).getData().get("documents_modified")).isEqualTo(1L); + return null; + }) + .get(); + + assertThat(dmlCol.document("book1").get().get().get("status")).isEqualTo("TxUpdate"); + } + + @Test + public void testInsertAutoGeneratedId() throws Exception { + CollectionReference dmlCol = testCollectionWithDocs(bookDocs); + List results = + firestore + .pipeline() + .collection(dmlCol.getPath()) + .where(equal(field("__name__").documentId(), constant("book1"))) + .removeFields("__name__") + .insert(new Insert().withCollection(dmlCol.getPath())) + .execute(new PipelineExecuteOptions().withAtomic(true)) + .get() + .getResults(); + + assertThat(results).hasSize(1); + assertThat(results.get(0).getData().get("documents_modified")).isEqualTo(1L); + QuerySnapshot allDocs = dmlCol.get().get(); + assertThat(allDocs.size()).isEqualTo(bookDocs.size() + 1); + } + + @Test + public void testInsertFailsWhenDocumentAlreadyExists() throws Exception { + CollectionReference dmlCol = testCollectionWithDocs(bookDocs); + assertThrows( + Exception.class, + () -> + firestore + .pipeline() + .collection(dmlCol.getPath()) + .where(equal(field("__name__").documentId(), constant("book1"))) + .insert( + new Insert() + .withCollection(dmlCol.getPath()) + .withDocumentIdExpression(constant("book2"))) + .execute(new PipelineExecuteOptions().withAtomic(true)) + .get()); + } + + @Test + public void testInsertIntoDifferentCollection() throws Exception { + CollectionReference sourceCol = testCollectionWithDocs(bookDocs); + CollectionReference targetCol = firestore.collection(LocalFirestoreHelper.autoId()); + List results = + firestore + .pipeline() + .collection(sourceCol.getPath()) + .where(equal(field("__name__").documentId(), constant("book1"))) + .insert(new Insert().withCollection(targetCol.getPath())) + .execute(new PipelineExecuteOptions().withAtomic(true)) + .get() + .getResults(); + + assertThat(results).hasSize(1); + assertThat(results.get(0).getData().get("documents_modified")).isEqualTo(1L); + QuerySnapshot targetDocs = targetCol.get().get(); + assertThat(targetDocs.size()).isEqualTo(1); + assertThat(targetDocs.getDocuments().get(0).getString("title")) + .isEqualTo("The Hitchhiker's Guide to the Galaxy"); + } + + @Test + public void testUpsertInPlaceWithVarargs() throws Exception { + CollectionReference dmlCol = testCollectionWithDocs(bookDocs); + List results = + firestore + .pipeline() + .collection(dmlCol.getPath()) + .where(equal(field("__name__").documentId(), constant("book1"))) + .upsert( + constant("Comedy Sci-Fi").as("genre"), + add(field("rating"), constant(0.5)).as("rating")) + .execute(new PipelineExecuteOptions().withAtomic(true)) + .get() + .getResults(); + + assertThat(results).hasSize(1); + assertThat(results.get(0).getData().get("documents_modified")).isEqualTo(1L); + DocumentSnapshot snap = dmlCol.document("book1").get().get(); + assertThat(snap.get("genre")).isEqualTo("Comedy Sci-Fi"); + assertThat(snap.get("rating")).isEqualTo(4.7); + } + + @Test + public void testUpsertInPlaceWithListViaStage() throws Exception { + CollectionReference dmlCol = testCollectionWithDocs(bookDocs); + List results = + firestore + .pipeline() + .collection(dmlCol.getPath()) + .where(equal(field("__name__").documentId(), constant("book1"))) + .upsert( + new Upsert() + .withAdditionalFields( + java.util.Arrays.asList( + constant("Updated Genre List").as("genre"), + constant(5.0).as("rating")))) + .execute(new PipelineExecuteOptions().withAtomic(true)) + .get() + .getResults(); + + assertThat(results).hasSize(1); + assertThat(results.get(0).getData().get("documents_modified")).isEqualTo(1L); + DocumentSnapshot snap = dmlCol.document("book1").get().get(); + assertThat(snap.get("genre")).isEqualTo("Updated Genre List"); + assertThat(snap.get("rating")).isEqualTo(5.0); + } + + @Test + public void testUpsertIntoDifferentCollectionWithoutAdditionalFields() throws Exception { + CollectionReference sourceCol = testCollectionWithDocs(bookDocs); + CollectionReference targetCol = firestore.collection(LocalFirestoreHelper.autoId()); + List results = + firestore + .pipeline() + .collection(sourceCol.getPath()) + .where(equal(field("__name__").documentId(), constant("book1"))) + .upsert( + new Upsert() + .withCollection(targetCol.getPath()) + .withDocumentIdExpression(constant("target_doc_2"))) + .execute(new PipelineExecuteOptions().withAtomic(true)) + .get() + .getResults(); + + assertThat(results).hasSize(1); + assertThat(results.get(0).getData().get("documents_modified")).isEqualTo(1L); + DocumentSnapshot snap = targetCol.document("target_doc_2").get().get(); + assertThat(snap.exists()).isTrue(); + assertThat(snap.getString("title")).isEqualTo("The Hitchhiker's Guide to the Galaxy"); + } + + @Test + public void testUpsertStageInsideTransaction() throws Exception { + CollectionReference dmlCol = testCollectionWithDocs(bookDocs); + firestore + .runTransaction( + transaction -> { + Pipeline upsertPpl = + firestore + .pipeline() + .collection(dmlCol.getPath()) + .where(equal(field("__name__").documentId(), constant("book1"))) + .upsert(constant("TxUpsert").as("status")); + List res = transaction.execute(upsertPpl).get().getResults(); + assertThat(res).hasSize(1); + assertThat(res.get(0).getData().get("documents_modified")).isEqualTo(1L); + return null; + }) + .get(); + + assertThat(dmlCol.document("book1").get().get().get("status")).isEqualTo("TxUpsert"); + } } From f3c9c0b47b26ed9066cdb2af331f88c0b14486bf Mon Sep 17 00:00:00 2001 From: wuandy Date: Tue, 6 Oct 2026 17:56:55 +0000 Subject: [PATCH 05/18] refactor(firestore): align DML stages with GA API surface by removing BetaApi --- .../main/java/com/google/cloud/firestore/Pipeline.java | 7 ------- .../google/cloud/firestore/pipeline/stages/Delete.java | 2 -- .../google/cloud/firestore/pipeline/stages/Insert.java | 5 ----- .../google/cloud/firestore/pipeline/stages/Update.java | 3 --- .../google/cloud/firestore/pipeline/stages/Upsert.java | 10 ---------- 5 files changed, 27 deletions(-) diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java index 124605dd34c4..56578c676f6f 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java @@ -1215,7 +1215,6 @@ public Pipeline unnest(Selectable field, UnnestOptions options) { * * @return A new {@code Pipeline} object with this stage appended to the stage list. */ - @BetaApi public Pipeline delete() { return append(new Delete()); } @@ -1254,7 +1253,6 @@ public Pipeline delete() { * * @return A new {@code Pipeline} object with this stage appended to the stage list. */ - @BetaApi public Pipeline update() { return append(new Update()); } @@ -1278,7 +1276,6 @@ public Pipeline update() { * @param transformedFields The transformations to apply. * @return A new {@code Pipeline} object with this stage appended to the stage list. */ - @BetaApi public Pipeline update(Selectable... transformedFields) { return append(new Update().withTransformedFields(transformedFields)); } @@ -1304,22 +1301,18 @@ public Pipeline update(Selectable... transformedFields) { * @param update The {@code Update} stage to append. * @return A new {@code Pipeline} object with this stage appended to the stage list. */ - @BetaApi public Pipeline update(Update update) { return append(update); } - @BetaApi public Pipeline insert(Insert insert) { return append(insert); } - @BetaApi public Pipeline upsert(Upsert upsert) { return append(upsert); } - @BetaApi public Pipeline upsert(Selectable... additionalFields) { return append(new Upsert(additionalFields)); } diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Delete.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Delete.java index c288f0d42a3b..b31d2ee35493 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Delete.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Delete.java @@ -16,7 +16,6 @@ package com.google.cloud.firestore.pipeline.stages; -import com.google.api.core.BetaApi; import com.google.api.core.InternalApi; import com.google.firestore.v1.Value; import java.util.ArrayList; @@ -27,7 +26,6 @@ private Delete(InternalOptions options) { super("delete", options); } - @BetaApi public Delete() { this(InternalOptions.EMPTY); } diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java index bf8602ade2a9..a19a3cba8d57 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java @@ -16,7 +16,6 @@ package com.google.cloud.firestore.pipeline.stages; -import com.google.api.core.BetaApi; import com.google.api.core.InternalApi; import com.google.cloud.firestore.PipelineUtils; import com.google.cloud.firestore.pipeline.expressions.Expression; @@ -42,22 +41,18 @@ private Insert( this.documentIdExpression = documentIdExpression; } - @BetaApi public Insert() { this(null, null, InternalOptions.EMPTY); } - @BetaApi public Insert withCollection(String collectionPath) { return new Insert(collectionPath, this.documentIdExpression, this.options); } - @BetaApi public Insert withDocumentIdExpression(Expression documentIdExpression) { return new Insert(this.collectionPath, documentIdExpression, this.options); } - @BetaApi public Insert withDocumentId(Expression documentIdExpression) { return withDocumentIdExpression(documentIdExpression); } diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Update.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Update.java index 22d029045b77..1b50475c10df 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Update.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Update.java @@ -16,7 +16,6 @@ package com.google.cloud.firestore.pipeline.stages; -import com.google.api.core.BetaApi; import com.google.api.core.InternalApi; import com.google.cloud.firestore.PipelineUtils; import com.google.cloud.firestore.pipeline.expressions.Expression; @@ -38,12 +37,10 @@ private Update(@Nullable Selectable[] transformedFields, InternalOptions options this.transformedFields = transformedFields; } - @BetaApi public Update() { this(null, InternalOptions.EMPTY); } - @BetaApi public Update withTransformedFields(Selectable... transformedFields) { return new Update(transformedFields, this.options); } diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java index 3a2264027d6c..1a6a111f99c3 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java @@ -16,7 +16,6 @@ package com.google.cloud.firestore.pipeline.stages; -import com.google.api.core.BetaApi; import com.google.api.core.InternalApi; import com.google.cloud.firestore.PipelineUtils; import com.google.cloud.firestore.pipeline.expressions.Expression; @@ -48,12 +47,10 @@ private Upsert( this.documentIdExpression = documentIdExpression; } - @BetaApi public Upsert() { this((ImmutableList) null, null, null, InternalOptions.EMPTY); } - @BetaApi public Upsert(Selectable... additionalFields) { this( additionalFields != null ? ImmutableList.copyOf(additionalFields) : null, @@ -62,7 +59,6 @@ public Upsert(Selectable... additionalFields) { InternalOptions.EMPTY); } - @BetaApi public Upsert withAdditionalFields(Selectable... additionalFields) { return new Upsert( ImmutableList.copyOf(additionalFields), @@ -71,7 +67,6 @@ public Upsert withAdditionalFields(Selectable... additionalFields) { this.options); } - @BetaApi public Upsert withAdditionalFields(List additionalFields) { return new Upsert( ImmutableList.copyOf(additionalFields), @@ -80,29 +75,24 @@ public Upsert withAdditionalFields(List additionalFields) { this.options); } - @BetaApi public Upsert withTransformedFields(Selectable... transformedFields) { return withAdditionalFields(transformedFields); } - @BetaApi public Upsert withTransformedFields(List transformedFields) { return withAdditionalFields(transformedFields); } - @BetaApi public Upsert withCollection(String collectionPath) { return new Upsert( this.additionalFields, collectionPath, this.documentIdExpression, this.options); } - @BetaApi public Upsert withDocumentIdExpression(Expression documentIdExpression) { return new Upsert( this.additionalFields, this.collectionPath, documentIdExpression, this.options); } - @BetaApi public Upsert withDocumentId(Expression documentIdExpression) { return withDocumentIdExpression(documentIdExpression); } From a7278e1b06de1f54f035c568313e619113f0e0e2 Mon Sep 17 00:00:00 2001 From: dlarocque Date: Thu, 8 Oct 2026 15:12:02 -0400 Subject: [PATCH 06/18] fix(firestore): evaluate expressions nested in pipeline literal values The backend evaluates expressions only in the top-level fields of each literal document. Previously, an Expression nested inside a Map or List value was sent as a raw function_value inside a map_value/array_value and the request was rejected with `INVALID_ARGUMENT: Value type is not supported: FUNCTION_VALUE`. A top-level Map or List value that contains an Expression at any depth is now converted into a map(...) / array(...) expression, so nested expressions are evaluated. Values that contain no expressions keep their existing encoding, so constant-only data (including maps shaped like a proto Value) is unaffected. literals() shipped in 3.40.0, so the conversion is applied only to inputs that failed before. Adds a proto unit test and an integration test with expressions nested in a map (two levels deep) and in a list. Proposal P-3. --- .../firestore/pipeline/stages/Literals.java | 58 +++++++++++++++++-- .../cloud/firestore/PipelineProtoTest.java | 49 ++++++++++++++++ .../cloud/firestore/it/ITPipelineTest.java | 27 +++++++++ 3 files changed, 129 insertions(+), 5 deletions(-) diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Literals.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Literals.java index f27e172701c7..a3e18cd51805 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Literals.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Literals.java @@ -18,9 +18,11 @@ import com.google.api.core.InternalApi; import com.google.cloud.firestore.PipelineUtils; +import com.google.cloud.firestore.pipeline.expressions.Expression; import com.google.firestore.v1.Value; import java.util.ArrayList; import java.util.Arrays; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -49,11 +51,13 @@ private Value encodeLiteralMap(Map map) { for (Map.Entry entry : map.entrySet()) { String key = String.valueOf(entry.getKey()); Object v = entry.getValue(); - if (v instanceof com.google.cloud.firestore.pipeline.expressions.Expression) { - mapValue.putFields( - key, - PipelineUtils.encodeValue( - (com.google.cloud.firestore.pipeline.expressions.Expression) v)); + if (v instanceof Expression) { + mapValue.putFields(key, PipelineUtils.encodeValue((Expression) v)); + } else if (containsExpression(v)) { + // The backend evaluates expressions only in the top-level fields of a literal document, + // so a map or list that contains a nested expression is sent as a map(...) or array(...) + // expression instead of a raw map or array value. + mapValue.putFields(key, PipelineUtils.encodeValue(toExpression(v))); } else if (v instanceof Map) { mapValue.putFields(key, encodeLiteralMap((Map) v)); } else { @@ -62,4 +66,48 @@ private Value encodeLiteralMap(Map map) { } return Value.newBuilder().setMapValue(mapValue.build()).build(); } + + /** Returns whether {@code value} is a map or list that contains an expression at any depth. */ + private static boolean containsExpression(Object value) { + if (value instanceof Map) { + for (Object child : ((Map) value).values()) { + if (child instanceof Expression || containsExpression(child)) { + return true; + } + } + } else if (value instanceof List) { + for (Object child : (List) value) { + if (child instanceof Expression || containsExpression(child)) { + return true; + } + } + } + return false; + } + + /** + * Converts a map or list that contains nested expressions into an equivalent {@code map(...)} or + * {@code array(...)} expression. Children without expressions are kept as constants. + */ + private static Expression toExpression(Object value) { + if (value instanceof Expression) { + return (Expression) value; + } else if (value instanceof Map) { + Map elements = new LinkedHashMap<>(); + for (Map.Entry entry : ((Map) value).entrySet()) { + elements.put(String.valueOf(entry.getKey()), toExpressionIfNeeded(entry.getValue())); + } + return Expression.map(elements); + } else { + List elements = new ArrayList<>(); + for (Object child : (List) value) { + elements.add(toExpressionIfNeeded(child)); + } + return Expression.array(elements); + } + } + + private static Object toExpressionIfNeeded(Object value) { + return value instanceof Expression || containsExpression(value) ? toExpression(value) : value; + } } diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java index 6508b0a63346..e825d297968f 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java @@ -465,4 +465,53 @@ public void testLiteralsStageProtoEncoding() { assertThat(doc2Fields.get("title").getStringValue()).isEqualTo("Book 2"); assertThat(doc2Fields.get("calc").getIntegerValue()).isEqualTo(42L); } + + @Test + public void testLiteralsWithNestedExpressionsProtoEncoding() { + FirestoreOptions options = + FirestoreOptions.newBuilder() + .setProjectId("new-project") + .setDatabaseId("(default)") + .build(); + Firestore firestore = options.getService(); + + java.util.Map inner = new java.util.HashMap<>(); + inner.put("sum", add(constant(1), constant(2))); + inner.put("plain", 1); + java.util.Map constantMap = new java.util.HashMap<>(); + constantMap.put("stringValue", "abc"); + + java.util.Map doc = new java.util.HashMap<>(); + doc.put("top", add(constant(1), constant(1))); + doc.put("nestedMap", inner); + doc.put("nestedList", java.util.Arrays.asList(1, add(constant(2), constant(3)))); + doc.put("constantMap", constantMap); + doc.put("constantList", java.util.Arrays.asList(1, 2)); + + Stage literalsStage = firestore.pipeline().literals(doc).toProto().getStages(0); + java.util.Map fields = literalsStage.getArgs(0).getMapValue().getFieldsMap(); + + // Top-level expressions are encoded as is. + assertThat(fields.get("top").getFunctionValue().getName()).isEqualTo("add"); + + // A map or list containing a nested expression is wrapped in a map(...) / array(...) + // expression, so the backend evaluates the nested expression. + assertThat(fields.get("nestedMap").getFunctionValue().getName()).isEqualTo("map"); + assertThat(fields.get("nestedList").getFunctionValue().getName()).isEqualTo("array"); + java.util.List listArgs = fields.get("nestedList").getFunctionValue().getArgsList(); + assertThat(listArgs.get(0).getIntegerValue()).isEqualTo(1L); + assertThat(listArgs.get(1).getFunctionValue().getName()).isEqualTo("add"); + + // Values without expressions keep their existing encoding. + assertThat(fields.get("constantMap").hasMapValue()).isTrue(); + assertThat( + fields + .get("constantMap") + .getMapValue() + .getFieldsMap() + .get("stringValue") + .getStringValue()) + .isEqualTo("abc"); + assertThat(fields.get("constantList").getArrayValue().getValuesCount()).isEqualTo(2); + } } diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java index b36658f155fc..90e7b0bf9a40 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java @@ -5185,4 +5185,31 @@ public void testUpsertStageInsideTransaction() throws Exception { assertThat(dmlCol.document("book1").get().get().get("status")).isEqualTo("TxUpsert"); } + + @Test + public void testLiteralsWithNestedExpressions() throws Exception { + java.util.Map deepest = new java.util.HashMap<>(); + deepest.put("product", multiply(constant(2), constant(3))); + java.util.Map nested = new java.util.HashMap<>(); + nested.put("sum", add(constant(1), constant(2))); + nested.put("plain", "value"); + nested.put("deeper", deepest); + + java.util.Map data = new java.util.HashMap<>(); + data.put("nestedMap", nested); + data.put("nestedList", java.util.Arrays.asList(1L, add(constant(1), constant(2)))); + + List results = firestore.pipeline().literals(data).execute().get().getResults(); + + assertThat(results).hasSize(1); + java.util.Map expectedDeepest = new java.util.HashMap<>(); + expectedDeepest.put("product", 6L); + java.util.Map expectedNested = new java.util.HashMap<>(); + expectedNested.put("sum", 3L); + expectedNested.put("plain", "value"); + expectedNested.put("deeper", expectedDeepest); + assertThat(results.get(0).getData().get("nestedMap")).isEqualTo(expectedNested); + assertThat(results.get(0).getData().get("nestedList")) + .isEqualTo(java.util.Arrays.asList(1L, 3L)); + } } From 9d19469e9795294dfec51952e4dd69b975fe6a49 Mon Sep 17 00:00:00 2001 From: dlarocque Date: Thu, 8 Oct 2026 15:13:10 -0400 Subject: [PATCH 07/18] fix(firestore): validate pipeline literals documents and values client-side - literals() now requires at least one document and fails fast with IllegalArgumentException "Function literals() requires at least one document." The backend previously rejected an empty literals stage with INVALID_ARGUMENT. The varargs signature shipped in 3.40.0, so this is a runtime check rather than a signature change. - A null document now fails with a clear NullPointerException ("Function literals() called with invalid data. Document 0 is null.") instead of an NPE from Map.entrySet(). - FieldValue sentinels anywhere in a literal value (including inside nested maps and lists) are rejected with literals() wording and the real field path, for example: "Function literals() called with invalid data. FieldValue.serverTimestamp() can only be used with update() and set() (found in field a.b)". Previously the message ended with "at field ''". Values are still rejected before any RPC is sent, now when literals() is called rather than when the pipeline is executed. - null field values remain allowed. POJO values are unchanged: they still fail with "Cannot convert ... to Firestore Value". Whether literals() should map POJOs the way set() does is left as an open decision (proposal P-4e). Proposal P-4 (a, b, c, d). --- .../google/cloud/firestore/PipelineUtils.java | 9 +++ .../firestore/pipeline/stages/Literals.java | 44 +++++++++++++ .../cloud/firestore/PipelineProtoTest.java | 64 +++++++++++++++++++ 3 files changed, 117 insertions(+) diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineUtils.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineUtils.java index 6a9ef738a9af..0a7857b6f7ea 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineUtils.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineUtils.java @@ -53,6 +53,15 @@ public static Value encodeValue(Object value) { return UserDataConverter.encodeValue(FieldPath.empty(), value, UserDataConverter.ARGUMENT); } + /** + * Returns the user-facing name of a {@link FieldValue} sentinel, for example {@code + * FieldValue.serverTimestamp()}. + */ + @InternalApi + public static String fieldValueMethodName(FieldValue fieldValue) { + return fieldValue.getMethodName(); + } + @InternalApi public static Value encodeValue(Expression value) { return exprToValue(value); diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Literals.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Literals.java index a3e18cd51805..800cc61fa9ef 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Literals.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Literals.java @@ -17,8 +17,11 @@ package com.google.cloud.firestore.pipeline.stages; import com.google.api.core.InternalApi; +import com.google.cloud.firestore.FieldPath; +import com.google.cloud.firestore.FieldValue; import com.google.cloud.firestore.PipelineUtils; import com.google.cloud.firestore.pipeline.expressions.Expression; +import com.google.common.base.Preconditions; import com.google.firestore.v1.Value; import java.util.ArrayList; import java.util.Arrays; @@ -33,9 +36,50 @@ public final class Literals extends Stage { public Literals(Map... data) { super("literals", InternalOptions.EMPTY); + Preconditions.checkNotNull(data, "Function literals() requires at least one document."); + Preconditions.checkArgument( + data.length > 0, "Function literals() requires at least one document."); + for (int i = 0; i < data.length; i++) { + Preconditions.checkNotNull( + data[i], "Function literals() called with invalid data. Document %s is null.", i); + validateNoSentinels(data[i], new ArrayList<>()); + } this.data = Arrays.asList(data); } + /** + * Rejects {@link FieldValue} sentinels anywhere in a literal value. Literal documents are not + * written with set() or update(), so there is no pipeline representation for field transforms. + */ + private static void validateNoSentinels(Object value, List path) { + if (value instanceof FieldValue) { + throw new IllegalArgumentException( + String.format( + "Function literals() called with invalid data. %s can only be used with update() and" + + " set() (found in field %s)", + PipelineUtils.fieldValueMethodName((FieldValue) value), formatPath(path))); + } else if (value instanceof Map) { + for (Map.Entry entry : ((Map) value).entrySet()) { + path.add(String.valueOf(entry.getKey())); + validateNoSentinels(entry.getValue(), path); + path.remove(path.size() - 1); + } + } else if (value instanceof List) { + for (Object child : (List) value) { + validateNoSentinels(child, path); + } + } + } + + private static String formatPath(List path) { + try { + return FieldPath.of(path.toArray(new String[0])).toString(); + } catch (IllegalArgumentException e) { + // Empty field names cannot be represented as a FieldPath. + return String.join(".", path); + } + } + @Override Iterable toStageArgs() { List args = new ArrayList<>(); diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java index e825d297968f..bcbbe2020d3e 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java @@ -20,6 +20,7 @@ import static com.google.cloud.firestore.pipeline.expressions.Expression.constant; import static com.google.cloud.firestore.pipeline.expressions.Expression.field; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; import com.google.cloud.firestore.pipeline.stages.Insert; import com.google.cloud.firestore.pipeline.stages.PipelineExecuteOptions; @@ -514,4 +515,67 @@ public void testLiteralsWithNestedExpressionsProtoEncoding() { .isEqualTo("abc"); assertThat(fields.get("constantList").getArrayValue().getValuesCount()).isEqualTo(2); } + + @Test + public void testLiteralsValidation() { + FirestoreOptions options = + FirestoreOptions.newBuilder() + .setProjectId("new-project") + .setDatabaseId("(default)") + .build(); + Firestore firestore = options.getService(); + + // At least one document is required. + IllegalArgumentException empty = + assertThrows(IllegalArgumentException.class, () -> firestore.pipeline().literals()); + assertThat(empty) + .hasMessageThat() + .isEqualTo("Function literals() requires at least one document."); + assertThrows( + IllegalArgumentException.class, + () -> firestore.pipeline().literals(new java.util.HashMap[0])); + + // A null document is rejected with a clear message. + NullPointerException nullDoc = + assertThrows( + NullPointerException.class, + () -> firestore.pipeline().literals((java.util.Map) null)); + assertThat(nullDoc) + .hasMessageThat() + .isEqualTo("Function literals() called with invalid data. Document 0 is null."); + + // FieldValue sentinels are rejected with the field path, including nested ones. + java.util.Map nested = new java.util.HashMap<>(); + nested.put("b", FieldValue.serverTimestamp()); + java.util.Map doc = new java.util.HashMap<>(); + doc.put("a", nested); + IllegalArgumentException sentinel = + assertThrows(IllegalArgumentException.class, () -> firestore.pipeline().literals(doc)); + assertThat(sentinel) + .hasMessageThat() + .isEqualTo( + "Function literals() called with invalid data. FieldValue.serverTimestamp() can only" + + " be used with update() and set() (found in field a.b)"); + + java.util.Map inList = new java.util.HashMap<>(); + inList.put("list", java.util.Arrays.asList(1, FieldValue.increment(1))); + IllegalArgumentException listSentinel = + assertThrows(IllegalArgumentException.class, () -> firestore.pipeline().literals(inList)); + assertThat(listSentinel).hasMessageThat().contains("FieldValue.increment()"); + assertThat(listSentinel).hasMessageThat().contains("(found in field list)"); + + java.util.Map deleteDoc = new java.util.HashMap<>(); + deleteDoc.put("a.b", FieldValue.delete()); + IllegalArgumentException delete = + assertThrows( + IllegalArgumentException.class, () -> firestore.pipeline().literals(deleteDoc)); + assertThat(delete).hasMessageThat().contains("FieldValue.delete()"); + assertThat(delete).hasMessageThat().contains("(found in field `a.b`)"); + + // null values are allowed. + java.util.Map withNull = new java.util.HashMap<>(); + withNull.put("n", null); + Stage stage = firestore.pipeline().literals(withNull).toProto().getStages(0); + assertThat(stage.getArgs(0).getMapValue().getFieldsMap().get("n").hasNullValue()).isTrue(); + } } From 09ebb29871e716c6dbf98b212608983a9394ff2a Mon Sep 17 00:00:00 2001 From: dlarocque Date: Thu, 8 Oct 2026 15:14:36 -0400 Subject: [PATCH 08/18] feat(firestore): add List overloads for pipeline literals() and Update.withTransformedFields() Add PipelineSource.literals(List>) and Update.withTransformedFields(List) to mirror the existing varargs overloads and the List overloads offered elsewhere in the pipeline API. Note: with both overloads present, an untyped literals(null) call is now ambiguous at compile time (edge case; null was already rejected). Proposal P-2/P-6 (minor). --- .../cloud/firestore/PipelineSource.java | 16 +++++++++ .../firestore/pipeline/stages/Update.java | 4 +++ .../cloud/firestore/PipelineProtoTest.java | 36 +++++++++++++++++++ 3 files changed, 56 insertions(+) diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineSource.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineSource.java index f5c11e76d748..2a522376759b 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineSource.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineSource.java @@ -174,6 +174,22 @@ public final Pipeline literals(java.util.Map... data) { return new Pipeline(this.rpcContext, new Literals(data)); } + /** + * Creates a new {@link Pipeline} that operates on a static set of documents represented as Maps. + * + *

This is equivalent to {@link #literals(java.util.Map...)}, taking the documents as a {@link + * java.util.List}. + * + * @param data The Maps representing documents to include in the pipeline. + * @return A new {@code Pipeline} instance with a literals source. + */ + @Nonnull + @SuppressWarnings("unchecked") + public Pipeline literals(@Nonnull java.util.List> data) { + Preconditions.checkNotNull(data, "Function literals() requires at least one document."); + return literals(data.toArray(new java.util.Map[0])); + } + /** * Creates a new {@link Pipeline} from the given {@link Query}. Under the hood, this will * translate the query semantics (order by document ID, etc.) to an equivalent pipeline. diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Update.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Update.java index 1b50475c10df..c4a82d4ed720 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Update.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Update.java @@ -45,6 +45,10 @@ public Update withTransformedFields(Selectable... transformedFields) { return new Update(transformedFields, this.options); } + public Update withTransformedFields(List transformedFields) { + return withTransformedFields(transformedFields.toArray(new Selectable[0])); + } + @Override Iterable toStageArgs() { List args = new ArrayList<>(); diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java index bcbbe2020d3e..8f32b94df31a 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java @@ -578,4 +578,40 @@ public void testLiteralsValidation() { Stage stage = firestore.pipeline().literals(withNull).toProto().getStages(0); assertThat(stage.getArgs(0).getMapValue().getFieldsMap().get("n").hasNullValue()).isTrue(); } + + @Test + public void testListOverloadsProtoEncoding() { + FirestoreOptions options = + FirestoreOptions.newBuilder() + .setProjectId("new-project") + .setDatabaseId("(default)") + .build(); + Firestore firestore = options.getService(); + + java.util.Map doc1 = new java.util.HashMap<>(); + doc1.put("title", "Book 1"); + java.util.Map doc2 = new java.util.HashMap<>(); + doc2.put("title", "Book 2"); + + // literals(List) encodes the same as literals(Map...). + assertThat(firestore.pipeline().literals(java.util.Arrays.asList(doc1, doc2)).toProto()) + .isEqualTo(firestore.pipeline().literals(doc1, doc2).toProto()); + assertThrows( + IllegalArgumentException.class, + () -> firestore.pipeline().literals(java.util.Collections.emptyList())); + + // Update.withTransformedFields(List) encodes the same as the varargs overload. + com.google.cloud.firestore.pipeline.stages.Update fromList = + new com.google.cloud.firestore.pipeline.stages.Update() + .withTransformedFields( + java.util.Arrays.asList( + constant("Updated").as("status"), + add(field("count"), constant(1)).as("count"))); + com.google.cloud.firestore.pipeline.stages.Update fromVarargs = + new com.google.cloud.firestore.pipeline.stages.Update() + .withTransformedFields( + constant("Updated").as("status"), add(field("count"), constant(1)).as("count")); + assertThat(firestore.pipeline().collection("books").update(fromList).toProto()) + .isEqualTo(firestore.pipeline().collection("books").update(fromVarargs).toProto()); + } } From 123e7e2532281f786c26a42e78aed7a1dc862ac9 Mon Sep 17 00:00:00 2001 From: dlarocque Date: Thu, 8 Oct 2026 15:15:38 -0400 Subject: [PATCH 09/18] feat(firestore): add Pipeline.insert() and Pipeline.delete(Delete); expose DML stage classes Add the missing convenience overloads so every DML stage has both a no-arg form and a form taking a pre-configured stage: - Pipeline.insert() (equivalent to insert(new Insert())) - Pipeline.delete(Delete) (equivalent to delete()) Remove the class-level @InternalApi annotation from the user-constructed stage classes Insert, Upsert, Update and Delete. They are passed to public Pipeline methods (insert(Insert), upsert(Upsert), update(Update), delete(Delete)) and must be constructed by users, so marking them internal contradicted the public API. Literals keeps @InternalApi: it is only created by PipelineSource.literals() and no public method accepts it. Proposal P-7 + P-12. --- .../com/google/cloud/firestore/Pipeline.java | 16 +++++++++++ .../firestore/pipeline/stages/Delete.java | 8 ++++-- .../firestore/pipeline/stages/Insert.java | 2 -- .../firestore/pipeline/stages/Update.java | 9 +++++-- .../firestore/pipeline/stages/Upsert.java | 2 -- .../cloud/firestore/PipelineProtoTest.java | 27 +++++++++++++++++++ .../cloud/firestore/it/ITPipelineTest.java | 22 +++++++++++++++ 7 files changed, 78 insertions(+), 8 deletions(-) diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java index 56578c676f6f..203e72a49117 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java @@ -1219,6 +1219,18 @@ public Pipeline delete() { return append(new Delete()); } + /** + * Performs a delete operation on documents from previous stages using a {@link Delete} stage. + * + *

This is equivalent to {@link #delete()}. + * + * @param delete The {@code Delete} stage to append. + * @return A new {@code Pipeline} object with this stage appended to the stage list. + */ + public Pipeline delete(Delete delete) { + return append(delete); + } + /** * Performs an update operation using documents from previous stages. * @@ -1305,6 +1317,10 @@ public Pipeline update(Update update) { return append(update); } + public Pipeline insert() { + return append(new Insert()); + } + public Pipeline insert(Insert insert) { return append(insert); } diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Delete.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Delete.java index b31d2ee35493..2185b46b71d9 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Delete.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Delete.java @@ -16,11 +16,15 @@ package com.google.cloud.firestore.pipeline.stages; -import com.google.api.core.InternalApi; import com.google.firestore.v1.Value; import java.util.ArrayList; -@InternalApi +/** + * A pipeline stage that deletes the documents produced by the previous stages. + * + *

Use with {@link com.google.cloud.firestore.Pipeline#delete(Delete)}, or call {@link + * com.google.cloud.firestore.Pipeline#delete()} directly. + */ public final class Delete extends Stage { private Delete(InternalOptions options) { super("delete", options); diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java index a19a3cba8d57..71ac89fde828 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java @@ -16,7 +16,6 @@ package com.google.cloud.firestore.pipeline.stages; -import com.google.api.core.InternalApi; import com.google.cloud.firestore.PipelineUtils; import com.google.cloud.firestore.pipeline.expressions.Expression; import com.google.common.collect.ImmutableMap; @@ -26,7 +25,6 @@ import java.util.Map; import javax.annotation.Nullable; -@InternalApi public final class Insert extends Stage { @Nullable private final String collectionPath; diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Update.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Update.java index c4a82d4ed720..4c33dd7b7610 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Update.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Update.java @@ -16,7 +16,6 @@ package com.google.cloud.firestore.pipeline.stages; -import com.google.api.core.InternalApi; import com.google.cloud.firestore.PipelineUtils; import com.google.cloud.firestore.pipeline.expressions.Expression; import com.google.cloud.firestore.pipeline.expressions.Selectable; @@ -27,7 +26,13 @@ import java.util.Map; import javax.annotation.Nullable; -@InternalApi +/** + * A pipeline stage that updates, in place, the documents produced by the previous stages. + * + *

Use with {@link com.google.cloud.firestore.Pipeline#update(Update)}, or call {@link + * com.google.cloud.firestore.Pipeline#update()} / {@link + * com.google.cloud.firestore.Pipeline#update(Selectable...)} directly. + */ public final class Update extends Stage { @Nullable private final Selectable[] transformedFields; diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java index 1a6a111f99c3..a66f0a3f6e52 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java @@ -16,7 +16,6 @@ package com.google.cloud.firestore.pipeline.stages; -import com.google.api.core.InternalApi; import com.google.cloud.firestore.PipelineUtils; import com.google.cloud.firestore.pipeline.expressions.Expression; import com.google.cloud.firestore.pipeline.expressions.Selectable; @@ -29,7 +28,6 @@ import java.util.Map; import javax.annotation.Nullable; -@InternalApi public final class Upsert extends Stage { @Nullable private final ImmutableList additionalFields; diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java index 8f32b94df31a..108b00d72379 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java @@ -614,4 +614,31 @@ public void testListOverloadsProtoEncoding() { assertThat(firestore.pipeline().collection("books").update(fromList).toProto()) .isEqualTo(firestore.pipeline().collection("books").update(fromVarargs).toProto()); } + + @Test + public void testInsertAndDeleteConvenienceOverloadsProtoEncoding() { + FirestoreOptions options = + FirestoreOptions.newBuilder() + .setProjectId("new-project") + .setDatabaseId("(default)") + .build(); + Firestore firestore = options.getService(); + + // insert() is equivalent to insert(new Insert()). + com.google.firestore.v1.Pipeline insertProto = + firestore.pipeline().collection("books").insert().toProto(); + assertThat(insertProto) + .isEqualTo(firestore.pipeline().collection("books").insert(new Insert()).toProto()); + assertThat(insertProto.getStages(1).getName()).isEqualTo("insert"); + + // delete(new Delete()) is equivalent to delete(). + com.google.firestore.v1.Pipeline deleteProto = + firestore + .pipeline() + .collection("books") + .delete(new com.google.cloud.firestore.pipeline.stages.Delete()) + .toProto(); + assertThat(deleteProto).isEqualTo(firestore.pipeline().collection("books").delete().toProto()); + assertThat(deleteProto.getStages(1).getName()).isEqualTo("delete"); + } } diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java index 90e7b0bf9a40..12f746f83654 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java @@ -5093,6 +5093,28 @@ public void testInsertIntoDifferentCollection() throws Exception { .isEqualTo("The Hitchhiker's Guide to the Galaxy"); } + @Test + public void testInsertInPlaceFromLiterals() throws Exception { + CollectionReference dmlCol = firestore.collection(LocalFirestoreHelper.autoId()); + DocumentReference target = dmlCol.document("literal_doc"); + Map doc = new HashMap<>(); + doc.put("__name__", target); + doc.put("title", "Inserted In Place"); + + List results = + firestore + .pipeline() + .literals(doc) + .insert() + .execute(new PipelineExecuteOptions().withAtomic(true)) + .get() + .getResults(); + + assertThat(results).hasSize(1); + assertThat(results.get(0).getData().get("documents_modified")).isEqualTo(1L); + assertThat(target.get().get().getString("title")).isEqualTo("Inserted In Place"); + } + @Test public void testUpsertInPlaceWithVarargs() throws Exception { CollectionReference dmlCol = testCollectionWithDocs(bookDocs); From 9ee4d4aeb55c9ac22e2bbe4bbb1a48c0740be8d4 Mon Sep 17 00:00:00 2001 From: dlarocque Date: Thu, 8 Oct 2026 15:16:24 -0400 Subject: [PATCH 10/18] refactor(firestore): remove unreleased alias methods from Insert and Upsert Remove the duplicate alias methods so each option has a single name across the pipeline DML stages: - Insert.withDocumentId(Expression) -> use withDocumentIdExpression - Upsert.withDocumentId(Expression) -> use withDocumentIdExpression - Upsert.withTransformedFields(Selectable...) and (List) -> use withAdditionalFields These aliases (and the Insert/Upsert classes themselves) exist only on this feature branch and were never released (absent from origin/main), so removing them breaks no released API. Update.withTransformedFields is released and unchanged. The "backward compatibility" blocks in PipelineProtoTest that exercised the aliases are removed. Proposal P-9. --- .../firestore/pipeline/stages/Insert.java | 4 -- .../firestore/pipeline/stages/Upsert.java | 12 ----- .../cloud/firestore/PipelineProtoTest.java | 46 ------------------- 3 files changed, 62 deletions(-) diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java index 71ac89fde828..a6be5c93b377 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java @@ -51,10 +51,6 @@ public Insert withDocumentIdExpression(Expression documentIdExpression) { return new Insert(this.collectionPath, documentIdExpression, this.options); } - public Insert withDocumentId(Expression documentIdExpression) { - return withDocumentIdExpression(documentIdExpression); - } - private static InternalOptions buildOptions( @Nullable String collectionPath, @Nullable Expression documentIdExpression, diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java index a66f0a3f6e52..b4ae01d99e13 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java @@ -73,14 +73,6 @@ public Upsert withAdditionalFields(List additionalFields) { this.options); } - public Upsert withTransformedFields(Selectable... transformedFields) { - return withAdditionalFields(transformedFields); - } - - public Upsert withTransformedFields(List transformedFields) { - return withAdditionalFields(transformedFields); - } - public Upsert withCollection(String collectionPath) { return new Upsert( this.additionalFields, collectionPath, this.documentIdExpression, this.options); @@ -91,10 +83,6 @@ public Upsert withDocumentIdExpression(Expression documentIdExpression) { this.additionalFields, this.collectionPath, documentIdExpression, this.options); } - public Upsert withDocumentId(Expression documentIdExpression) { - return withDocumentIdExpression(documentIdExpression); - } - private static InternalOptions buildOptions( @Nullable String collectionPath, @Nullable Expression documentIdExpression, diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java index 108b00d72379..bd6b1ca4538e 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java @@ -141,19 +141,6 @@ public void testInsertStageProtoEncoding() { java.util.Map optionsMap = insertStage.getOptionsMap(); assertThat(optionsMap.get("collection").getReferenceValue()).isEqualTo("/books"); assertThat(optionsMap.get("document_id").getStringValue()).isEqualTo("book1"); - - // Test backward compatibility with withDocumentId - Pipeline pipelineDeprecated = - firestore - .pipeline() - .literals(data) - .insert( - new com.google.cloud.firestore.pipeline.stages.Insert() - .withCollection("books") - .withDocumentId(constant("book1"))); - Stage insertStageDeprecated = pipelineDeprecated.toProto().getStages(1); - assertThat(insertStageDeprecated.getOptionsMap().get("document_id").getStringValue()) - .isEqualTo("book1"); } @Test @@ -210,39 +197,6 @@ public void testUpsertStageProtoEncoding() { .withDocumentIdExpression(constant("book1"))); Stage upsertStageWithList = pipelineWithList.toProto().getStages(1); assertThat(upsertStageWithList.getArgs(0).getMapValue().getFieldsMap()).containsKey("count"); - - // Test backward compatibility with withTransformedFields - Pipeline pipelineWithTransformedFields = - firestore - .pipeline() - .literals(data) - .upsert( - new com.google.cloud.firestore.pipeline.stages.Upsert() - .withTransformedFields( - com.google.cloud.firestore.pipeline.expressions.Expression.add( - field("count"), constant(1)) - .as("count")) - .withCollection("books") - .withDocumentIdExpression(constant("book1"))); - Stage upsertStageWithTransformedFields = pipelineWithTransformedFields.toProto().getStages(1); - assertThat(upsertStageWithTransformedFields.getArgs(0).getMapValue().getFieldsMap()) - .containsKey("count"); - - // Test backward compatibility with withDocumentId - Pipeline pipelineDeprecated = - firestore - .pipeline() - .literals(data) - .upsert( - new com.google.cloud.firestore.pipeline.stages.Upsert( - com.google.cloud.firestore.pipeline.expressions.Expression.add( - field("count"), constant(1)) - .as("count")) - .withCollection("books") - .withDocumentId(constant("book1"))); - Stage upsertStageDeprecated = pipelineDeprecated.toProto().getStages(1); - assertThat(upsertStageDeprecated.getOptionsMap().get("document_id").getStringValue()) - .isEqualTo("book1"); } @Test From 74d47c44224381f1af23f14445b1f2460c690cbc Mon Sep 17 00:00:00 2001 From: dlarocque Date: Thu, 8 Oct 2026 15:17:39 -0400 Subject: [PATCH 11/18] fix(firestore): reject atomic pipeline execution inside a transaction or at a read time PipelineExecuteOptions.withAtomic(true) makes the backend start and auto-commit its own read-write transaction. Combining it with an enclosing transaction (Transaction.execute(pipeline, options)) or with a read time (read-only transactions) was silently ignored: the transaction ID took precedence and the atomic flag was dropped. Throw IllegalArgumentException from the request builder instead, before any RPC is sent: - atomic + transaction: "Atomic pipeline execution cannot be used inside a transaction. ..." - atomic + read time: "Atomic pipeline execution cannot be combined with a read time." Non-atomic execution in transactions and at a read time is unchanged. Adds a PipelineProtoTest unit test and the ITPipelineTest testAtomicExecutionInsideTransactionIsRejected integration test. (proposal P-14; pending team decision) --- .../com/google/cloud/firestore/Pipeline.java | 12 +++++ .../cloud/firestore/PipelineProtoTest.java | 50 +++++++++++++++++++ .../cloud/firestore/it/ITPipelineTest.java | 28 +++++++++++ 3 files changed, 90 insertions(+) diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java index 203e72a49117..15ad71f41411 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java @@ -1540,6 +1540,18 @@ ExecutePipelineRequest toExecutePipelineRequest( @Nonnull PipelineExecuteOptions options, @Nullable final ByteString transactionId, @Nullable com.google.protobuf.Timestamp readTime) { + if (options.isAtomic()) { + if (transactionId != null) { + throw new IllegalArgumentException( + "Atomic pipeline execution cannot be used inside a transaction. The transaction" + + " already makes its writes atomic; remove withAtomic(true)."); + } + if (readTime != null) { + throw new IllegalArgumentException( + "Atomic pipeline execution cannot be combined with a read time."); + } + } + java.util.Map optionsMap = new java.util.HashMap<>(StageUtils.toMap(options)); optionsMap.remove("atomic"); diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java index bd6b1ca4538e..bc8215aa16c9 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java @@ -595,4 +595,54 @@ public void testInsertAndDeleteConvenienceOverloadsProtoEncoding() { assertThat(deleteProto).isEqualTo(firestore.pipeline().collection("books").delete().toProto()); assertThat(deleteProto.getStages(1).getName()).isEqualTo("delete"); } + + @Test + public void testAtomicExecutionRejectedWithTransactionOrReadTime() { + FirestoreOptions options = + FirestoreOptions.newBuilder() + .setProjectId("new-project") + .setDatabaseId("(default)") + .build(); + Firestore firestore = options.getService(); + + java.util.Map data = new java.util.HashMap<>(); + data.put("title", "Atomic Book"); + Pipeline pipeline = + firestore + .pipeline() + .literals(data) + .insert( + new com.google.cloud.firestore.pipeline.stages.Insert() + .withCollection("books") + .withDocumentIdExpression(constant("book1"))); + + PipelineExecuteOptions atomic = new PipelineExecuteOptions().withAtomic(true); + com.google.protobuf.ByteString transactionId = + com.google.protobuf.ByteString.copyFromUtf8("txn"); + com.google.protobuf.Timestamp readTime = + com.google.protobuf.Timestamp.newBuilder().setSeconds(1).build(); + + IllegalArgumentException inTransaction = + assertThrows( + IllegalArgumentException.class, + () -> pipeline.toExecutePipelineRequest(atomic, transactionId, null)); + assertThat(inTransaction).hasMessageThat().contains("inside a transaction"); + + IllegalArgumentException withReadTime = + assertThrows( + IllegalArgumentException.class, + () -> pipeline.toExecutePipelineRequest(atomic, null, readTime)); + assertThat(withReadTime).hasMessageThat().contains("read time"); + + // Non-atomic execution inside a transaction or at a read time is unaffected. + PipelineExecuteOptions nonAtomic = new PipelineExecuteOptions(); + ExecutePipelineRequest txRequest = + pipeline.toExecutePipelineRequest(nonAtomic, transactionId, null); + assertThat(txRequest.getTransaction()).isEqualTo(transactionId); + assertThat(txRequest.hasNewTransaction()).isFalse(); + assertThat(txRequest.getAutoCommitTransaction()).isFalse(); + ExecutePipelineRequest readTimeRequest = + pipeline.toExecutePipelineRequest(nonAtomic, null, readTime); + assertThat(readTimeRequest.getReadTime()).isEqualTo(readTime); + } } diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java index 12f746f83654..93cc999722c0 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java @@ -4951,6 +4951,34 @@ public void testDeleteStageInsideTransaction() throws Exception { assertThat(dmlCol.document("book1").get().get().exists()).isFalse(); } + @Test + public void testAtomicExecutionInsideTransactionIsRejected() throws Exception { + CollectionReference dmlCol = testCollectionWithDocs(bookDocs); + ExecutionException exception = + assertThrows( + ExecutionException.class, + () -> + firestore + .runTransaction( + transaction -> { + Pipeline deletePpl = + firestore + .pipeline() + .collection(dmlCol.getPath()) + .where(equal(field("__name__").documentId(), constant("book1"))) + .delete(); + return transaction + .execute(deletePpl, new PipelineExecuteOptions().withAtomic(true)) + .get(); + }) + .get()); + assertThat(exception.getCause()).isInstanceOf(IllegalArgumentException.class); + assertThat(exception.getCause()).hasMessageThat().contains("inside a transaction"); + + // The pipeline was rejected client-side, so nothing was deleted. + assertThat(dmlCol.document("book1").get().get().exists()).isTrue(); + } + @Test public void testUpdateSingleDocWithAddFields() throws Exception { CollectionReference dmlCol = testCollectionWithDocs(bookDocs); From 71d716adbd9d21799d4317693fec47b3f0a39dbd Mon Sep 17 00:00:00 2001 From: dlarocque Date: Thu, 8 Oct 2026 15:21:27 -0400 Subject: [PATCH 12/18] docs(firestore): document pipeline DML stages, literals and atomic execution - Added Javadoc for the Insert and Upsert classes, their constructors and builder methods, Pipeline.insert(), insert(Insert), upsert(Upsert), upsert(Selectable...), and PipelineExecuteOptions.withAtomic()/isAtomic(). - Documented the insert/upsert target rules: with neither a collection nor a document ID expression the input path is reused (so inserting documents read from the database fails with ALREADY_EXISTS); with only a collection the input document ID is reused (generated if the input has none); with only a document ID expression the input document's parent collection is used. The document ID expression must evaluate to an ID, not a path. Upsert replaces the target document; stored fields are not merged. - Documented atomic execution: default false; when true, all writes are applied or none are; not allowed in Transaction.execute() or with a read time (throws IllegalArgumentException). - Documented the literals() contract on both overloads: at least one document, expressions (including nested ones) are evaluated, FieldValue sentinels are rejected (use currentTimestamp()), and __name__ sets the document path. - Extended the delete() and update() Javadoc (result shape, replace semantics, no document creation, atomicity). - Removed the stray "Performs an insert operation..." first sentence from the rawStage() Javadoc. CHANGELOG.md is generated by release-please and is not edited. Proposal P-16 (and the P-10 target rules). --- .../com/google/cloud/firestore/Pipeline.java | 119 +++++++++++++++++- .../cloud/firestore/PipelineSource.java | 20 +++ .../google/cloud/firestore/Transaction.java | 4 + .../firestore/pipeline/stages/Insert.java | 55 ++++++++ .../stages/PipelineExecuteOptions.java | 22 ++++ .../firestore/pipeline/stages/Upsert.java | 76 +++++++++++ 6 files changed, 294 insertions(+), 2 deletions(-) diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java index 15ad71f41411..40ac4019c734 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java @@ -1201,6 +1201,12 @@ public Pipeline unnest(Selectable field, UnnestOptions options) { /** * Performs a delete operation on documents from previous stages. * + *

Each document produced by the previous stages is deleted from the database. The result + * contains a single document with a {@code documents_modified} field. + * + *

Writes are applied atomically (all or none) only if the pipeline is executed with {@link + * PipelineExecuteOptions#withAtomic(boolean) withAtomic(true)} or inside a transaction. + * *

Example: * *

{@code
@@ -1237,6 +1243,14 @@ public Pipeline delete(Delete delete) {
    * 

This method updates the documents in place based on the data flowing through the pipeline. * To specify transformations, use {@link #update(Selectable...)}. * + *

Each document is written back to its existing path. The stored document is replaced by the + * pipeline document, so fields removed by earlier stages (for example, with {@link + * #removeFields(String, String...)}) are also removed from the stored document. This stage does + * not create documents: documents that don't exist are skipped. + * + *

Writes are applied atomically (all or none) only if the pipeline is executed with {@link + * PipelineExecuteOptions#withAtomic(boolean) withAtomic(true)} or inside a transaction. + * *

Example 1: Update a collection's schema by adding a new field and removing an old one. * *

{@code
@@ -1317,25 +1331,126 @@ public Pipeline update(Update update) {
     return append(update);
   }
 
+  /**
+   * Inserts each document produced by the previous stages at its own path.
+   *
+   * 

This is equivalent to {@code insert(new Insert())}. Because documents read from the database + * already exist at their own path, this is mainly useful with a {@link + * PipelineSource#literals(java.util.Map...) literals} source whose documents set a {@code + * __name__} field. To insert into another collection or under another ID, use {@link + * #insert(Insert)}. + * + *

The insert fails with {@code ALREADY_EXISTS} if a target document already exists. + * + *

Writes are applied atomically (all or none) only if the pipeline is executed with {@link + * PipelineExecuteOptions#withAtomic(boolean) withAtomic(true)} or inside a transaction. + * + * @return A new {@code Pipeline} object with this stage appended to the stage list. + */ public Pipeline insert() { return append(new Insert()); } + /** + * Inserts each document produced by the previous stages as a new document, using a configured + * {@link Insert} stage. + * + *

The insert fails with {@code ALREADY_EXISTS} if a target document already exists. The target + * path of each document is determined as follows: + * + *

    + *
  • With neither a collection nor a document ID expression, the input document's own path is + * reused, so inserting documents read from the database fails. + *
  • With only {@link Insert#withCollection(String) withCollection}, the document is written + * to that collection under the input document's ID (or a generated ID if it has none). + *
  • With only {@link Insert#withDocumentIdExpression(Expression) withDocumentIdExpression}, + * the document is written to the input document's parent collection. + *
  • With both, the document is written to the collection under the evaluated ID. + *
+ * + *

Writes are applied atomically (all or none) only if the pipeline is executed with {@link + * PipelineExecuteOptions#withAtomic(boolean) withAtomic(true)} or inside a transaction. + * + *

Example: + * + *

{@code
+   * Map book = new HashMap<>();
+   * book.put("title", "Dune");
+   *
+   * firestore.pipeline()
+   *     .literals(book)
+   *     .insert(new Insert().withCollection("books"))
+   *     .execute(new PipelineExecuteOptions().withAtomic(true))
+   *     .get();
+   * }
+ * + * @param insert The {@code Insert} stage to append. + * @return A new {@code Pipeline} object with this stage appended to the stage list. + */ public Pipeline insert(Insert insert) { return append(insert); } + /** + * Writes each document produced by the previous stages, using a configured {@link Upsert} stage. + * The target document is created if it doesn't exist and replaced if it does; stored fields are + * not merged. + * + *

The target path follows the same rules as {@link #insert(Insert)}: by default each document + * is written back to its own path; {@link Upsert#withCollection(String) withCollection} and + * {@link Upsert#withDocumentIdExpression(Expression) withDocumentIdExpression} change the target + * collection and ID. + * + *

Writes are applied atomically (all or none) only if the pipeline is executed with {@link + * PipelineExecuteOptions#withAtomic(boolean) withAtomic(true)} or inside a transaction. + * + *

Example: + * + *

{@code
+   * firestore.pipeline()
+   *     .collection("books")
+   *     .where(field("title").equal("Dune"))
+   *     .upsert(new Upsert().withCollection("archive"))
+   *     .execute()
+   *     .get();
+   * }
+ * + * @param upsert The {@code Upsert} stage to append. + * @return A new {@code Pipeline} object with this stage appended to the stage list. + */ public Pipeline upsert(Upsert upsert) { return append(upsert); } + /** + * Writes each document produced by the previous stages back to its own path, after adding the + * given fields. The document is created if it doesn't exist and replaced if it does; stored + * fields are not merged. + * + *

Writes are applied atomically (all or none) only if the pipeline is executed with {@link + * PipelineExecuteOptions#withAtomic(boolean) withAtomic(true)} or inside a transaction. + * + *

Example: + * + *

{@code
+   * firestore.pipeline()
+   *     .collection("books")
+   *     .where(field("rating").greaterThan(4))
+   *     .upsert(constant("Featured").as("status"))
+   *     .execute()
+   *     .get();
+   * }
+ * + * @param additionalFields Fields to add to each input document before it is written. A field with + * the same name as an existing input field overwrites that field. + * @return A new {@code Pipeline} object with this stage appended to the stage list. + */ public Pipeline upsert(Selectable... additionalFields) { return append(new Upsert(additionalFields)); } /** - * Performs an insert operation using documents from previous stages. Adds a generic stage to the - * pipeline. + * Adds a generic stage to the pipeline. * *

This method provides a flexible way to extend the pipeline's functionality by adding custom * stages. Each generic stage is defined by a unique `name` and a set of `params` that control its diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineSource.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineSource.java index 2a522376759b..6f7504532fd2 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineSource.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineSource.java @@ -152,6 +152,14 @@ public Pipeline documents(String... docs) { /** * Creates a new {@link Pipeline} that operates on a static set of documents represented as Maps. * + *

Each map becomes one input document. At least one document is required. Field values can be + * constants or {@link com.google.cloud.firestore.pipeline.expressions.Expression}s; expressions + * are evaluated, including expressions nested inside map or list values. {@link FieldValue} + * sentinels (for example, {@code FieldValue.serverTimestamp()}) are not supported and are + * rejected; use an expression such as {@code currentTimestamp()} instead. To give a document a + * path (for example, for an in-place {@code insert()}, {@code upsert()} or {@code update()}), set + * its {@code __name__} field to a {@link DocumentReference}. + * *

Example: * *

{@code
@@ -168,6 +176,8 @@ public Pipeline documents(String... docs) {
    *
    * @param data The Maps representing documents to include in the pipeline.
    * @return A new {@code Pipeline} instance with a literals source.
+   * @throws IllegalArgumentException if no documents are given or a value contains a {@link
+   *     FieldValue} sentinel.
    */
   @Nonnull
   public final Pipeline literals(java.util.Map... data) {
@@ -180,8 +190,18 @@ public final Pipeline literals(java.util.Map... data) {
    * 

This is equivalent to {@link #literals(java.util.Map...)}, taking the documents as a {@link * java.util.List}. * + *

Each map becomes one input document. At least one document is required. Field values can be + * constants or {@link com.google.cloud.firestore.pipeline.expressions.Expression}s; expressions + * are evaluated, including expressions nested inside map or list values. {@link FieldValue} + * sentinels (for example, {@code FieldValue.serverTimestamp()}) are not supported and are + * rejected; use an expression such as {@code currentTimestamp()} instead. To give a document a + * path (for example, for an in-place {@code insert()}, {@code upsert()} or {@code update()}), set + * its {@code __name__} field to a {@link DocumentReference}. + * * @param data The Maps representing documents to include in the pipeline. * @return A new {@code Pipeline} instance with a literals source. + * @throws IllegalArgumentException if no documents are given or a value contains a {@link + * FieldValue} sentinel. */ @Nonnull @SuppressWarnings("unchecked") diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Transaction.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Transaction.java index e11f300e069c..c92c746b90ad 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Transaction.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Transaction.java @@ -154,7 +154,11 @@ public abstract ApiFuture> getAll( /** * Executes the provided pipeline with specified options, within the transaction context. * + *

The pipeline's writes are part of the transaction, so {@link + * PipelineExecuteOptions#withAtomic(boolean) withAtomic(true)} must not be set. + * * @return The result of the execution. + * @throws IllegalArgumentException if {@code options} enables atomic execution. */ @Nonnull public abstract ApiFuture execute( diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java index a6be5c93b377..1b3cbae1e3c0 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java @@ -25,6 +25,42 @@ import java.util.Map; import javax.annotation.Nullable; +/** + * A pipeline stage that inserts each document produced by the previous stages as a new document. + * + *

The insert fails with {@code ALREADY_EXISTS} if a target document already exists. Because + * documents read from the database keep their path, inserting them without a collection or document + * ID expression (or into their own collection under their own ID) fails. + * + *

The target path of each document is determined as follows: + * + *

    + *
  • With neither a collection nor a document ID expression, the input document's own path is + * reused. + *
  • With only a collection, the document is written to that collection under the input + * document's ID. If the input document has no ID (for example, a literal document without a + * {@code __name__} field, or a document whose {@code __name__} field was removed), an ID is + * generated. + *
  • With only a document ID expression, the document is written to the input document's parent + * collection under the ID that the expression evaluates to. + *
  • With both, the document is written to the collection under the ID that the expression + * evaluates to. + *
+ * + *

Example: + * + *

{@code
+ * // Copy a document into another collection under a new ID.
+ * firestore.pipeline()
+ *     .collection("books")
+ *     .where(field("title").equal("Dune"))
+ *     .insert(new Insert().withCollection("archive").withDocumentIdExpression(constant("dune")))
+ *     .execute()
+ *     .get();
+ * }
+ * + * @see com.google.cloud.firestore.Pipeline#insert(Insert) + */ public final class Insert extends Stage { @Nullable private final String collectionPath; @@ -39,14 +75,33 @@ private Insert( this.documentIdExpression = documentIdExpression; } + /** + * Creates an insert stage with no target collection and no document ID expression, so each + * document is inserted at the input document's own path. + */ public Insert() { this(null, null, InternalOptions.EMPTY); } + /** + * Sets the collection that documents are inserted into. + * + * @param collectionPath The path of the target collection, for example {@code "books"} or {@code + * "users/alice/books"}. + * @return A new {@code Insert} stage with the target collection set. + */ public Insert withCollection(String collectionPath) { return new Insert(collectionPath, this.documentIdExpression, this.options); } + /** + * Sets the expression that produces the ID of each inserted document. + * + * @param documentIdExpression An expression that is evaluated against each input document and + * must evaluate to a document ID (not a document path). Use {@code constant("id")} for a + * fixed ID. + * @return A new {@code Insert} stage with the document ID expression set. + */ public Insert withDocumentIdExpression(Expression documentIdExpression) { return new Insert(this.collectionPath, documentIdExpression, this.options); } diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/PipelineExecuteOptions.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/PipelineExecuteOptions.java index edb84515e043..8181c0944964 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/PipelineExecuteOptions.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/PipelineExecuteOptions.java @@ -45,10 +45,32 @@ public PipelineExecuteOptions withIndexMode(String indexMode) { return with("index_mode", indexMode); } + /** + * Sets whether the pipeline is executed atomically. + * + *

When {@code true}, the pipeline runs in a single read-write transaction that is committed + * automatically after the pipeline has executed, so either all of its writes (from {@code + * insert}, {@code upsert}, {@code update} and {@code delete} stages) are applied or none are. The + * default is {@code false}. + * + *

Atomic execution cannot be used with {@link + * com.google.cloud.firestore.Transaction#execute(com.google.cloud.firestore.Pipeline, + * PipelineExecuteOptions)}, because the enclosing transaction already makes the writes atomic, or + * in a read-only transaction with a read time. Executing such a pipeline throws {@link + * IllegalArgumentException}. + * + * @param atomic Whether to execute the pipeline atomically. + * @return A new {@code PipelineExecuteOptions} with the atomic setting applied. + */ public PipelineExecuteOptions withAtomic(boolean atomic) { return new PipelineExecuteOptions(this.options, atomic); } + /** + * Returns whether the pipeline is executed atomically. The default is {@code false}. + * + * @see #withAtomic(boolean) + */ public boolean isAtomic() { return this.atomic; } diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java index b4ae01d99e13..5c8453193e6e 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java @@ -28,6 +28,42 @@ import java.util.Map; import javax.annotation.Nullable; +/** + * A pipeline stage that writes each document produced by the previous stages, creating the target + * document if it doesn't exist and replacing it if it does. + * + *

Upsert replaces the whole target document; it does not merge with the stored fields. Stored + * fields that are not in the pipeline document are removed. + * + *

The target path of each document is determined as follows: + * + *

    + *
  • With neither a collection nor a document ID expression, the input document's own path is + * reused. + *
  • With only a collection, the document is written to that collection under the input + * document's ID. If the input document has no ID (for example, a literal document without a + * {@code __name__} field, or a document whose {@code __name__} field was removed), an ID is + * generated. + *
  • With only a document ID expression, the document is written to the input document's parent + * collection under the ID that the expression evaluates to. + *
  • With both, the document is written to the collection under the ID that the expression + * evaluates to. + *
+ * + *

Example: + * + *

{@code
+ * // Set "status" on each matching document and write it back to its own path.
+ * firestore.pipeline()
+ *     .collection("books")
+ *     .where(field("rating").greaterThan(4))
+ *     .upsert(new Upsert().withAdditionalFields(constant("Featured").as("status")))
+ *     .execute()
+ *     .get();
+ * }
+ * + * @see com.google.cloud.firestore.Pipeline#upsert(Upsert) + */ public final class Upsert extends Stage { @Nullable private final ImmutableList additionalFields; @@ -45,10 +81,21 @@ private Upsert( this.documentIdExpression = documentIdExpression; } + /** + * Creates an upsert stage with no additional fields, no target collection and no document ID + * expression, so each document is written back to its own path. + */ public Upsert() { this((ImmutableList) null, null, null, InternalOptions.EMPTY); } + /** + * Creates an upsert stage that adds the given fields to each input document before it is written + * back to its own path. + * + * @param additionalFields Fields to add to each input document. A field with the same name as an + * existing input field overwrites that field. + */ public Upsert(Selectable... additionalFields) { this( additionalFields != null ? ImmutableList.copyOf(additionalFields) : null, @@ -57,6 +104,13 @@ public Upsert(Selectable... additionalFields) { InternalOptions.EMPTY); } + /** + * Sets the fields to add to each input document before it is written. + * + * @param additionalFields Fields to add to each input document. A field with the same name as an + * existing input field overwrites that field. + * @return A new {@code Upsert} stage with the additional fields set. + */ public Upsert withAdditionalFields(Selectable... additionalFields) { return new Upsert( ImmutableList.copyOf(additionalFields), @@ -65,6 +119,13 @@ public Upsert withAdditionalFields(Selectable... additionalFields) { this.options); } + /** + * Sets the fields to add to each input document before it is written. + * + * @param additionalFields Fields to add to each input document. A field with the same name as an + * existing input field overwrites that field. + * @return A new {@code Upsert} stage with the additional fields set. + */ public Upsert withAdditionalFields(List additionalFields) { return new Upsert( ImmutableList.copyOf(additionalFields), @@ -73,11 +134,26 @@ public Upsert withAdditionalFields(List additionalFields) { this.options); } + /** + * Sets the collection that documents are written to. + * + * @param collectionPath The path of the target collection, for example {@code "books"} or {@code + * "users/alice/books"}. + * @return A new {@code Upsert} stage with the target collection set. + */ public Upsert withCollection(String collectionPath) { return new Upsert( this.additionalFields, collectionPath, this.documentIdExpression, this.options); } + /** + * Sets the expression that produces the ID of each target document. + * + * @param documentIdExpression An expression that is evaluated against each input document and + * must evaluate to a document ID (not a document path). Use {@code constant("id")} for a + * fixed ID. + * @return A new {@code Upsert} stage with the document ID expression set. + */ public Upsert withDocumentIdExpression(Expression documentIdExpression) { return new Upsert( this.additionalFields, this.collectionPath, documentIdExpression, this.options); From f3360d39142a7ae3355a4ef46c3adc5cbc1c7931 Mon Sep 17 00:00:00 2001 From: dlarocque Date: Thu, 8 Oct 2026 17:24:51 -0400 Subject: [PATCH 13/18] fix(firestore): leave the empty pipeline literals() check to the backend Remove the client-side "at least one document" check from Literals (and therefore from both PipelineSource.literals() overloads). The backend already rejects an empty literals stage with INVALID_ARGUMENT ("... must have at least one document"), and the SDK should not duplicate checks the backend enforces. The error now surfaces from execute() instead of at pipeline build time. Kept client-side: - checkNotNull for a null documents array or list and for a null document, since the SDK would otherwise fail with a bare NullPointerException while encoding. The null-array message is now "Function literals() called with invalid data. Documents must not be null." - FieldValue sentinel rejection, because sentinels cannot be encoded in a pipeline, so the backend never sees them. The literals() Javadoc now says that an empty set of documents is rejected by the backend at execute time. The unit tests assert that an empty literals() builds a stage with no arguments, and that literals(emptyList()) encodes the same way. A new IT, testLiteralsWithNoDocumentsIsRejectedByBackend, checks the backend error. Follows the decision not to duplicate backend-enforced checks (proposal D-1 / P-4a). --- .../cloud/firestore/PipelineSource.java | 15 +++++------ .../firestore/pipeline/stages/Literals.java | 6 ++--- .../cloud/firestore/PipelineProtoTest.java | 25 +++++++++++-------- .../cloud/firestore/it/ITPipelineTest.java | 9 +++++++ 4 files changed, 34 insertions(+), 21 deletions(-) diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineSource.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineSource.java index 6f7504532fd2..293b9d764d74 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineSource.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineSource.java @@ -152,7 +152,8 @@ public Pipeline documents(String... docs) { /** * Creates a new {@link Pipeline} that operates on a static set of documents represented as Maps. * - *

Each map becomes one input document. At least one document is required. Field values can be + *

Each map becomes one input document. At least one document is required; an empty set of + * documents is rejected by the backend when the pipeline is executed. Field values can be * constants or {@link com.google.cloud.firestore.pipeline.expressions.Expression}s; expressions * are evaluated, including expressions nested inside map or list values. {@link FieldValue} * sentinels (for example, {@code FieldValue.serverTimestamp()}) are not supported and are @@ -176,8 +177,7 @@ public Pipeline documents(String... docs) { * * @param data The Maps representing documents to include in the pipeline. * @return A new {@code Pipeline} instance with a literals source. - * @throws IllegalArgumentException if no documents are given or a value contains a {@link - * FieldValue} sentinel. + * @throws IllegalArgumentException if a value contains a {@link FieldValue} sentinel. */ @Nonnull public final Pipeline literals(java.util.Map... data) { @@ -190,7 +190,8 @@ public final Pipeline literals(java.util.Map... data) { *

This is equivalent to {@link #literals(java.util.Map...)}, taking the documents as a {@link * java.util.List}. * - *

Each map becomes one input document. At least one document is required. Field values can be + *

Each map becomes one input document. At least one document is required; an empty set of + * documents is rejected by the backend when the pipeline is executed. Field values can be * constants or {@link com.google.cloud.firestore.pipeline.expressions.Expression}s; expressions * are evaluated, including expressions nested inside map or list values. {@link FieldValue} * sentinels (for example, {@code FieldValue.serverTimestamp()}) are not supported and are @@ -200,13 +201,13 @@ public final Pipeline literals(java.util.Map... data) { * * @param data The Maps representing documents to include in the pipeline. * @return A new {@code Pipeline} instance with a literals source. - * @throws IllegalArgumentException if no documents are given or a value contains a {@link - * FieldValue} sentinel. + * @throws IllegalArgumentException if a value contains a {@link FieldValue} sentinel. */ @Nonnull @SuppressWarnings("unchecked") public Pipeline literals(@Nonnull java.util.List> data) { - Preconditions.checkNotNull(data, "Function literals() requires at least one document."); + Preconditions.checkNotNull( + data, "Function literals() called with invalid data. Documents must not be null."); return literals(data.toArray(new java.util.Map[0])); } diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Literals.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Literals.java index 800cc61fa9ef..ba8323ff917a 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Literals.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Literals.java @@ -36,9 +36,9 @@ public final class Literals extends Stage { public Literals(Map... data) { super("literals", InternalOptions.EMPTY); - Preconditions.checkNotNull(data, "Function literals() requires at least one document."); - Preconditions.checkArgument( - data.length > 0, "Function literals() requires at least one document."); + // An empty set of documents is rejected by the backend when the pipeline is executed. + Preconditions.checkNotNull( + data, "Function literals() called with invalid data. Documents must not be null."); for (int i = 0; i < data.length; i++) { Preconditions.checkNotNull( data[i], "Function literals() called with invalid data. Document %s is null.", i); diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java index bc8215aa16c9..ca9fe7e61b0f 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java @@ -479,15 +479,19 @@ public void testLiteralsValidation() { .build(); Firestore firestore = options.getService(); - // At least one document is required. - IllegalArgumentException empty = - assertThrows(IllegalArgumentException.class, () -> firestore.pipeline().literals()); - assertThat(empty) + // An empty set of documents is not rejected client-side; the backend rejects it on execute. + Stage emptyStage = firestore.pipeline().literals().toProto().getStages(0); + assertThat(emptyStage.getName()).isEqualTo("literals"); + assertThat(emptyStage.getArgsCount()).isEqualTo(0); + + // A null documents array is rejected with a clear message. + NullPointerException nullArray = + assertThrows( + NullPointerException.class, + () -> firestore.pipeline().literals((java.util.Map[]) null)); + assertThat(nullArray) .hasMessageThat() - .isEqualTo("Function literals() requires at least one document."); - assertThrows( - IllegalArgumentException.class, - () -> firestore.pipeline().literals(new java.util.HashMap[0])); + .isEqualTo("Function literals() called with invalid data. Documents must not be null."); // A null document is rejected with a clear message. NullPointerException nullDoc = @@ -550,9 +554,8 @@ public void testListOverloadsProtoEncoding() { // literals(List) encodes the same as literals(Map...). assertThat(firestore.pipeline().literals(java.util.Arrays.asList(doc1, doc2)).toProto()) .isEqualTo(firestore.pipeline().literals(doc1, doc2).toProto()); - assertThrows( - IllegalArgumentException.class, - () -> firestore.pipeline().literals(java.util.Collections.emptyList())); + assertThat(firestore.pipeline().literals(java.util.Collections.emptyList()).toProto()) + .isEqualTo(firestore.pipeline().literals().toProto()); // Update.withTransformedFields(List) encodes the same as the varargs overload. com.google.cloud.firestore.pipeline.stages.Update fromList = diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java index 93cc999722c0..14c2421d2316 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java @@ -5262,4 +5262,13 @@ public void testLiteralsWithNestedExpressions() throws Exception { assertThat(results.get(0).getData().get("nestedList")) .isEqualTo(java.util.Arrays.asList(1L, 3L)); } + + @Test + public void testLiteralsWithNoDocumentsIsRejectedByBackend() throws Exception { + ExecutionException exception = + assertThrows( + ExecutionException.class, () -> firestore.pipeline().literals().execute().get()); + assertThat(exception).hasMessageThat().contains("INVALID_ARGUMENT"); + assertThat(exception).hasMessageThat().contains("at least one document"); + } } From 9b40f5c315e1f868e6017ac2b156bc82c0465265 Mon Sep 17 00:00:00 2001 From: dlarocque Date: Fri, 9 Oct 2026 11:41:49 -0400 Subject: [PATCH 14/18] docs(firestore): clean up pipeline DML comments Tighten the Javadoc and comments added for the pipeline DML stages and literals so they describe behavior or explain non-obvious code, without narrating how the change was made: - Document that insert() and upsert() without a target collection use the input document's __name__ path, and that a document without __name__ fails with INVALID_ARGUMENT. - State the empty literals() behavior plainly (execution fails with INVALID_ARGUMENT) and drop an internal comment that described why no client-side check exists. - Reword the sentinel validation Javadoc to say why sentinels are rejected. - Remove test comments that only restate the assertions, and reword the remaining ones as statements of behavior. No code changes. --- .../com/google/cloud/firestore/Pipeline.java | 19 ++++++----- .../cloud/firestore/PipelineSource.java | 32 +++++++++---------- .../firestore/pipeline/stages/Insert.java | 10 +++--- .../firestore/pipeline/stages/Literals.java | 5 ++- .../firestore/pipeline/stages/Upsert.java | 9 +++--- .../cloud/firestore/PipelineProtoTest.java | 17 ++-------- .../cloud/firestore/it/ITPipelineTest.java | 2 +- 7 files changed, 43 insertions(+), 51 deletions(-) diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java index 40ac4019c734..397b5e385569 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java @@ -1332,13 +1332,14 @@ public Pipeline update(Update update) { } /** - * Inserts each document produced by the previous stages at its own path. + * Inserts each document produced by the previous stages at its own path, given by its {@code + * __name__} field. * - *

This is equivalent to {@code insert(new Insert())}. Because documents read from the database - * already exist at their own path, this is mainly useful with a {@link - * PipelineSource#literals(java.util.Map...) literals} source whose documents set a {@code - * __name__} field. To insert into another collection or under another ID, use {@link - * #insert(Insert)}. + *

This is equivalent to {@code insert(new Insert())}. It is mainly useful with a {@link + * PipelineSource#literals(java.util.Map...) literals} source whose documents set {@code __name__} + * to a {@link DocumentReference}: documents read from the database already exist, and a document + * without a {@code __name__} field fails with {@code INVALID_ARGUMENT}. To insert into another + * collection or under another ID, use {@link #insert(Insert)}. * *

The insert fails with {@code ALREADY_EXISTS} if a target document already exists. * @@ -1360,7 +1361,8 @@ public Pipeline insert() { * *

    *
  • With neither a collection nor a document ID expression, the input document's own path is - * reused, so inserting documents read from the database fails. + * used, so inserting documents read from the database fails with {@code ALREADY_EXISTS}, + * and a document without a {@code __name__} field fails with {@code INVALID_ARGUMENT}. *
  • With only {@link Insert#withCollection(String) withCollection}, the document is written * to that collection under the input document's ID (or a generated ID if it has none). *
  • With only {@link Insert#withDocumentIdExpression(Expression) withDocumentIdExpression}, @@ -1425,7 +1427,8 @@ public Pipeline upsert(Upsert upsert) { /** * Writes each document produced by the previous stages back to its own path, after adding the * given fields. The document is created if it doesn't exist and replaced if it does; stored - * fields are not merged. + * fields are not merged. A document without a {@code __name__} field fails with {@code + * INVALID_ARGUMENT}. * *

    Writes are applied atomically (all or none) only if the pipeline is executed with {@link * PipelineExecuteOptions#withAtomic(boolean) withAtomic(true)} or inside a transaction. diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineSource.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineSource.java index 293b9d764d74..025051850789 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineSource.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineSource.java @@ -152,14 +152,14 @@ public Pipeline documents(String... docs) { /** * Creates a new {@link Pipeline} that operates on a static set of documents represented as Maps. * - *

    Each map becomes one input document. At least one document is required; an empty set of - * documents is rejected by the backend when the pipeline is executed. Field values can be - * constants or {@link com.google.cloud.firestore.pipeline.expressions.Expression}s; expressions - * are evaluated, including expressions nested inside map or list values. {@link FieldValue} - * sentinels (for example, {@code FieldValue.serverTimestamp()}) are not supported and are - * rejected; use an expression such as {@code currentTimestamp()} instead. To give a document a - * path (for example, for an in-place {@code insert()}, {@code upsert()} or {@code update()}), set - * its {@code __name__} field to a {@link DocumentReference}. + *

    Each map becomes one input document. At least one document is required: executing a pipeline + * with no documents fails with {@code INVALID_ARGUMENT}. Field values can be constants or {@link + * com.google.cloud.firestore.pipeline.expressions.Expression}s; expressions are evaluated, + * including expressions nested inside map or list values. {@link FieldValue} sentinels (for + * example, {@code FieldValue.serverTimestamp()}) are not supported; use an expression such as + * {@code currentTimestamp()} instead. To give a document a path (for example, for an in-place + * {@code insert()}, {@code upsert()} or {@code update()}), set its {@code __name__} field to a + * {@link DocumentReference}. * *

    Example: * @@ -190,14 +190,14 @@ public final Pipeline literals(java.util.Map... data) { *

    This is equivalent to {@link #literals(java.util.Map...)}, taking the documents as a {@link * java.util.List}. * - *

    Each map becomes one input document. At least one document is required; an empty set of - * documents is rejected by the backend when the pipeline is executed. Field values can be - * constants or {@link com.google.cloud.firestore.pipeline.expressions.Expression}s; expressions - * are evaluated, including expressions nested inside map or list values. {@link FieldValue} - * sentinels (for example, {@code FieldValue.serverTimestamp()}) are not supported and are - * rejected; use an expression such as {@code currentTimestamp()} instead. To give a document a - * path (for example, for an in-place {@code insert()}, {@code upsert()} or {@code update()}), set - * its {@code __name__} field to a {@link DocumentReference}. + *

    Each map becomes one input document. At least one document is required: executing a pipeline + * with no documents fails with {@code INVALID_ARGUMENT}. Field values can be constants or {@link + * com.google.cloud.firestore.pipeline.expressions.Expression}s; expressions are evaluated, + * including expressions nested inside map or list values. {@link FieldValue} sentinels (for + * example, {@code FieldValue.serverTimestamp()}) are not supported; use an expression such as + * {@code currentTimestamp()} instead. To give a document a path (for example, for an in-place + * {@code insert()}, {@code upsert()} or {@code update()}), set its {@code __name__} field to a + * {@link DocumentReference}. * * @param data The Maps representing documents to include in the pipeline. * @return A new {@code Pipeline} instance with a literals source. diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java index 1b3cbae1e3c0..e3ed0b929186 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java @@ -28,15 +28,15 @@ /** * A pipeline stage that inserts each document produced by the previous stages as a new document. * - *

    The insert fails with {@code ALREADY_EXISTS} if a target document already exists. Because - * documents read from the database keep their path, inserting them without a collection or document - * ID expression (or into their own collection under their own ID) fails. + *

    The insert fails with {@code ALREADY_EXISTS} if a target document already exists. * *

    The target path of each document is determined as follows: * *

      - *
    • With neither a collection nor a document ID expression, the input document's own path is - * reused. + *
    • With neither a collection nor a document ID expression, the input document's own path (its + * {@code __name__} field) is used, so inserting documents read from the database fails with + * {@code ALREADY_EXISTS}, and a document without a {@code __name__} field fails with {@code + * INVALID_ARGUMENT}. *
    • With only a collection, the document is written to that collection under the input * document's ID. If the input document has no ID (for example, a literal document without a * {@code __name__} field, or a document whose {@code __name__} field was removed), an ID is diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Literals.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Literals.java index ba8323ff917a..81c80f17f580 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Literals.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Literals.java @@ -36,7 +36,6 @@ public final class Literals extends Stage { public Literals(Map... data) { super("literals", InternalOptions.EMPTY); - // An empty set of documents is rejected by the backend when the pipeline is executed. Preconditions.checkNotNull( data, "Function literals() called with invalid data. Documents must not be null."); for (int i = 0; i < data.length; i++) { @@ -48,8 +47,8 @@ public Literals(Map... data) { } /** - * Rejects {@link FieldValue} sentinels anywhere in a literal value. Literal documents are not - * written with set() or update(), so there is no pipeline representation for field transforms. + * Throws if {@code value} contains a {@link FieldValue} sentinel at any depth. Sentinels are + * field transforms of set() and update() and have no pipeline encoding. */ private static void validateNoSentinels(Object value, List path) { if (value instanceof FieldValue) { diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java index 5c8453193e6e..222ce551fcac 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java @@ -32,14 +32,15 @@ * A pipeline stage that writes each document produced by the previous stages, creating the target * document if it doesn't exist and replacing it if it does. * - *

      Upsert replaces the whole target document; it does not merge with the stored fields. Stored - * fields that are not in the pipeline document are removed. + *

      The whole target document is replaced, not merged: stored fields that are not in the pipeline + * document are removed. * *

      The target path of each document is determined as follows: * *

        - *
      • With neither a collection nor a document ID expression, the input document's own path is - * reused. + *
      • With neither a collection nor a document ID expression, the input document's own path (its + * {@code __name__} field) is used. A document without a {@code __name__} field fails with + * {@code INVALID_ARGUMENT}. *
      • With only a collection, the document is written to that collection under the input * document's ID. If the input document has no ID (for example, a literal document without a * {@code __name__} field, or a document whose {@code __name__} field was removed), an ID is diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java index ca9fe7e61b0f..0103217f8550 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java @@ -446,18 +446,15 @@ public void testLiteralsWithNestedExpressionsProtoEncoding() { Stage literalsStage = firestore.pipeline().literals(doc).toProto().getStages(0); java.util.Map fields = literalsStage.getArgs(0).getMapValue().getFieldsMap(); - // Top-level expressions are encoded as is. assertThat(fields.get("top").getFunctionValue().getName()).isEqualTo("add"); - // A map or list containing a nested expression is wrapped in a map(...) / array(...) - // expression, so the backend evaluates the nested expression. assertThat(fields.get("nestedMap").getFunctionValue().getName()).isEqualTo("map"); assertThat(fields.get("nestedList").getFunctionValue().getName()).isEqualTo("array"); java.util.List listArgs = fields.get("nestedList").getFunctionValue().getArgsList(); assertThat(listArgs.get(0).getIntegerValue()).isEqualTo(1L); assertThat(listArgs.get(1).getFunctionValue().getName()).isEqualTo("add"); - // Values without expressions keep their existing encoding. + // Maps and lists without expressions are encoded as plain map and array values. assertThat(fields.get("constantMap").hasMapValue()).isTrue(); assertThat( fields @@ -479,12 +476,10 @@ public void testLiteralsValidation() { .build(); Firestore firestore = options.getService(); - // An empty set of documents is not rejected client-side; the backend rejects it on execute. Stage emptyStage = firestore.pipeline().literals().toProto().getStages(0); assertThat(emptyStage.getName()).isEqualTo("literals"); assertThat(emptyStage.getArgsCount()).isEqualTo(0); - // A null documents array is rejected with a clear message. NullPointerException nullArray = assertThrows( NullPointerException.class, @@ -493,7 +488,6 @@ public void testLiteralsValidation() { .hasMessageThat() .isEqualTo("Function literals() called with invalid data. Documents must not be null."); - // A null document is rejected with a clear message. NullPointerException nullDoc = assertThrows( NullPointerException.class, @@ -502,7 +496,6 @@ public void testLiteralsValidation() { .hasMessageThat() .isEqualTo("Function literals() called with invalid data. Document 0 is null."); - // FieldValue sentinels are rejected with the field path, including nested ones. java.util.Map nested = new java.util.HashMap<>(); nested.put("b", FieldValue.serverTimestamp()); java.util.Map doc = new java.util.HashMap<>(); @@ -530,7 +523,7 @@ public void testLiteralsValidation() { assertThat(delete).hasMessageThat().contains("FieldValue.delete()"); assertThat(delete).hasMessageThat().contains("(found in field `a.b`)"); - // null values are allowed. + // Null field values are allowed. java.util.Map withNull = new java.util.HashMap<>(); withNull.put("n", null); Stage stage = firestore.pipeline().literals(withNull).toProto().getStages(0); @@ -551,13 +544,11 @@ public void testListOverloadsProtoEncoding() { java.util.Map doc2 = new java.util.HashMap<>(); doc2.put("title", "Book 2"); - // literals(List) encodes the same as literals(Map...). assertThat(firestore.pipeline().literals(java.util.Arrays.asList(doc1, doc2)).toProto()) .isEqualTo(firestore.pipeline().literals(doc1, doc2).toProto()); assertThat(firestore.pipeline().literals(java.util.Collections.emptyList()).toProto()) .isEqualTo(firestore.pipeline().literals().toProto()); - // Update.withTransformedFields(List) encodes the same as the varargs overload. com.google.cloud.firestore.pipeline.stages.Update fromList = new com.google.cloud.firestore.pipeline.stages.Update() .withTransformedFields( @@ -581,14 +572,12 @@ public void testInsertAndDeleteConvenienceOverloadsProtoEncoding() { .build(); Firestore firestore = options.getService(); - // insert() is equivalent to insert(new Insert()). com.google.firestore.v1.Pipeline insertProto = firestore.pipeline().collection("books").insert().toProto(); assertThat(insertProto) .isEqualTo(firestore.pipeline().collection("books").insert(new Insert()).toProto()); assertThat(insertProto.getStages(1).getName()).isEqualTo("insert"); - // delete(new Delete()) is equivalent to delete(). com.google.firestore.v1.Pipeline deleteProto = firestore .pipeline() @@ -637,7 +626,7 @@ public void testAtomicExecutionRejectedWithTransactionOrReadTime() { () -> pipeline.toExecutePipelineRequest(atomic, null, readTime)); assertThat(withReadTime).hasMessageThat().contains("read time"); - // Non-atomic execution inside a transaction or at a read time is unaffected. + // Non-atomic execution is allowed in a transaction and at a read time. PipelineExecuteOptions nonAtomic = new PipelineExecuteOptions(); ExecutePipelineRequest txRequest = pipeline.toExecutePipelineRequest(nonAtomic, transactionId, null); diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java index 14c2421d2316..af4324e97db4 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java @@ -4975,7 +4975,7 @@ public void testAtomicExecutionInsideTransactionIsRejected() throws Exception { assertThat(exception.getCause()).isInstanceOf(IllegalArgumentException.class); assertThat(exception.getCause()).hasMessageThat().contains("inside a transaction"); - // The pipeline was rejected client-side, so nothing was deleted. + // The pipeline is rejected before it is sent, so nothing is deleted. assertThat(dmlCol.document("book1").get().get().exists()).isTrue(); } From 180ad3bbdfb8ca2221cb47e0c07dae87dd4c1c2a Mon Sep 17 00:00:00 2001 From: dlarocque Date: Fri, 9 Oct 2026 16:00:31 -0400 Subject: [PATCH 15/18] revert(firestore): don't reject atomic pipeline execution in a transaction or at a read time This reverts commit 74d47c44224. Atomic execution inside Transaction.execute is redundant but harmless, and atomic execution with a read time is rejected by the backend, so the SDK doesn't need dedicated checks for either. --- .../com/google/cloud/firestore/Pipeline.java | 12 ----- .../cloud/firestore/PipelineProtoTest.java | 50 ------------------- .../cloud/firestore/it/ITPipelineTest.java | 28 ----------- 3 files changed, 90 deletions(-) diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java index 397b5e385569..8d2c59510ef0 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java @@ -1658,18 +1658,6 @@ ExecutePipelineRequest toExecutePipelineRequest( @Nonnull PipelineExecuteOptions options, @Nullable final ByteString transactionId, @Nullable com.google.protobuf.Timestamp readTime) { - if (options.isAtomic()) { - if (transactionId != null) { - throw new IllegalArgumentException( - "Atomic pipeline execution cannot be used inside a transaction. The transaction" - + " already makes its writes atomic; remove withAtomic(true)."); - } - if (readTime != null) { - throw new IllegalArgumentException( - "Atomic pipeline execution cannot be combined with a read time."); - } - } - java.util.Map optionsMap = new java.util.HashMap<>(StageUtils.toMap(options)); optionsMap.remove("atomic"); diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java index 0103217f8550..2feb811e1e26 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java @@ -587,54 +587,4 @@ public void testInsertAndDeleteConvenienceOverloadsProtoEncoding() { assertThat(deleteProto).isEqualTo(firestore.pipeline().collection("books").delete().toProto()); assertThat(deleteProto.getStages(1).getName()).isEqualTo("delete"); } - - @Test - public void testAtomicExecutionRejectedWithTransactionOrReadTime() { - FirestoreOptions options = - FirestoreOptions.newBuilder() - .setProjectId("new-project") - .setDatabaseId("(default)") - .build(); - Firestore firestore = options.getService(); - - java.util.Map data = new java.util.HashMap<>(); - data.put("title", "Atomic Book"); - Pipeline pipeline = - firestore - .pipeline() - .literals(data) - .insert( - new com.google.cloud.firestore.pipeline.stages.Insert() - .withCollection("books") - .withDocumentIdExpression(constant("book1"))); - - PipelineExecuteOptions atomic = new PipelineExecuteOptions().withAtomic(true); - com.google.protobuf.ByteString transactionId = - com.google.protobuf.ByteString.copyFromUtf8("txn"); - com.google.protobuf.Timestamp readTime = - com.google.protobuf.Timestamp.newBuilder().setSeconds(1).build(); - - IllegalArgumentException inTransaction = - assertThrows( - IllegalArgumentException.class, - () -> pipeline.toExecutePipelineRequest(atomic, transactionId, null)); - assertThat(inTransaction).hasMessageThat().contains("inside a transaction"); - - IllegalArgumentException withReadTime = - assertThrows( - IllegalArgumentException.class, - () -> pipeline.toExecutePipelineRequest(atomic, null, readTime)); - assertThat(withReadTime).hasMessageThat().contains("read time"); - - // Non-atomic execution is allowed in a transaction and at a read time. - PipelineExecuteOptions nonAtomic = new PipelineExecuteOptions(); - ExecutePipelineRequest txRequest = - pipeline.toExecutePipelineRequest(nonAtomic, transactionId, null); - assertThat(txRequest.getTransaction()).isEqualTo(transactionId); - assertThat(txRequest.hasNewTransaction()).isFalse(); - assertThat(txRequest.getAutoCommitTransaction()).isFalse(); - ExecutePipelineRequest readTimeRequest = - pipeline.toExecutePipelineRequest(nonAtomic, null, readTime); - assertThat(readTimeRequest.getReadTime()).isEqualTo(readTime); - } } diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java index af4324e97db4..0562ce3c6948 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java @@ -4951,34 +4951,6 @@ public void testDeleteStageInsideTransaction() throws Exception { assertThat(dmlCol.document("book1").get().get().exists()).isFalse(); } - @Test - public void testAtomicExecutionInsideTransactionIsRejected() throws Exception { - CollectionReference dmlCol = testCollectionWithDocs(bookDocs); - ExecutionException exception = - assertThrows( - ExecutionException.class, - () -> - firestore - .runTransaction( - transaction -> { - Pipeline deletePpl = - firestore - .pipeline() - .collection(dmlCol.getPath()) - .where(equal(field("__name__").documentId(), constant("book1"))) - .delete(); - return transaction - .execute(deletePpl, new PipelineExecuteOptions().withAtomic(true)) - .get(); - }) - .get()); - assertThat(exception.getCause()).isInstanceOf(IllegalArgumentException.class); - assertThat(exception.getCause()).hasMessageThat().contains("inside a transaction"); - - // The pipeline is rejected before it is sent, so nothing is deleted. - assertThat(dmlCol.document("book1").get().get().exists()).isTrue(); - } - @Test public void testUpdateSingleDocWithAddFields() throws Exception { CollectionReference dmlCol = testCollectionWithDocs(bookDocs); From 200aadb7c345d8f921fcc121f1d0436ea6ce911b Mon Sep 17 00:00:00 2001 From: dlarocque Date: Fri, 9 Oct 2026 16:03:06 -0400 Subject: [PATCH 16/18] refactor(firestore): reject FieldValue sentinels in literals through UserDataConverter Literal values are already encoded with UserDataConverter, which rejects FieldValue sentinels in pipeline arguments. Drop the separate sentinel walk and the literals-specific message, and use plain null-check messages. Sentinels are now reported when the pipeline is encoded rather than when literals() is called. --- .../cloud/firestore/PipelineSource.java | 32 ++++---------- .../google/cloud/firestore/PipelineUtils.java | 9 ---- .../firestore/pipeline/stages/Literals.java | 42 +------------------ .../cloud/firestore/PipelineProtoTest.java | 25 ++++------- 4 files changed, 19 insertions(+), 89 deletions(-) diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineSource.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineSource.java index 025051850789..a5c6db1be74f 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineSource.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineSource.java @@ -152,14 +152,12 @@ public Pipeline documents(String... docs) { /** * Creates a new {@link Pipeline} that operates on a static set of documents represented as Maps. * - *

        Each map becomes one input document. At least one document is required: executing a pipeline - * with no documents fails with {@code INVALID_ARGUMENT}. Field values can be constants or {@link - * com.google.cloud.firestore.pipeline.expressions.Expression}s; expressions are evaluated, - * including expressions nested inside map or list values. {@link FieldValue} sentinels (for - * example, {@code FieldValue.serverTimestamp()}) are not supported; use an expression such as - * {@code currentTimestamp()} instead. To give a document a path (for example, for an in-place - * {@code insert()}, {@code upsert()} or {@code update()}), set its {@code __name__} field to a - * {@link DocumentReference}. + *

        Each map becomes one input document. Field values can be constants or {@link + * com.google.cloud.firestore.pipeline.expressions.Expression}s, including expressions nested + * inside map or list values. {@link FieldValue} sentinels are not supported; use an expression + * such as {@code currentTimestamp()} instead. To give a document a path, set its {@code __name__} + * field to a {@link DocumentReference}. Executing a pipeline with no documents fails with {@code + * INVALID_ARGUMENT}. * *

        Example: * @@ -177,7 +175,6 @@ public Pipeline documents(String... docs) { * * @param data The Maps representing documents to include in the pipeline. * @return A new {@code Pipeline} instance with a literals source. - * @throws IllegalArgumentException if a value contains a {@link FieldValue} sentinel. */ @Nonnull public final Pipeline literals(java.util.Map... data) { @@ -186,28 +183,15 @@ public final Pipeline literals(java.util.Map... data) { /** * Creates a new {@link Pipeline} that operates on a static set of documents represented as Maps. - * - *

        This is equivalent to {@link #literals(java.util.Map...)}, taking the documents as a {@link - * java.util.List}. - * - *

        Each map becomes one input document. At least one document is required: executing a pipeline - * with no documents fails with {@code INVALID_ARGUMENT}. Field values can be constants or {@link - * com.google.cloud.firestore.pipeline.expressions.Expression}s; expressions are evaluated, - * including expressions nested inside map or list values. {@link FieldValue} sentinels (for - * example, {@code FieldValue.serverTimestamp()}) are not supported; use an expression such as - * {@code currentTimestamp()} instead. To give a document a path (for example, for an in-place - * {@code insert()}, {@code upsert()} or {@code update()}), set its {@code __name__} field to a - * {@link DocumentReference}. + * Equivalent to {@link #literals(java.util.Map...)}. * * @param data The Maps representing documents to include in the pipeline. * @return A new {@code Pipeline} instance with a literals source. - * @throws IllegalArgumentException if a value contains a {@link FieldValue} sentinel. */ @Nonnull @SuppressWarnings("unchecked") public Pipeline literals(@Nonnull java.util.List> data) { - Preconditions.checkNotNull( - data, "Function literals() called with invalid data. Documents must not be null."); + Preconditions.checkNotNull(data, "Documents must not be null."); return literals(data.toArray(new java.util.Map[0])); } diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineUtils.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineUtils.java index 0a7857b6f7ea..6a9ef738a9af 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineUtils.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineUtils.java @@ -53,15 +53,6 @@ public static Value encodeValue(Object value) { return UserDataConverter.encodeValue(FieldPath.empty(), value, UserDataConverter.ARGUMENT); } - /** - * Returns the user-facing name of a {@link FieldValue} sentinel, for example {@code - * FieldValue.serverTimestamp()}. - */ - @InternalApi - public static String fieldValueMethodName(FieldValue fieldValue) { - return fieldValue.getMethodName(); - } - @InternalApi public static Value encodeValue(Expression value) { return exprToValue(value); diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Literals.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Literals.java index 81c80f17f580..b6230a0a68f6 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Literals.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Literals.java @@ -17,8 +17,6 @@ package com.google.cloud.firestore.pipeline.stages; import com.google.api.core.InternalApi; -import com.google.cloud.firestore.FieldPath; -import com.google.cloud.firestore.FieldValue; import com.google.cloud.firestore.PipelineUtils; import com.google.cloud.firestore.pipeline.expressions.Expression; import com.google.common.base.Preconditions; @@ -36,49 +34,13 @@ public final class Literals extends Stage { public Literals(Map... data) { super("literals", InternalOptions.EMPTY); - Preconditions.checkNotNull( - data, "Function literals() called with invalid data. Documents must not be null."); + Preconditions.checkNotNull(data, "Documents must not be null."); for (int i = 0; i < data.length; i++) { - Preconditions.checkNotNull( - data[i], "Function literals() called with invalid data. Document %s is null.", i); - validateNoSentinels(data[i], new ArrayList<>()); + Preconditions.checkNotNull(data[i], "Document %s must not be null.", i); } this.data = Arrays.asList(data); } - /** - * Throws if {@code value} contains a {@link FieldValue} sentinel at any depth. Sentinels are - * field transforms of set() and update() and have no pipeline encoding. - */ - private static void validateNoSentinels(Object value, List path) { - if (value instanceof FieldValue) { - throw new IllegalArgumentException( - String.format( - "Function literals() called with invalid data. %s can only be used with update() and" - + " set() (found in field %s)", - PipelineUtils.fieldValueMethodName((FieldValue) value), formatPath(path))); - } else if (value instanceof Map) { - for (Map.Entry entry : ((Map) value).entrySet()) { - path.add(String.valueOf(entry.getKey())); - validateNoSentinels(entry.getValue(), path); - path.remove(path.size() - 1); - } - } else if (value instanceof List) { - for (Object child : (List) value) { - validateNoSentinels(child, path); - } - } - } - - private static String formatPath(List path) { - try { - return FieldPath.of(path.toArray(new String[0])).toString(); - } catch (IllegalArgumentException e) { - // Empty field names cannot be represented as a FieldPath. - return String.join(".", path); - } - } - @Override Iterable toStageArgs() { List args = new ArrayList<>(); diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java index 2feb811e1e26..030f24261317 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java @@ -484,44 +484,37 @@ public void testLiteralsValidation() { assertThrows( NullPointerException.class, () -> firestore.pipeline().literals((java.util.Map[]) null)); - assertThat(nullArray) - .hasMessageThat() - .isEqualTo("Function literals() called with invalid data. Documents must not be null."); + assertThat(nullArray).hasMessageThat().isEqualTo("Documents must not be null."); NullPointerException nullDoc = assertThrows( NullPointerException.class, () -> firestore.pipeline().literals((java.util.Map) null)); - assertThat(nullDoc) - .hasMessageThat() - .isEqualTo("Function literals() called with invalid data. Document 0 is null."); + assertThat(nullDoc).hasMessageThat().isEqualTo("Document 0 must not be null."); java.util.Map nested = new java.util.HashMap<>(); nested.put("b", FieldValue.serverTimestamp()); java.util.Map doc = new java.util.HashMap<>(); doc.put("a", nested); IllegalArgumentException sentinel = - assertThrows(IllegalArgumentException.class, () -> firestore.pipeline().literals(doc)); - assertThat(sentinel) - .hasMessageThat() - .isEqualTo( - "Function literals() called with invalid data. FieldValue.serverTimestamp() can only" - + " be used with update() and set() (found in field a.b)"); + assertThrows( + IllegalArgumentException.class, () -> firestore.pipeline().literals(doc).toProto()); + assertThat(sentinel).hasMessageThat().contains("FieldValue.serverTimestamp()"); java.util.Map inList = new java.util.HashMap<>(); inList.put("list", java.util.Arrays.asList(1, FieldValue.increment(1))); IllegalArgumentException listSentinel = - assertThrows(IllegalArgumentException.class, () -> firestore.pipeline().literals(inList)); + assertThrows( + IllegalArgumentException.class, () -> firestore.pipeline().literals(inList).toProto()); assertThat(listSentinel).hasMessageThat().contains("FieldValue.increment()"); - assertThat(listSentinel).hasMessageThat().contains("(found in field list)"); java.util.Map deleteDoc = new java.util.HashMap<>(); deleteDoc.put("a.b", FieldValue.delete()); IllegalArgumentException delete = assertThrows( - IllegalArgumentException.class, () -> firestore.pipeline().literals(deleteDoc)); + IllegalArgumentException.class, + () -> firestore.pipeline().literals(deleteDoc).toProto()); assertThat(delete).hasMessageThat().contains("FieldValue.delete()"); - assertThat(delete).hasMessageThat().contains("(found in field `a.b`)"); // Null field values are allowed. java.util.Map withNull = new java.util.HashMap<>(); From a910279f56b2482881a1042a858bfbfdf1c61bcd Mon Sep 17 00:00:00 2001 From: dlarocque Date: Fri, 9 Oct 2026 16:04:09 -0400 Subject: [PATCH 17/18] docs(firestore): trim pipeline DML Javadoc Document atomic execution once, on PipelineExecuteOptions.withAtomic, instead of on every DML method. Describe the insert and upsert target rules only on the Insert and Upsert classes, and remove the Javadoc for the rejected-atomic-in-transaction behavior that is no longer thrown. --- .../com/google/cloud/firestore/Pipeline.java | 45 +++---------------- .../google/cloud/firestore/Transaction.java | 4 -- .../firestore/pipeline/stages/Insert.java | 4 +- .../stages/PipelineExecuteOptions.java | 6 --- .../firestore/pipeline/stages/Upsert.java | 9 +--- 5 files changed, 8 insertions(+), 60 deletions(-) diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java index 8d2c59510ef0..9b9b36cf6322 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java @@ -1204,9 +1204,6 @@ public Pipeline unnest(Selectable field, UnnestOptions options) { *

        Each document produced by the previous stages is deleted from the database. The result * contains a single document with a {@code documents_modified} field. * - *

        Writes are applied atomically (all or none) only if the pipeline is executed with {@link - * PipelineExecuteOptions#withAtomic(boolean) withAtomic(true)} or inside a transaction. - * *

        Example: * *

        {@code
        @@ -1248,9 +1245,6 @@ public Pipeline delete(Delete delete) {
            * #removeFields(String, String...)}) are also removed from the stored document. This stage does
            * not create documents: documents that don't exist are skipped.
            *
        -   * 

        Writes are applied atomically (all or none) only if the pipeline is executed with {@link - * PipelineExecuteOptions#withAtomic(boolean) withAtomic(true)} or inside a transaction. - * *

        Example 1: Update a collection's schema by adding a new field and removing an old one. * *

        {@code
        @@ -1335,17 +1329,11 @@ public Pipeline update(Update update) {
            * Inserts each document produced by the previous stages at its own path, given by its {@code
            * __name__} field.
            *
        -   * 

        This is equivalent to {@code insert(new Insert())}. It is mainly useful with a {@link - * PipelineSource#literals(java.util.Map...) literals} source whose documents set {@code __name__} - * to a {@link DocumentReference}: documents read from the database already exist, and a document - * without a {@code __name__} field fails with {@code INVALID_ARGUMENT}. To insert into another - * collection or under another ID, use {@link #insert(Insert)}. + *

        This is equivalent to {@code insert(new Insert())}. A document without a {@code __name__} + * field fails with {@code INVALID_ARGUMENT}. * *

        The insert fails with {@code ALREADY_EXISTS} if a target document already exists. * - *

        Writes are applied atomically (all or none) only if the pipeline is executed with {@link - * PipelineExecuteOptions#withAtomic(boolean) withAtomic(true)} or inside a transaction. - * * @return A new {@code Pipeline} object with this stage appended to the stage list. */ public Pipeline insert() { @@ -1356,22 +1344,8 @@ public Pipeline insert() { * Inserts each document produced by the previous stages as a new document, using a configured * {@link Insert} stage. * - *

        The insert fails with {@code ALREADY_EXISTS} if a target document already exists. The target - * path of each document is determined as follows: - * - *

          - *
        • With neither a collection nor a document ID expression, the input document's own path is - * used, so inserting documents read from the database fails with {@code ALREADY_EXISTS}, - * and a document without a {@code __name__} field fails with {@code INVALID_ARGUMENT}. - *
        • With only {@link Insert#withCollection(String) withCollection}, the document is written - * to that collection under the input document's ID (or a generated ID if it has none). - *
        • With only {@link Insert#withDocumentIdExpression(Expression) withDocumentIdExpression}, - * the document is written to the input document's parent collection. - *
        • With both, the document is written to the collection under the evaluated ID. - *
        - * - *

        Writes are applied atomically (all or none) only if the pipeline is executed with {@link - * PipelineExecuteOptions#withAtomic(boolean) withAtomic(true)} or inside a transaction. + *

        The insert fails with {@code ALREADY_EXISTS} if a target document already exists. See {@link + * Insert} for how the target path of each document is determined. * *

        Example: * @@ -1398,13 +1372,7 @@ public Pipeline insert(Insert insert) { * The target document is created if it doesn't exist and replaced if it does; stored fields are * not merged. * - *

        The target path follows the same rules as {@link #insert(Insert)}: by default each document - * is written back to its own path; {@link Upsert#withCollection(String) withCollection} and - * {@link Upsert#withDocumentIdExpression(Expression) withDocumentIdExpression} change the target - * collection and ID. - * - *

        Writes are applied atomically (all or none) only if the pipeline is executed with {@link - * PipelineExecuteOptions#withAtomic(boolean) withAtomic(true)} or inside a transaction. + *

        See {@link Upsert} for how the target path of each document is determined. * *

        Example: * @@ -1430,9 +1398,6 @@ public Pipeline upsert(Upsert upsert) { * fields are not merged. A document without a {@code __name__} field fails with {@code * INVALID_ARGUMENT}. * - *

        Writes are applied atomically (all or none) only if the pipeline is executed with {@link - * PipelineExecuteOptions#withAtomic(boolean) withAtomic(true)} or inside a transaction. - * *

        Example: * *

        {@code
        diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Transaction.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Transaction.java
        index c92c746b90ad..e11f300e069c 100644
        --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Transaction.java
        +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Transaction.java
        @@ -154,11 +154,7 @@ public abstract ApiFuture> getAll(
           /**
            * Executes the provided pipeline with specified options, within the transaction context.
            *
        -   * 

        The pipeline's writes are part of the transaction, so {@link - * PipelineExecuteOptions#withAtomic(boolean) withAtomic(true)} must not be set. - * * @return The result of the execution. - * @throws IllegalArgumentException if {@code options} enables atomic execution. */ @Nonnull public abstract ApiFuture execute( diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java index e3ed0b929186..c87fab63e16f 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Insert.java @@ -38,9 +38,7 @@ * {@code ALREADY_EXISTS}, and a document without a {@code __name__} field fails with {@code * INVALID_ARGUMENT}. *

      • With only a collection, the document is written to that collection under the input - * document's ID. If the input document has no ID (for example, a literal document without a - * {@code __name__} field, or a document whose {@code __name__} field was removed), an ID is - * generated. + * document's ID, or under a generated ID if the input document has no {@code __name__}. *
      • With only a document ID expression, the document is written to the input document's parent * collection under the ID that the expression evaluates to. *
      • With both, the document is written to the collection under the ID that the expression diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/PipelineExecuteOptions.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/PipelineExecuteOptions.java index 8181c0944964..b9d52aac6d66 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/PipelineExecuteOptions.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/PipelineExecuteOptions.java @@ -53,12 +53,6 @@ public PipelineExecuteOptions withIndexMode(String indexMode) { * insert}, {@code upsert}, {@code update} and {@code delete} stages) are applied or none are. The * default is {@code false}. * - *

        Atomic execution cannot be used with {@link - * com.google.cloud.firestore.Transaction#execute(com.google.cloud.firestore.Pipeline, - * PipelineExecuteOptions)}, because the enclosing transaction already makes the writes atomic, or - * in a read-only transaction with a read time. Executing such a pipeline throws {@link - * IllegalArgumentException}. - * * @param atomic Whether to execute the pipeline atomically. * @return A new {@code PipelineExecuteOptions} with the atomic setting applied. */ diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java index 222ce551fcac..7a24bad867d4 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Upsert.java @@ -30,10 +30,7 @@ /** * A pipeline stage that writes each document produced by the previous stages, creating the target - * document if it doesn't exist and replacing it if it does. - * - *

        The whole target document is replaced, not merged: stored fields that are not in the pipeline - * document are removed. + * document if it doesn't exist and replacing it if it does. Stored fields are not merged. * *

        The target path of each document is determined as follows: * @@ -42,9 +39,7 @@ * {@code __name__} field) is used. A document without a {@code __name__} field fails with * {@code INVALID_ARGUMENT}. *

      • With only a collection, the document is written to that collection under the input - * document's ID. If the input document has no ID (for example, a literal document without a - * {@code __name__} field, or a document whose {@code __name__} field was removed), an ID is - * generated. + * document's ID, or under a generated ID if the input document has no {@code __name__}. *
      • With only a document ID expression, the document is written to the input document's parent * collection under the ID that the expression evaluates to. *
      • With both, the document is written to the collection under the ID that the expression From b5b57131d61812c60a7c0114f933394f8f61f3a6 Mon Sep 17 00:00:00 2001 From: dlarocque Date: Tue, 20 Oct 2026 09:39:25 -0400 Subject: [PATCH 18/18] refactor(firestore): drop unnecessary Delete/List overloads and literals validation --- .../com/google/cloud/firestore/Pipeline.java | 12 --- .../cloud/firestore/PipelineSource.java | 14 --- .../firestore/pipeline/stages/Delete.java | 8 +- .../firestore/pipeline/stages/Literals.java | 5 - .../firestore/pipeline/stages/Update.java | 4 - .../cloud/firestore/PipelineProtoTest.java | 101 +----------------- .../cloud/firestore/it/ITPipelineTest.java | 9 -- 7 files changed, 3 insertions(+), 150 deletions(-) diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java index 9b9b36cf6322..50cb84b94f47 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/Pipeline.java @@ -1222,18 +1222,6 @@ public Pipeline delete() { return append(new Delete()); } - /** - * Performs a delete operation on documents from previous stages using a {@link Delete} stage. - * - *

        This is equivalent to {@link #delete()}. - * - * @param delete The {@code Delete} stage to append. - * @return A new {@code Pipeline} object with this stage appended to the stage list. - */ - public Pipeline delete(Delete delete) { - return append(delete); - } - /** * Performs an update operation using documents from previous stages. * diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineSource.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineSource.java index a5c6db1be74f..080e10d19a5c 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineSource.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/PipelineSource.java @@ -181,20 +181,6 @@ public final Pipeline literals(java.util.Map... data) { return new Pipeline(this.rpcContext, new Literals(data)); } - /** - * Creates a new {@link Pipeline} that operates on a static set of documents represented as Maps. - * Equivalent to {@link #literals(java.util.Map...)}. - * - * @param data The Maps representing documents to include in the pipeline. - * @return A new {@code Pipeline} instance with a literals source. - */ - @Nonnull - @SuppressWarnings("unchecked") - public Pipeline literals(@Nonnull java.util.List> data) { - Preconditions.checkNotNull(data, "Documents must not be null."); - return literals(data.toArray(new java.util.Map[0])); - } - /** * Creates a new {@link Pipeline} from the given {@link Query}. Under the hood, this will * translate the query semantics (order by document ID, etc.) to an equivalent pipeline. diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Delete.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Delete.java index 2185b46b71d9..b31d2ee35493 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Delete.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Delete.java @@ -16,15 +16,11 @@ package com.google.cloud.firestore.pipeline.stages; +import com.google.api.core.InternalApi; import com.google.firestore.v1.Value; import java.util.ArrayList; -/** - * A pipeline stage that deletes the documents produced by the previous stages. - * - *

        Use with {@link com.google.cloud.firestore.Pipeline#delete(Delete)}, or call {@link - * com.google.cloud.firestore.Pipeline#delete()} directly. - */ +@InternalApi public final class Delete extends Stage { private Delete(InternalOptions options) { super("delete", options); diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Literals.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Literals.java index b6230a0a68f6..a3e18cd51805 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Literals.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Literals.java @@ -19,7 +19,6 @@ import com.google.api.core.InternalApi; import com.google.cloud.firestore.PipelineUtils; import com.google.cloud.firestore.pipeline.expressions.Expression; -import com.google.common.base.Preconditions; import com.google.firestore.v1.Value; import java.util.ArrayList; import java.util.Arrays; @@ -34,10 +33,6 @@ public final class Literals extends Stage { public Literals(Map... data) { super("literals", InternalOptions.EMPTY); - Preconditions.checkNotNull(data, "Documents must not be null."); - for (int i = 0; i < data.length; i++) { - Preconditions.checkNotNull(data[i], "Document %s must not be null.", i); - } this.data = Arrays.asList(data); } diff --git a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Update.java b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Update.java index 4c33dd7b7610..3ce538018777 100644 --- a/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Update.java +++ b/java-firestore/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/stages/Update.java @@ -50,10 +50,6 @@ public Update withTransformedFields(Selectable... transformedFields) { return new Update(transformedFields, this.options); } - public Update withTransformedFields(List transformedFields) { - return withTransformedFields(transformedFields.toArray(new Selectable[0])); - } - @Override Iterable toStageArgs() { List args = new ArrayList<>(); diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java index 030f24261317..eb4bb7d4287b 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/PipelineProtoTest.java @@ -20,7 +20,6 @@ import static com.google.cloud.firestore.pipeline.expressions.Expression.constant; import static com.google.cloud.firestore.pipeline.expressions.Expression.field; import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.assertThrows; import com.google.cloud.firestore.pipeline.stages.Insert; import com.google.cloud.firestore.pipeline.stages.PipelineExecuteOptions; @@ -468,96 +467,7 @@ public void testLiteralsWithNestedExpressionsProtoEncoding() { } @Test - public void testLiteralsValidation() { - FirestoreOptions options = - FirestoreOptions.newBuilder() - .setProjectId("new-project") - .setDatabaseId("(default)") - .build(); - Firestore firestore = options.getService(); - - Stage emptyStage = firestore.pipeline().literals().toProto().getStages(0); - assertThat(emptyStage.getName()).isEqualTo("literals"); - assertThat(emptyStage.getArgsCount()).isEqualTo(0); - - NullPointerException nullArray = - assertThrows( - NullPointerException.class, - () -> firestore.pipeline().literals((java.util.Map[]) null)); - assertThat(nullArray).hasMessageThat().isEqualTo("Documents must not be null."); - - NullPointerException nullDoc = - assertThrows( - NullPointerException.class, - () -> firestore.pipeline().literals((java.util.Map) null)); - assertThat(nullDoc).hasMessageThat().isEqualTo("Document 0 must not be null."); - - java.util.Map nested = new java.util.HashMap<>(); - nested.put("b", FieldValue.serverTimestamp()); - java.util.Map doc = new java.util.HashMap<>(); - doc.put("a", nested); - IllegalArgumentException sentinel = - assertThrows( - IllegalArgumentException.class, () -> firestore.pipeline().literals(doc).toProto()); - assertThat(sentinel).hasMessageThat().contains("FieldValue.serverTimestamp()"); - - java.util.Map inList = new java.util.HashMap<>(); - inList.put("list", java.util.Arrays.asList(1, FieldValue.increment(1))); - IllegalArgumentException listSentinel = - assertThrows( - IllegalArgumentException.class, () -> firestore.pipeline().literals(inList).toProto()); - assertThat(listSentinel).hasMessageThat().contains("FieldValue.increment()"); - - java.util.Map deleteDoc = new java.util.HashMap<>(); - deleteDoc.put("a.b", FieldValue.delete()); - IllegalArgumentException delete = - assertThrows( - IllegalArgumentException.class, - () -> firestore.pipeline().literals(deleteDoc).toProto()); - assertThat(delete).hasMessageThat().contains("FieldValue.delete()"); - - // Null field values are allowed. - java.util.Map withNull = new java.util.HashMap<>(); - withNull.put("n", null); - Stage stage = firestore.pipeline().literals(withNull).toProto().getStages(0); - assertThat(stage.getArgs(0).getMapValue().getFieldsMap().get("n").hasNullValue()).isTrue(); - } - - @Test - public void testListOverloadsProtoEncoding() { - FirestoreOptions options = - FirestoreOptions.newBuilder() - .setProjectId("new-project") - .setDatabaseId("(default)") - .build(); - Firestore firestore = options.getService(); - - java.util.Map doc1 = new java.util.HashMap<>(); - doc1.put("title", "Book 1"); - java.util.Map doc2 = new java.util.HashMap<>(); - doc2.put("title", "Book 2"); - - assertThat(firestore.pipeline().literals(java.util.Arrays.asList(doc1, doc2)).toProto()) - .isEqualTo(firestore.pipeline().literals(doc1, doc2).toProto()); - assertThat(firestore.pipeline().literals(java.util.Collections.emptyList()).toProto()) - .isEqualTo(firestore.pipeline().literals().toProto()); - - com.google.cloud.firestore.pipeline.stages.Update fromList = - new com.google.cloud.firestore.pipeline.stages.Update() - .withTransformedFields( - java.util.Arrays.asList( - constant("Updated").as("status"), - add(field("count"), constant(1)).as("count"))); - com.google.cloud.firestore.pipeline.stages.Update fromVarargs = - new com.google.cloud.firestore.pipeline.stages.Update() - .withTransformedFields( - constant("Updated").as("status"), add(field("count"), constant(1)).as("count")); - assertThat(firestore.pipeline().collection("books").update(fromList).toProto()) - .isEqualTo(firestore.pipeline().collection("books").update(fromVarargs).toProto()); - } - - @Test - public void testInsertAndDeleteConvenienceOverloadsProtoEncoding() { + public void testInsertConvenienceOverloadProtoEncoding() { FirestoreOptions options = FirestoreOptions.newBuilder() .setProjectId("new-project") @@ -570,14 +480,5 @@ public void testInsertAndDeleteConvenienceOverloadsProtoEncoding() { assertThat(insertProto) .isEqualTo(firestore.pipeline().collection("books").insert(new Insert()).toProto()); assertThat(insertProto.getStages(1).getName()).isEqualTo("insert"); - - com.google.firestore.v1.Pipeline deleteProto = - firestore - .pipeline() - .collection("books") - .delete(new com.google.cloud.firestore.pipeline.stages.Delete()) - .toProto(); - assertThat(deleteProto).isEqualTo(firestore.pipeline().collection("books").delete().toProto()); - assertThat(deleteProto.getStages(1).getName()).isEqualTo("delete"); } } diff --git a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java index 0562ce3c6948..12f746f83654 100644 --- a/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java +++ b/java-firestore/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITPipelineTest.java @@ -5234,13 +5234,4 @@ public void testLiteralsWithNestedExpressions() throws Exception { assertThat(results.get(0).getData().get("nestedList")) .isEqualTo(java.util.Arrays.asList(1L, 3L)); } - - @Test - public void testLiteralsWithNoDocumentsIsRejectedByBackend() throws Exception { - ExecutionException exception = - assertThrows( - ExecutionException.class, () -> firestore.pipeline().literals().execute().get()); - assertThat(exception).hasMessageThat().contains("INVALID_ARGUMENT"); - assertThat(exception).hasMessageThat().contains("at least one document"); - } }