mirror of
https://github.com/Cockatrice/Cockatrice.git
synced 2026-11-05 01:23:24 -08:00
[Oracle] Don't report a cancelled card database update as successful (#7082)
Some checks are pending
CodeQL / Analyze (cpp) (push) Waiting to run
CodeQL / Analyze (actions) (push) Waiting to run
Build Desktop / Configure (push) Waiting to run
Build Desktop / Debian 13 (push) Blocked by required conditions
Build Desktop / Debian 12 (push) Blocked by required conditions
Build Desktop / Fedora 44 (push) Blocked by required conditions
Build Desktop / Fedora 43 (push) Blocked by required conditions
Build Desktop / Servatrice_Debian 12 (push) Blocked by required conditions
Build Desktop / Ubuntu 24.04 (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 (push) Blocked by required conditions
Build Desktop / Arch (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 cover (push) Blocked by required conditions
Build Desktop / macOS 13 Intel (push) Blocked by required conditions
Build Desktop / macOS 14 (push) Blocked by required conditions
Build Desktop / macOS 15 (push) Blocked by required conditions
Build Desktop / macOS 26 Debug (push) Blocked by required conditions
Build Desktop / Windows 10 (push) Blocked by required conditions
Build Desktop / Prune caches (push) Waiting to run
Build Docker / Servatrice (arm) (push) Waiting to run
Build Docker / Servatrice (x86) (push) Waiting to run
Build Docker / Publish multi-platform Servatrice image (push) Blocked by required conditions
Some checks are pending
CodeQL / Analyze (cpp) (push) Waiting to run
CodeQL / Analyze (actions) (push) Waiting to run
Build Desktop / Configure (push) Waiting to run
Build Desktop / Debian 13 (push) Blocked by required conditions
Build Desktop / Debian 12 (push) Blocked by required conditions
Build Desktop / Fedora 44 (push) Blocked by required conditions
Build Desktop / Fedora 43 (push) Blocked by required conditions
Build Desktop / Servatrice_Debian 12 (push) Blocked by required conditions
Build Desktop / Ubuntu 24.04 (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 (push) Blocked by required conditions
Build Desktop / Arch (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 cover (push) Blocked by required conditions
Build Desktop / macOS 13 Intel (push) Blocked by required conditions
Build Desktop / macOS 14 (push) Blocked by required conditions
Build Desktop / macOS 15 (push) Blocked by required conditions
Build Desktop / macOS 26 Debug (push) Blocked by required conditions
Build Desktop / Windows 10 (push) Blocked by required conditions
Build Desktop / Prune caches (push) Waiting to run
Build Docker / Servatrice (arm) (push) Waiting to run
Build Docker / Servatrice (x86) (push) Waiting to run
Build Docker / Publish multi-platform Servatrice image (push) Blocked by required conditions
* [Oracle] Don't report a cancelled card database update as successful (#3470) Took 3 minutes * [Oracle] Only report cancel when nothing was saved yet Rejecting the wizard after SaveSetsPage or a download page already wrote the database to disk still replaced cards.xml on disk, but wasCancelled() forced exit code 1 so Cockatrice skipped the reload. Track successful saves with OracleWizard::markDatabaseSaved() and only treat a cancel as a failure when nothing was saved. * [Oracle] Restore updater stderr in card update error dialog --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
This commit is contained in:
parent
bbf0425655
commit
96b0888a73
7 changed files with 50 additions and 10 deletions
|
|
@ -1253,7 +1253,7 @@ void MainWindow::createCardUpdateProcess(bool background)
|
||||||
<< dir.absoluteFilePath(binaryName);
|
<< dir.absoluteFilePath(binaryName);
|
||||||
QMessageBox::warning(this, tr("Error"),
|
QMessageBox::warning(this, tr("Error"),
|
||||||
tr("Unable to run the card database updater: ") + dir.absoluteFilePath(binaryName));
|
tr("Unable to run the card database updater: ") + dir.absoluteFilePath(binaryName));
|
||||||
exitCardDatabaseUpdate();
|
exitCardDatabaseUpdate(false);
|
||||||
emit cardDatabaseUpdateFinished(false);
|
emit cardDatabaseUpdateFinished(false);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
@ -1330,7 +1330,7 @@ void MainWindow::cardUpdateStandardError()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
void MainWindow::exitCardDatabaseUpdate()
|
void MainWindow::exitCardDatabaseUpdate(bool doReload)
|
||||||
{
|
{
|
||||||
if (!cardUpdateProcess) {
|
if (!cardUpdateProcess) {
|
||||||
return;
|
return;
|
||||||
|
|
@ -1339,9 +1339,11 @@ void MainWindow::exitCardDatabaseUpdate()
|
||||||
cardUpdateProcess = nullptr;
|
cardUpdateProcess = nullptr;
|
||||||
statusBar()->clearMessage();
|
statusBar()->clearMessage();
|
||||||
|
|
||||||
// This can be the first successful load of the session (e.g. the onboarding
|
if (doReload) {
|
||||||
// download), so it must own the set check for that load.
|
// This can be the first successful load of the session (e.g. the onboarding
|
||||||
reloadCardDatabaseAndCheckSets();
|
// download), so it must own the set check for that load.
|
||||||
|
reloadCardDatabaseAndCheckSets();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
void MainWindow::reloadCardDatabaseAndCheckSets()
|
void MainWindow::reloadCardDatabaseAndCheckSets()
|
||||||
|
|
@ -1387,7 +1389,7 @@ void MainWindow::cardUpdateError(QProcess::ProcessError err)
|
||||||
}
|
}
|
||||||
|
|
||||||
qCWarning(CardDatabaseUpdateLog) << "The card database updater exited with an error:" << error;
|
qCWarning(CardDatabaseUpdateLog) << "The card database updater exited with an error:" << error;
|
||||||
exitCardDatabaseUpdate();
|
exitCardDatabaseUpdate(false);
|
||||||
QMessageBox::warning(this, tr("Error"), tr("The card database updater exited with an error:\n%1").arg(error));
|
QMessageBox::warning(this, tr("Error"), tr("The card database updater exited with an error:\n%1").arg(error));
|
||||||
emit cardDatabaseUpdateFinished(false);
|
emit cardDatabaseUpdateFinished(false);
|
||||||
}
|
}
|
||||||
|
|
@ -1400,12 +1402,14 @@ void MainWindow::cardUpdateFinished(int exitCode, QProcess::ExitStatus exitStatu
|
||||||
cardUpdateProgressOutput(); // drain any progress lines not yet parsed
|
cardUpdateProgressOutput(); // drain any progress lines not yet parsed
|
||||||
|
|
||||||
const bool success = (exitStatus == QProcess::NormalExit) && (exitCode == 0);
|
const bool success = (exitStatus == QProcess::NormalExit) && (exitCode == 0);
|
||||||
if (exitStatus == QProcess::NormalExit) {
|
if (success) {
|
||||||
SettingsCache::instance().updates().setLastCardUpdateCheck(QDateTime::currentDateTime().date());
|
SettingsCache::instance().updates().setLastCardUpdateCheck(QDateTime::currentDateTime().date());
|
||||||
|
} else if (exitStatus == QProcess::NormalExit) {
|
||||||
|
statusBar()->showMessage(tr("Card database update cancelled."), 10000);
|
||||||
}
|
}
|
||||||
qCInfo(CardDatabaseUpdateLog) << "The card database updater finished with exit code" << exitCode << "exit status"
|
qCInfo(CardDatabaseUpdateLog) << "The card database updater finished with exit code" << exitCode << "exit status"
|
||||||
<< static_cast<int>(exitStatus) << "success:" << success;
|
<< static_cast<int>(exitStatus) << "success:" << success;
|
||||||
exitCardDatabaseUpdate();
|
exitCardDatabaseUpdate(success);
|
||||||
notifyCardUpdateFinishedFromTray(success);
|
notifyCardUpdateFinishedFromTray(success);
|
||||||
emit cardDatabaseUpdateFinished(success);
|
emit cardDatabaseUpdateFinished(success);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -157,7 +157,7 @@ private:
|
||||||
return "oracle";
|
return "oracle";
|
||||||
}
|
}
|
||||||
void createCardUpdateProcess(bool background = false);
|
void createCardUpdateProcess(bool background = false);
|
||||||
void exitCardDatabaseUpdate();
|
void exitCardDatabaseUpdate(bool doReload);
|
||||||
/**
|
/**
|
||||||
* @brief Reloads the card database on a worker thread and runs checkUnknownSets() once the finished
|
* @brief Reloads the card database on a worker thread and runs checkUnknownSets() once the finished
|
||||||
* snapshot has been swapped in.
|
* snapshot has been swapped in.
|
||||||
|
|
|
||||||
|
|
@ -85,5 +85,8 @@ int main(int argc, char *argv[])
|
||||||
QTimer::singleShot(0, &wizard, [&wizard]() { wizard.runInBackground(); });
|
QTimer::singleShot(0, &wizard, [&wizard]() { wizard.runInBackground(); });
|
||||||
}
|
}
|
||||||
|
|
||||||
return app.exec();
|
const int execResult = app.exec();
|
||||||
|
// A cancel only counts when nothing was written yet: closing the wizard
|
||||||
|
// after the card database was already saved on disk is still an update.
|
||||||
|
return (wizard.wasCancelled() && !wizard.hasSavedDatabase()) ? 1 : execResult;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -128,12 +128,20 @@ void OracleWizard::accept()
|
||||||
|
|
||||||
void OracleWizard::reject()
|
void OracleWizard::reject()
|
||||||
{
|
{
|
||||||
|
cancelled = true;
|
||||||
|
|
||||||
// The wizard is being closed while a page may still run a worker on the
|
// The wizard is being closed while a page may still run a worker on the
|
||||||
// importer. Ask it to stop before the wizard (and the importer child) is
|
// importer. Ask it to stop before the wizard (and the importer child) is
|
||||||
// destroyed, so the worker thread never touches freed memory.
|
// destroyed, so the worker thread never touches freed memory.
|
||||||
if (auto *active = dynamic_cast<OracleWizardPage *>(currentPage())) {
|
if (auto *active = dynamic_cast<OracleWizardPage *>(currentPage())) {
|
||||||
active->cancelWork();
|
active->cancelWork();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const auto replies = nam->findChildren<QNetworkReply *>();
|
||||||
|
for (QNetworkReply *reply : replies) {
|
||||||
|
reply->abort();
|
||||||
|
}
|
||||||
|
|
||||||
QWizard::reject();
|
QWizard::reject();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -20,6 +20,19 @@ public:
|
||||||
explicit OracleWizard(QWidget *parent = nullptr);
|
explicit OracleWizard(QWidget *parent = nullptr);
|
||||||
void accept() override;
|
void accept() override;
|
||||||
void reject() override;
|
void reject() override;
|
||||||
|
bool wasCancelled()
|
||||||
|
{
|
||||||
|
return cancelled;
|
||||||
|
}
|
||||||
|
/** @brief Records that the card database was written to disk, so a later cancel still counts as an update. */
|
||||||
|
void markDatabaseSaved()
|
||||||
|
{
|
||||||
|
databaseSaved = true;
|
||||||
|
}
|
||||||
|
bool hasSavedDatabase()
|
||||||
|
{
|
||||||
|
return databaseSaved;
|
||||||
|
}
|
||||||
void enableButtons();
|
void enableButtons();
|
||||||
void disableButtons();
|
void disableButtons();
|
||||||
void retranslateUi();
|
void retranslateUi();
|
||||||
|
|
@ -67,6 +80,8 @@ private:
|
||||||
QByteArray tokensData;
|
QByteArray tokensData;
|
||||||
QString cardSourceUrl;
|
QString cardSourceUrl;
|
||||||
QString cardSourceVersion;
|
QString cardSourceVersion;
|
||||||
|
bool cancelled = false;
|
||||||
|
bool databaseSaved = false;
|
||||||
|
|
||||||
void migrateOracleSettings();
|
void migrateOracleSettings();
|
||||||
/**
|
/**
|
||||||
|
|
|
||||||
|
|
@ -484,6 +484,10 @@ void LoadSetsPage::actDownloadFinishedSetsFile()
|
||||||
{
|
{
|
||||||
// check for a reply
|
// check for a reply
|
||||||
auto *reply = dynamic_cast<QNetworkReply *>(sender());
|
auto *reply = dynamic_cast<QNetworkReply *>(sender());
|
||||||
|
if (wizard()->wasCancelled()) {
|
||||||
|
reply->deleteLater();
|
||||||
|
return;
|
||||||
|
}
|
||||||
auto errorCode = reply->error();
|
auto errorCode = reply->error();
|
||||||
if (errorCode != QNetworkReply::NoError) {
|
if (errorCode != QNetworkReply::NoError) {
|
||||||
QMessageBox::critical(this, tr("Error"), tr("Network error: %1.").arg(reply->errorString()));
|
QMessageBox::critical(this, tr("Error"), tr("Network error: %1.").arg(reply->errorString()));
|
||||||
|
|
@ -895,6 +899,7 @@ bool SaveSetsPage::validatePage()
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
wizard()->markDatabaseSaved();
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -178,6 +178,10 @@ void SimpleDownloadFilePage::actDownloadFinished()
|
||||||
{
|
{
|
||||||
// check for a reply
|
// check for a reply
|
||||||
auto *reply = dynamic_cast<QNetworkReply *>(sender());
|
auto *reply = dynamic_cast<QNetworkReply *>(sender());
|
||||||
|
if (wizard()->wasCancelled()) {
|
||||||
|
reply->deleteLater();
|
||||||
|
return;
|
||||||
|
}
|
||||||
QNetworkReply::NetworkError errorCode = reply->error();
|
QNetworkReply::NetworkError errorCode = reply->error();
|
||||||
if (errorCode != QNetworkReply::NoError) {
|
if (errorCode != QNetworkReply::NoError) {
|
||||||
QMessageBox::critical(this, tr("Error"), tr("Network error: %1.").arg(reply->errorString()));
|
QMessageBox::critical(this, tr("Error"), tr("Network error: %1.").arg(reply->errorString()));
|
||||||
|
|
@ -242,6 +246,7 @@ bool SimpleDownloadFilePage::saveToFile()
|
||||||
|
|
||||||
// clean saved downloadData
|
// clean saved downloadData
|
||||||
downloadData = QByteArray();
|
downloadData = QByteArray();
|
||||||
|
wizard()->markDatabaseSaved();
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue