From e48168e6d86366789167e5fa0fe981ad5e43c37f Mon Sep 17 00:00:00 2001 From: spaced4ndy <8711996+spaced4ndy@users.noreply.github.com> Date: Fri, 25 Sep 2026 14:47:00 +0400 Subject: [PATCH] improve comments --- apps/ios/Shared/Model/ChatModel.swift | 8 ++--- apps/ios/Shared/Model/DirectorySearch.swift | 16 +++++----- apps/ios/Shared/Model/SimpleXAPI.swift | 2 +- .../Shared/Views/ChatList/ChatListView.swift | 10 ++----- .../Views/ChatList/DirectorySearchView.swift | 14 ++------- apps/ios/SimpleXChat/APITypes.swift | 4 +-- .../chat/simplex/common/model/ChatModel.kt | 8 ++--- .../simplex/common/model/DirectorySearch.kt | 12 ++++---- .../chat/simplex/common/model/SimpleXAPI.kt | 5 ++-- .../src/Directory/Rpc.hs | 9 +++--- .../src/Directory/Search.hs | 4 +-- .../src/Directory/Service.hs | 9 +++--- src/Simplex/Chat/Library/Internal.hs | 3 +- src/Simplex/Chat/Library/Subscriber.hs | 3 +- src/Simplex/Chat/Protocol.hs | 10 +++---- tests/Bots/DirectoryTests.hs | 30 +++++++------------ 16 files changed, 57 insertions(+), 90 deletions(-) diff --git a/apps/ios/Shared/Model/ChatModel.swift b/apps/ios/Shared/Model/ChatModel.swift index 1b02516e2d..55b11d4086 100644 --- a/apps/ios/Shared/Model/ChatModel.swift +++ b/apps/ios/Shared/Model/ChatModel.swift @@ -298,8 +298,8 @@ class ChatItemDummyModel: ObservableObject { func sendUpdate() { objectWillChange.send() } } -// A directory search and a connection can overlap - tapping a result starts a connection while -// the search is still running - so the single progress slot records its owner. +// There is one spinner, and a search and a connection can run at once - tapping a result starts +// connecting while the search is still going - so it records which of them it belongs to. enum ConnectProgressOwner { case connect case directorySearch @@ -322,7 +322,7 @@ class ConnectProgressManager: ObservableObject { } } - // a late directory search result must not clear the spinner that now belongs to a connection + // a search finishing late must not stop the spinner if a connection has taken it over func stopConnectProgress(_ owner: ConnectProgressOwner = .connect) { if let current = self.owner, current != owner { return } connectInProgress = nil @@ -331,7 +331,7 @@ class ConnectProgressManager: ObservableObject { connectProgressByTimeout = false } - // a user-initiated cancel, and the takeover in planAndConnect, cancel whatever is running + // unlike stopConnectProgress, this cancels whoever owns the spinner func cancelConnectProgress() { let cancel = onCancel owner = nil diff --git a/apps/ios/Shared/Model/DirectorySearch.swift b/apps/ios/Shared/Model/DirectorySearch.swift index 494cd097f8..cd6dc3b3f7 100644 --- a/apps/ios/Shared/Model/DirectorySearch.swift +++ b/apps/ios/Shared/Model/DirectorySearch.swift @@ -9,13 +9,11 @@ import Foundation import SimpleXChat -// The directory's contact address, as published in docs/DIRECTORY.md. It must be the short -// link: only that form carries the address DR keys that service requests require, so the full -// links on the What's New cards cannot be substituted here. +// The directory's address, as published in docs/DIRECTORY.md. It must be the short link form - +// only that one carries the keys a service request needs; a full link fails every time. let DIRECTORY_SERVICE_LINK = "https://smp4.simplex.im/a#lXUjJW5vHYQzoLYgmi8GbxkGP41_kjefFvBrdwg-0Ok" -// A service request is a full DR handshake, so it is slower than a local API call; the user -// gets a cancellable spinner while it runs and a retry row if it times out. +// A search sets up an encrypted connection, so it takes seconds, not milliseconds. let DIRECTORY_SEARCH_TIMEOUT_SEC: Double = 10 struct DirectoryPublicLink: Decodable, Hashable { @@ -45,8 +43,8 @@ struct DirectorySearchEntry: Decodable, Hashable, Identifiable { var id: String { connectLink ?? displayName } } -// entries stay as JSONValue so they can be decoded one by one: an entry the app cannot decode -// must not fail the whole response, as it would on a future directory field +// entries stay undecoded here so they can be decoded one at a time: one entry the app does not +// understand, because a newer directory added a field, must not discard the whole response private struct DirectorySearchResponse: Decodable { var type: String var entries: [JSONValue]? @@ -65,8 +63,8 @@ func directorySearchRequestJSON(_ text: String, _ cursor: JSONValue?) -> String return encodeJSON(JSONValue.object(req)) } -// The response is a tagged object: searchResults or error. Anything else is a failure rather -// than something to parse leniently - it comes from outside the app. +// Anything but a well-formed results response is a failure, not something to salvage: it comes +// from another party, not from our own core. func parseDirectorySearchResponse(_ resp: JSONValue) -> DirectorySearchResults? { guard let r: DirectorySearchResponse = decodeJSONValue(resp), r.type == "searchResults" else { return nil diff --git a/apps/ios/Shared/Model/SimpleXAPI.swift b/apps/ios/Shared/Model/SimpleXAPI.swift index 74fb78cc75..9935bc96c0 100644 --- a/apps/ios/Shared/Model/SimpleXAPI.swift +++ b/apps/ios/Shared/Model/SimpleXAPI.swift @@ -1040,7 +1040,7 @@ func apiChangeConnectionUser(connId: Int64, userId: Int64) async throws -> Pendi if let r { throw r.unexpected } else { return nil } } -// Blocks until the directory replies or the timeout elapses. +// Blocks for up to the timeout, unlike most API calls here. func apiSearchDirectory(_ text: String, cursor: JSONValue?) async -> DirectorySearchResults? { guard let userId = ChatModel.shared.currentUser?.userId else { logger.error("apiSearchDirectory: no current user") diff --git a/apps/ios/Shared/Views/ChatList/ChatListView.swift b/apps/ios/Shared/Views/ChatList/ChatListView.swift index 73f6240aca..852d88525c 100644 --- a/apps/ios/Shared/Views/ChatList/ChatListView.swift +++ b/apps/ios/Shared/Views/ChatList/ChatListView.swift @@ -433,8 +433,7 @@ struct ChatListView: View { @ViewBuilder private var chatList: some View { if shouldShowOnboarding { - // the onboarding content stays, but below a live search bar rather than instead of - // it: a user with no conversations is exactly who needs to find some + // the search bar is shown here too: someone with no chats yet is who most needs to find some VStack(spacing: 0) { ChatListSearchBar( searchMode: $searchMode, @@ -501,8 +500,6 @@ struct ChatListView: View { return ZStack { ScrollViewReader { scrollProxy in List { - // always shown: the search field is now the way to discover chats, not only - // to filter the ones that already exist ChatListSearchBar( searchMode: $searchMode, searchFocussed: $searchFocussed, @@ -636,7 +633,7 @@ struct ChatListView: View { } } } - // the overlay covers the list, so it yields to the directory section and its own empty and retry rows + // this covers the whole list, so it must not hide the directory section or its own messages if cs.isEmpty && !chatModel.chats.isEmpty && !directorySearch.showResults { noChatsView() .scaleEffect(x: 1, y: oneHandUI ? -1 : 1, anchor: .center) @@ -921,7 +918,6 @@ struct ChatListSearchBar: View { withAnimation { searchMode = sf } } .onChange(of: searchText) { _ in - // results belong to the text that produced them directorySearch.reset() } .onChange(of: m.currentUser?.userId) { _ in @@ -1028,7 +1024,7 @@ struct ChatListSearchBar: View { } } - // The search text leaves the device, so the first time it does the user is asked first. + // whatever is typed here gets sent to the directory, so ask before the first time private func runDirectorySearch() { let text = searchTrimmed guard !text.isEmpty, !searchShowingSimplexLink else { return } diff --git a/apps/ios/Shared/Views/ChatList/DirectorySearchView.swift b/apps/ios/Shared/Views/ChatList/DirectorySearchView.swift index beb7385b3a..d22624bb0b 100644 --- a/apps/ios/Shared/Views/ChatList/DirectorySearchView.swift +++ b/apps/ios/Shared/Views/ChatList/DirectorySearchView.swift @@ -9,21 +9,18 @@ import SwiftUI import SimpleXChat -// Results of searching the directory over the service RPC. They are not chats and are never -// persisted: they live as long as the search text does. +// Directory results are not chats and are never saved: they live as long as the search text does. @MainActor class DirectorySearchModel: ObservableObject { @Published private(set) var entries: [DirectorySearchEntry] = [] @Published private(set) var loading = false @Published private(set) var failed = false - // set once a search has actually run, so the empty state can tell "not searched yet" from - // "searched and found nothing" + // tells "nothing searched yet" apart from "searched and found nothing" @Published private(set) var searched = false private var cursor: JSONValue? = nil private var searchedText = "" - // bumped on every reset, so a reply that arrives after the text, profile or host changed - // cannot repopulate a list the user has moved on from + // bumped on every reset, so a reply that arrives too late cannot refill a list already cleared private var generation = 0 var hasMore: Bool { cursor != nil } @@ -79,15 +76,12 @@ class DirectorySearchModel: ObservableObject { return } cursor = r.cursor - // the link is the identity of a result, so a row cannot appear twice across pages let known = Set(entries.map { $0.id }) let fresh = r.entries.filter { !known.contains($0.id) } entries = append ? entries + fresh : fresh } } -// Offered whenever there is search text, next to the connect-by-name row. Tapping it sends the -// text to the directory. struct SearchInDirectoryRow: View { @EnvironmentObject var theme: AppTheme @FocusState.Binding var searchFocussed: Bool @@ -159,8 +153,6 @@ struct DirectorySearchRow: View { } } -// Shown before the first directory search of the session: the search text leaves the device, -// so the user is told before it does, not after. func showDirectorySearchAlert(onSearch: @escaping () -> Void) { showAlert( NSLocalizedString("Search in Directory?", comment: "alert title"), diff --git a/apps/ios/SimpleXChat/APITypes.swift b/apps/ios/SimpleXChat/APITypes.swift index 0913842780..0ab21be679 100644 --- a/apps/ios/SimpleXChat/APITypes.swift +++ b/apps/ios/SimpleXChat/APITypes.swift @@ -716,8 +716,8 @@ private func encodeCJSON(_ value: T) -> [CChar] { encodeJSON(value).cString(using: .utf8)! } -// Type-erased JSON, so a service response can cross the API layer without it knowing which -// service produced the payload. Callers re-decode it into their own type with decodeJSONValue. +// Any JSON, so a service response can pass through this layer without it knowing the service's +// own types. Callers decode it into their own type with decodeJSONValue. public enum JSONValue: Codable, Hashable { case null case bool(Bool) diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/ChatModel.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/ChatModel.kt index 172ed30834..7be3130d49 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/ChatModel.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/ChatModel.kt @@ -45,9 +45,8 @@ import kotlin.collections.ArrayList import kotlin.random.Random import kotlin.time.* -// A directory search and a connection can overlap - tapping a result starts a connection while -// the search is still running - so the single progress slot records its owner: a late search -// result must not clear the spinner that now belongs to the connection. +// There is one spinner, and a search and a connection can run at once - tapping a result starts +// connecting while the search is still going - so it records which of them it belongs to. enum class ConnectProgressOwner { Connect, DirectorySearch } object ConnectProgressManager { @@ -68,6 +67,7 @@ object ConnectProgressManager { } } + // a search finishing late must not stop the spinner if a connection has taken it over fun stopConnectProgress(owner: ConnectProgressOwner = ConnectProgressOwner.Connect) { if (this.owner != null && this.owner != owner) return connectInProgress.value = null @@ -76,7 +76,7 @@ object ConnectProgressManager { connectProgressByTimeout.value = false } - // a user-initiated cancel, and the takeover in planAndConnect, cancel whatever is running + // unlike stopConnectProgress, this cancels whoever owns the spinner fun cancelConnectProgress() { val cancel = onCancel owner = null diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/DirectorySearch.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/DirectorySearch.kt index 198a7c60bf..bf9675adc9 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/DirectorySearch.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/DirectorySearch.kt @@ -4,13 +4,11 @@ import kotlinx.datetime.Instant import kotlinx.serialization.Serializable import kotlinx.serialization.json.* -// The directory's contact address, as published in docs/DIRECTORY.md. It must be the short -// link: only that form carries the address DR keys that service requests require, so the full -// links on the What's New cards cannot be substituted here. +// The directory's address, as published in docs/DIRECTORY.md. It must be the short link form - +// only that one carries the keys a service request needs; a full link fails every time. const val DIRECTORY_SERVICE_LINK = "https://smp4.simplex.im/a#lXUjJW5vHYQzoLYgmi8GbxkGP41_kjefFvBrdwg-0Ok" -// A service request is a full DR handshake, so it is slower than a local API call; the user -// gets a cancellable spinner while it runs and a retry row if it times out. +// A search sets up an encrypted connection, so it takes seconds, not milliseconds. const val DIRECTORY_SEARCH_TIMEOUT_SEC = 10.0 @Serializable @@ -52,8 +50,8 @@ fun directorySearchRequest(text: String, cursor: JsonObject?): JsonObject = buil if (cursor != null) put("searchCursor", cursor) } -// The response is a tagged object: searchResults or error. Anything else is treated as a failure -// rather than parsed leniently - it comes from outside the app. +// Anything but a well-formed results response is a failure, not something to salvage: it comes +// from another party, not from our own core. fun parseDirectorySearchResponse(resp: JsonObject): DirectorySearchResults? = when ((resp["type"] as? JsonPrimitive)?.contentOrNull) { "searchResults" -> { diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/SimpleXAPI.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/SimpleXAPI.kt index c039875eae..cb8de452d9 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/SimpleXAPI.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/SimpleXAPI.kt @@ -1630,9 +1630,8 @@ object ChatController { return null } - // Blocks until the directory replies or the timeout elapses, so callers must use - // withLongRunningApi, not the single-threaded withBGApi. A timeout becomes a retry row, - // not the retry alert sendCmdWithRetry would show. + // Blocks for up to the timeout, so call it from withLongRunningApi, not withBGApi. Plain sendCmd + // on purpose: a timeout shows a retry row in the results, not the alert sendCmdWithRetry would pop. suspend fun apiSearchDirectory(rh: Long?, text: String, cursor: JsonObject?): DirectorySearchResults? { val userId = kotlin.runCatching { currentUserId("apiSearchDirectory") }.getOrElse { return null } val req = directorySearchRequest(text, cursor) diff --git a/apps/simplex-directory-service/src/Directory/Rpc.hs b/apps/simplex-directory-service/src/Directory/Rpc.hs index c718dbefb9..d55240c47e 100644 --- a/apps/simplex-directory-service/src/Directory/Rpc.hs +++ b/apps/simplex-directory-service/src/Directory/Rpc.hs @@ -36,7 +36,7 @@ data DirectorySearchEntry = DirectorySearchEntry displayName :: Text, simplexName :: Maybe Text, groupLink :: PublicLink, - -- stored text, not DirectoryEntry's MarkdownList: the apps parse markdown locally + -- plain text, not parsed markdown: the apps parse it themselves, and their format differs shortDescr :: Maybe Text, image :: Maybe ImageData, activeAt :: Maybe UTCTime, @@ -61,8 +61,8 @@ responseObject resp = case J.toJSON resp of J.Object o -> o _ -> JM.fromList [("type", J.String "error"), ("errorMessage", J.String "internal error")] --- The page is shrunk from the end until its compressed encoding fits the envelope, and the cursor is the --- last row consumed. A lone entry that does not fit is retried without its image, then skipped, or paging would stall. +-- Entries are dropped from the end until the response fits, so the cursor must point at the last row +-- included, not the last one read. A single entry that still does not fit is skipped, or paging stalls on it. searchResultsPage :: (row -> SearchCursor) -> Bool -> [(row, Maybe DirectorySearchEntry)] -> DirectoryResponse searchResultsPage rowCursor storeHasMore rows = fit entryRows where @@ -99,8 +99,7 @@ searchEntry now g@GroupInfo {groupProfile, chatTs, createdAt = groupCreatedAt, g -- the apps connect through the short link, and a full link is hundreds of bytes of the envelope groupLink = if isJust connShortLink then link {connFullLink = Nothing} else link, shortDescr, - -- a profile received from its owner is not size-checked, so bound what is relayed - -- rather than passing an arbitrarily large data URI on to the apps + -- an owner can put any size of image in the profile we store, so bound what we relay image = image >>= \img@(ImageData t) -> if T.length t > maxProfileImageSize then Nothing else Just img, activeAt = recentRoundedTime 900 now $ fromMaybe groupCreatedAt chatTs, createdAt = recentRoundedTime 86400 now groupCreatedAt diff --git a/apps/simplex-directory-service/src/Directory/Search.hs b/apps/simplex-directory-service/src/Directory/Search.hs index aee368dc74..99d42ed94b 100644 --- a/apps/simplex-directory-service/src/Directory/Search.hs +++ b/apps/simplex-directory-service/src/Directory/Search.hs @@ -15,8 +15,8 @@ data SearchRequest = SearchRequest searchCursor :: SearchCursor } --- Position of the last sent row in the sort order of its search type. Each mode --- reads the value it sorts by; the group ID breaks ties, as neither sort key is unique. +-- Where the last page ended: each search mode reads the field it sorts by, and the group ID +-- breaks ties, because member counts and timestamps are not unique. data SearchCursor = SearchCursor { lastMembers :: Int64, lastCreatedAt :: UTCTime, diff --git a/apps/simplex-directory-service/src/Directory/Service.hs b/apps/simplex-directory-service/src/Directory/Service.hs index 8eec717e30..2ed1afd37c 100644 --- a/apps/simplex-directory-service/src/Directory/Service.hs +++ b/apps/simplex-directory-service/src/Directory/Service.hs @@ -137,8 +137,8 @@ newServiceState opts = do serviceRequestsInFlight <- newTVarIO 0 pure ServiceState {searchRequests, blockedWordsCfg, pendingCaptchas, serviceCC, eventQ, updateListingsJob, serviceRequestsInFlight} --- bounds the LIKE scan an unauthenticated request can demand; --- no substring of a name or description worth matching is longer +-- anyone can search without connecting first, so limit the work one request can ask for. +-- No name or description is long enough for a longer search term to be useful. maxSearchTextLength :: Int maxSearchTextLength = 100 @@ -357,7 +357,7 @@ directoryServiceEvent opts@DirectoryOpts {adminUsers, superUsers, serviceName, o where deServiceRequest :: AgentInvId -> J.Object -> IO () deServiceRequest reqId req = do - -- the loop is shared with registrations and captchas, so the bound is on the forked handlers + -- the event loop is shared with registrations and captchas, so the bound is on the forked handlers accepted <- atomically $ stateTVar serviceRequestsInFlight $ \n -> if n < maxServiceRequestsInFlight then (True, n + 1) else (False, n) if accepted @@ -365,8 +365,7 @@ directoryServiceEvent opts@DirectoryOpts {adminUsers, superUsers, serviceName, o else reject "service is busy" where releaseSlot = atomically $ modifyTVar' serviceRequestsInFlight (subtract 1) - -- runs on the shared event loop, so it must not block on the network: the reject - -- is enqueued, like the response is + -- this runs on the shared event loop, so the reject is enqueued rather than sent here reject reason = sendChatCmd cc (APIRejectServiceRequest userId reqId $ Just reason) >>= \case Right _ -> pure () diff --git a/src/Simplex/Chat/Library/Internal.hs b/src/Simplex/Chat/Library/Internal.hs index d2c06a9de3..f98b1a7167 100644 --- a/src/Simplex/Chat/Library/Internal.hs +++ b/src/Simplex/Chat/Library/Internal.hs @@ -2443,8 +2443,7 @@ encodeConnInfoPQ pqSup chatMsgEvent = do let info = ChatMessage {chatVRange = vr cxt, msgId = Nothing, chatMsgEvent} case encodeChatMessage maxEncodedInfoLength info of ECMEncoded connInfo -> case pqSup of - -- with PQ off the budget is larger, so compressing is wasted work; service payloads need no - -- such gate because the request JOIN is PQSupportOn and the reply inherits it + -- with PQ off the size budget is larger, so the body always fits and compressing is wasted work PQSupportOn -> maybe (throwChatError $ CEException "large compressed info") pure $ compressBodyTo maxCompressedInfoLength connInfo _ -> pure connInfo ECMLarge -> throwChatError $ CEException "large info" diff --git a/src/Simplex/Chat/Library/Subscriber.hs b/src/Simplex/Chat/Library/Subscriber.hs index 96cd4e43fc..c7b917776f 100644 --- a/src/Simplex/Chat/Library/Subscriber.hs +++ b/src/Simplex/Chat/Library/Subscriber.hs @@ -1404,8 +1404,7 @@ processAgentMessageConn cxt user@User {userId} entity gks_ corrId agentConnId ag True -> case parseServiceBody payload of Right request -> toView $ CEvtServiceRequest user (AgentInvId invId) sigKey_ request Left e -> logError ("service request dropped, invalid payload: " <> tshow e) >> dropSReq - -- the requester gets no reply and waits out its timeout, so this must be visible - -- to whoever deployed the service without enabling service requests + -- logged, not silent: this is a deployment mistake, and the requester only sees a timeout False -> logError "service request dropped: service requests are not enabled" >> dropSReq where dropSReq = withAgent $ \a -> rejectServiceRequest a NRMBackground (aUserId user) invId Nothing diff --git a/src/Simplex/Chat/Protocol.hs b/src/Simplex/Chat/Protocol.hs index 101cb4c09e..d6ec4bb0c2 100644 --- a/src/Simplex/Chat/Protocol.hs +++ b/src/Simplex/Chat/Protocol.hs @@ -1039,7 +1039,6 @@ markCompressedBatch :: ByteString -> ByteString markCompressedBatch = B.cons 'X' {-# INLINE markCompressedBatch #-} --- Compress a body that is over the bound, and fail when it is still over it compressed. compressBodyTo :: Int -> ByteString -> Maybe ByteString compressBodyTo maxLen body | B.length body <= maxLen = Just body @@ -1048,9 +1047,8 @@ compressBodyTo maxLen body where body' = compressedBatchMsgBody_ body --- Service payloads are padded to e2eEncConnInfoLength, the same budget as connection info, --- so they use the compression, marker and size bound of encodeConnInfoPQ. A JSON payload --- never starts with 'X', so the marker is unambiguous. +-- A service payload is padded to the same size as connection info, so it gets the same bound. +-- JSON never starts with 'X', so that marker unambiguously means the body is compressed. compressServiceBody :: ByteString -> Either String ByteString compressServiceBody = maybe (Left "service payload is too large") Right . compressBodyTo maxCompressedInfoLength @@ -1067,8 +1065,8 @@ decompressServiceBody body = case B.uncons body of Right _ -> Left "unexpected compressed batch" _ -> Right body --- The apps decode a service payload recursively on a fixed stack, and no service nests deeper --- than a few levels, so depth is bounded here rather than left to each client. +-- The apps decode this payload recursively on a small stack, so deep nesting crashes them. +-- Bounded here rather than in each client, as no service needs more than a few levels. maxServiceBodyDepth :: Int maxServiceBodyDepth = 32 diff --git a/tests/Bots/DirectoryTests.hs b/tests/Bots/DirectoryTests.hs index 234fa895ec..a3d34032aa 100644 --- a/tests/Bots/DirectoryTests.hs +++ b/tests/Bots/DirectoryTests.hs @@ -687,7 +687,6 @@ testSearchGroupsPaging ps = u <##. "Link to join the group " u <## (show count <> " members") --- the app path: a client that is not a contact searches the directory over the service RPC testDirectorySearchRpc :: HasCallStack => TestParams -> IO () testDirectorySearchRpc ps = withDirectoryService ps $ \superUser (dsShortLink, _) -> @@ -706,7 +705,7 @@ testDirectorySearchRpc ps = cath ##> ("/_service_request 1 " <> dsShortLink <> " {\"type\":\"nonsense\"}") cath <## "service response: {\"errorMessage\":\"unsupported request\",\"type\":\"error\"}" --- the contract the apps page by: the cursor is opaque, echoed back as received, and continues where the page stopped +-- the cursor is echoed back exactly as received, which is what the apps do with it testDirectorySearchRpcPaging :: HasCallStack => TestParams -> IO () testDirectorySearchRpcPaging ps = withDirectoryService ps $ \superUser (dsShortLink, _) -> @@ -754,9 +753,8 @@ testDirectorySearchRpcBusy ps = cath ##> ("/_service_request 1 " <> dsShortLink <> " {\"type\":\"search\",\"searchText\":\"privacy\"}") cath <## "smp agent error: AGENT {agentErr = A_SERVICE {serviceError = ASERejected {rejectReason = \"service is busy\"}}}" --- The response is read from the controller, not the terminal: an entry with a profile image is --- longer than a terminal row, and a wrapped line reaches the test queue as its last row only. --- The cursor stays a raw J.Value: echoing back what was received is the contract the apps follow. +-- Read from the controller, not the terminal: an entry with an image is longer than a terminal +-- row, and the test terminal only queues the last row of a line that wrapped. searchDirectory :: TestCC -> String -> String -> Maybe J.Value -> IO ([DirectorySearchEntry], Maybe J.Value) searchDirectory TestCC {chatController = cc} dsLink text cursor_ = do let req = J.object $ ["type" .= ("search" :: String), "searchText" .= text] <> maybe [] (\c -> ["searchCursor" .= c]) cursor_ @@ -781,7 +779,6 @@ searchEntryOnly u dsLink text = do ([e], Nothing) -> pure e _ -> fail $ "expected one entry and no cursor, got: " <> show (first (map entryName) r) --- every field the app renders or connects with, for a group registered the ordinary way testDirectorySearchEntryFields :: HasCallStack => TestParams -> IO () testDirectorySearchEntryFields ps = withDirectoryService ps $ \superUser (dsShortLink, dsLink) -> @@ -826,9 +823,8 @@ testDirectorySearchChannelEntry ps = (B.unpack . strEncode <$> connShortLink) `shouldBe` Just shortLink isNothing connFullLink `shouldBe` True --- searchEntry drops an image over maxProfileImageSize and relays the rest of the entry. No client --- can send such a profile - group creation and profile update both check the size - and only the --- receiving side stores one unchecked, so the bound is exercised on a real GroupInfo from the store. +-- Tested directly rather than end to end: our own client checks the image size when a profile is +-- created or updated, so only a group received from someone else can carry an oversize one. testSearchEntryImageBound :: HasCallStack => TestParams -> IO () testSearchEntryImageBound ps = withNewTestChat ps "bob" bobProfile $ \bob -> do @@ -852,8 +848,7 @@ testSearchEntryImageBound ps = isJust connShortLink `shouldBe` True Nothing -> expectationFailure "entry over the image bound was dropped" --- an entry with a near-cap image nearly fills the envelope, so a page holds one and the cursor --- must come from the last row included, not the last row read +-- one entry with a large image nearly fills a response, so both groups match but only one is sent testDirectorySearchImagePaging :: HasCallStack => TestParams -> IO () testDirectorySearchImagePaging ps = withDirectoryService ps $ \superUser (dsShortLink, dsLink) -> @@ -869,7 +864,7 @@ testDirectorySearchImagePaging ps = page2 `shouldSatisfy` notElem (head page1) sort (page1 <> page2) `shouldBe` ["photos1", "photos2"] --- the link behind a tap in the app is usable: cath joins with the short link from the entry +-- the link in a result actually works: this is what happens when a user taps a row in the app testDirectorySearchJoinGroup :: HasCallStack => TestParams -> IO () testDirectorySearchJoinGroup ps = withDirectoryService ps $ \superUser (dsShortLink, dsLink) -> @@ -895,8 +890,6 @@ testDirectorySearchJoinGroup ps = bob <## "#privacy: 'SimpleX Directory' added cath (Catherine) to the group (connecting...)" bob <## "#privacy: new member cath is connected" --- a real GroupInfo and its link from the owner's store, so a pure-function test does not --- hand-build a 24-field record ownerGroup :: TestCC -> String -> IO (GroupInfo, Maybe GroupLink) ownerGroup TestCC {chatController = cc@ChatController {chatStore, currentUser}} gName = do u_ <- readTVarIO currentUser @@ -925,8 +918,7 @@ registerGroupWithImage su u n descr gId = do groupAccepted u n gId void $ completeRegistrationId su u n descr gId gId --- share the channel card with the directory, wait for it to join via the relay, and approve; --- simplexName_ is the name line the admin sees when the channel has a verified domain +-- simplexName_ is the extra line the admin sees when the channel has a verified domain registerChannel :: HasCallStack => TestCC -> TestCC -> TestCC -> String -> Maybe String -> IO () registerChannel su u relay n simplexName_ = do uName <- userName u @@ -971,8 +963,7 @@ channelFoundSubscribers u name = do line <- getTermLine u maybe (fail $ "unexpected subscribers line: " <> line) pure $ readMaybe (takeWhile (/= ' ') line) --- the page is bounded by the envelope, not by searchResults: entries are cut from the end, the cursor --- follows the last row consumed, and a lone oversize entry loses its image or is skipped +-- what limits a page is the response size, not the configured page size testSearchResultsPage :: HasCallStack => TestParams -> IO () testSearchResultsPage _ps = do g <- C.newRandom @@ -2186,8 +2177,7 @@ withDirectoryService ps = withDirectoryServiceCfg ps testCfg withDirectoryServiceCfg :: HasCallStack => TestParams -> ChatConfig -> (TestCC -> (String, String) -> IO ()) -> IO () withDirectoryServiceCfg ps cfg = withDirectoryServiceCfgOwnersGroup ps cfg False Nothing --- the short link is the only form that carries the address DR keys, so service request tests --- need it; tests that only connect take the full link and void the other +-- passes both link forms: only the short one works for service requests, so those tests need it withDirectoryServiceCfgOwnersGroup :: HasCallStack => TestParams -> ChatConfig -> Bool -> Maybe FilePath -> (TestCC -> (String, String) -> IO ()) -> IO () withDirectoryServiceCfgOwnersGroup ps cfg createOwnersGroup webFolder test = do dsLinks <-