From 11d5f15a761103827758af4e6a4af1abe5c0fd1e Mon Sep 17 00:00:00 2001 From: Javen O'Neal Date: Sun, 17 Apr 2016 01:29:19 +0000 Subject: [PATCH] bug 59336: patch from Mark Murphy: deprecate functions in CellUtil that require an unnecessary Workbook argument. git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1739536 13f79535-47bb-0310-9956-ffa450edef68 --- src/java/org/apache/poi/ss/util/CellUtil.java | 86 ++++++++++++++----- .../org/apache/poi/ss/util/RegionUtil.java | 25 +++--- .../poi/xssf/usermodel/TestUnfixedBugs.java | 4 +- .../org/apache/poi/ss/util/TestCellUtil.java | 35 +++----- 4 files changed, 90 insertions(+), 60 deletions(-) diff --git a/src/java/org/apache/poi/ss/util/CellUtil.java b/src/java/org/apache/poi/ss/util/CellUtil.java index fc229105e9..a806518ef8 100644 --- a/src/java/org/apache/poi/ss/util/CellUtil.java +++ b/src/java/org/apache/poi/ss/util/CellUtil.java @@ -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 font and cell 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 font and cell 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 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.

* + * @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 values = Collections.singletonMap(propertyName, propertyValue); - setCellStyleProperties(cell, values); + /** + *

This method attempts to find an existing CellStyle that matches the cell's + * current style plus a single style property propertyName with value + * propertyValue. + * A new style is created if the workbook does not contain a matching style.

+ * + *

Modifies the cell style of cell without affecting other cells that use the + * same style.

+ * + *

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.

+ * + * @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 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 style, so subsequent changes - * to style 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 style, so subsequent changes + * to style 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) diff --git a/src/java/org/apache/poi/ss/util/RegionUtil.java b/src/java/org/apache/poi/ss/util/RegionUtil.java index 9c425b4c75..0984d95ef3 100644 --- a/src/java/org/apache/poi/ss/util/RegionUtil.java +++ b/src/java/org/apache/poi/ss/util/RegionUtil.java @@ -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); diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestUnfixedBugs.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestUnfixedBugs.java index 0297cf5f8b..04dbd7b269 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestUnfixedBugs.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestUnfixedBugs.java @@ -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); diff --git a/src/testcases/org/apache/poi/ss/util/TestCellUtil.java b/src/testcases/org/apache/poi/ss/util/TestCellUtil.java index eddb6ac9ee..a0de297840 100644 --- a/src/testcases/org/apache/poi/ss/util/TestCellUtil.java +++ b/src/testcases/org/apache/poi/ss/util/TestCellUtil.java @@ -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(); } } } -- 2.39.5