diff --git a/configure-aspen.py b/configure-aspen.py index 3711e4ef19..0b6f143d8a 100644 --- a/configure-aspen.py +++ b/configure-aspen.py @@ -27,7 +27,7 @@ website.renderer_factories['jinja2'].Renderer.global_context = { 'range': range, 'unicode': unicode, - 'enumerate': enumerate, + 'enumerate': enumerate, 'len': len, 'float': float, 'type': type, @@ -55,6 +55,7 @@ def update_homepage_queries(): try: utils.update_global_stats(website) utils.update_homepage_queries_once(website.db) + website.db.self_check() except: if tell_sentry: tell_sentry(None) diff --git a/gittip/billing/payday.py b/gittip/billing/payday.py index 9278f96414..92c6e6fb29 100644 --- a/gittip/billing/payday.py +++ b/gittip/billing/payday.py @@ -127,6 +127,8 @@ def run(self): crashes. """ + self.db.self_check() + _start = aspen.utils.utcnow() log("Greetings, program! It's PAYDAY!!!!") ts_start = self.start() @@ -141,6 +143,8 @@ def run(self): self.end() + self.db.self_check() + _end = aspen.utils.utcnow() _delta = _end - _start fmt_past = "Script ran for {age} (%s)." % _delta diff --git a/gittip/models/__init__.py b/gittip/models/__init__.py index 74e614a264..c1c1d96543 100644 --- a/gittip/models/__init__.py +++ b/gittip/models/__init__.py @@ -6,3 +6,157 @@ everything on Gittip. """ +from postgres import Postgres + +class GittipDB(Postgres): + + def self_check(self): + """ + Runs all available self checks on the database. + """ + self._check_balances() + self._check_tips() + self._check_orphans() + self._check_orphans_no_tips() + + def _check_tips(self): + """ + Checks that there are no rows in tips with duplicate (tipper, tippee, mtime). + + https://github.com/gittip/www.gittip.com/issues/1704 + """ + conflicting_tips = self.one(""" + SELECT count(*) + FROM + ( + SELECT * FROM tips + EXCEPT + SELECT DISTINCT ON(tipper, tippee, mtime) * + FROM tips + ORDER BY tipper, tippee, mtime + ) AS foo + """) + assert conflicting_tips == 0 + + def _check_balances(self): + """ + Recalculates balances for all participants from transfers and exchanges. + + https://github.com/gittip/www.gittip.com/issues/1118 + """ + with self.get_cursor() as cursor: + if cursor.one("select exists (select * from paydays where ts_end < ts_start) as running"): + # payday is running and the query bellow does not account for pending + return + b = cursor.one(""" + select count(*) + from ( + select username, sum(a) as balance + from ( + select participant as username, sum(amount) as a + from exchanges + where amount > 0 + group by participant + + union + + select participant as username, sum(amount-fee) as a + from exchanges + where amount < 0 + group by participant + + union + + select tipper as username, sum(-amount) as a + from transfers + group by tipper + + union + + select tippee as username, sum(amount) as a + from transfers + group by tippee + ) as foo + group by username + + except + + select username, balance + from participants + ) as foo2 + """) + assert b == 0, "conflicting balances: {}".format(b) + + def _check_orphans(self): + """ + Finds participants that + * does not have corresponding elsewhere account + * have not been absorbed by other participant + + These are broken because new participants arise from elsewhere + and elsewhere is detached only by take over which makes a note + in absorptions if it removes the last elsewhere account. + + Especially bad case is when also claimed_time is set because + there must have been elsewhere account attached and used to sign in. + + https://github.com/gittip/www.gittip.com/issues/617 + """ + orphans = self.all(""" + select username + from participants + where not exists (select * from elsewhere where elsewhere.participant=username) + and not exists (select * from absorptions where archived_as=username) + """) + known = set(( + "4c46cc22afdd", + "82b0d81fe9e8", + "6b6527ac6c02", + "cbde8c31c11c", + "bcfc65158eaf", + "31d54a0c19ae", + "afbddadaac3c", + "a78c4e42bb93", + "42eb93b3ab89", + "b1bc5e47fe8e", + "3ac515cc8da6", + "14a79340c40d", + "14d60c6884e7", + "0c783dee50ed", + "e2020536ef6d", + "60a5099d49c7", + "64f4f959b322", + "0bdf90d51786" + )) + real = set(orphans) - known + assert len(real) == 0, "missing elsewheres: {}".format(list(real)) + + def _check_orphans_no_tips(self): + """ + Finds participants + * without elsewhere account attached + * having non zero outstanding tip + + This should not happen because when we remove the last elsewhere account + in take_over we also zero out all tips. + """ + tips_with_orphans = self.all(""" + WITH orphans AS ( + SELECT username FROM participants + WHERE NOT EXISTS (SELECT 1 FROM elsewhere WHERE participant=username) + ), valid_tips AS ( + SELECT * FROM ( + SELECT DISTINCT ON (tipper, tippee) * + FROM tips + ORDER BY tipper, tippee, mtime DESC + ) AS foo + WHERE amount > 0 + ) + SELECT id FROM valid_tips + WHERE tipper IN (SELECT * FROM orphans) + OR tippee IN (SELECT * FROM orphans) + """) + known = set([25206]) # '4c074000c7bc', 'naderman', '3.00' + real = set(tips_with_orphans) - known + assert len(real) == 0, real +# diff --git a/gittip/testing/__init__.py b/gittip/testing/__init__.py index deb29a5e92..099aa3d929 100644 --- a/gittip/testing/__init__.py +++ b/gittip/testing/__init__.py @@ -105,6 +105,7 @@ def setUpClass(cls): cls.db = cls.client.website.db cls.tablenames = cls.db.all("SELECT tablename FROM pg_tables " "WHERE schemaname='public'") + cls.seq = 0 def setUp(self): @@ -152,6 +153,13 @@ def make_participant(self, username, **kw): participant = Participant.with_random_username() participant.change_username(username) + if 'elsewhere' in kw or 'claimed_time' in kw: + platform = kw.pop('elsewhere', 'github') + user_info = dict(login=username) + self.seq += 1 + self.db.run("INSERT INTO elsewhere (platform, user_id, participant, user_info) " + "VALUES (%s,%s,%s,%s)", (platform, self.seq, username, user_info)) + # brute force update for use in testing for k,v in kw.items(): if k == 'claimed_time': diff --git a/gittip/wireup.py b/gittip/wireup.py index 45a3526e89..47a4cd570b 100644 --- a/gittip/wireup.py +++ b/gittip/wireup.py @@ -12,7 +12,7 @@ import stripe from gittip.models.community import Community from gittip.models.participant import Participant -from postgres import Postgres +from gittip.models import GittipDB def canonical(): @@ -23,7 +23,7 @@ def canonical(): def db(): dburl = os.environ['DATABASE_URL'] maxconn = int(os.environ['DATABASE_MAXCONN']) - db = Postgres(dburl, maxconn=maxconn) + db = GittipDB(dburl, maxconn=maxconn) # register hstore type with db.get_cursor() as cursor: diff --git a/tests/test_billing.py b/tests/test_billing.py index f65bc954df..3d02135a43 100644 --- a/tests/test_billing.py +++ b/tests/test_billing.py @@ -16,7 +16,7 @@ class TestBillingBase(Harness): def setUp(self): Harness.setUp(self) - self.alice = self.make_participant('alice') + self.alice = self.make_participant('alice', elsewhere='github') class TestBalancedCard(Harness): diff --git a/tests/test_charts_json.py b/tests/test_charts_json.py index 1b3793d6a6..d84f3538e3 100644 --- a/tests/test_charts_json.py +++ b/tests/test_charts_json.py @@ -15,6 +15,12 @@ def make_participants_and_tips(self): alice = self.make_participant('alice', balance=10, claimed_time='now') bob = self.make_participant('bob', balance=10, claimed_time='now') self.make_participant('carl', claimed_time='now') + self.db.run(""" + INSERT INTO EXCHANGES + (amount, fee, participant) VALUES + (10.00, 0.00, 'alice'), + (10.00, 0.00, 'bob') + """) self.make_participant('notactive', claimed_time='now') alice.set_tip_to('carl', '1.00') diff --git a/tests/test_participant.py b/tests/test_participant.py index 2d601deb07..66cba4a352 100644 --- a/tests/test_participant.py +++ b/tests/test_participant.py @@ -118,19 +118,6 @@ def test_there_is_no_more_deadbeef(self): class TestTakeOver(Harness): - def self_test(self): - a = self.db.one(""" - SELECT count(*) - FROM ( - SELECT * FROM tips - EXCEPT - SELECT DISTINCT ON(tipper, tippee, mtime) * - FROM tips - ORDER BY tipper, tippee, mtime - ) AS foo - """) - assert a == 0 - def test_cross_tip_doesnt_become_self_tip(self): alice = TwitterAccount(self.db, 1, dict(screen_name='alice')) bob = TwitterAccount(self.db, 2, dict(screen_name='bob')) @@ -138,7 +125,7 @@ def test_cross_tip_doesnt_become_self_tip(self): bob_participant = bob.opt_in('bob')[0].participant alice_participant.set_tip_to('bob', '1.00') bob_participant.take_over(alice, have_confirmation=True) - self.self_test() + self.db.self_check() def test_zero_cross_tip_doesnt_become_self_tip(self): alice = TwitterAccount(self.db, 1, dict(screen_name='alice')) @@ -148,7 +135,7 @@ def test_zero_cross_tip_doesnt_become_self_tip(self): alice_participant.set_tip_to('bob', '1.00') alice_participant.set_tip_to('bob', '0.00') bob_participant.take_over(alice, have_confirmation=True) - self.self_test() + self.db.self_check() def test_do_not_take_over_zero_tips_giving(self): alice = TwitterAccount(self.db, 1, dict(screen_name='alice')) @@ -162,7 +149,7 @@ def test_do_not_take_over_zero_tips_giving(self): alice_participant.take_over(carl, have_confirmation=True) ntips = self.db.one("select count(*) from tips") assert 2 == ntips - self.self_test() + self.db.self_check() def test_do_not_take_over_zero_tips_receiving(self): alice = TwitterAccount(self.db, 1, dict(screen_name='alice')) @@ -176,17 +163,15 @@ def test_do_not_take_over_zero_tips_receiving(self): alice_participant.take_over(carl, have_confirmation=True) ntips = self.db.one("select count(*) from tips") assert 2 == ntips - self.self_test() + self.db.self_check() class TestParticipant(Harness): def setUp(self): Harness.setUp(self) now = utcnow() - for idx, username in enumerate(['alice', 'bob', 'carl'], start=1): - self.make_participant(username, claimed_time=now) - twitter_account = TwitterAccount(self.db, idx, {'screen_name': username}) - Participant.from_username(username).take_over(twitter_account) + for username in ['alice', 'bob', 'carl']: + self.make_participant(username, claimed_time=now, elsewhere='twitter') def test_bob_is_singular(self): expected = True