From 001ea22917f3890183daa2b6c9b9e09197c20cce Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?kleines=20Filmr=C3=B6llchen?= <filmroellchen@serenityos.org>
Date: Tue, 4 Jul 2023 17:47:23 +0200
Subject: [PATCH] AK+LibCore: Make output buffered stream seekable

Just like with input buffered streams, we don't currently have a use
case for output buffered streams which aren't seekable, since the main
application are files.
---
 AK/BufferedStream.h                 | 36 +++++++++++++++++++++--------
 Tests/LibCore/TestLibCoreStream.cpp | 18 +++++++++++++++
 Userland/Libraries/LibCore/File.h   |  2 +-
 3 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/AK/BufferedStream.h b/AK/BufferedStream.h
index 4d8accc240a..69e23347264 100644
--- a/AK/BufferedStream.h
+++ b/AK/BufferedStream.h
@@ -310,10 +310,10 @@ private:
     BufferedHelper<T> m_helper;
 };
 
-template<StreamLike T>
-class OutputBufferedStream final : public Stream {
+template<SeekableStreamLike T>
+class OutputBufferedSeekable : public SeekableStream {
 public:
-    static ErrorOr<NonnullOwnPtr<OutputBufferedStream<T>>> create(NonnullOwnPtr<T> stream, size_t buffer_size = 16 * KiB)
+    static ErrorOr<NonnullOwnPtr<OutputBufferedSeekable<T>>> create(NonnullOwnPtr<T> stream, size_t buffer_size = 16 * KiB)
     {
         if (buffer_size == 0)
             return Error::from_errno(EINVAL);
@@ -322,11 +322,11 @@ public:
 
         auto buffer = TRY(CircularBuffer::create_empty(buffer_size));
 
-        return adopt_nonnull_own_or_enomem(new OutputBufferedStream<T>(move(stream), move(buffer)));
+        return adopt_nonnull_own_or_enomem(new OutputBufferedSeekable<T>(move(stream), move(buffer)));
     }
 
-    OutputBufferedStream(OutputBufferedStream&& other) = default;
-    OutputBufferedStream& operator=(OutputBufferedStream&& other) = default;
+    OutputBufferedSeekable(OutputBufferedSeekable&& other) = default;
+    OutputBufferedSeekable& operator=(OutputBufferedSeekable&& other) = default;
 
     virtual ErrorOr<Bytes> read_some(Bytes buffer) override
     {
@@ -368,13 +368,31 @@ public:
         return {};
     }
 
-    virtual ~OutputBufferedStream() override
+    // Since tell() doesn't involve moving the write offset, we can skip flushing the buffer here.
+    virtual ErrorOr<size_t> tell() const override
+    {
+        return TRY(m_stream->tell()) + m_buffer.used_space();
+    }
+
+    virtual ErrorOr<size_t> seek(i64 offset, SeekMode mode) override
+    {
+        TRY(flush_buffer());
+        return m_stream->seek(offset, mode);
+    }
+
+    virtual ErrorOr<void> truncate(size_t length) override
+    {
+        TRY(flush_buffer());
+        return m_stream->truncate(length);
+    }
+
+    virtual ~OutputBufferedSeekable() override
     {
         MUST(flush_buffer());
     }
 
 private:
-    OutputBufferedStream(NonnullOwnPtr<T> stream, CircularBuffer buffer)
+    OutputBufferedSeekable(NonnullOwnPtr<T> stream, CircularBuffer buffer)
         : m_stream(move(stream))
         , m_buffer(move(buffer))
     {
@@ -389,5 +407,5 @@ private:
 #if USING_AK_GLOBALLY
 using AK::BufferedHelper;
 using AK::InputBufferedSeekable;
-using AK::OutputBufferedStream;
+using AK::OutputBufferedSeekable;
 #endif
diff --git a/Tests/LibCore/TestLibCoreStream.cpp b/Tests/LibCore/TestLibCoreStream.cpp
index daff4901191..2468301a7d9 100644
--- a/Tests/LibCore/TestLibCoreStream.cpp
+++ b/Tests/LibCore/TestLibCoreStream.cpp
@@ -111,6 +111,24 @@ BENCHMARK_CASE(file_tell)
     }
 }
 
+TEST_CASE(file_buffered_write_and_seek)
+{
+    auto file = TRY_OR_FAIL(Core::OutputBufferedFile::create(TRY_OR_FAIL(Core::File::open("/tmp/file-buffered-write-test.txt"sv, Core::File::OpenMode::Truncate | Core::File::OpenMode::ReadWrite))));
+
+    TRY_OR_FAIL(file->write_some("0123456789"sv.bytes()));
+    EXPECT_EQ(file->tell().release_value(), 10ul);
+
+    // Reads don't go through the buffer, so after we seek, the data must be available from the underlying file.
+    TRY_OR_FAIL(file->seek(0, AK::SeekMode::SetPosition));
+    auto first_byte = TRY_OR_FAIL(file->read_value<u8>());
+    EXPECT_EQ(first_byte, static_cast<u8>('0'));
+
+    TRY_OR_FAIL(file->seek(9, AK::SeekMode::SetPosition));
+    auto last_byte = TRY_OR_FAIL(file->read_value<u8>());
+    EXPECT_EQ(last_byte, static_cast<u8>('9'));
+    EXPECT_EQ(file->tell().release_value(), 10ul);
+}
+
 TEST_CASE(file_adopt_fd)
 {
     int rc = ::open("/usr/Tests/LibCore/long_lines.txt", O_RDONLY);
diff --git a/Userland/Libraries/LibCore/File.h b/Userland/Libraries/LibCore/File.h
index dc5e91fe305..23fc7626cb2 100644
--- a/Userland/Libraries/LibCore/File.h
+++ b/Userland/Libraries/LibCore/File.h
@@ -117,6 +117,6 @@ private:
 AK_ENUM_BITWISE_OPERATORS(File::OpenMode)
 
 using InputBufferedFile = InputBufferedSeekable<File>;
-using OutputBufferedFile = OutputBufferedStream<File>;
+using OutputBufferedFile = OutputBufferedSeekable<File>;
 
 }