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