Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposed changes on empty value handling for opensanctions search #863

Merged
merged 1 commit into from
Feb 19, 2025
Merged

proposed changes on empty value handling for opensanctions search #863

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions usecases/ast_eval/evaluate/eval_string_concat.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,5 @@ func (f StringConcat) Evaluate(ctx context.Context, arguments ast.Arguments) (an
}
}

if sb.Len() == 0 {
return MakeEvaluateError(ast.ErrNullFieldRead)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we actually used to have a lot of those, they have been rather deprecated in favor of "cleanly" returning nil in Evaluate if a nil value is "read" (AKA some nil received in a required input or actual nil value read in the logic of the function)

}

return sb.String(), nil
}
22 changes: 12 additions & 10 deletions usecases/ast_eval/evaluate/eval_string_concat_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,20 @@ import (

func TestStringConcat(t *testing.T) {
tts := []struct {
name string
in []any
out string
withSeparator bool
separator *string
error bool
}{
{[]any{"abc", "def"}, "abcdef", false, nil, false},
{[]any{42, "abc", 12}, "42abc12", false, nil, false},
{[]any{42, "abc", 12}, "42 abc 12", true, nil, false},
{[]any{42, "abc", 12}, "42-abc-12", true, utils.Ptr("-"), false},
{[]any{42, "abc", true}, "42 abc 12", true, nil, true},
{[]any{"hello", nil, "world"}, "hello world", true, nil, false},
{[]any{nil, nil, nil}, "", true, nil, true},
{"nominal", []any{"abc", "def"}, "abcdef", false, nil, false},
{"mixed types", []any{42, "abc", 12}, "42abc12", false, nil, false},
{"mixed types and separator", []any{42, "abc", 12}, "42 abc 12", true, nil, false},
{"mixed types custom separator", []any{42, "abc", 12}, "42-abc-12", true, utils.Ptr("-"), false},
{"boolean input returns error", []any{42, "abc", true}, "", true, nil, true},
{"with nil", []any{"hello", nil, "world"}, "hello world", true, nil, false},
{"only nil", []any{nil, nil, nil}, "", true, nil, false},
}

eval := StringConcat{}
Expand All @@ -42,12 +43,13 @@ func TestStringConcat(t *testing.T) {
Args: tt.in, NamedArgs: namedArgs,
})

asserts := assert.New(t)
switch tt.error {
case true:
assert.NotEmpty(t, err)
asserts.NotEmpty(err, tt.name, "expected ast eval errors")
default:
assert.Empty(t, err)
assert.Equal(t, tt.out, result)
asserts.Empty(err, tt.name, "unexpected ast eval errors")
asserts.Equal(tt.out, result, tt.name, "incorrect result")
}
}
}
97 changes: 58 additions & 39 deletions usecases/evaluate_scenario/evaluate_sanction_check.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
)

const (
ErrSanctionCheckAllFieldsNull = "all_fields_null"
ErrSanctionCheckAllFieldsNullOrEmpty = "all_fields_null_or_empty"
)

func (e ScenarioEvaluator) evaluateSanctionCheck(
Expand Down Expand Up @@ -48,37 +48,35 @@ func (e ScenarioEvaluator) evaluateSanctionCheck(
}

queries := []models.OpenSanctionsCheckQuery{}
nullFieldRead := 0
emptyFieldRead := 0
nbEvaluatedFields := 0
emptyInput := false

if iteration.SanctionCheckConfig.Query.Name != nil {
queries, err = e.evaluateSanctionCheckName(ctx, queries, iteration, dataAccessor)
nbEvaluatedFields += 1
queries, emptyInput, err = e.evaluateSanctionCheckName(ctx, queries, iteration, dataAccessor)
if err != nil {
switch {
case errors.Is(err, ast.ErrNullFieldRead):
nullFieldRead += 1
default:
return nil, true, err
}
return nil, true, errors.Wrap(err, "could not evaluate sanction check name")
} else if emptyInput {
emptyFieldRead += 1
}
}

if iteration.SanctionCheckConfig.Query.Label != nil {
queries, err = e.evaluateSanctionCheckLabel(ctx, queries, iteration, dataAccessor)
nbEvaluatedFields += 1
queries, emptyInput, err = e.evaluateSanctionCheckLabel(ctx, queries, iteration, dataAccessor)
if err != nil {
switch {
case errors.Is(err, ast.ErrNullFieldRead):
nullFieldRead += 1
default:
return nil, true, err
}
return nil, true, errors.Wrap(err, "could not evaluate sanction check label")
} else if emptyInput {
emptyFieldRead += 1
}
}

if nullFieldRead >= 2 {
if emptyFieldRead == nbEvaluatedFields {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this condition was incorrect if one of the two inputs was left empty

sanctionCheck = &models.SanctionCheckWithMatches{
SanctionCheck: models.SanctionCheck{
Status: models.SanctionStatusError,
ErrorCodes: []string{ErrSanctionCheckAllFieldsNull},
ErrorCodes: []string{ErrSanctionCheckAllFieldsNullOrEmpty},
},
}

Expand Down Expand Up @@ -150,69 +148,90 @@ func (e ScenarioEvaluator) evaluateSanctionCheck(
return
}

func (e ScenarioEvaluator) evaluateSanctionCheckName(ctx context.Context, queries []models.OpenSanctionsCheckQuery,
iteration models.ScenarioIteration, dataAccessor DataAccessor,
) ([]models.OpenSanctionsCheckQuery, error) {
func (e ScenarioEvaluator) evaluateSanctionCheckName(
ctx context.Context,
queries []models.OpenSanctionsCheckQuery,
iteration models.ScenarioIteration,
dataAccessor DataAccessor,
) (queriesOut []models.OpenSanctionsCheckQuery, emptyInput bool, err error) {
queriesOut = queries
nameFilterAny, err := e.evaluateAstExpression.EvaluateAstExpression(ctx, nil,
*iteration.SanctionCheckConfig.Query.Name, iteration.OrganizationId,
dataAccessor.ClientObject, dataAccessor.DataModel)
if err != nil {
return queries, err
return
}
if nameFilterAny.ReturnValue == nil {
emptyInput = true
return
}

nameFilter, ok := nameFilterAny.ReturnValue.(string)
if !ok {
return queries, errors.New("name filter name query did not return a string")
return nil, false, errors.New("name filter name query did not return a string")
}
if nameFilter == "" {
emptyInput = true
return
}

queries = append(queries, models.OpenSanctionsCheckQuery{
queriesOut = append(queriesOut, models.OpenSanctionsCheckQuery{
Type: "Thing",
Filters: models.OpenSanctionCheckFilter{
"name": []string{nameFilter},
},
})

return queries, nil
return queriesOut, false, nil
}

func (e ScenarioEvaluator) evaluateSanctionCheckLabel(ctx context.Context, queries []models.OpenSanctionsCheckQuery,
iteration models.ScenarioIteration, dataAccessor DataAccessor,
) ([]models.OpenSanctionsCheckQuery, error) {
func (e ScenarioEvaluator) evaluateSanctionCheckLabel(
ctx context.Context,
queries []models.OpenSanctionsCheckQuery,
iteration models.ScenarioIteration,
dataAccessor DataAccessor,
) (queriesOut []models.OpenSanctionsCheckQuery, emptyInput bool, err error) {
queriesOut = queries
labelFilterAny, err := e.evaluateAstExpression.EvaluateAstExpression(ctx, nil,
*iteration.SanctionCheckConfig.Query.Label, iteration.OrganizationId,
dataAccessor.ClientObject, dataAccessor.DataModel)
if err != nil {
return queries, err
return
}
if labelFilterAny.ReturnValue == nil {
return queries, ast.ErrNullFieldRead
emptyInput = true
return
}

labelFilter, ok := labelFilterAny.ReturnValue.(string)
if !ok {
return queries, errors.New("label filter name query did not return a string")
return nil, false, errors.New("label filter name query did not return a string")
}
if labelFilter == "" {
emptyInput = true
return
}

if e.nameRecognizer == nil || !e.nameRecognizer.IsConfigured() {
switch len(queries) {
switch len(queriesOut) {
case 0:
queries = append(queries, models.OpenSanctionsCheckQuery{
queriesOut = append(queriesOut, models.OpenSanctionsCheckQuery{
Type: "Thing",
Filters: models.OpenSanctionCheckFilter{
"name": []string{labelFilter},
},
})

default:
queries[0].Filters["name"] = append(queries[0].Filters["name"], labelFilter)
queriesOut[0].Filters["name"] = append(queriesOut[0].Filters["name"], labelFilter)
}

return queries, nil
return queriesOut, false, nil
}

matches, err := e.nameRecognizer.PerformNameRecognition(ctx, labelFilter)
if err != nil {
return queries, errors.New("could not perform name recognition on label")
return queriesOut, false, errors.New("could not perform name recognition on label")
}

var personQuery *models.OpenSanctionsCheckQuery = nil
Expand Down Expand Up @@ -244,11 +263,11 @@ func (e ScenarioEvaluator) evaluateSanctionCheckLabel(ctx context.Context, queri
}

if personQuery != nil {
queries = append(queries, *personQuery)
queriesOut = append(queriesOut, *personQuery)
}
if companyQuery != nil {
queries = append(queries, *companyQuery)
queriesOut = append(queriesOut, *companyQuery)
}

return queries, nil
return queriesOut, false, nil
}
Loading