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

[SSDK-541] Add searchbox validation, change Demo to searchbox #164

Merged
merged 23 commits into from
Feb 28, 2024
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
68be932
Replace supportSBS binary with expandable ApiType enum - SSDK-544
aokj4ck Jan 25, 2024
d61124e
Fix PlaceAutocomplete ApiType - SSDK-544
aokj4ck Jan 26, 2024
03f62f5
Improve default parameters for SBS ApiType and make it default - SSDK…
aokj4ck Jan 26, 2024
44daef9
Change SearchUI to use searchBox - SSDK-541
aokj4ck Jan 26, 2024
459a898
Change Demo app Discover and PlaceAutocomplete to searchBox - SSDK-541
aokj4ck Jan 26, 2024
3dad1be
Add known validation options for search-box SSDK-541
aokj4ck Jan 30, 2024
90c9aa2
Accept SDK version bump - SSDK-541
aokj4ck Jan 30, 2024
77cca06
Merge branch 'main' of github.com:mapbox/mapbox-search-ios into SSDK-…
aokj4ck Feb 7, 2024
b485654
Change PlaceAutocomplete back to SBS until we can build search-box te…
aokj4ck Feb 7, 2024
445445f
Merge branch 'main' into SSDK-541-add-searchbox-validation
aokj4ck Feb 15, 2024
2323ce2
Bump search version swift
aokj4ck Feb 22, 2024
b9cfadb
Merge branch 'main' of github.com:mapbox/mapbox-search-ios into SSDK-…
aokj4ck Feb 22, 2024
cf7f975
Update unit tests for search-box API type, add mock data
aokj4ck Feb 23, 2024
6e2a824
Fix JSON format for SearchBox_SearchEngineIntegrationTests.testSugges…
aokj4ck Feb 26, 2024
385d69d
Finish fixing SearchBox_SearchEngineIntegrationTests.testSuggestionTy…
aokj4ck Feb 26, 2024
fc2948c
Fix Categor Search UI tests for search-box API
aokj4ck Feb 26, 2024
6992770
Fix CategorySuggestionsNavigation with Café, fix Feedback with mock S…
aokj4ck Feb 26, 2024
27b8483
Fix search integration UI test
aokj4ck Feb 26, 2024
fa75ab1
Disable 6 tests that rely on SAPI-1176
aokj4ck Feb 26, 2024
1101633
Restore previous behavior for Search Box testSuggestionTypeQuery
aokj4ck Feb 26, 2024
aa1b605
Nudge CI with empty commit
aokj4ck Feb 26, 2024
80585cd
Tryfix switch to successExpectation
aokj4ck Feb 27, 2024
b370ae3
Add mock for retrive/ recursion result identifier
aokj4ck Feb 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ Guide: https://keepachangelog.com/en/1.0.0/
## 2.0.0-rc.1

- [Discover] Fix charging station category canonical ID
- [Core] Change AbstractSearchEngine.init `supportSBS: Bool = false` parameter to `apiType: ApiType = .SBS`. This changes the default API engine for discover/category and other API requests to SBS. Add ApiType enum to represent non-Autofill and non-PlaceAutocomplete SearchEngine API types.
- [SearchUI] Rename MapboxPanelController.Configuration to .PanelConfiguration. This disambiguates PanelConfiguration from the broader Configuration struct.
- [Core] Update SwiftLint to 0.54.0 and SwiftFormat to 0.52.11
- [Core] Fix project compliance with linter, reformat Swift files
Expand Down
112 changes: 109 additions & 3 deletions MapboxSearch.xcodeproj/project.pbxproj

Large diffs are not rendered by default.

16 changes: 10 additions & 6 deletions Sources/Demo/DiscoverViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ final class DiscoverViewController: UIViewController {
@IBOutlet private var mapView: MKMapView!
@IBOutlet private var segmentedControl: UISegmentedControl!

private let discover = Discover()
private let discover = Discover(apiType: .searchBox)

override func viewDidLoad() {
super.viewDidLoad()
Expand All @@ -17,16 +17,20 @@ final class DiscoverViewController: UIViewController {
// MARK: - Actions

extension DiscoverViewController {
fileprivate enum Constants {
static let regionResultsLimit = 50
}

@IBAction
private func handleSearchInRegionAction() {
let regionResultsLimit: Int
switch discover.apiType {
case .geocoding:
regionResultsLimit = 10
default:
regionResultsLimit = 100
}

discover.search(
for: currentSelectedCategory,
in: currentBoundingBox,
options: .init(limit: Constants.regionResultsLimit)
options: .init(limit: regionResultsLimit)
) { result in
switch result {
case .success(let results):
Expand Down
2 changes: 2 additions & 0 deletions Sources/MapboxSearch/InternalAPI/Engine/ApiType+Core.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ extension ApiType {
return .geocoding
case .SBS:
return .SBS
case .searchBox:
return .searchBox
}
}
}
2 changes: 2 additions & 0 deletions Sources/MapboxSearch/PublicAPI/Engine/ApiType.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,6 @@ public enum ApiType {

/// The Mapbox Single Box Search (a.k.a Federation API) - https://docs.mapbox.com/api/search/search/
case SBS

case searchBox
}
2 changes: 1 addition & 1 deletion Sources/MapboxSearch/PublicAPI/RouteOptions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public struct RouteOptions: Hashable {
/// Construct Route Options with Time route deviation
/// - Parameters:
/// - route: Route with coordinates
/// - time: Time route deviation
/// - time: Time route deviation in seconds. Must be within 60 seconds (1 minute) and 1\_800 seconds (30 minutes)
/// - sarType: Quality of deviation calculation (better cost more). Defaults to `.isochrone`
public init(route: Route, time: TimeInterval, sarType: Deviation.SARType = .isochrone) {
self.init(route: route, deviation: .time(Measurement(value: time, unit: .seconds), sarType))
Expand Down
54 changes: 53 additions & 1 deletion Sources/MapboxSearch/PublicAPI/SearchOptions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,26 @@ public struct SearchOptions {
info("SBS API doesn't support multiple languages at once. Search SDK will use the first")
}

if case .time(let value, _) = validSearchOptions.routeOptions?.deviation {
let minimumTime = Measurement(
value: 1,
unit: UnitDuration.minutes
)
.converted(to: .seconds)
let maximumTime = Measurement(
value: 30,
unit: UnitDuration.minutes
)
.converted(to: .seconds)
let timeRange = (minimumTime...maximumTime)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can extract some logic here in a function to reuse it down below


if !timeRange.contains(value) {
info(
"SBS API time_deviation must be within 1 minute and 30 minutes (found \(value.value) seconds)"
)
}
}

case .autofill:
let unsupportedFilterTypes: [SearchQueryType] = [.category]

Expand All @@ -318,7 +338,39 @@ public struct SearchOptions {
}

case .searchBox:
_Logger.searchSDK.warning("SearchBox API is not supported yet.")
let topLimit = 10

validSearchOptions.limit = limit.map { min($0, topLimit) }
if validSearchOptions.limit != limit {
info("search-box API supports as maximum as \(topLimit) limit.")
}

if languages.count > 1, let first = languages.first {
validSearchOptions.languages = [first]
info(
"search-box API doesn't support multiple languages at once. Search SDK will use the first ('\(first)')"
)
}

if case .time(let value, _) = validSearchOptions.routeOptions?.deviation {
let minimumTime = Measurement(
value: 1,
unit: UnitDuration.minutes
)
.converted(to: .seconds)
let maximumTime = Measurement(
value: 30,
unit: UnitDuration.minutes
)
.converted(to: .seconds)
let timeRange = (minimumTime...maximumTime)

if !timeRange.contains(value) {
info(
"search-box API time_deviation must be within 1 minute and 30 minutes (found \(value.value) seconds)"
Copy link
Contributor

@azarovalex azarovalex Feb 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(just my thoughts)
so if the argument is out of bounds we just silently log and do nothing?
maybe we should force change deviation to be in bounds or log with higher log level, like warning
or maybe fail the request alltogether

)
}
}

@unknown default:
_Logger.searchSDK.warning("Unexpected engine API Type: \(apiType)")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,17 @@ public final class Discover {
private let searchEngine: CategorySearchEngine
private let userActivityReporter: CoreUserActivityReporter

public let apiType: ApiType

/// Basic internal initializer
/// - Parameters:
/// - accessToken: Mapbox Access Token to be used. Info.plist value for key `MGLMapboxAccessToken` will be used
/// for `nil` argument
/// - locationProvider: Provider configuration of LocationProvider that would grant location data by default
public convenience init(
accessToken: String? = nil,
locationProvider: LocationProvider? = DefaultLocationProvider()
locationProvider: LocationProvider? = DefaultLocationProvider(),
apiType: ApiType = .SBS
) {
guard let accessToken = accessToken ?? ServiceProvider.shared.getStoredAccessToken() else {
fatalError(
Expand All @@ -23,7 +26,7 @@ public final class Discover {
let searchEngine = CategorySearchEngine(
accessToken: accessToken,
locationProvider: locationProvider,
apiType: .SBS
apiType: apiType
)

let userActivityReporter = CoreUserActivityReporter.getOrCreate(
Expand All @@ -38,6 +41,7 @@ public final class Discover {

init(searchEngine: CategorySearchEngine, userActivityReporter: CoreUserActivityReporter) {
self.searchEngine = searchEngine
self.apiType = searchEngine.apiType
self.userActivityReporter = userActivityReporter
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import Foundation
extension Discover {
public struct Options {
/// Maximum number of results to return.
/// The maximum allowed value for SBS APIs is 100 results.
/// The maximum allowed value for geocoding APIs is 10 results.
public let limit: Int

/// List of language codes which used to provide localized results, order matters.
Expand Down
19 changes: 15 additions & 4 deletions Sources/MapboxSearchUI/MapboxSearchController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,14 @@ public class MapboxSearchController: UIViewController {
public required init(accessToken: String, configuration: Configuration = Configuration()) {
self.categorySearchEngine = CategorySearchEngine(
accessToken: accessToken,
locationProvider: configuration.locationProvider
locationProvider: configuration.locationProvider,
apiType: .searchBox
)
self.searchEngine = SearchEngine(
accessToken: accessToken,
locationProvider: configuration.locationProvider,
apiType: .searchBox
)
self.searchEngine = SearchEngine(accessToken: accessToken, locationProvider: configuration.locationProvider)
self.configuration = configuration

super.init(nibName: nil, bundle: .mapboxSearchUI)
Expand All @@ -171,8 +176,14 @@ public class MapboxSearchController: UIViewController {
/// - Parameters:
/// - configuration: configuration for search and categorySearch engines.
public required init(configuration: Configuration = Configuration()) {
self.categorySearchEngine = CategorySearchEngine(locationProvider: configuration.locationProvider)
self.searchEngine = SearchEngine(locationProvider: configuration.locationProvider)
self.categorySearchEngine = CategorySearchEngine(
locationProvider: configuration.locationProvider,
apiType: .searchBox
)
self.searchEngine = SearchEngine(
locationProvider: configuration.locationProvider,
apiType: .searchBox
)
self.configuration = configuration

super.init(nibName: nil, bundle: .mapboxSearchUI)
Expand Down
8 changes: 7 additions & 1 deletion Tests/Demo.xctestplan
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@
{
"skippedTests" : [
"MockServerIntegrationTestCase",
"OfflineIntegrationTests"
"OfflineIntegrationTests",
"SearchBox_SearchEngineIntegrationTests\/testResolvedSearchResult()",
"SearchBox_SearchEngineIntegrationTests\/testResolvedSearchResultWhenQueryChanged()"
],
"target" : {
"containerPath" : "container:MapboxSearch.xcodeproj",
Expand All @@ -54,6 +56,10 @@
{
"skippedTests" : [
"BaseTestCase",
"FavoritesIntegrationTestCase\/testAddEditLocationRemoveFavorite()",
"FavoritesIntegrationTestCase\/testAddRemoveFavorite()",
"FavoritesIntegrationTestCase\/testAddRenameCancelRemoveFavorite()",
"FavoritesIntegrationTestCase\/testAddRenameRemoveFavorite()",
"MockServerUITestCase"
],
"target" : {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import CoreLocation
@testable import MapboxSearch
import XCTest

final class CategorySearchEngineIntegrationTests: MockServerIntegrationTestCase<SBSMockResponse> {
final class SBS_CategorySearchEngineIntegrationTests: MockServerIntegrationTestCase<SBSMockResponse> {
private var searchEngine: CategorySearchEngine!

override func setUpWithError() throws {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import CoreLocation
@testable import MapboxSearch
import XCTest

class SearchEngineIntegrationTests: MockServerIntegrationTestCase<SBSMockResponse> {
class SBS_SearchEngineIntegrationTests: MockServerIntegrationTestCase<SBSMockResponse> {
let delegate = SearchEngineDelegateStub()
var searchEngine: SearchEngine!

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import CoreLocation
@testable import MapboxSearch
import XCTest

final class SearchBox_CategorySearchEngineIntegrationTests: MockServerIntegrationTestCase<SearchBoxMockResponse> {
private var searchEngine: CategorySearchEngine!

override func setUpWithError() throws {
try super.setUpWithError()

let apiType = try XCTUnwrap(Mock.coreApiType.toSDKType())
searchEngine = CategorySearchEngine(
accessToken: "access-token",
serviceProvider: LocalhostMockServiceProvider.shared,
apiType: apiType
)
}

func testCategorySearch() throws {
try server.setResponse(.categoryCafe)

let expectation = XCTestExpectation(description: "Expecting results")
searchEngine.search(categoryName: "cafe") { result in
switch result {
case .success(let searchResults):
XCTAssertFalse(searchResults.isEmpty)
expectation.fulfill()
case .failure:
XCTFail("Error not expected")
}
expectation.fulfill()
}
wait(for: [expectation], timeout: 10)
}

func testCategorySearchFailed() throws {
try server.setResponse(.categoryCafe, statusCode: 500)

let expectation = XCTestExpectation(description: "Expecting failure")
searchEngine.search(categoryName: "cafe") { result in
switch result {
case .success:
XCTFail("Not expected")
case .failure(let searchError):
if case .generic(let code, _, _) = searchError {
XCTAssert(code == 500)
} else {
XCTFail("Not expected")
}
}
expectation.fulfill()
}
wait(for: [expectation], timeout: 10)
}
}
Loading