Simplify file router, add documenting comments.

The file router was starting to get out of hand, so I've done my best to
simplify it and get ahead of more spaghetti. As part of this, tidied up
some other code and added some much-needed comments and constants to
make things more readable. ObjectInfo() is much more useful as well,
determing if a file exists, if it's a directory and the location.

Feeling other bits of the project can be simplified, but mostly been
focused on the file providers and associated router.

files/backblaze.go

@@ -138,7 +138,7 @@ func (bp *BackblazeProvider) GetDirectory(path string) Directory {
 	return finalDir
 }
 
-func (bp *BackblazeProvider) ViewFile(path string) string {
+func (bp *BackblazeProvider) FilePath(path string) string {
 	return ""
 }
 
@@ -257,10 +257,12 @@ func (bp *BackblazeProvider) SaveFile(file io.Reader, filename string, path stri
 	return true
 }
 
-func (bp *BackblazeProvider) ObjectInfo(path string) (string, string) {
+func (bp *BackblazeProvider) ObjectInfo(path string) (bool, bool, string) {
 	// B2 is really a "flat" filesystem, with directories being virtual.
 	// Therefore, we can assume everything is a file ;)
-	return "file", "remote"
+	// TODO: Return true value.
+	isDir := path == ""
+	return true, isDir, FILE_IS_REMOTE
 }
 
 func (bp *BackblazeProvider) CreateDirectory(path string) bool {

files/disk.go

@@ -42,7 +42,7 @@ func (dp *DiskProvider) GetDirectory(path string) Directory {
 	}
 }
 
-func (dp *DiskProvider) ViewFile(path string) string {
+func (dp *DiskProvider) FilePath(path string) string {
 	rp := strings.Join([]string{dp.Location,path}, "/")
 	return rp
 }
@@ -68,18 +68,18 @@ func (dp *DiskProvider) SaveFile(file io.Reader, filename string, path string) b
 	return true
 }
 
-func (dp *DiskProvider) ObjectInfo(path string) (string, string) {
+func (dp *DiskProvider) ObjectInfo(path string) (bool, bool, string) {
 	rp := strings.Join([]string{dp.Location,path}, "/")
 	fileStat, err := os.Stat(rp)
 	if err != nil {
-		fmt.Printf("error stat'ing file %v: %v", rp, err.Error())
+		fmt.Printf("error gather stats for file %v: %v", rp, err.Error())
-		return "", ""
+		return false, false, FILE_IS_LOCAL
 	}
 
 	if fileStat.IsDir() {
-		return "directory", ""
+		return true, true, FILE_IS_LOCAL
 	}
-	return "file", "local"
+	return true, false, FILE_IS_LOCAL
 }
 
 func (dp *DiskProvider) CreateDirectory(path string) bool {

files/fileprovider.go

@@ -4,6 +4,9 @@ import (
 	"io"
 )
 
+const FILE_IS_REMOTE = "remote"
+const FILE_IS_LOCAL = "local"
+
 type FileProvider struct {
 	Name           string            `yaml:"name"`
 	Provider       string            `yaml:"provider"`
@@ -29,14 +32,24 @@ type FileContents struct {
 }
 
 type FileProviderInterface interface {
-	Setup(args map[string]string) bool
+	// Called on initial startup of the application.
-	GetDirectory(path string) Directory
+	Setup(args map[string]string) (ok bool)
-	ViewFile(path string) string
+	// Fetches the contents of a "directory".
+	GetDirectory(path string) (directory Directory)
+	// Builds a path to a file, for serving.
+	FilePath(path string) (realpath string)
+	// Fetch and pass along a remote file directly to the response writer.
 	RemoteFile(path string, writer io.Writer)
-	SaveFile(file io.Reader, filename string, path string) bool
+	// Save a file from an io.Reader.
-	ObjectInfo(path string) (string, string)
+	SaveFile(file io.Reader, filename string, path string) (ok bool)
-	CreateDirectory(path string) bool
+	// Fetch the info for an object given a path to if the file exists and location.
-	Delete(path string) bool
+	// Should return whether the path exists, if the path is a directory, and if it lives on disk.
+	// (see constants defined: `FILE_IS_REMOTE` and `FILE_IS_LOCAL`)
+	ObjectInfo(path string) (exists bool, isDir bool, location string)
+	// Create a directory if possible, returns the result.
+	CreateDirectory(path string) (ok bool)
+	// Delete a file or directory.
+	Delete(path string) (ok bool)
 }
 
 /** DO NOT USE THESE DEFAULTS **/
@@ -48,7 +61,7 @@ func (f FileProvider) GetDirectory(path string) Directory {
 	return Directory{}
 }
 
-func (f FileProvider) ViewFile(path string) string {
+func (f FileProvider) FilePath(path string) string {
 	return ""
 }
 
@@ -60,8 +73,8 @@ func (f FileProvider) SaveFile(file io.Reader, filename string, path string) boo
 	return false
 }
 
-func (f FileProvider) ObjectInfo(path string) (string, string) {
+func (f FileProvider) ObjectInfo(path string) (bool, bool, string) {
-	return "", ""
+	return false, false, ""
 }
 
 func (f FileProvider) CreateDirectory(path string) bool {

files/files_test.go

@@ -9,7 +9,7 @@ import (
 	"testing"
 )
 
-const DISK_TESTING_GROUNDS = ".testing_data_directory/"
+const DISK_TESTING_GROUNDS = ".testing_data_directory"
 
 // Test basic file provider.
 // Should return, essentially, nothing. Can be used as a template for other providers.
@@ -24,16 +24,16 @@ func TestFileProvider(t *testing.T) {
 		t.Errorf("Default FileProvider GetDirectory() files returned %v, expected none.", getdirectory.Files)
 	}
 
-	viewfile := fp.ViewFile(""); if viewfile != "" {
+	filepath := fp.FilePath(""); if filepath != "" {
-		t.Errorf("Default FileProvider ViewFile() %v, expected nothing.", w)
+		t.Errorf("Default FileProvider FilePath() %v, expected nothing.", filepath)
 	}
 
 	savefile := fp.SaveFile(nil, "", ""); if savefile != false {
 		t.Errorf("Default FileProvider SaveFile() attempted to save a file.")
 	}
 
-	determinetype := fp.ObjectInfo(""); if determinetype != "" {
+	exists, isDir, location := fp.ObjectInfo(""); if exists || isDir || location != "" {
-		t.Errorf("Default FileProvider ObjectInfo() did not return an empty string.")
+		t.Errorf("Default FileProvider ObjectInfo() did not return default empty values.")
 	}
 
 	createdirectory := fp.CreateDirectory(""); if createdirectory {
@@ -73,7 +73,7 @@ func TestDiskProvider(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Failed to create testing directory for DiskProvider: %v", err.Error())
 	}
-	err = ioutil.WriteFile(DISK_TESTING_GROUNDS + "testing.txt", []byte("testing file!"), 0755)
+	err = ioutil.WriteFile(DISK_TESTING_GROUNDS + "/testing.txt", []byte("testing file!"), 0755)
 	if err != nil {
 		t.Fatalf("Failed to create testing file for DiskProvider: %v", err.Error())
 	}
@@ -87,8 +87,8 @@ func TestDiskProvider(t *testing.T) {
 		t.Errorf("DiskProvider GetDirectory() files returned %v, expected 1.", getdirectory.Files)
 	}
 
-	viewfile := dp.ViewFile("testing.txt"); if viewfile !=  DISK_TESTING_GROUNDS + "testing.txt"{
+	filepath := dp.FilePath("testing.txt"); if filepath !=  DISK_TESTING_GROUNDS + "/testing.txt"{
-		t.Errorf("DiskProvider ViewFile() returned %v, expected path.", viewfile)
+		t.Errorf("DiskProvider FilePath() returned %v, expected path.", filepath)
 	}
 
  	testfile := bytes.NewReader([]byte("second test file!"))
@@ -96,8 +96,8 @@ func TestDiskProvider(t *testing.T) {
 		t.Errorf("DiskProvider SaveFile() could not save a file.")
 	}
 
-	determinetype := dp.ObjectInfo("second_test.txt"); if determinetype != "file" {
+	exists, isDir, location := dp.ObjectInfo("second_test.txt"); if !exists || isDir || location != "local" {
-		t.Errorf("DiskProvider ObjectInfo() returned %v, expected \"file\".", determinetype)
+		t.Errorf("DiskProvider ObjectInfo() returned %v %v %v, expected true, false, local", exists, isDir, location)
 	}
 
 	createdirectory := dp.CreateDirectory("test_dir"); if !createdirectory {

router/filerouter.go

@@ -22,72 +22,61 @@ func HandleProvider() Handler {
 
 		// Directory listing or serve file.
 		if r.Method == "GET" {
-			fileList := provider.GetDirectory("")
+			filename, err := url.QueryUnescape(vars["file"])
-			if vars["file"] != "" {
-				filename, err := url.QueryUnescape(vars["file"])
-				if err != nil {
-					return &HTTPError{
-						Message:    fmt.Sprintf("error determining filetype for %s\n", filename),
-						StatusCode: http.StatusInternalServerError,
-					}
-				}
-				fileType, location := provider.ObjectInfo(filename)
-
-				if fileType == "" {
-					return &HTTPError{
-						Message:    fmt.Sprintf("error determining filetype for %s\n", filename),
-						StatusCode: http.StatusInternalServerError,
-					}
-				}
-				if fileType == "file" {
-					if location == "local" {
-						rp := provider.ViewFile(filename)
-						if rp != "" {
-							f, _ := os.Open(rp)
-							http.ServeContent(w, r, filename, time.Time{}, f)
-						}
-					}
-					if location == "remote" {
-						provider.RemoteFile(filename, w)
-					}
-					return nil
-				}
-				fileList = provider.GetDirectory(filename)
-			}
-			data, err := json.Marshal(fileList)
 			if err != nil {
 				return &HTTPError{
-					Message:    fmt.Sprintf("error fetching filelisting for %s\n", vars["file"]),
+					Message:    fmt.Sprintf("error determining filetype for %s\n", filename),
 					StatusCode: http.StatusInternalServerError,
 				}
 			}
-			w.Write(data)
+			ok, isDir, location := provider.ObjectInfo(filename)
+			if !ok {
+				return &HTTPError{
+					Message:    fmt.Sprintf("error locating file %s\n", filename),
+					StatusCode: http.StatusNotFound,
+				}
+			}
+
+			if isDir {
+				fileList := provider.GetDirectory(filename)
+
+				data, err := json.Marshal(fileList)
+				if err != nil {
+					return &HTTPError{
+						Message:    fmt.Sprintf("error fetching filelisting for %s\n", vars["file"]),
+						StatusCode: http.StatusNotFound,
+					}
+				}
+				w.Write(data)
+				return nil
+			}
+
+			// If the file is local, attempt to use http.ServeContent for correct headers.
+			if location == files.FILE_IS_LOCAL {
+				rp := provider.FilePath(filename)
+				if rp != "" {
+					f, err := os.Open(rp)
+					if err != nil {
+						return &HTTPError{
+							Message:    fmt.Sprintf("error opening file %s\n", rp),
+							StatusCode: http.StatusInternalServerError,
+						}
+					}
+					http.ServeContent(w, r, filename, time.Time{}, f)
+				}
+			}
+			// If the file is remote, then delegate the writing to the response to the provider.
+			// This isn't a great workaround, but avoids caching the whole file in mem or on disk.
+			if location == files.FILE_IS_REMOTE {
+				provider.RemoteFile(filename, w)
+			}
+			return nil
 		}
 
 		// File upload or directory creation.
 		if r.Method == "POST" {
 			xType := r.Header.Get("X-NAS-Type")
-
+			// We only really care about this header of creating a directory.
-			if xType == "file" || xType == ""{
-				err := r.ParseMultipartForm(32 << 20)
-				if err != nil {
-					return &HTTPError{
-						Message:    fmt.Sprintf("error parsing form for %s\n", vars["file"]),
-						StatusCode: http.StatusInternalServerError,
-					}
-				}
-				file, handler, err := r.FormFile("file")
-				defer file.Close()
-
-				success := provider.SaveFile(file, handler.Filename, vars["file"])
-				if !success {
-					return &HTTPError{
-						Message:    fmt.Sprintf("error saving file %s\n", vars["file"]),
-						StatusCode: http.StatusInternalServerError,
-					}
-				}
-				w.Write([]byte("saved file"))
-			}
 			if xType == "directory" {
 				dirname := vars["file"]
 				success := provider.CreateDirectory(dirname)
@@ -98,7 +87,27 @@ func HandleProvider() Handler {
 					}
 				}
 				_, _ = w.Write([]byte("created directory"))
+				return nil
 			}
+
+			err := r.ParseMultipartForm(32 << 20)
+			if err != nil {
+				return &HTTPError{
+					Message:    fmt.Sprintf("error parsing form for %s\n", vars["file"]),
+					StatusCode: http.StatusInternalServerError,
+				}
+			}
+			file, handler, err := r.FormFile("file")
+			defer file.Close()
+
+			success := provider.SaveFile(file, handler.Filename, vars["file"])
+			if !success {
+				return &HTTPError{
+					Message:    fmt.Sprintf("error saving file %s\n", vars["file"]),
+					StatusCode: http.StatusInternalServerError,
+				}
+			}
+			_, _  = w.Write([]byte("saved file"))
 		}
 
 		// Delete file.
@@ -114,7 +123,7 @@ func HandleProvider() Handler {
 			_, _ = w.Write([]byte("deleted"))
 		}
 
-			return nil
+		return nil
 	}
 }
 
@@ -132,7 +141,7 @@ func ListProviders() Handler {
 				StatusCode: http.StatusInternalServerError,
 			}
 		}
-		w.Write(data)
+		_, _ = w.Write(data)
 		return nil
 	}
 }