From 70a9520cc3e2caf5f9062eac565cc3a297b881a5 Mon Sep 17 00:00:00 2001
From: David Kocik <kocikdav@gmail.com>
Date: Wed, 25 Jan 2023 17:45:40 +0100
Subject: [PATCH] App udpater fixes - checks of path, error reporting and
translations
---
src/slic3r/GUI/DownloaderFileGet.cpp | 4 +-
src/slic3r/GUI/UpdateDialogs.cpp | 42 +++++++++++++-----
src/slic3r/Utils/AppUpdater.cpp | 66 ++++++++++++++++++++--------
3 files changed, 80 insertions(+), 32 deletions(-)
diff --git a/src/slic3r/GUI/DownloaderFileGet.cpp b/src/slic3r/GUI/DownloaderFileGet.cpp
index 2e591a75a..d81261296 100644
--- a/src/slic3r/GUI/DownloaderFileGet.cpp
+++ b/src/slic3r/GUI/DownloaderFileGet.cpp
@@ -119,8 +119,8 @@ void FileGet::priv::get_perform()
wxString temp_path_wstring(m_tmp_path.wstring());
- std::cout << "dest_path: " << dest_path.string() << std::endl;
- std::cout << "m_tmp_path: " << m_tmp_path.string() << std::endl;
+ //std::cout << "dest_path: " << dest_path.string() << std::endl;
+ //std::cout << "m_tmp_path: " << m_tmp_path.string() << std::endl;
BOOST_LOG_TRIVIAL(info) << GUI::format("Starting download from %1% to %2%. Temp path is %3%",m_url, dest_path, m_tmp_path);
diff --git a/src/slic3r/GUI/UpdateDialogs.cpp b/src/slic3r/GUI/UpdateDialogs.cpp
index 9c4f12eaf..5663262ca 100644
--- a/src/slic3r/GUI/UpdateDialogs.cpp
+++ b/src/slic3r/GUI/UpdateDialogs.cpp
@@ -167,10 +167,14 @@ AppUpdateDownloadDialog::AppUpdateDownloadDialog( const Semver& ver_online, boos
wxString wxext = boost::nowide::widen(extension);
wildcard = GUI::format_wxstr("%1% Files (*.%2%)|*.%2%", wxext.Upper(), wxext);
}
+ boost::system::error_code ec;
+ boost::filesystem::path dir = boost::filesystem::absolute(boost::filesystem::path(GUI::format(txtctrl_path->GetValue())), ec);
+ if (ec)
+ dir = GUI::format(txtctrl_path->GetValue());
wxDirDialog save_dlg(
this
, _L("Select directory:")
- , txtctrl_path->GetValue()
+ , GUI::format_wxstr(dir.string())
/*
, filename //boost::nowide::widen(AppUpdater::get_filename_from_url(txtctrl_path->GetValue().ToUTF8().data()))
, wildcard
@@ -188,35 +192,46 @@ AppUpdateDownloadDialog::AppUpdateDownloadDialog( const Semver& ver_online, boos
if (auto* btn_ok = get_button(wxID_OK); btn_ok != NULL) {
btn_ok->SetLabel(_L("Download"));
btn_ok->Bind(wxEVT_BUTTON, ([this, path](wxCommandEvent& e){
- boost::filesystem::path path = boost::filesystem::path(txtctrl_path->GetValue().ToUTF8().data()) / GUI::format(filename);
boost::system::error_code ec;
- if (path.parent_path().string().empty()) {
+ std::string input = GUI::format(txtctrl_path->GetValue());
+ boost::filesystem::path dir = boost::filesystem::absolute(boost::filesystem::path(input), ec);
+ if (ec)
+ dir = boost::filesystem::path(input);
+ bool show_change = (dir.string() != input);
+ boost::filesystem::path path = dir / GUI::format(filename);
+ ec.clear();
+ if (dir.string().empty()) {
MessageDialog msgdlg(nullptr, _L("Directory path is empty."), _L("Notice"), wxOK);
msgdlg.ShowModal();
return;
}
ec.clear();
- if (!boost::filesystem::exists(path.parent_path(), ec) || !boost::filesystem::is_directory(path.parent_path(),ec) || ec) {
+ if (!boost::filesystem::exists(dir, ec) || !boost::filesystem::is_directory(dir,ec) || ec) {
ec.clear();
- if (!boost::filesystem::exists(path.parent_path().parent_path(), ec) || !boost::filesystem::is_directory(path.parent_path().parent_path(), ec) || ec) {
+ if (!boost::filesystem::exists(dir.parent_path(), ec) || !boost::filesystem::is_directory(dir.parent_path(), ec) || ec) {
MessageDialog msgdlg(nullptr, _L("Directory path is incorrect."), _L("Notice"), wxOK);
msgdlg.ShowModal();
return;
}
-
- MessageDialog msgdlg(nullptr, GUI::format_wxstr(_L("Directory %1% doesn't exists. Do you wish to create it?"), GUI::format_wxstr(path.parent_path().string())), _L("Notice"), wxYES_NO);
+ show_change = false;
+ MessageDialog msgdlg(nullptr, GUI::format_wxstr(_L("Directory %1% doesn't exists. Do you wish to create it?"), dir.string()), _L("Notice"), wxYES_NO);
if (msgdlg.ShowModal() != wxID_YES)
return;
ec.clear();
- if(!boost::filesystem::create_directory(path.parent_path(), ec) || ec)
- {
+ if(!boost::filesystem::create_directory(dir, ec) || ec) {
MessageDialog msgdlg(nullptr, _L("Failed to create directory."), _L("Notice"), wxOK);
msgdlg.ShowModal();
return;
}
}
if (boost::filesystem::exists(path)) {
- MessageDialog msgdlg(nullptr, GUI::format_wxstr(_L("File %1% already exists. Do you wish to overwrite it?"), GUI::format_wxstr(path.string())),_L("Notice"), wxYES_NO);
+ show_change = false;
+ MessageDialog msgdlg(nullptr, GUI::format_wxstr(_L("File %1% already exists. Do you wish to overwrite it?"), path.string()),_L("Notice"), wxYES_NO);
+ if (msgdlg.ShowModal() != wxID_YES)
+ return;
+ }
+ if (show_change) {
+ MessageDialog msgdlg(nullptr, GUI::format_wxstr(_L("Download path is %1%. Do you wish to continue?"), path.string()), _L("Notice"), wxYES_NO);
if (msgdlg.ShowModal() != wxID_YES)
return;
}
@@ -241,7 +256,12 @@ bool AppUpdateDownloadDialog::run_after_download() const
boost::filesystem::path AppUpdateDownloadDialog::get_download_path() const
{
- return boost::filesystem::path(txtctrl_path->GetValue().ToUTF8().data()) / GUI::format(filename);
+ boost::system::error_code ec;
+ std::string input = GUI::format(txtctrl_path->GetValue());
+ boost::filesystem::path dir = boost::filesystem::absolute(boost::filesystem::path(input), ec);
+ if (ec)
+ dir = boost::filesystem::path(input);
+ return dir / GUI::format(filename);
}
// MsgUpdateConfig
diff --git a/src/slic3r/Utils/AppUpdater.cpp b/src/slic3r/Utils/AppUpdater.cpp
index fc847ec53..d054db4b5 100644
--- a/src/slic3r/Utils/AppUpdater.cpp
+++ b/src/slic3r/Utils/AppUpdater.cpp
@@ -13,6 +13,7 @@
#include "slic3r/GUI/format.hpp"
#include "slic3r/GUI/GUI_App.hpp"
#include "slic3r/GUI/GUI.hpp"
+#include "slic3r/GUI/I18N.hpp"
#include "slic3r/Utils/Http.hpp"
#include "libslic3r/Utils.hpp"
@@ -36,7 +37,7 @@ namespace {
std::string msg;
bool res = GUI::create_process(path, std::wstring(), msg);
if (!res) {
- std::string full_message = GUI::format("Running downloaded instaler of %1% has failed:\n%2%", SLIC3R_APP_NAME, msg);
+ std::string full_message = GUI::format(_utf8("Running downloaded instaler of %1% has failed:\n%2%"), SLIC3R_APP_NAME, msg);
BOOST_LOG_TRIVIAL(error) << full_message; // lm: maybe UI error msg? // dk: bellow. (maybe some general show error evt would be better?)
wxCommandEvent* evt = new wxCommandEvent(EVT_SLIC3R_APP_DOWNLOAD_FAILED);
evt->SetString(full_message);
@@ -170,8 +171,10 @@ bool AppUpdater::priv::http_get_file(const std::string& url, size_t size_limit,
// progress function returns true as success (to continue)
cancel = (m_cancel ? true : !progress_fn(std::move(progress)));
if (cancel) {
- error_message = GUI::format("Error getting: `%1%`: Download was canceled.", //lm:typo //dk: am i blind? :)
- url);
+ // Lets keep error_message empty here - if there is need to show error dialog, the message will be probably shown by whatever caused the cancel.
+ /*
+ error_message = GUI::format(_utf8("Error getting: `%1%`: Download was canceled."), url);
+ */
BOOST_LOG_TRIVIAL(debug) << "AppUpdater::priv::http_get_file message: "<< error_message;
}
})
@@ -200,7 +203,30 @@ boost::filesystem::path AppUpdater::priv::download_file(const DownloadAppData& d
assert(!dest_path.empty());
if (dest_path.empty())
{
- BOOST_LOG_TRIVIAL(error) << "Download from " << data.url << " could not start. Destination path is empty.";
+ std::string line1 = GUI::format(_utf8("Internal download error for url %1%:"), data.url);
+ std::string line2 = _utf8("Destination path is empty.");
+ std::string message = GUI::format("%1%\n%2%", line1, line2);
+ BOOST_LOG_TRIVIAL(error) << message;
+ wxCommandEvent* evt = new wxCommandEvent(EVT_SLIC3R_APP_DOWNLOAD_FAILED);
+ evt->SetString(message);
+ GUI::wxGetApp().QueueEvent(evt);
+ return boost::filesystem::path();
+ }
+
+ boost::filesystem::path tmp_path = dest_path;
+ tmp_path += format(".%1%%2%", get_current_pid(), ".download");
+ FILE* file;
+ wxString temp_path_wstring(tmp_path.wstring());
+ file = fopen(temp_path_wstring.c_str(), "wb");
+ assert(file != NULL);
+ if (file == NULL) {
+ std::string line1 = GUI::format(_utf8("Download from %1% couldn't start:"), data.url);
+ std::string line2 = GUI::format(_utf8("Can't create file at %1%."), tmp_path.string());
+ std::string message = GUI::format("%1%\n%2%", line1, line2);
+ BOOST_LOG_TRIVIAL(error) << message;
+ wxCommandEvent* evt = new wxCommandEvent(EVT_SLIC3R_APP_DOWNLOAD_FAILED);
+ evt->SetString(message);
+ GUI::wxGetApp().QueueEvent(evt);
return boost::filesystem::path();
}
@@ -217,7 +243,7 @@ boost::filesystem::path AppUpdater::priv::download_file(const DownloadAppData& d
GUI::wxGetApp().QueueEvent(evt);
return false;
} else if (progress.dltotal > 0 && progress.dltotal < expected_size) {
- //lm:When will this happen? Is that not an error? // dk: It is possible error, but we cannot know until the download is finished. Somehow the total size can grow during the download.
+ // This is possible error, but we cannot know until the download is finished. Somehow the total size can grow during the download.
BOOST_LOG_TRIVIAL(info) << GUI::format("Downloading new %1% has incorrect size. The download will continue. \nExpected size: %2%\nDownload size: %3%", SLIC3R_APP_NAME, expected_size, progress.dltotal);
}
// progress event
@@ -232,27 +258,26 @@ boost::filesystem::path AppUpdater::priv::download_file(const DownloadAppData& d
return true;
}
// on_complete
- , [dest_path, expected_size](std::string body, std::string& error_message){
+ , [&file, dest_path, tmp_path, expected_size](std::string body, std::string& error_message){
// Size check. Does always 1 char == 1 byte?
size_t body_size = body.size();
if (body_size != expected_size) {
- //lm:UI message? // dk: changed. Now it propagates to UI.
- error_message = GUI::format("Downloaded file has wrong size. Expected size: %1% Downloaded size: %2%", expected_size, body_size);
+ error_message = GUI::format(_utf8("Downloaded file has wrong size. Expected size: %1% Downloaded size: %2%"), expected_size, body_size);
+ return false;
+ }
+ if (file == NULL) {
+ error_message = GUI::format(_utf8("Can't create file at %1%."), tmp_path.string());
return false;
}
- boost::filesystem::path tmp_path = dest_path;
- tmp_path += format(".%1%%2%", get_current_pid(), ".download");
try
{
- FILE* file;
- file = fopen(tmp_path.string().c_str(), "wb");
fwrite(body.c_str(), 1, body.size(), file);
fclose(file);
boost::filesystem::rename(tmp_path, dest_path);
}
catch (const std::exception& e)
{
- error_message = GUI::format("Failed to write and move %1% to %2%:/n%3%", tmp_path, dest_path, e.what());
+ error_message = GUI::format(_utf8("Failed to write to file or to move %1% to %2%:\n%3%"), tmp_path, dest_path, e.what());
return false;
}
return true;
@@ -261,16 +286,19 @@ boost::filesystem::path AppUpdater::priv::download_file(const DownloadAppData& d
);
if (!res)
{
- if (m_cancel)
- {
- BOOST_LOG_TRIVIAL(info) << error_message; //lm:Is this an error? // dk: changed to info
+ if (m_cancel) {
+ BOOST_LOG_TRIVIAL(info) << error_message;
wxCommandEvent* evt = new wxCommandEvent(EVT_SLIC3R_APP_DOWNLOAD_FAILED); // FAILED with empty msg only closes progress notification
GUI::wxGetApp().QueueEvent(evt);
} else {
- std::string message = GUI::format("Downloading new %1% has failed:\n%2%", SLIC3R_APP_NAME, error_message);
- BOOST_LOG_TRIVIAL(error) << message;
+ std::string message = (error_message.empty()
+ ? std::string()
+ : GUI::format(_utf8("Downloading new %1% has failed:\n%2%"), SLIC3R_APP_NAME, error_message));
wxCommandEvent* evt = new wxCommandEvent(EVT_SLIC3R_APP_DOWNLOAD_FAILED);
- evt->SetString(message);
+ if (!message.empty()) {
+ BOOST_LOG_TRIVIAL(error) << message;
+ evt->SetString(message);
+ }
GUI::wxGetApp().QueueEvent(evt);
}
return boost::filesystem::path();