Skip to content

Commit

Permalink
tests(to-files): ensure error handling is covered
Browse files Browse the repository at this point in the history 
this adds bunch of tests that guard UX around importing multiple files
into MFS, and the logic around trailing slash to indicate if the MFS
destination if a directory.

If the destination has a trailing slash, we ensure that the directory
exists and is a dir and not a file. this allows us to support
adding multipl files into MFS dir:

ipfs add file1.txt file2.txt --to-files /some/mfs/dir/
  • Loading branch information
@lidel
lidel committed Sep 21, 2022
1 parent c216c38 commit ca2b36b
Show file tree
Hide file tree
Showing 2 changed files with 113 additions and 10 deletions.
33 changes: 25 additions & 8 deletions core/commands/add.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -268,12 +268,9 @@ See 'dag export' and 'dag import' for more information.
errCh <- err
return
}
if toFilesSet {
if fileAddedToMFS {
errCh <- fmt.Errorf("%s: more than one entry to copy to MFS path %q", toFilesOptionName, toFilesStr)
return
}

// creating MFS pointers when optional --to-files is set
if toFilesSet {
if toFilesStr == "" {
toFilesStr = "/"
}
Expand All @@ -282,13 +279,33 @@ See 'dag export' and 'dag import' for more information.
errCh <- fmt.Errorf("%s: %w", toFilesOptionName, err)
return
}
if toFilesDst[len(toFilesDst)-1] == '/' {
dstAsDir := toFilesDst[len(toFilesDst)-1] == '/'

if dstAsDir {
mfsNode, err := mfs.Lookup(ipfsNode.FilesRoot, toFilesDst)
// confirm dst exists
if err != nil {
errCh <- fmt.Errorf("%s: MFS destination directory %q does not exist: %w", toFilesOptionName, toFilesDst, err)
return
}
// confirm dst is a dir
if mfsNode.Type() != mfs.TDir {
errCh <- fmt.Errorf("%s: MFS destination %q is not a directory", toFilesOptionName, toFilesDst)
return
}
// if MFS destination is a dir, append filename to the dir path
toFilesDst += path.Base(addit.Name())
}

// error if we try to overwrite a preexisting file destination
if fileAddedToMFS && !dstAsDir {
errCh <- fmt.Errorf("%s: MFS destination is a file: only one entry can be copied to %q", toFilesOptionName, toFilesDst)
return
}

_, err = mfs.Lookup(ipfsNode.FilesRoot, path.Dir(toFilesDst))
if err != nil {
errCh <- fmt.Errorf("%s: MFS destination directory %q does not exist: %w", toFilesOptionName, path.Dir(toFilesDst), err)
errCh <- fmt.Errorf("%s: MFS destination parent %q %q does not exist: %w", toFilesOptionName, toFilesDst, path.Dir(toFilesDst), err)
return
}

Expand All @@ -300,7 +317,7 @@ See 'dag export' and 'dag import' for more information.
}
err = mfs.PutNode(ipfsNode.FilesRoot, toFilesDst, nodeAdded)
if err != nil {
errCh <- fmt.Errorf("%s: cannot put node in path %s: %s", toFilesOptionName, toFilesDst, err)
errCh <- fmt.Errorf("%s: cannot put node in path %q: %w", toFilesOptionName, toFilesDst, err)
return
}
fileAddedToMFS = true
Expand Down
90 changes: 88 additions & 2 deletions test/sharness/t0040-add-and-cat.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,10 @@ test_add_cat_file() {
rmdir mountdir/same-file
'

test_expect_success "ipfs add --to-files succeeds" '
echo "Hello MFS!" > mountdir/mfs.txt &&
## --to-files with single source

test_expect_success "ipfs add --to-files /mfspath succeeds" '
mkdir -p mountdir && echo "Hello MFS!" > mountdir/mfs.txt &&
ipfs add mountdir/mfs.txt --to-files /ipfs-add-to-files >actual
'

Expand All @@ -381,9 +383,93 @@ test_add_cat_file() {

test_expect_success "ipfs cat output looks good" '
echo "Hello MFS!" >expected &&
test_cmp expected actual
'

test_expect_success "ipfs add --to-files requires argument" '
test_expect_code 1 ipfs add mountdir/mfs.txt --to-files >actual 2>&1 &&
test_should_contain "Error: missing argument for option \"to-files\"" actual
'

test_expect_success "ipfs add --to-files / (MFS root) works" '
echo "Hello MFS!" >expected &&
ipfs add mountdir/mfs.txt --to-files / &&
ipfs files read /mfs.txt >actual &&
test_cmp expected actual &&
rm mountdir/mfs.txt
'

## --to-files with multiple sources

test_expect_success "ipfs add file1 file2 --to-files /mfspath0 (without trailing slash) fails" '
mkdir -p test &&
echo "file1" > test/mfs1.txt &&
echo "file2" > test/mfs2.txt &&
test_expect_code 1 ipfs add test/mfs1.txt test/mfs2.txt --to-files /mfspath0 >actual 2>&1 &&
test_should_contain "MFS destination is a file: only one entry can be copied to \"/mfspath0\"" actual &&
ipfs files rm -r --force /mfspath0
'

test_expect_success "ipfs add file1 file2 --to-files /mfsfile1 (without trailing slash + with preexisting file) fails" '
echo test | ipfs files write --create /mfsfile1 &&
test_expect_code 1 ipfs add test/mfs1.txt test/mfs2.txt --to-files /mfsfile1 >actual 2>&1 &&
test_should_contain "Error: to-files: cannot put node in path \"/mfsfile1\"" actual &&
ipfs files rm -r --force /mfsfile1
'

test_expect_success "ipfs add file1 file2 --to-files /mfsdir1 (without trailing slash + with preexisting dir) fails" '
ipfs files mkdir -p /mfsdir1 &&
test_expect_code 1 ipfs add test/mfs1.txt test/mfs2.txt --to-files /mfsdir1 >actual 2>&1 &&
test_should_contain "Error: to-files: cannot put node in path \"/mfsdir1\"" actual &&
ipfs files rm -r --force /mfsdir1
'

test_expect_success "ipfs add file1 file2 --to-files /mfsdir2/ (with trailing slash) succeeds" '
ipfs files mkdir -p /mfsdir2 &&
test_expect_code 0 ipfs add --cid-version 1 test/mfs1.txt test/mfs2.txt --to-files /mfsdir2/ > actual 2>&1 &&
test_should_contain "added bafkreihm3rktn5z33luic3youqdsn326toaq3ekesmdvsa53sbrd3f5r3a mfs1.txt" actual &&
test_should_contain "added bafkreidh5zkhr2vnwa2luwmuj24xo6l3jhfgvkgtk5cyp43oxs7owzpxby mfs2.txt" actual &&
test_should_not_contain "Error" actual &&
ipfs files ls /mfsdir2/ > lsout &&
test_should_contain "mfs1.txt" lsout &&
test_should_contain "mfs2.txt" lsout &&
ipfs files rm -r --force /mfsdir2
'

test_expect_success "ipfs add file1 file2 --to-files /mfsfile2/ (with trailing slash + with preexisting file) fails" '
echo test | ipfs files write --create /mfsfile2 &&
test_expect_code 1 ipfs add test/mfs1.txt test/mfs2.txt --to-files /mfsfile2/ >actual 2>&1 &&
test_should_contain "Error: to-files: MFS destination \"/mfsfile2/\" is not a directory" actual &&
ipfs files rm -r --force /mfsfile2
'

## --to-files with recursive dir

# test MFS destination without trailing slash
test_expect_success "ipfs add with --to-files /mfs/subdir3 fails because /mfs/subdir3 exists" '
ipfs files mkdir -p /mfs/subdir3 &&
test_expect_code 1 ipfs add -r test --to-files /mfs/subdir3 >actual 2>&1 &&
test_should_contain "cannot put node in path \"/mfs/subdir3\": directory already has entry by that name" actual &&
ipfs files rm -r --force /mfs
'

# test recursive import of a dir into MFS subdirectory
test_expect_success "ipfs add -r dir --to-files /mfs/subdir4/ succeeds (because of trailing slash)" '
ipfs files mkdir -p /mfs/subdir4 &&
ipfs add --cid-version 1 -r test --to-files /mfs/subdir4/ >actual 2>&1 &&
test_should_contain "added bafkreihm3rktn5z33luic3youqdsn326toaq3ekesmdvsa53sbrd3f5r3a test/mfs1.txt" actual &&
test_should_contain "added bafkreidh5zkhr2vnwa2luwmuj24xo6l3jhfgvkgtk5cyp43oxs7owzpxby test/mfs2.txt" actual &&
test_should_contain "added bafybeic7xwqwovt4g4bax6d3udp6222i63vj2rblpbim7uy2uw4a5gahha test" actual &&
test_should_not_contain "Error" actual
ipfs files ls /mfs/subdir4/ > lsout &&
test_should_contain "test" lsout &&
test_should_not_contain "mfs1.txt" lsout &&
test_should_not_contain "mfs2.txt" lsout &&
ipfs files rm -r --force /mfs
'



}

test_add_cat_5MB() {
Expand Down

0 comments on commit ca2b36b

Please sign in to comment.