planetteamspeak / ts3phpframework

Modern use-at-will framework that provides individual components to manage TeamSpeak 3 Server instances
https://www.planetteamspeak.com
GNU General Public License v3.0
211 stars 59 forks source link

server got an invalid status for this operation #104

Closed Sebbo94BY closed 6 years ago

Sebbo94BY commented 6 years ago

Hey, I try to delete a virtual server, which is already offline. Shouldn't this do the trick already?

$uri = "serverquery://serveradmin:v3rYSeCr3tP4s5w0Rd@10.100.1.134:10011/?server_id=136&blocking=1&nickname=TS3PHPFramework&use_offline_as_virtual#no_query_clients";

$TS3PHPFramework = new TeamSpeak3();
$virtualserver= $TS3PHPFramework->factory("$uri");
$virtualserver->delete();

Your TeamSpeak3_Exception is always returning this message: server got an invalid status for this operation

snegrini commented 6 years ago

This shouldn't be a framework issue. You are trying to delete a server with the flag use_offline_as_virtual enabled. From Teamspeak 3 ServerQuery docs:

Whenever you select a virtual server which is currently stopped with the -virtual parameter, it will be started in virtual mode which means you are able to change its configuration, create channels or change permissions, but no regular TeamSpeak 3 Client can connect. As soon as the last ServerQuery client deselects the virtual server, its status will be changed back to offline.

Removing use_offline_as_virtual in the $uri should fix your problem.

Sebbo94BY commented 6 years ago

I've already tried this, but when I remove this flag, I get this error message: server is not running

snegrini commented 6 years ago

Right, you can't select a server if it is offline, then just connect as host without selecting the virtual server, then pass the server id to the method serverDelete(). Code example (tested myself):

    $serverId = 136;
    $uri = "serverquery://serveradmin:v3rYSeCr3tP4s5w0Rd@10.100.1.134:10011/?nickname=TS3PHPFramework";
    $host = TeamSpeak3::factory($uri);
    $host->serverDelete($serverId);  

EDIT: fixed typo. But now I'm wondering how delete() should be used in a Server Node. We can't delete an online server...

Sebbo94BY commented 6 years ago

Thanks, this worked fine.

I've tried to delete the server all the time using the method delete(): https://docs.planetteamspeak.com/ts3/php/framework/class_team_speak3___node___server.html#a13bdffdd926f26b825ea57066334ff01

This method does not allow any parameters. However: It worked with your solution - I've just updated my code. Thanks a lot! :)

snegrini commented 6 years ago

Please don't close this yet, I think that delete() is unusable in Node Server. A virtual server must be online to be selected, but if it is online it can't be deleted! Maybe we need a clarification from @ronindesign 😄

ronindesign commented 6 years ago

I see this, let me check and see what's going on. There may very well be a logic bug here.

ronindesign commented 6 years ago

I can confirm this as a bug.

The two relevant pieces are:

TeamSpeak3_Node_Server::delete()

  /**
   * Deletes the virtual server.
   *
   * @return void
   */
  public function delete()
  {
    $this->getParent()->serverDelete($this->getId());
  }

TeamSpeak3_Node_Host::serverDelete()

  /**
   * Deletes the virtual server specified by ID.
   *
   * @param  integer $sid
   * @return void
   */
  public function serverDelete($sid)
  {
    $this->serverListReset();
    $this->execute("serverdelete", array("sid" => $sid));
    TeamSpeak3_Helper_Signal::getInstance()->emit("notifyServerdeleted", $this, $sid);
  }

I would say TeamSpeak3_Node_Server::delete() is just a shorter path to auto-fill the server ID when deleting.

So two possible fixes might be:

If one of these solutions sounds better let me know your thoughts on it.

If we can find a way that works for the first solution, I think it would be the most intuitive.

snegrini commented 6 years ago

In my opinion it is better to remove TeamSpeak3_Node_Server::delete() in order to prevent unintended actions while the virtualserver is selected.

Sebbo94BY commented 6 years ago

Thanks for checking, @ronindesign !

I would remove TeamSpeak3_Node_Server::delete() and rework TeamSpeak3_Node_Host::serverDelete() as you already suggested.

In my opinion, I would expect, that this code would delete the server, when it's offline / stopped:

$uri = "serverquery://serveradmin:v3rYSeCr3tP4s5w0Rd@10.100.1.134:10011/?server_id=136";

$TS3PHPFramework = new TeamSpeak3();
$virtualserver= $TS3PHPFramework->factory("$uri");
$virtualserver->serverDelete();

When the server is online / started, I would expect an error message like "not possible as server is running".

If the server is started, the user would need to stop the server first by adding a line before serverDelete() for stopping the server:

$uri = "serverquery://serveradmin:v3rYSeCr3tP4s5w0Rd@10.100.1.134:10011/?server_id=136";

$TS3PHPFramework = new TeamSpeak3();
$virtualserver= $TS3PHPFramework->factory("$uri");
$virtualserver->serverStop(); // Just as example; not sure, if it's the correct method :D
$virtualserver->serverDelete();
ronindesign commented 6 years ago

Sounds good @snegrini, I agree and think a "top down" approach for managing virtual servers makes the most sense. I.e., connect to the host, then use TeamSpeak3_Node_Host object to administer its virtual servers.

@Sebi94nbg I'm a bit confused by your code example, but I think I understand what you'd like:

In TeamSpeak3::factory(), for the URI example you give, the following option will be triggered:

      if($uri->hasQueryVar("server_id"))
      {
        $node = $node->serverGetById($uri->getQueryVar("server_id"));
      }

In this case, $node changes from an instance of TeamSpeak3_Node_Host to to an instance of TeamSpeak3_Node_Server, when TeamSpeak3_Node_Host::serverGetById() returns given a valid server_id.

The issue / confusion in your example is that $node->serverDelete() does not exist for virtual server objects. I.e. TeamSpeak3_Node_Server::serverDelete() is not defined, only TeamSpeak3_Node_Host::serverDelete() exists.

Also, as you can see from the included signatures above, a parameter is required: TeamSpeak3_Node_Host::serverDelete($server_id) which follows since no virtual server has been selected. The same is true for TeamSpeak3_Node_Host::serverStop($server_id)

However, the rest of your logic would work fine, it just needs to be executed from the parent object (i.e. the host not virtual server object).

So, would something like this be close to what you're thinking?

Delete a child virtual server from a parent host

$uri = "serverquery://serveradmin:v3rYSeCr3tP4s5w0Rd@10.100.1.134:10011";

$TS3PHPFramework = new TeamSpeak3();
$host = $TS3PHPFramework->factory("$uri"); // TeamSpeak3_Node_Host
$host->serverDelete(136);

Stop child virtual server first before deleting if running

$uri = "serverquery://serveradmin:v3rYSeCr3tP4s5w0Rd@10.100.1.134:10011";

$TS3PHPFramework = new TeamSpeak3();
$host = $TS3PHPFramework->factory("$uri"); // TeamSpeak3_Node_Host
$host->serverStop(136);
$host->serverDelete(136);

I apologize if this is response is a bit overdone. I just wanted to clarify and make sure you do like the idea of deleting the virtual server from the host rather than directly from the virtual server (since your code shows the latter, but you agreed verbally to the former).

svenpaulsen commented 6 years ago

So... one problem in the initial report is that the parameter value for use_offline_as_virtual is missing. Also, blocking=1 can be omitted as this is a default option. The URI should be:

$uri = "serverquery://serveradmin:v3rYSeCr3tP4s5w0Rd@10.100.1.134:10011/?server_id=136&nickname=TS3PHPFramework&use_offline_as_virtual=1#no_query_clients";

I agree that TeamSpeak3_Node_Server::delete() did not make sense in previous versions as the virtual server needs to be stopped before it can be deleted. I've just pushed 4e75bf0c0797137f780c5f19950822beaa5ba97b which should resolve this issue.

Please-re-test with the current master and let me know if it works for you.

Sebbo94BY commented 6 years ago

@ronindesign Yeah, sorry - I just wrote a quick comment without any documentation on my hand, but you understood it very good. :)

Your examples are perfect!

@svenpaulsen I'm sure, that your pushed change will solve the issue - it looks fine.

I'll test it asap.

svenpaulsen commented 6 years ago

I actually commited total crap and made a stupid copy & paste error there... fixed with 66838e18751c595bd0eb77d5cf89f0e371dc955d.