Commit bd873755 authored by David Jarvie's avatar David Jarvie
Browse files

Fix bad calendar files preventing command line actions from running

If a calendar file was unparsable, KAlarm initialisation did not
fully complete, so that command line actions hung.
parent 3076baff
Pipeline #114888 passed with stage
in 1 minute and 26 seconds
KAlarm Change Log
=== Version 3.3.4 (KDE Applications 21.12.1) --- 17 December 2021 ===
=== Version 3.3.4 (KDE Applications 21.12.1) --- 28 December 2021 ===
* Treat empty read-only, or non-existent, calendar files as loaded.
* Fix bad calendar files preventing command line actions from running.
* Ensure KAlarm command line actions are performed if KAlarm is already running [KDE Bug 446749]
* Don't disable alarms after KAlarm command line action while KAlarm is already running [KDE Bug 446749]
* Fix resource ID numbers not working in command line & DBUS commands.
......
......@@ -24,6 +24,9 @@ FileResource::FileResource(FileResourceSettings* settings)
: ResourceType(settings->id())
, mSettings(settings)
{
if (!mSettings || !mSettings->isValid()
|| id() < 0 || mSettings->id() != id())
mStatus = Status::NotConfigured;
}
FileResource::~FileResource()
......@@ -35,7 +38,7 @@ bool FileResource::isValid() const
#ifdef DEBUG_DETAIL
if (!mSettings || !mSettings->isValid())
qDebug() << "FileResource::isValid:" << displayId() << "NO (mSettings)";
if (mStatus == Status::NotConfigured)
if (mStatus == Status::NotConfigured || mStatus == Status::Unusable)
qDebug() << "FileResource::isValid:" << displayId() << "NO (Not configured)";
if (mStatus == Status::Closed)
qDebug() << "FileResource::isValid:" << displayId() << "NO (Closed)";
......@@ -330,7 +333,7 @@ bool FileResource::load(bool readThroughCache)
{
// Don't load a disabled resource, but mark it as usable (but not loaded).
qCDebug(KALARM_LOG) << "FileResource::load: Resource disabled" << displayName();
mStatus = Status::Ready;
setStatus(Status::Ready);
return false;
}
......@@ -701,4 +704,20 @@ void FileResource::handleEnabledChange(Changes changes, CalEvent::Types oldEnabl
}
}
/******************************************************************************
* Set the new status of the resource.
* If the resource status is already Unusable, it cannot be set usable again.
*/
void FileResource::setStatus(Status newStatus)
{
if (newStatus != mStatus
&& (mStatus < Status::Unusable || mStatus == Status::Unusable || newStatus > mStatus))
{
mStatus = newStatus;
if (mStatus > Status::Unusable)
setFailed();
setError(mStatus == Status::Broken);
}
}
// vim: et sw=4:
......@@ -29,15 +29,17 @@ class FileResource : public ResourceType
Q_OBJECT
public:
/** Current status of resource. */
// IF YOU ALTER THE ORDER OF THIS ENUM, ENSURE THAT ALL VALUES WHICH INDICATE AN
// UNUSABLE RESOURCE ARE >= 'Unusable'.
// IF YOU ALTER THE ORDER OF THIS ENUM, ENSURE THAT:
// - ALL VALUES WHICH INDICATE AN UNUSABLE RESOURCE ARE >= 'Broken'.
// - ALL VALUES WHICH INDICATE AN RESOURCE WHICH CANNOT BECOME USABLE ARE > 'Unusable'.
enum class Status
{
Ready, // the resource is ready to use
Loading, // the resource is loading, and will be ready soon
Saving, // the resource is saving, and will be ready soon
Broken, // the resource is in error
Unusable, // ... values greater than this indicate an unusable resource
// Values >= Unusable indicate an unusable resource:
Unusable, // the resource has not yet initialised
Closed, // the resource has been closed. (Closed resources cannot be reopened.)
NotConfigured // the resource lacks necessary configuration
};
......@@ -49,7 +51,7 @@ public:
~FileResource() override;
/** Return whether the resource has a valid configuration. */
/** Return whether the resource has a valid configuration and is not closed. */
bool isValid() const override;
/** Return the resource's unique ID, as shown to the user. */
......@@ -380,6 +382,11 @@ protected:
*/
virtual void handleSettingsChange(Changes&);
/** Set the new status of the resource.
* If the resource status is already Unusable, it cannot be set usable again.
*/
void setStatus(Status);
FileResourceSettings* mSettings; // the resource's configuration
int mVersion {KACalendar::IncompatibleFormat}; // the calendar format version
KACalendar::Compat mCompatibility {KACalendar::Incompatible}; // whether resource is in compatible format
......@@ -388,10 +395,11 @@ protected:
typedef QHash<const QString&, KACalendar::Compat> CompatibilityMap; // indexed by event ID
CompatibilityMap mCompatibilityMap; // whether individual events are in compatible format
*/
Status mStatus {Status::NotConfigured}; // current status of resource
private:
void handleEnabledChange(Changes changes, CalEvent::Types oldEnabled);
Status mStatus {Status::Unusable}; // current status of resource
};
......
......@@ -54,6 +54,11 @@ bool Resource::failed() const
return mResource.isNull() ? true : mResource->failed();
}
bool Resource::inError() const
{
return mResource.isNull() ? true : mResource->inError();
}
ResourceId Resource::id() const
{
return mResource.isNull() ? -1 : mResource->id();
......
......@@ -57,6 +57,11 @@ public:
*/
bool failed() const;
/** Return whether the resource has an error (fatal or non-fatal), and
* cannot currently be used. This will be true if failed() is true.
*/
bool inError() const;
/** Return the resource's unique ID. */
ResourceId id() const;
......
......@@ -394,7 +394,7 @@ void Resources::notifyResourcesCreated()
}
/******************************************************************************
* Called when a resource's events have been loaded.
* Called when a resource's events have been loaded, or when loading fails.
* Emits a signal if all collections have been populated.
*/
void Resources::notifyResourcePopulated(const ResourceType* res)
......@@ -402,7 +402,7 @@ void Resources::notifyResourcePopulated(const ResourceType* res)
if (res)
{
Resource r = resource(res->id());
if (r.isValid())
if (r.isValid() && !r.inError())
Q_EMIT instance()->resourcePopulated(r);
}
......@@ -608,17 +608,18 @@ void Resources::removeResource(ResourceId id)
/******************************************************************************
* To be called when a resource has been created or loaded.
* If all resources have now loaded for the first time, emit signal.
* If all resources have now loaded for the first time, or cannot currently be
* loaded, emit signal.
*/
void Resources::checkResourcesPopulated()
{
if (!mPopulated && mCreated)
{
// Check whether all resources have now loaded at least once.
// Check whether all resources have now loaded at least once, or cannot currently be loaded.
for (auto it = mResources.constBegin(); it != mResources.constEnd(); ++it)
{
const Resource& res = it.value();
if (res.isEnabled(CalEvent::EMPTY) && !res.isPopulated())
if (res.isEnabled(CalEvent::EMPTY) && !res.inError() && !res.isPopulated())
return;
}
mPopulated = true;
......
......@@ -170,7 +170,8 @@ public:
* been created. */
static void notifyResourcesCreated();
/** Called by a resource to notify that loading of events has successfully completed. */
/** Called by a resource to notify that loading of events has successfully completed,
* or that loading has failed. */
static void notifyResourcePopulated(const ResourceType*);
/** Called to notify that a resource is about to be removed. */
......@@ -223,7 +224,8 @@ Q_SIGNALS:
void resourcesCreated();
/** Emitted when all configured and migrated resources have been loaded for
* the first time. This is always emitted after resourcesCreated().
* the first time, or cannot currently be loaded.
* This is always emitted after resourcesCreated().
*/
void resourcesPopulated();
......
......@@ -30,6 +30,11 @@ bool ResourceType::failed() const
return mFailed || !isValid();
}
bool ResourceType::inError() const
{
return mInError || failed();
}
ResourceId ResourceType::displayId() const
{
return id();
......@@ -318,8 +323,7 @@ void ResourceType::setLoaded(bool loaded) const
if (loaded != mLoaded)
{
mLoaded = loaded;
if (loaded)
Resources::notifyResourcePopulated(this);
Resources::notifyResourcePopulated(this);
}
}
......@@ -328,6 +332,11 @@ void ResourceType::setFailed()
mFailed = true;
}
void ResourceType::setError(bool error)
{
mInError = error;
}
QString ResourceType::storageTypeString(StorageType type)
{
switch (type)
......
......@@ -90,6 +90,11 @@ public:
*/
bool failed() const;
/** Return whether the resource has an error (fatal or non-fatal), and
* cannot currently be used. This will be true if failed() is true.
*/
bool inError() const;
/** Return the resource's unique ID. */
ResourceId id() const { return mId; }
......@@ -457,6 +462,11 @@ protected:
*/
void setFailed();
/** To be called if the resource has encountered a non-fatal error.
* A non-fatal error is one that can be recovered from.
*/
void setError(bool error);
static QString storageTypeStr(bool description, bool file, bool local);
template <class T> static T* resource(Resource&);
......@@ -471,6 +481,7 @@ private:
QList<KAEvent> mEventsUpdated; // events updated in mEvents but not yet notified
ResourceId mId {-1}; // resource's ID, which can't be changed
bool mFailed {false}; // the resource has a fatal error
bool mInError {false}; // the resource has a non-fatal error
mutable bool mLoaded {false}; // the resource has finished loading
bool mBeingDeleted {false}; // the resource is currently being deleted
};
......
......@@ -64,9 +64,7 @@ SingleFileResource::SingleFileResource(FileResourceSettings* settings)
, mSaveTimer(new QTimer(this))
{
qCDebug(KALARM_LOG) << "SingleFileResource: Starting" << displayName();
if (!load())
setFailed();
else
if (load())
{
// Monitor local file changes (but not cache files).
connect(KDirWatch::self(), &KDirWatch::dirty, this, &SingleFileResource::localFileChanged);
......@@ -210,7 +208,7 @@ int SingleFileResource::doLoad(QHash<QString, KAEvent>& newEvents, bool readThro
// The resource's location should never change, so this code should
// never be reached!
qCWarning(KALARM_LOG) << "SingleFileResource::load:" << displayId() << "Error? File location changed to" << localFileName;
setLoadFailure(true);
setLoadFailure(true, Status::NotConfigured);
mFileStorage.clear();
mCalendar.clear();
mLoadedEvents.clear();
......@@ -231,9 +229,8 @@ int SingleFileResource::doLoad(QHash<QString, KAEvent>& newEvents, bool readThro
const QString path = mSettings->displayLocation();
qCWarning(KALARM_LOG) << "SingleFileResource::load:" << displayId() << "Could not create file" << path;
errorMessage = xi18nc("@info", "Could not create calendar file <filename>%1</filename>.", path);
mStatus = Status::Broken;
mSaveUrl.clear();
setLoadFailure(false);
setLoadFailure(false, Status::Broken);
return -1;
}
// Check whether this user can actually write to the newly created file.
......@@ -246,13 +243,12 @@ int SingleFileResource::doLoad(QHash<QString, KAEvent>& newEvents, bool readThro
const QString path = mSettings->displayLocation();
qCWarning(KALARM_LOG) << "SingleFileResource::load:" << displayId() << "Could not create writable file" << path;
errorMessage = xi18nc("@info", "Could not create calendar file <filename>%1</filename>.", path);
mStatus = Status::Broken;
mSaveUrl.clear();
setLoadFailure(false);
setLoadFailure(false, Status::Broken);
return -1;
}
mFileReadOnly = false;
mStatus = Status::Ready;
setStatus(Status::Ready);
}
else
mFileReadOnly = !QFileInfo(localFileName).isWritable();
......@@ -274,7 +270,7 @@ int SingleFileResource::doLoad(QHash<QString, KAEvent>& newEvents, bool readThro
connect(mDownloadJob, &KJob::result,
this, &SingleFileResource::slotDownloadJobResult);
// connect(mDownloadJob, SIGNAL(percent(KJob*,ulong)), SLOT(handleProgress(KJob*,ulong)));
mStatus = Status::Loading;
setStatus(Status::Loading);
return 0; // loading initiated
}
......@@ -287,8 +283,7 @@ int SingleFileResource::doLoad(QHash<QString, KAEvent>& newEvents, bool readThro
{
qCWarning(KALARM_LOG) << "SingleFileResource::load:" << displayId() << "Could not read file" << localFileName;
// A user error message has been set by readLocalFile().
mStatus = Status::Broken;
setLoadFailure(true);
setLoadFailure(true, Status::Broken);
return -1;
}
......@@ -296,7 +291,7 @@ int SingleFileResource::doLoad(QHash<QString, KAEvent>& newEvents, bool readThro
KDirWatch::self()->addFile(settingsLocalFileName);
newEvents = mLoadedEvents;
mStatus = Status::Ready;
setStatus(Status::Ready);
return 1; // success
}
......@@ -305,8 +300,9 @@ int SingleFileResource::doLoad(QHash<QString, KAEvent>& newEvents, bool readThro
* If the resource file doesn't exist or can't be created, the resource is still
* regarded as loaded.
*/
void SingleFileResource::setLoadFailure(bool exists)
void SingleFileResource::setLoadFailure(bool exists, Status newStatus)
{
setStatus(newStatus);
mLoadedEvents.clear();
QHash<QString, KAEvent> events;
setLoadedEvents(events);
......@@ -331,7 +327,7 @@ int SingleFileResource::doSave(bool writeThroughCache, bool force, QString& erro
if (mSaveUrl.isEmpty())
{
qCWarning(KALARM_LOG) << "SingleFileResource::save:" << displayId() << "No file specified";
mStatus = Status::Broken;
setStatus(Status::Broken);
return -1;
}
......@@ -379,7 +375,7 @@ int SingleFileResource::doSave(bool writeThroughCache, bool force, QString& erro
{
qCWarning(KALARM_LOG) << "SingleFileResource::save:" << displayId() << "Error writing to file" << localFileName;
// A user error message has been set by writeToFile().
mStatus = Status::Broken;
setStatus(Status::Broken);
return -1;
}
......@@ -393,11 +389,11 @@ int SingleFileResource::doSave(bool writeThroughCache, bool force, QString& erro
connect(mUploadJob, &KJob::result,
this, &SingleFileResource::slotUploadJobResult);
// connect(mUploadJob, SIGNAL(percent(KJob*,ulong)), SLOT(handleProgress(KJob*,ulong)));
mStatus = Status::Saving;
setStatus(Status::Saving);
return 0;
}
mStatus = Status::Ready;
setStatus(Status::Ready);
return 1;
}
......@@ -438,7 +434,7 @@ void SingleFileResource::close()
KDirWatch::self()->removeFile(mSettings->url().toLocalFile());
mCalendar.clear();
mFileStorage.clear();
mStatus = Status::Closed;
setStatus(Status::Closed);
}
/******************************************************************************
......@@ -740,9 +736,7 @@ void SingleFileResource::slotDownloadJobResult(KJob* job)
QString errorMessage;
if (job->error() && job->error() != KIO::ERR_DOES_NOT_EXIST)
{
if (mStatus != Status::Closed)
mStatus = Status::Broken;
setLoadFailure(false);
setLoadFailure(false, Status::Broken);
const QString path = displayLocation();
qCWarning(KALARM_LOG) << "SingleFileResource::slotDownloadJobResult:" << displayId() << "Could not load file" << path << job->errorString();
errorMessage = xi18nc("@info", "Could not load file <filename>%1</filename>. (%2)", path, job->errorString());
......@@ -755,13 +749,11 @@ void SingleFileResource::slotDownloadJobResult(KJob* job)
{
qCWarning(KALARM_LOG) << "SingleFileResource::slotDownloadJobResult:" << displayId() << "Could not load local file" << localFileName;
// A user error message has been set by readLocalFile().
if (mStatus != Status::Closed)
mStatus = Status::Broken;
setLoadFailure(true);
setLoadFailure(true, Status::Broken);
success = false;
}
else if (mStatus != Status::Closed)
mStatus = Status::Ready;
else
setStatus(Status::Ready);
}
mDownloadJob = nullptr;
......@@ -781,15 +773,14 @@ void SingleFileResource::slotUploadJobResult(KJob* job)
bool success = true;
if (job->error())
{
if (mStatus != Status::Closed)
mStatus = Status::Broken;
setStatus(Status::Broken);
const QString path = displayLocation();
qCWarning(KALARM_LOG) << "SingleFileResource::slotDownloadJobResult:" << displayId() << "Could not save file" << path << job->errorString();
errorMessage = xi18nc("@info", "Could not save file <filename>%1</filename>. (%2)", path, job->errorString());
success = false;
}
else if (mStatus != Status::Closed)
mStatus = Status::Ready;
else
setStatus(Status::Ready);
mUploadJob = nullptr;
auto ref = job->property("QEventLoopLocker").value<QEventLoopLocker*>();
......
......@@ -201,7 +201,7 @@ private Q_SLOTS:
bool addLoadedEvent(const KCalendarCore::Event::Ptr&);
private:
void setLoadFailure(bool exists);
void setLoadFailure(bool exists, Status);
QUrl mSaveUrl; // current local file for save() to use (may be temporary)
KIO::FileCopyJob* mDownloadJob {nullptr};
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment