Spreadsheet: Port XSV writer to Core::Stream

Author: https://github.com/krkk
Commit: https://github.com/SerenityOS/serenity/commit/3a8450ae11
Pull-request: https://github.com/SerenityOS/serenity/pull/16765
Reviewed-by: https://github.com/kleinesfilmroellchen ✅

Userland/Applications/Spreadsheet/ExportDialog.cpp

@@ -10,10 +10,9 @@
 #include <AK/DeprecatedString.h>
 #include <AK/JsonArray.h>
 #include <AK/LexicalPath.h>
-#include <AK/MemoryStream.h>
 #include <Applications/Spreadsheet/CSVExportGML.h>
-#include <LibCore/File.h>
 #include <LibCore/FileStream.h>
+#include <LibCore/MemoryStream.h>
 #include <LibCore/StandardPaths.h>
 #include <LibGUI/Application.h>
 #include <LibGUI/CheckBox.h>
@@ -90,7 +89,7 @@ CSVExportDialogPage::CSVExportDialogPage(Sheet const& sheet)
     update_preview();
 }
 
-auto CSVExportDialogPage::make_writer(OutputStream& stream) -> ErrorOr<XSV>
+auto CSVExportDialogPage::make_writer(Core::Stream::Handle<Core::Stream::Stream> stream) -> ErrorOr<NonnullOwnPtr<XSV>>
 {
     auto delimiter = TRY([this]() -> ErrorOr<DeprecatedString> {
         if (m_delimiter_other_radio->is_checked()) {
@@ -152,23 +151,22 @@ auto CSVExportDialogPage::make_writer(OutputStream& stream) -> ErrorOr<XSV>
     if (should_quote_all_fields)
         behaviors = behaviors | Writer::WriterBehavior::QuoteAll;
 
-    return XSV(stream, m_data, move(traits), *headers, behaviors);
+    return try_make<XSV>(move(stream), m_data, move(traits), *headers, behaviors);
 }
 
 void CSVExportDialogPage::update_preview()
 {
-    DuplexMemoryStream memory_stream;
-    auto writer_or_error = make_writer(memory_stream);
-    if (!writer_or_error.is_error()) {
-        m_data_preview_text_editor->set_text(DeprecatedString::formatted("Cannot update preview: {}", writer_or_error.error()));
-        return;
-    }
-    auto writer = writer_or_error.release_value();
-
-    writer.generate_preview();
-    auto buffer = memory_stream.copy_into_contiguous_buffer();
-    m_data_preview_text_editor->set_text(StringView(buffer));
-    m_data_preview_text_editor->update();
+    auto maybe_error = [this]() -> ErrorOr<void> {
+        Core::Stream::AllocatingMemoryStream memory_stream;
+        auto writer = TRY(make_writer(Core::Stream::Handle<Core::Stream::Stream>(memory_stream)));
+        TRY(writer->generate_preview());
+        auto buffer = TRY(memory_stream.read_until_eof());
+        m_data_preview_text_editor->set_text(StringView(buffer));
+        m_data_preview_text_editor->update();
+        return {};
+    }();
+    if (maybe_error.is_error())
+        m_data_preview_text_editor->set_text(DeprecatedString::formatted("Cannot update preview: {}", maybe_error.error()));
 }
 
 ErrorOr<void> ExportDialog::make_and_run_for(StringView mime, Core::File& file, Workbook& workbook)
@@ -188,12 +186,9 @@ ErrorOr<void> ExportDialog::make_and_run_for(StringView mime, Core::File& file,
         if (wizard->exec() != GUI::Dialog::ExecResult::OK)
             return Error::from_string_literal("CSV Export was cancelled");
 
-        auto file_stream = Core::OutputFileStream(file);
-        auto writer = TRY(page.make_writer(file_stream));
-        writer.generate();
-        if (writer.has_error())
-            return Error::from_string_literal("CSV Export failed");
-        return {};
+        auto file_stream = TRY(try_make<Core::Stream::WrappedAKOutputStream>(TRY(try_make<Core::OutputFileStream>(file))));
+        auto writer = TRY(page.make_writer(move(file_stream)));
+        return writer->generate();
     };
 
     auto export_worksheet = [&]() -> ErrorOr<void> {

Userland/Applications/Spreadsheet/ExportDialog.h

@@ -23,7 +23,7 @@ struct CSVExportDialogPage {
     explicit CSVExportDialogPage(Sheet const&);
 
     NonnullRefPtr<GUI::WizardPage> page() { return *m_page; }
-    ErrorOr<XSV> make_writer(OutputStream&);
+    ErrorOr<NonnullOwnPtr<XSV>> make_writer(Core::Stream::Handle<Core::Stream::Stream>);
 
 protected:
     void update_preview();

Userland/Applications/Spreadsheet/Writers/CSV.h

@@ -15,8 +15,8 @@ namespace Writer {
 template<typename ContainerType>
 class CSV : public XSV<ContainerType> {
 public:
-    CSV(OutputStream& output, ContainerType const& data, Vector<StringView> const& headers = {}, WriterBehavior behaviors = default_behaviors())
-        : XSV<ContainerType>(output, data, { ",", "\"", WriterTraits::Repeat }, headers, behaviors)
+    CSV(Core::Stream::Handle<Core::Stream::Stream> output, ContainerType const& data, Vector<StringView> headers = {}, WriterBehavior behaviors = default_behaviors())
+        : XSV<ContainerType>(move(output), data, { ",", "\"", WriterTraits::Repeat }, move(headers), behaviors)
     {
     }
 };

Userland/Applications/Spreadsheet/Writers/Test/TestXSVWriter.cpp

@@ -8,7 +8,7 @@
 
 #include "../CSV.h"
 #include "../XSV.h"
-#include <AK/MemoryStream.h>
+#include <LibCore/MemoryStream.h>
 
 TEST_CASE(can_write)
 {
@@ -18,17 +18,17 @@ TEST_CASE(can_write)
         { 7, 8, 9 },
     };
 
-    auto buffer = ByteBuffer::create_uninitialized(1024).release_value();
-    OutputMemoryStream stream { buffer };
-
-    Writer::CSV csv(stream, data);
+    Core::Stream::AllocatingMemoryStream stream;
+    auto csv = Writer::CSV(Core::Stream::Handle<Core::Stream::Stream>(stream), data);
+    MUST(csv.generate());
 
     auto expected_output = R"~(1,2,3
 4,5,6
 7,8,9
-)~";
+)~"sv;
 
-    EXPECT_EQ(StringView { stream.bytes() }, expected_output);
+    auto buffer = MUST(stream.read_until_eof());
+    EXPECT_EQ(StringView { buffer.bytes() }, expected_output);
 }
 
 TEST_CASE(can_write_with_header)
@@ -39,18 +39,18 @@ TEST_CASE(can_write_with_header)
         { 7, 8, 9 },
     };
 
-    auto buffer = ByteBuffer::create_uninitialized(1024).release_value();
-    OutputMemoryStream stream { buffer };
-
-    Writer::CSV csv(stream, data, { "A"sv, "B\""sv, "C"sv });
+    Core::Stream::AllocatingMemoryStream stream;
+    auto csv = Writer::CSV(Core::Stream::Handle<Core::Stream::Stream>(stream), data, { "A"sv, "B\""sv, "C"sv });
+    MUST(csv.generate());
 
     auto expected_output = R"~(A,"B""",C
 1,2,3
 4,5,6
 7,8,9
-)~";
+)~"sv;
 
-    EXPECT_EQ(StringView { stream.bytes() }, expected_output);
+    auto buffer = MUST(stream.read_until_eof());
+    EXPECT_EQ(StringView { buffer.bytes() }, expected_output);
 }
 
 TEST_CASE(can_write_with_different_behaviors)
@@ -60,15 +60,15 @@ TEST_CASE(can_write_with_different_behaviors)
         { "We\"ll", "Hello,", "   Friends" },
     };
 
-    auto buffer = ByteBuffer::create_uninitialized(1024).release_value();
-    OutputMemoryStream stream { buffer };
-
-    Writer::CSV csv(stream, data, { "A"sv, "B\""sv, "C"sv }, Writer::WriterBehavior::QuoteOnlyInFieldStart | Writer::WriterBehavior::WriteHeaders);
+    Core::Stream::AllocatingMemoryStream stream;
+    auto csv = Writer::CSV(Core::Stream::Handle<Core::Stream::Stream>(stream), data, { "A"sv, "B\""sv, "C"sv }, Writer::WriterBehavior::QuoteOnlyInFieldStart | Writer::WriterBehavior::WriteHeaders);
+    MUST(csv.generate());
 
     auto expected_output = R"~(A,B",C
 Well,Hello",Friends
 We"ll,"Hello,",   Friends
-)~";
+)~"sv;
 
-    EXPECT_EQ(StringView { stream.bytes() }, expected_output);
+    auto buffer = MUST(stream.read_until_eof());
+    EXPECT_EQ(StringView { buffer.bytes() }, expected_output);
 }

Userland/Applications/Spreadsheet/Writers/XSV.h

@@ -9,10 +9,10 @@
 #include <AK/DeprecatedString.h>
 #include <AK/GenericLexer.h>
 #include <AK/OwnPtr.h>
-#include <AK/Stream.h>
 #include <AK/StringView.h>
 #include <AK/Types.h>
 #include <AK/Vector.h>
+#include <LibCore/Stream.h>
 
 namespace Writer {
 
@@ -34,17 +34,6 @@ struct WriterTraits {
     } quote_escape { Repeat };
 };
 
-#define ENUMERATE_WRITE_ERRORS()                                                  \
-    E(None, "No errors")                                                          \
-    E(NonConformingColumnCount, "Header count does not match given column count") \
-    E(InternalError, "Internal error")
-
-enum class WriteError {
-#define E(name, _) name,
-    ENUMERATE_WRITE_ERRORS()
-#undef E
-};
-
 constexpr WriterBehavior default_behaviors()
 {
     return WriterBehavior::None;
@@ -53,12 +42,12 @@ constexpr WriterBehavior default_behaviors()
 template<typename ContainerType, typename HeaderType = Vector<StringView>>
 class XSV {
 public:
-    XSV(OutputStream& output, ContainerType const& data, WriterTraits traits, HeaderType const& headers = {}, WriterBehavior behaviors = default_behaviors())
+    XSV(Core::Stream::Handle<Core::Stream::Stream> output, ContainerType const& data, WriterTraits traits, HeaderType headers = {}, WriterBehavior behaviors = default_behaviors())
         : m_data(data)
         , m_traits(move(traits))
         , m_behaviors(behaviors)
         , m_names(headers)
-        , m_output(output)
+        , m_output(move(output))
     {
         if (!headers.is_empty())
             m_behaviors = m_behaviors | WriterBehavior::WriteHeaders;
@@ -66,94 +55,71 @@ public:
 
     virtual ~XSV() = default;
 
-    bool has_error() const { return m_error != WriteError::None; }
-    WriteError error() const { return m_error; }
-    DeprecatedString error_string() const
-    {
-        switch (m_error) {
-#define E(x, y)         \
-    case WriteError::x: \
-        return y;
-
-            ENUMERATE_WRITE_ERRORS();
-#undef E
-        }
-        VERIFY_NOT_REACHED();
-    }
-
-    void generate()
+    ErrorOr<void> generate()
     {
         auto with_headers = has_flag(m_behaviors, WriterBehavior::WriteHeaders);
         if (with_headers) {
-            write_row(m_names);
-            if (m_output.write({ "\n", 1 }) != 1)
-                set_error(WriteError::InternalError);
+            TRY(write_row(m_names));
+            TRY(m_output->write_entire_buffer({ "\n", 1 }));
         }
 
         for (auto&& row : m_data) {
             if (with_headers) {
                 if (row.size() != m_names.size())
-                    set_error(WriteError::NonConformingColumnCount);
+                    return Error::from_string_literal("Header count does not match given column count");
             }
 
-            write_row(row);
-            if (m_output.write({ "\n", 1 }) != 1)
-                set_error(WriteError::InternalError);
+            TRY(write_row(row));
+            TRY(m_output->write_entire_buffer({ "\n", 1 }));
         }
+        return {};
     }
 
-    void generate_preview()
+    ErrorOr<void> generate_preview()
     {
         auto lines_written = 0;
         constexpr auto max_preview_lines = 8;
 
         auto with_headers = has_flag(m_behaviors, WriterBehavior::WriteHeaders);
         if (with_headers) {
-            write_row(m_names);
-            if (m_output.write({ "\n", 1 }) != 1)
-                set_error(WriteError::InternalError);
+            TRY(write_row(m_names));
+            TRY(m_output->write_entire_buffer({ "\n", 1 }));
             ++lines_written;
         }
 
         for (auto&& row : m_data) {
             if (with_headers) {
                 if (row.size() != m_names.size())
-                    set_error(WriteError::NonConformingColumnCount);
+                    return Error::from_string_literal("Header count does not match given column count");
             }
 
-            write_row(row);
-            if (m_output.write({ "\n", 1 }) != 1)
-                set_error(WriteError::InternalError);
+            TRY(write_row(row));
+            TRY(m_output->write_entire_buffer({ "\n", 1 }));
             ++lines_written;
 
             if (lines_written >= max_preview_lines)
                 break;
         }
+        return {};
     }
 
 private:
-    void set_error(WriteError error)
-    {
-        if (m_error == WriteError::None)
-            m_error = error;
-    }
-
     template<typename T>
-    void write_row(T&& row)
+    ErrorOr<void> write_row(T&& row)
     {
         bool first = true;
         for (auto&& entry : row) {
             if (!first) {
-                if (m_output.write(m_traits.separator.bytes()) != m_traits.separator.length())
-                    set_error(WriteError::InternalError);
+                TRY(m_output->write_entire_buffer(m_traits.separator.bytes()));
             }
             first = false;
-            write_entry(entry);
+            TRY(write_entry(entry));
         }
+        return {};
     }
 
     template<typename T>
-    void write_entry(T&& entry)
+    ErrorOr<void> write_entry(T&& entry)
     {
         auto string = DeprecatedString::formatted("{}", FormatIfSupported(entry));
 
@@ -169,49 +135,42 @@ private:
         }
 
         if (safe_to_write_normally) {
-            if (m_output.write(string.bytes()) != string.length())
-                set_error(WriteError::InternalError);
-            return;
+            if (!string.is_empty())
+                TRY(m_output->write_entire_buffer(string.bytes()));
+            return {};
         }
 
-        if (m_output.write(m_traits.quote.bytes()) != m_traits.quote.length())
-            set_error(WriteError::InternalError);
+        TRY(m_output->write_entire_buffer(m_traits.quote.bytes()));
 
         GenericLexer lexer(string);
         while (!lexer.is_eof()) {
             if (lexer.consume_specific(m_traits.quote)) {
                 switch (m_traits.quote_escape) {
                 case WriterTraits::Repeat:
-                    if (m_output.write(m_traits.quote.bytes()) != m_traits.quote.length())
-                        set_error(WriteError::InternalError);
-                    if (m_output.write(m_traits.quote.bytes()) != m_traits.quote.length())
-                        set_error(WriteError::InternalError);
+                    TRY(m_output->write_entire_buffer(m_traits.quote.bytes()));
+                    TRY(m_output->write_entire_buffer(m_traits.quote.bytes()));
                     break;
                 case WriterTraits::Backslash:
-                    if (m_output.write({ "\\", 1 }) != 1)
-                        set_error(WriteError::InternalError);
-                    if (m_output.write(m_traits.quote.bytes()) != m_traits.quote.length())
-                        set_error(WriteError::InternalError);
+                    TRY(m_output->write_entire_buffer({ "\\", 1 }));
+                    TRY(m_output->write_entire_buffer(m_traits.quote.bytes()));
                     break;
                 }
                 continue;
             }
 
             auto ch = lexer.consume();
-            if (m_output.write({ &ch, 1 }) != 1)
-                set_error(WriteError::InternalError);
+            TRY(m_output->write_entire_buffer({ &ch, 1 }));
         }
 
-        if (m_output.write(m_traits.quote.bytes()) != m_traits.quote.length())
-            set_error(WriteError::InternalError);
+        TRY(m_output->write_entire_buffer(m_traits.quote.bytes()));
+        return {};
     }
 
     ContainerType const& m_data;
     WriterTraits m_traits;
     WriterBehavior m_behaviors;
-    HeaderType const& m_names;
-    WriteError m_error { WriteError::None };
-    OutputStream& m_output;
+    HeaderType m_names;
+    Core::Stream::Handle<Core::Stream::Stream> m_output;
 };
 
 }