Refactor HREF normalization and models (#358)

Author: mickael-menu

.github/workflows/checks.yml

@@ -7,17 +7,17 @@ on:
 
 env:
   platform: ${{ 'iOS Simulator' }}
-  device: ${{ 'iPhone 12' }}
+  device: ${{ 'iPhone 15' }}
   commit_sha: ${{ github.sha }}
+  DEVELOPER_DIR: /Applications/Xcode_15.0.1.app/Contents/Developer
 
 jobs:
   build:
     name: Build
-    runs-on: macos-12
+    runs-on: macos-13
     if: ${{ !github.event.pull_request.draft }}
     env:
       scheme: ${{ 'Readium-Package' }}
-      DEVELOPER_DIR: /Applications/Xcode_13.4.1.app/Contents/Developer
 
     steps:
       - name: Checkout
@@ -40,7 +40,7 @@ jobs:
 
   lint:
     name: Lint
-    runs-on: macos-12
+    runs-on: macos-13
     if: ${{ !github.event.pull_request.draft }}
     env:
       scripts: ${{ 'Sources/Navigator/EPUB/Scripts' }}
@@ -63,7 +63,7 @@ jobs:
 
   int-dev:
     name: Integration (Local)
-    runs-on: macos-12
+    runs-on: macos-13
     if: ${{ !github.event.pull_request.draft }}
     defaults:
       run:
@@ -84,7 +84,7 @@ jobs:
 
   int-spm:
     name: Integration (Swift Package Manager)
-    runs-on: macos-12
+    runs-on: macos-13
     if: ${{ !github.event.pull_request.draft }}
     defaults:
       run:
@@ -111,7 +111,7 @@ jobs:
 
   int-carthage:
     name: Integration (Carthage)
-    runs-on: macos-12
+    runs-on: macos-13
     if: ${{ !github.event.pull_request.draft }}
     defaults:
       run:
@@ -141,7 +141,7 @@ jobs:
   int-cocoapods:
     name: Integration (CocoaPods)
     if: github.event_name == 'push'
-    runs-on: macos-12
+    runs-on: macos-13
     defaults:
       run:
         working-directory: TestApp

CHANGELOG.md

@@ -4,7 +4,21 @@ All notable changes to this project will be documented in this file. Take a look
 
 **Warning:** Features marked as *alpha* may change or be removed in a future release without notice. Use with caution.
 
-<!-- ## [Unreleased] -->
+## [Unreleased]
+
+### Changed
+
+* Many APIs now expect one of the new URL types (`RelativeURL`, `AbsoluteURL`, `HTTPURL` and `FileURL`). This is helpful because:
+    * It validates at compile time that we provide a URL that is supported.
+    * The API's capabilities are better documented, e.g. a download API could look like this : `download(url: HTTPURL) -> FileURL`. 
+
+#### Shared
+
+* `Link` and `Locator`'s `href` are normalized as valid URLs to improve interoperability with the Readium Web toolkits.
+   * **You MUST migrate your database if you were persisting HREFs and Locators**. Take a look at [the migration guide](Documentation/Migration%20Guide.md) for guidance.
+* Links are not resolved to the `self` URL of a manifest anymore. However, you can still normalize the HREFs yourselves by calling `Manifest.normalizeHREFsToSelf()`.
+* `Publication.localizedTitle` is now optional, as we cannot guarantee a publication will always have a title.
+
 
 ## [2.6.1]
 

Documentation/Migration Guide.md

@@ -2,6 +2,55 @@
 
 All migration steps necessary in reading apps to upgrade to major versions of the Swift Readium toolkit will be documented in this file.
 
+## Unreleased
+
+### Migration of HREFs and Locators (bookmarks, annotations, etc.)
+
+ :warning: This requires a database migration in your application, if you were persisting `Locator` objects.
+
+ In Readium v2.x, a `Link` or `Locator`'s `href` could be either:
+
+ * a valid absolute URL for a streamed publication, e.g. `https://domain.com/isbn/dir/my%20chapter.html`,
+ * a percent-decoded path for a local archive such as an EPUB, e.g. `/dir/my chapter.html`.
+     * Note that it was relative to the root of the archive (`/`).
+
+ To improve the interoperability with other Readium toolkits (in particular the Readium Web Toolkits, which only work in a streaming context) **Readium v3 now generates and expects valid URLs** for `Locator` and `Link`'s `href`.
+
+ * `https://domain.com/isbn/dir/my%20chapter.html` is left unchanged, as it was already a valid URL.
+ * `/dir/my chapter.html` becomes the relative URL path `dir/my%20chapter.html`
+     * We dropped the `/` prefix to avoid issues when resolving to a base URL.
+     * Special characters are percent-encoded.
+
+ **You must migrate the HREFs or Locators stored in your database** when upgrading to Readium 3. To assist you, two helpers are provided: `AnyURL(legacyHREF:)` and `Locator(legacyJSONString:)`.
+
+ Here's an example of a [GRDB migration](https://swiftpackageindex.com/groue/grdb.swift/master/documentation/grdb/migrations) that can serve as inspiration:
+
+ ```swift
+ migrator.registerMigration("normalizeHREFs") { db in
+    let normalizedRows: [(id: Int, href: String, locator: String)] =
+        try Row.fetchAll(db, sql: "SELECT id, href, locator FROM bookmarks")
+            .compactMap { row in
+                guard
+                    let normalizedHREF = AnyURL(legacyHREF: row["href"])?.string,
+                    let normalizedLocator = try Locator(legacyJSONString: row["locator"])?.jsonString
+                else {
+                    return nil
+                }
+                return (row["id"], normalizedHREF, normalizedLocator)
+            }
+            
+    let updateStmt = try db.makeStatement(sql: "UPDATE bookmarks SET href = :href, locator = :locator WHERE id = :id")
+    for (id, href, locator) in normalizedRows {
+        try updateStmt.execute(arguments: [
+            "id": id,
+            "href": href
+            "locator": locator
+        ])
+    }
+}
+```
+
+
 ## 2.5.0
 
 In the following migration steps, only the `ReadiumInternal` one is mandatory with 2.5.0.

Makefile

@@ -23,7 +23,7 @@ scripts:
 .PHONY: test
 test:
 	# To limit to a particular test suite: -only-testing:R2SharedTests
-	xcodebuild test -scheme "Readium-Package" -destination "platform=iOS Simulator,name=iPhone 12" | xcbeautify -q
+	xcodebuild test -scheme "Readium-Package" -destination "platform=iOS Simulator,name=iPhone 15" | xcbeautify -q
 
 .PHONY: lint-format
 lint-format:

README.md

@@ -16,12 +16,12 @@ A [Test App](TestApp) demonstrates how to integrate the Readium Swift toolkit in
 
 <!-- https://swiftversion.net/ -->
 
-| Readium   | iOS  | Swift compiler | Xcode |
-|-----------|------|----------------|-------|
-| `develop` | 11.0 | 5.6.1          | 13.4  |
-| 2.5.1     | 11.0 | 5.6.1          | 13.4  |
-| 2.5.0     | 10.0 | 5.6.1          | 13.4  |
-| 2.4.0     | 10.0 | 5.3.2          | 12.4  |
+| Readium   | iOS  | Swift compiler | Xcode  |
+|-----------|------|----------------|--------|
+| `develop` | 11.0 | 5.9            | 15.0.1 |
+| 2.5.1     | 11.0 | 5.6.1          | 13.4   |
+| 2.5.0     | 10.0 | 5.6.1          | 13.4   |
+| 2.4.0     | 10.0 | 5.3.2          | 12.4   |
 
 ## Using Readium
 

Sources/Adapters/GCDWebServer/GCDHTTPServer.swift

@@ -12,6 +12,7 @@ import UIKit
 public enum GCDHTTPServerError: Error {
     case failedToStartServer(cause: Error)
     case serverNotStarted
+    case invalidEndpoint(HTTPServerEndpoint)
     case nullServerURL
 }
 
@@ -24,14 +25,14 @@ public class GCDHTTPServer: HTTPServer, Loggable {
     private let server = GCDWebServer()
 
     /// Mapping between endpoints and their handlers.
-    private var handlers: [HTTPServerEndpoint: (HTTPServerRequest) -> Resource] = [:]
+    private var handlers: [HTTPURL: (HTTPServerRequest) -> Resource] = [:]
 
     /// Mapping between endpoints and resource transformers.
-    private var transformers: [HTTPServerEndpoint: [ResourceTransformer]] = [:]
+    private var transformers: [HTTPURL: [ResourceTransformer]] = [:]
 
     private enum State {
         case stopped
-        case started(port: UInt, baseURL: URL)
+        case started(port: UInt, baseURL: HTTPURL)
     }
 
     private var state: State = .stopped
@@ -112,12 +113,12 @@ public class GCDHTTPServer: HTTPServer, Loggable {
         }
 
         queue.async { [self] in
-            var path = request.path.removingPrefix("/")
-            path = path.removingPercentEncoding ?? path
-            // Remove anchors and query params
-            let pathWithoutAnchor = path.components(separatedBy: .init(charactersIn: "#?")).first ?? path
+            guard let url = request.url.httpURL else {
+                completion(FailureResource(link: Link(href: request.url.absoluteString), error: .notFound(nil)))
+                return
+            }
 
-            func transform(resource: Resource, at endpoint: HTTPServerEndpoint) -> Resource {
+            func transform(resource: Resource, at endpoint: HTTPURL) -> Resource {
                 guard let transformers = transformers[endpoint], !transformers.isEmpty else {
                     return resource
                 }
@@ -129,15 +130,15 @@ public class GCDHTTPServer: HTTPServer, Loggable {
             }
 
             for (endpoint, handler) in handlers {
-                if endpoint == pathWithoutAnchor {
-                    let resource = handler(HTTPServerRequest(url: request.url, href: nil))
+                if endpoint == url.removingQuery().removingFragment() {
+                    let resource = handler(HTTPServerRequest(url: url, href: nil))
                     completion(transform(resource: resource, at: endpoint))
                     return
 
-                } else if path.hasPrefix(endpoint.addingSuffix("/")) {
+                } else if let href = endpoint.relativize(url) {
                     let resource = handler(HTTPServerRequest(
-                        url: request.url,
-                        href: path.removingPrefix(endpoint.removingSuffix("/"))
+                        url: url,
+                        href: href
                     ))
                     completion(transform(resource: resource, at: endpoint))
                     return
@@ -150,34 +151,46 @@ public class GCDHTTPServer: HTTPServer, Loggable {
 
     // MARK: HTTPServer
 
-    public func serve(at endpoint: HTTPServerEndpoint, handler: @escaping (HTTPServerRequest) -> Resource) throws -> URL {
+    public func serve(at endpoint: HTTPServerEndpoint, handler: @escaping (HTTPServerRequest) -> Resource) throws -> HTTPURL {
         try queue.sync(flags: .barrier) {
             if case .stopped = state {
                 try start()
             }
-            guard case let .started(port: _, baseURL: baseURL) = state else {
-                throw GCDHTTPServerError.serverNotStarted
-            }
 
-            handlers[endpoint] = handler
-
-            return baseURL.appendingPathComponent(endpoint)
+            let url = try url(for: endpoint)
+            handlers[url] = handler
+            return url
         }
     }
 
-    public func transformResources(at endpoint: HTTPServerEndpoint, with transformer: @escaping ResourceTransformer) {
-        queue.sync(flags: .barrier) {
-            var trs = transformers[endpoint] ?? []
+    public func transformResources(at endpoint: HTTPServerEndpoint, with transformer: @escaping ResourceTransformer) throws {
+        try queue.sync(flags: .barrier) {
+            let url = try url(for: endpoint)
+            var trs = transformers[url] ?? []
             trs.append(transformer)
-            transformers[endpoint] = trs
+            transformers[url] = trs
         }
     }
 
-    public func remove(at endpoint: HTTPServerEndpoint) {
-        queue.sync(flags: .barrier) {
-            handlers.removeValue(forKey: endpoint)
-            transformers.removeValue(forKey: endpoint)
+    public func remove(at endpoint: HTTPServerEndpoint) throws {
+        try queue.sync(flags: .barrier) {
+            let url = try url(for: endpoint)
+            handlers.removeValue(forKey: url)
+            transformers.removeValue(forKey: url)
+        }
+    }
+
+    private func url(for endpoint: HTTPServerEndpoint) throws -> HTTPURL {
+        guard case let .started(port: _, baseURL: baseURL) = state else {
+            throw GCDHTTPServerError.serverNotStarted
+        }
+        guard
+            let endpointPath = RelativeURL(string: endpoint.addingSuffix("/")),
+            let endpointURL = baseURL.resolve(endpointPath)
+        else {
+            throw GCDHTTPServerError.invalidEndpoint(endpoint)
         }
+        return endpointURL
     }
 
     // MARK: Server lifecycle
@@ -230,7 +243,7 @@ public class GCDHTTPServer: HTTPServer, Loggable {
             throw GCDHTTPServerError.failedToStartServer(cause: error)
         }
 
-        guard let baseURL = server.serverURL else {
+        guard let baseURL = server.serverURL?.httpURL else {
             stop()
             throw GCDHTTPServerError.nullServerURL
         }

Sources/Internal/Extensions/Array.swift

@@ -57,6 +57,10 @@ public extension Array where Element: Hashable {
         array.removeAll { other in other == element }
         return array
     }
+
+    @inlinable mutating func remove(_ element: Element) {
+        removeAll { other in other == element }
+    }
 }
 
 public extension Array where Element: Equatable {

Sources/Internal/Extensions/String.swift

@@ -84,4 +84,11 @@ public extension String {
         }
         return index
     }
+
+    func orNilIfEmpty() -> String? {
+        guard !isEmpty else {
+            return nil
+        }
+        return self
+    }
 }

Sources/Internal/Extensions/URL.swift

@@ -0,0 +1,36 @@
+//
+//  Copyright 2023 Readium Foundation. All rights reserved.
+//  Use of this source code is governed by the BSD-style license
+//  available in the top-level LICENSE file of the project.
+//
+
+import Foundation
+
+public extension URL {
+    /// Removes the fragment portion of the receiver and returns it.
+    mutating func removeFragment() -> String? {
+        var fragment: String?
+        guard let result = copy({
+            fragment = $0.fragment
+            $0.fragment = nil
+        }) else {
+            return nil
+        }
+        self = result
+        return fragment
+    }
+
+    /// Creates a copy of the receiver after removing its fragment portion.
+    func removingFragment() -> URL? {
+        copy { $0.fragment = nil }
+    }
+
+    /// Creates a copy of the receiver after modifying its components.
+    func copy(_ changes: (inout URLComponents) -> Void) -> URL? {
+        guard var components = URLComponents(url: self, resolvingAgainstBaseURL: true) else {
+            return nil
+        }
+        changes(&components)
+        return components.url
+    }
+}

Sources/LCP/Content Protection/LCPContentProtection.swift

@@ -34,7 +34,7 @@ final class LCPContentProtection: ContentProtection, Loggable {
             ?? self.authentication
 
         service.retrieveLicense(
-            from: file.url,
+            from: file.file,
             authentication: authentication,
             allowUserInteraction: allowUserInteraction,
             sender: sender