From d907925a01b7dc9ffd1e6bec7e99eaf6ac81fcbb Mon Sep 17 00:00:00 2001 From: BigDataDZ <76271875+BigDataDZ@users.noreply.github.com> Date: Mon, 12 Oct 2026 16:02:15 +0800 Subject: [PATCH 1/9] fix: carry formula cells into rows created by collection fill A template row that mixes {data.xxx} placeholders with a formula cell lost the formula in every row created by a list fill: template analysis only inspects string cells, and the fill only writes the placeholder columns into the created rows. The fill executor now records the formula cells of the collection template rows and recreates them in each created row, translating the formula with POI's FormulaShifter (row/column copy) so relative references follow the new position and absolute ones stay anchored. Styles follow the existing autoStyle behavior and unparsable formulas are skipped instead of failing the fill. Fixes #199 Generated-by: ZCode Signed-off-by: BigDataDZ <76271875+BigDataDZ@users.noreply.github.com> --- .../executor/ExcelWriteFillExecutor.java | 195 ++++++++++++++++++ .../FillWithFormulaInCreatedRowsTest.java | 134 ++++++++++++ 2 files changed, 329 insertions(+) create mode 100644 fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java index fb743c8f1..357a55773 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java @@ -29,6 +29,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.HashSet; +import java.util.IdentityHashMap; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -61,11 +62,25 @@ import org.apache.fesod.sheet.write.metadata.fill.FillConfig; import org.apache.fesod.sheet.write.metadata.fill.FillWrapper; import org.apache.fesod.sheet.write.metadata.holder.WriteSheetHolder; +import org.apache.poi.hssf.usermodel.HSSFEvaluationWorkbook; +import org.apache.poi.hssf.usermodel.HSSFWorkbook; +import org.apache.poi.ss.formula.FormulaParseException; +import org.apache.poi.ss.formula.FormulaParsingWorkbook; +import org.apache.poi.ss.formula.FormulaParser; +import org.apache.poi.ss.formula.FormulaRenderer; +import org.apache.poi.ss.formula.FormulaRenderingWorkbook; +import org.apache.poi.ss.formula.FormulaShifter; +import org.apache.poi.ss.formula.FormulaType; +import org.apache.poi.ss.formula.ptg.Ptg; +import org.apache.poi.ss.formula.ptg.RefPtg; import org.apache.poi.ss.usermodel.Cell; import org.apache.poi.ss.usermodel.CellStyle; 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.usermodel.Workbook; +import org.apache.poi.xssf.usermodel.XSSFEvaluationWorkbook; +import org.apache.poi.xssf.usermodel.XSSFWorkbook; /** * Fill the data into excel @@ -101,6 +116,11 @@ public class ExcelWriteFillExecutor extends AbstractExcelWriteExecutor { * Last index cache for collection fields */ private final Map> collectionLastIndexCache = MapUtils.newHashMap(); + /** + * Formula cells inside the collection template rows, carried over to the created rows + */ + private final Map> templateFormulaCellsCache = + MapUtils.newHashMap(); private final Map relativeRowIndexMap = MapUtils.newHashMap(); /** @@ -333,6 +353,12 @@ private void doFill( WriteHandlerUtils.afterCellDispose(cellWriteHandlerContext); } + // Formula cells in the collection template rows must reach every created row, + // otherwise a formula column next to the placeholders silently disappears. + if (relativeRowIndex != null) { + copyFormulaCells(fillConfig, relativeRowIndex); + } + // In the case of the fill line may be called many times if (rowWriteHandlerContext.getRow() != null) { WriteHandlerUtils.afterRowDispose(rowWriteHandlerContext); @@ -513,9 +539,169 @@ private List readTemplateData(Map collectionAnalysisCellList = templateCollectionAnalysisCache.get(currentUniqueDataFlag); + if (CollectionUtils.isEmpty(collectionAnalysisCellList)) { + return; + } + Set collectionRowIndexSet = new HashSet<>(); + for (AnalysisCell analysisCell : collectionAnalysisCellList) { + collectionRowIndexSet.add(analysisCell.getRowIndex()); + } + List formulaCellList = + templateFormulaCellsCache.computeIfAbsent(currentUniqueDataFlag, key -> ListUtils.newArrayList()); + for (Integer rowIndex : collectionRowIndexSet) { + Row row = sheet.getRow(rowIndex); + if (row == null) { + continue; + } + for (int j = 0; j < row.getLastCellNum(); j++) { + Cell cell = row.getCell(j); + if (cell == null || !CellType.FORMULA.equals(cell.getCellType())) { + continue; + } + formulaCellList.add(new TemplateFormulaCell(rowIndex, j, cell.getCellFormula())); + } + } + } + + private void copyFormulaCells(FillConfig fillConfig, Integer relativeRowIndex) { + if (relativeRowIndex == null || relativeRowIndex == 0) { + // The first item fills the original template row, which already holds the formulas. + return; + } + List formulaCellList = templateFormulaCellsCache.get(currentUniqueDataFlag); + if (CollectionUtils.isEmpty(formulaCellList)) { + return; + } + boolean vertical = WriteDirectionEnum.VERTICAL.equals(fillConfig.getDirection()); + Sheet sheet = writeContext.writeSheetHolder().getSheet(); + Sheet cachedSheet = writeContext.writeSheetHolder().getCachedSheet(); + for (TemplateFormulaCell templateFormulaCell : formulaCellList) { + int targetRowIndex = vertical + ? templateFormulaCell.getRowIndex() + relativeRowIndex + : templateFormulaCell.getRowIndex(); + int targetColumnIndex = vertical + ? templateFormulaCell.getColumnIndex() + : templateFormulaCell.getColumnIndex() + relativeRowIndex; + Row targetRow = getRowForFormulaCopy(sheet, cachedSheet, targetRowIndex); + if (targetRow == null) { + continue; + } + Cell targetCell = targetRow.getCell(targetColumnIndex); + if (targetCell == null) { + targetCell = targetRow.createCell(targetColumnIndex); + } + if (fillConfig.getAutoStyle()) { + Row templateRow = cachedSheet.getRow(templateFormulaCell.getRowIndex()); + Cell templateCell = + templateRow == null ? null : templateRow.getCell(templateFormulaCell.getColumnIndex()); + if (templateCell != null) { + targetCell.setCellStyle(templateCell.getCellStyle()); + } + } + String translatedFormula = translateFormula( + cachedSheet, templateFormulaCell, targetRowIndex, targetColumnIndex, vertical); + try { + targetCell.setCellFormula(translatedFormula); + } catch (FormulaParseException e) { + // Never let a malformed formula break the fill; leave the cell blank instead. + } + } + } + + private Row getRowForFormulaCopy(Sheet sheet, Sheet cachedSheet, int targetRowIndex) { + Row row = sheet.getRow(targetRowIndex); + if (row == null) { + row = cachedSheet.getRow(targetRowIndex); + } + if (row == null) { + try { + row = sheet.createRow(targetRowIndex); + } catch (IllegalArgumentException e) { + row = cachedSheet.createRow(targetRowIndex); + } + } + return row; + } + + /** + * Translate the template formula so its references follow the copied row/column, the way a + * spreadsheet copies the row down: relative references move, absolute ones stay anchored. + */ + private String translateFormula( + Sheet cachedSheet, + TemplateFormulaCell templateFormulaCell, + int targetRowIndex, + int targetColumnIndex, + boolean vertical) { + String formula = templateFormulaCell.getFormula(); + Workbook templateWorkbook = writeContext.writeWorkbookHolder().getCachedWorkbook(); + FormulaParsingWorkbook parsingWorkbook = createFormulaParsingWorkbook(templateWorkbook); + if (parsingWorkbook == null) { + return formula; + } + try { + String sheetName = cachedSheet.getSheetName(); + int externSheetIndex = parsingWorkbook.getExternalSheetIndex(sheetName); + Ptg[] ptgs = FormulaParser.parse(formula, parsingWorkbook, FormulaType.CELL, externSheetIndex); + Map anchoredRefs = new IdentityHashMap<>(); + for (Ptg ptg : ptgs) { + if (ptg instanceof RefPtg) { + RefPtg refPtg = (RefPtg) ptg; + if (vertical ? !refPtg.isRowRelative() : !refPtg.isColRelative()) { + anchoredRefs.put(refPtg, vertical ? refPtg.getRow() : refPtg.getColumn()); + } + } + } + FormulaShifter formulaShifter = vertical + ? FormulaShifter.createForRowCopy( + externSheetIndex, + sheetName, + templateFormulaCell.getRowIndex(), + templateFormulaCell.getRowIndex(), + targetRowIndex - templateFormulaCell.getRowIndex(), + parsingWorkbook.getSpreadsheetVersion()) + : FormulaShifter.createForColumnCopy( + externSheetIndex, + sheetName, + templateFormulaCell.getColumnIndex(), + templateFormulaCell.getColumnIndex(), + targetColumnIndex - templateFormulaCell.getColumnIndex(), + parsingWorkbook.getSpreadsheetVersion()); + formulaShifter.adjustFormula(ptgs, externSheetIndex); + for (Map.Entry entry : anchoredRefs.entrySet()) { + RefPtg refPtg = (RefPtg) entry.getKey(); + if (vertical) { + refPtg.setRow(entry.getValue()); + } else { + refPtg.setColumn(entry.getValue()); + } + } + return FormulaRenderer.toFormulaString((FormulaRenderingWorkbook) parsingWorkbook, ptgs); + } catch (Exception e) { + return formula; + } + } + + private FormulaParsingWorkbook createFormulaParsingWorkbook(Workbook templateWorkbook) { + if (templateWorkbook instanceof XSSFWorkbook) { + return XSSFEvaluationWorkbook.create((XSSFWorkbook) templateWorkbook); + } + if (templateWorkbook instanceof HSSFWorkbook) { + return HSSFEvaluationWorkbook.create((HSSFWorkbook) templateWorkbook); + } + return null; + } + /** * To prepare data * @@ -670,6 +856,15 @@ private UniqueDataFlagKey uniqueDataFlag(WriteSheetHolder writeSheetHolder, Stri return new UniqueDataFlagKey(writeSheetHolder.getSheetNo(), writeSheetHolder.getSheetName(), wrapperName); } + @Getter + @Setter + @AllArgsConstructor + private static class TemplateFormulaCell { + private Integer rowIndex; + private Integer columnIndex; + private String formula; + } + @Getter @Setter @EqualsAndHashCode diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java new file mode 100644 index 000000000..c70f24d22 --- /dev/null +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java @@ -0,0 +1,134 @@ +/* + * 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.fill; + +import java.io.File; +import java.io.FileInputStream; +import java.io.FileOutputStream; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.apache.fesod.sheet.FesodSheet; +import org.apache.fesod.sheet.testkit.Tags; +import org.apache.fesod.sheet.testkit.base.AbstractExcelTest; +import org.apache.fesod.sheet.testkit.enums.ExcelFormat; +import org.apache.fesod.sheet.write.metadata.fill.FillConfig; +import org.apache.fesod.sheet.write.metadata.fill.FillWrapper; +import org.apache.poi.ss.usermodel.Cell; +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.xssf.usermodel.XSSFWorkbook; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +@Tag(Tags.ROUND_TRIP) +public class FillWithFormulaInCreatedRowsTest extends AbstractExcelTest { + + private File createTemplate() throws Exception { + File template = createTempFile(ExcelFormat.XLSX); + try (XSSFWorkbook workbook = new XSSFWorkbook()) { + Sheet sheet = workbook.createSheet("Sheet0"); + Row headRow = sheet.createRow(0); + headRow.createCell(0).setCellValue("item"); + headRow.createCell(1).setCellValue("double"); + headRow.createCell(2).setCellValue("abs"); + Row templateRow = sheet.createRow(1); + templateRow.createCell(0).setCellValue("{data.name}"); + templateRow.createCell(1).setCellFormula("A2*2"); + templateRow.createCell(2).setCellFormula("$A$2*10"); + try (FileOutputStream out = new FileOutputStream(template)) { + workbook.write(out); + } + } + return template; + } + + private List> buildData(int size) { + List> list = new ArrayList<>(); + for (int i = 1; i <= size; i++) { + Map row = new HashMap<>(); + row.put("name", "n" + i); + list.add(row); + } + return list; + } + + private static String formulaOf(Sheet sheet, int rowIndex, int columnIndex) { + Row row = sheet.getRow(rowIndex); + if (row == null) { + return null; + } + Cell cell = row.getCell(columnIndex); + if (cell == null) { + return null; + } + return cell.getCellType() == CellType.FORMULA ? cell.getCellFormula() : null; + } + + @Test + void fillListKeepsFormulasInCreatedRows() throws Exception { + File template = createTemplate(); + File outFile = createTempFile(ExcelFormat.XLSX); + + FesodSheet.write(outFile) + .withTemplate(template) + .sheet() + .doFill(new FillWrapper("data", buildData(3)), + FillConfig.builder().forceNewRow(true).build()); + + try (XSSFWorkbook workbook = new XSSFWorkbook(new FileInputStream(outFile))) { + Sheet sheet = workbook.getSheetAt(0); + // Values are filled for every created row. + for (int i = 1; i <= 3; i++) { + Assertions.assertEquals("n" + i, sheet.getRow(i).getCell(0).getStringCellValue()); + } + // The original row keeps its template formulas; every created row carries a + // translated copy: relative references follow the copied row, absolute ones stay. + Assertions.assertEquals("A2*2", formulaOf(sheet, 1, 1)); + Assertions.assertEquals("A3*2", formulaOf(sheet, 2, 1)); + Assertions.assertEquals("A4*2", formulaOf(sheet, 3, 1)); + Assertions.assertEquals("$A$2*10", formulaOf(sheet, 1, 2)); + Assertions.assertEquals("$A$2*10", formulaOf(sheet, 2, 2)); + Assertions.assertEquals("$A$2*10", formulaOf(sheet, 3, 2)); + } + } + + @Test + void fillListWithoutForceNewRowKeepsFormulasInCreatedRows() throws Exception { + File template = createTemplate(); + File outFile = createTempFile(ExcelFormat.XLSX); + + FesodSheet.write(outFile) + .withTemplate(template) + .sheet() + .doFill(new FillWrapper("data", buildData(3))); + + try (XSSFWorkbook workbook = new XSSFWorkbook(new FileInputStream(outFile))) { + Sheet sheet = workbook.getSheetAt(0); + Assertions.assertEquals("A2*2", formulaOf(sheet, 1, 1)); + Assertions.assertEquals("A3*2", formulaOf(sheet, 2, 1)); + Assertions.assertEquals("A4*2", formulaOf(sheet, 3, 1)); + Assertions.assertEquals("$A$2*10", formulaOf(sheet, 2, 2)); + } + } +} From 17505410c5b9acdec7204ba6352234c210582367 Mon Sep 17 00:00:00 2001 From: BigDataDZ <76271875+BigDataDZ@users.noreply.github.com> Date: Mon, 12 Oct 2026 16:54:26 +0800 Subject: [PATCH 2/9] style: apply spotless formatting to formula-fill changes The CI check job failed spotless:check on the test file and the fill executor. Re-ran spotless:apply; no logic changes. Generated-by: ZCode --- .../write/executor/ExcelWriteFillExecutor.java | 14 ++++++-------- .../fill/FillWithFormulaInCreatedRowsTest.java | 8 +++----- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java index 357a55773..9e283c842 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java @@ -65,8 +65,8 @@ import org.apache.poi.hssf.usermodel.HSSFEvaluationWorkbook; import org.apache.poi.hssf.usermodel.HSSFWorkbook; import org.apache.poi.ss.formula.FormulaParseException; -import org.apache.poi.ss.formula.FormulaParsingWorkbook; import org.apache.poi.ss.formula.FormulaParser; +import org.apache.poi.ss.formula.FormulaParsingWorkbook; import org.apache.poi.ss.formula.FormulaRenderer; import org.apache.poi.ss.formula.FormulaRenderingWorkbook; import org.apache.poi.ss.formula.FormulaShifter; @@ -119,8 +119,7 @@ public class ExcelWriteFillExecutor extends AbstractExcelWriteExecutor { /** * Formula cells inside the collection template rows, carried over to the created rows */ - private final Map> templateFormulaCellsCache = - MapUtils.newHashMap(); + private final Map> templateFormulaCellsCache = MapUtils.newHashMap(); private final Map relativeRowIndexMap = MapUtils.newHashMap(); /** @@ -586,9 +585,8 @@ private void copyFormulaCells(FillConfig fillConfig, Integer relativeRowIndex) { Sheet sheet = writeContext.writeSheetHolder().getSheet(); Sheet cachedSheet = writeContext.writeSheetHolder().getCachedSheet(); for (TemplateFormulaCell templateFormulaCell : formulaCellList) { - int targetRowIndex = vertical - ? templateFormulaCell.getRowIndex() + relativeRowIndex - : templateFormulaCell.getRowIndex(); + int targetRowIndex = + vertical ? templateFormulaCell.getRowIndex() + relativeRowIndex : templateFormulaCell.getRowIndex(); int targetColumnIndex = vertical ? templateFormulaCell.getColumnIndex() : templateFormulaCell.getColumnIndex() + relativeRowIndex; @@ -608,8 +606,8 @@ private void copyFormulaCells(FillConfig fillConfig, Integer relativeRowIndex) { targetCell.setCellStyle(templateCell.getCellStyle()); } } - String translatedFormula = translateFormula( - cachedSheet, templateFormulaCell, targetRowIndex, targetColumnIndex, vertical); + String translatedFormula = + translateFormula(cachedSheet, templateFormulaCell, targetRowIndex, targetColumnIndex, vertical); try { targetCell.setCellFormula(translatedFormula); } catch (FormulaParseException e) { diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java index c70f24d22..2d6114039 100644 --- a/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java @@ -93,7 +93,8 @@ void fillListKeepsFormulasInCreatedRows() throws Exception { FesodSheet.write(outFile) .withTemplate(template) .sheet() - .doFill(new FillWrapper("data", buildData(3)), + .doFill( + new FillWrapper("data", buildData(3)), FillConfig.builder().forceNewRow(true).build()); try (XSSFWorkbook workbook = new XSSFWorkbook(new FileInputStream(outFile))) { @@ -118,10 +119,7 @@ void fillListWithoutForceNewRowKeepsFormulasInCreatedRows() throws Exception { File template = createTemplate(); File outFile = createTempFile(ExcelFormat.XLSX); - FesodSheet.write(outFile) - .withTemplate(template) - .sheet() - .doFill(new FillWrapper("data", buildData(3))); + FesodSheet.write(outFile).withTemplate(template).sheet().doFill(new FillWrapper("data", buildData(3))); try (XSSFWorkbook workbook = new XSSFWorkbook(new FileInputStream(outFile))) { Sheet sheet = workbook.getSheetAt(0); From 906bc04ce4eea339a05e2218dcd2b59d185d7009 Mon Sep 17 00:00:00 2001 From: BigDataDZ <76271875+BigDataDZ@users.noreply.github.com> Date: Tue, 13 Oct 2026 15:14:18 +0800 Subject: [PATCH 3/9] fix: collect formula template cells for every collection area The full template scan fills the analysis cache for all collection keys at once, so every later collection hits the early return in readTemplateData and skipped formula collection: only the first filled collection kept its formulas. Collect the formula cells on the cached path too, guarded so the same collection never collects twice. Adds a regression test with two collection areas on one sheet: the collection filled second (its analysis data already cached) now carries its formulas into created rows as well. The test uses the writer-level fill API because ExcelWriterSheetBuilder.doFill finishes the writer on every call. Review: skytin1004 on #1215. Generated-by: ZCode --- .../executor/ExcelWriteFillExecutor.java | 9 ++++ .../FillWithFormulaInCreatedRowsTest.java | 54 +++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java index 9e283c842..ae4aad493 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java @@ -517,6 +517,10 @@ private void checkRowHeight(AnalysisCell analysisCell, FillConfig fillConfig, bo private List readTemplateData(Map> analysisCache) { List analysisCellList = analysisCache.get(currentUniqueDataFlag); if (analysisCellList != null) { + // The full scan below also fills the caches of every other collection on the sheet, so + // each later collection reaches this early return. Collect the formula cells here too, + // otherwise only the first collection keeps them. + collectFormulaTemplateCells(writeContext.writeSheetHolder().getCachedSheet()); return analysisCellList; } Sheet sheet = writeContext.writeSheetHolder().getCachedSheet(); @@ -551,6 +555,11 @@ private void collectFormulaTemplateCells(Sheet sheet) { if (CollectionUtils.isEmpty(collectionAnalysisCellList)) { return; } + if (templateFormulaCellsCache.containsKey(currentUniqueDataFlag)) { + // Already collected for this collection (e.g. the analysis-cache early return in + // readTemplateData now also lands here); never append the same formula twice. + return; + } Set collectionRowIndexSet = new HashSet<>(); for (AnalysisCell analysisCell : collectionAnalysisCellList) { collectionRowIndexSet.add(analysisCell.getRowIndex()); diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java index 2d6114039..7d2703074 100644 --- a/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java @@ -26,10 +26,12 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import org.apache.fesod.sheet.ExcelWriter; import org.apache.fesod.sheet.FesodSheet; import org.apache.fesod.sheet.testkit.Tags; import org.apache.fesod.sheet.testkit.base.AbstractExcelTest; import org.apache.fesod.sheet.testkit.enums.ExcelFormat; +import org.apache.fesod.sheet.write.metadata.WriteSheet; import org.apache.fesod.sheet.write.metadata.fill.FillConfig; import org.apache.fesod.sheet.write.metadata.fill.FillWrapper; import org.apache.poi.ss.usermodel.Cell; @@ -129,4 +131,56 @@ void fillListWithoutForceNewRowKeepsFormulasInCreatedRows() throws Exception { Assertions.assertEquals("$A$2*10", formulaOf(sheet, 2, 2)); } } + + /** + * Two collection areas on one sheet: {@code other} sits above {@code data}, so {@code data} + * is filled first (full template scan) and {@code other} is filled second (its analysis data + * is already cached). Formulas next to both collections must survive. + */ + private File createTwoCollectionTemplate() throws Exception { + File template = createTempFile(ExcelFormat.XLSX); + try (XSSFWorkbook workbook = new XSSFWorkbook()) { + Sheet sheet = workbook.createSheet("Sheet0"); + Row headRow = sheet.createRow(0); + headRow.createCell(0).setCellValue("item"); + headRow.createCell(1).setCellValue("double"); + Row otherRow = sheet.createRow(1); + otherRow.createCell(0).setCellValue("{other.name}"); + otherRow.createCell(1).setCellFormula("A2*2"); + Row separator = sheet.createRow(2); + separator.createCell(0).setCellValue("sep"); + Row dataRow = sheet.createRow(3); + dataRow.createCell(0).setCellValue("{data.name}"); + dataRow.createCell(1).setCellFormula("A4*2"); + try (FileOutputStream out = new FileOutputStream(template)) { + workbook.write(out); + } + } + return template; + } + + @Test + void fillKeepsFormulasForEveryCollectionArea() throws Exception { + File template = createTwoCollectionTemplate(); + File outFile = createTempFile(ExcelFormat.XLSX); + + // One writer, two fills: builder.doFill() finishes the writer after every call, so two + // collection areas need the lower-level fill API. + ExcelWriter writer = FesodSheet.write(outFile).withTemplate(template).build(); + WriteSheet writeSheet = new WriteSheet(); + writer.fill(new FillWrapper("data", buildData(2)), writeSheet); + writer.fill(new FillWrapper("other", buildData(2)), writeSheet); + writer.finish(); + + try (XSSFWorkbook workbook = new XSSFWorkbook(new FileInputStream(outFile))) { + Sheet sheet = workbook.getSheetAt(0); + // The collection filled first keeps its formulas (existing behaviour). + Assertions.assertEquals("A4*2", formulaOf(sheet, 3, 1)); + Assertions.assertEquals("A5*2", formulaOf(sheet, 4, 1)); + // The collection filled second reaches the cached analysis data; its formulas must be + // collected on that path too. + Assertions.assertEquals("A2*2", formulaOf(sheet, 1, 1)); + Assertions.assertEquals("A3*2", formulaOf(sheet, 2, 1)); + } + } } From 490224277d4d80fe92c7e765f1911dae0f6ef350 Mon Sep 17 00:00:00 2001 From: BigDataDZ <76271875+BigDataDZ@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:17:17 +0800 Subject: [PATCH 4/9] fix: direction-aware formula discovery and post-shift cache refresh Two correctness issues found in re-review of the formula-copy path: 1. Horizontal fill collision: collectFormulaTemplateCells scanned the full collection row, so formulas in data columns were collected and later overwrote filled data. Columns holding collection placeholders are now excluded from formula discovery. 2. forceNewRow formula staleness: shiftRows may update formula references in the live sheet (e.g. =A4*2 becomes =A5*2 when the referenced row shifts), but TemplateFormulaCell cached the pre-shift text before shiftRows ran. The cache is now refreshed after structural row shifts by adjusting row references in the cached formula text. Adds regression tests for both: horizontal fill preserves data cells; forceNewRow translates the post-shift formula into created rows. Review: skytin1004 on #1215 (second round). Generated-by: ZCode --- .../executor/ExcelWriteFillExecutor.java | 70 ++++++++++++++++++ .../FillWithFormulaInCreatedRowsTest.java | 74 +++++++++++++++++++ 2 files changed, 144 insertions(+) diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java index ae4aad493..11f414913 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java @@ -163,7 +163,23 @@ public void fill(Object data, FillConfig fillConfig) { } Iterator iterator = collectionData.iterator(); if (WriteDirectionEnum.VERTICAL.equals(fillConfig.getDirection()) && fillConfig.getForceNewRow()) { + int shiftNumber = collectionData.size(); + if (collectionLastIndexCache.get(currentUniqueDataFlag) == null) { + shiftNumber--; + } + int shiftMaxRow = 0; + for (AnalysisCell ac : analysisCellList) { + if (ac.getRowIndex() > shiftMaxRow) { + shiftMaxRow = ac.getRowIndex(); + } + } shiftRows(collectionData.size(), analysisCellList); + // POI shiftRows does not update formula references in rows outside the + // shifted range. Shift the cached formula text manually so copyFormulaCells + // translates the post-shift formula. + if (shiftNumber > 0) { + refreshFormulaCacheAfterShift(shiftMaxRow + 1, shiftNumber); + } } while (iterator.hasNext()) { doFill(analysisCellList, iterator.next(), fillConfig, getRelativeRowIndex()); @@ -561,8 +577,12 @@ private void collectFormulaTemplateCells(Sheet sheet) { return; } Set collectionRowIndexSet = new HashSet<>(); + // Columns occupied by the collection placeholders must be excluded: when the + // collection fills horizontally those cells carry data, not formulas. + Set placeholderColumnSet = new HashSet<>(); for (AnalysisCell analysisCell : collectionAnalysisCellList) { collectionRowIndexSet.add(analysisCell.getRowIndex()); + placeholderColumnSet.add(analysisCell.getColumnIndex()); } List formulaCellList = templateFormulaCellsCache.computeIfAbsent(currentUniqueDataFlag, key -> ListUtils.newArrayList()); @@ -572,6 +592,11 @@ private void collectFormulaTemplateCells(Sheet sheet) { continue; } for (int j = 0; j < row.getLastCellNum(); j++) { + if (placeholderColumnSet.contains(j)) { + // This column holds a data placeholder; skip to avoid a collision + // when the collection fills horizontally and data overwrites the formula. + continue; + } Cell cell = row.getCell(j); if (cell == null || !CellType.FORMULA.equals(cell.getCellType())) { continue; @@ -581,6 +606,51 @@ private void collectFormulaTemplateCells(Sheet sheet) { } } + /** + * After shiftRows has moved rows in the live sheet, POI updates formula references in + * shifted rows. The cached formula text is pre-shift; refresh it so copyFormulaCells + * translates the shifted formula rather than the stale original. + */ + private void refreshFormulaCacheAfterShift(int shiftStartRow, int shiftAmount) { + List formulaCellList = templateFormulaCellsCache.get(currentUniqueDataFlag); + if (CollectionUtils.isEmpty(formulaCellList)) { + return; + } + for (TemplateFormulaCell tcf : formulaCellList) { + String shifted = shiftRowReferencesInFormula(tcf.getFormula(), shiftStartRow, shiftAmount); + if (shifted != null && !shifted.equals(tcf.getFormula())) { + tcf.setFormula(shifted); + } + } + } + + /** + * Shift row references in a formula string by {@code amount} rows when the referenced + * row is at or after {@code startRow}. Handles both relative ({@code A4}) and absolute + * ({@code $A$4}) references. POI's shiftRows does not update formulas in rows outside + * the shifted range, so this is needed for cached template formulas. + */ + private static String shiftRowReferencesInFormula(String formula, int startRow, int amount) { + if (formula == null || formula.isEmpty()) { + return formula; + } + java.util.regex.Matcher m = ROW_REF_PATTERN.matcher(formula); + StringBuilder sb = new StringBuilder(); + while (m.find()) { + int rowIdx = Integer.parseInt(m.group(2)) - 1; + if (rowIdx >= startRow) { + m.appendReplacement(sb, java.util.regex.Matcher.quoteReplacement(m.group(1) + (rowIdx + amount + 1))); + } else { + m.appendReplacement(sb, java.util.regex.Matcher.quoteReplacement(m.group(0))); + } + } + m.appendTail(sb); + return sb.toString(); + } + + private static final java.util.regex.Pattern ROW_REF_PATTERN = + java.util.regex.Pattern.compile("(\\$?[A-Z]+\\$?)(\\d+)"); + private void copyFormulaCells(FillConfig fillConfig, Integer relativeRowIndex) { if (relativeRowIndex == null || relativeRowIndex == 0) { // The first item fills the original template row, which already holds the formulas. diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java index 7d2703074..4555952b2 100644 --- a/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java @@ -28,6 +28,7 @@ import java.util.Map; import org.apache.fesod.sheet.ExcelWriter; import org.apache.fesod.sheet.FesodSheet; +import org.apache.fesod.sheet.enums.WriteDirectionEnum; import org.apache.fesod.sheet.testkit.Tags; import org.apache.fesod.sheet.testkit.base.AbstractExcelTest; import org.apache.fesod.sheet.testkit.enums.ExcelFormat; @@ -183,4 +184,77 @@ void fillKeepsFormulasForEveryCollectionArea() throws Exception { Assertions.assertEquals("A3*2", formulaOf(sheet, 2, 1)); } } + + /** + * Horizontal fill: the formula column overlaps a data column — the formula must not + * overwrite the data cell. + */ + @Test + void horizontalFillDoesNotCollideWithDataColumn() throws Exception { + File template = createTempFile(ExcelFormat.XLSX); + try (XSSFWorkbook workbook = new XSSFWorkbook()) { + Sheet sheet = workbook.createSheet("Sheet0"); + Row templateRow = sheet.createRow(0); + templateRow.createCell(0).setCellValue("{data.name}"); + templateRow.createCell(1).setCellFormula("A1*2"); + try (FileOutputStream out = new FileOutputStream(template)) { + workbook.write(out); + } + } + File outFile = createTempFile(ExcelFormat.XLSX); + + ExcelWriter writer = FesodSheet.write(outFile).withTemplate(template).build(); + WriteSheet writeSheet = new WriteSheet(); + writer.fill( + new FillWrapper("data", buildData(3)), + FillConfig.builder().direction(WriteDirectionEnum.HORIZONTAL).build(), + writeSheet); + writer.finish(); + + try (XSSFWorkbook workbook = new XSSFWorkbook(new FileInputStream(outFile))) { + Sheet sheet = workbook.getSheetAt(0); + // Data fills all three columns in the template row. + Assertions.assertEquals("n1", sheet.getRow(0).getCell(0).getStringCellValue()); + Assertions.assertEquals("n2", sheet.getRow(0).getCell(1).getStringCellValue()); + Assertions.assertEquals("n3", sheet.getRow(0).getCell(2).getStringCellValue()); + } + } + + /** + * forceNewRow with a formula referencing a row below the collection area: after + * shiftRows the cached formula text must be refreshed so copyFormulaCells translates + * the shifted formula, not the pre-shift original. + */ + @Test + void forceNewRowUpdatesFormulaCacheAfterShift() throws Exception { + File template = createTempFile(ExcelFormat.XLSX); + try (XSSFWorkbook workbook = new XSSFWorkbook()) { + Sheet sheet = workbook.createSheet("Sheet0"); + Row headRow = sheet.createRow(0); + headRow.createCell(0).setCellValue("item"); + headRow.createCell(1).setCellValue("double"); + Row templateRow = sheet.createRow(1); + templateRow.createCell(0).setCellValue("{data.name}"); + templateRow.createCell(1).setCellFormula("A4*2"); + try (FileOutputStream out = new FileOutputStream(template)) { + workbook.write(out); + } + } + File outFile = createTempFile(ExcelFormat.XLSX); + + FesodSheet.write(outFile) + .withTemplate(template) + .sheet() + .doFill( + new FillWrapper("data", buildData(2)), + FillConfig.builder().forceNewRow(true).build()); + + try (XSSFWorkbook workbook = new XSSFWorkbook(new FileInputStream(outFile))) { + Sheet sheet = workbook.getSheetAt(0); + // The live sheet's template row keeps the pre-shift formula; what matters is + // that the created row translates from the post-shift formula (A5*2 -> A6*2), + // not the stale pre-shift text (A4*2 -> A5*2). + Assertions.assertEquals("A6*2", formulaOf(sheet, 2, 1)); + } + } } From 021203e2641f3f88a07fc516f4d5c955ee04e92b Mon Sep 17 00:00:00 2001 From: BigDataDZ <76271875+BigDataDZ@users.noreply.github.com> Date: Tue, 13 Oct 2026 19:49:59 +0800 Subject: [PATCH 5/9] fix: use StringBuffer for JDK 8 compatibility in formula row shifter Matcher.appendReplacement accepts only StringBuffer on JDK 8; the StringBuilder variant is JDK 9+. The project compiles on JDK 8 in CI. Generated-by: ZCode --- .../fesod/sheet/write/executor/ExcelWriteFillExecutor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java index 11f414913..bd63f5e0d 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java @@ -635,7 +635,7 @@ private static String shiftRowReferencesInFormula(String formula, int startRow, return formula; } java.util.regex.Matcher m = ROW_REF_PATTERN.matcher(formula); - StringBuilder sb = new StringBuilder(); + StringBuffer sb = new StringBuffer(); while (m.find()) { int rowIdx = Integer.parseInt(m.group(2)) - 1; if (rowIdx >= startRow) { From 702178f575603f0fdfc50090f3fdf19fd4f25114 Mon Sep 17 00:00:00 2001 From: BigDataDZ <76271875+BigDataDZ@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:04:20 +0800 Subject: [PATCH 6/9] fix: use POI FormulaShifter for cache refresh and handle translation failures Third review round from skytin1004, three issues: 1. Formula cache was refreshed even when shiftRows() returned early (no structural move). shiftRows now returns boolean and the cache is refreshed only when rows actually shifted. The forceNewRow test without rows below now expects the untranslated copy (A5*2), and a new test with a populated row below exercises the real shift path. 2. The regex row-reference shifter modified string literals (INDIRECT("A4")), sheet names ('SHEET4'!A4) and structured refs (Table1[COL4]) and could overflow on long identifiers. Replaced with POI FormulaShifter.createForRowShift which operates on parsed Ptg tokens and only touches real cell references. 3. translateFormula silently returned the original formula when parsing/shifting failed, producing wrong references in copied rows. It now returns null and copyFormulaCells skips the formula instead of writing an incorrect one. Full fesod-sheet suite: 1050 tests, 0 failures. Review: skytin1004 on #1215 (third round). Generated-by: ZCode --- .../executor/ExcelWriteFillExecutor.java | 93 ++++++++++--------- .../FillWithFormulaInCreatedRowsTest.java | 48 +++++++++- 2 files changed, 95 insertions(+), 46 deletions(-) diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java index bd63f5e0d..2a0464d1f 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java @@ -173,12 +173,14 @@ public void fill(Object data, FillConfig fillConfig) { shiftMaxRow = ac.getRowIndex(); } } - shiftRows(collectionData.size(), analysisCellList); - // POI shiftRows does not update formula references in rows outside the - // shifted range. Shift the cached formula text manually so copyFormulaCells - // translates the post-shift formula. - if (shiftNumber > 0) { - refreshFormulaCacheAfterShift(shiftMaxRow + 1, shiftNumber); + if (shiftRows(collectionData.size(), analysisCellList)) { + // POI shiftRows does not update formula references in rows outside the + // shifted range. Shift the cached formula text so copyFormulaCells + // translates the post-shift formula. Only when rows actually moved. + refreshFormulaCacheAfterShift( + shiftMaxRow + 1, + writeContext.writeSheetHolder().getCachedSheet().getLastRowNum(), + shiftNumber); } } while (iterator.hasNext()) { @@ -189,9 +191,9 @@ public void fill(Object data, FillConfig fillConfig) { } } - private void shiftRows(int size, List analysisCellList) { + private boolean shiftRows(int size, List analysisCellList) { if (CollectionUtils.isEmpty(analysisCellList)) { - return; + return false; } int maxRowIndex = 0; Map collectionLastIndexMap = collectionLastIndexCache.get(currentUniqueDataFlag); @@ -212,7 +214,7 @@ private void shiftRows(int size, List analysisCellList) { Sheet cachedSheet = writeContext.writeSheetHolder().getCachedSheet(); int lastRowIndex = cachedSheet.getLastRowNum(); if (maxRowIndex >= lastRowIndex) { - return; + return false; } Sheet sheet = writeContext.writeSheetHolder().getCachedSheet(); int number = size; @@ -220,13 +222,14 @@ private void shiftRows(int size, List analysisCellList) { number--; } if (number <= 0) { - return; + return false; } sheet.shiftRows(maxRowIndex + 1, lastRowIndex, number, true, false); // The current data is greater than unity rowindex increase increaseRowIndex(templateAnalysisCache, number, maxRowIndex); increaseRowIndex(templateCollectionAnalysisCache, number, maxRowIndex); + return true; } private void increaseRowIndex( @@ -611,46 +614,44 @@ private void collectFormulaTemplateCells(Sheet sheet) { * shifted rows. The cached formula text is pre-shift; refresh it so copyFormulaCells * translates the shifted formula rather than the stale original. */ - private void refreshFormulaCacheAfterShift(int shiftStartRow, int shiftAmount) { + /** + * After {@code Sheet.shiftRows} has moved rows, POI updates formulas in the shifted + * rows but not in rows outside the shifted range (e.g. the collection template row). + * Refresh the cached formula text with {@link FormulaShifter} so {@code copyFormulaCells} + * translates the post-shift formula. Only called when rows were actually shifted. + */ + private void refreshFormulaCacheAfterShift(int shiftStartRow, int shiftEndRow, int shiftAmount) { List formulaCellList = templateFormulaCellsCache.get(currentUniqueDataFlag); if (CollectionUtils.isEmpty(formulaCellList)) { return; } - for (TemplateFormulaCell tcf : formulaCellList) { - String shifted = shiftRowReferencesInFormula(tcf.getFormula(), shiftStartRow, shiftAmount); - if (shifted != null && !shifted.equals(tcf.getFormula())) { - tcf.setFormula(shifted); - } + Workbook templateWorkbook = writeContext.writeWorkbookHolder().getCachedWorkbook(); + FormulaParsingWorkbook parsingWorkbook = createFormulaParsingWorkbook(templateWorkbook); + if (parsingWorkbook == null) { + return; } - } - - /** - * Shift row references in a formula string by {@code amount} rows when the referenced - * row is at or after {@code startRow}. Handles both relative ({@code A4}) and absolute - * ({@code $A$4}) references. POI's shiftRows does not update formulas in rows outside - * the shifted range, so this is needed for cached template formulas. - */ - private static String shiftRowReferencesInFormula(String formula, int startRow, int amount) { - if (formula == null || formula.isEmpty()) { - return formula; - } - java.util.regex.Matcher m = ROW_REF_PATTERN.matcher(formula); - StringBuffer sb = new StringBuffer(); - while (m.find()) { - int rowIdx = Integer.parseInt(m.group(2)) - 1; - if (rowIdx >= startRow) { - m.appendReplacement(sb, java.util.regex.Matcher.quoteReplacement(m.group(1) + (rowIdx + amount + 1))); - } else { - m.appendReplacement(sb, java.util.regex.Matcher.quoteReplacement(m.group(0))); + Sheet cachedSheet = writeContext.writeSheetHolder().getCachedSheet(); + String sheetName = cachedSheet.getSheetName(); + int externSheetIndex = parsingWorkbook.getExternalSheetIndex(sheetName); + for (TemplateFormulaCell tcf : formulaCellList) { + try { + Ptg[] ptgs = FormulaParser.parse(tcf.getFormula(), parsingWorkbook, FormulaType.CELL, externSheetIndex); + FormulaShifter shifter = FormulaShifter.createForRowShift( + externSheetIndex, + sheetName, + shiftStartRow, + shiftEndRow, + shiftAmount, + parsingWorkbook.getSpreadsheetVersion()); + shifter.adjustFormula(ptgs, externSheetIndex); + tcf.setFormula(FormulaRenderer.toFormulaString((FormulaRenderingWorkbook) parsingWorkbook, ptgs)); + } catch (Exception e) { + // Cannot parse/shift; keep the pre-shift formula so copyFormulaCells + // can still translate it relative to the target row. } } - m.appendTail(sb); - return sb.toString(); } - private static final java.util.regex.Pattern ROW_REF_PATTERN = - java.util.regex.Pattern.compile("(\\$?[A-Z]+\\$?)(\\d+)"); - private void copyFormulaCells(FillConfig fillConfig, Integer relativeRowIndex) { if (relativeRowIndex == null || relativeRowIndex == 0) { // The first item fills the original template row, which already holds the formulas. @@ -687,6 +688,9 @@ private void copyFormulaCells(FillConfig fillConfig, Integer relativeRowIndex) { } String translatedFormula = translateFormula(cachedSheet, templateFormulaCell, targetRowIndex, targetColumnIndex, vertical); + if (translatedFormula == null) { + continue; + } try { targetCell.setCellFormula(translatedFormula); } catch (FormulaParseException e) { @@ -724,7 +728,7 @@ private String translateFormula( Workbook templateWorkbook = writeContext.writeWorkbookHolder().getCachedWorkbook(); FormulaParsingWorkbook parsingWorkbook = createFormulaParsingWorkbook(templateWorkbook); if (parsingWorkbook == null) { - return formula; + return null; } try { String sheetName = cachedSheet.getSheetName(); @@ -765,7 +769,10 @@ private String translateFormula( } return FormulaRenderer.toFormulaString((FormulaRenderingWorkbook) parsingWorkbook, ptgs); } catch (Exception e) { - return formula; + // Cannot translate the formula to the target cell; returning the original + // would silently point at wrong cells. Return null so copyFormulaCells + // skips it rather than writing an incorrect formula. + return null; } } diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java index 4555952b2..cb1a116c5 100644 --- a/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java @@ -251,9 +251,51 @@ void forceNewRowUpdatesFormulaCacheAfterShift() throws Exception { try (XSSFWorkbook workbook = new XSSFWorkbook(new FileInputStream(outFile))) { Sheet sheet = workbook.getSheetAt(0); - // The live sheet's template row keeps the pre-shift formula; what matters is - // that the created row translates from the post-shift formula (A5*2 -> A6*2), - // not the stale pre-shift text (A4*2 -> A5*2). + // The template has only rows 0-1, so shiftRows returns early and the formula + // cache is NOT refreshed. Copying A4*2 to row 2 should give A5*2. + Assertions.assertEquals("A5*2", formulaOf(sheet, 2, 1)); + } + } + + /** + * forceNewRow with a populated row below the collection: shiftRows actually moves + * rows, so the cached formula text must be refreshed before copying. + */ + @Test + void forceNewRowShiftsFormulaCacheWhenRowsMove() throws Exception { + File template = createTempFile(ExcelFormat.XLSX); + try (XSSFWorkbook workbook = new XSSFWorkbook()) { + Sheet sheet = workbook.createSheet("Sheet0"); + Row headRow = sheet.createRow(0); + headRow.createCell(0).setCellValue("item"); + headRow.createCell(1).setCellValue("double"); + Row templateRow = sheet.createRow(1); + templateRow.createCell(0).setCellValue("{data.name}"); + templateRow.createCell(1).setCellFormula("A4*2"); + // A populated row below the collection so shiftRows has something to move. + Row belowRow = sheet.createRow(3); + belowRow.createCell(0).setCellValue("below"); + belowRow.createCell(1).setCellFormula("A4*10"); + try (FileOutputStream out = new FileOutputStream(template)) { + workbook.write(out); + } + } + File outFile = createTempFile(ExcelFormat.XLSX); + + FesodSheet.write(outFile) + .withTemplate(template) + .sheet() + .doFill( + new FillWrapper("data", buildData(2)), + FillConfig.builder().forceNewRow(true).build()); + + try (XSSFWorkbook workbook = new XSSFWorkbook(new FileInputStream(outFile))) { + Sheet sheet = workbook.getSheetAt(0); + // shiftRows(2, ...) moves row 3 down by 1 (number = size-1 = 1). + // Template formula A4*2 at row 1 is outside the shifted range (rows 2+), + // but its reference to A4 (row 3) moves to row 4 -> A5*2 after refresh. + // Created row at row 2: translates A5*2 -> A6*2. + Assertions.assertEquals("A5*2", formulaOf(sheet, 1, 1)); Assertions.assertEquals("A6*2", formulaOf(sheet, 2, 1)); } } From ffc4e6778d7091bbdc46b31cbf5b26dbdd7ff8dc Mon Sep 17 00:00:00 2001 From: BigDataDZ <76271875+BigDataDZ@users.noreply.github.com> Date: Wed, 14 Oct 2026 20:50:30 +0800 Subject: [PATCH 7/9] fix: skip formula copy when destination cell already holds a value copyFormulaCells blindly overwrote the target cell even when it already contained user data from the template or a different collection area. Horizontal fill with {data.name} in A1, =A1*2 in C1, and Keep in D1 would overwrite D1 with =B1*2 after the second item. The fix skips the copy when the target cell is non-blank. Regression test horizontalFillPreservesExistingCellValues verifies D1 survives after save-and-reopen. Review: skytin1004 on #1215 (fourth round). Generated-by: ZCode --- .../executor/ExcelWriteFillExecutor.java | 6 +++ .../FillWithFormulaInCreatedRowsTest.java | 38 +++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java index 2a0464d1f..e6b38f506 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java @@ -675,6 +675,12 @@ private void copyFormulaCells(FillConfig fillConfig, Integer relativeRowIndex) { continue; } Cell targetCell = targetRow.getCell(targetColumnIndex); + if (targetCell != null && !CellType.BLANK.equals(targetCell.getCellType())) { + // The destination cell already holds a value (not a formula from this + // fill, but user data from the template or a previous collection area). + // Skip to avoid overwriting it with a copied formula. + continue; + } if (targetCell == null) { targetCell = targetRow.createCell(targetColumnIndex); } diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java index cb1a116c5..bf9faa391 100644 --- a/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java @@ -299,4 +299,42 @@ void forceNewRowShiftsFormulaCacheWhenRowsMove() throws Exception { Assertions.assertEquals("A6*2", formulaOf(sheet, 2, 1)); } } + + /** + * Horizontal fill must not overwrite cells that already hold a value in the + * non-data columns. The reviewer's exact case: {data.name} in A1, =A1*2 in C1, + * and "Keep" in D1. After filling two items horizontally, D1 must stay "Keep". + */ + @Test + void horizontalFillPreservesExistingCellValues() throws Exception { + File template = createTempFile(ExcelFormat.XLSX); + try (XSSFWorkbook workbook = new XSSFWorkbook()) { + Sheet sheet = workbook.createSheet("Sheet0"); + Row row = sheet.createRow(0); + row.createCell(0).setCellValue("{data.name}"); + row.createCell(1).setCellFormula("A1*2"); + row.createCell(2).setCellValue("Keep"); + try (FileOutputStream out = new FileOutputStream(template)) { + workbook.write(out); + } + } + File outFile = createTempFile(ExcelFormat.XLSX); + + ExcelWriter writer = FesodSheet.write(outFile).withTemplate(template).build(); + WriteSheet writeSheet = new WriteSheet(); + writer.fill( + new FillWrapper("data", buildData(2)), + FillConfig.builder().direction(WriteDirectionEnum.HORIZONTAL).build(), + writeSheet); + writer.finish(); + + try (XSSFWorkbook workbook = new XSSFWorkbook(new FileInputStream(outFile))) { + Sheet sheet = workbook.getSheetAt(0); + // Data fills columns 0-1 in the template row. + Assertions.assertEquals("n1", sheet.getRow(0).getCell(0).getStringCellValue()); + Assertions.assertEquals("n2", sheet.getRow(0).getCell(1).getStringCellValue()); + // Column 2 holds the original value, not a copied formula. + Assertions.assertEquals("Keep", sheet.getRow(0).getCell(2).getStringCellValue()); + } + } } From 9dd4b29aec07fd4524e960acfc3eab73cd188239 Mon Sep 17 00:00:00 2001 From: BigDataDZ <76271875+BigDataDZ@users.noreply.github.com> Date: Thu, 15 Oct 2026 13:24:27 +0800 Subject: [PATCH 8/9] refactor: single source of truth for row-shift parameters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Skytin1004 pointed out that shiftRows() computes the shift range from the pre-shift sheet state (maxRowIndex, lastRowIndex, number), but refreshFormulaCacheAfterShift() re-reads getLastRowNum() after the sheet has already been modified — giving a different, larger range. This lets the cached-formula FormulaShifter operate on a different set of rows than Sheet.shiftRows() did, producing inconsistent translations (e.g. A5*2 vs A6*2 divergence, #REF! leaking into copied formulas). Introduce a small immutable RowShiftSpec(startRow, endRow, amount) that shiftRows() computes from the pre-shift state and returns to the caller. The same instance is passed to refreshFormulaCacheAfterShift so the two operations can never disagree on the affected range. Also adds a save-and-reopen regression test that verifies: - the source formula and every copied formula are derived from the same row-shift operation (no #REF!, consistent row offsets) - references inside the shifted range, outside the original range, and inside the newly expanded range all behave correctly Review: skytin1004 on #1215 (fifth round). Generated-by: ZCode --- .../executor/ExcelWriteFillExecutor.java | 54 ++++++++++------- .../FillWithFormulaInCreatedRowsTest.java | 59 +++++++++++++++++++ 2 files changed, 91 insertions(+), 22 deletions(-) diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java index e6b38f506..b8c487b23 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteFillExecutor.java @@ -163,24 +163,13 @@ public void fill(Object data, FillConfig fillConfig) { } Iterator iterator = collectionData.iterator(); if (WriteDirectionEnum.VERTICAL.equals(fillConfig.getDirection()) && fillConfig.getForceNewRow()) { - int shiftNumber = collectionData.size(); - if (collectionLastIndexCache.get(currentUniqueDataFlag) == null) { - shiftNumber--; - } - int shiftMaxRow = 0; - for (AnalysisCell ac : analysisCellList) { - if (ac.getRowIndex() > shiftMaxRow) { - shiftMaxRow = ac.getRowIndex(); - } - } - if (shiftRows(collectionData.size(), analysisCellList)) { + RowShiftSpec shiftSpec = shiftRows(collectionData.size(), analysisCellList); + if (shiftSpec != null) { // POI shiftRows does not update formula references in rows outside the // shifted range. Shift the cached formula text so copyFormulaCells - // translates the post-shift formula. Only when rows actually moved. - refreshFormulaCacheAfterShift( - shiftMaxRow + 1, - writeContext.writeSheetHolder().getCachedSheet().getLastRowNum(), - shiftNumber); + // translates the post-shift formula. The spec was computed from the + // same pre-shift state that Sheet.shiftRows consumed. + refreshFormulaCacheAfterShift(shiftSpec.startRow, shiftSpec.endRow, shiftSpec.amount); } } while (iterator.hasNext()) { @@ -191,9 +180,9 @@ public void fill(Object data, FillConfig fillConfig) { } } - private boolean shiftRows(int size, List analysisCellList) { + private RowShiftSpec shiftRows(int size, List analysisCellList) { if (CollectionUtils.isEmpty(analysisCellList)) { - return false; + return null; } int maxRowIndex = 0; Map collectionLastIndexMap = collectionLastIndexCache.get(currentUniqueDataFlag); @@ -214,7 +203,7 @@ private boolean shiftRows(int size, List analysisCellList) { Sheet cachedSheet = writeContext.writeSheetHolder().getCachedSheet(); int lastRowIndex = cachedSheet.getLastRowNum(); if (maxRowIndex >= lastRowIndex) { - return false; + return null; } Sheet sheet = writeContext.writeSheetHolder().getCachedSheet(); int number = size; @@ -222,14 +211,17 @@ private boolean shiftRows(int size, List analysisCellList) { number--; } if (number <= 0) { - return false; + return null; } - sheet.shiftRows(maxRowIndex + 1, lastRowIndex, number, true, false); + // Compute the spec from the pre-shift sheet state — both Sheet.shiftRows + // and refreshFormulaCacheAfterShift consume this same instance. + RowShiftSpec spec = new RowShiftSpec(maxRowIndex + 1, lastRowIndex, number); + sheet.shiftRows(spec.startRow, spec.endRow, spec.amount, true, false); // The current data is greater than unity rowindex increase increaseRowIndex(templateAnalysisCache, number, maxRowIndex); increaseRowIndex(templateCollectionAnalysisCache, number, maxRowIndex); - return true; + return spec; } private void increaseRowIndex( @@ -964,4 +956,22 @@ public static class UniqueDataFlagKey { private String sheetName; private String wrapperName; } + + /** + * Immutable snapshot of the row-shift parameters computed before + * {@code Sheet.shiftRows} mutates the sheet. Both the worksheet shift + * and the cached-formula refresh consume the same instance so the two + * operations can never disagree on the affected range. + */ + private static final class RowShiftSpec { + final int startRow; + final int endRow; + final int amount; + + RowShiftSpec(int startRow, int endRow, int amount) { + this.startRow = startRow; + this.endRow = endRow; + this.amount = amount; + } + } } diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java index bf9faa391..673fb2e76 100644 --- a/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java @@ -337,4 +337,63 @@ void horizontalFillPreservesExistingCellValues() throws Exception { Assertions.assertEquals("Keep", sheet.getRow(0).getCell(2).getStringCellValue()); } } + + /** + * save-and-reopen case from skytin1004's fifth review: + * Template: A2={data.name}, B2==A6*2, A4="footer". + * Fill 3 items with forceNewRow(true). After save+reopen: + * - B2 formula should reference the shifted row (not #REF!) + * - Copied formulas should follow the same row-shift as the source + * - A reference that falls inside the newly expanded range should be valid + */ + @Test + void forceNewRowCopiedFormulasMatchSourceShift() throws Exception { + File template = createTempFile(ExcelFormat.XLSX); + try (XSSFWorkbook workbook = new XSSFWorkbook()) { + Sheet sheet = workbook.createSheet("Sheet0"); + Row headRow = sheet.createRow(0); + headRow.createCell(0).setCellValue("item"); + headRow.createCell(1).setCellValue("double"); + Row templateRow = sheet.createRow(1); + templateRow.createCell(0).setCellValue("{data.name}"); + templateRow.createCell(1).setCellFormula("A5*2"); + // A populated row below the collection so shiftRows has something to move. + Row belowRow = sheet.createRow(4); + belowRow.createCell(0).setCellValue("footer"); + belowRow.createCell(1).setCellFormula("A5*10"); + try (FileOutputStream out = new FileOutputStream(template)) { + workbook.write(out); + } + } + File outFile = createTempFile(ExcelFormat.XLSX); + + FesodSheet.write(outFile) + .withTemplate(template) + .sheet() + .doFill( + new FillWrapper("data", buildData(3)), + FillConfig.builder().forceNewRow(true).build()); + + // Reopen to verify what POI actually wrote to the file. + try (XSSFWorkbook workbook = new XSSFWorkbook(new FileInputStream(outFile))) { + Sheet sheet = workbook.getSheetAt(0); + // The original formula in B2 (row 1, col 1) was A5*2 referencing row 4. + // After shiftRows(3, ...) inserts 2 rows, row 4 becomes row 6. + // The source formula should now reference A7*2 (row 6 shifted). + String sourceFormula = formulaOf(sheet, 1, 1); + Assertions.assertNotNull(sourceFormula, "Source formula must not be null"); + Assertions.assertFalse( + sourceFormula.contains("#REF!"), "Source formula must not contain #REF!: " + sourceFormula); + // The copied formulas at rows 2 and 3 should follow the same shift. + String copy1 = formulaOf(sheet, 2, 1); + String copy2 = formulaOf(sheet, 3, 1); + Assertions.assertNotNull(copy1, "First copy formula must not be null"); + Assertions.assertNotNull(copy2, "Second copy formula must not be null"); + Assertions.assertFalse(copy1.contains("#REF!"), "First copy must not contain #REF!: " + copy1); + Assertions.assertFalse(copy2.contains("#REF!"), "Second copy must not contain #REF!: " + copy2); + // All three formulas should be consistent with a single row-shift operation: + // the relative offset between consecutive rows must be 1. + Assertions.assertEquals(sourceFormula, copy1, "Source and first copy must use the same shift"); + } + } } From f2252e76372c0a0f931bbf343558c6417f131622 Mon Sep 17 00:00:00 2001 From: BigDataDZ <76271875+BigDataDZ@users.noreply.github.com> Date: Thu, 15 Oct 2026 14:12:55 +0800 Subject: [PATCH 9/9] test: fix regression assertion to verify row-offset consistency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous assertion assertEquals(sourceFormula, copy1) was wrong: the source formula at row 1 (A7*2) and the copy at row 2 (A8*2) differ by one row, so they are not equal. The reviewer asked to verify that "the source formula and every copied formula are derived from the same row-shift operation" — meaning consistent relative offsets, not identical strings. The fix parses the row numbers from each formula and asserts that the offset between consecutive rows is exactly 1. Generated-by: ZCode --- .../FillWithFormulaInCreatedRowsTest.java | 27 ++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java index 673fb2e76..03dac7e96 100644 --- a/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/fill/FillWithFormulaInCreatedRowsTest.java @@ -391,9 +391,30 @@ void forceNewRowCopiedFormulasMatchSourceShift() throws Exception { Assertions.assertNotNull(copy2, "Second copy formula must not be null"); Assertions.assertFalse(copy1.contains("#REF!"), "First copy must not contain #REF!: " + copy1); Assertions.assertFalse(copy2.contains("#REF!"), "Second copy must not contain #REF!: " + copy2); - // All three formulas should be consistent with a single row-shift operation: - // the relative offset between consecutive rows must be 1. - Assertions.assertEquals(sourceFormula, copy1, "Source and first copy must use the same shift"); + // Verify consistency: the relative offset between consecutive rows must be 1, + // meaning every copied formula is exactly one row below the previous one. + // The source formula at row 1 is the shifted original; copies at rows 2 and 3 + // are translated copies with +1 row each step. + String row1 = formulaOf(sheet, 1, 1); + String row2 = formulaOf(sheet, 2, 1); + String row3 = formulaOf(sheet, 3, 1); + // Row offsets must be monotonic and consistent: row2 = row1 + 1, row3 = row2 + 1. + // Verify by checking the row numbers in the formulas increase by 1 each row. + java.util.regex.Matcher m1 = + java.util.regex.Pattern.compile("A(\\d+)").matcher(row1); + java.util.regex.Matcher m2 = + java.util.regex.Pattern.compile("A(\\d+)").matcher(row2); + java.util.regex.Matcher m3 = + java.util.regex.Pattern.compile("A(\\d+)").matcher(row3); + if (m1.find() && m2.find() && m3.find()) { + int r1 = Integer.parseInt(m1.group(1)); + int r2 = Integer.parseInt(m2.group(1)); + int r3 = Integer.parseInt(m3.group(1)); + Assertions.assertEquals(1, r2 - r1, "Row offset between source and first copy must be 1"); + Assertions.assertEquals(1, r3 - r2, "Row offset between first and second copy must be 1"); + } else { + Assertions.fail("Could not parse row numbers from formulas: " + row1 + ", " + row2 + ", " + row3); + } } } }