Changeset 6085

Timestamp:

Aug 17, 2012, 2:04:45 PM (11 years ago)

Author:

Nicklas Nordborg

Message:

References #1707: Make it possible for a derived bioassay to have multiple physical bioassays as parents

The core should now make sure that parent physical bioassays are synchronized on child items if parents are removed or added to a derived bioassay. There might be some optimizations to be done when multiple items are updated in the same transaction, but I'll have to fix the batch importer before this can be tested.

Location:

trunk

Files:

• src/core/net/sf/basedb/core/DerivedBioAssay.java (modified) (7 diffs)
• src/core/net/sf/basedb/core/data/DerivedBioAssayData.java (modified) (1 diff)
• www/views/derivedbioassays/edit_bioassay.jsp (modified) (4 diffs)

trunk/src/core/net/sf/basedb/core/DerivedBioAssay.java

@@ -24 +24 @@
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Date;
 import java.util.HashSet;
+import java.util.IdentityHashMap;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
@@ -437 +443 @@
   }
   
+  public void testFixChildren()
+  {
+    FixDerivedBioAssayParentsRecursivelyAction.get(getDbControl()).add(getData());
+  }
+  
   /**
     Add a physical bioassay as a parent to this derived bioassay.
@@ -452 +463 @@
     if (bioAssay == null) throw new InvalidUseOfNullException("bioAssay");
     bioAssay.checkPermission(Permission.USE);
-    getData().getPhysicalBioAssays().add(bioAssay.getData());
+    boolean wasAdded = getData().getPhysicalBioAssays().add(bioAssay.getData());
+    if (wasAdded)
+    {
+      FixDerivedBioAssayParentsRecursivelyAction.get(getDbControl()).add(getData());
+    }
   }
   
@@ -470 +485 @@
     if (!isRoot()) throw new PermissionDeniedException("Can't remove physical bioassay from non-root derived bioassay");
     if (bioAssay == null) throw new InvalidUseOfNullException("bioAssay");
-    getData().getPhysicalBioAssays().remove(bioAssay.getData());
+    boolean wasRemoved = getData().getPhysicalBioAssays().remove(bioAssay.getData());
+    if (wasRemoved)
+    {
+      FixDerivedBioAssayParentsRecursivelyAction.get(getDbControl()).add(getData());
+    }
   }
   
@@ -521 +540 @@
     if (this.equals(bioAssay)) throw new InvalidDataException("Can't add self as parent: " + this);
     bioAssay.checkPermission(Permission.USE);
-    getData().getParents().add(bioAssay.getData());
-    getData().getPhysicalBioAssays().addAll(bioAssay.getData().getPhysicalBioAssays());
+    boolean wasAdded = getData().getParents().add(bioAssay.getData());
+    if (wasAdded)
+    {
+      FixDerivedBioAssayParentsRecursivelyAction.get(getDbControl()).add(getData());
+    }
   }
 
@@ -539 +561 @@
     if (isRoot()) throw new PermissionDeniedException("Root derived bioassays can't have parent derived bioassays");
     if (bioAssay == null) throw new InvalidUseOfNullException("bioAssay");
-    getData().getParents().remove(bioAssay.getData());
+    boolean wasRemoved = getData().getParents().remove(bioAssay.getData());
+    if (wasRemoved)
+    {
+      FixDerivedBioAssayParentsRecursivelyAction.get(getDbControl()).add(getData());
+    }
   }
 
@@ -773 +799 @@
   }
 
+  /**
+    Whenever we modify the parent items to a derived bioassay,
+    we must make sure that the physical bioassays for all
+    child items are synchronized with the same changes.
+    This is a bit complicated due to the fact that a single transaction
+    may affect multiple derived bioassays at different levels in
+    the parent-child tree. The fix must be made top-down and
+    must handle accidentally created circular references.
+    
+    Whenever a change is deteced a single instance of this
+    class is associated with the DbControl. All changed
+    derived bioassays are collected with the {@link #add(DerivedBioAssayData)}
+    method. The actual fix is done as a last step before committing
+    the transaction {@link #onBeforeCommit()}.
+  */
+  static class FixDerivedBioAssayParentsRecursivelyAction
+    implements TransactionalAction
+  {
+
+    private static Map<DbControl, FixDerivedBioAssayParentsRecursivelyAction> cache = 
+      Collections.synchronizedMap(new IdentityHashMap<DbControl, DerivedBioAssay.FixDerivedBioAssayParentsRecursivelyAction>());
+    
+    /**
+      Get an instance tht handles the fixes for the given DbControl.
+      If no instance exists yet, a new one is created.
+    */
+    static FixDerivedBioAssayParentsRecursivelyAction get(DbControl dc)
+    {
+      FixDerivedBioAssayParentsRecursivelyAction dbt = cache.get(dc);
+      if (dbt == null)
+      {
+        dbt = new FixDerivedBioAssayParentsRecursivelyAction(dc);
+        cache.put(dc, dbt);
+      }
+      return dbt;
+    }
+    
+    private final DbControl dc;
+    private final Set<DerivedBioAssayData> allModified;
+    
+    private FixDerivedBioAssayParentsRecursivelyAction(DbControl dc)
+    {
+      this.dc = dc;
+      this.allModified = new HashSet<DerivedBioAssayData>();
+      dc.addTransactionalAction(this);
+    }
+    
+    /**
+      Register the given derived bioassay as one that has had it's
+      parent items changed.
+    */
+    void add(DerivedBioAssayData dba)
+    {
+      allModified.add(dba);
+    }
+    
+    /*
+      From the TransactionalAction interface
+      --------------------------------------
+    */
+    @Override
+    public void onBeforeCommit() 
+    {
+      
+      Set<DerivedBioAssayData> allToFix = new HashSet<DerivedBioAssayData>();
+      List<DerivedBioAssayData> moreToFix = new LinkedList<DerivedBioAssayData>();
+      
+      // First step is to find all child items recursively
+      // All non-root bioassays need to be fixed and all children may also have to be fixed
+      for (DerivedBioAssayData dba : allModified)
+      {
+        if (!dba.isRoot()) allToFix.add(dba);
+        if (dba.getChildren() != null)
+        {
+          moreToFix.addAll(dba.getChildren());
+        }
+      }
+      
+      // Check each child item and then also grandchildren
+      while (moreToFix.size() > 0)
+      {
+        DerivedBioAssayData dba = moreToFix.remove(0);
+        if (allToFix.add(dba) && dba.getChildren() != null)
+        {
+          moreToFix.addAll(dba.getChildren());
+        }
+      }
+      // We now have all items that must be fixed...
+
+      //System.out.println("allToFix: " + allToFix);
+
+      // We don't know the actual order that items can be fixed, so
+      // we have to keep trying until the set is empty
+      while (allToFix.size() > 0)
+      {
+        int numFixed = 0;
+        Iterator<DerivedBioAssayData> it = allToFix.iterator();
+        while (it.hasNext())
+        {
+          DerivedBioAssayData dba = it.next();
+          // Collect all physical bioassays from all parents in this set
+          Set<PhysicalBioAssayData> allPhysicalBioAssays = new HashSet<PhysicalBioAssayData>();
+          boolean canFix = true;
+          
+          // For each item that should be fixed, load all parent items...
+          for (DerivedBioAssayData parent : dba.getParents())
+          {
+            if (allToFix.contains(parent))
+            {
+              // If at least one parent has not yet been fixed, we can't fix this item now
+              canFix = false;
+              break;
+            }
+            allPhysicalBioAssays.addAll(parent.getPhysicalBioAssays());
+          }
+          
+          if (canFix)
+          {
+            //System.out.println("Fixing: " + dba.getName() + "; " + allPhysicalBioAssays);
+            it.remove();
+            dba.getPhysicalBioAssays().clear();
+            dba.getPhysicalBioAssays().addAll(allPhysicalBioAssays);
+            numFixed++;
+          }
+          else
+          {
+            //System.out.println("Can't fix " + dba.getName() + " yet, since it has unfixed parents");
+          }
+        }
+        // If we couldn't fix any item in the last iteration, but there are more
+        // items to fix, throw an exception since this is an indication of circular
+        // reference
+        if (numFixed == 0 && allToFix.size() > 0)
+        {
+          // Hmm... a circular reference prevents us from fixing 
+          throw new InvalidDataException("Circular reference between derived bioassays: " + allToFix);
+        }
+      }
+      
+    }
+
+    @Override
+    public void onAfterCommit() 
+    {
+      cache.remove(dc);
+    }
+
+    @Override
+    public void onRollback() 
+    {
+      cache.remove(dc);
+    }
+    // ---------------------------------------
+    
+  }
 }

trunk/src/core/net/sf/basedb/core/data/DerivedBioAssayData.java

@@ -157 +157 @@
     @hibernate.collection-many-to-many column="`derivedbioassay_id`" class="net.sf.basedb.core.data.DerivedBioAssayData"
   */
-  Set<DerivedBioAssayData> getChildren()
+  public Set<DerivedBioAssayData> getChildren()
   {
     return children;

trunk/www/views/derivedbioassays/edit_bioassay.jsp

@@ -403 +403 @@
     }
     
-    function selectPhysicalBioAssayOnClick()
-    {
-      var frm = document.forms['bioAssay'];
-      var url = '../physicalbioassays/index.jsp?ID=<%=ID%>&cmd=UpdateContext&mode=selectone';
-      url += '&callback=setPhysicalBioAssayCallback&resetTemporary=1';
-      url += ItemSubtype.createRelatedFilter('bioAssay', 'PHYSICALBIOASSAY');
-      if (frm.physicalbioassay_id.selectedIndex >= 0) 
-      {
-        var id = Math.abs(parseInt(frm.physicalbioassay_id[frm.physicalbioassay_id.selectedIndex].value));        
-        url += '&item_id='+id;
-      }
-      Main.openPopup(url, 'SelectPhysicalBioAssay', 1050, 700);
-    }
-    function setPhysicalBioAssayCallback(id, name)
-    {
-      var frm = document.forms['bioAssay'];
-      var list = frm.physicalbioassay_id;
-      if (list.length < 1 || list[0].value == '0') // >
-      {
-        Forms.addListOption(list, 0, new Option());
-      }
-      list[0].value = id;
-      list[0].text = name;
-      list.selectedIndex = 0;
-      physicalBioAssayOnChange();
-    }
     function physicalBioAssayOnChange()
     {
@@ -435 +409 @@
     }
     
-    function selectParentBioAssayOnClick()
-    {
-      var frm = document.forms['bioAssay'];
-      var url = 'index.jsp?ID=<%=ID%>&cmd=UpdateContext&mode=selectone';
-      url += '&callback=setParentBioAssayCallback&resetTemporary=1';
-      url += ItemSubtype.createRelatedFilter('bioAssay', 'DERIVEDBIOASSAY');
-      if (frm.parents.selectedIndex >= 0) 
-      {
-        var id = Math.abs(parseInt(frm.parents[frm.parents.selectedIndex].value));        
-        url += '&item_id='+id;
-      }
-      Main.openPopup(url, 'SelectParentBioAssay', 1050, 700);
-    }
-    function setParentBioAssayCallback(id, name)
-    {
-      var frm = document.forms['bioAssay'];
-      var list = frm.parents;
-      if (list.length < 1 || list[0].value == '0') // >
-      {
-        Forms.addListOption(list, 0, new Option());
-      }
-      list[0].value = id;
-      list[0].text = name;
-      list.selectedIndex = 0;
-      parentBioAssayOnChange();
-    }
     function parentBioAssayOnChange()
     {
@@ -536 +484 @@
     }
 
+    function removePhysicalBioAssayOnClick()
+    {
+      Link.removeSelected(document.forms['bioAssay'].physicalBioAssays);
+      parentsChanged = true;
+    }
+
+    
     function addParentOnClick()
     {
@@ -553 +508 @@
       parentsChanged = true;
     }
+
+    function removeParentOnClick()
+    {
+      Link.removeSelected(document.forms['bioAssay'].parents);
+      parentsChanged = true;
+    }
+
     
     function init()