Clang: Use FilePathId in ChangedFilePathCompressor

Change-Id: I622d9196ec12c6fe6e07ef9c953586f2f40b9a5c
Reviewed-by: Ivan Donchevskii <ivan.donchevskii@qt.io>

src/libs/clangsupport/changedfilepathcompressor.h

@@ -27,10 +27,13 @@
 
 #include "clangsupport_global.h"
 
-#include <filepath.h>
+#include <filepathid.h>
+#include <filepathcache.h>
 
 #include <QTimer>
 
+#include <filepathcachinginterface.h>
+
 #include <functional>
 
 namespace ClangBackEnd {
@@ -39,7 +42,8 @@ template <typename Timer>
 class ChangedFilePathCompressor
 {
 public:
-    ChangedFilePathCompressor()
+    ChangedFilePathCompressor(FilePathCachingInterface &filePathCache)
+        : m_filePathCache(filePathCache)
     {
         m_timer.setSingleShot(true);
     }
@@ -50,21 +54,26 @@ public:
 
     void addFilePath(const QString &filePath)
     {
-        m_filePaths.emplace_back(filePath);
+        FilePathId filePathId = m_filePathCache.filePathId(FilePath(filePath));
+
+        auto found = std::lower_bound(m_filePaths.begin(), m_filePaths.end(), filePathId);
+
+        if (found == m_filePaths.end() || *found != filePathId)
+            m_filePaths.insert(found, filePathId);
 
         restartTimer();
     }
 
-    FilePaths takeFilePaths()
+    FilePathIds takeFilePathIds()
     {
         return std::move(m_filePaths);
     }
 
-    virtual void setCallback(std::function<void(ClangBackEnd::FilePaths &&)> &&callback)
+    virtual void setCallback(std::function<void(ClangBackEnd::FilePathIds &&)> &&callback)
     {
         QObject::connect(&m_timer,
                          &Timer::timeout,
-                         [this, callback=std::move(callback)] { callback(takeFilePaths()); });
+                         [this, callback=std::move(callback)] { callback(takeFilePathIds()); });
     }
 
 unittest_public:
@@ -79,8 +88,9 @@ unittest_public:
     }
 
 private:
-    FilePaths m_filePaths;
+    FilePathIds m_filePaths;
     Timer m_timer;
+    FilePathCachingInterface &m_filePathCache;
 };
 
 } // namespace ClangBackEnd

src/libs/clangsupport/clangpathwatcher.h

@@ -77,21 +77,22 @@ class ClangPathWatcher : public ClangPathWatcherInterface
 public:
     ClangPathWatcher(FilePathCachingInterface &pathCache,
                      ClangPathWatcherNotifier *notifier=nullptr)
-        : m_pathCache(pathCache),
+        : m_changedFilePathCompressor(pathCache),
+          m_pathCache(pathCache),
           m_notifier(notifier)
     {
         QObject::connect(&m_fileSystemWatcher,
                          &FileSystemWatcher::fileChanged,
                          [&] (const QString &filePath) { compressChangedFilePath(filePath); });
 
-        m_changedFilePathCompressor.setCallback([&] (ClangBackEnd::FilePaths &&filePaths) {
-            addChangedPathForFilePath(std::move(filePaths));
+        m_changedFilePathCompressor.setCallback([&] (ClangBackEnd::FilePathIds &&filePathIds) {
+            addChangedPathForFilePath(std::move(filePathIds));
         });
     }
 
     ~ClangPathWatcher()
     {
-        m_changedFilePathCompressor.setCallback([&] (FilePaths &&) {});
+        m_changedFilePathCompressor.setCallback([&] (FilePathIds &&) {});
     }
 
     void updateIdPaths(const std::vector<IdPaths> &idPaths) override
@@ -376,14 +377,12 @@ unittest_public:
         m_changedFilePathCompressor.addFilePath(filePath);
     }
 
-    WatcherEntries watchedEntriesForPaths(ClangBackEnd::FilePaths &&filePaths)
+    WatcherEntries watchedEntriesForPaths(ClangBackEnd::FilePathIds &&filePathIds)
     {
-        FilePathIds pathIds = m_pathCache.filePathIds(filePaths);
-
         WatcherEntries foundEntries;
-        foundEntries.reserve(pathIds.size());
+        foundEntries.reserve(filePathIds.size());
 
-        for (FilePathId pathId : pathIds) {
+        for (FilePathId pathId : filePathIds) {
             auto range = std::equal_range(m_watchedEntries.begin(), m_watchedEntries.end(), pathId);
             foundEntries.insert(foundEntries.end(), range.first, range.second);
         }
@@ -415,10 +414,10 @@ unittest_public:
         return std::move(ids);
     }
 
-    void addChangedPathForFilePath(ClangBackEnd::FilePaths &&filePaths)
+    void addChangedPathForFilePath(ClangBackEnd::FilePathIds &&filePathIds)
     {
         if (m_notifier) {
-            WatcherEntries foundEntries = watchedEntriesForPaths(std::move(filePaths));
+            WatcherEntries foundEntries = watchedEntriesForPaths(std::move(filePathIds));
 
             Utils::SmallStringVector changedIds = idsForWatcherEntries(foundEntries);
 
@@ -439,7 +438,7 @@ unittest_public:
 private:
     IdCache m_idCache;
     WatcherEntries m_watchedEntries;
-    ChangedFilePathCompressor<Timer> m_changedFilePathCompressor;
+    ChangedFilePathCompressor<Timer> m_changedFilePathCompressor{};
     FileSystemWatcher m_fileSystemWatcher;
     FilePathCachingInterface &m_pathCache;
     ClangPathWatcherNotifier *m_notifier;

tests/unit/unittest/changedfilepathcompressor-test.cpp

@@ -25,7 +25,11 @@
 
 #include "googletest.h"
 
-#include <mockchangedfilepathcompressor.h>
+#include "mocktimer.h"
+
+#include <changedfilepathcompressor.h>
+#include <filepathcaching.h>
+#include <refactoringdatabaseinitializer.h>
 
 namespace {
 
@@ -35,56 +39,65 @@ using testing::IsEmpty;
 using testing::NiceMock;
 
 using ClangBackEnd::FilePath;
-
+using ClangBackEnd::FilePathId;
 
 class ChangedFilePathCompressor : public testing::Test
 {
 protected:
-    void SetUp();
+    void SetUp()
+    {
+        compressor.setCallback(mockCompressor.AsStdFunction());
+    }
+
+    FilePathId filePathId(const QString &filePath)
+    {
+        Utils::SmallString utf8FilePath{filePath};
+
+        return filePathCache.filePathId(ClangBackEnd::FilePathView{utf8FilePath});
+    }
 
 protected:
-    NiceMock<MockChangedFilePathCompressor> mockCompressor;
-    ClangBackEnd::ChangedFilePathCompressor<FakeTimer> compressor;
+    Sqlite::Database database{":memory:", Sqlite::JournalMode::Memory};
+    ClangBackEnd::RefactoringDatabaseInitializer<Sqlite::Database> initializer{database};
+    ClangBackEnd::FilePathCaching filePathCache{database};
+    NiceMock<MockFunction<void (const ClangBackEnd::FilePathIds &filePathIds)>> mockCompressor;
+    ClangBackEnd::ChangedFilePathCompressor<NiceMock<MockTimer>> compressor{filePathCache};
+    NiceMock<MockTimer> &mockTimer = compressor.timer();
     QString filePath1{"filePath1"};
     QString filePath2{"filePath2"};
 };
 
 TEST_F(ChangedFilePathCompressor, AddFilePath)
 {
-    mockCompressor.addFilePath(filePath1);
+    compressor.addFilePath(filePath1);
 
-    ASSERT_THAT(mockCompressor.takeFilePaths(), ElementsAre(FilePath{filePath1}));
+    ASSERT_THAT(compressor.takeFilePathIds(), ElementsAre(filePathId(filePath1)));
 }
 
 TEST_F(ChangedFilePathCompressor, NoFilePathsAferTakenThem)
 {
-    mockCompressor.addFilePath(filePath1);
+    compressor.addFilePath(filePath1);
 
-    mockCompressor.takeFilePaths();
+    compressor.takeFilePathIds();
 
-    ASSERT_THAT(mockCompressor.takeFilePaths(), IsEmpty());
+    ASSERT_THAT(compressor.takeFilePathIds(), IsEmpty());
 }
 
 TEST_F(ChangedFilePathCompressor, CallRestartTimerAfterAddingPath)
 {
-    EXPECT_CALL(mockCompressor, restartTimer());
+    EXPECT_CALL(mockTimer, start(20));
 
-    mockCompressor.addFilePath(filePath1);
+    compressor.addFilePath(filePath1);
 }
 
 TEST_F(ChangedFilePathCompressor, CallTimeOutAfterAddingPath)
 {
-    EXPECT_CALL(mockCompressor, callbackCalled(ElementsAre(FilePath{filePath1}, FilePath{filePath2})));
+    auto id1 = filePathId(filePath1);
+    auto id2 = filePathId(filePath2);
+    EXPECT_CALL(mockCompressor, Call(ElementsAre(id1, id2)));
 
     compressor.addFilePath(filePath1);
     compressor.addFilePath(filePath2);
 }
 
-void ChangedFilePathCompressor::SetUp()
-{
-    compressor.setCallback([&] (ClangBackEnd::FilePaths &&filePaths) {
-        mockCompressor.callbackCalled(filePaths);
-    });
-}
-
 }

tests/unit/unittest/clangpathwatcher-test.cpp

@@ -25,7 +25,7 @@
 
 #include "googletest.h"
 
-#include "faketimer.h"
+#include "mocktimer.h"
 #include "mockfilepathcaching.h"
 #include "mockqfilesystemwatcher.h"
 #include "mockclangpathwatchernotifier.h"
@@ -42,7 +42,7 @@ using testing::IsEmpty;
 using testing::SizeIs;
 using testing::NiceMock;
 
-using Watcher = ClangBackEnd::ClangPathWatcher<NiceMock<MockQFileSytemWatcher>, FakeTimer>;
+using Watcher = ClangBackEnd::ClangPathWatcher<NiceMock<MockQFileSytemWatcher>, NiceMock<MockTimer>>;
 using ClangBackEnd::WatcherEntry;
 using ClangBackEnd::FilePath;
 using ClangBackEnd::FilePathView;

tests/unit/unittest/google-using-declarations.h

@@ -31,6 +31,7 @@ using testing::_;
 using testing::AllOf;
 using testing::AnyNumber;
 using testing::AnyOf;
+using testing::Assign;
 using testing::Contains;
 using testing::ElementsAre;
 using testing::Field;
@@ -38,6 +39,7 @@ using testing::HasSubstr;
 using testing::InSequence;
 using testing::IsEmpty;
 using testing::Mock;
+using testing::MockFunction;
 using testing::NiceMock;
 using testing::Not;
 using testing::Pair;

tests/unit/unittest/mockchangedfilepathcompressor.h

@@ -1,43 +0,0 @@
-/****************************************************************************
-**
-** Copyright (C) 2016 The Qt Company Ltd.
-** Contact: https://www.qt.io/licensing/
-**
-** This file is part of Qt Creator.
-**
-** Commercial License Usage
-** Licensees holding valid commercial Qt licenses may use this file in
-** accordance with the commercial license agreement provided with the
-** Software or, alternatively, in accordance with the terms contained in
-** a written agreement between you and The Qt Company. For licensing terms
-** and conditions see https://www.qt.io/terms-conditions. For further
-** information use the contact form at https://www.qt.io/contact-us.
-**
-** GNU General Public License Usage
-** Alternatively, this file may be used under the terms of the GNU
-** General Public License version 3 as published by the Free Software
-** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
-** included in the packaging of this file. Please review the following
-** information to ensure the GNU General Public License requirements will
-** be met: https://www.gnu.org/licenses/gpl-3.0.html.
-**
-****************************************************************************/
-
-#pragma once
-
-#include "googletest.h"
-
-#include "faketimer.h"
-
-#include <changedfilepathcompressor.h>
-
-class MockChangedFilePathCompressor : public ClangBackEnd::ChangedFilePathCompressor<FakeTimer>
-{
-public:
-    MOCK_METHOD0(restartTimer,
-                 void ());
-
-    MOCK_METHOD1(callbackCalled,
-                 void (const ClangBackEnd::FilePaths &filePaths));
-};
-

tests/unit/unittest/mocktimer.cpp

@@ -23,23 +23,23 @@
 **
 ****************************************************************************/
 
-#include "faketimer.h"
+#include "mocktimer.h"
 
-FakeTimer::~FakeTimer()
+MockTimer::MockTimer()
+{
+    ON_CALL(*this, start(_)).WillByDefault(Assign(&m_isStarted, true));
+}
+
+MockTimer::~MockTimer()
 {
     emitTimoutIfStarted();
 }
 
-void FakeTimer::start(int)
-{
-    m_isStarted = true;
-}
-
-void FakeTimer::setSingleShot(bool)
+void MockTimer::setSingleShot(bool)
 {
 }
 
-void FakeTimer::emitTimoutIfStarted()
+void MockTimer::emitTimoutIfStarted()
 {
     if (m_isStarted)
         emit timeout();

tests/unit/unittest/mocktimer.h

@@ -24,16 +24,21 @@
 ****************************************************************************/
 #pragma once
 
+#include "googletest.h"
+
 #include <QObject>
 
-class FakeTimer : public QObject
+class MockTimer : public QObject
 {
     Q_OBJECT
 
 public:
-    ~FakeTimer();
+    MockTimer();
+    ~MockTimer();
+
+    MOCK_METHOD1(start,
+                 void (int));
 
-    void start(int interval);
     void setSingleShot(bool);
 
     void emitTimoutIfStarted();

tests/unit/unittest/unittest.pro

@@ -48,7 +48,6 @@ SOURCES += \
     cppprojectinfogenerator-test.cpp \
     cppprojectpartchooser-test.cpp \
     fakeprocess.cpp \
-    faketimer.cpp \
     filepath-test.cpp \
     filepathview-test.cpp \
     gtest-creator-printing.cpp \
@@ -85,7 +84,8 @@ SOURCES += \
     filepathstoragesqlitestatementfactory-test.cpp \
     processcreator-test.cpp \
     nativefilepath-test.cpp \
-    nativefilepathview-test.cpp
+    nativefilepathview-test.cpp \
+    mocktimer.cpp
 
 !isEmpty(LIBCLANG_LIBS) {
 SOURCES += \
@@ -180,13 +180,11 @@ HEADERS += \
     dynamicastmatcherdiagnosticcontainer-matcher.h \
     eventspy.h \
     fakeprocess.h \
-    faketimer.h \
     filesystem-utilities.h \
     googletest.h \
     gtest-creator-printing.h \
     gtest-qt-printing.h \
     mimedatabase-utilities.h \
-    mockchangedfilepathcompressor.h \
     mockclangcodemodelclient.h \
     mockclangcodemodelserver.h \
     mockclangpathwatcher.h \
@@ -221,7 +219,8 @@ HEADERS += \
     unittest-utility-functions.h \
     mocksymbolquery.h \
     runprojectcreateorupdate-utility.h \
-    rundocumentparse-utility.h
+    rundocumentparse-utility.h \
+    mocktimer.h
 !isEmpty(LIBCLANG_LIBS) {
 HEADERS += \
     chunksreportedmonitor.h \