[Database] Database update improvements, content db and terminal checks (#3814)

* [Database] Database update improvements, content db and terminal checks

* Update manifest entries with content flag

* Update database_update_manifest.cpp

common/CMakeLists.txt

@@ -13,7 +13,6 @@ SET(common_sources
     crc32.cpp
     database/database_dump_service.cpp
     database.cpp
-    database_conversions.cpp
     database_instances.cpp
     database/database_update_manifest.cpp
     database/database_update_manifest_bots.cpp

common/database.h

@@ -226,11 +226,6 @@ public:
 
 	void    PurgeAllDeletedDataBuckets();
 
-	/* Database Conversions 'database_conversions.cpp' */
-
-	bool	CheckDatabaseConversions();
-	bool	CheckDatabaseConvertCorpseDeblob();
-	bool	CheckDatabaseConvertPPDeblob();
 
 	/* Database Variables */
 

common/database/database_update.cpp

@@ -76,9 +76,9 @@ void DatabaseUpdate::CheckDbUpdates()
 	}
 }
 
-std::string DatabaseUpdate::GetQueryResult(std::string query)
+std::string DatabaseUpdate::GetQueryResult(const ManifestEntry& e)
 {
-	auto results = m_database->QueryDatabase(query);
+	auto results = (e.content_schema_update ? m_content_database : m_database)->QueryDatabase(e.check);
 
 	std::vector<std::string> result_lines = {};
 
@@ -121,6 +121,16 @@ bool DatabaseUpdate::ShouldRunMigration(ManifestEntry &e, std::string query_resu
 	return false;
 }
 
+// check if we are running in a terminal
+bool is_atty()
+{
+#ifdef _WINDOWS
+	return ::_isatty(_fileno(stdin));
+#else
+	return isatty(fileno(stdin));
+#endif
+}
+
 // return true if we ran updates
 bool DatabaseUpdate::UpdateManifest(
 	std::vector<ManifestEntry> entries,
@@ -136,7 +146,7 @@ bool DatabaseUpdate::UpdateManifest(
 			for (auto &e: entries) {
 				if (e.version == version) {
 					bool        has_migration = true;
-					std::string r             = GetQueryResult(e.check);
+					std::string r             = GetQueryResult(e);
 					if (ShouldRunMigration(e, r)) {
 						has_migration = false;
 						missing_migrations.emplace_back(e.version);
@@ -179,7 +189,7 @@ bool DatabaseUpdate::UpdateManifest(
 				if (e.version == m) {
 					bool errored_migration = false;
 
-					auto r = m_database->QueryDatabaseMulti(e.sql);
+					auto r = (e.content_schema_update ? m_content_database : m_database)->QueryDatabaseMulti(e.sql);
 
 					// ignore empty query result "errors"
 					if (r.ErrorNumber() != 1065 && !r.ErrorMessage().empty()) {
@@ -187,6 +197,9 @@ bool DatabaseUpdate::UpdateManifest(
 						errored_migration = true;
 
 						LogInfo("Required database update failed. This could be a problem");
+
+						// if terminal attached then prompt for skip
+						if (is_atty()) {
 							LogInfo("Would you like to skip this update? [y/n] (Timeout 60s)");
 
 							// user input
@@ -214,6 +227,10 @@ bool DatabaseUpdate::UpdateManifest(
 								errored_migration = false;
 								LogInfo("Skipping update [{}] [{}]", e.version, e.description);
 							}
+						} else {
+							errored_migration = true;
+							LogInfo("Skipping update [{}] [{}]", e.version, e.description);
+						}
 					}
 
 					LogInfo(
@@ -247,6 +264,13 @@ DatabaseUpdate *DatabaseUpdate::SetDatabase(Database *db)
 	return this;
 }
 
+DatabaseUpdate *DatabaseUpdate::SetContentDatabase(Database *db)
+{
+	m_content_database = db;
+
+	return this;
+}
+
 bool DatabaseUpdate::CheckVersionsUpToDate(DatabaseVersion v, DatabaseVersion b)
 {
 	LogInfo("{}", Strings::Repeat("-", BREAK_LENGTH));

common/database/database_update.h

@@ -10,6 +10,7 @@ struct ManifestEntry {
 	std::string condition{};   // condition or "match_type" - Possible values [contains|match|missing|empty|not_empty]
 	std::string match{};       // match field that is not always used, but works in conjunction with "condition" values [missing|match|contains]
 	std::string sql{};         // the SQL DDL that gets ran when the condition is true
+	bool        content_schema_update{}; // if true, this migration is a content schema update and should be ran against the content database
 };
 
 struct DatabaseVersion {
@@ -22,14 +23,16 @@ public:
 	DatabaseVersion GetDatabaseVersions();
 	DatabaseVersion GetBinaryDatabaseVersions();
 	void CheckDbUpdates();
-	std::string GetQueryResult(std::string query);
+	std::string GetQueryResult(const ManifestEntry& e);
 	static bool ShouldRunMigration(ManifestEntry &e, std::string query_result);
 	bool UpdateManifest(std::vector<ManifestEntry> entries, int version_low, int version_high);
 
 	DatabaseUpdate *SetDatabase(Database *db);
+	DatabaseUpdate *SetContentDatabase(Database *db);
 	bool HasPendingUpdates();
 private:
 	Database *m_database;
+	Database *m_content_database;
 	static bool CheckVersionsUpToDate(DatabaseVersion v, DatabaseVersion b);
 	void InjectBotsVersionColumn();
 };

common/database/database_update_manifest.cpp

@@ -5016,7 +5016,7 @@ CREATE TABLE `spawn2_disabled` (
 ) ENGINE=InnoDB AUTO_INCREMENT=1 DEFAULT CHARSET=utf8mb4;
 INSERT INTO spawn2_disabled (spawn2_id, disabled) SELECT id, 1 FROM spawn2 WHERE enabled = 0;
 ALTER TABLE `spawn2` DROP COLUMN `enabled`;
-)"
+)",
 	},
 	ManifestEntry{
 		.version = 9242,
@@ -5028,7 +5028,8 @@ ALTER TABLE `spawn2` DROP COLUMN `enabled`;
 ALTER TABLE `spawnentry`
 ADD COLUMN `min_time` smallint(4) NOT NULL DEFAULT 0 AFTER `condition_value_filter`,
 ADD COLUMN `max_time` smallint(4) NOT NULL DEFAULT 0 AFTER `min_time`;
-)"
+)",
+		.content_schema_update = true
 	},
 	ManifestEntry{
 		.version = 9243,
@@ -5082,7 +5083,8 @@ INSERT INTO
 
 DROP TABLE `starting_items`;
 RENAME TABLE `starting_items_new` TO `starting_items`;
-)"
+)",
+		.content_schema_update = true
 	},
 	ManifestEntry{
 		.version = 9244,
@@ -5092,7 +5094,8 @@ RENAME TABLE `starting_items_new` TO `starting_items`;
 		.match = "0000-00-00 00:00:00",
 		.sql = R"(
 ALTER TABLE `items` MODIFY COLUMN `updated` datetime NULL DEFAULT NULL;
-		)"
+		)",
+		.content_schema_update = true
 	},
 	ManifestEntry{
 		.version = 9245,
@@ -5104,7 +5107,8 @@ ALTER TABLE `items` MODIFY COLUMN `updated` datetime NULL DEFAULT NULL;
 ALTER TABLE `object` CHANGE COLUMN `unknown08` `size_percentage` float NOT NULL DEFAULT 0 AFTER `icon`;
 ALTER TABLE `object` CHANGE COLUMN `unknown10` `solid_type` mediumint(5) NOT NULL DEFAULT 0 AFTER `size`;
 ALTER TABLE `object` CHANGE COLUMN `unknown20` `incline` int(11) NOT NULL DEFAULT 0 AFTER `solid_type`;
-)"
+)",
+		.content_schema_update = true
 	},
 	ManifestEntry{
 		.version = 9246,
@@ -5133,7 +5137,8 @@ CHANGE COLUMN `slot` `inventory_slot` mediumint(9) NOT NULL DEFAULT -1 AFTER `st
 
 ALTER TABLE `starting_items`
 CHANGE COLUMN `temporary` `class_list` text CHARACTER SET latin1 COLLATE latin1_swedish_ci NULL DEFAULT NULL AFTER `id`;
-)"
+)",
+		.content_schema_update = true
 	},
 	ManifestEntry{
 		.version = 9248,
@@ -5143,7 +5148,8 @@ CHANGE COLUMN `temporary` `class_list` text CHARACTER SET latin1 COLLATE latin1_
 		.match = "",
 		.sql = R"(
 ALTER TABLE `npc_emotes` DROP INDEX `emoteid`;
-)"
+)",
+		.content_schema_update = true
 	},
 	ManifestEntry{
 		.version = 9249,
@@ -5154,7 +5160,8 @@ ALTER TABLE `npc_emotes` DROP INDEX `emoteid`;
 		.sql = R"(
 ALTER TABLE `tasks`
 ADD COLUMN `enabled` smallint NULL DEFAULT 1 AFTER `faction_amount`
-)"
+)",
+		.content_schema_update = true
 	}
 
 // -- template; copy/paste this when you need to create a new entry

common/database_conversions.cpp

@@ -1,39 +0,0 @@
-#include "../common/global_define.h"
-#include "../common/rulesys.h"
-#include "../common/strings.h"
-
-#include "database.h"
-#include "database/database_update.h"
-
-
-// Disgrace: for windows compile
-#ifdef _WINDOWS
-#include <windows.h>
-#define snprintf	_snprintf
-#define strncasecmp	_strnicmp
-#define strcasecmp	_stricmp
-#else
-
-#include "unix.h"
-#include <netinet/in.h>
-#include <sys/time.h>
-
-#endif
-
-#pragma pack(1)
-
-DatabaseUpdate database_update;
-
-bool Database::CheckDatabaseConversions()
-{
-	auto *r = RuleManager::Instance();
-	r->LoadRules(this, "default", false);
-	if (!RuleB(Bots, Enabled) && DoesTableExist("bot_data")) {
-		LogInfo("Bot tables found but rule not enabled, enabling");
-		r->SetRule("Bots:Enabled", "true", this, true, true);
-	}
-
-	database_update.SetDatabase(this)->CheckDbUpdates();
-
-	return true;
-}

world/cli/database_updates.cpp

@@ -9,5 +9,7 @@ void WorldserverCLI::DatabaseUpdates(int argc, char **argv, argh::parser &cmd, s
 	}
 
 	DatabaseUpdate update;
-	update.SetDatabase(&database)->CheckDbUpdates();
+	update.SetDatabase(&database)
+		->SetContentDatabase(&content_db)
+		->CheckDbUpdates();
 }

world/world_boot.cpp

@@ -26,6 +26,7 @@
 #include "../common/ip_util.h"
 #include "../common/zone_store.h"
 #include "../common/path_manager.h"
+#include "../common/database/database_update.h"
 
 extern ZSList      zoneserver_list;
 extern WorldConfig Config;
@@ -293,7 +294,18 @@ bool WorldBoot::DatabaseLoadRoutines(int argc, char **argv)
 	const auto c = EQEmuConfig::get();
 	if (c->auto_database_updates) {
 		LogInfo("Checking Database Conversions");
-		database.CheckDatabaseConversions();
+
+		auto *r = RuleManager::Instance();
+		r->LoadRules(&database, "default", false);
+		if (!RuleB(Bots, Enabled) && database.DoesTableExist("bot_data")) {
+			LogInfo("Bot tables found but rule not enabled, enabling");
+			r->SetRule("Bots:Enabled", "true", &database, true, true);
+		}
+
+		DatabaseUpdate update{};
+		update.SetDatabase(&database)
+			->SetContentDatabase(&content_db)
+			->CheckDbUpdates();
 	}
 
 	if (RuleB(Logging, WorldGMSayLogging)) {