fix POI Bug 60197 - setSheetOrder doesn't update names scope

main/HSSF/UserModel/HSSFWorkbook.cs

@@ -460,20 +460,64 @@ public void SetSheetOrder(String sheetname, int pos)
             }
 
             workbook.UpdateNamesAfterCellShift(shifter);
+            UpdateNamedRangesAfterSheetReorder(oldSheetIndex, pos);
 
+            UpdateActiveSheetAfterSheetReorder(oldSheetIndex, pos);
+        }
+
+        /**
+         * copy-pasted from XSSFWorkbook#updateNamedRangesAfterSheetReorder(int, int)
+         * <p>
+         * update sheet-scoped named ranges in this workbook after changing the sheet order
+         * of a sheet at oldIndex to newIndex.
+         * Sheets between these indices will move left or right by 1.
+         *
+         * @param oldIndex the original index of the re-ordered sheet
+         * @param newIndex the new index of the re-ordered sheet
+         */
+        private void UpdateNamedRangesAfterSheetReorder(int oldIndex, int newIndex)
+        {
+            // update sheet index of sheet-scoped named ranges
+            foreach (HSSFName name in names)
+            {
+                int i = name.SheetIndex;
+                // name has sheet-level scope
+                if (i != -1)
+                {
+                    // name refers to this sheet
+                    if (i == oldIndex)
+                    {
+                        name.SheetIndex = newIndex;
+                    }
+                    // if oldIndex > newIndex then this sheet moved left and sheets between newIndex and oldIndex moved right
+                    else if (newIndex <= i && i < oldIndex)
+                    {
+                        name.SheetIndex = i + 1;
+                    }
+                    // if oldIndex < newIndex then this sheet moved right and sheets between oldIndex and newIndex moved left
+                    else if (oldIndex < i && i <= newIndex)
+                    {
+                        name.SheetIndex = i - 1;
+                    }
+                }
+            }
+        }
+
+        private void UpdateActiveSheetAfterSheetReorder(int oldIndex, int newIndex)
+        {
             // adjust active sheet if necessary
             int active = ActiveSheetIndex;
-            if (active == oldSheetIndex)
+            if (active == oldIndex)
             {
                 // moved sheet was the active one
-                SetActiveSheet(pos);
+                SetActiveSheet(newIndex);
             }
-            else if ((active < oldSheetIndex && active < pos) ||
-                  (active > oldSheetIndex && active > pos))
+            else if ((active < oldIndex && active < newIndex) ||
+                     (active > oldIndex && active > newIndex))
             {
                 // not affected
             }
-            else if (pos > oldSheetIndex)
+            else if (newIndex > oldIndex)
             {
                 // moved sheet was below before and is above now => active is one less
                 SetActiveSheet(active - 1);

ooxml/XSSF/UserModel/XSSFWorkbook.cs

@@ -1665,6 +1665,7 @@ public void SetSheetOrder(String sheetname, int pos)
             XSSFSheet sheet = sheets[idx];
             sheets.RemoveAt(idx);
             sheets.Insert(pos, sheet);
+
             // Reorder CT_Sheets
             CT_Sheets ct = workbook.sheets;
             CT_Sheet cts = ct.GetSheetArray(idx).Copy();
@@ -1674,34 +1675,75 @@ public void SetSheetOrder(String sheetname, int pos)
 
             //notify sheets
             List<CT_Sheet> sheetArray = ct.sheet;
-            for (int i = 0; i < sheets.Count; i++)
+            for (int i = 0; i < sheetArray.Count; i++)
             {
                 sheets[i].sheet = sheetArray[i];
             }
 
+            UpdateNamedRangesAfterSheetReorder(idx, pos);
+            UpdateActiveSheetAfterSheetReorder(idx, pos);
+        }
+
+        /**
+         * update sheet-scoped named ranges in this workbook after changing the sheet order
+         * of a sheet at oldIndex to newIndex.
+         * Sheets between these indices will move left or right by 1.
+         *
+         * @param oldIndex the original index of the re-ordered sheet
+         * @param newIndex the new index of the re-ordered sheet
+         */
+        private void UpdateNamedRangesAfterSheetReorder(int oldIndex, int newIndex)
+        {
+            // update sheet index of sheet-scoped named ranges
+            foreach (XSSFName name in namedRanges)
+            {
+                int i = name.SheetIndex;
+                // name has sheet-level scope
+                if (i != -1)
+                {
+                    // name refers to this sheet
+                    if (i == oldIndex)
+                    {
+                        name.SheetIndex = newIndex;
+                    }
+                    // if oldIndex > newIndex then this sheet moved left and sheets between newIndex and oldIndex moved right
+                    else if (newIndex <= i && i < oldIndex)
+                    {
+                        name.SheetIndex = i + 1;
+                    }
+                    // if oldIndex < newIndex then this sheet moved right and sheets between oldIndex and newIndex moved left
+                    else if (oldIndex < i && i <= newIndex)
+                    {
+                        name.SheetIndex = i - 1;
+                    }
+                }
+            }
+        }
+
+        private void UpdateActiveSheetAfterSheetReorder(int oldIndex, int newIndex)
+        {
             // adjust active sheet if necessary
             int active = ActiveSheetIndex;
-            if (active == idx)
+            if (active == oldIndex)
             {
-                // Moved sheet was the active one
-                SetActiveSheet(pos);
+                // moved sheet was the active one
+                SetActiveSheet(newIndex);
             }
-            else if ((active < idx && active < pos) ||
-                  (active > idx && active > pos))
+            else if ((active < oldIndex && active < newIndex) ||
+                     (active > oldIndex && active > newIndex))
             {
                 // not affected
             }
-            else if (pos > idx)
+            else if (newIndex > oldIndex)
             {
-                // Moved sheet was below before and is above now => active is one less
+                // moved sheet was below before and is above now => active is one less
                 SetActiveSheet(active - 1);
             }
             else
             {
-                // remaining case: Moved sheet was higher than active before and is lower now => active is one more
+                // remaining case: moved sheet was higher than active before and is lower now => active is one more
                 SetActiveSheet(active + 1);
             }
-
         }
 
         /**

testcases/main/SS/UserModel/BaseTestBugzillaIssues.cs

@@ -348,7 +348,7 @@ private static String CreateFunction(String name, int maxArgs)
             return fmla.ToString();
         }
 
-        
+
         [Test]
         public void Bug50681_TestAutoSize()
         {
@@ -481,7 +481,7 @@ private double ComputeCellWidthManually(ICell cell0, IFont font)
             //TextLayout layout = new TextLayout(str.getIterator(), fontRenderContext);
             //width = ((layout.getBounds().getWidth() / 1) / 8);
             Font wfont = SheetUtil.IFont2Font(font);
-            width= (double)TextMeasurer.Measure(txt, new TextOptions(wfont)).Width;
+            width = (double)TextMeasurer.Measure(txt, new TextOptions(wfont)).Width;
             return width;
         }
 
@@ -492,7 +492,7 @@ private double ComputeCellWidthFixed(IFont font, String txt)
             width = (double)TextMeasurer.Measure(txt, new TextOptions(wfont)).Width;
             return width;
         }
-        
+
         //private static void copyAttributes(Font font, AttributedString str, int startIdx, int endIdx)
         //{
         //    str.addAttribute(TextAttribute.FAMILY, font.getFontName(), startIdx, endIdx);
@@ -1098,7 +1098,7 @@ public void Test57973()
             IFont font = wb.CreateFont();
             font.FontName = ("Arial");
             font.FontHeightInPoints = ((short)14);
-            font.IsBold=true;
+            font.IsBold = true;
             font.Color = (IndexedColors.Red.Index);
             str2.ApplyFont(font);
 
@@ -1660,5 +1660,102 @@ private void checkFormulaPreevaluatedString(IWorkbook readFile)
             }
         }
 
+        // bug 60197: setSheetOrder should update sheet-scoped named ranges to maintain references to the sheets before the re-order
+        [Test]
+        public virtual void bug60197_NamedRangesReferToCorrectSheetWhenSheetOrderIsChanged()
+        {
+            using (IWorkbook wb = _testDataProvider.CreateWorkbook())
+            {
+                ISheet sheet1 = wb.CreateSheet("Sheet1");
+                ISheet sheet2 = wb.CreateSheet("Sheet2");
+                ISheet sheet3 = wb.CreateSheet("Sheet3");
+
+                IName nameOnSheet1 = wb.CreateName();
+                nameOnSheet1.SheetIndex = wb.GetSheetIndex(sheet1);
+                nameOnSheet1.NameName = "NameOnSheet1";
+                nameOnSheet1.RefersToFormula = "Sheet1!A1";
+
+                IName nameOnSheet2 = wb.CreateName();
+                nameOnSheet2.SheetIndex = wb.GetSheetIndex(sheet2);
+                nameOnSheet2.NameName = "NameOnSheet2";
+                nameOnSheet2.RefersToFormula = "Sheet2!A1";
+
+                IName nameOnSheet3 = wb.CreateName();
+                nameOnSheet3.SheetIndex = wb.GetSheetIndex(sheet3);
+                nameOnSheet3.NameName = "NameOnSheet3";
+                nameOnSheet3.RefersToFormula = "Sheet3!A1";
+
+                // workbook-scoped name
+                IName name = wb.CreateName();
+                name.NameName = "WorkbookScopedName";
+                name.RefersToFormula = "Sheet2!A1";
+
+                Assert.AreEqual("Sheet1", nameOnSheet1.SheetName);
+                Assert.AreEqual("Sheet2", nameOnSheet2.SheetName);
+                Assert.AreEqual("Sheet3", nameOnSheet3.SheetName);
+                Assert.AreEqual(-1, name.SheetIndex);
+                Assert.AreEqual("Sheet2!A1", name.RefersToFormula);
+
+                // rearrange the sheets several times to make sure the names always refer to the right sheet
+                for (int i = 0; i <= 9; i++)
+                {
+                    wb.SetSheetOrder("Sheet3", i % 3);
+
+                    // Current bug in XSSF:
+                    // Call stack:
+                    //   XSSFWorkbook.write(OutputStream)
+                    //   XSSFWorkbook.commit()
+                    //   XSSFWorkbook.saveNamedRanges()
+                    // This dumps the current namedRanges to CTDefinedName and writes these to the CTWorkbook
+                    // Then the XSSFName namedRanges list is cleared and rebuilt
+                    // Thus, any XSSFName object becomes invalid after a write
+                    // This re-assignment to the XSSFNames is not necessary if wb.write is not called.
+                    nameOnSheet1 = wb.GetName("NameOnSheet1");
+                    nameOnSheet2 = wb.GetName("NameOnSheet2");
+                    nameOnSheet3 = wb.GetName("NameOnSheet3");
+                    name = wb.GetName("WorkbookScopedName");
+
+                    // The name should still refer to the same sheet after the sheets are re-ordered
+                    Assert.AreEqual(i % 3, wb.GetSheetIndex("Sheet3"));
+                    Assert.AreEqual("Sheet1", nameOnSheet1.SheetName);
+                    Assert.AreEqual("Sheet2", nameOnSheet2.SheetName);
+                    Assert.AreEqual("Sheet3", nameOnSheet3.SheetName);
+                    Assert.AreEqual(-1, name.SheetIndex);
+                    Assert.AreEqual("Sheet2!A1", name.RefersToFormula);
+
+                    // make sure the changes to the names stick after writing out the workbook
+                    using (IWorkbook wb2 = _testDataProvider.WriteOutAndReadBack(wb))
+                    {
+                        // See note above. XSSFNames become invalid after workbook write
+                        // Without reassignment here, an XmlValueDisconnectedException may occur
+                        nameOnSheet1 = wb2.GetName("NameOnSheet1");
+                        nameOnSheet2 = wb2.GetName("NameOnSheet2");
+                        nameOnSheet3 = wb2.GetName("NameOnSheet3");
+                        name = wb2.GetName("WorkbookScopedName");
+
+                        // Saving the workbook should not change the sheet names
+                        Assert.AreEqual(i % 3, wb2.GetSheetIndex("Sheet3"));
+                        Assert.AreEqual("Sheet1", nameOnSheet1.SheetName);
+                        Assert.AreEqual("Sheet2", nameOnSheet2.SheetName);
+                        Assert.AreEqual("Sheet3", nameOnSheet3.SheetName);
+                        Assert.AreEqual(-1, name.SheetIndex);
+                        Assert.AreEqual("Sheet2!A1", name.RefersToFormula);
+
+                        // Verify names in wb2
+                        nameOnSheet1 = wb2.GetName("NameOnSheet1");
+                        nameOnSheet2 = wb2.GetName("NameOnSheet2");
+                        nameOnSheet3 = wb2.GetName("NameOnSheet3");
+                        name = wb2.GetName("WorkbookScopedName");
+
+                        Assert.AreEqual(i % 3, wb2.GetSheetIndex("Sheet3"));
+                        Assert.AreEqual("Sheet1", nameOnSheet1.SheetName);
+                        Assert.AreEqual("Sheet2", nameOnSheet2.SheetName);
+                        Assert.AreEqual("Sheet3", nameOnSheet3.SheetName);
+                        Assert.AreEqual(-1, name.SheetIndex);
+                        Assert.AreEqual("Sheet2!A1", name.RefersToFormula);
+                    }
+                }
+            }
+        }
     }
-}
+}

testcases/ooxml/XSSF/Streaming/TestSXSSFBugs.cs

@@ -1,8 +1,10 @@
 using NPOI.SS.UserModel;
 using NPOI.SS.Util;
+using NPOI.Util;
 using NPOI.XSSF;
 using NPOI.XSSF.Streaming;
 using NUnit.Framework;
+using System.IO;
 using TestCases.SS.UserModel;
 
 namespace TestCases.XSSF.Streaming
@@ -24,7 +26,7 @@ public TestSXSSFBugs()
         [Ignore("Reading data is not supported")] public override void Bug57798() { /* Reading data is not supported */ }
 
         [Test]
-        public void Tug49253()
+        public void Bug49253()
         {
             IWorkbook wb1 = new SXSSFWorkbook();
             IWorkbook wb2 = new SXSSFWorkbook();
@@ -57,5 +59,30 @@ public void Tug49253()
             wb1.Close();
             wb2.Close();
         }
+
+        // bug 60197: setSheetOrder should update sheet-scoped named ranges to maintain references to the sheets before the re-order
+        [Test]
+        override
+        public void bug60197_NamedRangesReferToCorrectSheetWhenSheetOrderIsChanged()
+        {
+            try
+            {
+                base.bug60197_NamedRangesReferToCorrectSheetWhenSheetOrderIsChanged();
+            }
+            catch (RuntimeException e)
+            {
+                var cause = e.InnerException;
+                if (cause is IOException && cause.Message == "Stream closed")
+                {
+                    // expected on the second time that _testDataProvider.writeOutAndReadBack(SXSSFWorkbook) is called
+                    // if the test makes it this far, then we know that XSSFName sheet indices are updated when sheet
+                    // order is changed, which is the purpose of this test. Therefore, consider this a passing test.
+                }
+                else
+                {
+                    throw e;
+                }
+            }
+        }
     }
 }