Add tests for remove pkg and refactor remove functions

baton/baton.go

@@ -74,7 +74,7 @@ type Baton struct {
 	collMu      sync.Mutex
 	//PutMetaPool  *ex.ClientPool
 	putClient    *ex.Client
-	MetaClient   *ex.Client
+	metaClient   *ex.Client
 	removeClient *ex.Client
 }
 
@@ -318,7 +318,7 @@ func (b *Baton) getClientByIndex(clientIndex int) *ex.Client {
 	case putClientIndex:
 		return b.putClient
 	case metaClientIndex:
-		return b.MetaClient
+		return b.metaClient
 	default:
 		return b.collClients[clientIndex]
 	}
@@ -329,7 +329,7 @@ func (b *Baton) setClientByIndex(clientIndex int, client *ex.Client) {
 	case putClientIndex:
 		b.putClient = client
 	case metaClientIndex:
-		b.MetaClient = client
+		b.metaClient = client
 	default:
 		b.collClients[clientIndex] = client
 	}
@@ -365,7 +365,7 @@ func (b *Baton) createPutRemoveMetaClients() error {
 	}
 
 	b.putClient = <-clientCh
-	b.MetaClient = <-clientCh
+	b.metaClient = <-clientCh
 	b.removeClient = <-clientCh
 
 	pool.Close()
@@ -376,12 +376,12 @@ func (b *Baton) createPutRemoveMetaClients() error {
 // InitClients sets three clients if they do not currently exist. Returns error
 // if some exist and some do not.
 func (b *Baton) InitClients() error {
-	if b.putClient == nil && b.MetaClient == nil && b.removeClient == nil {
+	if b.putClient == nil && b.metaClient == nil && b.removeClient == nil {
 		return b.createPutRemoveMetaClients()
 	}
 
-	if b.putClient != nil && b.MetaClient != nil && b.removeClient != nil {
-		if !b.putClient.IsRunning() && !b.MetaClient.IsRunning() && !b.removeClient.IsRunning() {
+	if b.putClient != nil && b.metaClient != nil && b.removeClient != nil {
+		if !b.putClient.IsRunning() && !b.metaClient.IsRunning() && !b.removeClient.IsRunning() {
 			return b.createPutRemoveMetaClients()
 		}
 
@@ -395,7 +395,7 @@ func (b *Baton) InitClients() error {
 func (b *Baton) CloseClients() {
 	var openClients []*ex.Client
 
-	for _, client := range []*ex.Client{b.removeClient, b.putClient, b.MetaClient} {
+	for _, client := range []*ex.Client{b.removeClient, b.putClient, b.metaClient} {
 		if client != nil {
 			openClients = append(openClients, client)
 		}
@@ -422,7 +422,7 @@ func (b *Baton) Stat(remote string) (bool, map[string]string, error) {
 
 	err := TimeoutOp(func() error {
 		var errl error
-		it, errl = b.MetaClient.ListItem(ex.Args{Timestamp: true, AVU: true}, *requestToRodsItem("", remote))
+		it, errl = b.metaClient.ListItem(ex.Args{Timestamp: true, AVU: true}, *requestToRodsItem("", remote))
 
 		return errl
 	}, "stat failed: "+remote)
@@ -518,7 +518,7 @@ func (b *Baton) RemoveMeta(path string, meta map[string]string) error {
 	it.IAVUs = MetaToAVUs(meta)
 
 	err := TimeoutOp(func() error {
-		_, errl := b.MetaClient.MetaRem(ex.Args{}, *it)
+		_, errl := b.metaClient.MetaRem(ex.Args{}, *it)
 
 		return errl
 	}, "remove meta error: "+path)
@@ -528,7 +528,7 @@ func (b *Baton) RemoveMeta(path string, meta map[string]string) error {
 
 // GetMeta gets all the metadata for the given object in iRODS.
 func (b *Baton) GetMeta(path string) (map[string]string, error) {
-	it, err := b.MetaClient.ListItem(ex.Args{AVU: true, Timestamp: true, Size: true}, ex.RodsItem{
+	it, err := b.metaClient.ListItem(ex.Args{AVU: true, Timestamp: true, Size: true}, ex.RodsItem{
 		IPath: filepath.Dir(path),
 		IName: filepath.Base(path),
 	})
@@ -550,7 +550,7 @@ func (b *Baton) AddMeta(path string, meta map[string]string) error {
 	it.IAVUs = MetaToAVUs(meta)
 
 	err := TimeoutOp(func() error {
-		_, errl := b.MetaClient.MetaAdd(ex.Args{}, *it)
+		_, errl := b.metaClient.MetaAdd(ex.Args{}, *it)
 
 		return errl
 	}, "add meta error: "+path)
@@ -560,7 +560,7 @@ func (b *Baton) AddMeta(path string, meta map[string]string) error {
 
 // Cleanup stops our clients and closes our client pool.
 func (b *Baton) Cleanup() error {
-	b.closeConnections(append(b.collClients, b.putClient, b.removeClient, b.MetaClient))
+	b.closeConnections(append(b.collClients, b.putClient, b.removeClient, b.metaClient))
 
 	// b.PutMetaPool.Close()
 
@@ -577,41 +577,11 @@ func (b *Baton) Cleanup() error {
 	return nil
 }
 
-// GetBatonHandlerWithMetaClient returns a Handler that uses Baton to interact
-// with iRODS and contains a meta client for interacting with metadata. If you
-// don't have baton-do in your PATH, you'll get an error.
-// func GetBatonHandlerWithMetaClient() (*Baton, error) {
-// 	setupExtendoLogger()
-
-// 	_, err := ex.FindBaton()
-// 	if err != nil {
-// 		return nil, err
-// 	}
-
-// 	params := ex.DefaultClientPoolParams
-// 	params.MaxSize = 1
-// 	pool := ex.NewClientPool(params, "")
-
-// 	metaClient, err := pool.Get()
-// 	if err != nil {
-// 		return nil, fmt.Errorf("failed to get metaClient: %w", err)
-// 	}
-
-// 	baton := &Baton{
-// 		Baton: put.Baton{
-// 			PutMetaPool: pool,
-// 			MetaClient:  metaClient,
-// 		},
-// 	}
-
-// 	return baton, nil
-// }
-
 func (b *Baton) RemoveFile(path string) error {
 	it := RemotePathToRodsItem(path)
 
 	err := TimeoutOp(func() error {
-		_, errl := b.MetaClient.RemObj(ex.Args{}, *it)
+		_, errl := b.removeClient.RemObj(ex.Args{}, *it)
 
 		return errl
 	}, "remove file error: "+path)
@@ -626,7 +596,7 @@ func (b *Baton) RemoveDir(path string) error {
 	}
 
 	err := TimeoutOp(func() error {
-		_, errl := b.MetaClient.RemDir(ex.Args{}, *it)
+		_, errl := b.removeClient.RemDir(ex.Args{}, *it)
 
 		return errl
 	}, "remove meta error: "+path)
@@ -645,7 +615,7 @@ func (b *Baton) QueryMeta(dirToSearch string, meta map[string]string) ([]string,
 	var err error
 
 	err = TimeoutOp(func() error {
-		items, err = b.MetaClient.MetaQuery(ex.Args{Object: true}, *it)
+		items, err = b.metaClient.MetaQuery(ex.Args{Object: true}, *it)
 
 		return err
 	}, "query meta error: "+dirToSearch)
@@ -660,5 +630,5 @@ func (b *Baton) QueryMeta(dirToSearch string, meta map[string]string) ([]string,
 }
 
 func (b *Baton) AllClientsStopped() bool {
-	return !b.putClient.IsRunning() && !b.MetaClient.IsRunning() && b.collClients == nil
+	return !b.putClient.IsRunning() && !b.metaClient.IsRunning() && b.collClients == nil
 }

baton/baton_test.go

@@ -63,7 +63,7 @@ func TestBaton(t *testing.T) {
 		So(err, ShouldBeNil)
 
 		So(h.removeClient.IsRunning(), ShouldBeTrue)
-		So(h.MetaClient.IsRunning(), ShouldBeTrue)
+		So(h.metaClient.IsRunning(), ShouldBeTrue)
 		So(h.putClient.IsRunning(), ShouldBeTrue)
 
 		meta := map[string]string{

main_test.go

@@ -2002,6 +2002,8 @@ func TestRemove(t *testing.T) {
 			dir1 := filepath.Join(testDir, "dir")
 			dir2 := filepath.Join(path, "path/to/other/dir/")
 
+			//dir3 := filepath.Join(dir1, "dir")
+
 			tempTestFileOfPaths, err := os.CreateTemp(dir, "testFileSet")
 			So(err, ShouldBeNil)
 
@@ -2011,15 +2013,21 @@ func TestRemove(t *testing.T) {
 			err = os.MkdirAll(dir2, 0755)
 			So(err, ShouldBeNil)
 
+			// err = os.MkdirAll(dir3, 0755)
+			// So(err, ShouldBeNil)
+
 			file1 := filepath.Join(path, "file1")
 			file2 := filepath.Join(path, "file2")
 			file3 := filepath.Join(dir1, "file3")
 			file4 := filepath.Join(testDir, "dir_not_removed")
 
+			// file5 := filepath.Join(dir3, "file5")
+
 			internal.CreateTestFile(t, file1, "some data1")
 			internal.CreateTestFile(t, file2, "some data2")
 			internal.CreateTestFile(t, file3, "some data3")
 			internal.CreateTestFile(t, file4, "some data3")
+			// internal.CreateTestFile(t, file5, "some data3")
 
 			err = os.Link(file1, linkPath)
 			So(err, ShouldBeNil)
@@ -2117,6 +2125,20 @@ func TestRemove(t *testing.T) {
 					0, dir2+" => ")
 			})
 
+			// TODO test fails
+			// SkipConvey("Remove removes the dir even if it is not specified in the set but is part of it", func() {
+			// 	s.removePath(t, setName, dir3, 2)
+
+			// 	s.confirmOutputContains(t, []string{"status", "--name", setName, "-d"},
+			// 		0, dir2)
+
+			// 	s.confirmOutputDoesNotContain(t, []string{"status", "--name", setName, "-d"},
+			// 		0, dir3+"/")
+
+			// 	s.confirmOutputDoesNotContain(t, []string{"status", "--name", setName, "-d"},
+			// 		0, dir3+" => ")
+			// })
+
 			Convey("Remove takes a flag --items and removes all provided files and dirs from the set", func() {
 				tempTestFileOfPathsToRemove, errt := os.CreateTemp(dir, "testFileSet")
 				So(errt, ShouldBeNil)

remove/remove.go

@@ -24,7 +24,7 @@
  * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  ******************************************************************************/
 
-// package remove is used to interact with iRODS.
+// package remove is used to remove from remote storage.
 
 package remove
 
@@ -44,7 +44,7 @@ type Handler interface {
 	// object.
 	AddMeta(path string, meta map[string]string) error
 
-	// GetMeta returns the meta for a given path in iRODS.
+	// GetMeta returns the meta for a given remote path.
 	GetMeta(path string) (map[string]string, error)
 
 	// RemoveDir deletes a given empty folder.
@@ -64,15 +64,10 @@ type Handler interface {
 	CloseClients()
 }
 
-// RemovePathFromSetInIRODS removes the given path from iRODS if the path is not
-// associated with any other sets. Otherwise it updates the iRODS metadata for
-// the path to not include the given set.
-func RemovePathFromSetInIRODS(handler Handler, transformer put.PathTransformer, path string,
+// UpdateSetsAndRequestersOnRemoteFile updates the given file's metadata in
+// remote storage for the path with the given sets and requesters.
+func UpdateSetsAndRequestersOnRemoteFile(handler Handler, path string,
 	sets, requesters []string, meta map[string]string) error {
-	if len(sets) == 0 {
-		return handleHardlinkAndRemoveFromIRODS(handler, path, transformer, meta)
-	}
-
 	metaToRemove := map[string]string{
 		put.MetaKeySets:      meta[put.MetaKeySets],
 		put.MetaKeyRequester: meta[put.MetaKeyRequester],
@@ -95,10 +90,11 @@ func RemovePathFromSetInIRODS(handler Handler, transformer put.PathTransformer,
 	return handler.AddMeta(path, newMeta)
 }
 
-// handleHardLinkAndRemoveFromIRODS removes the given path from iRODS. If the
-// path is found to be a hardlink, it checks if there are other hardlinks to the
-// same file, if not, it removes the file.
-func handleHardlinkAndRemoveFromIRODS(handler Handler, path string, transformer put.PathTransformer,
+// RemoveRemoteFileAndHandleHardlink removes the given path from remote storage.
+// If the path is found to be a hardlink, it checks if there are other hardlinks
+// (inside the provided dir to search) to the same file, if not, it removes the
+// file.
+func RemoveRemoteFileAndHandleHardlink(handler Handler, path string, dirToSearch string, //nolint:revive
 	meta map[string]string) error {
 	err := handler.RemoveFile(path)
 	if err != nil {
@@ -109,12 +105,9 @@ func handleHardlinkAndRemoveFromIRODS(handler Handler, path string, transformer
 		return nil
 	}
 
-	dirToSearch, err := transformer("/")
-	if err != nil {
-		return err
-	}
-
-	items, err := handler.QueryMeta(dirToSearch, map[string]string{put.MetaKeyRemoteHardlink: meta[put.MetaKeyRemoteHardlink]})
+	items, err := handler.QueryMeta(dirToSearch, map[string]string{
+		put.MetaKeyRemoteHardlink: meta[put.MetaKeyRemoteHardlink],
+	})
 	if err != nil {
 		return err
 	}
@@ -126,8 +119,9 @@ func handleHardlinkAndRemoveFromIRODS(handler Handler, path string, transformer
 	return handler.RemoveFile(meta[put.MetaKeyRemoteHardlink])
 }
 
-// RemoveDirFromIRODS removes the remote path of a given directory from iRODS.
-func RemoveDirFromIRODS(handler Handler, path string, transformer put.PathTransformer) error {
+// RemoveRemoteDir removes the remote path of a given directory from the remote
+// storage.
+func RemoveRemoteDir(handler Handler, path string, transformer put.PathTransformer) error { //nolint:revive
 	rpath, err := transformer(path)
 	if err != nil {
 		return err