Skip to content
Merged
12 changes: 7 additions & 5 deletions Sources/ColumbaApp/Services/AppServices.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2854,11 +2854,13 @@ public final class AppServices {
// conversation title would otherwise stay stuck on the "Peer <hash>"
// fallback even though the announce tells us the real name. This is
// UPDATE-only (never creates a conversation for a bare announce) and
// only fills an empty/nil name (never clobbers one we already have).
if !displayName.isEmpty, let repo = self.messageRepository,
let convo = try? await repo.fetchConversation(data) {
if (convo.displayName ?? "").isEmpty {
try? await repo.updateDisplayName(data, displayName: displayName)
// only fills an empty/nil name or the exact generated hash fallback
// (never clobbers a custom or previously announced name).
if !displayName.isEmpty, let repo = self.messageRepository {
if (try? await repo.applyAnnouncedDisplayName(
data,
displayName: displayName
)) == true {
DiagLog.log("[RNS] stamped display name onto convo \(data.map { String(format: "%02x", $0) }.joined().prefix(8))")
}
Comment thread
greptile-apps[bot] marked this conversation as resolved.
}
Expand Down
47 changes: 47 additions & 0 deletions Sources/ColumbaApp/Services/MessageRepository.swift
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ public actor MessageRepository {
/// messages already use `IncomingMessageHandler.messageReceivedNotification`.
public static let conversationActivityNotification =
Notification.Name("network.columba.conversationActivity")
/// Posted after durable conversation metadata changes without a new message.
public static let conversationMetadataChangedNotification =
Notification.Name("network.columba.conversationMetadataChanged")

public static let conversationHashUserInfoKey = "conversationHash"
public static let stagedRetryMarker = "columba-app-retry-staged-v1"
Expand Down Expand Up @@ -150,6 +153,50 @@ public actor MessageRepository {
try await database.updateDisplayName(hash: conversationHash, displayName: displayName)
}

/// Apply an announced peer name only while the durable row still contains
/// an empty name or its generated hash placeholder. The predicate and write
/// execute in one SQL statement so a concurrently saved custom nickname
/// cannot be overwritten between a read and a later update.
@discardableResult
public func applyAnnouncedDisplayName(
_ conversationHash: Data,
displayName: String
) async throws -> Bool {
guard !displayName.isEmpty else { return false }
let generatedFallback = AppDataParser.generatedConversationName(
destinationHash: conversationHash
)
let updated = try await replacementPool.write { db in
try db.execute(
sql: """
UPDATE conversations
SET display_name = ?, updated_at = ?
WHERE destination_hash = ?
AND (
display_name IS NULL
OR display_name = ''
OR display_name = ? COLLATE NOCASE
)
""",
arguments: [
displayName,
Date().timeIntervalSince1970,
conversationHash,
generatedFallback,
]
)
return db.changesCount == 1
}
if updated {
NotificationCenter.default.post(
name: Self.conversationMetadataChangedNotification,
object: nil,
userInfo: [Self.conversationHashUserInfoKey: conversationHash]
)
}
return updated
}

// MARK: - Icon Appearance

/// Update peer icon appearance for a conversation.
Expand Down
14 changes: 14 additions & 0 deletions Sources/ColumbaApp/ViewModels/ChatsViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ public final class ChatsViewModel {
private var inProcessObserver: NSObjectProtocol?
private var conversationReadObserver: NSObjectProtocol?
private var conversationActivityObserver: NSObjectProtocol?
private var conversationMetadataObserver: NSObjectProtocol?
private var conversationLoadGeneration: UInt64 = 0
private var activeConversationLoadCount: Int = 0
private var activeConversationRefreshCount: Int = 0
Expand Down Expand Up @@ -207,6 +208,16 @@ public final class ChatsViewModel {
await self?.loadConversations()
}
}

conversationMetadataObserver = NotificationCenter.default.addObserver(
forName: MessageRepository.conversationMetadataChangedNotification,
object: nil,
queue: .main
) { [weak self] _ in
Task { @MainActor in
await self?.loadConversations()
}
}
}

deinit {
Expand All @@ -219,6 +230,9 @@ public final class ChatsViewModel {
if let observer = conversationActivityObserver {
NotificationCenter.default.removeObserver(observer)
}
if let observer = conversationMetadataObserver {
NotificationCenter.default.removeObserver(observer)
}
}

// MARK: - Public Methods
Expand Down
24 changes: 24 additions & 0 deletions Sources/RNSAPI/Util/AppDataParser.swift
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,30 @@ public enum AppDataParser {
return ""
}

/// Whether an announced name may replace a conversation's current name.
///
/// Inbound messages can create a conversation before its peer announce is
/// observed. That row receives the generated `Peer <hash>` placeholder, so
/// treating every non-empty name as user-owned leaves the placeholder stuck
/// forever. Replace only an empty value or the exact generated placeholder;
/// preserve custom nicknames and unrelated peer-like names.
public static func shouldReplaceConversationName(
_ existingName: String?,
destinationHash: Data
) -> Bool {
guard let existingName, !existingName.isEmpty else { return true }
let generatedFallback = generatedConversationName(destinationHash: destinationHash)
return existingName.caseInsensitiveCompare(generatedFallback) == .orderedSame
}

/// The placeholder used when a message arrives before its peer announce.
public static func generatedConversationName(destinationHash: Data) -> String {
let hashPrefix = destinationHash.prefix(4)
.map { String(format: "%02x", $0) }
.joined()
return "Peer \(hashPrefix)"
}

/// Pull a UTF-8 string out of a `.string` or `.binary` MessagePackValue.
/// `.nil` / other cases → nil (so callers can fall back to "").
private static func string(_ value: MessagePackValue?) -> String? {
Expand Down
37 changes: 37 additions & 0 deletions Tests/ColumbaAppTests/AnnounceClassificationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,43 @@ final class MessageRepositoryAtomicReplacementTests: XCTestCase {
}
}

func testAnnouncedNameAtomicallyReplacesGeneratedFallback() async throws {
let databaseURL = temporaryDatabaseURL()
defer { removeDatabase(at: databaseURL) }
let repository = try MessageRepository(grdbPath: databaseURL.path)
let destination = Data([0x05, 0xc5, 0x7e, 0x42] + Array(repeating: 0xaa, count: 12))

try await repository.ensureConversation(destination, displayName: "Peer 05c57e42")
let applied = try await repository.applyAnnouncedDisplayName(
destination,
displayName: "Hermes Homelab"
)

let storedConversation = try await repository.fetchConversation(destination)
let conversation = try XCTUnwrap(storedConversation)
XCTAssertTrue(applied)
XCTAssertEqual(conversation.displayName, "Hermes Homelab")
}

func testAnnouncedNameCompareAndSetPreservesCurrentCustomName() async throws {
let databaseURL = temporaryDatabaseURL()
defer { removeDatabase(at: databaseURL) }
let repository = try MessageRepository(grdbPath: databaseURL.path)
let destination = Data([0x05, 0xc5, 0x7e, 0x42] + Array(repeating: 0xaa, count: 12))

try await repository.ensureConversation(destination, displayName: "Peer 05c57e42")
try await repository.updateDisplayName(destination, displayName: "My Server")
let applied = try await repository.applyAnnouncedDisplayName(
destination,
displayName: "Hermes Homelab"
)

let storedConversation = try await repository.fetchConversation(destination)
let conversation = try XCTUnwrap(storedConversation)
XCTAssertFalse(applied)
XCTAssertEqual(conversation.displayName, "My Server")
}

func testRetryReplacementRekeysExactlyOneDurableRow() async throws {
let databaseURL = temporaryDatabaseURL()
defer { removeDatabase(at: databaseURL) }
Expand Down
48 changes: 48 additions & 0 deletions Tests/ColumbaAppTests/MicronParserTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -772,6 +772,54 @@ final class MessageRepositoryAdapterTests: XCTestCase {
XCTAssertEqual(viewModel.conversations.map(\.destinationHash), [newerHash, olderHash])
}

@MainActor
func testAnnouncedDisplayNameRefreshesVisibleChatList() async throws {
let databaseURL = FileManager.default.temporaryDirectory
.appendingPathComponent("columba-chat-announced-name-\(UUID().uuidString).sqlite")
defer {
try? FileManager.default.removeItem(at: databaseURL)
try? FileManager.default.removeItem(atPath: databaseURL.path + "-shm")
try? FileManager.default.removeItem(atPath: databaseURL.path + "-wal")
}

let repository = try MessageRepository(grdbPath: databaseURL.path)
let viewModel = ChatsViewModel(
repository: repository,
notificationObserver: NotificationObserver()
)
let destination = Data([0x05, 0xc5, 0x7e, 0x42] + Array(repeating: 0xaa, count: 12))
let message = RNSAPI.LXMessage(
destinationHash: destination,
sourceIdentity: nil,
content: Data("hello".utf8)
)
message.hash = Data(repeating: 0xbb, count: 32)
message.timestamp = 100
message.incoming = true
message.sourceHash = destination
message.state = .received
message.method = .opportunistic
try await repository.saveMessage(message)
try await repository.ensureConversation(destination, displayName: "Peer 05c57e42")
await viewModel.loadConversations()
for _ in 0..<100 where viewModel.conversations.first?.displayName != "Peer 05c57e42" {
try await Task.sleep(for: .milliseconds(10))
}
XCTAssertEqual(viewModel.conversations.first?.displayName, "Peer 05c57e42")

let applied = try await repository.applyAnnouncedDisplayName(
destination,
displayName: "Hermes Homelab"
)
XCTAssertTrue(applied)

for _ in 0..<100 where viewModel.conversations.first?.displayName != "Hermes Homelab" {
try await Task.sleep(for: .milliseconds(10))
}

XCTAssertEqual(viewModel.conversations.first?.displayName, "Hermes Homelab")
}

@MainActor
func testConversationReadNotificationClearsVisibleUnreadBadge() async throws {
let databaseURL = FileManager.default.temporaryDirectory
Expand Down
13 changes: 13 additions & 0 deletions Tests/RNSAPITests/AppDataParserTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,19 @@ final class AppDataParserTests: XCTestCase {
XCTAssertEqual(AppDataParser.displayName(from: Data("LegacyName".utf8), aspect: "lxmf.delivery"), "LegacyName")
}

func testAnnouncedNameReplacesOnlyEmptyOrGeneratedHashFallback() {
let destinationHash = Data([0x05, 0xc5, 0x7e, 0x42] + Array(repeating: 0xaa, count: 12))

XCTAssertTrue(AppDataParser.shouldReplaceConversationName(nil, destinationHash: destinationHash))
XCTAssertTrue(AppDataParser.shouldReplaceConversationName("", destinationHash: destinationHash))
XCTAssertTrue(AppDataParser.shouldReplaceConversationName("Peer 05c57e42", destinationHash: destinationHash))
XCTAssertTrue(AppDataParser.shouldReplaceConversationName("Peer 05C57E42", destinationHash: destinationHash))

XCTAssertFalse(AppDataParser.shouldReplaceConversationName("Hermes", destinationHash: destinationHash))
XCTAssertFalse(AppDataParser.shouldReplaceConversationName("Peer Alice", destinationHash: destinationHash))
XCTAssertFalse(AppDataParser.shouldReplaceConversationName("Peer deadbeef", destinationHash: destinationHash))
}

// MARK: - Propagation (the regression)

func testPropagationNoNameIsEmptyNotFalse() {
Expand Down
Loading