Merge pull request #55 from toddr-bot/koan.toddr.bot/fix-disconnect-method-safety

fix: guard public API methods against calls while disconnected

Author: toddr

lib/Net/Jabber/Bot.pm

@@ -472,6 +472,11 @@ sub JoinForum {
     my $self       = shift;
     my $forum_name = shift;
 
+    if ( !$self->IsConnected ) {
+        WARN("Cannot join forum '$forum_name': not connected");
+        return;
+    }
+
     DEBUG( "Joining $forum_name on " . $self->conference_server . " as " . $self->alias );
 
     $self->jabber_client->MUCJoin(
@@ -498,6 +503,8 @@ sub Process {    # Call connection process.
     my $self            = shift;
     my $timeout_seconds = shift;
 
+    return if !$self->IsConnected;
+
     #If not passed explicitly
     $timeout_seconds = $self->process_timeout if ( !defined $timeout_seconds );
 
@@ -1140,6 +1147,11 @@ sub ChangeStatus {
     my $presence_mode = shift;
     my $status_string = shift;    # (optional)
 
+    if ( !$self->IsConnected ) {
+        WARN("Cannot change status: not connected");
+        return 0;
+    }
+
     $self->jabber_client->PresenceSend( show => $presence_mode, status => $status_string );
 
     return 1;
@@ -1157,6 +1169,11 @@ In which case we need another sub for this.
 sub GetRoster {
     my $self = shift;
 
+    if ( !$self->IsConnected ) {
+        WARN("Cannot get roster: not connected");
+        return ();
+    }
+
     my @rosterlist;
     foreach my $jid ( $self->jabber_client->RosterDBJIDs() ) {
         my $username = $jid->GetJID();
@@ -1176,6 +1193,8 @@ sub GetStatus {
     my $self = shift;
     my ($jid) = shift;
 
+    return "unavailable" if !$self->IsConnected;
+
     my $Pres = $self->jabber_client->PresenceDBQuery($jid);
 
     if ( !( defined($Pres) ) ) {
@@ -1203,6 +1222,11 @@ sub AddUser {
     my $self = shift;
     my $user = shift;
 
+    if ( !$self->IsConnected ) {
+        WARN("Cannot add user '$user': not connected");
+        return;
+    }
+
     $self->jabber_client->Subscription( type => "subscribe",  to => $user );
     $self->jabber_client->Subscription( type => "subscribed", to => $user );
 }
@@ -1217,6 +1241,11 @@ sub RmUser {
     my $self = shift;
     my $user = shift;
 
+    if ( !$self->IsConnected ) {
+        WARN("Cannot remove user '$user': not connected");
+        return;
+    }
+
     $self->jabber_client->Subscription( type => "unsubscribe",  to => $user );
     $self->jabber_client->Subscription( type => "unsubscribed", to => $user );
 }

t/07-test_disconnect_public_api.t

@@ -0,0 +1,73 @@
+#!perl
+
+use strict;
+use warnings;
+
+BEGIN { *CORE::GLOBAL::sleep = sub { }; }
+
+use Test::More tests => 9;
+use Net::Jabber::Bot;
+
+use FindBin;
+use lib "$FindBin::Bin/lib";
+use MockJabberClient;
+
+my $bot_alias = 'make_test_bot';
+my $server    = 'talk.google.com';
+
+my %forums_and_responses;
+$forums_and_responses{'test_forum1'} = [ "jbot:", "" ];
+
+my $bot = Net::Jabber::Bot->new(
+    server                 => $server,
+    conference_server      => "conference.$server",
+    port                   => 5222,
+    username               => 'test_username',
+    password               => 'test_pass',
+    alias                  => $bot_alias,
+    message_function       => sub { },
+    background_function    => sub { },
+    loop_sleep_time        => 5,
+    process_timeout        => 5,
+    forums_and_responses   => \%forums_and_responses,
+    ignore_server_messages => 1,
+    ignore_self_messages   => 1,
+    out_messages_per_second => 5,
+    max_message_size       => 1000,
+    max_messages_per_hour  => 100,
+    forum_join_grace       => 0,
+);
+
+isa_ok( $bot, "Net::Jabber::Bot" );
+ok( $bot->IsConnected(), "Bot starts connected" );
+
+# Disconnect the bot
+$bot->Disconnect();
+ok( !$bot->IsConnected(), "Bot is disconnected" );
+
+# Test that public API methods don't crash when disconnected.
+# Before this fix, each of these would die with:
+#   "Can't call method ... on an undefined value"
+# because jabber_client is undef after Disconnect().
+
+# ChangeStatus should return 0 (failure), not crash
+my $status_result = eval { $bot->ChangeStatus("available", "test status") };
+ok( !$@, "ChangeStatus does not crash when disconnected" )
+    or diag("ChangeStatus died: $@");
+
+# GetRoster should return empty list, not crash
+my @roster = eval { $bot->GetRoster() };
+ok( !$@, "GetRoster does not crash when disconnected" )
+    or diag("GetRoster died: $@");
+is( scalar @roster, 0, "GetRoster returns empty list when disconnected" );
+
+# GetStatus should return 'unavailable', not crash
+my $get_status = eval { $bot->GetStatus("someone\@$server") };
+ok( !$@, "GetStatus does not crash when disconnected" )
+    or diag("GetStatus died: $@");
+is( $get_status, "unavailable", "GetStatus returns 'unavailable' when disconnected" );
+
+# Process should return undef, not crash
+my $process_result = eval { $bot->Process(1) };
+ok( !$@, "Process does not crash when disconnected" )
+    or diag("Process died: $@");