Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 5 additions & 16 deletions Core/Core/Domain/Model/CourseBlockModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@

import Foundation

// Equality is memberwise on purpose. SwiftUI skips redrawing a view whose inputs compare
// equal, so an id-only `==` kept the outline showing stale completion, due dates and
// progress until the course screen was opened again.
public struct CourseStructure: Equatable, Sendable {
public static func == (lhs: CourseStructure, rhs: CourseStructure) -> Bool {
return lhs.id == rhs.id
}

public let id: String
public let graded: Bool
public let completion: Double
Expand Down Expand Up @@ -101,7 +100,7 @@ public struct CourseImage: Decodable, Sendable, Equatable {
}
}

public struct CourseProgress: Sendable {
public struct CourseProgress: Sendable, Equatable {
public let totalAssignmentsCount: Int?
public let assignmentsCompleted: Int?

Expand All @@ -112,11 +111,6 @@ public struct CourseProgress: Sendable {
}

public struct CourseChapter: Identifiable, Sendable, Equatable {
public static func == (lhs: CourseChapter, rhs: CourseChapter) -> Bool {
lhs.id == rhs.id &&
lhs.blockId == rhs.blockId
}

public let blockId: String
public let id: String
public let displayName: String
Expand All @@ -139,11 +133,6 @@ public struct CourseChapter: Identifiable, Sendable, Equatable {
}

public struct CourseSequential: Identifiable, Sendable, Equatable {
public static func == (lhs: CourseSequential, rhs: CourseSequential) -> Bool {
lhs.id == rhs.id &&
lhs.blockId == rhs.blockId
}

public let blockId: String
public let id: String
public let displayName: String
Expand Down Expand Up @@ -231,7 +220,7 @@ public struct SubtitleUrl: Equatable, Sendable {
}
}

public struct SequentialProgress: Sendable {
public struct SequentialProgress: Sendable, Equatable {
public let assignmentType: String?
public let numPointsEarned: Int?
public let numPointsPossible: Int?
Expand Down
6 changes: 4 additions & 2 deletions Core/Core/Extensions/DateExtension.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ public extension Date {
var date: Date
var dateFormatter: DateFormatter?
dateFormatter = DateFormatter()
dateFormatter?.locale = .current

// A fixed-format API date must not follow the user's locale: with the 24-hour
// time switch overridden, `HH` stops matching hours past noon (Apple QA1480).
dateFormatter?.locale = Locale(identifier: "en_US_POSIX")

date = formats.compactMap { format -> Date? in
dateFormatter?.dateFormat = format
guard let formattedDate = dateFormatter?.date(from: iso8601) else { return nil }
Expand Down
36 changes: 25 additions & 11 deletions Core/Core/Network/DownloadManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ public actor DownloadManager: DownloadManagerProtocol, @unchecked Sendable {
private let appStorage: CoreStorage
private let connectivity: ConnectivityProtocol
private let filePathProvider: InstanceFilePathProvider
private let session: Session
private var downloadRequest: DownloadRequest?
nonisolated
private let currentDownloadEventPublisher: PassthroughSubject<DownloadManagerEvent, Never> = .init()
Expand All @@ -230,12 +231,14 @@ public actor DownloadManager: DownloadManagerProtocol, @unchecked Sendable {
persistence: CorePersistenceProtocol,
appStorage: CoreStorage,
connectivity: ConnectivityProtocol,
filePathProvider: InstanceFilePathProvider
filePathProvider: InstanceFilePathProvider,
session: Session = AF
) {
self.persistence = persistence
self.appStorage = appStorage
self.connectivity = connectivity
self.filePathProvider = filePathProvider
self.session = session
if let userId = appStorage.user?.id {
persistence.set(userId: userId)
}
Expand Down Expand Up @@ -606,9 +609,9 @@ public actor DownloadManager: DownloadManagerProtocol, @unchecked Sendable {
}

if let resumeData = download.resumeData {
downloadRequest = AF.download(resumingWith: resumeData, to: destination)
downloadRequest = session.download(resumingWith: resumeData, to: destination).validate()
} else {
downloadRequest = AF.download(url, to: destination)
downloadRequest = session.download(url, to: destination).validate()
}

downloadRequest?.downloadProgress { @Sendable [weak self] prog in
Expand Down Expand Up @@ -636,6 +639,21 @@ public actor DownloadManager: DownloadManagerProtocol, @unchecked Sendable {
currentDownloadEventPublisher.send(.started(download))
}

/// Drops a download the server refused or that broke for a reason other than the
/// connection, so `newDownload()` moves on instead of picking it up again, and leaves
/// it to be reported with the others once the queue is done. `file` is whatever the
/// server sent instead, such as an error page, which must not stay on disk.
private func failDownload(_ download: DownloadDataTask, file: URL?) {
if let file {
try? FileManager.default.removeItem(at: file)
}
failedDownloads.append(download)
try? cancelDownloading(task: download)
Task {
try? await self.newDownload()
}
}

private func setCurrentTask(with task: DownloadDataTask?) async {
currentDownloadTask = task
}
Expand All @@ -646,10 +664,7 @@ public actor DownloadManager: DownloadManagerProtocol, @unchecked Sendable {
if error.isInternetError {
state = .waiting
} else if error.asAFError?.isExplicitlyCancelledError == false {
self.failedDownloads.append(download)
Task {
try? await self.newDownload()
}
failDownload(download, file: response.fileURL)
return
}
}
Expand Down Expand Up @@ -697,9 +712,9 @@ public actor DownloadManager: DownloadManagerProtocol, @unchecked Sendable {
currentDownloadTask = download
currentDownloadTask?.state = .inProgress
if let resumeData = download.resumeData {
downloadRequest = AF.download(resumingWith: resumeData, to: destination)
downloadRequest = session.download(resumingWith: resumeData, to: destination).validate()
} else {
downloadRequest = AF.download(url, to: destination)
downloadRequest = session.download(url, to: destination).validate()
}
downloadRequest?.downloadProgress { [weak self] prog in
guard let self else { return }
Expand Down Expand Up @@ -729,8 +744,7 @@ public actor DownloadManager: DownloadManagerProtocol, @unchecked Sendable {
private func completeHTMLDownload(with response: AFDownloadResponse<URL>, for download: DownloadDataTask) async {
if let error = response.error {
if error.asAFError?.isExplicitlyCancelledError == false {
failedDownloads.append(download)
try? await self.newDownload()
failDownload(download, file: response.fileURL)
return
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ public struct ColorInversionInjection: WebViewScriptInjectionProtocol, CSSInject
img, video, iframe {
filter: invert(100%) hue-rotate(180deg) !important;
}
img {
/* Course figures are often PNGs with dark text on a transparent
background. Give them the white page they were drawn for; the
double inversion above keeps it white. */
background-color: #ffffff;
}
}
"""
return cssScript(with: css)
Expand Down
59 changes: 59 additions & 0 deletions Core/CoreTests/DownloadManager/DownloadManagerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import XCTest
import Combine
import Alamofire
@testable import Core

@MainActor
Expand All @@ -15,6 +16,7 @@ final class DownloadManagerTests: XCTestCase {
var persistence: CorePersistenceProtocolMock!
var storage: CoreStorageMock!
var connectivity: ConnectivityProtocolMock!
var cancellables = Set<AnyCancellable>()

override func setUp() {
super.setUp()
Expand Down Expand Up @@ -120,6 +122,42 @@ final class DownloadManagerTests: XCTestCase {
XCTAssertEqual(currentDownloadTask?.id, mockTask.id)
}

func testNewDownload_WhenServerRefusesTheFile_ReportsItAsFailedNotDownloaded() async throws {
// Given
let task = createMockDownloadTask()
persistence.getDownloadDataTasksHandler = { [task] }
connectivity.isMobileData = false
let configuration = URLSessionConfiguration.af.default
configuration.protocolClasses = [ForbiddenURLProtocol.self]

let downloadManager = DownloadManager(
persistence: persistence,
appStorage: storage,
connectivity: connectivity,
filePathProvider: InstanceFilePathProvider(instanceStore: InstanceProviderMock()),
session: Session(configuration: configuration)
)
var finishedCount = 0
await downloadManager.eventPublisher()
.sink { event in
if case .finished = event {
finishedCount += 1
}
}
.store(in: &cancellables)
let reported = expectation(forNotification: .showDownloadFailed, object: nil) { notification in
(notification.object as? [DownloadDataTask])?.map(\.id) == [task.id]
}

// When
try await downloadManager.resumeDownloading()

// Then
await fulfillment(of: [reported], timeout: 10)
XCTAssertEqual(finishedCount, 0)
XCTAssertEqual(persistence.deleteDownloadDataTasksCallCount, 1)
}

// MARK: - Test Cancel Downloads

func testCancelDownloading_ForSpecificTask_ShouldRemoveFileAndTask() async throws {
Expand Down Expand Up @@ -366,3 +404,24 @@ final class DownloadManagerTests: XCTestCase {
)
}
}

/// Answers every request the way a CDN answers a link it refuses: 403 with an XML body.
private final class ForbiddenURLProtocol: URLProtocol {
override class func canInit(with request: URLRequest) -> Bool { true }
override class func canonicalRequest(for request: URLRequest) -> URLRequest { request }

override func startLoading() {
guard let url = request.url,
let response = HTTPURLResponse(
url: url,
statusCode: 403,
httpVersion: "HTTP/1.1",
headerFields: ["Content-Type": "application/xml"]
) else { return }
client?.urlProtocol(self, didReceive: response, cacheStoragePolicy: .notAllowed)
client?.urlProtocol(self, didLoad: Data("<Error><Code>AccessDenied</Code></Error>".utf8))
client?.urlProtocolDidFinishLoading(self)
}

override func stopLoading() {}
}
23 changes: 23 additions & 0 deletions Course/Course/Presentation/Container/CourseContainerView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ public struct CourseContainerView: View {
self.courseDatesViewModel = courseDatesViewModel
self.courseProgressViewModel = courseProgressViewModel
Task {
// Without access the server has no content to give; don't ask for it.
guard viewModel.isActive != false else { return }
await withTaskGroup(of: Void.self) { group in
group.addTask {
await viewModel.getCourseBlocks(courseID: courseID)
Expand All @@ -95,6 +97,9 @@ public struct CourseContainerView: View {
.onChange(of: viewModel.selection, perform: didSelect)
.onChange(of: coordinate, perform: collapseHeader)
.background(Theme.Colors.background)
.sheet(isPresented: $viewModel.isDownloadsListPresented) {
DownloadsView(router: viewModel.router, courseHelper: viewModel.courseHelper)
}
}

@ViewBuilder
Expand All @@ -112,6 +117,8 @@ public struct CourseContainerView: View {
viewHeight: $viewHeight,
dateTabIndex: CourseTab.dates.rawValue
)
} else if viewModel.isActive == false {
noAccessContent
} else {
ZStack(alignment: .top) {
tabs
Expand Down Expand Up @@ -161,6 +168,22 @@ public struct CourseContainerView: View {
}
}

private var noAccessContent: some View {
VStack(spacing: 0) {
NavigationBar(
title: title,
leftButtonAction: { viewModel.router.back() }
)
FullScreenErrorView(
type: .noContent(
CourseLocalization.Error.courseAccessUnavailable,
image: CoreAssets.lockIcon.swiftUIImage
)
)
}
.background(Theme.Colors.background.ignoresSafeArea())
}

private func showDatesSuccessView(title: String, message: String) -> some View {
return DatesSuccessView(
title: title,
Expand Down
Loading
Loading