Skip to content

Commit

Permalink
Merge pull request kubernetes#807 from smarterclayton/abstract_apiser…
Browse files Browse the repository at this point in the history 
…ver_encoding

Decouple apiserver from codec implementation
  • Loading branch information
@lavalamp
lavalamp committed Aug 8, 2014
2 parents bff752b + c9fc0bc commit ac6d6ec
Show file tree
Hide file tree
Showing 16 changed files with 140 additions and 128 deletions.
42 changes: 29 additions & 13 deletions pkg/apiserver/apiserver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}]
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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)
}
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
70 changes: 35 additions & 35 deletions pkg/apiserver/apiserver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{})
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}

Expand All @@ -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 {
Expand All @@ -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{}

Expand Down Expand Up @@ -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")
Expand All @@ -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")
Expand All @@ -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")
Expand Down Expand Up @@ -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")
Expand All @@ -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")
Expand All @@ -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{}
Expand All @@ -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{}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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{}

Expand All @@ -464,15 +464,15 @@ 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{}

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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)

Expand All @@ -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 == "" {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 4 additions & 1 deletion pkg/apiserver/interfaces.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)
}
Expand Down
Loading

0 comments on commit ac6d6ec

Please sign in to comment.