Skip to content
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

Make mark consistent with skip and IOBuffer #201

Merged
merged 5 commits into from
Mar 31, 2024
Merged
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
1 change: 1 addition & 0 deletions docs/src/reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ TranscodingStreams.unsafe_read
TranscodingStreams.unread
TranscodingStreams.unsafe_unread
Base.position(stream::TranscodingStream)
Base.skip
```

Statistics
Expand Down
6 changes: 5 additions & 1 deletion ext/TestExt.jl
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,15 @@ function TranscodingStreams.test_chunked_read(Encoder, Decoder)
ok = true
for chunk in chunks
stream = TranscodingStream(Decoder(), buffer, stop_on_end=true)
ok &= hash(read(stream)) == hash(chunk)
ok &= read(stream) == chunk
ok &= eof(stream)
ok &= isreadable(stream)
close(stream)
end
# read without stop_on_end should read the full data.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test isn't related to this PR but was something I noticed wasn't covered by current tests.

stream = TranscodingStream(Decoder(), IOBuffer(data))
ok &= read(stream) == reduce(vcat, chunks)
close(stream)
Test.@test ok
end
finalize(encoder)
Expand Down
74 changes: 38 additions & 36 deletions src/stream.jl
Original file line number Diff line number Diff line change
Expand Up @@ -229,50 +229,26 @@ end
end
end

function Base.ismarked(stream::TranscodingStream)
function Base.ismarked(stream::TranscodingStream)::Bool
checkmode(stream)
return ismarked(stream.buffer1)
isopen(stream) && ismarked(stream.buffer1)
end

function Base.mark(stream::TranscodingStream)
checkmode(stream)
return mark!(stream.buffer1)
function Base.mark(stream::TranscodingStream)::Int64
ready_to_read!(stream)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This throws an error if mark is used on a stream in write, close, or panic modes.

mark!(stream.buffer1)
position(stream)
end

function Base.unmark(stream::TranscodingStream)
function Base.unmark(stream::TranscodingStream)::Bool
checkmode(stream)
return unmark!(stream.buffer1)
isopen(stream) && unmark!(stream.buffer1)
end

function Base.reset(stream::TranscodingStream)
checkmode(stream)
return reset!(stream.buffer1)
end

function Base.skip(stream::TranscodingStream, offset::Integer)
checkmode(stream)
if offset < 0
throw(ArgumentError("negative offset"))
end
mode = stream.state.mode
buffer1 = stream.buffer1
skipped = 0
if mode === :read || mode === :stop
while !eof(stream) && buffersize(buffer1) < offset - skipped
n = buffersize(buffer1)
emptybuffer!(buffer1)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of emptybuffer! here was discarding potentially marked data.

skipped += n
end
if eof(stream)
emptybuffer!(buffer1)
else
skipbuffer!(buffer1, offset - skipped)
end
else
# TODO: support skip in write mode
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what this TODO means, but I add a docstring to clarify that skip behaves like a read that ignores eof and discards the read bytes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also moved the skip to be near the other read functions

throw(ArgumentError("not in read mode"))
end
return stream
function Base.reset(stream::T) where T<:TranscodingStream
Base.ismarked(stream) || throw(ArgumentError("$T not marked"))
reset!(stream.buffer1)
position(stream)
end

"""
Expand Down Expand Up @@ -389,6 +365,32 @@ function Base.readuntil(stream::TranscodingStream, delim::UInt8; keep::Bool=fals
return ret
end

"""
skip(stream::TranscodingStream, offset)

Read bytes from `stream` until `offset` bytes have been read or `eof(stream)` is reached.

Return `stream`, discarding read bytes.

This function will not throw an `EOFError` if `eof(stream)` is reached before
`offset` bytes can be read.
"""
function Base.skip(stream::TranscodingStream, offset::Integer)
if offset < 0
# TODO support negative offset if stream is marked
throw(ArgumentError("negative offset"))
end
ready_to_read!(stream)
buffer1 = stream.buffer1
skipped = 0
while skipped < offset && !eof(stream)
n = min(buffersize(buffer1), offset - skipped)
skipbuffer!(buffer1, n)
skipped += n
end
return stream
end

function Base.unsafe_read(stream::TranscodingStream, output::Ptr{UInt8}, nbytes::UInt)
ready_to_read!(stream)
buffer = stream.buffer1
Expand Down
2 changes: 1 addition & 1 deletion test/codecdoubleframe.jl
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ DoubleFrameDecoderStream(stream::IO; kwargs...) = TranscodingStream(DoubleFrameD
@testset "reading zero bytes from invalid stream" begin
# This behavior is required to avoid breaking JLD2.jl
# `s` must go into read mode, but not actually call `eof`
for readnone in (io -> read!(io, UInt8[]), io -> read(io, 0))
for readnone in (io -> read!(io, UInt8[]), io -> read(io, 0), io -> skip(io, 0))
for invalid_data in (b"", b"asdf")
s = DoubleFrameDecoderStream(IOBuffer(invalid_data;read=true,write=true))
@test iswritable(s)
Expand Down
12 changes: 8 additions & 4 deletions test/codecnoop.jl
Original file line number Diff line number Diff line change
Expand Up @@ -59,17 +59,21 @@
data = collect(0x00:0x0f)
stream = TranscodingStream(Noop(), IOBuffer(data))
@test !ismarked(stream)
mark(stream)
@test mark(stream) == 0
@test ismarked(stream)
@test [read(stream, UInt8) for _ in 1:3] == data[1:3]
reset(stream)
@test reset(stream) == 0
@test_throws ArgumentError reset(stream)
@test !ismarked(stream)
@test [read(stream, UInt8) for _ in 1:3] == data[1:3]
mark(stream)
@test mark(stream) == 3
@test ismarked(stream)
unmark(stream)
@test unmark(stream)
@test !ismarked(stream)
@test !unmark(stream)
@test mark(stream) == 3
close(stream)
@test !ismarked(stream)

stream = TranscodingStream(Noop(), IOBuffer(b"foobarbaz"))
@test stream == seek(stream, 2)
Expand Down
41 changes: 41 additions & 0 deletions test/codecquadruple.jl
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,47 @@ end
end
end

@testset "skip" begin
@testset "position" begin
# make sure position is updated correctly by skip
iob = IOBuffer(repeat("x", 400))
source = IOBuffer(repeat("x", 100))
stream = TranscodingStream(QuadrupleCodec(), source, bufsize=16)
@test position(stream) == position(iob)
for len in 0:10:100
skip(stream, len)
skip(iob, len)
@test position(stream) == position(iob)
end
close(stream)
end
@testset "mark" begin
iob = IOBuffer(repeat("x", 400))
source = IOBuffer(repeat("x", 100))
stream = TranscodingStream(QuadrupleCodec(), source, bufsize=16)
@test position(stream) == position(iob)
@test mark(stream) == mark(iob)
for len in 0:10:100
skip(stream, len)
skip(iob, len)
@test position(stream) == position(iob)
end
@test Base.reset(stream) == Base.reset(iob)
for len in 0:10:100
skip(stream, len)
skip(iob, len)
@test position(stream) == position(iob)
end
@test mark(stream) == mark(iob)
@test Base.reset(stream) == Base.reset(iob)
@test mark(stream) == mark(iob)
close(stream)
close(iob)
@test !ismarked(stream)
@test !ismarked(iob)
end
end

@testset "seekstart" begin
data = Vector(b"abracadabra")
source = IOBuffer(data)
Expand Down
Loading