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

Add support for float type in mapstriface #6870

Merged
merged 2 commits into from
Apr 18, 2018
Merged

Add support for float type in mapstriface #6870

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7af007a
Add support for float type in mapstriface
ruflin Apr 16, 2018
0c9ba80
address review comment
ruflin Apr 17, 2018
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
38 changes: 36 additions & 2 deletions libbeat/common/schema/mapstriface/mapstriface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,9 +193,43 @@ func toInteger(key string, data map[string]interface{}) (interface{}, error) {
if err == nil {
return int64(f64), nil
}
return 0, fmt.Errorf("Expected integer, found json.Number (%v) that cannot be converted", num)
return 0, fmt.Errorf("expected integer, found json.Number (%v) that cannot be converted", num)
default:
return 0, fmt.Errorf("Expected integer, found %T", emptyIface)
return 0, fmt.Errorf("expected integer, found %T", emptyIface)
}
}

// Float creates a Conv object for converting floats. Acceptable input
// types are int64, int, and float64.
func Float(key string, opts ...schema.SchemaOption) schema.Conv {
return schema.SetOptions(schema.Conv{Key: key, Func: toFloat}, opts)
}

func toFloat(key string, data map[string]interface{}) (interface{}, error) {
emptyIface, exists := data[key]
if !exists {
return 0, fmt.Errorf("Key %s not found", key)
Copy link
Contributor

Choose a reason for hiding this comment

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

s/Key/key/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed

}
switch emptyIface.(type) {
case float64:
Copy link
Member

Choose a reason for hiding this comment

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

Should string be accepted?

Copy link

Choose a reason for hiding this comment

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

+1 on parsing string via strconv.ParseFloat.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So far we don't need it and it's also not supported in the Int type. I would add it when we need it.

Copy link
Contributor

Choose a reason for hiding this comment

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

When I've reviewed this, I've made the same conclusion by look at the int handling.

@andrewkroh @urso for me its not necessary right now, wdyt?

Copy link
Member

Choose a reason for hiding this comment

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

That’s fine by me to omit it for now. It’s an easy add at any point.

return emptyIface.(float64), nil
case int:
return float64(emptyIface.(int)), nil
case int64:
return float64(emptyIface.(int64)), nil
case json.Number:
num := emptyIface.(json.Number)
i64, err := num.Float64()
if err == nil {
return i64, nil
}
f64, err := num.Float64()
if err == nil {
return f64, nil
}
return 0, fmt.Errorf("expected float, found json.Number (%v) that cannot be converted", num)
default:
return 0, fmt.Errorf("expected float, found %T", emptyIface)
}
}

Expand Down
23 changes: 16 additions & 7 deletions libbeat/common/schema/mapstriface/mapstriface_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,16 @@ func TestConversions(t *testing.T) {
cTs := common.Time{}

input := map[string]interface{}{
"testString": "hello",
"testInt": 42,
"testIntFromFloat": 42.0,
"testIntFromInt32": int32(32),
"testIntFromInt64": int64(42),
"testJsonNumber": json.Number("3910564293633576924"),
"testBool": true,
"testString": "hello",
"testInt": 42,
"testIntFromFloat": 42.2,
"testFloat": 42.7,
"testFloatFromInt": 43,
"testIntFromInt32": int32(32),
"testIntFromInt64": int64(42),
"testJsonNumber": json.Number("3910564293633576924"),
"testJsonNumberFloat": json.Number("43.7"),
"testBool": true,
"testObj": map[string]interface{}{
"testObjString": "hello, object",
},
Expand All @@ -49,7 +52,10 @@ func TestConversions(t *testing.T) {
"test_int": Int("testInt"),
"test_int_from_float": Int("testIntFromFloat"),
"test_int_from_int64": Int("testIntFromInt64"),
"test_float": Float("testFloat"),
"test_float_from_int": Float("testFloatFromInt"),
"test_int_from_json": Int("testJsonNumber"),
"test_float_from_json": Float("testJsonNumberFloat"),
"test_string_from_num": StrFromNum("testIntFromInt32"),
"test_string_from_json_num": StrFromNum("testJsonNumber"),
"test_bool": Bool("testBool"),
Expand All @@ -74,7 +80,10 @@ func TestConversions(t *testing.T) {
"test_int": int64(42),
"test_int_from_float": int64(42),
"test_int_from_int64": int64(42),
"test_float": float64(42.7),
"test_float_from_int": float64(43),
"test_int_from_json": int64(3910564293633576924),
"test_float_from_json": float64(43.7),
"test_string_from_num": "32",
"test_string_from_json_num": "3910564293633576924",
"test_bool": true,
Expand Down