From 6f8f8d0cbf87deb1f6793049a42ff843feddaadb Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Thu, 14 Dec 2017 11:37:19 +0100 Subject: [PATCH] Clang: Extend ClangPathWatcher to communicate path changes Change-Id: Iad2d6adc53fbc2ee32fd63b9ccdc8999d41a89da Reviewed-by: Ivan Donchevskii --- src/libs/clangsupport/clangpathwatcher.h | 42 ++++++++--- .../clangsupport/clangpathwatchernotifier.h | 3 + .../source/pchmanagerserver.cpp | 4 ++ .../source/pchmanagerserver.h | 1 + .../changedfilepathcompressor-test.cpp | 19 +++-- tests/unit/unittest/clangpathwatcher-test.cpp | 69 ++++++++++++++----- .../unittest/mockclangpathwatchernotifier.h | 3 + 7 files changed, 108 insertions(+), 33 deletions(-) diff --git a/src/libs/clangsupport/clangpathwatcher.h b/src/libs/clangsupport/clangpathwatcher.h index ea492425fa9..13e026ee092 100644 --- a/src/libs/clangsupport/clangpathwatcher.h +++ b/src/libs/clangsupport/clangpathwatcher.h @@ -41,25 +41,30 @@ public: int id; FilePathId pathId; - friend bool operator==(const WatcherEntry &first, const WatcherEntry &second) + friend bool operator==(WatcherEntry first, WatcherEntry second) { return first.id == second.id && first.pathId == second.pathId; } - friend bool operator<(const WatcherEntry &first, const WatcherEntry &second) + friend bool operator<(WatcherEntry first, WatcherEntry second) { return std::tie(first.pathId, first.id) < std::tie(second.pathId, second.id); } - friend bool operator<(const WatcherEntry &entry, FilePathId pathId) + friend bool operator<(WatcherEntry entry, FilePathId pathId) { return entry.pathId < pathId; } - friend bool operator<(FilePathId pathId, const WatcherEntry &entry) + friend bool operator<(FilePathId pathId, WatcherEntry entry) { return pathId < entry.pathId; } + + operator FilePathId() const + { + return pathId; + } }; using WatcherEntries = std::vector; @@ -224,7 +229,7 @@ unittest_public: std::transform(watcherEntries.begin(), watcherEntries.end(), std::back_inserter(paths), - [&] (const WatcherEntry &entry) { + [&] (WatcherEntry entry) { return QString(m_pathCache.filePath(entry.pathId).path()); }); @@ -233,7 +238,7 @@ unittest_public: template WatcherEntries notWatchedEntries(const WatcherEntries &entries, - Compare compare) const + Compare compare) const { WatcherEntries notWatchedEntries; notWatchedEntries.reserve(entries.size()); @@ -255,7 +260,7 @@ unittest_public: WatcherEntries notWatchedPaths(const WatcherEntries &entries) const { - auto compare = [] (const WatcherEntry &first, const WatcherEntry &second) { + auto compare = [] (WatcherEntry first, WatcherEntry second) { return first.pathId < second.pathId; }; @@ -317,7 +322,7 @@ unittest_public: WatcherEntries uniqueEntries; uniqueEntries.reserve(pathEntries.size()); - auto compare = [] (const WatcherEntry &first, const WatcherEntry &second) { + auto compare = [] (WatcherEntry first, WatcherEntry second) { return first.pathId == second.pathId; }; @@ -342,7 +347,7 @@ unittest_public: WatcherEntries removeIdsFromWatchedEntries(const std::vector &ids) { - auto keep = [&] (const WatcherEntry &entry) { + auto keep = [&] (WatcherEntry entry) { return !std::binary_search(ids.begin(), ids.end(), entry.id); }; @@ -390,6 +395,20 @@ unittest_public: return foundEntries; } + FilePathIds watchedPaths(const FilePathIds &filePathIds) const + { + FilePathIds watchedFilePathIds; + watchedFilePathIds.reserve(filePathIds.size()); + + std::set_intersection(m_watchedEntries.begin(), + m_watchedEntries.end(), + filePathIds.begin(), + filePathIds.end(), + std::back_inserter(watchedFilePathIds)); + + return watchedFilePathIds; + } + Utils::SmallStringVector idsForWatcherEntries(const WatcherEntries &foundEntries) { Utils::SmallStringVector ids; @@ -398,7 +417,7 @@ unittest_public: std::transform(foundEntries.begin(), foundEntries.end(), std::back_inserter(ids), - [&] (const WatcherEntry &entry) { + [&] (WatcherEntry entry) { return Utils::SmallString(m_idCache.string(entry.id)); }); @@ -414,7 +433,7 @@ unittest_public: return std::move(ids); } - void addChangedPathForFilePath(ClangBackEnd::FilePathIds &&filePathIds) + void addChangedPathForFilePath(FilePathIds &&filePathIds) { if (m_notifier) { WatcherEntries foundEntries = watchedEntriesForPaths(std::move(filePathIds)); @@ -422,6 +441,7 @@ unittest_public: Utils::SmallStringVector changedIds = idsForWatcherEntries(foundEntries); m_notifier->pathsWithIdsChanged(uniqueIds(std::move(changedIds))); + m_notifier->pathsChanged(watchedPaths(filePathIds)); } } diff --git a/src/libs/clangsupport/clangpathwatchernotifier.h b/src/libs/clangsupport/clangpathwatchernotifier.h index 15001186f36..a2ee330c2fb 100644 --- a/src/libs/clangsupport/clangpathwatchernotifier.h +++ b/src/libs/clangsupport/clangpathwatchernotifier.h @@ -27,6 +27,8 @@ #include "clangsupport_global.h" +#include + #include namespace ClangBackEnd { @@ -37,6 +39,7 @@ public: virtual ~ClangPathWatcherNotifier(); virtual void pathsWithIdsChanged(const Utils::SmallStringVector &ids) = 0; + virtual void pathsChanged(const FilePathIds &filePathIds) = 0; }; } // namespace ClangBackEnd diff --git a/src/tools/clangpchmanagerbackend/source/pchmanagerserver.cpp b/src/tools/clangpchmanagerbackend/source/pchmanagerserver.cpp index 8abcda8d659..5bd40b18277 100644 --- a/src/tools/clangpchmanagerbackend/source/pchmanagerserver.cpp +++ b/src/tools/clangpchmanagerbackend/source/pchmanagerserver.cpp @@ -75,6 +75,10 @@ void PchManagerServer::pathsWithIdsChanged(const Utils::SmallStringVector &ids) m_fileSystemWatcher.updateIdPaths(m_pchCreator.takeProjectsIncludes()); } +void PchManagerServer::pathsChanged(const FilePathIds &/*filePathIds*/) +{ +} + void PchManagerServer::taskFinished(TaskFinishStatus status, const ProjectPartPch &projectPartPch) { if (status == TaskFinishStatus::Successfully) diff --git a/src/tools/clangpchmanagerbackend/source/pchmanagerserver.h b/src/tools/clangpchmanagerbackend/source/pchmanagerserver.h index bac2dcd00f6..5e736b18873 100644 --- a/src/tools/clangpchmanagerbackend/source/pchmanagerserver.h +++ b/src/tools/clangpchmanagerbackend/source/pchmanagerserver.h @@ -55,6 +55,7 @@ public: void removePchProjectParts(RemovePchProjectPartsMessage &&message) override; void pathsWithIdsChanged(const Utils::SmallStringVector &ids) override; + void pathsChanged(const FilePathIds &filePathIds) override; void taskFinished(TaskFinishStatus status, const ProjectPartPch &projectPartPch) override; private: diff --git a/tests/unit/unittest/changedfilepathcompressor-test.cpp b/tests/unit/unittest/changedfilepathcompressor-test.cpp index ab31136842d..08e66a2541d 100644 --- a/tests/unit/unittest/changedfilepathcompressor-test.cpp +++ b/tests/unit/unittest/changedfilepathcompressor-test.cpp @@ -46,7 +46,7 @@ class ChangedFilePathCompressor : public testing::Test protected: void SetUp() { - compressor.setCallback(mockCompressor.AsStdFunction()); + compressor.setCallback(mockCompressorCallback.AsStdFunction()); } FilePathId filePathId(const QString &filePath) @@ -60,11 +60,13 @@ protected: Sqlite::Database database{":memory:", Sqlite::JournalMode::Memory}; ClangBackEnd::RefactoringDatabaseInitializer initializer{database}; ClangBackEnd::FilePathCaching filePathCache{database}; - NiceMock> mockCompressor; + NiceMock> mockCompressorCallback; ClangBackEnd::ChangedFilePathCompressor> compressor{filePathCache}; NiceMock &mockTimer = compressor.timer(); QString filePath1{"filePath1"}; QString filePath2{"filePath2"}; + FilePathId filePathId1 = filePathId(filePath1); + FilePathId filePathId2 = filePathId(filePath2); }; TEST_F(ChangedFilePathCompressor, AddFilePath) @@ -92,12 +94,19 @@ TEST_F(ChangedFilePathCompressor, CallRestartTimerAfterAddingPath) TEST_F(ChangedFilePathCompressor, CallTimeOutAfterAddingPath) { - auto id1 = filePathId(filePath1); - auto id2 = filePathId(filePath2); - EXPECT_CALL(mockCompressor, Call(ElementsAre(id1, id2))); + EXPECT_CALL(mockCompressorCallback, Call(ElementsAre(filePathId1, filePathId2))); compressor.addFilePath(filePath1); compressor.addFilePath(filePath2); } +TEST_F(ChangedFilePathCompressor, RemoveDuplicates) +{ + EXPECT_CALL(mockCompressorCallback, Call(ElementsAre(filePathId1, filePathId2))); + + compressor.addFilePath(filePath1); + compressor.addFilePath(filePath2); + compressor.addFilePath(filePath1); +} + } diff --git a/tests/unit/unittest/clangpathwatcher-test.cpp b/tests/unit/unittest/clangpathwatcher-test.cpp index cb3a7a609a1..bdc20348bc6 100644 --- a/tests/unit/unittest/clangpathwatcher-test.cpp +++ b/tests/unit/unittest/clangpathwatcher-test.cpp @@ -44,6 +44,7 @@ using testing::NiceMock; using Watcher = ClangBackEnd::ClangPathWatcher, NiceMock>; using ClangBackEnd::WatcherEntry; +using ClangBackEnd::WatcherEntries; using ClangBackEnd::FilePath; using ClangBackEnd::FilePathView; using ClangBackEnd::FilePathId; @@ -53,6 +54,12 @@ class ClangPathWatcher : public testing::Test { protected: void SetUp(); + static WatcherEntries sorted(WatcherEntries &&entries) + { + std::stable_sort(entries.begin(), entries.end()); + + return entries; + } protected: NiceMock filePathCache; @@ -77,14 +84,14 @@ protected: TEST_F(ClangPathWatcher, ConvertWatcherEntriesToQStringList) { - auto convertedList = watcher.convertWatcherEntriesToQStringList({watcherEntry1, watcherEntry3}); + auto convertedList = watcher.convertWatcherEntriesToQStringList(sorted({watcherEntry1, watcherEntry3})); ASSERT_THAT(convertedList, ElementsAre(path1QString, path2QString)); } TEST_F(ClangPathWatcher, UniquePaths) { - auto uniqueEntries = watcher.uniquePaths({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4}); + auto uniqueEntries = watcher.uniquePaths(sorted({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4})); ASSERT_THAT(uniqueEntries, ElementsAre(watcherEntry1, watcherEntry3)); } @@ -93,7 +100,7 @@ TEST_F(ClangPathWatcher, NotWatchedEntries) { watcher.addEntries({watcherEntry1, watcherEntry4}); - auto newEntries = watcher.notWatchedEntries({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4}); + auto newEntries = watcher.notWatchedEntries(sorted({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4})); ASSERT_THAT(newEntries, ElementsAre(watcherEntry2, watcherEntry3)); } @@ -158,7 +165,7 @@ TEST_F(ClangPathWatcher, ExtractSortedIdsFromConvertIdPaths) TEST_F(ClangPathWatcher, NotWatchedPaths) { - watcher.mergeToWatchedEntries({watcherEntry1}); + watcher.mergeToWatchedEntries(sorted({watcherEntry1})); auto newEntries = watcher.notWatchedPaths({watcherEntry2, watcherEntry3}); @@ -167,7 +174,7 @@ TEST_F(ClangPathWatcher, NotWatchedPaths) TEST_F(ClangPathWatcher, AddedPaths) { - watcher.mergeToWatchedEntries({watcherEntry1, watcherEntry2}); + watcher.mergeToWatchedEntries(sorted({watcherEntry1, watcherEntry2})); auto filteredEntries = watcher.filterNotWatchedPaths({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4}); @@ -176,16 +183,16 @@ TEST_F(ClangPathWatcher, AddedPaths) TEST_F(ClangPathWatcher, MergeEntries) { - watcher.mergeToWatchedEntries({watcherEntry1, watcherEntry4}); + watcher.mergeToWatchedEntries(sorted({watcherEntry1, watcherEntry4})); ASSERT_THAT(watcher.watchedEntries(), ElementsAre(watcherEntry1, watcherEntry4)); } TEST_F(ClangPathWatcher, MergeMoreEntries) { - watcher.mergeToWatchedEntries({watcherEntry1, watcherEntry4}); + watcher.mergeToWatchedEntries(sorted({watcherEntry1, watcherEntry4})); - watcher.mergeToWatchedEntries({watcherEntry2, watcherEntry3}); + watcher.mergeToWatchedEntries(sorted({watcherEntry2, watcherEntry3})); ASSERT_THAT(watcher.watchedEntries(), ElementsAre(watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4)); } @@ -234,7 +241,7 @@ TEST_F(ClangPathWatcher, DontAddNewEntriesWithDifferentIdAndSamePaths) TEST_F(ClangPathWatcher, RemoveEntriesWithId) { - watcher.addEntries({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5}); + watcher.addEntries(sorted({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5})); watcher.removeIdsFromWatchedEntries({ids[0]}); @@ -251,7 +258,7 @@ TEST_F(ClangPathWatcher, RemoveNoPathsForEmptyIds) TEST_F(ClangPathWatcher, RemoveNoPathsForOneId) { - watcher.addEntries({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4}); + watcher.addEntries(sorted({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4})); EXPECT_CALL(mockQFileSytemWatcher, removePaths(_)) .Times(0); @@ -261,7 +268,7 @@ TEST_F(ClangPathWatcher, RemoveNoPathsForOneId) TEST_F(ClangPathWatcher, RemovePathForOneId) { - watcher.addEntries({watcherEntry1, watcherEntry2, watcherEntry3}); + watcher.addEntries(sorted({watcherEntry1, watcherEntry2, watcherEntry3})); EXPECT_CALL(mockQFileSytemWatcher, removePaths(QStringList{path2QString})); @@ -270,7 +277,7 @@ TEST_F(ClangPathWatcher, RemovePathForOneId) TEST_F(ClangPathWatcher, RemoveAllPathsForThreeId) { - watcher.addEntries({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5}); + watcher.addEntries(sorted({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5})); EXPECT_CALL(mockQFileSytemWatcher, removePaths(QStringList{path1QString, path2QString})); @@ -279,7 +286,7 @@ TEST_F(ClangPathWatcher, RemoveAllPathsForThreeId) TEST_F(ClangPathWatcher, RemoveOnePathForTwoId) { - watcher.addEntries({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5}); + watcher.addEntries(sorted({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5})); EXPECT_CALL(mockQFileSytemWatcher, removePaths(QStringList{path1QString})); @@ -288,7 +295,7 @@ TEST_F(ClangPathWatcher, RemoveOnePathForTwoId) TEST_F(ClangPathWatcher, NotAnymoreWatchedEntriesWithId) { - watcher.addEntries({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5}); + watcher.addEntries(sorted({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5})); auto oldEntries = watcher.notAnymoreWatchedEntriesWithIds({watcherEntry1, watcherEntry4}, {ids[0], ids[1]}); @@ -297,7 +304,7 @@ TEST_F(ClangPathWatcher, NotAnymoreWatchedEntriesWithId) TEST_F(ClangPathWatcher, RemoveUnusedEntries) { - watcher.addEntries({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5}); + watcher.addEntries(sorted({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5})); watcher.removeFromWatchedEntries({watcherEntry2, watcherEntry3}); @@ -315,7 +322,7 @@ TEST_F(ClangPathWatcher, EmptyVectorNotifyFileChange) TEST_F(ClangPathWatcher, NotifyFileChange) { - watcher.addEntries({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5}); + watcher.addEntries(sorted({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5})); EXPECT_CALL(notifier, pathsWithIdsChanged(ElementsAre(id2, id1))); @@ -324,7 +331,7 @@ TEST_F(ClangPathWatcher, NotifyFileChange) TEST_F(ClangPathWatcher, TwoNotifyFileChanges) { - watcher.addEntries({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5}); + watcher.addEntries(sorted({watcherEntry1, watcherEntry2, watcherEntry3, watcherEntry4, watcherEntry5})); EXPECT_CALL(notifier, pathsWithIdsChanged(ElementsAre(id2, id3, id1))); @@ -332,6 +339,34 @@ TEST_F(ClangPathWatcher, TwoNotifyFileChanges) mockQFileSytemWatcher.fileChanged(path1QString); } +TEST_F(ClangPathWatcher, NotifyForPathChanges) +{ + watcher.addEntries({watcherEntry1}); + + EXPECT_CALL(notifier, pathsChanged(ElementsAre(pathIds[0]))); + + mockQFileSytemWatcher.fileChanged(path1QString); +} + +TEST_F(ClangPathWatcher, NoNotifyForUnwatchedPathChanges) +{ + watcher.addEntries({watcherEntry3}); + + EXPECT_CALL(notifier, pathsChanged(IsEmpty())); + + mockQFileSytemWatcher.fileChanged(path1QString); +} + +TEST_F(ClangPathWatcher, NoDuplicatePathChanges) +{ + watcher.addEntries({watcherEntry1}); + + EXPECT_CALL(notifier, pathsChanged(ElementsAre(pathIds[0]))); + + mockQFileSytemWatcher.fileChanged(path1QString); + mockQFileSytemWatcher.fileChanged(path1QString); +} + void ClangPathWatcher::SetUp() { ON_CALL(filePathCache, filePathId(Eq(path1))) diff --git a/tests/unit/unittest/mockclangpathwatchernotifier.h b/tests/unit/unittest/mockclangpathwatchernotifier.h index fa1b09972b6..c704ca5704c 100644 --- a/tests/unit/unittest/mockclangpathwatchernotifier.h +++ b/tests/unit/unittest/mockclangpathwatchernotifier.h @@ -34,5 +34,8 @@ class MockClangPathWatcherNotifier : public ClangBackEnd::ClangPathWatcherNotifi public: MOCK_METHOD1(pathsWithIdsChanged, void (const Utils::SmallStringVector &ids)); + + MOCK_METHOD1(pathsChanged, + void (const ClangBackEnd::FilePathIds &filePathIds)); };