Done. Vincent Massol wrote:

Hi Sergiu,

First let me say this is cool! :) Some minor comments below.

On Mar 4, 2008, at 10:55 PM, sdumitriu (SVN) wrote:

...

Author: sdumitriu Date: 2008-03-04 22:55:30 +0100 (Tue, 04 Mar 2008) New Revision: 8200

Modified: xwiki-platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/ XWiki.java xwiki-platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/ web/XWikiAction.java Log: XWIKI-1522: Rewrite the notification mechanism Notifications are sent

Modified: xwiki-platform/core/trunk/xwiki-core/src/main/java/com/xpn/ xwiki/XWiki.java =================================================================== --- xwiki-platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/ XWiki.java 2008-03-04 18:33:04 UTC (rev 8199) +++ xwiki-platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/ XWiki.java 2008-03-04 21:55:30 UTC (rev 8200) @@ -76,6 +76,11 @@ import org.exoplatform.container.RootContainer; import org.hibernate.HibernateException; import org.securityfilter.filter.URLPatternMatcher; +import org.xwiki.observation.ObservationManager; +import org.xwiki.observation.event.ActionExecutionEvent; +import org.xwiki.observation.event.DocumentDeleteEvent; +import org.xwiki.observation.event.DocumentSaveEvent; +import org.xwiki.observation.event.DocumentUpdateEvent;

import com.xpn.xwiki.api.Api; import com.xpn.xwiki.api.Document; @@ -1070,14 +1075,37 @@ XWikiDocument originalDocument = doc.getOriginalDocument();

// Notify listeners about the document change - getNotificationManager().preverify(doc, originalDocument, - XWikiDocChangeNotificationInterface.EVENT_CHANGE, context); + if (originalDocument == null || originalDocument.isNew()) { + getNotificationManager().preverify(doc, originalDocument, + XWikiDocChangeNotificationInterface.EVENT_NEW, context); + } else { + getNotificationManager().preverify(doc, originalDocument, + XWikiDocChangeNotificationInterface.EVENT_CHANGE, context); + }

getStore().saveXWikiDoc(doc, context);

- // Notify listeners about the document change - getNotificationManager().verify(doc, originalDocument, - XWikiDocChangeNotificationInterface.EVENT_CHANGE, context); + try { + ObservationManager om = + (ObservationManager) Utils.getComponent(ObservationManager.ROLE, null, + context); + // Notify listeners about the document change + if (originalDocument == null || originalDocument.isNew()) { + getNotificationManager().verify(doc, originalDocument, + XWikiDocChangeNotificationInterface.EVENT_NEW, context); + if (om != null) { + om.notify(new DocumentSaveEvent(doc.getFullName()), doc, context); + }

It seems you're using both notification mechanisms, which is fine but I think this warrants some javadoc explanations.

...

+ } else { + getNotificationManager().verify(doc, originalDocument, + XWikiDocChangeNotificationInterface.EVENT_CHANGE, context); + if (om != null) { + om.notify(new DocumentUpdateEvent(doc.getFullName()), doc, context); + } + } + } catch (Exception ex) { + LOG.warn("Failed to send document notifications", ex);

Our rule is: no stack trace printing for warnings. Only for errors. I would consider this an error. Also I would put more information in the text like for what document it failed.

...

+ } } finally { if ((server != null) && (database != null)) { context.setDatabase(database); @@ -3406,8 +3434,16 @@ true); } getStore().deleteXWikiDoc(doc, context); - getNotificationManager().verify(new XWikiDocument(doc.getSpace(), doc.getName()), doc, - XWikiDocChangeNotificationInterface.EVENT_CHANGE, context); + try { + getNotificationManager().verify(new XWikiDocument(doc.getSpace(), doc.getName()), doc, + XWikiDocChangeNotificationInterface.EVENT_DELETE, context); + ObservationManager om = + (ObservationManager) Utils.getComponent(ObservationManager.ROLE, null, + context); + om.notify(new DocumentDeleteEvent(doc.getFullName()), doc, context); + } catch (Exception ex) { + LOG.warn("Failed to send document notifications", ex); + }

Same comments here.

...

}

public String getDatabase()

Modified: xwiki-platform/core/trunk/xwiki-core/src/main/java/com/xpn/ xwiki/web/XWikiAction.java =================================================================== --- xwiki-platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/ web/XWikiAction.java 2008-03-04 18:33:04 UTC (rev 8199) +++ xwiki-platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/ web/XWikiAction.java 2008-03-04 21:55:30 UTC (rev 8200) @@ -35,6 +35,8 @@ import org.apache.struts.action.ActionForward; import org.apache.struts.action.ActionMapping; import org.apache.velocity.VelocityContext; +import org.xwiki.observation.ObservationManager; +import org.xwiki.observation.event.ActionExecutionEvent;

import com.xpn.xwiki.XWiki; import com.xpn.xwiki.XWikiContext; @@ -284,6 +286,14 @@ } catch (Throwable e) { e.printStackTrace(); } + try { + ObservationManager om = + (ObservationManager) Utils.getComponent(ObservationManager.ROLE, null, + context); + om.notify(new ActionExecutionEvent(mapping.getName()), context.getDoc(), context); + } catch (Throwable ex) { + LOG.warn("Cannot send notifications", ex);

Same here too.

...

+ }

if (monitor != null) { monitor.endTimer("notify");

Thanks -Vincent

-- Sergiu Dumitriu http://purl.org/net/sergiu/