serve http/webdav: factor common http server creation to httplib

cmd/serve/http/http.go

@@ -3,7 +3,6 @@ package http
 import (
 	"fmt"
 	"html/template"
-	"log"
 	"net/http"
 	"os"
 	"path"
@@ -11,6 +10,7 @@ import (
 	"strings"
 
 	"github.com/ncw/rclone/cmd"
+	"github.com/ncw/rclone/cmd/serve/httplib"
 	"github.com/ncw/rclone/fs"
 	"github.com/ncw/rclone/fs/accounting"
 	"github.com/ncw/rclone/lib/rest"
@@ -19,13 +19,8 @@ import (
 	"github.com/spf13/cobra"
 )
 
-// Globals
-var (
-	bindAddress = "localhost:8080"
-)
-
 func init() {
-	Command.Flags().StringVarP(&bindAddress, "addr", "", bindAddress, "IPaddress:Port to bind server to.")
+	httplib.AddFlags(Command.Flags())
 	vfsflags.AddFlags(Command.Flags())
 }
 
@@ -37,10 +32,6 @@ var Command = &cobra.Command{
 over HTTP.  This can be viewed in a web browser or you can make a
 remote of type http read from it.
 
-Use --addr to specify which IP address and port the server should
-listen on, eg --addr 1.2.3.4:8000 or --addr :8080 to listen to all
-IPs.  By default it only listens on localhost.
-
 You can use the filter flags (eg --include, --exclude) to control what
 is served.
 
@@ -48,12 +39,12 @@ The server will log errors.  Use -v to see access logs.
 
 --bwlimit will be respected for file transfers.  Use --stats to
 control the stats printing.
-` + vfs.Help,
+` + httplib.Help + vfs.Help,
 	Run: func(command *cobra.Command, args []string) {
 		cmd.CheckArgs(1, 1, command, args)
 		f := cmd.NewFsSrc(args)
 		cmd.Run(false, true, command, func() error {
-			s := newServer(f, bindAddress)
+			s := newServer(f)
 			s.serve()
 			return nil
 		})
@@ -63,32 +54,25 @@ control the stats printing.
 // server contains everything to run the server
 type server struct {
 	f   fs.Fs
-	bindAddress string
 	vfs *vfs.VFS
+	srv *httplib.Server
 }
 
-func newServer(f fs.Fs, bindAddress string) *server {
+func newServer(f fs.Fs) *server {
+	mux := http.NewServeMux()
 	s := &server{
 		f:   f,
-		bindAddress: bindAddress,
 		vfs: vfs.New(f, &vfsflags.Opt),
+		srv: httplib.NewServer(mux),
 	}
+	mux.HandleFunc("/", s.handler)
 	return s
 }
 
-// serve creates the http server
+// serve runs the http server - doesn't return
 func (s *server) serve() {
-	mux := http.NewServeMux()
+	fs.Logf(s.f, "Serving on %s", s.srv.URL())
-	mux.HandleFunc("/", s.handler)
+	s.srv.Serve()
-	// FIXME make a transport?
-	httpServer := &http.Server{
-		Addr:           s.bindAddress,
-		Handler:        mux,
-		MaxHeaderBytes: 1 << 20,
-	}
-	initServer(httpServer)
-	fs.Logf(s.f, "Serving on http://%s/", bindAddress)
-	log.Fatal(httpServer.ListenAndServe())
 }
 
 // handler reads incoming requests and dispatches them

cmd/serve/http/http_test.go

@@ -28,7 +28,8 @@ const (
 )
 
 func startServer(t *testing.T, f fs.Fs) {
-	s := newServer(f, testBindAddress)
+	s := newServer(f)
+	s.srv.SetBindAddress(testBindAddress)
 	go s.serve()
 
 	// try to connect to the test server

cmd/serve/httplib/http_new.go

@@ -2,14 +2,14 @@
 
 //+build go1.8
 
-package http
+package httplib
 
 import (
 	"net/http"
 	"time"
 )
 
-// Initialise the http.Server for pre go1.8
+// Initialise the http.Server for post go1.8
 func initServer(s *http.Server) {
 	s.ReadHeaderTimeout = 10 * time.Second // time to send the headers
 	s.IdleTimeout = 60 * time.Second       // time to keep idle connections open

cmd/serve/httplib/http_old.go

@@ -2,7 +2,7 @@
 
 //+build !go1.8
 
-package http
+package httplib
 
 import (
 	"net/http"

cmd/serve/httplib/httplib.go

@@ -0,0 +1,68 @@
+// Package httplib provides common functionality for http servers
+package httplib
+
+import (
+	"fmt"
+	"log"
+	"net/http"
+
+	"github.com/spf13/pflag"
+)
+
+// Globals
+var (
+	bindAddress = "localhost:8080"
+)
+
+// AddFlags adds the http server specific flags
+func AddFlags(flagSet *pflag.FlagSet) {
+	flagSet.StringVarP(&bindAddress, "addr", "", bindAddress, "IPaddress:Port to bind server to.")
+}
+
+// Help contains text describing the http server to add to the command
+// help.
+var Help = `
+### Server options
+
+Use --addr to specify which IP address and port the server should
+listen on, eg --addr 1.2.3.4:8000 or --addr :8080 to listen to all
+IPs.  By default it only listens on localhost.
+`
+
+// Server contains info about the running http server
+type Server struct {
+	bindAddress string
+	httpServer  *http.Server
+}
+
+// NewServer creates an http server
+func NewServer(handler http.Handler) *Server {
+	s := &Server{
+		bindAddress: bindAddress,
+	}
+	// FIXME make a transport?
+	s.httpServer = &http.Server{
+		Addr:           s.bindAddress,
+		Handler:        handler,
+		MaxHeaderBytes: 1 << 20,
+	}
+	// go version specific initialisation
+	initServer(s.httpServer)
+	return s
+}
+
+// SetBindAddress overrides the config flag
+func (s *Server) SetBindAddress(addr string) {
+	s.bindAddress = addr
+	s.httpServer.Addr = addr
+}
+
+// Serve runs the server - doesn't return
+func (s *Server) Serve() {
+	log.Fatal(s.httpServer.ListenAndServe())
+}
+
+// URL returns the serving address of this server
+func (s *Server) URL() string {
+	return fmt.Sprintf("http://%s/", s.bindAddress)
+}

cmd/serve/webdav/webdav.go

@@ -8,6 +8,8 @@ import (
 	"os"
 
 	"github.com/ncw/rclone/cmd"
+	"github.com/ncw/rclone/cmd/serve/httplib"
+	"github.com/ncw/rclone/cmd/serve/httplib/httpflags"
 	"github.com/ncw/rclone/fs"
 	"github.com/ncw/rclone/fs/log"
 	"github.com/ncw/rclone/vfs"
@@ -17,13 +19,8 @@ import (
 	"golang.org/x/net/webdav"
 )
 
-// Globals
-var (
-	bindAddress = "localhost:8081"
-)
-
 func init() {
-	Command.Flags().StringVarP(&bindAddress, "addr", "", bindAddress, "IPaddress:Port to bind server to.")
+	httpflags.AddFlags(Command.Flags())
 	vfsflags.AddFlags(Command.Flags())
 }
 
@@ -39,37 +36,18 @@ write it.
 
 NB at the moment each directory listing reads the start of each file
 which is undesirable: see https://github.com/golang/go/issues/22577
-
+` + httplib.Help + vfs.Help,
-` + vfs.Help,
 	Run: func(command *cobra.Command, args []string) {
 		cmd.CheckArgs(1, 1, command, args)
-		fsrc := cmd.NewFsSrc(args)
+		f := cmd.NewFsSrc(args)
 		cmd.Run(false, false, command, func() error {
-			return serveWebDav(fsrc)
+			w := newWebDAV(f, &httpflags.Opt)
+			w.serve()
+			return nil
 		})
 	},
 }
 
-// serve the remote
-func serveWebDav(f fs.Fs) error {
-	fs.Logf(f, "WebDav Server started on %v", bindAddress)
-
-	webdavFS := &WebDAV{
-		f:   f,
-		vfs: vfs.New(f, &vfsflags.Opt),
-	}
-
-	handler := &webdav.Handler{
-		FileSystem: webdavFS,
-		LockSystem: webdav.NewMemLS(),
-		Logger:     webdavFS.logRequest, // FIXME
-	}
-
-	// FIXME use our HTTP transport
-	http.Handle("/", handler)
-	return http.ListenAndServe(bindAddress, nil)
-}
-
 // WebDAV is a webdav.FileSystem interface
 //
 // A FileSystem implements access to a collection of named files. The elements
@@ -85,11 +63,35 @@ func serveWebDav(f fs.Fs) error {
 type WebDAV struct {
 	f   fs.Fs
 	vfs *vfs.VFS
+	srv *httplib.Server
 }
 
 // check interface
 var _ webdav.FileSystem = (*WebDAV)(nil)
 
+// Make a new WebDAV to serve the remote
+func newWebDAV(f fs.Fs, opt *httplib.Options) *WebDAV {
+	w := &WebDAV{
+		f:   f,
+		vfs: vfs.New(f, &vfsflags.Opt),
+	}
+
+	handler := &webdav.Handler{
+		FileSystem: w,
+		LockSystem: webdav.NewMemLS(),
+		Logger:     w.logRequest, // FIXME
+	}
+
+	w.srv = httplib.NewServer(handler, opt)
+	return w
+}
+
+// serve runs the http server - doesn't return
+func (w *WebDAV) serve() {
+	fs.Logf(w.f, "WebDav Server started on %s", w.srv.URL())
+	w.srv.Serve()
+}
+
 // logRequest is called by the webdav module on every request
 func (w *WebDAV) logRequest(r *http.Request, err error) {
 	fs.Infof(r.URL.Path, "%s from %s", r.Method, r.RemoteAddr)

cmd/serve/webdav/webdav_test.go

@@ -13,13 +13,22 @@ import (
 	"testing"
 
 	_ "github.com/ncw/rclone/backend/local"
+	"github.com/ncw/rclone/cmd/serve/httplib"
 	"github.com/ncw/rclone/fstest"
 	"github.com/stretchr/testify/assert"
 )
 
+const (
+	testBindAddress = "localhost:51778"
+	testURL         = "http://" + testBindAddress + "/"
+)
+
 // TestWebDav runs the webdav server then runs the unit tests for the
 // webdav remote against it.
 func TestWebDav(t *testing.T) {
+	opt := httplib.DefaultOpt
+	opt.ListenAddr = testBindAddress
+
 	fstest.Initialise()
 
 	fremote, _, clean, err := fstest.RandomRemote(*fstest.RemoteName, *fstest.SubDir)
@@ -31,8 +40,8 @@ func TestWebDav(t *testing.T) {
 
 	// Start the server
 	go func() {
-		err := serveWebDav(fremote)
+		w := newWebDAV(fremote, &opt)
-		assert.NoError(t, err)
+		w.serve()
 	}()
 	// FIXME shut it down somehow?
 
@@ -52,7 +61,7 @@ func TestWebDav(t *testing.T) {
 	cmd := exec.Command("go", args...)
 	cmd.Env = append(os.Environ(),
 		"RCLONE_CONFIG_WEBDAVTEST_TYPE=webdav",
-		"RCLONE_CONFIG_WEBDAVTEST_URL=http://localhost:8081/",
+		"RCLONE_CONFIG_WEBDAVTEST_URL="+testURL,
 		"RCLONE_CONFIG_WEBDAVTEST_VENDOR=other",
 	)
 	out, err := cmd.CombinedOutput()