eclipse-emf / org.eclipse.emf

Eclipse Public License 2.0
10 stars 13 forks source link

EStructuralFeatureImpl#dynamicUnset creates notification of type SET instead of UNSET #31

Closed castortech closed 3 months ago

castortech commented 3 months ago

Not sure if there is a valid reason for it, but having an issue reasoning on a SET notification when doing an unset operation

    public void dynamicUnset(InternalEObject owner, EStructuralFeature.Internal.DynamicValueHolder settings, int index)
    {
      if (owner.eNotificationRequired())
      {
        Object oldValue = dynamicGet(owner, settings, index, false, true);
        settings.dynamicUnset(index);
        owner.eNotify
          (notificationCreator.createNotification
             (owner, Notification.SET, feature, oldValue, this.defaultValue));
      }
      else
      {
        settings.dynamicUnset(index);
      }
    }
merks commented 3 months ago

Only unsettable features generate an unset notification, e.g.,

org.eclipse.emf.ecore.impl.EStructuralFeatureImpl.InternalSettingDelegateSingleDataUnsettable.dynamicUnset(InternalEObject, DynamicValueHolder, int)

https://github.com/eclipse-emf/org.eclipse.emf/blob/1ee74398ae1efa36e45df4c94d31c122e4ba27b1/plugins/org.eclipse.emf.ecore/src/org/eclipse/emf/ecore/impl/EStructuralFeatureImpl.java#L2467

https://github.com/eclipse-emf/org.eclipse.emf/blob/1ee74398ae1efa36e45df4c94d31c122e4ba27b1/plugins/org.eclipse.emf.ecore/src/org/eclipse/emf/ecore/impl/EStructuralFeatureImpl.java#L2798

https://github.com/eclipse-emf/org.eclipse.emf/blob/1ee74398ae1efa36e45df4c94d31c122e4ba27b1/plugins/org.eclipse.emf.ecore/src/org/eclipse/emf/ecore/impl/EStructuralFeatureImpl.java#L2893-L2896

castortech commented 3 months ago

Thanks for the precision Ed, had not noticed that since it was calling an unset method.