From 7baadccdb7cc6eab76ee3a838a18d3348b3b1f18 Mon Sep 17 00:00:00 2001 From: Russell Pecka Date: Mon, 8 Jul 2024 23:43:38 -0700 Subject: [PATCH] Made chunk reading explicit when using read or pread --- Sources/NIOFileSystem/FileChunks.swift | 19 +++++--------- Sources/NIOFileSystem/FileHandle.swift | 8 ++++++ .../NIOFileSystem/FileHandleProtocol.swift | 9 ++++++- .../Internal/SystemFileHandle.swift | 8 +++++- .../FileHandleTests.swift | 26 +++++++++++++++++++ 5 files changed, 55 insertions(+), 15 deletions(-) diff --git a/Sources/NIOFileSystem/FileChunks.swift b/Sources/NIOFileSystem/FileChunks.swift index 4b87221410..a20e97bd30 100644 --- a/Sources/NIOFileSystem/FileChunks.swift +++ b/Sources/NIOFileSystem/FileChunks.swift @@ -22,8 +22,8 @@ import NIOPosix @available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) public struct FileChunks: AsyncSequence { enum ChunkRange { - case entireFile - case partial(Range) + case filePointerToEnd + case range(Range) } public typealias Element = ByteBuffer @@ -39,22 +39,15 @@ public struct FileChunks: AsyncSequence { internal init( handle: SystemFileHandle, chunkLength: ByteCount, - range: Range + range: ChunkRange ) { - let chunkRange: ChunkRange - if range.lowerBound == 0, range.upperBound == .max { - chunkRange = .entireFile - } else { - chunkRange = .partial(range) - } - // TODO: choose reasonable watermarks; this should likely be at least somewhat dependent // on the chunk size. let stream = BufferedStream.makeFileChunksStream( of: ByteBuffer.self, handle: handle, chunkLength: chunkLength.bytes, - range: chunkRange, + range: range, lowWatermark: 4, highWatermark: 8 ) @@ -96,9 +89,9 @@ extension BufferedStream where Element == ByteBuffer { ) -> BufferedStream { let state: ProducerState switch range { - case .entireFile: + case .filePointerToEnd: state = ProducerState(handle: handle, range: nil) - case .partial(let partialRange): + case .range(let partialRange): state = ProducerState(handle: handle, range: partialRange) } let protectedState = NIOLockedValueBox(state) diff --git a/Sources/NIOFileSystem/FileHandle.swift b/Sources/NIOFileSystem/FileHandle.swift index 2f6f0ed47f..40a5014b99 100644 --- a/Sources/NIOFileSystem/FileHandle.swift +++ b/Sources/NIOFileSystem/FileHandle.swift @@ -194,6 +194,10 @@ public struct ReadFileHandle: ReadableFileHandleProtocol, _HasFileHandle { self.fileHandle.systemFileHandle.readChunks(in: range, chunkLength: chunkLength) } + public func readChunksFromFilePointer(chunkLength size: ByteCount) -> FileChunks { + self.fileHandle.systemFileHandle.readChunksFromFilePointer(chunkLength: size) + } + public func setTimes( lastAccess: FileInfo.Timespec?, lastDataModification: FileInfo.Timespec? @@ -269,6 +273,10 @@ public struct ReadWriteFileHandle: ReadableAndWritableFileHandleProtocol, _HasFi self.fileHandle.systemFileHandle.readChunks(in: offset, chunkLength: chunkLength) } + public func readChunksFromFilePointer(chunkLength size: ByteCount) -> FileChunks { + self.fileHandle.systemFileHandle.readChunksFromFilePointer(chunkLength: size) + } + @discardableResult public func write( contentsOf bytes: some (Sequence & Sendable), diff --git a/Sources/NIOFileSystem/FileHandleProtocol.swift b/Sources/NIOFileSystem/FileHandleProtocol.swift index e7607f7829..ae8c4789a9 100644 --- a/Sources/NIOFileSystem/FileHandleProtocol.swift +++ b/Sources/NIOFileSystem/FileHandleProtocol.swift @@ -202,6 +202,13 @@ public protocol ReadableFileHandleProtocol: FileHandleProtocol { /// - chunkLength: The maximum length of the chunk to read as a ``ByteCount``. /// - Returns: A sequence of chunks read from the file. func readChunks(in range: Range, chunkLength: ByteCount) -> FileChunks + + /// Returns an asynchronous sequence of chunks read from the file starting from the current file pointer. + /// + /// - Parameters: + /// - size: The maximum length of the chunk to read as a ``ByteCount``. + /// - Returns: A sequence of chunks read from the file. + func readChunksFromFilePointer(chunkLength size: ByteCount) -> FileChunks } // MARK: - Read chunks with default chunk length @@ -415,7 +422,7 @@ extension ReadableFileHandleProtocol { var accumulator = ByteBuffer() accumulator.reserveCapacity(readSize) - for try await chunk in self.readChunks(in: ..., chunkLength: .mebibytes(8)) { + for try await chunk in self.readChunksFromFilePointer(chunkLength: .mebibytes(8)) { accumulator.writeImmutableBuffer(chunk) if accumulator.readableBytes > maximumSizeAllowed.bytes { throw FileSystemError( diff --git a/Sources/NIOFileSystem/Internal/SystemFileHandle.swift b/Sources/NIOFileSystem/Internal/SystemFileHandle.swift index 34c8f62353..f11dfb4f81 100644 --- a/Sources/NIOFileSystem/Internal/SystemFileHandle.swift +++ b/Sources/NIOFileSystem/Internal/SystemFileHandle.swift @@ -1072,7 +1072,13 @@ extension SystemFileHandle: ReadableFileHandleProtocol { in range: Range, chunkLength size: ByteCount ) -> FileChunks { - return FileChunks(handle: self, chunkLength: size, range: range) + return FileChunks(handle: self, chunkLength: size, range: .range(range)) + } + + public func readChunksFromFilePointer( + chunkLength size: ByteCount + ) -> FileChunks { + return FileChunks(handle: self, chunkLength: size, range: .filePointerToEnd) } } diff --git a/Tests/NIOFileSystemIntegrationTests/FileHandleTests.swift b/Tests/NIOFileSystemIntegrationTests/FileHandleTests.swift index cbfd56887a..31da5d0e2d 100644 --- a/Tests/NIOFileSystemIntegrationTests/FileHandleTests.swift +++ b/Tests/NIOFileSystemIntegrationTests/FileHandleTests.swift @@ -610,6 +610,32 @@ final class FileHandleTests: XCTestCase { } } + func testUnboundedRangeAfterRead() async throws { + // Reading chunks from an UnboundedRange after the file position has been moved to non-zero. + try await self.withHandle(forFileAtPath: Self.thisFile) { handle in + // trigger an initial read of the entire file to attempt to move the file pointer + var firstRead = ByteBuffer() + for try await chunk in handle.readChunks(in: ..., chunkLength: .bytes(128)) { + XCTAssertLessThanOrEqual(chunk.readableBytes, 128) + firstRead.writeImmutableBuffer(chunk) + } + var secondRead = ByteBuffer() + for try await chunk in handle.readChunks(in: ..., chunkLength: .bytes(128)) { + XCTAssertLessThanOrEqual(chunk.readableBytes, 128) + secondRead.writeImmutableBuffer(chunk) + } + // We should read bytes until EOF. + XCTAssertEqual( + secondRead.readableBytes, + firstRead.readableBytes, + """ + Read \(secondRead.readableBytes) which were different to the \(firstRead.readableBytes) \ + expected bytes. + """ + ) + } + } + func testReadPartialRange() async throws { // Reading chunks of bytes from a PartialRangeThrough with the upper bound inside the file. try await self.withHandle(forFileAtPath: Self.thisFile) { handle in