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

fix(datastore-v1): correct the filter predicate logic applied to optional fields #3485

Merged
merged 1 commit into from
Feb 5, 2024
Merged

fix(datastore-v1): correct the filter predicate logic applied to optional fields #3485

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 36 additions & 0 deletions Amplify/Categories/DataStore/Model/Internal/Persistable.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ struct PersistableHelper {
return lhs == rhs
case let (lhs, rhs) as (String, String):
return lhs == rhs
case let (lhs, rhs) as (any EnumPersistable, String):
return lhs.rawValue == rhs
case let (lhs, rhs) as (String, any EnumPersistable):
return lhs == rhs.rawValue
case let (lhs, rhs) as (any EnumPersistable, any EnumPersistable):
return lhs.rawValue == rhs.rawValue
default:
return false
}
Expand Down Expand Up @@ -95,6 +101,12 @@ struct PersistableHelper {
return lhs == Double(rhs)
case let (lhs, rhs) as (String, String):
return lhs == rhs
case let (lhs, rhs) as (any EnumPersistable, String):
return lhs.rawValue == rhs
case let (lhs, rhs) as (String, any EnumPersistable):
return lhs == rhs.rawValue
case let (lhs, rhs) as (any EnumPersistable, any EnumPersistable):
return lhs.rawValue == rhs.rawValue
default:
return false
}
Expand Down Expand Up @@ -123,6 +135,12 @@ struct PersistableHelper {
return lhs <= Double(rhs)
case let (lhs, rhs) as (String, String):
return lhs <= rhs
case let (lhs, rhs) as (any EnumPersistable, String):
return lhs.rawValue <= rhs
case let (lhs, rhs) as (String, any EnumPersistable):
return lhs <= rhs.rawValue
case let (lhs, rhs) as (any EnumPersistable, any EnumPersistable):
return lhs.rawValue <= rhs.rawValue
default:
return false
}
Expand Down Expand Up @@ -151,6 +169,12 @@ struct PersistableHelper {
return lhs < Double(rhs)
case let (lhs, rhs) as (String, String):
return lhs < rhs
case let (lhs, rhs) as (any EnumPersistable, String):
return lhs.rawValue < rhs
case let (lhs, rhs) as (String, any EnumPersistable):
return lhs < rhs.rawValue
case let (lhs, rhs) as (any EnumPersistable, any EnumPersistable):
return lhs.rawValue < rhs.rawValue
default:
return false
}
Expand Down Expand Up @@ -179,6 +203,12 @@ struct PersistableHelper {
return lhs >= Double(rhs)
case let (lhs, rhs) as (String, String):
return lhs >= rhs
case let (lhs, rhs) as (any EnumPersistable, String):
return lhs.rawValue >= rhs
case let (lhs, rhs) as (String, any EnumPersistable):
return lhs >= rhs.rawValue
case let (lhs, rhs) as (any EnumPersistable, any EnumPersistable):
return lhs.rawValue >= rhs.rawValue
default:
return false
}
Expand Down Expand Up @@ -207,6 +237,12 @@ struct PersistableHelper {
return Double(lhs) > rhs
case let (lhs, rhs) as (String, String):
return lhs > rhs
case let (lhs, rhs) as (any EnumPersistable, String):
return lhs.rawValue > rhs
case let (lhs, rhs) as (String, any EnumPersistable):
return lhs > rhs.rawValue
case let (lhs, rhs) as (any EnumPersistable, any EnumPersistable):
return lhs.rawValue > rhs.rawValue
default:
return false
}
Expand Down
6 changes: 3 additions & 3 deletions Amplify/Categories/DataStore/Query/QueryOperator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public enum QueryOperator {
case beginsWith(_ value: String)

// swiftlint:disable:next cyclomatic_complexity
public func evaluate(target: Any) -> Bool {
public func evaluate(target: Any?) -> Bool {
switch self {
case .notEqual(let predicateValue):
return !PersistableHelper.isEqual(target, predicateValue)
Expand All @@ -34,14 +34,14 @@ public enum QueryOperator {
case .greaterThan(let predicateValue):
return PersistableHelper.isGreaterThan(target, predicateValue)
case .contains(let predicateString):
if let targetString = target as? String {
if let targetString = target.flatMap({ $0 as? String }) {
return targetString.contains(predicateString)
}
return false
case .between(let start, let end):
return PersistableHelper.isBetween(start, end, target)
case .beginsWith(let predicateValue):
if let targetString = target as? String {
if let targetString = target.flatMap({ $0 as? String }) {
return targetString.starts(with: predicateValue)
}
}
Expand Down
30 changes: 1 addition & 29 deletions Amplify/Categories/DataStore/Query/QueryPredicate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,34 +127,6 @@ public class QueryPredicateOperation: QueryPredicate {
}

public func evaluate(target: Model) -> Bool {
guard let fieldValue = target[field] else {
return false
}

guard let value = fieldValue else {
return false
}

if let booleanValue = value as? Bool {
return self.operator.evaluate(target: booleanValue)
}

if let doubleValue = value as? Double {
return self.operator.evaluate(target: doubleValue)
}

if let intValue = value as? Int {
return self.operator.evaluate(target: intValue)
}

if let timeValue = value as? Temporal.Time {
return self.operator.evaluate(target: timeValue)
}

if let enumValue = value as? EnumPersistable {
return self.operator.evaluate(target: enumValue.rawValue)
}

return self.operator.evaluate(target: value)
return self.operator.evaluate(target: target[field]?.flatMap { $0 })
}
}
4 changes: 2 additions & 2 deletions ...uthPluginTests/AuthenticationProviderTests/AuthenticationProviderConfirmSignupTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class AuthenticationProviderConfirmSignupTests: BaseAuthenticationProviderTest {
///
func testSuccessfulConfirmSignUp() {

let mockSignupResult = SignUpResult(signUpState: .confirmed, codeDeliveryDetails: nil)
let mockSignupResult = SignUpResult(signUpState: .confirmed, codeDeliveryDetails: nil, userSub: nil)
lawmicha marked this conversation as resolved.
Show resolved Hide resolved
mockAWSMobileClient?.confirmSignUpMockResult = .success(mockSignupResult)

let resultExpectation = expectation(description: "Should receive a result")
Expand Down Expand Up @@ -60,7 +60,7 @@ class AuthenticationProviderConfirmSignupTests: BaseAuthenticationProviderTest {
///
func testConfirmSignUpWithEmptyUserName() {

let mockSignupResult = SignUpResult(signUpState: .confirmed, codeDeliveryDetails: nil)
let mockSignupResult = SignUpResult(signUpState: .confirmed, codeDeliveryDetails: nil, userSub: nil)
mockAWSMobileClient?.confirmSignUpMockResult = .success(mockSignupResult)

let resultExpectation = expectation(description: "Should receive a result")
Expand Down
5 changes: 3 additions & 2 deletions ...PluginTests/AuthenticationProviderTests/AuthenticationProviderResendSignupCodeTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class AuthenticationProviderResendSignupCodeTests: BaseAuthenticationProviderTes
destination: nil,
attributeName: nil)
let mockResendSignUpCodeResult = SignUpResult(signUpState: .confirmed,
codeDeliveryDetails: codeDelieveryDetails)
codeDeliveryDetails: codeDelieveryDetails, userSub: nil)
mockAWSMobileClient?.resendSignUpCodeMockResult = .success(mockResendSignUpCodeResult)
let resultExpectation = expectation(description: "Should receive a result")
_ = plugin.resendSignUpCode(for: "username") { result in
Expand Down Expand Up @@ -66,7 +66,8 @@ class AuthenticationProviderResendSignupCodeTests: BaseAuthenticationProviderTes
destination: nil,
attributeName: nil)
let mockResendSignUpCodeResult = SignUpResult(signUpState: .confirmed,
codeDeliveryDetails: codeDelieveryDetails)
codeDeliveryDetails: codeDelieveryDetails,
userSub: nil)
mockAWSMobileClient?.resendSignUpCodeMockResult = .success(mockResendSignUpCodeResult)
let resultExpectation = expectation(description: "Should receive a result")
_ = plugin.resendSignUpCode(for: "") { result in
Expand Down
12 changes: 8 additions & 4 deletions ...ognitoAuthPluginTests/AuthenticationProviderTests/AuthenticationProviderSignupTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class AuthenticationProviderSignupTests: BaseAuthenticationProviderTest {
///
func testSignupWithSuccess() {

let mockSignupResult = SignUpResult(signUpState: .confirmed, codeDeliveryDetails: nil)
let mockSignupResult = SignUpResult(signUpState: .confirmed, codeDeliveryDetails: nil, userSub: nil)
mockAWSMobileClient?.signupMockResult = .success(mockSignupResult)

let emailAttribute = AuthUserAttribute(.email, value: "email")
Expand Down Expand Up @@ -61,7 +61,7 @@ class AuthenticationProviderSignupTests: BaseAuthenticationProviderTest {
///
func testSignupWithEmptyUserName() {

let mockSignupResult = SignUpResult(signUpState: .confirmed, codeDeliveryDetails: nil)
let mockSignupResult = SignUpResult(signUpState: .confirmed, codeDeliveryDetails: nil, userSub: nil)
mockAWSMobileClient?.signupMockResult = .success(mockSignupResult)

let emailAttribute = AuthUserAttribute(.email, value: "email")
Expand Down Expand Up @@ -96,7 +96,7 @@ class AuthenticationProviderSignupTests: BaseAuthenticationProviderTest {
///
func testSignupWithEmptyPassword() {

let mockSignupResult = SignUpResult(signUpState: .confirmed, codeDeliveryDetails: nil)
let mockSignupResult = SignUpResult(signUpState: .confirmed, codeDeliveryDetails: nil, userSub: nil)
mockAWSMobileClient?.signupMockResult = .success(mockSignupResult)

let emailAttribute = AuthUserAttribute(.email, value: "email")
Expand Down Expand Up @@ -138,7 +138,11 @@ class AuthenticationProviderSignupTests: BaseAuthenticationProviderTest {
let mockCodeDelivery = UserCodeDeliveryDetails(deliveryMedium: .email,
destination: mockEmail,
attributeName: "email")
let mockSignupResult = SignUpResult(signUpState: .unconfirmed, codeDeliveryDetails: mockCodeDelivery)
let mockSignupResult = SignUpResult(
signUpState: .unconfirmed,
codeDeliveryDetails: mockCodeDelivery,
userSub: nil
)
mockAWSMobileClient?.signupMockResult = .success(mockSignupResult)

let resultExpectation = expectation(description: "Should receive a result")
Expand Down
2 changes: 1 addition & 1 deletion AmplifyPlugins/Auth/Podfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,4 @@ SPEC CHECKSUMS:

PODFILE CHECKSUM: 371cf67fe35ebb5167d0880bad12b01618a0fb0e

COCOAPODS: 1.11.3
COCOAPODS: 1.14.3
4 changes: 2 additions & 2 deletions AmplifyPlugins/Core/AWSPluginsCoreTests/Query/QueryPredicateEvaluateGeneratedBoolTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class QueryPredicateEvaluateGeneratedBoolTests: XCTestCase {

let evaluation = try predicate.evaluate(target: instance.eraseToAnyModel().instance)

XCTAssertFalse(evaluation)
XCTAssertTrue(evaluation)
Copy link
Contributor

Choose a reason for hiding this comment

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

[nit] let's add either a test comment or improve the test name to testBooltruenotEqualBoolNil it's not immediately obvious that the instance has a nil Bool due to the default parameter value. Here and other tests that have similar changes

}

func testBoolfalsenotEqualBooltrue() throws {
Expand Down Expand Up @@ -70,7 +70,7 @@ class QueryPredicateEvaluateGeneratedBoolTests: XCTestCase {

let evaluation = try predicate.evaluate(target: instance.eraseToAnyModel().instance)

XCTAssertFalse(evaluation)
XCTAssertTrue(evaluation)
}

func testBooltrueequalsBooltrue() throws {
Expand Down
8 changes: 4 additions & 4 deletions AmplifyPlugins/Core/AWSPluginsCoreTests/Query/QueryPredicateEvaluateGeneratedDateTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class QueryPredicateEvaluateGeneratedDateTests: XCTestCase {

let evaluation = try predicate.evaluate(target: instance.eraseToAnyModel().instance)

XCTAssertFalse(evaluation)
XCTAssertTrue(evaluation)
}

func testTemporalDateTemporal_Date_now_addvalue1to_daynotEqualTemporalDateTemporal_Date_now() throws {
Expand Down Expand Up @@ -109,7 +109,7 @@ class QueryPredicateEvaluateGeneratedDateTests: XCTestCase {

let evaluation = try predicate.evaluate(target: instance.eraseToAnyModel().instance)

XCTAssertFalse(evaluation)
XCTAssertTrue(evaluation)
}

func testTemporalDateTemporal_Date_now_addvalue2to_daynotEqualTemporalDateTemporal_Date_now() throws {
Expand Down Expand Up @@ -158,7 +158,7 @@ class QueryPredicateEvaluateGeneratedDateTests: XCTestCase {

let evaluation = try predicate.evaluate(target: instance.eraseToAnyModel().instance)

XCTAssertFalse(evaluation)
XCTAssertTrue(evaluation)
}

func testTemporalDateTemporal_Date_now_addvalue3to_daynotEqualTemporalDateTemporal_Date_now() throws {
Expand Down Expand Up @@ -207,7 +207,7 @@ class QueryPredicateEvaluateGeneratedDateTests: XCTestCase {

let evaluation = try predicate.evaluate(target: instance.eraseToAnyModel().instance)

XCTAssertFalse(evaluation)
XCTAssertTrue(evaluation)
}

func testTemporalDateTemporal_Date_nowequalsTemporalDateTemporal_Date_now() throws {
Expand Down
8 changes: 4 additions & 4 deletions ...Plugins/Core/AWSPluginsCoreTests/Query/QueryPredicateEvaluateGeneratedDateTimeTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class QueryPredicateEvaluateGeneratedDateTimeTests: XCTestCase {

let evaluation = try predicate.evaluate(target: instance.eraseToAnyModel().instance)

XCTAssertFalse(evaluation)
XCTAssertTrue(evaluation)
}

func testTemporalDateTimeTemporal_DateTime_now_addvalue1to_hournotEqualTemporalDateTimeTemporal_DateTime_now() throws {
Expand Down Expand Up @@ -120,7 +120,7 @@ class QueryPredicateEvaluateGeneratedDateTimeTests: XCTestCase {

let evaluation = try predicate.evaluate(target: instance.eraseToAnyModel().instance)

XCTAssertFalse(evaluation)
XCTAssertTrue(evaluation)
}

func testTemporalDateTimeTemporal_DateTime_now_addvalue2to_hournotEqualTemporalDateTimeTemporal_DateTime_now() throws {
Expand Down Expand Up @@ -174,7 +174,7 @@ class QueryPredicateEvaluateGeneratedDateTimeTests: XCTestCase {

let evaluation = try predicate.evaluate(target: instance.eraseToAnyModel().instance)

XCTAssertFalse(evaluation)
XCTAssertTrue(evaluation)
}

func testTemporalDateTimeTemporal_DateTime_now_addvalue3to_hournotEqualTemporalDateTimeTemporal_DateTime_now() throws {
Expand Down Expand Up @@ -228,7 +228,7 @@ class QueryPredicateEvaluateGeneratedDateTimeTests: XCTestCase {

let evaluation = try predicate.evaluate(target: instance.eraseToAnyModel().instance)

XCTAssertFalse(evaluation)
XCTAssertTrue(evaluation)
}

func testTemporalDateTimeTemporal_DateTime_nowequalsTemporalDateTimeTemporal_DateTime_now() throws {
Expand Down
12 changes: 6 additions & 6 deletions ...lugins/Core/AWSPluginsCoreTests/Query/QueryPredicateEvaluateGeneratedDoubleIntTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class QueryPredicateEvaluateGeneratedDoubleIntTests: XCTestCase {

let evaluation = try predicate.evaluate(target: instance.eraseToAnyModel().instance)

XCTAssertFalse(evaluation)
XCTAssertTrue(evaluation)
}

func testDouble2_1notEqualInt1() throws {
Expand Down Expand Up @@ -89,7 +89,7 @@ class QueryPredicateEvaluateGeneratedDoubleIntTests: XCTestCase {

let evaluation = try predicate.evaluate(target: instance.eraseToAnyModel().instance)

XCTAssertFalse(evaluation)
XCTAssertTrue(evaluation)
}

func testDouble3_1notEqualInt1() throws {
Expand Down Expand Up @@ -128,7 +128,7 @@ class QueryPredicateEvaluateGeneratedDoubleIntTests: XCTestCase {

let evaluation = try predicate.evaluate(target: instance.eraseToAnyModel().instance)

XCTAssertFalse(evaluation)
XCTAssertTrue(evaluation)
}

func testDouble1notEqualInt1() throws {
Expand Down Expand Up @@ -167,7 +167,7 @@ class QueryPredicateEvaluateGeneratedDoubleIntTests: XCTestCase {

let evaluation = try predicate.evaluate(target: instance.eraseToAnyModel().instance)

XCTAssertFalse(evaluation)
XCTAssertTrue(evaluation)
}

func testDouble2notEqualInt1() throws {
Expand Down Expand Up @@ -206,7 +206,7 @@ class QueryPredicateEvaluateGeneratedDoubleIntTests: XCTestCase {

let evaluation = try predicate.evaluate(target: instance.eraseToAnyModel().instance)

XCTAssertFalse(evaluation)
XCTAssertTrue(evaluation)
}

func testDouble3notEqualInt1() throws {
Expand Down Expand Up @@ -245,7 +245,7 @@ class QueryPredicateEvaluateGeneratedDoubleIntTests: XCTestCase {

let evaluation = try predicate.evaluate(target: instance.eraseToAnyModel().instance)

XCTAssertFalse(evaluation)
XCTAssertTrue(evaluation)
}

func testDouble1_1equalsInt1() throws {
Expand Down
Loading
Loading