Decouple apiserver from codec implementation

The apiserver on initialization must be provided with a codec
for encoding and decoding all handled objects including api.Status
and api.ServerOp.  In addition, the RESTStorage Extract() method
has been changed to New(), which returns a pointer object that the
codec must decode into (the internal object).  Switched registry
methods to use pointers for Create/Update instead of values.

pkg/apiserver/apiserver.go

@@ -33,6 +33,14 @@ import (
 	"github.com/golang/glog"
 )
 
+// Codec defines methods for serializing and deserializing API
+// objects
+type Codec interface {
+	Encode(obj interface{}) (data []byte, err error)
+	Decode(data []byte) (interface{}, error)
+	DecodeInto(data []byte, obj interface{}) error
+}
+
 // APIServer is an HTTPHandler that delegates to RESTStorage objects.
 // It handles URLs of the form:
 // ${prefix}/${storage_key}[/${object_name}]
@@ -42,18 +50,24 @@ import (
 type APIServer struct {
 	prefix      string
 	storage     map[string]RESTStorage
+	codec       Codec
 	ops         *Operations
 	mux         *http.ServeMux
 	asyncOpWait time.Duration
 }
 
-// New creates a new APIServer object.
-// 'storage' contains a map of handlers.
-// 'prefix' is the hosting path prefix.
-func New(storage map[string]RESTStorage, prefix string) *APIServer {
+// New creates a new APIServer object. 'storage' contains a map of handlers. 'codec'
+// is an interface for decoding to and from JSON. 'prefix' is the hosting path prefix.
+//
+// The codec will be used to decode the request body into an object pointer returned by
+// RESTStorage.New().  The Create() and Update() methods should cast their argument to
+// the type returned by New().
+// TODO: add multitype codec serialization
+func New(storage map[string]RESTStorage, codec Codec, prefix string) *APIServer {
 	s := &APIServer{
-		storage: storage,
 		prefix:  strings.TrimRight(prefix, "/"),
+		storage: storage,
+		codec:   codec,
 		ops:     NewOperations(),
 		mux:     http.NewServeMux(),
 		// Delay just long enough to handle most simple write operations
@@ -153,7 +167,7 @@ func (s *APIServer) handleRESTStorage(parts []string, req *http.Request, w http.
 				internalError(err, w)
 				return
 			}
-			writeJSON(http.StatusOK, list, w)
+			writeJSON(http.StatusOK, s.codec, list, w)
 		case 2:
 			item, err := storage.Get(parts[1])
 			if IsNotFound(err) {
@@ -164,7 +178,7 @@ func (s *APIServer) handleRESTStorage(parts []string, req *http.Request, w http.
 				internalError(err, w)
 				return
 			}
-			writeJSON(http.StatusOK, item, w)
+			writeJSON(http.StatusOK, s.codec, item, w)
 		default:
 			notFound(w, req)
 		}
@@ -179,7 +193,8 @@ func (s *APIServer) handleRESTStorage(parts []string, req *http.Request, w http.
 			internalError(err, w)
 			return
 		}
-		obj, err := storage.Extract(body)
+		obj := storage.New()
+		err = s.codec.DecodeInto(body, obj)
 		if IsNotFound(err) {
 			notFound(w, req)
 			return
@@ -227,7 +242,8 @@ func (s *APIServer) handleRESTStorage(parts []string, req *http.Request, w http.
 			internalError(err, w)
 			return
 		}
-		obj, err := storage.Extract(body)
+		obj := storage.New()
+		err = s.codec.DecodeInto(body, obj)
 		if IsNotFound(err) {
 			notFound(w, req)
 			return
@@ -286,15 +302,15 @@ func (s *APIServer) finishReq(op *Operation, w http.ResponseWriter) {
 				status = stat.Code
 			}
 		}
-		writeJSON(status, obj, w)
+		writeJSON(status, s.codec, obj, w)
 	} else {
-		writeJSON(http.StatusAccepted, obj, w)
+		writeJSON(http.StatusAccepted, s.codec, obj, w)
 	}
 }
 
 // writeJSON renders an object as JSON to the response
-func writeJSON(statusCode int, object interface{}, w http.ResponseWriter) {
-	output, err := api.Encode(object)
+func writeJSON(statusCode int, codec Codec, object interface{}, w http.ResponseWriter) {
+	output, err := codec.Encode(object)
 	if err != nil {
 		internalError(err, w)
 		return

pkg/apiserver/apiserver_test.go

@@ -39,6 +39,8 @@ func convert(obj interface{}) (interface{}, error) {
 	return obj, nil
 }
 
+var codec = api.Codec
+
 func init() {
 	api.AddKnownTypes("", Simple{}, SimpleList{})
 	api.AddKnownTypes("v1beta1", Simple{}, SimpleList{})
@@ -59,8 +61,8 @@ type SimpleRESTStorage struct {
 	list    []Simple
 	item    Simple
 	deleted string
-	updated Simple
-	created Simple
+	updated *Simple
+	created *Simple
 
 	// Valid if WatchAll or WatchSingle is called
 	fakeWatch *watch.FakeWatcher
@@ -97,14 +99,12 @@ func (storage *SimpleRESTStorage) Delete(id string) (<-chan interface{}, error)
 	}), nil
 }
 
-func (storage *SimpleRESTStorage) Extract(body []byte) (interface{}, error) {
-	var item Simple
-	api.DecodeInto(body, &item)
-	return item, storage.errors["extract"]
+func (storage *SimpleRESTStorage) New() interface{} {
+	return &Simple{}
 }
 
 func (storage *SimpleRESTStorage) Create(obj interface{}) (<-chan interface{}, error) {
-	storage.created = obj.(Simple)
+	storage.created = obj.(*Simple)
 	if err := storage.errors["create"]; err != nil {
 		return nil, err
 	}
@@ -117,7 +117,7 @@ func (storage *SimpleRESTStorage) Create(obj interface{}) (<-chan interface{}, e
 }
 
 func (storage *SimpleRESTStorage) Update(obj interface{}) (<-chan interface{}, error) {
-	storage.updated = obj.(Simple)
+	storage.updated = obj.(*Simple)
 	if err := storage.errors["update"]; err != nil {
 		return nil, err
 	}
@@ -154,7 +154,7 @@ func extractBody(response *http.Response, object interface{}) (string, error) {
 	if err != nil {
 		return string(body), err
 	}
-	err = api.DecodeInto(body, object)
+	err = codec.DecodeInto(body, object)
 	return string(body), err
 }
 
@@ -178,7 +178,7 @@ func TestNotFound(t *testing.T) {
 	}
 	handler := New(map[string]RESTStorage{
 		"foo": &SimpleRESTStorage{},
-	}, "/prefix/version")
+	}, codec, "/prefix/version")
 	server := httptest.NewServer(handler)
 	client := http.Client{}
 	for k, v := range cases {
@@ -199,7 +199,7 @@ func TestNotFound(t *testing.T) {
 }
 
 func TestVersion(t *testing.T) {
-	handler := New(map[string]RESTStorage{}, "/prefix/version")
+	handler := New(map[string]RESTStorage{}, codec, "/prefix/version")
 	server := httptest.NewServer(handler)
 	client := http.Client{}
 
@@ -228,7 +228,7 @@ func TestSimpleList(t *testing.T) {
 	storage := map[string]RESTStorage{}
 	simpleStorage := SimpleRESTStorage{}
 	storage["simple"] = &simpleStorage
-	handler := New(storage, "/prefix/version")
+	handler := New(storage, codec, "/prefix/version")
 	server := httptest.NewServer(handler)
 
 	resp, err := http.Get(server.URL + "/prefix/version/simple")
@@ -247,7 +247,7 @@ func TestErrorList(t *testing.T) {
 		errors: map[string]error{"list": fmt.Errorf("test Error")},
 	}
 	storage["simple"] = &simpleStorage
-	handler := New(storage, "/prefix/version")
+	handler := New(storage, codec, "/prefix/version")
 	server := httptest.NewServer(handler)
 
 	resp, err := http.Get(server.URL + "/prefix/version/simple")
@@ -271,7 +271,7 @@ func TestNonEmptyList(t *testing.T) {
 		},
 	}
 	storage["simple"] = &simpleStorage
-	handler := New(storage, "/prefix/version")
+	handler := New(storage, codec, "/prefix/version")
 	server := httptest.NewServer(handler)
 
 	resp, err := http.Get(server.URL + "/prefix/version/simple")
@@ -306,7 +306,7 @@ func TestGet(t *testing.T) {
 		},
 	}
 	storage["simple"] = &simpleStorage
-	handler := New(storage, "/prefix/version")
+	handler := New(storage, codec, "/prefix/version")
 	server := httptest.NewServer(handler)
 
 	resp, err := http.Get(server.URL + "/prefix/version/simple/id")
@@ -327,7 +327,7 @@ func TestGetMissing(t *testing.T) {
 		errors: map[string]error{"get": NewNotFoundErr("simple", "id")},
 	}
 	storage["simple"] = &simpleStorage
-	handler := New(storage, "/prefix/version")
+	handler := New(storage, codec, "/prefix/version")
 	server := httptest.NewServer(handler)
 
 	resp, err := http.Get(server.URL + "/prefix/version/simple/id")
@@ -345,7 +345,7 @@ func TestDelete(t *testing.T) {
 	simpleStorage := SimpleRESTStorage{}
 	ID := "id"
 	storage["simple"] = &simpleStorage
-	handler := New(storage, "/prefix/version")
+	handler := New(storage, codec, "/prefix/version")
 	server := httptest.NewServer(handler)
 
 	client := http.Client{}
@@ -367,7 +367,7 @@ func TestDeleteMissing(t *testing.T) {
 		errors: map[string]error{"delete": NewNotFoundErr("simple", ID)},
 	}
 	storage["simple"] = &simpleStorage
-	handler := New(storage, "/prefix/version")
+	handler := New(storage, codec, "/prefix/version")
 	server := httptest.NewServer(handler)
 
 	client := http.Client{}
@@ -387,13 +387,13 @@ func TestUpdate(t *testing.T) {
 	simpleStorage := SimpleRESTStorage{}
 	ID := "id"
 	storage["simple"] = &simpleStorage
-	handler := New(storage, "/prefix/version")
+	handler := New(storage, codec, "/prefix/version")
 	server := httptest.NewServer(handler)
 
 	item := Simple{
 		Name: "bar",
 	}
-	body, err := api.Encode(item)
+	body, err := codec.Encode(item)
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
@@ -417,13 +417,13 @@ func TestUpdateMissing(t *testing.T) {
 		errors: map[string]error{"update": NewNotFoundErr("simple", ID)},
 	}
 	storage["simple"] = &simpleStorage
-	handler := New(storage, "/prefix/version")
+	handler := New(storage, codec, "/prefix/version")
 	server := httptest.NewServer(handler)
 
 	item := Simple{
 		Name: "bar",
 	}
-	body, err := api.Encode(item)
+	body, err := codec.Encode(item)
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
@@ -441,7 +441,7 @@ func TestUpdateMissing(t *testing.T) {
 }
 
 func TestBadPath(t *testing.T) {
-	handler := New(map[string]RESTStorage{}, "/prefix/version")
+	handler := New(map[string]RESTStorage{}, codec, "/prefix/version")
 	server := httptest.NewServer(handler)
 	client := http.Client{}
 
@@ -464,7 +464,7 @@ func TestCreate(t *testing.T) {
 	simpleStorage := &SimpleRESTStorage{}
 	handler := New(map[string]RESTStorage{
 		"foo": simpleStorage,
-	}, "/prefix/version")
+	}, codec, "/prefix/version")
 	handler.asyncOpWait = 0
 	server := httptest.NewServer(handler)
 	client := http.Client{}
@@ -472,7 +472,7 @@ func TestCreate(t *testing.T) {
 	simple := Simple{
 		Name: "foo",
 	}
-	data, _ := api.Encode(simple)
+	data, _ := codec.Encode(simple)
 	request, err := http.NewRequest("POST", server.URL+"/prefix/version/foo", bytes.NewBuffer(data))
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
@@ -505,12 +505,12 @@ func TestCreateNotFound(t *testing.T) {
 			// See https://github.com/GoogleCloudPlatform/kubernetes/pull/486#discussion_r15037092.
 			errors: map[string]error{"create": NewNotFoundErr("simple", "id")},
 		},
-	}, "/prefix/version")
+	}, codec, "/prefix/version")
 	server := httptest.NewServer(handler)
 	client := http.Client{}
 
 	simple := Simple{Name: "foo"}
-	data, _ := api.Encode(simple)
+	data, _ := codec.Encode(simple)
 	request, err := http.NewRequest("POST", server.URL+"/prefix/version/simple", bytes.NewBuffer(data))
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
@@ -547,14 +547,14 @@ func TestSyncCreate(t *testing.T) {
 	}
 	handler := New(map[string]RESTStorage{
 		"foo": &storage,
-	}, "/prefix/version")
+	}, codec, "/prefix/version")
 	server := httptest.NewServer(handler)
 	client := http.Client{}
 
 	simple := Simple{
 		Name: "foo",
 	}
-	data, _ := api.Encode(simple)
+	data, _ := codec.Encode(simple)
 	request, err := http.NewRequest("POST", server.URL+"/prefix/version/foo?sync=true", bytes.NewBuffer(data))
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
@@ -616,7 +616,7 @@ func TestAsyncDelayReturnsError(t *testing.T) {
 			return nil, errors.New("error")
 		},
 	}
-	handler := New(map[string]RESTStorage{"foo": &storage}, "/prefix/version")
+	handler := New(map[string]RESTStorage{"foo": &storage}, codec, "/prefix/version")
 	handler.asyncOpWait = time.Millisecond / 2
 	server := httptest.NewServer(handler)
 
@@ -634,12 +634,12 @@ func TestAsyncCreateError(t *testing.T) {
 			return nil, errors.New("error")
 		},
 	}
-	handler := New(map[string]RESTStorage{"foo": &storage}, "/prefix/version")
+	handler := New(map[string]RESTStorage{"foo": &storage}, codec, "/prefix/version")
 	handler.asyncOpWait = 0
 	server := httptest.NewServer(handler)
 
 	simple := Simple{Name: "foo"}
-	data, _ := api.Encode(simple)
+	data, _ := codec.Encode(simple)
 
 	status := expectApiStatus(t, "POST", fmt.Sprintf("%s/prefix/version/foo", server.URL), data, http.StatusAccepted)
 	if status.Status != api.StatusWorking || status.Details == nil || status.Details.ID == "" {
@@ -670,7 +670,7 @@ func TestWriteJSONDecodeError(t *testing.T) {
 		type T struct {
 			Value string
 		}
-		writeJSON(http.StatusOK, &T{"Undecodable"}, w)
+		writeJSON(http.StatusOK, api.Codec, &T{"Undecodable"}, w)
 	}))
 	client := http.Client{}
 	resp, err := client.Get(server.URL)
@@ -715,11 +715,11 @@ func TestSyncCreateTimeout(t *testing.T) {
 	}
 	handler := New(map[string]RESTStorage{
 		"foo": &storage,
-	}, "/prefix/version")
+	}, codec, "/prefix/version")
 	server := httptest.NewServer(handler)
 
 	simple := Simple{Name: "foo"}
-	data, _ := api.Encode(simple)
+	data, _ := codec.Encode(simple)
 	itemOut := expectApiStatus(t, "POST", server.URL+"/prefix/version/foo?sync=true&timeout=4ms", data, http.StatusAccepted)
 	if itemOut.Status != api.StatusWorking || itemOut.Details == nil || itemOut.Details.ID == "" {
 		t.Errorf("Unexpected status %#v", itemOut)

pkg/apiserver/interfaces.go

@@ -24,6 +24,10 @@ import (
 // RESTStorage is a generic interface for RESTful storage services
 // Resources which are exported to the RESTful API of apiserver need to implement this interface.
 type RESTStorage interface {
+	// New returns an empty object that can be used with Create and Update after request data has been put into it.
+	// This object must be a pointer type for use with Codec.DecodeInto([]byte, interface{})
+	New() interface{}
+
 	// List selects resources in the storage which match to the selector.
 	List(labels.Selector) (interface{}, error)
 
@@ -35,7 +39,6 @@ type RESTStorage interface {
 	// Although it can return an arbitrary error value, IsNotFound(err) is true for the returned error value err when the specified resource is not found.
 	Delete(id string) (<-chan interface{}, error)
 
-	Extract(body []byte) (interface{}, error)
 	Create(interface{}) (<-chan interface{}, error)
 	Update(interface{}) (<-chan interface{}, error)
 }

pkg/apiserver/minionproxy_test.go

@@ -127,7 +127,7 @@ func TestApiServerMinionProxy(t *testing.T) {
 	proxyServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
 		w.Write([]byte(req.URL.Path))
 	}))
-	server := httptest.NewServer(New(nil, "/prefix"))
+	server := httptest.NewServer(New(nil, nil, "/prefix"))
 	proxy, _ := url.Parse(proxyServer.URL)
 	resp, err := http.Get(fmt.Sprintf("%s/proxy/minion/%s%s", server.URL, proxy.Host, "/test"))
 	if err != nil {

pkg/apiserver/operation.go

@@ -56,7 +56,7 @@ func (s *APIServer) handleOperation(w http.ResponseWriter, req *http.Request) {
 	if len(parts) == 0 {
 		// List outstanding operations.
 		list := s.ops.List()
-		writeJSON(http.StatusOK, list, w)
+		writeJSON(http.StatusOK, s.codec, list, w)
 		return
 	}
 
@@ -68,9 +68,9 @@ func (s *APIServer) handleOperation(w http.ResponseWriter, req *http.Request) {
 
 	obj, complete := op.StatusOrResult()
 	if complete {
-		writeJSON(http.StatusOK, obj, w)
+		writeJSON(http.StatusOK, s.codec, obj, w)
 	} else {
-		writeJSON(http.StatusAccepted, obj, w)
+		writeJSON(http.StatusAccepted, s.codec, obj, w)
 	}
 }
 

pkg/apiserver/operation_test.go

@@ -95,7 +95,7 @@ func TestOperationsList(t *testing.T) {
 	simpleStorage := &SimpleRESTStorage{}
 	handler := New(map[string]RESTStorage{
 		"foo": simpleStorage,
-	}, "/prefix/version")
+	}, codec, "/prefix/version")
 	handler.asyncOpWait = 0
 	server := httptest.NewServer(handler)
 	client := http.Client{}
@@ -103,7 +103,7 @@ func TestOperationsList(t *testing.T) {
 	simple := Simple{
 		Name: "foo",
 	}
-	data, err := api.Encode(simple)
+	data, err := codec.Encode(simple)
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
@@ -126,7 +126,7 @@ func TestOperationsList(t *testing.T) {
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
-	obj, err := api.Decode(body)
+	obj, err := codec.Decode(body)
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
@@ -143,7 +143,7 @@ func TestOpGet(t *testing.T) {
 	simpleStorage := &SimpleRESTStorage{}
 	handler := New(map[string]RESTStorage{
 		"foo": simpleStorage,
-	}, "/prefix/version")
+	}, codec, "/prefix/version")
 	handler.asyncOpWait = 0
 	server := httptest.NewServer(handler)
 	client := http.Client{}
@@ -151,7 +151,7 @@ func TestOpGet(t *testing.T) {
 	simple := Simple{
 		Name: "foo",
 	}
-	data, err := api.Encode(simple)
+	data, err := codec.Encode(simple)
 	t.Log(string(data))
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)

pkg/apiserver/watch_test.go

@@ -42,7 +42,7 @@ func TestWatchWebsocket(t *testing.T) {
 	simpleStorage := &SimpleRESTStorage{}
 	handler := New(map[string]RESTStorage{
 		"foo": simpleStorage,
-	}, "/prefix/version")
+	}, codec, "/prefix/version")
 	server := httptest.NewServer(handler)
 
 	dest, _ := url.Parse(server.URL)
@@ -92,7 +92,7 @@ func TestWatchHTTP(t *testing.T) {
 	simpleStorage := &SimpleRESTStorage{}
 	handler := New(map[string]RESTStorage{
 		"foo": simpleStorage,
-	}, "/prefix/version")
+	}, codec, "/prefix/version")
 	server := httptest.NewServer(handler)
 	client := http.Client{}