Merge branch 'uploadImprovements' into clientSideEncryptionV3

This commit is contained in:
Tomaz Canabrava 2017-12-03 18:15:33 +01:00
commit 8c4928d9f6
5 changed files with 86 additions and 35 deletions

View file

@ -437,6 +437,7 @@ public:
*/
bool hasCaseClashAccessibilityProblem(const QString &relfile);
/* returns the local file path for the given tmp_file_name */
QString getFilePath(const QString &tmp_file_name) const;
PropagateItemJob *createJob(const SyncFileItemPtr &item);

View file

@ -163,31 +163,41 @@ void PropagateUploadFileCommon::setDeleteExisting(bool enabled)
_deleteExisting = enabled;
}
void PropagateUploadFileCommon::start()
{
qDebug() << "Starting to upload a file.";
qDebug() << _item->_file;
qDebug() << _item->_fileId;
/* Currently the File to upload has the same information as the SyncFileItem,
* but the idea is to make a separation between the local file and the file that's
* being uploaded, in our current case they are the same, but perhaps we can apply
* some filters in the future like 'resize' for pictures and so on.
*
* this by no means is a finished job, but a first step.
*/
_fileToUpload._file = _item->_file;
_fileToUpload._size = _item->_size;
_fileToUpload._path = propagator()->getFilePath(_fileToUpload._file);
if (propagator()->_abortRequested.fetchAndAddRelaxed(0)) {
return;
}
// Check if the specific file can be accessed
if (propagator()->hasCaseClashAccessibilityProblem(_item->_file)) {
if (propagator()->hasCaseClashAccessibilityProblem(_fileToUpload._file)) {
done(SyncFileItem::NormalError, tr("File %1 cannot be uploaded because another file with the same name, differing only in case, exists").arg(QDir::toNativeSeparators(_item->_file)));
return;
}
// Check if we believe that the upload will fail due to remote quota limits
const quint64 quotaGuess = propagator()->_folderQuota.value(
QFileInfo(_item->_file).path(), std::numeric_limits<quint64>::max());
if (_item->_size > quotaGuess) {
QFileInfo(_fileToUpload._file).path(), std::numeric_limits<quint64>::max());
if (_fileToUpload._size > quotaGuess) {
// Necessary for blacklisting logic
_item->_httpErrorCode = 507;
emit propagator()->insufficientRemoteStorage();
done(SyncFileItem::DetailError, tr("Upload of %1 exceeds the quota for the folder").arg(Utility::octetsToString(_item->_size)));
done(SyncFileItem::DetailError, tr("Upload of %1 exceeds the quota for the folder").arg(Utility::octetsToString(_fileToUpload._size)));
return;
}
@ -198,7 +208,7 @@ void PropagateUploadFileCommon::start()
}
auto job = new DeleteJob(propagator()->account(),
propagator()->_remoteFolder + _item->_file,
propagator()->_remoteFolder + _fileToUpload._file,
this);
_jobs.append(job);
connect(job, &DeleteJob::finishedSignal, this, &PropagateUploadFileCommon::slotComputeContentChecksum);
@ -212,10 +222,12 @@ void PropagateUploadFileCommon::slotComputeContentChecksum()
return;
}
const QString filePath = propagator()->getFilePath(_item->_file);
const QString filePath = propagator()->getFilePath(_fileToUpload._file);
// remember the modtime before checksumming to be able to detect a file
// change during the checksum calculation
// change during the checksum calculation - This goes inside of the _item->_file
// and not the _fileToUpload because we are checking the original file, not there
// probably temporary one.
_item->_modtime = FileSystem::getModTime(filePath);
#ifdef WITH_TESTING
@ -225,6 +237,9 @@ void PropagateUploadFileCommon::slotComputeContentChecksum()
QByteArray checksumType = contentChecksumType();
// Maybe the discovery already computed the checksum?
// Should I compute the checksum of the original (_item->_file)
// or the maybe-modified? (_fileToUpload._file) ?
QByteArray existingChecksumType, existingChecksum;
parseChecksumHeader(_item->_checksumHeader, &existingChecksumType, &existingChecksum);
if (existingChecksumType == checksumType) {
@ -289,7 +304,7 @@ void PropagateUploadFileCommon::slotStartUpload(const QByteArray &transmissionCh
_item->_checksumHeader = _transmissionChecksumHeader;
}
const QString fullFilePath = propagator()->getFilePath(_item->_file);
const QString fullFilePath = propagator()->getFilePath(_fileToUpload._file);
if (!FileSystem::fileExists(fullFilePath)) {
done(SyncFileItem::SoftError, tr("File Removed"));
@ -311,7 +326,7 @@ void PropagateUploadFileCommon::slotStartUpload(const QByteArray &transmissionCh
}
quint64 fileSize = FileSystem::getSize(fullFilePath);
_item->_size = fileSize;
_fileToUpload._size = fileSize;
// But skip the file if the mtime is too close to 'now'!
// That usually indicates a file that is still being changed
@ -540,17 +555,20 @@ void PropagateUploadFileCommon::commonErrorHandling(AbstractNetworkJob *job)
// Insufficient remote storage.
if (_item->_httpErrorCode == 507) {
// Update the quota expectation
/* store the quota for the real local file using the information
* on the file to upload, that could have been modified by
* filters or something. */
const auto path = QFileInfo(_item->_file).path();
auto quotaIt = propagator()->_folderQuota.find(path);
if (quotaIt != propagator()->_folderQuota.end()) {
quotaIt.value() = qMin(quotaIt.value(), _item->_size - 1);
quotaIt.value() = qMin(quotaIt.value(), _fileToUpload._size - 1);
} else {
propagator()->_folderQuota[path] = _item->_size - 1;
propagator()->_folderQuota[path] = _fileToUpload._size - 1;
}
// Set up the error
status = SyncFileItem::DetailError;
errorString = tr("Upload of %1 exceeds the quota for the folder").arg(Utility::octetsToString(_item->_size));
errorString = tr("Upload of %1 exceeds the quota for the folder").arg(Utility::octetsToString(_fileToUpload._size));
emit propagator()->insufficientRemoteStorage();
}
@ -619,9 +637,9 @@ void PropagateUploadFileCommon::finalize()
// Update the quota, if known
auto quotaIt = propagator()->_folderQuota.find(QFileInfo(_item->_file).path());
if (quotaIt != propagator()->_folderQuota.end())
quotaIt.value() -= _item->_size;
quotaIt.value() -= _fileToUpload._size;
// Update the database entry
// Update the database entry - use the local file, not the temporary one.
if (!propagator()->_journal->setFileRecord(_item->toSyncJournalFileRecordWithInode(propagator()->getFilePath(_item->_file)))) {
done(SyncFileItem::FatalError, tr("Error writing metadata to the database"));
return;

View file

@ -212,6 +212,20 @@ protected:
bool _deleteExisting BITFIELD(1);
quint64 _abortCount; /// Keep track of number of aborted items
/* This is a minified version of the SyncFileItem,
* that holds only the specifics about the file that's
* being uploaded.
*
* This is needed if we wanna apply changes on the file
* that's being uploaded while keeping the original on disk.
*/
struct UploadFileInfo {
QString _file; /// I'm still unsure if I should use a SyncFilePtr here.
QString _path; /// the full path on disk.
quint64 _size;
};
UploadFileInfo _fileToUpload;
// measure the performance of checksum calc and upload
#ifdef WITH_TESTING
Utility::StopWatch _stopWatch;

View file

@ -137,12 +137,12 @@ void PropagateUploadFileNG::slotPropfindFinished()
++_currentChunk;
}
if (_sent > _item->_size) {
if (_sent > _fileToUpload._size) {
// Normally this can't happen because the size is xor'ed with the transfer id, and it is
// therefore impossible that there is more data on the server than on the file.
qCCritical(lcPropagateUpload) << "Inconsistency while resuming " << _item->_file
<< ": the size on the server (" << _sent << ") is bigger than the size of the file ("
<< _item->_size << ")";
<< _fileToUpload._size << ")";
startNewUpload();
return;
}
@ -220,7 +220,7 @@ void PropagateUploadFileNG::slotDeleteJobFinished()
void PropagateUploadFileNG::startNewUpload()
{
ASSERT(propagator()->_activeJobList.count(this) == 1);
_transferId = qrand() ^ _item->_modtime ^ (_item->_size << 16) ^ qHash(_item->_file);
_transferId = qrand() ^ _item->_modtime ^ (_fileToUpload._size << 16) ^ qHash(_fileToUpload._file);
_sent = 0;
_currentChunk = 0;
@ -230,10 +230,14 @@ void PropagateUploadFileNG::startNewUpload()
pi._valid = true;
pi._transferid = _transferId;
pi._modtime = _item->_modtime;
// Journal should store the actual file
propagator()->_journal->setUploadInfo(_item->_file, pi);
propagator()->_journal->commit("Upload info");
QMap<QByteArray, QByteArray> headers;
headers["OC-Total-Length"] = QByteArray::number(_item->_size);
// But we should send the temporary (or something) one.
headers["OC-Total-Length"] = QByteArray::number(_fileToUpload._size);
auto job = new MkColJob(propagator()->account(), chunkUrl(), headers, this);
connect(job, SIGNAL(finished(QNetworkReply::NetworkError)),
@ -264,7 +268,7 @@ void PropagateUploadFileNG::startNextChunk()
if (propagator()->_abortRequested.fetchAndAddRelaxed(0))
return;
quint64 fileSize = _item->_size;
quint64 fileSize = _fileToUpload._size;
ENFORCE(fileSize >= _sent, "Sent data exceeds file size");
// prevent situation that chunk size is bigger then required one to send
@ -274,8 +278,9 @@ void PropagateUploadFileNG::startNextChunk()
Q_ASSERT(_jobs.isEmpty()); // There should be no running job anymore
_finished = true;
// Finish with a MOVE
// If we changed the file name, we must store the changed filename in the remote folder, not the original one.
QString destination = QDir::cleanPath(propagator()->account()->url().path() + QLatin1Char('/')
+ propagator()->account()->davPath() + propagator()->_remoteFolder + _item->_file);
+ propagator()->account()->davPath() + propagator()->_remoteFolder + _fileToUpload._file);
auto headers = PropagateUploadFileCommon::headers();
// "If-Match applies to the source, but we are interested in comparing the etag of the destination
@ -300,7 +305,7 @@ void PropagateUploadFileNG::startNextChunk()
}
auto device = new UploadDevice(&propagator()->_bandwidthManager);
const QString fileName = propagator()->getFilePath(_item->_file);
const QString fileName = _fileToUpload._path;
if (!device->prepareAndOpen(fileName, _sent, _currentChunkSize)) {
qCWarning(lcPropagateUpload) << "Could not prepare upload device: " << device->errorString();
@ -357,7 +362,7 @@ void PropagateUploadFileNG::slotPutFinished()
return;
}
ENFORCE(_sent <= _item->_size, "can't send more than size");
ENFORCE(_sent <= _fileToUpload._size, "can't send more than size");
// Adjust the chunk size for the time taken.
//
@ -390,11 +395,18 @@ void PropagateUploadFileNG::slotPutFinished()
<< propagator()->_chunkSize << "bytes";
}
bool finished = _sent == _item->_size;
bool finished = _sent == _fileToUpload._size;
// Check if the file still exists
/* Check if the file still exists,
* but we could be operating in a temporary file, so check both if
* the file to upload is different than the file on disk
*/
const QString fileToUploadPath = _fileToUpload._path;
const QString fullFilePath(propagator()->getFilePath(_item->_file));
if (!FileSystem::fileExists(fullFilePath)) {
bool fileExists = fileToUploadPath == fullFilePath ? FileSystem::fileExists(fullFilePath)
: (FileSystem::fileExists(fileToUploadPath) && FileSystem::fileExists(fullFilePath));
if (!fileExists) {
if (!finished) {
abortWithError(SyncFileItem::SoftError, tr("The local file was removed during sync."));
return;
@ -403,7 +415,7 @@ void PropagateUploadFileNG::slotPutFinished()
}
}
// Check whether the file changed since discovery.
// Check whether the file changed since discovery - this acts on the original file.
if (!FileSystem::verifyFileUnchanged(fullFilePath, _item->_size, _item->_modtime)) {
propagator()->_anotherSyncNeeded = true;
if (!finished) {

View file

@ -37,9 +37,9 @@ namespace OCC {
void PropagateUploadFileV1::doStartUpload()
{
_chunkCount = std::ceil(_item->_size / double(chunkSize()));
_chunkCount = std::ceil(_fileToUpload._size / double(chunkSize()));
_startChunk = 0;
_transferId = qrand() ^ _item->_modtime ^ (_item->_size << 16);
_transferId = qrand() ^ _item->_modtime ^ (_fileToUpload._size << 16);
const SyncJournalDb::UploadInfo progressInfo = propagator()->_journal->getUploadInfo(_item->_file);
@ -68,12 +68,12 @@ void PropagateUploadFileV1::startNextChunk()
// is sent last.
return;
}
quint64 fileSize = _item->_size;
quint64 fileSize = _fileToUpload._size;
auto headers = PropagateUploadFileCommon::headers();
headers["OC-Total-Length"] = QByteArray::number(fileSize);
headers["OC-Chunk-Size"] = QByteArray::number(quint64(chunkSize()));
QString path = _item->_file;
QString path = _fileToUpload._file;
UploadDevice *device = new UploadDevice(&propagator()->_bandwidthManager);
qint64 chunkStart = 0;
@ -108,7 +108,7 @@ void PropagateUploadFileV1::startNextChunk()
headers[checkSumHeaderC] = _transmissionChecksumHeader;
}
const QString fileName = propagator()->getFilePath(_item->_file);
const QString fileName = _fileToUpload._path;
if (!device->prepareAndOpen(fileName, chunkStart, currentChunkSize)) {
qCWarning(lcPropagateUpload) << "Could not prepare upload device: " << device->errorString();
@ -153,7 +153,6 @@ void PropagateUploadFileV1::startNextChunk()
}
}
if (_currentChunk + _startChunk >= _chunkCount - 1) {
// Don't do parallel upload of chunk if this might be the last chunk because the server cannot handle that
// https://github.com/owncloud/core/issues/11106
@ -221,9 +220,16 @@ void PropagateUploadFileV1::slotPutFinished()
QByteArray etag = getEtagFromReply(job->reply());
bool finished = etag.length() > 0;
// Check if the file still exists
/* Check if the file still exists,
* but we could be operating in a temporary file, so check both if
* the file to upload is different than the file on disk
*/
const QString fileToUploadPath = _fileToUpload._path;
const QString fullFilePath(propagator()->getFilePath(_item->_file));
if (!FileSystem::fileExists(fullFilePath)) {
bool fileExists = fileToUploadPath == fullFilePath ? FileSystem::fileExists(fullFilePath)
: (FileSystem::fileExists(fileToUploadPath) && FileSystem::fileExists(fullFilePath));
if (!fileExists) {
if (!finished) {
abortWithError(SyncFileItem::SoftError, tr("The local file was removed during sync."));
return;
@ -232,7 +238,7 @@ void PropagateUploadFileV1::slotPutFinished()
}
}
// Check whether the file changed since discovery.
// Check whether the file changed since discovery. the file check here is the original and not the temprary.
if (!FileSystem::verifyFileUnchanged(fullFilePath, _item->_size, _item->_modtime)) {
propagator()->_anotherSyncNeeded = true;
if (!finished) {