From 2554192c06b48b341ad3a3b81d297a55973b3da5 Mon Sep 17 00:00:00 2001 From: Gani Georgiev Date: Thu, 3 Apr 2025 15:55:34 +0300 Subject: [PATCH] added geoDistance docs and tests --- apis/record_crud.go | 2 +- tools/search/filter.go | 44 +---- tools/search/token_functions.go | 56 +++++++ tools/search/token_functions_test.go | 232 +++++++++++++++++++++++++++ 4 files changed, 290 insertions(+), 44 deletions(-) create mode 100644 tools/search/token_functions.go create mode 100644 tools/search/token_functions_test.go diff --git a/apis/record_crud.go b/apis/record_crud.go index 4f6421c6..e0c0358f 100644 --- a/apis/record_crud.go +++ b/apis/record_crud.go @@ -453,7 +453,7 @@ func recordUpdate(optFinalizer func(data any) error) func(e *core.RequestEvent) form.SetRecord(e.Record) manageRuleQuery := e.App.DB().Select("(1)").From(e.Collection.Name).AndWhere(dbx.HashExp{ - // note: use the original record id and not e.Record.Id because the record validations because may get overwritten + // note: use the original record id and not e.Record.Id because it may get overwritten e.Collection.Name + ".id": e.Record.LastSavedPK(), }) if !form.HasManageAccess() && diff --git a/tools/search/filter.go b/tools/search/filter.go index 5025c692..dcbeb065 100644 --- a/tools/search/filter.go +++ b/tools/search/filter.go @@ -249,48 +249,6 @@ func buildResolversExpr( return expr, nil } -// @todo test and docs -var filterFunctions = map[string]func( - argTokenResolverFunc func(fexpr.Token) (*ResolverResult, error), - args ...fexpr.Token, -) (*ResolverResult, error){ - // geoDistance(lonA, latA, lonB, latB) calculates the Haversine - // distance between 2 coordinates in metres - // (https://www.movable-type.co.uk/scripts/latlong.html). - "geoDistance": func(argTokenResolverFunc func(fexpr.Token) (*ResolverResult, error), args ...fexpr.Token) (*ResolverResult, error) { - if len(args) != 4 { - return nil, fmt.Errorf("[geoDistance] expected 4 arguments, got %d", len(args)) - } - - resolvedArgs := make([]*ResolverResult, 4) - for i, arg := range args { - if arg.Type != fexpr.TokenIdentifier && arg.Type != fexpr.TokenNumber && arg.Type != fexpr.TokenFunction { - return nil, fmt.Errorf("[geoDistance] argument %d must be an identifier, number or function", i) - } - resolved, err := argTokenResolverFunc(arg) - if err != nil { - return nil, fmt.Errorf("[geoDistance] failed to resolve argument %d: %w", i, err) - } - resolvedArgs[i] = resolved - } - - lonA := resolvedArgs[0].Identifier - latA := resolvedArgs[1].Identifier - lonB := resolvedArgs[2].Identifier - latB := resolvedArgs[3].Identifier - - return &ResolverResult{ - NoCoalesce: true, - Identifier: `(6371 * acos(` + - `cos(radians(` + latA + `)) * cos(radians(` + latB + `)) * ` + - `cos(radians(` + lonB + `) - radians(` + lonA + `)) + ` + - `sin(radians(` + latA + `)) * sin(radians(` + latB + `))` + - `))`, - Params: mergeParams(resolvedArgs[0].Params, resolvedArgs[1].Params, resolvedArgs[2].Params, resolvedArgs[3].Params), - }, nil - }, -} - var normalizedIdentifiers = map[string]string{ // if `null` field is missing, treat `null` identifier as NULL token "null": "NULL", @@ -347,7 +305,7 @@ func resolveToken(token fexpr.Token, fieldResolver FieldResolver) (*ResolverResu Params: dbx.Params{placeholder: cast.ToFloat64(token.Literal)}, }, nil case fexpr.TokenFunction: - fn, ok := filterFunctions[token.Literal] + fn, ok := tokenFunctions[token.Literal] if !ok { return nil, fmt.Errorf("unknown function %q", token.Literal) } diff --git a/tools/search/token_functions.go b/tools/search/token_functions.go new file mode 100644 index 00000000..c0b1c434 --- /dev/null +++ b/tools/search/token_functions.go @@ -0,0 +1,56 @@ +package search + +import ( + "fmt" + + "github.com/ganigeorgiev/fexpr" +) + +var tokenFunctions = map[string]func( + argTokenResolverFunc func(fexpr.Token) (*ResolverResult, error), + args ...fexpr.Token, +) (*ResolverResult, error){ + // geoDistance(lonA, latA, lonB, latB) calculates the Haversine + // distance between 2 coordinates in metres (https://www.movable-type.co.uk/scripts/latlong.html). + // + // The accepted arguments at the moment could be either a plain number or a column identifier (including NULL). + // If the column identifier cannot be resolved and converted to a numeric value, it resolves to NULL. + // + // Similar to the built-in SQLite functions, geoDistance doesn't apply + // a "match-all" constraints in case there are multiple relation fields arguments. + // Or in other words, if a collection has "orgs" multiple relation field pointing to "orgs" collection that has "office" as "geoPoint" field, + // then the filter: `geoDistance(orgs.office.lon, orgs.office.lat, 1, 2) < 200` + // will evaluate to true if for at-least-one of the "orgs.office" records the function result in a value satisfying the condition (aka. "result < 200"). + "geoDistance": func(argTokenResolverFunc func(fexpr.Token) (*ResolverResult, error), args ...fexpr.Token) (*ResolverResult, error) { + if len(args) != 4 { + return nil, fmt.Errorf("[geoDistance] expected 4 arguments, got %d", len(args)) + } + + resolvedArgs := make([]*ResolverResult, 4) + for i, arg := range args { + if arg.Type != fexpr.TokenIdentifier && arg.Type != fexpr.TokenNumber { + return nil, fmt.Errorf("[geoDistance] argument %d must be an identifier or number", i) + } + resolved, err := argTokenResolverFunc(arg) + if err != nil { + return nil, fmt.Errorf("[geoDistance] failed to resolve argument %d: %w", i, err) + } + resolvedArgs[i] = resolved + } + + lonA := resolvedArgs[0].Identifier + latA := resolvedArgs[1].Identifier + lonB := resolvedArgs[2].Identifier + latB := resolvedArgs[3].Identifier + + return &ResolverResult{ + NoCoalesce: true, + Identifier: `(6371 * acos(` + + `cos(radians(` + latA + `)) * cos(radians(` + latB + `)) * ` + + `cos(radians(` + lonB + `) - radians(` + lonA + `)) + ` + + `sin(radians(` + latA + `)) * sin(radians(` + latB + `))` + + `))`, + Params: mergeParams(resolvedArgs[0].Params, resolvedArgs[1].Params, resolvedArgs[2].Params, resolvedArgs[3].Params), + }, nil + }, +} diff --git a/tools/search/token_functions_test.go b/tools/search/token_functions_test.go new file mode 100644 index 00000000..e53d263c --- /dev/null +++ b/tools/search/token_functions_test.go @@ -0,0 +1,232 @@ +package search + +import ( + "errors" + "fmt" + "strings" + "testing" + + "github.com/ganigeorgiev/fexpr" + "github.com/pocketbase/dbx" + "github.com/pocketbase/pocketbase/tools/security" +) + +func TestTokenFunctionsGeoDistance(t *testing.T) { + t.Parallel() + + testDB, err := createTestDB() + if err != nil { + t.Fatal(err) + } + defer testDB.Close() + + fn, ok := tokenFunctions["geoDistance"] + if !ok { + t.Error("Expected geoDistance token function to be registered.") + } + + baseTokenResolver := func(t fexpr.Token) (*ResolverResult, error) { + placeholder := "t" + security.PseudorandomString(5) + return &ResolverResult{Identifier: "{:" + placeholder + "}", Params: map[string]any{placeholder: t.Literal}}, nil + } + + scenarios := []struct { + name string + args []fexpr.Token + resolver func(t fexpr.Token) (*ResolverResult, error) + result *ResolverResult + expectErr bool + }{ + { + "no args", + nil, + baseTokenResolver, + nil, + true, + }, + { + "< 4 args", + []fexpr.Token{ + {Literal: "1", Type: fexpr.TokenNumber}, + {Literal: "2", Type: fexpr.TokenNumber}, + {Literal: "3", Type: fexpr.TokenNumber}, + }, + baseTokenResolver, + nil, + true, + }, + { + "> 4 args", + []fexpr.Token{ + {Literal: "1", Type: fexpr.TokenNumber}, + {Literal: "2", Type: fexpr.TokenNumber}, + {Literal: "3", Type: fexpr.TokenNumber}, + {Literal: "4", Type: fexpr.TokenNumber}, + {Literal: "5", Type: fexpr.TokenNumber}, + }, + baseTokenResolver, + nil, + true, + }, + { + "unsupported function argument", + []fexpr.Token{ + {Literal: "1", Type: fexpr.TokenFunction}, + {Literal: "2", Type: fexpr.TokenNumber}, + {Literal: "3", Type: fexpr.TokenNumber}, + {Literal: "4", Type: fexpr.TokenNumber}, + }, + baseTokenResolver, + nil, + true, + }, + { + "unsupported text argument", + []fexpr.Token{ + {Literal: "1", Type: fexpr.TokenText}, + {Literal: "2", Type: fexpr.TokenNumber}, + {Literal: "3", Type: fexpr.TokenNumber}, + {Literal: "4", Type: fexpr.TokenNumber}, + }, + baseTokenResolver, + nil, + true, + }, + { + "4 valid arguments but with resolver error", + []fexpr.Token{ + {Literal: "1", Type: fexpr.TokenNumber}, + {Literal: "2", Type: fexpr.TokenNumber}, + {Literal: "3", Type: fexpr.TokenNumber}, + {Literal: "4", Type: fexpr.TokenNumber}, + }, + func(t fexpr.Token) (*ResolverResult, error) { + return nil, errors.New("test") + }, + nil, + true, + }, + { + "4 valid arguments", + []fexpr.Token{ + {Literal: "1", Type: fexpr.TokenNumber}, + {Literal: "2", Type: fexpr.TokenNumber}, + {Literal: "3", Type: fexpr.TokenNumber}, + {Literal: "4", Type: fexpr.TokenNumber}, + }, + baseTokenResolver, + &ResolverResult{ + NoCoalesce: true, + Identifier: `(6371 * acos(cos(radians({:latA})) * cos(radians({:latB})) * cos(radians({:lonB}) - radians({:lonA})) + sin(radians({:latA})) * sin(radians({:latB}))))`, + Params: map[string]any{ + "lonA": 1, + "latA": 2, + "lonB": 3, + "latB": 4, + }, + }, + false, + }, + { + "mixed arguments", + []fexpr.Token{ + {Literal: "null", Type: fexpr.TokenIdentifier}, + {Literal: "2", Type: fexpr.TokenNumber}, + {Literal: "false", Type: fexpr.TokenIdentifier}, + {Literal: "4", Type: fexpr.TokenNumber}, + }, + baseTokenResolver, + &ResolverResult{ + NoCoalesce: true, + Identifier: `(6371 * acos(cos(radians({:latA})) * cos(radians({:latB})) * cos(radians({:lonB}) - radians({:lonA})) + sin(radians({:latA})) * sin(radians({:latB}))))`, + Params: map[string]any{ + "lonA": "null", + "latA": 2, + "lonB": false, + "latB": 4, + }, + }, + false, + }, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + result, err := fn(s.resolver, s.args...) + + hasErr := err != nil + if hasErr != s.expectErr { + t.Fatalf("Expected hasErr %v, got %v (%v)", s.expectErr, hasErr, err) + } + + testCompareResults(t, s.result, result) + }) + } +} + +// ------------------------------------------------------------------- + +func testCompareResults(t *testing.T, a, b *ResolverResult) { + testDB, err := createTestDB() + if err != nil { + t.Fatal(err) + } + defer testDB.Close() + + aIsNil := a == nil + bIsNil := b == nil + if aIsNil != bIsNil { + t.Fatalf("Expected aIsNil and bIsNil to be the same, got %v vs %v", aIsNil, bIsNil) + } + + if aIsNil && bIsNil { + return + } + + aHasAfterBuild := a.AfterBuild == nil + bHasAfterBuild := b.AfterBuild == nil + if aHasAfterBuild != bHasAfterBuild { + t.Fatalf("Expected aHasAfterBuild and bHasAfterBuild to be the same, got %v vs %v", aHasAfterBuild, bHasAfterBuild) + } + + var aAfterBuild string + if a.AfterBuild != nil { + aAfterBuild = a.AfterBuild(dbx.NewExp("test")).Build(testDB.DB, a.Params) + } + var bAfterBuild string + if b.AfterBuild != nil { + bAfterBuild = b.AfterBuild(dbx.NewExp("test")).Build(testDB.DB, a.Params) + } + if aAfterBuild != bAfterBuild { + t.Fatalf("Expected bAfterBuild and bAfterBuild to be the same, got\n%s\nvs\n%s", aAfterBuild, bAfterBuild) + } + + var aMultiMatchSubQuery string + if a.MultiMatchSubQuery != nil { + aMultiMatchSubQuery = a.MultiMatchSubQuery.Build(testDB.DB, a.Params) + } + var bMultiMatchSubQuery string + if b.MultiMatchSubQuery != nil { + bMultiMatchSubQuery = b.MultiMatchSubQuery.Build(testDB.DB, b.Params) + } + if aMultiMatchSubQuery != bMultiMatchSubQuery { + t.Fatalf("Expected bMultiMatchSubQuery and bMultiMatchSubQuery to be the same, got\n%s\nvs\n%s", aMultiMatchSubQuery, bMultiMatchSubQuery) + } + + if a.NoCoalesce != b.NoCoalesce { + t.Fatalf("Expected NoCoalesce to match, got %v vs %v", a.NoCoalesce, b.NoCoalesce) + } + + // loose placeholders replacement + var aResolved = a.Identifier + for k, v := range a.Params { + aResolved = strings.ReplaceAll(aResolved, "{:"+k+"}", fmt.Sprintf("%v", v)) + } + var bResolved = b.Identifier + for k, v := range b.Params { + bResolved = strings.ReplaceAll(bResolved, "{:"+k+"}", fmt.Sprintf("%v", v)) + } + if aResolved != bResolved { + t.Fatalf("Expected resolved identifiers to match, got\n%s\nvs\n%s", aResolved, bResolved) + } +}