Repository: brooklyn-server Updated Branches: refs/heads/master 8a7d243c3 -> 8768c890d
Delete usage of Groovy ObservableList Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/d3c7b911 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/d3c7b911 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/d3c7b911 Branch: refs/heads/master Commit: d3c7b911fda2b26d3bd1e2e872bf6d0ef686480c Parents: 3c35a1e Author: Aled Sage <[email protected]> Authored: Wed May 17 19:24:01 2017 +0200 Committer: Aled Sage <[email protected]> Committed: Fri May 19 10:14:46 2017 +0100 ---------------------------------------------------------------------- ...PropertyChangeToCollectionChangeAdapter.java | 68 ------- .../core/mgmt/internal/LocalEntityManager.java | 14 +- .../core/mgmt/internal/ObservableSet.java | 76 ++++++++ .../core/mgmt/internal/ObservableSetTest.java | 182 +++++++++++++++++++ 4 files changed, 266 insertions(+), 74 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/d3c7b911/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/GroovyObservablesPropertyChangeToCollectionChangeAdapter.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/GroovyObservablesPropertyChangeToCollectionChangeAdapter.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/GroovyObservablesPropertyChangeToCollectionChangeAdapter.java deleted file mode 100644 index 9b1bdc6..0000000 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/GroovyObservablesPropertyChangeToCollectionChangeAdapter.java +++ /dev/null @@ -1,68 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.brooklyn.core.mgmt.internal; - -import groovy.util.ObservableList; - -import java.beans.PropertyChangeEvent; -import java.beans.PropertyChangeListener; - -public class GroovyObservablesPropertyChangeToCollectionChangeAdapter implements PropertyChangeListener { - @SuppressWarnings("rawtypes") - private final CollectionChangeListener delegate; - - public GroovyObservablesPropertyChangeToCollectionChangeAdapter(@SuppressWarnings("rawtypes") CollectionChangeListener delegate) { - this.delegate = delegate; - } - - @Override - @SuppressWarnings("unchecked") - public void propertyChange(PropertyChangeEvent evt) { - if (evt instanceof ObservableList.ElementAddedEvent) { - delegate.onItemAdded(evt.getNewValue()); - } else if (evt instanceof ObservableList.ElementRemovedEvent) { - delegate.onItemRemoved(evt.getOldValue()); - } else if (evt instanceof ObservableList.ElementUpdatedEvent) { - delegate.onItemRemoved(evt.getOldValue()); - delegate.onItemAdded(evt.getNewValue()); - } else if (evt instanceof ObservableList.ElementClearedEvent) { - for (Object value : ((ObservableList.ElementClearedEvent) evt).getValues()) { - delegate.onItemAdded(value); - } - } else if(evt instanceof ObservableList.MultiElementAddedEvent ) { - for(Object value: ((ObservableList.MultiElementAddedEvent)evt).getValues()){ - delegate.onItemAdded(value); - } - } - } - - @Override - public int hashCode() { - return delegate.hashCode(); - } - - @Override - public boolean equals(Object other) { - if (other instanceof GroovyObservablesPropertyChangeToCollectionChangeAdapter) - return delegate.equals(((GroovyObservablesPropertyChangeToCollectionChangeAdapter) other).delegate); - if (other instanceof CollectionChangeListener) - return delegate.equals(other); - return false; - } -} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/d3c7b911/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalEntityManager.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalEntityManager.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalEntityManager.java index 6710c4d..155286b 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalEntityManager.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalEntityManager.java @@ -20,8 +20,6 @@ package org.apache.brooklyn.core.mgmt.internal; import static com.google.common.base.Preconditions.checkNotNull; -import groovy.util.ObservableList; - import java.lang.reflect.Proxy; import java.util.Collection; import java.util.Collections; @@ -105,8 +103,12 @@ public class LocalEntityManager implements EntityManagerInternal { /** Management mode for each entity */ private final Map<String,ManagementTransitionMode> entityModesById = Collections.synchronizedMap(Maps.<String,ManagementTransitionMode>newLinkedHashMap()); - /** Proxies of the managed entities */ - private final ObservableList entities = new ObservableList(); + /** + * Proxies of the managed entities. + * + * Access to this is always done in a synchronized block (synchronizing on `this`). + */ + private final ObservableSet<Entity> entities = new ObservableSet<Entity>(); /** Proxies of the managed entities that are applications */ private final Set<Application> applications = Sets.newConcurrentHashSet(); @@ -800,12 +802,12 @@ public class LocalEntityManager implements EntityManagerInternal { void addEntitySetListener(CollectionChangeListener<Entity> listener) { //must notify listener in a different thread to avoid deadlock (issue #378) AsyncCollectionChangeAdapter<Entity> wrappedListener = new AsyncCollectionChangeAdapter<Entity>(managementContext.getExecutionManager(), listener); - entities.addPropertyChangeListener(new GroovyObservablesPropertyChangeToCollectionChangeAdapter(wrappedListener)); + entities.addListener(wrappedListener); } void removeEntitySetListener(CollectionChangeListener<Entity> listener) { AsyncCollectionChangeAdapter<Entity> wrappedListener = new AsyncCollectionChangeAdapter<Entity>(managementContext.getExecutionManager(), listener); - entities.removePropertyChangeListener(new GroovyObservablesPropertyChangeToCollectionChangeAdapter(wrappedListener)); + entities.removeListener(wrappedListener); } private boolean shouldSkipUnmanagement(Entity e) { http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/d3c7b911/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/ObservableSet.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/ObservableSet.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/ObservableSet.java new file mode 100644 index 0000000..297a9fa --- /dev/null +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/ObservableSet.java @@ -0,0 +1,76 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.brooklyn.core.mgmt.internal; + +import static com.google.common.base.Preconditions.checkNotNull; + +import java.util.LinkedHashSet; +import java.util.Set; + +import com.google.common.collect.Sets; + +/** + * Replacement for use of Groovy's ObservableList; for internal purposes only. + */ +class ObservableSet<T> { + + private final Set<T> delegate; + + private final Set<CollectionChangeListener<? super T>> listeners = Sets.newCopyOnWriteArraySet(); + + public ObservableSet() { + this(new LinkedHashSet<T>()); + } + + public ObservableSet(Set<T> delegate) { + this.delegate = checkNotNull(delegate, "delegate"); + } + + public void addListener(CollectionChangeListener<? super T> listener) { + listeners.add(checkNotNull(listener, "listener")); + } + + public void removeListener(CollectionChangeListener<? super T> listener) { + listeners.remove(checkNotNull(listener, "listener")); + } + + public boolean add(T val) { + boolean changed = delegate.add(val); + if (changed) { + for (CollectionChangeListener<? super T> listener : listeners) { + listener.onItemAdded(val); + } + } + return changed; + } + + public boolean remove(T val) { + boolean changed = delegate.remove(val); + if (changed) { + for (CollectionChangeListener<? super T> listener : listeners) { + listener.onItemRemoved(val); + } + } + return changed; + } + + public boolean contains(T val) { + return delegate.contains(val); + } +} http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/d3c7b911/core/src/test/java/org/apache/brooklyn/core/mgmt/internal/ObservableSetTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/internal/ObservableSetTest.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/internal/ObservableSetTest.java new file mode 100644 index 0000000..40758d3 --- /dev/null +++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/internal/ObservableSetTest.java @@ -0,0 +1,182 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.brooklyn.core.mgmt.internal; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; + +import java.util.List; + +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import com.google.common.base.Objects; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Lists; + +public class ObservableSetTest { + + private ObservableSet<Object> set; + private RecordingListener listener; + + @BeforeMethod(alwaysRun=true) + public void setUp() throws Exception { + set = new ObservableSet<Object>(); + listener = new RecordingListener(); + set.addListener(listener); + } + + @Test + public void testAdd() { + boolean result1 = set.add("val1"); + assertTrue(result1); + assertEquals(listener.getCalls(), ImmutableList.of(new Call(NotificationType.ADDED, "val1"))); + listener.clear(); + + boolean result2 = set.add("val1"); + assertFalse(result2); + assertEquals(listener.getCalls(), ImmutableList.of()); + } + + @Test + public void testRemove() { + set.add("val1"); + listener.clear(); + + boolean result1 = set.remove("val1"); + assertTrue(result1); + assertEquals(listener.getCalls(), ImmutableList.of(new Call(NotificationType.REMOVED, "val1"))); + listener.clear(); + + boolean result2 = set.remove("val1"); + assertFalse(result2); + assertEquals(listener.getCalls(), ImmutableList.of()); + + boolean result3 = set.remove("different"); + assertFalse(result3); + assertEquals(listener.getCalls(), ImmutableList.of()); + } + + @Test + public void testContains() { + set.add("val1"); + assertTrue(set.contains("val1")); + + set.remove("val1"); + assertFalse(set.contains("val1")); + + assertFalse(set.contains("different")); + } + + @Test + public void testMultipleListeners() { + List<RecordingListener> listeners = Lists.newArrayList(); + for (int i = 0; i < 2; i++) { + RecordingListener l = new RecordingListener(); + listeners.add(l); + set.addListener(l); + } + + set.add("val1"); + for (RecordingListener listener: listeners) { + assertEquals(listener.getCalls(), ImmutableList.of(new Call(NotificationType.ADDED, "val1"))); + listener.clear(); + } + + set.remove("val1"); + for (RecordingListener listener: listeners) { + assertEquals(listener.getCalls(), ImmutableList.of(new Call(NotificationType.REMOVED, "val1"))); + listener.clear(); + } + } + + @Test + public void testRemoveListenerNotSubsequentlyNotified() { + set.removeListener(listener); + + set.add("val1"); + assertEquals(listener.getCalls(), ImmutableList.of()); + + set.remove("val1"); + assertEquals(listener.getCalls(), ImmutableList.of()); + } + + @Test + public void testAddAndRemoveListenerIdempotent() { + RecordingListener listener2 = new RecordingListener(); + + set.addListener(listener2); + set.addListener(listener2); + + set.add("val1"); + assertEquals(listener2.getCalls(), ImmutableList.of(new Call(NotificationType.ADDED, "val1"))); + listener2.clear(); + + set.removeListener(listener2); + + set.add("val2"); + assertEquals(listener2.getCalls(), ImmutableList.of()); + + set.removeListener(listener2); + } + + static enum NotificationType { + ADDED, + REMOVED; + } + + static class Call { + final NotificationType type; + final Object val; + + Call(NotificationType type, Object val) { + this.type = type; + this.val = val; + } + + @Override + public boolean equals(Object obj) { + return (obj instanceof Call) && Objects.equal(((Call)obj).type, type) + && Objects.equal(((Call)obj).val, val); + } + } + + static class RecordingListener implements CollectionChangeListener<Object> { + final List<Call> calls = Lists.newCopyOnWriteArrayList(); + + @Override + public void onItemAdded(Object item) { + calls.add(new Call(NotificationType.ADDED, item)); + } + + @Override + public void onItemRemoved(Object item) { + calls.add(new Call(NotificationType.REMOVED, item)); + } + + public List<Call> getCalls() { + return ImmutableList.copyOf(calls); + } + + public void clear() { + calls.clear(); + } + } +}
