using X-pypi-* headers now, cleaned up success/fail handling

7 files changed, 209 insertions, 130 deletions

diff --git a/TODO.txt b/TODO.txt
index 4a50364..21cba68 100644
--- a/TODO.txt
+++ b/TODO.txt
@@ -3,13 +3,13 @@ Now:
 - finalise database schema
   - multiple download links?
 - file upload of PKG_INFO (or whole package)
-- clean up HTML interface
 - Trove
 - change notification emails
 - password reset through the web
 - password change through the web
 - saving of login info in register command
 - better journal entries (than "update")
+- clean up HTML interface
 
 Later:
 
diff --git a/config.pyc b/config.pyc
index 6b1054e..6f4af47 100644
--- a/config.pyc
+++ b/config.pyc
diff --git a/doc/download.ideas.txt b/doc/download.ideas.txt
new file mode 100644
index 0000000..75a46e7
--- /dev/null
+++ b/doc/download.ideas.txt
@@ -0,0 +1,16 @@
+I've included a public_key in the user schema in anticipation of handling
+signing. I've also got a download_url in the package schema - though a
+friend has recommended that I have several. I'm not so sure. I _think_ I'd
+rather have:
+
+- a definition of download formats for each package (filename, format)
+  where
+  format is rpm, deb, source gz, source zip, ...
+- a definition of download mirrors which give (mirror name, base URL)
+
+and then each mirror can support the same paths starting from their base
+URL.
+
+None of this is intended for the initial release though.
+
+
diff --git a/doc/pep.txt b/doc/pep.txt
index d473a5a..1c399ec 100644
--- a/doc/pep.txt
+++ b/doc/pep.txt
@@ -15,13 +15,14 @@ Abstract
 ========
 
 This PEP proposes several extensions to the distutils packaging
-system [1]_. These enhancements include a central package index, tools
-for submitting package information to the index and extensions to the
-package metadata to include Trove [2]_ information.
+system [1]_. These enhancements include a central package index
+server, tools for submitting package information to the index and
+extensions to the package metadata to include Trove [2]_ information.
 
-This PEP does not address either issues of package dependency, nor
-centralised storage of packages. Nor is it proposing a local
-database of packages as described in PEP 262 [6]_.
+This PEP does not address either issues of package dependency. It
+also does not address storage and download of packages as described
+in PEP 243 [6]_. Nor is it proposing a local database of packages as
+described in PEP 262 [7]_.
 
 Existing package repositories such as the Vaults of Parnassus [3]_,
 CPAN [4]_ and PAUSE [5]_ will be investigated as prior art in this
@@ -57,7 +58,10 @@ was dropped as the author realised that platform packaging systems
 
 Issues of package dissemination (storage on a central server) are
 not addressed because they require assumptions about availability of
-storage and bandwidth that I am not in a position to make.
+storage and bandwidth that I am not in a position to make. PEP 243,
+which is still being developed, is tackling these issues and many
+more. This proposal is considered compatible with, and adjunct to
+the proposal in PEP 243.
 
 
 Specification
@@ -108,8 +112,8 @@ The web interface implements the following commands/interfaces:
 
 **user**
   Registers a new user with the index. Requires username, password and
-  email address. Passwords will be stored on the server as SHA hashes.
-  If the username already exists in the database:
+  email address. Passwords will be stored in the index database as SHA
+  hashes. If the username already exists in the database:
 
   1. If valid HTTP Basic auth is provided, the password and email
      address are updated with the submission information, or
@@ -119,7 +123,7 @@ The web interface implements the following commands/interfaces:
   Registration will be a three-step process, involving:
 
   1. User submission of details via the distutils *register* command,
-  2. Package server sending email to the user's email address with a URL
+  2. Index server sending email to the user's email address with a URL
      to visit to confirm registration with a random one-time key, and
   3. User visits URL with the key and confirms registration.
 
@@ -151,7 +155,7 @@ Distutils Register Command
 --------------------------
 
 An additional distutils command, "register" is implemented which
-posts the package metadata to the central server. The register command
+posts the package metadata to the central index. The register command
 automatically handles user registration; the user is presented with
 three options:
 
@@ -168,12 +172,22 @@ who have submitted information about the package. That is, the original
 submitter and any subsequent updaters.
 
 The register command will include a --verify option which performs a
-test submission to the server without actually committing the data.
-The server will perform its submission verification checks as usual
+test submission to the index without actually committing the data.
+The index will perform its submission verification checks as usual
 and report any errors it would have reported during a normal
 submission. This is useful for verifying correctness of Trove
 discriminators.
 
+The index server will return custom headers (inspired by PEP 243)
+which the register command will use to give feedback to the user:
+
+**X-Pypi-Status**
+  Either "success" or "fail".
+
+**X-Pypi-Reason**
+  A description of the reason for failure, or additional information
+  in the case of a success.
+
 
 Distutils Trove Categorisation
 ------------------------------
@@ -239,7 +253,7 @@ Reference code is available from the sourceforge project:
 
 A demonstration will be available at:
 
-  http://www.amk.ca/cgi-bin/package_server.cgi
+  http://www.amk.ca/cgi-bin/pypi.cgi
 
 ===== ===================================================
 Done  Feature
@@ -279,7 +293,10 @@ References
 .. [5] PAUSE
    (http://pause.cpan.org/)
 
-.. [6] PEP 262, A Database of Installed Python Packages
+.. [6] PEP 243, Module Repository Upload Mechanism
+   (http://www.python.org/peps/pep-0243.html)
+
+.. [7] PEP 262, A Database of Installed Python Packages
    (http://www.python.org/peps/pep-0262.html)
 
 Copyright
diff --git a/register.py b/register.py
index a245b00..0a51dd9 100644
--- a/register.py
+++ b/register.py
@@ -17,7 +17,9 @@ class register(Command):
 
     description = "register the distribution with the repository"
 
-    DEFAULT_REPOSITORY = 'https://localhost/cgi-bin/pypi.cgi'
+    #DEFAULT_REPOSITORY = 'http://mechanicalcat.net/cgi-bin/pypi.cgi'
+    DEFAULT_REPOSITORY = 'http://localhost/cgi-bin/pypi.cgi'
+    #DEFAULT_REPOSITORY = 'http://www.amk.ca/cgi-bin/pypi.cgi'
 
     user_options = [
         ('repository=', 'r',
@@ -97,7 +99,7 @@ class register(Command):
 
         # send the info to the server and report the result
         (code, result) = self.post_to_server(data)
-        print 'Server response: %s'%result
+        print 'Server response (%s): %s'%(code, result)
 
     def send_metadata(self):
         ''' Send the metadata to the package index server.
@@ -182,7 +184,7 @@ Your selection [default 1]: ''',
 
             # send the info to the server and report the result
             (code, result) = self.post_to_server(data, auth)
-            print 'Server response: %s'%result
+            print 'Server response (%s): %s'%(code, result)
         elif choice == '2':
             data = {':action': 'user'}
             data['name'] = data['password'] = data['email'] = ''
@@ -202,7 +204,7 @@ Your selection [default 1]: ''',
                 data['email'] = raw_input('   EMail: ')
             (code, result) = self.post_to_server(data)
             if result != 'Registration OK':
-                print 'Server response: %s'%result
+                print 'Server response (%s): %s'%(code, result)
             else:
                 print 'You will receive an email shortly.'
                 print 'Follow the instructions in it to complete registration.'
@@ -212,7 +214,7 @@ Your selection [default 1]: ''',
             while not data['email']:
                 data['email'] = raw_input('Your email address: ')
             (code, result) = self.post_to_server(data)
-            print 'Server response: %s'%result
+            print 'Server response (%s): %s'%(code, result)
 
     def post_to_server(self, data, auth=None):
         ''' Post a query to the server, and return a string response.
@@ -251,9 +253,16 @@ Your selection [default 1]: ''',
         try:
             result = opener.open(req)
         except urllib2.HTTPError, e:
-            return (e.code, e.fp.read())
+            if e.headers.has_key('x-pypi-reason'):
+                reason = e.headers['x-pypi-reason']
+                if reason == 'error':
+                    return 'fail', e.fp.read()
+                else:
+                    return 'fail', reason
+            else:
+                return 'fail', e.fp.read()
         except urllib2.URLError, e:
-            return (None, str(e))
+            return 'fail', str(e)
 
-        return (200, result.read())
+        return result.headers['x-pypi-status'], result.headers['x-pypi-reason']
 
diff --git a/store.py b/store.py
index e4cfa73..f90d0ad 100644
--- a/store.py
+++ b/store.py
@@ -73,27 +73,19 @@ class Store:
     def store_package(self, name, version, info):
         ''' Store info about the package to the database.
 
+            The name and version must not appear in the info dict.
+
             We automatically set the "submitted_date" column here, don't
             send it in.
         '''
-        # make sure the user is identified
-        if not self.username:
-            raise StorageError, \
-                "You must be identified to store package information"
-
         date = time.strftime('%Y-%m-%d %H:%M:%S')
         cols = info.keys()
+        # XXX delete name/version if they're in info
         if self.has_package(name, version):
-            # make sure the user has permission to do stuff
-            if not (self.has_role('Maintainer', name) or
-                    self.has_role('Owner', name)):
-                raise StorageError, \
-                    "You are not allowed to store '%s' package information"%name
             # update
-            args = [info[k] for k in cols]
+            args = tuple([info[k] for k in cols] + [name, version])
             info = ','.join(['%s=%%s'%x for x in cols])
-            sql = "update packages set %s where name='%s' and version='%s'"%(
-                info, name, version)
+            sql = "update packages set %s where name=%%s and version=%%s"%info
             self.cursor.execute(sql, args)
             self.cursor.execute('''insert into journal (
                   name, version, action, submitted_date, submitted_by,
@@ -103,7 +95,7 @@ class Store:
             # insert
             info['name'] = name
             info['version'] = version
-            args = [info[k] for k in cols]
+            args = tuple([info[k] for k in cols])
             cols = ','.join(cols)
             params = ','.join(['%s']*len(info))
             sql = 'insert into packages (%s) values (%s)'%(cols, params)
@@ -123,8 +115,8 @@ class Store:
 
             Returns true/false.
         '''
-        self.cursor.execute("select count(*) from packages where name='%s' "
-            " and version='%s'"%(name, version))
+        sql = 'select count(*) from packages where name=%s and version=%s'
+        self.cursor.execute(sql, (name, version))
         res = int(self.cursor.fetchone()[0])
         return res
 
@@ -133,8 +125,8 @@ class Store:
 
             Returns a mapping with the package info.
         '''
-        self.cursor.execute("select * from packages where name='%s' "
-            " and version='%s'"%(name, version))
+        sql = "select * from packages where name=%s and version=%s"
+        self.cursor.execute(sql, (name, version))
         return self.cursor.fetchone()
 
     def get_journal(self, name, version):
@@ -142,8 +134,9 @@ class Store:
 
             Returns a mapping with the package info.
         '''
-        self.cursor.execute("select * from journal where name=%s "
-            " and (version=%s or version is NULL)", (name, version))
+        sql = 'select * from journal where name=%s and (version=%s '\
+            'or version is NULL)'
+        self.cursor.execute(sql, (name, version))
         return self.cursor.fetchall()
 
     def query_packages(self, spec, andor='and'):
diff --git a/webui.py b/webui.py
index b163144..70e416f 100644
--- a/webui.py
+++ b/webui.py
@@ -69,30 +69,71 @@ class WebUI:
             except NotFound:
                 self.handler.send_error(404)
             except Unauthorised, message:
-                self.handler.send_response(401)
-                self.handler.send_header('Content-Type', 'text/plain')
-                self.handler.send_header('WWW-Authenticate',
-                    'Basic realm="python packages index"')
-                self.handler.end_headers()
-                self.wfile.write(str(message))
+                message = str(message)
+                if not message:
+                    message = 'You must login to access this feature'
+                self.fail(message, code=401, heading='Login required',
+                    headers={'WWW-Authenticate':
+                    'Basic realm="python packages index"'})
             except Redirect, path:
                 self.handler.send_response(301)
                 self.handler.send_header('Location', path)
             except FormError, message:
-                self.page_head('Python Packages Index', 'Error processing form',
-                    400)
-                self.wfile.write('<p class="errror">Error processing form:'\
-                    ' %s</p>'%message)
+                message = str(message)
+                self.fail(message, code=400, heading='Error processing form')
             except:
-                self.page_head('Python Packages Index', 'Error...', 400)
-                self.wfile.write("<pre>")
                 s = StringIO.StringIO()
                 traceback.print_exc(None, s)
-                self.wfile.write(cgi.escape(s.getvalue()))
-                self.wfile.write("</pre>\n")
+                s = cgi.escape(s.getvalue())
+                self.fail('error', code=400, heading='Error...',
+                    content='<pre>%s</pre>'%s)
         finally:
             self.store.close()
 
+    def fail(self, message, title="Python Packages Index", code=400,
+            heading=None, headers={}, content=''):
+        status = ('fail', message)
+        self.page_head(title, status, heading, code, headers)
+        self.wfile.write('<p class="error">%s</p>'%message)
+        self.wfile.write(content)
+        self.page_foot()
+
+    def success(self, message=None, title="Python Packages Index", code=200,
+            heading=None, headers={}, content=''):
+        if message:
+            status = ('success', message)
+        else:
+            status = None
+        self.page_head(title, status, heading, code, headers)
+        if message:
+            self.wfile.write('<p class="ok">%s</p>'%message)
+        self.wfile.write(content)
+        self.page_foot()
+
+    def page_head(self, title, status=None, heading=None, code=200, headers={}):
+        self.handler.send_response(code)
+        self.handler.send_header('Content-Type', 'text/html')
+        if status is None:
+            status = ('success', 'success')
+        self.handler.send_header('X-Pypi-Status', status[0])
+        self.handler.send_header('X-Pypi-Reason', status[1])
+        for k,v in headers.items():
+            self.handler.send_header(k, v)
+        self.handler.end_headers()
+        if heading is None: heading = title
+        self.wfile.write('''
+<html><head><title>%s</title>
+<link rel="stylesheet" type="text/css" href="http://mechanicalcat.net/style.css">
+<link rel="stylesheet" type="text/css" href="http://mechanicalcat.net/page.css">
+</head>
+<body>
+<div id="header"><h1>%s</h1></div>
+<div id="content">
+'''%(title, heading))
+
+    def page_foot(self):
+        self.wfile.write('\n</div>\n</body></html>\n')
+
     def inner_run(self):
         ''' Figure out what the request is, and farm off to the appropriate
             handler.
@@ -134,31 +175,11 @@ class WebUI:
         # commit any database changes
         self.store.commit()
 
-    def page_head(self, title, heading=None, code=200):
-        ''' Page header
-        '''
-        self.handler.send_response(code)
-        self.handler.send_header('Content-Type', 'text/html')
-        self.handler.end_headers()
-        if heading is None: heading = title
-        self.wfile.write('''
-<html><head><title>%s</title>
-<link rel="stylesheet" type="text/css" href="http://mechanicalcat.net/style.css">
-<link rel="stylesheet" type="text/css" href="http://mechanicalcat.net/page.css">
-</head>
-<body>
-<div id="header"><h1>%s</h1></div>
-<div id="content">
-'''%(title, heading))
-
-    def page_foot(self):
-        self.wfile.write('\n</div>\n</body></html>\n')
-
     def index(self):
         ''' Print up an index page
         '''
-        self.page_head('Python modules index')
-        w = self.wfile.write
+        content = StringIO.StringIO()
+        w = content.write
         w('<a href="?:action=search_form">search</a>\n')
         w('| <a href="?:action=role_form">admin</a>\n')
         w('| <a href="?:action=submit_form">manual submission</a>\n')
@@ -203,7 +224,7 @@ searching, but the web interface doesn't expose it yet :)</p>
 <p>Entries are unique by (name, version) and multiple submissions of the same
 (name, version) result in updates to the existing entry.</p>
 ''')
-        self.page_foot()
+        self.success(heading='Index of packages', content=content.getvalue())
 
     def search_form(self):
         ''' A form used to generate filtered index displays
@@ -268,8 +289,7 @@ searching, but the web interface doesn't expose it yet :)</p>
 </tr>
 '''
         # now write the body
-        self.page_head('Python package index', 'Role maintenance')
-        self.wfile.write('''
+        s = '''
 <form>
 <input type="hidden" name=":action" value="role">
 <table class="form">
@@ -288,8 +308,8 @@ searching, but the web interface doesn't expose it yet :)</p>
         <input type="submit" name=":operation" value="Remove Role"></td></tr>
 </table>
 </form>
-'''%package)
-        self.page_foot()
+'''%package
+        self.success(heading='Role maintenance', content=s)
 
     def role(self):
         ''' Add a Role to a user.
@@ -321,25 +341,33 @@ searching, but the web interface doesn't expose it yet :)</p>
             if self.store.has_role(role_name, package_name, user_name):
                 raise FormError, 'user has that role already'
             self.store.add_role(user_name, role_name, package_name)
-            self.plain_response('Role Added OK')
+            message = 'Role Added OK'
         else:
             self.store.delete_role(user_name, role_name, package_name)
-            self.plain_response('Role Removed OK')
+            message = 'Role Removed OK'
+
+        self.success(message=message, heading='Role maintenance')
 
     def display(self):
         ''' Print up an entry
         '''
-        w = self.wfile.write
+        content = StringIO.StringIO()
+        w = content.write
+
+        # get the appropriate package info from the database
         name = self.form['name'].value
         version = self.form['version'].value
         info = self.store.get_package(name, version)
-        self.page_head('Python module: %s %s'%(name, version))
+
+        # top links
         un = urllib.quote(name)
         uv = urllib.quote(version)
         w('<a href="?:action=index">index</a>\n')
         w('| <a href="?:action=role_form&package_name=%s">admin</a>\n'%un)
         w('| <a href="?:action=submit_form&name=%s&version=%s"'
             '>edit</a><br>'%(un, uv))
+
+        # now the package info
         w('<table class="form">\n')
         keys = info.keys()
         keys.sort()
@@ -355,6 +383,7 @@ searching, but the web interface doesn't expose it yet :)</p>
                 w('<tr><th nowrap>%s: </th><td>%s</td></tr>\n'%(label, value))
         w('\n</table>\n')
 
+        # package's journal
         w('<table class="journal">\n')
         w('<tr><th>Date</th><th>User</th><th>IP Address</th><th>Action</th></tr>\n')
         for entry in self.store.get_journal(name, version):
@@ -363,7 +392,8 @@ searching, but the web interface doesn't expose it yet :)</p>
                 entry['submitted_from'], entry['action']))
         w('\n</table>\n')
 
-        self.page_foot()
+        self.success(heading='Python module: %s %s'%(name, version),
+            content=content.getvalue())
 
     def submit_form(self):
         ''' A form used to submit or edit package metadata.
@@ -383,12 +413,16 @@ searching, but the web interface doesn't expose it yet :)</p>
             for k,v in self.store.get_package(name, version).items():
                 info[k] = v
 
-        self.page_head('Python package index', 'Manual submission')
-        w = self.wfile.write
+        # submission of this form requires a login, so we should check
+        # before someone fills it in ;)
+        if not self.username:
+            raise Unauthorised, 'You must log in'
+
+        content = StringIO.StringIO()
+        w = content.write
         w('''
 <form>
 <input type="hidden" name=":action" value="submit">
-<input type="hidden" name=":display" value="html">
 <table class="form">
 ''')
 
@@ -425,11 +459,19 @@ searching, but the web interface doesn't expose it yet :)</p>
 </table>
 </form>
 ''')
-        self.page_foot()
+
+        self.success(heading='Manual submission form',
+            content=content.getvalue())
 
     def submit(self):
         ''' Handle the submission of distro metadata.
         '''
+        # make sure the user is identified
+        if not self.username:
+            raise Unauthorised, \
+                "You must be identified to store package information"
+
+        # pull the package information out of the form submission
         data = {}
         for k in self.form.keys():
             if k.startswith(':'): continue
@@ -438,21 +480,29 @@ searching, but the web interface doesn't expose it yet :)</p>
                 data[k.lower().replace('-','_')]=','.join([x.value for x in v])
             else:
                 data[k.lower().replace('-','_')] = v.value
-        result = 'plain'
-        if self.form.has_key(':display'):
-            result = self.form[':display'].value
+
+        # validate the data
         try:
             self.validate_metadata(data)
         except ValueError, message:
-            if result == 'html':
-                raise FormError, message
-            raise
-        self.store.store_package(data['name'], data['version'], data)
+            raise FormError, message
+
+        name = data['name']
+        version = data['version']
+
+        # make sure the user has permission to do stuff
+        if self.store.has_package(data['name'], data['version']) and not (
+                self.store.has_role('Owner', name) or
+                self.store.has_role('Maintainer', name)):
+            raise Unauthorised, \
+                "You are not allowed to store '%s' package information"%name
+
+        # save off the data
+        self.store.store_package(name, version, data)
         self.store.commit()
-        if result == 'html':
-            self.display()
-        else:
-            self.plain_response('Submission OK')
+
+        # return a display of the package
+        self.display()
 
     def verify(self):
         ''' Validate the input data.
@@ -468,9 +518,10 @@ searching, but the web interface doesn't expose it yet :)</p>
         try:
             self.validate_metadata(data)
         except ValueError, message:
-            self.plain_response('Error: %s'%message)
-        else:
-            self.plain_response('Validated OK')
+            self.fail(message, heading='Package verification')
+            return
+
+        self.success(heading='Package verification', message='Validated OK')
 
     def validate_metadata(self, data):
         ''' Validate the contents of the metadata.
@@ -485,18 +536,11 @@ searching, but the web interface doesn't expose it yet :)</p>
         if data.has_key('metadata_version'):
             del data['metadata_version']
 
-    def plain_response(self, message):
-        ''' Return a plain-text response to the user.
-        '''
-        self.handler.send_response(200)
-        self.handler.send_header('Content-Type', 'text/plain')
-        self.handler.end_headers()
-        self.wfile.write(message)
-
     def register_form(self):
         ''' Throw up a form for regstering.
         '''
-        self.page_head('Python package index', 'Manual user registration')
+        self.page_head('Python package index',
+            heading='Manual user registration')
         w = self.wfile.write
         w('''
 <form>
@@ -550,10 +594,12 @@ email.</p>
             # new user, create entry and email otk
             name = info['name']
             if self.store.has_user(name):
-                self.plain_response('Error: user "%s" already exists'%name)
+                self.fail('user "%s" already exists'%name,
+                    heading='User registration')
                 return
             if info.has_key('confirm') and info['password'] != info['confirm']:
-                self.plain_response("Error: password and confirm don't match")
+                self.fail("password and confirm don't match",
+                    heading='User registration')
                 return
             info['otk'] = self.store.store_user(name, info['password'],
                 info['email'])
@@ -568,8 +614,7 @@ email.</p>
             email = info.get('email', user['email'])
             self.store.store_user(self.username, password, email)
             response = 'Details updated OK'
-
-        self.plain_response(response)
+        self.success(message=response, heading='User registration')
 
     def password_reset(self):
         ''' Reset the user's password and send an email to the address given.
@@ -577,15 +622,14 @@ email.</p>
         email = self.form['email'].value
         user = self.store.get_user_by_email(email)
         if user is None:
-            response = 'Error: email address unknown to me'
-        else:
-            pw = ''.join([whrandom.choice(chars) for x in range(10)])
-            self.store.store_user(user['name'], pw, user['email'])
-            info = {'name': user['name'], 'password': pw,
-                'email':user['email']}
-            self.send_email(email, password_message%info)
-            response = 'Email sent OK'
-        self.plain_response(response)
+            self.error('email address unknown to me')
+            return
+        pw = ''.join([whrandom.choice(chars) for x in range(10)])
+        self.store.store_user(user['name'], pw, user['email'])
+        info = {'name': user['name'], 'password': pw,
+            'email':user['email']}
+        self.send_email(email, password_message%info)
+        self.success(message='Email sent OK')
 
     def send_email(self, recipient, message):
         ''' Send an administrative email to the recipient