From 55d730bd5c9787897299b4cebb93b41e72129106 Mon Sep 17 00:00:00 2001 From: Karol Kosek Date: Tue, 29 Aug 2023 14:35:23 +0200 Subject: [PATCH] LibIMAP+Mail+test-imap: Make Client requests infailable --- Userland/Applications/Mail/MailWidget.cpp | 17 +++-- Userland/Libraries/LibIMAP/Client.cpp | 82 +++++++++++------------ Userland/Libraries/LibIMAP/Client.h | 42 ++++++------ Userland/Utilities/test-imap.cpp | 26 +++---- 4 files changed, 83 insertions(+), 84 deletions(-) diff --git a/Userland/Applications/Mail/MailWidget.cpp b/Userland/Applications/Mail/MailWidget.cpp index 4370f7f9e5..2acdd28816 100644 --- a/Userland/Applications/Mail/MailWidget.cpp +++ b/Userland/Applications/Mail/MailWidget.cpp @@ -137,7 +137,7 @@ ErrorOr MailWidget::connect_and_login() m_statusbar->set_text(String::formatted("Connected. Logging in as {}...", username).release_value_but_fixme_should_propagate_errors()); - auto response = TRY(TRY(m_imap_client->login(username, password))->await()).release_value(); + auto response = TRY(m_imap_client->login(username, password)->await()).release_value(); if (response.status() != IMAP::ResponseStatus::OK) { dbgln("Failed to login. The server says: '{}'", response.response_text()); @@ -147,7 +147,7 @@ ErrorOr MailWidget::connect_and_login() } m_statusbar->set_text("Logged in. Loading mailboxes..."_string); - response = TRY(TRY(m_imap_client->list(""sv, "*"sv))->await()).release_value(); + response = TRY(m_imap_client->list(""sv, "*"sv)->await()).release_value(); if (response.status() != IMAP::ResponseStatus::OK) { dbgln("Failed to retrieve mailboxes. The server says: '{}'", response.response_text()); @@ -174,7 +174,7 @@ void MailWidget::on_window_close() // User closed main window before a connection was established return; } - auto response = move(MUST(MUST(m_imap_client->send_simple_command(IMAP::CommandType::Logout))->await()).release_value().get()); + auto response = move(MUST(m_imap_client->send_simple_command(IMAP::CommandType::Logout)->await()).release_value().get()); VERIFY(response.status() == IMAP::ResponseStatus::OK); m_imap_client->close(); @@ -265,7 +265,7 @@ void MailWidget::selected_mailbox() if (mailbox.flags & (unsigned)IMAP::MailboxFlag::NoSelect) return; - auto response = MUST(MUST(m_imap_client->select(mailbox.name))->await()).release_value(); + auto response = MUST(m_imap_client->select(mailbox.name)->await()).release_value(); if (response.status() != IMAP::ResponseStatus::OK) { dbgln("Failed to select mailbox. The server says: '{}'", response.response_text()); @@ -297,9 +297,8 @@ void MailWidget::selected_mailbox() }, }; - auto fetch_response = MUST(MUST(m_imap_client->fetch(fetch_command, false))->await()).release_value(); - - if (response.status() != IMAP::ResponseStatus::OK) { + auto fetch_response = MUST(m_imap_client->fetch(fetch_command, false)->await()).release_value(); + if (fetch_response.status() != IMAP::ResponseStatus::OK) { dbgln("Failed to retrieve subject/from for e-mails. The server says: '{}'", response.response_text()); m_statusbar->set_text(String::formatted("[{}]: Failed to fetch messages :^(", mailbox.name).release_value_but_fixme_should_propagate_errors()); GUI::MessageBox::show_error(window(), DeprecatedString::formatted("Failed to retrieve e-mails. The server says: '{}'", response.response_text())); @@ -453,7 +452,7 @@ void MailWidget::selected_email_to_load() }, }; - auto fetch_response = MUST(MUST(m_imap_client->fetch(fetch_command, false))->await()).release_value(); + auto fetch_response = MUST(m_imap_client->fetch(fetch_command, false)->await()).release_value(); if (fetch_response.status() != IMAP::ResponseStatus::OK) { dbgln("Failed to retrieve the body structure of the selected e-mail. The server says: '{}'", fetch_response.response_text()); @@ -517,7 +516,7 @@ void MailWidget::selected_email_to_load() }, }; - fetch_response = MUST(MUST(m_imap_client->fetch(fetch_command, false))->await()).release_value(); + fetch_response = MUST(m_imap_client->fetch(fetch_command, false)->await()).release_value(); if (fetch_response.status() != IMAP::ResponseStatus::OK) { dbgln("Failed to retrieve the body of the selected e-mail. The server says: '{}'", fetch_response.response_text()); diff --git a/Userland/Libraries/LibIMAP/Client.cpp b/Userland/Libraries/LibIMAP/Client.cpp index 452fc57a32..a38f351235 100644 --- a/Userland/Libraries/LibIMAP/Client.cpp +++ b/Userland/Libraries/LibIMAP/Client.cpp @@ -171,7 +171,7 @@ ErrorOr Client::send_raw(StringView data) return {}; } -ErrorOr>>> Client::send_command(Command&& command) +NonnullRefPtr>> Client::send_command(Command&& command) { m_command_queue.append(move(command)); m_current_command++; @@ -189,7 +189,7 @@ ErrorOr>>> Client::send_command(Command&& comm } template -RefPtr>> cast_promise(RefPtr>> promise_variant) +NonnullRefPtr>> cast_promise(NonnullRefPtr>> promise_variant) { auto new_promise = promise_variant->map>( [](Optional& variant) { @@ -198,44 +198,44 @@ RefPtr>> cast_promise(RefPtr>> pr return new_promise; } -ErrorOr>>> Client::login(StringView username, StringView password) +NonnullRefPtr>> Client::login(StringView username, StringView password) { auto command = Command { CommandType::Login, m_current_command, { serialize_astring(username), serialize_astring(password) } }; - return cast_promise(TRY(send_command(move(command)))); + return cast_promise(send_command(move(command))); } -ErrorOr>>> Client::list(StringView reference_name, StringView mailbox) +NonnullRefPtr>> Client::list(StringView reference_name, StringView mailbox) { auto command = Command { CommandType::List, m_current_command, { DeprecatedString::formatted("\"{}\"", reference_name), DeprecatedString::formatted("\"{}\"", mailbox) } }; - return cast_promise(TRY(send_command(move(command)))); + return cast_promise(send_command(move(command))); } -ErrorOr>>> Client::lsub(StringView reference_name, StringView mailbox) +NonnullRefPtr>> Client::lsub(StringView reference_name, StringView mailbox) { auto command = Command { CommandType::ListSub, m_current_command, { DeprecatedString::formatted("\"{}\"", reference_name), DeprecatedString::formatted("\"{}\"", mailbox) } }; - return cast_promise(TRY(send_command(move(command)))); + return cast_promise(send_command(move(command))); } -ErrorOr>>> Client::fetch(FetchCommand request, bool uid) +NonnullRefPtr>> Client::fetch(FetchCommand request, bool uid) { auto command = Command { uid ? CommandType::UIDFetch : CommandType::Fetch, m_current_command, { request.serialize() } }; - return cast_promise(TRY(send_command(move(command)))); + return cast_promise(send_command(move(command))); } -ErrorOr>>> Client::send_simple_command(CommandType type) +NonnullRefPtr>> Client::send_simple_command(CommandType type) { auto command = Command { type, m_current_command, {} }; - return TRY(send_command(move(command))); + return send_command(move(command)); } -ErrorOr>>> Client::select(StringView string) +NonnullRefPtr>> Client::select(StringView string) { auto command = Command { CommandType::Select, m_current_command, { serialize_astring(string) } }; - return cast_promise(TRY(send_command(move(command)))); + return cast_promise(send_command(move(command))); } ErrorOr Client::handle_parsed_response(ParseStatus&& parse_status) @@ -287,25 +287,25 @@ ErrorOr Client::send_next_command() return {}; } -ErrorOr>>> Client::examine(StringView string) +NonnullRefPtr>> Client::examine(StringView string) { auto command = Command { CommandType::Examine, m_current_command, { serialize_astring(string) } }; - return cast_promise(TRY(send_command(move(command)))); + return cast_promise(send_command(move(command))); } -ErrorOr>>> Client::create_mailbox(StringView name) +NonnullRefPtr>> Client::create_mailbox(StringView name) { auto command = Command { CommandType::Create, m_current_command, { serialize_astring(name) } }; - return cast_promise(TRY(send_command(move(command)))); + return cast_promise(send_command(move(command))); } -ErrorOr>>> Client::delete_mailbox(StringView name) +NonnullRefPtr>> Client::delete_mailbox(StringView name) { auto command = Command { CommandType::Delete, m_current_command, { serialize_astring(name) } }; - return cast_promise(TRY(send_command(move(command)))); + return cast_promise(send_command(move(command))); } -ErrorOr>>> Client::store(StoreMethod method, Sequence sequence_set, bool silent, Vector const& flags, bool uid) +NonnullRefPtr>> Client::store(StoreMethod method, Sequence sequence_set, bool silent, Vector const& flags, bool uid) { StringBuilder data_item_name; switch (method) { @@ -329,9 +329,9 @@ ErrorOr>>> Client::store(StoreMethod meth flags_builder.append(')'); auto command = Command { uid ? CommandType::UIDStore : CommandType::Store, m_current_command, { sequence_set.serialize(), data_item_name.to_deprecated_string(), flags_builder.to_deprecated_string() } }; - return cast_promise(TRY(send_command(move(command)))); + return cast_promise(send_command(move(command))); } -ErrorOr>>> Client::search(Optional charset, Vector&& keys, bool uid) +NonnullRefPtr>> Client::search(Optional charset, Vector&& keys, bool uid) { Vector args; if (charset.has_value()) { @@ -342,15 +342,15 @@ ErrorOr>>> Client::search(Optional(TRY(send_command(move(command)))); + return cast_promise(send_command(move(command))); } -ErrorOr>>> Client::idle() +NonnullRefPtr>> Client::idle() { - auto promise = TRY(send_simple_command(CommandType::Idle)); + auto promise = send_simple_command(CommandType::Idle); return cast_promise(promise); } -ErrorOr>>> Client::finish_idle() +NonnullRefPtr>> Client::finish_idle() { auto promise = Promise>::construct(); m_pending_promises.append(promise); @@ -359,7 +359,7 @@ ErrorOr>>> Client::finish_idle() return cast_promise(promise); } -ErrorOr>>> Client::status(StringView mailbox, Vector const& types) +NonnullRefPtr>> Client::status(StringView mailbox, Vector const& types) { Vector args; for (auto type : types) { @@ -386,10 +386,10 @@ ErrorOr>>> Client::status(StringView mail types_list.join(' ', args); types_list.append(')'); auto command = Command { CommandType::Status, m_current_command, { mailbox, types_list.to_deprecated_string() } }; - return cast_promise(TRY(send_command(move(command)))); + return cast_promise(send_command(move(command))); } -ErrorOr>>> Client::append(StringView mailbox, Message&& message, Optional> flags, Optional date_time) +NonnullRefPtr>> Client::append(StringView mailbox, Message&& message, Optional> flags, Optional date_time) { Vector args = { mailbox }; if (flags.has_value()) { @@ -404,7 +404,7 @@ ErrorOr>>> Client::append(StringView mail args.append(DeprecatedString::formatted("{{{}}}", message.data.length())); - auto continue_req = TRY(send_command(Command { CommandType::Append, m_current_command, args })); + auto continue_req = send_command(Command { CommandType::Append, m_current_command, args }); auto response_promise = Promise>::construct(); m_pending_promises.append(response_promise); @@ -421,33 +421,33 @@ ErrorOr>>> Client::append(StringView mail return cast_promise(response_promise); } -ErrorOr>>> Client::subscribe(StringView mailbox) +NonnullRefPtr>> Client::subscribe(StringView mailbox) { auto command = Command { CommandType::Subscribe, m_current_command, { serialize_astring(mailbox) } }; - return cast_promise(TRY(send_command(move(command)))); + return cast_promise(send_command(move(command))); } -ErrorOr>>> Client::unsubscribe(StringView mailbox) +NonnullRefPtr>> Client::unsubscribe(StringView mailbox) { auto command = Command { CommandType::Unsubscribe, m_current_command, { serialize_astring(mailbox) } }; - return cast_promise(TRY(send_command(move(command)))); + return cast_promise(send_command(move(command))); } -ErrorOr>>> Client::authenticate(StringView method) +NonnullRefPtr>> Client::authenticate(StringView method) { auto command = Command { CommandType::Authenticate, m_current_command, { method } }; - return TRY(send_command(move(command))); + return send_command(move(command)); } -ErrorOr>>> Client::rename(StringView from, StringView to) +NonnullRefPtr>> Client::rename(StringView from, StringView to) { auto command = Command { CommandType::Rename, m_current_command, { serialize_astring(from), serialize_astring(to) } }; - return cast_promise(TRY(send_command(move(command)))); + return cast_promise(send_command(move(command))); } -ErrorOr>>> Client::copy(Sequence sequence_set, StringView name, bool uid) +NonnullRefPtr>> Client::copy(Sequence sequence_set, StringView name, bool uid) { auto command = Command { uid ? CommandType::UIDCopy : CommandType::Copy, m_current_command, { sequence_set.serialize(), serialize_astring(name) } }; - return cast_promise(TRY(send_command(move(command)))); + return cast_promise(send_command(move(command))); } void Client::close() diff --git a/Userland/Libraries/LibIMAP/Client.h b/Userland/Libraries/LibIMAP/Client.h index 376d7e3309..d284b4feaa 100644 --- a/Userland/Libraries/LibIMAP/Client.h +++ b/Userland/Libraries/LibIMAP/Client.h @@ -30,28 +30,28 @@ public: return m_connect_pending; } - ErrorOr>>> send_command(Command&&); - ErrorOr>>> send_simple_command(CommandType); + NonnullRefPtr>> send_command(Command&&); + NonnullRefPtr>> send_simple_command(CommandType); ErrorOr send_raw(StringView data); - ErrorOr>>> login(StringView username, StringView password); - ErrorOr>>> list(StringView reference_name, StringView mailbox_name); - ErrorOr>>> lsub(StringView reference_name, StringView mailbox_name); - ErrorOr>>> select(StringView string); - ErrorOr>>> examine(StringView string); - ErrorOr>>> search(Optional charset, Vector&& search_keys, bool uid); - ErrorOr>>> fetch(FetchCommand request, bool uid); - ErrorOr>>> store(StoreMethod, Sequence, bool silent, Vector const& flags, bool uid); - ErrorOr>>> copy(Sequence sequence_set, StringView name, bool uid); - ErrorOr>>> create_mailbox(StringView name); - ErrorOr>>> delete_mailbox(StringView name); - ErrorOr>>> subscribe(StringView mailbox); - ErrorOr>>> unsubscribe(StringView mailbox); - ErrorOr>>> rename(StringView from, StringView to); - ErrorOr>>> authenticate(StringView method); - ErrorOr>>> idle(); - ErrorOr>>> finish_idle(); - ErrorOr>>> status(StringView mailbox, Vector const& types); - ErrorOr>>> append(StringView mailbox, Message&& message, Optional> flags = {}, Optional date_time = {}); + NonnullRefPtr>> login(StringView username, StringView password); + NonnullRefPtr>> list(StringView reference_name, StringView mailbox_name); + NonnullRefPtr>> lsub(StringView reference_name, StringView mailbox_name); + NonnullRefPtr>> select(StringView string); + NonnullRefPtr>> examine(StringView string); + NonnullRefPtr>> search(Optional charset, Vector&& search_keys, bool uid); + NonnullRefPtr>> fetch(FetchCommand request, bool uid); + NonnullRefPtr>> store(StoreMethod, Sequence, bool silent, Vector const& flags, bool uid); + NonnullRefPtr>> copy(Sequence sequence_set, StringView name, bool uid); + NonnullRefPtr>> create_mailbox(StringView name); + NonnullRefPtr>> delete_mailbox(StringView name); + NonnullRefPtr>> subscribe(StringView mailbox); + NonnullRefPtr>> unsubscribe(StringView mailbox); + NonnullRefPtr>> rename(StringView from, StringView to); + NonnullRefPtr>> authenticate(StringView method); + NonnullRefPtr>> idle(); + NonnullRefPtr>> finish_idle(); + NonnullRefPtr>> status(StringView mailbox, Vector const& types); + NonnullRefPtr>> append(StringView mailbox, Message&& message, Optional> flags = {}, Optional date_time = {}); bool is_open(); void close(); diff --git a/Userland/Utilities/test-imap.cpp b/Userland/Utilities/test-imap.cpp index 77db05cdab..218a2ea6fb 100644 --- a/Userland/Utilities/test-imap.cpp +++ b/Userland/Utilities/test-imap.cpp @@ -46,18 +46,18 @@ ErrorOr serenity_main(Main::Arguments arguments) auto client = TRY(tls ? IMAP::Client::connect_tls(host, port) : IMAP::Client::connect_plaintext(host, port)); TRY(client->connection_promise()->await()); - auto response = TRY(TRY(client->login(username, password.view()))->await()).release_value(); + auto response = TRY(client->login(username, password.view())->await()).release_value(); outln("[LOGIN] Login response: {}", response.response_text()); - response = move(TRY(TRY(client->send_simple_command(IMAP::CommandType::Capability))->await()).value().get()); + response = move(TRY(client->send_simple_command(IMAP::CommandType::Capability)->await()).value().get()); outln("[CAPABILITY] First capability: {}", response.data().capabilities().first()); bool idle_supported = !response.data().capabilities().find_if([](auto capability) { return capability.equals_ignoring_ascii_case("IDLE"sv); }).is_end(); - response = TRY(TRY(client->list(""sv, "*"sv))->await()).release_value(); + response = TRY(client->list(""sv, "*"sv)->await()).release_value(); outln("[LIST] First mailbox: {}", response.data().list_items().first().name); auto mailbox = "Inbox"sv; - response = TRY(TRY(client->select(mailbox))->await()).release_value(); + response = TRY(client->select(mailbox)->await()).release_value(); outln("[SELECT] Select response: {}", response.response_text()); auto message = Message { @@ -70,7 +70,7 @@ ErrorOr serenity_main(Main::Arguments arguments) "This is a message just to say hello.\r\n" "So, \"Hello\"." }; - auto promise = TRY(client->append("INBOX"sv, move(message))); + auto promise = client->append("INBOX"sv, move(message)); response = TRY(promise->await()).release_value(); outln("[APPEND] Response: {}", response.response_text()); @@ -79,13 +79,13 @@ ErrorOr serenity_main(Main::Arguments arguments) IMAP::SearchKey::From { "jdoe@machine.example" } }); keys.append(IMAP::SearchKey { IMAP::SearchKey::Subject { "Saying Hello" } }); - response = TRY(TRY(client->search({}, move(keys), false))->await()).release_value(); + response = TRY(client->search({}, move(keys), false)->await()).release_value(); Vector search_results = move(response.data().search_results()); auto added_message = search_results.first(); outln("[SEARCH] Number of results: {}", search_results.size()); - response = TRY(TRY(client->status("INBOX"sv, { IMAP::StatusItemType::Recent, IMAP::StatusItemType::Messages }))->await()).release_value(); + response = TRY(client->status("INBOX"sv, { IMAP::StatusItemType::Recent, IMAP::StatusItemType::Messages })->await()).release_value(); outln("[STATUS] Recent items: {}", response.data().status_item().get(IMAP::StatusItemType::Recent)); for (auto item : search_results) { @@ -118,7 +118,7 @@ ErrorOr serenity_main(Main::Arguments arguments) }; // clang-format on - auto fetch_response = TRY(TRY(client->fetch(fetch_command, false))->await()).release_value(); + auto fetch_response = TRY(client->fetch(fetch_command, false)->await()).release_value(); outln("[FETCH] Subject of search result: {}", fetch_response.data() .fetch_data() @@ -136,22 +136,22 @@ ErrorOr serenity_main(Main::Arguments arguments) // FIXME: There is a discrepancy between IMAP::Sequence wanting signed ints // and IMAP search results returning unsigned ones. Find which one is // more correct and fix this. - response = TRY(TRY(client->store(IMAP::StoreMethod::Add, { static_cast(added_message), static_cast(added_message) }, false, { "\\Deleted" }, false))->await()).release_value(); + response = TRY(client->store(IMAP::StoreMethod::Add, { static_cast(added_message), static_cast(added_message) }, false, { "\\Deleted" }, false)->await()).release_value(); outln("[STORE] Store response: {}", response.response_text()); - response = move(TRY(TRY(client->send_simple_command(IMAP::CommandType::Expunge))->await()).release_value().get()); + response = move(TRY(client->send_simple_command(IMAP::CommandType::Expunge)->await()).release_value().get()); outln("[EXPUNGE] Number of expunged entries: {}", response.data().expunged().size()); if (idle_supported) { - VERIFY(TRY(TRY(client->idle())->await()).has_value()); + VERIFY(TRY(client->idle()->await()).has_value()); sleep(3); - response = TRY(TRY(client->finish_idle())->await()).release_value(); + response = TRY(client->finish_idle()->await()).release_value(); outln("[IDLE] Idle response: {}", response.response_text()); } else { outln("[IDLE] Skipped. No IDLE support."); } - response = move(TRY(TRY(client->send_simple_command(IMAP::CommandType::Logout))->await()).release_value().get()); + response = move(TRY(client->send_simple_command(IMAP::CommandType::Logout)->await()).release_value().get()); outln("[LOGOUT] Bye: {}", response.data().bye_message().value()); client->close();