Mercurial > libervia-pubsub
changeset 159:6fe78048baf9
Rework error handling, depend on Twisted Words 0.4.0.
Twisted Words 0.4.0 introduced support for stanza error handling, much better
than the custom error handling in Idavoll. Also, all protocol-level errors
were examined and brought up to date with version 1.8 of JEP-0060.
As a result of the error examination, the retrieval of default configuration
options using <default/> is now supported properly.
| author | Ralph Meijer <ralphm@ik.nu> |
|---|---|
| date | Wed, 06 Sep 2006 12:38:47 +0000 |
| parents | b2149e448465 |
| children | d183e48a5049 |
| files | INSTALL idavoll/backend.py idavoll/generic_backend.py idavoll/idavoll.py idavoll/implementation_issues.txt idavoll/pubsub.py idavoll/xmpp_error.py |
| diffstat | 7 files changed, 109 insertions(+), 169 deletions(-) [+] |
line wrap: on
line diff
--- a/INSTALL Thu Jun 18 11:52:01 2009 +0000 +++ b/INSTALL Wed Sep 06 12:38:47 2006 +0000 @@ -2,7 +2,7 @@ ============ - Twisted Core >= 2.0.0 -- Twisted Words >= 0.3.0 +- Twisted Words >= 0.4.0 - uuid.py (http://ofxsuite.berlios.de/uuid.py) - A jabber server that supports the component protocol (JEP-0114)
--- a/idavoll/backend.py Thu Jun 18 11:52:01 2009 +0000 +++ b/idavoll/backend.py Wed Sep 06 12:38:47 2006 +0000 @@ -10,21 +10,18 @@ def __str__(self): return self.msg -class NotAuthorized(Error): +class Forbidden(Error): pass -class PayloadExpected(Error): - msg = 'Payload expected' +class ItemForbidden(Error): + pass -class NoPayloadAllowed(Error): - msg = 'No payload allowed' +class ItemRequired(Error): + pass class NoInstantNodes(Error): pass -class NotImplemented(Error): - pass - class NotSubscribed(Error): pass @@ -34,6 +31,12 @@ class InvalidConfigurationValue(Error): msg = 'Bad configuration value' +class NodeNotPersistent(Error): + pass + +class NoRootNode(Error): + pass + class IBackendService(Interface): """ Interface to a backend service of a pubsub service. """
--- a/idavoll/generic_backend.py Thu Jun 18 11:52:01 2009 +0000 +++ b/idavoll/generic_backend.py Wed Sep 06 12:38:47 2006 +0000 @@ -78,7 +78,7 @@ def _check_auth(self, node, requestor): def check(affiliation, node): if affiliation not in ['owner', 'publisher']: - raise backend.NotAuthorized + raise backend.Forbidden return node d = node.get_affiliation(requestor) @@ -97,9 +97,9 @@ deliver_payloads = configuration["pubsub#deliver_payloads"] if items and not persist_items and not deliver_payloads: - raise backend.NoPayloadAllowed + raise backend.ItemForbidden elif not items and (persist_items or deliver_payloads): - raise backend.PayloadExpected + raise backend.ItemRequired if persist_items or deliver_payloads: for item in items: @@ -149,7 +149,7 @@ def subscribe(self, node_id, subscriber, requestor): subscriber_entity = subscriber.userhostJID() if subscriber_entity != requestor: - return defer.fail(backend.NotAuthorized()) + return defer.fail(backend.Forbidden()) d = self.parent.storage.get_node(node_id) d.addCallback(_get_affiliation, subscriber_entity) @@ -160,7 +160,7 @@ node, affiliation = result if affiliation == 'outcast': - raise backend.NotAuthorized + raise backend.Forbidden d = node.add_subscription(subscriber, 'subscribed') d.addCallback(lambda _: 'subscribed') @@ -177,7 +177,7 @@ def unsubscribe(self, node_id, subscriber, requestor): if subscriber.userhostJID() != requestor: - raise backend.NotAuthorized + return defer.fail(backend.Forbidden()) d = self.parent.storage.get_node(node_id) d.addCallback(lambda node: node.remove_subscription(subscriber)) @@ -200,13 +200,17 @@ d.addCallback(lambda _: node_id) return d + def get_default_configuration(self): + d = defer.succeed(self.parent.default_config) + d.addCallback(self._make_config) + return d + def get_node_configuration(self, node_id): - if node_id: - d = self.parent.storage.get_node(node_id) - d.addCallback(lambda node: node.get_configuration()) - else: - # XXX: this is disabled in pubsub.py - d = defer.succeed(self.parent.default_config) + if not node_id: + raise backend.NoRootNode + + d = self.parent.storage.get_node(node_id) + d.addCallback(lambda node: node.get_configuration()) d.addCallback(self._make_config) return d @@ -223,6 +227,9 @@ return options def set_node_configuration(self, node_id, options, requestor): + if not node_id: + raise backend.NoRootNode + for key, value in options.iteritems(): if not self.parent.options.has_key(key): raise backend.InvalidConfigurationOption @@ -241,7 +248,7 @@ node, affiliation = result if affiliation != 'owner': - raise backend.NotAuthorized + raise backend.Forbidden return node.set_configuration(options) @@ -271,7 +278,7 @@ node, subscribed = result if not subscribed: - raise backend.NotAuthorized + raise backend.NotSubscribed if item_ids: return node.get_items_by_id(item_ids) @@ -293,7 +300,7 @@ persist_items = node.get_configuration()["pubsub#persist_items"] if affiliation not in ['owner', 'publisher']: - raise backend.NotAuthorized + raise backend.Forbidden if not persist_items: raise backend.NodeNotPersistent @@ -317,7 +324,7 @@ persist_items = node.get_configuration()["pubsub#persist_items"] if affiliation != 'owner': - raise backend.NotAuthorized + raise backend.Forbidden if not persist_items: raise backend.NodeNotPersistent @@ -354,7 +361,7 @@ node, affiliation = result if affiliation != 'owner': - raise backend.NotAuthorized + raise backend.Forbidden d = defer.DeferredList([cb(node.id) for cb in self._callback_list], consumeErrors=1)
--- a/idavoll/idavoll.py Thu Jun 18 11:52:01 2009 +0000 +++ b/idavoll/idavoll.py Wed Sep 06 12:38:47 2006 +0000 @@ -1,12 +1,11 @@ # Copyright (c) 2003-2006 Ralph Meijer # See LICENSE for details. -from twisted.words.protocols.jabber import component +from twisted.words.protocols.jabber import component, error from twisted.application import service from twisted.internet import defer import backend import pubsub -import xmpp_error import disco import sys @@ -69,7 +68,7 @@ info.extend(i[1]) if node and not info: - return xmpp_error.error_from_iq(iq, 'item-not-found') + return error.StanzaError('item-not-found').toResponse(iq) else: iq.swapAttributeValues("to", "from") iq["type"] = "result" @@ -85,7 +84,7 @@ def _error(self, result, iq): print "Got error on index %d:" % result.value[1] result.value[0].printBriefTraceback() - return xmpp_error.error_from_iq(iq, 'internal-server-error') + return error.StanzaError('internal-server-error').toResponse(iq) def onDiscoItems(self, iq): dl = [] @@ -118,7 +117,7 @@ if iq.handled == True: return - self.send(xmpp_error.error_from_iq(iq, 'service-unavailable')) + self.send(error.StanzaError('service-unavailable').toResponse(iq)) class LogService(component.Service):
--- a/idavoll/implementation_issues.txt Thu Jun 18 11:52:01 2009 +0000 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,49 +0,0 @@ -- What happens when a node creation request is accompanied by a configure - node, but node configuration is not implemented? - - Solution: - - Reply with a not-acceptable stanza error, with a node-not-configurable - error attached. - -- Node purging with many items in storage and notification of retraction enabled -could produce a large number of notifications. Suggestion: instead send a -<event><purge/></event> - -- Examples 58 and 62 are the same - -- What error to return for outcast user requests? - - What if some admin requested it? - -- Why not return forbidden when JIDs do not match on subscription? - -- What to do when receiving an unsubscription request with a subid, when - subids are not supported? Ignore or reply with bad-request? - -- unsubscription: why check for the requestor being subscribed. You want to - check if the jid is subscribed. An not-authorized can be sent if the - requestor and jid don't match. Otherwise, unexpected-request might - be appropriate when the jid is not subscribed along with a special - child element of not-subscribed in the pubsub#error namespace. - -- 8.1.7 states: - If subscription identifiers are supported by the service and the entity's - subscription is not NONE or OUTCAST, the 'subid' attribute MUST be present - as well as the 'jid' attribute. - There is no such thing as a subscription of 'OUTCAST'. Also, I'm not sure - why NONE is capitalized. - -- On meta-data: - http://mail.jabber.org/pipermail/standards-jig/2004-July/005783.html - Assumed meta-data as per JEP-0128 is indeed optional, advertised with the - pubsub#meta-data disco feature. Also, how do you actually set this meta-data - information? - -- on items attribute max_items. xs:string?? - -- no disco feature for retracting items, purging nodes, deleting nodes, other - owner use cases? - -- no pubsub#error for bad node configuration options? - -- configure in pubsub#owner namespace??
--- a/idavoll/pubsub.py Thu Jun 18 11:52:01 2009 +0000 +++ b/idavoll/pubsub.py Wed Sep 06 12:38:47 2006 +0000 @@ -1,7 +1,7 @@ # Copyright (c) 2003-2006 Ralph Meijer # See LICENSE for details. -from twisted.words.protocols.jabber import component,jid +from twisted.words.protocols.jabber import component, jid, error from twisted.words.xish import utility, domish from twisted.python import components from twisted.internet import defer @@ -9,7 +9,6 @@ import backend import storage -import xmpp_error import disco import data_form @@ -40,6 +39,7 @@ PUBSUB_UNSUBSCRIBE = PUBSUB_SET + '/unsubscribe' PUBSUB_OPTIONS_GET = PUBSUB_GET + '/options' PUBSUB_OPTIONS_SET = PUBSUB_SET + '/options' +PUBSUB_DEFAULT = PUBSUB_OWNER_GET + '/default' PUBSUB_CONFIGURE_GET = PUBSUB_OWNER_GET + '/configure' PUBSUB_CONFIGURE_SET = PUBSUB_OWNER_SET + '/configure' PUBSUB_SUBSCRIPTIONS = PUBSUB_GET + '/subscriptions' @@ -49,37 +49,39 @@ PUBSUB_PURGE = PUBSUB_OWNER_SET + '/purge' PUBSUB_DELETE = PUBSUB_OWNER_SET + '/delete' -class Error(Exception): - pubsub_error = None - stanza_error = None - msg = '' - -class NotImplemented(Error): - stanza_error = 'feature-not-implemented' +class BadRequest(error.StanzaError): + def __init__(self): + error.StanzaError.__init__(self, 'bad-request') -class BadRequest(Error): - stanza_error = 'bad-request' +class PubSubError(error.StanzaError): + def __init__(self, condition, pubsubCondition, feature=None, text=None): + appCondition = domish.Element((NS_PUBSUB_ERRORS, pubsubCondition)) + if feature: + appCondition['feature'] = feature + error.StanzaError.__init__(self, condition, + text=text, + appCondition=appCondition) -class OptionsUnavailable(Error): - stanza_error = 'feature-not-implemented' - pubsub_error = 'subscription-options-unavailable' - -class NodeNotConfigurable(Error): - stanza_error = 'feature-not-implemented' - pubsub_error = 'node-not-configurable' +class OptionsUnavailable(PubSubError): + def __init__(self): + PubSubError.__init__(self, 'feature-not-implemented', + 'unsupported', + 'subscription-options-unavailable') error_map = { - storage.NodeNotFound: ('item-not-found', None), - storage.NodeExists: ('conflict', None), - storage.SubscriptionNotFound: ('not-authorized', - 'not-subscribed'), - backend.NotAuthorized: ('not-authorized', None), - backend.NoPayloadAllowed: ('bad-request', None), - backend.PayloadExpected: ('bad-request', None), - backend.NoInstantNodes: ('not-acceptable', None), - backend.NotImplemented: ('feature-not-implemented', None), - backend.InvalidConfigurationOption: ('not-acceptable', None), - backend.InvalidConfigurationValue: ('not-acceptable', None), + storage.NodeNotFound: ('item-not-found', None, None), + storage.NodeExists: ('conflict', None, None), + storage.SubscriptionNotFound: ('not-authorized', 'not-subscribed', None), + backend.Forbidden: ('forbidden', None, None), + backend.ItemForbidden: ('bad-request', 'item-forbidden', None), + backend.ItemRequired: ('bad-request', 'item-required', None), + backend.NoInstantNodes: ('not-acceptable', 'unsupported', 'instant-nodes'), + backend.NotSubscribed: ('not-authorized', 'not-subscribed', None), + backend.InvalidConfigurationOption: ('not-acceptable', None, None), + backend.InvalidConfigurationValue: ('not-acceptable', None, None), + backend.NodeNotPersistent: ('feature-not-implemented', 'unsupported', + 'persistent-node'), + backend.NoRootNode: ('bad-request', None, None), } class Service(component.Service): @@ -91,24 +93,23 @@ def error(self, failure, iq): try: - e = failure.trap(Error, *error_map.keys()) + e = failure.trap(error.StanzaError, *error_map.keys()) except: failure.printBriefTraceback() - xmpp_error.error_from_iq(iq, 'internal-server-error') - return iq + return error.StanzaError('internal-server-error').toResponse(iq) else: - if e == Error: - stanza_error = failure.value.stanza_error - pubsub_error = failure.value.pubsub_error - msg = '' + if e == error.StanzaError: + exc = failure.value else: - stanza_error, pubsub_error = error_map[e] + condition, pubsubCondition, feature = error_map[e] msg = failure.value.msg - xmpp_error.error_from_iq(iq, stanza_error, msg) - if pubsub_error: - iq.error.addElement((NS_PUBSUB_ERRORS, pubsub_error)) - return iq + if pubsubCondition: + exc = PubSubError(condition, pubsubCondition, feature, msg) + else: + exc = error.StanzaError(condition, text=msg) + + return exc.toResponse(iq) def success(self, result, iq): iq.swapAttributeValues("to", "from") @@ -362,6 +363,7 @@ def componentConnected(self, xmlstream): xmlstream.addObserver(PUBSUB_CREATE, self.onCreate) + xmlstream.addObserver(PUBSUB_DEFAULT, self.onDefault) xmlstream.addObserver(PUBSUB_CONFIGURE_GET, self.onConfigureGet) xmlstream.addObserver(PUBSUB_CONFIGURE_SET, self.onConfigureSet) @@ -371,6 +373,7 @@ if not node: info.append(disco.Feature(NS_PUBSUB + "#create-nodes")) info.append(disco.Feature(NS_PUBSUB + "#config-node")) + info.append(disco.Feature(NS_PUBSUB + "#retrieve-default")) if self.backend.supports_instant_nodes(): info.append(disco.Feature(NS_PUBSUB + "#instant-nodes")) @@ -378,6 +381,7 @@ return defer.succeed(info) def onCreate(self, iq): + print "onCreate" self.handler_wrapper(self._onCreate, iq) def _onCreate(self, iq): @@ -397,14 +401,26 @@ entity['node'] = result return [reply] + def onDefault(self, iq): + self.handler_wrapper(self._onDefault, iq) + + def _onDefault(self, iq): + d = self.backend.get_default_configuration() + d.addCallback(self._return_default_response) + return d + + def _return_default_response(self, options): + reply = domish.Element((NS_PUBSUB_OWNER, "pubsub")) + default = reply.addElement("default") + default.addChild(self._form_from_configuration(options)) + + return [reply] + def onConfigureGet(self, iq): self.handler_wrapper(self._onConfigureGet, iq) def _onConfigureGet(self, iq): - try: - node_id = iq.pubsub.configure["node"] - except KeyError: - raise NodeNotConfigurable + node_id = iq.pubsub.configure.getAttribute("node") d = self.backend.get_node_configuration(node_id) d.addCallback(self._return_configuration_response, node_id) @@ -415,26 +431,25 @@ configure = reply.addElement("configure") if node_id: configure["node"] = node_id + configure.addChild(self._form_from_configuration(options)) + + return [reply] + + def _form_from_configuration(self, options): form = data_form.Form(type="form", form_type=NS_PUBSUB + "#node_config") for option in options: form.add_field(**option) - form.parent = configure - configure.addChild(form) - - return [reply] + return form def onConfigureSet(self, iq): + print "onConfigureSet" self.handler_wrapper(self._onConfigureSet, iq) def _onConfigureSet(self, iq): - try: - node_id = iq.pubsub.configure["node"] - except KeyError: - raise BadRequest - + node_id = iq.pubsub.configure["node"] requestor = jid.internJID(iq["from"]).userhostJID() for element in iq.pubsub.configure.elements():
--- a/idavoll/xmpp_error.py Thu Jun 18 11:52:01 2009 +0000 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,35 +0,0 @@ -# Copyright (c) 2003-2006 Ralph Meijer -# See LICENSE for details. - -NS_XMPP_STANZAS = "urn:ietf:params:xml:ns:xmpp-stanzas" - -conditions = { - 'bad-request': {'code': '400', 'type': 'modify'}, - 'not-authorized': {'code': '401', 'type': 'cancel'}, - 'item-not-found': {'code': '404', 'type': 'cancel'}, - 'not-acceptable': {'code': '406', 'type': 'modify'}, - 'conflict': {'code': '409', 'type': 'cancel'}, - 'internal-server-error': {'code': '500', 'type': 'wait'}, - 'feature-not-implemented': {'code': '501', 'type': 'cancel'}, - 'service-unavailable': {'code': '503', 'type': 'cancel'}, -} - -def error_from_iq(iq, condition, text = '', type = None): - iq.swapAttributeValues("to", "from") - iq["type"] = 'error' - e = iq.addElement("error") - - c = e.addElement((NS_XMPP_STANZAS, condition), NS_XMPP_STANZAS) - - if type == None: - type = conditions[condition]['type'] - - code = conditions[condition]['code'] - - e["code"] = code - e["type"] = type - - if text: - t = e.addElement((NS_XMPP_STANZAS, "text"), NS_XMPP_STANZAS, text) - - return iq