src/database/writes.py

@@ -18,8 +18,7 @@ logger = logging.getLogger(__name__)
 
 def create_comment_or_error(conn, comment, claim_id=None, channel_id=None, channel_name=None,
                             signature=None, signing_ts=None, parent_id=None) -> dict:
-    if channel_id and channel_name:
-        insert_channel_or_error(conn, channel_name, channel_id)
+    insert_channel_or_error(conn, channel_name, channel_id)
     fn = db.insert_comment if parent_id is None else db.insert_reply
     comment_id = fn(
         conn=conn,
@@ -65,7 +64,7 @@ async def _abandon_comment(app, comment_id):  # DELETE
 
 
 async def create_comment(app, params):
-    if is_valid_base_comment(**params) and is_valid_credential_input(**params):
+    if is_valid_base_comment(**params):
         job = await app['comment_scheduler'].spawn(_create_comment(app, params))
         comment = await job.wait()
         if comment:

src/server/validation.py

@@ -50,24 +50,28 @@ def claim_id_is_valid(claim_id: str) -> bool:
     return re.fullmatch('([a-z0-9]{40}|[A-Z0-9]{40})', claim_id) is not None
 
 
-def is_valid_base_comment(comment: str, claim_id: str, parent_id: str = None, **kwargs) -> bool:
-    return comment is not None and body_is_valid(comment) and \
-           ((claim_id is not None and claim_id_is_valid(claim_id)) or
-            (parent_id is not None and comment_id_is_valid(parent_id)))
+# default to None so params can be treated as kwargs; param count becomes more manageable
+def is_valid_base_comment(comment: str = None, claim_id: str = None, parent_id: str = None, **kwargs) -> bool:
+    return comment and body_is_valid(comment) and \
+           ((claim_id and claim_id_is_valid(claim_id)) or  # parentid is used in place of claimid in replies
+            (parent_id and comment_id_is_valid(parent_id))) \
+           and is_valid_credential_input(**kwargs)
 
 
 def is_valid_credential_input(channel_id: str = None, channel_name: str = None,
-                              signature: str = None, signing_ts: str = None, **kwargs) -> bool:
-    if channel_id or channel_name or signature or signing_ts:
-        try:
-            assert channel_id and channel_name and signature and signing_ts
-            assert is_valid_channel(channel_id, channel_name)
-            assert len(signature) == 128
-            assert signing_ts.isalnum()
+                              signature: str = None, signing_ts: str = None) -> bool:
+    try:
+        assert None not in (channel_id, channel_name, signature, signing_ts)
+        assert is_valid_channel(channel_id, channel_name)
+        assert len(signature) == 128
+        assert signing_ts.isalnum()
 
-        except Exception:
-            return False
-    return True
+        return True
+
+    except Exception as e:
+        logger.exception(f'Failed to validate channel: lbry://{channel_name}#{channel_id}, '
+                         f'signature: {signature} signing_ts: {signing_ts}')
+        return False
 
 
 def validate_signature_from_claim(claim: dict, signature: typing.Union[str, bytes],

test/test_database.py

@@ -1,4 +1,4 @@
-import unittest
+import sqlite3
 
 from random import randint
 import faker
@@ -53,21 +53,13 @@ class TestDatabaseOperations(DatabaseTestCase):
         self.assertEqual(reply['parent_id'], comment['comment_id'])
 
     def test02AnonymousComments(self):
-        comment = create_comment_or_error(
+        self.assertRaises(
+            sqlite3.IntegrityError,
+            create_comment_or_error,
             conn=self.conn,
             claim_id=self.claimId,
             comment='This is an ANONYMOUS comment'
         )
-        self.assertIsNotNone(comment)
-        previous_id = comment['comment_id']
-        reply = create_comment_or_error(
-            conn=self.conn,
-            claim_id=self.claimId,
-            comment='This is an unnamed response',
-            parent_id=previous_id
-        )
-        self.assertIsNotNone(reply)
-        self.assertEqual(reply['parent_id'], comment['comment_id'])
 
     def test03SignedComments(self):
         comment = create_comment_or_error(
@@ -142,61 +134,7 @@ class TestDatabaseOperations(DatabaseTestCase):
             comment='this username is too short'
         )
 
-    def test05InsertRandomComments(self):
-        # TODO: Fix this test into something practical
-        self.skipTest('This is a bad test')
-        top_comments, claim_ids = generate_top_comments_random()
-        total = 0
-        success = 0
-        for _, comments in top_comments.items():
-            for i, comment in enumerate(comments):
-                with self.subTest(comment=comment):
-                    result = create_comment_or_error(self.conn, **comment)
-                    if result:
-                        success += 1
-                    comments[i] = result
-                    del comment
-                total += len(comments)
-        self.assertLessEqual(success, total)
-        self.assertGreater(success, 0)
-        success = 0
-        for reply in generate_replies_random(top_comments):
-            reply_id = create_comment_or_error(self.conn, **reply)
-            if reply_id:
-                success += 1
-        self.assertGreater(success, 0)
-        self.assertLess(success, total)
-        del top_comments
-        del claim_ids
-
-    def test06GenerateAndListComments(self):
-        # TODO: Make this test not suck
-        self.skipTest('this is a stupid test')
-        top_comments, claim_ids = generate_top_comments()
-        total, success = 0, 0
-        for _, comments in top_comments.items():
-            for i, comment in enumerate(comments):
-                result = create_comment_or_error(self.conn, **comment)
-                if result:
-                    success += 1
-                comments[i] = result
-                del comment
-            total += len(comments)
-        self.assertEqual(total, success)
-        self.assertGreater(total, 0)
-        for reply in generate_replies(top_comments):
-            create_comment_or_error(self.conn, **reply)
-        for claim_id in claim_ids:
-            comments_ids = get_comment_ids(self.conn, claim_id)
-            with self.subTest(comments_ids=comments_ids):
-                self.assertIs(type(comments_ids), list)
-                self.assertGreaterEqual(len(comments_ids), 0)
-                self.assertLessEqual(len(comments_ids), 50)
-                replies = get_comments_by_id(self.conn, comments_ids)
-                self.assertLessEqual(len(replies), 50)
-                self.assertEqual(len(replies), len(comments_ids))
-
-    def test07HideComments(self):
+    def test05HideComments(self):
         comm = create_comment_or_error(self.conn, 'Comment #1', self.claimId, '1'*40, '@Doge123', 'a'*128, '123')
         comment = get_comments_by_id(self.conn, [comm['comment_id']]).pop()
         self.assertFalse(comment['is_hidden'])
@@ -209,7 +147,7 @@ class TestDatabaseOperations(DatabaseTestCase):
         comment = get_comments_by_id(self.conn, [comm['comment_id']]).pop()
         self.assertTrue(comment['is_hidden'])
 
-    def test08DeleteComments(self):
+    def test06DeleteComments(self):
         comm = create_comment_or_error(self.conn, 'Comment #1', self.claimId, '1'*40, '@Doge123', 'a'*128, '123')
         comments = get_claim_comments(self.conn, self.claimId)
         match = list(filter(lambda x: comm['comment_id'] == x['comment_id'], comments['items']))

test/test_server.py

@@ -11,7 +11,6 @@ from faker.providers import misc
 
 from src.settings import config
 from src.server import app
-from src.server.validation import is_valid_channel
 from src.server.validation import is_valid_base_comment
 
 from test.testcase import AsyncioTestCase
@@ -97,22 +96,6 @@ class ServerTest(AsyncioTestCase):
     async def post_comment(self, **params):
         return await jsonrpc_post(self.url, 'create_comment', **params)
 
-    @staticmethod
-    def is_valid_message(comment=None, claim_id=None, parent_id=None,
-                         channel_name=None, channel_id=None, signature=None, signing_ts=None):
-        try:
-            assert is_valid_base_comment(comment, claim_id, parent_id)
-
-            if channel_name or channel_id or signature or signing_ts:
-                assert channel_id and channel_name and signature and signing_ts
-                assert is_valid_channel(channel_id, channel_name)
-                assert len(signature) == 128
-                assert signing_ts.isalnum()
-
-        except Exception:
-            return False
-        return True
-
     async def test01CreateCommentNoReply(self):
         anonymous_test = create_test_comments(
             ('claim_id', 'channel_id', 'channel_name', 'comment'),
@@ -122,13 +105,13 @@ class ServerTest(AsyncioTestCase):
             claim_id=None
         )
         for test in anonymous_test:
-            with self.subTest(test=test):
+            with self.subTest(test='null fields: ' + ', '.join(k for k, v in test.items() if not v)):
                 message = await self.post_comment(**test)
                 self.assertTrue('result' in message or 'error' in message)
                 if 'error' in message:
-                    self.assertFalse(self.is_valid_message(**test))
+                    self.assertFalse(is_valid_base_comment(**test))
                 else:
-                    self.assertTrue(self.is_valid_message(**test))
+                    self.assertTrue(is_valid_base_comment(**test))
 
     async def test02CreateNamedCommentsNoReply(self):
         named_test = create_test_comments(
@@ -144,9 +127,9 @@ class ServerTest(AsyncioTestCase):
                 message = await self.post_comment(**test)
                 self.assertTrue('result' in message or 'error' in message)
                 if 'error' in message:
-                    self.assertFalse(self.is_valid_message(**test))
+                    self.assertFalse(is_valid_base_comment(**test))
                 else:
-                    self.assertTrue(self.is_valid_message(**test))
+                    self.assertTrue(is_valid_base_comment(**test))
 
     async def test03CreateAllTestComments(self):
         test_all = create_test_comments(replace.keys(), **{
@@ -157,9 +140,9 @@ class ServerTest(AsyncioTestCase):
                 message = await self.post_comment(**test)
                 self.assertTrue('result' in message or 'error' in message)
                 if 'error' in message:
-                    self.assertFalse(self.is_valid_message(**test))
+                    self.assertFalse(is_valid_base_comment(**test))
                 else:
-                    self.assertTrue(self.is_valid_message(**test))
+                    self.assertTrue(is_valid_base_comment(**test))
 
     async def test04CreateAllReplies(self):
         claim_id = '1d8a5cc39ca02e55782d619e67131c0a20843be8'
@@ -189,9 +172,37 @@ class ServerTest(AsyncioTestCase):
                     message = await self.post_comment(**test)
                     self.assertTrue('result' in message or 'error' in message)
                     if 'error' in message:
-                        self.assertFalse(self.is_valid_message(**test))
+                        self.assertFalse(is_valid_base_comment(**test))
                     else:
-                        self.assertTrue(self.is_valid_message(**test))
+                        self.assertTrue(is_valid_base_comment(**test))
+
+    async def testSlackWebhook(self):
+        claim_id = '1d8a5cc39ca02e55782d619e67131c0a20843be8'
+        channel_name = '@name'
+        channel_id = fake.sha1()
+        signature = '{}'*64
+        signing_ts = '1234'
+
+        base = await self.post_comment(
+            channel_name=channel_name,
+            channel_id=channel_id,
+            comment='duplicate',
+            claim_id=claim_id,
+            signing_ts=signing_ts,
+            signature=signature
+        )
+
+        comment_id = base['result']['comment_id']
+
+        with self.subTest(test=comment_id):
+            await self.post_comment(
+                channel_name=channel_name,
+                channel_id=channel_id,
+                comment='duplicate',
+                claim_id=claim_id,
+                signing_ts=signing_ts,
+                signature=signature
+            )
 
 
 class ListCommentsTest(AsyncioTestCase):