diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index f58cf5430..1fe905a68 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -37,6 +37,7 @@ + 45682 - Fix for cloning of CFRecordsAggregate Initial support for evaluating external add-in functions like YEARFRAC 45672 - Fix for MissingRecordAwareHSSFListener to prevent multiple LastCellOfRowDummyRecords when shared formulas are present 45645 - Fix for HSSFSheet.autoSizeColumn() for widths exceeding Short.MAX_VALUE diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index a3548f5ec..6233176ed 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 45682 - Fix for cloning of CFRecordsAggregate Initial support for evaluating external add-in functions like YEARFRAC 45672 - Fix for MissingRecordAwareHSSFListener to prevent multiple LastCellOfRowDummyRecords when shared formulas are present 45645 - Fix for HSSFSheet.autoSizeColumn() for widths exceeding Short.MAX_VALUE diff --git a/src/java/org/apache/poi/hssf/record/aggregates/CFRecordsAggregate.java b/src/java/org/apache/poi/hssf/record/aggregates/CFRecordsAggregate.java index eb117eae4..3f1b64da3 100644 --- a/src/java/org/apache/poi/hssf/record/aggregates/CFRecordsAggregate.java +++ b/src/java/org/apache/poi/hssf/record/aggregates/CFRecordsAggregate.java @@ -18,14 +18,12 @@ package org.apache.poi.hssf.record.aggregates; import java.util.ArrayList; -import java.util.Iterator; import java.util.List; import org.apache.poi.hssf.model.RecordStream; import org.apache.poi.hssf.record.CFHeaderRecord; import org.apache.poi.hssf.record.CFRuleRecord; import org.apache.poi.hssf.record.Record; -import org.apache.poi.hssf.record.RecordInputStream; import org.apache.poi.hssf.util.CellRangeAddress; /** @@ -36,12 +34,10 @@ import org.apache.poi.hssf.util.CellRangeAddress; * @author Dmitriy Kumshayev * */ -public final class CFRecordsAggregate extends Record { +public final class CFRecordsAggregate extends RecordAggregate { /** Excel allows up to 3 conditional formating rules */ private static final int MAX_CONDTIONAL_FORMAT_RULES = 3; - public final static short sid = -2008; // not a real BIFF record - private final CFHeaderRecord header; /** List of CFRuleRecord objects */ @@ -107,45 +103,6 @@ public final class CFRecordsAggregate extends Record { return new CFRecordsAggregate((CFHeaderRecord) header.clone(), newRecs); } - protected void fillFields(RecordInputStream in) - { - // You never fill an aggregate record - } - - public short getSid() - { - return sid; - } - - /** - * 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. - * - * @param offset to begin writing at - * @param data byte array containing instance data - * @return number of bytes written - */ - - public int serialize(int offset, byte[] data) - { - int nRules = rules.size(); - header.setNumberOfConditionalFormats(nRules); - - int pos = offset; - - pos += header.serialize(pos, data); - for(int i=0; i< nRules; i++) { - pos += getRule(i).serialize(pos, data); - } - return pos - offset; - } - - protected void validateSid(short id) - { - // do nothing here - } - /** * @return the header. Never null. */ @@ -165,10 +122,16 @@ public final class CFRecordsAggregate extends Record { return (CFRuleRecord) rules.get(idx); } public void setRule(int idx, CFRuleRecord r) { + if (r == null) { + throw new IllegalArgumentException("r must not be null"); + } checkRuleIndex(idx); rules.set(idx, r); } public void addRule(CFRuleRecord r) { + if (r == null) { + throw new IllegalArgumentException("r must not be null"); + } if(rules.size() >= MAX_CONDTIONAL_FORMAT_RULES) { throw new IllegalStateException("Cannot have more than " + MAX_CONDTIONAL_FORMAT_RULES + " conditional format rules"); @@ -180,26 +143,6 @@ public final class CFRecordsAggregate extends Record { return rules.size(); } - /** - * @return sum of sizes of all aggregated records - */ - public int getRecordSize() - { - int size = 0; - if( header != null) - { - size += header.getRecordSize(); - } - if( rules != null) - { - for(Iterator irecs = rules.iterator(); irecs.hasNext(); ) - { - size += (( Record ) irecs.next()).getRecordSize(); - } - } - return size; - } - /** * String representation of CFRecordsAggregate */ @@ -215,12 +158,17 @@ public final class CFRecordsAggregate extends Record { for(int i=0; i @@ -53,7 +52,8 @@ public final class ConditionalFormattingTable extends RecordAggregate { public void visitContainedRecords(RecordVisitor rv) { for (int i = 0; i < _cfHeaders.size(); i++) { - rv.visitRecord((Record) _cfHeaders.get(i)); + CFRecordsAggregate subAgg = (CFRecordsAggregate) _cfHeaders.get(i); + subAgg.visitContainedRecords(rv); } } diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFConditionalFormattingRule.java b/src/java/org/apache/poi/hssf/usermodel/HSSFConditionalFormattingRule.java index 10d86015a..fc34b9da9 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFConditionalFormattingRule.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFConditionalFormattingRule.java @@ -41,6 +41,12 @@ public final class HSSFConditionalFormattingRule private final HSSFWorkbook workbook; HSSFConditionalFormattingRule(HSSFWorkbook pWorkbook, CFRuleRecord pRuleRecord) { + if (pWorkbook == null) { + throw new IllegalArgumentException("pWorkbook must not be null"); + } + if (pRuleRecord == null) { + throw new IllegalArgumentException("pRuleRecord must not be null"); + } workbook = pWorkbook; cfRuleRecord = pRuleRecord; } diff --git a/src/testcases/org/apache/poi/hssf/record/aggregates/TestCFRecordsAggregate.java b/src/testcases/org/apache/poi/hssf/record/aggregates/TestCFRecordsAggregate.java index 138a8bc30..2b2971453 100644 --- a/src/testcases/org/apache/poi/hssf/record/aggregates/TestCFRecordsAggregate.java +++ b/src/testcases/org/apache/poi/hssf/record/aggregates/TestCFRecordsAggregate.java @@ -63,7 +63,8 @@ public final class TestCFRecordsAggregate extends TestCase record = CFRecordsAggregate.createCFAggregate(new RecordStream(recs, 0)); // Serialize - byte [] serializedRecord = record.serialize(); + byte [] serializedRecord = new byte[record.getRecordSize()]; + record.serialize(0, serializedRecord); InputStream in = new ByteArrayInputStream(serializedRecord); //Parse diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFConditionalFormatting.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFConditionalFormatting.java index ab503737a..b5acb8550 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFConditionalFormatting.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFConditionalFormatting.java @@ -17,6 +17,7 @@ package org.apache.poi.hssf.usermodel; +import junit.framework.AssertionFailedError; import junit.framework.TestCase; import org.apache.poi.hssf.record.CFRuleRecord.ComparisonOperator; @@ -107,4 +108,44 @@ public final class TestHSSFConditionalFormatting extends TestCase assertEquals("2",rule2.getFormula2()); assertEquals("1",rule2.getFormula1()); } + + public void testClone() { + + HSSFWorkbook wb = new HSSFWorkbook(); + HSSFSheet sheet = wb.createSheet(); + String formula = "7"; + + HSSFSheetConditionalFormatting sheetCF = sheet.getSheetConditionalFormatting(); + + HSSFConditionalFormattingRule rule1 = sheetCF.createConditionalFormattingRule(formula); + HSSFFontFormatting fontFmt = rule1.createFontFormatting(); + fontFmt.setFontStyle(true, false); + + HSSFPatternFormatting patternFmt = rule1.createPatternFormatting(); + patternFmt.setFillBackgroundColor(HSSFColor.YELLOW.index); + + + HSSFConditionalFormattingRule rule2 = sheetCF.createConditionalFormattingRule(ComparisonOperator.BETWEEN, "1", "2"); + HSSFConditionalFormattingRule [] cfRules = + { + rule1, rule2 + }; + + short col = 1; + CellRangeAddress [] regions = { + new CellRangeAddress(0, 65535, col, col) + }; + + sheetCF.addConditionalFormatting(regions, cfRules); + + try { + wb.cloneSheet(0); + } catch (RuntimeException e) { + if (e.getMessage().indexOf("needs to define a clone method") > 0) { + throw new AssertionFailedError("Indentified bug 45682"); + } + throw e; + } + assertEquals(2, wb.getNumberOfSheets()); + } }