chore: create searchContext on demand - WPB-20362#4246
Conversation
…te-search-context-WPB-20362
This reverts commit d03e77a. # Conflicts: # wire-ios-sync-engine/Source/UserSession/Search/SearchTask.swift
…hore/delete-search-context-WPB-20362
There was a problem hiding this comment.
Pull request overview
This PR removes the dedicated Core Data searchContext and refactors search to create background contexts on demand, updating both production code and tests to match the new API surface.
Changes:
- Remove
searchContextfromContextProvider,CoreDataStack,ZMUserSession, and related mocks/tests. - Refactor
SearchTask/SearchDirectoryto rely oncontextProvider.newBackgroundContext()instead of an injected/persistent search MOC. - Remove “search context” identification/typing from Core Data context helpers/logging.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| wire-ios/Tests/Mocks/UserSessionMock.swift | Drops searchContext conformance from the app’s session mock. |
| wire-ios-sync-engine/Source/UserSession/Search/SearchTask.swift | Replaces injected searchContext with ad-hoc background contexts during search operations. |
| wire-ios-sync-engine/Source/UserSession/Search/SearchDirectory.swift | Removes searchContext plumbing; constructs SearchTask without a dedicated MOC. |
| wire-ios-sync-engine/Source/UserSession/ZMUserSession/ZMUserSession.swift | Removes public searchManagedObjectContext and ContextProvider.searchContext. |
| wire-ios-data-model/Source/ManagedObjectContext/CoreDataStack.swift | Removes searchContext from the stack and ContextProvider protocol. |
| wire-ios-data-model/Source/ManagedObjectContext/NSManagedObjectContext+zmessaging.(h/m) | Removes “search context” keys and helpers from context typing/validation. |
| wire-ios-sync-engine/Tests/... + wire-ios-data-model/Tests/... | Updates tests to stop injecting/using searchContext/searchMOC (in some places). |
Comments suppressed due to low confidence (6)
wire-ios-sync-engine/Tests/Source/MessagingTest.m:217
searchMOCaccessor was removed, but this class still declares/usessearchMOC(e.g. inallManagedObjectContexts). This will either fail to compile (missing getter) or crash at runtime with an unrecognized selector. Update the test base to stop referencingsearchMOC(and remove the property from the header) or provide an alternative (e.g. create an on-demand background context for tests that need it).
- (NSManagedObjectContext *)uiMOC
{
return self.coreDataStack.viewContext;
}
- (NSManagedObjectContext *)syncMOC
{
return self.coreDataStack.syncContext;
}
- (NSManagedObjectContext *)eventMOC
{
return self.coreDataStack.eventContext;
}
wire-ios-sync-engine/Source/UserSession/Search/SearchTask.swift:442
ZMTaskCreatedHandlerkeepsgroupQueueas a weak reference. Since thissearchContextis a local variable (not retained bySearchTask), it may be deallocated before the transport layer calls the task-created handlers, souserLookupTaskIdentifieris never set andcancel()won’t be able to cancel the underlying request. Retain a long-lived groupQueue (e.g. a lazily-created context stored onSearchTask, or use an existing long-lived queue/context) for task-created handlers.
let searchContext = contextProvider.newBackgroundContext()
searchContext.perform { [self] in
let request = type(of: self).searchRequestForUser(qualifiedID: qualifiedID, apiVersion: apiVersion)
request.add(ZMCompletionHandler(on: contextProvider.viewContext) { [weak self] response in
defer {
self?.tasksRemaining -= 1
}
guard
let self,
let payload = response.payload?.asDictionary(),
let partialResult = SearchResult(
userLookupPayload: payload,
contextProvider: contextProvider,
searchUsersCache: searchUsersCache
)
else { return }
let updatedResult = aggregatedResult.union(withDirectoryResult: partialResult)
aggregatedResult = updatedResult
})
request.add(ZMTaskCreatedHandler(on: searchContext) { [weak self] taskIdentifier in
self?.userLookupTaskIdentifier = taskIdentifier
})
wire-ios-sync-engine/Source/UserSession/Search/SearchTask.swift:508
- Same issue as above:
ZMTaskCreatedHandlerholds thegroupQueueweakly, butsearchContextis not retained beyond this method scope. If the context is deallocated before the task is created,directoryTaskIdentifierwon’t be captured and cancellation becomes ineffective. Keep the context/groupQueue alive for the lifetime of theSearchTask(or use a stable groupQueue).
let searchContext = contextProvider.newBackgroundContext()
searchContext.perform { [self] in
let request = Self.searchRequestInDirectory(withRequest: searchRequest, apiVersion: apiVersion)
request.add(ZMCompletionHandler(on: contextProvider.viewContext) { [weak self] response in
guard let self else { return }
guard
let payload = response.payload?.asDictionary(),
let partialResult = SearchResult(
payload: payload,
query: searchRequest.query,
searchOptions: searchRequest.searchOptions,
contextProvider: contextProvider,
searchUsersCache: searchUsersCache
)
else {
completeRemoteSearch()
return
}
if searchRequest.searchOptions.contains(.teamMembers) {
performTeamMembershipLookup(on: partialResult, searchRequest: searchRequest)
} else {
completeRemoteSearch(searchResult: partialResult)
}
})
request.add(ZMTaskCreatedHandler(on: searchContext) { [weak self] taskIdentifier in
self?.directoryTaskIdentifier = taskIdentifier
})
wire-ios-sync-engine/Source/UserSession/Search/SearchTask.swift:560
searchContextis created only to be passed intoZMTaskCreatedHandler, which stores the queue weakly. This context is very likely to be deallocated immediately afterenqueueOneTime, soteamMembershipTaskIdentifiermay never be set andcancel()can’t cancel the request. Use a retained groupQueue (e.g. store the context onSearchTask) instead of an ephemeral context here.
let searchContext = contextProvider.newBackgroundContext()
request.add(ZMTaskCreatedHandler(on: searchContext) { [weak self] taskIdentifier in
self?.teamMembershipTaskIdentifier = taskIdentifier
})
transportSession.enqueueOneTime(request)
wire-ios-sync-engine/Source/UserSession/Search/SearchTask.swift:691
ZMTaskCreatedHandlerstoresgroupQueueweakly. BecausesearchContextis not retained bySearchTask, it can be released before the handler is invoked, preventinghandleTaskIdentifierfrom being recorded (and breaking cancellation). Consider retaining a single lazily-created search context onSearchTaskand reuse it for these handlers.
let searchContext = contextProvider.newBackgroundContext()
searchContext.perform { [self] in
let request = type(of: self).searchRequestInDirectory(
withHandle: searchRequest.query.string,
apiVersion: apiVersion
)
request.add(ZMCompletionHandler(on: contextProvider.viewContext) { [weak self] response in
defer {
self?.tasksRemaining -= 1
}
guard
let self,
let payload = response.payload?.asArray(),
let userPayload = (payload.first as? ZMTransportData)?.asDictionary()
else {
return
}
guard
let handle = userPayload["handle"] as? String,
let name = userPayload["name"] as? String,
let id = userPayload["id"] as? String
else {
return
}
let document = ["handle": handle, "name": name, "id": id]
let documentPayload = ["documents": [document]]
guard let partialResult = SearchResult(
payload: documentPayload,
query: searchRequest.query,
searchOptions: searchRequest.searchOptions,
contextProvider: contextProvider,
searchUsersCache: searchUsersCache
) else {
return
}
if let user = partialResult.directory.first, !user.isSelfUser {
let prevResult = aggregatedResult
// prepend result to prevResult only if it doesn't contain it
if !prevResult.directory.contains(user) {
aggregatedResult = SearchResult(
context: prevResult.context,
contacts: prevResult.contacts,
teamMembers: prevResult.teamMembers,
directory: partialResult.directory + prevResult.directory,
conversations: prevResult.conversations,
services: prevResult.services,
searchUsersCache: searchUsersCache
)
}
}
})
request.add(ZMTaskCreatedHandler(on: searchContext) { [weak self] taskIdentifier in
self?.handleTaskIdentifier = taskIdentifier
})
wire-ios-sync-engine/Source/UserSession/Search/SearchTask.swift:762
searchContextis only retained as a local variable, butZMTaskCreatedHandlerkeeps the queue weakly. If the context goes away before the task is created,servicesTaskIdentifierwon’t be set andcancel()can’t cancel the service search request. Retain a long-lived groupQueue/context for the lifetime of the task.
func performRemoteSearchForServices() {
let searchContext = contextProvider.newBackgroundContext()
let teamIdentifier = searchContext.performAndWait {
ZMUser.selfUser(in: searchContext).team?.remoteIdentifier
}
guard
let apiVersion,
let teamIdentifier,
case let .search(searchRequest) = task,
!searchRequest.searchOptions.contains(.localResultsOnly),
searchRequest.searchOptions.contains(.services)
else { return }
tasksRemaining += 1
searchContext.perform { [self] in
let request = type(of: self).servicesSearchRequest(
teamIdentifier: teamIdentifier,
query: searchRequest.query.string,
apiVersion: apiVersion
)
request.add(ZMCompletionHandler(on: contextProvider.viewContext) { [weak self] response in
defer {
self?.tasksRemaining -= 1
}
guard
let self,
let payload = response.payload?.asDictionary(),
let partialResult = SearchResult(
servicesPayload: payload,
query: searchRequest.query.string,
contextProvider: contextProvider,
searchUsersCache: searchUsersCache
)
else {
return
}
let updatedResult = aggregatedResult.union(withServiceResult: partialResult)
aggregatedResult = updatedResult
})
request.add(ZMTaskCreatedHandler(on: searchContext) { [weak self] taskIdentifier in
self?.servicesTaskIdentifier = taskIdentifier
})
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Test Results5 373 tests 5 346 ✅ 8m 8s ⏱️ Results for commit af607dd. ♻️ This comment has been updated with latest results. Summary: workflow run #21670088645 |
…hore/delete-search-context-WPB-20362
…0-7b20975c44be491ed30dc6d915798f5214d023dc' into chore/delete-zmselfuser-inusersession-WPB-20362
…hore/delete-search-context-WPB-20362
|
| tasksRemaining += 1 | ||
|
|
||
| searchContext.performGroupedBlock { [self] in | ||
| let searchContext = contextProvider.newBackgroundContext() |
There was a problem hiding this comment.
question: I see that we create the search context multiple times in this file instead of once for the whole search task. Is each use of the context independent from each other? I'm wondering if we are losing accumulative changes to the search context when we perform the different types of lookup.
There was a problem hiding this comment.
The search results are mapped to the viewContext, that behavior I haven't changed. So I hope we don't lose results.
However, if we should have a single context or open multiple ones while performing fetch requests, that I'm not sure. I'll think about it for the follow-up PR #4245



Issue
This PR removes
searchContextfromContextProviderand the Core Data stacks and instead creates a new background context on demand for searching.Testing
Search for users and/or bots in teams or personal ones.
Checklist
[WPB-XXX].UI accessibility checklist
If your PR includes UI changes, please utilize this checklist: