blob/fileblob: ensure a failed write due to IfNotExist doesn't update metadata (#3726)

Author: vangent

blob/fileblob/fileblob.go

@@ -840,14 +840,6 @@ func (w *writerWithSidecar) Close() error {
 		return err
 	}
 
-	md5sum := w.md5hash.Sum(nil)
-	w.attrs.MD5 = md5sum
-
-	// Write the attributes file.
-	if err := setAttrs(w.path, w.attrs); err != nil {
-		return err
-	}
-
 	if w.ifNotExist {
 		w.mu.Lock()
 		defer w.mu.Unlock()
@@ -856,6 +848,12 @@ func (w *writerWithSidecar) Close() error {
 			return gcerr.New(gcerrors.FailedPrecondition, err, 1, "File already exist")
 		}
 	}
+	// Write the attributes file.
+	md5sum := w.md5hash.Sum(nil)
+	w.attrs.MD5 = md5sum
+	if err := setAttrs(w.path, w.attrs); err != nil {
+		return err
+	}
 	// Rename the temp file to path.
 	if err := os.Rename(w.f.Name(), w.path); err != nil {
 		_ = os.Remove(w.path + attrsExt)

blob/fileblob/fileblob_test.go

@@ -260,6 +260,40 @@ func TestNewBucket(t *testing.T) {
 	})
 }
 
+func TestIfNotExistHasNoSideEffects(t *testing.T) {
+	ctx := context.Background()
+	dir := t.TempDir()
+	const (
+		key          = "key"
+		origContents = "hello world"
+	)
+
+	b, err := OpenBucket(dir, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer b.Close()
+	if err := b.WriteAll(ctx, key, []byte(origContents), &blob.WriterOptions{
+		Metadata: map[string]string{"md": "a"}}); err != nil {
+		t.Fatalf("failed to write blob: %v", err)
+	}
+	if err := b.WriteAll(ctx, key, []byte("goodbye world"), &blob.WriterOptions{
+		IfNotExist: true,
+		Metadata:   map[string]string{"md": "b"}}); err == nil || !errors.Is(err, gcerrors.ErrFailedPrecondition) {
+		t.Fatalf("expected FailedPrecondition error, got %v", err)
+	}
+	if got, err := b.ReadAll(ctx, key); err != nil {
+		t.Errorf("failed to read blob: %v", err)
+	} else if string(got) != origContents {
+		t.Errorf("expected to read original contents, got %q", string(got))
+	}
+	if attrs, err := b.Attributes(ctx, key); err != nil {
+		t.Errorf("failed to get attributes for blob: %v", err)
+	} else if attrs.Metadata["md"] != "a" {
+		t.Errorf("expected metadata to be unchanged, got %v", attrs.Metadata)
+	}
+}
+
 func TestSignedURLReturnsUnimplementedWithNoURLSigner(t *testing.T) {
 	dir := t.TempDir()