Fixed a query rename bug

server/src/client/SpeakingClient.cpp

@@ -786,9 +786,6 @@ command_result SpeakingClient::handleCommand(Command &command) {
             return this->handleCommandRtcSessionDescribe(command);
         } else if(command.command() == "rtcicecandidate") {
             return this->handleCommandRtcIceCandidate(command);
-        } else if(command.command() == "rtcbroadcast") {
-            /* TODO: Remove this command once the first 1.5.0 stable is out */
-            return this->handleCommandRtcBroadcast(command);
         } else if(command.command() == "rtcsessionreset") {
             return this->handleCommandRtcSessionReset(command);
         } else if(command.command() == "broadcastaudio") {
@@ -994,61 +991,6 @@ command_result SpeakingClient::handleCommandBroadcastVideo(Command &command) {
     return broadcast_start_result_to_command_result(broadcast_result);
 }
 
-command_result SpeakingClient::handleCommandRtcBroadcast(Command &command) {
-    CMD_REQ_SERVER;
-    CMD_CHK_AND_INC_FLOOD_POINTS(15);
-
-    std::vector<std::tuple<uint8_t, uint32_t>> broadcasts{};
-    broadcasts.reserve(command.bulkCount());
-
-    ts::command_result_bulk result{};
-    for(size_t index{0}; index < command.bulkCount(); index++) {
-        auto& bulk = command[index];
-
-        auto ssrc = bulk.has("ssrc") ? bulk["ssrc"].as<uint32_t>() : (uint32_t) 0;
-        auto type = bulk["type"].as<uint8_t>();
-
-        for(const auto& entry : broadcasts) {
-            if(std::get<0>(entry) == type) {
-                return ts::command_result{error::parameter_constraint_violation};
-            }
-        }
-
-        broadcasts.push_back(std::make_tuple(type, ssrc));
-        switch(type) {
-            case 1: {
-                ts::Command cmd{""};
-                cmd["ssrc"] = ssrc;
-                result.insert_result(this->handleCommandBroadcastAudio(cmd));
-                break;
-            }
-            case 2: {
-                ts::Command cmd{""};
-                cmd["broadcast_bitrate_max"] = 1500000;
-                cmd["broadcast_keyframe_interval"] = 0;
-                cmd["ssrc"] = ssrc;
-                cmd["type"] = (uint8_t) rtc::VideoBroadcastType::Camera;
-                result.insert_result(this->handleCommandBroadcastVideo(cmd));
-                break;
-            }
-            case 3: {
-                ts::Command cmd{""};
-                cmd["broadcast_bitrate_max"] = 1500000;
-                cmd["broadcast_keyframe_interval"] = 0;
-                cmd["ssrc"] = ssrc;
-                cmd["type"] = (uint8_t) rtc::VideoBroadcastType::Screen;
-                result.insert_result(this->handleCommandBroadcastVideo(cmd));
-                break;
-            }
-            default:
-                result.emplace_result(error::parameter_invalid, "type");
-                break;
-        }
-    }
-
-    return ts::command_result{std::move(result)};
-}
-
 command_result SpeakingClient::handleCommandBroadcastVideoJoin(Command &cmd) {
     CMD_REQ_SERVER;
 
@@ -1214,15 +1156,6 @@ command_result SpeakingClient::handleCommandBroadcastVideoConfigure(Command &cmd
         result.release_data();
     }
 
-    {
-        auto result = test_broadcast_options(*this, this->getChannelId(), options);
-        if(result.has_error()) {
-            return result;
-        }
-
-        result.release_data();
-    }
-
     auto result = this->server->rtc_server().client_broadcast_video_configure(this->rtc_client_id, broadcast_type, &options);
     return broadcast_config_result_to_command_result(result);
 }

server/src/client/SpeakingClient.h

@@ -70,7 +70,6 @@ namespace ts::server {
             virtual command_result handleCommandRtcSessionDescribe(Command &command);
             virtual command_result handleCommandRtcSessionReset(Command &command);
             virtual command_result handleCommandRtcIceCandidate(Command &);
-            virtual command_result handleCommandRtcBroadcast(Command &); /* Old method used 'till 1.5.0b4 */
             virtual command_result handleCommandBroadcastAudio(Command &);
             virtual command_result handleCommandBroadcastVideo(Command &);
             virtual command_result handleCommandBroadcastVideoJoin(Command &);

server/src/client/command_handler/groups.cpp

@@ -70,8 +70,9 @@ command_result ConnectedClient::handleCommandGroupAdd(Command &cmd, GroupTarget
 
         case groups::GroupType::GROUP_TYPE_NORMAL:
             if (!this->server) {
-                return command_result{error::parameter_invalid, "you cant create normal groups on the template server!"};
+                return ts::command_result{error::parameter_invalid, "you cant create normal groups on the template server!"};
             }
+
             log_group_type = log::GroupType::NORMAL;
             group_manager = this->server->group_manager();
             break;
@@ -260,6 +261,7 @@ command_result ConnectedClient::handleCommandGroupCopy(Command &cmd, GroupTarget
 
     auto global_update = false;
     if (target_group) {
+        /* Copy permissions into an already existing group */
         if(!target_group->permission_granted(permission::i_channel_group_needed_modify_power, this->calculate_permission(permission::i_channel_group_modify_power, 0), true)) {
             return ts::command_result{permission::i_channel_group_needed_modify_power};
         }
@@ -328,7 +330,7 @@ command_result ConnectedClient::handleCommandGroupCopy(Command &cmd, GroupTarget
 
         global_update = !this->server || target_group_global;
     } else {
-        //Copy to a new group
+        /* Copy permissions into a new group */
         auto target_type = cmd["type"].as<groups::GroupType>();
 
         log::GroupType log_group_type;
@@ -495,12 +497,12 @@ command_result ConnectedClient::handleCommandGroupRename(Command &cmd, GroupTarg
     }
     assert(group);
 
-    auto type = group->group_type();
+    auto group_type = group->group_type();
     log::GroupType log_group_type;
-    if (type == groups::GroupType::GROUP_TYPE_QUERY) {
+    if (group_type == groups::GroupType::GROUP_TYPE_QUERY) {
         ACTION_REQUIRES_GLOBAL_PERMISSION(permission::b_serverinstance_modify_querygroup, 1);
         log_group_type = log::GroupType::QUERY;
-    } else if (type == groups::GroupType::GROUP_TYPE_TEMPLATE) {
+    } else if (group_type == groups::GroupType::GROUP_TYPE_TEMPLATE) {
         ACTION_REQUIRES_GLOBAL_PERMISSION(permission::b_serverinstance_modify_templates, 1);
         log_group_type = log::GroupType::TEMPLATE;
     } else {
@@ -542,14 +544,31 @@ command_result ConnectedClient::handleCommandGroupRename(Command &cmd, GroupTarg
             return ts::command_result{error::vs_critical};
     }
 
-    serverInstance->action_logger()->group_logger.log_group_rename(this->getServerId(), this->ref(), log_group_target, log_group_type, group->group_id(), group->display_name(), old_name);
+    ServerId log_server_id;
     std::deque<std::shared_ptr<VirtualServer>> server_updates{};
-    if(!this->server) {
+    switch(group_type) {
-        server_updates = serverInstance->getVoiceServerManager()->serverInstances();
+        case groups::GroupType::GROUP_TYPE_QUERY:
-    } else {
+        case groups::GroupType::GROUP_TYPE_TEMPLATE:
-        server_updates.push_back(this->server);
+            log_server_id = 0;
+            server_updates = serverInstance->getVoiceServerManager()->serverInstances();
+            break;
+
+        case groups::GroupType::GROUP_TYPE_NORMAL:
+            /* Normal groups can only exist on regular servers */
+            assert(this->server);
+            server_updates.push_back(this->server);
+            log_server_id = this->getServerId();
+            break;
+
+        case groups::GroupType::GROUP_TYPE_UNKNOWN:
+        default:
+            log_server_id = 0;
+            assert(false);
+            break;
     }
 
+    serverInstance->action_logger()->group_logger.log_group_rename(log_server_id, this->ref(), log_group_target, log_group_type, group->group_id(), group->display_name(), old_name);
+
     for(const auto& server : server_updates) {
         if(!server) {
             continue;
@@ -658,15 +677,18 @@ command_result ConnectedClient::handleCommandGroupDel(Command &cmd, GroupTarget
 
     bool global_update;
     log::GroupType log_group_type;
+    ServerId log_server_id;
     switch (group->group_type()) {
         case groups::GroupType::GROUP_TYPE_QUERY:
             ACTION_REQUIRES_GLOBAL_PERMISSION(permission::b_serverinstance_modify_querygroup, 1);
+            log_server_id = 0;
             log_group_type = log::GroupType::QUERY;
             global_update = true;
             break;
 
         case groups::GroupType::GROUP_TYPE_TEMPLATE:
             ACTION_REQUIRES_GLOBAL_PERMISSION(permission::b_serverinstance_modify_templates, 1);
+            log_server_id = 0;
             log_group_type = log::GroupType::TEMPLATE;
             global_update = true;
             break;
@@ -682,6 +704,7 @@ command_result ConnectedClient::handleCommandGroupDel(Command &cmd, GroupTarget
                     break;
             }
 
+            log_server_id = this->getServerId();
             log_group_type = log::GroupType::NORMAL;
             global_update = false;
             break;
@@ -731,7 +754,7 @@ command_result ConnectedClient::handleCommandGroupDel(Command &cmd, GroupTarget
             return ts::command_result{error::vs_critical};
     }
 
-    serverInstance->action_logger()->group_logger.log_group_delete(this->getServerId(), this->ref(), log_group_target, log_group_type, group->group_id(), group->display_name());
+    serverInstance->action_logger()->group_logger.log_group_delete(log_server_id, this->ref(), log_group_target, log_group_type, group->group_id(), group->display_name());
 
     std::deque<std::shared_ptr<VirtualServer>> server_updates{};
     if(global_update) {

server/src/groups/GroupManager.cpp

@@ -83,6 +83,8 @@ bool AbstractGroupManager::initialize(std::string &) {
 }
 
 GroupLoadResult AbstractGroupManager::load_data(bool initialize) {
+    (void) initialize;
+
     std::lock_guard manage_lock{this->group_manage_mutex_};
     {
         std::lock_guard list_lock{this->group_mutex_};
@@ -330,27 +332,29 @@ GroupCopyResult AbstractGroupManager::copy_group_(GroupId source, GroupType targ
     return this->copy_group_permissions_(source, result->group_id());
 }
 
+/* FIXME: This implementation is flawed since it may operates on parent groups without locking them properly. */
 GroupCopyResult AbstractGroupManager::copy_group_permissions_(GroupId source, GroupId target) {
     std::lock_guard manage_lock{this->group_manage_mutex_};
 
-    std::shared_ptr<AbstractGroupManager> owning_manager{};
+    std::shared_ptr<AbstractGroupManager> source_owning_manager{};
-    auto source_group = this->find_group_(owning_manager, groups::GroupCalculateMode::GLOBAL, source);
+    auto source_group = this->find_group_(source_owning_manager, groups::GroupCalculateMode::GLOBAL, source);
     if(!source_group) {
         return GroupCopyResult::UNKNOWN_SOURCE_GROUP;
     }
 
-    auto target_group = this->find_group_(owning_manager, groups::GroupCalculateMode::GLOBAL, target);
+    std::shared_ptr<AbstractGroupManager> target_owning_manager{};
+    auto target_group = this->find_group_(target_owning_manager, groups::GroupCalculateMode::GLOBAL, target);
     if(!target_group) {
         return GroupCopyResult::UNKNOWN_TARGET_GROUP;
     }
 
-    if(target_group->permissions()->require_db_updates()) {
+    if(source_group->permissions()->require_db_updates()) {
         /* TODO: Somehow flush all pending changes */
     }
 
     {
         auto res = sql::command(this->sql_manager(), "DELETE FROM `permissions` WHERE `serverId` = :sid AND `type` = :type AND `id` = :id",
-                                variable{":sid", this->server_id()},
+                                variable{":sid", source_owning_manager->server_id()},
                                 variable{":type", ts::permission::SQL_PERM_GROUP},
                                 variable{":id", target}).execute();
         if(!res) {
@@ -363,8 +367,8 @@ GroupCopyResult AbstractGroupManager::copy_group_permissions_(GroupId source, Gr
         constexpr static auto kSqlCommand = "INSERT INTO `permissions` (`serverId`, `type`, `id`, `channelId`, `permId`, `value`, `grant`, `flag_skip`, `flag_negate`) "
                                             "SELECT :tsid AS `serverId`, `type`, :target AS `id`, 0 AS `channelId`, `permId`, `value`,`grant`,`flag_skip`, `flag_negate` FROM `permissions` WHERE `serverId` = :ssid AND `type` = :type AND `id` = :source";
         auto res = sql::command(this->sql_manager(), kSqlCommand,
-                           variable{":ssid", source_group->virtual_server_id()},
+                           variable{":ssid", source_owning_manager->server_id()},
-                           variable{":tsid", target_group->virtual_server_id()},
+                           variable{":tsid", target_owning_manager->server_id()},
                            variable{":type", ts::permission::SQL_PERM_GROUP},
                            variable{":source", source},
                            variable{":target", target}).execute();
@@ -375,23 +379,30 @@ GroupCopyResult AbstractGroupManager::copy_group_permissions_(GroupId source, Gr
         }
     }
 
-    target_group->set_permissions(serverInstance->databaseHelper()->loadGroupPermissions(this->server_id(), target, (uint8_t) this->database_target_));
+    target_group->set_permissions(serverInstance->databaseHelper()->loadGroupPermissions(target_owning_manager->server_id(), target, (uint8_t) this->database_target_));
     return GroupCopyResult::SUCCESS;
 }
 
 GroupRenameResult AbstractGroupManager::rename_group_(GroupId group_id, const std::string &name) {
     std::lock_guard manage_lock{this->group_manage_mutex_};
+
+    std::shared_ptr<AbstractGroupManager> owning_manager{};
+    auto group = this->find_group_(owning_manager, groups::GroupCalculateMode::LOCAL, group_id);
+    if(!group) {
+        /* Group hasn't been found locally. Trying our parent manager. */
+        if(this->parent_manager_) {
+            return this->parent_manager_->rename_group_(group_id, name);
+        }
+
+        return GroupRenameResult::INVALID_GROUP_ID;
+    }
+
+    assert(&*owning_manager == this);
     auto name_length = utf8::count_characters(name);
     if(name_length <= 0 || name_length > kGroupMaxNameLength) {
         return GroupRenameResult::NAME_INVALID;
     }
 
-    std::shared_ptr<AbstractGroupManager> owning_manager{};
-    auto group = this->find_group_(owning_manager, groups::GroupCalculateMode::GLOBAL, group_id);
-    if(!group) {
-        return GroupRenameResult::INVALID_GROUP_ID;
-    }
-
     if(this->find_group_by_name_(GroupCalculateMode::LOCAL, name)) {
         return GroupRenameResult::NAME_ALREADY_USED;
     }
@@ -399,6 +410,7 @@ GroupRenameResult AbstractGroupManager::rename_group_(GroupId group_id, const st
     sql::command(this->sql_manager(), "UPDATE `groups` SET `displayName` = :name WHERE `serverId` = :server AND `groupId` = :group_id AND `target` = :target",
                  variable{":server", this->server_id()},
                  variable{":target", (uint8_t) this->database_target_},
+                 variable{":name", name},
                  variable{":group_id", group_id}).executeLater().waitAndGetLater(LOG_SQL_CMD, {-1, "future failed"});
     group->name_ = name;
     return GroupRenameResult::SUCCESS;

shared

@@ -1 +1 @@
-Subproject commit b155bf8d5f587b3b8970ef8ff81e0deb3b5bdf10
+Subproject commit 9f03f33a23de2a69b2f464311838f5174f5d78ac