Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 309649 - NPE on converting CDO deltas to notifications
Summary: NPE on converting CDO deltas to notifications
Status: CLOSED FIXED
Alias: None
Product: EMF
Classification: Modeling
Component: cdo.core (show other bugs)
Version: 2.0   Edit
Hardware: PC Mac OS X - Carbon (unsup.)
: P3 major (vote)
Target Milestone: ---   Edit
Assignee: Eike Stepper CLA
QA Contact: Eike Stepper CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-19 06:26 EDT by Egidijus Vaisnora CLA
Modified: 2010-06-29 09:22 EDT (History)
0 users

See Also:

Attachments
Patch (973 bytes, text/plain)
2010-04-20 03:50 EDT, Egidijus Vaisnora CLA 		stepper: iplog+
Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Egidijus Vaisnora CLA 2010-04-19 06:26:16 EDT 
Build Identifier: cdo 2.0

Appeared on listening object changes with CDOAdapter. Transaction delivered feature delta to server : 
CDOFeatureDelta[versions, LIST, list=[CDOFeatureDelta[versions, CLEAR], CDOFeatureDelta[versions, ADD, value=OID147, index=0], CDOFeatureDelta[versions, ADD, value=NULL, index=1], CDOFeatureDelta[versions, REMOVE, index=1], CDOFeatureDelta[versions, ADD, value=OID150, index=1]]]

But on another client, in attempt to convert delta to the Notification, NPE was thrown:

java.lang.NullPointerException
	at org.eclipse.emf.common.notify.impl.NotificationImpl.merge(NotificationImpl.java:836)
	at org.eclipse.emf.common.notify.impl.NotificationImpl.add(NotificationImpl.java:1000)
	at org.eclipse.emf.common.notify.impl.NotificationImpl.add(NotificationImpl.java:1020)
	at org.eclipse.emf.internal.cdo.CDONotificationBuilder.add(CDONotificationBuilder.java:117)
	at org.eclipse.emf.internal.cdo.CDONotificationBuilder.visit(CDONotificationBuilder.java:72)
	at org.eclipse.emf.cdo.internal.common.revision.delta.CDORemoveFeatureDeltaImpl.accept(CDORemoveFeatureDeltaImpl.java:72)
	at org.eclipse.emf.internal.cdo.CDONotificationBuilder.visit(CDONotificationBuilder.java:92)
	at org.eclipse.emf.cdo.internal.common.revision.delta.CDOListFeatureDeltaImpl.accept(CDOListFeatureDeltaImpl.java:215)
	at org.eclipse.emf.cdo.internal.common.revision.delta.CDORevisionDeltaImpl.accept(CDORevisionDeltaImpl.java:227)
	at org.eclipse.emf.internal.cdo.CDONotificationBuilder.buildNotification(CDONotificationBuilder.java:52)
	at org.eclipse.emf.internal.cdo.view.CDOViewImpl.handleChangeSubscription(CDOViewImpl.java:1321)

Reproducible: Always
Comment 1 Egidijus Vaisnora CLA 2010-04-19 07:07:31 EDT 
CDOClearFeatureDelta is created as CDODeltaNotificationImpl with event NotificationImpl.REMOVE_MANY and new value of this event is passed *null*. Merge code is expecting to receive array of removed indexes. Hence any other event of type NotificationImpl.REMOVE will rise NPE on merge code.
Comment 2 Egidijus Vaisnora CLA 2010-04-20 03:46:59 EDT 
I think that REMOVE action shouldn't be merged with REMOVE_MANY action. If this REMOVE_MANY was originated from feature delta CLEAR.
Comment 3 Egidijus Vaisnora CLA 2010-04-20 03:50:12 EDT 
Created attachment 165388 [details]
Patch

I would prefer to add concept in EMF like REMOVE_ALL, but here is fix in CDO code.
Comment 4 Eike Stepper CLA 2010-04-20 04:13:57 EDT 
Hi Egidijus,

Thanks for the patch! Can you please confirm the following:

1) The number of lines that you changed is smaller than 250.
2) You are the only author of these changed lines.
3) You apply the EPL to these changed lines.
Comment 5 Egidijus Vaisnora CLA 2010-04-20 04:22:44 EDT 
(In reply to comment #4)

> 1) The number of lines that you changed is smaller than 250.
Yes, it is obvious :)
> 2) You are the only author of these changed lines.
Yes
> 3) You apply the EPL to these changed lines.
Yes
Comment 6 Eike Stepper CLA 2010-04-20 04:24:14 EDT 
Obvious for you and me, but for the lawyers?! :P

Committed to R2_0_maintenance