hk2
  1. hk2
  2. HK2-124

hk2-config: vetoing adding/removing config elements

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 2.2.0
    • Fix Version/s: 2.4.0
    • Component/s: None
    • Labels:
      None

      Description

      For "reference constraint" validation, we'd like to deny (veto) removing an element referenced, when it is referenced by some other config element, similarly to https://hk2.java.net/config/configuration.html#Vetoing_changes (but it is for property changes only).

        Activity

        Hide
        andriy.zhdanov added a comment - - edited

        Please review proposed patch, that adds similar check (interceptor.beforeChange) for both add() and remove() methods, which allows a ConfigListener to react on add/remove change and perform necessary reference check and veto transaction, if required.

        diff --git a/hk2/hk2-config/src/main/java/org/jvnet/hk2/config/WriteableView.java b/hk2/hk2-config/src/main/java/org/jvnet/hk2/config/WriteableView.java
        index 10b1037..8940682 100644
        --- a/hk2/hk2-config/src/main/java/org/jvnet/hk2/config/WriteableView.java
        +++ b/hk2/hk2-config/src/main/java/org/jvnet/hk2/config/WriteableView.java
        @@ -593,8 +593,20 @@ private class ProtectedList extends AbstractList {
                             (Class<ConfigBeanProxy>) master.getImplementationClass());
         
                 }
        -        changeEvents.add(new PropertyChangeEvent(defaultView, id, null, param));
        -        return proxied.add(object);
        +        PropertyChangeEvent evt = new PropertyChangeEvent(defaultView, id, null, param);
        +        changeEvents.add(evt);
        +
        +        boolean added =  proxied.add(object);
        +
        +        try {
        +            for (ConfigBeanInterceptor interceptor : bean.getOptionalFeatures()) {
        +                interceptor.beforeChange(evt);
        +            }
        +        } catch(PropertyVetoException e) {
        +            throw new RuntimeException(e);
        +        }
        +
        +        return added;
             }
         
             @Override
        @@ -633,16 +645,17 @@ private class ProtectedList extends AbstractList {
         
             @Override
             public synchronized boolean remove(Object object) {
        -        changeEvents.add(new PropertyChangeEvent(defaultView, id, object, null));
        +        PropertyChangeEvent evt = new PropertyChangeEvent(defaultView, id, object, null);
        +        boolean removed = false;
         
                 try {
                     ConfigView handler = ((ConfigView) Proxy.getInvocationHandler(object)).getMasterView();
        -            for (int index = 0 ; index<proxied.size() ; index++) {
        +            for (int index = 0 ; index<proxied.size() && !removed; index++) {
                         Object target = proxied.get(index);
                         try {
                             ConfigView targetHandler = ((ConfigView) Proxy.getInvocationHandler(target)).getMasterView();
                             if (targetHandler==handler) {
        -                        return (proxied.remove(index)!=null);
        +                        removed = (proxied.remove(index)!=null);
                             }
                         } catch(IllegalArgumentException ex) {
                             // ignore
        @@ -650,10 +663,21 @@ private class ProtectedList extends AbstractList {
                     }
                 } catch(IllegalArgumentException e) {
                     // ignore, this is a leaf
        -            return proxied.remove(object);
        +            removed = proxied.remove(object);
         
                 }
        -        return false;
        +
        +        try {
        +            for (ConfigBeanInterceptor interceptor : bean.getOptionalFeatures()) {
        +                interceptor.beforeChange(evt);
        +            }
        +        } catch(PropertyVetoException e) {
        +            throw new RuntimeException(e);
        +        }
        +
        +        changeEvents.add(evt);
        +
        +        return removed;
             }
         
         }
        
        Show
        andriy.zhdanov added a comment - - edited Please review proposed patch, that adds similar check (interceptor.beforeChange) for both add() and remove() methods, which allows a ConfigListener to react on add/remove change and perform necessary reference check and veto transaction, if required. diff --git a/hk2/hk2-config/src/main/java/org/jvnet/hk2/config/WriteableView.java b/hk2/hk2-config/src/main/java/org/jvnet/hk2/config/WriteableView.java index 10b1037..8940682 100644 --- a/hk2/hk2-config/src/main/java/org/jvnet/hk2/config/WriteableView.java +++ b/hk2/hk2-config/src/main/java/org/jvnet/hk2/config/WriteableView.java @@ -593,8 +593,20 @@ private class ProtectedList extends AbstractList { ( Class <ConfigBeanProxy>) master.getImplementationClass()); } - changeEvents.add( new PropertyChangeEvent(defaultView, id, null , param)); - return proxied.add(object); + PropertyChangeEvent evt = new PropertyChangeEvent(defaultView, id, null , param); + changeEvents.add(evt); + + boolean added = proxied.add(object); + + try { + for (ConfigBeanInterceptor interceptor : bean.getOptionalFeatures()) { + interceptor.beforeChange(evt); + } + } catch (PropertyVetoException e) { + throw new RuntimeException(e); + } + + return added; } @Override @@ -633,16 +645,17 @@ private class ProtectedList extends AbstractList { @Override public synchronized boolean remove( Object object) { - changeEvents.add( new PropertyChangeEvent(defaultView, id, object, null )); + PropertyChangeEvent evt = new PropertyChangeEvent(defaultView, id, object, null ); + boolean removed = false ; try { ConfigView handler = ((ConfigView) Proxy.getInvocationHandler(object)).getMasterView(); - for ( int index = 0 ; index<proxied.size() ; index++) { + for ( int index = 0 ; index<proxied.size() && !removed; index++) { Object target = proxied.get(index); try { ConfigView targetHandler = ((ConfigView) Proxy.getInvocationHandler(target)).getMasterView(); if (targetHandler==handler) { - return (proxied.remove(index)!= null ); + removed = (proxied.remove(index)!= null ); } } catch (IllegalArgumentException ex) { // ignore @@ -650,10 +663,21 @@ private class ProtectedList extends AbstractList { } } catch (IllegalArgumentException e) { // ignore, this is a leaf - return proxied.remove(object); + removed = proxied.remove(object); } - return false ; + + try { + for (ConfigBeanInterceptor interceptor : bean.getOptionalFeatures()) { + interceptor.beforeChange(evt); + } + } catch (PropertyVetoException e) { + throw new RuntimeException(e); + } + + changeEvents.add(evt); + + return removed; } }
        Hide
        andriy.zhdanov added a comment -

        Patch above was committed on Jul 24, 2013, and is available for usage as follows since 2.2.0-b13:

        public class ExampleDocument extends DomDocument<ConfigBean>{
            private final VetoableChangeListener vetoableChangeListener;
        
            public ExampleDocument(ServiceLocator serviceLocator) {
                super(serviceLocator);
                vetoableChangeListener = new VetoableExampleConfigChangeListener();
            }
        
            @Override
            public ConfigBean make(final ServiceLocator habitat, XMLStreamReader xmlStreamReader,
                    ConfigBean dom, ConfigModel configModel) {
                ConfigBean configBean = new ConfigBean(habitat,this, dom, configModel, xmlStreamReader);
                configBean.getOptionalFeature(ConstrainedBeanListener.class).addVetoableChangeListener(vetoableChangeListener);
                return configBean;
            }
        }
        
        Show
        andriy.zhdanov added a comment - Patch above was committed on Jul 24, 2013, and is available for usage as follows since 2.2.0-b13: public class ExampleDocument extends DomDocument<ConfigBean>{ private final VetoableChangeListener vetoableChangeListener; public ExampleDocument(ServiceLocator serviceLocator) { super (serviceLocator); vetoableChangeListener = new VetoableExampleConfigChangeListener(); } @Override public ConfigBean make( final ServiceLocator habitat, XMLStreamReader xmlStreamReader, ConfigBean dom, ConfigModel configModel) { ConfigBean configBean = new ConfigBean(habitat, this , dom, configModel, xmlStreamReader); configBean.getOptionalFeature(ConstrainedBeanListener.class).addVetoableChangeListener(vetoableChangeListener); return configBean; } }

          People

          • Assignee:
            Mahesh Kannan
            Reporter:
            andriy.zhdanov
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated: