commit 7a039fb7e4d0becfecce7721dc03d65262c9bba5 parent 9f0389d0970a644cb1b7fad044491c0210a940d0 Author: Kyle Milz <kyle@0x30.net> Date: Sat, 20 Feb 2016 19:30:23 -0700 sl: unify how request/response data is specified - in the request/response root object we now have a 'data' key that maps to whatever data this message type needs - before this field would be dependent on message type, making more work for ourselves by special casing each message type - now all messages follow the same format: { "device_id": "somedeviceid", "data": { "phone_number": 4037082091, "name": "list" } } Diffstat:
26 files changed, 220 insertions(+), 295 deletions(-)
diff --git a/server/sl b/server/sl @@ -106,7 +106,7 @@ while (my $client_socket = $server_socket->accept()) { # 'device_id' in the request. Check that here. my $device = undef; if ($msg_type != $msg_num{device_add}) { - (my $err, $device) = get_device($db, $request); + (my $err, $device) = get_device($db, $request->{device_id}); if ($err) { send_msg($client_socket, $ver, $msg_type, make_error($err)); @@ -118,7 +118,7 @@ while (my $client_socket = $server_socket->accept()) { # Call appropriate message handler. Each handler returns both # data that should be sent back over the main socket and # notification data that gets sent over vendor specific API. - my ($response, $notify) = $msg_func[$msg_type]->($db, $request, $device); + my ($response, $notify) = $msg_func[$msg_type]->($db, $request->{data}, $device); $db->{dbh}->commit; if ($@) { @@ -157,14 +157,13 @@ while (my $client_socket = $server_socket->accept()) { # Takes a request and verifies device_id is present and valid sub get_device { - my ($db, $request) = @_; + my ($db, $device_id) = @_; - unless (exists $request->{'device_id'}) { + unless (defined $device_id) { $log->print("bad request, missing key 'device_id'\n"); return ("a missing message argument was required"); } - my $device_id = $request->{'device_id'}; unless ($device_id && $device_id =~ m/^[a-zA-Z0-9+\/=]+$/) { $log->print("bad device id\n"); return ('the client sent a device id that was not base64'); @@ -375,7 +374,7 @@ sub msg_device_add { $db->{new_device}->execute($device_id, $ph_num, $os, undef, time, time); $log->print("success, '$ph_num':'$fp' os '$os'\n"); - return (make_ok( { device_id => $device_id } ), undef); + return (make_ok( { data => $device_id } ), undef); } # 'device_update' message handler. Takes a device_id and a token and updates the @@ -389,17 +388,16 @@ sub msg_device_update { $db->{update_device}->execute($hex_token, $dev->{num}); # $log->print("push token = '$hex_token'\n"); - return make_ok(); + return make_ok( { data => {} }); } # Takes a device_id and a list structure and records this list in the database. # Also prepares an friend_added_list notification that should be sent to all my # mutual friends. sub msg_list_add { - my ($db, $request, $dev) = @_; + my ($db, $list, $dev) = @_; # XXX: check that $list contains the necessary keys! - my $list = $request->{list}; $log->print("device '$dev->{fp}'\n"); $log->print("new list name '$list->{name}'\n"); @@ -424,7 +422,7 @@ sub msg_list_add { members => [ $dev->{phnum} ], num_members => 1 }; - my $response = make_ok( { list => $resp_list } ); + my $response = make_ok( { data => $resp_list } ); $log->print("new list number is '$list_num'\n"); @@ -438,7 +436,7 @@ sub msg_list_add { # selected above. Their client shows your new lists in their other lists # section, which doesn't need a lot of information. $notify->{msg_type} = 'friend_added_list'; - $notify->{payload} = { + $notify->{data} = { num => $resp_list->{num}, name => $list->{name}, members => [ $dev->{phnum} ], @@ -449,9 +447,7 @@ sub msg_list_add { } sub msg_list_update { - my ($db, $request, $dev) = @_; - - my $list = $request->{'list'}; + my ($db, $list, $dev) = @_; my ($err) = list_number_valid($db, $list->{num}); return make_error($err) if ($err); @@ -475,7 +471,7 @@ sub msg_list_update { $notify->{devices} = [@{ $mutual_friends }, @{ $list_members }]; $notify->{msg_type} = 'updated_list'; - $notify->{payload} = { + $notify->{data} = { num => $list->{num}, name => $list->{name}, date => $list->{date} @@ -488,7 +484,7 @@ sub msg_list_update { $log->print("name = '$list->{name}'\n") if (exists $list->{name}); $log->print("date = $list->{date}\n") if (exists $list->{date}); - return (make_ok(), $notify); + return (make_ok( { data => {} } ), $notify); } sub msg_list_item_add { @@ -511,8 +507,7 @@ sub msg_list_item_add { } sub msg_list_join { - my ($db, $request, $dev) = @_; - my $list_num = $request->{list_num}; + my ($db, $list_num, $dev) = @_; my ($list_err, $list_num_num, $list_name, $list_date) = list_number_valid($db, $list_num); return make_error($list_err) if ($list_err); @@ -546,12 +541,11 @@ sub msg_list_join { $log->print("device '$dev->{fp}'\n"); $log->print("list '$list_num'\n"); - return make_ok( { list => $list } ); + return make_ok( { data => $list } ); } sub msg_list_leave { - my ($db, $request, $dev) = @_; - my $list_num = $request->{list_num}; + my ($db, $list_num, $dev) = @_; my ($err) = list_number_valid($db, $list_num); return make_error($err) if ($err); @@ -587,12 +581,11 @@ sub msg_list_leave { $log->print("device '$dev->{fp}'\n"); $log->print("list '$list_num'\n"); - return make_ok($response); + return make_ok( { data => $response } ); } sub msg_friend_add { - my ($db, $request, $dev) = @_; - my $friend_phnum = $request->{'friend_phnum'}; + my ($db, $friend_phnum, $dev) = @_; $log->print("'$dev->{fp}' adding '$friend_phnum'\n"); @@ -629,12 +622,11 @@ sub msg_friend_add { } } - return make_ok( { friend_phnum => $friend_phnum } ); + return make_ok( { data => $friend_phnum } ); } sub msg_friend_delete { - my ($db, $request, $dev) = @_; - my $friend_phnum = $request->{'friend_phnum'}; + my ($db, $friend_phnum, $dev) = @_; unless (looks_like_number($friend_phnum)) { $log->print("bad friends number '$friend_phnum'\n"); @@ -660,7 +652,7 @@ sub msg_friend_delete { $db->{mutual_friends_delete}->execute($friend_num, $dev->{num}); } - return make_ok( { friend_phnum => $friend_phnum } ); + return make_ok( { data => $friend_phnum } ); } # Takes no arguments and finds all of the lists that the given device_id is in. @@ -700,7 +692,7 @@ sub msg_lists_get { $log->print("list has 0 items\n"); } - return make_ok({ lists => \@lists, num_lists => scalar(@lists) }); + return make_ok( { data => \@lists} ); } sub msg_lists_get_other { @@ -743,11 +735,7 @@ sub msg_lists_get_other { } my @other_lists = values(%list_nums); - my $response = { - other_lists => \@other_lists, - num_other_lists => scalar(@other_lists) - }; - return make_ok($response); + return make_ok( { data => \@other_lists } ); } sub msg_list_items_get { diff --git a/server/t/client.pm b/server/t/client.pm @@ -10,7 +10,6 @@ use test; require "msgs.pl"; our (%msg_num, @msg_str); -my $i = 0; sub new { my $class = shift; @@ -32,155 +31,109 @@ sub new { $self->{device_id} = undef; - # Register this device immediately by default if ($dont_register == 0) { - $self->device_add({ phone_number => rand_phnum(), os => 'unix' }); - $self->device_update({ pushtoken_hex => "BADBEEF_$i" }); - $i++; + $self->{phnum} = rand_phnum(); + + my $args = { phone_number => $self->{phnum}, os => 'unix' }; + $self->{device_id} = $self->device_add($args); + + $self->device_update({ pushtoken_hex => "token_$self->{phnum}" }, 'ok'); } return $self; } -sub device_update { - my $self = shift; - my $msg_args = shift; - my $status = shift || 'ok'; +sub device_add { + my ($self, $args, $status) = @_; + return $self->communicate('device_add', $status, $args); +} - my $response = communicate($self, 'device_update', $status, $msg_args); +sub device_update { + my ($self, $args, $status) = @_; + return $self->communicate('device_update', $status, $args); } sub list_add { - my $self = shift; - my $list = { - name => shift, - date => 0 - }; - my $status = shift || 'ok'; - - my $response = communicate($self, 'list_add', $status, { list => $list }); - return if ($status eq 'err'); - - push @{$self->{lists}}, $response->{list}; + my ($self, $args, $status) = @_; + return $self->communicate('list_add', $status, $args); } sub list_update { - my $self = shift; - my $list_ref = shift; - my $status = shift || 'ok'; - - my $list_data = communicate($self, 'list_update', $status, { list => $list_ref }); - return if ($status eq 'err'); + my ($self, $args, $status) = @_; + return $self->communicate('list_update', $status, $args); } sub list_join { - my $self = shift; - my $msg_args = { - list_num => shift, - }; - my $status = shift || 'ok'; - - my $list_data = communicate($self, 'list_join', $status, $msg_args); + my ($self, $args, $status) = @_; + return $self->communicate('list_join', $status, $args); } sub list_leave { - my $self = shift; - my $msg_args = { - list_num => shift, - }; - my $status = shift || 'ok'; - - my $list_data = communicate($self, 'list_leave', $status, $msg_args); + my ($self, $args, $status) = @_; + return $self->communicate('list_leave', $status, $args); } sub friend_add { - my $self = shift; - my $msg_args = { - friend_phnum => shift, - }; - my $status = shift || 'ok'; - - communicate($self, 'friend_add', $status, $msg_args); + my ($self, $args, $status) = @_; + return $self->communicate('friend_add', $status, $args); } sub friend_delete { - my $self = shift; - my $msg_args = { - friend_phnum => shift, - }; - my $status = shift || 'ok'; - - communicate($self, 'friend_delete', $status, $msg_args); + my ($self, $args, $status) = @_; + return $self->communicate('friend_delete', $status, $args); } sub lists_get { - my $self = shift; - my $status = shift || 'ok'; - - my $response = communicate($self, 'lists_get', $status); - return if ($response->{status} eq 'err'); - - return @{ $response->{lists} }; + my ($self, $status) = @_; + return $self->communicate('lists_get', $status); } sub lists_get_other { - my $self = shift; - my $status = shift || 'ok'; - - my $response = communicate($self, 'lists_get_other', $status); - return if ($response->{status} eq 'err'); - - return @{ $response->{other_lists} }; + my ($self, $status) = @_; + return $self->communicate('lists_get_other', $status); } -sub device_add { - my $self = shift; - my $msg_args = shift; - my $exp_status = shift || 'ok'; +sub communicate { + my ($self, $msg_type, $exp_status, $msg_data) = @_; - # Reset error messages to guard against stale state - $self->{err_msg} = undef; - $self->{msg_type} = $msg_num{'device_add'}; + # If no expected status was passed in assume 'ok' + $exp_status = 'ok' if (! defined $exp_status); - send_msg($self, $msg_args); - my $response = recv_msg($self, $exp_status); + my $msg_args->{data} = $msg_data; - if ($response->{status} eq 'ok') { - $self->{phnum} = $msg_args->{phone_number}; - $self->{device_id} = $response->{device_id}; - } -} + # device_add is the only message type that does not require device_id as + # a mandatory argument + $msg_args->{device_id} = $self->{device_id} if ($msg_type ne 'device_add'); -sub communicate { - my ($self, $msg_type, $exp_status, $msg_args) = @_; + $self->send_msg($msg_type, $msg_args); + my $resp = $self->recv_msg($msg_type); - # Reset error message so it doesn't get reused - $self->{err_msg} = undef; - $self->{msg_type} = $msg_num{$msg_type}; + # Check that the received status was the same as the expected status + my $status = $resp->{status}; + confess "wrong message status '$status'" if ($status ne $exp_status); - # Add device id to message arguments - $msg_args->{device_id} = $self->{device_id}; + # Response indicated error, return the reason + return $resp->{reason} if ($status eq 'err'); - send_msg($self, $msg_args); - return recv_msg($self, $exp_status); + # Everything looks good, return the response data + return $resp->{data}; } sub send_msg { - my ($self, $request) = @_; + my ($self, $msg_type, $request) = @_; # Request comes in as a hash ref, do this now to figure out length my $payload = encode_json($request); - my $msg_type = $self->{msg_type}; - confess "invalid message type $msg_type" if ($msg_type > @msg_str); + confess "invalid message type $msg_type" unless (grep { $_ eq $msg_type } @msg_str); my $version = 0; my $payload_len = length($payload); - my $header = pack("nnn", $version, $msg_type, $payload_len); + my $header = pack("nnn", $version, $msg_num{$msg_type}, $payload_len); my $sent_bytes = 0; - $sent_bytes += send_all($self, $header, length($header)); - $sent_bytes += send_all($self, $payload, $payload_len); + $sent_bytes += $self->send_all($header, length($header)); + $sent_bytes += $self->send_all($payload, $payload_len); return $sent_bytes; } @@ -197,20 +150,20 @@ sub send_all { } sub recv_msg { - my ($self, $exp_status) = @_; + my ($self, $exp_msg_type) = @_; # Read header - my $header = read_all($self, 6); + my $header = $self->read_all(6); my ($version, $msg_type, $payload_size) = unpack("nnn", $header); # Check some things confess "unsupported protocol version $version" if ($version != 0); confess "unknown message type $msg_type" if ($msg_type >= @msg_str); confess "0 byte payload" if ($payload_size == 0); - confess "unexpected message type $self->{msg_type}" if ($self->{msg_type} != $msg_type); + confess "unexpected message type $msg_type" if ($msg_num{$exp_msg_type} != $msg_type); # Read again for payload, $payload_size > 0 - my $payload = read_all($self, $payload_size); + my $payload = $self->read_all($payload_size); my $response; try { @@ -219,15 +172,11 @@ sub recv_msg { confess "server sent invalid json"; }; - # Don't accept messages with an array root + # Don't accept messages without an object root (ie array roots) if (ref($response) ne "HASH") { confess "server didn't send back object root element"; } - my $status = $response->{status}; - confess "wrong message status '$status'" if ($status ne $exp_status); - $self->{err_msg} = $response->{reason} if ($status eq 'err'); - return $response; } @@ -249,16 +198,6 @@ sub read_all { return $data; } -sub num_lists { - my ($self) = @_; - return scalar(@{$self->{lists}}); -} - -sub lists { - my ($self, $i) = @_; - return $self->{lists}[$i]; -} - sub phnum { my ($self) = @_; return $self->{phnum}; @@ -274,14 +213,4 @@ sub set_device_id { $self->{device_id} = $new_id; } -sub set_msg_type { - my ($self, $msg_num) = @_; - $self->{msg_type} = $msg_num{$msg_num}; -} - -sub get_error { - my $self = shift; - return $self->{err_msg}; -} - 1; diff --git a/server/t/device_add/test.pl b/server/t/device_add/test.pl @@ -13,16 +13,16 @@ fail "device id '$devid' not base64" unless ($devid =~ m/^[a-zA-Z0-9+\/=]+$/); fail "expected device id length of 43, got $length" if ($length != 43); # Duplicate phone number -$A->device_add({ phone_number => $A->phnum, os => 'unix' }, 'err'); -fail_msg_ne 'the sent phone number already exists', $A->get_error(); +my $err = $A->device_add({ phone_number => $A->phnum, os => 'unix' }, 'err'); +fail_msg_ne 'the sent phone number already exists', $err; # Bad phone number -$A->device_add({ phone_number => '403867530&', os => 'unix' }, 'err'); -fail_msg_ne 'the sent phone number is not a number', $A->get_error(); +$err = $A->device_add({ phone_number => '403867530&', os => 'unix' }, 'err'); +fail_msg_ne 'the sent phone number is not a number', $err; # Bad operating system -$A->device_add({ phone_number => rand_phnum(), os => 'bados' }, 'err'); -fail_msg_ne 'operating system not supported', $A->get_error(); +$err = $A->device_add({ phone_number => rand_phnum(), os => 'bados' }, 'err'); +fail_msg_ne 'operating system not supported', $err; # Good operating systems $A->device_add({ phone_number => rand_phnum(), os => 'android' }); diff --git a/server/t/friend_add/test.pl b/server/t/friend_add/test.pl @@ -13,13 +13,13 @@ $A->friend_add('54321'); $A->friend_add('54321'); # Non numeric phone number -$A->friend_add('123asdf', 'err'); -fail_msg_ne 'friends phone number is not a valid phone number', $A->get_error(); +my $err = $A->friend_add('123asdf', 'err'); +fail_msg_ne 'friends phone number is not a valid phone number', $err; # Empty phone number -$A->friend_add('', 'err'); -fail_msg_ne 'friends phone number is not a valid phone number', $A->get_error(); +$err = $A->friend_add('', 'err'); +fail_msg_ne 'friends phone number is not a valid phone number', $err; # Friending yourself -$A->friend_add($A->phnum(), 'err'); -fail_msg_ne 'device cannot add itself as a friend', $A->get_error(); +$err = $A->friend_add($A->phnum(), 'err'); +fail_msg_ne 'device cannot add itself as a friend', $err; diff --git a/server/t/friend_delete/test.pl b/server/t/friend_delete/test.pl @@ -12,13 +12,13 @@ $A->friend_add($B->phnum()); $B->friend_add($A->phnum()); # A creates 2 lists -$A->list_add("this is a's first list"); -$A->list_add("this is a's second list"); +my $As_first_list = $A->list_add({ name => "this is a's first list", date => 0 }); +$A->list_add({ name => "this is a's second list", date => 0 }); # B creates 1 list -$B->list_add("this is b's first list"); +$B->list_add({ name => "this is b's first list", date => 0}); -# B joins one of A's list -$B->list_join($A->lists(0)->{num}); +# B joins A's first list +$B->list_join($As_first_list->{num}); # A deletes B's friendship $A->friend_delete($B->phnum()); @@ -27,8 +27,12 @@ $A->friend_delete($B->phnum()); # - A and B are both in A's first list # - B can't see A's other list # - A can't see B's other list -fail "expected A to have 0 other lists" if ($A->lists_get_other() != 0); -fail "expected B to have 0 other lists" if ($B->lists_get_other() != 0); +my $A_other_lists = scalar @{ $A->lists_get_other() }; +my $B_other_lists = scalar @{ $B->lists_get_other() }; +fail "expected A to have 0 other lists" if ($A_other_lists != 0); +fail "expected B to have 0 other lists" if ($B_other_lists != 0); -fail "expected A to have 2 lists" if ($A->lists_get() != 2); -fail "expected B to have 2 lists" if ($B->lists_get() != 2); +my $A_num_lists = scalar @{ $A->lists_get() }; +my $B_num_lists = scalar @{ $B->lists_get() }; +fail "expected A to have 2 lists" if ($A_num_lists != 2); +fail "expected B to have 2 lists" if ($B_num_lists != 2); diff --git a/server/t/friend_delete_unit/test.pl b/server/t/friend_delete_unit/test.pl @@ -6,14 +6,18 @@ use test; my $A = client->new(); -# try deleting someone who is not your friend -$A->friend_delete('12345', 'err'); -fail_msg_ne 'friend sent for deletion was not a friend', $A->get_error(); +# Someone who is not your friend +my $err = $A->friend_delete('12345', 'err'); +fail_msg_ne 'friend sent for deletion was not a friend', $err; -# also verify that a non numeric friends phone number isn't accepted -$A->friend_delete('asdf123', 'err'); -fail_msg_ne 'friends phone number is not a valid phone number', $A->get_error(); +# Non numeric friends phone number +$err = $A->friend_delete('asdf123', 'err'); +fail_msg_ne 'friends phone number is not a valid phone number', $err; -# also verify an empty phone number isn't accepted -$A->friend_delete('', 'err'); -fail_msg_ne 'friends phone number is not a valid phone number', $A->get_error(); +# Empty phone number +$err = $A->friend_delete('', 'err'); +fail_msg_ne 'friends phone number is not a valid phone number', $err; + +# Add/delete cycle works +$A->friend_add('12345'); +$A->friend_delete('12345'); diff --git a/server/t/get_other_lists_filters_my_lists/test.pl b/server/t/get_other_lists_filters_my_lists/test.pl @@ -12,16 +12,16 @@ my $A = client->new(); my $B = client->new(); # B adds a new list -$B->list_add('bs new list'); +$B->list_add({ name => 'bs new list', date => 0 }); # A and B become mutual friends $A->friend_add($B->phnum()); $B->friend_add($A->phnum()); # A adds a new list, B joins A's new list -$A->list_add('as new list'); -$B->list_join($A->lists(0)->{num}); +my $list = $A->list_add({ name => 'as new list', date => 0 }); +$B->list_join($list->{num}); # A should only see B's list that it never joined -my @other = $A->lists_get_other(); -fail_num_ne 'wrong number of other lists ', scalar(@other), 1; +my $other = $A->lists_get_other(); +fail_num_ne 'wrong number of other lists ', scalar(@$other), 1; diff --git a/server/t/invalid_deviceid/test.pl b/server/t/invalid_deviceid/test.pl @@ -6,7 +6,9 @@ use test; # test that sending invalid device id's results in errors +# Don't register my $A = client->new(1); + my @device_ids = ('' , 'somebull$hit', 'legit'); my @good_msgs = ('the client sent a device id that was not base64', 'the client sent a device id that was not base64', @@ -17,25 +19,25 @@ for (0..2) { $A->set_device_id($device_ids[$_]); # for messages that send 2 arguments, send an empty 2nd argument - $A->friend_add('', 'err'); - fail_msg_ne $good_msgs[$_], $A->get_error(); + my $err = $A->friend_add('', 'err'); + fail_msg_ne $good_msgs[$_], $err; - $A->friend_delete('', 'err'); - fail_msg_ne $good_msgs[$_], $A->get_error(); + $err = $A->friend_delete('', 'err'); + fail_msg_ne $good_msgs[$_], $err; - $A->list_add('', 'err'); - fail_msg_ne $good_msgs[$_], $A->get_error(); + $err = $A->list_add('', 'err'); + fail_msg_ne $good_msgs[$_], $err; - $A->list_join('', 'err'); - fail_msg_ne $good_msgs[$_], $A->get_error(); + $err = $A->list_join('', 'err'); + fail_msg_ne $good_msgs[$_], $err; - $A->list_leave('', 'err'); - fail_msg_ne $good_msgs[$_], $A->get_error(); + $err = $A->list_leave('', 'err'); + fail_msg_ne $good_msgs[$_], $err; # messages that send 1 argument - $A->lists_get('err'); - fail_msg_ne $good_msgs[$_], $A->get_error(); + $err = $A->lists_get('err'); + fail_msg_ne $good_msgs[$_], $err; - $A->lists_get_other('err'); - fail_msg_ne $good_msgs[$_], $A->get_error(); + $err = $A->lists_get_other('err'); + fail_msg_ne $good_msgs[$_], $err; } diff --git a/server/t/large_response/test.pl b/server/t/large_response/test.pl @@ -8,13 +8,11 @@ use test; # only handle that much data at a time my $A = client->new(); -for (1..200) { - $A->list_add($_); -} +$A->list_add({ name => $_, date => 0}) for (1..200); # The response to this lists_get request clocks in at ~24 KB my $count = 0; -for my $list ($A->lists_get()) { +for my $list (@{ $A->lists_get() }) { $count += 1; fail_msg_ne "$count", $list->{name}; } diff --git a/server/t/leave_list_your_not_in/test.pl b/server/t/leave_list_your_not_in/test.pl @@ -10,8 +10,8 @@ use test; my $A = client->new(); my $B = client->new(); -$A->list_add('only a can see this list'); +my $list = $A->list_add({ name => 'only a can see this list', date => 0 }); # Who knows how B got this list id, but he did -$B->list_leave($A->lists(0)->{num}, 'err'); -fail_msg_ne 'the client was not a member of the list', $B->get_error(); +my $err = $B->list_leave($list->{num}, 'err'); +fail_msg_ne 'the client was not a member of the list', $err; diff --git a/server/t/list_add/server.log.good b/server/t/list_add/server.log.good @@ -7,4 +7,11 @@ list_add: new list number is <digits> list_add: device <base64> list_add: new list name <digits> list_add: new list number is <digits> +lists_get: gathering lists for <base64> +lists_get: found list <digits>:<string> +lists_get: list has 1 members +lists_get: list has 0 items +lists_get: found list <digits>:<digits> +lists_get: list has 1 members +lists_get: list has 0 items disconnected! diff --git a/server/t/list_add/test.pl b/server/t/list_add/test.pl @@ -8,8 +8,8 @@ use Scalar::Util qw(looks_like_number); my $A = client->new(); # make sure normal list_add works -$A->list_add(my $name = 'this is a new list'); -my $list = $A->lists(0); +my $name = 'this is a new list'; +my $list = $A->list_add({ name => $name, date => 0 }); fail "list num isn't numeric" unless (looks_like_number($list->{num})); fail_msg_ne $name, $list->{name}; @@ -17,6 +17,7 @@ fail_num_ne "wrong number of members", $list->{num_members}, 1; fail_msg_ne $list->{members}->[0], $A->phnum(); # verify a new_list request with an empty list name succeeds -$A->list_add(''); +$A->list_add({ name => '', date => 0 }); -fail_num_ne "wrong number of lists", $A->num_lists(), 2; +my $num_lists = scalar( @{ $A->lists_get() } ); +fail_num_ne "wrong number of lists", $num_lists, 2; diff --git a/server/t/list_join/test.pl b/server/t/list_join/test.pl @@ -13,13 +13,11 @@ $B->friend_add($A->phnum()); # A creates a new list my $list_name = "this is a new list"; -$A->list_add($list_name); -my $list_num = $A->lists(0)->{num}; +my $As_list = $A->list_add({ name => $list_name, date => 0 }); # B joins A's list -my $response = $B->list_join($list_num); -my $list = $response->{list}; -fail_num_ne 'list num mismatch', $list->{num}, $list_num; +my $list = $B->list_join($As_list->{num}); +fail_num_ne 'list num mismatch', $list->{num}, $As_list->{num}; fail_msg_ne 'this is a new list', $list->{name}; fail_num_ne 'date mismatch', $list->{date}, 0; fail_num_ne 'items complete mismatch', $list->{items_complete}, 0; @@ -27,10 +25,9 @@ fail_num_ne 'items total mismatch', $list->{items_total}, 0; fail_num_ne 'num members mismatch', $list->{num_members}, 2; # B requests its lists to make sure its committed to the list -($list) = $B->lists_get(); +($list) = @{ $B->lists_get() }; # Verify what we get from server -my $stored_list = $A->lists(0); for ('num', 'name', 'date') { - fail_msg_ne $stored_list->{$_}, $list->{$_}; + fail_msg_ne $As_list->{$_}, $list->{$_}; } diff --git a/server/t/list_join_unit/test.pl b/server/t/list_join_unit/test.pl @@ -7,10 +7,10 @@ use test; my $A = client->new(); # Try joining a list that doesn't exist -$A->list_join('12345678', 'err'); -fail_msg_ne 'the client sent an unknown list number', $A->get_error(); +my $err = $A->list_join('12345678', 'err'); +fail_msg_ne 'the client sent an unknown list number', $err; # Test joining a list your already in -$A->list_add('my new test list'); -$A->list_join($A->lists(0)->{num}, 'err'); -fail_msg_ne 'the device is already part of this list', $A->get_error(); +my $list = $A->list_add({ name => 'my new test list', date => 0 }); +$err = $A->list_join($list->{num}, 'err'); +fail_msg_ne 'the device is already part of this list', $err; diff --git a/server/t/list_leave/test.pl b/server/t/list_leave/test.pl @@ -6,12 +6,12 @@ use test; my $A = client->new(); -$A->list_add('this list was made for leaving'); -$A->list_leave($A->lists(0)->{num}); +my $list = $A->list_add({ name => 'this list was made for leaving', date => 0 }); +$A->list_leave($list->{num}); # verify we don't get this list back when requesting all lists -my @lists = $A->lists_get(); -my @other_lists = $A->lists_get_other(); +my $num_lists = scalar( @{ $A->lists_get() } ); +my $num_other_lists = scalar(@{ $A->lists_get_other() }); -fail_num_ne 'wrong number of lists ', scalar @lists, 0; -fail_num_ne 'wrong number of other lists ', scalar @other_lists, 0; +fail_num_ne 'wrong number of lists ',$num_lists, 0; +fail_num_ne 'wrong number of other lists ', $num_other_lists, 0; diff --git a/server/t/list_leave_unit/test.pl b/server/t/list_leave_unit/test.pl @@ -7,9 +7,9 @@ use test; my $A = client->new(); # Try leaving a list your not in -$A->list_leave('1234567', 'err'); -fail_msg_ne 'the client sent an unknown list number', $A->get_error(); +my $err = $A->list_leave('1234567', 'err'); +fail_msg_ne 'the client sent an unknown list number', $err; # Try leaving the empty list -$A->list_leave('', 'err'); -fail_msg_ne 'the client sent a list number that was not a number', $A->get_error(); +$err = $A->list_leave('', 'err'); +fail_msg_ne 'the client sent a list number that was not a number', $err; diff --git a/server/t/list_reference_counting/test.pl b/server/t/list_reference_counting/test.pl @@ -1,17 +1,15 @@ #!/usr/bin/perl -I../ use strict; use warnings; - use client; use test; -# test list reference counting to make sure they stay alive when needed +# Test list reference counting to make sure they stay alive when needed my $A = client->new(); my $B = client->new(); # A creates a new list -$A->list_add('this list will belong to B soon enough'); -my $list_id = $A->lists(0)->{num}; +my $list = $A->list_add({ name => 'this list will belong to B soon enough', date => 0 }); # XXX: missing steps # - A and B become mutual friends @@ -19,12 +17,12 @@ my $list_id = $A->lists(0)->{num}; # - B joins A's list # B joins A's list, A leaves its own list -$B->list_join($list_id); -$A->list_leave($list_id); +$B->list_join($list->{num}); +$A->list_leave($list->{num}); # B verifies its still in the list -my @lists = $B->lists_get(); -fail_num_ne 'wrong number of lists ', scalar(@lists), 1; +my $num_lists = scalar(@{ $B->lists_get() }); +fail_num_ne 'wrong number of lists ', $num_lists, 1; # B also leaves the list -$B->list_leave($list_id); +$B->list_leave($list->{num}); diff --git a/server/t/list_update/test.pl b/server/t/list_update/test.pl @@ -7,35 +7,35 @@ use test; my $A = client->new(); # Test sending a request with no 'num' key -$A->list_update({ name => 'some name' }, 'err'); -fail_msg_ne 'the client did not send a list number', $A->get_error(); +my $err = $A->list_update({ name => 'some name' }, 'err'); +fail_msg_ne 'the client did not send a list number', $err; # Try and update a list that doesn't exist -$A->list_update({ num => 123456, name => 'some name' }, 'err'); -fail_msg_ne 'the client sent an unknown list number', $A->get_error(); +$err = $A->list_update({ num => 123456, name => 'some name' }, 'err'); +fail_msg_ne 'the client sent an unknown list number', $err; -$A->list_add('this is a new list'); -my $list_num = $A->lists(0)->{num}; +# All checks after this require a valid list, create one now +my $list = $A->list_add({ name => 'this is a new list', date => 0 }); # Update only the list name first -$A->list_update({ num => $list_num, name => 'this is an updated name' }); +$A->list_update({ num => $list->{num}, name => 'this is an updated name' }); # Verify the name change persisted -my @lists = $A->lists_get(); +my @lists = @{ $A->lists_get() }; fail_msg_ne 'this is an updated name', $lists[0]->{name}; fail_num_ne 'date mismatch', $lists[0]->{date}, 0; # Update only the date -$A->list_update({ num => $list_num, date => 12345 }); +$A->list_update({ num => $list->{num}, date => 12345 }); # Verify the date change persisted -@lists = $A->lists_get(); +@lists = @{ $A->lists_get() }; fail_msg_ne $lists[0]->{name}, 'this is an updated name'; fail_num_ne 'date mismatch', $lists[0]->{date}, 12345; # Now update both the name and date -$A->list_update({ num => $list_num, date => 54321, name => 'updated again' }); +$A->list_update({ num => $list->{num}, date => 54321, name => 'updated again' }); -@lists = $A->lists_get(); +@lists = @{ $A->lists_get() }; fail_msg_ne $lists[0]->{name}, 'updated again'; fail_num_ne 'date mismatch', $lists[0]->{date}, 54321; diff --git a/server/t/lists_get/test.pl b/server/t/lists_get/test.pl @@ -7,15 +7,16 @@ use test; my $A = client->new(); # Create 3 new lists +my @stored_lists; for ('new list 1', 'new list 2', 'new list 3') { - $A->list_add($_); + push @stored_lists, $A->list_add({ name => $_, date => 0 }); } my $i = 0; -# Verify the information from lists_get matches what we know if true -for my $list ($A->lists_get()) { +# Verify the information from lists_get matches what we know is true +for my $list (@{ $A->lists_get() }) { my $num = $list->{num}; - my $stored_list = $A->lists($i++); + my $stored_list = $stored_lists[$i]; fail_msg_ne $list->{num}, $stored_list->{num}; fail_num_ne 'wrong number of members', $list->{num_members}, $stored_list->{num_members}; @@ -24,5 +25,6 @@ for my $list ($A->lists_get()) { fail_num_ne 'date not the same', $list->{date}, $stored_list->{date}; fail_num_ne 'items total not the same', $list->{items_total}, 0; fail_num_ne 'items complete not the same', $list->{items_complete}, 0; + $i++; } fail_num_ne 'wrong number of lists', $i, 3; diff --git a/server/t/lists_get_other/test.pl b/server/t/lists_get_other/test.pl @@ -13,11 +13,10 @@ $A->friend_add($B->phnum()); $B->friend_add($A->phnum()); # A adds a new list -$A->list_add('this is a new list that B can see'); -my $as_list = $A->lists(0); +my $as_list = $A->list_add({ name => 'this is a new list that B can see', date => 0 }); # Check that B can see As list -my @other_lists = $B->lists_get_other(); +my @other_lists = @{ $B->lists_get_other() }; fail_msg_ne $other_lists[0]->{name}, $as_list->{'name'}; fail_msg_ne $other_lists[0]->{num}, $as_list->{'num'}; fail_num_ne 'wrong number of list members', $other_lists[0]->{num_members}, 1; diff --git a/server/t/multiple_friends_same_other_list/test.pl b/server/t/multiple_friends_same_other_list/test.pl @@ -21,13 +21,13 @@ $A->friend_add($C->phnum()); $C->friend_add($A->phnum()); # B and C need to be in the same list -$B->list_add('this is Bs new list'); -$C->list_join($B->lists(0)->{num}); +my $list = $B->list_add({ name => 'this is Bs new list', date => 0 }); +$C->list_join($list->{num}); # A makes sure he got a single list -my @other = $A->lists_get_other(); +my @other = @{ $A->lists_get_other() }; fail_num_ne 'wrong number of list members', $other[0]->{num_members}, 2; -fail_msg_ne $other[0]->{num}, $B->lists(0)->{num}; +fail_msg_ne $other[0]->{num}, $list->{num}; fail_num_ne 'wrong number of lists', scalar(@other), 1; fail "A found unexpectedly" if (grep {$_ eq $A->phnum()} @{$other[0]->{members}}); fail "member B not found" unless (grep {$_ eq $B->phnum()} @{$other[0]->{members}}); diff --git a/server/t/response_too_large/test.pl b/server/t/response_too_large/test.pl @@ -7,9 +7,7 @@ use test; # Test that a message greater than 65KB doesn't get sent my $A = client->new(); -for (1..600) { - $A->list_add($_); -} +$A->list_add({ name => $_, date => 0 }) for (1..600); -$A->lists_get('err'); -fail_msg_ne 'response too large', $A->get_error(); +my $err = $A->lists_get('err'); +fail_msg_ne 'response too large', $err; diff --git a/server/t/two_lists_same_name/test.pl b/server/t/two_lists_same_name/test.pl @@ -1,7 +1,6 @@ #!/usr/bin/perl -I../ use strict; use warnings; - use client; use test; @@ -9,8 +8,9 @@ my $A = client->new(); # check that adding the same list twice works my $name = 'some list thats going to be added twice'; -$A->list_add($name); -$A->list_add($name); +$A->list_add({ name => $name, date => 0 }); +$A->list_add({ name => $name, date => 0 }); -my @lists = $A->lists_get(); +my $num_lists = scalar(@{ $A->lists_get() }); +fail_num_ne "wrong number of lists", $num_lists, 2; # XXX: add validation this gives back 2 independent lists diff --git a/server/t/update_list_youre_not_in/test.pl b/server/t/update_list_youre_not_in/test.pl @@ -9,9 +9,9 @@ my $A = client->new(); my $B = client->new(); # A adds a new list -$A->list_add('this is a new list for a'); -my $list_num = $A->lists(0)->{num}; +my $list = $A->list_add({ name => 'this is a new list for a', date => 0 }); # B tries to update A's list without joining it first -$B->list_update({ num => $list_num, name => 'some new name', date => 1 }, 'err'); -fail_msg_ne 'client tried to update a list it was not in', $B->get_error(); +my $request = { num => $list->{num}, name => 'some new name', date => 1 }; +my $err = $B->list_update($request, 'err'); +fail_msg_ne 'client tried to update a list it was not in', $err; diff --git a/server/t/utf8/test.pl b/server/t/utf8/test.pl @@ -10,8 +10,8 @@ my $A = client->new(); # - a left double quotation mark and # - ae sorta character thing but where they touch # - face with medical mask -$A->list_add("\xE2\x80\x9C \xC3\xA6 \xF0\x9F\x98\xB8"); -my ($list) = $A->lists_get(); +$A->list_add({ name => "\xE2\x80\x9C \xC3\xA6 \xF0\x9F\x98\xB8", date => 0 }); +my ($list) = @{ $A->lists_get() }; # Check the list name we get back hasn't been mangled in the round trip fail_msg_ne "\xE2\x80\x9C \xC3\xA6 \xF0\x9F\x98\xB8", $list->{name}; diff --git a/server/t/zero_payload/test.pl b/server/t/zero_payload/test.pl @@ -12,15 +12,13 @@ my $A = client->new(1); # Send size zero payload to all message types for (@msg_str) { - $A->set_msg_type( $_ ); - my $msg_good = 'a missing message argument was required'; if ($_ eq 'device_add') { $msg_good = 'the sent phone number is not a number'; } # Send empty dictionary - $A->send_msg( {} ); - my $response = $A->recv_msg('err'); + $A->send_msg($_, {} ); + my $response = $A->recv_msg($_); fail_msg_ne $msg_good, $response->{reason}; }