feature: Support Dynamic Column in Fill Operation for Collection Data #455
Conversation
|
Is it possible to use @ExcelProperty and @DynamicColumn together? |
之前没考虑过这个问题，但看代码应该是支持的，原本的convert方法都会被正常调用。 |
代码还未合并，是哪一部分在生效呢，我要在当前version可以实现吗，通过SheetWriteHandler等方式啥的 |
不行，跟handler实现逻辑不一样。不确定owner什么时候会合并这个mergeRequest。想用的话可以fork我的分支 |
好的 |
|
hi, @milixiang, sorry for not responding to this PR. There are still some conflicts, can you fix that? 抱歉没有及时反馈。现在有些文件有冲突，你能处理一下吗？ 如果我没有及时回复，随时pin我就好。 |
我clone你的项目后使用了FillTempTest的dynamicFill的数据，修改了导出方式，测试 @ExcelProperty和@DynamicColumn，似乎没有生效 错误信息如下： |
- 新增 DynamicColumnInfo 类封装动态列信息 - 修改 FillConfig 类，使用 Map 存储多个动态列信息 - 更新动态列相关的处理逻辑和异常提示 - 优化单元测试用例
- 新增自引用变量标识符 "$" - 在单元格填充时，如果变量为自引用标识符，则使用当前行数据进行替换 - 优化了变量替换逻辑，增加了对自引用变量的处理
|
@yuzhi-jiang 这个改动是给模板填充使用的 不支持write 而且如提示，使用的时候要配置好FillConfig.dynamicColumnInfoMap |
milixiang
force-pushed
the
dynamicColumnInCollectionFill
branch
from
a86aacf to
2fb9bc7
Compare
|
@psxjoy 冲突已解决，请重新review。 |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for dynamic columns in Excel fill operations, allowing fields with the @DynamicColumn annotation to generate multiple columns based on runtime data. The implementation enables templates to dynamically expand horizontally based on Map-type data structures.
Key changes:
- Introduces
@DynamicColumnannotation to mark fields for dynamic column processing - Adds
DynamicColumnInfoclass to store configuration for dynamic column keys and group sizes - Implements cell shifting logic to prevent column overwriting when expanding dynamic columns
- Adds
$variable rule for direct collection element traversal in fill operations
Reviewed Changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| FillConfig.java | Adds dynamic column configuration storage and builder methods |
| DynamicColumnInfo.java | New class to hold dynamic column metadata (keys and group size) |
| CellWriteHandlerContext.java | Adds cloning support and dynamic column context storage |
| ExcelWriteFillExecutor.java | Implements core dynamic column logic, cell shifting, and $ variable handling |
| AbstractExcelWriteExecutor.java | Refactors cell conversion logic to handle dynamic columns |
| DynamicColumn.java | New annotation to mark fields for dynamic column processing |
| Test files | Adds comprehensive test cases for dynamic column functionality |
Comments suppressed due to low confidence (1)
fesod/src/main/java/org/apache/fesod/excel/write/executor/ExcelWriteFillExecutor.java:131
- [nitpick] Missing space after 'else'. Should be 'else {' for consistent code formatting.
} else {
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if (null == fieldName || !dynamicColumnInfoMap.containsKey(fieldName)) { | ||
| return dynamicColumnInfoMap.get(DEFAULT_DYNAMIC_INFO_KEY); | ||
| }else{ |
There was a problem hiding this comment.
Potential null pointer exception if dynamicColumnInfoMap is null. The method should check if dynamicColumnInfoMap is null before calling containsKey() or get() methods.
| if (null == fieldName || !dynamicColumnInfoMap.containsKey(fieldName)) { | |
| return dynamicColumnInfoMap.get(DEFAULT_DYNAMIC_INFO_KEY); | |
| }else{ | |
| if (dynamicColumnInfoMap == null) { | |
| return null; | |
| } | |
| if (fieldName == null || !dynamicColumnInfoMap.containsKey(fieldName)) { | |
| return dynamicColumnInfoMap.get(DEFAULT_DYNAMIC_INFO_KEY); | |
| } else { |
| 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())); |
There was a problem hiding this comment.
Spelling error: 'Plase' should be 'Please'.
| throw new ExcelGenerateException(String.format("Plase set dynamic column keys for %s in fillConfig", field.getName())); | |
| throw new ExcelGenerateException(String.format("Please set dynamic column keys for %s in fillConfig", field.getName())); |
| List<String> variableList = analysisCell.getVariableList(); | ||
| for (String fieldName : dynamicFieldMap.keySet()) { | ||
| for (String variable : variableList) { | ||
| String variableFieldName = variable.split("\\.")[0]; |
There was a problem hiding this 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.
| } else if (variable.contains(COLLECTION_PREFIX)) { | ||
| variable = variable.split("\\.")[0]; |
There was a problem hiding this 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.
| key = originalVariable.split("\\.")[1]; | ||
| Object itemBean = o; | ||
| if (null == itemBean) { | ||
| o = null; | ||
| }else{ | ||
| BeanMap beanMap = BeanMapUtils.create(itemBean); | ||
| o = beanMap.get(key); |
There was a problem hiding this 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.
| key = originalVariable.split("\\.")[1]; | |
| Object itemBean = o; | |
| if (null == itemBean) { | |
| o = null; | |
| }else{ | |
| BeanMap beanMap = BeanMapUtils.create(itemBean); | |
| o = beanMap.get(key); | |
| 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; |
| 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()){ |
There was a problem hiding this comment.
The method getDynamicColumnInfoMap() doesn't exist in the FillConfig class. It should be dynamicColumnInfoMap field access or a proper getter method.
| if(null == fillConfig || null == fillConfig.getDynamicColumnInfoMap()){ | |
| if(null == fillConfig || null == fillConfig.dynamicColumnInfoMap){ |
| excelWriter.fill(data(), fillConfig, writeSheet); | ||
|
|
||
| Map<String, Object> map = new HashMap<String, Object>(); | ||
| Map<String, Object> map = new HashMap(); |
There was a problem hiding this comment.
Raw type usage. Should use new HashMap<>() or new HashMap<String, Object>() for type safety.
| Map<String, Object> map = new HashMap(); | |
| Map<String, Object> map = new HashMap<String, Object>(); |
确实我看代码就觉得怪怪的，因为都涉及到fill，而我的需求是使用wirte,去写类中有map的，不知道能不能支持 |
|
@yuzhi-jiang write的需求我理解了，后边我会看看怎么实现，但是应该没那么快，你可以先看看fill能满足需求不。 |
好的，不过fill的方式确实满足不了需求，因为map中的key都是动态的，无法使用固定模板 |
|
@psxjoy 之前merge好像没成功 还需要我做什么吗？ |
3 participants
Purpose of the pull request
模板导出时不支持类似下图的需求，故做此次修改
from:
to
What's changed?