From 496ace25d3099bb49ba011336dff793286428de4 Mon Sep 17 00:00:00 2001 From: Alex Roitman Date: Tue, 11 Jun 2019 17:13:33 -0700 Subject: [PATCH] Refactoring/cleanup/fixes. --- internal/server/expression.go | 246 ++++++++++++++++++++-------------- internal/server/token.go | 24 ++-- tests/testcmd_test.go | 1 - 3 files changed, 160 insertions(+), 111 deletions(-) diff --git a/internal/server/expression.go b/internal/server/expression.go index 0fd87471..6ad4478f 100644 --- a/internal/server/expression.go +++ b/internal/server/expression.go @@ -12,6 +12,11 @@ const ( NOOP BinaryOp = iota AND OR + tokenAND = "and" + tokenOR = "or" + tokenNOT = "not" + tokenLParen = "(" + tokenRParen = ")" ) // areaExpression is either an object or operator+children @@ -24,169 +29,214 @@ type areaExpression struct { type children []*areaExpression -func (e *areaExpression) String() string { +func (e *areaExpression) String() (res string) { if e.obj != nil { - return e.obj.String() + res = e.obj.String() + } else { + var chStrings []string + for _, c := range e.children { + chStrings = append(chStrings, c.String()) + } + switch e.op { + case NOOP: + res = "empty operator" + case AND: + res = "(" + strings.Join(chStrings, " "+tokenAND+" ") + ")" + case OR: + res = "(" + strings.Join(chStrings, " "+tokenOR+" ") + ")" + default: + res = "unknown operator" + } } - var chStrings []string - for _, c := range e.children { - chStrings = append(chStrings, c.String()) + if e.negate { + res = tokenNOT + " " + res } - switch e.op { - case NOOP: - return "empty operator" - case AND: - return "(" + strings.Join(chStrings, " AND ") + ")" - case OR: - return "(" + strings.Join(chStrings, " OR ") + ")" - } - return "unknown operator" + return } // Return boolean value modulo negate field of the expression. -func (e *areaExpression) booleanize(val bool) bool { +func (e *areaExpression) maybeNegate(val bool) bool { if e.negate { return !val } return val } -func (e *areaExpression) Intersects(o geojson.Object) bool { +// Methods for testing an areaExpression against the spatial object +func (e *areaExpression) rawIntersects(o geojson.Object) bool { if e.obj != nil { - return e.booleanize(e.obj.Intersects(o)) + return e.obj.Intersects(o) } switch e.op { case AND: for _, c := range e.children { if !c.Intersects(o) { - return e.booleanize(false) + return false } } - return e.booleanize(true) + return true case OR: for _, c := range e.children { if c.Intersects(o) { - return e.booleanize(true) + return true } } - return e.booleanize(false) + return false } - return e.booleanize(false) + return false } -// object within an expression means anything of this expression contains object -func (e *areaExpression) Contains(o geojson.Object) bool { +func (e *areaExpression) rawContains(o geojson.Object) bool { if e.obj != nil { - return e.booleanize(e.obj.Contains(o)) + return e.obj.Contains(o) } switch e.op { case AND: for _, c:= range e.children { if !c.Contains(o) { - return e.booleanize(false) + return false } } - return e.booleanize(true) + return true case OR: for _, c:= range e.children { if c.Contains(o) { - return e.booleanize(true) + return true } } - return e.booleanize(false) + return false } - return e.booleanize(false) + return false } -func (e *areaExpression) Within(o geojson.Object) bool { +func (e *areaExpression) rawWithin(o geojson.Object) bool { if e.obj != nil { - return e.booleanize(e.obj.Within(o)) + return e.obj.Within(o) } switch e.op { case AND: for _, c:= range e.children { if !c.Within(o) { - return e.booleanize(false) + return false } } - return e.booleanize(true) + return true case OR: for _, c:= range e.children { if c.Within(o) { - return e.booleanize(true) + return true } } - return e.booleanize(false) + return false } - return e.booleanize(false) + return false +} + +func (e *areaExpression) Intersects(o geojson.Object) bool { + return e.maybeNegate(e.rawIntersects(o)) +} + +func (e *areaExpression) Contains(o geojson.Object) bool { + return e.maybeNegate(e.rawContains(o)) +} + +func (e *areaExpression) Within(o geojson.Object) bool { + return e.maybeNegate(e.rawWithin(o)) +} + +// Methods for testing an areaExpression against another areaExpression +func (e *areaExpression) rawIntersectsExpr(oe *areaExpression) bool { + if oe.negate { + e2 := &areaExpression{negate:!e.negate, obj:e.obj, op: e.op, children:e.children} + oe2 := &areaExpression{negate:false, obj:oe.obj, op:oe.op, children:oe.children} + return e2.rawIntersectsExpr(oe2) + } + if oe.obj != nil { + return e.Intersects(oe.obj) + } + switch oe.op { + case AND: + for _, c := range oe.children { + if !e.rawIntersectsExpr(c) { + return false + } + } + return true + case OR: + for _, c := range oe.children { + if e.rawIntersectsExpr(c) { + return true + } + } + return false + } + return false +} + +func (e *areaExpression) rawWithinExpr(oe *areaExpression) bool { + if oe.negate { + e2 := &areaExpression{negate:!e.negate, obj:e.obj, op: e.op, children:e.children} + oe2 := &areaExpression{negate:false, obj:oe.obj, op:oe.op, children:oe.children} + return e2.rawWithinExpr(oe2) + } + if oe.obj != nil { + return e.Within(oe.obj) + } + switch oe.op { + case AND: + for _, c:= range oe.children { + if !e.rawWithinExpr(c) { + return false + } + } + return true + case OR: + for _, c:= range oe.children { + if e.rawWithinExpr(c) { + return true + } + } + return false + } + return false +} + +func (e *areaExpression) rawContainsExpr(oe *areaExpression) bool { + if oe.negate { + e2 := &areaExpression{negate:!e.negate, obj:e.obj, op: e.op, children:e.children} + oe2 := &areaExpression{negate:false, obj:oe.obj, op:oe.op, children:oe.children} + return e2.rawContainsExpr(oe2) + } + if oe.obj != nil { + return e.Contains(oe.obj) + } + switch oe.op { + case AND: + for _, c:= range oe.children { + if !e.rawContainsExpr(c) { + return false + } + } + return true + case OR: + for _, c:= range oe.children { + if e.rawContainsExpr(c) { + return true + } + } + return false + } + return false } func (e *areaExpression) IntersectsExpr(oe *areaExpression) bool { - if oe.obj != nil { - return oe.booleanize(e.Intersects(oe.obj)) - } - switch oe.op { - case AND: - for _, c := range oe.children { - if !e.IntersectsExpr(c) { - return e.booleanize(false) - } - } - return e.booleanize(true) - case OR: - for _, c := range oe.children { - if e.IntersectsExpr(c) { - return e.booleanize(true) - } - } - return e.booleanize(false) - } - return e.booleanize(false) - + return e.maybeNegate(e.rawIntersectsExpr(oe)) } func (e *areaExpression) WithinExpr(oe *areaExpression) bool { - if oe.obj != nil { - return oe.booleanize(e.Within(oe.obj)) - } - switch oe.op { - case AND: - for _, c:= range oe.children { - if !e.WithinExpr(c) { - return e.booleanize(false) - } - } - return e.booleanize(true) - case OR: - for _, c:= range oe.children { - if e.WithinExpr(c) { - return e.booleanize(true) - } - } - return e.booleanize(false) - } - return e.booleanize(false) + return e.maybeNegate(e.rawWithinExpr(oe)) } func (e *areaExpression) ContainsExpr(oe *areaExpression) bool { - if oe.obj != nil { - return oe.booleanize(e.Contains(oe.obj)) - } - switch oe.op { - case AND: - for _, c:= range oe.children { - if !e.ContainsExpr(c) { - return e.booleanize(false) - } - } - return e.booleanize(true) - case OR: - for _, c:= range oe.children { - if e.ContainsExpr(c) { - return e.booleanize(true) - } - } - return e.booleanize(false) - } - return e.booleanize(false) + return e.maybeNegate(e.rawContainsExpr(oe)) } diff --git a/internal/server/token.go b/internal/server/token.go index 5adcdc56..5b8c26a5 100644 --- a/internal/server/token.go +++ b/internal/server/token.go @@ -720,7 +720,7 @@ loop: break } switch strings.ToLower(wtok) { - case "(": + case tokenLParen: newExpr := &areaExpression{negate: negate, op: NOOP} negate = false if ae != nil { @@ -729,28 +729,28 @@ loop: } ae = newExpr vsout = nvs - case ")": + case tokenRParen: if negate { - err = errInvalidArgument("NOT") + err = errInvalidArgument(tokenNOT) return } if parent, empty := ps.pop(); empty { - err = errInvalidArgument(")") + err = errInvalidArgument(tokenRParen) return } else { ae = parent } vsout = nvs - case "not": + case tokenNOT: negate = true vsout = nvs - case "and": + case tokenAND: if negate { - err = errInvalidArgument("NOT") + err = errInvalidArgument(tokenNOT) return } if ae == nil { - err = errInvalidArgument("AND") + err = errInvalidArgument(tokenAND) return } else if ae.obj == nil { switch ae.op { @@ -773,13 +773,13 @@ loop: ae = &areaExpression{op: AND, children: []*areaExpression{ae}} } vsout = nvs - case "or": + case tokenOR: if negate { - err = errInvalidArgument("NOT") + err = errInvalidArgument(tokenNOT) return } if ae == nil { - err = errInvalidArgument("OR") + err = errInvalidArgument(tokenOR) return } else if ae.obj == nil { switch ae.op { @@ -820,7 +820,7 @@ loop: } default: if negate { - err = errInvalidArgument("NOT") + err = errInvalidArgument(tokenNOT) return } break loop diff --git a/tests/testcmd_test.go b/tests/testcmd_test.go index 2b67d3ef..0a5ba0a1 100644 --- a/tests/testcmd_test.go +++ b/tests/testcmd_test.go @@ -162,5 +162,4 @@ func testcmd_expression_test(mc *mockServer) error { "(", "OBJECT", poly, "AND", "NOT", "GET", "mykey", "line3", ")"}, {"1"}, {"TEST", "OBJECT", poly9, "WITHIN", "NOT", "GET", "mykey", "line3"}, {"1"}, }) - }