Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

make selfLink namespace aware #2568

Merged
merged 1 commit into from
Dec 6, 2014
Merged

make selfLink namespace aware #2568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 15 additions & 6 deletions pkg/apiserver/apiserver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,8 @@ func TestSimpleList(t *testing.T) {
storage["simple"] = &simpleStorage
selfLinker := &setTestSelfLinker{
t: t,
expectedSet: "/prefix/version/simple",
namespace: "other",
expectedSet: "/prefix/version/simple?namespace=other",
}
handler := Handle(storage, codec, "/prefix", testVersion, selfLinker)
server := httptest.NewServer(handler)
Expand Down Expand Up @@ -360,8 +361,9 @@ func TestNonEmptyList(t *testing.T) {
simpleStorage := SimpleRESTStorage{
list: []Simple{
{
TypeMeta: api.TypeMeta{Kind: "Simple"},
Other: "foo",
TypeMeta: api.TypeMeta{Kind: "Simple"},
ObjectMeta: api.ObjectMeta{Namespace: "other"},
Other: "foo",
},
},
}
Expand Down Expand Up @@ -394,6 +396,10 @@ func TestNonEmptyList(t *testing.T) {
if listOut.Items[0].Other != simpleStorage.list[0].Other {
t.Errorf("Unexpected data: %#v, %s", listOut.Items[0], string(body))
}
expectedSelfLink := "/prefix/version/simple?namespace=other"
if listOut.Items[0].ObjectMeta.SelfLink != expectedSelfLink {
t.Errorf("Unexpected data: %#v, %s", listOut.Items[0].ObjectMeta.SelfLink, expectedSelfLink)
}
}

func TestGet(t *testing.T) {
Expand Down Expand Up @@ -651,11 +657,13 @@ type setTestSelfLinker struct {
t *testing.T
expectedSet string
name string
namespace string
called bool
}

func (s *setTestSelfLinker) Name(runtime.Object) (string, error) { return s.name, nil }
func (*setTestSelfLinker) SelfLink(runtime.Object) (string, error) { return "", nil }
func (s *setTestSelfLinker) Namespace(runtime.Object) (string, error) { return s.namespace, nil }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please exercise this in a test :)

func (s *setTestSelfLinker) Name(runtime.Object) (string, error) { return s.name, nil }
func (*setTestSelfLinker) SelfLink(runtime.Object) (string, error) { return "", nil }
func (s *setTestSelfLinker) SetSelfLink(obj runtime.Object, selfLink string) error {
if e, a := s.expectedSet, selfLink; e != a {
s.t.Errorf("expected '%v', got '%v'", e, a)
Expand All @@ -674,7 +682,8 @@ func TestSyncCreate(t *testing.T) {
selfLinker := &setTestSelfLinker{
t: t,
name: "bar",
expectedSet: "/prefix/version/foo/bar",
namespace: "other",
expectedSet: "/prefix/version/foo/bar?namespace=other",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding-- is there no test for list types?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a test that gets a non-empty list, but the way that setTestSelfLinker works, it will error when any of the descendant items links are set because their links will be different than the expectedSet. As a result, I see no test that uses expectedSet when fetching lists of items.

}
handler := Handle(map[string]RESTStorage{
"foo": &storage,
Expand Down
22 changes: 21 additions & 1 deletion pkg/apiserver/resthandler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,17 @@ func (h *RESTHandler) setSelfLink(obj runtime.Object, req *http.Request) error {
newURL.Path = path.Join(h.canonicalPrefix, req.URL.Path)
newURL.RawQuery = ""
newURL.Fragment = ""
err := h.selfLinker.SetSelfLink(obj, newURL.String())
namespace, err := h.selfLinker.Namespace(obj)
if err != nil {
return err
}
// TODO Remove this when namespace is in path
if len(namespace) > 0 {
query := newURL.Query()
query.Set("namespace", namespace)
newURL.RawQuery = query.Encode()
}
err = h.selfLinker.SetSelfLink(obj, newURL.String())
if err != nil {
return err
}
Expand All @@ -89,10 +99,20 @@ func (h *RESTHandler) setSelfLinkAddName(obj runtime.Object, req *http.Request)
if err != nil {
return err
}
namespace, err := h.selfLinker.Namespace(obj)
if err != nil {
return err
}
newURL := *req.URL
newURL.Path = path.Join(h.canonicalPrefix, req.URL.Path, name)
newURL.RawQuery = ""
newURL.Fragment = ""
// TODO Remove this when namespace is in path
if len(namespace) > 0 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must be in setSelfLink, too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, was looking at the items in a list and not the list itself. Will add.

query := newURL.Query()
query.Set("namespace", namespace)
newURL.RawQuery = query.Encode()
}
return h.selfLinker.SetSelfLink(obj, newURL.String())
}

Expand Down
2 changes: 2 additions & 0 deletions pkg/runtime/interfaces.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ type SelfLinker interface {

// Knowing Name is sometimes necessary to use a SelfLinker.
Name(obj Object) (string, error)
// Knowing Namespace is sometimes necessary to use a SelfLinker
Namespace(obj Object) (string, error)
}

// All api types must support the Object interface. It's deliberately tiny so that this is not an onerous
Expand Down