diff --git a/connectors/cache/fallback_test.go b/connectors/cache/fallback_test.go index 6ca1b862..f9a42b85 100644 --- a/connectors/cache/fallback_test.go +++ b/connectors/cache/fallback_test.go @@ -273,7 +273,7 @@ func createReadNotFoundTestCase() testCase { encoder: NewJSONEncoder(), cachedEntities: cacheableEntities, originRead: &expectArgs{ - err: originErr, + err: originErr, resp: originResponse, }, expectedResp: originResponse, diff --git a/connectors/routing/connector.go b/connectors/routing/connector.go index 54b3360e..2e44c999 100644 --- a/connectors/routing/connector.go +++ b/connectors/routing/connector.go @@ -183,7 +183,7 @@ func (rc *Connector) Scan(ctx context.Context, ei *dosa.EntityInfo, minimumField func (rc *Connector) CheckSchema(ctx context.Context, scope, namePrefix string, ed []*dosa.EntityDefinition) (int32, error) { connector, err := rc.getConnector(scope, namePrefix, "CheckSchema") if err != nil { - return dosa.InvalidVersion, base.ErrNoMoreConnector{} + return dosa.InvalidVersion, err } return connector.CheckSchema(ctx, scope, namePrefix, ed) } @@ -192,7 +192,7 @@ func (rc *Connector) CheckSchema(ctx context.Context, scope, namePrefix string, func (rc *Connector) UpsertSchema(ctx context.Context, scope, namePrefix string, ed []*dosa.EntityDefinition) (*dosa.SchemaStatus, error) { connector, err := rc.getConnector(scope, namePrefix, "UpsertSchema") if err != nil { - return nil, base.ErrNoMoreConnector{} + return nil, err } return connector.UpsertSchema(ctx, scope, namePrefix, ed) } @@ -201,7 +201,7 @@ func (rc *Connector) UpsertSchema(ctx context.Context, scope, namePrefix string, func (rc *Connector) CheckSchemaStatus(ctx context.Context, scope string, namePrefix string, version int32) (*dosa.SchemaStatus, error) { connector, err := rc.getConnector(scope, namePrefix, "CheckSchemaStatus") if err != nil { - return nil, base.ErrNoMoreConnector{} + return nil, err } return connector.CheckSchemaStatus(ctx, scope, namePrefix, version) } @@ -211,7 +211,7 @@ func (rc *Connector) CreateScope(ctx context.Context, scope string) error { // will fall to default connector connector, err := rc.getConnector(scope, "", "CreateScope") if err != nil { - return base.ErrNoMoreConnector{} + return err } return connector.CreateScope(ctx, scope) } @@ -221,7 +221,7 @@ func (rc *Connector) TruncateScope(ctx context.Context, scope string) error { // will fall to default connector connector, err := rc.getConnector(scope, "", "TruncateScope") if err != nil { - return base.ErrNoMoreConnector{} + return err } return connector.TruncateScope(ctx, scope) } @@ -231,7 +231,7 @@ func (rc *Connector) DropScope(ctx context.Context, scope string) error { // will fall to default connector connector, err := rc.getConnector(scope, "", "DropScope") if err != nil { - return base.ErrNoMoreConnector{} + return err } return connector.DropScope(ctx, scope) } @@ -241,7 +241,7 @@ func (rc *Connector) ScopeExists(ctx context.Context, scope string) (bool, error // will fall to default connector connector, err := rc.getConnector(scope, "", "ScopeExists") if err != nil { - return false, base.ErrNoMoreConnector{} + return false, err } return connector.ScopeExists(ctx, scope) } diff --git a/connectors/yarpc/yarpc.go b/connectors/yarpc/yarpc.go index 0bcbbffc..065951c5 100644 --- a/connectors/yarpc/yarpc.go +++ b/connectors/yarpc/yarpc.go @@ -228,8 +228,12 @@ func (c *Connector) CreateIfNotExists(ctx context.Context, ei *dosa.EntityInfo, return errors.Wrap(&dosa.ErrAlreadyExists{}, "failed to create") } } + + if !dosarpc.Dosa_CreateIfNotExists_Helper.IsException(err) { + return errors.Wrap(err, "failed to CreateIfNotExists due to network issue") + } } - return errors.Wrap(err, "failed to create") + return errors.Wrap(err, "failed to CreateIfNotExists") } // Upsert inserts or updates your data @@ -242,7 +246,14 @@ func (c *Connector) Upsert(ctx context.Context, ei *dosa.EntityInfo, values map[ Ref: entityInfoToSchemaRef(ei), EntityValues: ev, } - return c.Client.Upsert(ctx, &upsertRequest, VersionHeader()) + + err = c.Client.Upsert(ctx, &upsertRequest, VersionHeader()) + + if !dosarpc.Dosa_Upsert_Helper.IsException(err) { + return errors.Wrap(err, "failed to Upsert due to network issue") + } + + return errors.Wrap(err, "failed to Upsert") } // Read reads a single entity @@ -284,7 +295,12 @@ func (c *Connector) Read(ctx context.Context, ei *dosa.EntityInfo, keys map[stri return nil, errors.Wrap(&dosa.ErrNotFound{}, "Read failed: not found") } } - return nil, errors.Wrap(err, "Read failed") + + if !dosarpc.Dosa_Read_Helper.IsException(err) { + return nil, errors.Wrap(err, "failed to Read due to network issue") + } + + return nil, errors.Wrap(err, "failed to Read") } // no error, so for each column, transform it into the map of (col->value) items @@ -323,7 +339,11 @@ func (c *Connector) MultiRead(ctx context.Context, ei *dosa.EntityInfo, keys []m response, err := c.Client.MultiRead(ctx, request, VersionHeader()) if err != nil { - return nil, errors.Wrap(err, "MultiRead failed") + if !dosarpc.Dosa_MultiRead_Helper.IsException(err) { + return nil, errors.Wrap(err, "failed to MultiRead due to network issue") + } + + return nil, errors.Wrap(err, "failed to MultiRead") } rpcResults := response.Results @@ -376,7 +396,11 @@ func (c *Connector) Remove(ctx context.Context, ei *dosa.EntityInfo, keys map[st err := c.Client.Remove(ctx, removeRequest, VersionHeader()) if err != nil { - return errors.Wrap(err, "Remove failed") + if !dosarpc.Dosa_Remove_Helper.IsException(err) { + return errors.Wrap(err, "failed to Remove due to network issue") + } + + return errors.Wrap(err, "failed to Remove") } return nil } @@ -394,7 +418,10 @@ func (c *Connector) RemoveRange(ctx context.Context, ei *dosa.EntityInfo, column } if err := c.Client.RemoveRange(ctx, request, VersionHeader()); err != nil { - return errors.Wrap(err, "RemoveRange failed") + if !dosarpc.Dosa_RemoveRange_Helper.IsException(err) { + return errors.Wrap(err, "failed to RemoveRange due to network issue") + } + return errors.Wrap(err, "failed to RemoveRange") } return nil } @@ -410,7 +437,7 @@ func (c *Connector) Range(ctx context.Context, ei *dosa.EntityInfo, columnCondit rpcMinimumFields := makeRPCminimumFields(minimumFields) rpcConditions, err := createRPCConditions(columnConditions) if err != nil { - return nil, "", errors.Wrap(err, "Range failed: invalid column conditions") + return nil, "", errors.Wrap(err, "failed to Range: invalid column conditions") } rangeRequest := dosarpc.RangeRequest{ Ref: entityInfoToSchemaRef(ei), @@ -421,7 +448,11 @@ func (c *Connector) Range(ctx context.Context, ei *dosa.EntityInfo, columnCondit } response, err := c.Client.Range(ctx, &rangeRequest, VersionHeader()) if err != nil { - return nil, "", errors.Wrap(err, "Range failed") + if !dosarpc.Dosa_Range_Helper.IsException(err) { + return nil, "", errors.Wrap(err, "failed to Range due to network issue") + } + + return nil, "", errors.Wrap(err, "failed to Range") } results := []map[string]dosa.FieldValue{} for _, entity := range response.Entities { @@ -466,7 +497,11 @@ func (c *Connector) Scan(ctx context.Context, ei *dosa.EntityInfo, minimumFields } response, err := c.Client.Scan(ctx, &scanRequest, VersionHeader()) if err != nil { - return nil, "", errors.Wrap(err, "Scan failed") + if !dosarpc.Dosa_Scan_Helper.IsException(err) { + return nil, "", errors.Wrap(err, "failed to Scan due to network issue") + } + + return nil, "", errors.Wrap(err, "failed to Scan") } results := []map[string]dosa.FieldValue{} for _, entity := range response.Entities { @@ -487,7 +522,11 @@ func (c *Connector) CheckSchema(ctx context.Context, scope, namePrefix string, e } response, err := c.Client.CheckSchema(ctx, &csr, VersionHeader()) if err != nil { - return dosa.InvalidVersion, wrapError(err, "CheckSchema failed", scope, c.Config.ServiceName) + if !dosarpc.Dosa_CheckSchema_Helper.IsException(err) { + return dosa.InvalidVersion, errors.Wrap(err, "failed to CheckSchema due to network issue") + } + + return dosa.InvalidVersion, wrapError(err, "failed to CheckSchema", scope, c.Config.ServiceName) } return *response.Version, nil @@ -506,7 +545,10 @@ func (c *Connector) CanUpsertSchema(ctx context.Context, scope, namePrefix strin } response, err := c.Client.CanUpsertSchema(ctx, &csr, VersionHeader()) if err != nil { - return dosa.InvalidVersion, wrapError(err, "Check schema compatibility failed", scope, c.Config.ServiceName) + if !dosarpc.Dosa_CanUpsertSchema_Helper.IsException(err) { + return dosa.InvalidVersion, errors.Wrap(err, "failed to CanUpsertSchema due to network issue") + } + return dosa.InvalidVersion, wrapError(err, "failed to CanUpsertSchema", scope, c.Config.ServiceName) } return *response.Version, nil @@ -523,7 +565,10 @@ func (c *Connector) UpsertSchema(ctx context.Context, scope, namePrefix string, response, err := c.Client.UpsertSchema(ctx, request, VersionHeader()) if err != nil { - return nil, wrapError(err, "UpsertSchema failed", scope, c.Config.ServiceName) + if !dosarpc.Dosa_UpsertSchema_Helper.IsException(err) { + return nil, errors.Wrap(err, "failed to UpsertSchema due to network issue") + } + return nil, wrapError(err, "failed to UpsertSchema", scope, c.Config.ServiceName) } status := "" @@ -532,7 +577,7 @@ func (c *Connector) UpsertSchema(ctx context.Context, scope, namePrefix string, } if response.Version == nil { - return nil, errors.New("UpsertSchema failed: server returns version nil") + return nil, errors.New("failed to UpsertSchema: server returns version nil") } return &dosa.SchemaStatus{ @@ -547,7 +592,11 @@ func (c *Connector) CheckSchemaStatus(ctx context.Context, scope, namePrefix str response, err := c.Client.CheckSchemaStatus(ctx, &request, VersionHeader()) if err != nil { - return nil, wrapError(err, "ChecksShemaStatus failed", scope, c.Config.ServiceName) + if !dosarpc.Dosa_CheckSchemaStatus_Helper.IsException(err) { + return nil, errors.Wrap(err, "failed to CheckSchemaStatus due to network issue") + } + + return nil, wrapError(err, "failed to CheckSchemaStatus", scope, c.Config.ServiceName) } status := "" @@ -556,7 +605,7 @@ func (c *Connector) CheckSchemaStatus(ctx context.Context, scope, namePrefix str } if response.Version == nil { - return nil, errors.New("ChecksShemaStatus failed: server returns version nil") + return nil, errors.New("failed to ChecksShemaStatus: server returns version nil") } return &dosa.SchemaStatus{ @@ -572,7 +621,11 @@ func (c *Connector) CreateScope(ctx context.Context, scope string) error { } if err := c.Client.CreateScope(ctx, request, VersionHeader()); err != nil { - return errors.Wrap(err, "CreateScope failed") + if !dosarpc.Dosa_CreateScope_Helper.IsException(err) { + return errors.Wrap(err, "failed to CreateScope due to network issue") + } + + return errors.Wrap(err, "failed to CreateScope") } return nil @@ -585,7 +638,11 @@ func (c *Connector) TruncateScope(ctx context.Context, scope string) error { } if err := c.Client.TruncateScope(ctx, request, VersionHeader()); err != nil { - return errors.Wrap(err, "TruncateScope failed") + if !dosarpc.Dosa_TruncateScope_Helper.IsException(err) { + return errors.Wrap(err, "failed to TruncateScope due to network issue") + } + + return errors.Wrap(err, "failed to TruncateScope") } return nil @@ -598,7 +655,11 @@ func (c *Connector) DropScope(ctx context.Context, scope string) error { } if err := c.Client.DropScope(ctx, request, VersionHeader()); err != nil { - return errors.Wrap(err, "DropScope failed") + if !dosarpc.Dosa_DropScope_Helper.IsException(err) { + return errors.Wrap(err, "failed to DropScope due to network issue") + } + + return errors.Wrap(err, "failed to DropScope") } return nil diff --git a/entity_parser.go b/entity_parser.go index 975ac3db..20a712de 100644 --- a/entity_parser.go +++ b/entity_parser.go @@ -100,6 +100,10 @@ func parsePartitionKey(pkStr string) []string { // parsePrimaryKey func parses the primary key of DOSA object func parsePrimaryKey(tableName, pkStr string) (*PrimaryKey, error) { + // parens must be matched + if !parensBalanced(pkStr) { + return nil, fmt.Errorf("unmatched parentheses: %q", pkStr) + } // filter out "trailing comma and space" pkStr = strings.TrimRight(pkStr, ", ") pkStr = strings.TrimSpace(pkStr) @@ -384,6 +388,26 @@ func parseField(typ Type, isPointer bool, name string, tag string) (*ColumnDefin return &ColumnDefinition{Name: name, IsPointer: isPointer, Type: typ}, nil } +func parensBalanced(s string) bool { + // This is effectively pushing left parens on the stack, and popping them when + // a right paren is seen. Since the stack only ever contains the same character, + // we don't actually need the stack -- only its size. + var ssize uint + for i := 0; i < len(s); i++ { + if s[i] == '(' { + ssize++ + } else if s[i] == ')' { + if ssize == 0 { + // Extra right paren + return false + } + ssize-- + } + } + // Stack must be empty + return ssize == 0 +} + var ( uuidType = reflect.TypeOf(UUID("")) blobType = reflect.TypeOf([]byte{}) diff --git a/entity_parser_test.go b/entity_parser_test.go index 0effd9af..9f3f562e 100644 --- a/entity_parser_test.go +++ b/entity_parser_test.go @@ -492,6 +492,35 @@ func TestInvalidFieldInTag(t *testing.T) { assert.Contains(t, err.Error(), "invalid") } +func TestInvalidSyntaxInTag(t *testing.T) { + type HasInvalidTagSyntax struct { + Entity `dosa:"primaryKey=((Val, Key), TS DESC"` + Val string + Key string + TS time.Time + } + table, err := TableFromInstance(&HasInvalidTagSyntax{}) + assert.Nil(t, table) + assert.Error(t, err) + assert.Contains(t, err.Error(), "unmatched parentheses") +} + +func TestParensBalanced(t *testing.T) { + assert.True(t, parensBalanced("()")) + assert.True(t, parensBalanced("()()")) + assert.True(t, parensBalanced("(()())")) + assert.True(t, parensBalanced("()")) + assert.True(t, parensBalanced("")) + assert.True(t, parensBalanced("()(()(()(()()))())()()")) + + assert.False(t, parensBalanced("(")) + assert.False(t, parensBalanced(")")) + assert.False(t, parensBalanced("(()")) + assert.False(t, parensBalanced(")(")) + assert.False(t, parensBalanced("(()))")) + assert.False(t, parensBalanced("((()())")) +} + /* These tests do not currently pass, but I think they should */ diff --git a/finder_test.go b/finder_test.go index 76a9e42b..60d66fae 100644 --- a/finder_test.go +++ b/finder_test.go @@ -80,7 +80,7 @@ func TestParser(t *testing.T) { assert.Equal(t, len(expectedEntities)+len(entitiesExcludedForTest), len(entities), fmt.Sprintf("%s", entities)) // TODO(jzhan): remove the hard-coded number of errors. - assert.Equal(t, 21, len(errs), fmt.Sprintf("%v", errs)) + assert.Equal(t, 22, len(errs), fmt.Sprintf("%v", errs)) assert.Nil(t, err) for _, entity := range entities { diff --git a/mocks/client.go b/mocks/client.go index 3bd25f2e..55e3f2f5 100644 --- a/mocks/client.go +++ b/mocks/client.go @@ -26,9 +26,10 @@ package mocks import ( context "context" + reflect "reflect" + gomock "github.com/golang/mock/gomock" dosa "github.com/uber-go/dosa" - reflect "reflect" ) // MockClient is a mock of Client interface diff --git a/mocks/connector.go b/mocks/connector.go index b873228e..f1ee7e00 100644 --- a/mocks/connector.go +++ b/mocks/connector.go @@ -26,9 +26,10 @@ package mocks import ( context "context" + reflect "reflect" + gomock "github.com/golang/mock/gomock" dosa "github.com/uber-go/dosa" - reflect "reflect" ) // MockConnector is a mock of Connector interface