Remove reflection from generic store code. (knative#583)

configmap/store.go

@@ -17,7 +17,6 @@ limitations under the License.
 package configmap
 
 import (
-	"reflect"
 	"sync/atomic"
 
 	corev1 "k8s.io/api/core/v1"
@@ -36,13 +35,11 @@ type Logger interface {
 
 // Constructors is a map for specifying config names to
 // their function constructors
-//
-// The values of this map must be functions with the definition
-//
-// func(*k8s.io/api/core/v1.ConfigMap) (... , error)
-//
-// These functions can return any type along with an error
-type Constructors map[string]interface{}
+type Constructors map[string]Constructor
+
+// Constructor produces any value or an error from a
+// ConfigMap.
+type Constructor func(*corev1.ConfigMap) (interface{}, error)
 
 // An UntypedStore is a responsible for storing and
 // constructing configs from Kubernetes ConfigMaps
@@ -54,7 +51,7 @@ type UntypedStore struct {
 	logger Logger
 
 	storages     map[string]*atomic.Value
-	constructors map[string]reflect.Value
+	constructors map[string]Constructor
 
 	onAfterStore []func(name string, value interface{})
 }
@@ -89,7 +86,7 @@ func NewUntypedStore(
 		name:         name,
 		logger:       logger,
 		storages:     make(map[string]*atomic.Value),
-		constructors: make(map[string]reflect.Value),
+		constructors: make(map[string]Constructor),
 		onAfterStore: onAfterStore,
 	}
 
@@ -100,25 +97,9 @@ func NewUntypedStore(
 	return store
 }
 
-func (s *UntypedStore) registerConfig(name string, constructor interface{}) {
-	cType := reflect.TypeOf(constructor)
-
-	if cType.Kind() != reflect.Func {
-		panic("config constructor must be a function")
-	}
-
-	if cType.NumIn() != 1 || cType.In(0) != reflect.TypeOf(&corev1.ConfigMap{}) {
-		panic("config constructor must be of the type func(*k8s.io/api/core/v1/ConfigMap) (..., error)")
-	}
-
-	errorType := reflect.TypeOf((*error)(nil)).Elem()
-
-	if cType.NumOut() != 2 || !cType.Out(1).Implements(errorType) {
-		panic("config constructor must be of the type func(*k8s.io/api/core/v1/ConfigMap) (..., error)")
-	}
-
+func (s *UntypedStore) registerConfig(name string, constructor Constructor) {
 	s.storages[name] = &atomic.Value{}
-	s.constructors[name] = reflect.ValueOf(constructor)
+	s.constructors[name] = constructor
 }
 
 // WatchConfigs uses the provided configmap.Watcher
@@ -148,16 +129,8 @@ func (s *UntypedStore) OnConfigChanged(c *corev1.ConfigMap) {
 	storage := s.storages[name]
 	constructor := s.constructors[name]
 
-	inputs := []reflect.Value{
-		reflect.ValueOf(c),
-	}
-
-	outputs := constructor.Call(inputs)
-	result := outputs[0].Interface()
-	errVal := outputs[1]
-
-	if !errVal.IsNil() {
-		err := errVal.Interface()
+	result, err := constructor(c)
+	if err != nil {
 		if storage.Load() != nil {
 			s.logger.Errorf("Error updating %s config %q: %q", s.name, name, err)
 		} else {

configmap/store_test.go

@@ -32,42 +32,6 @@ import (
 	. "knative.dev/pkg/logging/testing"
 )
 
-func TestStoreBadConstructors(t *testing.T) {
-	tests := []struct {
-		name        string
-		constructor interface{}
-	}{{
-		name:        "not a function",
-		constructor: "i'm pretending to be a function",
-	}, {
-		name:        "no function arguments",
-		constructor: func() (bool, error) { return true, nil },
-	}, {
-		name:        "single argument is not a configmap",
-		constructor: func(bool) (bool, error) { return true, nil },
-	}, {
-		name:        "single return",
-		constructor: func(*corev1.ConfigMap) error { return nil },
-	}, {
-		name:        "wrong second return",
-		constructor: func(*corev1.ConfigMap) (bool, bool) { return true, true },
-	}}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			defer func() {
-				if r := recover(); r == nil {
-					t.Error("expected NewUntypedStore to panic")
-				}
-			}()
-
-			NewUntypedStore("store", nil, Constructors{
-				"test": test.constructor,
-			})
-		})
-	}
-}
-
 func TestStoreWatchConfigs(t *testing.T) {
 	constructor := func(c *corev1.ConfigMap) (interface{}, error) {
 		return c.Name, nil