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

chore: apply API v8 changes - WPB-15840 #2570

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from
1 change: 0 additions & 1 deletion wire-ios-mocktransport/Source/MockTransportSession.m
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,6 @@ - (NSArray *)methodMap
@[@"/register", @"processRegistrationRequest:"],
@[@"/activate/send", @"processVerificationCodeRequest:"],
@[@"/activate", @"processPhoneActivationRequest:"],
@[@"/onboarding/v3", @"processOnboardingRequest:"],
@[@"/invitations", @"processInvitationsRequest:"],
@[@"/teams", @"processTeamsRequest:"],
@[@"/broadcast", @"processBroadcastRequest:"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,5 @@
@interface MockTransportSession (Search)

- (ZMTransportResponse *)processSearchRequest:(ZMTransportRequest *)request;
- (ZMTransportResponse *)processOnboardingRequest:(ZMTransportRequest *)request


@end
34 changes: 0 additions & 34 deletions wire-ios-mocktransport/Source/Search/MockTransportSession+search.m
Original file line number Diff line number Diff line change
Expand Up @@ -66,38 +66,4 @@ - (ZMTransportResponse *)processSearchRequest:(ZMTransportRequest *)request;
return [self errorResponseWithCode:404 reason:@"no-endpoint" apiVersion:request.apiVersion];
}

// handles /onboarding
- (ZMTransportResponse *)processOnboardingRequest:(ZMTransportRequest *)request
{
if(request.method == ZMTransportRequestMethodPost) {
NSArray *selfEmailArray = [[request.payload asDictionary] arrayForKey:@"self"];
if(selfEmailArray.count == 0) {
return [self errorResponseWithCode:400 reason:@"no self email" apiVersion:request.apiVersion];
}
NSArray *cards = [[request.payload asDictionary] arrayForKey:@"cards"];
if(cards == nil) {
return [self errorResponseWithCode:400 reason:@"missing contacts" apiVersion:request.apiVersion];
}

NSFetchRequest *fetchRequest = [MockUser sortedFetchRequest];
NSArray *users = [self.managedObjectContext executeFetchRequestOrAssert_mt:fetchRequest];

// This method is just a simulation, it does not do any actual matching, it just returns all users
NSMutableArray *results = [NSMutableArray array];
for (MockUser *user in users) {
if (user == self.selfUser) {
continue;
}
[results addObject:@{
@"id" : user.identifier,
@"cards" : @[]
}];
}
return [ZMTransportResponse responseWithPayload:@{@"results" : results} HTTPStatus:200 transportSessionError:nil apiVersion:request.apiVersion];

}
return [self errorResponseWithCode:404 reason:@"no-endpoint" apiVersion:request.apiVersion];
}


@end
Original file line number Diff line number Diff line change
Expand Up @@ -37,77 +37,6 @@ - (void)testThatWhenPostingAddressBookItReturnsAllUserIDsThatAreNotSelf
user2 = [session insertUserWithName:@"User2 AABB"];
}];
WaitForAllGroupsToBeEmpty(0.5);


// WHEN
NSDictionary *upstreamPayload = @{
@"self":@[@"2312rfw32434234"],
@"cards": @[@{@"contact":@[]}]
};
NSString *path = [NSString pathWithComponents:@[@"/", @"onboarding", @"v3"]];
ZMTransportResponse *response = [self responseForPayload:upstreamPayload path:path method:ZMTransportRequestMethodPost apiVersion:0];

// THEN
NSDictionary *expectedPayload = @{
@"results": @[
@{@"id": user1.identifier, @"cards": @[]},
@{@"id": user2.identifier, @"cards": @[]},
]
};
XCTAssertEqual(response.HTTPStatus, 200);
XCTAssertEqualObjects(response.payload, expectedPayload);
}

- (void)testThatWhenPostingAddressBookItFailsIfMissingAnySelfElement
{
// GIVEN
NSDictionary *upstreamPayload = @{
@"self":@[],
@"contacts":@[]
};
NSString *path = [NSString pathWithComponents:@[@"/", @"onboarding", @"v3"]];

// WHEN
ZMTransportResponse *response = [self responseForPayload:upstreamPayload path:path method:ZMTransportRequestMethodPost apiVersion:0];

// THEN
XCTAssertEqual(response.HTTPStatus, 400);
}

- (void)testThatWhenPostingAddressBookItFailsIfMissingSelf
{
// GIVEN
NSDictionary *upstreamPayload = @{
@"contacts":@[]
};
NSString *path = [NSString pathWithComponents:@[@"/", @"onboarding", @"v3"]];

// WHEN
[self performIgnoringZMLogError:^{
ZMTransportResponse *response = [self responseForPayload:upstreamPayload path:path method:ZMTransportRequestMethodPost apiVersion:0];

// THEN
XCTAssertEqual(response.HTTPStatus, 400);
}];
}


- (void)testThatWhenPostingAddressBookItFailsIfMissingContacts
{
// GIVEN
NSDictionary *upstreamPayload = @{
@"self":@[@"fooooooooo"]
};
NSString *path = [NSString pathWithComponents:@[@"/", @"onboarding", @"v3"]];

// WHEN
[self performIgnoringZMLogError:^{
ZMTransportResponse *response = [self responseForPayload:upstreamPayload path:path method:ZMTransportRequestMethodPost apiVersion:0];

// THEN
XCTAssertEqual(response.HTTPStatus, 400);
}];
}


@end
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
return nil
}

// TODO: .v8 requires ciphersuite or ciphersuites!

Check failure on line 42 in wire-ios-request-strategy/Sources/Request Strategies/MLS/Actions/CountSelfMLSKeyPackagesActionHandler.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Add JIRA reference to TODOs and FIX MEs like [WPB-680]. (todo_requires_jira_link)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes it looks like it. The optional query parameter was added from v5 but we never used it. I think it's fine to start using if from v8. The question is... which cipher suite should we pass?

return ZMTransportRequest(
path: "/mls/key-packages/self/\(action.clientID)/count",
method: .get,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,14 @@
guard let identifier = conversation.remoteIdentifier?.transportString()
else { fatal("conversation inserted on backend") }

let path: String
if apiVersion < .v8 {
path = "/conversations/\(identifier)/message-timer"
} else {
guard let domain = conversation.domain else { fatal("conversation has no domain") } // TODO: unit test?

Check failure on line 100 in wire-ios-sync-engine/Source/Data Model/Conversation+MessageDestructionTimeout.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Add JIRA reference to TODOs and FIX MEs like [WPB-680]. (todo_requires_jira_link)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would suggest to fallback to the local domain, which there should be (even though it's optional 🙃).

Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't crash the app if there was no domain

path = "/conversations/\(domain)/\(identifier)/message-timer"
}

let payload: [AnyHashable: Any?]
if timeout == 0 {
payload = ["message_timer": nil]
Expand All @@ -102,7 +110,7 @@
payload = ["message_timer": timeoutInMS]
}
return .init(
path: "/conversations/\(identifier)/message-timer",
path: path,
method: .put,
payload: payload as ZMTransportData,
apiVersion: apiVersion.rawValue
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,17 @@
guard let conversationId = remoteIdentifier?.transportString()
else { return completion(.failure(ReadReceiptModeError.noConversation)) }

let path: String
if apiVersion >= .v8 {
guard let domain else { return completion(.failure(ReadReceiptModeError.noConversation)) }
Copy link
Collaborator

Choose a reason for hiding this comment

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

suggestion: also here it'd be good to fallback on the local domain.

path = "/conversations/\(domain)/\(conversationId)/receipt-mode" // TODO: unit test?

Check failure on line 55 in wire-ios-sync-engine/Source/Data Model/Conversation+ReadReceiptMode.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Add JIRA reference to TODOs and FIX MEs like [WPB-680]. (todo_requires_jira_link)
} else {
path = "/conversations/\(conversationId)/receipt-mode"
}

let payload = ["receipt_mode": enabled ? 1 : 0] as ZMTransportData
let request = ZMTransportRequest(
path: "/conversations/\(conversationId)/receipt-mode",
path: path,
method: .put,
payload: payload,
apiVersion: apiVersion.rawValue
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
userInfoParser?.upgradeToAuthenticatedSession(with: $0)
}
registrationStatus.success()
} else {
} else { // TODO: handle new errors? 400: "invalid-domain" and 403: "condition-failed"

Check failure on line 63 in wire-ios-sync-engine/Source/Synchronization/Strategies/RegistrationStrategy.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Add JIRA reference to TODOs and FIX MEs like [WPB-680]. (todo_requires_jira_link)
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we don't specifically handle these new errors, does the registration status still consider it an error? The login and registration flows are being re-written so probably doesn't make much sense to handle these errors if not absolutely necessary.

let error = NSError.blacklistedEmail(with: response) ??
NSError.invalidActivationCode(with: response) ??
NSError.emailAddressInUse(with: response) ??
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
)

request.add(ZMCompletionHandler(on: managedObjectContext, block: { [weak self] response in
self?.processResponse(response, for: email)
self?.processResponse(response, for: email) // TODO: handle new error? 403 "condition-failed"

Check failure on line 80 in wire-ios-sync-engine/Source/Synchronization/Strategies/TeamInvitationRequestStrategy.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Add JIRA reference to TODOs and FIX MEs like [WPB-680]. (todo_requires_jira_link)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there something to handle or would it end up as an error log?

Copy link
Contributor

@KaterinaWire KaterinaWire Feb 20, 2025

Choose a reason for hiding this comment

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

Agree, I don't think we have tickets or design changes to handle this error

}))

return request
Expand Down
Loading