PchManager: Update only after generated files have been updated

Task-number: QTCREATORBUG-21843
Change-Id: I881e86dac4074438880d657a07f2e473489ab42d
Reviewed-by: Ivan Donchevskii <ivan.donchevskii@qt.io>

src/libs/clangsupport/generatedfiles.cpp

@@ -100,6 +100,15 @@ void GeneratedFiles::remove(const FilePaths &filePaths)
     m_fileContainers = std::move(differenceFileContainers);
 }
 
+bool GeneratedFiles::isValid() const
+{
+    return std::all_of(m_fileContainers.begin(),
+                       m_fileContainers.end(),
+                       [](const V2::FileContainer &container) {
+                           return container.unsavedFileContent.hasContent();
+                       });
+}
+
 const V2::FileContainers &GeneratedFiles::fileContainers() const
 {
     return m_fileContainers;

src/libs/clangsupport/generatedfiles.h

@@ -35,6 +35,7 @@ public:
     void update(V2::FileContainers &&fileContainers);
     void update(const V2::FileContainers &fileContainers);
     void remove(const FilePaths &filePaths);
+    bool isValid() const;
 
     const V2::FileContainers &fileContainers() const;
 

src/libs/clangsupport/generatedfilesinterface.h

@@ -35,6 +35,7 @@ public:
     virtual void update(V2::FileContainers &&fileContainers) = 0;
     virtual void update(const V2::FileContainers &fileContainers) = 0;
     virtual void remove(const FilePaths &filePaths) = 0;
+    virtual bool isValid() const = 0;
 
     virtual const V2::FileContainers &fileContainers() const = 0;
 

src/libs/clangsupport/projectpartcontainer.h

@@ -156,6 +156,7 @@ public:
     Utils::Language language = Utils::Language::Cxx;
     Utils::LanguageVersion languageVersion = Utils::LanguageVersion::CXX98;
     Utils::LanguageExtension languageExtension = Utils::LanguageExtension::None;
+    bool updateIsDeferred = false;
 };
 
 using ProjectPartContainers = std::vector<ProjectPartContainer>;

src/tools/clangpchmanagerbackend/source/pchmanagerserver.cpp

@@ -61,8 +61,12 @@ void PchManagerServer::updateProjectParts(UpdateProjectPartsMessage &&message)
 {
     m_toolChainsArgumentsCache.update(message.projectsParts, message.toolChainArguments);
 
-    m_pchTaskGenerator.addProjectParts(
+    ProjectPartContainers newProjectParts = m_projectParts.update(message.takeProjectsParts());
-        m_projectParts.update(message.takeProjectsParts()), std::move(message.toolChainArguments));
+
+    if (m_generatedFiles.isValid()) {
+        m_pchTaskGenerator.addProjectParts(std::move(newProjectParts),
+                                           std::move(message.toolChainArguments));
+    }
 }
 
 void PchManagerServer::removeProjectParts(RemoveProjectPartsMessage &&message)
@@ -76,9 +80,35 @@ void PchManagerServer::removeProjectParts(RemoveProjectPartsMessage &&message)
     m_toolChainsArgumentsCache.remove(message.projectsPartIds);
 }
 
+namespace {
+Utils::SmallStringVector projectPartIds(const ProjectPartContainers &projectParts)
+{
+    Utils::SmallStringVector ids;
+    ids.reserve(projectParts.size());
+
+    std::transform(projectParts.cbegin(),
+                   projectParts.cend(),
+                   std::back_inserter(ids),
+                   [](const ProjectPartContainer &projectPart) { return projectPart.projectPartId; });
+
+    return ids;
+}
+}
+
 void PchManagerServer::updateGeneratedFiles(UpdateGeneratedFilesMessage &&message)
 {
     m_generatedFiles.update(message.takeGeneratedFiles());
+
+    if (m_generatedFiles.isValid()) {
+        ProjectPartContainers deferredProjectParts = m_projectParts.deferredUpdates();
+        ArgumentsEntries entries = m_toolChainsArgumentsCache.arguments(
+            projectPartIds(deferredProjectParts));
+
+        for (ArgumentsEntry &entry : entries) {
+            m_pchTaskGenerator.addProjectParts(std::move(deferredProjectParts),
+                                               std::move(entry.arguments));
+        }
+    }
 }
 
 void PchManagerServer::removeGeneratedFiles(RemoveGeneratedFilesMessage &&message)

src/tools/clangpchmanagerbackend/source/projectparts.cpp

@@ -66,6 +66,59 @@ ProjectPartContainers ProjectParts::projects(const Utils::SmallStringVector &pro
     return projectPartsWithIds;
 }
 
+void ProjectParts::updateDeferred(const ProjectPartContainers &deferredProjectsParts)
+{
+    using ProjectPartContainerReferences = std::vector<std::reference_wrapper<ProjectPartContainer>>;
+
+    class BackInserterIterator : public std::back_insert_iterator<ProjectPartContainerReferences>
+    {
+    public:
+        BackInserterIterator(ProjectPartContainerReferences &container)
+            : std::back_insert_iterator<ProjectPartContainerReferences>(container)
+        {}
+
+        BackInserterIterator &operator=(ProjectPartContainer &projectPart)
+        {
+            container->push_back(std::ref(projectPart));
+
+            return *this;
+        }
+
+        BackInserterIterator &operator*() { return *this; }
+    };
+
+    ProjectPartContainerReferences deferredProjectPartPointers;
+    deferredProjectPartPointers.reserve(deferredProjectsParts.size());
+
+    std::set_intersection(m_projectParts.begin(),
+                          m_projectParts.end(),
+                          deferredProjectsParts.begin(),
+                          deferredProjectsParts.end(),
+                          BackInserterIterator(deferredProjectPartPointers),
+                          [](const ProjectPartContainer &first, const ProjectPartContainer &second) {
+                              return first.projectPartId < second.projectPartId;
+                          });
+
+    for (ProjectPartContainer &projectPart : deferredProjectPartPointers)
+        projectPart.updateIsDeferred = true;
+}
+
+ProjectPartContainers ProjectParts::deferredUpdates()
+{
+    ProjectPartContainers deferredProjectParts;
+    deferredProjectParts.reserve(m_projectParts.size());
+
+    std::copy_if(m_projectParts.cbegin(),
+                 m_projectParts.cend(),
+                 std::back_inserter(deferredProjectParts),
+                 [](const ProjectPartContainer &projectPart) { return projectPart.updateIsDeferred; });
+
+    for (ProjectPartContainer &projectPart : m_projectParts)
+        projectPart.updateIsDeferred = false;
+
+    return deferredProjectParts;
+}
+
 ProjectPartContainers ProjectParts::newProjectParts(ProjectPartContainers &&projectsParts) const
 {
     ProjectPartContainers updatedProjectPartContainers;

src/tools/clangpchmanagerbackend/source/projectparts.h

@@ -41,8 +41,9 @@ public:
     ProjectPartContainers update(ProjectPartContainers &&projectsParts) override;
     void remove(const Utils::SmallStringVector &projectPartIds) override;
     ProjectPartContainers projects(const Utils::SmallStringVector &projectPartIds) const override;
+    void updateDeferred(const ProjectPartContainers &projectsParts) override;
+    ProjectPartContainers deferredUpdates() override;
 
-unittest_public:
     ProjectPartContainers newProjectParts(ProjectPartContainers &&projectsParts) const;
     void mergeProjectParts(const ProjectPartContainers &projectsParts);
     const ProjectPartContainers &projectParts() const;

src/tools/clangpchmanagerbackend/source/projectpartsinterface.h

@@ -39,7 +39,8 @@ public:
     virtual ProjectPartContainers update(ProjectPartContainers &&projectsParts) = 0;
     virtual void remove(const Utils::SmallStringVector &projectPartIds) = 0;
     virtual ProjectPartContainers projects(const Utils::SmallStringVector &projectPartIds) const = 0;
-
+    virtual void updateDeferred(const ProjectPartContainers &projectsParts) = 0;
+    virtual ProjectPartContainers deferredUpdates() = 0;
 protected:
     ~ProjectPartsInterface() = default;
 };

tests/unit/unittest/generatedfiles-test.cpp

@@ -69,4 +69,21 @@ TEST_F(GeneratedFiles, RemoveGeneratedFiles)
     ASSERT_THAT(generatedFiles.fileContainers(), ElementsAre(file1, file3));
 }
 
+TEST_F(GeneratedFiles, IsValidForNoGeneratedFiles)
+{
+    ASSERT_TRUE(generatedFiles.isValid());
 }
+
+TEST_F(GeneratedFiles, IsNotValidIfFilesWithNotContentExists)
+{
+    generatedFiles.update({{"/file2", ""}});
+
+    ASSERT_FALSE(generatedFiles.isValid());
+}
+
+TEST_F(GeneratedFiles, IsValidIfAllFilesHasContent)
+{
+    generatedFiles.update({file1, file2, file3, file4});
+
+    ASSERT_TRUE(generatedFiles.isValid());
+}}

tests/unit/unittest/mockgeneratedfiles.h

@@ -38,6 +38,7 @@ public:
                  void (const ClangBackEnd::FilePaths &filePaths));
     MOCK_CONST_METHOD0(fileContainers,
                        const ClangBackEnd::V2::FileContainers &());
+    MOCK_CONST_METHOD0(isValid, bool());
 
     void update(ClangBackEnd::V2::FileContainers &&fileContainers)
     {

tests/unit/unittest/mockprojectparts.h

@@ -33,11 +33,13 @@ class MockProjectParts : public ClangBackEnd::ProjectPartsInterface
 {
 public:
     MOCK_METHOD1(update,
-                 ClangBackEnd::ProjectPartContainers(const ClangBackEnd::ProjectPartContainers &projectsParts));
+                 ClangBackEnd::ProjectPartContainers(
-    MOCK_METHOD1(remove,
+                     const ClangBackEnd::ProjectPartContainers &projectsParts));
-                 void(const Utils::SmallStringVector &projectPartIds));
+    MOCK_METHOD1(remove, void(const Utils::SmallStringVector &projectPartIds));
-    MOCK_CONST_METHOD1(projects,
+    MOCK_CONST_METHOD1(
-                       ClangBackEnd::ProjectPartContainers(const Utils::SmallStringVector &projectPartIds));
+        projects, ClangBackEnd::ProjectPartContainers(const Utils::SmallStringVector &projectPartIds));
+    MOCK_METHOD1(updateDeferred, void(const ClangBackEnd::ProjectPartContainers &projectsParts));
+    MOCK_METHOD0(deferredUpdates, ClangBackEnd::ProjectPartContainers());
 
     ClangBackEnd::ProjectPartContainers update(ClangBackEnd::ProjectPartContainers &&projectsParts) override
     {

tests/unit/unittest/pchmanagerserver-test.cpp

@@ -54,8 +54,8 @@ class PchManagerServer : public ::testing::Test
     {
         server.setClient(&mockPchManagerClient);
 
-        ON_CALL(mockProjectParts, update(projectParts))
+        ON_CALL(mockProjectParts, update(projectParts)).WillByDefault(Return(projectParts));
-                .WillByDefault(Return(projectParts));
+        ON_CALL(mockGeneratedFiles, isValid()).WillByDefault(Return(true));
    }
 
     ClangBackEnd::FilePathId id(Utils::SmallStringView path) const
@@ -183,7 +183,6 @@ TEST_F(PchManagerServer, SetProgress)
     server.setProgress(20, 30);
 }
 
-
 TEST_F(PchManagerServer, RemoveToolChainsArguments)
 {
     server.updateProjectParts(
@@ -195,5 +194,60 @@ TEST_F(PchManagerServer, RemoveToolChainsArguments)
     server.pathsWithIdsChanged({projectPart1.projectPartId});
 }
 
+TEST_F(PchManagerServer, DontGeneratePchIfGeneratedFilesAreNotValid)
+{
+    InSequence s;
+
+    EXPECT_CALL(mockGeneratedFiles, isValid()).WillOnce(Return(false));
+    EXPECT_CALL(mockPchTaskGenerator, addProjectParts(_, _)).Times(0);
+
+    server.updateProjectParts(
+        ClangBackEnd::UpdateProjectPartsMessage{{projectPart1}, {"toolChainArgument"}});
+}
+
+TEST_F(PchManagerServer, GeneratePchIfGeneratedFilesAreValid)
+{
+    InSequence s;
+
+    EXPECT_CALL(mockGeneratedFiles, isValid()).WillOnce(Return(true));
+    EXPECT_CALL(mockPchTaskGenerator, addProjectParts(_, _));
+
+    server.updateProjectParts(
+        ClangBackEnd::UpdateProjectPartsMessage{{projectPart1}, {"toolChainArgument"}});
+}
+
+TEST_F(PchManagerServer, AfterUpdatingGeneratedFilesAreValidSoGeneratePchs)
+{
+    InSequence s;
+    ClangBackEnd::UpdateGeneratedFilesMessage updateGeneratedFilesMessage{{generatedFile}};
+    ON_CALL(mockGeneratedFiles, isValid()).WillByDefault(Return(false));
+    server.updateProjectParts(
+        ClangBackEnd::UpdateProjectPartsMessage{{projectPart1}, {"toolChainArgument"}});
+
+    EXPECT_CALL(mockGeneratedFiles, update(updateGeneratedFilesMessage.generatedFiles));
+    EXPECT_CALL(mockGeneratedFiles, isValid()).WillOnce(Return(true));
+    EXPECT_CALL(mockProjectParts, deferredUpdates())
+        .WillOnce(Return(ClangBackEnd::ProjectPartContainers{projectPart1}));
+    EXPECT_CALL(mockPchTaskGenerator,
+                addProjectParts(ElementsAre(projectPart1), ElementsAre("toolChainArgument")));
+
+    server.updateGeneratedFiles(updateGeneratedFilesMessage.clone());
+}
+
+TEST_F(PchManagerServer, AfterUpdatingGeneratedFilesAreStillInvalidSoNoPchsGeneration)
+{
+    InSequence s;
+    ClangBackEnd::UpdateGeneratedFilesMessage updateGeneratedFilesMessage{{generatedFile}};
+    ON_CALL(mockGeneratedFiles, isValid()).WillByDefault(Return(false));
+    server.updateProjectParts(
+        ClangBackEnd::UpdateProjectPartsMessage{{projectPart1}, {"toolChainArgument"}});
+
+    EXPECT_CALL(mockGeneratedFiles, update(updateGeneratedFilesMessage.generatedFiles));
+    EXPECT_CALL(mockGeneratedFiles, isValid()).WillOnce(Return(false));
+    EXPECT_CALL(mockProjectParts, deferredUpdates()).Times(0);
+    EXPECT_CALL(mockPchTaskGenerator, addProjectParts(_, _)).Times(0);
+
+    server.updateGeneratedFiles(updateGeneratedFilesMessage.clone());
+}
 
 }

tests/unit/unittest/projectparts-test.cpp

@@ -191,7 +191,6 @@ TEST_F(ProjectParts, GetProjectById)
     ASSERT_THAT(projectPartContainers, ElementsAre(projectPartContainer1));
 }
 
-
 TEST_F(ProjectParts, GetProjectsByIds)
 {
     projectParts.update({projectPartContainer1, projectPartContainer2});
@@ -201,4 +200,31 @@ TEST_F(ProjectParts, GetProjectsByIds)
     ASSERT_THAT(projectPartContainers, UnorderedElementsAre(projectPartContainer1, projectPartContainer2));
 }
 
+TEST_F(ProjectParts, UpdateDeferred)
+{
+    auto projectPartContainers = projectParts.update({projectPartContainer1, projectPartContainer2});
+
+    projectParts.updateDeferred({projectPartContainer1});
+
+    ASSERT_THAT(projectParts.deferredUpdates(), ElementsAre(projectPartContainer1));
+
+}
+
+TEST_F(ProjectParts, NotUpdateDeferred)
+{
+    auto projectPartContainers = projectParts.update({projectPartContainer1, projectPartContainer2});
+
+    ASSERT_THAT(projectParts.deferredUpdates(), IsEmpty());
+}
+
+TEST_F(ProjectParts, UpdateDeferredCleansDeferredUpdates)
+{
+    auto projectPartContainers = projectParts.update({projectPartContainer1, projectPartContainer2});
+    projectParts.updateDeferred({projectPartContainer1});
+
+    projectParts.deferredUpdates();
+
+    ASSERT_THAT(projectParts.deferredUpdates(), IsEmpty());
+
+}
 }