JAL-2541 cutWithFeatures_exhaustive test including dataset maintenance

diff --git a/src/jalview/commands/EditCommand.java b/src/jalview/commands/EditCommand.java
index 874948f..3f2ace9 100644 (file)
--- a/src/jalview/commands/EditCommand.java
+++ b/src/jalview/commands/EditCommand.java
@@ -118,7 +118,7 @@ public class EditCommand implements CommandI
     public abstract Action getUndoAction();
   };
 
-  private List<Edit> edits = new ArrayList<Edit>();
+  private List<Edit> edits = new ArrayList<>();
 
   String description;
 
@@ -526,11 +526,6 @@ public class EditCommand implements CommandI
         command.string[i] = sequence.getSequence(command.position,
                 command.position + command.number);
         SequenceI oldds = sequence.getDatasetSequence();
-        if (command.oldds != null && command.oldds[i] != null)
-        {
-          // we are redoing an undone cut.
-          sequence.setDatasetSequence(null);
-        }
         Range cutPositions = sequence.findPositions(command.position + 1,
                 command.position + command.number);
         boolean cutIsInternal = cutPositions != null
@@ -544,27 +539,23 @@ public class EditCommand implements CommandI
         SequenceI ds = sequence.getDatasetSequence();
         sequence.deleteChars(command.position, command.position
                 + command.number);
-        SequenceI newDs = sequence.getDatasetSequence();
-        if (newDs != ds && command.al != null
-                && command.al.getDataset() != null
-                && !command.al.getDataset().getSequences().contains(newDs))
-        {
-          command.al.getDataset().addSequence(newDs);
-        }
 
         if (command.oldds != null && command.oldds[i] != null)
         {
-          // Undoing previous Paste - so
-          // oldds entry contains the cut dataset sequence,
-          // with sequence features in expected place.
+          /*
+           * we are Redoing a Cut, or Undoing a Paste - so
+           * oldds entry contains the cut dataset sequence,
+           * with sequence features in expected place
+           */
           sequence.setDatasetSequence(command.oldds[i]);
           command.oldds[i] = oldds;
         }
         else
         {
-          // New cut operation
-          // We always keep track of the dataset sequence so we can safely
-          // restore it during the Undo
+          /* 
+           * new cut operation: save the dataset sequence 
+           * so it can be restored in an Undo
+           */
           if (command.oldds == null)
           {
             command.oldds = new SequenceI[command.seqs.length];
@@ -584,6 +575,13 @@ public class EditCommand implements CommandI
             }
           }
         }
+        SequenceI newDs = sequence.getDatasetSequence();
+        if (newDs != ds && command.al != null
+                && command.al.getDataset() != null
+                && !command.al.getDataset().getSequences().contains(newDs))
+        {
+          command.al.getDataset().addSequence(newDs);
+        }
       }
 
       if (sequence.getLength() < 1)
@@ -720,6 +718,12 @@ public class EditCommand implements CommandI
           command.oldds[i] = sequence.getDatasetSequence();
           sameDatasetSequence = ds == sequence.getDatasetSequence();
           ds.setSequenceFeatures(sequence.getSequenceFeatures());
+          if (!sameDatasetSequence && command.al.getDataset() != null)
+          {
+            // delete 'undone' sequence from alignment dataset
+            command.al.getDataset()
+                    .deleteSequence(sequence.getDatasetSequence());
+          }
           sequence.setDatasetSequence(ds);
         }
         undoCutFeatures(command, command.seqs[i], start, length,
@@ -733,7 +737,7 @@ public class EditCommand implements CommandI
 
   static void replace(Edit command)
   {
-    StringBuffer tmp;
+    StringBuilder tmp;
     String oldstring;
     int start = command.position;
     int end = command.number;
@@ -765,10 +769,9 @@ public class EditCommand implements CommandI
               .findPositions(end + 1, command.seqs[i].getLength());
       
       oldstring = command.seqs[i].getSequenceAsString();
-      tmp = new StringBuffer(oldstring.substring(0, start));
+      tmp = new StringBuilder(oldstring.substring(0, start));
       tmp.append(command.string[i]);
-      String nogaprep = jalview.analysis.AlignSeq.extractGaps(
-              jalview.util.Comparison.GapChars,
+      String nogaprep = AlignSeq.extractGaps(Comparison.GapChars,
               new String(command.string[i]));
       if (end < oldstring.length())
       {
@@ -886,8 +889,7 @@ public class EditCommand implements CommandI
               // edit covered both start and end. Here we can only guess the
               // new
               // start/end
-              String nogapalseq = jalview.analysis.AlignSeq.extractGaps(
-                      jalview.util.Comparison.GapChars,
+              String nogapalseq = AlignSeq.extractGaps(Comparison.GapChars,
                       command.seqs[i].getSequenceAsString().toUpperCase());
               int newStart = command.seqs[i].getDatasetSequence()
                       .getSequenceAsString().indexOf(nogapalseq);
@@ -917,7 +919,7 @@ public class EditCommand implements CommandI
     if (modifyVisibility && !insert)
     {
       // only occurs if a sequence was added or deleted.
-      command.deletedAnnotationRows = new Hashtable<SequenceI, AlignmentAnnotation[]>();
+      command.deletedAnnotationRows = new Hashtable<>();
     }
     if (command.fullAlignmentHeight)
     {
@@ -1075,7 +1077,7 @@ public class EditCommand implements CommandI
 
     if (!insert)
     {
-      command.deletedAnnotations = new Hashtable<String, Annotation[]>();
+      command.deletedAnnotations = new Hashtable<>();
     }
 
     int aSize;
@@ -1359,7 +1361,7 @@ public class EditCommand implements CommandI
    */
   public Map<SequenceI, SequenceI> priorState(boolean forUndo)
   {
-    Map<SequenceI, SequenceI> result = new HashMap<SequenceI, SequenceI>();
+    Map<SequenceI, SequenceI> result = new HashMap<>();
     if (getEdits() == null)
     {
       return result;
@@ -1392,7 +1394,7 @@ public class EditCommand implements CommandI
      * Work backwards through the edit list, deriving the sequences before each
      * was applied. The final result is the sequence set before any edits.
      */
-    Iterator<Edit> editList = new ReverseListIterator<Edit>(getEdits());
+    Iterator<Edit> editList = new ReverseListIterator<>(getEdits());
     while (editList.hasNext())
     {
       Edit oldEdit = editList.next();
@@ -1570,7 +1572,7 @@ public class EditCommand implements CommandI
     }
     else
     {
-      return new ReverseListIterator<Edit>(getEdits());
+      return new ReverseListIterator<>(getEdits());
     }
   }
 

diff --git a/test/jalview/commands/EditCommandTest.java b/test/jalview/commands/EditCommandTest.java
index c13e729..9c91705 100644 (file)
--- a/test/jalview/commands/EditCommandTest.java
+++ b/test/jalview/commands/EditCommandTest.java
@@ -153,18 +153,23 @@ public class EditCommandTest
   }
 
   /**
-   * Test a Paste action, where this adds sequences to an alignment.
+   * Test a Paste action, followed by Undo and Redo
    */
-  @Test(groups = { "Functional" }, enabled = true)
-  // TODO fix so it works
-  public void testPaste_addToAlignment()
+  @Test(groups = { "Functional" }, enabled = false)
+  public void testPaste_undo_redo()
   {
+    // TODO code this test properly, bearing in mind that:
+    // Paste action requires something on the clipboard (Cut/Copy)
+    // - EditCommand.paste doesn't add sequences to the alignment
+    // ... that is done in AlignFrame.paste()
+    // ... unless as a Redo
+    // ...
+
     SequenceI[] newSeqs = new SequenceI[2];
     newSeqs[0] = new Sequence("newseq0", "ACEFKL");
     newSeqs[1] = new Sequence("newseq1", "JWMPDH");
 
-    Edit ec = testee.new Edit(Action.PASTE, newSeqs, 0, al.getWidth(), al);
-    EditCommand.paste(ec, new AlignmentI[] { al });
+    new EditCommand("Paste", Action.PASTE, newSeqs, 0, al.getWidth(), al);
     assertEquals(6, al.getSequences().size());
     assertEquals("1234567890", seqs[3].getSequenceAsString());
     assertEquals("ACEFKL", seqs[4].getSequenceAsString());
@@ -308,9 +313,12 @@ public class EditCommandTest
     assertEquals("ABXYZDEF", seq.getDatasetSequence().getSequenceAsString());
     assertEquals(8, seq.getDatasetSequence().getEnd());
 
-    edit.undoCommand(
-            new AlignmentI[]
-            { new Alignment(new SequenceI[] { seq }) });
+    /*
+     * undo the edit
+     */
+    AlignmentI[] views = new AlignmentI[]
+    { new Alignment(new SequenceI[] { seq }) };
+    edit.undoCommand(views);
 
     assertEquals("ABC--DEF", seq.getSequenceAsString());
     assertEquals("ABCDEF", seq.getDatasetSequence().getSequenceAsString());
@@ -318,9 +326,10 @@ public class EditCommandTest
     assertEquals(6, seq.getEnd());
     assertEquals(6, seq.getDatasetSequence().getEnd());
 
-    edit.undoCommand(
-            new AlignmentI[]
-            { new Alignment(new SequenceI[] { seq }) });
+    /*
+     * redo the edit
+     */
+    edit.doCommand(views);
 
     assertEquals("ABXYZ-DEF", seq.getSequenceAsString());
     assertEquals(1, seq.getStart());
@@ -911,10 +920,11 @@ public class EditCommandTest
           /*
            * verify a new dataset sequence has appeared
            */
-          assertEquals("Wrong Dataset size after cut",
+          assertEquals("Wrong Dataset size after " + msg,
                   newDatasetSequence ? 2 : 1, alignment.getDataset()
                           .getHeight());
         }
+
         /*
          * undo and verify all restored
          */
@@ -936,7 +946,7 @@ public class EditCommandTest
           /*
            * verify dataset sequence has shrunk
            */
-          assertEquals("Wrong Dataset size after cut", 1,
+          assertEquals("Wrong Dataset size after Undo of " + msg, 1,
                   alignment.getDataset().getHeight());
         }
 
@@ -958,7 +968,7 @@ public class EditCommandTest
           /*
            * verify a new dataset sequence has appeared again
            */
-          assertEquals("Wrong Dataset size after cut",
+          assertEquals("Wrong Dataset size after Redo of " + msg,
                   newDatasetSequence ? 2 : 1, alignment.getDataset()
                           .getHeight());
         }
@@ -979,7 +989,7 @@ public class EditCommandTest
           /*
            * and that dataset sequence has shrunk
            */
-          assertEquals("Wrong Dataset size after cut", 1,
+          assertEquals("Wrong Dataset size after final Undo of " + msg, 1,
                   alignment.getDataset().getHeight());
         }
       }