swarm/api: improve FUSE build constraints, logging and APIs (#3818)

* swarm/api: fix build/tests on unsupported platforms

Skip FUSE tests if FUSE is unavailable and change build constraints so
the 'lesser' platforms aren't mentioned explicitly. The test are
compiled on all platforms to prevent regressions in _fallback.go

Also gofmt -w -s because why not.

* internal/web3ext: fix swarmfs wrappers

Remove inputFormatter specifications so users get an error
when passing the wrong number of arguments.

* swarm/api: improve FUSE-related logging and APIs

The API now returns JSON objects instead of strings.
Log messages for invalid arguments are removed.

6 files changed, 124 insertions, 168 deletions

diff --git a/internal/web3ext/web3ext.go b/internal/web3ext/web3ext.go
index 79f781781..4a8425d5c 100644
--- a/internal/web3ext/web3ext.go
+++ b/internal/web3ext/web3ext.go
@@ -29,10 +29,8 @@ var Modules = map[string]string{
 	"shh":        Shh_JS,
 	"swarmfs":    SWARMFS_JS,
 	"txpool":     TxPool_JS,
-
 }
 
-
 const Chequebook_JS = `
 web3._extend({
   property: 'chequebook',
@@ -491,28 +489,25 @@ web3._extend({
 `
 const SWARMFS_JS = `
 web3._extend({
-  property: 'swarmfs',
-  methods:
-  [
-    new web3._extend.Method({
-      name: 'mount',
-      call: 'swarmfs_mount',
-      params: 2,
-      inputFormatter: [null,null]
-    }),
-    new web3._extend.Method({
-      name: 'unmount',
-      call: 'swarmfs_unmount',
-      params: 1,
-      inputFormatter: [null]
-    }),
-    new web3._extend.Method({
-      name: 'listmounts',
-      call: 'swarmfs_listmounts',
-      params: 0,
-      inputFormatter: []
-    })
-  ]
+	property: 'swarmfs',
+	methods:
+	[
+		new web3._extend.Method({
+			name: 'mount',
+			call: 'swarmfs_mount',
+			params: 2
+		}),
+		new web3._extend.Method({
+			name: 'unmount',
+			call: 'swarmfs_unmount',
+			params: 1
+		}),
+		new web3._extend.Method({
+			name: 'listmounts',
+			call: 'swarmfs_listmounts',
+			params: 0
+		})
+	]
 });
 `
 
diff --git a/swarm/api/fuse.go b/swarm/api/fuse.go
index 4b1f817f8..2a1cc9bf1 100644
--- a/swarm/api/fuse.go
+++ b/swarm/api/fuse.go
@@ -14,7 +14,9 @@
 // You should have received a copy of the GNU Lesser General Public License
 // along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
 
-// +build !windows
+// +build linux darwin freebsd
+
+// Data structures used for Fuse filesystem, serving directories and serving files to Fuse driver.
 
 package api
 
@@ -29,10 +31,6 @@ import (
 	"golang.org/x/net/context"
 )
 
-
-
-
-// Data structures used for Fuse filesystem, serving directories and serving files to Fuse driver
 type FS struct {
 	root *Dir
 }
@@ -55,7 +53,6 @@ type File struct {
 	reader   storage.LazySectionReader
 }
 
-
 // Functions which satisfy the Fuse File System requests
 func (filesystem *FS) Root() (fs.Node, error) {
 	return filesystem.root, nil
@@ -104,14 +101,12 @@ func (d *Dir) ReadDirAll(ctx context.Context) ([]fuse.Dirent, error) {
 }
 
 func (file *File) Attr(ctx context.Context, a *fuse.Attr) error {
-
 	a.Inode = file.inode
 	//TODO: need to get permission as argument
 	a.Mode = 0500
 	a.Uid = uint32(os.Getuid())
 	a.Gid = uint32(os.Getegid())
 
-
 	reader := file.swarmApi.Retrieve(file.key)
 	quitC := make(chan bool)
 	size, err := reader.Size(quitC)
@@ -135,5 +130,4 @@ func (file *File) Read(ctx context.Context, req *fuse.ReadRequest, resp *fuse.Re
 	}
 	resp.Data = buf[:n]
 	return err
-
 }
diff --git a/swarm/api/swarmfs.go b/swarm/api/swarmfs.go
index 8427d3c5b..78a61cf9d 100644
--- a/swarm/api/swarmfs.go
+++ b/swarm/api/swarmfs.go
@@ -17,25 +17,22 @@
 package api
 
 import (
-	"time"
 	"sync"
+	"time"
 )
 
 const (
 	Swarmfs_Version = "0.1"
-	mountTimeout   = time.Second * 5
-	maxFuseMounts  = 5
+	mountTimeout    = time.Second * 5
+	maxFuseMounts   = 5
 )
 
-
 type SwarmFS struct {
 	swarmApi     *Api
 	activeMounts map[string]*MountInfo
 	activeLock   *sync.RWMutex
 }
 
-
-
 func NewSwarmFS(api *Api) *SwarmFS {
 	swarmfs := &SwarmFS{
 		swarmApi:     api,
@@ -44,5 +41,3 @@ func NewSwarmFS(api *Api) *SwarmFS {
 	}
 	return swarmfs
 }
-
-
diff --git a/swarm/api/swarmfs_windows.go b/swarm/api/swarmfs_fallback.go
index 525a25399..c6ac07d14 100644
--- a/swarm/api/swarmfs_windows.go
+++ b/swarm/api/swarmfs_fallback.go
@@ -14,35 +14,37 @@
 // You should have received a copy of the GNU Lesser General Public License
 // along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
 
-// +build windows
+// +build !linux,!darwin,!freebsd
 
 package api
 
 import (
-	"github.com/ethereum/go-ethereum/log"
+	"errors"
 )
 
-// Dummy struct and functions to satsfy windows build
-type MountInfo struct {
+var errNoFUSE = errors.New("FUSE is not supported on this platform")
+
+func isFUSEUnsupportedError(err error) bool {
+	return err == errNoFUSE
 }
 
+type MountInfo struct {
+	MountPoint   string
+	ManifestHash string
+}
 
-func (self *SwarmFS) Mount(mhash, mountpoint string) error {
-	log.Info("Platform not supported")
-	return nil
+func (self *SwarmFS) Mount(mhash, mountpoint string) (*MountInfo, error) {
+	return nil, errNoFUSE
 }
 
-func (self *SwarmFS) Unmount(mountpoint string) error {
-	log.Info("Platform not supported")
-	return nil
+func (self *SwarmFS) Unmount(mountpoint string) (bool, error) {
+	return false, errNoFUSE
 }
 
-func (self *SwarmFS) Listmounts() (string, error) {
-	log.Info("Platform not supported")
-	return "",nil
+func (self *SwarmFS) Listmounts() ([]*MountInfo, error) {
+	return nil, errNoFUSE
 }
 
 func (self *SwarmFS) Stop() error {
-	log.Info("Platform not supported")
 	return nil
-}
\ No newline at end of file
+}
diff --git a/swarm/api/swarmfs_unix_test.go b/swarm/api/swarmfs_test.go
index 4f59dba5b..45d2dc169 100644
--- a/swarm/api/swarmfs_unix_test.go
+++ b/swarm/api/swarmfs_test.go
@@ -14,8 +14,6 @@
 // You should have received a copy of the GNU Lesser General Public License
 // along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
 
-// +build linux darwin
-
 package api
 
 import (
@@ -35,7 +33,6 @@ func testFuseFileSystem(t *testing.T, f func(*FileSystem)) {
 }
 
 func createTestFiles(t *testing.T, files []string) {
-
 	os.RemoveAll(testUploadDir)
 	os.RemoveAll(testMountDir)
 	defer os.MkdirAll(testMountDir, 0777)
@@ -58,9 +55,7 @@ func createTestFiles(t *testing.T, files []string) {
 }
 
 func compareFiles(t *testing.T, files []string) {
-
 	for f := range files {
-
 		sourceFile := filepath.Join(testUploadDir, files[f])
 		destinationFile := filepath.Join(testMountDir, files[f])
 
@@ -81,12 +76,10 @@ func compareFiles(t *testing.T, files []string) {
 		if dfinfo.Mode().Perm().String() != "-r-x------" {
 			t.Fatalf("Permission is not 0500for file: %v", err)
 		}
-
 	}
 }
 
 func doHashTest(fs *FileSystem, t *testing.T, ensName string, files ...string) {
-
 	createTestFiles(t, files)
 	bzzhash, err := fs.Upload(testUploadDir, "")
 	if err != nil {
@@ -94,29 +87,29 @@ func doHashTest(fs *FileSystem, t *testing.T, ensName string, files ...string) {
 	}
 
 	swarmfs := NewSwarmFS(fs.api)
-	_ ,err1 := swarmfs.Mount(bzzhash, testMountDir)
-	if err1 != nil {
+	defer swarmfs.Stop()
 
-		t.Fatalf("Error mounting hash  %v: %v", bzzhash, err)
+	_, err = swarmfs.Mount(bzzhash, testMountDir)
+	if isFUSEUnsupportedError(err) {
+		t.Skip("FUSE not supported:", err)
+	} else if err != nil {
+		t.Fatalf("Error mounting hash %v: %v", bzzhash, err)
 	}
+
 	compareFiles(t, files)
-	_, err2 := swarmfs.Unmount(testMountDir)
-	if err2 != nil {
-		t.Fatalf("Error unmounting path  %v: %v", testMountDir, err)
-	}
-	swarmfs.Stop()
 
+	if _, err := swarmfs.Unmount(testMountDir); err != nil {
+		t.Fatalf("Error unmounting path %v: %v", testMountDir, err)
+	}
 }
 
 // mounting with manifest Hash
 func TestFuseMountingScenarios(t *testing.T) {
 	testFuseFileSystem(t, func(fs *FileSystem) {
-
 		//doHashTest(fs,t, "test","1.txt")
 		doHashTest(fs, t, "", "1.txt")
 		doHashTest(fs, t, "", "1.txt", "11.txt", "111.txt", "two/2.txt", "two/two/2.txt", "three/3.txt")
 		doHashTest(fs, t, "", "1/2/3/4/5/6/7/8/9/10/11/12/1.txt")
 		doHashTest(fs, t, "", "one/one.txt", "one.txt", "once/one.txt", "one/one/one.txt")
-
 	})
 }
diff --git a/swarm/api/swarmfs_unix.go b/swarm/api/swarmfs_unix.go
index ae0216e1d..e696c6b9a 100644
--- a/swarm/api/swarmfs_unix.go
+++ b/swarm/api/swarmfs_unix.go
@@ -14,77 +14,86 @@
 // You should have received a copy of the GNU Lesser General Public License
 // along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
 
-// +build linux darwin
+// +build linux darwin freebsd
 
 package api
 
 import (
-	"path/filepath"
+	"errors"
 	"fmt"
+	"os"
+	"path/filepath"
 	"strings"
+	"sync"
 	"time"
-	"github.com/ethereum/go-ethereum/swarm/storage"
+
 	"bazil.org/fuse"
-	"github.com/ethereum/go-ethereum/log"
-	"github.com/ethereum/go-ethereum/common"
 	"bazil.org/fuse/fs"
-	"sync"
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/log"
+	"github.com/ethereum/go-ethereum/swarm/storage"
 )
 
+var (
+	inode     uint64 = 1
+	inodeLock sync.RWMutex
+)
 
 var (
-	inode  uint64  = 1
-	inodeLock   sync.RWMutex
+	errEmptyMountPoint = errors.New("need non-empty mount point")
+	errMaxMountCount   = errors.New("max FUSE mount count reached")
+	errMountTimeout    = errors.New("mount timeout")
 )
 
-// information about every active mount
+func isFUSEUnsupportedError(err error) bool {
+	if perr, ok := err.(*os.PathError); ok {
+		return perr.Op == "open" && perr.Path == "/dev/fuse"
+	}
+	return err == fuse.ErrOSXFUSENotFound
+}
+
+// MountInfo contains information about every active mount
 type MountInfo struct {
-	mountPoint     string
-	manifestHash   string
+	MountPoint     string
+	ManifestHash   string
 	resolvedKey    storage.Key
 	rootDir        *Dir
 	fuseConnection *fuse.Conn
 }
 
+// newInode creates a new inode number.
 // Inode numbers need to be unique, they are used for caching inside fuse
-func NewInode() uint64 {
+func newInode() uint64 {
 	inodeLock.Lock()
-	defer  inodeLock.Unlock()
+	defer inodeLock.Unlock()
 	inode += 1
 	return inode
 }
 
-
-
-func (self *SwarmFS) Mount(mhash, mountpoint string) (string, error)  {
+func (self *SwarmFS) Mount(mhash, mountpoint string) (*MountInfo, error) {
+	if mountpoint == "" {
+		return nil, errEmptyMountPoint
+	}
+	cleanedMountPoint, err := filepath.Abs(filepath.Clean(mountpoint))
+	if err != nil {
+		return nil, err
+	}
 
 	self.activeLock.Lock()
 	defer self.activeLock.Unlock()
 
 	noOfActiveMounts := len(self.activeMounts)
 	if noOfActiveMounts >= maxFuseMounts {
-		err := fmt.Errorf("Max mount count reached. Cannot mount %s ", mountpoint)
-		log.Warn(err.Error())
-		return err.Error(), err
-	}
-
-	cleanedMountPoint, err := filepath.Abs(filepath.Clean(mountpoint))
-	if err != nil {
-		return err.Error(), err
+		return nil, errMaxMountCount
 	}
 
 	if _, ok := self.activeMounts[cleanedMountPoint]; ok {
-		err := fmt.Errorf("Mountpoint %s already mounted.", cleanedMountPoint)
-		log.Warn(err.Error())
-		return err.Error(), err
+		return nil, fmt.Errorf("%s is already mounted", cleanedMountPoint)
 	}
 
-	log.Info(fmt.Sprintf("Attempting to mount %s ", cleanedMountPoint))
 	key, _, path, err := self.swarmApi.parseAndResolve(mhash, true)
 	if err != nil {
-		errStr := fmt.Sprintf("Could not resolve %s : %v", mhash, err)
-		log.Warn(errStr)
-		return errStr, err
+		return nil, fmt.Errorf("can't resolve %q: %v", mhash, err)
 	}
 
 	if len(path) > 0 {
@@ -94,15 +103,13 @@ func (self *SwarmFS) Mount(mhash, mountpoint string) (string, error)  {
 	quitC := make(chan bool)
 	trie, err := loadManifest(self.swarmApi.dpa, key, quitC)
 	if err != nil {
-		errStr := fmt.Sprintf("fs.Download: loadManifestTrie error: %v", err)
-		log.Warn(errStr)
-		return errStr, err
+		return nil, fmt.Errorf("can't load manifest %v: %v", key.String(), err)
 	}
 
 	dirTree := map[string]*Dir{}
 
 	rootDir := &Dir{
-		inode:       NewInode(),
+		inode:       newInode(),
 		name:        "root",
 		directories: nil,
 		files:       nil,
@@ -110,7 +117,6 @@ func (self *SwarmFS) Mount(mhash, mountpoint string) (string, error)  {
 	dirTree["root"] = rootDir
 
 	err = trie.listWithPrefix(path, quitC, func(entry *manifestTrieEntry, suffix string) {
-
 		key = common.Hex2Bytes(entry.Hash)
 		fullpath := "/" + suffix
 		basepath := filepath.Dir(fullpath)
@@ -126,7 +132,7 @@ func (self *SwarmFS) Mount(mhash, mountpoint string) (string, error)  {
 
 				if _, ok := dirTree[dirUntilNow]; !ok {
 					dirTree[dirUntilNow] = &Dir{
-						inode:       NewInode(),
+						inode:       newInode(),
 						name:        thisDir,
 						path:        dirUntilNow,
 						directories: nil,
@@ -142,7 +148,7 @@ func (self *SwarmFS) Mount(mhash, mountpoint string) (string, error)  {
 			}
 		}
 		thisFile := &File{
-			inode:    NewInode(),
+			inode:    newInode(),
 			name:     filename,
 			path:     fullpath,
 			key:      key,
@@ -154,113 +160,84 @@ func (self *SwarmFS) Mount(mhash, mountpoint string) (string, error)  {
 	fconn, err := fuse.Mount(cleanedMountPoint, fuse.FSName("swarmfs"), fuse.VolumeName(mhash))
 	if err != nil {
 		fuse.Unmount(cleanedMountPoint)
-		errStr := fmt.Sprintf("Mounting %s encountered error: %v", cleanedMountPoint, err)
-		log.Warn(errStr)
-		return errStr, err
+		log.Warn("Error mounting swarm manifest", "mountpoint", cleanedMountPoint, "err", err)
+		return nil, err
 	}
 
 	mounterr := make(chan error, 1)
 	go func() {
-		log.Info(fmt.Sprintf("Serving %s at %s", mhash, cleanedMountPoint))
 		filesys := &FS{root: rootDir}
 		if err := fs.Serve(fconn, filesys); err != nil {
-			log.Warn(fmt.Sprintf("Could not Serve FS error: %v", err))
+			mounterr <- err
 		}
 	}()
 
 	// Check if the mount process has an error to report.
 	select {
-
 	case <-time.After(mountTimeout):
-		err := fmt.Errorf("Mounting %s timed out.", cleanedMountPoint)
-		log.Warn(err.Error())
-		return err.Error(), err
+		fuse.Unmount(cleanedMountPoint)
+		return nil, errMountTimeout
 
 	case err := <-mounterr:
-	        errStr := fmt.Sprintf("Mounting %s encountered error: %v", cleanedMountPoint, err)
-		log.Warn(errStr)
-		return errStr, err
+		log.Warn("Error serving swarm FUSE FS", "mountpoint", cleanedMountPoint, "err", err)
+		return nil, err
 
 	case <-fconn.Ready:
-		log.Debug(fmt.Sprintf("Mounting connection succeeded for : %v", cleanedMountPoint))
+		log.Info("Now serving swarm FUSE FS", "manifest", mhash, "mountpoint", cleanedMountPoint)
 	}
 
-
-
-	//Assemble and Store the mount information for future use
-	mountInformation := &MountInfo{
-		mountPoint:     cleanedMountPoint,
-		manifestHash:   mhash,
+	// Assemble and Store the mount information for future use
+	mi := &MountInfo{
+		MountPoint:     cleanedMountPoint,
+		ManifestHash:   mhash,
 		resolvedKey:    key,
 		rootDir:        rootDir,
 		fuseConnection: fconn,
 	}
-	self.activeMounts[cleanedMountPoint] = mountInformation
-
-	succString := fmt.Sprintf("Mounting successful for %s", cleanedMountPoint)
-	log.Info(succString)
-
-	return succString, nil
+	self.activeMounts[cleanedMountPoint] = mi
+	return mi, nil
 }
 
-func (self *SwarmFS) Unmount(mountpoint string) (string, error)  {
-
+func (self *SwarmFS) Unmount(mountpoint string) (bool, error) {
 	self.activeLock.Lock()
 	defer self.activeLock.Unlock()
 
 	cleanedMountPoint, err := filepath.Abs(filepath.Clean(mountpoint))
 	if err != nil {
-		return err.Error(), err
+		return false, err
 	}
 
-	// Get the mount information based on the mountpoint argument
 	mountInfo := self.activeMounts[cleanedMountPoint]
-
-
-	if mountInfo == nil || mountInfo.mountPoint != cleanedMountPoint {
-		err := fmt.Errorf("Could not find mount information for %s ", cleanedMountPoint)
-		log.Warn(err.Error())
-		return err.Error(), err
+	if mountInfo == nil || mountInfo.MountPoint != cleanedMountPoint {
+		return false, fmt.Errorf("%s is not mounted", cleanedMountPoint)
 	}
-
 	err = fuse.Unmount(cleanedMountPoint)
 	if err != nil {
-		//TODO: try forceful unmount if normal unmount fails
-		errStr := fmt.Sprintf("UnMount error: %v", err)
-		log.Warn(errStr)
-		return errStr, err
+		// TODO(jmozah): try forceful unmount if normal unmount fails
+		return false, err
 	}
 
+	// remove the mount information from the active map
 	mountInfo.fuseConnection.Close()
-
-	//remove the mount information from the active map
 	delete(self.activeMounts, cleanedMountPoint)
-
-	succString := fmt.Sprintf("UnMounting %v succeeded", cleanedMountPoint)
-	log.Info(succString)
-	return succString, nil
+	return true, nil
 }
 
-func (self *SwarmFS) Listmounts() (string, error) {
-
+func (self *SwarmFS) Listmounts() []*MountInfo {
 	self.activeLock.RLock()
 	defer self.activeLock.RUnlock()
 
-	var rows []string
-	for mp := range self.activeMounts {
-		mountInfo := self.activeMounts[mp]
-		rows = append(rows, fmt.Sprintf("Swarm Root: %s, Mount Point: %s ", mountInfo.manifestHash, mountInfo.mountPoint))
+	rows := make([]*MountInfo, 0, len(self.activeMounts))
+	for _, mi := range self.activeMounts {
+		rows = append(rows, mi)
 	}
-
-	return strings.Join(rows, "\n"), nil
+	return rows
 }
 
 func (self *SwarmFS) Stop() bool {
-
 	for mp := range self.activeMounts {
 		mountInfo := self.activeMounts[mp]
-		self.Unmount(mountInfo.mountPoint)
+		self.Unmount(mountInfo.MountPoint)
 	}
-
 	return true
 }