Merge pull request #1629 from NYPL-Simplified/IOS-631/publicationBaseURL-crash

ettore · web-flow · commit 229e32669b2b · 2024-02-05T15:05:13.000-08:00
IOS-631 Fix crash when Publication::baseURL is nil
diff --git a/Simplified.xcodeproj/project.pbxproj b/Simplified.xcodeproj/project.pbxproj
@@ -5014,7 +5014,7 @@
 				CLANG_WARN_NON_LITERAL_NULL_CONVERSION = YES_ERROR;
 				CODE_SIGN_ENTITLEMENTS = Simplified/SimplyE.entitlements;
 				CODE_SIGN_IDENTITY = "iPhone Developer";
-				CURRENT_PROJECT_VERSION = 4;
+				CURRENT_PROJECT_VERSION = 0;
 				DEVELOPMENT_TEAM = 7262U6ST2R;
 				"EXCLUDED_ARCHS[sdk=iphonesimulator*]" = arm64;
 				GCC_PRECOMPILE_PREFIX_HEADER = YES;
@@ -5034,7 +5034,7 @@
 					"$(inherited)",
 					"@executable_path/Frameworks",
 				);
-				MARKETING_VERSION = 3.9.5;
+				MARKETING_VERSION = 3.9.6;
 				OTHER_LDFLAGS = (
 					"$(inherited)",
 					"-ld_classic",
@@ -5068,7 +5068,7 @@
 				CLANG_WARN_NON_LITERAL_NULL_CONVERSION = YES_ERROR;
 				CODE_SIGN_ENTITLEMENTS = Simplified/SimplyE.entitlements;
 				CODE_SIGN_IDENTITY = "iPhone Distribution";
-				CURRENT_PROJECT_VERSION = 4;
+				CURRENT_PROJECT_VERSION = 0;
 				DEVELOPMENT_TEAM = 7262U6ST2R;
 				GCC_PRECOMPILE_PREFIX_HEADER = YES;
 				GCC_PREFIX_HEADER = "Simplified/AppInfrastructure/Simplified-Prefix.pch";
@@ -5087,7 +5087,7 @@
 					"$(inherited)",
 					"@executable_path/Frameworks",
 				);
-				MARKETING_VERSION = 3.9.5;
+				MARKETING_VERSION = 3.9.6;
 				OTHER_LDFLAGS = (
 					"$(inherited)",
 					"-ld_classic",
diff --git a/Simplified/Book/UI/NYPLBookButtonsView.h b/Simplified/Book/UI/NYPLBookButtonsView.h
@@ -8,7 +8,8 @@
 
 - (void)didSelectReturnForBook:(NYPLBook *)book;
 - (void)didSelectDownloadForBook:(NYPLBook *)book;
-- (void)didSelectReadForBook:(NYPLBook *)book successCompletion:(void(^)(void))completion;
+- (void)didSelectReadForBook:(NYPLBook *)book
+                  completion:(void(^)(BOOL success))completion;
 
 @end
 
diff --git a/Simplified/Book/UI/NYPLBookButtonsView.m b/Simplified/Book/UI/NYPLBookButtonsView.m
@@ -434,7 +434,7 @@ - (void)didSelectRead
   self.activityIndicator.center = self.readButton.center;
   [self updateProcessingState:YES];
   [self.delegate didSelectReadForBook:self.book
-                    successCompletion:^{
+                           completion:^(__unused BOOL success) {
     [self updateProcessingState];
   }];
 }
diff --git a/Simplified/Book/UI/NYPLBookCellDelegate.m b/Simplified/Book/UI/NYPLBookCellDelegate.m
@@ -63,7 +63,8 @@ - (void)didSelectDownloadForBook:(NYPLBook *)book
   [[NYPLMyBooksDownloadCenter sharedDownloadCenter] startDownloadForBook:book];
 }
 
-- (void)didSelectReadForBook:(NYPLBook *)book successCompletion:(void(^)(void))successCompletion
+- (void)didSelectReadForBook:(NYPLBook *)book 
+                  completion:(void(^)(BOOL success))completion
 {
 #if FEATURE_DRM_CONNECTOR
   // Try to prevent blank books bug
@@ -81,37 +82,40 @@ - (void)didSelectReadForBook:(NYPLBook *)book successCompletion:(void(^)(void))s
                                                        completion:^(BOOL isSignedIn) {
       if (isSignedIn) {
         dispatch_async(dispatch_get_main_queue(), ^{
-          [self openBook:book successCompletion:successCompletion];
+          [self openBook:book completion:completion];
           // with ARC, retain the reauthenticator until we're done, then release it
           reauthenticator = nil;
         });
       }
     }];
   } else {
-    [self openBook:book successCompletion:successCompletion];
+    [self openBook:book completion:completion];
   }
 #else
-  [self openBook:book successCompletion:successCompletion];
+  [self openBook:book completion:completion];
 #endif//FEATURE_DRM_CONNECTOR
 }
 
-- (void)openBook:(NYPLBook *)book successCompletion:(void(^)(void))successCompletion
+- (void)openBook:(NYPLBook *)book completion:(void(^)(BOOL success))completion
 {
   [NYPLCirculationAnalytics postEvent:@"open_book" withBook:book];
 
   switch (book.defaultBookContentType) {
     case NYPLBookContentTypeEPUB:
-      [[[NYPLEPUBOpener alloc] init] open:book successCompletion:successCompletion];
+      [[[NYPLEPUBOpener alloc] init] open:book completion:completion];
       break;
     case NYPLBookContentTypePDF:
       [self openPDF:book];
       break;
 #if FEATURE_AUDIOBOOKS
-    case NYPLBookContentTypeAudiobook:
-      [self openAudiobook:book successCompletion:successCompletion];
+    case NYPLBookContentTypeAudiobook: {
+      [self openAudiobook:book successCompletion:^{
+        completion(YES);
+      }];
       break;
+    }
 #endif
-    default:
+    case NYPLBookContentTypeUnsupported:
       [self presentUnsupportedItemError];
       break;
   }
@@ -181,7 +185,7 @@ - (void)didSelectCancelForBookDownloadingCell:(NYPLBookDownloadingCell *const)ce
 
 - (void)didSelectListenForBookDownloadingCell:(NYPLBookDownloadingCell *)cell
 {
-  [self didSelectReadForBook:cell.book successCompletion:nil];
+  [self didSelectReadForBook:cell.book completion:nil];
 }
 
 @end
diff --git a/Simplified/Book/UI/NYPLBookDetailViewController.m b/Simplified/Book/UI/NYPLBookDetailViewController.m
@@ -139,15 +139,17 @@ - (void)didSelectDownloadForBook:(NYPLBook *)book
 }
 
 - (void)didSelectReadForBook:(NYPLBook *)book
-           successCompletion:(__unused void(^)(void))successCompletion
+                  completion:(void(^)(BOOL success))completion
 {
   [[NYPLBookCellDelegate sharedDelegate] didSelectReadForBook:book
-                                            successCompletion:^{
+                                                   completion:^(BOOL success) {
     // dismiss ourselves if we were presented, since we want to show the ereader
     if (self.modalPresentationStyle == UIModalPresentationFormSheet) {
-      [self dismissViewControllerAnimated:true completion:successCompletion];
+      [self dismissViewControllerAnimated:true completion:^{
+        completion(success);
+      }];
     } else {
-      successCompletion();
+      completion(success);
     }
   }];
 }
diff --git a/Simplified/Book/UI/NYPLEPUBOpener.swift b/Simplified/Book/UI/NYPLEPUBOpener.swift
@@ -10,7 +10,7 @@ import Foundation
 
 class NYPLEPUBOpener: NSObject {
   @objc
-  func open(_ book: NYPLBook, successCompletion: @escaping () -> Void) {
+  func open(_ book: NYPLBook, completion: @escaping (_ success: Bool) -> Void) {
 
     let url = NYPLMyBooksDownloadCenter.shared()?
       .fileURL(forBookIndentifier: book.identifier)
@@ -21,8 +21,8 @@ class NYPLEPUBOpener: NSObject {
     rootTabController?.presentBook(book,
                                    fromFileURL: url,
                                    syncPermission: syncPermission,
-                                   successCompletion: successCompletion)
-    
+                                   completion: completion)
+
     rootTabController?.annotationsSynchronizer?
       .checkServerSyncStatus(settings: NYPLSettings.shared,
                              syncPermissionGranted: syncPermission) { enableSync, error in
diff --git a/Simplified/Reader2/ReaderPresentation/NYPLRootTabBarController+R2.swift b/Simplified/Reader2/ReaderPresentation/NYPLRootTabBarController+R2.swift
@@ -12,7 +12,7 @@ import Foundation
   func presentBook(_ book: NYPLBook,
                    fromFileURL fileURL: URL?,
                    syncPermission: Bool,
-                   successCompletion: (() -> Void)?) {
+                   completion: ((_ success: Bool) -> Void)?) {
     guard let libraryService = r2Owner?.libraryService, let readerModule = r2Owner?.readerModule else {
       return
     }
@@ -30,7 +30,7 @@ import Foundation
                                         syncPermission: syncPermission,
                                         deviceID: drmDeviceID,
                                         in: navVC,
-                                        successCompletion: successCompletion)
+                                        completion: completion)
       case .cancelled:
         // .cancelled is returned when publication has restricted access to its resources and can't be rendered
         NYPLErrorLogger.logError(nil, summary: "Error accessing book resources", metadata: [
diff --git a/Simplified/Reader2/ReaderPresentation/ReaderError.swift b/Simplified/Reader2/ReaderPresentation/ReaderError.swift
@@ -14,15 +14,16 @@ import Foundation
 
 enum ReaderError: LocalizedError {
   case formatNotSupported
-  case epubNotValid
-  
+  case epubNotValid(String)
+
   var errorDescription: String? {
     switch self {
     case .formatNotSupported:
       return NSLocalizedString("The book you were trying to read is in an unsupported format.", comment: "Error message when trying to read a publication with a unsupported format")
-    case .epubNotValid:
-      return NSLocalizedString("The book you were trying to read is corrupted. Please try downloading it again.", comment: "Error message when trying to read an EPUB that is invalid")
+    case .epubNotValid(let errorCause):
+      return String.localizedStringWithFormat(
+        NSLocalizedString("The book you were trying to read is corrupted (%@). Please try downloading it again.", comment: "Error message when trying to read an EPUB that is invalid"),
+        errorCause)
     }
   }
-  
 }
diff --git a/Simplified/Reader2/ReaderPresentation/ReaderModule.swift b/Simplified/Reader2/ReaderPresentation/ReaderModule.swift
@@ -41,7 +41,7 @@ protocol ReaderModuleAPI {
                           syncPermission: Bool,
                           deviceID: String?,
                           in navigationController: UINavigationController,
-                          successCompletion: (() -> Void)?)
+                          completion: ((_ success: Bool) -> Void)?)
   
 }
 
@@ -82,7 +82,7 @@ final class ReaderModule: ReaderModuleAPI {
                           syncPermission: Bool,
                           deviceID: String?,
                           in navigationController: UINavigationController,
-                          successCompletion: (() -> Void)?) {
+                          completion: ((_ success: Bool) -> Void)?) {
     if delegate == nil {
       NYPLErrorLogger.logError(nil, summary: "ReaderModule delegate is not set")
     }
@@ -102,7 +102,7 @@ final class ReaderModule: ReaderModuleAPI {
                                    formatModule: formatModule,
                                    positioningAt: initialLocator,
                                    in: navigationController,
-                                   successCompletion: successCompletion)
+                                   completion: completion)
       }
   }
 
@@ -112,7 +112,7 @@ final class ReaderModule: ReaderModuleAPI {
                                     formatModule: ReaderFormatModule,
                                     positioningAt initialLocator: Locator?,
                                     in navigationController: UINavigationController,
-                                    successCompletion: (() -> Void)?) {
+                                    completion: ((_ success: Bool) -> Void)?) {
     do {
       let readerVC = try formatModule.makeReaderViewController(
         for: publication,
@@ -126,9 +126,10 @@ final class ReaderModule: ReaderModuleAPI {
       readerVC.hidesBottomBarWhenPushed = true
       navigationController.pushViewController(readerVC, animated: true)
 
-      successCompletion?()
+      completion?(true)
     } catch {
       delegate?.presentError(error, from: navigationController)
+      completion?(false)
     }
   }
 }
diff --git a/Simplified/Reader2/ReaderPresentation/ReadingFormats/EPUBModule.swift b/Simplified/Reader2/ReaderPresentation/ReadingFormats/EPUBModule.swift
@@ -39,9 +39,13 @@ final class EPUBModule: ReaderFormatModule {
                                 initialLocation: Locator?) throws -> UIViewController {
       
     guard publication.metadata.identifier != nil else {
-      throw ReaderError.epubNotValid
+      throw ReaderError.epubNotValid("nil metadata id")
     }
     
+    guard publication.baseURL != nil else {
+      throw ReaderError.epubNotValid("nil baseURL")
+    }
+
     let epubVC = NYPLEPUBViewController(publication: publication,
                                         book: book,
                                         initialLocation: initialLocation,
diff --git a/Simplified/Reader2/UI/NYPLEPUBViewController.swift b/Simplified/Reader2/UI/NYPLEPUBViewController.swift
@@ -40,16 +40,21 @@ fileprivate protocol EPUBNavigatorViewControllerMaking {
 }
 
 fileprivate class EPUBNavigatorViewControllerFactory: EPUBNavigatorViewControllerMaking {
+  /// - Important: `publication.baseURL` MUST not be nil when invoking this
+  /// function.
   @available(*, deprecated, message: "To suppress this warning, cast to EPUBNavigatorViewControllerMaking protocol")
   fileprivate
   func make(publication: Publication,
             initialLocation: Locator?,
             resourcesServer: ResourcesServer,
             config: EPUBNavigatorViewController.Configuration) -> EPUBNavigatorViewController {
-    EPUBNavigatorViewController(publication: publication,
-                                initialLocation: initialLocation,
-                                resourcesServer: resourcesServer,
-                                config: config)
+
+    assert(publication.baseURL != nil, "Publication.baseURL MUST not be nil")
+
+    return EPUBNavigatorViewController(publication: publication,
+                                       initialLocation: initialLocation,
+                                       resourcesServer: resourcesServer,
+                                       config: config)
   }
 }
 
@@ -63,6 +68,8 @@ class NYPLEPUBViewController: NYPLBaseReaderViewController {
 
   let userSettings: NYPLR1R2UserSettings
 
+  /// - Important: `publication.baseURL` MUST not be nil when invoking this
+  /// initializer.
   init(publication: Publication,
        book: NYPLBook,
        initialLocation: Locator?,
@@ -92,7 +99,8 @@ class NYPLEPUBViewController: NYPLBaseReaderViewController {
         preloadNextPositionCount: 0,
         debugState: true)
 
-    // when changing the type of the navigator, also change `epubNavigator` getter
+    // create navigator
+    assert(publication.baseURL != nil, "Publication.baseURL MUST not be nil when creating a EPUBNavigatorViewController")
     let factory: EPUBNavigatorViewControllerMaking = EPUBNavigatorViewControllerFactory()
     let navigator = factory.make(publication: publication,
                                  initialLocation: initialLocation,
diff --git a/Simplified/en.lproj/Localizable.strings b/Simplified/en.lproj/Localizable.strings
@@ -196,7 +196,7 @@
 "The book you were trying to open is invalid." = "The book you were trying to open is invalid.";
 "An error was encountered while trying to open this book." = "An error was encountered while trying to open this book.";
 "The book you were trying to read is in an unsupported format." = "The book you were trying to read is in an unsupported format.";
-"The book you were trying to read is corrupted. Please try downloading it again." = "The book you were trying to read is corrupted. Please try downloading it again.";
+"The book you were trying to read is corrupted (%@). Please try downloading it again." = "The book you were trying to read is corrupted (%@). Please try downloading it again.";
 
 // MARK: - Authentication generic
 "Authentication Expired" = "Authentication Expired";
diff --git a/Simplified/it.lproj/Localizable.strings b/Simplified/it.lproj/Localizable.strings
@@ -192,7 +192,7 @@
 "The book you were trying to open is invalid." = "Il libro che stavi cercando di leggere è in un formato non valido.";
 "An error was encountered while trying to open this book." = "Si è verificato un errore durante l'apertura di questo libro.";
 "The book you were trying to read is in an unsupported format." = "Il libro che stavi cercando di aprire è in un formato non supportato.";
-"The book you were trying to read is corrupted. Please try downloading it again." = "Il libro che stavi cercando di leggere è danneggiato. Prova a scaricarlo di nuovo.";
+"The book you were trying to read is corrupted (%@). Please try downloading it again." = "Il libro che stavi cercando di leggere è danneggiato (%@). Prova a scaricarlo di nuovo.";
 
 // MARK: - Authentication generic
 "Authentication Expired" = "Credenziali di accesso scadute";
diff --git a/scripts/detect-app-to-build.sh b/scripts/detect-app-to-build.sh
@@ -42,6 +42,6 @@ echo "** Version / build number changes **"
 echo "SimplyE: ($SIMPLYE_CHANGED)"
 echo "Open eBooks: ($OPENEBOOKS_CHANGED)"
 
-echo "::set-output name=simplye_changed::$SIMPLYE_CHANGED"
-echo "::set-output name=openebooks_changed::$OPENEBOOKS_CHANGED"
-
+# make values accessible to GitHub workflows
+echo "simplye_changed=$SIMPLYE_CHANGED" >> $GITHUB_OUTPUT
+echo "openebooks_changed=$OPENEBOOKS_CHANGED" >> $GITHUB_OUTPUT

Original file line number	Diff line number	Diff line change
`@@ -434,7 +434,7 @@ - (void)didSelectRead`
`434`	`434`	`self.activityIndicator.center = self.readButton.center;`
`435`	`435`	`[self updateProcessingState:YES];`
`436`	`436`	`[self.delegate didSelectReadForBook:self.book`
`437`		`- successCompletion:^{`
	`437`	`+ completion:^(__unused BOOL success) {`
`438`	`438`	`[self updateProcessingState];`
`439`	`439`	`}];`
`440`	`440`	`}`
Original file line number	Diff line number	Diff line change
`@@ -139,15 +139,17 @@ - (void)didSelectDownloadForBook:(NYPLBook *)book`
`139`	`139`	`}`
`140`	`140`
`141`	`141`	`- (void)didSelectReadForBook:(NYPLBook *)book`
`142`		`- successCompletion:(__unused void(^)(void))successCompletion`
	`142`	`+ completion:(void(^)(BOOL success))completion`
`143`	`143`	`{`
`144`	`144`	`[[NYPLBookCellDelegate sharedDelegate] didSelectReadForBook:book`
`145`		`- successCompletion:^{`
	`145`	`+ completion:^(BOOL success) {`
`146`	`146`	`// dismiss ourselves if we were presented, since we want to show the ereader`
`147`	`147`	`if (self.modalPresentationStyle == UIModalPresentationFormSheet) {`
`148`		`- [self dismissViewControllerAnimated:true completion:successCompletion];`
	`148`	`+ [self dismissViewControllerAnimated:true completion:^{`
	`149`	`+ completion(success);`
	`150`	`+ }];`
`149`	`151`	`} else {`
`150`		`- successCompletion();`
	`152`	`+ completion(success);`
`151`	`153`	`}`
`152`	`154`	`}];`
`153`	`155`	`}`