moparisthebest/poi
1
0
Fork 0
Code Issues Pull Requests Releases 1 Wiki Activity

bug 59336: patch from Mark Murphy: deprecate functions in CellUtil that require an unnecessary Workbook argument.

Browse Source 
git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1739536 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Javen O'Neal 2016-04-17 01:29:19 +00:00
parent f3af2a116b
commit 4e33c06871
4 changed files with 90 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

86
src/java/org/apache/poi/ss/util/CellUtil.java
View File

@ -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)

25
src/java/org/apache/poi/ss/util/RegionUtil.java
View File

@ -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);

4
src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestUnfixedBugs.java
View File

@ -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);

35
src/testcases/org/apache/poi/ss/util/TestCellUtil.java
View File

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