Skip to content

Commit

Permalink
Do not read from fallback for not found errors (#256)
Browse files Browse the repository at this point in the history
  • Loading branch information
tiffanycheng authored and Qiyao Qin committed Dec 18, 2017
1 parent a5b4ca8 commit 57b0076
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 26 deletions.
8 changes: 8 additions & 0 deletions connectors/cache/fallback.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,10 @@ func (c *Connector) Read(ctx context.Context, ei *dosa.EntityInfo, keys map[stri

return source, sourceErr
}
if dosa.ErrorIsNotFound(sourceErr) {
return source, sourceErr
}

// if source of truth fails, try the fallback. If the fallback fails,
// return the original error
value, err := c.getValueFromFallback(ctx, adaptedEi, cacheKey)
Expand Down Expand Up @@ -188,6 +192,10 @@ func (c *Connector) Range(ctx context.Context, ei *dosa.EntityInfo, columnCondit

return sourceRows, sourceToken, sourceErr
}
if dosa.ErrorIsNotFound(sourceErr) {
return sourceRows, sourceToken, sourceErr
}

value, err := c.getValueFromFallback(ctx, adaptedEi, cacheKey)
c.logFallback("RANGE", ei.Def.Name, err)
if err != nil {
Expand Down
42 changes: 42 additions & 0 deletions connectors/cache/fallback_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ func TestReadCases(t *testing.T) {
createReadSuccessTestCase(),
createReadUncachedEntityTestCase(),
createReadFailTestCase(),
createReadNotFoundTestCase(),
createReadEncodeErrorTestCase(),
createReadDecodeErrorTestCase(),
createReadFallbackFailTestCase(),
Expand Down Expand Up @@ -265,6 +266,22 @@ func createReadFailTestCase() testCase {
}
}

func createReadNotFoundTestCase() testCase {
originResponse := map[string]dosa.FieldValue{"a": "b"}
originErr := &dosa.ErrNotFound{}
return testCase{
encoder: NewJSONEncoder(),
cachedEntities: cacheableEntities,
originRead: &expectArgs{
err: originErr,
resp: originResponse,
},
expectedResp: originResponse,
expectedErr: originErr,
description: "Test that when read origin has err not found, do not read from fallback, return origin response and error",
}
}

func createReadEncodeErrorTestCase() testCase {
originResponse := map[string]dosa.FieldValue{"a": "b"}

Expand Down Expand Up @@ -425,6 +442,7 @@ func TestRangeCases(t *testing.T) {
createRangeSuccessTestCase(),
createRangeUncachedEntityTestCase(),
createRangeFailTestCase(),
createRangeNotFoundTestCase(),
createRangeEncodeErrorTestCase(),
createRangeDecodeErrorTestCase(),
createRangeFallbackFailTestCase(),
Expand Down Expand Up @@ -510,6 +528,30 @@ func createRangeFailTestCase() testCase {
}
}

func createRangeNotFoundTestCase() testCase {
conditions := map[string][]*dosa.Condition{"column": {{Op: dosa.GtOrEq, Value: "columnVal"}}}
rangeResponse := []map[string]dosa.FieldValue{{"a": "b"}}
rangeTok := "nextToken"
rangeErr := &dosa.ErrNotFound{}

return testCase{
encoder: &BadEncoder{},
cachedEntities: cacheableEntities,
originRange: &rangeArgs{
columnConditions: conditions,
token: "token",
limit: 2,
resp: rangeResponse,
nextToken: rangeTok,
err: rangeErr,
},
expectedErr: rangeErr,
expectedManyResp: rangeResponse,
expectedTok: rangeTok,
description: "Test that when origin has err not found, do not read from fallback, return origin response and error",
}
}

func createRangeEncodeErrorTestCase() testCase {
rangeResponse := []map[string]dosa.FieldValue{{"a": "b"}}
rangeTok := "nextToken"
Expand Down
9 changes: 6 additions & 3 deletions connectors/routing/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,17 @@ type Config struct {
}

// FindRouter finds the router information based on scope and namePrefix.
func (c *Config) FindRouter(scope, namePrefix string) *Rule {
func (c *Config) FindRouter(scope, namePrefix string, shouldFail bool) (*Rule, error) {
if shouldFail {
return nil, errors.New("requests failed because they are explicitly set to fail")
}
for _, router := range c.Routers {
if router.RouteTo(scope, namePrefix) {
return router
return router, nil
}
}

return c.findDefaultRouter()
return c.findDefaultRouter(), nil
}

// findDefaultRouter finds the default router information.
Expand Down
11 changes: 7 additions & 4 deletions connectors/routing/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,15 +102,18 @@ routers:
cfg := testCfg.findDefaultRouter()
assert.Equal(t, cfg.Scope, "default")

cfg = testCfg.FindRouter("production", "serviceA")
cfg, _ = testCfg.FindRouter("production", "serviceA", false)
assert.Equal(t, cfg, buildRouter("production", "serviceA", "cassandra"))

cfg = testCfg.FindRouter("ebook", "apple.k")
cfg, _ = testCfg.FindRouter("ebook", "apple.k", false)
assert.Equal(t, cfg, buildRouter("ebook", "apple.*", "ebook"))

cfg = testCfg.FindRouter("ebook", "d.k")
cfg, _ = testCfg.FindRouter("ebook", "d.k", false)
assert.Equal(t, cfg, buildRouter("ebook", "*", "ebook"))

cfg = testCfg.FindRouter("a", "d.k")
cfg, _ = testCfg.FindRouter("a", "d.k", false)
assert.Equal(t, cfg, buildRouter("default", "default", "dosa"))

cfg, err = testCfg.FindRouter("ebook", "d.k", true)
assert.Contains(t, err.Error(), "requests failed because they are explicitly set to fail")
}
17 changes: 8 additions & 9 deletions connectors/routing/connector.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,26 +59,25 @@ func NewConnector(cfg Config, connectorMap map[string]dosa.Connector, plugin Plu

// get connector by scope, namePrefix and operation name provided
func (rc *Connector) getConnector(scope string, namePrefix string, opName string) (_ dosa.Connector, err error) {
var shouldFail bool
if rc.PluginFunc != nil {
// plugin operation
// plugin should always be first considered if it exists
var fallToDefaultConn bool
scope, namePrefix, fallToDefaultConn, err = rc.PluginFunc(scope, namePrefix, opName)
scope, namePrefix, shouldFail, err = rc.PluginFunc(scope, namePrefix, opName)
if err != nil {
return nil, errors.Wrap(err, "failed to execute getConnector due to Plugin function error")
}

if !fallToDefaultConn {
return nil, errors.New("requests failed because they are explicitly set to fail")
}
}
return rc._getConnector(scope, namePrefix)
return rc._getConnector(scope, namePrefix, shouldFail)
}

// if no specific scope is found,
// Connector routes to the default scope that defined in routing config yaml file
func (rc *Connector) _getConnector(scope, namePrefix string) (dosa.Connector, error) {
router := rc.config.FindRouter(scope, namePrefix)
func (rc *Connector) _getConnector(scope, namePrefix string, shouldFail bool) (dosa.Connector, error) {
router, err := rc.config.FindRouter(scope, namePrefix, shouldFail)
if err != nil {
return nil, errors.Wrap(err, "get connector failure")
}

c, ok := rc.connectors[router.Connector]
if !ok {
Expand Down
11 changes: 1 addition & 10 deletions connectors/routing/connector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,18 +202,9 @@ func TestGetConnector(t *testing.T) {
assert.NotNil(t, conn)
assert.Equal(t, reflect.TypeOf(conn), reflect.TypeOf(memory.NewConnector()))

// with plugin
rc.PluginFunc = func(scope, namePrefix, opName string) (string, string, bool, error) {
return "ebook", "ebook-store", true, nil
}

conn, err = rc.getConnector(ei.Ref.Scope, ei.Ref.NamePrefix, "Read")
assert.NoError(t, err)
assert.NotNil(t, conn)

// plugin indicates to throw out error rather than fall into default connector
rc.PluginFunc = func(scope, namePrefix, opName string) (string, string, bool, error) {
return "", "", false, nil
return "", "", true, nil
}
conn, err = rc.getConnector(ei.Ref.Scope, ei.Ref.NamePrefix, "Read")
assert.Contains(t, err.Error(), "requests failed because they are explicitly set to fail")
Expand Down

0 comments on commit 57b0076

Please sign in to comment.