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.

Sign up for GitHub

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

Jump to bottom

Do not add default query if it exists in the original URL #733

Merged
merged 4 commits into from
Feb 21, 2024
Merged

Do not add default query if it exists in the original URL #733

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
427ac61
Update some WordPressComRestApi unit tests
crazytonyli Feb 21, 2024
2e67d20
Do not add default query if it exists in the original URL
crazytonyli Feb 21, 2024
0b64235
Use more strict assertions
crazytonyli Feb 21, 2024
67cfbe2
Change test functions names
crazytonyli Feb 21, 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
3 changes: 2 additions & 1 deletion WordPressKit/HTTPRequestBuilder.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,9 @@ final class HTTPRequestBuilder {
// Add default query items if they don't exist in `appendedQuery`.
var newQuery = appendedQuery
if !defaultQuery.isEmpty {
let allQuery = (original.queryItems ?? []) + newQuery
let toBeAdded = defaultQuery.filter { item in
!newQuery.contains(where: { $0.name == item.name})
!allQuery.contains(where: { $0.name == item.name})
}
newQuery.append(contentsOf: toBeAdded)
}
Expand Down
9 changes: 9 additions & 0 deletions WordPressKitTests/Utilities/HTTPRequestBuilderTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,15 @@ class HTTPRequestBuilderTests: XCTestCase {
try XCTAssertEqual(builder.query(name: "foo", value: "bar").build().url?.query, "locale=zh&foo=bar")
}

func testDefaultQueryDoesNotOverriedQueryItemInOriginalURL() throws {
let url = try HTTPRequestBuilder(url: URL(string: "https://wordpress.org/hello?locale=foo")!)
.query(defaults: [URLQueryItem(name: "locale", value: "en")])
.build()
.url

XCTAssertEqual(url?.query, "locale=foo")
}

func testJSONBody() throws {
var request = try HTTPRequestBuilder(url: URL(string: "https://wordpress.org")!)
.method(.post)
Expand Down
286 changes: 56 additions & 230 deletions WordPressKitTests/WordPressComRestApiTests+Locale.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,272 +1,98 @@
import Foundation
import XCTest
import OHHTTPStubs

import WordPressShared
@testable import WordPressKit

extension WordPressComRestApiTests {

func testThatAppendingLocaleWorks() {
// Given
let path = "/path/path"
let preferredLanguageIdentifier = WordPressComLanguageDatabase().deviceLanguage.slug
func testAddsLocaleToURLQueryByDefault() async throws {
var request: URLRequest?
stub(condition: { _ in true }, response: {
request = $0
return HTTPStubsResponse(error: URLError(.networkConnectionLost))
})

// When
let api = WordPressComRestApi()
let localeAppendedPath = api.buildRequestURLFor(path: path)

// Then
XCTAssertNotNil(localeAppendedPath)
let actualURL = URL(string: localeAppendedPath!, relativeTo: api.baseURL)
XCTAssertNotNil(actualURL)

let actualURLComponents = URLComponents(url: actualURL!, resolvingAgainstBaseURL: false)
XCTAssertNotNil(actualURLComponents)

let expectedPath = path
let actualPath = actualURLComponents!.path
XCTAssertEqual(expectedPath, actualPath)

let actualQueryItems = actualURLComponents!.queryItems
XCTAssertNotNil(actualQueryItems)

let expectedQueryItemCount = 1
let actualQueryItemCount = actualQueryItems!.count
XCTAssertEqual(expectedQueryItemCount, actualQueryItemCount)

let actualQueryItem = actualQueryItems!.first
XCTAssertNotNil(actualQueryItem!)

let actualQueryItemKey = actualQueryItem!.name
let expectedQueryItemKey = WordPressComRestApi.LocaleKeyDefault
XCTAssertEqual(expectedQueryItemKey, actualQueryItemKey)
let _ = await api.perform(.get, URLString: "/path/path")

let actualQueryItemValue = actualQueryItem!.value
XCTAssertNotNil(actualQueryItemValue)

let expectedQueryItemValue = preferredLanguageIdentifier
XCTAssertEqual(expectedQueryItemValue, actualQueryItemValue!)
let preferredLanguageIdentifier = WordPressComLanguageDatabase().deviceLanguage.slug
XCTAssertEqual(request?.url?.query, "locale=\(preferredLanguageIdentifier)")
}

func testThatAppendingLocaleWorksWithExistingParams() {
// Given
func testAddsLocaleToURLQueryByDefaultAndMaintainsInputParameters() async throws {
var request: URLRequest?
stub(condition: { _ in true }, response: {
request = $0
return HTTPStubsResponse(error: URLError(.networkConnectionLost))
})

let path = "/path/path"
let params: [String: AnyObject] = [
"someKey": "value" as AnyObject
]

// When
let api = WordPressComRestApi()
let localeAppendedPath = api.buildRequestURLFor(path: path, parameters: params)

// Then
XCTAssertNotNil(localeAppendedPath)
let actualURL = URL(string: localeAppendedPath!, relativeTo: api.baseURL)
XCTAssertNotNil(actualURL)

let actualURLComponents = URLComponents(url: actualURL!, resolvingAgainstBaseURL: false)
XCTAssertNotNil(actualURLComponents)

let expectedPath = "/path/path"
let actualPath = actualURLComponents!.path
XCTAssertEqual(expectedPath, actualPath)

let actualQueryItems = actualURLComponents!.queryItems
XCTAssertNotNil(actualQueryItems)

let expectedQueryItemCount = 1
let actualQueryItemCount = actualQueryItems!.count
XCTAssertEqual(expectedQueryItemCount, actualQueryItemCount)

let actualQueryString = actualURLComponents?.query
XCTAssertNotNil(actualQueryString)

let queryStringIncludesLocale = actualQueryString!.contains(WordPressComRestApi.LocaleKeyDefault)
XCTAssertTrue(queryStringIncludesLocale)
}

func testThatLocaleIsNotAppendedIfAlreadyIncludedInPath() {
// Given
let preferredLanguageIdentifier = "foo"
let path = "/path/path?locale=\(preferredLanguageIdentifier)"

// When
let api = WordPressComRestApi()
let localeAppendedPath = api.buildRequestURLFor(path: path)

// Then
XCTAssertNotNil(localeAppendedPath)
let actualURL = URL(string: localeAppendedPath!, relativeTo: api.baseURL)
XCTAssertNotNil(actualURL)

let actualURLComponents = URLComponents(url: actualURL!, resolvingAgainstBaseURL: false)
XCTAssertNotNil(actualURLComponents)

let expectedPath = "/path/path"
let actualPath = actualURLComponents!.path
XCTAssertEqual(expectedPath, actualPath)

let actualQueryItems = actualURLComponents!.queryItems
XCTAssertNotNil(actualQueryItems)

let expectedQueryItemCount = 1
let actualQueryItemCount = actualQueryItems!.count
XCTAssertEqual(expectedQueryItemCount, actualQueryItemCount)

let actualQueryItem = actualQueryItems!.first
XCTAssertNotNil(actualQueryItem!)

let actualQueryItemKey = actualQueryItem!.name
let expectedQueryItemKey = WordPressComRestApi.LocaleKeyDefault
XCTAssertEqual(expectedQueryItemKey, actualQueryItemKey)

let actualQueryItemValue = actualQueryItem!.value
XCTAssertNotNil(actualQueryItemValue)

let expectedQueryItemValue = preferredLanguageIdentifier
XCTAssertEqual(expectedQueryItemValue, actualQueryItemValue!)
}
let _ = await api.perform(.get, URLString: path, parameters: params)

func testThatAppendingLocaleIgnoresIfAlreadyIncludedInRequestParameters() {
// Given
let inputPath = "/path/path"
let expectedLocaleValue = "foo"
let params: [String: AnyObject] = [
WordPressComRestApi.LocaleKeyDefault: expectedLocaleValue as AnyObject
]

// When
let requestURLString = WordPressComRestApi().buildRequestURLFor(path: inputPath, parameters: params)

// Then
let preferredLanguageIdentifier = WordPressComLanguageDatabase().deviceLanguage.slug
XCTAssertFalse(requestURLString!.contains(preferredLanguageIdentifier))
let query = try XCTUnwrap(request?.url?.query?.split(separator: "&"))
XCTAssertEqual(Set(query), Set(["locale=\(preferredLanguageIdentifier)", "someKey=value"]))
}

func testThatLocaleIsNotAppendedWhenDisabled() {
// Given
let path = "/path/path"
func testThatLocaleIsNotAppendedIfAlreadyIncludedInPath() async {
var request: URLRequest?
stub(condition: { _ in true }, response: {
request = $0
return HTTPStubsResponse(error: URLError(.networkConnectionLost))
})

// When
let api = WordPressComRestApi()
api.appendsPreferredLanguageLocale = false
let localeAppendedPath = api.buildRequestURLFor(path: path)

// Then
XCTAssertNotNil(localeAppendedPath)
let actualURL = URL(string: localeAppendedPath!, relativeTo: api.baseURL)
XCTAssertNotNil(actualURL)

let actualURLComponents = URLComponents(url: actualURL!, resolvingAgainstBaseURL: false)
XCTAssertNotNil(actualURLComponents)

let expectedPath = path
let actualPath = actualURLComponents!.path
XCTAssertEqual(expectedPath, actualPath)
let _ = await api.perform(.get, URLString: "/path?locale=foo")

let actualQueryItems = actualURLComponents!.queryItems
XCTAssertNil(actualQueryItems)
try XCTAssertEqual(XCTUnwrap(request?.url?.query), "locale=foo")
}

func testThatAlternateLocaleKeyIsHonoredWhenSpecified() {
// Given
let path = "/path/path"
let expectedKey = "foo"

// When
let api = WordPressComRestApi(localeKey: expectedKey)
let localeAppendedPath = api.buildRequestURLFor(path: path)

// Then
XCTAssertNotNil(localeAppendedPath)
let actualURL = URL(string: localeAppendedPath!, relativeTo: api.baseURL)
XCTAssertNotNil(actualURL)

let actualURLComponents = URLComponents(url: actualURL!, resolvingAgainstBaseURL: false)
XCTAssertNotNil(actualURLComponents)

let expectedPath = path
let actualPath = actualURLComponents!.path
XCTAssertEqual(expectedPath, actualPath)

let actualQueryItems = actualURLComponents!.queryItems
XCTAssertNotNil(actualQueryItems)

let expectedQueryItemCount = 1
let actualQueryItemCount = actualQueryItems!.count
XCTAssertEqual(expectedQueryItemCount, actualQueryItemCount)

let actualQueryItem = actualQueryItems!.first
XCTAssertNotNil(actualQueryItem!)
func testThatAppendingLocaleIgnoresIfAlreadyIncludedInRequestParameters() async throws {
var request: URLRequest?
stub(condition: { _ in true }, response: {
request = $0
return HTTPStubsResponse(error: URLError(.networkConnectionLost))
})

let actualQueryItemKey = actualQueryItem!.name
XCTAssertEqual(expectedKey, actualQueryItemKey)
}

func testThatAppendingLocaleWorksWhenPassingNilParameters() {
// Given
let path = "/path/path"
let preferredLanguageIdentifier = WordPressComLanguageDatabase().deviceLanguage.slug

// When
let api = WordPressComRestApi()
let localeAppendedPath = WordPressComRestApi().buildRequestURLFor(path: path, parameters: nil)

// Then
XCTAssertNotNil(localeAppendedPath)
let actualURL = URL(string: localeAppendedPath!, relativeTo: api.baseURL)
XCTAssertNotNil(actualURL)

let actualURLComponents = URLComponents(url: actualURL!, resolvingAgainstBaseURL: false)
XCTAssertNotNil(actualURLComponents)

let expectedPath = path
let actualPath = actualURLComponents!.path
XCTAssertEqual(expectedPath, actualPath)
let _ = await api.perform(.get, URLString: "/path", parameters: ["locale": "foo"] as [String: AnyObject])

let actualQueryItems = actualURLComponents!.queryItems
XCTAssertNotNil(actualQueryItems)

let expectedQueryItemCount = 1
let actualQueryItemCount = actualQueryItems!.count
XCTAssertEqual(expectedQueryItemCount, actualQueryItemCount)

let actualQueryItem = actualQueryItems!.first
XCTAssertNotNil(actualQueryItem!)

let actualQueryItemKey = actualQueryItem!.name
let expectedQueryItemKey = WordPressComRestApi.LocaleKeyDefault
XCTAssertEqual(expectedQueryItemKey, actualQueryItemKey)

let actualQueryItemValue = actualQueryItem!.value
XCTAssertNotNil(actualQueryItemValue)

let expectedQueryItemValue = preferredLanguageIdentifier
XCTAssertEqual(expectedQueryItemValue, actualQueryItemValue!)
try XCTAssertEqual(XCTUnwrap(request?.url?.query), "locale=foo")
}

func testThatLocaleIsNotAppendedWhenDisabledAndParametersAreNil() {
// Given
let path = "/path/path"
func testThatLocaleIsNotAppendedWhenDisabled() async {
var request: URLRequest?
stub(condition: { _ in true }, response: {
request = $0
return HTTPStubsResponse(error: URLError(.networkConnectionLost))
})

// When
let api = WordPressComRestApi()
api.appendsPreferredLanguageLocale = false
let localeAppendedPath = api.buildRequestURLFor(path: path, parameters: nil)
let _ = await api.perform(.get, URLString: "/path")

// Then
XCTAssertNotNil(localeAppendedPath)
let actualURL = URL(string: localeAppendedPath!, relativeTo: api.baseURL)
XCTAssertNotNil(actualURL)
XCTAssertNotNil(request?.url)
XCTAssertNil(request?.url?.query)
}

let actualURLComponents = URLComponents(url: actualURL!, resolvingAgainstBaseURL: false)
XCTAssertNotNil(actualURLComponents)
func testThatAlternateLocaleKeyIsHonoredWhenSpecified() async {
var request: URLRequest?
stub(condition: { _ in true }, response: {
request = $0
return HTTPStubsResponse(error: URLError(.networkConnectionLost))
})

let expectedPath = path
let actualPath = actualURLComponents!.path
XCTAssertEqual(expectedPath, actualPath)
let api = WordPressComRestApi(localeKey: "foo")

let actualQueryItems = actualURLComponents!.queryItems
XCTAssertNil(actualQueryItems)
let preferredLanguageIdentifier = WordPressComLanguageDatabase().deviceLanguage.slug
let _ = await api.perform(.get, URLString: "/path/path")
XCTAssertEqual(request?.url?.query, "foo=\(preferredLanguageIdentifier)")
}
}
14 changes: 11 additions & 3 deletions WordPressKitTests/WordPressComRestApiTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,22 @@ class WordPressComRestApiTests: XCTestCase {
self.waitForExpectations(timeout: 2, handler: nil)
}

func testBaseUrl() {
func testBaseUrl() async throws {
var request: URLRequest?
stub(condition: { _ in true }, response: {
request = $0
return HTTPStubsResponse(error: URLError(.networkConnectionLost))
})

let defaultApi = WordPressComRestApi()
XCTAssertEqual(defaultApi.baseURL.absoluteString, "https://public-api.wordpress.com/")
XCTAssertTrue(defaultApi.buildRequestURLFor(path: "/path")!.hasPrefix("https://public-api.wordpress.com/path"))
let _ = await defaultApi.perform(.get, URLString: "/path")
try XCTAssertTrue(XCTUnwrap(request?.url?.absoluteString).hasPrefix("https://public-api.wordpress.com/path"))

let localhostApi = WordPressComRestApi(baseURL: URL(string: "http://localhost:8080")!)
XCTAssertEqual(localhostApi.baseURL.absoluteString, "http://localhost:8080")
XCTAssertTrue(localhostApi.buildRequestURLFor(path: "/path")!.hasPrefix("http://localhost:8080/path"))
let _ = await localhostApi.perform(.get, URLString: "/local")
try XCTAssertTrue(XCTUnwrap(request?.url?.absoluteString).hasPrefix("http://localhost:8080/local"))
}

func testURLStringWithQuery() async {
Expand Down