Repository: oom-ai/oomstore (go)

Issue:
Modify API `Join`, `OnlineGet`: use full_name instead of feature_name

Pull request:
Jinghan/use `FullName` for API `OnlineGet`
This PR does:
- use `FullName` for API `OnlineGet` and `MultiOnlineGet`
- add `category` for command `register group`

close #811 

Patch:
diff --git a/internal/database/online/cassandra/get.go b/internal/database/online/cassandra/get.go
index 5e294261..6838b653 100644
--- a/internal/database/online/cassandra/get.go
+++ b/internal/database/online/cassandra/get.go
@@ -30,8 +30,8 @@ func (db *DB) Get(ctx context.Context, opt online.GetOpt) (dbutil.RowMap, error)
 		return nil, err
 	}
 
-	for k, v := range rs {
-		rs[k] = deserializeString(v)
+	for _, feature := range opt.FeatureList {
+		rs[feature.FullName] = deserializeString(rs[feature.Name])
 	}
 	return rs, nil
 }
@@ -84,8 +84,8 @@ func deserializeIntoRowMap(values map[string]interface{}, entityName string, fea
 	entityKey := values[entityName].(string)
 	delete(values, entityName)
 
-	for k, v := range values {
-		values[k] = deserializeString(v)
+	for _, feature := range features {
+		values[feature.FullName] = deserializeString(values[feature.Name])
 	}
 	return entityKey, values
 }
diff --git a/internal/database/online/dynamodb/get.go b/internal/database/online/dynamodb/get.go
index 6716e86c..41e91ef0 100644
--- a/internal/database/online/dynamodb/get.go
+++ b/internal/database/online/dynamodb/get.go
@@ -116,7 +116,7 @@ func deserializeFeatureValues(features oomTypes.FeatureList, item map[string]typ
 		if err != nil {
 			return nil, err
 		}
-		rowMap[feature.Name] = typedValue
+		rowMap[feature.FullName] = typedValue
 	}
 	return rowMap, nil
 }
diff --git a/internal/database/online/redis/get.go b/internal/database/online/redis/get.go
index 7a09ccd9..1f55a022 100644
--- a/internal/database/online/redis/get.go
+++ b/internal/database/online/redis/get.go
@@ -36,7 +36,7 @@ func (db *DB) Get(ctx context.Context, opt online.GetOpt) (dbutil.RowMap, error)
 		if err != nil {
 			return nil, err
 		}
-		rowMap[opt.FeatureList[i].Name] = typedValue
+		rowMap[opt.FeatureList[i].FullName] = typedValue
 	}
 	return rowMap, nil
 }
diff --git a/internal/database/online/sqlutil/get.go b/internal/database/online/sqlutil/get.go
index e890f772..4357cf55 100644
--- a/internal/database/online/sqlutil/get.go
+++ b/internal/database/online/sqlutil/get.go
@@ -83,7 +83,7 @@ func deserializeIntoRowMap(values []interface{}, features types.FeatureList, bac
 		if err != nil {
 			return nil, err
 		}
-		rs[features[i].Name] = typedValue
+		rs[features[i].FullName] = typedValue
 	}
 	return rs, nil
 }
diff --git a/oomagent/server.go b/oomagent/server.go
index fabeb76f..116f0720 100644
--- a/oomagent/server.go
+++ b/oomagent/server.go
@@ -34,8 +34,8 @@ func (s *server) HealthCheck(ctx context.Context, req *codegen.HealthCheckReques
 
 func (s *server) OnlineGet(ctx context.Context, req *codegen.OnlineGetRequest) (*codegen.OnlineGetResponse, error) {
 	result, err := s.oomstore.OnlineGet(ctx, types.OnlineGetOpt{
-		FeatureNames: req.FeatureNames,
-		EntityKey:    req.EntityKey,
+		FeatureFullNames: req.FeatureNames,
+		EntityKey:        req.EntityKey,
 	})
 	if err != nil {
 		return &codegen.OnlineGetResponse{
@@ -59,8 +59,8 @@ func (s *server) OnlineGet(ctx context.Context, req *codegen.OnlineGetRequest) (
 
 func (s *server) OnlineMultiGet(ctx context.Context, req *codegen.OnlineMultiGetRequest) (*codegen.OnlineMultiGetResponse, error) {
 	result, err := s.oomstore.OnlineMultiGet(ctx, types.OnlineMultiGetOpt{
-		FeatureNames: req.FeatureNames,
-		EntityKeys:   req.EntityKeys,
+		FeatureFullNames: req.FeatureNames,
+		EntityKeys:       req.EntityKeys,
 	})
 	if err != nil {
 		return &codegen.OnlineMultiGetResponse{
diff --git a/oomcli/cmd/get_online.go b/oomcli/cmd/get_online.go
index 92ff468f..9d405b63 100644
--- a/oomcli/cmd/get_online.go
+++ b/oomcli/cmd/get_online.go
@@ -43,7 +43,7 @@ func init() {
 	flags.StringVarP(&onlineGetOpt.EntityKey, "entity-key", "k", "", "entity keys")
 	_ = getOnlineCmd.MarkFlagRequired("entity-key")
 
-	flags.StringSliceVar(&onlineGetOpt.FeatureNames, "feature", nil, "feature names")
+	flags.StringSliceVar(&onlineGetOpt.FeatureFullNames, "feature", nil, "feature full names")
 	_ = getOnlineCmd.MarkFlagRequired("feature")
 }
 
@@ -59,7 +59,7 @@ func printOnlineFeatures(featureValues *types.FeatureValues, output string) erro
 }
 
 func printOnlineFeaturesInCSV(featureValues *types.FeatureValues) error {
-	header := append([]string{featureValues.EntityName}, featureValues.FeatureNames...)
+	header := append([]string{featureValues.EntityName}, featureValues.FeatureFullNames...)
 	record := append([]string{featureValues.EntityKey}, cast.ToStringSlice(featureValues.FeatureValueSlice())...)
 
 	w := csv.NewWriter(os.Stdout)
@@ -74,7 +74,7 @@ func printOnlineFeaturesInCSV(featureValues *types.FeatureValues) error {
 }
 
 func printOnlineFeaturesInASCIITable(featureValues *types.FeatureValues) error {
-	header := append([]string{featureValues.EntityName}, featureValues.FeatureNames...)
+	header := append([]string{featureValues.EntityName}, featureValues.FeatureFullNames...)
 	record := append([]string{featureValues.EntityKey}, cast.ToStringSlice(featureValues.FeatureValueSlice())...)
 
 	table := tablewriter.NewWriter(os.Stdout)
diff --git a/oomcli/cmd/register_group.go b/oomcli/cmd/register_group.go
index 13f9b699..5f234dbe 100644
--- a/oomcli/cmd/register_group.go
+++ b/oomcli/cmd/register_group.go
@@ -9,12 +9,14 @@ import (
 )
 
 var registerGroupOpt types.CreateGroupOpt
+var category string
 var registerGroupCmd = &cobra.Command{
 	Use:   "group <group_name>",
 	Short: "Register a new feature group",
 	Args:  cobra.ExactArgs(1),
 	PreRun: func(cmd *cobra.Command, args []string) {
 		registerGroupOpt.GroupName = args[0]
+		registerGroupOpt.Category = types.Category(category)
 	},
 	Run: func(cmd *cobra.Command, args []string) {
 		ctx := context.Background()
@@ -35,5 +37,8 @@ func init() {
 	flags.StringVarP(&registerGroupOpt.EntityName, "entity", "e", "", "entity name")
 	_ = registerGroupCmd.MarkFlagRequired("entity")
 
+	flags.StringVarP(&category, "category", "c", "", "group category")
+	_ = registerGroupCmd.MarkFlagRequired("category")
+
 	flags.StringVarP(&registerGroupOpt.Description, "description", "d", "", "group description")
 }
diff --git a/pkg/oomstore/online_query.go b/pkg/oomstore/online_query.go
index 8050df4a..b9646072 100644
--- a/pkg/oomstore/online_query.go
+++ b/pkg/oomstore/online_query.go
@@ -13,7 +13,7 @@ import (
 // Get online features of a particular entity instance.
 func (s *OomStore) OnlineGet(ctx context.Context, opt types.OnlineGetOpt) (*types.FeatureValues, error) {
 	features := s.metadata.CacheListFeature(ctx, metadata.ListFeatureOpt{
-		FeatureFullNames: &opt.FeatureNames,
+		FeatureFullNames: &opt.FeatureFullNames,
 	}).Filter(func(f *types.Feature) bool {
 		return f.Group.OnlineRevisionID != nil
 	})
@@ -27,10 +27,10 @@ func (s *OomStore) OnlineGet(ctx context.Context, opt types.OnlineGetOpt) (*type
 	}
 
 	rs := types.FeatureValues{
-		EntityName:      entity.Name,
-		EntityKey:       opt.EntityKey,
-		FeatureNames:    opt.FeatureNames,
-		FeatureValueMap: make(map[string]interface{}),
+		EntityName:       entity.Name,
+		EntityKey:        opt.EntityKey,
+		FeatureFullNames: opt.FeatureFullNames,
+		FeatureValueMap:  make(map[string]interface{}),
 	}
 
 	featureMap := groupFeaturesByRevisionID(features)
@@ -58,7 +58,7 @@ func (s *OomStore) OnlineGet(ctx context.Context, opt types.OnlineGetOpt) (*type
 // Get online features of multiple entity instances.
 func (s *OomStore) OnlineMultiGet(ctx context.Context, opt types.OnlineMultiGetOpt) (map[string]*types.FeatureValues, error) {
 	features := s.metadata.CacheListFeature(ctx, metadata.ListFeatureOpt{
-		FeatureFullNames: &opt.FeatureNames,
+		FeatureFullNames: &opt.FeatureFullNames,
 	})
 
 	features = features.Filter(func(f *types.Feature) bool {
@@ -86,10 +86,10 @@ func (s *OomStore) OnlineMultiGet(ctx context.Context, opt types.OnlineMultiGetO
 	result := make(map[string]*types.FeatureValues)
 	for _, entityKey := range opt.EntityKeys {
 		result[entityKey] = &types.FeatureValues{
-			EntityName:      entity.Name,
-			EntityKey:       entityKey,
-			FeatureNames:    opt.FeatureNames,
-			FeatureValueMap: make(map[string]interface{}),
+			EntityName:       entity.Name,
+			EntityKey:        entityKey,
+			FeatureFullNames: opt.FeatureFullNames,
+			FeatureValueMap:  make(map[string]interface{}),
 		}
 		for featureName, featureValue := range featureValueMap[entityKey] {
 			result[entityKey].FeatureValueMap[featureName] = featureValue
diff --git a/pkg/oomstore/types/feature_values.go b/pkg/oomstore/types/feature_values.go
index 7e9f03b5..41364546 100644
--- a/pkg/oomstore/types/feature_values.go
+++ b/pkg/oomstore/types/feature_values.go
@@ -1,15 +1,15 @@
 package types
 
 type FeatureValues struct {
-	EntityName      string
-	EntityKey       string
-	FeatureNames    []string
-	FeatureValueMap map[string]interface{}
+	EntityName       string
+	EntityKey        string
+	FeatureFullNames []string
+	FeatureValueMap  map[string]interface{}
 }
 
 func (fv *FeatureValues) FeatureValueSlice() []interface{} {
-	values := make([]interface{}, 0, len(fv.FeatureNames))
-	for _, name := range fv.FeatureNames {
+	values := make([]interface{}, 0, len(fv.FeatureFullNames))
+	for _, name := range fv.FeatureFullNames {
 		values = append(values, fv.FeatureValueMap[name])
 	}
 	return values
diff --git a/pkg/oomstore/types/options.go b/pkg/oomstore/types/options.go
index ce89df74..e05bc4ad 100644
--- a/pkg/oomstore/types/options.go
+++ b/pkg/oomstore/types/options.go
@@ -45,13 +45,13 @@ type ExportOpt struct {
 }
 
 type OnlineGetOpt struct {
-	FeatureNames []string
-	EntityKey    string
+	FeatureFullNames []string
+	EntityKey        string
 }
 
 type OnlineMultiGetOpt struct {
-	FeatureNames []string
-	EntityKeys   []string
+	FeatureFullNames []string
+	EntityKeys       []string
 }
 
 type ChannelJoinOpt struct {


Test patch:
diff --git a/internal/database/online/test_impl/get.go b/internal/database/online/test_impl/get.go
index d3bf4c06..06cee5bf 100644
--- a/internal/database/online/test_impl/get.go
+++ b/internal/database/online/test_impl/get.go
@@ -26,7 +26,7 @@ func TestGetExisted(t *testing.T, prepareStore PrepareStoreFn, destroyStore Dest
 		require.NoError(t, err)
 
 		for i, f := range s.Features {
-			compareFeatureValue(t, target.ValueAt(i), rs[f.Name], f.ValueType)
+			compareFeatureValue(t, target.ValueAt(i), rs[f.FullName], f.ValueType)
 		}
 	}
 }
@@ -68,10 +68,9 @@ func TestMultiGet(t *testing.T, prepareStore PrepareStoreFn, destroystore Destro
 		EntityKeys:  keys,
 	})
 	require.NoError(t, err)
-
 	for _, record := range s.Data {
 		for i, feature := range s.Features {
-			compareFeatureValue(t, record.ValueAt(i), rs[record.EntityKey()][feature.Name], feature.ValueType)
+			compareFeatureValue(t, record.ValueAt(i), rs[record.EntityKey()][feature.FullName], feature.ValueType)
 		}
 	}
 
diff --git a/internal/database/online/test_impl/purge.go b/internal/database/online/test_impl/purge.go
index 9fa60182..5a30ff9d 100644
--- a/internal/database/online/test_impl/purge.go
+++ b/internal/database/online/test_impl/purge.go
@@ -48,7 +48,7 @@ func TestPurgeNotRemovesOtherRevisions(t *testing.T, prepareStore PrepareStoreFn
 		})
 		require.NoError(t, err)
 		for i, f := range SampleSmall.Features {
-			compareFeatureValue(t, record.ValueAt(i), rs[f.Name], f.ValueType)
+			compareFeatureValue(t, record.ValueAt(i), rs[f.FullName], f.ValueType)
 		}
 	}
 }
diff --git a/internal/database/online/test_impl/util.go b/internal/database/online/test_impl/util.go
index b741406e..b0144077 100644
--- a/internal/database/online/test_impl/util.go
+++ b/internal/database/online/test_impl/util.go
@@ -37,30 +37,35 @@ func init() {
 				&types.Feature{
 					ID:        1,
 					Name:      "age",
+					FullName:  "user.age",
 					GroupID:   1,
 					ValueType: types.Int64,
 				},
 				&types.Feature{
 					ID:        2,
 					Name:      "gender",
+					FullName:  "user.gender",
 					GroupID:   1,
 					ValueType: types.String,
 				},
 				&types.Feature{
 					ID:        3,
 					Name:      "account",
+					FullName:  "user.account",
 					GroupID:   1,
 					ValueType: types.Float64,
 				},
 				&types.Feature{
 					ID:        4,
 					Name:      "is_active",
+					FullName:  "user.is_active",
 					GroupID:   1,
 					ValueType: types.Bool,
 				},
 				&types.Feature{
 					ID:        5,
 					Name:      "register_time",
+					FullName:  "user.register_time",
 					GroupID:   1,
 					ValueType: types.Time,
 				},
@@ -81,6 +86,7 @@ func init() {
 			&types.Feature{
 				ID:        2,
 				Name:      "charge",
+				FullName:  "user.charge",
 				GroupID:   2,
 				ValueType: types.Float64,
 			},
diff --git a/oomcli/test/test_online_get.sh b/oomcli/test/test_online_get.sh
index 1a661042..14782ed9 100755
--- a/oomcli/test/test_online_get.sh
+++ b/oomcli/test/test_online_get.sh
@@ -9,25 +9,25 @@ sync $revisionID
 
 case="query single feature"
 expected='
-device,model
+device,phone.model
 1,xiaomi-mix3
 '
-actual=$(oomcli get online --feature model -k 1 -o csv)
+actual=$(oomcli get online --feature phone.model -k 1 -o csv)
 assert_eq "$case" "$expected" "$actual"
 
 
 case="query multiple features 1"
 expected='
-device,model,price
+device,phone.model,phone.price
 6,apple-iphone11,4999
 '
-actual=$(oomcli get online --feature model,price -k 6 -o csv)
+actual=$(oomcli get online --feature phone.model,phone.price -k 6 -o csv)
 assert_eq "$case" "$expected" "$actual"
 
 case="query multiple features 2"
 expected='
-device,price,model
+device,phone.price,phone.model
 6,4999,apple-iphone11
 '
-actual=$(oomcli get online --feature price,model -k 6 -o csv)
+actual=$(oomcli get online --feature phone.price,phone.model -k 6 -o csv)
 assert_eq "$case" "$expected" "$actual"
diff --git a/oomcli/test/test_update_feature.sh b/oomcli/test/test_update_feature.sh
index 26c5d132..5e2cd34b 100755
--- a/oomcli/test/test_update_feature.sh
+++ b/oomcli/test/test_update_feature.sh
@@ -7,11 +7,11 @@ register_features
 import_sample > /dev/null
 
 case='oomcli update feature works'
-oomcli update feature price --description "new description"
+oomcli update feature phone.price --description "new description"
 expected='
 ID,NAME,GROUP,ENTITY,CATEGORY,VALUE-TYPE,DESCRIPTION,ONLINE-REVISION-ID
 1,price,phone,device,batch,int64,new description,<NULL>
 '
-actual=$(oomcli get meta feature price -o csv --wide)
+actual=$(oomcli get meta feature phone.price -o csv --wide)
 ignore_time() { cut -d ',' -f 1-8 <<<"$1"; }
 assert_eq "$case"  "$expected" "$(ignore_time "$actual")"
diff --git a/oomcli/test/util.sh b/oomcli/test/util.sh
index 8b7bdb3f..ae973262 100644
--- a/oomcli/test/util.sh
+++ b/oomcli/test/util.sh
@@ -49,8 +49,8 @@ register_features() {
     oomcli register entity device --length 32     --description "device"
     oomcli register entity user   --length 64     --description "user"
 
-    oomcli register group phone    --entity device  --description "phone"
-    oomcli register group student  --entity user    --description "student"
+    oomcli register group phone    --entity device  --category "batch" --description "phone"
+    oomcli register group student  --entity user    --category "batch" --description "student"
 
     oomcli register batch-feature price --group phone   --value-type "int64"  --description "price"
     oomcli register batch-feature model --group phone   --value-type "string" --description "model"
diff --git a/pkg/oomstore/online_query_test.go b/pkg/oomstore/online_query_test.go
index 512f1bac..2871cef3 100644
--- a/pkg/oomstore/online_query_test.go
+++ b/pkg/oomstore/online_query_test.go
@@ -38,8 +38,8 @@ func TestOnlineGet(t *testing.T) {
 		{
 			description: "no available features",
 			opt: types.OnlineGetOpt{
-				FeatureNames: unavailableFeatures.Names(),
-				EntityKey:    "1234",
+				FeatureFullNames: unavailableFeatures.FullNames(),
+				EntityKey:        "1234",
 			},
 			features:      unavailableFeatures,
 			expectedError: nil,
@@ -48,8 +48,8 @@ func TestOnlineGet(t *testing.T) {
 		{
 			description: "inconsistent entity type, fail",
 			opt: types.OnlineGetOpt{
-				FeatureNames: inconsistentFeatures.Names(),
-				EntityKey:    "1234",
+				FeatureFullNames: inconsistentFeatures.FullNames(),
+				EntityKey:        "1234",
 			},
 			features:      inconsistentFeatures,
 			expectedError: fmt.Errorf("expected 1 entity, got 2 entities"),
@@ -58,16 +58,16 @@ func TestOnlineGet(t *testing.T) {
 		{
 			description: "consistent entity type, succeed",
 			opt: types.OnlineGetOpt{
-				FeatureNames: consistentFeatures.Names(),
-				EntityKey:    "1234",
+				FeatureFullNames: consistentFeatures.FullNames(),
+				EntityKey:        "1234",
 			},
 			features:      consistentFeatures,
 			entityName:    &entityName,
 			expectedError: nil,
 			expected: &types.FeatureValues{
-				EntityName:   entityName,
-				EntityKey:    "1234",
-				FeatureNames: consistentFeatures.Names(),
+				EntityName:       entityName,
+				EntityKey:        "1234",
+				FeatureFullNames: consistentFeatures.FullNames(),
 				FeatureValueMap: map[string]interface{}{
 					"price": int64(100),
 					"model": "xiaomi",
@@ -78,7 +78,7 @@ func TestOnlineGet(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.description, func(t *testing.T) {
-			metadataStore.EXPECT().CacheListFeature(gomock.Any(), metadata.ListFeatureOpt{FeatureFullNames: &tc.opt.FeatureNames}).Return(tc.features)
+			metadataStore.EXPECT().CacheListFeature(gomock.Any(), metadata.ListFeatureOpt{FeatureFullNames: &tc.opt.FeatureFullNames}).Return(tc.features)
 			if tc.entityName != nil {
 				onlineStore.EXPECT().Get(gomock.Any(), gomock.Any()).Return(dbutil.RowMap{
 					"price": int64(100),
@@ -122,8 +122,8 @@ func TestOnlineMultiGet(t *testing.T) {
 		{
 			description: "no available features, return nil",
 			opt: types.OnlineMultiGetOpt{
-				FeatureNames: unavailableFeatures.Names(),
-				EntityKeys:   []string{"1234", "1235"},
+				FeatureFullNames: unavailableFeatures.FullNames(),
+				EntityKeys:       []string{"1234", "1235"},
 			},
 			features:      unavailableFeatures,
 			expectedError: nil,
@@ -132,8 +132,8 @@ func TestOnlineMultiGet(t *testing.T) {
 		{
 			description: "inconsistent entity type, fail",
 			opt: types.OnlineMultiGetOpt{
-				FeatureNames: inconsistentFeatures.Names(),
-				EntityKeys:   []string{"1234", "1235"},
+				FeatureFullNames: inconsistentFeatures.FullNames(),
+				EntityKeys:       []string{"1234", "1235"},
 			},
 			features:      inconsistentFeatures,
 			expectedError: fmt.Errorf("expected 1 entity, got 2 entities"),
@@ -142,26 +142,26 @@ func TestOnlineMultiGet(t *testing.T) {
 		{
 			description: "consistent entity type, succeed",
 			opt: types.OnlineMultiGetOpt{
-				FeatureNames: consistentFeatures.Names(),
-				EntityKeys:   []string{"1234", "1235"},
+				FeatureFullNames: consistentFeatures.FullNames(),
+				EntityKeys:       []string{"1234", "1235"},
 			},
 			features:      consistentFeatures,
 			entityName:    &entityName,
 			expectedError: nil,
 			expected: map[string]*types.FeatureValues{
 				"1234": {
-					EntityName:   "device",
-					EntityKey:    "1234",
-					FeatureNames: consistentFeatures.Names(),
+					EntityName:       "device",
+					EntityKey:        "1234",
+					FeatureFullNames: consistentFeatures.FullNames(),
 					FeatureValueMap: map[string]interface{}{
 						"model": "xiaomi",
 						"price": int64(100),
 					},
 				},
 				"1235": {
-					EntityName:   "device",
-					EntityKey:    "1235",
-					FeatureNames: consistentFeatures.Names(),
+					EntityName:       "device",
+					EntityKey:        "1235",
+					FeatureFullNames: consistentFeatures.FullNames(),
 					FeatureValueMap: map[string]interface{}{
 						"model": "apple",
 						"price": int64(200),
@@ -173,7 +173,7 @@ func TestOnlineMultiGet(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.description, func(t *testing.T) {
-			metadataStore.EXPECT().CacheListFeature(gomock.Any(), metadata.ListFeatureOpt{FeatureFullNames: &tc.opt.FeatureNames}).Return(tc.features)
+			metadataStore.EXPECT().CacheListFeature(gomock.Any(), metadata.ListFeatureOpt{FeatureFullNames: &tc.opt.FeatureFullNames}).Return(tc.features)
 			if tc.entityName != nil {
 				onlineStore.EXPECT().MultiGet(gomock.Any(), gomock.Any()).Return(map[string]dbutil.RowMap{
 					"1234": {