Skip to content

Commit

Permalink
remove variable cache flags (#64)
Browse files Browse the repository at this point in the history
  • Loading branch information
nejisama authored Sep 9, 2022
1 parent 1bde9d0 commit 4d6d910
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 65 deletions.
30 changes: 7 additions & 23 deletions variable/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func getByVariable(ctx context.Context, variable Variable) (interface{}, error)
// 1.2 use variable.Getter() to get value
getter := variable.Getter()
if getter == nil {
return "", errors.New(errGetterNotFound + variable.Name())
return "", errors.New(errValueNotFound + variable.Name())
}
return getter.Get(ctx, nil, variable.Data())
}
Expand All @@ -88,7 +88,7 @@ func getByName(ctx context.Context, name string) (interface{}, error) {
if strings.HasPrefix(name, prefix) {
getter := variable.Getter()
if getter == nil {
return "", errors.New(errGetterNotFound + name)
return "", errors.New(errValueNotFound + name)
}
return getter.Get(ctx, nil, name)
}
Expand Down Expand Up @@ -142,14 +142,9 @@ func getFlushedValue(ctx context.Context, index uint32) (interface{}, error) {
if variables := ctx.Value(mosnctx.KeyVariables); variables != nil {
if values, ok := variables.([]IndexedValue); ok {
value := &values[index]
if value.Valid || value.NotFound {
if !value.noCacheable {
return value.data, nil
}

// clear flags
//value.Valid = false
//value.NotFound = false
if value.Valid {
return value.data, nil

}

return getIndexedValue(ctx, value, index)
Expand All @@ -162,25 +157,18 @@ func getFlushedValue(ctx context.Context, index uint32) (interface{}, error) {
func getIndexedValue(ctx context.Context, value *IndexedValue, index uint32) (interface{}, error) {
variable := indexedVariables[index]

//if value.NotFound || value.Valid {
// return value.data, nil
//}

getter := variable.Getter()
if getter == nil {
return "", errors.New(errGetterNotFound + variable.Name())
return "", errors.New(errValueNotFound + variable.Name())
}
vdata, err := getter.Get(ctx, value, variable.Data())
if err != nil {
value.Valid = false
value.NotFound = true
return vdata, err
}

value.data = vdata
if (variable.Flags() & MOSN_VAR_FLAG_NOCACHEABLE) == MOSN_VAR_FLAG_NOCACHEABLE {
value.noCacheable = true
}
value.Valid = true
return value.data, nil
}

Expand All @@ -189,10 +177,6 @@ func setFlushedValue(ctx context.Context, index uint32, value interface{}) error
if values, ok := variables.([]IndexedValue); ok {
variable := indexedVariables[index]
variableValue := &values[index]
// should check variable.Flags
if (variable.Flags() & MOSN_VAR_FLAG_NOCACHEABLE) == MOSN_VAR_FLAG_NOCACHEABLE {
variableValue.noCacheable = true
}

setter := variable.Setter()
if setter == nil {
Expand Down
53 changes: 31 additions & 22 deletions variable/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,27 +76,6 @@ func TestGetVariableValue_normal(t *testing.T) {
t.Error("Check unknown variable failed")
}

//test variable noCacheable
name = "nocache"
value = "nocache Value"
Register(NewStringVariable(name, nil, func(ctx context.Context, variableValue *IndexedValue, data interface{}) (s string, err error) {
return value, nil
}, DefaultStringSetter, MOSN_VAR_FLAG_NOCACHEABLE))
ctx = NewVariableContext(context.Background())
err = SetString(ctx, name, value)
if err != nil {
t.Error(err)
}

vv, err = GetString(ctx, name)
if err != nil {
t.Error(err)
}

if vv != value {
t.Errorf("get/set nocache variable value not equal, expected: %s, acutal: %s", value, vv)
}

}

func TestSetVariableValue_normal(t *testing.T) {
Expand Down Expand Up @@ -186,7 +165,37 @@ func TestVarNotGetterHint(t *testing.T) {
ctx = NewVariableContext(ctx)

_, err := Get(ctx, name)
assert.Equal(t, err.Error(), errGetterNotFound+name)
assert.Equal(t, err.Error(), errValueNotFound+name)

_, err2 := Get(ctx, name)
assert.Equal(t, err2.Error(), errValueNotFound+name)
}

func TestVariableGetSetCached(t *testing.T) {
name := "cache getter"
cacheValue := "cached"
getterCall := 0
Register(NewStringVariable(name, nil, func(ctx context.Context, variableValue *IndexedValue, data interface{}) (s string, err error) {
getterCall++
return cacheValue, nil
}, DefaultStringSetter, 0))
ctx := NewVariableContext(context.Background())
value, err := GetString(ctx, name)
assert.Nil(t, err)
assert.Equal(t, cacheValue, value)
assert.Equal(t, 1, getterCall)
// get from cache
value, err = GetString(ctx, name)
assert.Nil(t, err)
assert.Equal(t, cacheValue, value)
assert.Equal(t, 1, getterCall)
// set will overwrite cache
SetString(ctx, name, "overwrite")
value, err = GetString(ctx, name)
assert.Nil(t, err)
assert.Equal(t, "overwrite", value)
assert.Equal(t, 1, getterCall)

}

func BenchmarkGetVariableValue2(b *testing.B) {
Expand Down
2 changes: 1 addition & 1 deletion variable/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ var (
errInvalidContext = "invalid context"
errNoVariablesInContext = "no variables found in context"
errSupportIndexedOnly = "this operation only support indexed variable"
errGetterNotFound = "getter function undefined, variable name: "
errSetterNotFound = "setter function undefined, variable name: "
errValueNotFound = "variable value not found, variable name: "
errVariableNotString = "variable type is not string"
errValueNotString = "set string variable with non-string type"
invalidVariableIndex = errors.New("get variable support name index or variable directly")
Expand Down
15 changes: 2 additions & 13 deletions variable/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,11 @@ package variable
import "context"

const (
MOSN_VAR_FLAG_CHANGEABLE = 1
MOSN_VAR_FLAG_NOCACHEABLE = 2
MOSN_VAR_FLAG_NOHASH = 4

ValueNotFound = "-"
)

// StringGetterFunc used to get the value of string-typed variable, the implementation should handle the field
// (Valid, NotFound) of IndexedValue if it was not nil, Valid means the value is valid; NotFound
// means the value can not be found. It indicates that value can be cached for next-time get handle
// if any one of (Valid, NotFound) is set to true.
// Valid of IndexedValue if it was not nil, Valid means the value is valid.
//
// Function should return ValueNotFound("-") if target value not exists.
// E.g. reference to the header which is not existed in current request.
Expand Down Expand Up @@ -61,8 +55,6 @@ type Variable interface {
Name() string
// variable data, which is useful for getter/setter
Data() interface{}
// variable flags
Flags() uint32
// value getter
Getter() Getter
// value setter
Expand All @@ -71,10 +63,7 @@ type Variable interface {

// IndexedValue used to store result value
type IndexedValue struct {
Valid bool
NotFound bool
noCacheable bool
//escape bool
Valid bool

data interface{}
}
Expand Down
7 changes: 1 addition & 6 deletions variable/var.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,6 @@ func (bv *BasicVariable) Data() interface{} {
return bv.data
}

// Flags returns variable's cache flag
func (bv *BasicVariable) Flags() uint32 {
return bv.flags
}

// Getter is the variable's value get function if the variable contains it
func (bv *BasicVariable) Getter() Getter {
return bv.getter
Expand Down Expand Up @@ -158,5 +153,5 @@ func (g *getterImpl) Get(ctx context.Context, value *IndexedValue, data interfac
return g.getter(ctx, value, data)
}

return nil, errors.New(errGetterNotFound + g.name)
return nil, errors.New(errValueNotFound + g.name)
}

0 comments on commit 4d6d910

Please sign in to comment.