From 8bad1b540049b45eba67b57ddb6ac649e474c98f Mon Sep 17 00:00:00 2001
From: jamesread <contact@jread.com>
Date: Thu, 27 Nov 2025 00:02:08 +0000
Subject: [PATCH] fix: #703 - Entities order was non-deterministic

---
 frontend/resources/vue/views/EntitiesView.vue |  4 +-
 service/internal/api/api.go                   | 45 +++++++----
 service/internal/api/api_test.go              | 80 +++++++++++++++++++
 3 files changed, 113 insertions(+), 16 deletions(-)

diff --git a/frontend/resources/vue/views/EntitiesView.vue b/frontend/resources/vue/views/EntitiesView.vue
index 96bac38..cfef396 100644
--- a/frontend/resources/vue/views/EntitiesView.vue
+++ b/frontend/resources/vue/views/EntitiesView.vue
@@ -7,12 +7,12 @@
 		</div>
 	</Section>
 	<template v-else>
-		<Section v-for="def in entityDefinitions" :key="def.name" :title="'Entity: ' + def.title ">
+		<Section v-for="def in entityDefinitions" :key="def.title" :title="'Entity: ' + def.title ">
 			<div class = "section-content">
 				<p>{{ def.instances.length }} instances.</p>
 
 				<ul>
-					<li v-for="inst in def.instances" :key="inst.id">
+					<li v-for="inst in def.instances" :key="inst.uniqueKey">
 						<router-link :to="{ name: 'EntityDetails', params: { entityType: inst.type, entityKey: inst.uniqueKey } }">
 							{{ inst.title }}
 						</router-link>
diff --git a/service/internal/api/api.go b/service/internal/api/api.go
index d35227c..2deacda 100644
--- a/service/internal/api/api.go
+++ b/service/internal/api/api.go
@@ -914,32 +914,49 @@ func (api *oliveTinAPI) GetEntities(ctx ctx.Context, req *connect.Request[apiv1.
 		return nil, err
 	}
 
-	res := &apiv1.GetEntitiesResponse{
-		EntityDefinitions: make([]*apiv1.EntityDefinition, 0),
+	entityMap := entities.GetEntities()
+	entityNames := make([]string, 0, len(entityMap))
+	for name := range entityMap {
+		entityNames = append(entityNames, name)
 	}
+	sort.Strings(entityNames)
 
-	for name, entityInstances := range entities.GetEntities() {
+	entityDefinitions := make([]*apiv1.EntityDefinition, 0, len(entityNames))
+	for _, name := range entityNames {
 		def := &apiv1.EntityDefinition{
 			Title:            name,
 			UsedOnDashboards: findDashboardsForEntity(name, api.cfg.Dashboards),
+			Instances:        buildSortedEntityInstances(name, entityMap[name]),
 		}
+		entityDefinitions = append(entityDefinitions, def)
+	}
 
-		for _, e := range entityInstances {
-			entity := &apiv1.Entity{
-				Title:     e.Title,
-				UniqueKey: e.UniqueKey,
-				Type:      name,
-			}
-
-			def.Instances = append(def.Instances, entity)
-		}
-
-		res.EntityDefinitions = append(res.EntityDefinitions, def)
+	res := &apiv1.GetEntitiesResponse{
+		EntityDefinitions: entityDefinitions,
 	}
 
 	return connect.NewResponse(res), nil
 }
 
+func buildSortedEntityInstances(entityType string, entityInstances map[string]*entities.Entity) []*apiv1.Entity {
+	instanceKeys := make([]string, 0, len(entityInstances))
+	for key := range entityInstances {
+		instanceKeys = append(instanceKeys, key)
+	}
+	sort.Strings(instanceKeys)
+
+	instances := make([]*apiv1.Entity, 0, len(instanceKeys))
+	for _, key := range instanceKeys {
+		e := entityInstances[key]
+		instances = append(instances, &apiv1.Entity{
+			Title:     e.Title,
+			UniqueKey: e.UniqueKey,
+			Type:      entityType,
+		})
+	}
+	return instances
+}
+
 func findDashboardsForEntity(entityTitle string, dashboards []*config.DashboardComponent) []string {
 	var foundDashboards []string
 
diff --git a/service/internal/api/api_test.go b/service/internal/api/api_test.go
index cd954b9..68341ae 100644
--- a/service/internal/api/api_test.go
+++ b/service/internal/api/api_test.go
@@ -12,6 +12,7 @@ import (
 	apiv1 "github.com/OliveTin/OliveTin/gen/olivetin/api/v1"
 	apiv1connect "github.com/OliveTin/OliveTin/gen/olivetin/api/v1/apiv1connect"
 	config "github.com/OliveTin/OliveTin/internal/config"
+	"github.com/OliveTin/OliveTin/internal/entities"
 	"github.com/OliveTin/OliveTin/internal/executor"
 
 	"net/http"
@@ -93,3 +94,82 @@ func TestGetActionsAndStart(t *testing.T) {
 
 	defer conn.Close()
 }
+
+func TestGetEntities(t *testing.T) {
+	cfg := config.DefaultConfig()
+
+	ts, client := getNewTestServerAndClient(t, cfg)
+	defer ts.Close()
+
+	setupTestEntities()
+
+	resp, err := client.GetEntities(context.Background(), connect.NewRequest(&apiv1.GetEntitiesRequest{}))
+
+	assert.NoError(t, err, "GetEntities should not return an error")
+	assert.NotNil(t, resp, "GetEntities response should not be nil")
+	assert.NotNil(t, resp.Msg, "GetEntities response message should not be nil")
+
+	entityDefinitions := resp.Msg.EntityDefinitions
+	assert.Equal(t, 3, len(entityDefinitions), "Should return 3 entity definitions")
+
+	validateEntityOrderAndStructure(t, entityDefinitions)
+	validateNoDuplicates(t, entityDefinitions)
+	validateConsistency(t, client, entityDefinitions)
+}
+
+func setupTestEntities() {
+	entities.ClearEntities("server")
+	entities.ClearEntities("database")
+	entities.ClearEntities("application")
+
+	entities.AddEntity("server", "zebra", map[string]any{"title": "Server Zebra", "hostname": "zebra.example.com"})
+	entities.AddEntity("server", "alpha", map[string]any{"title": "Server Alpha", "hostname": "alpha.example.com"})
+	entities.AddEntity("server", "beta", map[string]any{"title": "Server Beta", "hostname": "beta.example.com"})
+
+	entities.AddEntity("database", "mysql", map[string]any{"title": "MySQL Database", "type": "mysql"})
+	entities.AddEntity("database", "postgres", map[string]any{"title": "PostgreSQL Database", "type": "postgres"})
+
+	entities.AddEntity("application", "webapp", map[string]any{"title": "Web Application", "port": 8080})
+}
+
+func validateEntityOrderAndStructure(t *testing.T, entityDefinitions []*apiv1.EntityDefinition) {
+	assert.Equal(t, "application", entityDefinitions[0].Title, "First entity should be 'application' (alphabetically first)")
+	assert.Equal(t, 1, len(entityDefinitions[0].Instances), "Application should have 1 instance")
+	assert.Equal(t, "webapp", entityDefinitions[0].Instances[0].UniqueKey, "Application instance should be 'webapp'")
+
+	assert.Equal(t, "database", entityDefinitions[1].Title, "Second entity should be 'database' (alphabetically second)")
+	assert.Equal(t, 2, len(entityDefinitions[1].Instances), "Database should have 2 instances")
+	assert.Equal(t, "mysql", entityDefinitions[1].Instances[0].UniqueKey, "First database instance should be 'mysql' (alphabetically first)")
+	assert.Equal(t, "postgres", entityDefinitions[1].Instances[1].UniqueKey, "Second database instance should be 'postgres' (alphabetically second)")
+
+	assert.Equal(t, "server", entityDefinitions[2].Title, "Third entity should be 'server' (alphabetically third)")
+	assert.Equal(t, 3, len(entityDefinitions[2].Instances), "Server should have 3 instances")
+	assert.Equal(t, "alpha", entityDefinitions[2].Instances[0].UniqueKey, "First server instance should be 'alpha' (alphabetically first)")
+	assert.Equal(t, "beta", entityDefinitions[2].Instances[1].UniqueKey, "Second server instance should be 'beta' (alphabetically second)")
+	assert.Equal(t, "zebra", entityDefinitions[2].Instances[2].UniqueKey, "Third server instance should be 'zebra' (alphabetically third)")
+}
+
+func validateNoDuplicates(t *testing.T, entityDefinitions []*apiv1.EntityDefinition) {
+	instanceKeys := make(map[string]map[string]bool)
+	for _, def := range entityDefinitions {
+		instanceKeys[def.Title] = make(map[string]bool)
+		for _, inst := range def.Instances {
+			assert.False(t, instanceKeys[def.Title][inst.UniqueKey], "Instance key %s should not be duplicated in entity %s", inst.UniqueKey, def.Title)
+			instanceKeys[def.Title][inst.UniqueKey] = true
+		}
+	}
+}
+
+func validateConsistency(t *testing.T, client apiv1connect.OliveTinApiServiceClient, entityDefinitions []*apiv1.EntityDefinition) {
+	resp2, err2 := client.GetEntities(context.Background(), connect.NewRequest(&apiv1.GetEntitiesRequest{}))
+	assert.NoError(t, err2, "Second GetEntities call should not return an error")
+	assert.Equal(t, len(entityDefinitions), len(resp2.Msg.EntityDefinitions), "Second call should return same number of entity definitions")
+
+	for i, def := range entityDefinitions {
+		assert.Equal(t, def.Title, resp2.Msg.EntityDefinitions[i].Title, "Entity order should be consistent across calls")
+		assert.Equal(t, len(def.Instances), len(resp2.Msg.EntityDefinitions[i].Instances), "Instance count should be consistent")
+		for j, inst := range def.Instances {
+			assert.Equal(t, inst.UniqueKey, resp2.Msg.EntityDefinitions[i].Instances[j].UniqueKey, "Instance order should be consistent across calls")
+		}
+	}
+}