Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace OfflineIndexObserver with AbstractSearchEngine.offlineEngineReady flag #209

Closed
Closed
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ Guide: https://keepachangelog.com/en/1.0.0/

<!-- Add changes for active work here -->

- [Offline] Add `AbstractSearchEngine.offlineEngineReady` to mark when offline searches are ready.

- [Offline] Add optional `language` parameter to SearchOfflineManager.createTilesetDescriptor and SearchOfflineManager.createPlacesTilesetDescriptor functions.
- [Tests] Add Spanish language offline search test.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ public class AbstractSearchEngine: FeedbackManagerDelegate {
/// `OfflineManager` with `default` TileStore.
public private(set) var offlineManager: SearchOfflineManager

/// Block offline searches until the tileset is ready
var offlineEngineReady = false

// Manager to send raw events to Mapbox Telemetry
let eventsManager: EventsManager

Expand Down
44 changes: 29 additions & 15 deletions Sources/MapboxSearch/PublicAPI/Engine/SearchEngine.swift
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ public class SearchEngine: AbstractSearchEngine {
case .enabled:
offlineManager.registerCurrentTileStore { [weak self] in
self?.offlineMode = mode
self?.offlineEngineReady = true
completion?()
}
case .disabled:
Expand Down Expand Up @@ -195,18 +196,6 @@ public class SearchEngine: AbstractSearchEngine {

override var dataResolvers: [IndexableDataResolver] { super.dataResolvers + [self] }

var engineSearchFunction: (String, [String], CoreSearchOptions, @escaping (CoreSearchResponseProtocol?) -> Void)
-> Void
{
offlineMode == .disabled ? engine.search : engine.searchOffline
}

var engineReverseGeocodingFunction: (CoreReverseGeoOptions, @escaping (CoreSearchResponseProtocol?) -> Void)
-> Void
{
offlineMode == .disabled ? engine.reverseGeocoding : engine.reverseGeocodingOffline
}

func retrieve(suggestion: SearchSuggestion) {
guard let responseProvider = suggestion as? CoreResponseProvider else {
assertionFailure()
Expand All @@ -230,9 +219,22 @@ public class SearchEngine: AbstractSearchEngine {
}

let options = options?.merged(defaultSearchOptions) ?? defaultSearchOptions
let coreOptions = options.toCore(apiType: engineApi)

engineSearchFunction(queryValueString, [], options.toCore(apiType: engineApi)) { [weak self] response in
self?.processResponse(response, suggestion: nil)
if offlineMode == .enabled {
guard offlineEngineReady else {
assertionFailure("Attempted offline search before engine was ready")
return
}

engine
.searchOffline(query: queryValueString, categories: [], options: coreOptions) { [weak self] response in
self?.processResponse(response, suggestion: nil)
}
} else {
engine.search(forQuery: queryValueString, categories: [], options: coreOptions) { [weak self] response in
self?.processResponse(response, suggestion: nil)
}
}
}

Expand Down Expand Up @@ -465,7 +467,8 @@ extension SearchEngine {
userActivityReporter.reportActivity(forComponent: "search-engine-reverse-geocoding")
}

engineReverseGeocodingFunction(options.toCore()) { [weak self] response in
let coreOptions = options.toCore()
let searchCompletionBlock: ((CoreSearchResponseProtocol?) -> Void) = { [weak self] response in
guard let self else {
assertionFailure("Owning object was deallocated")
return
Expand Down Expand Up @@ -495,6 +498,17 @@ extension SearchEngine {
completion(.failure(wrappedError))
}
}

if offlineMode == .enabled {
guard offlineEngineReady else {
assertionFailure("Attempted offline search before engine was ready")
return
}

engine.reverseGeocodingOffline(for: coreOptions, completion: searchCompletionBlock)
} else {
engine.reverseGeocoding(for: coreOptions, completion: searchCompletionBlock)
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ public class SearchOfflineManager {
self.tileStore = tileStore
}

// MARK: - Tile Store setup

func registerCurrentTileStore(completion: (() -> Void)?) {
setTileStore(tileStore, completion: completion)
}
Expand Down
96 changes: 71 additions & 25 deletions Tests/MapboxSearchIntegrationTests/OfflineIntegrationTests.swift
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
@testable import MapboxSearch

import CoreGraphics
import CoreLocation
import MapboxCommon
@testable import MapboxSearch
import XCTest

/// Note: ``OfflineIntegrationTests`` does not use Mocked data.
class OfflineIntegrationTests: MockServerIntegrationTestCase<SBSMockResponse> {
class OfflineIntegrationTests: XCTestCase {
let delegate = SearchEngineDelegateStub()
let searchEngine = SearchEngine()

Expand Down Expand Up @@ -65,22 +63,23 @@ class OfflineIntegrationTests: MockServerIntegrationTestCase<SBSMockResponse> {

// MARK: - Tests

func testLoadData() throws {
func testLoadDataOfflineSearch() throws {
clearData()

// Set up index observer before the fetch starts to validate changes after it completes
let indexChangedExpectation = expectation(description: "Received offline index changed event")
let offlineIndexObserver = OfflineIndexObserver(onIndexChangedBlock: { changeEvent in
_Logger.searchSDK.info("Index changed: \(changeEvent)")
indexChangedExpectation.fulfill()
}, onErrorBlock: { error in
_Logger.searchSDK.error("Encountered error in OfflineIndexObserver \(error)")
XCTFail(error.debugDescription)
let booleanIsTruePredicate = NSPredicate(block: { input, _ in
guard let input = input as? Bool else {
return false
}
return input == true
})
searchEngine.offlineManager.engine.addOfflineIndexObserver(for: offlineIndexObserver)

let engineReadyExpectation = expectation(
for: booleanIsTruePredicate,
evaluatedWith: searchEngine.offlineEngineReady
)

// Perform the offline fetch
let loadDataExpectation = expectation(description: "Load Data")
let loadDataExpectation = XCTestExpectation(description: "Load Data")
_ = loadData { result in
switch result {
case .success(let region):
Expand All @@ -93,7 +92,7 @@ class OfflineIntegrationTests: MockServerIntegrationTestCase<SBSMockResponse> {
loadDataExpectation.fulfill()
}
wait(
for: [loadDataExpectation, indexChangedExpectation],
for: [engineReadyExpectation, loadDataExpectation],
timeout: 200,
enforceOrder: true
)
Expand All @@ -109,19 +108,66 @@ class OfflineIntegrationTests: MockServerIntegrationTestCase<SBSMockResponse> {
XCTAssertFalse(searchEngine.suggestions.isEmpty)
}

func testLoadDataReverseGeocodingOffline() throws {
clearData()

let booleanIsTruePredicate = NSPredicate(block: { input, _ in
guard let input = input as? Bool else {
return false
}
return input == true
})

let engineReadyExpectation = expectation(
for: booleanIsTruePredicate,
evaluatedWith: searchEngine.offlineEngineReady
)

// Perform the offline fetch
let loadDataExpectation = XCTestExpectation(description: "Load Data")
_ = loadData { result in
switch result {
case .success(let region):
XCTAssert(region.id == self.regionId)
XCTAssert(region.completedResourceCount > 0)
XCTAssertEqual(region.requiredResourceCount, region.completedResourceCount)
case .failure(let error):
XCTFail("Unable to load Region, \(error.localizedDescription)")
}
loadDataExpectation.fulfill()
}
wait(
for: [engineReadyExpectation, loadDataExpectation],
timeout: 200,
enforceOrder: true
)

let options = ReverseGeocodingOptions(point: dcLocation, languages: ["en"])
searchEngine.reverse(options: options) { result in
switch result {
case .success(let success):
XCTAssertFalse(success.isEmpty)
case .failure(let failure):
XCTFail(failure.localizedDescription)
}
}
}

func testSpanishLanguageSupport() throws {
clearData()

// Set up index observer before the fetch starts to validate changes after it completes
let indexChangedExpectation = expectation(description: "Received offline index changed event")
let offlineIndexObserver = OfflineIndexObserver(onIndexChangedBlock: { changeEvent in
_Logger.searchSDK.info("Index changed: \(changeEvent)")
indexChangedExpectation.fulfill()
}, onErrorBlock: { error in
_Logger.searchSDK.error("Encountered error in OfflineIndexObserver \(error)")
XCTFail(error.debugDescription)
let booleanIsTruePredicate = NSPredicate(block: { input, _ in
guard let input = input as? Bool else {
return false
}
return input == true
})
searchEngine.offlineManager.engine.addOfflineIndexObserver(for: offlineIndexObserver)

// Set up index observer before the fetch starts to validate changes after it completes
let engineReadyExpectation = expectation(
for: booleanIsTruePredicate,
evaluatedWith: searchEngine.offlineEngineReady
)

// Perform the offline fetch
let spanishTileset = SearchOfflineManager.createTilesetDescriptor(
Expand All @@ -141,7 +187,7 @@ class OfflineIntegrationTests: MockServerIntegrationTestCase<SBSMockResponse> {
loadDataExpectation.fulfill()
}
wait(
for: [loadDataExpectation, indexChangedExpectation],
for: [engineReadyExpectation, loadDataExpectation],
timeout: 200,
enforceOrder: true
)
Expand Down