Skip to content

Commit

Permalink
itemsync: Improve performance/consistency when moving items
Browse files Browse the repository at this point in the history 
Fixes app getting stuck with pinned items.
  • Loading branch information
@hluk
hluk committed May 3, 2024
1 parent 76233c7 commit 19c8846
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 44 deletions.
144 changes: 101 additions & 43 deletions plugins/itemsync/filewatcher.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
#include <QRegularExpression>
#include <QUrl>

#include <QDebug>

#include <array>
#include <vector>

Expand Down Expand Up @@ -364,7 +362,7 @@ QStringList listFiles(const QDir &dir)

/// Return true only if no file name in @a fileNames starts with @a baseName.
bool isUniqueBaseName(const QString &baseName, const QStringList &fileNames,
const QStringList &baseNames = QStringList())
const QSet<QString> &baseNames = {})
{
if ( baseNames.contains(baseName) )
return false;
Expand Down Expand Up @@ -402,7 +400,7 @@ void removeFormatFiles(const QString &path, const QVariantMap &mimeToExtension)
}

bool renameToUnique(
const QDir &dir, const QStringList &baseNames, QString *name,
const QDir &dir, const QSet<QString> &baseNames, QString *name,
const QList<FileFormat> &formatSettings)
{
if ( name->isEmpty() ) {
Expand All @@ -426,14 +424,14 @@ bool renameToUnique(
QString baseName;
getBaseNameAndExtension(*name, &baseName, &ext, formatSettings);

qulonglong i = 0;
int fieldWidth = 0;
int i = 0;
int fieldWidth = 4;

QRegularExpression re(QLatin1String(R"(\d+$)"));
QRegularExpression re(QStringLiteral(R"(\d{1,4}$)"));
const auto m = re.match(baseName);
if (m.hasMatch()) {
const QString num = m.captured();
i = num.toULongLong();
i = num.toInt();
fieldWidth = num.size();
baseName = baseName.mid( 0, baseName.size() - fieldWidth );
} else {
Expand All @@ -452,6 +450,16 @@ bool renameToUnique(
return false;
}

QString findLastOwnBaseName(QAbstractItemModel *model, int fromRow) {
for (int row = fromRow; row < model->rowCount(); ++row) {
const QModelIndex index = model->index(row, 0);
const QString baseName = FileWatcher::getBaseName(index);
if ( FileWatcher::isOwnBaseName(baseName) )
return baseName;
}
return {};
}

} // namespace

QString FileWatcher::getBaseName(const QModelIndex &index)
Expand Down Expand Up @@ -522,6 +530,7 @@ FileWatcher::FileWatcher(
, m_lock(m_path + QLatin1String("/.copyq_lock"))
{
m_updateTimer.setSingleShot(true);
m_moveTimer.setSingleShot(true);

m_lock.setStaleLockTime(staleLockTimeMs);

Expand All @@ -531,6 +540,8 @@ FileWatcher::FileWatcher(

connect( &m_updateTimer, &QTimer::timeout,
this, &FileWatcher::updateItems );
connect( &m_moveTimer, &QTimer::timeout,
this, &FileWatcher::updateMovedRows );

connect( m_model, &QAbstractItemModel::rowsInserted,
this, &FileWatcher::onRowsInserted );
Expand All @@ -549,8 +560,20 @@ FileWatcher::FileWatcher(

bool FileWatcher::lock()
{
if ( !m_valid || !m_lock.lock() )
if ( !m_valid )
return false;

// Create path if doesn't exist.
QDir dir(m_path);
if ( !dir.mkpath(QStringLiteral(".")) ) {
log( tr("Failed to create synchronization directory \"%1\"!").arg(m_path), LogError );
return false;
}

if ( !m_lock.lock() ) {
log( QStringLiteral("Failed to create lock file in \"%1\"!").arg(m_path), LogError );
return false;
}

m_valid = false;
return true;
Expand Down Expand Up @@ -750,6 +773,9 @@ void FileWatcher::setUpdatesEnabled(bool enabled)

void FileWatcher::onRowsInserted(const QModelIndex &, int first, int last)
{
if (first < m_moveEnd)
m_moveEnd += last - first + 1;

saveItems(first, last, UpdateType::Inserted);
}

Expand All @@ -760,6 +786,9 @@ void FileWatcher::onDataChanged(const QModelIndex &a, const QModelIndex &b)

void FileWatcher::onRowsRemoved(const QModelIndex &, int first, int last)
{
if (first < m_moveEnd)
m_moveEnd -= std::min(m_moveEnd, last) - first + 1;

const bool wasFull = m_maxItems >= m_model->rowCount();

for ( const auto &index : indexList(first, last) ) {
Expand All @@ -778,38 +807,68 @@ void FileWatcher::onRowsRemoved(const QModelIndex &, int first, int last)

void FileWatcher::onRowsMoved(const QModelIndex &, int start, int end, const QModelIndex &, int destinationRow)
{
/* If own items were moved, change their base names in data to trigger
* updating/renaming file names so they are also moved in other app instances.
*
* The implementation works in common cases but will fail if:
* - Items are moved after the last own item.
* - Items move repeatedly to some not-top position.
*/
// Update copyq_* base names for moved rows and all rows above so that file
// are ordered the same way as the rows.
//
// The update is postponed and batched since it may need to go through a
// lot of items.
const int count = end - start + 1;
if (destinationRow < start) {
m_moveEnd = std::max(m_moveEnd, destinationRow + count - 1);
} else if (destinationRow > end) {
m_moveEnd = std::max(m_moveEnd, destinationRow - 1);
} else {
m_moveEnd = std::max(m_moveEnd, end);
}
m_moveTimer.start(0);
}

const int baseRow = destinationRow < start
? destinationRow + count
: destinationRow;
QString baseName;
if (destinationRow > 0) {
const QModelIndex baseIndex = m_model->index(baseRow, 0);
baseName = FileWatcher::getBaseName(baseIndex);
void FileWatcher::updateMovedRows()
{
if ( !lock() ) {
m_moveTimer.start();
return;
}

if ( !isOwnBaseName(baseName) )
return;
QString baseName = findLastOwnBaseName(m_model, m_moveEnd + 1);
QSet<QString> baseNames;

if ( !baseName.isEmpty() && !baseName.contains(QLatin1Char('-')) )
baseName.append(QLatin1String("-0000"));
}
QDir dir(m_path);

const int batchStart = std::max(0, m_moveEnd - 100);
const auto indexList = this->indexList(batchStart, m_moveEnd);

for (int row = baseRow - 1; row >= baseRow - count; --row) {
const auto index = m_model->index(row, 0);
for (const auto &index : indexList) {
const QString currentBaseName = FileWatcher::getBaseName(index);
if ( currentBaseName.isEmpty() || isOwnBaseName(currentBaseName) ) {
const QVariantMap data = {{mimeBaseName, baseName}};
m_model->setData(index, data, contentType::updateData);

// Skip renaming non-owned items
if ( !currentBaseName.isEmpty() && !isOwnBaseName(currentBaseName) )
continue;

// Skip already sorted
if ( isBaseNameLessThan(currentBaseName, baseName) ) {
baseName = currentBaseName;
continue;
}

if ( !renameToUnique(dir, baseNames, &baseName, m_formatSettings) )
return;

baseNames.insert(baseName);
const QVariantMap data = {{mimeBaseName, baseName}};
m_model->setData(index, data, contentType::updateData);
}

if ( !renameMoveCopy(dir, indexList, UpdateType::Changed) )
return;

unlock();

m_moveEnd = batchStart - 1;
if (0 <= m_moveEnd)
m_moveTimer.start(batchItemUpdateIntervalMs);
else
m_moveEnd = -1;
}

QString FileWatcher::oldBaseName(const QModelIndex &index) const
Expand Down Expand Up @@ -889,13 +948,7 @@ void FileWatcher::saveItems(int first, int last, UpdateType updateType)

const auto indexList = this->indexList(first, last);

// Create path if doesn't exist.
QDir dir(m_path);
if ( !dir.mkpath(".") ) {
log( tr("Failed to create synchronization directory \"%1\"!").arg(m_path) );
return;
}

if ( !renameMoveCopy(dir, indexList, updateType) )
return;

Expand Down Expand Up @@ -977,7 +1030,11 @@ void FileWatcher::saveItems(int first, int last, UpdateType updateType)
bool FileWatcher::renameMoveCopy(
const QDir &dir, const QList<QPersistentModelIndex> &indexList, UpdateType updateType)
{
QStringList baseNames;
if ( indexList.isEmpty() )
return false;

QSet<QString> baseNames;
QString baseName = findLastOwnBaseName(m_model, indexList.first().row() + 1);

for (const auto &index : indexList) {
if ( !index.isValid() )
Expand All @@ -988,15 +1045,16 @@ bool FileWatcher::renameMoveCopy(
if (updateType == UpdateType::Changed && olderBaseName.isEmpty())
olderBaseName = oldBaseName;

QString baseName = oldBaseName;
if ( !oldBaseName.isEmpty() )
baseName = oldBaseName;

bool newItem = updateType != UpdateType::Changed && olderBaseName.isEmpty();
bool itemRenamed = olderBaseName != baseName;
if ( newItem || itemRenamed ) {
if ( !renameToUnique(dir, baseNames, &baseName, m_formatSettings) )
return false;
itemRenamed = olderBaseName != baseName;
baseNames.append(baseName);
baseNames.insert(baseName);
}

QVariantMap itemData = index.data(contentType::data).toMap();
Expand Down Expand Up @@ -1079,7 +1137,7 @@ void FileWatcher::updateDataAndWatchFile(const QDir &dir, const BaseNameExtensio
}
}

bool FileWatcher::copyFilesFromUriList(const QByteArray &uriData, int targetRow, const QStringList &baseNames)
bool FileWatcher::copyFilesFromUriList(const QByteArray &uriData, int targetRow, const QSet<QString> &baseNames)
{
QMimeData tmpData;
tmpData.setData(mimeUriList, uriData);
Expand Down
7 changes: 6 additions & 1 deletion plugins/itemsync/filewatcher.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <QLockFile>
#include <QObject>
#include <QPersistentModelIndex>
#include <QSet>
#include <QStringList>
#include <QTimer>
#include <QVector>
Expand Down Expand Up @@ -125,10 +126,14 @@ class FileWatcher final : public QObject {
const QDir &dir, const BaseNameExtensions &baseNameWithExts,
QVariantMap *dataMap, QVariantMap *mimeToExtension);

bool copyFilesFromUriList(const QByteArray &uriData, int targetRow, const QStringList &baseNames);
bool copyFilesFromUriList(const QByteArray &uriData, int targetRow, const QSet<QString> &baseNames);

void updateMovedRows();

QAbstractItemModel *m_model;
QTimer m_updateTimer;
QTimer m_moveTimer;
int m_moveEnd = -1;
int m_interval = 0;
const QList<FileFormat> &m_formatSettings;
QString m_path;
Expand Down
4 changes: 4 additions & 0 deletions plugins/itemsync/tests/itemsynctests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -748,6 +748,10 @@ void ItemSyncTests::moveOwnItemsSortsBaseNames()
RUN(args << "keys" << "END" << "UP" << "CTRL+HOME", "");
RUN(args << "read(0,1,2,3)", "B,C,D,A");
RUN(args << testScript, "");

RUN(args << "keys" << "HOME" << "CTRL+END", "");
RUN(args << "read(0,1,2,3)", "C,D,A,B");
RUN(args << testScript, "");
}

void ItemSyncTests::avoidDuplicateItemsAddedFromClipboard()
Expand Down

0 comments on commit 19c8846

Please sign in to comment.