From fb53e42579b53ba930f068525696116698d3e8ea Mon Sep 17 00:00:00 2001 From: Josh Micich Date: Fri, 7 Nov 2008 23:16:48 +0000 Subject: [PATCH] Fixed problem with linking shared formulas when ranges overlap git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@712307 13f79535-47bb-0310-9956-ffa450edef68 --- src/documentation/content/xdocs/changes.xml | 1 + src/documentation/content/xdocs/status.xml | 1 + .../apache/poi/hssf/record/FormulaRecord.java | 4 + .../poi/hssf/record/SharedFormulaRecord.java | 3 + .../aggregates/FormulaRecordAggregate.java | 28 ++- .../record/aggregates/SharedValueManager.java | 192 +++++++++++++----- .../org/apache/poi/ss/formula/Formula.java | 40 ++++ .../poi/ss/util/CellRangeAddressBase.java | 7 +- .../poi/hssf/data/overlapSharedFormula.xls | Bin 0 -> 19456 bytes .../aggregates/AllRecordAggregateTests.java | 1 + .../aggregates/TestSharedValueManager.java | 126 ++++++++++++ .../apache/poi/hssf/usermodel/TestBugs.java | 1 + 12 files changed, 339 insertions(+), 65 deletions(-) create mode 100644 src/testcases/org/apache/poi/hssf/data/overlapSharedFormula.xls create mode 100644 src/testcases/org/apache/poi/hssf/record/aggregates/TestSharedValueManager.java diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index e448fa313c..0d04e7ef44 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -37,6 +37,7 @@ + Fixed problem with linking shared formulas when ranges overlap 45784 - More fixes to SeriesTextRecord 46033 - fixed TableCell to correctly set text type 46122 - fixed Picture.draw to skip rendering if picture data was not found diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index adfb211b3a..430a136bfb 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + Fixed problem with linking shared formulas when ranges overlap 45784 - More fixes to SeriesTextRecord 46033 - fixed TableCell to correctly set text type 46122 - fixed Picture.draw to skip rendering if picture data was not found diff --git a/src/java/org/apache/poi/hssf/record/FormulaRecord.java b/src/java/org/apache/poi/hssf/record/FormulaRecord.java index 4e4edcc9a6..a87ae562b8 100644 --- a/src/java/org/apache/poi/hssf/record/FormulaRecord.java +++ b/src/java/org/apache/poi/hssf/record/FormulaRecord.java @@ -345,6 +345,10 @@ public final class FormulaRecord extends Record implements CellValueRecordInterf return field_8_parsed_expr.getTokens(); } + public Formula getFormula() { + return field_8_parsed_expr; + } + public void setParsedExpression(Ptg[] ptgs) { field_8_parsed_expr = Formula.create(ptgs); } diff --git a/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java b/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java index 52b14d3db9..f87c96f5a2 100755 --- a/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java +++ b/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java @@ -182,4 +182,7 @@ public final class SharedFormulaRecord extends SharedValueRecordBase { result.field_7_parsed_expr = field_7_parsed_expr.copy(); return result; } + public boolean isFormulaSame(SharedFormulaRecord other) { + return field_7_parsed_expr.isSame(other.field_7_parsed_expr); + } } diff --git a/src/java/org/apache/poi/hssf/record/aggregates/FormulaRecordAggregate.java b/src/java/org/apache/poi/hssf/record/aggregates/FormulaRecordAggregate.java index 282afc427c..18653e952e 100644 --- a/src/java/org/apache/poi/hssf/record/aggregates/FormulaRecordAggregate.java +++ b/src/java/org/apache/poi/hssf/record/aggregates/FormulaRecordAggregate.java @@ -25,6 +25,8 @@ import org.apache.poi.hssf.record.SharedFormulaRecord; import org.apache.poi.hssf.record.StringRecord; import org.apache.poi.hssf.record.formula.ExpPtg; import org.apache.poi.hssf.record.formula.Ptg; +import org.apache.poi.hssf.util.CellReference; +import org.apache.poi.ss.formula.Formula; /** * The formula record aggregate is used to join together the formula record and it's @@ -57,15 +59,19 @@ public final class FormulaRecordAggregate extends RecordAggregate implements Cel + (hasCachedStringFlag ? "" : "not ") + " set"); } - _formulaRecord = formulaRec; - _sharedValueManager = svm; - _stringRecord = stringRec; if (formulaRec.isSharedFormula()) { - _sharedFormulaRecord = svm.linkSharedFormulaRecord(this); - if (_sharedFormulaRecord == null) { + CellReference cr = new CellReference(formulaRec.getRow(), formulaRec.getColumn()); + + CellReference firstCell = formulaRec.getFormula().getExpReference(); + if (firstCell == null) { handleMissingSharedFormulaRecord(formulaRec); + } else { + _sharedFormulaRecord = svm.linkSharedFormulaRecord(firstCell, this); } } + _formulaRecord = formulaRec; + _sharedValueManager = svm; + _stringRecord = stringRec; } /** * Sometimes the shared formula flag "seems" to be erroneously set (because the corresponding @@ -132,10 +138,14 @@ public final class FormulaRecordAggregate extends RecordAggregate implements Cel public void visitContainedRecords(RecordVisitor rv) { rv.visitRecord(_formulaRecord); - // TODO - only bother with this if array or table formula - Record sharedFormulaRecord = _sharedValueManager.getRecordForFirstCell(_formulaRecord); - if (sharedFormulaRecord != null) { - rv.visitRecord(sharedFormulaRecord); + CellReference sharedFirstCell = _formulaRecord.getFormula().getExpReference(); + // perhaps this could be optimised by consulting the (somewhat unreliable) isShared flag + // and/or distinguishing between tExp and tTbl. + if (sharedFirstCell != null) { + Record sharedFormulaRecord = _sharedValueManager.getRecordForFirstCell(sharedFirstCell, this); + if (sharedFormulaRecord != null) { + rv.visitRecord(sharedFormulaRecord); + } } if (_stringRecord != null) { rv.visitRecord(_stringRecord); diff --git a/src/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java b/src/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java index 92eece49a9..f007ebe6e9 100644 --- a/src/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java +++ b/src/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java @@ -17,6 +17,8 @@ package org.apache.poi.hssf.record.aggregates; +import java.util.Arrays; +import java.util.Comparator; import java.util.HashMap; import java.util.Map; @@ -25,6 +27,10 @@ import org.apache.poi.hssf.record.FormulaRecord; import org.apache.poi.hssf.record.SharedFormulaRecord; import org.apache.poi.hssf.record.SharedValueRecordBase; import org.apache.poi.hssf.record.TableRecord; +import org.apache.poi.hssf.record.formula.ExpPtg; +import org.apache.poi.hssf.record.formula.TblPtg; +import org.apache.poi.hssf.util.CellRangeAddress8Bit; +import org.apache.poi.hssf.util.CellReference; /** * Manages various auxiliary records while constructing a @@ -34,15 +40,15 @@ import org.apache.poi.hssf.record.TableRecord; *
  • {@link ArrayRecord}s
    • *
  • {@link TableRecord}s
    • * - * + * * @author Josh Micich */ public final class SharedValueManager { - + // This class should probably be generalised to handle array and table groups too private static final class SharedValueGroup { private final SharedValueRecordBase _svr; - private final FormulaRecordAggregate[] _frAggs; + private FormulaRecordAggregate[] _frAggs; private int _numberOfFormulas; public SharedValueGroup(SharedValueRecordBase svr) { @@ -54,6 +60,12 @@ public final class SharedValueManager { } public void add(FormulaRecordAggregate agg) { + if (_numberOfFormulas >= _frAggs.length) { + // this probably shouldn't occur - problems with sample file "15228.xls" + FormulaRecordAggregate[] temp = new FormulaRecordAggregate[_numberOfFormulas*2]; + System.arraycopy(_frAggs, 0, temp, 0, _frAggs.length); + _frAggs = temp; + } _frAggs[_numberOfFormulas++] = agg; } @@ -63,10 +75,6 @@ public final class SharedValueManager { } } - public boolean isInRange(int rowIx, int columnIx) { - return _svr.isInRange(rowIx, columnIx); - } - public SharedValueRecordBase getSVR() { return _svr; } @@ -77,13 +85,16 @@ public final class SharedValueManager { * that is not the top-left corner of the range. * @return true if this is the first formula cell in the group */ - public boolean isFirstCell(int row, int column) { - // hack for the moment, just check against the first formula that - // came in through the add() method. - FormulaRecordAggregate fra = _frAggs[0]; - return fra.getRow() == row && fra.getColumn() == column; + public boolean isFirstMember(FormulaRecordAggregate agg) { + return agg == _frAggs[0]; + } + public final String toString() { + StringBuffer sb = new StringBuffer(64); + sb.append(getClass().getName()).append(" ["); + sb.append(_svr.getRange().toString()); + sb.append("]"); + return sb.toString(); } - } public static final SharedValueManager EMPTY = new SharedValueManager( @@ -106,6 +117,13 @@ public final class SharedValueManager { _groupsBySharedFormulaRecord = m; } + /** + * @param recs list of sheet records (possibly contains records for other parts of the Excel file) + * @param startIx index of first row/cell record for current sheet + * @param endIx one past index of last row/cell record for current sheet. It is important + * that this code does not inadvertently collect SharedFormulaRecords from any other + * sheet (which could happen if endIx is chosen poorly). (see bug 44449) + */ public static SharedValueManager create(SharedFormulaRecord[] sharedFormulaRecords, ArrayRecord[] arrayRecords, TableRecord[] tableRecords) { if (sharedFormulaRecords.length + arrayRecords.length + tableRecords.length < 1) { @@ -116,77 +134,145 @@ public final class SharedValueManager { /** - * @return null if the specified formula does not have any corresponding - * {@link SharedFormulaRecord} + * @param firstCell as extracted from the {@link ExpPtg} from the cell's formula. + * @return never null */ - public SharedFormulaRecord linkSharedFormulaRecord(FormulaRecordAggregate agg) { - FormulaRecord formula = agg.getFormulaRecord(); - int row = formula.getRow(); - int column = formula.getColumn(); - // Traverse the list of shared formulas in - // reverse order, and try to find the correct one - // for us - - SharedValueGroup[] groups = getGroups(); + public SharedFormulaRecord linkSharedFormulaRecord(CellReference firstCell, FormulaRecordAggregate agg) { + + SharedValueGroup result = findGroup(getGroups(), firstCell); + result.add(agg); + return (SharedFormulaRecord) result.getSVR(); + } + + private static SharedValueGroup findGroup(SharedValueGroup[] groups, CellReference firstCell) { + int row = firstCell.getRow(); + int column = firstCell.getCol(); + // Traverse the list of shared formulas and try to find the correct one for us + + // perhaps this could be optimised to some kind of binary search for (int i = 0; i < groups.length; i++) { - SharedValueGroup svr = groups[i]; - if (svr.isInRange(row, column)) { - svr.add(agg); - return (SharedFormulaRecord) svr.getSVR(); + SharedValueGroup svg = groups[i]; + if (svg.getSVR().isFirstCell(row, column)) { + return svg; } } - return null; + // else - no SharedFormulaRecord was found with the specified firstCell. + // This is unusual, but one sample file exhibits the anomaly: "ex45046-21984.xls" + // Excel seems to handle the problem OK, and doesn't even correct it. Perhaps POI should. + + // search for shared formula by range + SharedValueGroup result = null; + for (int i = 0; i < groups.length; i++) { + SharedValueGroup svg = groups[i]; + if (svg.getSVR().isInRange(row, column)) { + if (result != null) { + // This happens in sample file "15228.xls" + if (sharedFormulasAreSame(result, svg)) { + // hopefully this is OK - just use the first one since they are the same + // not quite + // TODO - fix file "15228.xls" so it opens in Excel after rewriting with POI + } else { + throw new RuntimeException("This cell is in the range of more than one distinct shared formula"); + } + } else { + result = svg; + } + } + } + if (result == null) { + throw new RuntimeException("Failed to find a matching shared formula record"); + } + return result; + } + + /** + * Handles the ugly situation (seen in example "15228.xls") where a shared formula cell is + * covered by more than one shared formula range, but the formula cell's {@link ExpPtg} + * doesn't identify any of them. + * @return true if the underlying shared formulas are the same + */ + private static boolean sharedFormulasAreSame(SharedValueGroup grpA, SharedValueGroup grpB) { + // safe to cast here because this findGroup() is never called for ARRAY or TABLE formulas + SharedFormulaRecord sfrA = (SharedFormulaRecord) grpA.getSVR(); + SharedFormulaRecord sfrB = (SharedFormulaRecord) grpB.getSVR(); + return sfrA.isFormulaSame(sfrB); } private SharedValueGroup[] getGroups() { if (_groups == null) { SharedValueGroup[] groups = new SharedValueGroup[_groupsBySharedFormulaRecord.size()]; _groupsBySharedFormulaRecord.values().toArray(groups); + Arrays.sort(groups, SVGComparator); // make search behaviour more deterministic _groups = groups; - } return _groups; } + private static final Comparator SVGComparator = new Comparator() { + public int compare(Object a, Object b) { + CellRangeAddress8Bit rangeA = ((SharedValueGroup)a).getSVR().getRange(); + CellRangeAddress8Bit rangeB = ((SharedValueGroup)b).getSVR().getRange(); + + int cmp; + cmp = rangeA.getFirstRow() - rangeB.getFirstRow(); + if (cmp != 0) { + return cmp; + } + cmp = rangeA.getFirstColumn() - rangeB.getFirstColumn(); + if (cmp != 0) { + return cmp; + } + return 0; + } + }; /** - * Note - does not return SharedFormulaRecords currently, because the corresponding formula - * records have been converted to 'unshared'. POI does not attempt to re-share formulas. On - * the other hand, there is no such conversion for array or table formulas, so this method - * returns the TABLE or ARRAY record (if it should be written after the specified - * formulaRecord. - * - * @return the TABLE or ARRAY record for this formula cell, if it is the first cell of a - * table or array region. + * The {@link SharedValueRecordBase} record returned by this method + * @param firstCell the cell coordinates as read from the {@link ExpPtg} or {@link TblPtg} + * of the current formula. Note - this is usually not the same as the cell coordinates + * of the formula's cell. + * + * @return the SHRFMLA, TABLE or ARRAY record for this formula cell, if it is the first cell of a + * table or array region. null if */ - public SharedValueRecordBase getRecordForFirstCell(FormulaRecord formulaRecord) { - int row = formulaRecord.getRow(); - int column = formulaRecord.getColumn(); - for (int i = 0; i < _tableRecords.length; i++) { - TableRecord tr = _tableRecords[i]; - if (tr.isFirstCell(row, column)) { - return tr; + public SharedValueRecordBase getRecordForFirstCell(CellReference firstCell, FormulaRecordAggregate agg) { + int row = firstCell.getRow(); + int column = firstCell.getCol(); + boolean isTopLeft = agg.getRow() == row && agg.getColumn() == column; + if (isTopLeft) { + for (int i = 0; i < _tableRecords.length; i++) { + TableRecord tr = _tableRecords[i]; + if (tr.isFirstCell(row, column)) { + return tr; + } } - } - for (int i = 0; i < _arrayRecords.length; i++) { - ArrayRecord ar = _arrayRecords[i]; - if (ar.isFirstCell(row, column)) { - return ar; + for (int i = 0; i < _arrayRecords.length; i++) { + ArrayRecord ar = _arrayRecords[i]; + if (ar.isFirstCell(row, column)) { + return ar; + } } + } else { + // Since arrays and tables cannot be sparse (all cells in range participate) + // no need to search arrays and tables if agg is not the top left cell } SharedValueGroup[] groups = getGroups(); for (int i = 0; i < groups.length; i++) { SharedValueGroup svg = groups[i]; - if (svg.isFirstCell(row, column)) { - return svg.getSVR(); + SharedValueRecordBase svr = svg.getSVR(); + if (svr.isFirstCell(row, column)) { + if (svg.isFirstMember(agg)) { + return svr; + } + return null; } } return null; } /** - * Converts all {@link FormulaRecord}s handled by sharedFormulaRecord + * Converts all {@link FormulaRecord}s handled by sharedFormulaRecord * to plain unshared formulas */ public void unlink(SharedFormulaRecord sharedFormulaRecord) { diff --git a/src/java/org/apache/poi/ss/formula/Formula.java b/src/java/org/apache/poi/ss/formula/Formula.java index 55314452a2..eaf73cd637 100644 --- a/src/java/org/apache/poi/ss/formula/Formula.java +++ b/src/java/org/apache/poi/ss/formula/Formula.java @@ -1,6 +1,15 @@ package org.apache.poi.ss.formula; +import java.util.Arrays; + +import org.apache.poi.hssf.record.ArrayRecord; +import org.apache.poi.hssf.record.SharedFormulaRecord; +import org.apache.poi.hssf.record.TableRecord; +import org.apache.poi.hssf.record.formula.ExpPtg; import org.apache.poi.hssf.record.formula.Ptg; +import org.apache.poi.hssf.record.formula.TblPtg; +import org.apache.poi.hssf.util.CellReference; +import org.apache.poi.util.LittleEndian; import org.apache.poi.util.LittleEndianByteArrayInputStream; import org.apache.poi.util.LittleEndianInput; import org.apache.poi.util.LittleEndianOutput; @@ -130,4 +139,35 @@ public class Formula { // OK to return this for the moment because currently immutable return this; } + + /** + * Gets the locator for the corresponding {@link SharedFormulaRecord}, {@link ArrayRecord} or + * {@link TableRecord} if this formula belongs to such a grouping. The {@link CellReference} + * returned by this method will match the top left corner of the range of that grouping. + * The return value is usually not the same as the location of the cell containing this formula. + * + * @return the firstRow & firstColumn of an array formula or shared formula that this formula + * belongs to. null if this formula is not part of an array or shared formula. + */ + public CellReference getExpReference() { + byte[] data = _byteEncoding; + if (data.length != 5) { + // tExp and tTbl are always 5 bytes long, and the only ptg in the formula + return null; + } + switch (data[0]) { + case ExpPtg.sid: + break; + case TblPtg.sid: + break; + default: + return null; + } + int firstRow = LittleEndian.getUShort(data, 1); + int firstColumn = LittleEndian.getUShort(data, 3); + return new CellReference(firstRow, firstColumn); + } + public boolean isSame(Formula other) { + return Arrays.equals(_byteEncoding, other._byteEncoding); + } } diff --git a/src/java/org/apache/poi/ss/util/CellRangeAddressBase.java b/src/java/org/apache/poi/ss/util/CellRangeAddressBase.java index b26780d85b..9e292929af 100644 --- a/src/java/org/apache/poi/ss/util/CellRangeAddressBase.java +++ b/src/java/org/apache/poi/ss/util/CellRangeAddressBase.java @@ -46,8 +46,7 @@ public abstract class CellRangeAddressBase { _firstCol = firstCol; _lastCol = lastCol; } - private static boolean isValid(int firstRow, int lastRow, int firstColumn, int lastColumn) - { + private static boolean isValid(int firstRow, int lastRow, int firstColumn, int lastColumn) { if(lastRow < 0 || lastRow > LAST_ROW_INDEX) { return false; } @@ -129,6 +128,8 @@ public abstract class CellRangeAddressBase { } public final String toString() { - return getClass().getName() + " ["+_firstRow+", "+_lastRow+", "+_firstCol+", "+_lastCol+"]"; + CellReference crA = new CellReference(_firstRow, _firstCol); + CellReference crB = new CellReference(_lastRow, _lastCol); + return getClass().getName() + " [" + crA.formatAsString() + ":" + crB.formatAsString() +"]"; } } diff --git a/src/testcases/org/apache/poi/hssf/data/overlapSharedFormula.xls b/src/testcases/org/apache/poi/hssf/data/overlapSharedFormula.xls new file mode 100644 index 0000000000000000000000000000000000000000..51c65672af749b6a10c7f077fb1827d78110510c GIT binary patch literal 19456 zcmeHPdvH|M89#R)*&9Gec&a?IgjW(Eh6E7=+yFku0FL5lM;r`c11T|~aj`HXLu~b* zVl6@y3uzr|ovDv%#rJ3{`iIt;*gCD&(Q2!XT3cINseNdb?eBNap51fL-g~n9cGN>O3J9Xzwht4fHec;NeyY&NTa zLg4Y(breb9Q}lHjlov&bp~O+B^8|{EQi(DQWjM+Rl;coFqKra;35aTx(I{h3#-fZv z8ILjn<#?16P)ZQh(q!&&g9334-R~|tMyr@|B1_Yk6ZSvQwvzXswOT1_dc;%te5@R ztXh?EA1xvd?N;}pl%`~T%Mw*HE-nyT#pU>Gmt#K%Qs|U@A|xZa@Z1wZv=00k1L;5P zLr7H*>SwvPxHuggI%FTxQUm4nXCe6Ov6sr9^3s94(uEgS#Kj#o&n2D(umZ?8L`3{J z_9X5?yos__Y+?1}7kMq0eQ0;sheKe#6j~mJh%^9r6`~5^&^_*BG5~p6h43?CBhz8s4$QhOF@_WMp&k(xAkuhm_dZA!{rdvc_pc z)Nn+~l84<>N6a6hhCq#Q?K{YdUa8w$m@~s^M35oFYdEmNp}15A(wGvPgvg96TsysX z+MH?An%31XUR$SM*49snO+}tZM%EzUFF8NG;Zkp zQOi|5MKp+JA=a@g4N2Xz-0Rvp30PadI0vYmTZ?|Jt7}~cE{&ode`sp9Ye<@n8j_wC zDKST!>Sd-z;N>T~X0S}93QQM>+3=(SQ{6t#S2vcd3oT!lvmqh=o=w6#^mQmnIOkXr z{^2qB=OG75(Gf{4OhiLV&JW~Jv0J?I&%=Kd0KYQ;eop}WKmh#S0QlVj@b3n|cLl(M z`V&O|eo2RObt(V69KII-SN$ojuliFA|3Ltq`vTw(1i*h90RKh+{Luh-x&3)i(&3uN zD<5SiT>D7)x2vo74c|9h#BP%HxfW9J&dw)lo~RMAhb26Y7$V{SLIrtg`>YRu&k2Ax z_`3!gZU3=k=@XIicY%i1YA>rI_ju*L$+Qf@Am!&NW~Tp2Q=DZ^13TTqDZ* zdHE>&=XxeI-zkn&8I4v&+u4nD~SGi-wRTNPEI!&DVyN4dz;m#F1JaATsV4XZxt_e7e zWRgUBX&8yfA1Q92W}0O*P!ql~8gOw^R+qR!D60WeT3HRKld>AnXv=CqrIgixW?ohU z`d3*E7*NV;K+hP><=;NDCtg`oSNw+lRI3S2t#WL zL}-S=MCjKg5TR)X6Jf9|fe6htm@SwuNhln|&n^ z3D*vH3rippt{rYSmOvz2JKV1k#Ox`eP$5Bi#L}j-)U|M*ABN_3q1)W z60RNY*_J>gTsz$KErCe5c9NwL3D?fl(ujm>2TR_ehoiRDgB?(9zUyQTihe3q`|`m= zrkB=}aP7<}jYzn5W|l@ITsyN$BNDEix;zoimj*;d)-w;I?+}rF=I7ahU~jOJS3PPZ zWb$I&k9P=vB8Lwj&J#)G2=CmvGn>p^t`DrzfPuEWhWWsDxOS%+P*-(|6FXkMBfI_f z+lxAtN~MZAWx{l)2velFm8B$Q!is0Hvr#>CUSQ9Y=ak!EZn+H_%WZH{z5%sO1$qQq zJS}NLH5Jg<0ILXZ18rkZ|HZU1dv1g6B_oVbr5>)d9RkCHJoeOvCX!8s6KN@hh@Eb1 zz0L=bf*}+Qr`ViNMuUQnrBYY=3o$$Rog#Do{Cvz-%7VNod>vmqbhh8gcdFPtPVo>K zNC%il_Pqgvi0$nQ^Wvc_i`J9c#HzE@OX#=*DviCDVxH*bHCjq{me0=pU9n;@HNn@w{uo7xaI zDiBFF@pR|m*IxB!Gumd;Qp{#%2pbiuB%6wK=N~_K&7aK}o6Y=UHVq+c7|fz@ItVI} z?tJ_8Kl!s6YqL4Im`!5{8-~Cf8yC8H|8M?m#@TEZ6tig#VWWbY98G1q^V2`P?9XOA z*~|~Ov>LFPUxK9#wmmZVmOq;bHX9#Hn-;>xw6wvtcOHGjpUv?$8y`!X6~e}}w86HQ z-g?}h%?UOeA4{7Z!p5|;!M4FeulTb$(PragX(xrSF)eMd?V;D+^=C8DX5(XNEg@`7 zOB-zau&K1e*mVSXy(-jrYH#*f5~s%Ez?HHX9#Hn;yc(w6x}ym-pT1 z&t{6v#>dj?LfDv=*4*;culM`2sj=DkSlXNrHm0RDw;a6n8GklOn~jg9HHEM-Ev>m_ z?{m2bZR=*L&Bn*l=7+E`Ev>ob<3qdsMl%9mPh;Jh&zsAvyg50W2+Wm2tiJju{zRu0 zh{B?5qEjuRrWN6i;ZxJ;u1V^cuu2t-57SESKKj>lS0!;b+0)0uE_UF zz3E=5H{C1swl@+Jv-_jjJ#Nz5m3S0PR^Zf|I&}wLf_3NgSDb^(npGO?GL*{GHoo%oOE>pDjsqCD2 zRbp&dpr6&-3qxJ(rR$uSjft}$v|h_pxwoKcftjW?5~o(MKX!JyeM48z#w6AXR!5^% zknGv$5N*E;9o>iysx^W%0XR&}IXe_NnqmSyQ-N@AWF!_D264nbn|QSkaTWgph-Xtj zJoH-(gv47+;)sDZ@zFlSRU8Z;uBHYNNqoLZ9I?;}2+=V<#2F8j#uBDj_C|(@lX;rw zoSrRPVG3KlDvON8czB+43v7`}FE|?aGPKgT;qe*WzX}s1#HF^bp3UhEIsZS`>df(o zOKnooo1yd#o(K@lTVp;J>FC1^jtU5Fgg6EL7pu}+Gkxs{FIHf7CLu7d0%4A{4>rZX zqE<9Ao??eXk9PpuAUT7FN%af3V~e-1jsVM5u=GB8ESIEm(s%G!Sgcm!Tp~!U>dmD0 zxa++%BE#S>q>(|VxP8u5kL(KeNpwi#9!DY2sy7|d)SC`z>TPdixL72xi@thmMr!=4 z7SnMcy;{nkSm$QQXN2_VoPa`4)SK>!dec2oZwzIp_QxT+>E15H<50wT7;|5K;ZKp# zR2S1!*K}pNA+34dR?$&V&l*@q28yG+imlxti6xRmZ-y3N?L9Lm&J~M=igw$|No&e7$?IxZc<# z*^SXv<8KX!te2y`0A1cLpH^b*42C=qz6et65?4wKD%79{ZA2E$AXF|8m&v2U*J4x? zxYrEJxL{K{ota0-OPud$Nb`R8c(LE;g)(Hdop&3KVBcoo*zP(Qwu8SX9Tx>Zn|RL= zpWu|GzOvAeD>~fqC>o}Ic)_=bxCu0GGBC^EcEka|4j3@@;X1&sGhl4ZV-4z9c$Mc^ z`0&VBqUFUeYv*FSyal5?4*cj{5y)v>&@I?a79X_kr;|gITXa{%lG8=m{txTCsi;;?snf;_zRz z0>5vfvNsxwATkI^F+kQTg!?Wq8coX%sS;W#Fmeq^HILnfLoWuoCuCYs)5qTyL4nvQ0o z;bbP7K4+rgZzh`VXQJVHCYqjTqTz)mnoen=;gBYp{%NA|Y@&%W(Qs1}O|Lc4@K_T~ z2R6}gUK34UHqr276HT`^(Qs)KO%FHG@NN@LXE)JsbQ4X#H_`A;6HR9|(RkJ9m$32u zKCEfbnTn?x(dg9RKJ?#H&InFd5g2F=IB<$@TON{6t@5sJyO5E0(VX&E1kq=dh)!fg zfz?mMUnH{8ogg+E)sTX@9@A(uj}e{A zV;XJdF`^+{v**Y17*q$bl$Xc;0qN>Irm|USc`Odw^yD#`ooMAPcLnCLNx3{W3BNuZ zGLLC$$3Fi{5?~%=|~Dp_8PPp%g>pfJaEqi_rv6ps5U6fQR?2g>6* z6w2pL6k8quNnd0Q(&82LIR#AEp88N5kA=r(F(QYhbV=c&-oDLOaC`C!$$$-UIk|7w zwqW0Sz4)>MmT~9*!C-nuDNSiBeWg?^u}-=COC(!zwWFE8gXkXRG!B>jf49tp1PXq* zy0f#ZBdy#q+1%9BlFVC_%oxkNIy)C~28=IB;jxX + * + * There are probably many ways to produce similar effects, but it should be noted that Excel + * is quite temperamental in this regard. Slight variations in technique can cause the shared + * formulas to spill out into plain formula records (which would make these tests pointless). + * + */ + private static final String SAMPLE_FILE_NAME = "overlapSharedFormula.xls"; + /** + * Some of these bugs are intermittent, and the test author couldn't think of a way to write + * test code to hit them bug deterministically. The reason for the unpredictability is that + * the bugs depended on the {@link SharedFormulaRecord}s being searched in a particular order. + * At the time of writing of the test, the order was being determined by the call to {@link + * Collection#toArray(Object[])} on {@link HashMap#values()} where the items in the map were + * using default {@link Object#hashCode()}
    + */ + private static final int MAX_ATTEMPTS=5; + + /** + * This bug happened when there were two or more shared formula ranges that overlapped. POI + * would sometimes associate formulas in the overlapping region with the wrong shared formula + */ + public void testPartiallyOverlappingRanges() throws IOException { + Record[] records; + + int attempt=1; + do { + HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook(SAMPLE_FILE_NAME); + + HSSFSheet sheet = wb.getSheetAt(0); + RecordInspector.getRecords(sheet, 0); + assertEquals("1+1", sheet.getRow(2).getCell(0).getCellFormula()); + if ("1+1".equals(sheet.getRow(3).getCell(0).getCellFormula())) { + throw new AssertionFailedError("Identified bug - wrong shared formula record chosen" + + " (attempt " + attempt + ")"); + } + assertEquals("2+2", sheet.getRow(3).getCell(0).getCellFormula()); + records = RecordInspector.getRecords(sheet, 0); + } while (attempt++ < MAX_ATTEMPTS); + + int count=0; + for (int i = 0; i < records.length; i++) { + if (records[i] instanceof SharedFormulaRecord) { + count++; + } + } + assertEquals(2, count); + } + + /** + * This bug occurs for similar reasons to the bug in {@link #testPartiallyOverlappingRanges()} + * but the symptoms are much uglier - serialization fails with {@link NullPointerException}.
    + */ + public void testCompletelyOverlappedRanges() throws IOException { + Record[] records; + + int attempt=1; + do { + HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook(SAMPLE_FILE_NAME); + + HSSFSheet sheet = wb.getSheetAt(1); + try { + records = RecordInspector.getRecords(sheet, 0); + } catch (NullPointerException e) { + throw new AssertionFailedError("Identified bug " + + "- cannot reserialize completely overlapped shared formula" + + " (attempt " + attempt + ")"); + } + } while (attempt++ < MAX_ATTEMPTS); + + int count=0; + for (int i = 0; i < records.length; i++) { + if (records[i] instanceof SharedFormulaRecord) { + count++; + } + } + assertEquals(2, count); + } +} diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java index 4ca87428af..1f7648ed75 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java @@ -60,6 +60,7 @@ public final class TestBugs extends TestCase { if (true) { // set to false to output test files return; } + System.setProperty("poi.keep.tmp.files", "true"); File file; try { file = TempFile.createTempFile(simpleFileName + "#", ".xls"); -- 2.39.5