-
Notifications
You must be signed in to change notification settings - Fork 489
feature: support multi-rows loop filling and merge strategies #842
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 6 commits
9dc4e9e
a989a16
873a019
7127ff9
5f7d288
ab370dd
021944f
67a5bcc
fae1f5f
4d4f076
012c076
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
|
|
||
| package org.apache.fesod.sheet.enums; | ||
|
|
||
| /** | ||
| * Strategy for handling merged regions during loop filling. | ||
| */ | ||
| public enum FillMergeStrategy { | ||
|
|
||
| /** | ||
| * Merged regions in the fill template will NOT be handing. | ||
| */ | ||
| NONE, | ||
|
|
||
| /** | ||
| * Automatically handling merged regions base on fill template. | ||
| */ | ||
| AUTO, | ||
|
|
||
| /** | ||
| * Automatically handling merged regions and unify the style using anchor cells base on fill template. | ||
| * <p /> | ||
| * <b>Warning: Too many CellStyle instances may lead to performance issues and can exceed number of cell styles | ||
| * limits (64000 for .xlsx and 4000 for .xls).</b> | ||
| */ | ||
| MERGE_CELL_STYLE | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -21,8 +21,10 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.ArrayList; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.Collection; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.Collections; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.HashMap; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.HashSet; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.IntSummaryStatistics; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.Iterator; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.List; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.Map; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -39,6 +41,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||
| import org.apache.fesod.common.util.StringUtils; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.apache.fesod.sheet.context.WriteContext; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.apache.fesod.sheet.enums.CellDataTypeEnum; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.apache.fesod.sheet.enums.FillMergeStrategy; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.apache.fesod.sheet.enums.WriteDirectionEnum; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.apache.fesod.sheet.enums.WriteTemplateAnalysisCellTypeEnum; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.apache.fesod.sheet.exception.ExcelGenerateException; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -60,6 +63,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||
| import org.apache.poi.ss.usermodel.CellType; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.apache.poi.ss.usermodel.Row; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.apache.poi.ss.usermodel.Sheet; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.apache.poi.ss.util.CellRangeAddress; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * Fill the data into excel | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -137,18 +141,116 @@ public void fill(Object data, FillConfig fillConfig) { | |||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| Iterator<?> iterator = collectionData.iterator(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| int rowSpan = calculateRowSpan(analysisCellList); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (WriteDirectionEnum.VERTICAL.equals(fillConfig.getDirection()) && fillConfig.getForceNewRow()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| shiftRows(collectionData.size(), analysisCellList); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| shiftRows(collectionData.size(), rowSpan, analysisCellList); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| while (iterator.hasNext()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| doFill(analysisCellList, iterator.next(), fillConfig, getRelativeRowIndex()); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| doFill(analysisCellList, iterator.next(), fillConfig, getRelativeRowIndex(), rowSpan); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| doFill(readTemplateData(templateAnalysisCache), realData, fillConfig, null); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| doFill(readTemplateData(templateAnalysisCache), realData, fillConfig, null, 0); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| private void shiftRows(int size, List<AnalysisCell> analysisCellList) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| private void addMergedRegionIfNecessary(List<AnalysisCell> analysisCellList, FillConfig fillConfig) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| FillMergeStrategy mergeStrategy = fillConfig.getMergeStrategy(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (FillMergeStrategy.NONE.equals(mergeStrategy)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| Set<CellRangeAddress> dataRowMergeRegions = determineMergedRegionsForRow(analysisCellList, fillConfig); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (CollectionUtils.isEmpty(dataRowMergeRegions)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| Sheet sheet = writeContext.writeSheetHolder().getSheet(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Unify the style using anchor cells | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (FillMergeStrategy.MERGE_CELL_STYLE.equals(mergeStrategy)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| Sheet cachedSheet = writeContext.writeSheetHolder().getCachedSheet(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| Map<CellCoordinate, Set<CellCoordinate>> cellCoordinateMap = indexMergedRegionsMap(dataRowMergeRegions); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| for (Map.Entry<CellCoordinate, Set<CellCoordinate>> entry : cellCoordinateMap.entrySet()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| CellCoordinate anchor = entry.getKey(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| CellStyle anchorStyle = | ||||||||||||||||||||||||||||||||||||||||||||||||||
| anchor.getOrCreateCell(sheet, cachedSheet).getCellStyle(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| for (CellCoordinate mergedCell : entry.getValue()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| Cell cell = mergedCell.getOrCreateCell(sheet, cachedSheet); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| cell.setCellStyle(anchorStyle); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Merge cells | ||||||||||||||||||||||||||||||||||||||||||||||||||
| for (CellRangeAddress range : dataRowMergeRegions) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| sheet.addMergedRegionUnsafe(range.copy()); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+192
to
+194
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Merge cells | |
| for (CellRangeAddress range : dataRowMergeRegions) { | |
| sheet.addMergedRegionUnsafe(range.copy()); | |
| // Merge cells | |
| // Avoid adding duplicate merged regions by tracking existing ones | |
| Set<String> existingMergedRegionKeys = new HashSet<>(); | |
| for (CellRangeAddress existingRange : sheet.getMergedRegions()) { | |
| String key = existingRange.getFirstRow() | |
| + ":" + existingRange.getLastRow() | |
| + ":" + existingRange.getFirstColumn() | |
| + ":" + existingRange.getLastColumn(); | |
| existingMergedRegionKeys.add(key); | |
| } | |
| for (CellRangeAddress range : dataRowMergeRegions) { | |
| String key = range.getFirstRow() | |
| + ":" + range.getLastRow() | |
| + ":" + range.getFirstColumn() | |
| + ":" + range.getLastColumn(); | |
| if (existingMergedRegionKeys.contains(key)) { | |
| continue; | |
| } | |
| sheet.addMergedRegionUnsafe(range.copy()); | |
| existingMergedRegionKeys.add(key); |
Copilot
AI
Feb 21, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
determineMergedRegionsForRow only considers merged regions that intersect cells containing template variables (analysisCellList). Any merged regions within the template row-span that do not cover a placeholder cell will never be replicated, which seems to conflict with the docs/PR description that say the template row’s merge structure is replicated. If the intent is to replicate all merges in the loop block, consider scanning merged regions by row-span bounds rather than by placeholder cells.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected PR description
Outdated
Copilot
AI
Feb 21, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
determineMergedRegionsForRow assumes collectionLastIndexMap is non-null, but addMergedRegionIfNecessary is invoked for non-collection fills as well (the else branch calls doFill(..., rowSpan=0)). If a caller sets mergeStrategy != NONE for a non-collection fill, collectionLastIndexCache.get(currentUniqueDataFlag) will be null and this will throw an NPE. Please guard by returning an empty set when collectionLastIndexMap is null (and/or only applying merge strategies when filling collection data / analysisCell.getCellType() == COLLECTION).
| for (AnalysisCell cell : cells) { | |
| for (CellRangeAddress range : sheet.getMergedRegions()) { | |
| if (range.isInRange(cell.getRowIndex(), cell.getColumnIndex())) { | |
| int firstRow = collectionLastIndexMap.get(cell); | |
| if (collectionLastIndexMap == null || collectionLastIndexMap.isEmpty()) { | |
| return Collections.emptySet(); | |
| } | |
| for (AnalysisCell cell : cells) { | |
| Integer firstRow = collectionLastIndexMap.get(cell); | |
| if (firstRow == null) { | |
| // No recorded last index for this cell; skip to avoid NPE. | |
| continue; | |
| } | |
| for (CellRangeAddress range : sheet.getMergedRegions()) { | |
| if (range.isInRange(cell.getRowIndex(), cell.getColumnIndex())) { |
Copilot
AI
Feb 21, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
determineMergedRegionsForRow calculates the replicated merged range using firstRow = collectionLastIndexMap.get(cell). This only works if the template variable cell is on the merged region’s first row; if a placeholder exists in any non-anchor cell inside a merged region, the computed firstRow/lastRow will be wrong and can create overlapping/incorrect merges. Consider computing a row offset (collectionLastIndexMap.get(cell) - cell.getRowIndex()) and shifting the original range by that offset (and/or only using the merged region’s anchor cell) to generate the replicated CellRangeAddress.
| for (CellRangeAddress range : sheet.getMergedRegions()) { | |
| if (range.isInRange(cell.getRowIndex(), cell.getColumnIndex())) { | |
| int firstRow = collectionLastIndexMap.get(cell); | |
| int lastRow = firstRow + (range.getLastRow() - range.getFirstRow()); | |
| Integer mappedRowIndex = collectionLastIndexMap.get(cell); | |
| if (mappedRowIndex == null) { | |
| continue; | |
| } | |
| int rowOffset = mappedRowIndex - cell.getRowIndex(); | |
| for (CellRangeAddress range : sheet.getMergedRegions()) { | |
| if (range.isInRange(cell.getRowIndex(), cell.getColumnIndex())) { | |
| int firstRow = range.getFirstRow() + rowOffset; | |
| int lastRow = range.getLastRow() + rowOffset; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected PR description.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JavaDoc/comment uses “handing” where “handling” is intended (e.g., “Merged regions … will NOT be handing”, “base on”). Please fix wording to avoid confusion in a public enum’s documentation.