Skip to content

Commit

Permalink
Implement requested changes
Browse files Browse the repository at this point in the history
  • Loading branch information
Hajime Hayano committed Aug 4, 2017
1 parent 61b77c6 commit 623de36
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 117 deletions.
49 changes: 14 additions & 35 deletions service/dynamodb/expression/condition.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,12 +245,6 @@ func (cond ConditionBuilder) buildCondition() (ExprNode, error) {
// ConditionBuilders. There will first be checks to make sure that the input
// ConditionBuilder has the correct format.
func compareBuildCondition(c ConditionBuilder) (ExprNode, error) {
if len(c.operandList) != 2 {
return ExprNode{}, fmt.Errorf("betweenBuildCondition error: unexpected number of operands, expected 2 operands")
}
if len(c.conditionList) != 0 {
return ExprNode{}, fmt.Errorf("betweenBuildCondition error: unexpected number of conditions, expected 0 conditions")
}
childNodes, err := c.buildChildNodes()
if err != nil {
return ExprNode{}, err
Expand All @@ -272,12 +266,6 @@ func compareBuildCondition(c ConditionBuilder) (ExprNode, error) {
// ConditionBuilders. There will first be checks to make sure that the input
// ConditionBuilder has the correct format.
func compoundBuildCondition(c ConditionBuilder) (ExprNode, error) {
if len(c.operandList) != 0 {
return ExprNode{}, fmt.Errorf("betweenBuildCondition error: unexpected number of operands, expected 0 operands")
}
if len(c.conditionList) < 2 {
return ExprNode{}, fmt.Errorf("betweenBuildCondition error: unexpected number of conditions, expected at least 2 conditions")
}
childNodes, err := c.buildChildNodes()
if err != nil {
return ExprNode{}, err
Expand All @@ -303,13 +291,6 @@ func compoundBuildCondition(c ConditionBuilder) (ExprNode, error) {
// ConditionBuilders. There will first be checks to make sure that the input
// ConditionBuilder has the correct format.
func betweenBuildCondition(c ConditionBuilder) (ExprNode, error) {
if len(c.operandList) != 3 {
return ExprNode{}, fmt.Errorf("betweenBuildCondition error: unexpected number of operands, expected 3 operands")
}
if len(c.conditionList) != 0 {
return ExprNode{}, fmt.Errorf("betweenBuildCondition error: unexpected number of conditions, expected 0 conditions")
}

childNodes, err := c.buildChildNodes()
if err != nil {
return ExprNode{}, err
Expand All @@ -328,24 +309,22 @@ func betweenBuildCondition(c ConditionBuilder) (ExprNode, error) {
// duplication of code amongst the various buildConditions.
func (cond ConditionBuilder) buildChildNodes() ([]ExprNode, error) {
var childNodes []ExprNode
if len(cond.operandList) == 0 {
childNodes = make([]ExprNode, 0, len(cond.conditionList))
for _, condition := range cond.conditionList {
en, err := condition.buildCondition()
if err != nil {
return []ExprNode{}, err
}
childNodes = append(childNodes, en)

childNodes = make([]ExprNode, 0, len(cond.conditionList)+len(cond.operandList))
for _, condition := range cond.conditionList {
en, err := condition.buildCondition()
if err != nil {
return []ExprNode{}, err
}
} else if len(cond.conditionList) == 0 {
childNodes = make([]ExprNode, 0, len(cond.operandList))
for _, ope := range cond.operandList {
en, err := ope.BuildOperand()
if err != nil {
return []ExprNode{}, err
}
childNodes = append(childNodes, en)
childNodes = append(childNodes, en)
}

for _, ope := range cond.operandList {
en, err := ope.BuildOperand()
if err != nil {
return []ExprNode{}, err
}
childNodes = append(childNodes, en)
}

return childNodes, nil
Expand Down
81 changes: 9 additions & 72 deletions service/dynamodb/expression/condition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,16 @@ import (
type condErrorMode int

const (
noMatch condErrorMode = iota + 1
operandNum
conditionNum
noConditionError condErrorMode = iota
// noMatchingMode error will occur when the ConditionBuilder's Mode is not
// supported
noMatchingMode
)

func (cem condErrorMode) String() string {
switch cem {
case noMatch:
case noMatchingMode:
return "no matching"
case operandNum:
return "unexpected number of operands"
case conditionNum:
return "unexpected number of conditions"
default:
return "no matching condErrorMode"
}
Expand Down Expand Up @@ -252,36 +249,15 @@ func TestBuildCondition(t *testing.T) {
{
name: "no match error",
input: ConditionBuilder{},
err: noMatch,
},
{
name: "operand number error",
input: ConditionBuilder{
Mode: EqualCond,
},
err: operandNum,
},
{
name: "condition number error",
input: ConditionBuilder{
Mode: EqualCond,
conditionList: []ConditionBuilder{
ConditionBuilder{},
},
operandList: []OperandBuilder{
Path("foo"),
Value(5),
},
},
err: conditionNum,
err: noMatchingMode,
},
}

for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
expr, err := c.input.buildCondition()

if c.err != 0 {
if c.err != noConditionError {
if err == nil {
t.Errorf("expect error %q, got no error", c.err)
} else {
Expand Down Expand Up @@ -363,29 +339,12 @@ func TestBoolCondition(t *testing.T) {
Expression: "(#0 = #0) AND (#1 = #0) AND (#2 = #0)",
},
},
{
name: "empty condition",
input: ConditionBuilder{
Mode: AndCond,
},
err: conditionNum,
},
{
name: "non-empty operand list",
input: ConditionBuilder{
operandList: []OperandBuilder{
Path("foo"),
},
Mode: AndCond,
},
err: operandNum,
},
}

for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
expr, err := c.input.BuildExpression()
if c.err != 0 {
if c.err != noConditionError {
if err == nil {
t.Errorf("expect error %q, got no error", c.err)
} else {
Expand Down Expand Up @@ -467,34 +426,12 @@ func TestBetweenCondition(t *testing.T) {
Expression: "size (#0) BETWEEN :0 AND :1",
},
},
{
name: "non-empty condition list",
input: ConditionBuilder{
conditionList: []ConditionBuilder{
Value("foo").Equal(Path("bar")),
},
operandList: []OperandBuilder{
Path("foo"),
Value(5),
Value(6),
},
Mode: BetweenCond,
},
err: conditionNum,
},
{
name: "invalid operand list",
input: ConditionBuilder{
Mode: BetweenCond,
},
err: operandNum,
},
}

for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
expr, err := c.input.BuildExpression()
if c.err != 0 {
if c.err != noConditionError {
if err == nil {
t.Errorf("expect error %q, got no error", c.err)
} else {
Expand Down
32 changes: 22 additions & 10 deletions service/dynamodb/expression/operand_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,32 @@ import (
type opeErrorMode int

const (
emptyPath opeErrorMode = iota + 1
pathIndex
escChar
noOperandError opeErrorMode = iota
// emptyPath error will occur if an empty string is passed into PathBuilder or
// a nested path has an empty intermediary attribute name (i.e. foo.bar..baz)
emptyPath
// invalidPathIndex error will occur if there is an invalid index between the
// square brackets or there is no attribute that a square bracket iterates
// over
invalidPathIndex
// invalidEscChar error will occer if the escape char '$' is either followed
// by an unsupported character or if the escape char is the last character
invalidEscChar
// outOfRange error will occur if there are more escaped chars than there are
// actual values to be aliased.
outOfRange
// nilAliasList error will occur if the aliasList passed in has not been
// initialized
nilAliasList
)

func (oem opeErrorMode) String() string {
switch oem {
case emptyPath:
return "path is empty"
case pathIndex:
case invalidPathIndex:
return "invalid path index"
case escChar:
case invalidEscChar:
return "invalid escape"
case outOfRange:
return "out of range"
Expand Down Expand Up @@ -114,15 +126,15 @@ func TestBuildOperand(t *testing.T) {
name: "invalid index",
input: Path("[foo]"),
expected: ExprNode{},
err: pathIndex,
err: invalidPathIndex,
},
}

for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
en, err := c.input.BuildOperand()

if c.err != 0 {
if c.err != noOperandError {
if err == nil {
t.Errorf("expect error %q, got no error", c.err)
} else {
Expand Down Expand Up @@ -274,7 +286,7 @@ func TestBuildExpression(t *testing.T) {
names: []string{"foo", "foo"},
fmtExpr: "$p.$",
},
err: escChar,
err: invalidEscChar,
},
{
name: "names out of range",
Expand Down Expand Up @@ -303,7 +315,7 @@ func TestBuildExpression(t *testing.T) {
input: ExprNode{
fmtExpr: "$!",
},
err: escChar,
err: invalidEscChar,
},
{
name: "empty ExprNode",
Expand All @@ -328,7 +340,7 @@ func TestBuildExpression(t *testing.T) {
expr, err = c.input.buildExprNodes(&aliasList{})
}

if c.err != 0 {
if c.err != noOperandError {
if err == nil {
t.Errorf("expect error %q, got no error", c.err)
} else {
Expand Down

0 comments on commit 623de36

Please sign in to comment.