You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@poi.apache.org by bu...@apache.org on 2015/09/20 23:46:08 UTC
[Bug 58432] New: Sheet and Row iterators expose remove method which
does not correctly remove the row or cell
https://bz.apache.org/bugzilla/show_bug.cgi?id=58432
Bug ID: 58432
Summary: Sheet and Row iterators expose remove method which
does not correctly remove the row or cell
Product: POI
Version: unspecified
Hardware: PC
OS: Linux
Status: NEW
Severity: normal
Priority: P2
Component: SS Common
Assignee: dev@poi.apache.org
Reporter: javenoneal@gmail.com
Sheet[1] and Row[2] interfaces expose an iterator interface that has a remove
method. This allows sheets to be removed from the underlying data structure
(TreeMap.values() -> Collection view of TreeMap's values), but doesn't do the
rest of the cleanup that is done in removeRow(Row)[3] or removeCell(Cell)[4].
If I understand the behavior of a Collection *view* correctly, this would
remove the row/cell from the TreeMap without cleaning up the rest of the data
structures.
The rowIterator and cellIterator should explicitly disallow the remove method
unless the method correctly handles the rest of the cleanup without causing a
ConcurrentModificationException.
Perhaps this would be a good usage for Collections.unmodifiableMap or
Collections.unmodifiableList[5]. Otherwise, a custom iterator could be written
like was done for XSSFWorkbook.iterator[6]
[1] XSSFSheet.iterator
https://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java?revision=1702802&view=markup#l1780
[2] XSSFRow.iterator
https://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFRow.java?revision=1649107&view=markup#l95
[3] XSSFSheet.removeRow
https://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java?revision=1702802&view=markup#l1689
[4] XSSFRow.removeCell
https://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFRow.java?revision=1649107&view=markup#l406
[5] Collections.unmodifiableMap
https://docs.oracle.com/javase/7/docs/api/java/util/Collections.html
[6] XSSFWorkbook.SheetIterator
https://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java?revision=1703573&view=markup#l1058
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 58432] Sheet and Row iterators expose remove method which does
not correctly remove the row or cell
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58432
--- Comment #2 from Javen ONeal <ja...@gmail.com> ---
Created attachment 33122
--> https://bz.apache.org/bugzilla/attachment.cgi?id=33122&action=edit
unit test results
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 58432] Sheet and Row iterators expose remove method which does
not correctly remove the row or cell
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58432
--- Comment #3 from Javen ONeal <ja...@gmail.com> ---
Additional unit tests are necessary to verify that the side-effects of
iterator.remove are identical to removeSheetAt, removeRow, and removeCell.
I propose making iterator.remove() an UnsupportedOperationException for now
until someone can find a solution that doesn't duplicate code and works
correctly.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 58432] Sheet and Row iterators expose remove method which does
not correctly remove the row or cell
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58432
--- Comment #1 from Javen ONeal <ja...@gmail.com> ---
Created attachment 33121
--> https://bz.apache.org/bugzilla/attachment.cgi?id=33121&action=edit
failing unit tests
I've written unit tests for Workbook, Sheet, and Row which should test the
sheetIterator, rowIterator, and cellIterator, respectively for the following
scenarios:
1) iterator.remove() either is unsupported or behaves the same as the
removeSheetAt, removeRow, or removeCell method
2) if removeSheetAt, removeRow, or removeCell are called after an iterator has
been created, the iterator should throw a ConcurrentModificationException the
next time a method is called on it.
The Workbook sheetIterator passes, but the rowIterator and cellIterator fail
for HSSF, XSSF, and SXSSF variants.
Summary of test results
HSSF XSSF SXSSF
BaseTestWorkbook pass pass pass
BaseTestSheet 1fail 1fail 1fail
BaseTestRow 2fails 1fail 1fail
I needed to modify HSSFRow.hashCode in order to have assertNull(HSSFRow) give a
more meaningful error.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org