Skip to content

Commit b208cd0

Browse files
rennoxrennox
committed
Fixing windows crash + Unit Tests
1 parent cbe162f commit b208cd0

File tree

8 files changed

+166
-143
lines changed

8 files changed

+166
-143
lines changed

‎modules/adminapi/mod_dba.cc

+104-114
Original file line numberDiff line numberDiff line change
@@ -516,7 +516,7 @@ shcore::Value Dba::exec_instance_op(const std::string &function, const shcore::A
516516
} else {
517517
if (function == "deploy")
518518
throw shcore::Exception::argument_error("Missing root password for the deployed instance");
519-
}
519+
}
520520

521521
shcore::Value::Array_type_ref errors;
522522

@@ -557,7 +557,7 @@ shcore::Value Dba::exec_instance_op(const std::string &function, const shcore::A
557557
}
558558

559559
return ret_val;
560-
}
560+
}
561561

562562
REGISTER_HELP(DBA_DEPLOYSANDBOXINSTANCE_BRIEF, "Creates a new MySQL Server instance on localhost.");
563563
REGISTER_HELP(DBA_DEPLOYSANDBOXINSTANCE_PARAM, "@param port The port where the new instance will listen for connections.");
@@ -873,10 +873,8 @@ shcore::Value Dba::config_local_instance(const shcore::Argument_list &args) {
873873

874874
if (shcore::is_local_host(opt_map.string_at("host"))) {
875875
ret_val = shcore::Value(_check_instance_config(args, true));
876-
}
877-
else
876+
} else
878877
throw shcore::Exception::runtime_error("This function only works with local instances");
879-
880878
}
881879
CATCH_AND_TRANSLATE_FUNCTION_EXCEPTION(get_function_name("configLocalInstance"));
882880

@@ -887,9 +885,7 @@ Dba::~Dba() {
887885
Dba::_session_cache.clear();
888886
}
889887

890-
891888
std::shared_ptr<mysh::mysql::ClassicSession> Dba::get_session(const shcore::Argument_list& args) {
892-
893889
std::shared_ptr<mysh::mysql::ClassicSession> ret_val;
894890

895891
auto options = args.map_at(0);
@@ -909,7 +905,6 @@ std::shared_ptr<mysh::mysql::ClassicSession> Dba::get_session(const shcore::Argu
909905
}
910906

911907
shcore::Value::Map_type_ref Dba::_check_instance_config(const shcore::Argument_list &args, bool allow_update) {
912-
913908
shcore::Value::Map_type_ref ret_val(new shcore::Value::Map_type());
914909

915910
// Validates the connection options
@@ -953,9 +948,9 @@ shcore::Value::Map_type_ref Dba::_check_instance_config(const shcore::Argument_l
953948
GRInstanceType type = get_gr_instance_type(session->connection());
954949

955950
if (type == GRInstanceType::GroupReplication)
956-
throw shcore::Exception::runtime_error("The instance '"+ session->uri()+ "' is already part of a Replication Group");
951+
throw shcore::Exception::runtime_error("The instance '" + session->uri() + "' is already part of a Replication Group");
957952
else if (type == GRInstanceType::InnoDBCluster)
958-
throw shcore::Exception::runtime_error("The instance '"+ session->uri()+ "' is already part of an InnoDB Cluster");
953+
throw shcore::Exception::runtime_error("The instance '" + session->uri() + "' is already part of an InnoDB Cluster");
959954
else if (type == GRInstanceType::Standalone) {
960955
std::string user;
961956
std::string password;
@@ -975,126 +970,121 @@ shcore::Value::Map_type_ref Dba::_check_instance_config(const shcore::Argument_l
975970

976971
(*ret_val)["status"] = shcore::Value("error");
977972

978-
for (auto error_object : *mp_errors) {
979-
auto map = error_object.as_map();
973+
if (mp_errors) {
974+
for (auto error_object : *mp_errors) {
975+
auto map = error_object.as_map();
980976

981-
std::string error_str;
982-
if (map->get_string("type") == "ERROR") {
983-
error_str = map->get_string("msg");
977+
std::string error_str;
978+
if (map->get_string("type") == "ERROR") {
979+
error_str = map->get_string("msg");
984980

985-
if (error_str.find("The operation could not continue due to the following requirements not being met") != std::string::npos) {
986-
auto lines = shcore::split_string(error_str, "\n");
981+
if (error_str.find("The operation could not continue due to the following requirements not being met") != std::string::npos) {
982+
auto lines = shcore::split_string(error_str, "\n");
987983

988-
bool loading_options = false;
984+
bool loading_options = false;
989985

990-
shcore::Value::Map_type_ref server_options(new shcore::Value::Map_type());
991-
std::string option_type;
986+
shcore::Value::Map_type_ref server_options(new shcore::Value::Map_type());
987+
std::string option_type;
992988

993-
for (size_t index = 1; index < lines.size(); index++) {
994-
if (loading_options) {
995-
auto option_tokens = shcore::split_string(lines[index], " ", true);
989+
for (size_t index = 1; index < lines.size(); index++) {
990+
if (loading_options) {
991+
auto option_tokens = shcore::split_string(lines[index], " ", true);
996992

993+
if (option_tokens[1] == "<no") {
994+
option_tokens[1] = "<no value>";
995+
option_tokens.erase(option_tokens.begin() + 2);
996+
}
997997

998-
if (option_tokens[1] == "<no") {
999-
option_tokens[1] = "<no value>";
1000-
option_tokens.erase(option_tokens.begin() + 2);
1001-
}
1002-
1003-
if (option_tokens[2] == "<not") {
1004-
option_tokens[2] = "<not set>";
1005-
option_tokens.erase(option_tokens.begin() + 3);
1006-
}
998+
if (option_tokens[2] == "<not") {
999+
option_tokens[2] = "<not set>";
1000+
option_tokens.erase(option_tokens.begin() + 3);
1001+
}
10071002

1008-
// The tokens describing each option have length of 5
1009-
if (option_tokens.size() > 5) {
1010-
index--;
1011-
loading_options = false;
1003+
// The tokens describing each option have length of 5
1004+
if (option_tokens.size() > 5) {
1005+
index--;
1006+
loading_options = false;
1007+
} else {
1008+
shcore::Value::Map_type_ref option;
1009+
if (!server_options->has_key(option_tokens[0])) {
1010+
option.reset(new shcore::Value::Map_type());
1011+
(*server_options)[option_tokens[0]] = shcore::Value(option);
1012+
} else
1013+
option = (*server_options)[option_tokens[0]].as_map();
1014+
1015+
(*option)["required"] = shcore::Value(option_tokens[1]); // The required value
1016+
(*option)[option_type] = shcore::Value(option_tokens[2]);// The current value
1017+
}
10121018
} else {
1013-
shcore::Value::Map_type_ref option;
1014-
if (!server_options->has_key(option_tokens[0])) {
1015-
option.reset(new shcore::Value::Map_type());
1016-
(*server_options)[option_tokens[0]] = shcore::Value(option);
1017-
} else
1018-
option = (*server_options)[option_tokens[0]].as_map();
1019-
1020-
1021-
(*option)["required"] = shcore::Value(option_tokens[1]); // The required value
1022-
(*option)[option_type] = shcore::Value(option_tokens[2]);// The current value
1023-
}
1024-
} else {
1025-
if (lines[index].find("Some active options on server") != std::string::npos) {
1026-
option_type = "server";
1027-
loading_options = true;
1028-
index += 3; // Skips to the actual option table
1029-
} else if (lines[index].find("Some of the configuration values on your options file") != std::string::npos) {
1030-
option_type = "config";
1031-
loading_options = true;
1032-
index += 3; // Skips to the actual option table
1019+
if (lines[index].find("Some active options on server") != std::string::npos) {
1020+
option_type = "server";
1021+
loading_options = true;
1022+
index += 3; // Skips to the actual option table
1023+
} else if (lines[index].find("Some of the configuration values on your options file") != std::string::npos) {
1024+
option_type = "config";
1025+
loading_options = true;
1026+
index += 3; // Skips to the actual option table
1027+
}
10331028
}
10341029
}
1035-
}
1036-
1037-
if (server_options->size()) {
1038-
shcore::Value::Array_type_ref config_errors(new shcore::Value::Array_type());
1039-
for(auto option: *server_options) {
1040-
auto state = option.second.as_map();
1041-
1042-
std::string required_value = state->get_string("required");
1043-
std::string server_value = state->get_string("server", "");
1044-
std::string config_value = state->get_string("config", "");
1045-
1046-
// Taken from MP, reading docs made me think all variables should require restart
1047-
// Even several of them are dynamic, it seems changing values may lead to problems
1048-
// An extransaction_write_set_extraction which apparently is reserved for future use
1049-
// So I just took what I saw on the MP code
1050-
// Source: http://dev.mysql.com/doc/refman/5.7/en/dynamic-system-variables.html
1051-
std::vector<std::string> dynamic_variables = {"binlog_format", "binlog_checksum"};
1052-
1053-
1054-
bool dynamic = std::find(dynamic_variables.begin(), dynamic_variables.end(), option.first) != dynamic_variables.end();
1055-
1056-
std::string action;
1057-
std::string current;
1058-
if (!server_value.empty() && !config_value.empty()) { // Both server and configuration are wrong
1059-
if (dynamic)
1060-
action = "server_update+config_update";
1061-
else {
1062-
action = "config_update+restart";
1063-
restart_required = true;
1064-
}
10651030

1066-
current = server_value;
1067-
1068-
}
1069-
else if (!config_value.empty()) { // Configuration is wrong, server is OK
1070-
action = "config_update";
1071-
current = config_value;
1072-
}
1073-
else if (!server_value.empty()) { // Server is wronf, configuration is OK
1074-
if (dynamic)
1075-
action = "server_update";
1076-
else {
1077-
action = "restart";
1078-
restart_required = true;
1031+
if (server_options->size()) {
1032+
shcore::Value::Array_type_ref config_errors(new shcore::Value::Array_type());
1033+
for (auto option : *server_options) {
1034+
auto state = option.second.as_map();
1035+
1036+
std::string required_value = state->get_string("required");
1037+
std::string server_value = state->get_string("server", "");
1038+
std::string config_value = state->get_string("config", "");
1039+
1040+
// Taken from MP, reading docs made me think all variables should require restart
1041+
// Even several of them are dynamic, it seems changing values may lead to problems
1042+
// An extransaction_write_set_extraction which apparently is reserved for future use
1043+
// So I just took what I saw on the MP code
1044+
// Source: http://dev.mysql.com/doc/refman/5.7/en/dynamic-system-variables.html
1045+
std::vector<std::string> dynamic_variables = {"binlog_format", "binlog_checksum"};
1046+
1047+
bool dynamic = std::find(dynamic_variables.begin(), dynamic_variables.end(), option.first) != dynamic_variables.end();
1048+
1049+
std::string action;
1050+
std::string current;
1051+
if (!server_value.empty() && !config_value.empty()) { // Both server and configuration are wrong
1052+
if (dynamic)
1053+
action = "server_update+config_update";
1054+
else {
1055+
action = "config_update+restart";
1056+
restart_required = true;
1057+
}
1058+
1059+
current = server_value;
1060+
} else if (!config_value.empty()) { // Configuration is wrong, server is OK
1061+
action = "config_update";
1062+
current = config_value;
1063+
} else if (!server_value.empty()) { // Server is wronf, configuration is OK
1064+
if (dynamic)
1065+
action = "server_update";
1066+
else {
1067+
action = "restart";
1068+
restart_required = true;
1069+
}
1070+
current = server_value;
10791071
}
1080-
current = server_value;
1081-
}
10821072

1083-
shcore::Value::Map_type_ref error(new shcore::Value::Map_type());
1073+
shcore::Value::Map_type_ref error(new shcore::Value::Map_type());
10841074

1085-
(*error)["option"] = shcore::Value(option.first);
1086-
(*error)["current"] = shcore::Value(current);
1087-
(*error)["required"] = shcore::Value(required_value);
1088-
(*error)["action"] = shcore::Value(action);
1089-
1090-
config_errors->push_back(shcore::Value(error));
1091-
}
1075+
(*error)["option"] = shcore::Value(option.first);
1076+
(*error)["current"] = shcore::Value(current);
1077+
(*error)["required"] = shcore::Value(required_value);
1078+
(*error)["action"] = shcore::Value(action);
10921079

1093-
(*ret_val)["config_errors"] = shcore::Value(config_errors);
1094-
}
1080+
config_errors->push_back(shcore::Value(error));
1081+
}
10951082

1096-
} else
1097-
errors->push_back(shcore::Value(error_str));
1083+
(*ret_val)["config_errors"] = shcore::Value(config_errors);
1084+
}
1085+
} else
1086+
errors->push_back(shcore::Value(error_str));
1087+
}
10981088
}
10991089
}
11001090

@@ -1104,4 +1094,4 @@ shcore::Value::Map_type_ref Dba::_check_instance_config(const shcore::Argument_l
11041094
}
11051095

11061096
return ret_val;
1107-
}
1097+
}

0 commit comments

Comments
 (0)