Fix assets iterator

- start from index -1 each time when we iterate. Each time when we add a
 FolderIterator to the stack we MUST reset the index (-1) otherwise it will
continue from it's last position. To fix it we are cloning the FolderIterator
to set the index to -1. The index must be -1 in order to set it to 0 when we
first call next() method.

- introduce "fileType" static method for a more reliable also much faster
file type lookup. The old version of checking if a file exists, is a folder or
a file was buggy that's why it skipped some file randomly.

Fixes: QTBUG-80178
Change-Id: I4b28e4616399b1bff35d792b55ded1bf19b62dd9
Reviewed-by: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@qt.io>
This commit is contained in:
BogDan Vatra 2019-11-28 18:36:48 +02:00
parent d463a63bb9
commit d027860201

View File

@ -72,9 +72,10 @@ static inline QString prefixedPath(QString path)
struct AssetItem { struct AssetItem {
enum class Type { enum class Type {
File, File,
Folder Folder,
Invalid
}; };
AssetItem() = default;
AssetItem (const QString &rawName) AssetItem (const QString &rawName)
: name(rawName) : name(rawName)
{ {
@ -92,21 +93,47 @@ using AssetItemList = QVector<AssetItem>;
class FolderIterator : public AssetItemList class FolderIterator : public AssetItemList
{ {
public: public:
static QSharedPointer<FolderIterator> fromCache(const QString &path) static QSharedPointer<FolderIterator> fromCache(const QString &path, bool clone)
{ {
QMutexLocker lock(&m_assetsCacheMutex); QMutexLocker lock(&m_assetsCacheMutex);
QSharedPointer<FolderIterator> *folder = m_assetsCache.object(path); QSharedPointer<FolderIterator> *folder = m_assetsCache.object(path);
if (!folder) { if (!folder) {
folder = new QSharedPointer<FolderIterator>{new FolderIterator{path}}; folder = new QSharedPointer<FolderIterator>{new FolderIterator{path}};
if (!m_assetsCache.insert(path, folder)) { if ((*folder)->empty() || !m_assetsCache.insert(path, folder)) {
QSharedPointer<FolderIterator> res = *folder; QSharedPointer<FolderIterator> res = *folder;
delete folder; delete folder;
return res; return res;
} }
} }
return *folder; return clone ? QSharedPointer<FolderIterator>{new FolderIterator{*(*folder)}} : *folder;
} }
static AssetItem::Type fileType(const QString &filePath)
{
const QStringList paths = filePath.split(QLatin1Char('/'));
QString fullPath;
AssetItem::Type res = AssetItem::Type::Invalid;
for (const auto &path: paths) {
auto folder = fromCache(fullPath, false);
auto it = std::lower_bound(folder->begin(), folder->end(), AssetItem{path}, [](const AssetItem &val, const AssetItem &assetItem) {
return val.name < assetItem.name;
});
if (it == folder->end() || it->name != path)
return AssetItem::Type::Invalid;
if (!fullPath.isEmpty())
fullPath.append(QLatin1Char('/'));
fullPath += path;
res = it->type;
}
return res;
}
FolderIterator(const FolderIterator &other)
: AssetItemList(other)
, m_index(-1)
, m_path(other.m_path)
{}
FolderIterator(const QString &path) FolderIterator(const QString &path)
: m_path(path) : m_path(path)
{ {
@ -118,8 +145,12 @@ public:
QJNIEnvironmentPrivate env; QJNIEnvironmentPrivate env;
jobjectArray jFiles = static_cast<jobjectArray>(files.object()); jobjectArray jFiles = static_cast<jobjectArray>(files.object());
const jint nFiles = env->GetArrayLength(jFiles); const jint nFiles = env->GetArrayLength(jFiles);
for (int i = 0; i < nFiles; ++i) for (int i = 0; i < nFiles; ++i) {
push_back({QJNIObjectPrivate(env->GetObjectArrayElement(jFiles, i)).toString()}); AssetItem item{QJNIObjectPrivate(env->GetObjectArrayElement(jFiles, i)).toString()};
insert(std::upper_bound(begin(), end(), item, [](const auto &a, const auto &b){
return a.name < b.name;
}), item);
}
} }
m_path = assetsPrefix + QLatin1Char('/') + m_path + QLatin1Char('/'); m_path = assetsPrefix + QLatin1Char('/') + m_path + QLatin1Char('/');
m_path.replace(QLatin1String("//"), QLatin1String("/")); m_path.replace(QLatin1String("//"), QLatin1String("/"));
@ -169,7 +200,7 @@ public:
const QString &path) const QString &path)
: QAbstractFileEngineIterator(filters, nameFilters) : QAbstractFileEngineIterator(filters, nameFilters)
{ {
m_stack.push_back(FolderIterator::fromCache(cleanedAssetPath(path))); m_stack.push_back(FolderIterator::fromCache(cleanedAssetPath(path), true));
if (m_stack.last()->empty()) if (m_stack.last()->empty())
m_stack.pop_back(); m_stack.pop_back();
} }
@ -215,7 +246,7 @@ public:
if (!res) if (!res)
return {}; return {};
if (res->second.type == AssetItem::Type::Folder) { if (res->second.type == AssetItem::Type::Folder) {
m_stack.push_back(FolderIterator::fromCache(cleanedAssetPath(currentFilePath()))); m_stack.push_back(FolderIterator::fromCache(cleanedAssetPath(currentFilePath()), true));
if (m_stack.last()->empty()) if (m_stack.last()->empty())
m_stack.pop_back(); m_stack.pop_back();
} }
@ -305,12 +336,12 @@ public:
FileFlags fileFlags(FileFlags type = FileInfoAll) const override FileFlags fileFlags(FileFlags type = FileInfoAll) const override
{ {
FileFlags flags(ReadOwnerPerm|ReadUserPerm|ReadGroupPerm|ReadOtherPerm|ExistsFlag); FileFlags commonFlags(ReadOwnerPerm|ReadUserPerm|ReadGroupPerm|ReadOtherPerm|ExistsFlag);
FileFlags flags;
if (m_assetFile) if (m_assetFile)
flags |= FileType; flags = FileType | commonFlags;
else if (m_isFolder) else if (m_isFolder)
flags |= DirectoryType; flags = DirectoryType | commonFlags;
return type & flags; return type & flags;
} }
@ -341,9 +372,20 @@ public:
void setFileName(const QString &file) override void setFileName(const QString &file) override
{ {
if (m_fileName == cleanedAssetPath(file))
return;
close(); close();
m_fileName = cleanedAssetPath(file); m_fileName = cleanedAssetPath(file);
m_isFolder = !open(QIODevice::ReadOnly) && !FolderIterator::fromCache(m_fileName)->empty(); switch (FolderIterator::fileType(m_fileName)) {
case AssetItem::Type::File:
open(QIODevice::ReadOnly);
break;
case AssetItem::Type::Folder:
m_isFolder = true;
break;
case AssetItem::Type::Invalid:
break;
}
} }
Iterator *beginEntryList(QDir::Filters filters, const QStringList &filterNames) override Iterator *beginEntryList(QDir::Filters filters, const QStringList &filterNames) override