[management] fix the issue with duplicated peers with the same key (#5053)

management/server/migration/migration.go

@@ -404,10 +404,11 @@ func CreateIndexIfNotExists[T any](ctx context.Context, db *gorm.DB, indexName s
 	if dialect == "mysql" {
 		var withLength []string
 		for _, col := range columns {
-			if col == "ip" || col == "dns_label" {
-				withLength = append(withLength, fmt.Sprintf("%s(64)", col))
+			quotedCol := fmt.Sprintf("`%s`", col)
+			if col == "ip" || col == "dns_label" || col == "key" {
+				withLength = append(withLength, fmt.Sprintf("%s(64)", quotedCol))
 			} else {
-				withLength = append(withLength, col)
+				withLength = append(withLength, quotedCol)
 			}
 		}
 		columnClause = strings.Join(withLength, ", ")
@@ -487,3 +488,54 @@ func MigrateJsonToTable[T any](ctx context.Context, db *gorm.DB, columnName stri
 	log.WithContext(ctx).Infof("Migration of JSON field %s from table %s into separate table completed", columnName, tableName)
 	return nil
 }
+
+func RemoveDuplicatePeerKeys(ctx context.Context, db *gorm.DB) error {
+	if !db.Migrator().HasTable("peers") {
+		log.WithContext(ctx).Debug("peers table does not exist, skipping duplicate key cleanup")
+		return nil
+	}
+
+	keyColumn := GetColumnName(db, "key")
+
+	var duplicates []struct {
+		Key   string
+		Count int64
+	}
+
+	if err := db.Table("peers").
+		Select(keyColumn + ", COUNT(*) as count").
+		Group(keyColumn).
+		Having("COUNT(*) > 1").
+		Find(&duplicates).Error; err != nil {
+		return fmt.Errorf("find duplicate keys: %w", err)
+	}
+
+	if len(duplicates) == 0 {
+		return nil
+	}
+
+	log.WithContext(ctx).Warnf("Found %d duplicate peer keys, cleaning up", len(duplicates))
+
+	for _, dup := range duplicates {
+		var peerIDs []string
+		if err := db.Table("peers").
+			Select("id").
+			Where(keyColumn+" = ?", dup.Key).
+			Order("peer_status_last_seen DESC").
+			Pluck("id", &peerIDs).Error; err != nil {
+			return fmt.Errorf("get peers for key: %w", err)
+		}
+
+		if len(peerIDs) <= 1 {
+			continue
+		}
+
+		idsToDelete := peerIDs[1:]
+
+		if err := db.Table("peers").Where("id IN ?", idsToDelete).Delete(nil).Error; err != nil {
+			return fmt.Errorf("delete duplicate peers: %w", err)
+		}
+	}
+
+	return nil
+}

management/server/migration/migration_test.go

@@ -340,3 +340,104 @@ func TestCreateIndexIfExists(t *testing.T) {
 	exist = db.Migrator().HasIndex(&nbpeer.Peer{}, indexName)
 	assert.True(t, exist, "Should have the index")
 }
+
+type testPeer struct {
+	ID                  string `gorm:"primaryKey"`
+	Key                 string `gorm:"index"`
+	PeerStatusLastSeen  time.Time
+	PeerStatusConnected bool
+}
+
+func (testPeer) TableName() string {
+	return "peers"
+}
+
+func setupPeerTestDB(t *testing.T) *gorm.DB {
+	t.Helper()
+	db := setupDatabase(t)
+	_ = db.Migrator().DropTable(&testPeer{})
+	err := db.AutoMigrate(&testPeer{})
+	require.NoError(t, err, "Failed to auto-migrate tables")
+	return db
+}
+
+func TestRemoveDuplicatePeerKeys_NoDuplicates(t *testing.T) {
+	db := setupPeerTestDB(t)
+
+	now := time.Now()
+	peers := []testPeer{
+		{ID: "peer1", Key: "key1", PeerStatusLastSeen: now},
+		{ID: "peer2", Key: "key2", PeerStatusLastSeen: now},
+		{ID: "peer3", Key: "key3", PeerStatusLastSeen: now},
+	}
+
+	for _, p := range peers {
+		err := db.Create(&p).Error
+		require.NoError(t, err)
+	}
+
+	err := migration.RemoveDuplicatePeerKeys(context.Background(), db)
+	require.NoError(t, err)
+
+	var count int64
+	db.Model(&testPeer{}).Count(&count)
+	assert.Equal(t, int64(len(peers)), count, "All peers should remain when no duplicates")
+}
+
+func TestRemoveDuplicatePeerKeys_WithDuplicates(t *testing.T) {
+	db := setupPeerTestDB(t)
+
+	now := time.Now()
+	peers := []testPeer{
+		{ID: "peer1", Key: "key1", PeerStatusLastSeen: now.Add(-2 * time.Hour)},
+		{ID: "peer2", Key: "key1", PeerStatusLastSeen: now.Add(-1 * time.Hour)},
+		{ID: "peer3", Key: "key1", PeerStatusLastSeen: now},
+		{ID: "peer4", Key: "key2", PeerStatusLastSeen: now},
+		{ID: "peer5", Key: "key3", PeerStatusLastSeen: now.Add(-1 * time.Hour)},
+		{ID: "peer6", Key: "key3", PeerStatusLastSeen: now},
+	}
+
+	for _, p := range peers {
+		err := db.Create(&p).Error
+		require.NoError(t, err)
+	}
+
+	err := migration.RemoveDuplicatePeerKeys(context.Background(), db)
+	require.NoError(t, err)
+
+	var count int64
+	db.Model(&testPeer{}).Count(&count)
+	assert.Equal(t, int64(3), count, "Should have 3 peers after removing duplicates")
+
+	var remainingPeers []testPeer
+	err = db.Find(&remainingPeers).Error
+	require.NoError(t, err)
+
+	remainingIDs := make(map[string]bool)
+	for _, p := range remainingPeers {
+		remainingIDs[p.ID] = true
+	}
+
+	assert.True(t, remainingIDs["peer3"], "peer3 should remain (most recent for key1)")
+	assert.True(t, remainingIDs["peer4"], "peer4 should remain (only peer for key2)")
+	assert.True(t, remainingIDs["peer6"], "peer6 should remain (most recent for key3)")
+
+	assert.False(t, remainingIDs["peer1"], "peer1 should be deleted (older duplicate)")
+	assert.False(t, remainingIDs["peer2"], "peer2 should be deleted (older duplicate)")
+	assert.False(t, remainingIDs["peer5"], "peer5 should be deleted (older duplicate)")
+}
+
+func TestRemoveDuplicatePeerKeys_EmptyTable(t *testing.T) {
+	db := setupPeerTestDB(t)
+
+	err := migration.RemoveDuplicatePeerKeys(context.Background(), db)
+	require.NoError(t, err, "Should not fail on empty table")
+}
+
+func TestRemoveDuplicatePeerKeys_NoTable(t *testing.T) {
+	db := setupDatabase(t)
+	_ = db.Migrator().DropTable(&testPeer{})
+
+	err := migration.RemoveDuplicatePeerKeys(context.Background(), db)
+	require.NoError(t, err, "Should not fail when table does not exist")
+}