Skip to content

Commit

Permalink
Make job state private
Browse files Browse the repository at this point in the history
Now getter and setter methods are used to access the state, which makes
debugging state changes easier to debug.
  • Loading branch information
erikjv authored and TheOneRing committed Aug 30, 2024
1 parent d57c51b commit 6a3c09e
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 27 deletions.
51 changes: 28 additions & 23 deletions src/libsync/owncloudpropagator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,12 @@ PropagateItemJob::~PropagateItemJob()

bool PropagateItemJob::scheduleSelfOrChild()
{
if (_state != NotYetStarted) {
if (state() != NotYetStarted) {
return false;
}
qCInfo(lcPropagator) << "Starting propagation of" << _item << "by" << this;

_state = Running;
setState(Running);
if (thread() != QApplication::instance()->thread()) {
QMetaObject::invokeMethod(this, &PropagateItemJob::start); // We could be in a different thread (neon jobs)
} else {
Expand Down Expand Up @@ -227,8 +227,8 @@ static void blacklistUpdate(SyncJournalDb *journal, SyncFileItem &item)
void PropagateItemJob::done(SyncFileItem::Status statusArg, const QString &errorString)
{
// Duplicate calls to done() are a logic error
OC_ENFORCE(_state != Finished);
_state = Finished;
OC_ENFORCE(state() != Finished);
setState(Finished);

_item->_status = statusArg;

Expand Down Expand Up @@ -857,11 +857,16 @@ Result<Vfs::ConvertToPlaceholderResult, QString> OwncloudPropagator::updateMetad

PropagatorJob::PropagatorJob(OwncloudPropagator *propagator, const QString &path)
: QObject(propagator)
, _state(NotYetStarted)
, _path(path)
, _jobState(NotYetStarted)
{
}

void PropagatorJob::setState(JobState state)
{
_jobState = state;
}

OwncloudPropagator *PropagatorJob::propagator() const
{
return qobject_cast<OwncloudPropagator *>(parent());
Expand Down Expand Up @@ -899,18 +904,18 @@ void PropagatorCompositeJob::appendJob(PropagatorJob *job)

bool PropagatorCompositeJob::scheduleSelfOrChild()
{
if (_state == Finished) {
if (state() == Finished) {
return false;
}

// Start the composite job
if (_state == NotYetStarted) {
_state = Running;
if (state() == NotYetStarted) {
setState(Running);
}

// Ask all the running composite jobs if they have something new to schedule.
for (int i = 0; i < _runningJobs.size(); ++i) {
OC_ASSERT(_runningJobs.at(i)->_state == Running);
OC_ASSERT(_runningJobs.at(i)->state() == Running);

if (possiblyRunNextJob(_runningJobs.at(i))) {
return true;
Expand Down Expand Up @@ -993,10 +998,10 @@ void PropagatorCompositeJob::finalize()
{
// The propagator will do parallel scheduling and this could be posted
// multiple times on the event loop, ignore the duplicate calls.
if (_state == Finished)
if (state() == Finished)
return;

_state = Finished;
setState(Finished);
Q_EMIT finished(_errorPaths.empty() ? SyncFileItem::Success : _errorPaths.last());
}

Expand Down Expand Up @@ -1043,19 +1048,19 @@ PropagatorJob::JobParallelism PropagateDirectory::parallelism()

bool PropagateDirectory::scheduleSelfOrChild()
{
if (_state == Finished) {
if (state() == Finished) {
return false;
}

if (_state == NotYetStarted) {
_state = Running;
if (state() == NotYetStarted) {
setState(Running);
}

if (_firstJob && _firstJob->_state == NotYetStarted) {
if (_firstJob && _firstJob->state() == NotYetStarted) {
return _firstJob->scheduleSelfOrChild();
}

if (_firstJob && _firstJob->_state == Running) {
if (_firstJob && _firstJob->state() == Running) {
// Don't schedule any more job until this is done.
return false;
}
Expand All @@ -1077,7 +1082,7 @@ void PropagateDirectory::slotFirstJobFinished(SyncFileItem::Status status)
break;
// handle all other cases as errors
default:
if (_state != Finished) {
if (state() != Finished) {
// Synchronously abort
abort(AbortType::Synchronous);
done(status);
Expand Down Expand Up @@ -1139,8 +1144,8 @@ void PropagateDirectory::slotSubJobsFinished(const SyncFileItem::Status status)

// don't call done, we only propagate the state of the child items
// and we don't want error handling for this folder for an error that happend on a child
Q_ASSERT(_state != Finished);
_state = Finished;
Q_ASSERT(state() != Finished);
setState(Finished);
Q_EMIT finished(status);
if (_item->_relevantDirectoyInstruction) {
Q_EMIT propagator()->itemCompleted(_item);
Expand Down Expand Up @@ -1201,7 +1206,7 @@ qint64 PropagateRootDirectory::committedDiskSpace() const

bool PropagateRootDirectory::scheduleSelfOrChild()
{
if (_state == Finished) {
if (state() == Finished) {
return false;
}

Expand All @@ -1210,7 +1215,7 @@ bool PropagateRootDirectory::scheduleSelfOrChild()
}

// Important: Finish _subJobs before scheduling any deletes.
if (_subJobs._state != Finished) {
if (_subJobs.state() != Finished) {
return false;
}

Expand All @@ -1230,7 +1235,7 @@ void PropagateRootDirectory::slotSubJobsFinished(SyncFileItem::Status status)
// handle all other cases as errors
default:
_status = status;
if (_state != Finished) {
if (state() != Finished) {
auto subJob = qobject_cast<PropagatorCompositeJob *>(sender());
if (OC_ENSURE(subJob)) {
const QMap<QString, SyncFileItem::Status> errorPaths = subJob->errorPaths();
Expand All @@ -1255,7 +1260,7 @@ void PropagateRootDirectory::slotSubJobsFinished(SyncFileItem::Status status)

void PropagateRootDirectory::slotDirDeletionJobsFinished(SyncFileItem::Status status)
{
_state = Finished;
setState(Finished);
Q_EMIT finished(_status != SyncFileItem::NoStatus ? _status : status);
}

Expand Down
6 changes: 4 additions & 2 deletions src/libsync/owncloudpropagator.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ class PropagatorJob : public QObject
};
Q_ENUM(JobState)

JobState _state;
JobState state() const { return _jobState; }
void setState(JobState state);

enum JobParallelism {

Expand Down Expand Up @@ -155,6 +156,7 @@ public Q_SLOTS:

private:
QString _path;
JobState _jobState;
};

/*
Expand Down Expand Up @@ -241,7 +243,7 @@ private Q_SLOTS:
void slotSubJobAbortFinished();
bool possiblyRunNextJob(PropagatorJob *next)
{
if (next->_state == NotYetStarted) {
if (next->state() == NotYetStarted) {
connect(next, &PropagatorJob::finished, this, &PropagatorCompositeJob::slotSubJobFinished);
}
return next->scheduleSelfOrChild();
Expand Down
4 changes: 2 additions & 2 deletions src/libsync/propagatedownload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ void PropagateDownloadFile::start()
deleteExistingFolder();

// check for error with deletion
if (_state == Finished) {
if (state() == Finished) {
return;
}
}
Expand Down Expand Up @@ -657,7 +657,7 @@ void PropagateDownloadFile::startFullDownload()

qint64 PropagateDownloadFile::committedDiskSpace() const
{
if (_state == Running) {
if (state() == Running) {
return qBound(0LL, _item->_size - _resumeStart - _downloadProgress, _item->_size);
}
return 0;
Expand Down
1 change: 1 addition & 0 deletions src/libsync/propagateupload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,7 @@ QMap<QByteArray, QByteArray> PropagateUploadFileCommon::headers()

void PropagateUploadFileCommon::finalize()
{
OC_ENFORCE(state() != Finished);
// Update the quota, if known
if (!_quotaUpdated) {
auto quotaIt = propagator()->_folderQuota.find(QFileInfo(_item->_file).path());
Expand Down

0 comments on commit 6a3c09e

Please sign in to comment.