refactor: enhance concurrency handling in BeanRegistryImplementation, improve logging and null checks, and streamline activationRun logic

Author: predic8

annot/src/main/java/com/predic8/membrane/annot/beanregistry/BeanRegistryImplementation.java

@@ -23,6 +23,13 @@
 import java.util.concurrent.*;
 import java.util.function.*;
 
+/**
+ * TODO:
+ * - More Tests
+ * - Document
+ *   - singletonBeans
+ *   - bcs
+ */
 public class BeanRegistryImplementation implements BeanRegistry, BeanCollector {
 
     private static final Logger log = LoggerFactory.getLogger(BeanRegistryImplementation.class);
@@ -85,38 +92,51 @@ public void handle(ChangeEvent changeEvent, boolean isLast) {
     }
 
     private void activationRun() {
-        Set<UidAction> uidsToRemove = new HashSet<>();
-        for (UidAction uidAction : uidsToActivate) {
+        for (UidAction uidAction : cloneUidActions()) {
             BeanContainer bc = bcs.get(uidAction.uid);
-            try {
-                Object bean = define(bc.getDefinition());
-                bc.setSingleton(bean);
+            if (bc == null) {
+                log.warn("Skipping activation for missing uid {}", uidAction.uid);
+                continue;
+            }
 
-                // e.g. inform router about new proxy
-                observer.handleBeanEvent(new BeanDefinitionChanged(uidAction.action, bc.getDefinition()), bean, getOldBean(uidAction.action, bc.getDefinition()));
+            BeanDefinition def = bc.getDefinition();
+            Object oldBean = getOldBean(uidAction.action, uidAction.uid); // capture first
+            Object newBean = null; // Do not inline!
 
-                if (uidAction.action.isAdded() || uidAction.action.isModified())
-                    singletonBeans.put(bc.getDefinition().getUid(), bean);
+            try {
                 if (uidAction.action.isDeleted()) {
-                    singletonBeans.remove(bc.getDefinition().getUid());
-                    bcs.remove(bc.getDefinition().getUid());
+                    singletonBeans.remove(uidAction.uid);
+                    bcs.remove(uidAction.uid);
+                } else {
+                    newBean = define(def);
+                    bc.setSingleton(newBean);
+                    singletonBeans.put(uidAction.uid, newBean);
                 }
-                uidsToRemove.add(uidAction);
+
+                observer.handleBeanEvent(
+                        new BeanDefinitionChanged(uidAction.action, def),
+                        newBean,
+                        oldBean
+                );
             } catch (Exception e) {
-                log.error("Could not handle {} {}/{}", uidAction.action,
-                        bc.getDefinition().getNamespace(), bc.getDefinition().getName(), e);
+                log.error("Could not handle {} {}/{}", uidAction.action, def.getNamespace(), def.getName(), e);
                 throw new RuntimeException(e);
             }
         }
-        for (UidAction uidAction : uidsToRemove)
-            uidsToActivate.remove(uidAction);
     }
 
-    private @Nullable Object getOldBean(WatchAction action, BeanDefinition bd) {
-        Object oldBean = null;
-        if (action.isModified() || action.isDeleted())
-            oldBean = singletonBeans.get(bd.getUid());
-        return oldBean;
+    private @NotNull List<UidAction> cloneUidActions() {
+        // Iterate safely over synchronizedSet
+        final List<UidAction> actions;
+        synchronized (uidsToActivate) {
+            actions = new ArrayList<>(uidsToActivate);
+            uidsToActivate.clear();
+        }
+        return actions;
+    }
+
+    private @Nullable Object getOldBean(WatchAction action, String uid) {
+        return (action.isModified() || action.isDeleted()) ? singletonBeans.get(uid) : null;
     }
 
     @Override