Fixed false error for implicit dir in rename and remove directory flow

internal/fs/fs.go

@@ -1375,6 +1375,18 @@ func (fs *fileSystem) CreateSymlink(
 
 // LOCKS_EXCLUDED(fs.mu)
 func (fs *fileSystem) RmDir(
+	// When rm -r or os.RemoveAll call is made, the following calls are made in order
+	//	 1. RmDir (only in the case of os.RemoveAll)
+	//	 2. Unlink all nested files,
+	//	 3. lookupInode call on implicit directory
+	//	 4. Rmdir on the directory.
+	//
+	// When type cache ttl is set, we construct an implicitDir even though one doesn't
+	// exist on GCS (https://github.com/GoogleCloudPlatform/gcsfuse/blob/master/internal/fs/inode/dir.go#L452),
+	// and thus, we get rmDir call to GCSFuse.
+	// Whereas when ttl is zero, lookupInode call itself fails and RmDir is not called
+	// because object is not present in GCS.
+
 	ctx context.Context,
 	op *fuseops.RmDirOp) (err error) {
 	// Find the parent.
@@ -1441,8 +1453,11 @@ func (fs *fileSystem) RmDir(
 	cleanUpAndUnlockChild()
 
 	// Delete the backing object.
+	fs.mu.Lock()
+	_, isImplicitDir := fs.implicitDirInodes[child.Name()]
+	fs.mu.Unlock()
 	parent.Lock()
-	err = parent.DeleteChildDir(ctx, op.Name)
+	err = parent.DeleteChildDir(ctx, op.Name, isImplicitDir)
 	parent.Unlock()
 
 	if err != nil {
@@ -1628,8 +1643,11 @@ func (fs *fileSystem) renameDir(
 	releaseInodes()
 
 	// Delete the backing object of the old directory.
+	fs.mu.Lock()
+	_, isImplicitDir := fs.implicitDirInodes[oldDir.Name()]
+	fs.mu.Unlock()
 	oldParent.Lock()
-	err = oldParent.DeleteChildDir(ctx, oldName)
+	err = oldParent.DeleteChildDir(ctx, oldName, isImplicitDir)
 	oldParent.Unlock()
 	if err != nil {
 		return fmt.Errorf("DeleteChildDir: %w", err)

internal/fs/implicit_dirs_with_cache_test.go

@@ -0,0 +1,107 @@
+// Copyright 2015 Google Inc. All Rights Reserved.
+//
+// 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.
+
+// A collection of tests for a file system where we do not attempt to write to
+// the file system at all. Rather we set up contents in a GCS bucket out of
+// band, wait for them to be available, and then read them via the file system.
+
+package fs_test
+
+import (
+	"os"
+	"os/exec"
+	"path"
+	"time"
+
+	. "github.com/jacobsa/oglematchers"
+	. "github.com/jacobsa/ogletest"
+)
+
+////////////////////////////////////////////////////////////////////////
+// Boilerplate
+////////////////////////////////////////////////////////////////////////
+
+type ImplicitDirsWithCacheTest struct {
+	fsTest
+}
+
+func init() {
+	RegisterTestSuite(&ImplicitDirsWithCacheTest{})
+}
+
+func (t *ImplicitDirsWithCacheTest) SetUpTestSuite() {
+	t.serverCfg.ImplicitDirectories = true
+	t.serverCfg.DirTypeCacheTTL = time.Minute * 3
+	t.fsTest.SetUpTestSuite()
+}
+
+////////////////////////////////////////////////////////////////////////
+// Tests
+////////////////////////////////////////////////////////////////////////
+
+func (t *ImplicitDirsWithCacheTest) TestRemoveAll() {
+	// Create an implicit directory.
+	AssertEq(
+		nil,
+		t.createObjects(
+			map[string]string{
+				// File
+				"foo/bar": "",
+			}))
+
+	// Attempt to recursively remove implicit dir.
+	cmd := exec.Command(
+		"rm",
+		"-r",
+		path.Join(mntDir, "foo/"),
+	)
+	output, err := cmd.CombinedOutput()
+
+	AssertEq("", string(output))
+	AssertEq(nil, err)
+}
+
+func (t *ImplicitDirsWithCacheTest) TestRenameImplicitDir() {
+	// Create an implicit directory.
+	AssertEq(
+		nil,
+		t.createObjects(
+			map[string]string{
+				// File
+				"foo/bar1": "",
+				"foo/bar2": "",
+			}))
+
+	// Attempt to rename implicit dir.
+	err := os.Rename(
+		path.Join(mntDir, "foo/"),
+		path.Join(mntDir, "fooNew/"),
+	)
+
+	//verify os.Rename successful
+	AssertEq(nil, err)
+	// verify the renamed files and directories
+	fi1, err := os.Stat(path.Join(mntDir, "fooNew"))
+	AssertEq(nil, err)
+	AssertTrue(fi1.IsDir())
+	fi2, err := os.Stat(path.Join(mntDir, "fooNew/bar1"))
+	AssertEq(nil, err)
+	AssertEq(fi2.Name(), "bar1")
+	fi3, err := os.Stat(path.Join(mntDir, "fooNew/bar2"))
+	AssertEq(nil, err)
+	AssertEq(fi3.Name(), "bar2")
+	fi4, err := os.Stat(path.Join(mntDir, "foo"))
+	ExpectThat(err, Error(HasSubstr("no such file or directory")))
+	AssertEq(nil, fi4)
+}

internal/fs/inode/base_dir.go

@@ -199,7 +199,8 @@ func (d *baseDirInode) DeleteChildFile(
 
 func (d *baseDirInode) DeleteChildDir(
 	ctx context.Context,
-	name string) (err error) {
+	name string,
+	isImplicitDir bool) (err error) {
 	err = fuse.ENOSYS
 	return
 }

internal/fs/inode/dir.go

@@ -117,10 +117,11 @@ type DirInode interface {
 		metaGeneration *int64) (err error)
 
 	// Delete the backing object for the child directory with the given
-	// (relative) name.
+	// (relative) name if it is not an Implicit Directory.
 	DeleteChildDir(
 		ctx context.Context,
-		name string) (err error)
+		name string,
+		isImplicitDir bool) (err error)
 }
 
 // An inode that represents a directory from a GCS bucket.
@@ -757,8 +758,15 @@ func (d *dirInode) DeleteChildFile(
 // LOCKS_REQUIRED(d)
 func (d *dirInode) DeleteChildDir(
 	ctx context.Context,
-	name string) (err error) {
+	name string,
+	isImplicitDir bool) (err error) {
 	d.cache.Erase(name)
+
+	// if the directory is an implicit directory, then no backing object
+	// exists in the gcs bucket, so returning from here.
+	if isImplicitDir {
+		return
+	}
 	childName := NewDirName(d.Name(), name)
 
 	// Delete the backing object. Unfortunately we have no way to precondition

internal/fs/inode/dir_test.go

@@ -1165,7 +1165,7 @@ func (t *DirTest) DeleteChildFile_TypeCaching() {
 func (t *DirTest) DeleteChildDir_DoesntExist() {
 	const name = "qux"
 
-	err := t.in.DeleteChildDir(t.ctx, name)
+	err := t.in.DeleteChildDir(t.ctx, name, false)
 	ExpectEq(nil, err)
 }
 
@@ -1180,11 +1180,18 @@ func (t *DirTest) DeleteChildDir_Exists() {
 	AssertEq(nil, err)
 
 	// Call the inode.
-	err = t.in.DeleteChildDir(t.ctx, name)
+	err = t.in.DeleteChildDir(t.ctx, name, false)
 	AssertEq(nil, err)
 
 	// Check the bucket.
 	_, err = gcsutil.ReadObject(t.ctx, t.bucket, objName)
 	var notFoundErr *gcs.NotFoundError
 	ExpectTrue(errors.As(err, &notFoundErr))
 }
+
+func (t *DirTest) DeleteChildDir_ImplicitDirTrue() {
+	const name = "qux"
+
+	err := t.in.DeleteChildDir(t.ctx, name, true)
+	ExpectEq(nil, err)
+}