Merge pull request #140 from moggers87/138-assert-misuse

Assertions should be errors

salmon/bounce.py

@@ -281,11 +281,14 @@ class bounce_to:
     If you don't go back to start immediately then you will mess with the state
     for this address, which can be bad.
     """
-    def __init__(self, soft=None, hard=None):
+    def __init__(self, soft, hard):
+        if not callable(soft):
+            raise TypeError("'soft' must be a callable")
         self.soft = soft
-        self.hard = hard
 
-        assert self.soft and self.hard, "You must give at least soft and/or hard"
+        if not callable(hard):
+            raise TypeError("'hard' must be a callable")
+        self.hard = hard
 
     def __call__(self, func):
         @wraps(func)

salmon/confirm.py

@@ -129,7 +129,7 @@ def cancel(self, target, from_address, expect_secret):
 
         secret, pending_id = self.storage.get(target, addr)
 
-        if secret == expect_secret:
+        if expect_secret and secret == expect_secret:
             self.storage.delete(target, addr)
             self.delete_pending(pending_id)
 
@@ -165,16 +165,13 @@ def verify(self, target, from_address, expect_secret):
         The message is *not* deleted from the pending queue.  You can do
         that yourself with delete_pending.
         """
-        assert expect_secret, "Must give an expected ID number."
         name, addr = parseaddr(from_address)
 
         secret, pending_id = self.storage.get(target, addr)
 
-        if secret == expect_secret:
+        if expect_secret and secret == expect_secret:
             self.storage.delete(target, addr)
             return self.get_pending(pending_id)
-        else:
-            return None
 
     def send(self, relay, target, message, template, vars):
         """

salmon/encoding.py

@@ -249,8 +249,8 @@ def attach_file(self, filename, data, ctype, disposition):
         A file attachment is a raw attachment with a disposition that
         indicates the file name.
         """
-        assert filename, "You can't attach a file without a filename."
-        assert ctype.lower() == ctype, "Hey, don't be an ass.  Use a lowercase content type."
+        if not ctype.islower():
+            raise EncodingError("ctype must be lowercase, was '{}'".format(ctype))
 
         part = MailBase(parent=self)
         part.body = data
@@ -264,7 +264,8 @@ def attach_text(self, data, ctype):
         This attaches a simpler text encoded part, which doesn't have a
         filename.
         """
-        assert ctype.lower() == ctype, "Hey, don't be an ass.  Use a lowercase content type."
+        if not ctype.islower():
+            raise EncodingError("ctype must be lowercase, was '{}'".format(ctype))
 
         part = MailBase(parent=self)
         part.body = data
@@ -317,8 +318,6 @@ def extract_payload(self, mail):
         ctype, ctype_params = mail.content_encoding['Content-Type']
         cdisp, cdisp_params = mail.content_encoding['Content-Disposition']
 
-        assert ctype, "Extract payload requires that mail.content_encoding have a valid Content-Type."
-
         if ctype.startswith("text/"):
             self.add_text(mail.body, charset=ctype_params.get('charset'))
         else:
@@ -365,10 +364,8 @@ def to_message(mail):
             ctype = 'multipart/mixed'
         else:
             ctype = 'text/plain'
-    else:
-        if mail.parts:
-            assert ctype.startswith("multipart") or ctype.startswith("message"), \
-                    "Content type should be multipart or message, not %r" % ctype
+    if mail.parts and not (ctype.startswith("multipart") or ctype.startswith("message")):
+        raise EncodingError("Content type should be multipart or message, not %r" % ctype)
 
     # adjust the content type according to what it should be now
     mail.content_encoding['Content-Type'] = (ctype, params)
@@ -413,7 +410,6 @@ def to_string(mail, envelope_header=False):
     """Returns a canonicalized email string you can use to send or store
     somewhere."""
     msg = to_message(mail).as_string(envelope_header)
-    assert "From nobody" not in msg
     return msg
 
 
@@ -612,7 +608,8 @@ def _parse_charset_header(data):
                 bm, eh, ed, continued = next(scanner)
 
                 if not eh:
-                    assert not ed, "Parsing error: %r" % data
+                    if ed:
+                        raise ValueError("Parsing error: %r" % data)
                     oddness = (" " + bm.lstrip(), eh, ed, continued)
                 elif eh[0] == enc_header[0] and eh[1] == enc_header[1]:
                     enc_data += ed

salmon/mail.py

@@ -206,15 +206,19 @@ def attach(self, filename=None, content_type=None, data=None, disposition=None):
         give data and filename then it will assume you've filled data with what
         the file's contents are and filename is just the name to use.
         """
-        assert filename or data, "You must give a filename or some data to attach."
-        assert data or os.path.exists(filename), "File doesn't exist, and no data given."
+        if data is None:
+            if filename is None:
+                raise TypeError("You must give a filename or some data to attach.")
+            elif not os.path.exists(filename):
+                raise TypeError("File doesn't exist, and no data given.")
 
         self.multipart = True
 
         if filename and not content_type:
             content_type, encoding = mimetypes.guess_type(filename)
 
-        assert content_type, "No content type given, and couldn't guess from the filename: %r" % filename
+        if not content_type:
+            raise ValueError("No content type given, and couldn't guess from the filename: %r" % filename)
 
         self.attachments.append({
             'filename': filename,
@@ -313,11 +317,9 @@ def to_message(self):
 
             for args in self.attachments:
                 self._encode_attachment(**args)
-
         elif self.Body:
             self.base.body = self.Body
             self.base.content_encoding['Content-Type'] = ('text/plain', {})
-
         elif self.Html:
             self.base.body = self.Html
             self.base.content_encoding['Content-Type'] = ('text/html', {})

salmon/routing.py

@@ -548,7 +548,8 @@ class route_like(route):
     modules.
     """
     def __init__(self, func):
-        assert_salmon_settings(func)
+        if not has_salmon_settings(func):
+            raise TypeError("{} is missing a @route".format(func))
         self.format = func._salmon_settings['format']
         self.captures = func._salmon_settings['captures']
 
@@ -565,8 +566,8 @@ def stateless(func):
 
     Stateless handlers are NOT guaranteed to run before the handler with state.
     """
-    if has_salmon_settings(func):
-        assert not salmon_setting(func, 'format'), "You must use @stateless AFTER @route or @route_like."
+    if has_salmon_settings(func) and salmon_setting(func, 'format'):
+        raise TypeError("You must use @stateless after @route or @route_like")
 
     attach_salmon_settings(func)
     func._salmon_settings['stateless'] = True

salmon/server.py

@@ -84,8 +84,10 @@ def __init__(self, host='127.0.0.1', port=25, username=None, password=None,
         self.starttls = starttls
         self.lmtp = lmtp
 
-        assert not (ssl and lmtp), "LMTP over SSL not supported. Use STARTTLS instead."
-        assert not (ssl and starttls), "SSL and STARTTLS make no sense together"
+        if ssl and lmtp:
+            raise TypeError("LMTP over SSL not supported. Use STARTTLS instead.")
+        if ssl and starttls:
+            raise TypeError("SSL and STARTTLS make no sense together")
 
     def configure_relay(self, hostname):
         if self.ssl:
@@ -102,7 +104,6 @@ def configure_relay(self, hostname):
         if self.username and self.password:
             relay_host.login(self.username, self.password)
 
-        assert relay_host, 'Code error, file a bug.'
         return relay_host
 
     def deliver(self, message, To=None, From=None):

salmon/view.py

@@ -17,7 +17,8 @@ def load(template):
     It assumes that your loader works like Jinja2 or Mako in that
     it has a LOADER.get_template() method that returns the template.
     """
-    assert LOADER, "You haven't set salmon.view.LOADER to a loader yet."
+    if LOADER is None:
+        raise TypeError("You haven't set salmon.view.LOADER to a loader yet.")
     return LOADER.get_template(template)
 
 
@@ -61,7 +62,8 @@ class and this (apart from variables passed to a template) are that
     in variables to modify those when needed (as in the %(person)s in Subject).
     """
 
-    assert Body or Html, "You need to give either the Body or Html template of the mail."
+    if Body is None and Html is None:
+        raise TypeError("You need to give either the Body or Html template of the mail.")
 
     for key in kwd:
         kwd[key] = kwd[key] % variables

tests/bounce_tests.py

@@ -1,4 +1,5 @@
 from salmon import mail
+from salmon.bounce import bounce_to
 from salmon.routing import Router
 
 from .handlers import bounce_filtered_mod
@@ -103,3 +104,12 @@ def test_bounce_no_headers_error_message(self):
         msg = mail.MailRequest(None, None, None, "Nothing")
         msg.is_bounce()
         self.assertEqual(msg.bounce.error_for_humans(), 'No status codes found in bounce message.')
+
+    def test_callables(self):
+        bounce_to(lambda x: x, lambda x: x)
+
+        with self.assertRaises(TypeError):
+            bounce_to(lambda x: x, None)
+
+        with self.assertRaises(TypeError):
+            bounce_to(None, lambda x: x)

tests/confirm_tests.py

@@ -86,5 +86,15 @@ def test_ConfirmationEngine_cancel(self):
 
         self.engine.cancel(target, confirm['To'], expect_secret)
 
-        found = self.engine.verify(target, confirm['To'], expect_secret)
-        assert not found
+        self.assertNotIn(b"testing:somedude@localhost", self.engine.storage.confirmations.keys())
+
+    def test_ConfirmationEngine_cancel_bad_secret(self):
+        confirm = self.test_ConfirmationEngine_send()
+        confirm = mail.MailRequest(None, None, None, confirm)
+
+        target = confirm['Reply-To'].split('-')[0]
+        expect_secret = "bad"
+
+        self.engine.cancel(target, confirm['To'], expect_secret)
+
+        self.assertIn(b"testing:somedude@localhost", self.engine.storage.confirmations.keys())

tests/encoding_tests.py

@@ -178,6 +178,14 @@ def test_to_message_from_message_with_spam(self):
                 assert part.body == m2.parts[i].body, \
                     "Part %d isn't the same: %r \nvs\n. %r" % (i, part.body, m2.parts[i].body)
 
+    def test_to_message_raises_encoding_error(self):
+        base = encoding.MailBase()
+        base.attach_text("hello", ctype="text/plain")
+        base["Content-Type"] = "text/plain"  # this is just broken
+
+        with self.assertRaises(encoding.EncodingError):
+            encoding.to_message(base)
+
     def test_to_file_from_file(self):
         mb = mailbox.mbox("tests/data/spam")
         msg = encoding.from_message(mb[0])
@@ -271,6 +279,11 @@ def test_attach_text(self):
         self.assertEqual(len(msg.get_payload()), 2)
         assert encoding.to_string(mail)
 
+    def test_attach_text_bad_content_type(self):
+        mail = encoding.MailBase()
+        with self.assertRaises(encoding.EncodingError):
+            mail.attach_text("This is some text.", 'text/pLAIn')
+
     def test_attach_file(self):
         mail = encoding.MailBase()
         with open("tests/data/salmon.png", "rb") as file_obj:
@@ -282,6 +295,11 @@ def test_attach_file(self):
         self.assertEqual(payload.get_payload(decode=True), png)
         self.assertEqual(payload.get_filename(), "salmon.png")
 
+    def test_attach_file_bad_content_type(self):
+        mail = encoding.MailBase()
+        with self.assertRaises(encoding.EncodingError):
+            mail.attach_file("salmon.png", "1234", "IMAGe/png", "attachment")
+
     def test_content_encoding_headers_are_maintained(self):
         with open("tests/data/signed.msg", "rb") as file_obj:
             inmail = encoding.from_file(file_obj)

tests/handlers/simple_fsm_mod.py

@@ -58,7 +58,17 @@ def PASSING(message, *args, **kw):
     @route("badstateless@(host)")
     def BAD_STATELESS(message, *args, **kw):
         print("BAD_STATELESS", args, kw)
-except AssertionError:
+except TypeError:
     pass
 else:
-    raise Exception("No assertion error raised on misordered decorators")
+    raise AssertionError("No TypeError raised on misordered decorators")
+
+try:
+    # `@route_like should reference another function that has had `route_like` or `route`
+    @route_like(simple_key_gen)
+    def BAD_ROUTLIKE(message, *args, **kwargs):
+        pass
+except TypeError:
+    pass
+else:
+    raise AssertionError("No TypeError raised when route_like was used on a non-routed function")

tests/message_tests.py

@@ -55,6 +55,26 @@ def test_mail_request(self):
 
         assert str(msg)
 
+    def test_mail_attach_no_data_or_file(self):
+        msg = mail.MailResponse(
+            To="receiver@localhost",
+            Subject="Test message",
+            From="sender@localhost",
+        )
+
+        with self.assertRaises(TypeError):
+            msg.attach()
+
+    def test_mail_attach_file_no_exist(self):
+        msg = mail.MailResponse(
+            To="receiver@localhost",
+            Subject="Test message",
+            From="sender@localhost",
+        )
+
+        with self.assertRaises(TypeError):
+            msg.attach(filename="/filethatshouldnotexist")
+
     def test_mail_response_plain_text(self):
         sample = mail.MailResponse(
             To="receiver@localhost",
@@ -73,7 +93,6 @@ def test_mail_response_html(self):
             From="sender@localhost",
             Html="<html><body><p>From test_mail_response_html</p></body></html>",
         )
-
         assert str(sample)
         return sample
 
@@ -85,7 +104,15 @@ def test_mail_response_html_and_plain_text(self):
             Html="<html><body><p>Hi there.</p></body></html>",
             Body="Test from test_mail_response_html_and_plain_text.",
         )
+        assert str(sample)
+        return sample
 
+    def test_mail_response_empty(self):
+        sample = mail.MailResponse(
+            To="receiver@localhost",
+            Subject="Test message",
+            From="sender@localhost",
+        )
         assert str(sample)
         return sample
 
@@ -99,7 +126,7 @@ def test_mail_response_attachments(self):
         with open("./README.rst") as file_obj:
             readme_data = file_obj.read()
 
-        with self.assertRaises(AssertionError):
+        with self.assertRaises(ValueError):
             sample.attach(data=readme_data, disposition="inline")
 
         sample.attach(filename="./README.rst", content_type="text/plain", disposition="inline")

tests/server_tests.py

@@ -151,14 +151,14 @@ def test_QueueReceiver_process_message(self, queue_mock):
             assert response is None, response
 
     def test_Relay_asserts_ssl_options(self):
-        """Relay raises an AssertionError if the ssl option is used in combination with starttls or lmtp"""
-        with self.assertRaises(AssertionError):
+        """Relay raises an TypeError if the ssl option is used in combination with starttls or lmtp"""
+        with self.assertRaises(TypeError):
             server.Relay("localhost", ssl=True, starttls=True)
 
-        with self.assertRaises(AssertionError):
+        with self.assertRaises(TypeError):
             server.Relay("localhost", ssl=True, lmtp=True)
 
-        with self.assertRaises(AssertionError):
+        with self.assertRaises(TypeError):
             server.Relay("localhost", ssl=True, starttls=True, lmtp=True)
 
         # no error

tests/view_tests.py

@@ -16,6 +16,11 @@ def test_load(self):
         assert template
         assert template.render()
 
+    def test_loader_not_set(self):
+        view.LOADER = None
+        with self.assertRaises(TypeError):
+            view.load("template.txt")
+
     def test_render(self):
         # try with some empty vars
         text = view.render({}, "template.txt")
@@ -94,3 +99,7 @@ def test_unicode(self):
         view.attach(mail, locals(), "unicode.html", filename="attached.html")
 
         assert str(mail)
+
+    def test_no_templates(self):
+        with self.assertRaises(TypeError):
+            view.respond(locals(), From="me@localhost", To="you@localhost", Subject="Hello")