Fix for bug 45682 - allow cloning of sheets with conditional formatting

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@688655 13f79535-47bb-0310-9956-ffa450edef68

src/documentation/content/xdocs/changes.xml

@@ -37,6 +37,7 @@
 
 		<!-- Don't forget to update status.xml too! -->
         <release version="3.1.1-alpha1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">45682 - Fix for cloning of CFRecordsAggregate</action>
            <action dev="POI-DEVELOPERS" type="add">Initial support for evaluating external add-in functions like YEARFRAC</action>
            <action dev="POI-DEVELOPERS" type="fix">45672 - Fix for MissingRecordAwareHSSFListener to prevent multiple LastCellOfRowDummyRecords when shared formulas are present</action>
            <action dev="POI-DEVELOPERS" type="fix">45645 - Fix for HSSFSheet.autoSizeColumn() for widths exceeding Short.MAX_VALUE</action>

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.1.1-alpha1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">45682 - Fix for cloning of CFRecordsAggregate</action>
            <action dev="POI-DEVELOPERS" type="add">Initial support for evaluating external add-in functions like YEARFRAC</action>
            <action dev="POI-DEVELOPERS" type="fix">45672 - Fix for MissingRecordAwareHSSFListener to prevent multiple LastCellOfRowDummyRecords when shared formulas are present</action>
            <action dev="POI-DEVELOPERS" type="fix">45645 - Fix for HSSFSheet.autoSizeColumn() for widths exceeding Short.MAX_VALUE</action>

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 <code>null</code>.
 	 */
@@ -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<rules.size(); i++)
 		{
 			CFRuleRecord cfRule = (CFRuleRecord)rules.get(i);
-			if(cfRule!=null)
-			{
-				buffer.append(cfRule.toString());
-			}
+			buffer.append(cfRule.toString());
 		}
 		buffer.append("[/CF]\n");
 		return buffer.toString();
 	}
+
+	public void visitContainedRecords(RecordVisitor rv) {
+		rv.visitRecord(header);
+		for(int i=0; i<rules.size(); i++) {
+			CFRuleRecord rule = (CFRuleRecord)rules.get(i);
+			rv.visitRecord(rule);
+		}
+	}
 }

src/java/org/apache/poi/hssf/record/aggregates/ConditionalFormattingTable.java

@@ -22,7 +22,6 @@ import java.util.List;
 
 import org.apache.poi.hssf.model.RecordStream;
 import org.apache.poi.hssf.record.CFHeaderRecord;
-import org.apache.poi.hssf.record.Record;
 
 /**
  * Holds all the conditional formatting for a workbook sheet.<p/>
@@ -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);
 		}
 	}
 

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;
 	}

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

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());
+	}
 }