Bug 62740 - XSSFTable constructor automatically assigns invalid (non-unique) column IDs

fix the logic and update the unit test to check for and catch the problem.

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

diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFTable.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFTable.java
index b1af13eaf0cd506043ec77e5c5a61550bdaca57e..411917f32e1d6834e3ea78abd55217b15077616e 100644 (file)
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFTable.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFTable.java
@@ -283,7 +283,7 @@ public class XSSFTable extends POIXMLDocumentPart implements Table {
         }
         
         // check if name is unique and calculate unique column id 
-        long nextColumnId = 1; 
+        long nextColumnId = 0; 
         for (XSSFTableColumn tableColumn : getColumns()) {
             if (columnName != null && columnName.equalsIgnoreCase(tableColumn.getName())) {
                 throw new IllegalArgumentException("Column '" + columnName
@@ -291,6 +291,8 @@ public class XSSFTable extends POIXMLDocumentPart implements Table {
             }
             nextColumnId = Math.max(nextColumnId, tableColumn.getId());
         }
+        // Bug #62740, the logic was just re-using the existing max ID, not incrementing beyond it.
+        nextColumnId++;
         
         // Add the new Column
         CTTableColumn column = columns.insertNewTableColumn(columnIndex);

diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFTable.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFTable.java
index 9e2f9246dd911ab16e2db1c2cf287335c51541c2..e8cda3ee92b9feec42500062c465ecf6a1c29186 100644 (file)
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFTable.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFTable.java
@@ -19,6 +19,7 @@ package org.apache.poi.xssf.usermodel;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
@@ -395,15 +396,21 @@ public final class TestXSSFTable {
         assertEquals(2, table.getRowCount());
 
         // add columns
-        table.createColumn("Column B");
-        table.createColumn("Column D");
-        table.createColumn("Column C", 2); // add between B and D
+        XSSFTableColumn c1 = table.getColumns().get(0);
+        XSSFTableColumn cB = table.createColumn("Column B");
+        XSSFTableColumn cD = table.createColumn("Column D");
+        XSSFTableColumn cC = table.createColumn("Column C", 2); // add between B and D
         table.updateReferences();
         table.updateHeaders();
 
         assertEquals(4, table.getColumnCount());
         assertEquals(2, table.getRowCount());
 
+        // column IDs start at 1, and increase in the order columns are added (see bug #62740)
+        assertEquals("Column c ID", 1, c1.getId());
+        assertTrue("Column B ID", c1.getId() < cB.getId());
+        assertTrue("Column D ID", cB.getId() < cD.getId());
+        assertTrue("Column C ID", cD.getId() < cC.getId());
         assertEquals("Column 1", table.getColumns().get(0).getName()); // generated name
         assertEquals("Column B", table.getColumns().get(1).getName());
         assertEquals("Column C", table.getColumns().get(2).getName());