4
0
Эх сурвалжийг харах

fix: address CodeRabbit review on entity display-name dedup

Extract display-name lookup and field serialization helpers to stay
within cyclomatic complexity limits, pick a deterministic raw key when
field names differ only by case, and omit only that selected key from
API field responses.

Co-authored-by: Cursor <cursoragent@cursor.com>
jamesread 1 өдөр өмнө
parent
commit
ee5515f1bc

+ 13 - 6
service/internal/api/api.go

@@ -1736,7 +1736,7 @@ func entityListFields(data any, properties []config.EntityProperty) map[string]s
 	displayFieldKey := entities.DisplayNameFieldKey(data)
 	fields := make(map[string]string, len(properties))
 	for _, property := range properties {
-		if entityFieldIsDisplayName(property.Name, displayFieldKey) {
+		if entityPropertyIsDisplayName(property.Name, displayFieldKey) {
 			continue
 		}
 		fields[property.Name] = entityPropertyValue(data, property.Name)
@@ -1792,10 +1792,13 @@ func serializeEntityFields(data any) map[string]string {
 		return nil
 	}
 
-	displayFieldKey := entities.DisplayNameFieldKey(data)
-	fields := make(map[string]string)
+	return serializeEntityFieldsFromMap(dataMap, entities.DisplayNameFieldKey(data))
+}
+
+func serializeEntityFieldsFromMap(dataMap map[string]any, omitFieldKey string) map[string]string {
+	fields := make(map[string]string, len(dataMap))
 	for k, v := range dataMap {
-		if entityFieldIsDisplayName(k, displayFieldKey) {
+		if entityFieldIsSelectedDisplayName(k, omitFieldKey) {
 			continue
 		}
 		fields[k] = fmt.Sprintf("%v", v)
@@ -1803,8 +1806,12 @@ func serializeEntityFields(data any) map[string]string {
 	return fields
 }
 
-func entityFieldIsDisplayName(fieldName, displayFieldKey string) bool {
-	return displayFieldKey != "" && strings.EqualFold(fieldName, displayFieldKey)
+func entityFieldIsSelectedDisplayName(fieldName, displayFieldKey string) bool {
+	return displayFieldKey != "" && fieldName == displayFieldKey
+}
+
+func entityPropertyIsDisplayName(propertyName, displayFieldKey string) bool {
+	return displayFieldKey != "" && strings.EqualFold(propertyName, displayFieldKey)
 }
 
 func (api *oliveTinAPI) RestartAction(ctx ctx.Context, req *connect.Request[apiv1.RestartActionRequest]) (*connect.Response[apiv1.StartActionResponse], error) {

+ 32 - 0
service/internal/api/api_entities_list_test.go

@@ -152,6 +152,38 @@ func TestGetEntityOmitsDisplayNameFieldWhenPropertiesUnset(t *testing.T) {
 	assert.NotContains(t, resp.Msg.Fields, "title")
 }
 
+func TestGetEntityRetainsNonSelectedDisplayNameCasingVariant(t *testing.T) {
+	entities.ClearEntitiesOfType("vehicle")
+	entities.AddEntity("vehicle", "0", map[string]any{
+		"title":  "lower-title-value",
+		"Title":  "upper-title-value",
+		"status": "parked",
+	})
+	t.Cleanup(func() {
+		entities.ClearEntitiesOfType("vehicle")
+	})
+
+	cfg := config.DefaultConfig()
+	cfg.Entities = []*config.EntityFile{{Name: "vehicle"}}
+	cfg.Sanitize()
+
+	ex := executor.DefaultExecutor(cfg)
+	ex.RebuildActionMap()
+	ts, client := getNewTestServerAndClientWithExecutor(cfg, ex)
+	defer ts.Close()
+
+	resp, err := client.GetEntity(context.Background(), connect.NewRequest(&apiv1.GetEntityRequest{
+		Type:      "vehicle",
+		UniqueKey: "0",
+	}))
+	require.NoError(t, err)
+
+	assert.Equal(t, "upper-title-value", resp.Msg.Title)
+	assert.Equal(t, "lower-title-value", resp.Msg.Fields["title"])
+	assert.NotContains(t, resp.Msg.Fields, "Title")
+	assert.Equal(t, "parked", resp.Msg.Fields["status"])
+}
+
 func TestGetEntityOmitsDisplayNamePropertyFromConfiguredFields(t *testing.T) {
 	entities.ClearEntitiesOfType("vehicle")
 	entities.AddEntity("vehicle", "0", map[string]any{

+ 17 - 0
service/internal/entities/entities_test.go

@@ -61,6 +61,23 @@ func TestDisplayNameFieldKey(t *testing.T) {
 	assert.Equal(t, "", DisplayNameFieldKey("not a map"))
 }
 
+func TestDisplayNameFieldKey_caseCollisionIsDeterministic(t *testing.T) {
+	data := map[string]any{
+		"title": "lower",
+		"Title": "upper",
+	}
+
+	assert.Equal(t, "Title", DisplayNameFieldKey(data))
+
+	ClearEntitiesOfType("display_name_collision")
+	defer ClearEntitiesOfType("display_name_collision")
+
+	AddEntity("display_name_collision", "0", data)
+	ordered := GetEntityInstancesOrdered("display_name_collision")
+	require.Len(t, ordered, 1)
+	assert.Equal(t, "upper", ordered[0].Title)
+}
+
 func TestGetEntityInstancesOrdered_emptyOrMissing(t *testing.T) {
 	ordered := GetEntityInstancesOrdered("nonexistent_type")
 	assert.Nil(t, ordered)

+ 30 - 18
service/internal/entities/storage.go

@@ -126,32 +126,45 @@ func AddEntity(entityName string, entityKey string, data any) {
 
 var entityDisplayNameCandidates = []string{"title", "name", "id", "hostname", "host", "label"}
 
-//gocyclo:ignore
-func entityDisplayNameLookup(data map[string]any) (fieldKey string, value string, found bool) {
+func entityDisplayNameKeysByLower(data map[string]any) map[string]string {
 	keys := make(map[string]string, len(data))
 	for k := range data {
-		keys[strings.ToLower(k)] = k
-	}
-
-	for _, candidate := range entityDisplayNameCandidates {
-		lookupKey, exists := keys[strings.ToLower(candidate)]
-		if !exists {
+		lower := strings.ToLower(k)
+		existing, exists := keys[lower]
+		if exists && k >= existing {
 			continue
 		}
+		keys[lower] = k
+	}
+	return keys
+}
 
-		rawValue, ok := data[lookupKey]
-		if !ok {
-			continue
-		}
+func entityDisplayNameCandidateValue(data map[string]any, keysByLower map[string]string, candidate string) (fieldKey string, value string, ok bool) {
+	lookupKey, exists := keysByLower[strings.ToLower(candidate)]
+	if !exists {
+		return "", "", false
+	}
 
-		valueStr, ok := rawValue.(string)
-		if !ok {
-			continue
-		}
+	rawValue, exists := data[lookupKey]
+	if !exists {
+		return "", "", false
+	}
 
-		return lookupKey, valueStr, true
+	valueStr, isString := rawValue.(string)
+	if !isString {
+		return "", "", false
 	}
 
+	return lookupKey, valueStr, true
+}
+
+func entityDisplayNameLookup(data map[string]any) (fieldKey string, value string, found bool) {
+	keysByLower := entityDisplayNameKeysByLower(data)
+	for _, candidate := range entityDisplayNameCandidates {
+		if fieldKey, value, ok := entityDisplayNameCandidateValue(data, keysByLower, candidate); ok {
+			return fieldKey, value, true
+		}
+	}
 	return "", "", false
 }
 
@@ -170,7 +183,6 @@ func DisplayNameFieldKey(data any) string {
 	return fieldKey
 }
 
-//gocyclo:ignore
 func findEntityTitle(data any) string {
 	mapData, ok := data.(map[string]any)
 	if !ok {