shlist

share and manage lists between multiple people
Log | Files | Refs

commit da76b2c388cb21781c6b75ad4074be5e4ec9f5c5
parent d4f402c4061888b189d11467f6bb35a86dd8913a
Author: Kyle Milz <kyle@0x30.net>
Date:   Sun, 24 Jan 2016 18:16:44 -0700

sl: split list_add into list_add and list_update

- it didn't make sense to have list_add handle both adding and updating
  functionality anymore
- so split this up into two distinct message types
  - list_update can update both/either name and date
  - list_add no longer requires a 'num' key mapped to the value 0 either

Diffstat:
Mandroid/shlist/app/src/main/java/drsocto/shlist/MsgTypes.java | 15++++++++-------
Mgen_msgs.sh | 1+
Mios/shlist/MsgTypes.h | 33++++++++-------------------------
Mserver/msgs.pl | 17++++++++++-------
Mserver/sl | 95+++++++++++++++++++++++++++++++++++++++++++++++++------------------------------
Mserver/tests/client.pm | 9++-------
Mserver/tests/list_update/server.log.good | 19+++++++++++++++++--
Mserver/tests/list_update/test.pl | 29++++++++++++++++++++++++++---
Mserver/tests/msgs.pl | 17++++++++++-------
Mserver/tests/update_list_youre_not_in/server.log.good | 2+-
Mserver/tests/update_list_youre_not_in/test.pl | 3++-
Mserver/tests/zero_payload/server.log.good | 1+
12 files changed, 145 insertions(+), 96 deletions(-)

diff --git a/android/shlist/app/src/main/java/drsocto/shlist/MsgTypes.java b/android/shlist/app/src/main/java/drsocto/shlist/MsgTypes.java @@ -1,4 +1,4 @@ -/* generated Wed, Jan 20, 2016 10:43:02 PM */ +/* generated Sun Jan 24 15:51:04 MST 2016 */ package drsocto.shlist; @@ -10,10 +10,11 @@ public final class MsgTypes { public final static int friend_add = 1; public final static int friend_delete = 2; public final static int list_add = 3; - public final static int list_join = 4; - public final static int list_leave = 5; - public final static int lists_get = 6; - public final static int lists_get_other = 7; - public final static int list_items_get = 8; - public final static int list_item_add = 9; + public final static int list_update = 4; + public final static int list_join = 5; + public final static int list_leave = 6; + public final static int lists_get = 7; + public final static int lists_get_other = 8; + public final static int list_items_get = 9; + public final static int list_item_add = 10; } diff --git a/gen_msgs.sh b/gen_msgs.sh @@ -6,6 +6,7 @@ msg_types=" friend_add friend_delete list_add + list_update list_join list_leave lists_get diff --git a/ios/shlist/MsgTypes.h b/ios/shlist/MsgTypes.h @@ -1,11 +1,4 @@ -/* generated Tue 12 Jan 2016 20:40:15 MST */ -#import "Network.h" - -/* -@interface MsgTypes : NSObject { - NSPointerArray *array; -} -*/ +/* generated Sun Jan 24 15:51:04 MST 2016 */" int protocol_version = 0; enum msg_types { @@ -13,21 +6,11 @@ enum msg_types { friend_add = 1, friend_delete = 2, list_add = 3, - list_join = 4, - list_leave = 5, - lists_get = 6, - lists_get_other = 7, - list_items_get = 8, - list_item_add = 9, + list_update = 4, + list_join = 5, + list_leave = 6, + lists_get = 7, + lists_get_other = 8, + list_items_get = 9, + list_item_add = 10, }; - -/* -@end -@implementation MsgTypes -+ (void)initialize { - //array = [NSPointerArray pointerArrayWithOptions:NSPointerFunctionsOpaqueMemory]; -} -*/ - -//NSPointerArray *array = [NSPointerArray pointerArrayWithOptions:NSPointerFunctionsOpaqueMemory]; -//NSArray *msg_func = @[[NSValue valueWithPointer:handlerA]]; diff --git a/server/msgs.pl b/server/msgs.pl @@ -1,5 +1,5 @@ #!/usr/bin/perl -# generated Wed, Jan 20, 2016 10:43:02 PM +# generated Sun Jan 24 15:51:04 MST 2016 use strict; use warnings; @@ -9,18 +9,20 @@ our %msg_num = ( friend_add => 1, friend_delete => 2, list_add => 3, - list_join => 4, - list_leave => 5, - lists_get => 6, - lists_get_other => 7, - list_items_get => 8, - list_item_add => 9, + list_update => 4, + list_join => 5, + list_leave => 6, + lists_get => 7, + lists_get_other => 8, + list_items_get => 9, + list_item_add => 10, ); our @msg_str = ( 'device_add', 'friend_add', 'friend_delete', 'list_add', + 'list_update', 'list_join', 'list_leave', 'lists_get', @@ -33,6 +35,7 @@ our @msg_func = ( \&msg_friend_add, \&msg_friend_delete, \&msg_list_add, + \&msg_list_update, \&msg_list_join, \&msg_list_leave, \&msg_lists_get, diff --git a/server/sl b/server/sl @@ -231,45 +231,24 @@ sub msg_list_add { my ($err, $dev, $list) = unpack_request($db, $request, 'device_id', 'list'); return make_error($err) if ($err); - if ($list->{num} == 0) { - # Sending a list number of 0 triggers new list mode - $log->print("device '$dev->{fp}'\n"); - $log->print("new list name '$list->{name}'\n"); + # XXX: check that $list contains the necessary keys! - my $now = time; - $db->{new_list}->execute($list->{name}, $list->{date}, $now, $now); + $log->print("device '$dev->{fp}'\n"); + $log->print("new list name '$list->{name}'\n"); - $list->{num} = $db->{dbh}->last_insert_id("", "", "", ""); - $log->print("new list number is '$list->{num}'\n"); + my $now = time; + $db->{new_list}->execute($list->{name}, $list->{date}, $now, $now); + my $list_num = $db->{dbh}->last_insert_id("", "", "", ""); - $db->{new_list_member}->execute($list->{num}, $dev->{num}, $now); + $log->print("new list number is '$list_num'\n"); - # XXX: send push notifications to all my mutual friends to - # update their 'other lists' section - } - else { - # Otherwise we're in list update mode - $err = list_number_valid($db, $list->{num}); - return make_error($err) if ($err); - - # Check that the device is in the list it wants to update - $db->{check_list_member}->execute($list->{num}, $dev->{num}); - unless ($db->{check_list_member}->fetchrow_array()) { - $log->print("device '$dev->{fp}' not in list '$list->{num}'\n"); - return make_error("client tried to update a list it was not in"); - } + $db->{new_list_member}->execute($list_num, $dev->{num}, $now); - # Do the update - $db->{update_list}->execute($list->{name}, $list->{date}, time, $list->{num}); - $log->print("updated list '$list->{num}'\n"); - - # XXX: send push notifications to all my mutual friends to update their - # 'other lists' section, also tell all list members to update their - # 'lists' section - } + # XXX: send push notifications to all my mutual friends to + # update their 'other lists' section my $resp_list = { - num => $list->{num}, + num => $list_num, name => $list->{name}, date => $list->{date}, items_complete => 0, @@ -280,6 +259,43 @@ sub msg_list_add { return make_ok( { list => $resp_list } ); } +sub msg_list_update { + my ($db, $request) = @_; + + my ($err, $dev, $list) = unpack_request($db, $request, 'device_id', 'list'); + return make_error($err) if ($err); + + $err = list_number_valid($db, $list->{num}); + return make_error($err) if ($err); + + # Check that the device is in the list it wants to update + $db->{check_list_member}->execute($list->{num}, $dev->{num}); + unless ($db->{check_list_member}->fetchrow_array()) { + $log->print("device '$dev->{fp}' not in list '$list->{num}'\n"); + return make_error("client tried to update a list it was not in"); + } + + # Do the update + $db->{update_list}->execute($list->{name}, $list->{date}, time, $list->{num}); + $log->print("num = $list->{num}\n"); + $log->print("name = '$list->{name}'\n") if (exists $list->{name}); + $log->print("date = $list->{date}\n") if (exists $list->{date}); + + $db->{list_select}->execute($list->{num}); + my ($num, $name, $date) = $db->{list_select}->fetchrow_array(); + + # XXX: send push notifications to all my mutual friends to update their + # 'other lists' section, also tell all list members to update their + # 'lists' section + + my $resp_list = { + num => $list->{num}, + name => $list->{name}, + date => $list->{date}, + }; + return make_ok( { list => $resp_list } ); +} + sub msg_list_item_add { my ($db, $request) = @_; @@ -467,7 +483,8 @@ sub msg_lists_get { my @lists; $db->{get_lists}->execute($dev->{num}); - while (my ($list_num, $list_name) = $db->{get_lists}->fetchrow_array()) { + while (my @row = $db->{get_lists}->fetchrow_array()) { + my ($list_num, $list_name, $list_date) = @row; $log->print("found list '$list_num':'$list_name'\n"); @@ -480,7 +497,7 @@ sub msg_lists_get { my $list = { num => $list_num, name => $list_name, - date => 0, + date => $list_date, items_complete => 0, items_total => 0, members => \@members, @@ -586,6 +603,11 @@ sub fingerprint { sub list_number_valid { my ($db, $list_num) = @_; + unless (defined $list_num) { + $log->print("list number key not found\n"); + return "the client did not send a list number"; + } + unless (looks_like_number($list_num)) { $log->print("'$list_num' is not a number\n"); return "the client sent a list number that was not a number"; @@ -769,7 +791,8 @@ sub prepare_stmt_handles { $sql = 'insert into lists (name, date, created, last_updated) values (?, ?, ?, ?)'; $self->{new_list} = $dbh->prepare($sql); - $sql = 'update lists set name = ?, date = ?, last_updated = ? where num = ?'; + $sql = qq{update lists set name = coalesce(?, name), + date = coalesce(?, date), last_updated = ? where num = ?}; $self->{update_list} = $dbh->prepare($sql); $sql = 'delete from lists where num = ?'; @@ -808,7 +831,7 @@ sub prepare_stmt_handles { $self->{mutual_friends_delete} = $dbh->prepare($sql); # lists/list_members compound queries - $sql = qq{select lists.num, lists.name from lists, list_members where + $sql = qq{select lists.num, lists.name, lists.date from lists, list_members where lists.num = list_members.list and list_members.device = ?}; $self->{get_lists} = $dbh->prepare($sql); diff --git a/server/tests/client.pm b/server/tests/client.pm @@ -41,7 +41,6 @@ sub new { sub list_add { my $self = shift; my $list = { - num => 0, name => shift, date => 0 }; @@ -55,14 +54,10 @@ sub list_add { sub list_update { my $self = shift; - my $list = { - num => shift, - name => shift, - date => shift - }; + my $list_ref = shift; my $status = shift || 'ok'; - my $list_data = communicate($self, 'list_add', $status, { list => $list }); + my $list_data = communicate($self, 'list_update', $status, { list => $list_ref }); return if ($status eq 'err'); } diff --git a/server/tests/list_update/server.log.good b/server/tests/list_update/server.log.good @@ -1,11 +1,26 @@ new connection (pid = <digits>) ssl ok, ver = 'TLSv1_2' cipher = 'ECDHE-RSA-AES128-SHA256' device_add: success, <digits>:<base64> os <base64> -list_add: unknown list number <digits> +list_update: list number key not found +list_update: unknown list number <digits> list_add: device <base64> list_add: new list name <string> list_add: new list number is <digits> -list_add: updated list <digits> +list_update: num = 211 +list_update: name = <string> +lists_get: gathering lists for <base64> +lists_get: found list <digits>:<string> +lists_get: list has 1 members +lists_get: list has 0 items +list_update: num = 211 +list_update: date = 12345 +lists_get: gathering lists for <base64> +lists_get: found list <digits>:<string> +lists_get: list has 1 members +lists_get: list has 0 items +list_update: num = 211 +list_update: name = <string> +list_update: date = 54321 lists_get: gathering lists for <base64> lists_get: found list <digits>:<string> lists_get: list has 1 members diff --git a/server/tests/list_update/test.pl b/server/tests/list_update/test.pl @@ -6,13 +6,36 @@ 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(); + # Try and update a list that doesn't exist -$A->list_update(123456, 'some name', 0, 'err'); +$A->list_update({ num => 123456, name => 'some name' }, 'err'); fail_msg_ne 'the client sent an unknown list number', $A->get_error(); -# Make sure a normal list_update works $A->list_add('this is a new list'); -$A->list_update($A->lists(0)->{num}, 'this is an updated name', 54345); +my $list_num = $A->lists(0)->{num}; + +# Update only the list name first +$A->list_update({ num => $list_num, name => 'this is an updated name' }); +# Verify the name change persisted 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 }); + +# Verify the date change persisted +@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' }); + +@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/tests/msgs.pl b/server/tests/msgs.pl @@ -1,5 +1,5 @@ #!/usr/bin/perl -# generated Wed, Jan 20, 2016 10:43:02 PM +# generated Sun Jan 24 15:51:04 MST 2016 use strict; use warnings; @@ -9,18 +9,20 @@ our %msg_num = ( friend_add => 1, friend_delete => 2, list_add => 3, - list_join => 4, - list_leave => 5, - lists_get => 6, - lists_get_other => 7, - list_items_get => 8, - list_item_add => 9, + list_update => 4, + list_join => 5, + list_leave => 6, + lists_get => 7, + lists_get_other => 8, + list_items_get => 9, + list_item_add => 10, ); our @msg_str = ( 'device_add', 'friend_add', 'friend_delete', 'list_add', + 'list_update', 'list_join', 'list_leave', 'lists_get', @@ -33,6 +35,7 @@ our @msg_func = ( \&msg_friend_add, \&msg_friend_delete, \&msg_list_add, + \&msg_list_update, \&msg_list_join, \&msg_list_leave, \&msg_lists_get, diff --git a/server/tests/update_list_youre_not_in/server.log.good b/server/tests/update_list_youre_not_in/server.log.good @@ -1,7 +1,7 @@ new connection (pid = <digits>) ssl ok, ver = 'TLSv1_2' cipher = 'ECDHE-RSA-AES128-SHA256' device_add: success, <digits>:<base64> os <base64> -list_add: device <base64> not in list <digits> +list_update: device <base64> not in list <digits> disconnected! new connection (pid = <digits>) ssl ok, ver = 'TLSv1_2' cipher = 'ECDHE-RSA-AES128-SHA256' diff --git a/server/tests/update_list_youre_not_in/test.pl b/server/tests/update_list_youre_not_in/test.pl @@ -10,7 +10,8 @@ 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}; # B tries to update A's list without joining it first -$B->list_update($A->lists(0)->{num}, 'some new title', 123, 'err'); +$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(); diff --git a/server/tests/zero_payload/server.log.good b/server/tests/zero_payload/server.log.good @@ -4,6 +4,7 @@ device_add: bad request, missing key 'phone_number' friend_add: bad request, missing key 'device_id' friend_delete: bad request, missing key 'device_id' list_add: bad request, missing key 'device_id' +list_update: bad request, missing key 'device_id' list_join: bad request, missing key 'device_id' list_leave: bad request, missing key 'device_id' lists_get: bad request, missing key 'device_id'