New subject: [xwiki-devs] [xwiki-notifications] r8200 - in xwiki-platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki: . web

5 Mar 2008

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

Re: [xwiki-devs] [xwiki-notifications] r8200 - in xwiki-platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki: . web