diff --git a/autotests/jobremotetest.h b/autotests/jobremotetest.h --- a/autotests/jobremotetest.h +++ b/autotests/jobremotetest.h @@ -45,6 +45,7 @@ void putAndGet(); void openFileWriting(); void openFileReading(); + void openFileRead0Bytes(); //void calculateRemainingSeconds(); @@ -74,11 +75,17 @@ void slotFileJob2Position(KIO::Job *job, KIO::filesize_t offset); void slotFileJob2Close(KIO::Job *job); + void slotFileJob3Open(KIO::Job *job); + void slotFileJob3Position(KIO::Job *job, KIO::filesize_t offset); + void slotFileJob3Data(KIO::Job *job, const QByteArray &data); + void slotFileJob3Close(KIO::Job *job); + private: void enterLoop(); enum { AlreadyExists = 1 }; int m_result; + bool m_closeSignalCalled; QByteArray m_data; QStringList m_names; int m_dataReqCount; diff --git a/autotests/jobremotetest.cpp b/autotests/jobremotetest.cpp --- a/autotests/jobremotetest.cpp +++ b/autotests/jobremotetest.cpp @@ -206,9 +206,10 @@ this, &JobRemoteTest::slotFileJobWritten); connect(fileJob, &KIO::FileJob::position, this, &JobRemoteTest::slotFileJobPosition); - connect(fileJob, SIGNAL(close(KIO::Job*)), - this, SLOT(slotFileJobClose(KIO::Job*))); + connect(fileJob, QOverload::of(&KIO::FileJob::close), + this, &JobRemoteTest::slotFileJobClose); m_result = -1; + m_closeSignalCalled = false; enterLoop(); QEXPECT_FAIL("", "Needs fixing in kio_file", Abort); @@ -220,6 +221,7 @@ this, &JobRemoteTest::slotGetResult); enterLoop(); QCOMPARE(m_result, 0); // no error + QVERIFY(m_closeSignalCalled); // close signal called. qDebug() << "m_data: " << m_data; QCOMPARE(m_data, QByteArray("test....test....test....test....test....test....end")); @@ -273,6 +275,7 @@ void JobRemoteTest::slotFileJobClose(KIO::Job *job) { Q_UNUSED(job); + m_closeSignalCalled = true; qDebug() << "+++++++++ closed"; } @@ -316,12 +319,14 @@ this, &JobRemoteTest::slotFileJob2Written); connect(fileJob, &KIO::FileJob::position, this, &JobRemoteTest::slotFileJob2Position); - connect(fileJob, SIGNAL(close(KIO::Job*)), - this, SLOT(slotFileJob2Close(KIO::Job*))); + connect(fileJob, QOverload::of(&KIO::FileJob::close), + this, &JobRemoteTest::slotFileJob2Close); m_result = -1; + m_closeSignalCalled = false; enterLoop(); QVERIFY(m_result == 0); // no error + QVERIFY(m_closeSignalCalled); // close signal called. qDebug() << "resulting m_data: " << QString(m_data); QCOMPARE(m_data, QByteArray("test5test4test3test2test1")); @@ -374,6 +379,7 @@ void JobRemoteTest::slotFileJob2Close(KIO::Job *job) { Q_UNUSED(job); + m_closeSignalCalled = true; qDebug() << "+++++++++ job2 closed"; } @@ -385,3 +391,79 @@ m_mimetype = type; } +void JobRemoteTest::openFileRead0Bytes() +{ + QUrl u(remoteTmpUrl()); + u.setPath(u.path() + "openFileReading"); + + const QByteArray putData("Doesn't matter"); + + KIO::StoredTransferJob *putJob = KIO::storedPut(putData, + u, + 0600, KIO::Overwrite | KIO::HideProgressInfo + ); + + quint64 secsSinceEpoch = QDateTime::currentSecsSinceEpoch(); // Use second granularity, supported on all filesystems + QDateTime mtime = QDateTime::fromSecsSinceEpoch(secsSinceEpoch - 30); // 30 seconds ago + putJob->setModificationTime(mtime); + putJob->setUiDelegate(nullptr); + connect(putJob, &KJob::result, + this, &JobRemoteTest::slotResult); + m_result = -1; + enterLoop(); + QVERIFY(m_result == 0); // no error + + m_data = QByteArray(); + + fileJob = KIO::open(u, QIODevice::ReadOnly); + + fileJob->setUiDelegate(nullptr); + connect(fileJob, &KJob::result, + this, &JobRemoteTest::slotResult); + connect(fileJob, &KIO::FileJob::data, + this, &JobRemoteTest::slotFileJob3Data); + connect(fileJob, &KIO::FileJob::open, + this, &JobRemoteTest::slotFileJob3Open); + // Can reuse this slot (it's a noop). + connect(fileJob, &KIO::FileJob::written, + this, &JobRemoteTest::slotFileJob2Written); + connect(fileJob, &KIO::FileJob::position, + this, &JobRemoteTest::slotFileJob3Position); + // Can reuse this as well. + connect(fileJob, QOverload::of(&KIO::FileJob::close), + this, &JobRemoteTest::slotFileJob3Close); + m_result = -1; + m_closeSignalCalled = false; + + enterLoop(); + // Previously reading 0 bytes would cause both data() and error() being emitted... + QVERIFY(m_result == 0); // no error + QVERIFY(m_closeSignalCalled); // close signal called. +} + +void JobRemoteTest::slotFileJob3Open(KIO::Job *job) +{ + Q_UNUSED(job); + fileJob->seek(0); +} + +void JobRemoteTest::slotFileJob3Position(KIO::Job *job, KIO::filesize_t offset) +{ + Q_UNUSED(job); + qDebug() << "position : " << offset << " -> read (0)"; + fileJob->read(0); +} + +void JobRemoteTest::slotFileJob3Data(KIO::Job *job, const QByteArray& data) +{ + Q_UNUSED(job); + QVERIFY(data.isEmpty()); + fileJob->close(); +} + +void JobRemoteTest::slotFileJob3Close(KIO::Job *job) +{ + Q_UNUSED(job); + m_closeSignalCalled = true; + qDebug() << "+++++++++ job2 closed"; +} diff --git a/src/core/filejob.h b/src/core/filejob.h --- a/src/core/filejob.h +++ b/src/core/filejob.h @@ -45,31 +45,55 @@ ~FileJob(); /** - * Read block + * This function attempts to read up to \p size bytes from the URL passed to + * KIO::open() and returns the bytes received via the data() signal. + * + * The read operation commences at the current file offset, and the file + * offset is incremented by the number of bytes read, but this change in the + * offset does not result in the position() signal being emitted. + * + * If the current file offset is at or past the end of file (i.e. EOD), no + * bytes are read, and the data() signal returns an empty QByteArray. + * + * On error the data() signal is not emitted. To catch errors please connect + * to the result() signal. + * + * @param size the requested amount of data to read * - * The slave emits the data through data(). - * @param size the requested amount of data */ void read(KIO::filesize_t size); /** - * Write block + * This function attempts to write all the bytes in \p data to the URL + * passed to KIO::open() and returns the bytes written received via the + * written() signal. + * + * The write operation commences at the current file offset, and the file + * offset is incremented by the number of bytes read, but this change in the + * offset does not result in the position() being emitted. + * + * On error the written() signal is not emitted. To catch errors please + * connect to the result() signal. * * @param data the data to write */ void write(const QByteArray &data); /** - * Close - * * Closes the file-slave + * + * The slave emits close() and result(). */ void close(); /** * Seek * - * The slave emits position() + * The slave emits position() on successful seek to the specified \p offset. + * + * On error the position() signal is not emitted. To catch errors please + * connect to the result() signal. + * * @param offset the position from start to go to */ void seek(KIO::filesize_t offset); @@ -83,9 +107,14 @@ Q_SIGNALS: /** - * Data from the slave has arrived. + * Data from the slave has arrived. Emitted after read(). + * + * Unless a read() request was sent for 0 bytes, End of data (EOD) has been + * reached if data.size() == 0 + * * @param job the job that emitted this signal * @param data data received from the slave. + * */ void data(KIO::Job *job, const QByteArray &data); @@ -112,7 +141,7 @@ void open(KIO::Job *job); /** - * Bytes written to the file. + * \p written bytes were written to the file. Emitted after write(). * @param job the job that emitted this signal * @param written bytes written. */ @@ -125,7 +154,7 @@ void close(KIO::Job *job); /** - * The file has reached this position. Emitted after seek. + * The file has reached this position. Emitted after seek(). * @param job the job that emitted this signal * @param offset the new position */ @@ -151,6 +180,10 @@ * Open ( random access I/O ) * * The file-job emits open() when opened + * + * On error the open() signal is not emitted. To catch errors please + * connect to the result() signal. + * * @param url the URL of the file * @param mode the access privileges: see \ref OpenMode * diff --git a/src/core/filejob.cpp b/src/core/filejob.cpp --- a/src/core/filejob.cpp +++ b/src/core/filejob.cpp @@ -180,10 +180,11 @@ { Q_Q(FileJob); //qDebug() << this << m_url; + m_open = false; emit q->close(q); // Return slave to the scheduler slaveDone(); -// Scheduler::doJob(this); + // Scheduler::doJob(this); q->emitResult(); } @@ -202,6 +203,9 @@ q->connect(slave, SIGNAL(open()), SLOT(slotOpen())); + q->connect(slave, SIGNAL(finished()), + SLOT(slotFinished())); + q->connect(slave, SIGNAL(position(KIO::filesize_t)), SLOT(slotPosition(KIO::filesize_t))); diff --git a/src/core/slavebase.h b/src/core/slavebase.h --- a/src/core/slavebase.h +++ b/src/core/slavebase.h @@ -97,7 +97,7 @@ /** * open succeeds - * @see open + * @see open() */ void opened(); @@ -418,9 +418,28 @@ */ virtual void open(const QUrl &url, QIODevice::OpenMode mode); + /** + * read. + * @param size the requested amount of data to read + * @see KIO::FileJob::read() + */ virtual void read(KIO::filesize_t size); + /** + * write. + * @param data the data to write + * @see KIO::FileJob::write() + */ virtual void write(const QByteArray &data); + /** + * seek. + * @param offset the requested amount of data to read + * @see KIO::FileJob::read() + */ virtual void seek(KIO::filesize_t offset); + /** + * close. + * @see KIO::FileJob::close() + */ virtual void close(); /** diff --git a/src/core/slaveinterface.h b/src/core/slaveinterface.h --- a/src/core/slaveinterface.h +++ b/src/core/slaveinterface.h @@ -146,6 +146,7 @@ void open(); void written(KIO::filesize_t); + void close(); void privilegeOperationRequested(); diff --git a/src/ioslaves/file/file.h b/src/ioslaves/file/file.h --- a/src/ioslaves/file/file.h +++ b/src/ioslaves/file/file.h @@ -113,6 +113,9 @@ void redirect(const QUrl &url); + // Close without calling finish(). Use this to close after error. + void closeWithoutFinish(); + private: mutable QHash mUsercache; mutable QHash mGroupcache; diff --git a/src/ioslaves/file/file.cpp b/src/ioslaves/file/file.cpp --- a/src/ioslaves/file/file.cpp +++ b/src/ioslaves/file/file.cpp @@ -515,43 +515,38 @@ // qDebug() << "File::open -- read"; Q_ASSERT(mFile && mFile->isOpen()); - while (true) { - QByteArray res = mFile->read(bytes); + QVarLengthArray buffer(bytes); - if (!res.isEmpty()) { - data(res); - bytes -= res.size(); - } else { - // empty array designates eof - data(QByteArray()); - if (!mFile->atEnd()) { - error(KIO::ERR_CANNOT_READ, mFile->fileName()); - close(); - } - break; - } - if (bytes <= 0) { - break; - } + qint64 bytesRead = mFile->read(buffer.data(), bytes); + + if (bytesRead == -1) { + qCWarning(KIO_FILE) << "Couldn't read. Error:" << mFile->errorString(); + error(KIO::ERR_CANNOT_READ, mFile->fileName()); + closeWithoutFinish(); + } else { + const QByteArray fileData = QByteArray::fromRawData(buffer.data(), bytesRead); + data(fileData); } } void FileProtocol::write(const QByteArray &data) { // qDebug() << "File::open -- write"; Q_ASSERT(mFile && mFile->isWritable()); - if (mFile->write(data) != data.size()) { + qint64 bytesWritten = mFile->write(data); + + if (bytesWritten == -1) { if (mFile->error() == QFileDevice::ResourceError) { // disk full error(KIO::ERR_DISK_FULL, mFile->fileName()); - close(); + closeWithoutFinish(); } else { qCWarning(KIO_FILE) << "Couldn't write. Error:" << mFile->errorString(); error(KIO::ERR_CANNOT_WRITE, mFile->fileName()); - close(); + closeWithoutFinish(); } } else { - written(data.size()); + written(bytesWritten); } } @@ -564,18 +559,22 @@ position(offset); } else { error(KIO::ERR_CANNOT_SEEK, mFile->fileName()); - close(); + closeWithoutFinish(); } } -void FileProtocol::close() +void FileProtocol::closeWithoutFinish() { - // qDebug() << "File::open -- close "; Q_ASSERT(mFile); delete mFile; mFile = nullptr; +} +void FileProtocol::close() +{ + // qDebug() << "File::open -- close "; + closeWithoutFinish(); finished(); }