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