Some questions: 1) I thought this notify package was going to be removed since all the code was updated to use the new observation module? 2) Why do you do the protection inside the rule instead of inside the Notification Manager. Funnily I had put a TODO comment 2 days ago there exactly for this... ;) Thanks -Vincent On Aug 26, 2009, at 5:04 PM, tmortagne (SVN) wrote:
Author: tmortagne Date: 2009-08-26 17:04:31 +0200 (Wed, 26 Aug 2009) New Revision: 22950
Modified: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/notify/ DocChangeRule.java platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/notify/ XWikiActionRule.java Log: XWIKI-4238: Nofification system should protectd itself from bad listeners
Modified: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/ notify/DocChangeRule.java =================================================================== --- platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/ notify/DocChangeRule.java 2009-08-26 15:04:15 UTC (rev 22949) +++ platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/ notify/DocChangeRule.java 2009-08-26 15:04:31 UTC (rev 22950) @@ -21,12 +21,17 @@
package com.xpn.xwiki.notify;
+import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + import com.xpn.xwiki.XWikiContext; import com.xpn.xwiki.doc.XWikiDocument;
@Deprecated public class DocChangeRule implements XWikiNotificationRule { + protected static final Log LOG = LogFactory.getLog(DocChangeRule.class); + private XWikiDocChangeNotificationInterface target;
private boolean preverify = false; @@ -71,7 +76,13 @@ return; }
- getTarget().notify(this, newdoc, olddoc, XWikiDocChangeNotificationInterface.EVENT_CHANGE, context); + try { + getTarget().notify(this, newdoc, olddoc, XWikiDocChangeNotificationInterface.EVENT_CHANGE, context); + } catch (Exception e) { + // protect from bad listeners + LOG.error("Fail to notify target [" + getTarget() + "] for event [" + this + ", newdoc=" + newdoc + + ", olddoc=" + olddoc + "]", e); + } }
/** @@ -86,7 +97,13 @@ return; }
- getTarget().notify(this, newdoc, olddoc, XWikiDocChangeNotificationInterface.EVENT_CHANGE, context); + try { + getTarget().notify(this, newdoc, olddoc, XWikiDocChangeNotificationInterface.EVENT_CHANGE, context); + } catch (Exception e) { + // protect from bad listeners + LOG.error("Fail to notify target [" + getTarget() + "] for pre event [" + this + ", newdoc=" + newdoc + + ", olddoc=" + olddoc + "]", e); + } }
/**
Modified: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/ notify/XWikiActionRule.java =================================================================== --- platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/ notify/XWikiActionRule.java 2009-08-26 15:04:15 UTC (rev 22949) +++ platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/ notify/XWikiActionRule.java 2009-08-26 15:04:31 UTC (rev 22950) @@ -21,12 +21,17 @@
package com.xpn.xwiki.notify;
+import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + import com.xpn.xwiki.XWikiContext; import com.xpn.xwiki.doc.XWikiDocument;
@Deprecated public class XWikiActionRule implements XWikiNotificationRule { + protected static final Log LOG = LogFactory.getLog(XWikiActionRule.class); + private XWikiActionNotificationInterface target;
private boolean preverify = false; @@ -74,9 +79,9 @@ try { getTarget().notify(this, doc, action, context); } catch (Throwable e) { - // Notification should never fail - // Just report an error - e.printStackTrace(); + // protect from bad listeners + LOG.error("Fail to notify target [" + getTarget() + "] for event [" + this + ", doc=" + doc + ", action=" + + action + "]", e); } }
@@ -87,9 +92,9 @@ try { getTarget().notify(this, doc, action, context); } catch (Throwable e) { - // Notification should never fail - // Just report an error - e.printStackTrace(); + // protect from bad listeners + LOG.error("Fail to notify target [" + getTarget() + "] for pre event [" + this + ", doc=" + doc + + ", action=" + action + "]", e); } }