Merge pull request #5506 from andydotxyz/fix/5493

Fix moving of folder/listable content

internal/repository/file.go

@@ -301,6 +301,15 @@ func (r *FileRepository) Copy(source, destination fyne.URI) error {
 //
 // Since: 2.0
 func (r *FileRepository) Move(source, destination fyne.URI) error {
+	listSrc, _ := r.CanList(source)
+	if listSrc {
+		err := os.Rename(source.Path(), destination.Path())
+		if err == nil {
+			return nil
+		}
+		// fallthrough to slow move
+	}
+
 	// NOTE: as far as I can tell, golang does not have an optimized Move
 	// function - everything I can find on the 'net suggests doing more
 	// or less the equivalent of GenericMove(), hence why that is used.

internal/repository/file_test.go

@@ -410,6 +410,39 @@ func TestFileRepositoryMove(t *testing.T) {
 	assert.False(t, ex)
 }
 
+func TestFileRepositoryMoveDirectory(t *testing.T) {
+	dir := t.TempDir()
+
+	// Create a file in a dir to test with
+	parentPath := path.Join(dir, "parent")
+	fooPath := path.Join(parentPath, "foo")
+	newParentPath := path.Join(dir, "newParent")
+	newFooPath := path.Join(newParentPath, "foo")
+
+	_ = os.Mkdir(parentPath, 0755)
+	err := os.WriteFile(fooPath, []byte{1, 2, 3, 4, 5}, 0755)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	parent := storage.NewFileURI(parentPath)
+	foo := storage.NewFileURI(fooPath)
+	newParent := storage.NewFileURI(newParentPath)
+
+	err = storage.Move(parent, newParent)
+	assert.Nil(t, err)
+
+	newData, err := os.ReadFile(newFooPath)
+	assert.Nil(t, err)
+
+	assert.Equal(t, []byte{1, 2, 3, 4, 5}, newData)
+
+	// Make sure that the source doesn't exist anymore.
+	ex, err := storage.Exists(foo)
+	assert.Nil(t, err)
+	assert.False(t, ex)
+}
+
 func TestFileRepositoryListing(t *testing.T) {
 	dir := t.TempDir()
 

internal/repository/memory.go

@@ -279,7 +279,18 @@ func (m *InMemoryRepository) Move(source, destination fyne.URI) error {
 //
 // Since: 2.0
 func (m *InMemoryRepository) CanList(u fyne.URI) (bool, error) {
-	return m.Exists(u)
+	path := u.Path()
+	exist, err := m.Exists(u)
+	if err != nil || !exist {
+		return false, err
+	}
+
+	if path == "" || path[len(path)-1] == '/' {
+		return true, nil
+	}
+
+	children, err := m.List(u)
+	return len(children) > 0, err
 }
 
 // List implements repository.ListableRepository.List()

internal/repository/memory_test.go

@@ -341,14 +341,19 @@ func TestInMemoryRepositoryListing(t *testing.T) {
 	m := NewInMemoryRepository("mem")
 	repository.Register("mem", m)
 	m.Data[""] = []byte{1, 2, 3}
+	m.Data["/empty/"] = []byte{1, 2, 3}
 	m.Data["/foo"] = []byte{1, 2, 3}
 	m.Data["/foo/bar"] = []byte{1, 2, 3}
 	m.Data["/foo/baz/"] = []byte{1, 2, 3}
 	m.Data["/foo/baz/quux"] = []byte{1, 2, 3}
 
-	foo, _ := storage.ParseURI("mem:///foo")
+	empty, _ := storage.ParseURI("mem:///empty/")
+	canList, err := storage.CanList(empty)
+	assert.Nil(t, err)
+	assert.True(t, canList)
 
-	canList, err := storage.CanList(foo)
+	foo, _ := storage.ParseURI("mem:///foo")
+	canList, err = storage.CanList(foo)
 	assert.Nil(t, err)
 	assert.True(t, canList)
 
@@ -360,7 +365,7 @@ func TestInMemoryRepositoryListing(t *testing.T) {
 	}
 	assert.ElementsMatch(t, []string{"mem:///foo/bar", "mem:///foo/baz/"}, stringListing)
 
-	empty, _ := storage.ParseURI("mem:") // invalid path
+	empty, _ = storage.ParseURI("mem:") // invalid path
 	canList, err = storage.CanList(empty)
 	assert.NotNil(t, err)
 	assert.False(t, canList)

storage/repository/generic.go

@@ -131,6 +131,17 @@ func GenericCopy(source fyne.URI, destination fyne.URI) error {
 		return ErrOperationNotSupported
 	}
 
+	if listable, ok := srcrepo.(ListableRepository); ok {
+		isParent, err := listable.CanList(source)
+		if err == nil && isParent {
+			if srcrepo != destwrepo { // cannot copy folders between repositories
+				return ErrOperationNotSupported
+			}
+
+			return genericCopyMoveListable(source, destination, srcrepo, false)
+		}
+	}
+
 	// Create a reader and a writer.
 	srcReader, err := srcrepo.Reader(source)
 	if err != nil {
@@ -188,6 +199,17 @@ func GenericMove(source fyne.URI, destination fyne.URI) error {
 		return ErrOperationNotSupported
 	}
 
+	if listable, ok := srcrepo.(ListableRepository); ok {
+		isParent, err := listable.CanList(source)
+		if err == nil && isParent {
+			if srcrepo != destwrepo { // cannot move between repositories
+				return ErrOperationNotSupported
+			}
+
+			return genericCopyMoveListable(source, destination, srcrepo, true)
+		}
+	}
+
 	// Create the reader and writer to perform the copy operation.
 	srcReader, err := srcrepo.Reader(source)
 	if err != nil {
@@ -210,3 +232,41 @@ func GenericMove(source fyne.URI, destination fyne.URI) error {
 	srcReader.Close()
 	return srcwrepo.Delete(source)
 }
+
+func genericCopyMoveListable(source, destination fyne.URI, repo Repository, deleteSource bool) error {
+	lister, ok1 := repo.(ListableRepository)
+	mover, ok2 := repo.(MovableRepository)
+	copier, ok3 := repo.(CopyableRepository)
+
+	if !ok1 || (deleteSource && !ok2) || (!deleteSource && !ok3) {
+		return ErrOperationNotSupported // cannot move a lister in a non-listable/movable repo
+	}
+
+	err := lister.CreateListable(destination)
+	if err != nil {
+		return err
+	}
+
+	list, err := lister.List(source)
+	if err != nil {
+		return err
+	}
+	for _, child := range list {
+		newChild, _ := repo.(HierarchicalRepository).Child(destination, child.Name())
+		if deleteSource {
+			err = mover.Move(child, newChild)
+		} else {
+			err = copier.Copy(child, newChild)
+		}
+		if err != nil {
+			return err
+		}
+	}
+
+	if !deleteSource {
+		return nil
+	}
+	// we know the repo is writable as well from earlier checks
+	writer, _ := repo.(WritableRepository)
+	return writer.Delete(source)
+}

storage/uri_test.go

@@ -435,6 +435,28 @@ func TestCopy(t *testing.T) {
 	assert.Equal(t, m.Data["/foo"], m.Data["/bar"])
 }
 
+func TestRepositoryCopyListable(t *testing.T) {
+	// set up our repository - it's OK if we already registered it
+	m := intRepo.NewInMemoryRepository("uritest")
+	repository.Register("uritest", m)
+	m.Data["/parent1"] = []byte{}
+	m.Data["/parent1/child"] = []byte("content")
+
+	parent, _ := storage.ParseURI("uritest:///parent1")
+	newParent, _ := storage.ParseURI("uritest:///parent2")
+
+	err := storage.Copy(parent, newParent)
+	assert.NoError(t, err)
+	exists, err := m.Exists(parent)
+	assert.NoError(t, err)
+	assert.True(t, exists)
+	exists, err = m.Exists(newParent)
+	assert.NoError(t, err)
+	assert.True(t, exists)
+	assert.Equal(t, []byte("content"), m.Data["/parent1/child"])
+	assert.Equal(t, []byte("content"), m.Data["/parent2/child"])
+}
+
 func TestRepositoryMove(t *testing.T) {
 	// set up our repository - it's OK if we already registered it
 	m := intRepo.NewInMemoryRepository("uritest")
@@ -454,6 +476,27 @@ func TestRepositoryMove(t *testing.T) {
 	assert.False(t, exists)
 }
 
+func TestRepositoryMoveListable(t *testing.T) {
+	// set up our repository - it's OK if we already registered it
+	m := intRepo.NewInMemoryRepository("uritest")
+	repository.Register("uritest", m)
+	m.Data["/parent1"] = []byte{}
+	m.Data["/parent1/child"] = []byte("content")
+
+	parent, _ := storage.ParseURI("uritest:///parent1")
+	newParent, _ := storage.ParseURI("uritest:///parent2")
+
+	err := storage.Move(parent, newParent)
+	assert.NoError(t, err)
+	exists, err := m.Exists(parent)
+	assert.NoError(t, err)
+	assert.False(t, exists)
+	exists, err = m.Exists(newParent)
+	assert.NoError(t, err)
+	assert.True(t, exists)
+	assert.Equal(t, []byte("content"), m.Data["/parent2/child"])
+}
+
 func TestRepositoryListing(t *testing.T) {
 	// set up our repository - it's OK if we already registered it
 	m := intRepo.NewInMemoryRepository("uritest")