From a1ac5450860ce018f9b996cc6df5b40a3fa13d1d Mon Sep 17 00:00:00 2001 From: eelke Date: Sun, 18 Oct 2026 12:32:16 +0200 Subject: [PATCH] Add tests for loading the catalog --- .idea/pgLab.iml | 2 - pglab/catalog/models/DatabasesTableModel.cpp | 19 +----- pglab/catalog/models/DatabasesTableModel.h | 14 +--- pglablib/CMakeLists.txt | 2 + pglablib/catalog/PgDatabaseSizes.cpp | 25 +++++++ pglablib/catalog/PgDatabaseSizes.h | 25 +++++++ tests/integration/CMakeLists.txt | 3 + tests/integration/tst_CatalogLoad.cpp | 36 +++++++++++ tests/integration/tst_DatabaseSizes.cpp | 68 ++++++++++++++++++++ tests/pgintegration/DbTest.cpp | 7 ++ tests/pgintegration/DbTest.h | 2 + tests/pgintegration/PgTestEnvironment.cpp | 26 +++++++- tests/pgintegration/PgTestEnvironment.h | 6 ++ 13 files changed, 203 insertions(+), 32 deletions(-) delete mode 100644 .idea/pgLab.iml create mode 100644 pglablib/catalog/PgDatabaseSizes.cpp create mode 100644 pglablib/catalog/PgDatabaseSizes.h create mode 100644 tests/integration/tst_CatalogLoad.cpp create mode 100644 tests/integration/tst_DatabaseSizes.cpp diff --git a/.idea/pgLab.iml b/.idea/pgLab.iml deleted file mode 100644 index 4c94235..0000000 --- a/.idea/pgLab.iml +++ /dev/null @@ -1,2 +0,0 @@ - - \ No newline at end of file diff --git a/pglab/catalog/models/DatabasesTableModel.cpp b/pglab/catalog/models/DatabasesTableModel.cpp index 289eea2..789c981 100644 --- a/pglab/catalog/models/DatabasesTableModel.cpp +++ b/pglab/catalog/models/DatabasesTableModel.cpp @@ -205,26 +205,9 @@ void DatabasesTableModel::StartLoadDatabaseSizes(std::map oidIndex) DatabasesTableModel::DatabaseSizes DatabasesTableModel::QueryDatabaseSizes() { - std::string q = - "SELECT pg_database.oid, " - " case when has_database_privilege(current_role, pg_database.oid, \n" - " 'connect') then pg_database_size(pg_database.oid) else -1 end \n" - "FROM pg_database"; - Pgsql::Connection connection; connection.connect(openDatabase->config().connectionString()); - Pgsql::Result sizesResult = connection.query(q.c_str()); - - DatabaseSizes result; - result.reserve(sizesResult.rows()); - for (auto& row : sizesResult) - { - DatabaseSize ds; - ds.oid = row.get(0); - ds.totalBytes = row.get(1); - result.push_back(ds); - } - return result; + return loadDatabaseSizes(connection); } void DatabasesTableModel::PopulateSizes(std::map oidIndex, DatabaseSizes sizes) diff --git a/pglab/catalog/models/DatabasesTableModel.h b/pglab/catalog/models/DatabasesTableModel.h index 8e46aaa..c25550c 100644 --- a/pglab/catalog/models/DatabasesTableModel.h +++ b/pglab/catalog/models/DatabasesTableModel.h @@ -3,6 +3,7 @@ #include "BaseTableModel.h" #include "catalog/PgDatabase.h" +#include "catalog/PgDatabaseSizes.h" #include class OpenDatabase; @@ -45,17 +46,8 @@ public: protected: virtual QVariant getDataMeaning(const QModelIndex &index) const override; private: - class DatabaseSize { - public: - Oid oid; - int64_t totalBytes; - - DatabaseSize() - : oid(InvalidOid) - , totalBytes(-1) - {} - }; - using DatabaseSizes = std::vector; + using DatabaseSize = PgDatabaseSize; + using DatabaseSizes = PgDatabaseSizes; class Database { public: diff --git a/pglablib/CMakeLists.txt b/pglablib/CMakeLists.txt index 1734d09..249e59c 100644 --- a/pglablib/CMakeLists.txt +++ b/pglablib/CMakeLists.txt @@ -21,6 +21,7 @@ add_library(pglablib STATIC catalog/PgIndexContainer.cpp catalog/PgConstraint.cpp catalog/PgConstraintContainer.cpp + catalog/PgDatabaseSizes.cpp ParamListJson.cpp ParamListModel.cpp sqlite/SQLiteConnection.cpp @@ -95,6 +96,7 @@ add_library(pglablib STATIC catalog/PgIndexContainer.h catalog/PgConstraint.h catalog/PgConstraintContainer.h + catalog/PgDatabaseSizes.h ParamListJson.h ParamListModel.h sqlite/SQLiteConnection.h diff --git a/pglablib/catalog/PgDatabaseSizes.cpp b/pglablib/catalog/PgDatabaseSizes.cpp new file mode 100644 index 0000000..244509a --- /dev/null +++ b/pglablib/catalog/PgDatabaseSizes.cpp @@ -0,0 +1,25 @@ +#include "PgDatabaseSizes.h" + +#include "Pgsql_Result.h" + +PgDatabaseSizes loadDatabaseSizes(Pgsql::Connection &connection) +{ + std::string q = + "SELECT pg_database.oid, " + " case when has_database_privilege(current_role, pg_database.oid, \n" + " 'connect') then pg_database_size(pg_database.oid) else -1 end \n" + "FROM pg_database"; + + Pgsql::Result sizesResult = connection.query(q.c_str()); + + PgDatabaseSizes result; + result.reserve(sizesResult.rows()); + for (auto& row : sizesResult) + { + PgDatabaseSize ds; + ds.oid = row.get(0); + ds.totalBytes = row.get(1); + result.push_back(ds); + } + return result; +} diff --git a/pglablib/catalog/PgDatabaseSizes.h b/pglablib/catalog/PgDatabaseSizes.h new file mode 100644 index 0000000..3c39671 --- /dev/null +++ b/pglablib/catalog/PgDatabaseSizes.h @@ -0,0 +1,25 @@ +#ifndef PGDATABASESIZES_H +#define PGDATABASESIZES_H + +#include "Pgsql_Connection.h" +#include +#include +#include + +class PgDatabaseSize { +public: + Oid oid; + int64_t totalBytes; + + PgDatabaseSize() + : oid(InvalidOid) + , totalBytes(-1) + {} +}; + +using PgDatabaseSizes = std::vector; + +/// Returns the size of every database. The size is -1 when the current role has no CONNECT privilege. +PgDatabaseSizes loadDatabaseSizes(Pgsql::Connection &connection); + +#endif // PGDATABASESIZES_H diff --git a/tests/integration/CMakeLists.txt b/tests/integration/CMakeLists.txt index 42c908b..10a87d0 100644 --- a/tests/integration/CMakeLists.txt +++ b/tests/integration/CMakeLists.txt @@ -1,10 +1,13 @@ add_executable(IntegrationTests main.cpp tst_Smoke.cpp + tst_CatalogLoad.cpp + tst_DatabaseSizes.cpp ) target_link_libraries(IntegrationTests PRIVATE pgintegration + pglablib GTest::gtest Qt6::Core ) diff --git a/tests/integration/tst_CatalogLoad.cpp b/tests/integration/tst_CatalogLoad.cpp new file mode 100644 index 0000000..5993337 --- /dev/null +++ b/tests/integration/tst_CatalogLoad.cpp @@ -0,0 +1,36 @@ +#include "DbTest.h" +#include "PgTestEnvironment.h" +#include "catalog/PgClassContainer.h" +#include "catalog/PgDatabaseCatalog.h" +#include "Pgsql_Result.h" + +using namespace pgtest; + +class CatalogLoad : public DbTest {}; + +TEST_F(CatalogLoad, SuperuserLoadsCatalog) +{ + PgDatabaseCatalog catalog; + catalog.loadAll(conn(), {}); + + EXPECT_EQ(catalog.serverVersion(), serverVersionNum()); + EXPECT_EQ(catalog.getDBName(), dbName()); + ASSERT_NE(catalog.classes(), nullptr); + EXPECT_NE(catalog.classes()->getByObjectNsAndName("public", "fixture_item"), nullptr); +} + +TEST_F(CatalogLoad, PlainRoleLoadsCatalog) +{ + auto plain = plainConn(); + Pgsql::Result r = plain->query("SELECT rolsuper FROM pg_roles WHERE rolname = current_user"); + ASSERT_EQ(r.rows(), 1); + EXPECT_EQ(QString::fromUtf8(r.val(0, 0)), QString("f")); + + PgDatabaseCatalog catalog; + catalog.loadAll(*plain, {}); + + EXPECT_EQ(catalog.serverVersion(), serverVersionNum()); + EXPECT_EQ(catalog.getDBName(), dbName()); + ASSERT_NE(catalog.classes(), nullptr); + EXPECT_NE(catalog.classes()->getByObjectNsAndName("public", "fixture_item"), nullptr); +} diff --git a/tests/integration/tst_DatabaseSizes.cpp b/tests/integration/tst_DatabaseSizes.cpp new file mode 100644 index 0000000..7ed8345 --- /dev/null +++ b/tests/integration/tst_DatabaseSizes.cpp @@ -0,0 +1,68 @@ +#include "DbTest.h" +#include "PgTestEnvironment.h" +#include "catalog/PgDatabaseSizes.h" +#include "Pgsql_Result.h" + +using namespace pgtest; + +namespace { + +Oid oidOfDatabase(Pgsql::Connection &c, const QString &db) +{ + Pgsql::Result r = c.query(QString("SELECT oid FROM pg_database WHERE datname = '%1'").arg(db)); + if (!r || r.rows() != 1) + return InvalidOid; + return QString::fromUtf8(r.val(0, 0)).toUInt(); +} + +const PgDatabaseSize *findSize(const PgDatabaseSizes &sizes, Oid oid) +{ + for (const auto &s : sizes) { + if (s.oid == oid) + return &s; + } + return nullptr; +} + +} // namespace + +class DatabaseSizes : public DbTest {}; + +TEST_F(DatabaseSizes, ConnectableDatabaseReturnsSize) +{ + auto plain = plainConn(); + Oid oid = oidOfDatabase(*plain, dbName()); + ASSERT_NE(oid, InvalidOid); + + PgDatabaseSizes sizes = loadDatabaseSizes(*plain); + const PgDatabaseSize *s = findSize(sizes, oid); + ASSERT_NE(s, nullptr); + EXPECT_GE(s->totalBytes, 0); +} + +TEST_F(DatabaseSizes, RevokedConnectReturnsMinusOne) +{ + Oid oid = oidOfDatabase(conn(), dbName()); + ASSERT_NE(oid, InvalidOid); + conn().query(QString("REVOKE CONNECT ON DATABASE %1 FROM PUBLIC").arg(quoteIdent(dbName()))); + + auto plain = std::make_unique(); + plain->connect(urlForDbAsPlainRole("postgres")); + + PgDatabaseSizes sizes = loadDatabaseSizes(*plain); + const PgDatabaseSize *s = findSize(sizes, oid); + ASSERT_NE(s, nullptr); + EXPECT_EQ(s->totalBytes, -1); +} + +TEST_F(DatabaseSizes, SuperuserSeesAllDatabases) +{ + PgDatabaseSizes sizes = loadDatabaseSizes(conn()); + + Pgsql::Result r = conn().query("SELECT count(*) FROM pg_database"); + ASSERT_EQ(r.rows(), 1); + EXPECT_EQ(static_cast(QString::fromUtf8(r.val(0, 0)).toULongLong()), sizes.size()); + + for (const auto &s : sizes) + EXPECT_GE(s.totalBytes, 0) << "database oid " << s.oid; +} diff --git a/tests/pgintegration/DbTest.cpp b/tests/pgintegration/DbTest.cpp index 5e80dc4..c6caaf2 100644 --- a/tests/pgintegration/DbTest.cpp +++ b/tests/pgintegration/DbTest.cpp @@ -15,6 +15,13 @@ QString DbTest::dbUrl() const return urlForDb(dbName_); } +std::unique_ptr DbTest::plainConn() const +{ + auto c = std::make_unique(); + c->connect(urlForDbAsPlainRole(dbName_)); + return c; +} + void DbTest::SetUp() { if (!available()) diff --git a/tests/pgintegration/DbTest.h b/tests/pgintegration/DbTest.h index 26fc457..37477a5 100644 --- a/tests/pgintegration/DbTest.h +++ b/tests/pgintegration/DbTest.h @@ -17,6 +17,8 @@ protected: Pgsql::Connection& conn() { return *conn_; } QString dbName() const { return dbName_; } QString dbUrl() const; + /// Opens a new connection to this test's database as the plain (non-superuser) role. + std::unique_ptr plainConn() const; private: QString dbName_; diff --git a/tests/pgintegration/PgTestEnvironment.cpp b/tests/pgintegration/PgTestEnvironment.cpp index 9a82dd2..e2557b2 100644 --- a/tests/pgintegration/PgTestEnvironment.cpp +++ b/tests/pgintegration/PgTestEnvironment.cpp @@ -16,6 +16,8 @@ namespace { bool g_available = false; int g_versionNum = 0; +const char *kPlainRolePassword = "pglab_plain_test"; + } // namespace std::optional baseUrl() @@ -56,6 +58,23 @@ QString templateDbName() return QString("tmpl_%1").arg(processId()); } +QString plainRoleName() +{ + return QString("tst_%1").arg(processId()); +} + +QString urlForDbAsPlainRole(const QString &db) +{ + auto base = baseUrl(); + if (!base) + throw std::runtime_error("TEST_DATABASE_URL is not set"); + QUrl url(*base); + url.setPath("/" + db); + url.setUserName(plainRoleName()); + url.setPassword(QString::fromUtf8(kPlainRolePassword)); + return url.toString(); +} + QString quoteIdent(const QString &name) { QString r = name; @@ -82,6 +101,9 @@ void PgEnv::SetUp() g_versionNum = QString::fromUtf8(r.val(0, 0)).toInt(); admin.query(QString("DROP DATABASE IF EXISTS %1 WITH (FORCE)").arg(quoteIdent(tmpl))); admin.query(QString("CREATE DATABASE %1").arg(quoteIdent(tmpl))); + admin.query(QString("DROP ROLE IF EXISTS %1").arg(quoteIdent(plainRoleName()))); + admin.query(QString("CREATE ROLE %1 LOGIN NOSUPERUSER NOCREATEDB NOCREATEROLE PASSWORD '%2'") + .arg(quoteIdent(plainRoleName()), QString::fromUtf8(kPlainRolePassword))); } { QFile f(fixtureDir_ + "/schema.sql"); @@ -101,6 +123,7 @@ void PgEnv::SetUp() Pgsql::Connection admin; admin.connect(*base); admin.query(QString("DROP DATABASE IF EXISTS %1 WITH (FORCE)").arg(quoteIdent(tmpl))); + admin.query(QString("DROP ROLE IF EXISTS %1").arg(quoteIdent(plainRoleName()))); } catch (const std::exception &) { } @@ -115,9 +138,10 @@ void PgEnv::TearDown() Pgsql::Connection admin; admin.connect(*baseUrl()); admin.query(QString("DROP DATABASE IF EXISTS %1 WITH (FORCE)").arg(quoteIdent(templateDbName()))); + admin.query(QString("DROP ROLE IF EXISTS %1").arg(quoteIdent(plainRoleName()))); } catch (const std::exception &e) { - ADD_FAILURE() << "Dropping template database failed: " << e.what(); + ADD_FAILURE() << "Dropping template database or plain role failed: " << e.what(); } g_available = false; } diff --git a/tests/pgintegration/PgTestEnvironment.h b/tests/pgintegration/PgTestEnvironment.h index b8782f0..0bf3f7a 100644 --- a/tests/pgintegration/PgTestEnvironment.h +++ b/tests/pgintegration/PgTestEnvironment.h @@ -24,6 +24,12 @@ qint64 processId(); /// Name of the template database for this process. QString templateDbName(); +/// Name of the login role without superuser rights, unique per process. +QString plainRoleName(); + +/// Same as urlForDb() but with the plain role as user. +QString urlForDbAsPlainRole(const QString &db); + /// Quotes an identifier for use in SQL. QString quoteIdent(const QString &name);