X-Git-Url: https://git.tokkee.org/?a=blobdiff_plain;f=roundup%2Fcgi%2Fclient.py;h=8ac665d32243a1b2dc0c0a68035e5118e3154179;hb=2adbb9799be27db1b6a6ad9c1355584533f7938c;hp=dc48ff61b9e0e311184fd44122f604e1a8a96622;hpb=8769973eccd4d10bd91d46b126c4ba825c9c362d;p=roundup.git diff --git a/roundup/cgi/client.py b/roundup/cgi/client.py index dc48ff6..8ac665d 100644 --- a/roundup/cgi/client.py +++ b/roundup/cgi/client.py @@ -1,4 +1,4 @@ -# $Id: client.py,v 1.13 2002-09-05 04:46:36 richard Exp $ +# $Id: client.py,v 1.52 2002-10-09 01:00:40 richard Exp $ __doc__ = """ WWW request handler (also used in the stand-alone server). @@ -10,10 +10,10 @@ import binascii, Cookie, time, random from roundup import roundupdb, date, hyperdb, password from roundup.i18n import _ -from roundup.cgi.templating import getTemplate, HTMLRequest +from roundup.cgi.templating import Templates, HTMLRequest, NoTemplate from roundup.cgi import cgitb -from PageTemplates import PageTemplate +from roundup.cgi.PageTemplates import PageTemplate class Unauthorised(ValueError): pass @@ -48,24 +48,36 @@ def initialiseSecurity(security): security.addPermissionToRole('Admin', p) class Client: - ''' - A note about login - ------------------ - - If the user has no login cookie, then they are anonymous. There - are two levels of anonymous use. If there is no 'anonymous' user, there - is no login at all and the database is opened in read-only mode. If the - 'anonymous' user exists, the user is logged in using that user (though - there is no cookie). This allows them to modify the database, and all - modifications are attributed to the 'anonymous' user. - - Once a user logs in, they are assigned a session. The Client instance - keeps the nodeid of the session as the "session" attribute. - - Client attributes: - "url" is the current url path - "path" is the PATH_INFO inside the instance + ''' Instantiate to handle one CGI request. + + See inner_main for request processing. + + Client attributes at instantiation: + "path" is the PATH_INFO inside the instance (with no leading '/') "base" is the base URL for the instance + "form" is the cgi form, an instance of FieldStorage from the standard + cgi module + "additional_headers" is a dictionary of additional HTTP headers that + should be sent to the client + "response_code" is the HTTP response code to send to the client + + During the processing of a request, the following attributes are used: + "error_message" holds a list of error messages + "ok_message" holds a list of OK messages + "session" is the current user session id + "user" is the current user's name + "userid" is the current user's id + "template" is the current :template context + "classname" is the current class context name + "nodeid" is the current context item id + + User Identification: + If the user has no login cookie, then they are anonymous and are logged + in as that user. This typically gives them all Permissions assigned to the + Anonymous Role. + + Once a user logs in, they are assigned a session. The Client instance + keeps the nodeid of the session as the "session" attribute. ''' def __init__(self, instance, request, env, form=None): @@ -74,35 +86,67 @@ class Client: self.request = request self.env = env + # save off the path self.path = env['PATH_INFO'] - self.split_path = self.path.split('/') - self.instance_path_name = env['INSTANCE_NAME'] # this is the base URL for this instance - url = self.env['SCRIPT_NAME'] + '/' + self.instance_path_name - self.base = urlparse.urlunparse(('http', env['HTTP_HOST'], url, - None, None, None)) - - # request.path is the full request path - x, x, path, x, x, x = urlparse.urlparse(request.path) - self.url = urlparse.urlunparse(('http', env['HTTP_HOST'], path, - None, None, None)) + self.base = self.instance.config.TRACKER_WEB + # see if we need to re-parse the environment for the form (eg Zope) if form is None: self.form = cgi.FieldStorage(environ=env) else: self.form = form - self.headers_done = 0 + + # turn debugging on/off try: self.debug = int(env.get("ROUNDUP_DEBUG", 0)) except ValueError: # someone gave us a non-int debug level, turn it off self.debug = 0 + # flag to indicate that the HTTP headers have been sent + self.headers_done = 0 + + # additional headers to send with the request - must be registered + # before the first write + self.additional_headers = {} + self.response_code = 200 + def main(self): - ''' Wrap the request and handle unauthorised requests + ''' Wrap the real main in a try/finally so we always close off the db. + ''' + try: + self.inner_main() + finally: + if hasattr(self, 'db'): + self.db.close() + + def inner_main(self): + ''' Process a request. + + The most common requests are handled like so: + 1. figure out who we are, defaulting to the "anonymous" user + see determine_user + 2. figure out what the request is for - the context + see determine_context + 3. handle any requested action (item edit, search, ...) + see handle_action + 4. render a template, resulting in HTML output + + In some situations, exceptions occur: + - HTTP Redirect (generally raised by an action) + - SendFile (generally raised by determine_context) + serve up a FileClass "content" property + - SendStaticFile (generally raised by determine_context) + serve up a file from the tracker "html" directory + - Unauthorised (generally raised by an action) + the action is cancelled, the request is rendered and an error + message is displayed indicating that permission was not + granted for the action to take place + - NotFound (raised wherever it needs to be) + percolates up to the CGI interface that called the client ''' - self.content_action = None self.ok_message = [] self.error_message = [] try: @@ -110,30 +154,38 @@ class Client: self.determine_user() # figure out the context and desired content template self.determine_context() - # possibly handle a form submit action (may change self.message, - # self.classname and self.template) + # possibly handle a form submit action (may change self.classname + # and self.template, and may also append error/ok_messages) self.handle_action() # now render the page - if self.form.has_key(':contentonly'): - # just the content - self.write(self.content()) - else: - # render the content inside the page template - self.write(self.renderTemplate('page', '', - ok_message=self.ok_message, - error_message=self.error_message)) + + # we don't want clients caching our dynamic pages + self.additional_headers['Cache-Control'] = 'no-cache' + self.additional_headers['Pragma'] = 'no-cache' + self.additional_headers['Expires'] = 'Thu, 1 Jan 1970 00:00:00 GMT' + + # render the content + self.write(self.renderContext()) except Redirect, url: # let's redirect - if the url isn't None, then we need to do # the headers, otherwise the headers have been set before the # exception was raised if url: - self.header({'Location': url}, response=302) + self.additional_headers['Location'] = url + self.response_code = 302 + self.write('Redirecting to %s'%(url, url)) except SendFile, designator: self.serve_file(designator) except SendStaticFile, file: self.serve_static_file(str(file)) except Unauthorised, message: - self.write(self.renderTemplate('page', '', error_message=message)) + self.classname=None + self.template='' + self.error_message.append(message) + self.write(self.renderContext()) + except NotFound: + # pass through + raise except: # everything else self.write(cgitb.html()) @@ -233,7 +285,7 @@ class Client: self.nodeid = None # determine the classname and possibly nodeid - path = self.split_path + path = self.path.split('/') if not path or path[0] in ('', 'home', 'index'): if self.form.has_key(':template'): self.template = self.form[':template'].value @@ -253,6 +305,8 @@ class Client: if m: self.classname = m.group(1) self.nodeid = m.group(2) + if not self.db.getclass(self.classname).hasnode(self.nodeid): + raise NotFound, '%s/%s'%(self.classname, self.nodeid) # with a designator, we default to item view self.template = 'item' else: @@ -263,7 +317,6 @@ class Client: if self.form.has_key(':template'): self.template = self.form[':template'].value - # see if we were passed in a message if self.form.has_key(':ok_message'): self.ok_message.append(self.form[':ok_message'].value) @@ -282,66 +335,57 @@ class Client: # we just want to serve up the file named file = self.db.file - self.header({'Content-Type': file.get(nodeid, 'type')}) + self.additional_headers['Content-Type'] = file.get(nodeid, 'type') self.write(file.get(nodeid, 'content')) def serve_static_file(self, file): # we just want to serve up the file named mt = mimetypes.guess_type(str(file))[0] - self.header({'Content-Type': mt}) - self.write(open(os.path.join(self.instance.TEMPLATES, file)).read()) + self.additional_headers['Content-Type'] = mt + self.write(open(os.path.join(self.instance.config.TEMPLATES, + file)).read()) - def renderTemplate(self, name, extension, **kwargs): + def renderContext(self): ''' Return a PageTemplate for the named page ''' - pt = getTemplate(self.instance.TEMPLATES, name, extension) - # XXX handle PT rendering errors here more nicely + name = self.classname + extension = self.template + pt = Templates(self.instance.config.TEMPLATES).get(name, extension) + + # catch errors so we can handle PT rendering errors more nicely + args = { + 'ok_message': self.ok_message, + 'error_message': self.error_message + } try: # let the template render figure stuff out - return pt.render(self, None, None, **kwargs) - except PageTemplate.PTRuntimeError, message: - return '%s
    %s
'%(message, - '
  • '.join(pt._v_errors)) + return pt.render(self, None, None, **args) + except NoTemplate, message: + return '%s'%message except: # everything else - return cgitb.html() - - def content(self): - ''' Callback used by the page template to render the content of - the page. - - If we don't have a specific class to display, that is none was - determined in determine_context(), then we display a "home" - template. - ''' - # now render the page content using the template we determined in - # determine_context - if self.classname is None: - name = 'home' - else: - name = self.classname - return self.renderTemplate(self.classname, self.template) + return cgitb.pt_html() # these are the actions that are available - actions = { - 'edit': 'editItemAction', - 'editCSV': 'editCSVAction', - 'new': 'newItemAction', - 'register': 'registerAction', - 'login': 'login_action', - 'logout': 'logout_action', - 'search': 'searchAction', - } + actions = ( + ('edit', 'editItemAction'), + ('editCSV', 'editCSVAction'), + ('new', 'newItemAction'), + ('register', 'registerAction'), + ('login', 'loginAction'), + ('logout', 'logout_action'), + ('search', 'searchAction'), + ) def handle_action(self): ''' Determine whether there should be an _action called. The action is defined by the form variable :action which identifies the method on this object to call. The four basic - actions are defined in the "actions" dictionary on this class: + actions are defined in the "actions" sequence on this class: "edit" -> self.editItemAction "new" -> self.newItemAction "register" -> self.registerAction - "login" -> self.login_action + "login" -> self.loginAction "logout" -> self.logout_action "search" -> self.searchAction @@ -351,13 +395,18 @@ class Client: try: # get the action, validate it action = self.form[':action'].value - if not self.actions.has_key(action): + for name, method in self.actions: + if name == action: + break + else: raise ValueError, 'No such action "%s"'%action # call the mapped action - getattr(self, self.actions[action])() + getattr(self, method)() except Redirect: raise + except Unauthorised: + raise except: self.db.rollback() s = StringIO.StringIO() @@ -369,11 +418,17 @@ class Client: self.header() self.request.wfile.write(content) - def header(self, headers=None, response=200): + def header(self, headers=None, response=None): '''Put up the appropriate header. ''' if headers is None: headers = {'Content-Type':'text/html'} + if response is None: + response = self.response_code + + # update with additional info + headers.update(self.additional_headers) + if not headers.has_key('Content-Type'): headers['Content-Type'] = 'text/html' self.request.send_response(response) @@ -384,9 +439,12 @@ class Client: if self.debug: self.headers_sent = headers - def set_cookie(self, user, password): + def set_cookie(self, user): + ''' Set up a session cookie for the user and store away the user's + login info against the session. + ''' # TODO generate a much, much stronger session key ;) - self.session = binascii.b2a_base64(repr(time.time())).strip() + self.session = binascii.b2a_base64(repr(random.random())).strip() # clean up the base64 if self.session[-1] == '=': @@ -405,10 +463,10 @@ class Client: expire = Cookie._getdate(86400*365) # generate the cookie path - make sure it has a trailing '/' - path = '/'.join((self.env['SCRIPT_NAME'], self.env['INSTANCE_NAME'], + path = '/'.join((self.env['SCRIPT_NAME'], self.env['TRACKER_NAME'], '')) - self.header({'Set-Cookie': 'roundup_user_2=%s; expires=%s; Path=%s;'%( - self.session, expire, path)}) + self.additional_headers['Set-Cookie'] = \ + 'roundup_user_2=%s; expires=%s; Path=%s;'%(self.session, expire, path) def make_user_anonymous(self): ''' Make us anonymous @@ -419,63 +477,81 @@ class Client: self.userid = self.db.user.lookup('anonymous') self.user = 'anonymous' - def logout(self): - ''' Make us really anonymous - nuke the cookie too - ''' - self.make_user_anonymous() - - # construct the logout cookie - now = Cookie._getdate() - path = '/'.join((self.env['SCRIPT_NAME'], self.env['INSTANCE_NAME'], - '')) - self.header({'Set-Cookie': - 'roundup_user_2=deleted; Max-Age=0; expires=%s; Path=%s;'%(now, - path)}) - self.login() - def opendb(self, user): ''' Open the database. ''' # open the db if the user has changed if not hasattr(self, 'db') or user != self.db.journaltag: + if hasattr(self, 'db'): + self.db.close() self.db = self.instance.open(user) # # Actions # - def login_action(self): - ''' Attempt to log a user in and set the cookie + def loginAction(self): + ''' Attempt to log a user in. + + Sets up a session for the user which contains the login + credentials. ''' # we need the username at a minimum if not self.form.has_key('__login_name'): self.error_message.append(_('Username required')) return + # get the login info self.user = self.form['__login_name'].value - # re-open the database for real, using the user - self.opendb(self.user) if self.form.has_key('__login_password'): password = self.form['__login_password'].value else: password = '' + # make sure the user exists try: self.userid = self.db.user.lookup(self.user) except KeyError: name = self.user - self.make_user_anonymous() self.error_message.append(_('No such user "%(name)s"')%locals()) + self.make_user_anonymous() return - # and that the password is correct - pw = self.db.user.get(self.userid, 'password') - if password != pw: + # verify the password + if not self.verifyPassword(self.userid, password): self.make_user_anonymous() self.error_message.append(_('Incorrect password')) return + # make sure we're allowed to be here + if not self.loginPermission(): + self.make_user_anonymous() + self.error_message.append(_("You do not have permission to login")) + return + + # now we're OK, re-open the database for real, using the user + self.opendb(self.user) + # set the session cookie - self.set_cookie(self.user, password) + self.set_cookie(self.user) + + def verifyPassword(self, userid, password): + ''' Verify the password that the user has supplied + ''' + stored = self.db.user.get(self.userid, 'password') + if password == stored: + return 1 + if not password and not stored: + return 1 + return 0 + + def loginPermission(self): + ''' Determine whether the user has permission to log in. + + Base behaviour is to check the user has "Web Access". + ''' + if not self.db.security.hasPermission('Web Access', self.userid): + return 0 + return 1 def logout_action(self): ''' Make us really anonymous - nuke the cookie too @@ -485,10 +561,10 @@ class Client: # construct the logout cookie now = Cookie._getdate() - path = '/'.join((self.env['SCRIPT_NAME'], self.env['INSTANCE_NAME'], + path = '/'.join((self.env['SCRIPT_NAME'], self.env['TRACKER_NAME'], '')) - self.header(headers={'Set-Cookie': - 'roundup_user_2=deleted; Max-Age=0; expires=%s; Path=%s;'%(now, path)}) + self.additional_headers['Set-Cookie'] = \ + 'roundup_user_2=deleted; Max-Age=0; expires=%s; Path=%s;'%(now, path) # Let the user know what's going on self.ok_message.append(_('You are logged out')) @@ -521,21 +597,32 @@ class Client: cl = self.db.user try: props = parsePropsFromForm(self.db, cl, self.form) - props['roles'] = self.instance.NEW_WEB_USER_ROLES + props['roles'] = self.instance.config.NEW_WEB_USER_ROLES self.userid = cl.create(**props) self.db.commit() - except ValueError, message: + except (ValueError, KeyError), message: self.error_message.append(message) + return # log the new user in self.user = cl.get(self.userid, 'username') # re-open the database for real, using the user self.opendb(self.user) - password = self.db.user.get(self.userid, 'password') - self.set_cookie(self.user, password) + + # if we have a session, update it + if hasattr(self, 'session'): + self.db.sessions.set(self.session, user=self.user, + last_use=time.time()) + else: + # new session cookie + self.set_cookie(self.user) # nice message - self.ok_message.append(_('You are now registered, welcome!')) + message = _('You are now registered, welcome!') + + # redirect to the item's edit page + raise Redirect, '%s%s%s?:ok_message=%s'%( + self.base, self.classname, self.userid, urllib.quote(message)) def registerPermission(self, props): ''' Determine whether the user has permission to register @@ -558,13 +645,17 @@ class Client: :multilink=designator:property The value specifies a node designator and the property on that node to add _this_ node to as a link or multilink. - __note + :note Create a message and attach it to the current node's "messages" property. - __file + :file Create a file and attach it to the current node's "files" property. Attach the file to the message created from - the __note if it's supplied. + the :note if it's supplied. + + :required=property,property,... + The named properties are required to be filled in the form. + ''' cl = self.db.classes[self.classname] @@ -599,15 +690,15 @@ class Client: if props: message = _('%(changes)s edited ok')%{'changes': ', '.join(props.keys())} - elif self.form.has_key('__note') and self.form['__note'].value: + elif self.form.has_key(':note') and self.form[':note'].value: message = _('note added') - elif (self.form.has_key('__file') and self.form['__file'].filename): + elif (self.form.has_key(':file') and self.form[':file'].filename): message = _('file added') else: message = _('nothing changed') # redirect to the item's edit page - raise Redirect, '%s/%s%s?:ok_message=%s'%(self.base, self.classname, + raise Redirect, '%s%s%s?:ok_message=%s'%(self.base, self.classname, self.nodeid, urllib.quote(message)) def editItemPermission(self, props): @@ -637,7 +728,8 @@ class Client: def newItemAction(self): ''' Add a new item to the database. - This follows the same form as the editItemAction + This follows the same form as the editItemAction, with the same + special form values. ''' cl = self.db.classes[self.classname] @@ -691,7 +783,7 @@ class Client: return # redirect to the new item's page - raise Redirect, '%s/%s%s?:ok_message=%s'%(self.base, self.classname, + raise Redirect, '%s%s%s?:ok_message=%s'%(self.base, self.classname, nid, urllib.quote(message)) def newItemPermission(self, props): @@ -847,7 +939,6 @@ class Client: # commit the query change to the database self.db.commit() - def searchPermission(self): ''' Determine whether the user has permission to search this class. @@ -858,10 +949,9 @@ class Client: return 0 return 1 - def XXXremove_action(self, dre=re.compile(r'([^\d]+)(\d+)')): + def remove_action(self, dre=re.compile(r'([^\d]+)(\d+)')): # XXX I believe this could be handled by a regular edit action that # just sets the multilink... - # XXX handle this ! target = self.index_arg(':target')[0] m = dre.match(target) if m: @@ -924,8 +1014,8 @@ class Client: ''' # handle file attachments files = [] - if self.form.has_key('__file'): - file = self.form['__file'] + if self.form.has_key(':file'): + file = self.form[':file'] if file.filename: filename = file.filename.split('\\')[-1] mime_type = mimetypes.guess_type(filename)[0] @@ -942,8 +1032,9 @@ class Client: note = None # in a nutshell, don't do anything if there's no note or there's no # NOSY - if self.form.has_key('__note'): - note = self.form['__note'].value.strip() + if self.form.has_key(':note'): + # fix the CRLF/CR -> LF stuff + note = fixNewlines(self.form[':note'].value.strip()) if not note: return None, files if not props.has_key('messages'): @@ -965,7 +1056,7 @@ class Client: # handle the messageid # TODO: handle inreplyto messageid = "<%s.%s.%s@%s>"%(time.time(), random.random(), - self.classname, self.instance.MAIL_DOMAIN) + self.classname, self.instance.config.MAIL_DOMAIN) # now create the message, attaching the files content = '\n'.join(m) @@ -987,7 +1078,7 @@ class Client: which issue to link the file to. TODO: I suspect that this and newfile will go away now that - there's the ability to upload a file using the issue __file form + there's the ability to upload a file using the issue :file form element! ''' cn = self.classname @@ -1015,44 +1106,86 @@ class Client: link = self.db.classes[link] link.set(nodeid, **{property: nid}) +def fixNewlines(text): + ''' Homogenise line endings. + + Different web clients send different line ending values, but + other systems (eg. email) don't necessarily handle those line + endings. Our solution is to convert all line endings to LF. + ''' + text = text.replace('\r\n', '\n') + return text.replace('\r', '\n') def parsePropsFromForm(db, cl, form, nodeid=0, num_re=re.compile('^\d+$')): - '''Pull properties for the given class out of the form. + ''' Pull properties for the given class out of the form. + + If a ":required" parameter is supplied, then the names property values + must be supplied or a ValueError will be raised. ''' + required = [] + if form.has_key(':required'): + value = form[':required'] + if isinstance(value, type([])): + required = [i.value.strip() for i in value] + else: + required = [i.strip() for i in value.value.split(',')] + props = {} keys = form.keys() + properties = cl.getprops() for key in keys: - if not cl.properties.has_key(key): + if not properties.has_key(key): continue - proptype = cl.properties[key] + proptype = properties[key] + + # Get the form value. This value may be a MiniFieldStorage or a list + # of MiniFieldStorages. + value = form[key] + + # make sure non-multilinks only get one value + if not isinstance(proptype, hyperdb.Multilink): + if isinstance(value, type([])): + raise ValueError, 'You have submitted more than one value'\ + ' for the %s property'%key + # we've got a MiniFieldStorage, so pull out the value and strip + # surrounding whitespace + value = value.value.strip() + if isinstance(proptype, hyperdb.String): - value = form[key].value.strip() + if not value: + continue + # fix the CRLF/CR -> LF stuff + value = fixNewlines(value) elif isinstance(proptype, hyperdb.Password): - value = form[key].value.strip() if not value: # ignore empty password values continue + if not form.has_key('%s:confirm'%key): + raise ValueError, 'Password and confirmation text do not match' + confirm = form['%s:confirm'%key] + if isinstance(confirm, type([])): + raise ValueError, 'You have submitted more than one value'\ + ' for the %s property'%key + if value != confirm.value: + raise ValueError, 'Password and confirmation text do not match' value = password.Password(value) elif isinstance(proptype, hyperdb.Date): - value = form[key].value.strip() if value: value = date.Date(form[key].value.strip()) else: - value = None + continue elif isinstance(proptype, hyperdb.Interval): - value = form[key].value.strip() if value: value = date.Interval(form[key].value.strip()) else: - value = None + continue elif isinstance(proptype, hyperdb.Link): - value = form[key].value.strip() # see if it's the "no selection" choice if value == '-1': value = None else: # handle key values - link = cl.properties[key].classname + link = proptype.classname if not num_re.match(value): try: value = db.classes[link].lookup(value) @@ -1060,13 +1193,19 @@ def parsePropsFromForm(db, cl, form, nodeid=0, num_re=re.compile('^\d+$')): raise ValueError, _('property "%(propname)s": ' '%(value)s not a %(classname)s')%{'propname':key, 'value': value, 'classname': link} + except TypeError, message: + raise ValueError, _('you may only enter ID values ' + 'for property "%(propname)s": %(message)s')%{ + 'propname':key, 'message': message} elif isinstance(proptype, hyperdb.Multilink): - value = form[key] - if not isinstance(value, type([])): - value = [i.strip() for i in value.value.split(',')] - else: + if isinstance(value, type([])): + # it's a list of MiniFieldStorages value = [i.value.strip() for i in value] - link = cl.properties[key].classname + else: + # it's a MiniFieldStorage, but may be a comma-separated list + # of values + value = [i.strip() for i in value.value.split(',')] + link = proptype.classname l = [] for entry in map(str, value): if entry == '': continue @@ -1077,16 +1216,22 @@ def parsePropsFromForm(db, cl, form, nodeid=0, num_re=re.compile('^\d+$')): raise ValueError, _('property "%(propname)s": ' '"%(value)s" not an entry of %(classname)s')%{ 'propname':key, 'value': entry, 'classname': link} + except TypeError, message: + raise ValueError, _('you may only enter ID values ' + 'for property "%(propname)s": %(message)s')%{ + 'propname':key, 'message': message} l.append(entry) l.sort() value = l elif isinstance(proptype, hyperdb.Boolean): - value = form[key].value.strip() props[key] = value = value.lower() in ('yes', 'true', 'on', '1') elif isinstance(proptype, hyperdb.Number): - value = form[key].value.strip() props[key] = value = int(value) + # register this as received if required? + if key in required and value is not None: + required.remove(key) + # get the old value if nodeid: try: @@ -1094,13 +1239,22 @@ def parsePropsFromForm(db, cl, form, nodeid=0, num_re=re.compile('^\d+$')): except KeyError: # this might be a new property for which there is no existing # value - if not cl.properties.has_key(key): raise + if not properties.has_key(key): raise # if changed, set it if value != existing: props[key] = value else: props[key] = value + + # see if all the required properties have been supplied + if required: + if len(required) > 1: + p = 'properties' + else: + p = 'property' + raise ValueError, 'Required %s %s not supplied'%(p, ', '.join(required)) + return props