From ddef898999e36c7095c276e360334db6dc54e65e Mon Sep 17 00:00:00 2001 From: Greg Woolsey Date: Wed, 19 Sep 2018 17:42:43 +0000 Subject: [PATCH] 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 --- .../org/apache/poi/xssf/usermodel/XSSFTable.java | 4 +++- .../apache/poi/xssf/usermodel/TestXSSFTable.java | 13 ++++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) 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 b1af13eaf0..411917f32e 100644 --- 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 9e2f9246dd..e8cda3ee92 100644 --- 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()); -- 2.39.5