Bläddra i källkod

Fix bug in setObserved(T, Collection<?>).
Fix infinite recursion in ObservableListeners

Sam Jaffe 5 år sedan
förälder
incheckning
35b4da5e4b

+ 2 - 29
pom.xml

@@ -4,7 +4,7 @@
 
   <groupId>org.leumasjaffe</groupId>
   <artifactId>observer</artifactId>
-  <version>0.4.0</version>
+  <version>0.4.1</version>
   <packaging>jar</packaging>
 
   <name>observer</name>
@@ -15,34 +15,6 @@
   </properties>
 
    <build>
-    <pluginManagement>
-      <plugins>
-        <plugin>
-          <groupId>org.eclipse.m2e</groupId>
-          <artifactId>lifecycle-mapping</artifactId>
-          <version>1.0.0</version>
-          <configuration>
-            <lifecycleMappingMetadata>
-              <pluginExecutions>
-                <pluginExecution>
-                  <pluginExecutionFilter>
-                    <groupId>org.projectlombok</groupId>
-                    <artifactId>lombok-maven-plugin</artifactId>
-                    <versionRange>[1,)</versionRange>
-                    <goals>
-                      <goal>delombok</goal>
-                    </goals>
-                  </pluginExecutionFilter>
-                  <action>
-                    <ignore />
-                  </action>
-                </pluginExecution>
-              </pluginExecutions>
-            </lifecycleMappingMetadata>
-          </configuration>
-        </plugin>
-      </plugins>
-    </pluginManagement>
     <sourceDirectory>target/generated-sources/delombok</sourceDirectory>
     <plugins>
       <plugin>
@@ -65,6 +37,7 @@
             <goals>
               <goal>delombok</goal>
             </goals>
+            <?m2e ignore?>
           </execution>
         </executions>
         <configuration>

+ 7 - 1
src/main/lombok/org/leumasjaffe/observer/IndirectObservableListener.java

@@ -21,6 +21,7 @@ public class IndirectObservableListener<C, T> {
 	
 	/** A model that our parent GUI is interested in, usually Observable, or an aggregation */
 	@NonFinal T model = null;
+	@NonFinal boolean isUpdating = false;
 	
 	/**
 	 * @param obs Sets the object that we are observing, this object specifically will then be
@@ -47,11 +48,14 @@ public class IndirectObservableListener<C, T> {
 	}
 	
 	public void setObserved( T obs, Collection<? extends Observable> extra ) {
-		setObserved(obs, (Observable[]) extra.toArray());
+		setObserved(obs, extra.toArray(new Observable[0]));
 	}
 
 	private void updateComponent() {
+		if (isUpdating) { return; }
+		isUpdating = true;
 		update.accept(component, model);
+		isUpdating = false;
 	}
 
 	/**
@@ -59,6 +63,8 @@ public class IndirectObservableListener<C, T> {
 	 * {@see org.leumasjaffe.observer.ObserverDispatch#notifySubscribers}
 	 */
 	public void notifySubscribers(Observable obs) {
+		isUpdating = true;
 		ObserverDispatch.notifySubscribers(obs, this);
+		isUpdating = false;
 	}
 }

+ 24 - 12
src/main/lombok/org/leumasjaffe/observer/ObserverDispatch.java

@@ -7,12 +7,12 @@ import com.google.common.collect.LinkedListMultimap;
 import com.google.common.collect.Multimap;
 
 import lombok.AccessLevel;
-import lombok.AllArgsConstructor;
+import lombok.RequiredArgsConstructor;
 import lombok.experimental.FieldDefaults;
 import lombok.experimental.UtilityClass;
 
 /**
- * The master object that associates observale objects with listeners and invocations
+ * The master object that associates observable objects with listeners and invocations
  * TODO: May crash if updating an object where a Listener was destroyed without unsub
  * TODO: Potentially leaks memory if Listeners go out of scope without unsub
  * TODO: No way to drop Observable objects as they go out of scope
@@ -20,14 +20,14 @@ import lombok.experimental.UtilityClass;
 @UtilityClass
 @FieldDefaults(level=AccessLevel.PRIVATE, makeFinal=true)
 public class ObserverDispatch {
-	@AllArgsConstructor
+	@RequiredArgsConstructor
 	@FieldDefaults(level=AccessLevel.PRIVATE, makeFinal=true)
-	private static class Pair {
-		WeakReference<Object> obj;
+	private static class SubscriberInfo {
+		WeakReference<Object> reference;
 		Subscriber sub;
 	}
 	
-	Multimap<UUID, Pair> observers = LinkedListMultimap.create();
+	Multimap<UUID, SubscriberInfo> observers = LinkedListMultimap.create();
 	
 	/**
 	 * Register a listener callback
@@ -39,7 +39,7 @@ public class ObserverDispatch {
 	 * we request an update.
 	 */
 	public void subscribe(Observable target, Object src, Subscriber sub) {
-		observers.put(target.observableId(), new Pair(new WeakReference<>(src), sub));
+		observers.put(target.observableId(), new SubscriberInfo(new WeakReference<>(src), sub));
 	}
 	
 	/**
@@ -48,8 +48,7 @@ public class ObserverDispatch {
 	 * call.
 	 */
 	public void unsubscribeAll(Object src) {
-		if (src == null) return;
-		observers.entries().removeIf( e -> e.getValue().obj.get() == src );
+		observers.entries().removeIf(e -> !notNullRef(e.getValue()) || refersTo(e.getValue(), src));
 	}
 	
 	/**
@@ -59,8 +58,8 @@ public class ObserverDispatch {
 	 * @param src An owning Listener that does not need to be updated.
 	 */
 	void notifySubscribers(Observable target, IndirectObservableListener<?, ?> src) {
-		observers.get(target.observableId()).stream().filter(
-				p -> p.obj.get() != src).forEach( p -> p.sub.notifyUpdate() );
+		observers.get(target.observableId()).stream().filter(ObserverDispatch::notNullRef)
+			.forEach(ObserverDispatch::notifyOnce);
 	}
 
 	/**
@@ -68,6 +67,19 @@ public class ObserverDispatch {
 	 * @param target The Observable that was updated by our caller
 	 */
 	public void notifySubscribers(Observable target) {
-		observers.get(target.observableId()).stream().forEach( p -> p.sub.notifyUpdate() );
+		observers.get(target.observableId()).stream().filter(
+				ObserverDispatch::notNullRef).forEach(ObserverDispatch::notifyOnce);
+	}
+	
+	private static boolean notNullRef(SubscriberInfo info) {
+		return info.reference.get() != null;
+	}
+	
+	private static boolean refersTo(SubscriberInfo info, Object source) {
+		return info.reference.get() == source;
+	}
+	
+	private static void notifyOnce(SubscriberInfo info) {
+		info.sub.notifyUpdate();
 	}
 }