Centralize path magic in apiserver into the New method

Make OperationHandler and WatchHandler properly encapsulate their
concerns and make them not depend on the global path

pkg/apiserver/apiserver.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
+	"path"
 	"runtime/debug"
 	"strings"
 	"time"
@@ -48,12 +49,11 @@ type Codec interface {
 //
 // TODO: consider migrating this to go-restful which is a more full-featured version of the same thing.
 type APIServer struct {
-	prefix      string
 	storage     map[string]RESTStorage
 	codec       Codec
 	ops         *Operations
-	mux         *http.ServeMux
 	asyncOpWait time.Duration
+	handler     http.Handler
 }
 
 // New creates a new APIServer object. 'storage' contains a map of handlers. 'codec'
@@ -65,31 +65,43 @@ type APIServer struct {
 // TODO: add multitype codec serialization
 func New(storage map[string]RESTStorage, codec Codec, prefix string) *APIServer {
 	s := &APIServer{
-		prefix:  strings.TrimRight(prefix, "/"),
 		storage: storage,
 		codec:   codec,
 		ops:     NewOperations(),
-		mux:     http.NewServeMux(),
 		// Delay just long enough to handle most simple write operations
 		asyncOpWait: time.Millisecond * 25,
 	}
 
-	// Primary API methods
+	mux := http.NewServeMux()
-	s.mux.HandleFunc(s.prefix+"/", s.handleREST)
+
-	s.mux.HandleFunc(s.watchPrefix()+"/", s.handleWatch)
+	prefix = strings.TrimRight(prefix, "/")
+
+	// Primary API handlers
+	restPrefix := prefix + "/"
+	mux.Handle(restPrefix, http.StripPrefix(restPrefix, http.HandlerFunc(s.handleREST)))
+
+	// Watch API handlers
+	watchPrefix := path.Join(prefix, "watch") + "/"
+	mux.Handle(watchPrefix, http.StripPrefix(watchPrefix, &WatchHandler{storage}))
 
 	// Support services for the apiserver
-	s.mux.Handle("/logs/", http.StripPrefix("/logs/", http.FileServer(http.Dir("/var/log/"))))
+	logsPrefix := "/logs/"
-	healthz.InstallHandler(s.mux)
+	mux.Handle(logsPrefix, http.StripPrefix(logsPrefix, http.FileServer(http.Dir("/var/log/"))))
-	s.mux.HandleFunc("/version", handleVersion)
+	healthz.InstallHandler(mux)
-	s.mux.HandleFunc("/", handleIndex)
+	mux.HandleFunc("/version", handleVersion)
+	mux.HandleFunc("/", handleIndex)
 
 	// Handle both operations and operations/* with the same handler
-	s.mux.HandleFunc(s.operationPrefix(), s.handleOperation)
+	handler := &OperationHandler{s.ops, s.codec}
-	s.mux.HandleFunc(s.operationPrefix()+"/", s.handleOperation)
+	operationPrefix := path.Join(prefix, "operations")
+	mux.Handle(operationPrefix, http.StripPrefix(operationPrefix, handler))
+	operationsPrefix := operationPrefix + "/"
+	mux.Handle(operationsPrefix, http.StripPrefix(operationsPrefix, handler))
 
 	// Proxy minion requests
-	s.mux.Handle("/proxy/minion/", http.StripPrefix("/proxy/minion", http.HandlerFunc(handleProxyMinion)))
+	mux.Handle("/proxy/minion/", http.StripPrefix("/proxy/minion", http.HandlerFunc(handleProxyMinion)))
+
+	s.handler = mux
 
 	return s
 }
@@ -112,29 +124,25 @@ func (s *APIServer) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		),
 	).Log()
 
-	// Dispatch via our mux.
+	// Dispatch to the internal handler
-	s.mux.ServeHTTP(w, req)
+	s.handler.ServeHTTP(w, req)
 }
 
 // handleREST handles requests to all our RESTStorage objects.
 func (s *APIServer) handleREST(w http.ResponseWriter, req *http.Request) {
-	if !strings.HasPrefix(req.URL.Path, s.prefix) {
+	parts := splitPath(req.URL.Path)
+	if len(parts) < 1 {
 		notFound(w, req)
 		return
 	}
-	requestParts := strings.Split(req.URL.Path[len(s.prefix):], "/")[1:]
+	storage := s.storage[parts[0]]
-	if len(requestParts) < 1 {
-		notFound(w, req)
-		return
-	}
-	storage := s.storage[requestParts[0]]
 	if storage == nil {
-		httplog.LogOf(w).Addf("'%v' has no storage object", requestParts[0])
+		httplog.LogOf(w).Addf("'%v' has no storage object", parts[0])
 		notFound(w, req)
 		return
 	}
 
-	s.handleRESTStorage(requestParts, req, w, storage)
+	s.handleRESTStorage(parts, req, w, storage)
 }
 
 // handleRESTStorage is the main dispatcher for a storage object.  It switches on the HTTP method, and then
@@ -347,3 +355,12 @@ func readBody(req *http.Request) ([]byte, error) {
 	defer req.Body.Close()
 	return ioutil.ReadAll(req.Body)
 }
+
+// splitPath returns the segments for a URL path
+func splitPath(path string) []string {
+	path = strings.Trim(path, "/")
+	if path == "" {
+		return []string{}
+	}
+	return strings.Split(path, "/")
+}

pkg/apiserver/operation.go

@@ -18,10 +18,8 @@ package apiserver
 
 import (
 	"net/http"
-	"path"
 	"sort"
 	"strconv"
-	"strings"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -30,37 +28,25 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 )
 
-func (s *APIServer) operationPrefix() string {
+type OperationHandler struct {
-	return path.Join(s.prefix, "operations")
+	ops   *Operations
+	codec Codec
 }
 
-func (s *APIServer) handleOperation(w http.ResponseWriter, req *http.Request) {
+func (h *OperationHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
-	opPrefix := s.operationPrefix()
+	parts := splitPath(req.URL.Path)
-	if !strings.HasPrefix(req.URL.Path, opPrefix) {
+	if len(parts) > 1 || req.Method != "GET" {
-		notFound(w, req)
-		return
-	}
-	trimmed := strings.TrimLeft(req.URL.Path[len(opPrefix):], "/")
-	parts := strings.Split(trimmed, "/")
-	if trimmed == "" {
-		parts = []string{}
-	}
-	if len(parts) > 1 {
-		notFound(w, req)
-		return
-	}
-	if req.Method != "GET" {
 		notFound(w, req)
 		return
 	}
 	if len(parts) == 0 {
 		// List outstanding operations.
-		list := s.ops.List()
+		list := h.ops.List()
-		writeJSON(http.StatusOK, s.codec, list, w)
+		writeJSON(http.StatusOK, h.codec, list, w)
 		return
 	}
 
-	op := s.ops.Get(parts[0])
+	op := h.ops.Get(parts[0])
 	if op == nil {
 		notFound(w, req)
 		return
@@ -68,9 +54,9 @@ func (s *APIServer) handleOperation(w http.ResponseWriter, req *http.Request) {
 
 	obj, complete := op.StatusOrResult()
 	if complete {
-		writeJSON(http.StatusOK, s.codec, obj, w)
+		writeJSON(http.StatusOK, h.codec, obj, w)
 	} else {
-		writeJSON(http.StatusAccepted, s.codec, obj, w)
+		writeJSON(http.StatusAccepted, h.codec, obj, w)
 	}
 }
 

pkg/apiserver/watch.go

@@ -19,8 +19,6 @@ package apiserver
 import (
 	"encoding/json"
 	"net/http"
-	"path"
-	"strings"
 
 	"code.google.com/p/go.net/websocket"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@@ -28,22 +26,17 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/watch"
 )
 
-func (s *APIServer) watchPrefix() string {
+type WatchHandler struct {
-	return path.Join(s.prefix, "watch")
+	storage map[string]RESTStorage
 }
 
 // handleWatch processes a watch request
-func (s *APIServer) handleWatch(w http.ResponseWriter, req *http.Request) {
+func (h *WatchHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
-	prefix := s.watchPrefix()
+	parts := splitPath(req.URL.Path)
-	if !strings.HasPrefix(req.URL.Path, prefix) {
+	if len(parts) < 1 || req.Method != "GET" {
-		notFound(w, req)
-		return
-	}
-	parts := strings.Split(req.URL.Path[len(prefix):], "/")[1:]
-	if req.Method != "GET" || len(parts) < 1 {
 		notFound(w, req)
 	}
-	storage := s.storage[parts[0]]
+	storage := h.storage[parts[0]]
 	if storage == nil {
 		notFound(w, req)
 	}