Skip to content

Commit 701df3b

Browse files
committed
Refactor and add unit tests
1 parent 97de3e9 commit 701df3b

File tree

4 files changed

+174
-127
lines changed

4 files changed

+174
-127
lines changed

mailman_hyperkitty/__init__.py

+36-20
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,9 @@
3232
from urlparse import urljoin # PY2 # pylint: disable=no-name-in-module
3333

3434
from zope.interface import implementer
35-
from mailman.interfaces.archiver import IArchiver # pylint: disable=import-error
36-
from mailman.config import config # pylint: disable=import-error
37-
from mailman.config.config import external_configuration # pylint: disable=import-error
35+
from mailman.interfaces.archiver import IArchiver
36+
from mailman.config import config
37+
from mailman.config.config import external_configuration
3838
import requests
3939

4040
import logging
@@ -79,17 +79,29 @@ def _load_conf(self):
7979
self._base_url += "/"
8080
self._api_key = archiver_config.get("general", "api_key")
8181

82+
def _get_url(self, params):
83+
params.update({"key": self.api_key})
84+
url = urljoin(self.base_url, "api/mailman/urls")
85+
result = requests.get(url, params=params)
86+
if result.status_code != 200:
87+
logger.error("HyperKitty failure on %s: %s (%s)",
88+
url, result.text, result.status_code)
89+
return ""
90+
try:
91+
result = result.json()
92+
except ValueError as e:
93+
logger.exception(
94+
"Invalid response from HyperKitty on %s: %s", url, e)
95+
return ""
96+
return urljoin(self.base_url, result["url"])
97+
8298
def list_url(self, mlist):
8399
"""Return the url to the top of the list's archive.
84100
85101
:param mlist: The IMailingList object.
86102
:returns: The url string.
87103
"""
88-
result = requests.get(urljoin(self.base_url, "api/mailman/urls"),
89-
params={"mlist": mlist.fqdn_listname, "key": self.api_key})
90-
# TODO: handle failures (check result.status_code)
91-
url = result.json()["url"]
92-
return urljoin(self.base_url, url)
104+
return self._get_url({"mlist": mlist.fqdn_listname})
93105

94106
def permalink(self, mlist, msg):
95107
"""Return the url to the message in the archive.
@@ -103,12 +115,7 @@ def permalink(self, mlist, msg):
103115
be calculated.
104116
"""
105117
msg_id = msg['Message-Id'].strip().strip("<>")
106-
result = requests.get(urljoin(self.base_url, "api/mailman/urls"),
107-
params={"mlist": mlist.fqdn_listname,
108-
"msgid": msg_id, "key": self.api_key})
109-
# TODO: handle failures (check result.status_code)
110-
url = result.json()["url"]
111-
return urljoin(self.base_url, url)
118+
return self._get_url({"mlist": mlist.fqdn_listname, "msgid": msg_id})
112119

113120
def archive_message(self, mlist, msg):
114121
"""Send the message to the archiver.
@@ -118,12 +125,21 @@ def archive_message(self, mlist, msg):
118125
:returns: The url string or None if the message's archive url cannot
119126
be calculated.
120127
"""
121-
result = requests.post(urljoin(self.base_url, "api/mailman/archive"),
122-
params={"key": self.api_key},
128+
url = urljoin(self.base_url, "api/mailman/archive")
129+
result = requests.post(url, params={"key": self.api_key},
123130
data={"mlist": mlist.fqdn_listname},
124131
files={"message": ("message.txt", msg.as_string())})
125-
# TODO: handle failures (check result.status_code)
126-
url = urljoin(self.base_url, result.json()["url"])
132+
if result.status_code != 200:
133+
logger.error("HyperKitty failure on %s: %s (%s)",
134+
url, result.text, result.status_code)
135+
raise ValueError(result.text)
136+
try:
137+
result = result.json()
138+
except ValueError as e:
139+
logger.exception(
140+
"Invalid response from HyperKitty on %s: %s", url, e)
141+
raise
142+
archived_url = urljoin(self.base_url, result["url"])
127143
logger.info("HyperKitty archived message %s to %s",
128-
msg['Message-Id'].strip(), url)
129-
return url
144+
msg['Message-Id'].strip(), archived_url)
145+
return archived_url

mailman_hyperkitty/tests/_test_archiver.py

-106
This file was deleted.
+137
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
# -*- coding: utf-8 -*-
2+
# Copyright (C) 1998-2012 by the Free Software Foundation, Inc.
3+
#
4+
# This file is part of HyperKitty.
5+
#
6+
# HyperKitty is free software: you can redistribute it and/or modify it under
7+
# the terms of the GNU General Public License as published by the Free
8+
# Software Foundation, either version 3 of the License, or (at your option)
9+
# any later version.
10+
#
11+
# HyperKitty is distributed in the hope that it will be useful, but WITHOUT
12+
# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
13+
# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
14+
# more details.
15+
#
16+
# You should have received a copy of the GNU General Public License along with
17+
# HyperKitty. If not, see <http://www.gnu.org/licenses/>.
18+
#
19+
# Author: Aurelien Bompard <[email protected]>
20+
#
21+
22+
23+
import json
24+
from unittest import TestCase
25+
26+
from mock import patch
27+
from mailman.email.message import Message
28+
29+
from mailman_hyperkitty import Archiver
30+
31+
32+
class FakeResponse:
33+
"""Fake a response from the "requests" library"""
34+
35+
def __init__(self, status_code, result):
36+
self.status_code = status_code
37+
self.result = result
38+
39+
def json(self):
40+
return self.result
41+
42+
@property
43+
def text(self):
44+
return json.dumps(self.result)
45+
46+
47+
class FakeList:
48+
"""Fake a Mailman list implementing the IMailingList interface"""
49+
50+
def __init__(self, name):
51+
self.fqdn_listname = name
52+
53+
54+
class ArchiverTestCase(TestCase):
55+
56+
def setUp(self):
57+
self.archiver = Archiver()
58+
self.archiver._base_url = "http://lists.example.com"
59+
self.archiver._api_key = "DummyKey"
60+
self.mlist = FakeList("[email protected]")
61+
# Patch requests
62+
self.requests_patcher = patch("mailman_hyperkitty.requests")
63+
self.requests = self.requests_patcher.start()
64+
self.fake_response = None
65+
self.requests.get.side_effect = lambda url, *a, **kw: self.fake_response
66+
self.requests.post.side_effect = lambda url, *a, **kw: self.fake_response
67+
68+
def tearDown(self):
69+
self.requests_patcher.stop()
70+
71+
def _get_msg(self):
72+
msg = Message()
73+
msg["From"] = "[email protected]"
74+
msg["Message-ID"] = "<dummy>"
75+
msg["Message-ID-Hash"] = "QKODQBCADMDSP5YPOPKECXQWEQAMXZL3"
76+
msg.set_payload("Dummy message")
77+
return msg
78+
79+
80+
def test_list_url(self):
81+
self.fake_response = FakeResponse(
82+
200, {"url": "/list/[email protected]/"})
83+
self.assertEqual(self.archiver.list_url(self.mlist),
84+
"http://lists.example.com/list/[email protected]/"
85+
)
86+
#print(self.requests.get.call_args_list)
87+
self.requests.get.assert_called_with(
88+
"http://lists.example.com/api/mailman/urls",
89+
params={'key': 'DummyKey', 'mlist': '[email protected]'}
90+
)
91+
92+
def test_permalink(self):
93+
msg = self._get_msg()
94+
relative_url = "/list/[email protected]/message/{}/".format(
95+
msg["Message-ID-Hash"])
96+
self.fake_response = FakeResponse(200, {"url": relative_url})
97+
self.assertEqual(self.archiver.permalink(self.mlist, msg),
98+
"http://lists.example.com" + relative_url)
99+
#print(self.requests.get.call_args_list)
100+
self.requests.get.assert_called_with(
101+
"http://lists.example.com/api/mailman/urls",
102+
params={'key': 'DummyKey', 'msgid': 'dummy',
103+
'mlist': '[email protected]'}
104+
)
105+
106+
def test_archive_message(self):
107+
msg = self._get_msg()
108+
relative_url = "/list/[email protected]/message/{}/".format(
109+
msg["Message-ID-Hash"])
110+
self.fake_response = FakeResponse(200, {"url": relative_url})
111+
with patch("mailman_hyperkitty.logger") as logger:
112+
url = self.archiver.archive_message(self.mlist, msg)
113+
self.assertTrue(logger.info.called)
114+
self.assertEqual(url, "http://lists.example.com" + relative_url)
115+
#print(self.requests.post.call_args_list)
116+
self.requests.post.assert_called_with(
117+
"http://lists.example.com/api/mailman/archive",
118+
params={'key': 'DummyKey'},
119+
data={'mlist': '[email protected]'},
120+
files={'message': ('message.txt', msg.as_string())},
121+
)
122+
123+
def test_list_url_permalink_error(self):
124+
# Don't raise exceptions for list_url and permalink
125+
self.fake_response = FakeResponse(500, "Fake error")
126+
with patch("mailman_hyperkitty.logger") as logger:
127+
self.assertEqual(self.archiver.list_url(self.mlist), "")
128+
self.assertEqual(
129+
self.archiver.permalink(self.mlist, self._get_msg()), "")
130+
self.assertEqual(logger.error.call_count, 2)
131+
132+
def test_archive_message_error(self):
133+
self.fake_response = FakeResponse(500, "Fake error")
134+
with patch("mailman_hyperkitty.logger") as logger:
135+
self.assertRaises(ValueError, self.archiver.archive_message,
136+
self.mlist, self._get_msg())
137+
self.assertTrue(logger.error.called)

setup.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,6 @@
3939
'requests',
4040
'zope.interface',
4141
],
42-
test_requires = ["mock"],
42+
test_requires = ["mock", "nose2"],
4343
test_suite = 'nose2.collector.collector',
4444
)

0 commit comments

Comments
 (0)