Skip to content

Commit df1d57a

Browse files
committed
Remove stored file size property from ChunkedFile
Store fileURL inside ChunkedFile when an active handle is available Simplify state checking inside ChunkedFile Prevent potential crash due to overflow Remove SIZE_UNKNOWN Keep a reference to FileManager for eventual dependency injection when initializing ChunkedFile # Conflicts: # Sources/MuxUploadSDK/InternalUtilities/ChunkedFile.swift
1 parent d287d3d commit df1d57a

File tree

3 files changed

+80
-47
lines changed

3 files changed

+80
-47
lines changed

Sources/MuxUploadSDK/InternalUtilities/ChunkedFile.swift

Lines changed: 43 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,28 @@ import Foundation
1313
/// Buffers are allocated for each new chunk so they can safely escape to other threads
1414
/// Call ``close`` when you're done with this object
1515
class ChunkedFile {
16-
17-
static let SIZE_UNKNOWN: UInt64 = 0
18-
/// The size of the file. Call ``open`` to populate this with a real value, otherwise it will be ``SIZE_UNKNOWN``
19-
var fileSize: UInt64 {
20-
return _fileSize
16+
17+
private struct State {
18+
var fileHandle: FileHandle
19+
var fileURL: URL
20+
var filePosition: UInt64 = 0
2121
}
22-
22+
2323
private let chunkSize: Int
24-
25-
private var fileHandle: FileHandle?
26-
private var filePos: UInt64 = 0
27-
private var _fileSize: UInt64 = SIZE_UNKNOWN
24+
25+
var fileManager = FileManager.default
26+
27+
private var state: State?
28+
29+
private var fileHandle: FileHandle? {
30+
state?.fileHandle
31+
}
32+
private var fileURL: URL? {
33+
state?.fileURL
34+
}
35+
private var filePos: UInt64 {
36+
state?.filePosition ?? 0
37+
}
2838

2939
/// Reads the next chunk from the file, advancing the file for the next read
3040
/// This method does synchronous I/O, so call it in the background
@@ -39,20 +49,21 @@ class ChunkedFile {
3949
return Result.failure(ChunkedFileError.fileHandle(error))
4050
}
4151
}
42-
52+
4353
/// Opens the internal file ahead of time. Calling this is optional, but it's available
4454
/// Calling this multiple times (on the same thread) will have no effect unless you also ``close`` it
4555
/// Throws if the file couldn't be opened
46-
public func openFile(fileURL: URL) throws {
47-
if fileHandle == nil {
56+
func openFile(fileURL: URL) throws {
57+
if state == nil {
4858
do {
49-
guard let fileSize = try FileManager.default.attributesOfItem(atPath: fileURL.path)[FileAttributeKey.size] as? UInt64 else {
59+
guard let fileSize = try fileManager.attributesOfItem(atPath: fileURL.path)[FileAttributeKey.size] as? UInt64 else {
5060
throw ChunkedFileError.invalidState("Cannot retrieve file size")
5161
}
52-
self._fileSize = fileSize
53-
54-
let handle = try FileHandle(forReadingFrom: fileURL)
55-
fileHandle = handle
62+
let fileHandle = try FileHandle(forReadingFrom: fileURL)
63+
state = State(
64+
fileHandle: fileHandle,
65+
fileURL: fileURL
66+
)
5667
MuxUploadSDK.logger?.info("Opened file with len \(String(describing: fileSize)) at path \(fileURL.path)")
5768
} catch {
5869
throw ChunkedFileError.fileHandle(error)
@@ -68,23 +79,31 @@ class ChunkedFile {
6879
} catch {
6980
MuxUploadSDK.logger?.warning("Swallowed error closing file: \(error.localizedDescription)")
7081
}
71-
fileHandle = nil
72-
filePos = 0
73-
_fileSize = ChunkedFile.SIZE_UNKNOWN
82+
state = nil
7483
}
7584

7685
public func seekTo(byte: UInt64) throws {
7786
// Worst case: we start from the begining and there's a few very quick chunk successes
7887
try fileHandle?.seek(toOffset: byte)
79-
filePos = byte
88+
state?.filePosition = byte
8089
}
8190

8291
private func doReadNextChunk() throws -> FileChunk {
8392
MuxUploadSDK.logger?.info("--doReadNextChunk")
84-
guard let fileHandle = fileHandle else {
93+
guard let fileHandle = fileHandle, let fileURL = fileURL else {
8594
throw ChunkedFileError.invalidState("doReadNextChunk called without file handle. Did you call open()?")
8695
}
8796
let data = try fileHandle.read(upToCount: chunkSize)
97+
98+
let fileAttributes = try fileManager.attributesOfItem(
99+
atPath: fileURL.path
100+
)
101+
guard let fileSize = fileAttributes[
102+
FileAttributeKey.size
103+
] as? UInt64 else {
104+
throw ChunkedFileError.invalidState("Cannot retrieve file size")
105+
}
106+
88107
guard let data = data else {
89108
// Called while already at the end of the file. We read zero bytes, "ending" at the end of the file
90109
return FileChunk(startByte: fileSize, endByte: fileSize, totalFileSize: fileSize, chunkData: Data(capacity: 0))
@@ -100,7 +119,7 @@ class ChunkedFile {
100119
chunkData: data
101120
)
102121

103-
self.filePos = newFilePos
122+
state?.filePosition = newFilePos
104123

105124
return chunk
106125
}

Sources/MuxUploadSDK/Upload/ChunkedFileUploader.swift

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ class ChunkedFileUploader {
2424
private var delegates: [String : ChunkedFileUploaderDelegate] = [:]
2525

2626
private let file: ChunkedFile
27-
private var currentWorkTask: Task<(), Never>? = nil
27+
private var currentWorkTask: Task<(), Error>? = nil
2828
private var _currentState: InternalUploadState = .ready
2929
private var overallProgress: Progress = Progress()
3030
private var lastReadCount: UInt64 = 0
@@ -93,7 +93,10 @@ class ChunkedFileUploader {
9393
let task = Task.detached { [self] in
9494
do {
9595
// It's fine if it's already open, that's handled by ignoring the call
96-
let fileSize = file.fileSize
96+
let fileSize = (try? FileManager.default.attributesOfItem(
97+
atPath: inputFileURL.path
98+
)[FileAttributeKey.size] as? UInt64) ?? 0
99+
97100
let result = try await makeWorker().performUpload()
98101
file.close()
99102

@@ -135,23 +138,31 @@ class ChunkedFileUploader {
135138
} else {
136139
MuxUploadSDK.logger?.debug("Task finished due to error in state \(String(describing: self.currentState))")
137140
let uploadError = InternalUploaderError(reason: error, lastByte: lastReadCount)
141+
notifyStateFromWorker(.failure(uploadError))
138142

139143
if shouldReport {
140-
let fileSize = try file.fetchFileSize(fileURL: inputFileURL)
144+
let fileAttributes = try FileManager.default.attributesOfItem(
145+
atPath: inputFileURL.path
146+
)
147+
guard let fileSize = fileAttributes[
148+
FileAttributeKey.size
149+
] as? UInt64 else {
150+
return
151+
}
141152

142153
#warning("Start and end time need to be fixed")
143154
reporter.reportUploadFailure(
144155
transportStartTime: 0,
145156
transportEndTime: 0,
146157
errorDescription: uploadError.localizedDescription,
147-
inputSize: file.fileSize,
158+
inputSize: fileSize,
148159
inputStandardizationEnabled: uploadInfo.options.inputStandardization.isEnabled,
149160
inputDuration: 0.0,
150161
uploadURL: uploadInfo.uploadURL
151162
)
152163
}
153164

154-
notifyStateFromWorker(.failure(uploadError))
165+
155166
}
156167
}
157168
}
@@ -288,9 +299,26 @@ fileprivate actor Worker {
288299
try chunkedFile.seekTo(byte: startingReadCount)
289300

290301
let startTime = Date().timeIntervalSince1970
291-
292-
let fileSize = chunkedFile.fileSize
293-
let wideFileSize = Int64(fileSize)
302+
303+
let fileAttributes = try FileManager.default.attributesOfItem(
304+
atPath: inputFileURL.path
305+
)
306+
guard let fileSize = fileAttributes[
307+
FileAttributeKey.size
308+
] as? UInt64 else {
309+
// TODO: Does the worker need its own error defintion?
310+
throw ChunkedFileError.invalidState("Cannot retrieve file size")
311+
}
312+
313+
let wideFileSize: Int64
314+
315+
// Prevent overflow if UInt64 exceeds Int64.max
316+
if fileSize >= Int64.max {
317+
wideFileSize = Int64.max
318+
} else {
319+
wideFileSize = Int64(fileSize)
320+
}
321+
294322
overallProgress.totalUnitCount = wideFileSize
295323
overallProgress.isCancellable = false
296324
overallProgress.completedUnitCount = Int64(startingReadCount)

Tests/MuxUploadSDKTests/Internal Utilities Tests/ChunkedFileTests.swift

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -77,14 +77,7 @@ final class ChunkedFileTests: XCTestCase {
7777

7878
func testMisuseBeforeOpen() throws {
7979
let file = ChunkedFile(chunkSize: 10 * 1000 * 1000)
80-
81-
let fileSizeBeforeOpen = file.fileSize
82-
XCTAssertEqual(
83-
fileSizeBeforeOpen,
84-
ChunkedFile.SIZE_UNKNOWN,
85-
"File size should not be known before open"
86-
)
87-
80+
8881
let readResult = file.readNextChunk()
8982
switch readResult {
9083
case .success(_): return XCTFail("readNextChunk should fail before open")
@@ -104,13 +97,6 @@ final class ChunkedFileTests: XCTestCase {
10497
try file.openFile(fileURL: testFileURL)
10598
file.close()
10699

107-
let fileSizeAfterClose = file.fileSize
108-
XCTAssertEqual(
109-
fileSizeAfterClose,
110-
ChunkedFile.SIZE_UNKNOWN,
111-
"File size should not be known after close"
112-
)
113-
114100
let chunkResult = file.readNextChunk()
115101
switch chunkResult {
116102
case .success(_): return XCTFail("read after close should not succeed")

0 commit comments

Comments
 (0)