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 (2006-09-06)
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