Repository: oom-ai/oomstore
Pull request #816 (2021-12-24 08:11:17)

refactor(database/metadata): set full_name as unique key

wfxr: @jinghancc How about instead adding an unique constraint to `name` and `group` ? 
jinghancc: > @jinghancc How about instead adding an unique constraint to `name` and `group` ? In the future we may remove the `full_name` field because it can be easily concated by `group:name`.

@wfxr do you mean `UNIQUE(group_id, name)`?
wfxr: > @wfxr do you mean `UNIQUE(group_id, name)`?

@jinghancc exactly

Patch:
diff --git a/internal/database/metadata/mysql/schema.go b/internal/database/metadata/mysql/schema.go
index fbd654e3..bf764e33 100644
--- a/internal/database/metadata/mysql/schema.go
+++ b/internal/database/metadata/mysql/schema.go
@@ -11,7 +11,8 @@ var META_TABLE_SCHEMAS = map[string]string{
 			description   	VARCHAR(128)	DEFAULT '',
 			create_time   	TIMESTAMP    	NOT	NULL DEFAULT CURRENT_TIMESTAMP,
 			modify_time   	TIMESTAMP    	NOT	NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
-			UNIQUE (name)
+			UNIQUE (group_id, name),
+			UNIQUE (full_name)
 		);
 		`,
 	"feature_group": `
diff --git a/internal/database/metadata/postgres/schema.go b/internal/database/metadata/postgres/schema.go
index 036a0cfb..0f0afc37 100644
--- a/internal/database/metadata/postgres/schema.go
+++ b/internal/database/metadata/postgres/schema.go
@@ -29,7 +29,8 @@ var META_TABLE_SCHEMAS = map[string]string{
 			description 	VARCHAR(128) 	DEFAULT '',
 			create_time   	TIMESTAMP    	NOT NULL DEFAULT CURRENT_TIMESTAMP,
 			modify_time   	TIMESTAMP    	NOT NULL DEFAULT CURRENT_TIMESTAMP,
-			UNIQUE (name)
+			UNIQUE (group_id, name),
+			UNIQUE (full_name)
 		);
 		COMMENT ON COLUMN feature.value_type    IS 'data type of feature value';
 		COMMENT ON COLUMN feature.full_name		IS '<group_name>:<feature_name>';
diff --git a/internal/database/metadata/sqlite/schema.go b/internal/database/metadata/sqlite/schema.go
index 7a45d341..bb3c495c 100644
--- a/internal/database/metadata/sqlite/schema.go
+++ b/internal/database/metadata/sqlite/schema.go
@@ -19,7 +19,8 @@ var META_TABLE_SCHEMAS = map[string]string{
 			description   	VARCHAR(128)	DEFAULT '',
 			create_time   	TIMESTAMP    	NOT	NULL DEFAULT CURRENT_TIMESTAMP,
 			modify_time   	TIMESTAMP    	NOT	NULL DEFAULT CURRENT_TIMESTAMP,
-			UNIQUE (name),
+			UNIQUE (group_id, name),
+			UNIQUE (full_name),
 			FOREIGN KEY (group_id) REFERENCES feature_group(id)
 		);
 		`,
diff --git a/pkg/oomstore/apply.go b/pkg/oomstore/apply.go
index 0420c807..5ec8167e 100644
--- a/pkg/oomstore/apply.go
+++ b/pkg/oomstore/apply.go
@@ -130,6 +130,7 @@ func (s *OomStore) applyFeature(ctx context.Context, tx metadata.DBStore, newFea
 		}
 		_, err = tx.CreateFeature(ctx, metadata.CreateFeatureOpt{
 			FeatureName: newFeature.Name,
+			FullName:    fmt.Sprintf("%s.%s", group.Name, newFeature.Name),
 			GroupID:     group.ID,
 			Description: newFeature.Description,
 			ValueType:   valueType,
@@ -190,11 +191,25 @@ func buildApplyStage(ctx context.Context, opt apply.ApplyOpt) (*apply.ApplyStage
 				return nil, err
 			}
 
-			group.Group = group.Name
-			stage.NewGroups = append(stage.NewGroups, buildApplyGroup(group, group.EntityName))
+			// We don't want group.Features to have values.
+			// The whole stage should be a flat structure, not a nested structure.
+			stage.NewGroups = append(stage.NewGroups, apply.Group{
+				Kind:        "Group",
+				Name:        group.Name,
+				Group:       group.Name,
+				EntityName:  group.EntityName,
+				Category:    group.Category,
+				Description: group.Description,
+			})
 
 			for _, feature := range group.Features {
-				stage.NewFeatures = append(stage.NewFeatures, buildApplyFeature(feature, group.Name))
+				stage.NewFeatures = append(stage.NewFeatures, apply.Feature{
+					Kind:        "Feature",
+					Name:        feature.Name,
+					GroupName:   group.Name,
+					ValueType:   feature.ValueType,
+					Description: feature.Description,
+				})
 			}
 
 		case "Feature":
diff --git a/pkg/oomstore/feature.go b/pkg/oomstore/feature.go
index eb5c0ec7..e8228b93 100644
--- a/pkg/oomstore/feature.go
+++ b/pkg/oomstore/feature.go
@@ -64,7 +64,7 @@ func (s *OomStore) CreateBatchFeature(ctx context.Context, opt types.CreateFeatu
 
 	return s.metadata.CreateFeature(ctx, metadata.CreateFeatureOpt{
 		FeatureName: opt.FeatureName,
-		FullName:    fmt.Sprintf("%s:%s", group.Name, opt.FeatureName),
+		FullName:    fmt.Sprintf("%s.%s", group.Name, opt.FeatureName),
 		GroupID:     group.ID,
 		ValueType:   opt.ValueType,
 		Description: opt.Description,


Test patch:
diff --git a/pkg/oomstore/feature_test.go b/pkg/oomstore/feature_test.go
index 422e6a44..84629458 100644
--- a/pkg/oomstore/feature_test.go
+++ b/pkg/oomstore/feature_test.go
@@ -71,7 +71,7 @@ func TestCreateBatchFeature(t *testing.T) {
 			if tc.group.Category == types.BatchFeatureCategory {
 				metadataOpt := metadata.CreateFeatureOpt{
 					FeatureName: tc.opt.FeatureName,
-					FullName:    fmt.Sprintf("%s:%s", tc.opt.GroupName, tc.opt.FeatureName),
+					FullName:    fmt.Sprintf("%s.%s", tc.opt.GroupName, tc.opt.FeatureName),
 					GroupID:     tc.group.ID,
 					ValueType:   tc.valueType,
 					Description: tc.opt.Description,