Split generic; add test, address other review comments

pkg/registry/event/registry.go

@@ -22,13 +22,14 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	etcderr "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors/etcd"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/registry/generic"
+	etcdgeneric "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/generic/etcd"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/tools"
 )
 
 // registry implements custom changes to generic.Etcd.
 type registry struct {
-	*generic.Etcd
+	*etcdgeneric.Etcd
 	ttl uint64
 }
 
@@ -42,7 +43,7 @@ func (r registry) Create(ctx api.Context, id string, obj runtime.Object) error {
 // EtcdHelper. ttl is the time that Events will be retained by the system.
 func NewEtcdRegistry(h tools.EtcdHelper, ttl uint64) generic.Registry {
 	return registry{
-		Etcd: &generic.Etcd{
+		Etcd: &etcdgeneric.Etcd{
 			NewFunc:      func() runtime.Object { return &api.Event{} },
 			NewListFunc:  func() runtime.Object { return &api.EventList{} },
 			EndpointName: "events",

pkg/registry/event/rest.go

@@ -89,14 +89,14 @@ func (rs *REST) getAttrs(obj runtime.Object) (objLabels, objFields labels.Set, e
 		return nil, nil, fmt.Errorf("invalid object type")
 	}
 	return labels.Set{}, labels.Set{
-		"InvolvedObject.Kind":            event.InvolvedObject.Kind,
+		"involvedObject.kind":            event.InvolvedObject.Kind,
-		"InvolvedObject.Name":            event.InvolvedObject.Name,
+		"involvedObject.name":            event.InvolvedObject.Name,
-		"InvolvedObject.UID":             event.InvolvedObject.UID,
+		"involvedObject.uid":             event.InvolvedObject.UID,
-		"InvolvedObject.APIVersion":      event.InvolvedObject.APIVersion,
+		"involvedObject.apiVersion":      event.InvolvedObject.APIVersion,
-		"InvolvedObject.ResourceVersion": fmt.Sprintf("%s", event.InvolvedObject.ResourceVersion),
+		"involvedObject.resourceVersion": fmt.Sprintf("%s", event.InvolvedObject.ResourceVersion),
-		"InvolvedObject.FieldPath":       event.InvolvedObject.FieldPath,
+		"involvedObject.fieldPath":       event.InvolvedObject.FieldPath,
-		"Status":                         event.Status,
+		"status":                         event.Status,
-		"Reason":                         event.Reason,
+		"reason":                         event.Reason,
 	}, nil
 }
 

pkg/registry/event/rest_test.go

@@ -114,14 +114,14 @@ func TestRESTgetAttrs(t *testing.T) {
 		t.Errorf("diff: %s", util.ObjectDiff(e, a))
 	}
 	expect := labels.Set{
-		"InvolvedObject.Kind":            "Pod",
+		"involvedObject.kind":            "Pod",
-		"InvolvedObject.Name":            "foo",
+		"involvedObject.name":            "foo",
-		"InvolvedObject.UID":             "long uid string",
+		"involvedObject.uid":             "long uid string",
-		"InvolvedObject.APIVersion":      testapi.Version(),
+		"involvedObject.apiVersion":      testapi.Version(),
-		"InvolvedObject.ResourceVersion": "0",
+		"involvedObject.resourceVersion": "0",
-		"InvolvedObject.FieldPath":       "",
+		"involvedObject.fieldPath":       "",
-		"Status":                         "tested",
+		"status":                         "tested",
-		"Reason":                         "forTesting",
+		"reason":                         "forTesting",
 	}
 	if e, a := expect, field; !reflect.DeepEqual(e, a) {
 		t.Errorf("diff: %s", util.ObjectDiff(e, a))
@@ -186,7 +186,7 @@ func TestRESTList(t *testing.T) {
 	reg.ObjectList = &api.EventList{
 		Items: []api.Event{*eventA, *eventB, *eventC},
 	}
-	got, err := rest.List(api.NewContext(), labels.Everything(), labels.Set{"Status": "tested"}.AsSelector())
+	got, err := rest.List(api.NewContext(), labels.Everything(), labels.Set{"status": "tested"}.AsSelector())
 	if err != nil {
 		t.Fatalf("Unexpected error %v", err)
 	}

pkg/registry/generic/etcd/doc.go

@@ -0,0 +1,19 @@
+/*
+Copyright 2014 Google Inc. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Package etcd has a generic implementation of a registry that
+// stores things in etcd.
+package etcd

pkg/registry/generic/etcd/etcd.go

@@ -14,11 +14,12 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package generic
+package etcd
 
 import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	etcderr "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors/etcd"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/registry/generic"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/tools"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/watch"
@@ -50,35 +51,13 @@ type Etcd struct {
 }
 
 // List returns a list of all the items matching m.
-func (e *Etcd) List(ctx api.Context, m Matcher) (runtime.Object, error) {
+func (e *Etcd) List(ctx api.Context, m generic.Matcher) (runtime.Object, error) {
 	list := e.NewListFunc()
 	err := e.Helper.ExtractToList(e.KeyRoot, list)
 	if err != nil {
 		return nil, err
 	}
-	return FilterList(list, m)
+	return generic.FilterList(list, m)
-}
-
-// FilterList filters any list object that conforms to the api conventions,
-// provided that 'm' works with the concrete type of list.
-func FilterList(list runtime.Object, m Matcher) (filtered runtime.Object, err error) {
-	// TODO: push a matcher down into tools.EtcdHelper to avoid all this
-	// nonsense. This is a lot of unnecessary copies.
-	items, err := runtime.ExtractList(list)
-	if err != nil {
-		return nil, err
-	}
-	var filteredItems []runtime.Object
-	for _, obj := range items {
-		if match, err := m.Matches(obj); err == nil && match {
-			filteredItems = append(filteredItems, obj)
-		}
-	}
-	err = runtime.SetList(list, filteredItems)
-	if err != nil {
-		return nil, err
-	}
-	return list, nil
 }
 
 // Create inserts a new item.
@@ -89,6 +68,7 @@ func (e *Etcd) Create(ctx api.Context, id string, obj runtime.Object) error {
 
 // Update updates the item.
 func (e *Etcd) Update(ctx api.Context, id string, obj runtime.Object) error {
+	// TODO: verify that SetObj checks ResourceVersion before succeeding.
 	err := e.Helper.SetObj(e.KeyFunc(id), obj)
 	return etcderr.InterpretUpdateError(err, e.EndpointName, id)
 }
@@ -111,7 +91,7 @@ func (e *Etcd) Delete(ctx api.Context, id string) error {
 
 // Watch starts a watch for the items that m matches.
 // TODO: Detect if m references a single object instead of a list.
-func (e *Etcd) Watch(ctx api.Context, m Matcher, resourceVersion uint64) (watch.Interface, error) {
+func (e *Etcd) Watch(ctx api.Context, m generic.Matcher, resourceVersion uint64) (watch.Interface, error) {
 	return e.Helper.WatchList(e.KeyRoot, resourceVersion, func(obj runtime.Object) bool {
 		matches, err := m.Matches(obj)
 		return err == nil && matches

pkg/registry/generic/etcd/etcd_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package generic
+package etcd
 
 import (
 	"fmt"
@@ -25,6 +25,7 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/testapi"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/registry/generic"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/tools"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
@@ -90,7 +91,7 @@ func TestEtcdList(t *testing.T) {
 
 	table := map[string]struct {
 		in      tools.EtcdResponseWithError
-		m       Matcher
+		m       generic.Matcher
 		out     runtime.Object
 		succeed bool
 	}{

pkg/registry/generic/registry.go

@@ -54,6 +54,19 @@ type Matcher interface {
 	Matches(obj runtime.Object) (bool, error)
 }
 
+// MatcherFunc makes a matcher from the provided function. For easy definition
+// of matchers for testing.
+func MatcherFunc(f func(obj runtime.Object) (bool, error)) Matcher {
+	return matcherFunc(f)
+}
+
+type matcherFunc func(obj runtime.Object) (bool, error)
+
+// Matches calls the embedded function.
+func (m matcherFunc) Matches(obj runtime.Object) (bool, error) {
+	return m(obj)
+}
+
 // Registry knows how to store & list any runtime.Object. Can be used for
 // any object types which don't require special features from the storage
 // layer.
@@ -65,3 +78,29 @@ type Registry interface {
 	Delete(ctx api.Context, id string) error
 	Watch(ctx api.Context, m Matcher, resourceVersion uint64) (watch.Interface, error)
 }
+
+// FilterList filters any list object that conforms to the api conventions,
+// provided that 'm' works with the concrete type of list.
+func FilterList(list runtime.Object, m Matcher) (filtered runtime.Object, err error) {
+	// TODO: push a matcher down into tools.EtcdHelper to avoid all this
+	// nonsense. This is a lot of unnecessary copies.
+	items, err := runtime.ExtractList(list)
+	if err != nil {
+		return nil, err
+	}
+	var filteredItems []runtime.Object
+	for _, obj := range items {
+		match, err := m.Matches(obj)
+		if err != nil {
+			return nil, err
+		}
+		if match {
+			filteredItems = append(filteredItems, obj)
+		}
+	}
+	err = runtime.SetList(list, filteredItems)
+	if err != nil {
+		return nil, err
+	}
+	return list, nil
+}

pkg/registry/generic/registry_test.go

@@ -18,15 +18,23 @@ package generic
 
 import (
 	"errors"
+	"reflect"
 	"testing"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
 )
 
-type Ignored struct{}
+type Ignored struct {
+	ID string
+}
 
-func (*Ignored) IsAnAPIObject() {}
+type IgnoredList struct {
+	Items []Ignored
+}
+
+func (*Ignored) IsAnAPIObject()     {}
+func (*IgnoredList) IsAnAPIObject() {}
 
 func TestSelectionPredicate(t *testing.T) {
 	table := map[string]struct {
@@ -90,3 +98,38 @@ func TestSelectionPredicate(t *testing.T) {
 		}
 	}
 }
+
+func TestFilterList(t *testing.T) {
+	try := &IgnoredList{
+		Items: []Ignored{
+			{"foo"},
+			{"bar"},
+			{"baz"},
+			{"qux"},
+			{"zot"},
+		},
+	}
+	expect := &IgnoredList{
+		Items: []Ignored{
+			{"bar"},
+			{"baz"},
+		},
+	}
+
+	got, err := FilterList(try,
+		MatcherFunc(func(obj runtime.Object) (bool, error) {
+			i, ok := obj.(*Ignored)
+			if !ok {
+				return false, errors.New("wrong type")
+			}
+			return i.ID[0] == 'b', nil
+		}),
+	)
+	if err != nil {
+		t.Fatalf("Unexpected error %v", err)
+	}
+
+	if e, a := expect, got; !reflect.DeepEqual(e, a) {
+		t.Errorf("Expected %#v, got %#v", e, a)
+	}
+}