From 9d2efaac5af3618a637abe2dba23f63387dd086e Mon Sep 17 00:00:00 2001
From: Michael Pratt <mpratt@google.com>
Date: Wed, 19 Jun 2019 13:39:57 -0700
Subject: [PATCH] Add renamed children pathNodes to target parent

Otherwise future renames may miss Renamed calls.

PiperOrigin-RevId: 254060946
---
 pkg/p9/p9test/client_test.go | 60 ++++++++++++++++++++++++++++++++++--
 pkg/p9/server.go             |  2 +-
 2 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/pkg/p9/p9test/client_test.go b/pkg/p9/p9test/client_test.go
index 16799a790..907afdf1b 100644
--- a/pkg/p9/p9test/client_test.go
+++ b/pkg/p9/p9test/client_test.go
@@ -269,14 +269,14 @@ type fileGenerator func(*Harness, string, p9.File) (*Mock, *Mock, p9.File)
 func walkHelper(h *Harness, name string, dir p9.File) (parentBackend *Mock, walkedBackend *Mock, walked p9.File) {
 	_, parent, err := dir.Walk(nil)
 	if err != nil {
-		h.t.Fatalf("got walk err %v, want nil", err)
+		h.t.Fatalf("Walk(nil) got err %v, want nil", err)
 	}
 	defer parent.Close()
 	parentBackend = h.Pop(parent)
 
 	_, walked, err = parent.Walk([]string{name})
 	if err != nil {
-		h.t.Fatalf("got walk err %v, want nil", err)
+		h.t.Fatalf("Walk(%s) got err %v, want nil", name, err)
 	}
 	walkedBackend = h.Pop(walked)
 
@@ -854,6 +854,62 @@ func TestRenameAtInvalid(t *testing.T) {
 	}
 }
 
+// TestRenameSecondOrder tests that indirect rename targets continue to receive
+// Renamed calls after a rename of its renamed parent. i.e.,
+//
+// 1. Create /one/file
+// 2. Create /directory
+// 3. Rename /one -> /directory/one
+// 4. Rename /directory -> /three/foo
+// 5. file from (1) should still receive Renamed.
+//
+// This is a regression test for b/135219260.
+func TestRenameSecondOrder(t *testing.T) {
+	h, c := NewHarness(t)
+	defer h.Finish()
+
+	rootBackend, root := newRoot(h, c)
+	defer root.Close()
+
+	// Walk to /one.
+	_, oneBackend, oneFile := walkHelper(h, "one", root)
+	defer oneFile.Close()
+
+	// Walk to and generate /one/file.
+	//
+	// walkHelper re-walks to oneFile, so we need the second backend,
+	// which will also receive Renamed calls.
+	oneSecondBackend, fileBackend, fileFile := walkHelper(h, "file", oneFile)
+	defer fileFile.Close()
+
+	// Walk to and generate /directory.
+	_, directoryBackend, directoryFile := walkHelper(h, "directory", root)
+	defer directoryFile.Close()
+
+	// Rename /one to /directory/one.
+	rootBackend.EXPECT().RenameAt("one", directoryBackend, "one").Return(nil)
+	expectRenamed(oneBackend, []string{}, directoryBackend, "one")
+	expectRenamed(oneSecondBackend, []string{}, directoryBackend, "one")
+	expectRenamed(fileBackend, []string{}, oneBackend, "file")
+	if err := renameAt(h, root, directoryFile, "one", "one", false); err != nil {
+		h.t.Fatalf("got rename err %v, want nil", err)
+	}
+
+	// Walk to /three.
+	_, threeBackend, threeFile := walkHelper(h, "three", root)
+	defer threeFile.Close()
+
+	// Rename /directory to /three/foo.
+	rootBackend.EXPECT().RenameAt("directory", threeBackend, "foo").Return(nil)
+	expectRenamed(directoryBackend, []string{}, threeBackend, "foo")
+	expectRenamed(oneBackend, []string{}, directoryBackend, "one")
+	expectRenamed(oneSecondBackend, []string{}, directoryBackend, "one")
+	expectRenamed(fileBackend, []string{}, oneBackend, "file")
+	if err := renameAt(h, root, threeFile, "directory", "foo", false); err != nil {
+		h.t.Fatalf("got rename err %v, want nil", err)
+	}
+}
+
 func TestReadlink(t *testing.T) {
 	for name := range newTypeMap(nil) {
 		t.Run(name, func(t *testing.T) {
diff --git a/pkg/p9/server.go b/pkg/p9/server.go
index c3ab7dad1..bdeb495c2 100644
--- a/pkg/p9/server.go
+++ b/pkg/p9/server.go
@@ -264,7 +264,7 @@ func (f *fidRef) renameChildTo(oldName string, target *fidRef, newName string) {
 	})
 
 	// Replace the previous (now deleted) path node.
-	f.pathNode.children.Store(newName, origPathNode)
+	target.pathNode.children.Store(newName, origPathNode)
 
 	// Call Renamed on everything above.
 	notifyNameChange(origPathNode)