Rationalize the Evaluator interface. (istio#1775)

Automatic merge from submit-queue.

Remove the TypeChecker interface from Evaluator

+ Also, the PredicateEvaluator interface is subsumed to the Evaluator interface.

**What this PR does / why we need it**:
This is a sanity-refactoring that teases out responsibilities for the interfaces. With this CL, the exact nature of dependencies to the expression language is clearly demarcated.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```

Author: ozevren

mixer/cmd/server/cmd/server.go

@@ -269,7 +269,7 @@ func setupServer(sa *serverArgs, info map[string]template.Info, adapters []adptr
 	if err != nil {
 		fatalf("Failed to connect to the configuration server. %v", err)
 	}
-	dispatcher, err = mixerRuntime.New(eval, gp, adapterGP,
+	dispatcher, err = mixerRuntime.New(eval, evaluator.NewTypeChecker(), gp, adapterGP,
 		sa.configIdentityAttribute, sa.configDefaultNamespace,
 		store2, adapterMap, info,
 	)
@@ -281,13 +281,13 @@ func setupServer(sa *serverArgs, info map[string]template.Info, adapters []adptr
 	repo := template.NewRepository(info)
 	store := configStore(sa.configStoreURL, sa.serviceConfigFile, sa.globalConfigFile, printf, fatalf)
 	adapterMgr := adapterManager.NewManager(legacyAdapters, aspect.Inventory(), evalForLegacy, gp, adapterGP)
-	configManager := config.NewManager(evalForLegacy, adapterMgr.AspectValidatorFinder, adapterMgr.BuilderValidatorFinder, adapters,
+	configManager := config.NewManager(evalForLegacy, evaluator.NewTypeChecker(), adapterMgr.AspectValidatorFinder, adapterMgr.BuilderValidatorFinder, adapters,
 		adapterMgr.SupportedKinds,
 		repo, store, time.Second*time.Duration(sa.configFetchIntervalSec),
 		sa.configIdentityAttribute,
 		sa.configIdentityAttributeDomain)
 
-	configAPIServer := config.NewAPI("v1", sa.configAPIPort, evalForLegacy,
+	configAPIServer := config.NewAPI("v1", sa.configAPIPort, evaluator.NewTypeChecker(),
 		adapterMgr.AspectValidatorFinder, adapterMgr.BuilderValidatorFinder, adapters,
 		adapterMgr.SupportedKinds, store, repo)
 

mixer/pkg/aspect/test/evaluator.go

@@ -25,7 +25,7 @@ import (
 type EvalBody func(string, attribute.Bag) (interface{}, error)
 
 type fakeEval struct {
-	expr.PredicateEvaluator
+	expr.Evaluator
 	expr.TypeChecker
 
 	body EvalBody

mixer/pkg/config/manager.go

@@ -85,7 +85,7 @@ type Manager struct {
 // as command line input parameters.
 // GlobalConfig specifies the location of Global Config.
 // ServiceConfig specifies the location of Service config.
-func NewManager(eval expr.Evaluator, aspectFinder AspectValidatorFinder, builderFinder BuilderValidatorFinder,
+func NewManager(eval expr.Evaluator, typeChecker expr.TypeChecker, aspectFinder AspectValidatorFinder, builderFinder BuilderValidatorFinder,
 	getBuilderInfoFns []adapter.InfoFn, findAspects AdapterToAspectMapper, repository template.Repository,
 	store store.KeyValueStore, loopDelay time.Duration, identityAttribute string,
 	identityAttributeDomain string) *Manager {
@@ -100,7 +100,7 @@ func NewManager(eval expr.Evaluator, aspectFinder AspectValidatorFinder, builder
 		identityAttributeDomain: identityAttributeDomain,
 		validate: func(cfg map[string]string) (*Validated, descriptor.Finder, *adapter.ConfigErrors) {
 			r := newRegistry2(getBuilderInfoFns, repository.SupportsTemplate)
-			v := newValidator(aspectFinder, builderFinder, r.FindAdapterInfo, SetupHandlers, repository, findAspects, true, eval)
+			v := newValidator(aspectFinder, builderFinder, r.FindAdapterInfo, SetupHandlers, repository, findAspects, true, typeChecker)
 			rt, ce := v.validate(cfg)
 			return rt, v.descriptorFinder, ce
 		},

mixer/pkg/config/manager_test.go

@@ -90,7 +90,7 @@ func TestConfigManager(t *testing.T) {
 			if mt.errStr != "" {
 				store.err = errors.New(mt.errStr)
 			}
-			ma := NewManager(evaluator, vf.FindAspectValidator, vf.FindAdapterValidator, nil, vf.AdapterToAspectMapperFunc,
+			ma := NewManager(evaluator, evaluator, vf.FindAspectValidator, vf.FindAdapterValidator, nil, vf.AdapterToAspectMapperFunc,
 				template.NewRepository(nil), store, loopDelay, keyTargetService, keyServiceDomain)
 			testConfigManager(t, ma, mt)
 		})
@@ -100,7 +100,7 @@ func TestConfigManager(t *testing.T) {
 func TestManager_FetchError(t *testing.T) {
 	errStr := "TestManager_FetchError"
 	store := newFakeStore("{}", "{}")
-	mgr := NewManager(nil, nil, nil, nil, nil,
+	mgr := NewManager(nil, nil,nil, nil, nil, nil,
 		template.NewRepository(nil), store, loopDelay, keyTargetService, keyServiceDomain)
 
 	mgr.validate = func(cfg map[string]string) (rt *Validated, desc descriptor.Finder, ce *adapter.ConfigErrors) {

mixer/pkg/config/runtime.go

@@ -33,7 +33,7 @@ type (
 	runtime struct {
 		Validated
 		// used to evaluate selectors
-		eval expr.PredicateEvaluator
+		eval expr.Evaluator
 
 		// config is organized around the identityAttribute.
 		identityAttribute       string
@@ -42,7 +42,7 @@ type (
 )
 
 // newRuntime returns a runtime object given a validated config and a predicate eval.
-func newRuntime(v *Validated, evaluator expr.PredicateEvaluator, identityAttribute, identityAttributeDomain string) *runtime {
+func newRuntime(v *Validated, evaluator expr.Evaluator, identityAttribute, identityAttributeDomain string) *runtime {
 	return &runtime{
 		Validated:               *v,
 		eval:                    evaluator,

mixer/pkg/config/runtime_test.go

@@ -35,6 +35,10 @@ type trueEval struct {
 	ret    bool
 }
 
+func (t *trueEval) Eval(expression string, attrs attribute.Bag) (interface{}, error) {
+	return t.EvalPredicate(expression, attrs)
+}
+
 func (t *trueEval) EvalPredicate(expression string, attrs attribute.Bag) (bool, error) {
 	if t.ncalls == 0 {
 		return t.ret, t.err

mixer/pkg/expr/evaluator.go

@@ -26,13 +26,6 @@ type (
 		// Eval evaluates given expression using the attribute bag
 		Eval(expr string, attrs attribute.Bag) (interface{}, error)
 
-		PredicateEvaluator
-
-		TypeChecker
-	}
-
-	// PredicateEvaluator evaluates a predicate to true or false
-	PredicateEvaluator interface {
 		// EvalPredicate evaluates given predicate using the attribute bag
 		EvalPredicate(expr string, attrs attribute.Bag) (bool, error)
 	}

mixer/pkg/runtime/controller.go

@@ -40,7 +40,8 @@ type Controller struct {
 	// Static information
 	adapterInfo            map[string]*adapter.Info // maps adapter shortName to Info.
 	templateInfo           map[string]template.Info // maps template name to Info.
-	eval                   expr.Evaluator           // Used to infer types. Used by resolver and dispatcher.
+	evaluator              expr.Evaluator           // used by resolver
+	typeChecker            expr.TypeChecker         // used to infer types
 	identityAttribute      string                   // used by resolver
 	defaultConfigNamespace string                   // used by resolver
 
@@ -114,7 +115,7 @@ func (c *Controller) publishSnapShot() {
 	// attribute manifests are used by type inference during handler creation.
 	attributes := c.processAttributeManifests()
 
-	if cl, ok := c.eval.(VocabularyChangeListener); ok {
+	if cl, ok := c.evaluator.(VocabularyChangeListener); ok {
 		cl.ChangeVocabulary(attributes)
 	}
 
@@ -127,7 +128,7 @@ func (c *Controller) publishSnapShot() {
 	instanceConfig := c.validInstanceConfigs()
 
 	// new handler factory is created for every config change.
-	hb := c.createHandlerFactory(c.templateInfo, c.eval, attributes, c.adapterInfo)
+	hb := c.createHandlerFactory(c.templateInfo, c.typeChecker, attributes, c.adapterInfo)
 
 	// new handler table is created for every config change. It uses handler factory
 	// to create new handlers.
@@ -151,7 +152,7 @@ func (c *Controller) publishSnapShot() {
 
 	// Create new resolver and cleanup the old resolver.
 	c.nextResolverID++
-	resolver := newResolver(c.eval, c.identityAttribute, c.defaultConfigNamespace, resolvedRules, c.nextResolverID)
+	resolver := newResolver(c.evaluator, c.identityAttribute, c.defaultConfigNamespace, resolvedRules, c.nextResolverID)
 	c.dispatcher.ChangeResolver(resolver)
 
 	// copy old for deletion.

mixer/pkg/runtime/controller_test.go

@@ -46,7 +46,8 @@ func TestControllerEmpty(t *testing.T) {
 	c := &Controller{
 		adapterInfo:            make(map[string]*adapter.Info),
 		templateInfo:           make(map[string]template.Info),
-		eval:                   nil,
+		evaluator:              nil,
+		typeChecker:            nil,
 		configState:            make(map[store.Key]*store.Resource),
 		dispatcher:             d,
 		resolver:               &resolver{}, // get an empty resolver
@@ -206,7 +207,8 @@ func TestController_workflow(t *testing.T) {
 	c := &Controller{
 		adapterInfo:            adapterInfo,
 		templateInfo:           templateInfo,
-		eval:                   nil,
+		evaluator:              nil,
+		typeChecker:            nil,
 		configState:            configState,
 		dispatcher:             d,
 		resolver:               res, // get an empty resolver

mixer/pkg/runtime/init.go

@@ -34,13 +34,13 @@ import (
 // New creates a new runtime Dispatcher
 // Create a new controller and a dispatcher.
 // Returns a ready to use dispatcher.
-func New(eval expr.Evaluator, gp *pool.GoroutinePool, handlerPool *pool.GoroutinePool,
+func New(eval expr.Evaluator, typeChecker expr.TypeChecker, gp *pool.GoroutinePool, handlerPool *pool.GoroutinePool,
 	identityAttribute string, defaultConfigNamespace string,
 	s store.Store2, adapterInfo map[string]*adapter.Info,
 	templateInfo map[string]template.Info) (Dispatcher, error) {
 	// controller will set Resolver before the dispatcher is used.
 	d := newDispatcher(eval, nil, gp, identityAttribute)
-	err := startController(s, adapterInfo, templateInfo, eval, d,
+	err := startController(s, adapterInfo, templateInfo, eval, typeChecker, d,
 		identityAttribute, defaultConfigNamespace, handlerPool)
 
 	return d, err
@@ -86,7 +86,7 @@ func kindMap(adapterInfo map[string]*adapter.Info,
 
 // startController creates a controller from the given params.
 func startController(s store.Store2, adapterInfo map[string]*adapter.Info,
-	templateInfo map[string]template.Info, eval expr.Evaluator,
+	templateInfo map[string]template.Info, eval expr.Evaluator, checker expr.TypeChecker,
 	dispatcher ResolverChangeListener,
 	identityAttribute string, defaultConfigNamespace string, handlerPool *pool.GoroutinePool) error {
 
@@ -98,7 +98,8 @@ func startController(s store.Store2, adapterInfo map[string]*adapter.Info,
 	c := &Controller{
 		adapterInfo:            adapterInfo,
 		templateInfo:           templateInfo,
-		eval:                   eval,
+		evaluator:              eval,
+		typeChecker:            checker,
 		configState:            data,
 		dispatcher:             dispatcher,
 		resolver:               &resolver{}, // get an empty resolver

mixer/pkg/runtime/resolver.go

@@ -51,7 +51,7 @@ func (r Rule) String() string {
 // resolver is the runtime view of the configuration database.
 type resolver struct {
 	// evaluator evaluates selectors
-	evaluator expr.PredicateEvaluator
+	evaluator expr.Evaluator
 
 	// identityAttribute defines which configuration scopes apply to a request.
 	// default: target.service
@@ -75,7 +75,7 @@ type resolver struct {
 }
 
 // newResolver returns a Resolver.
-func newResolver(evaluator expr.PredicateEvaluator, identityAttribute string, defaultConfigNamespace string,
+func newResolver(evaluator expr.Evaluator, identityAttribute string, defaultConfigNamespace string,
 	rules map[string][]*Rule, id int) *resolver {
 	return &resolver{
 		evaluator:              evaluator,

mixer/pkg/runtime/resolver_test.go

@@ -185,6 +185,10 @@ func fakePred(reject bool, err string) *fakePredEval {
 	return f
 }
 
+func (f *fakePredEval) Eval(expr string, bag attribute.Bag) (interface{}, error) {
+	return f.EvalPredicate(expr, bag)
+}
+
 func (f *fakePredEval) EvalPredicate(expr string, _ attribute.Bag) (bool, error) {
 	return !f.reject, f.err
 }