On Wed, Aug 26, 2009 at 17:24, Vincent Massol<[email protected]> wrote:

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?

Because we don't remove it its just deprecated so there is code still using it somewhere plus i did not had time to update activity stream plugin JV added to XE very recently and some other minor code.

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... ;)

Simply because there was already a protection for action events but not for documents events so i just put the missing one.

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);         }     }

_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs

-- Thomas Mortagne