From bca41fa9f66fb1fe60d738c807f5cb56118687e6 Mon Sep 17 00:00:00 2001
From: Marco Bubke <marco.bubke@qt.io>
Date: Tue, 25 Sep 2018 15:42:52 +0200
Subject: [PATCH] ClangRefactoring: Initialize sqlite database only once

Initializing the database is not cheap so it is better to initializing it
only once. We simply check if the database file already exists and then
skip the initializing step.

Task-number: QTCREATORBUG-21174
Change-Id: I151090c5081c009f7913a30517065be2833791d8
Reviewed-by: Ivan Donchevskii <ivan.donchevskii@qt.io>
---
 .../refactoringdatabaseinitializer.h          |  28 +++++++------
 src/libs/sqlite/sqlitedatabase.cpp            |  13 ++++++
 src/libs/sqlite/sqlitedatabase.h              |   6 ++-
 tests/unit/unittest/data/sqlite_database.db   | Bin 0 -> 4096 bytes
 tests/unit/unittest/mocksqlitedatabase.h      |   6 +++
 .../refactoringdatabaseinitializer-test.cpp   |  38 +++++++++++++++++-
 tests/unit/unittest/sqlitedatabase-test.cpp   |  27 +++++++++++++
 7 files changed, 104 insertions(+), 14 deletions(-)
 create mode 100644 tests/unit/unittest/data/sqlite_database.db

diff --git a/src/libs/clangsupport/refactoringdatabaseinitializer.h b/src/libs/clangsupport/refactoringdatabaseinitializer.h
index 0b212c3a58a..ab15cf81e0f 100644
--- a/src/libs/clangsupport/refactoringdatabaseinitializer.h
+++ b/src/libs/clangsupport/refactoringdatabaseinitializer.h
@@ -39,20 +39,24 @@ public:
     RefactoringDatabaseInitializer(DatabaseType &database)
         : database(database)
     {
-        Sqlite::ImmediateTransaction transaction{database};
+        if (!database.isInitialized()) {
+            Sqlite::ExclusiveTransaction transaction{database};
 
-        createSymbolsTable();
-        createLocationsTable();
-        createSourcesTable();
-        createDirectoriesTable();
-        createProjectPartsTable();
-        createProjectPartsSourcesTable();
-        createUsedMacrosTable();
-        createFileStatusesTable();
-        createSourceDependenciesTable();
-        createPrecompiledHeadersTable();
+            createSymbolsTable();
+            createLocationsTable();
+            createSourcesTable();
+            createDirectoriesTable();
+            createProjectPartsTable();
+            createProjectPartsSourcesTable();
+            createUsedMacrosTable();
+            createFileStatusesTable();
+            createSourceDependenciesTable();
+            createPrecompiledHeadersTable();
 
-        transaction.commit();
+            transaction.commit();
+
+            database.setIsInitialized(true);
+        }
     }
 
     void createSymbolsTable()
diff --git a/src/libs/sqlite/sqlitedatabase.cpp b/src/libs/sqlite/sqlitedatabase.cpp
index a90c8caff0f..1ddaf2cfbcb 100644
--- a/src/libs/sqlite/sqlitedatabase.cpp
+++ b/src/libs/sqlite/sqlitedatabase.cpp
@@ -29,6 +29,8 @@
 #include "sqlitetransaction.h"
 #include "sqlitereadwritestatement.h"
 
+#include <QFileInfo>
+
 #include <chrono>
 
 using namespace std::chrono_literals;
@@ -85,6 +87,7 @@ void Database::open()
 
 void Database::open(Utils::PathString &&databaseFilePath)
 {
+    m_isInitialized = QFileInfo::exists(QString(databaseFilePath));
     setDatabaseFilePath(std::move(databaseFilePath));
     open();
 }
@@ -96,6 +99,16 @@ void Database::close()
     m_databaseBackend.close();
 }
 
+bool Database::isInitialized() const
+{
+    return m_isInitialized;
+}
+
+void Database::setIsInitialized(bool isInitialized)
+{
+    m_isInitialized = isInitialized;
+}
+
 bool Database::isOpen() const
 {
     return m_isOpen;
diff --git a/src/libs/sqlite/sqlitedatabase.h b/src/libs/sqlite/sqlitedatabase.h
index ad82f6eaa62..d0eae28f790 100644
--- a/src/libs/sqlite/sqlitedatabase.h
+++ b/src/libs/sqlite/sqlitedatabase.h
@@ -56,7 +56,7 @@ public:
 
     Database();
     Database(Utils::PathString &&databaseFilePath,
-             JournalMode journalMode=JournalMode::Wal);
+             JournalMode journalMode);
     Database(Utils::PathString &&databaseFilePath,
              std::chrono::milliseconds busyTimeout = 1000ms,
              JournalMode journalMode=JournalMode::Wal);
@@ -69,6 +69,9 @@ public:
     void open(Utils::PathString &&databaseFilePath);
     void close();
 
+    bool isInitialized() const;
+    void setIsInitialized(bool isInitialized);
+
     bool isOpen() const;
 
     Table &addTable();
@@ -133,6 +136,7 @@ private:
     JournalMode m_journalMode = JournalMode::Wal;
     OpenMode m_openMode = OpenMode::ReadWrite;
     bool m_isOpen = false;
+    bool m_isInitialized = false;
 };
 
 } // namespace Sqlite
diff --git a/tests/unit/unittest/data/sqlite_database.db b/tests/unit/unittest/data/sqlite_database.db
new file mode 100644
index 0000000000000000000000000000000000000000..9c5879579e55a731615ad4ea4725f6c658f17751
GIT binary patch
literal 4096
zcmWFz^vNtqRY=P(%1ta$FlG>7U}9o$P*7lCU|@t|1_}TpgPz0*UXTF-AYv4chQMeD
mjE2By2#kinXb6mkz-S1JhQMeDjE2By2#kinXb22M2mk<b-3IXh

literal 0
HcmV?d00001

diff --git a/tests/unit/unittest/mocksqlitedatabase.h b/tests/unit/unittest/mocksqlitedatabase.h
index 36fb370e707..0b890e01f0a 100644
--- a/tests/unit/unittest/mocksqlitedatabase.h
+++ b/tests/unit/unittest/mocksqlitedatabase.h
@@ -50,5 +50,11 @@ public:
 
     MOCK_CONST_METHOD1(setLastInsertedRowId,
                        void (int64_t));
+
+    MOCK_CONST_METHOD0(isInitialized,
+                      bool ());
+
+    MOCK_METHOD1(setIsInitialized,
+                 void (bool));
 };
 
diff --git a/tests/unit/unittest/refactoringdatabaseinitializer-test.cpp b/tests/unit/unittest/refactoringdatabaseinitializer-test.cpp
index 6b9257facf0..347802f3503 100644
--- a/tests/unit/unittest/refactoringdatabaseinitializer-test.cpp
+++ b/tests/unit/unittest/refactoringdatabaseinitializer-test.cpp
@@ -148,7 +148,8 @@ TEST_F(RefactoringDatabaseInitializer, CreateInTheContructor)
 {
     InSequence s;
 
-    EXPECT_CALL(mockDatabase, immediateBegin());
+    EXPECT_CALL(mockDatabase, isInitialized()).WillOnce(Return(false));
+    EXPECT_CALL(mockDatabase, exclusiveBegin());
     EXPECT_CALL(mockDatabase, execute(Eq("CREATE TABLE IF NOT EXISTS symbols(symbolId INTEGER PRIMARY KEY, usr TEXT, symbolName TEXT, symbolKind INTEGER, signature TEXT)")));
     EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_symbols_usr ON symbols(usr)")));
     EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_symbols_symbolKind_symbolName ON symbols(symbolKind, symbolName)")));
@@ -172,7 +173,42 @@ TEST_F(RefactoringDatabaseInitializer, CreateInTheContructor)
     EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_sourceDependencies_sourceId_dependencySourceId ON sourceDependencies(sourceId, dependencySourceId)")));
     EXPECT_CALL(mockDatabase, execute(Eq("CREATE TABLE IF NOT EXISTS precompiledHeaders(projectPartId INTEGER PRIMARY KEY, pchPath TEXT, pchBuildTime INTEGER)")));
     EXPECT_CALL(mockDatabase, commit());
+    EXPECT_CALL(mockDatabase, setIsInitialized(true));
 
     Initializer initializer{mockDatabase};
 }
+
+TEST_F(RefactoringDatabaseInitializer, DontCreateIfAlreadyInitialized)
+{
+    InSequence s;
+
+    EXPECT_CALL(mockDatabase, isInitialized()).WillOnce(Return(true));
+    EXPECT_CALL(mockDatabase, exclusiveBegin()).Times(0);
+    EXPECT_CALL(mockDatabase, execute(Eq("CREATE TABLE IF NOT EXISTS symbols(symbolId INTEGER PRIMARY KEY, usr TEXT, symbolName TEXT, symbolKind INTEGER, signature TEXT)"))).Times(0);
+    EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_symbols_usr ON symbols(usr)"))).Times(0);
+    EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_symbols_symbolKind_symbolName ON symbols(symbolKind, symbolName)"))).Times(0);
+    EXPECT_CALL(mockDatabase, execute(Eq("CREATE TABLE IF NOT EXISTS locations(symbolId INTEGER, line INTEGER, column INTEGER, sourceId INTEGER, locationKind INTEGER)"))).Times(0);
+    EXPECT_CALL(mockDatabase, execute(Eq("CREATE UNIQUE INDEX IF NOT EXISTS index_locations_sourceId_line_column ON locations(sourceId, line, column)"))).Times(0);
+    EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_locations_sourceId_locationKind ON locations(sourceId, locationKind)"))).Times(0);
+    EXPECT_CALL(mockDatabase, execute(Eq("CREATE TABLE IF NOT EXISTS sources(sourceId INTEGER PRIMARY KEY, directoryId INTEGER, sourceName TEXT, sourceType INTEGER)"))).Times(0);
+    EXPECT_CALL(mockDatabase, execute(Eq("CREATE UNIQUE INDEX IF NOT EXISTS index_sources_directoryId_sourceName ON sources(directoryId, sourceName)"))).Times(0);
+    EXPECT_CALL(mockDatabase, execute(Eq("CREATE TABLE IF NOT EXISTS directories(directoryId INTEGER PRIMARY KEY, directoryPath TEXT)"))).Times(0);
+    EXPECT_CALL(mockDatabase, execute(Eq("CREATE UNIQUE INDEX IF NOT EXISTS index_directories_directoryPath ON directories(directoryPath)"))).Times(0);
+    EXPECT_CALL(mockDatabase, execute(Eq("CREATE TABLE IF NOT EXISTS projectParts(projectPartId INTEGER PRIMARY KEY, projectPartName TEXT, compilerArguments TEXT, compilerMacros TEXT, includeSearchPaths TEXT)"))).Times(0);
+    EXPECT_CALL(mockDatabase, execute(Eq("CREATE UNIQUE INDEX IF NOT EXISTS index_projectParts_projectPartName ON projectParts(projectPartName)"))).Times(0);
+    EXPECT_CALL(mockDatabase, execute(Eq("CREATE TABLE IF NOT EXISTS projectPartsSources(projectPartId INTEGER, sourceId INTEGER)"))).Times(0);
+    EXPECT_CALL(mockDatabase, execute(Eq("CREATE UNIQUE INDEX IF NOT EXISTS index_projectPartsSources_sourceId_projectPartId ON projectPartsSources(sourceId, projectPartId)"))).Times(0);
+    EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_projectPartsSources_projectPartId ON projectPartsSources(projectPartId)"))).Times(0);
+    EXPECT_CALL(mockDatabase, execute(Eq("CREATE TABLE IF NOT EXISTS usedMacros(usedMacroId INTEGER PRIMARY KEY, sourceId INTEGER, macroName TEXT)"))).Times(0);
+    EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_usedMacros_sourceId_macroName ON usedMacros(sourceId, macroName)"))).Times(0);
+    EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_usedMacros_macroName ON usedMacros(macroName)"))).Times(0);
+    EXPECT_CALL(mockDatabase, execute(Eq("CREATE TABLE IF NOT EXISTS fileStatuses(sourceId INTEGER PRIMARY KEY, size INTEGER, lastModified INTEGER, isInPrecompiledHeader INTEGER)"))).Times(0);
+    EXPECT_CALL(mockDatabase, execute(Eq("CREATE TABLE IF NOT EXISTS sourceDependencies(sourceId INTEGER, dependencySourceId INTEGER)"))).Times(0);
+    EXPECT_CALL(mockDatabase, execute(Eq("CREATE INDEX IF NOT EXISTS index_sourceDependencies_sourceId_dependencySourceId ON sourceDependencies(sourceId, dependencySourceId)"))).Times(0);
+    EXPECT_CALL(mockDatabase, execute(Eq("CREATE TABLE IF NOT EXISTS precompiledHeaders(projectPartId INTEGER PRIMARY KEY, pchPath TEXT, pchBuildTime INTEGER)"))).Times(0);
+    EXPECT_CALL(mockDatabase, commit()).Times(0);
+
+    Initializer initializer{mockDatabase};
+}
+
 }
diff --git a/tests/unit/unittest/sqlitedatabase-test.cpp b/tests/unit/unittest/sqlitedatabase-test.cpp
index 8ccc2f652c6..f003fac0e09 100644
--- a/tests/unit/unittest/sqlitedatabase-test.cpp
+++ b/tests/unit/unittest/sqlitedatabase-test.cpp
@@ -33,6 +33,7 @@
 #include <utf8string.h>
 
 #include <QSignalSpy>
+#include <QTemporaryFile>
 #include <QVariant>
 
 namespace {
@@ -105,6 +106,32 @@ TEST_F(SqliteDatabase, CloseDatabase)
     ASSERT_FALSE(database.isOpen());
 }
 
+TEST_F(SqliteDatabase, DatabaseIsNotInitializedAfterOpening)
+{
+    ASSERT_FALSE(database.isInitialized());
+}
+
+TEST_F(SqliteDatabase, DatabaseIsIntializedAfterSettingItBeforeOpening)
+{
+    database.setIsInitialized(true);
+
+    ASSERT_TRUE(database.isInitialized());
+}
+
+TEST_F(SqliteDatabase, DatabaseIsInitializedIfDatabasePathExistsAtOpening)
+{
+    Sqlite::Database database{TESTDATA_DIR "/sqlite_database.db"};
+
+    ASSERT_TRUE(database.isInitialized());
+}
+
+TEST_F(SqliteDatabase, DatabaseIsNotInitializedIfDatabasePathDoesNotExistAtOpening)
+{
+    Sqlite::Database database{Utils::PathString{QDir::tempPath() + "/database_does_not_exist.db"}};
+
+    ASSERT_FALSE(database.isInitialized());
+}
+
 TEST_F(SqliteDatabase, AddTable)
 {
     auto sqliteTable = database.addTable();