diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 21336b6ae7..b7b6e6c423 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 49185 - Support for HSSFNames where the comment is stored in a NameCommentRecord 49599 - Correct writing of NoteRecord author text when switching between ASCII and Unicode HWPF: Improve reading of auto-saved ("complex") documents Paragraph level as well as whole-file text extraction for Word 6/95 files through HWPF diff --git a/src/java/org/apache/poi/hssf/dev/BiffViewer.java b/src/java/org/apache/poi/hssf/dev/BiffViewer.java index ba79aa80d2..ee6e159c4f 100644 --- a/src/java/org/apache/poi/hssf/dev/BiffViewer.java +++ b/src/java/org/apache/poi/hssf/dev/BiffViewer.java @@ -197,6 +197,7 @@ public final class BiffViewer { case MulBlankRecord.sid: return new MulBlankRecord(in); case MulRKRecord.sid: return new MulRKRecord(in); case NameRecord.sid: return new NameRecord(in); + case NameCommentRecord.sid: return new NameCommentRecord(in); case NoteRecord.sid: return new NoteRecord(in); case NumberRecord.sid: return new NumberRecord(in); case ObjRecord.sid: return new ObjRecord(in); diff --git a/src/java/org/apache/poi/hssf/model/InternalWorkbook.java b/src/java/org/apache/poi/hssf/model/InternalWorkbook.java index 567b986e70..f2719bba5e 100644 --- a/src/java/org/apache/poi/hssf/model/InternalWorkbook.java +++ b/src/java/org/apache/poi/hssf/model/InternalWorkbook.java @@ -20,8 +20,11 @@ package org.apache.poi.hssf.model; import java.security.AccessControlException; import java.util.ArrayList; import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; +import java.util.Map; +import java.util.Map.Entry; import org.apache.poi.ddf.EscherBSERecord; import org.apache.poi.ddf.EscherBoolProperty; @@ -57,6 +60,7 @@ import org.apache.poi.hssf.record.HyperlinkRecord; import org.apache.poi.hssf.record.InterfaceEndRecord; import org.apache.poi.hssf.record.InterfaceHdrRecord; import org.apache.poi.hssf.record.MMSRecord; +import org.apache.poi.hssf.record.NameCommentRecord; import org.apache.poi.hssf.record.NameRecord; import org.apache.poi.hssf.record.PaletteRecord; import org.apache.poi.hssf.record.PasswordRecord; @@ -165,6 +169,11 @@ public final class InternalWorkbook { private WriteAccessRecord writeAccess; private WriteProtectRecord writeProtect; + /** + * Hold the {@link NameCommentRecord}s indexed by the name of the {@link NameRecord} to which they apply. + */ + private final Map commentRecords; + private InternalWorkbook() { records = new WorkbookRecordList(); @@ -176,6 +185,7 @@ public final class InternalWorkbook { maxformatid = -1; uses1904datewindowing = false; escherBSERecords = new ArrayList(); + commentRecords = new LinkedHashMap(); } /** @@ -261,7 +271,7 @@ public final class InternalWorkbook { // LinkTable can start with either of these if (log.check( POILogger.DEBUG )) log.log(DEBUG, "found SupBook record at " + k); - retval.linkTable = new LinkTable(recs, k, retval.records); + retval.linkTable = new LinkTable(recs, k, retval.records, retval.commentRecords); k+=retval.linkTable.getRecordCount() - 1; continue; case FormatRecord.sid : @@ -299,6 +309,13 @@ public final class InternalWorkbook { if (log.check( POILogger.DEBUG )) log.log(DEBUG, "found FileSharing at " + k); retval.fileShare = (FileSharingRecord) rec; + break; + + case NameCommentRecord.sid: + final NameCommentRecord ncr = (NameCommentRecord) rec; + if (log.check( POILogger.DEBUG )) + log.log(DEBUG, "found NameComment at " + k); + retval.commentRecords.put(ncr.getNameText(), ncr); default : } records.add(rec); @@ -1823,6 +1840,14 @@ public final class InternalWorkbook { return linkTable.getNameRecord(index); } + /** gets the name comment record + * @param nameRecord name record who's comment is required. + * @return name comment record or null if there isn't one for the given name. + */ + public NameCommentRecord getNameCommentRecord(final NameRecord nameRecord){ + return commentRecords.get(nameRecord.getNameText()); + } + /** creates new name * @return new name record */ @@ -1880,6 +1905,22 @@ public final class InternalWorkbook { } } + /** + * If a {@link NameCommentRecord} is added or the name it references + * is renamed, then this will update the lookup cache for it. + */ + public void updateNameCommentRecordCache(final NameCommentRecord commentRecord) { + if(commentRecords.containsValue(commentRecord)) { + for(Entry entry : commentRecords.entrySet()) { + if(entry.getValue().equals(commentRecord)) { + commentRecords.remove(entry.getKey()); + break; + } + } + } + commentRecords.put(commentRecord.getNameText(), commentRecord); + } + /** * Returns a format index that matches the passed in format. It does not tie into HSSFDataFormat. * @param format the format string diff --git a/src/java/org/apache/poi/hssf/model/LinkTable.java b/src/java/org/apache/poi/hssf/model/LinkTable.java index 10a6b3063b..fd3b7a6e62 100644 --- a/src/java/org/apache/poi/hssf/model/LinkTable.java +++ b/src/java/org/apache/poi/hssf/model/LinkTable.java @@ -20,12 +20,14 @@ package org.apache.poi.hssf.model; import java.util.ArrayList; import java.util.Iterator; import java.util.List; +import java.util.Map; import org.apache.poi.hssf.record.CRNCountRecord; import org.apache.poi.hssf.record.CRNRecord; import org.apache.poi.hssf.record.CountryRecord; import org.apache.poi.hssf.record.ExternSheetRecord; import org.apache.poi.hssf.record.ExternalNameRecord; +import org.apache.poi.hssf.record.NameCommentRecord; import org.apache.poi.hssf.record.NameRecord; import org.apache.poi.hssf.record.Record; import org.apache.poi.hssf.record.SupBookRecord; @@ -149,7 +151,7 @@ final class LinkTable { private final int _recordCount; private final WorkbookRecordList _workbookRecordList; // TODO - would be nice to remove this - public LinkTable(List inputList, int startIndex, WorkbookRecordList workbookRecordList) { + public LinkTable(List inputList, int startIndex, WorkbookRecordList workbookRecordList, Map commentRecords) { _workbookRecordList = workbookRecordList; RecordStream rs = new RecordStream(inputList, startIndex); @@ -176,10 +178,21 @@ final class LinkTable { } _definedNames = new ArrayList(); - // collect zero or more DEFINEDNAMEs id=0x18 - while(rs.peekNextClass() == NameRecord.class) { - NameRecord nr = (NameRecord)rs.getNext(); - _definedNames.add(nr); + // collect zero or more DEFINEDNAMEs id=0x18, + // with their comments if present + while(true) { + Class nextClass = rs.peekNextClass(); + if (nextClass == NameRecord.class) { + NameRecord nr = (NameRecord)rs.getNext(); + _definedNames.add(nr); + } + else if (nextClass == NameCommentRecord.class) { + NameCommentRecord ncr = (NameCommentRecord)rs.getNext(); + commentRecords.put(ncr.getNameText(), ncr); + } + else { + break; + } } _recordCount = rs.getCountRead(); diff --git a/src/java/org/apache/poi/hssf/record/NameCommentRecord.java b/src/java/org/apache/poi/hssf/record/NameCommentRecord.java new file mode 100644 index 0000000000..36d6132d0b --- /dev/null +++ b/src/java/org/apache/poi/hssf/record/NameCommentRecord.java @@ -0,0 +1,150 @@ +/* ==================================================================== + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +==================================================================== */ + +package org.apache.poi.hssf.record; + +import org.apache.poi.util.HexDump; +import org.apache.poi.util.LittleEndianInput; +import org.apache.poi.util.LittleEndianOutput; +import org.apache.poi.util.StringUtil; + +/** + * Title: NAMECMT Record (0x0894) + *

+ * Description: Defines a comment associated with a specified name. + *

+ * REFERENCE: + *

+ * + * @author Andrew Shirley (aks at corefiling.co.uk) + */ +public final class NameCommentRecord extends StandardRecord { + public final static short sid = 0x0894; + + private final short field_1_record_type; + private final short field_2_frt_cell_ref_flag; + private final long field_3_reserved; + //private short field_4_name_length; + //private short field_5_comment_length; + private String field_6_name_text; + private String field_7_comment_text; + + public NameCommentRecord(final String name, final String comment) { + field_1_record_type = 0; + field_2_frt_cell_ref_flag = 0; + field_3_reserved = 0; + field_6_name_text = name; + field_7_comment_text = comment; + } + + @Override + public void serialize(final LittleEndianOutput out) { + final int field_4_name_length = field_6_name_text.length(); + final int field_5_comment_length = field_7_comment_text.length(); + + out.writeShort(field_1_record_type); + out.writeShort(field_2_frt_cell_ref_flag); + out.writeLong(field_3_reserved); + out.writeShort(field_4_name_length); + out.writeShort(field_5_comment_length); + + out.writeByte(0); + out.write(field_6_name_text.getBytes()); + out.writeByte(0); + out.write(field_7_comment_text.getBytes()); + } + + @Override + protected int getDataSize() { + return 18 // 4 shorts + 1 long + 2 spurious 'nul's + + field_6_name_text.length() + + field_7_comment_text.length(); + } + + /** + * @param ris the RecordInputstream to read the record from + */ + public NameCommentRecord(final RecordInputStream ris) { + final LittleEndianInput in = ris; + field_1_record_type = in.readShort(); + field_2_frt_cell_ref_flag = in.readShort(); + field_3_reserved = in.readLong(); + final int field_4_name_length = in.readShort(); + final int field_5_comment_length = in.readShort(); + + in.readByte(); //spurious NUL + field_6_name_text = StringUtil.readCompressedUnicode(in, field_4_name_length); + in.readByte(); //spurious NUL + field_7_comment_text = StringUtil.readCompressedUnicode(in, field_5_comment_length); + } + + /** + * return the non static version of the id for this record. + */ + @Override + public short getSid() { + return sid; + } + + @Override + public String toString() { + final StringBuffer sb = new StringBuffer(); + + sb.append("[NAMECMT]\n"); + sb.append(" .record type = ").append(HexDump.shortToHex(field_1_record_type)).append("\n"); + sb.append(" .frt cell ref flag = ").append(HexDump.byteToHex(field_2_frt_cell_ref_flag)).append("\n"); + sb.append(" .reserved = ").append(field_3_reserved).append("\n"); + sb.append(" .name length = ").append(field_6_name_text.length()).append("\n"); + sb.append(" .comment length = ").append(field_7_comment_text.length()).append("\n"); + sb.append(" .name = ").append(field_6_name_text).append("\n"); + sb.append(" .comment = ").append(field_7_comment_text).append("\n"); + sb.append("[/NAMECMT]\n"); + + return sb.toString(); + } + + /** + * @return the name of the NameRecord to which this comment applies. + */ + public String getNameText() { + return field_6_name_text; + } + + /** + * Updates the name we're associated with, normally used + * when renaming that Name + */ + public void setNameText(String newName) { + field_6_name_text = newName; + } + + /** + * @return the text of the comment. + */ + public String getCommentText() { + return field_7_comment_text; + } + + public void setCommentText(String comment) { + field_7_comment_text = comment; + } + + public short getRecordType() { + return field_1_record_type; + } + +} diff --git a/src/java/org/apache/poi/hssf/record/RecordFactory.java b/src/java/org/apache/poi/hssf/record/RecordFactory.java index e99534ce62..07db07133e 100644 --- a/src/java/org/apache/poi/hssf/record/RecordFactory.java +++ b/src/java/org/apache/poi/hssf/record/RecordFactory.java @@ -173,6 +173,7 @@ public final class RecordFactory { MulBlankRecord.class, MulRKRecord.class, NameRecord.class, + NameCommentRecord.class, NoteRecord.class, NumberRecord.class, ObjectProtectRecord.class, diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFName.java b/src/java/org/apache/poi/hssf/usermodel/HSSFName.java index 579d3494e9..e88a672beb 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFName.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFName.java @@ -19,6 +19,7 @@ package org.apache.poi.hssf.usermodel; import org.apache.poi.hssf.model.HSSFFormulaParser; import org.apache.poi.hssf.model.InternalWorkbook; +import org.apache.poi.hssf.record.NameCommentRecord; import org.apache.poi.hssf.record.NameRecord; import org.apache.poi.hssf.record.formula.Ptg; import org.apache.poi.ss.formula.FormulaType; @@ -33,8 +34,10 @@ import org.apache.poi.ss.usermodel.Name; public final class HSSFName implements Name { private HSSFWorkbook _book; private NameRecord _definedNameRec; + private NameCommentRecord _commentRec; - /** Creates new HSSFName - called by HSSFWorkbook to create a sheet from + /** + * Creates new HSSFName - called by HSSFWorkbook to create a name from * scratch. * * @see org.apache.poi.hssf.usermodel.HSSFWorkbook#createName() @@ -42,8 +45,21 @@ public final class HSSFName implements Name { * @param book workbook object associated with the sheet. */ /* package */ HSSFName(HSSFWorkbook book, NameRecord name) { + this(book, name, null); + } + /** + * Creates new HSSFName - called by HSSFWorkbook to create a name from + * scratch. + * + * @see org.apache.poi.hssf.usermodel.HSSFWorkbook#createName() + * @param name the Name Record + * @param comment the Name Comment Record, optional. + * @param book workbook object associated with the sheet. + */ + /* package */ HSSFName(final HSSFWorkbook book, final NameRecord name, final NameCommentRecord comment) { _book = book; _definedNameRec = name; + _commentRec = comment; } /** Get the sheets name which this named range is referenced to @@ -131,6 +147,13 @@ public final class HSSFName implements Name { } } } + + // Update our comment, if there is one + if(_commentRec != null) { + String oldName = _commentRec.getNameText(); + _commentRec.setNameText(nameName); + _book.getWorkbook().updateNameCommentRecordCache(_commentRec); + } } private static void validateName(String name){ @@ -230,7 +253,14 @@ public final class HSSFName implements Name { * * @return the user comment for this named range */ - public String getComment(){ + public String getComment() { + if(_commentRec != null) { + // Prefer the comment record if it has text in it + if(_commentRec.getCommentText() != null && + _commentRec.getCommentText().length() > 0) { + return _commentRec.getCommentText(); + } + } return _definedNameRec.getDescriptionText(); } @@ -240,7 +270,12 @@ public final class HSSFName implements Name { * @param comment the user comment for this named range */ public void setComment(String comment){ + // Update the main record _definedNameRec.setDescriptionText(comment); + // If we have a comment record too, update that as well + if(_commentRec != null) { + _commentRec.setCommentText(comment); + } } /** diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java index 141f1a9db9..de1cb50550 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java @@ -67,7 +67,6 @@ import org.apache.poi.hssf.util.CellReference; import org.apache.poi.poifs.filesystem.DirectoryNode; import org.apache.poi.poifs.filesystem.POIFSFileSystem; import org.apache.poi.ss.usermodel.CreationHelper; -import org.apache.poi.ss.usermodel.PictureData; import org.apache.poi.ss.usermodel.Row.MissingCellPolicy; import org.apache.poi.ss.formula.FormulaType; import org.apache.poi.util.POILogFactory; @@ -276,7 +275,8 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss } for (int i = 0 ; i < workbook.getNumNames() ; ++i){ - HSSFName name = new HSSFName(this, workbook.getNameRecord(i)); + NameRecord nameRecord = workbook.getNameRecord(i); + HSSFName name = new HSSFName(this, nameRecord, workbook.getNameCommentRecord(nameRecord)); names.add(name); } } @@ -970,7 +970,7 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss if (isNewRecord) { - HSSFName newName = new HSSFName(this, nameRecord); + HSSFName newName = new HSSFName(this, nameRecord, nameRecord.isBuiltInName() ? null : workbook.getNameCommentRecord(nameRecord)); names.add(newName); } diff --git a/src/testcases/org/apache/poi/hssf/model/TestLinkTable.java b/src/testcases/org/apache/poi/hssf/model/TestLinkTable.java index f88cd4860f..72097b2a61 100644 --- a/src/testcases/org/apache/poi/hssf/model/TestLinkTable.java +++ b/src/testcases/org/apache/poi/hssf/model/TestLinkTable.java @@ -18,12 +18,17 @@ package org.apache.poi.hssf.model; import java.util.Arrays; +import java.util.Collections; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import junit.framework.AssertionFailedError; import junit.framework.TestCase; import org.apache.poi.hssf.HSSFTestDataSamples; +import org.apache.poi.hssf.record.NameCommentRecord; +import org.apache.poi.hssf.record.NameRecord; import org.apache.poi.hssf.record.Record; import org.apache.poi.hssf.record.SSTRecord; import org.apache.poi.hssf.record.SupBookRecord; @@ -138,7 +143,7 @@ public final class TestLinkTable extends TestCase { LinkTable lt; try { - lt = new LinkTable(recList, 0, wrl); + lt = new LinkTable(recList, 0, wrl, Collections.emptyMap()); } catch (RuntimeException e) { if (e.getMessage().equals("Expected an EXTERNSHEET record but got (org.apache.poi.hssf.record.SSTRecord)")) { throw new AssertionFailedError("Identified bug 47001b"); @@ -148,4 +153,30 @@ public final class TestLinkTable extends TestCase { } assertNotNull(lt); } + + /** + * + */ + public void testNameCommentRecordBetweenNameRecords() { + + final Record[] recs = { + new NameRecord(), + new NameCommentRecord("name1", "comment1"), + new NameRecord(), + new NameCommentRecord("name2", "comment2"), + + }; + final List recList = Arrays.asList(recs); + final WorkbookRecordList wrl = new WorkbookRecordList(); + final Map commentRecords = new LinkedHashMap(); + + final LinkTable lt = new LinkTable(recList, 0, wrl, commentRecords); + assertNotNull(lt); + + assertEquals(2, commentRecords.size()); + assertTrue(recs[1] == commentRecords.get("name1")); //== is intentionally not .equals()! + assertTrue(recs[3] == commentRecords.get("name2")); //== is intentionally not .equals()! + + assertEquals(2, lt.getNumNames()); + } } diff --git a/src/testcases/org/apache/poi/hssf/record/TestNameCommentRecord.java b/src/testcases/org/apache/poi/hssf/record/TestNameCommentRecord.java new file mode 100644 index 0000000000..f7a29eb466 --- /dev/null +++ b/src/testcases/org/apache/poi/hssf/record/TestNameCommentRecord.java @@ -0,0 +1,42 @@ +/* ==================================================================== + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +==================================================================== */ + +package org.apache.poi.hssf.record; + +import junit.framework.TestCase; + +import org.apache.poi.util.HexRead; + +/** + * Tests the NameCommentRecord serializes/deserializes correctly + * + * @author Andrew Shirley (aks at corefiling.co.uk) + */ +public final class TestNameCommentRecord extends TestCase { + public void testReserialize() { + final byte[] data = HexRead + .readFromString("" + + "94 08 00 00 00 00 00 00 00 00 00 00 04 00 07 00 00 6E 61 6D 65 00 63 6F 6D 6D 65 6E 74]"); + final RecordInputStream in = TestcaseRecordInputStream.create(NameCommentRecord.sid, data); + final NameCommentRecord ncr = new NameCommentRecord(in); + assertEquals(0x0894, ncr.getRecordType()); + assertEquals("name", ncr.getNameText()); + assertEquals("comment", ncr.getCommentText()); + final byte[] data2 = ncr.serialize(); + TestcaseRecordInputStream.confirmRecordEncoding(NameCommentRecord.sid, data, data2); + } +} diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java index 5901f53124..706ccbfa04 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java @@ -1734,4 +1734,31 @@ if(1==2) { assertEquals(234.0, row.getCell(1).getNumericCellValue()); } } + + /** + * Test for a file with NameRecord with NameCommentRecord comments + */ + public void test49185() throws Exception { + HSSFWorkbook wb = openSample("49185.xls"); + Name name = wb.getName("foobarName"); + assertEquals("This is a comment", name.getComment()); + + // Rename the name, comment comes with it + name.setNameName("ChangedName"); + assertEquals("This is a comment", name.getComment()); + + // Save and re-check + wb = writeOutAndReadBack(wb); + name = wb.getName("ChangedName"); + assertEquals("This is a comment", name.getComment()); + + // Now try to change it + name.setComment("Changed Comment"); + assertEquals("Changed Comment", name.getComment()); + + // Save and re-check + wb = writeOutAndReadBack(wb); + name = wb.getName("ChangedName"); + assertEquals("Changed Comment", name.getComment()); + } } diff --git a/test-data/spreadsheet/49185.xls b/test-data/spreadsheet/49185.xls new file mode 100644 index 0000000000..f40a837899 Binary files /dev/null and b/test-data/spreadsheet/49185.xls differ