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
_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs
