Skip to content

Commit

Permalink
[PP-746] Make annotation sync client side (#371)
Browse files Browse the repository at this point in the history 
* in progress

* Clean up annotation deletion, switch management and resolve infinite refresh call issue

* Update project.pbxproj
  • Loading branch information
@mauricecarrier7
mauricecarrier7 authored Dec 7, 2023
1 parent 5229f29 commit b4448b9
Show file tree
Hide file tree
Showing 13 changed files with 18 additions and 332 deletions.
8 changes: 4 additions & 4 deletions Palace.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4782,7 +4782,7 @@
CODE_SIGN_IDENTITY = "Apple Distribution";
"CODE_SIGN_IDENTITY[sdk=iphoneos*]" = "Apple Distribution";
CODE_SIGN_STYLE = Manual;
CURRENT_PROJECT_VERSION = 225;
CURRENT_PROJECT_VERSION = 226;
DEVELOPMENT_TEAM = 88CBA74T8K;
"DEVELOPMENT_TEAM[sdk=iphoneos*]" = 88CBA74T8K;
ENABLE_BITCODE = NO;
Expand Down Expand Up @@ -4840,7 +4840,7 @@
CLANG_WARN_NON_LITERAL_NULL_CONVERSION = YES_ERROR;
CODE_SIGN_ENTITLEMENTS = Palace/SimplyE.entitlements;
CODE_SIGN_IDENTITY = "iPhone Distribution";
CURRENT_PROJECT_VERSION = 225;
CURRENT_PROJECT_VERSION = 226;
DEVELOPMENT_TEAM = 88CBA74T8K;
"DEVELOPMENT_TEAM[sdk=iphoneos*]" = 88CBA74T8K;
ENABLE_BITCODE = NO;
Expand Down Expand Up @@ -5024,7 +5024,7 @@
CODE_SIGN_IDENTITY = "Apple Development";
"CODE_SIGN_IDENTITY[sdk=iphoneos*]" = "Apple Distribution";
CODE_SIGN_STYLE = Manual;
CURRENT_PROJECT_VERSION = 225;
CURRENT_PROJECT_VERSION = 226;
DEVELOPMENT_TEAM = "";
"DEVELOPMENT_TEAM[sdk=iphoneos*]" = 88CBA74T8K;
ENABLE_BITCODE = NO;
Expand Down Expand Up @@ -5084,7 +5084,7 @@
CLANG_WARN_NON_LITERAL_NULL_CONVERSION = YES_ERROR;
CODE_SIGN_ENTITLEMENTS = Palace/SimplyE.entitlements;
CODE_SIGN_IDENTITY = "iPhone Distribution";
CURRENT_PROJECT_VERSION = 225;
CURRENT_PROJECT_VERSION = 226;
DEVELOPMENT_TEAM = 88CBA74T8K;
"DEVELOPMENT_TEAM[sdk=iphoneos*]" = 88CBA74T8K;
ENABLE_BITCODE = NO;
Expand Down
2 changes: 1 addition & 1 deletion Palace/Accounts/Library/Account.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ protocol AccountLogoDelegate: AnyObject {
}
var syncPermissionGranted:Bool {
get {
return getAccountDictionaryKey(accountSyncEnabledKey) as? Bool ?? false
return getAccountDictionaryKey(accountSyncEnabledKey) as? Bool ?? true
}
set {
setAccountDictionaryKey(accountSyncEnabledKey, toValue: newValue as AnyObject)
Expand Down
7 changes: 0 additions & 7 deletions Palace/Book/UI/TPPBookCellDelegate.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,13 +140,6 @@ - (void)openBook:(TPPBook *)book completion:(void (^ _Nullable)(void))completion
- (void)openEPUB:(TPPBook *)book
{
[[TPPRootTabBarController sharedController] presentBook:book];

[TPPAnnotations requestServerSyncStatusForAccount:[TPPUserAccount sharedAccount] completion:^(BOOL enableSync) {
if (enableSync == YES) {
Account *currentAccount = [[AccountsManager sharedInstance] currentAccount];
currentAccount.details.syncPermissionGranted = enableSync;
}
}];
}

- (void)openPDF:(TPPBook *)book {
Expand Down
6 changes: 3 additions & 3 deletions Palace/Network/TPPNetworkExecutor.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ extension TPPNetworkExecutor: TPPRequestExecuting {
/// the network or from the cache.
/// - Returns: The task issueing the given request.
@discardableResult
func executeRequest(_ req: URLRequest, completion: @escaping (_: NYPLResult<Data>) -> Void) -> URLSessionDataTask? {
func executeRequest(_ req: URLRequest, completion: @escaping (_: NYPLResult<Data>) -> Void) -> URLSessionDataTask {
let userAccount = TPPUserAccount.sharedAccount()

if let authDefinition = userAccount.authDefinition, authDefinition.isSaml {
Expand All @@ -87,13 +87,13 @@ extension TPPNetworkExecutor: TPPRequestExecuting {

if userAccount.isTokenRefreshRequired() {
handleTokenRefresh(for: req, completion: completion)
return nil
return URLSessionDataTask()
}

if req.hasRetried {
let error = createErrorForRetryFailure()
completion(NYPLResult.failure(error, nil))
return nil
return URLSessionDataTask()
}

return performDataTask(with: req, completion: completion)
Expand Down
2 changes: 1 addition & 1 deletion Palace/Network/TPPRequestExecuting.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ protocol TPPRequestExecuting {
/// - Returns: The task issueing the given request.
@discardableResult
func executeRequest(_ req: URLRequest,
completion: @escaping (_: NYPLResult<Data>) -> Void) -> URLSessionDataTask?
completion: @escaping (_: NYPLResult<Data>) -> Void) -> URLSessionDataTask

var requestTimeout: TimeInterval {get}

Expand Down
204 changes: 1 addition & 203 deletions Palace/Reader2/Bookmarks/TPPAnnotations.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,203 +30,6 @@ protocol AnnotationsManager {
}

@objcMembers final class TPPAnnotations: NSObject {
// MARK: - Sync Settings

/// Shows (if needed) the opt-in flow for syncing the user bookmarks and
/// reading position on the server.
///
/// This is implemented with an alert that is displayed once for the current
/// library once the user is signed in, i.e.:
/// - If the user has never seen it before, show it.
/// - If the user has seen it on one of their other devices, don't show it.
/// Opting in will attempt to enable on the server, with appropriate error handling.
/// - Note: This flow will be run only for the user account on the currently
/// selected library. Anything else will result in a no-op.
/// - Parameters:
/// - userAccount: the account to attempt to enable annotations-syncing on.
/// - completion: if a network request is actually performed, this block
/// is guaranteed to be called on the Main queue. Otherwise, this is called
/// either on the same thread the function was invoked on or on the main
/// thread.
class func requestServerSyncStatus(forAccount userAccount: TPPUserAccount,
completion: @escaping (_ enableSync: Bool) -> ()) {

guard syncIsPossible(userAccount) else {
Log.debug(#file, "Account does not satisfy conditions for sync setting request.")
completion(false)
return
}

let settings = TPPSettings.shared

if (settings.userHasSeenFirstTimeSyncMessage == true &&
AccountsManager.shared.currentAccount?.details?.syncPermissionGranted == false) {
completion(false)
return
}

self.permissionUrlRequest { (initialized, syncIsPermitted) in

if (initialized && syncIsPermitted) {
completion(true)
settings.userHasSeenFirstTimeSyncMessage = true;
Log.debug(#file, "Sync has already been enabled on the server. Enable here as well.")
return
} else if (!initialized && settings.userHasSeenFirstTimeSyncMessage == false) {
Log.debug(#file, "Sync has never been initialized for the patron. Showing UIAlertController flow.")
let title = "Palace Sync"
let message = "Enable sync to save your reading position and bookmarks to your other devices.\n\nYou can change this any time in Settings."
let alertController = UIAlertController.init(title: title, message: message, preferredStyle: .alert)
let notNowAction = UIAlertAction.init(title: "Not Now", style: .default, handler: { action in
completion(false)
settings.userHasSeenFirstTimeSyncMessage = true;
})
let enableSyncAction = UIAlertAction.init(title: "Enable Sync", style: .default, handler: { action in
self.updateServerSyncSetting(toEnabled: true) { success in
completion(success)
settings.userHasSeenFirstTimeSyncMessage = true;
}
})
alertController.addAction(notNowAction)
alertController.addAction(enableSyncAction)
alertController.preferredAction = enableSyncAction
TPPAlertUtils.presentFromViewControllerOrNil(alertController: alertController, viewController: nil, animated: true, completion: nil)
} else {
completion(false)
}
}
}

/// Ask the server to enable Annotations on the current user account for the
/// currently selected library. Server will return null, true, or false. Null
/// assumes the user has never been introduced to the feature ("initialized").
/// The closure expects "enabled" which is strictly to inform this single client
/// how to respond based on the server's info.
/// - Parameters:
/// - enabled: whether to enable annotation-syncing or not.
/// - completion: if a network request is actually performed, this block
/// is guaranteed to be called on the Main queue. Otherwise, this is called
/// on the same thread the function was invoked on.
class func updateServerSyncSetting(toEnabled enabled: Bool, completion:@escaping (Bool)->()) {
if (TPPUserAccount.sharedAccount().hasCredentials() &&
AccountsManager.shared.currentAccount?.details?.supportsSimplyESync == true) {
guard let userProfileUrl = URL(string: AccountsManager.shared.currentAccount?.details?.userProfileUrl ?? "") else {
Log.error(#file, "Could not create user profile URL from string. Abandoning attempt to update sync setting.")
completion(false)
return
}
let parameters = ["settings": ["simplified:synchronize_annotations": enabled]] as [String : Any]
syncSettingUrlRequest(userProfileUrl, parameters, 20, { success in
if !success {
handleSyncSettingError()
}
completion(success)
})
}
}


/// - Parameter successHandler: Called only if the request succeeds.
/// Always called on the main thread.
private class func permissionUrlRequest(successHandler: @escaping (_ initialized: Bool, _ syncIsPermitted: Bool) -> ()) {

guard let userProfileUrl = URL(string: AccountsManager.shared.currentAccount?.details?.userProfileUrl ?? "") else {
Log.error(#file, "Failed to create user profile URL from string. Abandoning attempt to retrieve sync setting.")
return
}

var request = TPPNetworkExecutor.shared.request(for: userProfileUrl)
request.timeoutInterval = 60

let dataTask = TPPNetworkExecutor.shared.GET(request: request) { (data, response, error) in
DispatchQueue.main.async {

if let error = error as NSError? {
Log.error(#file, "Request Error Code: \(error.code). Description: \(error.localizedDescription)")
return
}
guard let data = data,
let response = (response as? HTTPURLResponse) else {
Log.error(#file, "No Data or No Server Response present after request.")
return
}

if response.statusCode == 200 {
if let json = try? JSONSerialization.jsonObject(with: data, options: []) as! [String:Any],
let settings = json["settings"] as? [String:Any],
let syncSetting = settings["simplified:synchronize_annotations"] {
if syncSetting is NSNull {
successHandler(false, false)
} else {
successHandler(true, syncSetting as? Bool ?? false)
}
} else {
Log.error(#file, "Error parsing JSON or finding sync-setting key/value.")
}
} else {
Log.error(#file, "Server response returned error code: \(response.statusCode))")
}
}
}
dataTask?.resume()
}

/// - parameter completion: if a network request is actually performed, this
/// is guaranteed to be called on the Main queue. Otherwise, this is called
/// on the same thread the function was invoked on.
private class func syncSettingUrlRequest(_ url: URL,
_ parameters: [String:Any],
_ timeout: Double?,
_ completion: @escaping (Bool)->()) {
guard let jsonData = try? JSONSerialization.data(withJSONObject: parameters, options: [.prettyPrinted]) else {
Log.error(#file, "Network request abandoned. Could not create JSON from given parameters.")
completion(false)
return
}

var request = TPPNetworkExecutor.shared.request(for: url)
request.httpBody = jsonData
request.setValue("vnd.librarysimplified/user-profile+json", forHTTPHeaderField: "Content-Type")
if let timeout = timeout {
request.timeoutInterval = timeout
}

let task = TPPNetworkExecutor.shared.PUT(request: request) { (data, response, error) in
DispatchQueue.main.async {

if let error = error as NSError? {
Log.error(#file, "Request Error Code: \(error.code). Description: \(error.localizedDescription)")
if NetworkQueue.StatusCodes.contains(error.code) {
self.addToOfflineQueue(nil, url, parameters)
}
completion(false)
return
}
guard let statusCode = (response as? HTTPURLResponse)?.statusCode else {
Log.error(#file, "No response received from server")
completion(false)
return
}

if statusCode == 200 {
completion(true)
} else {
Log.error(#file, "Server Response Error. Status Code: \(statusCode)")
completion(false)
}
}
}
task?.resume()
}

class func handleSyncSettingError() {
let title = Strings.Error.syncSettingChangeErrorTitle
let message = Strings.Error.syncSettingsChangeErrorBody
let alert = UIAlertController.init(title: title, message: message, preferredStyle: .alert)
alert.addAction(UIAlertAction.init(title: Strings.Generic.ok, style: .default, handler: nil))
TPPAlertUtils.presentFromViewControllerOrNil(alertController: alert, viewController: nil, animated: true, completion: nil)
}

// MARK: - Reading Position

/// Reads the current reading position from the server, parses the response
Expand Down Expand Up @@ -483,11 +286,6 @@ protocol AnnotationsManager {

class func deleteBookmarks(_ bookmarks: [TPPReadiumBookmark]) {

if !syncIsPossibleAndPermitted() {
Log.debug(#file, "Account does not support sync or sync is disabled.")
return
}

for localBookmark in bookmarks {
if let annotationID = localBookmark.annotationId {
deleteBookmark(annotationId: annotationID) { success in
Expand All @@ -506,7 +304,7 @@ protocol AnnotationsManager {

if !syncIsPossibleAndPermitted() {
Log.debug(#file, "Account does not support sync or sync is disabled.")
completionHandler(false)
completionHandler(true)
return
}

Expand Down
9 changes: 0 additions & 9 deletions Palace/Settings/TPPSettings.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,6 @@ import Foundation
}
}

var userHasSeenFirstTimeSyncMessage: Bool {
get {
UserDefaults.standard.bool(forKey: TPPSettings.userSeenFirstTimeSyncMessageKey)
}
set(b) {
UserDefaults.standard.set(b, forKey: TPPSettings.userSeenFirstTimeSyncMessageKey)
}
}

var useBetaLibraries: Bool {
get {
UserDefaults.standard.bool(forKey: TPPSettings.useBetaLibrariesKey)
Expand Down
30 changes: 4 additions & 26 deletions Palace/Settings/TPPSettingsAccountDetailViewController.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,6 @@ - (void)setupViews
[self setupTableData];

self.syncSwitch = [[UISwitch alloc] initWithFrame:CGRectZero];
[self checkSyncPermissionForCurrentPatron];
}

- (NSArray *) cellsForAuthMethod:(AccountDetailsAuthentication *)authenticationMethod {
Expand Down Expand Up @@ -904,6 +903,8 @@ - (UITableViewCell *)tableView:(__attribute__((unused)) UITableView *)tableView
initWithStyle:UITableViewCellStyleDefault
reuseIdentifier:nil];
self.syncSwitch.on = self.selectedAccount.details.syncPermissionGranted;
self.syncSwitch.enabled = true;

cell.accessoryView = self.syncSwitch;
[self.syncSwitch addTarget:self action:@selector(syncSwitchChanged:) forControlEvents:UIControlEventValueChanged];
cell.selectionStyle = UITableViewCellSelectionStyleNone;
Expand Down Expand Up @@ -1195,7 +1196,6 @@ - (void)accountDidChange
{
[[NSOperationQueue mainQueue] addOperationWithBlock:^{
if(self.selectedUserAccount.hasCredentials) {
[self checkSyncPermissionForCurrentPatron];
self.usernameTextField.text = self.selectedUserAccount.barcode;
self.usernameTextField.enabled = NO;
self.usernameTextField.textColor = [UIColor grayColor];
Expand Down Expand Up @@ -1367,30 +1367,8 @@ - (void)confirmAgeChange:(void (^)(BOOL))completion

- (void)syncSwitchChanged:(UISwitch*)sender
{
const BOOL currentSwitchState = sender.on;

if (sender.on) {
self.syncSwitch.enabled = NO;
} else {
self.syncSwitch.on = NO;
}

__weak __auto_type weakSelf = self;
[self.businessLogic changeSyncPermissionTo:currentSwitchState
postServerSyncCompletion:^(BOOL success) {
weakSelf.syncSwitch.enabled = YES;
weakSelf.syncSwitch.on = success;
}];
}

- (void)checkSyncPermissionForCurrentPatron
{
[self.businessLogic checkSyncPermissionWithPreWork:^{
self.syncSwitch.enabled = NO;
} postWork:^(BOOL enableSync){
self.syncSwitch.on = enableSync;
self.syncSwitch.enabled = YES;
}];
self.syncSwitch.on = sender.on;
self.selectedAccount.details.syncPermissionGranted = sender.on;
}

#pragma mark - UIApplication callbacks
Expand Down
Loading

0 comments on commit b4448b9

Please sign in to comment.