Re: [xwiki-devs] [xwiki-notifications] r9649 - xwiki-platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki
Hi Sergiu, On May 5, 2008, at 7:59 PM, sdumitriu (SVN) wrote:
Author: sdumitriu Date: 2008-05-05 19:59:18 +0200 (Mon, 05 May 2008) New Revision: 9649
Modified: xwiki-platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/ XWiki.java Log: [misc] Fix NPE during tests
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-05-05 16:57:49 UTC (rev 9648) +++ xwiki-platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/ XWiki.java 2008-05-05 17:59:18 UTC (rev 9649) @@ -3580,7 +3580,9 @@ // doc.getOriginalDocument() ObservationManager om = (ObservationManager) Utils.getComponent(ObservationManager.ROLE, null, context); - om.notify(new DocumentDeleteEvent(doc.getFullName()), doc, context); + if (om != null) { + om.notify(new DocumentDeleteEvent(doc.getFullName()), doc, context); + }
Question: Utils.getComponents throws a RuntimeException when a component cannot be found. Thus I'm not sure why you're testing for null? Am I missing something? In addition I think that if we really wanted to test for null then we should also have an else and display a warning if a notification cannot be sent. However I don't think it's normal that such a component cannot be lookup and we can safely assume it is found. The test should probably be modified to use AbstractXWikiComponentTestCase. WDYT? Thanks -Vincent
Vincent Massol wrote:
Hi Sergiu,
On May 5, 2008, at 7:59 PM, sdumitriu (SVN) wrote:
Author: sdumitriu Date: 2008-05-05 19:59:18 +0200 (Mon, 05 May 2008) New Revision: 9649
Modified: xwiki-platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/ XWiki.java Log: [misc] Fix NPE during tests
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-05-05 16:57:49 UTC (rev 9648) +++ xwiki-platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/ XWiki.java 2008-05-05 17:59:18 UTC (rev 9649) @@ -3580,7 +3580,9 @@ // doc.getOriginalDocument() ObservationManager om = (ObservationManager) Utils.getComponent(ObservationManager.ROLE, null, context); - om.notify(new DocumentDeleteEvent(doc.getFullName()), doc, context); + if (om != null) { + om.notify(new DocumentDeleteEvent(doc.getFullName()), doc, context); + }
Question: Utils.getComponents throws a RuntimeException when a component cannot be found. Thus I'm not sure why you're testing for null? Am I missing something?
In addition I think that if we really wanted to test for null then we should also have an else and display a warning if a notification cannot be sent. However I don't think it's normal that such a component cannot be lookup and we can safely assume it is found. The test should probably be modified to use AbstractXWikiComponentTestCase.
Not sure what to say here... The test WAS failing on a NPE at that point, because Utils.getComponent returns null if the component manager cannot be found. As you said, maybe the tests should be modified to extend AbstractXWikiComponentTestCase, but the fastest solution was to simply check for null in the changed code. The same check was performed in other places where we're sending notification, just this one was left out. And sending notifications was not interesting for this test, anyway. -- Sergiu Dumitriu http://purl.org/net/sergiu/
participants (2)
-
Sergiu Dumitriu -
Vincent Massol