Rev 227: Merged changes to xmlrpc thread from jml's review. in http://bzr.daniel-watkins.co.uk/pqm/xmlrpc
Daniel Watkins
daniel at daniel-watkins.co.uk
Thu Aug 7 03:57:55 BST 2008
At http://bzr.daniel-watkins.co.uk/pqm/xmlrpc
------------------------------------------------------------
revno: 227
revision-id: daniel at daniel-watkins.co.uk-20080807025609-57is5lmna7789as1
parent: daniel at daniel-watkins.co.uk-20080801033334-cxo81u0wbfecvv8o
parent: daniel at daniel-watkins.co.uk-20080807024055-c237c3o5y1lbqowy
committer: Daniel Watkins <daniel at daniel-watkins.co.uk>
branch nick: xmlrpc-validation
timestamp: Thu 2008-08-07 03:56:09 +0100
message:
Merged changes to xmlrpc thread from jml's review.
modified:
bin/pqm i_Simple_patch_queue_manager_for_tla
pqm/script.py script.py-20080528044209-lpacj0a0ub4woseh-2
pqm/ui/tests/test_xmlrpc.py xmlrpc.py-20080801002352-raj3r20bympisjau-1
pqm/ui/xmlrpc.py xmlrpc.py-20080731041643-5n1q2p7g2njf1mbc-1
------------------------------------------------------------
revno: 206.1.10
revision-id: daniel at daniel-watkins.co.uk-20080807024055-c237c3o5y1lbqowy
parent: daniel at daniel-watkins.co.uk-20080807022924-s6orqd7kmpky3nfz
committer: Daniel Watkins <daniel at daniel-watkins.co.uk>
branch nick: xmlrpc
timestamp: Thu 2008-08-07 03:40:55 +0100
message:
Changed return types and exception handling, as per jml's review.
modified:
pqm/ui/tests/test_xmlrpc.py xmlrpc.py-20080801002352-raj3r20bympisjau-1
pqm/ui/xmlrpc.py xmlrpc.py-20080731041643-5n1q2p7g2njf1mbc-1
------------------------------------------------------------
revno: 206.1.9
revision-id: daniel at daniel-watkins.co.uk-20080807022924-s6orqd7kmpky3nfz
parent: daniel at daniel-watkins.co.uk-20080807022715-cs7xg9620ivyqka9
committer: Daniel Watkins <daniel at daniel-watkins.co.uk>
branch nick: xmlrpc
timestamp: Thu 2008-08-07 03:29:24 +0100
message:
Comment cleanup, as per jml's review.
modified:
pqm/ui/xmlrpc.py xmlrpc.py-20080731041643-5n1q2p7g2njf1mbc-1
------------------------------------------------------------
revno: 206.1.8
revision-id: daniel at daniel-watkins.co.uk-20080807022715-cs7xg9620ivyqka9
parent: daniel at daniel-watkins.co.uk-20080807021819-7b80l71d2ygpvrai
committer: Daniel Watkins <daniel at daniel-watkins.co.uk>
branch nick: xmlrpc
timestamp: Thu 2008-08-07 03:27:15 +0100
message:
Added XMLRPC docstrings.
modified:
pqm/ui/xmlrpc.py xmlrpc.py-20080731041643-5n1q2p7g2njf1mbc-1
------------------------------------------------------------
revno: 206.1.7
revision-id: daniel at daniel-watkins.co.uk-20080807021819-7b80l71d2ygpvrai
parent: daniel at daniel-watkins.co.uk-20080807021754-w1bln7i18t95e1ci
committer: Daniel Watkins <daniel at daniel-watkins.co.uk>
branch nick: xmlrpc
timestamp: Thu 2008-08-07 03:18:19 +0100
message:
Added test for malformed input, as per jml's review.
modified:
pqm/ui/tests/test_xmlrpc.py xmlrpc.py-20080801002352-raj3r20bympisjau-1
------------------------------------------------------------
revno: 206.1.6
revision-id: daniel at daniel-watkins.co.uk-20080807021754-w1bln7i18t95e1ci
parent: daniel at daniel-watkins.co.uk-20080807021129-0tft5ozta0y3qhpi
committer: Daniel Watkins <daniel at daniel-watkins.co.uk>
branch nick: xmlrpc
timestamp: Thu 2008-08-07 03:17:54 +0100
message:
Extracted assertQueueLength in tests.
modified:
pqm/ui/tests/test_xmlrpc.py xmlrpc.py-20080801002352-raj3r20bympisjau-1
------------------------------------------------------------
revno: 206.1.5
revision-id: daniel at daniel-watkins.co.uk-20080807021129-0tft5ozta0y3qhpi
parent: daniel at daniel-watkins.co.uk-20080807020506-si9cmfzk8fcrtbcw
committer: Daniel Watkins <daniel at daniel-watkins.co.uk>
branch nick: xmlrpc
timestamp: Thu 2008-08-07 03:11:29 +0100
message:
Added comment to test, as per jml's review.
modified:
pqm/ui/tests/test_xmlrpc.py xmlrpc.py-20080801002352-raj3r20bympisjau-1
------------------------------------------------------------
revno: 206.1.4
revision-id: daniel at daniel-watkins.co.uk-20080807020506-si9cmfzk8fcrtbcw
parent: daniel at daniel-watkins.co.uk-20080807020420-6seg494lf57h9yuo
committer: Daniel Watkins <daniel at daniel-watkins.co.uk>
branch nick: xmlrpc
timestamp: Thu 2008-08-07 03:05:06 +0100
message:
More specifically test for None, as per jml's review.
modified:
pqm/script.py script.py-20080528044209-lpacj0a0ub4woseh-2
------------------------------------------------------------
revno: 206.1.3
revision-id: daniel at daniel-watkins.co.uk-20080807020420-6seg494lf57h9yuo
parent: daniel at daniel-watkins.co.uk-20080807014409-e2c7qy30w93zfeqe
committer: Daniel Watkins <daniel at daniel-watkins.co.uk>
branch nick: xmlrpc
timestamp: Thu 2008-08-07 03:04:20 +0100
message:
Added docstrings, as per jml's review.
modified:
pqm/script.py script.py-20080528044209-lpacj0a0ub4woseh-2
------------------------------------------------------------
revno: 206.1.2
revision-id: daniel at daniel-watkins.co.uk-20080807014409-e2c7qy30w93zfeqe
parent: daniel at daniel-watkins.co.uk-20080807013206-p7yx2610y00ht8dh
committer: Daniel Watkins <daniel at daniel-watkins.co.uk>
branch nick: xmlrpc
timestamp: Thu 2008-08-07 02:44:09 +0100
message:
Fixed some indentation.
modified:
bin/pqm i_Simple_patch_queue_manager_for_tla
------------------------------------------------------------
revno: 206.1.1
revision-id: daniel at daniel-watkins.co.uk-20080807013206-p7yx2610y00ht8dh
parent: daniel at daniel-watkins.co.uk-20080801010152-y7lskmc4fqy6vc8o
committer: Daniel Watkins <daniel at daniel-watkins.co.uk>
branch nick: xmlrpc
timestamp: Thu 2008-08-07 02:32:06 +0100
message:
Simplified canonicalisation of line endings, as per John's review comments.
modified:
pqm/script.py script.py-20080528044209-lpacj0a0ub4woseh-2
-------------- next part --------------
=== modified file 'bin/pqm'
--- a/bin/pqm 2008-07-31 06:38:30 +0000
+++ b/bin/pqm 2008-08-07 01:44:09 +0000
@@ -103,9 +103,10 @@
try:
try:
logger.info('trying script ' + script.filename)
- logname = os.path.join(logdir, os.path.basename(script.filename) + '.log')
- (sender, subject, msg, sig) = \
- read_email_from_file(logger, open(script.filename))
+ logname = os.path.join(logdir,
+ os.path.basename(script.filename) + '.log')
+ (sender, subject, msg, sig) = read_email_from_file(
+ logger, open(script.filename))
if options.verify_sigs:
sigid,siguid = verify_sig(
script.getSender(), msg, sig, 0, logger, options.keyring)
=== modified file 'pqm/script.py'
--- a/pqm/script.py 2008-08-01 03:33:34 +0000
+++ b/pqm/script.py 2008-08-07 02:56:09 +0000
@@ -37,7 +37,16 @@
def get_email_structure(logger, msg):
- """Read an email and check it has the required structure for commands."""
+ """Read an email and check it has the required structure for commands.
+
+ :param logger: The logger to which messages should be sent.
+ :param msg: A Message to be checked.
+
+ :return: A tuple consisting of ( message sender, message subject, message
+ text, message signature ). The message signature will be None unless it
+ the message is multipart and the second part of the message is a PGP
+ signature.
+ """
sender = msg['From']
logger.info("recieved email from %s", sender)
subject = msg['Subject']
@@ -70,18 +79,57 @@
def read_email_from_file(logger, file=None):
- if not file:
+ """Read an email from a file, check for the structure for commands.
+
+ This is a convenience wrapper around get_email_structure, so that callers
+ don't have to worry about using the email module correctly.
+
+ :param logger: The logger to which messages should be sent.
+ :param file: An open file-like object containing an email message. If not
+ specified, defaults to sys.stdin.
+
+ :return: A tuple consisting of ( message sender, message subject, message
+ text, message signature ). The message signature will be None unless it
+ the message is multipart and the second part of the message is a PGP
+ signature.
+ """
+ if file is None:
file = sys.stdin
msg = email.message_from_file(file)
return get_email_structure(logger, msg)
def read_email_from_string(logger, string):
+ """Read an email from a string, check for the structure for commands.
+
+ This is a convenience wrapper around get_email_structure, so that callers
+ don't have to worry about using the email module correctly.
+
+ :param logger: The logger to which messages should be sent.
+ :param string: A string containing an email message.
+
+ :return: A tuple consisting of ( message sender, message subject, message
+ text, message signature ). The message signature will be None unless it
+ the message is multipart and the second part of the message is a PGP
+ signature.
+ """
msg = email.message_from_string(string)
return get_email_structure(logger, msg)
def get_email_string(logger, options, email):
+ """Get the string for an email, checking it is signed correctly.
+
+ :param logger: The logger to which messages should be sent.
+ :param options: An object containing the options which should be used for
+ signing.
+ :param email: A tuple consisting of ( message sender, message subject,
+ message text, message signature ). The message signature should be None
+ if either there is no signature for the email or the signature was not
+ in its own part of a multipart email.
+
+ :return: A string representing the given email tuple.
+ """
(sender, subject, msg, sig) = email
if options.verify_sigs:
# TODO: Daniel Watkins, 2008-07-31
@@ -91,7 +139,7 @@
options.keyring)
open(transaction_file, 'a').write(sigid + '\n')
# canonicalize line endings
- body = '\n'.join(re.split('\r?\n', msg))
+ body = msg.replace('\r\n', '\n')
return ("From: %(sender)s\n"
"Subject: %(subject)s\n"
"%(body)s" % {'sender': sender,
=== modified file 'pqm/ui/tests/test_xmlrpc.py'
--- a/pqm/ui/tests/test_xmlrpc.py 2008-08-01 03:32:52 +0000
+++ b/pqm/ui/tests/test_xmlrpc.py 2008-08-07 02:56:09 +0000
@@ -17,6 +17,8 @@
import os
+from twisted.web.xmlrpc import Fault
+
from pqm.errors import PQMCmdFailure
from pqm.tests import (sample_message_bad,
sample_message_template,
@@ -27,6 +29,11 @@
class TestPQM_XMLRPC(TestCaseWithQueue):
+ def assertQueueLength(self, queuedir, length):
+ patches = [f for f in os.listdir(queuedir) if f.startswith('patch.')]
+ self.assertEqual(length, len(patches))
+ return patches
+
def get_xmlrpc(self, empty=True, verify_sigs=False):
queue = self.getQueue(empty=empty, verify_sigs=verify_sigs)
pqminfo = PQMInfo([queue.configFileName])
@@ -34,48 +41,46 @@
def test_empty_submission(self):
xmlrpc = self.get_xmlrpc()
- out = xmlrpc.xmlrpc_submit('')
- self.assertEqual("Error: 'No From specified'", out)
+ out = self.assertRaises(Fault, xmlrpc.xmlrpc_submit, '')
+ self.assertEqual("'No From specified'", out.faultString)
def test_valid_submission(self):
+ # Tests that a valid message (i.e. one with a From and Subject line)
+ # receives the appropriate message from XMLRPC.
xmlrpc = self.get_xmlrpc()
msg = (sample_message_template %
('star-merge baz baz',))
- out = xmlrpc.xmlrpc_submit(msg)
- self.assertEqual('Success!', out)
+ self.assertTrue(xmlrpc.xmlrpc_submit(msg))
queuedir = xmlrpc.pqminfo.queuedir
- patches = [f for f in os.listdir(queuedir) if f.startswith('patch.')]
- self.assertEqual(1, len(patches)) # We should have only one patch
+ patches = self.assertQueueLength(queuedir, 1)
self.assertFileEqual(msg, os.path.join(queuedir, patches[0]))
def test_invalid_submission(self):
+ # Tests that a meaningless message receives an appropriate response
+ # from XMLRPC.
xmlrpc = self.get_xmlrpc()
- out = xmlrpc.xmlrpc_submit(sample_message_bad)
- self.assertEqual("Error: 'Sender not authorised to commit to branch"
+ out = self.assertRaises(Fault, xmlrpc.xmlrpc_submit, sample_message_bad)
+ self.assertEqual("'Sender not authorised to commit to branch"
" http://www.example.com/foo/unregistered'",
- out)
+ out.faultString)
queuedir = xmlrpc.pqminfo.queuedir
- patches = [f for f in os.listdir(queuedir) if f.startswith('patch.')]
- self.assertEqual(0, len(patches))
+ self.assertQueueLength(queuedir, 0)
def test_invalid_target(self):
xmlrpc = self.get_xmlrpc()
msg = (sample_message_template %
('star-merge baz http://www.example.com/foo/unregistered',))
- out = xmlrpc.xmlrpc_submit(msg)
- self.assertEqual("Error: 'Sender not authorised to commit to branch"
+ out = self.assertRaises(Fault, xmlrpc.xmlrpc_submit, msg)
+ self.assertEqual("'Sender not authorised to commit to branch"
" http://www.example.com/foo/unregistered'",
- out)
+ out.faultString)
queuedir = xmlrpc.pqminfo.queuedir
- patches = [f for f in os.listdir(queuedir) if f.startswith('patch.')]
- self.assertEqual(0, len(patches))
+ self.assertQueueLength(queuedir, 0)
def test_invalid_source(self):
xmlrpc = self.get_xmlrpc()
- msg = (sample_message_template %
- ('star-merge fluff baz',))
- out = xmlrpc.xmlrpc_submit(msg)
- self.assertEqual("Error: 'fluff is not a branch'", out)
+ msg = (sample_message_template % ('star-merge fluff baz',))
+ out = self.assertRaises(Fault, xmlrpc.xmlrpc_submit, msg)
+ self.assertEqual("'fluff is not a branch'", out.faultString)
queuedir = xmlrpc.pqminfo.queuedir
- patches = [f for f in os.listdir(queuedir) if f.startswith('patch.')]
- self.assertEqual(0, len(patches))
+ self.assertQueueLength(queuedir, 0)
=== modified file 'pqm/ui/xmlrpc.py'
--- a/pqm/ui/xmlrpc.py 2008-08-01 03:32:52 +0000
+++ b/pqm/ui/xmlrpc.py 2008-08-07 02:56:09 +0000
@@ -20,7 +20,7 @@
from StringIO import StringIO
import time
-from twisted.web.xmlrpc import XMLRPC
+from twisted.web.xmlrpc import Fault, XMLRPC
import pqm
from pqm.errors import PQMCmdFailure, PQMException
@@ -29,8 +29,18 @@
class PQM_XMLRPC(XMLRPC):
+ """An XMLRPC object for PQM submissions.
+
+ :ivar pqminfo: A PQMInfo object.
+ :ivar logger: The logger to which messages should be sent.
+ """
def __init__(self, pqminfo):
+ """Construct a PQM_XMLRPC instance.
+
+ :param pqminfo: A PQMInfo object representing the PQM queue that
+ requests are to be submitted for.
+ """
XMLRPC.__init__(self)
self.pqminfo = pqminfo
self.logger = logging.getLogger('pqm')
@@ -46,6 +56,13 @@
command.check_is_valid()
def xmlrpc_submit(self, text):
+ """A method exposed via XMLRPC to submit items to the PQM queue.
+
+ :param text: The text of the submission email message to be added to
+ the queue.
+ :return: True if the submission was successful.
+ :raises Fault: if the submission was not successful.
+ """
try:
queuedir = self.pqminfo.queuedir
queuedir = os.path.abspath(queuedir)
@@ -56,20 +73,20 @@
if options.verify_sigs:
options.keyring = self.pqminfo.configp.get('DEFAULT',
'keyring')
- # TODO: Eww.
+ # TODO: Setting stuff in the pqm module shouldn't be necessary,
+ # this should be fixed along with all the other places this
+ # happens.
pqm.gpgv_path = self.pqminfo.configp.get_option('DEFAULT',
'gpgv_path', 'gpgv')
pqm_subdir = os.path.join(queuedir, 'pqm')
pqm.pqm_subdir = pqm_subdir
- # Process input
email = read_email_from_string(self.logger, text)
email_string = get_email_string(self.logger, options, email)
try:
self.validate(email_string)
except PQMCmdFailure, e:
raise PQMException(None, '\n'.join(e.output))
- # Write script
pqm.write_script(queuedir, email_string)
- return "Success!"
+ return True
except PQMException, e:
- return "Error: %s" % (e,)
+ raise Fault(0, "%s" % (e,))
More information about the bazaar-commits
mailing list