Copilot commented on code in PR #455:
URL: https://github.com/apache/fesod/pull/455#discussion_r2385388873
##########
fesod/src/main/java/org/apache/fesod/excel/write/metadata/fill/FillConfig.java:
##########
@@ -57,6 +62,28 @@ public class FillConfig {
private boolean hasInit;
+ /**
+ * dynamic column info
+ * */
+ private Map<String,DynamicColumnInfo> dynamicColumnInfoMap;
+
+ /**
+ * get dynamic column info
+ *
+ * if field name is null or not exist, return default dynamic column info
+ * else return dynamic column info by field name
+ *
+ * @param fieldName field name nullable
+ * @return dynamic column info
+ * */
+ public DynamicColumnInfo getDynamicColumnInfo(String fieldName) {
+ if (null == fieldName || !dynamicColumnInfoMap.containsKey(fieldName))
{
+ return dynamicColumnInfoMap.get(DEFAULT_DYNAMIC_INFO_KEY);
+ }else{
Review Comment:
Potential null pointer exception if `dynamicColumnInfoMap` is null. The
method should check if `dynamicColumnInfoMap` is null before calling
`containsKey()` or `get()` methods.
```suggestion
if (dynamicColumnInfoMap == null) {
return null;
}
if (fieldName == null ||
!dynamicColumnInfoMap.containsKey(fieldName)) {
return dynamicColumnInfoMap.get(DEFAULT_DYNAMIC_INFO_KEY);
} else {
```
##########
fesod/src/main/java/org/apache/fesod/excel/write/executor/ExcelWriteFillExecutor.java:
##########
@@ -148,6 +154,40 @@ public void fill(Object data, FillConfig fillConfig) {
}
}
+ private void shiftCellsIfNecessary(FillConfig fillConfig, Collection<?>
collectionData, List<AnalysisCell> analysisCellList) {
+ if (WriteDirectionEnum.VERTICAL.equals(fillConfig.getDirection()) &&
CollectionUtils.isNotEmpty(collectionData) && collectionData.size() > 1) {
+ Sheet sheet = writeContext.writeSheetHolder().getCachedSheet();
+ Object item = collectionData.iterator().next();
+ Class<?> itemClass = item.getClass();
+ Field[] declaredFields = itemClass.getDeclaredFields();
+ Map<String, Field> dynamicFieldMap = new HashMap<>();
+ for (Field declaredField : declaredFields) {
+ if (declaredField.isAnnotationPresent(DynamicColumn.class)) {
+ dynamicFieldMap.put(declaredField.getName(),
declaredField);
+ }
+ }
+
analysisCellList.stream().sorted(Comparator.comparingInt(AnalysisCell::getColumnIndex).reversed()).forEach(analysisCell
-> {
+ int rowIndex = analysisCell.getRowIndex();
+ int columnIndex = analysisCell.getColumnIndex();
+ Row row = sheet.getRow(rowIndex);
+ List<String> variableList = analysisCell.getVariableList();
+ for (String fieldName : dynamicFieldMap.keySet()) {
+ for (String variable : variableList) {
+ String variableFieldName = variable.split("\\.")[0];
Review Comment:
Potential IndexOutOfBoundsException if the variable doesn't contain a dot.
Should check if the split result has at least one element before accessing
index 0.
##########
fesod/src/main/java/org/apache/fesod/excel/write/executor/ExcelWriteFillExecutor.java:
##########
@@ -403,10 +472,47 @@ private void createCell(
Cell cell = createCellIfNecessary(row, lastColumnIndex,
cellWriteHandlerContext);
cellWriteHandlerContext.setCell(cell);
+ if (null != field && field.isAnnotationPresent(DynamicColumn.class)) {
+ if (cellWriteHandlerContext.getCellMap() == null) {
+ cellWriteHandlerContext.setCellMap(new HashMap<>());
+ }
+ cellWriteHandlerContext.getCellMap().put(lastRowIndex + "_" +
lastColumnIndex, cellWriteHandlerContext);
+ DynamicColumnInfo dynamicColumnInfo =
fillConfig.getDynamicColumnInfo(field.getName());
+ if (null == dynamicColumnInfo ||
CollectionUtils.isEmpty(dynamicColumnInfo.getKeys())) {
+ throw new ExcelGenerateException(String.format("Plase set
dynamic column keys for %s in fillConfig", field.getName()));
Review Comment:
Spelling error: 'Plase' should be 'Please'.
```suggestion
throw new ExcelGenerateException(String.format("Please set
dynamic column keys for %s in fillConfig", field.getName()));
```
##########
fesod/src/main/java/org/apache/fesod/excel/write/executor/ExcelWriteFillExecutor.java:
##########
@@ -232,9 +272,15 @@ private void doFill(
ExcelContentProperty.EMPTY);
if (analysisCell.getOnlyOneVariable()) {
- String variable = analysisCell.getVariableList().get(0);
+ String originalVariable =
analysisCell.getVariableList().get(0);
+ String variable = originalVariable;
Object value = null;
- if (dataKeySet.contains(variable)) {
+ if (FILL_VARIABLE_SELF.equals(variable)) {
+ value = oneRowData;
+ } else if (dataKeySet.contains(variable)) {
+ value = dataMap.get(variable);
+ } else if (variable.contains(COLLECTION_PREFIX)) {
+ variable = variable.split("\\.")[0];
Review Comment:
Same issue as above - potential IndexOutOfBoundsException if the variable
doesn't contain a dot after the `contains()` check passes. Should verify the
split result has elements.
##########
fesod/src/main/java/org/apache/fesod/excel/write/executor/AbstractExcelWriteExecutor.java:
##########
@@ -126,6 +139,64 @@ protected void converterAndSet(CellWriteHandlerContext
cellWriteHandlerContext)
}
}
+ /**
+ * Transform the data and then to set into the cell
+ *
+ * @param cellWriteHandlerContext context
+ */
+ protected void converterAndSet(CellWriteHandlerContext
cellWriteHandlerContext) {
+ Object originalValue = cellWriteHandlerContext.getOriginalValue();
+ Field field =
cellWriteHandlerContext.getExcelContentProperty().getField();
+ if (null != field && field.isAnnotationPresent(DynamicColumn.class)) {
+ Map<String, Object> dynamicColumnMap = (Map<String, Object>)
originalValue;
+ FillConfig fillConfig = cellWriteHandlerContext.getFillConfig();
+ if(null == fillConfig || null ==
fillConfig.getDynamicColumnInfoMap()){
+ throw new
ExcelWriteDataConvertException(cellWriteHandlerContext, "DynamicColumn
annotation must be used with FillConfig.dynamicColumnInfoMap");
+ }
+ DynamicColumnInfo dynamicColumnInfo =
fillConfig.getDynamicColumnInfo(field.getName());
+ Integer columnIndex = cellWriteHandlerContext.getColumnIndex();
+ Integer rowIndex = cellWriteHandlerContext.getRowIndex();
+ for (int i = 0; i < dynamicColumnInfo.getKeys().size(); i++) {
+ String key = dynamicColumnInfo.getKeys().get(i);
+ Object o = dynamicColumnMap.get(key);
+ String originalVariable =
cellWriteHandlerContext.getOriginalVariable();
+ if(originalVariable.contains(".")){
+ key = originalVariable.split("\\.")[1];
+ Object itemBean = o;
+ if (null == itemBean) {
+ o = null;
+ }else{
+ BeanMap beanMap = BeanMapUtils.create(itemBean);
+ o = beanMap.get(key);
Review Comment:
Potential IndexOutOfBoundsException if the originalVariable contains a dot
but the split result doesn't have at least 2 elements. Should check array
length before accessing index 1.
```suggestion
String[] splitVar = originalVariable.split("\\.");
if (splitVar.length >= 2) {
key = splitVar[1];
Object itemBean = o;
if (null == itemBean) {
o = null;
}else{
BeanMap beanMap = BeanMapUtils.create(itemBean);
o = beanMap.get(key);
}
} else {
// Optionally handle the case where there is no
second part
key = null;
o = null;
```
##########
fesod/src/main/java/org/apache/fesod/excel/write/executor/AbstractExcelWriteExecutor.java:
##########
@@ -126,6 +139,64 @@ protected void converterAndSet(CellWriteHandlerContext
cellWriteHandlerContext)
}
}
+ /**
+ * Transform the data and then to set into the cell
+ *
+ * @param cellWriteHandlerContext context
+ */
+ protected void converterAndSet(CellWriteHandlerContext
cellWriteHandlerContext) {
+ Object originalValue = cellWriteHandlerContext.getOriginalValue();
+ Field field =
cellWriteHandlerContext.getExcelContentProperty().getField();
+ if (null != field && field.isAnnotationPresent(DynamicColumn.class)) {
+ Map<String, Object> dynamicColumnMap = (Map<String, Object>)
originalValue;
+ FillConfig fillConfig = cellWriteHandlerContext.getFillConfig();
+ if(null == fillConfig || null ==
fillConfig.getDynamicColumnInfoMap()){
Review Comment:
The method `getDynamicColumnInfoMap()` doesn't exist in the FillConfig
class. It should be `dynamicColumnInfoMap` field access or a proper getter
method.
```suggestion
if(null == fillConfig || null ==
fillConfig.dynamicColumnInfoMap){
```
##########
fesod-examples/src/test/java/org/apache/fesod/excel/temp/fill/FillTempTest.java:
##########
@@ -205,7 +269,7 @@ public void horizontalFill() {
excelWriter.fill(data(), fillConfig, writeSheet);
excelWriter.fill(data(), fillConfig, writeSheet);
- Map<String, Object> map = new HashMap<String, Object>();
+ Map<String, Object> map = new HashMap();
Review Comment:
Raw type usage. Should use `new HashMap<>()` or `new HashMap<String,
Object>()` for type safety.
```suggestion
Map<String, Object> map = new HashMap<String, Object>();
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
