-
Notifications
You must be signed in to change notification settings - Fork 210
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
Support identity server v2 API #712
Conversation
…vers and abstract identity server REST client token and version management.
MXRestClient: Move IS API to its own
…ation and use the hashed v2 lookup API for 3PIDs.
… use the hashed v2 lookup API for 3PIDs.
MatrixSDK/MXSession.m
Outdated
if (!self.identityService) | ||
{ | ||
NSLog(@"[MXSession] Missing identity service"); | ||
failure(nil); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should return an error to display something to the end user, even if this is a technical error.
MatrixSDK/MXSession.h
Outdated
@@ -357,6 +358,11 @@ FOUNDATION_EXPORT NSString *const kMXSessionNoRoomTag; | |||
*/ | |||
@property (nonatomic, readonly) MXRestClient *matrixRestClient; | |||
|
|||
/** | |||
The identity service used to handle Matrix identity server requests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is old code. We do not have NS_ASSUME_NONNULL_BEGIN
but we can say it can be nil in the comments.
return apiPath; | ||
} | ||
|
||
- (NSString*)buildAPIPathWithAPIPathPrefix:(NSString*)apiPathPrefix andPath:(NSString*)path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method not used.
return operation; | ||
} | ||
|
||
- (MXHTTPOperation*)checkAPIVersionAvailabilityAndPerformOperationOnSuccessHandler:(MXHTTPOperation* (^)(void))successHandler |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duped and unused method.
} failure:^(NSError * _Nonnull error) { | ||
self.restClient.preferredAPIPathPrefix = kMXIdentityAPIPrefixPathV1; | ||
|
||
if (failure) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we call success
instead?
If failure
is called, that will break the requests flow. It seems we want to continue the flow but with V1 api.
{ | ||
__block MXHTTPOperation *operation; | ||
|
||
operation = [self checkAPIVersionAvailabilityWithSuccess:^{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check is already done.
|
||
NSString *accessToken = self.accessToken; | ||
|
||
if (httpClient && [httpClient.baseURL.absoluteString hasPrefix:self.identityServer] && [mxError.errcode isEqualToString:kMXErrCodeStringTermsNotSigned] && accessToken) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we make code (and comments) fit in 120 char width, please?
@@ -254,9 +254,20 @@ - (MXHTTPOperation*)checkAPIVersionAvailabilityWithSuccess:(void (^)(void))succe | |||
} failure:^(NSError * _Nonnull error) { | |||
self.restClient.preferredAPIPathPrefix = kMXIdentityAPIPrefixPathV1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.restClient.preferredAPIPathPrefix
should be set only in case of "success"
Handle element-hq/element-ios/issues/2603 and element-hq/element-ios/issues/2652