Fixed issue-973 (timeout in Salt Manager) (#981)

The implementation now offers "infinite timeout" with
the option to abort download.
This commit is contained in:
Matthias Köfferlein 2022-02-08 19:05:53 +01:00 committed by GitHub
parent 71c9073bc5
commit c80f789e5a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 250 additions and 59 deletions

View File

@ -308,6 +308,13 @@
</property>
</spacer>
</item>
<item>
<widget class="QPushButton" name="abort_button">
<property name="text">
<string>Abort</string>
</property>
</widget>
</item>
<item>
<widget class="QPushButton" name="close_button">
<property name="text">

View File

@ -380,7 +380,7 @@ public:
}
bool
Salt::create_grain (const SaltGrain &templ, SaltGrain &target)
Salt::create_grain (const SaltGrain &templ, SaltGrain &target, double timeout, tl::InputHttpStreamCallback *callback)
{
tl_assert (m_root.begin_collections () != m_root.end_collections ());
@ -471,7 +471,7 @@ Salt::create_grain (const SaltGrain &templ, SaltGrain &target)
// otherwise download from the URL
tl::info << QObject::tr ("Downloading package from '%1' to '%2' ..").arg (tl::to_qstring (templ.url ())).arg (tl::to_qstring (target.path ()));
res = tl::WebDAVObject::download (templ.url (), target.path ());
res = tl::WebDAVObject::download (templ.url (), target.path (), timeout, callback);
} else {

View File

@ -183,7 +183,7 @@ public:
*
* Returns true, if the package could be created successfully.
*/
bool create_grain (const SaltGrain &templ, SaltGrain &target);
bool create_grain (const SaltGrain &templ, SaltGrain &target, double timeout = 60.0, tl::InputHttpStreamCallback *callback = 0);
/**
* @brief Removes redundant entries with same names

View File

@ -26,6 +26,7 @@
#include "tlProgress.h"
#include "tlFileUtils.h"
#include "tlWebDAV.h"
#include "tlLog.h"
#include <memory>
#include <QTreeWidgetItem>
@ -38,16 +39,18 @@ namespace lay
// ----------------------------------------------------------------------------------
ConfirmationDialog::ConfirmationDialog (QWidget *parent)
: QDialog (parent), m_confirmed (false), m_cancelled (false), m_file (50000, true)
: QDialog (parent), m_confirmed (false), m_cancelled (false), m_aborted (false), m_file (50000, true)
{
Ui::SaltManagerInstallConfirmationDialog::setupUi (this);
connect (ok_button, SIGNAL (clicked ()), this, SLOT (confirm_pressed ()));
connect (cancel_button, SIGNAL (clicked ()), this, SLOT (cancel_pressed ()));
connect (close_button, SIGNAL (clicked ()), this, SLOT (close_pressed ()));
connect (abort_button, SIGNAL (clicked ()), this, SLOT (abort_pressed ()));
log_panel->hide ();
attn_frame->hide ();
abort_button->hide ();
log_view->setModel (&m_file);
connect (&m_file, SIGNAL (layoutChanged ()), log_view, SLOT (scrollToBottom ()));
@ -147,13 +150,15 @@ ConfirmationDialog::start ()
{
confirm_panel->hide ();
log_panel->show ();
close_button->setEnabled (false);
close_button->hide ();
abort_button->show ();
}
void
ConfirmationDialog::finish ()
{
close_button->setEnabled (true);
close_button->show ();
abort_button->hide ();
}
// ----------------------------------------------------------------------------------
@ -400,16 +405,28 @@ SaltDownloadManager::make_confirmation_dialog (QWidget *parent, const lay::Salt
namespace
{
class DownloadProgressAdaptor
: public tl::ProgressAdaptor
: public tl::ProgressAdaptor, public tl::InputHttpStreamCallback
{
public:
DownloadProgressAdaptor (lay::ConfirmationDialog *dialog, const std::string &name)
: mp_dialog (dialog), m_name (name)
: mp_dialog (dialog), m_name (name), m_is_aborted (false)
{
mp_dialog->mark_fetching (m_name);
}
virtual void yield (tl::Progress * /*progress*/) { }
virtual void yield (tl::Progress * /*progress*/)
{
QCoreApplication::processEvents (QEventLoop::AllEvents | QEventLoop::WaitForMoreEvents, 100);
if (mp_dialog->is_aborted ()) {
m_is_aborted = true;
throw tl::CancelException ();
}
}
virtual void wait_for_input ()
{
yield (0);
}
virtual void trigger (tl::Progress *progress)
{
@ -426,9 +443,15 @@ namespace
mp_dialog->mark_success (m_name);
}
bool is_aborted () const
{
return m_is_aborted;
}
private:
lay::ConfirmationDialog *mp_dialog;
std::string m_name;
bool m_is_aborted;
};
}
@ -473,15 +496,17 @@ SaltDownloadManager::execute (lay::SaltManagerDialog *parent, lay::Salt &salt)
int status = 1;
{
DownloadProgressAdaptor pa (dialog.get (), p->name);
if (! salt.create_grain (p->grain, target)) {
pa.error ();
result = false;
status = 0;
} else {
pa.success ();
}
DownloadProgressAdaptor pa (dialog.get (), p->name);
if (! salt.create_grain (p->grain, target, 0.0 /*infinite timeout*/, &pa)) {
pa.error ();
result = false;
status = 0;
} else {
pa.success ();
}
if (pa.is_aborted ()) {
break;
}
try {
@ -517,7 +542,7 @@ SaltDownloadManager::execute (lay::SaltManagerDialog *parent, lay::Salt &salt)
target.set_path (g->path ());
}
if (! salt.create_grain (p->grain, target)) {
if (! salt.create_grain (p->grain, target, 60.0 /*timeout for offline installation*/)) {
tl::error << tl::to_string (QObject::tr ("Installation failed for package %1").arg (tl::to_qstring (target.name ())));
result = false;
} else {

View File

@ -34,6 +34,11 @@
#include <string>
#include <map>
namespace tl
{
class InputHttpStreamCallback;
}
namespace lay
{
@ -52,6 +57,7 @@ public:
bool is_confirmed () const { return m_confirmed; }
bool is_cancelled () const { return m_cancelled; }
bool is_aborted () const { return m_aborted; }
void start ();
void separator ();
@ -65,10 +71,11 @@ public:
private slots:
void confirm_pressed () { m_confirmed = true; }
void cancel_pressed () { m_cancelled = true; }
void abort_pressed () { m_aborted = true; }
void close_pressed () { hide (); }
private:
bool m_confirmed, m_cancelled;
bool m_confirmed, m_cancelled, m_aborted;
lay::LogFile m_file;
std::map<std::string, QTreeWidgetItem *> m_items_by_name;

View File

@ -512,7 +512,7 @@ SaltGrain::from_path (const std::string &path)
}
tl::InputStream *
SaltGrain::stream_from_url (std::string &url)
SaltGrain::stream_from_url (std::string &url, double timeout, tl::InputHttpStreamCallback *callback)
{
if (url.empty ()) {
throw tl::Exception (tl::to_string (QObject::tr ("No download link available")));
@ -535,17 +535,17 @@ SaltGrain::stream_from_url (std::string &url)
std::string spec_url = SaltGrain::spec_url (url);
if (spec_url.find ("http:") == 0 || spec_url.find ("https:") == 0) {
return tl::WebDAVObject::download_item (spec_url);
return tl::WebDAVObject::download_item (spec_url, timeout, callback);
} else {
return new tl::InputStream (spec_url);
}
}
SaltGrain
SaltGrain::from_url (const std::string &url_in)
SaltGrain::from_url (const std::string &url_in, double timeout, tl::InputHttpStreamCallback *callback)
{
std::string url = url_in;
std::unique_ptr<tl::InputStream> stream (stream_from_url (url));
std::unique_ptr<tl::InputStream> stream (stream_from_url (url, timeout, callback));
SaltGrain g;
g.load (*stream);

View File

@ -31,6 +31,11 @@
#include <QTime>
#include <QImage>
namespace tl
{
class InputHttpStreamCallback;
}
namespace lay
{
@ -462,7 +467,7 @@ public:
* The data is read from "URL/grain.xml". This method will throw an
* exception if an error occurs during reading.
*/
static SaltGrain from_url (const std::string &url);
static SaltGrain from_url (const std::string &url, double timeout = 60.0, tl::InputHttpStreamCallback *callback = 0);
/**
* @brief Returns a stream prepared for downloading the grain
@ -470,7 +475,7 @@ public:
* "url" is the download URL on input and gets modified to match the
* actual URL if it is a relative one.
*/
static tl::InputStream *stream_from_url (std::string &url);
static tl::InputStream *stream_from_url (std::string &url, double timeout = 60.0, tl::InputHttpStreamCallback *callback = 0);
/**
* @brief Forms the spec file download URL from a given download URL

View File

@ -63,6 +63,18 @@ public:
static std::string format_error (const std::string &em, int ec, const std::string &url, const std::string &body);
};
/**
* @brief A callback function during waiting for a response
*/
class TL_PUBLIC InputHttpStreamCallback
{
public:
InputHttpStreamCallback () { }
virtual ~InputHttpStreamCallback () { }
virtual void wait_for_input () { }
};
class InputHttpStreamPrivateData;
/**
@ -99,9 +111,32 @@ public:
/**
* @brief Polling: call this function regularly to explicitly establish polling
* (in the Qt framework, this is done automatically within the event loop)
* May throw a tl::CancelException to stop.
* Returns true if a message has arrived.
*/
void tick ();
/**
* @brief Sets a timeout callback
* The callback's wait_for_input method is called regularily while the stream
* waits for HTTP responses.
* The implementation may throw a tl::CancelException to stop the polling.
*/
void set_callback (tl::InputHttpStreamCallback *callback)
{
mp_callback = callback;
}
/**
* @brief Sets the timeout in seconds
*/
void set_timeout (double to);
/**
* @brief Gets the timeout in seconds or zero if no timeout is set.
*/
double timeout () const;
/**
* @brief Sends the request for data
* To ensure prompt delivery of data, this method can be used prior to
@ -171,6 +206,7 @@ public:
private:
InputHttpStreamPrivateData *mp_data;
InputHttpStreamCallback *mp_callback;
};
}

View File

@ -524,7 +524,12 @@ public:
* @brief Must be called in regular intervals to update the status
* Returns the number of open connections.
*/
int tick ();
void tick ();
/**
* @brief Returns true if a reply has arrived
*/
bool has_reply () const;
/**
* @brief The singleton instance
@ -555,7 +560,8 @@ private:
InputHttpStream::InputHttpStream (const std::string &url)
{
mp_data = new InputHttpStreamPrivateData (url);
mp_data = new InputHttpStreamPrivateData (this, url);
mp_callback = 0;
}
InputHttpStream::~InputHttpStream ()
@ -658,9 +664,25 @@ InputHttpStream::is_available ()
void
InputHttpStream::tick ()
{
if (mp_callback) {
mp_callback->wait_for_input ();
}
CurlNetworkManager::instance ()->tick ();
}
void
InputHttpStream::set_timeout (double to)
{
mp_data->set_timeout (to);
}
double
InputHttpStream::timeout () const
{
return mp_data->timeout ();
}
// ----------------------------------------------------------------------
// CurlConnection implementation
@ -1081,7 +1103,8 @@ void CurlNetworkManager::release_connection (CurlConnection *connection)
void CurlNetworkManager::on_tick ()
{
if (tick ()) {
tick ();
if (! has_reply ()) {
// NOTE: don't reschedule if there is no DM scheduler. This will cause deep
// recursion.
if (tl::DeferredMethodScheduler::instance ()) {
@ -1090,13 +1113,18 @@ void CurlNetworkManager::on_tick ()
}
}
int CurlNetworkManager::tick ()
bool CurlNetworkManager::has_reply () const
{
return m_still_running <= 0;
}
void CurlNetworkManager::tick ()
{
#if defined(DEBUG_CURL)
std::cerr << "CurlNetworkManager::tick()" << std::endl;
#endif
if (m_still_running <= 0) {
return 0;
return;
}
struct timeval timeout;
@ -1189,14 +1217,13 @@ int CurlNetworkManager::tick ()
}
}
return m_still_running;
}
// ---------------------------------------------------------------
// InputHttpStreamPrivateData implementation
InputHttpStreamPrivateData::InputHttpStreamPrivateData (const std::string &url)
InputHttpStreamPrivateData::InputHttpStreamPrivateData (InputHttpStream *stream, const std::string &url)
: m_timeout (10.0), mp_stream (stream)
{
m_sent = false;
m_ready = false;
@ -1212,6 +1239,18 @@ InputHttpStreamPrivateData::~InputHttpStreamPrivateData ()
// .. nothing yet ..
}
void
InputHttpStreamPrivateData::set_timeout (double to)
{
m_timeout = to;
}
double
InputHttpStreamPrivateData::timeout () const
{
return m_timeout;
}
bool
InputHttpStreamPrivateData::data_available ()
{
@ -1292,7 +1331,9 @@ InputHttpStreamPrivateData::read (char *b, size_t n)
m_progress.reset (new tl::AbsoluteProgress (tl::to_string (tr ("Downloading")) + " " + m_connection->url (), 1));
}
while (n > m_connection->read_available () && ! m_connection->finished () && CurlNetworkManager::instance ()->tick ()) {
tl::Clock start_time = tl::Clock::current ();
while (n > m_connection->read_available () && ! m_connection->finished () && (m_timeout <= 0.0 || (tl::Clock::current() - start_time).seconds () < m_timeout) && ! tl::CurlNetworkManager::instance ()->has_reply ()) {
mp_stream->tick ();
++*m_progress;
}
}

View File

@ -48,7 +48,7 @@ public:
/**
* @brief Open a stream with the given URL
*/
InputHttpStreamPrivateData (const std::string &url);
InputHttpStreamPrivateData (InputHttpStream *stream, const std::string &url);
/**
* @brief Close the file
@ -135,6 +135,16 @@ public:
*/
bool data_available ();
/**
* @brief Sets the timeout in seconds
*/
void set_timeout (double to);
/**
* @brief Gets the timeout in seconds
*/
double timeout () const;
// Basic interface
virtual void reset ();
virtual void close ();
@ -143,12 +153,14 @@ public:
virtual std::string filename () const;
private:
std::auto_ptr<CurlConnection> m_connection;
std::unique_ptr<CurlConnection> m_connection;
tl::Event m_ready_event;
tl::Event m_data_ready_event;
bool m_sent;
bool m_ready;
std::auto_ptr<tl::AbsoluteProgress> m_progress;
std::unique_ptr<tl::AbsoluteProgress> m_progress;
double m_timeout;
InputHttpStream *mp_stream;
void on_data_available ();
void on_finished ();

View File

@ -142,4 +142,17 @@ InputHttpStream::tick ()
// .. nothing yet ..
}
void
InputHttpStream::set_timeout (double)
{
// .. nothing yet ..
}
double
InputHttpStream::timeout () const
{
return 0.0;
}
}

View File

@ -127,7 +127,8 @@ AuthenticationHandler::proxyAuthenticationRequired (const QNetworkProxy &proxy,
InputHttpStream::InputHttpStream (const std::string &url)
{
mp_data = new InputHttpStreamPrivateData (url);
mp_data = new InputHttpStreamPrivateData (this, url);
mp_callback = 0;
}
InputHttpStream::~InputHttpStream ()
@ -229,17 +230,32 @@ InputHttpStream::is_available ()
void
InputHttpStream::tick ()
{
if (mp_callback) {
mp_callback->wait_for_input ();
}
QCoreApplication::processEvents (QEventLoop::ExcludeUserInputEvents);
}
void
InputHttpStream::set_timeout (double to)
{
mp_data->set_timeout (to);
}
double
InputHttpStream::timeout () const
{
return mp_data->timeout ();
}
// ---------------------------------------------------------------
// InputHttpStreamPrivateData implementation
static QNetworkAccessManager *s_network_manager (0);
static AuthenticationHandler *s_auth_handler (0);
InputHttpStreamPrivateData::InputHttpStreamPrivateData (const std::string &url)
: m_url (url), mp_reply (0), m_request ("GET"), mp_buffer (0), mp_resend_timer (new QTimer (this))
InputHttpStreamPrivateData::InputHttpStreamPrivateData (InputHttpStream *stream, const std::string &url)
: m_url (url), mp_reply (0), m_request ("GET"), mp_buffer (0), mp_resend_timer (new QTimer (this)), m_timeout (10.0), mp_stream (stream)
{
if (! s_network_manager) {
@ -263,6 +279,18 @@ InputHttpStreamPrivateData::~InputHttpStreamPrivateData ()
close ();
}
void
InputHttpStreamPrivateData::set_timeout (double to)
{
m_timeout = to;
}
double
InputHttpStreamPrivateData::timeout () const
{
return m_timeout;
}
void
InputHttpStreamPrivateData::close ()
{
@ -417,11 +445,9 @@ InputHttpStreamPrivateData::read (char *b, size_t n)
issue_request (QUrl (tl::to_qstring (m_url)));
}
// TODO: progress
tl::Clock start_time = tl::Clock::current ();
double timeout = 10; // TODO: make variable
while (mp_reply == 0 && (tl::Clock::current() - start_time).seconds () < timeout) {
QCoreApplication::processEvents (QEventLoop::ExcludeUserInputEvents);
while (mp_reply == 0 && (m_timeout <= 0.0 || (tl::Clock::current() - start_time).seconds () < m_timeout)) {
mp_stream->tick ();
}
if (! mp_reply) {

View File

@ -42,6 +42,7 @@ namespace tl
{
class HttpCredentialProvider;
class InputHttpStream;
class AuthenticationHandler
: public QObject
@ -66,7 +67,7 @@ class InputHttpStreamPrivateData
Q_OBJECT
public:
InputHttpStreamPrivateData (const std::string &url);
InputHttpStreamPrivateData (InputHttpStream *stream, const std::string &url);
virtual ~InputHttpStreamPrivateData ();
@ -76,6 +77,8 @@ public:
void set_data (const char *data);
void set_data (const char *data, size_t n);
void add_header (const std::string &name, const std::string &value);
void set_timeout (double to);
double timeout () const;
tl::Event &ready ()
{
@ -118,6 +121,8 @@ private:
tl::Event m_ready;
QTimer *mp_resend_timer;
std::string m_ssl_errors;
double m_timeout;
InputHttpStream *mp_stream;
void issue_request (const QUrl &url);
};

View File

@ -148,11 +148,13 @@ static std::string item_name (const std::string &path1, const std::string &path2
}
void
WebDAVObject::read (const std::string &url, int depth)
WebDAVObject::read (const std::string &url, int depth, double timeout, tl::InputHttpStreamCallback *callback)
{
tl::URI base_uri (url);
tl::InputHttpStream http (url);
http.set_timeout (timeout);
http.set_callback (callback);
http.add_header ("User-Agent", "SVN");
http.add_header ("Depth", tl::to_string (depth));
http.set_request ("PROPFIND");
@ -202,12 +204,12 @@ struct DownloadItem
}
static
void fetch_download_items (const std::string &url, const std::string &target, std::list<DownloadItem> &items, tl::AbsoluteProgress &progress)
void fetch_download_items (const std::string &url, const std::string &target, std::list<DownloadItem> &items, tl::AbsoluteProgress &progress, double timeout, tl::InputHttpStreamCallback *callback)
{
++progress;
WebDAVObject object;
object.read (url, 1);
object.read (url, 1, timeout, callback);
if (object.is_collection ()) {
@ -231,7 +233,7 @@ void fetch_download_items (const std::string &url, const std::string &target, st
throw tl::Exception (tl::to_string (tr ("Download failed: unable to create subdirectory '%s' in '%s' - no write permissions")), i->name (), target);
}
fetch_download_items (i->url (), item_path, items, progress);
fetch_download_items (i->url (), item_path, items, progress, timeout, callback);
} else {
@ -250,16 +252,18 @@ void fetch_download_items (const std::string &url, const std::string &target, st
}
tl::InputStream *
WebDAVObject::download_item (const std::string &url)
WebDAVObject::download_item (const std::string &url, double timeout, tl::InputHttpStreamCallback *callback)
{
tl::InputHttpStream *http = new tl::InputHttpStream (url);
http->set_timeout (timeout);
http->set_callback (callback);
// This trick allows accessing GitHub repos through their SVN API
http->add_header ("User-Agent", "SVN");
return new tl::InputStream (http);
}
bool
WebDAVObject::download (const std::string &url, const std::string &target)
WebDAVObject::download (const std::string &url, const std::string &target, double timeout, tl::InputHttpStreamCallback *callback)
{
std::list<DownloadItem> items;
@ -267,7 +271,7 @@ WebDAVObject::download (const std::string &url, const std::string &target)
tl::info << tr ("Fetching file structure from ") << url;
tl::AbsoluteProgress progress (tl::sprintf (tl::to_string (tr ("Fetching directory structure from %s")), url));
fetch_download_items (url, target, items, progress);
fetch_download_items (url, target, items, progress, timeout, callback);
} catch (tl::Exception &ex) {
tl::error << tr ("Error downloading file structure from '") << url << "':" << tl::endl << ex.msg ();
@ -288,16 +292,24 @@ WebDAVObject::download (const std::string &url, const std::string &target)
try {
tl::OutputStream os (i->path);
std::unique_ptr<tl::InputStream> is (download_item (i->url));
std::unique_ptr<tl::InputStream> is (download_item (i->url, timeout, callback));
is->copy_to (os);
++progress;
} catch (tl::BreakException &ex) {
tl::info << tr ("Download was cancelled") << tl::endl << ex.msg ();
has_errors = true;
break;
} catch (tl::CancelException &ex) {
tl::info << tr ("Download was cancelled") << tl::endl << ex.msg ();
has_errors = true;
break;
} catch (tl::Exception &ex) {
tl::error << tr ("Error downloading file from '") << i->url << "':" << tl::endl << ex.msg ();
has_errors = true;
}
++progress;
}
}

View File

@ -33,6 +33,8 @@
namespace tl
{
class InputHttpStreamCallback;
/**
* @brief Represents an item in a WebDAV collection
*/
@ -109,7 +111,7 @@ public:
* @brief Populates the collection from the given URL
* The depth value can be 0 (self only) or 1 (self + collection members).
*/
void read (const std::string &url, int depth);
void read (const std::string &url, int depth, double timeout = 60.0, tl::InputHttpStreamCallback *callback = 0);
/**
* @brief Gets the items of this collection (begin iterator)
@ -142,14 +144,14 @@ public:
* This method throws an exception if the directory structure could
* not be obtained or downloading of one file failed.
*/
static bool download (const std::string &url, const std::string &target);
static bool download (const std::string &url, const std::string &target, double timeout = 60.0, tl::InputHttpStreamCallback *callback = 0);
/**
* @brief Gets a stream object for downloading the single item of the given URL
*
* The stream object returned needs to be deleted by the caller.
*/
static tl::InputStream *download_item (const std::string &url);
static tl::InputStream *download_item (const std::string &url, double timeout = 60.0, tl::InputHttpStreamCallback *callback = 0);
private:
container m_items;