Skip to content

Commit

Permalink
fix(firestore): Replace usage of transform with update_transform in b…
Browse files Browse the repository at this point in the history 
…atch write (#7864)

* fix(firestore): allow bulkwriter to set and update transforms

* feat(firestore): Fixing conformance tests

---------

Co-authored-by: kolea2 <45548808+kolea2@users.noreply.github.com>
  • Loading branch information
@bhshkh @kolea2
bhshkh and kolea2 committed May 16, 2023
1 parent de25670 commit 949e4d8
Show file tree
Hide file tree
Showing 64 changed files with 1,127 additions and 1,359 deletions.
37 changes: 20 additions & 17 deletions firestore/docref.go
View file
Expand Up @@ -363,40 +363,43 @@ func (d *DocumentRef) fpvsToWrites(fpvs []fpv, pc *pb.Precondition) ([]*pb.Write
}

// newUpdateWithTransform constructs operations for a commit. Most generally, it
// returns an update operation followed by a transform.
// returns an update operation with update transforms.
//
// If there are no serverTimestampPaths, the transform is omitted.
//
// If doc.Fields is empty, there are no updatePaths, and there is no precondition,
// the update is omitted, unless updateOnEmpty is true.
func (d *DocumentRef) newUpdateWithTransform(doc *pb.Document, updatePaths []FieldPath, pc *pb.Precondition, transforms []*pb.DocumentTransform_FieldTransform, updateOnEmpty bool) []*pb.Write {
var ws []*pb.Write
var w *pb.Write
initializedW := &pb.Write{
Operation: &pb.Write_Update{
Update: doc,
},
CurrentDocument: pc,
// If the mask is not set for an `update` and the document exists, any
// existing data will be overwritten.
UpdateMask: &pb.DocumentMask{},
}
if updateOnEmpty || len(doc.Fields) > 0 ||
len(updatePaths) > 0 || (pc != nil && len(transforms) == 0) {
w = initializedW
var mask *pb.DocumentMask
if updatePaths != nil {
sfps := toServiceFieldPaths(updatePaths)
sort.Strings(sfps) // TODO(jba): make tests pass without this
mask = &pb.DocumentMask{FieldPaths: sfps}
}
w := &pb.Write{
Operation: &pb.Write_Update{doc},
UpdateMask: mask,
CurrentDocument: pc,
}
ws = append(ws, w)
pc = nil // If the precondition is in the write, we don't need it in the transform.
w.UpdateMask = mask
}
if len(transforms) > 0 || pc != nil {
ws = append(ws, &pb.Write{
Operation: &pb.Write_Transform{
Transform: &pb.DocumentTransform{
Document: d.Path,
FieldTransforms: transforms,
},
},
CurrentDocument: pc,
})
if w == nil {
w = initializedW
}
w.UpdateTransforms = transforms
}
if w != nil {
ws = append(ws, w)
}
return ws
}
Expand Down
13 changes: 13 additions & 0 deletions firestore/integration_test.go
View file
Expand Up @@ -1929,6 +1929,19 @@ func TestIntegration_ColGroupRefPartitionsLarge(t *testing.T) {
t.Errorf("Unexpected number of documents across partitions: got %d, want %d", got, want)
}
}
func TestIntegration_BulkWriter_Set(t *testing.T) {
doc := iColl.NewDoc()
c := integrationClient(t)
ctx := context.Background()
bw := c.BulkWriter(ctx)

f := copyMap(integrationTestMap)
f["serverTimeStamp"] = ServerTimestamp
_, err := bw.Set(doc, f)
if err != nil {
t.Errorf("bulkwriter: error performing a set write: %v\n", err)
}
}

func TestIntegration_BulkWriter(t *testing.T) {
doc := iColl.NewDoc()
Expand Down
83 changes: 39 additions & 44 deletions firestore/internal/conformance/testdata/create-all-transforms.json
View file
Expand Up @@ -18,56 +18,51 @@
}
}
},
"updateTransforms": [
{
"fieldPath": "b",
"setToServerValue": "REQUEST_TIME"
},
{
"fieldPath": "c",
"appendMissingElements": {
"values": [
{
"integerValue": "1"
},
{
"integerValue": "2"
},
{
"integerValue": "3"
}
]
}
},
{
"fieldPath": "d",
"removeAllFromArray": {
"values": [
{
"integerValue": "4"
},
{
"integerValue": "5"
},
{
"integerValue": "6"
}
]
}
}
],
"currentDocument": {
"exists": false
}
},
{
"transform": {
"document": "projects/projectID/databases/(default)/documents/C/d",
"fieldTransforms": [
{
"fieldPath": "b",
"setToServerValue": "REQUEST_TIME"
},
{
"fieldPath": "c",
"appendMissingElements": {
"values": [
{
"integerValue": "1"
},
{
"integerValue": "2"
},
{
"integerValue": "3"
}
]
}
},
{
"fieldPath": "d",
"removeAllFromArray": {
"values": [
{
"integerValue": "4"
},
{
"integerValue": "5"
},
{
"integerValue": "6"
}
]
}
}
]
}
}
]
}
}
}
]
}
}
75 changes: 35 additions & 40 deletions firestore/internal/conformance/testdata/create-arrayremove-multi.json
View file
Expand Up @@ -18,52 +18,47 @@
}
}
},
"updateTransforms": [
{
"fieldPath": "b",
"removeAllFromArray": {
"values": [
{
"integerValue": "1"
},
{
"integerValue": "2"
},
{
"integerValue": "3"
}
]
}
},
{
"fieldPath": "c.d",
"removeAllFromArray": {
"values": [
{
"integerValue": "4"
},
{
"integerValue": "5"
},
{
"integerValue": "6"
}
]
}
}
],
"currentDocument": {
"exists": false
}
},
{
"transform": {
"document": "projects/projectID/databases/(default)/documents/C/d",
"fieldTransforms": [
{
"fieldPath": "b",
"removeAllFromArray": {
"values": [
{
"integerValue": "1"
},
{
"integerValue": "2"
},
{
"integerValue": "3"
}
]
}
},
{
"fieldPath": "c.d",
"removeAllFromArray": {
"values": [
{
"integerValue": "4"
},
{
"integerValue": "5"
},
{
"integerValue": "6"
}
]
}
}
]
}
}
]
}
}
}
]
}
}
43 changes: 19 additions & 24 deletions firestore/internal/conformance/testdata/create-arrayremove-nested.json
View file
Expand Up @@ -18,36 +18,31 @@
}
}
},
"updateTransforms": [
{
"fieldPath": "b.c",
"removeAllFromArray": {
"values": [
{
"integerValue": "1"
},
{
"integerValue": "2"
},
{
"integerValue": "3"
}
]
}
}
],
"currentDocument": {
"exists": false
}
},
{
"transform": {
"document": "projects/projectID/databases/(default)/documents/C/d",
"fieldTransforms": [
{
"fieldPath": "b.c",
"removeAllFromArray": {
"values": [
{
"integerValue": "1"
},
{
"integerValue": "2"
},
{
"integerValue": "3"
}
]
}
}
]
}
}
]
}
}
}
]
}
}
43 changes: 19 additions & 24 deletions firestore/internal/conformance/testdata/create-arrayremove.json
View file
Expand Up @@ -18,36 +18,31 @@
}
}
},
"updateTransforms": [
{
"fieldPath": "b",
"removeAllFromArray": {
"values": [
{
"integerValue": "1"
},
{
"integerValue": "2"
},
{
"integerValue": "3"
}
]
}
}
],
"currentDocument": {
"exists": false
}
},
{
"transform": {
"document": "projects/projectID/databases/(default)/documents/C/d",
"fieldTransforms": [
{
"fieldPath": "b",
"removeAllFromArray": {
"values": [
{
"integerValue": "1"
},
{
"integerValue": "2"
},
{
"integerValue": "3"
}
]
}
}
]
}
}
]
}
}
}
]
}
}

0 comments on commit 949e4d8

Please sign in to comment.