fixed bug #45728: SlideShow.reorderSlide didn't work properly

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@691533 13f79535-47bb-0310-9956-ffa450edef68

src/documentation/content/xdocs/changes.xml

@@ -37,6 +37,7 @@
 
 		<!-- Don't forget to update status.xml too! -->
         <release version="3.1.1-alpha1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">45728 Fix for SlideShow.reorderSlide in HSLF</action>
            <action dev="POI-DEVELOPERS" type="add">Initial support for embedded movies and controls in HSLF</action>
            <action dev="POI-DEVELOPERS" type="fix">45358 - signed/unsigned error when parsing 3-d area refs, performance problem evaluating area refs, and ClassCastExcecption in IF()</action>
            <action dev="POI-DEVELOPERS" type="add">Support for HPBF Publisher hyperlinks, including during text extraction</action>

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.1.1-alpha1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">45728 Fix for SlideShow.reorderSlide in HSLF</action>
            <action dev="POI-DEVELOPERS" type="add">Initial support for embedded movies and controls in HSLF</action>
            <action dev="POI-DEVELOPERS" type="fix">45358 - signed/unsigned error when parsing 3-d area refs, performance problem evaluating area refs, and ClassCastExcecption in IF()</action>
            <action dev="POI-DEVELOPERS" type="add">Support for HPBF Publisher hyperlinks, including during text extraction</action>

src/scratchpad/src/org/apache/poi/hslf/record/Document.java

@@ -71,27 +71,44 @@ public class Document extends PositionDependentRecordContainer
 	 * This will normally return an array of size 2 or 3
 	 */
 	public SlideListWithText[] getSlideListWithTexts() { return slwts; }
-	/**
+
+    /**
 	 * Returns the SlideListWithText that deals with the
 	 *  Master Slides 
 	 */
 	public SlideListWithText getMasterSlideListWithText() { 
-		if(slwts.length > 0) { return slwts[0]; }
+        for (int i = 0; i < slwts.length; i++) {
-		return null; }
+            if(slwts[i].getInstance() == SlideListWithText.MASTER) {
+                return slwts[i];
+            }
+        }
+        return null;
+    }
+
 	/**
 	 * Returns the SlideListWithText that deals with the
 	 *  Slides, or null if there isn't one
 	 */
-	public SlideListWithText getSlideSlideListWithText() { 
+	public SlideListWithText getSlideSlideListWithText() {
-		if(slwts.length > 1) { return slwts[1]; }
+        for (int i = 0; i < slwts.length; i++) {
-		return null; }
+            if(slwts[i].getInstance() == SlideListWithText.SLIDES) {
+                return slwts[i];
+            }
+        }
+		return null;
+    }
 	/**
 	 * Returns the SlideListWithText that deals with the
 	 *  notes, or null if there isn't one
 	 */
 	public SlideListWithText getNotesSlideListWithText() {
-		if(slwts.length > 2) { return slwts[2]; }
+        for (int i = 0; i < slwts.length; i++) {
-		return null; }
+            if(slwts[i].getInstance() == SlideListWithText.NOTES) {
+                return slwts[i];
+            }
+        }
+		return null;
+    }
 
 
 	/** 

src/scratchpad/src/org/apache/poi/hslf/record/RecordContainer.java

@@ -243,8 +243,16 @@ public abstract class RecordContainer extends Record
 			moveChildRecords(oldLoc, newLoc, number);
 		}
 	}
-	
+
-	
+    /**
+     * Set child records. 
+     *
+     * @param records   the new child records
+     */
+    public void setChildRecord(Record[] records) {
+        this._children = records;
+    }
+
 	/* ===============================================================
 	 *                 External Serialisation Methods
 	 * ===============================================================

src/scratchpad/src/org/apache/poi/hslf/record/SlideListWithText.java

@@ -50,7 +50,24 @@ import java.util.Vector;
 // For now, pretend to be an atom
 public class SlideListWithText extends RecordContainer
 {
-	private byte[] _header;
+
+    /**
+     * Instance filed of the record header indicates that this SlideListWithText stores
+     * references to slides
+     */
+    public static final int SLIDES = 0;
+    /**
+     * Instance filed of the record header indicates that this SlideListWithText stores
+     * references to master slides
+     */
+    public static final int MASTER = 1;
+    /**
+     * Instance filed of the record header indicates that this SlideListWithText stores
+     * references to notes
+     */
+    public static final int NOTES = 2;
+
+    private byte[] _header;
 	private static long _type = 4080;
 
 	private SlideAtomsSet[] slideAtomsSets;
@@ -123,9 +140,9 @@ public class SlideListWithText extends RecordContainer
 	public void addSlidePersistAtom(SlidePersistAtom spa) {
 		// Add the new SlidePersistAtom at the end
 		appendChildRecord(spa);
-		
+
 		SlideAtomsSet newSAS = new SlideAtomsSet(spa, new Record[0]);
-		
+
 		// Update our SlideAtomsSets with this
 		SlideAtomsSet[] sas = new SlideAtomsSet[slideAtomsSets.length+1];
 		System.arraycopy(slideAtomsSets, 0, sas, 0, slideAtomsSets.length);
@@ -133,7 +150,15 @@ public class SlideListWithText extends RecordContainer
 		slideAtomsSets = sas;
 	}
 
-	/**
+    public int getInstance(){
+        return LittleEndian.getShort(_header, 0) >> 4;
+    }
+
+    public void setInstance(int inst){
+        LittleEndian.putShort(_header, (short)((inst << 4) | 0xF));
+    }
+
+    /**
 	 * Get access to the SlideAtomsSets of the children of this record
 	 */
 	public SlideAtomsSet[] getSlideAtomsSets() { return slideAtomsSets; }
@@ -152,35 +177,6 @@ public class SlideListWithText extends RecordContainer
 	}
 
 	/**
-	 * Shifts a SlideAtomsSet to a new position.
-	 * Works by shifting the child records about, then updating
-	 *  the SlideAtomSets array
-	 * @param toMove The SlideAtomsSet to move
-	 * @param newPosition The new (0 based) position for the SlideAtomsSet
-	 */
-	public void repositionSlideAtomsSet(SlideAtomsSet toMove, int newPosition) {
-		// Ensure it's one of ours
-		int curPos = -1;
-		for(int i=0; i<slideAtomsSets.length; i++) {
-			if(slideAtomsSets[i] == toMove) { curPos = i; }
-		}
-		if(curPos == -1) {
-			throw new IllegalArgumentException("The supplied SlideAtomsSet didn't belong to this SlideListWithText");
-		}
-		
-		// Ensure the newPosision is valid
-		if(newPosition < 0 || newPosition >= slideAtomsSets.length) {
-			throw new IllegalArgumentException("The new position must be between 0, and the number of SlideAtomsSets");
-		}
-		
-		// Build the new records list
-		moveChildrenBefore(toMove.getSlidePersistAtom(), toMove.slideRecords.length, slideAtomsSets[newPosition].getSlidePersistAtom());
-		
-		// Build the new SlideAtomsSets list
-		ArrayUtil.arrayMoveWithin(slideAtomsSets, curPos, newPosition, 1);
-	}
-
-	/** 
 	 * Inner class to wrap up a matching set of records that hold the
 	 *  text for a given sheet. Contains the leading SlidePersistAtom,
 	 *  and all of the records until the next SlidePersistAtom. This 

src/scratchpad/src/org/apache/poi/hslf/usermodel/SlideShow.java

@@ -536,32 +536,37 @@ public final class SlideShow {
 	
 	/**
 	 * Re-orders a slide, to a new position.
-	 * @param oldSlideNumer The old slide number (1 based)
+	 * @param oldSlideNumber The old slide number (1 based)
 	 * @param newSlideNumber The new slide number (1 based)
 	 */
-	public void reorderSlide(int oldSlideNumer, int newSlideNumber) {
+	public void reorderSlide(int oldSlideNumber, int newSlideNumber) {
 		// Ensure these numbers are valid
-		if(oldSlideNumer < 1 || newSlideNumber < 1) {
+		if(oldSlideNumber < 1 || newSlideNumber < 1) {
 			throw new IllegalArgumentException("Old and new slide numbers must be greater than 0");
 		}
-		if(oldSlideNumer > _slides.length || newSlideNumber > _slides.length) {
+		if(oldSlideNumber > _slides.length || newSlideNumber > _slides.length) {
 			throw new IllegalArgumentException("Old and new slide numbers must not exceed the number of slides (" + _slides.length + ")");
 		}
-		
+
-		// Shift the SlideAtomsSet
+        //  The order of slides is defined by the order of slide atom sets in the SlideListWithText container.
-		SlideListWithText slwt = _documentRecord.getSlideSlideListWithText(); 
+        SlideListWithText slwt = _documentRecord.getSlideSlideListWithText();
-		slwt.repositionSlideAtomsSet( 
+        SlideAtomsSet[] sas = slwt.getSlideAtomsSets();
-				slwt.getSlideAtomsSets()[(oldSlideNumer-1)],
+
-				(newSlideNumber-1)
+        SlideAtomsSet tmp = sas[oldSlideNumber-1];
-		);
+        sas[oldSlideNumber-1] = sas[newSlideNumber-1];
-		
+        sas[newSlideNumber-1] = tmp;
-		// Re-order the slides
+
-		ArrayUtil.arrayMoveWithin(_slides, (oldSlideNumer-1), (newSlideNumber-1), 1);
+        ArrayList lst = new ArrayList();
-		
+        for (int i = 0; i < sas.length; i++) {
-		// Tell the appropriate slides their new numbers
+            lst.add(sas[i].getSlidePersistAtom());
-		for(int i=0; i<_slides.length; i++) {
+            Record[] r = sas[i].getSlideRecords();
-			_slides[i].setSlideNumber( (i+1) );
+            for (int j = 0; j < r.length; j++) {
-		}
+                lst.add(r[j]);
+            }
+            _slides[i].setSlideNumber(i+1);
+        }
+        Record[] r = (Record[])lst.toArray(new Record[lst.size()]);
+        slwt.setChildRecord(r);
 	}
 
 	/* ===============================================================
@@ -585,7 +590,8 @@ public final class SlideShow {
   		if(slist == null) {
   			// Need to add a new one
   			slist = new SlideListWithText();
-  			_documentRecord.addSlideListWithText(slist);
+  			slist.setInstance(SlideListWithText.SLIDES);
+            _documentRecord.addSlideListWithText(slist);
   		}
 
   		// Grab the SlidePersistAtom with the highest Slide Number.
@@ -678,11 +684,11 @@ public final class SlideShow {
 		ptr.addSlideLookup(sp.getRefID(), slideOffset);
 		logger.log(POILogger.INFO, "New slide ended up at " + slideOffset);
 
-  		// All done and added
+        slide.setMasterSheet(_masters[0]);
+          // All done and added
   		return slide;
 	}
 
-
     /**
      * Adds a picture to this presentation and returns the associated index.
      *

src/scratchpad/testcases/org/apache/poi/hslf/usermodel/TestReOrderingSlides.java

@@ -279,18 +279,23 @@ public class TestReOrderingSlides extends TestCase {
 		assertEquals(3, ss_read.getSlides().length);
 		
 		// And check it's as expected
-		s1 = ss_read.getSlides()[0];
+		Slide _s1 = ss_read.getSlides()[0];
-		s2 = ss_read.getSlides()[1];
+		Slide _s2 = ss_read.getSlides()[1];
-		s3 = ss_read.getSlides()[2];
+		Slide _s3 = ss_read.getSlides()[2];
-		
+
-		assertEquals(257, s1._getSheetNumber());
+        // 1 --> 3
-		assertEquals(4, s1._getSheetRefId());
+        assertEquals(s1._getSheetNumber(), _s3._getSheetNumber());
+		assertEquals(s1._getSheetRefId(), _s3._getSheetRefId());
 		assertEquals(1, s1.getSlideNumber());
-		assertEquals(256, s2._getSheetNumber());
+
-		assertEquals(3, s2._getSheetRefId());
+        // 2nd slide is not updated
+        assertEquals(s2._getSheetNumber(), _s2._getSheetNumber());
+        assertEquals(s2._getSheetRefId(), _s2._getSheetRefId());
 		assertEquals(2, s2.getSlideNumber());
-		assertEquals(258, s3._getSheetNumber());
+
-		assertEquals(5, s3._getSheetRefId());
+        // 3 --> 1
+        assertEquals(s3._getSheetNumber(), _s1._getSheetNumber());
+        assertEquals(s3._getSheetRefId(), _s1._getSheetRefId());
 		assertEquals(3, s3.getSlideNumber());
 	}
 }