Priority support, testing problems

Nikolas Coukouma atrus at atrus.org
Mon Jul 17 09:00:32 UTC 2006


-----BEGIN PGP SIGNED MESSAGE-----
Hash: RIPEMD160

Attached is a new patch that includes about ten test cases for priority
stuff, plus a test for unavailable presence in two-resources. There's
also a little fix for LOGLEVEL clobbering. Let me know if I should break
the patch up.

I spent quite a few hours wrangling the tests because DJabberd is
asynchronous; sometimes stanzas aren't handled in the intended order,
which is a problem if state is modified. So, I've added a method
cleverly called sync and you can pass a true value to send_xml to do a
sync after the XML is sent. The sync works by sending a service
discovery request  to the server and waiting for the response. I tried
simply sleeping a bit, but that was slow and didn't consistently avoid
the problem. Better ideas are certainly welcome.

- -Nikolas
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (Darwin)

iD8DBQFEu1Gvs2zR4YuWmeERA9lAAJ9KQLBZcyuDgmdXv95PZijWvQJbVQCdGo0S
RkBByDmDvRqSa2YsCvLUdRg=
=JlQf
-----END PGP SIGNATURE-----
-------------- next part --------------
Index: t/two-resources.t
===================================================================
--- t/two-resources.t	(revision 646)
+++ t/two-resources.t	(working copy)
@@ -1,8 +1,8 @@
 #!/usr/bin/perl
 use strict;
-use Test::More tests => 24;
+use Test::More tests => 32;
 use lib 't/lib';
-BEGIN { $ENV{LOGLEVEL} = 'FATAL'; }
+BEGIN { $ENV{LOGLEVEL} ||= 'FATAL'; }
 require 'djabberd-test.pl';
 
 two_parties(sub {
@@ -23,16 +23,13 @@
                                           );
     $pa2->login;
     $pa2->send_xml("<presence/>");
-    $xml = $pa2->recv_xml;
-    like($xml, qr{^<presence.+from=.partyb}, "partya2 now also knows of partyb being online");
+    like($pa2->recv_xml, qr{^<presence.+from=.partyb}, "partya2 now also knows of partyb being online");
 
     $pb->send_xml(qq{<presence><status>BisHere</status></presence>});
 
     # both of A's resources should get it.
-    $xml = $pa->recv_xml;
-    like($xml, qr{BisHere}, "partya1 got B's presence");
-    $xml = $pa2->recv_xml;
-    like($xml, qr{BisHere}, "partya2 got B's presence");
+    like($pa->recv_xml, qr{BisHere}, "partya1 got B's presence");
+    like($pa2->recv_xml, qr{BisHere}, "partya2 got B's presence");
 
     # both of A's resources should get a message from pb to pa's bare JID
     unlike("$pa", qr!/!, "stringification of pa doesn't include a resource");
@@ -46,6 +43,20 @@
     like($pa->recv_xml,  qr{you are testsuite}, "pa got uniq message");
     like($pa2->recv_xml, qr{you are conn2}, "pb got uniq message");
 
+    # make pa unavailable
+    $pa->send_xml("<presence type='unavailable' />", 1);
+
+    # pa is not available, so fall back to bare jid handling
+    $pb->send_xml("<message type='chat' to='$pa'>HelloMsg from $pb.</message>");
+    ok(!$pa->recv_xml(0.1), "didn't get message (unavailable)");
+    like($pa2->recv_xml, qr{HelloMsg from}, "got message for pa2 from pb (fall back)");
+
+    $pb->send_xml(qq{<presence><status>BisHere</status></presence>});
+
+    # unavailable resources should not get presence notifications
+    ok(!$pa->recv_xml(0.1), "partya1 didn't get B's presence (unavailable)");
+    like($pa2->recv_xml, qr{BisHere}, "partya2 got B's presence");
+
     # TODO: try connecting a third $pa, but with a dup resource, and
     # watch it get bitch slapped
 
@@ -58,9 +69,4 @@
 
     like($pa2->recv_xml, qr{conflict}, "got stream conflict error");
     is($pa2->get_event(2, 1),"end-stream", "got closed stream");
-
-    # TODO: test that conn2 goes unavailable, then sending a message to /conn2 should
-    # really act as if it's to a barejid, and go to all available resources.
-    # -- we need the test and the implementation
-
 });
Index: t/priorities.t
===================================================================
--- t/priorities.t	(revision 0)
+++ t/priorities.t	(revision 0)
@@ -0,0 +1,105 @@
+#!/usr/bin/perl
+use strict;
+use Test::More tests => 48;
+use lib 't/lib';
+BEGIN { $ENV{LOGLEVEL} ||= 'FATAL'; }
+require 'djabberd-test.pl';
+
+two_parties(sub {
+    my ($pa, $pb) = @_;
+    $pa->login;
+    $pb->login;
+    $pa->send_xml("<presence/>");
+    $pb->send_xml("<presence/>");
+    select(undef,undef,undef,0.25);
+
+    $pa->subscribe_successfully($pb);
+
+    my $xml;
+
+    my $pa2 = Test::DJabberd::Client->new(server   => $pa->server,
+                                          name     => $pa->username,
+                                          resource => "conn2",
+                                          );
+    $pa2->login;
+    $pa2->send_xml("<presence/>");
+    $xml = $pa2->recv_xml;
+    like($xml, qr{^<presence.+from=.partyb}, "partya2 now also knows of partyb being online");
+
+    $pb->send_xml(qq{<presence><status>BisHere</status></presence>});
+
+    # both of A's resources should get it.
+    like($pa->recv_xml, qr{BisHere}, "partya1 got B's presence");
+    like($pa2->recv_xml, qr{BisHere}, "partya2 got B's presence");
+
+    # both of A's resources should get a message from pb to pa's bare JID
+    unlike("$pa", qr!/!, "stringification of pa doesn't include a resource");
+    $pb->send_xml("<message type='chat' to='$pa'>HelloMsg from $pb.</message>");
+    like($pa->recv_xml, qr{HelloMsg from}, "message to pa from pb (pa=0, pa2=0)");
+    like($pa2->recv_xml, qr{HelloMsg from}, "message to pa2 from pb (pa=0, pa2=0)");
+
+    # adjust priority of pa. Now pa=1 pa2=0
+    $pa->send_xml("<presence><priority>1</priority></presence>", 1);
+
+    # only pa should recieve this
+    $pb->send_xml("<message type='chat' to='$pa'>HelloMsg from $pb.</message>");
+
+    like($pa->recv_xml, qr{HelloMsg from}, "message to pa from pb (pa=1, pa2=0)");
+    ok(!$pa2->recv_xml(0.1), "no message to pa2 from pb (pa=1, pa2=0)");
+
+    # now send presence
+    $pb->send_xml(qq{<presence><status>BisHere</status></presence>});
+
+    # both of A's resources should get it. (priority only applies to messages)
+    like($pa->recv_xml, qr{BisHere}, "partya1 got B's presence (pa=1, pa2=0)");
+    like($pa2->recv_xml, qr{BisHere}, "partya2 got B's presence (pa=1, pa2=0)");
+
+    # adjust priority of pa2. Now pa=1 pa2=1
+    $pa2->send_xml("<presence><priority>1</priority></presence>", 1);
+
+    # both A's should recieve this
+    $pb->send_xml("<message type='chat' to='$pa'>HelloMsg from $pb.</message>");
+    like($pa->recv_xml, qr{HelloMsg from}, "message to pa from pb (pa=1, pa2=1)");
+    like($pa2->recv_xml, qr{HelloMsg from}, "message to pa2 from pb (pa=1, pa2=1)");
+
+    # set both to negative priorities
+    $pa->send_xml("<presence><priority>-1</priority></presence>", 1);
+    $pa2->send_xml("<presence><priority>-1</priority></presence>", 1);
+
+    # we shouldn't send messages to resources with negative jids
+    $pb->send_xml("<message type='chat' to='$pa'>HelloMsg from $pb.</message>");
+    ok(!$pa->recv_xml(0.1), "no message to pa from pb (pa=-1, pa2=-1)");
+    ok(!$pa2->recv_xml(0.1), "no message to pa2 from pb (pa=-1, pa2=-1)");
+
+    # edge values for priorities
+    $pa->send_xml("<presence><priority>-128</priority></presence>", 1);
+    $pa2->send_xml("<presence><priority>127</priority></presence>", 1);
+
+    # this should work just like the pa=0, pa2=1 case above
+    $pb->send_xml("<message type='chat' to='$pa'>HelloMsg from $pb.</message>");
+    ok(!$pa->recv_xml(0.1), "no message to pa from pb (pa=-128, pa2=127)");
+    like($pa2->recv_xml, qr{HelloMsg from}, "message to pa2 from pb (pa=-128, pa2=127)");
+
+    # make sure that no priority element is interpreted as priority = 0
+    $pa->send_xml("<presence><priority>1</priority></presence>", 1);
+    $pa2->send_xml("<presence/>", 1);
+
+    # should behave as if pa=1, pa2=0 so only pa2 should get it
+    $pb->send_xml("<message type='chat' to='$pa'>HelloMsg from $pb.</message>");
+    like($pa->recv_xml, qr{HelloMsg from}, "message to pa from pb (pa=1, pa2=0)");
+    ok(!$pa2->recv_xml(0.1), "no message to pa2 from pb (pa=1, pa2=0)");
+
+    # make pa unavailable. pa=10, pa2=0
+    $pa->send_xml("<presence type='unavailable'><priority>10</priority></presence>", 1);
+
+    # although pa has a higher priority, it is not available
+    $pb->send_xml("<message type='chat' to='$pa'>HelloMsg from $pb.</message>");
+    ok(!$pa->recv_xml(0.1), "no message to pa from pb (pa unavailable, pa=10, pa2=0)");
+    like($pa2->recv_xml, qr{HelloMsg from}, "message to pa2 from pb (pa unavailable, pa=10, pa2=0)");
+
+    # pa is unavailable, so messages to it's full jid should go to pa2
+    $pb->send_xml("<message type='chat' to='$pa/testsuite'>HelloMsg from $pb.</message>");
+    ok(!$pa->recv_xml(0.1), "no message to testsuite from pb (pa unavailable, pa=10, pa2=0)");
+    like($pa2->recv_xml, qr{HelloMsg from}, "message to testsuite from pb (pa unavailable, pa=10, pa2=0)");
+
+});
Index: t/lib/djabberd-test.pl
===================================================================
--- t/lib/djabberd-test.pl	(revision 646)
+++ t/lib/djabberd-test.pl	(working copy)
@@ -433,9 +433,23 @@
 sub send_xml {
     my $self = shift;
     my $xml  = shift;
+    my $sync  = shift;
     $self->{sock}->print($xml);
+    $self->sync if $sync;
 }
 
+sub sync {
+    my $self = shift;
+    my $cjid = $self->as_string;
+    my $sjid = $self->{server}->hostname;
+    $self->send_xml(<<XXX);
+<iq type='get' from='$cjid' to='$sjid' id='sync'>
+  <query xmlns='http://jabber.org/protocol/disco#items'/>
+</iq>
+XXX
+    $self->recv_xml;
+}
+
 sub create_fresh_account {
     my $self = shift;
     eval {
Index: doc/TODO
===================================================================
--- doc/TODO	(revision 646)
+++ doc/TODO	(working copy)
@@ -50,8 +50,6 @@
        4 < <iq id='SgSeu-6' type='get'><query xmlns='jabber:iq:anything'/></iq> INFO  DJabberd.Connection.XML.ClientIn         <iq  to='user0 at jabber.bradfitz.com/Smack' type='error' id='SgSeu-6'></iq> WARN  DJabberd.IQ                              Unknown IQ packet: get-{jabber:iq:anything}query
 
 
--- presence priority
-
 -- allow a fast path when bulk sending tons of messages (like hundreds of unavailable stanzas
    on disconnect):  allow a means to, given a set of full/bare (at least bare) JIDs, filter out
    the ones to be known not available.  (for instance, local/cluster
Index: lib/DJabberd/Presence.pm
===================================================================
--- lib/DJabberd/Presence.pm	(revision 646)
+++ lib/DJabberd/Presence.pm	(working copy)
@@ -467,6 +467,15 @@
     DJabberd::Presence->set_local_presence($jid, $self->clone);
 
     $conn->set_available(1);
+    
+    my $pri = 0;
+    foreach my $elm ($self->children_elements) {
+        if ($elm->namespace eq "jabber:client" && $elm->element_name eq "priority") {
+            
+            $pri = $elm->first_child+0 if $elm->first_child && !(ref $elm->first_child);
+        }
+    }
+    $conn->set_priority($pri);
 
     if ($conn->is_initial_presence) {
         $conn->on_initial_presence;
Index: lib/DJabberd/Delivery/Local.pm
===================================================================
--- lib/DJabberd/Delivery/Local.pm	(revision 646)
+++ lib/DJabberd/Delivery/Local.pm	(working copy)
@@ -11,7 +11,19 @@
 
     my @dconns;
     my $find_bares = sub {
-        @dconns = grep { $_->is_available || $stanza->deliver_when_unavailable } $vhost->find_conns_of_bare($to)
+        @dconns = grep { $_->is_available || $stanza->deliver_when_unavailable } $vhost->find_conns_of_bare($to);
+        
+        if ($stanza->isa("DJabberd::Message")) {
+            my $max_pri = -1;
+            foreach my $p (@dconns) { $max_pri = $p->priority if $p->priority > $max_pri }
+            if ($max_pri < 0) {
+                @dconns = ();
+            } else {
+                # FIXME: should we deliver to all resources with highest priority,
+                #   or just pick one? If picking one, then how?
+                @dconns = grep { $_->priority == $max_pri } @dconns;
+            }
+        }
     };
 
     if ($to->is_bare) {
Index: lib/DJabberd/Connection/ClientIn.pm
===================================================================
--- lib/DJabberd/Connection/ClientIn.pm	(revision 646)
+++ lib/DJabberd/Connection/ClientIn.pm	(working copy)
@@ -10,6 +10,7 @@
             'is_available',          # bool: is an "available resource"
             'directed_presence',     # the jids we have sent directed presence too
             'pend_in_subscriptions', # undef or arrayref of presence type='subscribe' packets to be redelivered when we become available
+            'priority',              # int: the priority of this resource
             );
 
 sub note_pend_in_subscription {
@@ -58,6 +59,17 @@
     return $self->{is_available};
 }
 
+sub set_priority {
+    my ($self, $val) = @_;
+    $self->{priority} = $val;
+}
+
+sub priority {
+    my $self = shift;
+    return $self->{priority};
+}
+
+
 # called when a presence broadcast is received.  on first time,
 # returns tru.
 sub is_initial_presence {


More information about the Djabberd mailing list