Merge pull request #1188 from oom-ai/fix/export

Fix/export
oom-ai · Feb 11, 2022 · 2b3109a · 2b3109a
2 parents 6d3f249 + 56373c0
commit 2b3109a
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 10 deletions.
diff --git a/oomagent/test/test_channel_export.sh b/oomagent/test/test_channel_export.sh
@@ -10,9 +10,44 @@ oomcli push --entity-key 1 --group user_fake_stream --feature f1=10
 oomcli snapshot user_fake_stream
 
 unix_milli=${3:-$(perl -MTime::HiRes=time -E 'say int(time * 1000)')}
-echo $unix_milli
 
-case1() {
+oomagent_export_no_register_feature() {
+    case="export all no register feature"
+    arg=$(cat <<-EOF
+    {
+       "features": ["a.b","a.c"],
+       "unix_milli": $unix_milli
+    }
+EOF
+)
+    actual=$(testgrpc ChannelExport <<<"$arg" 2>&1 || true)
+    expected='
+ERROR:
+  Code: Internal
+  Message: invalid feature names [a.b a.c]
+'
+    assert_eq "$case" "$expected" "$actual"
+}
+
+oomagent_export_has_no_register_feature() {
+    case="export has no register feature"
+    arg=$(cat <<-EOF
+    {
+       "features": ["account.state","account.a"],
+       "unix_milli": $unix_milli
+    }
+EOF
+)
+    actual=$(testgrpc ChannelExport <<<"$arg" 2>&1 || true)
+    expected='
+ERROR:
+  Code: Internal
+  Message: invalid feature names [account.a]
+'
+    assert_eq "$case" "$expected" "$actual"
+}
+
+oomagent_export_no_features() {
     case="export and no features"
     arg=$(cat <<-EOF
     {
@@ -26,7 +61,7 @@ EOF
     assert_json_eq "$case" "$expected" "$actual"
 }
 
-case2() {
+oomagent_export_one_features() {
     prefix="export some features"
     arg=$(cat <<-EOF
     {
@@ -59,7 +94,7 @@ EOF
     assert_json_eq "$case" "$(sort <<<"$expected_rows")" "$actual_rows"
 }
 
-case3() {
+oomagent_export_multi_features() {
     prefix="export some features"
     arg=$(cat <<-EOF
     {
@@ -92,6 +127,12 @@ EOF
     assert_json_eq "$case" "$(sort <<<"$expected_rows")" "$actual_rows"
 }
 
-case1
-case2
-case3
+main() {
+    oomagent_export_no_register_feature
+    oomagent_export_has_no_register_feature
+    oomagent_export_no_features
+    oomagent_export_one_features
+    oomagent_export_multi_features
+}
+
+main
diff --git a/oomcli/test/test_export.sh b/oomcli/test/test_export.sh
@@ -19,6 +19,20 @@ t3=${1:-$(perl -MTime::HiRes=time -E 'say int(time * 1000)')}
 oomcli push --entity-key 2 --group user-click --feature last_5_click_posts=1,2,3,4,5 --feature number_of_user_starred_posts=10
 t4=${1:-$(perl -MTime::HiRes=time -E 'say int(time * 1000)')}
 
+oomcli_export_no_register_feature() {
+    case="export all no register feature"
+    actual=$(oomcli export --feature a.b,a.c --unix-milli $t0 2>&1 || true)
+    expected='Error: failed exporting features: invalid feature names [a.b a.c]'
+    assert_eq "$case" "$expected" "$actual"
+}
+
+oomcli_export_has_no_register_feature() {
+    case="export has no register feature"
+    actual=$(oomcli export --feature user-click.last_5_click_posts,user-click.a --unix-milli $t0 2>&1 ||true)
+    expected='Error: failed exporting features: invalid feature names [user-click.a]'
+    assert_eq "$case" "$expected" "$actual"
+}
+
 oomcli_export_push_feature() {
     case="push feature"
     expected='user,user-click.last_5_click_posts,user-click.number_of_user_starred_posts
@@ -77,6 +91,8 @@ oomcli_export_batch_and_stream() {
 }
 
 main() {
+    oomcli_export_no_register_feature
+    oomcli_export_has_no_register_feature
     oomcli_export_push_feature
     oomcli_export_update_feature
     oomcli_export_batch

diff --git a/pkg/oomstore/export.go b/pkg/oomstore/export.go
@@ -38,9 +38,6 @@ func (s *OomStore) ChannelExport(ctx context.Context, opt types.ChannelExportOpt
 	if err != nil {
 		return nil, err
 	}
-	if len(features) == 0 {
-		return nil, nil
-	}
 
 	if len(features) != len(opt.FeatureNames) {
 		invalid := features.FindMissingFeatures(opt.FeatureNames)