You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@poi.apache.org by on...@apache.org on 2016/04/17 03:29:19 UTC

svn commit: r1739536 - in /poi/trunk/src: java/org/apache/poi/ss/util/CellUtil.java java/org/apache/poi/ss/util/RegionUtil.java ooxml/testcases/org/apache/poi/xssf/usermodel/TestUnfixedBugs.java testcases/org/apache/poi/ss/util/TestCellUtil.java

Author: onealj
Date: Sun Apr 17 01:29:19 2016
New Revision: 1739536

URL: http://svn.apache.org/viewvc?rev=1739536&view=rev
Log:
bug 59336: patch from Mark Murphy: deprecate functions in CellUtil that require an unnecessary Workbook argument.

Modified:
    poi/trunk/src/java/org/apache/poi/ss/util/CellUtil.java
    poi/trunk/src/java/org/apache/poi/ss/util/RegionUtil.java
    poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestUnfixedBugs.java
    poi/trunk/src/testcases/org/apache/poi/ss/util/TestCellUtil.java

Modified: poi/trunk/src/java/org/apache/poi/ss/util/CellUtil.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/util/CellUtil.java?rev=1739536&r1=1739535&r2=1739536&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/util/CellUtil.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/util/CellUtil.java Sun Apr 17 01:29:19 2016
@@ -146,38 +146,66 @@ public final class CellUtil {
         return createCell(row, column, value, null);
     }
 
-
     /**
      * Take a cell, and align it.
      *
-     *@param cell the cell to set the alignment for
-     *@param workbook The workbook that is being worked with.
-     *@param align the column alignment to use.
+     * @param cell the cell to set the alignment for
+     * @param workbook The workbook that is being worked with.
+     * @param align the column alignment to use.
+     *
+     * @deprecated 3.15-beta2. Removed in 3.17. Use {@link #setAlignment(Cell, short)} instead.
      *
      * @see CellStyle for alignment options
      */
     public static void setAlignment(Cell cell, Workbook workbook, short align) {
-        setCellStyleProperty(cell, workbook, ALIGNMENT, Short.valueOf(align));
+        setAlignment(cell, align);
     }
 
     /**
+     * Take a cell, and align it.
+     *
+     * @param cell the cell to set the alignment for
+     * @param align the column alignment to use.
+     *
+     * @see CellStyle for alignment options
+     */
+    public static void setAlignment(Cell cell, short align) {
+        setCellStyleProperty(cell, ALIGNMENT, Short.valueOf(align));
+    }
+    
+    /**
      * Take a cell, and apply a font to it
      *
-     *@param cell the cell to set the alignment for
-     *@param workbook The workbook that is being worked with.
-     *@param font The Font that you want to set...
+     * @param cell the cell to set the alignment for 
+     * @param workbook The workbook that is being worked with.
+     * @param font The Font that you want to set.
+     * @throws IllegalArgumentException if <tt>font</tt> and <tt>cell</tt> do not belong to the same workbook
+     *
+     * @deprecated 3.15-beta2. Removed in 3.17. Use {@link #setFont(Cell, Font)} instead.
      */
     public static void setFont(Cell cell, Workbook workbook, Font font) {
+        setFont(cell, font);
+    }
+    
+    /**
+     * Take a cell, and apply a font to it
+     *
+     * @param cell the cell to set the alignment for
+     * @param font The Font that you want to set.
+     * @throws IllegalArgumentException if <tt>font</tt> and <tt>cell</tt> do not belong to the same workbook
+     */
+    public static void setFont(Cell cell, Font font) {
         // Check if font belongs to workbook
+        Workbook wb = cell.getSheet().getWorkbook();
         final short fontIndex = font.getIndex();
-        if (!workbook.getFontAt(fontIndex).equals(font)) {
+        if (!wb.getFontAt(fontIndex).equals(font)) {
             throw new IllegalArgumentException("Font does not belong to this workbook");
         }
 
         // Check if cell belongs to workbook
         // (checked in setCellStyleProperty)
 
-        setCellStyleProperty(cell, workbook, FONT, fontIndex);
+        setCellStyleProperty(cell, FONT, fontIndex);
     }
 
     /**
@@ -206,7 +234,6 @@ public final class CellUtil {
      * @since POI 3.14 beta 2
      */
     public static void setCellStyleProperties(Cell cell, Map<String, Object> properties) {
-        @SuppressWarnings("resource")
         Workbook workbook = cell.getSheet().getWorkbook();
         CellStyle originalStyle = cell.getCellStyle();
         CellStyle newStyle = null;
@@ -250,26 +277,45 @@ public final class CellUtil {
      * {@link #setCellStyleProperties(org.apache.poi.ss.usermodel.Cell, Map)},
      * which is faster and does not add unnecessary intermediate CellStyles to the workbook.</p>
      * 
+     * @param cell The cell that is to be changed.
      * @param workbook The workbook that is being worked with.
      * @param propertyName The name of the property that is to be changed.
      * @param propertyValue The value of the property that is to be changed.
-     * @param cell The cell that needs it's style changes
+     * 
+     * @deprecated 3.15-beta2. Removed in 3.17. Use {@link #setCellStyleProperty(Cell, String, Object)} instead.
      */
     public static void setCellStyleProperty(Cell cell, Workbook workbook, String propertyName,
             Object propertyValue) {
-        if (cell.getSheet().getWorkbook() != workbook) {
-            throw new IllegalArgumentException("Cannot set cell style property. Cell does not belong to workbook.");
-        }
+        setCellStyleProperty(cell, propertyName, propertyValue);
+    }
 
-        Map<String, Object> values = Collections.singletonMap(propertyName, propertyValue);
-        setCellStyleProperties(cell, values);
+    /**
+     * <p>This method attempts to find an existing CellStyle that matches the <code>cell</code>'s
+     * current style plus a single style property <code>propertyName</code> with value
+     * <code>propertyValue</code>.
+     * A new style is created if the workbook does not contain a matching style.</p>
+     * 
+     * <p>Modifies the cell style of <code>cell</code> without affecting other cells that use the
+     * same style.</p>
+     * 
+     * <p>If setting more than one cell style property on a cell, use
+     * {@link #setCellStyleProperties(org.apache.poi.ss.usermodel.Cell, Map)},
+     * which is faster and does not add unnecessary intermediate CellStyles to the workbook.</p>
+     * 
+     * @param cell The cell that is to be changed.
+     * @param propertyName The name of the property that is to be changed.
+     * @param propertyValue The value of the property that is to be changed.
+     */
+    public static void setCellStyleProperty(Cell cell, String propertyName, Object propertyValue) {
+        Map<String, Object> property = Collections.singletonMap(propertyName, propertyValue);
+        setCellStyleProperties(cell, property);
     }
 
     /**
      * Returns a map containing the format properties of the given cell style.
-         * The returned map is not tied to <code>style</code>, so subsequent changes
-         * to <code>style</code> will not modify the map, and changes to the returned
-         * map will not modify the cell style. The returned map is mutable.
+     * The returned map is not tied to <code>style</code>, so subsequent changes
+     * to <code>style</code> will not modify the map, and changes to the returned
+     * map will not modify the cell style. The returned map is mutable.
      *
      * @param style cell style
      * @return map of format properties (String -> Object)

Modified: poi/trunk/src/java/org/apache/poi/ss/util/RegionUtil.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/util/RegionUtil.java?rev=1739536&r1=1739535&r2=1739536&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/util/RegionUtil.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/util/RegionUtil.java Sun Apr 17 01:29:19 2016
@@ -39,13 +39,11 @@ public final class RegionUtil {
 	 */
 	private static final class CellPropertySetter {
 
-		private final Workbook _workbook;
 		private final String _propertyName;
 		private final Short _propertyValue;
 
 
-		public CellPropertySetter(Workbook workbook, String propertyName, int value) {
-			_workbook = workbook;
+		public CellPropertySetter(String propertyName, int value) {
 			_propertyName = propertyName;
 			_propertyValue = Short.valueOf((short) value);
 		}
@@ -53,7 +51,7 @@ public final class RegionUtil {
 
 		public void setProperty(Row row, int column) {
 			Cell cell = CellUtil.getCell(row, column);
-			CellUtil.setCellStyleProperty(cell, _workbook, _propertyName, _propertyValue);
+			CellUtil.setCellStyleProperty(cell, _propertyName, _propertyValue);
 		}
 	}
 
@@ -72,7 +70,7 @@ public final class RegionUtil {
 		int rowEnd = region.getLastRow();
 		int column = region.getFirstColumn();
 
-		CellPropertySetter cps = new CellPropertySetter(workbook, CellUtil.BORDER_LEFT, border);
+		CellPropertySetter cps = new CellPropertySetter(CellUtil.BORDER_LEFT, border);
 		for (int i = rowStart; i <= rowEnd; i++) {
 			cps.setProperty(CellUtil.getRow(i, sheet), column);
 		}
@@ -92,8 +90,7 @@ public final class RegionUtil {
 		int rowEnd = region.getLastRow();
 		int column = region.getFirstColumn();
 
-		CellPropertySetter cps = new CellPropertySetter(workbook, CellUtil.LEFT_BORDER_COLOR,
-				color);
+		CellPropertySetter cps = new CellPropertySetter(CellUtil.LEFT_BORDER_COLOR, color);
 		for (int i = rowStart; i <= rowEnd; i++) {
 			cps.setProperty(CellUtil.getRow(i, sheet), column);
 		}
@@ -113,7 +110,7 @@ public final class RegionUtil {
 		int rowEnd = region.getLastRow();
 		int column = region.getLastColumn();
 
-		CellPropertySetter cps = new CellPropertySetter(workbook, CellUtil.BORDER_RIGHT, border);
+		CellPropertySetter cps = new CellPropertySetter(CellUtil.BORDER_RIGHT, border);
 		for (int i = rowStart; i <= rowEnd; i++) {
 			cps.setProperty(CellUtil.getRow(i, sheet), column);
 		}
@@ -133,8 +130,7 @@ public final class RegionUtil {
 		int rowEnd = region.getLastRow();
 		int column = region.getLastColumn();
 
-		CellPropertySetter cps = new CellPropertySetter(workbook, CellUtil.RIGHT_BORDER_COLOR,
-				color);
+		CellPropertySetter cps = new CellPropertySetter(CellUtil.RIGHT_BORDER_COLOR, color);
 		for (int i = rowStart; i <= rowEnd; i++) {
 			cps.setProperty(CellUtil.getRow(i, sheet), column);
 		}
@@ -153,7 +149,7 @@ public final class RegionUtil {
 		int colStart = region.getFirstColumn();
 		int colEnd = region.getLastColumn();
 		int rowIndex = region.getLastRow();
-		CellPropertySetter cps = new CellPropertySetter(workbook, CellUtil.BORDER_BOTTOM, border);
+		CellPropertySetter cps = new CellPropertySetter(CellUtil.BORDER_BOTTOM, border);
 		Row row = CellUtil.getRow(rowIndex, sheet);
 		for (int i = colStart; i <= colEnd; i++) {
 			cps.setProperty(row, i);
@@ -173,8 +169,7 @@ public final class RegionUtil {
 		int colStart = region.getFirstColumn();
 		int colEnd = region.getLastColumn();
 		int rowIndex = region.getLastRow();
-		CellPropertySetter cps = new CellPropertySetter(workbook, CellUtil.BOTTOM_BORDER_COLOR,
-				color);
+		CellPropertySetter cps = new CellPropertySetter(CellUtil.BOTTOM_BORDER_COLOR, color);
 		Row row = CellUtil.getRow(rowIndex, sheet);
 		for (int i = colStart; i <= colEnd; i++) {
 			cps.setProperty(row, i);
@@ -194,7 +189,7 @@ public final class RegionUtil {
 		int colStart = region.getFirstColumn();
 		int colEnd = region.getLastColumn();
 		int rowIndex = region.getFirstRow();
-		CellPropertySetter cps = new CellPropertySetter(workbook, CellUtil.BORDER_TOP, border);
+		CellPropertySetter cps = new CellPropertySetter(CellUtil.BORDER_TOP, border);
 		Row row = CellUtil.getRow(rowIndex, sheet);
 		for (int i = colStart; i <= colEnd; i++) {
 			cps.setProperty(row, i);
@@ -214,7 +209,7 @@ public final class RegionUtil {
 		int colStart = region.getFirstColumn();
 		int colEnd = region.getLastColumn();
 		int rowIndex = region.getFirstRow();
-		CellPropertySetter cps = new CellPropertySetter(workbook, CellUtil.TOP_BORDER_COLOR, color);
+		CellPropertySetter cps = new CellPropertySetter(CellUtil.TOP_BORDER_COLOR, color);
 		Row row = CellUtil.getRow(rowIndex, sheet);
 		for (int i = colStart; i <= colEnd; i++) {
 			cps.setProperty(row, i);

Modified: poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestUnfixedBugs.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestUnfixedBugs.java?rev=1739536&r1=1739535&r2=1739536&view=diff
==============================================================================
--- poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestUnfixedBugs.java (original)
+++ poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestUnfixedBugs.java Sun Apr 17 01:29:19 2016
@@ -254,9 +254,9 @@ public final class TestUnfixedBugs {
            row2.getCell(1).getCellStyle().setBorderBottom(CellStyle.BORDER_THIN);
 
            Cell cell0 = CellUtil.getCell(row3, 0);
-           CellUtil.setCellStyleProperty(cell0, wb, CellUtil.BORDER_BOTTOM, CellStyle.BORDER_THIN);
+           CellUtil.setCellStyleProperty(cell0, CellUtil.BORDER_BOTTOM, CellStyle.BORDER_THIN);
            Cell cell1 = CellUtil.getCell(row3, 1);
-           CellUtil.setCellStyleProperty(cell1, wb, CellUtil.BORDER_BOTTOM, CellStyle.BORDER_THIN);
+           CellUtil.setCellStyleProperty(cell1, CellUtil.BORDER_BOTTOM, CellStyle.BORDER_THIN);
 
            RegionUtil.setBorderBottom(CellStyle.BORDER_THIN, range4, sheet, wb);
     

Modified: poi/trunk/src/testcases/org/apache/poi/ss/util/TestCellUtil.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/ss/util/TestCellUtil.java?rev=1739536&r1=1739535&r2=1739536&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/ss/util/TestCellUtil.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/ss/util/TestCellUtil.java Sun Apr 17 01:29:19 2016
@@ -18,9 +18,8 @@
 package org.apache.poi.ss.util;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNotEquals;
-import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.fail;
 
@@ -53,13 +52,13 @@ public final class TestCellUtil {
 
         // Add a border should create a new style
         int styCnt1 = wb.getNumCellStyles();
-        CellUtil.setCellStyleProperty(c, wb, CellUtil.BORDER_BOTTOM, BorderStyle.THIN);
+        CellUtil.setCellStyleProperty(c, CellUtil.BORDER_BOTTOM, BorderStyle.THIN);
         int styCnt2 = wb.getNumCellStyles();
         assertEquals(styCnt2, styCnt1+1);
 
         // Add same border to another cell, should not create another style
         c = r.createCell(1);
-        CellUtil.setCellStyleProperty(c, wb, CellUtil.BORDER_BOTTOM, BorderStyle.THIN);
+        CellUtil.setCellStyleProperty(c, CellUtil.BORDER_BOTTOM, BorderStyle.THIN);
         int styCnt3 = wb.getNumCellStyles();
         assertEquals(styCnt3, styCnt2);
 
@@ -82,7 +81,7 @@ public final class TestCellUtil {
         props.put(CellUtil.BORDER_RIGHT, BorderStyle.THIN);
         CellUtil.setCellStyleProperties(c, props);
         int styCnt2 = wb.getNumCellStyles();
-        assertEquals(styCnt1+1, styCnt2);
+        assertEquals("Only one additional style should have been created", styCnt1 + 1, styCnt2);
 
         // Add same border another to same cell, should not create another style
         c = r.createCell(1);
@@ -181,7 +180,7 @@ public final class TestCellUtil {
         assertEquals(CellStyle.ALIGN_GENERAL, B1.getCellStyle().getAlignment());
 
         // get/set alignment modifies the cell's style
-        CellUtil.setAlignment(A1, wb, CellStyle.ALIGN_RIGHT);
+        CellUtil.setAlignment(A1, CellStyle.ALIGN_RIGHT);
         assertEquals(CellStyle.ALIGN_RIGHT, A1.getCellStyle().getAlignment());
 
         // get/set alignment doesn't affect the style of cells with
@@ -214,7 +213,7 @@ public final class TestCellUtil {
         assertEquals(defaultFontIndex, B1.getCellStyle().getFontIndex());
 
         // get/set alignment modifies the cell's style
-        CellUtil.setFont(A1, wb, font);
+        CellUtil.setFont(A1, font);
         assertEquals(customFontIndex, A1.getCellStyle().getFontIndex());
 
         // get/set alignment doesn't affect the style of cells with
@@ -237,12 +236,12 @@ public final class TestCellUtil {
         Cell A1 = wb1.createSheet().createRow(0).createCell(0);
         
         // okay
-        CellUtil.setFont(A1, wb1, font1);
+        CellUtil.setFont(A1, font1);
 
         // font belongs to different workbook
         try {
-            CellUtil.setFont(A1, wb1, font2);
-            fail("setFont not allowed if font belongs to a different workbook"); 
+            CellUtil.setFont(A1, font2);
+            fail("setFont not allowed if font belongs to a different workbook");
         } catch (final IllegalArgumentException e) {
             if (e.getMessage().startsWith("Font does not belong to this workbook")) {
                 // expected
@@ -250,19 +249,9 @@ public final class TestCellUtil {
             else {
                 throw e;
             }
-        }
-
-        // cell belongs to different workbook
-        try {
-            CellUtil.setFont(A1, wb2, font2);
-            fail("setFont not allowed if cell belongs to a different workbook"); 
-        } catch (final IllegalArgumentException e) {
-            if (e.getMessage().startsWith("Cannot set cell style property. Cell does not belong to workbook.")) {
-               // expected
-            }
-            else {
-                throw e;
-            }
+        } finally {
+            wb1.close();
+            wb2.close();
         }
     }
 }



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org