diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index f0c5d67ac..d0c00a646 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -50,6 +50,8 @@ Created a common interface for handling Excel files, irrespective of if they are .xls or .xlsx + 45126 - Avoid generating multiple NamedRanges with the same name, which Excel dislikes + Fix cell.getRichStringCellValue() for formula cells with string results 45365 - Handle more excel number formatting rules in FormatTrackingHSSFListener / XLS2CSVmra 45373 - Improve the performance of HSSFSheet.shiftRows 45367 - Fixed bug when last row removed from sheet is row zero diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 693fc42e2..78937b325 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -47,6 +47,8 @@ Created a common interface for handling Excel files, irrespective of if they are .xls or .xlsx + 45126 - Avoid generating multiple NamedRanges with the same name, which Excel dislikes + Fix cell.getRichStringCellValue() for formula cells with string results 45365 - Handle more excel number formatting rules in FormatTrackingHSSFListener / XLS2CSVmra 45373 - Improve the performance of HSSFSheet.shiftRows 45367 - Fixed bug when last row removed from sheet is row zero diff --git a/src/java/org/apache/poi/hssf/model/LinkTable.java b/src/java/org/apache/poi/hssf/model/LinkTable.java index a19971b7d..1d8781d97 100755 --- a/src/java/org/apache/poi/hssf/model/LinkTable.java +++ b/src/java/org/apache/poi/hssf/model/LinkTable.java @@ -63,6 +63,8 @@ import org.apache.poi.hssf.record.SupBookRecord; * @author Josh Micich */ final class LinkTable { + + // TODO make this class into a record aggregate private static final class CRNBlock { @@ -233,13 +235,39 @@ final class LinkTable { if (idx == -1) idx = findFirstRecordLocBySid(CountryRecord.sid); int countNames = _definedNames.size(); _workbookRecordList.add(idx+countNames, name); - } public void removeName(int namenum) { _definedNames.remove(namenum); } + /** + * checks if the given name is already included in the linkTable + */ + public boolean nameAlreadyExists(NameRecord name) + { + // Check to ensure no other names have the same case-insensitive name + for ( int i = getNumNames()-1; i >=0; i-- ) { + NameRecord rec = getNameRecord(i); + if (rec != name) { + if (isDuplicatedNames(name, rec)) + return true; + } + } + return false; + } + + private boolean isDuplicatedNames(NameRecord firstName, NameRecord lastName) + { + return lastName.getNameText().equalsIgnoreCase(firstName.getNameText()) + && isSameSheetNames(firstName, lastName); + } + private boolean isSameSheetNames(NameRecord firstName, NameRecord lastName) + { + return lastName.getEqualsToIndexToSheet() == firstName.getEqualsToIndexToSheet(); + } + + public short getIndexToSheet(short num) { return _externSheetRecord.getREFRecordAt(num).getIndexToFirstSupBook(); } diff --git a/src/java/org/apache/poi/hssf/model/Workbook.java b/src/java/org/apache/poi/hssf/model/Workbook.java index fa22cfb68..329e217a8 100644 --- a/src/java/org/apache/poi/hssf/model/Workbook.java +++ b/src/java/org/apache/poi/hssf/model/Workbook.java @@ -105,6 +105,8 @@ public class Workbook implements Model private static POILogger log = POILogFactory.getLogger(Workbook.class); + protected static final String EXCEL_REPEATING_NAME_PREFIX_ = "Excel_Name_Record_Titles_"; + /** * Creates new Workbook with no intitialization --useless right now * @see #createWorkbook(List) @@ -1918,13 +1920,20 @@ public class Workbook implements Model */ public NameRecord addName(NameRecord name) { - - getOrCreateLinkTable().addName(name); + + LinkTable linkTable = getOrCreateLinkTable(); + if(linkTable.nameAlreadyExists(name)) { + throw new IllegalArgumentException( + "You are trying to assign a duplicated name record: " + + name.getNameText()); + } + linkTable.addName(name); return name; } - - /**Generates a NameRecord to represent a built-in region + + /** + * Generates a NameRecord to represent a built-in region * @return a new NameRecord unless the index is invalid */ public NameRecord createBuiltInName(byte builtInName, int index) @@ -1933,9 +1942,21 @@ public class Workbook implements Model throw new IllegalArgumentException("Index is not valid ["+index+"]"); NameRecord name = new NameRecord(builtInName, (short)(index)); - - addName(name); + String prefix = EXCEL_REPEATING_NAME_PREFIX_ + index + "_"; + int cont = 0; + while(linkTable.nameAlreadyExists(name)) { + cont++; + String altNameName = prefix + cont; + + // It would be better to set a different builtInName here. + // It does not seem possible, so we create it as a + // non built-in name from this point on + name = new NameRecord(); + name.setNameText(altNameName); + name.setNameTextLength((byte)altNameName.length()); + } + addName(name); return name; } diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java b/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java index 2c8234894..a5279993e 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java @@ -610,13 +610,28 @@ public class HSSFCell implements Cell // Set the 'pre-evaluated result' for the formula // note - formulas do not preserve text formatting. FormulaRecordAggregate fr = (FormulaRecordAggregate) record; - // must make new sr because fr.getStringRecord() may be null - StringRecord sr = new StringRecord(); - sr.setString(hvalue.getString()); // looses format - fr.setStringRecord(sr); + + // Save the string into a String Record, creating + // one if required + StringRecord sr = fr.getStringRecord(); + if(sr == null) { + // Wasn't a string before, need a new one + sr = new StringRecord(); + fr.setStringRecord(sr); + } + + // Save, loosing the formatting + sr.setString(hvalue.getString()); + // Update our local cache to the un-formatted version + stringValue = new HSSFRichTextString(sr.getString()); + + // All done return; } + // If we get here, we're not dealing with a formula, + // so handle things as a normal rich text cell + if (cellType != CELL_TYPE_STRING) { setCellType(CELL_TYPE_STRING, false, row, col, styleIndex); } diff --git a/src/testcases/org/apache/poi/hssf/data/43623.xls b/src/testcases/org/apache/poi/hssf/data/43623.xls new file mode 100644 index 000000000..401cb2a88 Binary files /dev/null and b/src/testcases/org/apache/poi/hssf/data/43623.xls differ diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java index 523d42ec9..bfd359318 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java @@ -33,9 +33,7 @@ import org.apache.poi.hssf.HSSFTestDataSamples; import org.apache.poi.hssf.model.Workbook; import org.apache.poi.hssf.record.CellValueRecordInterface; import org.apache.poi.hssf.record.EmbeddedObjectRefSubRecord; -import org.apache.poi.hssf.record.FormulaRecord; import org.apache.poi.hssf.record.NameRecord; -import org.apache.poi.hssf.record.Record; import org.apache.poi.hssf.record.aggregates.FormulaRecordAggregate; import org.apache.poi.hssf.record.formula.DeletedArea3DPtg; import org.apache.poi.util.TempFile; @@ -1152,10 +1150,12 @@ public final class TestBugs extends TestCase { s.createRow(0); HSSFCell c1 = s.getRow(0).createCell((short)0); HSSFCell c2 = s.getRow(0).createCell((short)1); + HSSFCell c3 = s.getRow(0).createCell((short)2); // As number and string c1.setCellFormula("70164"); c2.setCellFormula("\"70164\""); + c3.setCellFormula("\"90210\""); // Check the formulas assertEquals("70164.0", c1.getCellFormula()); @@ -1166,20 +1166,31 @@ public final class TestBugs extends TestCase { assertEquals("", c1.getRichStringCellValue().getString()); assertEquals(0.0, c2.getNumericCellValue(), 0.00001); assertEquals("", c2.getRichStringCellValue().getString()); + assertEquals(0.0, c3.getNumericCellValue(), 0.00001); + assertEquals("", c3.getRichStringCellValue().getString()); - // Now evaluate + // Try changing the cached value on one of the string + // formula cells, so we can see it updates properly + c3.setCellValue(new HSSFRichTextString("test")); + assertEquals(0.0, c3.getNumericCellValue(), 0.00001); + assertEquals("test", c3.getRichStringCellValue().getString()); + + + // Now evaluate, they should all be changed HSSFFormulaEvaluator eval = new HSSFFormulaEvaluator(s, wb); eval.setCurrentRow(s.getRow(0)); eval.evaluateFormulaCell(c1); eval.evaluateFormulaCell(c2); + eval.evaluateFormulaCell(c3); - // Check + // Check that the cells now contain + // the correct values assertEquals(70164.0, c1.getNumericCellValue(), 0.00001); assertEquals("", c1.getRichStringCellValue().getString()); assertEquals(0.0, c2.getNumericCellValue(), 0.00001); - - // TODO - why isn't this working? -// assertEquals("70164", c2.getRichStringCellValue().getString()); + assertEquals("70164", c2.getRichStringCellValue().getString()); + assertEquals(0.0, c3.getNumericCellValue(), 0.00001); + assertEquals("90210", c3.getRichStringCellValue().getString()); // Write and read @@ -1187,12 +1198,15 @@ public final class TestBugs extends TestCase { HSSFSheet ns = nwb.getSheetAt(0); HSSFCell nc1 = ns.getRow(0).getCell((short)0); HSSFCell nc2 = ns.getRow(0).getCell((short)1); + HSSFCell nc3 = ns.getRow(0).getCell((short)2); // Re-check assertEquals(70164.0, nc1.getNumericCellValue(), 0.00001); assertEquals("", nc1.getRichStringCellValue().getString()); assertEquals(0.0, nc2.getNumericCellValue(), 0.00001); assertEquals("70164", nc2.getRichStringCellValue().getString()); + assertEquals(0.0, nc3.getNumericCellValue(), 0.00001); + assertEquals("90210", nc3.getRichStringCellValue().getString()); // Now check record level stuff too ns.getSheet().setLoc(0); @@ -1205,15 +1219,58 @@ public final class TestBugs extends TestCase { if(fn == 0) { assertEquals(70164.0, fr.getFormulaRecord().getValue(), 0.0001); assertNull(fr.getStringRecord()); - } else { + } else if (fn == 1) { assertEquals(0.0, fr.getFormulaRecord().getValue(), 0.0001); assertNotNull(fr.getStringRecord()); assertEquals("70164", fr.getStringRecord().getString()); + } else { + assertEquals(0.0, fr.getFormulaRecord().getValue(), 0.0001); + assertNotNull(fr.getStringRecord()); + assertEquals("90210", fr.getStringRecord().getString()); } fn++; } } - assertEquals(2, fn); + assertEquals(3, fn); + } + + /** + * Problem with "Vector Rows", eg a whole + * column which is set to the result of + * {=sin(B1:B9)}(9,1), so that each cell is + * shown to have the contents + * {=sin(B1:B9){9,1)[rownum][0] + * In this sample file, the vector column + * is C, and the data column is B. + * + * For now, blows up with an exception from ExtPtg + * Expected ExpPtg to be converted from Shared to Non-Shared... + */ + public void DISABLEDtest43623() throws Exception { + HSSFWorkbook wb = openSample("43623.xls"); + assertEquals(1, wb.getNumberOfSheets()); + + HSSFSheet s1 = wb.getSheetAt(0); + + HSSFCell c1 = s1.getRow(0).getCell(2); + HSSFCell c2 = s1.getRow(1).getCell(2); + HSSFCell c3 = s1.getRow(2).getCell(2); + + // These formula contents are a guess... + assertEquals("{=sin(B1:B9){9,1)[0][0]", c1.getCellFormula()); + assertEquals("{=sin(B1:B9){9,1)[1][0]", c2.getCellFormula()); + assertEquals("{=sin(B1:B9){9,1)[2][0]", c3.getCellFormula()); + + // Save and re-open, ensure it still works + HSSFWorkbook nwb = writeOutAndReadBack(wb); + HSSFSheet ns1 = nwb.getSheetAt(0); + HSSFCell nc1 = ns1.getRow(0).getCell(2); + HSSFCell nc2 = ns1.getRow(1).getCell(2); + HSSFCell nc3 = ns1.getRow(2).getCell(2); + + assertEquals("{=sin(B1:B9){9,1)[0][0]", nc1.getCellFormula()); + assertEquals("{=sin(B1:B9){9,1)[1][0]", nc2.getCellFormula()); + assertEquals("{=sin(B1:B9){9,1)[2][0]", nc3.getCellFormula()); } } diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestNamedRange.java b/src/testcases/org/apache/poi/hssf/usermodel/TestNamedRange.java index 591a7ba24..0a508805d 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestNamedRange.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestNamedRange.java @@ -17,11 +17,17 @@ package org.apache.poi.hssf.usermodel; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.FileOutputStream; + import junit.framework.TestCase; import org.apache.poi.hssf.HSSFTestDataSamples; import org.apache.poi.hssf.util.AreaReference; import org.apache.poi.hssf.util.CellReference; +import org.apache.poi.poifs.filesystem.POIFSFileSystem; /** * @@ -485,4 +491,57 @@ public final class TestNamedRange extends TestCase { // expected during successful test } } + + public void testRepeatingRowsAndColumsNames() throws Exception { + HSSFWorkbook wb = new HSSFWorkbook(); + HSSFSheet sheet = wb.createSheet(); + + for (int rowItem = 0; rowItem < 10; rowItem++) { + HSSFRow r = sheet.createRow(rowItem); + for (int column = 0; column < 2; column++) { + HSSFCell cellItem = r.createCell((short) column); + cellItem.setCellType(HSSFCell.CELL_TYPE_STRING); + cellItem.setCellValue(new HSSFRichTextString("Some value here")); + if (rowItem == 2) { + wb.setRepeatingRowsAndColumns(0, 0, 0, 0, 3 - 1); + sheet.createFreezePane(0, 3); + } + } + } + + assertEquals(2, wb.getNumberOfNames()); + HSSFName nr1 = wb.getNameAt(0); + HSSFName nr2 = wb.getNameAt(1); + + assertEquals("Print_Titles", nr1.getNameName()); + assertEquals("Sheet0!$A$1:$A$0,Sheet0!$A$1:$IV$3", nr1.getReference()); + + assertEquals("Excel_Name_Record_Titles_1_1", nr2.getNameName()); + assertEquals("Sheet0!$A$1:$A$0,Sheet0!$A$1:$IV$3", nr2.getReference()); + + // Save and re-open + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + wb.write(baos); + ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray()); + HSSFWorkbook nwb = new HSSFWorkbook(new POIFSFileSystem(bais)); + + assertEquals(2, nwb.getNumberOfNames()); + nr1 = nwb.getNameAt(0); + nr2 = nwb.getNameAt(1); + + // TODO - + // should these references really have been corrected? + // and if so, why not also above? + assertEquals("Print_Titles", nr1.getNameName()); + assertEquals("Sheet0!A:A,Sheet0!$A$1:$IV$3", nr1.getReference()); + + assertEquals("Excel_Name_Record_Titles_1_1", nr2.getNameName()); + assertEquals("Sheet0!A:A,Sheet0!$A$1:$IV$3", nr2.getReference()); + + // In case you fancy checking in excel, to ensure it + // won't complain about the file now + FileOutputStream fout = new FileOutputStream(File.createTempFile("POI-45126-", ".xls")); + wb.write(fout); + fout.close(); + } }