mirror of
https://github.com/Cockatrice/Cockatrice.git
synced 2026-11-05 09:33:25 -08:00
Fix the Oracle refresh verdict and cover the parser write path
A background refresh that already installed cards.xml exited 1 when the token save afterwards failed, so the client kept its old in-memory database and reported the update as failed. Return the same verdict main() already computes: a reject after the database was written is a successful update. Cover the writer Oracle actually uses: add a short-write death test for CockatriceXml4Parser::saveToFile, which reverted to QFile would truncate an installed database without any test noticing. Route a failed background token download through reject() instead of a modal box on a hidden wizard, which left the client waiting with no timeout and blocked later updates. Test the first non-space byte in place rather than copying the payload, and report an import refusal with the importer's own message.
This commit is contained in:
parent
b719ee4006
commit
18b2896bf5
4 changed files with 89 additions and 6 deletions
|
|
@ -143,9 +143,11 @@ void OracleWizard::reject()
|
||||||
|
|
||||||
QWizard::reject();
|
QWizard::reject();
|
||||||
// A hidden wizard does not close the application's last visible window.
|
// A hidden wizard does not close the application's last visible window.
|
||||||
// Stop background refreshes explicitly so the hosting client receives failure.
|
// Stop background refreshes explicitly so the hosting client receives a verdict,
|
||||||
|
// using the same rule as main(): a reject after the database was written is still
|
||||||
|
// a successful update, only a reject that wrote nothing counts as a failure.
|
||||||
if (backgroundMode) {
|
if (backgroundMode) {
|
||||||
QCoreApplication::exit(1);
|
QCoreApplication::exit(hasSavedDatabase() ? 0 : 1);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -44,6 +44,7 @@
|
||||||
#include <QWizard>
|
#include <QWizard>
|
||||||
#include <QtConcurrentRun>
|
#include <QtConcurrentRun>
|
||||||
#include <QtGlobal>
|
#include <QtGlobal>
|
||||||
|
#include <algorithm>
|
||||||
#include <cstdio>
|
#include <cstdio>
|
||||||
#include <functional>
|
#include <functional>
|
||||||
#include <libcockatrice/settings/personal_settings.h>
|
#include <libcockatrice/settings/personal_settings.h>
|
||||||
|
|
@ -170,7 +171,12 @@ LoadSetsResult loadSetsData(const QPointer<OracleImporter> &importer, QByteArray
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (data.trimmed().startsWith("<")) {
|
// Test the first non-space byte in place: trimmed() would duplicate the whole
|
||||||
|
// payload, which on the plain-XML path is the largest download we handle.
|
||||||
|
const auto firstNonSpace = std::find_if_not(data.cbegin(), data.cend(),
|
||||||
|
[](char c) { return c == ' ' || c == '\t' || c == '\n' || c == '\r'; });
|
||||||
|
const bool looksLikeXml = firstNonSpace != data.cend() && *firstNonSpace == '<';
|
||||||
|
if (looksLikeXml) {
|
||||||
result.errorMessage = OracleDatabase::validateXml(data);
|
result.errorMessage = OracleDatabase::validateXml(data);
|
||||||
if (!result.errorMessage.isEmpty()) {
|
if (!result.errorMessage.isEmpty()) {
|
||||||
return result;
|
return result;
|
||||||
|
|
@ -916,7 +922,10 @@ bool SaveSetsPage::validatePage()
|
||||||
wizard()->xmlData.clear();
|
wizard()->xmlData.clear();
|
||||||
} else if (!wizard()->importer->saveToFile(fileName, wizard()->getCardSourceUrl(),
|
} else if (!wizard()->importer->saveToFile(fileName, wizard()->getCardSourceUrl(),
|
||||||
wizard()->getCardSourceVersion())) {
|
wizard()->getCardSourceVersion())) {
|
||||||
return failSave(tr("The file could not be saved to %1").arg(fileName));
|
// The importer refuses to write a database whose import did not finish, and
|
||||||
|
// reports the reason there; do not dress an import failure up as a disk failure.
|
||||||
|
const QString importError = wizard()->importer->getImportError();
|
||||||
|
return failSave(importError.isEmpty() ? tr("The file could not be saved to %1").arg(fileName) : importError);
|
||||||
}
|
}
|
||||||
|
|
||||||
wizard()->markDatabaseSaved();
|
wizard()->markDatabaseSaved();
|
||||||
|
|
|
||||||
|
|
@ -185,9 +185,17 @@ void SimpleDownloadFilePage::actDownloadFinished()
|
||||||
}
|
}
|
||||||
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()));
|
const QString message = tr("Network error: %1.").arg(reply->errorString());
|
||||||
wizard()->enableButtons();
|
|
||||||
reply->deleteLater();
|
reply->deleteLater();
|
||||||
|
// A hidden wizard cannot show a modal box: without an explicit stop it would sit
|
||||||
|
// here forever and the hosting client would wait with no timeout.
|
||||||
|
if (wizard()->backgroundMode) {
|
||||||
|
qWarning() << message;
|
||||||
|
wizard()->reject();
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
QMessageBox::critical(this, tr("Error"), message);
|
||||||
|
wizard()->enableButtons();
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -36,6 +36,12 @@
|
||||||
#include <libcockatrice/utility/card_ref.h>
|
#include <libcockatrice/utility/card_ref.h>
|
||||||
#include <string>
|
#include <string>
|
||||||
|
|
||||||
|
#ifdef Q_OS_UNIX
|
||||||
|
#include <csignal>
|
||||||
|
#include <sys/resource.h>
|
||||||
|
#include <unistd.h>
|
||||||
|
#endif
|
||||||
|
|
||||||
namespace
|
namespace
|
||||||
{
|
{
|
||||||
|
|
||||||
|
|
@ -204,6 +210,64 @@ TEST(CardDatabaseTest, Xml4LocalizedDataRoundTrip)
|
||||||
ASSERT_EQ(printings.first().getProperty("manacost"), "1R");
|
ASSERT_EQ(printings.first().getProperty("manacost"), "1R");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#ifdef Q_OS_UNIX
|
||||||
|
/**
|
||||||
|
* @brief A failed write keeps the previously installed card database intact.
|
||||||
|
*
|
||||||
|
* This is the writer Oracle uses to install a refreshed database, so the truncation
|
||||||
|
* bug has to be covered here and not only through OracleDatabase::saveXml.
|
||||||
|
*/
|
||||||
|
TEST(CardDatabaseTest, Xml4ShortWriteKeepsInstalledFile)
|
||||||
|
{
|
||||||
|
NoopCardSetPriorityController controller;
|
||||||
|
CardSetPtr set =
|
||||||
|
CardSet::newInstance(&controller, "TST", "Test Set", "expansion", QDate(), CardSet::PriorityPrimary);
|
||||||
|
|
||||||
|
QHash<QString, QString> props;
|
||||||
|
props["manacost"] = "1R";
|
||||||
|
PrintingInfo printing(set, LazyPropertiesHash(props));
|
||||||
|
SetToPrintingsMap setsInfo;
|
||||||
|
setsInfo["TST"].append(printing);
|
||||||
|
|
||||||
|
CardInfo::UiAttributes attributes = {.tableRow = 1};
|
||||||
|
CardInfoPtr card =
|
||||||
|
CardInfo::newInstance("Lightning Bolt", QString(1024, 'x'), false, {}, {}, {}, setsInfo, attributes);
|
||||||
|
|
||||||
|
SetNameMap sets;
|
||||||
|
sets.insert("TST", set);
|
||||||
|
CardNameMap cards;
|
||||||
|
cards.insert("Lightning Bolt", card);
|
||||||
|
|
||||||
|
QTemporaryDir tempDir;
|
||||||
|
ASSERT_TRUE(tempDir.isValid());
|
||||||
|
const QString fileName = tempDir.filePath("cards.xml");
|
||||||
|
|
||||||
|
QFile installed(fileName);
|
||||||
|
ASSERT_TRUE(installed.open(QIODevice::WriteOnly));
|
||||||
|
ASSERT_EQ(installed.write("installed database"), 18);
|
||||||
|
installed.close();
|
||||||
|
|
||||||
|
// A real filesystem write failure, isolated in the death-test child so the
|
||||||
|
// process-wide file limit never affects other cases or the parent runner.
|
||||||
|
ASSERT_EXIT(
|
||||||
|
{
|
||||||
|
std::signal(SIGXFSZ, SIG_IGN);
|
||||||
|
struct rlimit limit;
|
||||||
|
limit.rlim_cur = 128;
|
||||||
|
limit.rlim_max = 128;
|
||||||
|
if (setrlimit(RLIMIT_FSIZE, &limit) != 0) {
|
||||||
|
_exit(2);
|
||||||
|
}
|
||||||
|
CockatriceXml4Parser writer(&controller);
|
||||||
|
_exit(writer.saveToFile({}, sets, cards, fileName) ? 1 : 0);
|
||||||
|
},
|
||||||
|
::testing::ExitedWithCode(0), "");
|
||||||
|
|
||||||
|
ASSERT_TRUE(installed.open(QIODevice::ReadOnly));
|
||||||
|
EXPECT_EQ(installed.readAll(), "installed database");
|
||||||
|
}
|
||||||
|
#endif
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @brief Path provider serving a sandboxed copy of the test card database.
|
* @brief Path provider serving a sandboxed copy of the test card database.
|
||||||
*/
|
*/
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue