You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@poi.apache.org by jo...@apache.org on 2008/06/19 21:07:21 UTC
svn commit: r669658 - in /poi/trunk/src: documentation/content/xdocs/
java/org/apache/poi/hssf/record/ java/org/apache/poi/hssf/usermodel/
testcases/org/apache/poi/hssf/record/
Author: josh
Date: Thu Jun 19 12:07:20 2008
New Revision: 669658
URL: http://svn.apache.org/viewvc?rev=669658&view=rev
Log:
Fix for bug 45234 - Removed incorrect shared formula conversion in CFRuleRecord
Modified:
poi/trunk/src/documentation/content/xdocs/changes.xml
poi/trunk/src/documentation/content/xdocs/status.xml
poi/trunk/src/java/org/apache/poi/hssf/record/CFRuleRecord.java
poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFSheetConditionalFormatting.java
poi/trunk/src/testcases/org/apache/poi/hssf/record/TestCFRuleRecord.java
Modified: poi/trunk/src/documentation/content/xdocs/changes.xml
URL: http://svn.apache.org/viewvc/poi/trunk/src/documentation/content/xdocs/changes.xml?rev=669658&r1=669657&r2=669658&view=diff
==============================================================================
--- poi/trunk/src/documentation/content/xdocs/changes.xml (original)
+++ poi/trunk/src/documentation/content/xdocs/changes.xml Thu Jun 19 12:07:20 2008
@@ -37,6 +37,7 @@
<!-- Don't forget to update status.xml too! -->
<release version="3.1-final" date="2008-06-??">
+ <action dev="POI-DEVELOPERS" type="fix">45234 - Removed incorrect shared formula conversion in CFRuleRecord</action>
<action dev="POI-DEVELOPERS" type="fix">45001 - Improved HWPF Range.replaceText()</action>
<action dev="POI-DEVELOPERS" type="fix">44692 - Fixed HSSFPicture.resize() to properly resize pictures if the underlying columns/rows have modified size</action>
<action dev="POI-DEVELOPERS" type="add">Support custom image renderers in HSLF</action>
Modified: poi/trunk/src/documentation/content/xdocs/status.xml
URL: http://svn.apache.org/viewvc/poi/trunk/src/documentation/content/xdocs/status.xml?rev=669658&r1=669657&r2=669658&view=diff
==============================================================================
--- poi/trunk/src/documentation/content/xdocs/status.xml (original)
+++ poi/trunk/src/documentation/content/xdocs/status.xml Thu Jun 19 12:07:20 2008
@@ -34,6 +34,7 @@
<!-- Don't forget to update changes.xml too! -->
<changes>
<release version="3.1-final" date="2008-06-??">
+ <action dev="POI-DEVELOPERS" type="fix">45234 - Removed incorrect shared formula conversion in CFRuleRecord</action>
<action dev="POI-DEVELOPERS" type="fix">45001 - Improved HWPF Range.replaceText()</action>
<action dev="POI-DEVELOPERS" type="fix">44692 - Fixed HSSFPicture.resize() to properly resize pictures if the underlying columns/rows have modified size</action>
<action dev="POI-DEVELOPERS" type="add">Support custom image renderers in HSLF</action>
Modified: poi/trunk/src/java/org/apache/poi/hssf/record/CFRuleRecord.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/record/CFRuleRecord.java?rev=669658&r1=669657&r2=669658&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/record/CFRuleRecord.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/record/CFRuleRecord.java Thu Jun 19 12:07:20 2008
@@ -14,14 +14,10 @@
See the License for the specific language governing permissions and
limitations under the License.
==================================================================== */
-
package org.apache.poi.hssf.record;
-import java.util.Stack;
-
import org.apache.poi.hssf.model.FormulaParser;
-import org.apache.poi.hssf.model.Workbook;
import org.apache.poi.hssf.record.cf.BorderFormatting;
import org.apache.poi.hssf.record.cf.FontFormatting;
import org.apache.poi.hssf.record.cf.PatternFormatting;
@@ -30,7 +26,6 @@
import org.apache.poi.util.BitField;
import org.apache.poi.util.BitFieldFactory;
import org.apache.poi.util.LittleEndian;
-import org.apache.poi.util.StringUtil;
/**
* Conditional Formatting Rule Record.
@@ -59,9 +54,6 @@
private byte field_2_comparison_operator;
- private short field_3_formula1_len;
- private short field_4_formula2_len;
-
private int field_5_options;
private static final BitField modificationBits = bf(0x003FFFFF); // Bits: font,align,bord,patt,prot
@@ -121,8 +113,6 @@
{
field_1_condition_type=conditionType;
field_2_comparison_operator=comparisonOperation;
- field_3_formula1_len = (short)0;
- field_4_formula2_len = (short)0;
// Set modification flags to 1: by default options are not modified
field_5_options = modificationBits.setValue(field_5_options, -1);
@@ -147,8 +137,8 @@
this(conditionType, comparisonOperation);
field_1_condition_type = CONDITION_TYPE_CELL_VALUE_IS;
field_2_comparison_operator = comparisonOperation;
- setParsedExpression1(formula1);
- setParsedExpression2(formula2);
+ field_17_formula1 = formula1;
+ field_18_formula2 = formula2;
}
/**
@@ -167,63 +157,38 @@
Ptg[] formula1 = parseFormula(formulaText1, workbook);
Ptg[] formula2 = parseFormula(formulaText2, workbook);
return new CFRuleRecord(CONDITION_TYPE_CELL_VALUE_IS, comparisonOperation, formula1, formula2);
-
}
- /**
- * Constructs a Formula record and sets its fields appropriately.
- * Note - id must be 0x06 (NOT 0x406 see MSKB #Q184647 for an
- * "explanation of this bug in the documentation) or an exception
- * will be throw upon validation
- *
- * @param in the RecordInputstream to read the record from
- */
-
- public CFRuleRecord(RecordInputStream in)
- {
+ public CFRuleRecord(RecordInputStream in) {
super(in);
}
+ protected void fillFields(RecordInputStream in) {
+ field_1_condition_type = in.readByte();
+ field_2_comparison_operator = in.readByte();
+ int field_3_formula1_len = in.readUShort();
+ int field_4_formula2_len = in.readUShort();
+ field_5_options = in.readInt();
+ field_6_not_used = in.readShort();
+ if (containsFontFormattingBlock()) {
+ fontFormatting = new FontFormatting(in);
+ }
- protected void fillFields(RecordInputStream in) {
- try {
- field_1_condition_type = in.readByte();
- field_2_comparison_operator = in.readByte();
- field_3_formula1_len = in.readShort();
- field_4_formula2_len = in.readShort();
- field_5_options = in.readInt();
- field_6_not_used = in.readShort();
-
- if (containsFontFormattingBlock()) {
- fontFormatting = new FontFormatting(in);
- }
-
- if (containsBorderFormattingBlock()) {
- borderFormatting = new BorderFormatting(in);
- }
-
- if (containsPatternFormattingBlock()) {
- patternFormatting = new PatternFormatting(in);
- }
-
- if (field_3_formula1_len > 0) {
- Stack ptgs = Ptg.createParsedExpressionTokens(field_3_formula1_len, in);
- // Now convert any fields as required
- ptgs = SharedFormulaRecord.convertSharedFormulas(ptgs, 0, 0);
- field_17_formula1 = toArray(ptgs);
- }
- if (field_4_formula2_len > 0) {
- Stack ptgs = Ptg.createParsedExpressionTokens(field_4_formula2_len, in);
-
- // Now convert any fields as required
- ptgs = SharedFormulaRecord.convertSharedFormulas(ptgs, 0, 0);
- field_18_formula2 = toArray(ptgs);
- }
- } catch (java.lang.UnsupportedOperationException uoe) {
- throw new RecordFormatException(uoe);
+ if (containsBorderFormattingBlock()) {
+ borderFormatting = new BorderFormatting(in);
+ }
+
+ if (containsPatternFormattingBlock()) {
+ patternFormatting = new PatternFormatting(in);
}
+ if (field_3_formula1_len > 0) {
+ field_17_formula1 = Ptg.readTokens(field_3_formula1_len, in);
+ }
+ if (field_4_formula2_len > 0) {
+ field_18_formula2 = Ptg.readTokens(field_4_formula2_len, in);
+ }
}
public byte getConditionType()
@@ -324,24 +289,6 @@
/**
- * get the length (in number of tokens) of the expression 1
- * @return expression length
- */
- private short getExpression1Length()
- {
- return field_3_formula1_len;
- }
-
-
- /**
- * get the length (in number of tokens) of the expression 2
- * @return expression length
- */
- private short getExpression2Length()
- {
- return field_4_formula2_len;
- }
- /**
* get the option flags
*
* @return bit mask
@@ -489,16 +436,6 @@
return field_18_formula2;
}
- private void setParsedExpression1(Ptg[] ptgs) {
- short len = getTotalPtgSize(field_17_formula1 = ptgs);
- field_3_formula1_len = len;
- }
-
- private void setParsedExpression2(Ptg[] ptgs) {
- short len = getTotalPtgSize(field_18_formula2 = ptgs);
- field_4_formula2_len = len;
- }
-
/**
* called by constructor, should throw runtime exception in the event of a
* record passed with a differing ID.
@@ -520,6 +457,17 @@
}
/**
+ * @param ptgs may be <code>null</code>
+ * @return encoded size of the formula
+ */
+ private static int getFormulaSize(Ptg[] ptgs) {
+ if (ptgs == null) {
+ return 0;
+ }
+ return Ptg.getEncodedSize(ptgs);
+ }
+
+ /**
* called by the class that is responsible for writing this sucker.
* Subclasses should implement this so that their data is passed back in a
* byte array.
@@ -528,18 +476,20 @@
* @param data byte array containing instance data
* @return number of bytes written
*/
-
public int serialize(int pOffset, byte [] data)
{
+ int formula1Len=getFormulaSize(field_17_formula1);
+ int formula2Len=getFormulaSize(field_18_formula2);
+
int offset = pOffset;
int recordsize = getRecordSize();
LittleEndian.putShort(data, 0 + offset, sid);
LittleEndian.putShort(data, 2 + offset, (short)(recordsize-4));
data[4 + offset] = field_1_condition_type;
data[5 + offset] = field_2_comparison_operator;
- LittleEndian.putShort(data, 6 + offset, field_3_formula1_len);
- LittleEndian.putShort(data, 8 + offset, field_4_formula2_len);
+ LittleEndian.putUShort(data, 6 + offset, formula1Len);
+ LittleEndian.putUShort(data, 8 + offset, formula2Len);
LittleEndian.putInt(data, 10 + offset, field_5_options);
LittleEndian.putShort(data,14 + offset, field_6_not_used);
@@ -562,16 +512,12 @@
offset += patternFormatting.serialize(offset, data);
}
- if (getExpression1Length()>0)
- {
- Ptg.serializePtgStack(convertToTokenStack(field_17_formula1), data, offset);
- offset += getExpression1Length();
+ if (field_17_formula1 != null) {
+ offset += Ptg.serializePtgs(field_17_formula1, data, offset);
}
- if (getExpression2Length()>0)
- {
- Ptg.serializePtgStack(convertToTokenStack(field_18_formula2), data, offset);
- offset += getExpression2Length();
+ if (field_18_formula2 != null) {
+ offset += Ptg.serializePtgs(field_18_formula2, data, offset);
}
if(offset - pOffset != recordsize) {
throw new IllegalStateException("write mismatch (" + (offset - pOffset) + "!=" + recordsize + ")");
@@ -586,24 +532,12 @@
(containsFontFormattingBlock()?fontFormatting.getRawRecord().length:0)+
(containsBorderFormattingBlock()?8:0)+
(containsPatternFormattingBlock()?4:0)+
- getExpression1Length()+
- getExpression2Length()
+ getFormulaSize(field_17_formula1)+
+ getFormulaSize(field_18_formula2)
;
return retval;
}
- private short getTotalPtgSize(Ptg[] ptgs)
- {
- if( ptgs == null) {
- return 0;
- }
- short retval = 0;
- for (int i = 0; i < ptgs.length; i++)
- {
- retval += ptgs[i].getSize();
- }
- return retval;
- }
public String toString()
{
@@ -629,8 +563,6 @@
public Object clone() {
CFRuleRecord rec = new CFRuleRecord(field_1_condition_type, field_2_comparison_operator);
- rec.field_3_formula1_len = field_3_formula1_len;
- rec.field_4_formula2_len = field_4_formula2_len;
rec.field_5_options = field_5_options;
rec.field_6_not_used = field_6_not_used;
if (containsFontFormattingBlock()) {
@@ -642,10 +574,10 @@
if (containsPatternFormattingBlock()) {
rec.patternFormatting = (PatternFormatting) patternFormatting.clone();
}
- if (field_3_formula1_len > 0) {
+ if (field_17_formula1 != null) {
rec.field_17_formula1 = (Ptg[]) field_17_formula1.clone();
}
- if (field_4_formula2_len > 0) {
+ if (field_18_formula2 != null) {
rec.field_18_formula2 = (Ptg[]) field_18_formula2.clone();
}
@@ -653,30 +585,17 @@
}
/**
+ * TODO - parse conditional format formulas properly i.e. produce tRefN and tAreaN instead of tRef and tArea
+ * this call will produce the wrong results if the formula contains any cell references
+ * One approach might be to apply the inverse of SharedFormulaRecord.convertSharedFormulas(Stack, int, int)
+ * Note - two extra parameters (rowIx & colIx) will be required. They probably come from one of the Region objects.
+ *
* @return <code>null</code> if <tt>formula</tt> was null.
*/
- private static Ptg[] parseFormula(String formula, HSSFWorkbook workbook)
- {
+ private static Ptg[] parseFormula(String formula, HSSFWorkbook workbook) {
if(formula == null) {
return null;
}
return FormulaParser.parse(formula, workbook);
}
-
- // TODO - treat formulas as token arrays instead of Stacks throughout the rest of POI
- private static Stack convertToTokenStack(Ptg[] ptgs)
- {
- Stack parsedExpression = new Stack();
- // fill the Ptg Stack with Ptgs of new formula
- for (int k = 0; k < ptgs.length; k++)
- {
- parsedExpression.push(ptgs[ k ]);
- }
- return parsedExpression;
- }
- private static Ptg[] toArray(Stack ptgs) {
- Ptg[] result = new Ptg[ptgs.size()];
- ptgs.toArray(result);
- return result;
- }
}
Modified: poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFSheetConditionalFormatting.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFSheetConditionalFormatting.java?rev=669658&r1=669657&r2=669658&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFSheetConditionalFormatting.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFSheetConditionalFormatting.java Thu Jun 19 12:07:20 2008
@@ -39,7 +39,8 @@
/**
* A factory method allowing to create a conditional formatting rule
- * with a cell comparison operator
+ * with a cell comparison operator<p/>
+ * TODO - formulas containing cell references are currently not parsed properly
*
* @param comparisonOperation - a constant value from
* <tt>{@link HSSFConditionalFormattingRule.ComparisonOperator}</tt>: <p>
@@ -72,8 +73,8 @@
/**
* A factory method allowing to create a conditional formatting rule with a formula.<br>
*
- * The formatting rules are applied by Excel when the value of the formula not equal to 0.
- *
+ * The formatting rules are applied by Excel when the value of the formula not equal to 0.<p/>
+ * TODO - formulas containing cell references are currently not parsed properly
* @param formula - formula for the valued, compared with the cell
*/
public HSSFConditionalFormattingRule createConditionalFormattingRule(String formula) {
Modified: poi/trunk/src/testcases/org/apache/poi/hssf/record/TestCFRuleRecord.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/record/TestCFRuleRecord.java?rev=669658&r1=669657&r2=669658&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/record/TestCFRuleRecord.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/record/TestCFRuleRecord.java Thu Jun 19 12:07:20 2008
@@ -17,12 +17,16 @@
package org.apache.poi.hssf.record;
+import junit.framework.AssertionFailedError;
import junit.framework.TestCase;
import org.apache.poi.hssf.record.CFRuleRecord.ComparisonOperator;
import org.apache.poi.hssf.record.cf.BorderFormatting;
import org.apache.poi.hssf.record.cf.FontFormatting;
import org.apache.poi.hssf.record.cf.PatternFormatting;
+import org.apache.poi.hssf.record.formula.Ptg;
+import org.apache.poi.hssf.record.formula.RefNPtg;
+import org.apache.poi.hssf.record.formula.RefPtg;
import org.apache.poi.hssf.usermodel.HSSFWorkbook;
import org.apache.poi.hssf.util.HSSFColor;
import org.apache.poi.util.LittleEndian;
@@ -296,7 +300,57 @@
// check all remaining flag bits (some are not well understood yet)
assertEquals(0x203FFFFF, flags);
}
+
+ private static final byte[] DATA_REFN = {
+ // formula extracted from bugzilla 45234 att 22141
+ 1, 3,
+ 9, // formula 1 length
+ 0, 0, 0, -1, -1, 63, 32, 2, -128, 0, 0, 0, 5,
+ // formula 1: "=B3=1" (formula is relative to B4)
+ 76, -1, -1, 0, -64, // tRefN(B1)
+ 30, 1, 0,
+ 11,
+ };
+
+ /**
+ * tRefN and tAreaN tokens must be preserved when re-serializing conditional format formulas
+ */
+ public void testReserializeRefNTokens() {
+
+ RecordInputStream is = new TestcaseRecordInputStream(CFRuleRecord.sid, DATA_REFN);
+ CFRuleRecord rr = new CFRuleRecord(is);
+ Ptg[] ptgs = rr.getParsedExpression1();
+ assertEquals(3, ptgs.length);
+ if (ptgs[0] instanceof RefPtg) {
+ throw new AssertionFailedError("Identified bug 45234");
+ }
+ assertEquals(RefNPtg.class, ptgs[0].getClass());
+ RefNPtg refNPtg = (RefNPtg) ptgs[0];
+ assertTrue(refNPtg.isColRelative());
+ assertTrue(refNPtg.isRowRelative());
+
+ byte[] data = rr.serialize();
+
+ if (!compareArrays(DATA_REFN, 0, data, 4, DATA_REFN.length)) {
+ fail("Did not re-serialize correctly");
+ }
+ }
+ private static boolean compareArrays(byte[] arrayA, int offsetA, byte[] arrayB, int offsetB, int length) {
+
+ if (offsetA + length > arrayA.length) {
+ return false;
+ }
+ if (offsetB + length > arrayB.length) {
+ return false;
+ }
+ for (int i = 0; i < length; i++) {
+ if (arrayA[i+offsetA] != arrayB[i+offsetB]) {
+ return false;
+ }
+ }
+ return true;
+ }
public static void main(String[] ignored_args)
{
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org