Skip to content

Refactor HREF normalization and models #358

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

Merged
merged 29 commits into from
Nov 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
26522e0
Additional deprecations
mickael-menu Oct 10, 2023
b2c51ee
Add URL helpers
mickael-menu Oct 10, 2023
83d2eb5
Add the URI models
mickael-menu Oct 11, 2023
01d9e1b
Use percent-encoded HREFs everywhere
mickael-menu Oct 13, 2023
8b8216a
Refactor URLs
mickael-menu Oct 14, 2023
6770114
Split the absolute URLs by scheme type
mickael-menu Oct 14, 2023
1fdd029
Use `FileURL` in more places
mickael-menu Oct 15, 2023
4910a94
Fix tests
mickael-menu Oct 15, 2023
7a2c5c2
More test
mickael-menu Oct 15, 2023
eadb964
Refactor absolute URL
mickael-menu Oct 15, 2023
15d89df
More tests
mickael-menu Oct 15, 2023
7d18f4e
Fix build
mickael-menu Nov 9, 2023
c43e31f
Enable model structs mutability
mickael-menu Nov 9, 2023
17864dd
Make `Publication.title` optional
mickael-menu Nov 9, 2023
d717964
Add the HREF normalizer
mickael-menu Nov 9, 2023
5752354
Don't normalize HREFs when parsing a Manifest anymore
mickael-menu Nov 9, 2023
bb40286
Add test plan
mickael-menu Nov 9, 2023
21f72aa
Fix hrefs of ZIP archives
mickael-menu Nov 10, 2023
f951d8a
Refactor parsing a CBZ title
mickael-menu Nov 10, 2023
cea32c7
Use `publication.<extension>` for the default HREF of a single file
mickael-menu Nov 10, 2023
b525825
Add helpers for migrating the HREFs
mickael-menu Nov 13, 2023
d76938a
Use `FileURL` in the LCP service
mickael-menu Nov 13, 2023
cd7774b
Various URL API changes
mickael-menu Nov 14, 2023
e30d13f
Make `path` non nullable
mickael-menu Nov 17, 2023
cb5a2eb
Fix changelog
mickael-menu Nov 20, 2023
c995cb3
Various fixes
mickael-menu Nov 20, 2023
7ae9b94
Remove HREF type
mickael-menu Nov 20, 2023
c0fcacc
Update Carthage project
mickael-menu Nov 20, 2023
a74d437
Fix CI
mickael-menu Nov 20, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions .github/workflows/checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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' }}
Expand All @@ -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:
Expand All @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand Down
16 changes: 15 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down
49 changes: 49 additions & 0 deletions Documentation/Migration Guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
12 changes: 6 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
71 changes: 42 additions & 29 deletions Sources/Adapters/GCDWebServer/GCDHTTPServer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import UIKit
public enum GCDHTTPServerError: Error {
case failedToStartServer(cause: Error)
case serverNotStarted
case invalidEndpoint(HTTPServerEndpoint)
case nullServerURL
}

Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
4 changes: 4 additions & 0 deletions Sources/Internal/Extensions/Array.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 7 additions & 0 deletions Sources/Internal/Extensions/String.swift
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,11 @@ public extension String {
}
return index
}

func orNilIfEmpty() -> String? {
guard !isEmpty else {
return nil
}
return self
}
}
36 changes: 36 additions & 0 deletions Sources/Internal/Extensions/URL.swift
Original file line number Diff line number Diff line change
@@ -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
}
}
2 changes: 1 addition & 1 deletion Sources/LCP/Content Protection/LCPContentProtection.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions Sources/LCP/LCPAcquisition.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public final class LCPAcquisition: Loggable, Cancellable {
public struct Publication {
/// Path to the downloaded publication.
/// You must move this file to the user library's folder.
public let localURL: URL
public let localURL: FileURL

/// Filename that should be used for the publication when importing it in the user library.
public let suggestedFilename: String
Expand Down Expand Up @@ -54,7 +54,7 @@ public final class LCPAcquisition: Loggable, Cancellable {

completion(result)

if case let .success(publication) = result, (try? publication.localURL.checkResourceIsReachable()) == true {
if case let .success(publication) = result, (try? publication.localURL.exists()) == true {
log(.warning, "The acquired LCP publication file was not moved in the completion closure. It will be removed from the file system.")
}
}
Expand Down
Loading