From ea07bde0e3b7bed026c7c3e73833fe7358412d97 Mon Sep 17 00:00:00 2001 From: Sean-Walker0 <309789691+Sean-Walker0@users.noreply.github.com> Date: Fri, 9 Oct 2026 10:25:31 +0800 Subject: [PATCH] fix(sheet): apply AUTO header merge context on the right axis The AUTO merge strategy in ExcelWriteHeadProperty validates merge context on the wrong axis, with two visible effects: - Horizontal runs still join same-name cells that live under different parent groups. This is what issue #666 reported for a dynamic head (head30/head31 next to head40/head31): the reported C2:D2 merge still happens on current main. - The AUTO-only canMergeVertically check compares the cell above a vertical run with a cell inside the run. A vertical run starting at row >= 1 only exists when the cell above differs from the run (an equal cell above would have consumed the run from the earlier row), so the comparison is provably false there and AUTO never merges equal cells below the first header row anymore - a regression against both the pre-strategy automatic merge and VERTICAL_ONLY. Fix both by validating context where it belongs: under AUTO, a horizontal run may only join cells whose parent labels (the cell above each column) are equal, and vertical runs no longer carry the wrong-axis check, so equal cells under the same parent merge again. Verified with AutoHeaderMergeContextTest (both cases red before the change, green after) plus the full fesod-sheet suite (950 tests) and spotless:check. --- .../property/ExcelWriteHeadProperty.java | 56 +++------- .../head/AutoHeaderMergeContextTest.java | 105 ++++++++++++++++++ 2 files changed, 118 insertions(+), 43 deletions(-) create mode 100644 fesod-sheet/src/test/java/org/apache/fesod/sheet/head/AutoHeaderMergeContextTest.java diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/property/ExcelWriteHeadProperty.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/property/ExcelWriteHeadProperty.java index fb97d78c0..b4cbdb79c 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/property/ExcelWriteHeadProperty.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/property/ExcelWriteHeadProperty.java @@ -161,10 +161,21 @@ public List headCellRangeList(HeaderMergeStrategy mergeStrategy) { || mergeStrategy == HeaderMergeStrategy.AUTO) { for (int k = i + 1; k < headList.size(); k++) { String key = k + "-" + j; + // Under AUTO, same-name cells in a deeper row describe the same group only when their parent + // labels (the cell above each column) are equal. + boolean sameParentContext = j == 0 + || mergeStrategy != HeaderMergeStrategy.AUTO + || (headList.get(k).getHeadNameList().size() > j - 1 + && Objects.equals( + headNameList.get(j - 1), + headList.get(k) + .getHeadNameList() + .get(j - 1))); if (headList.get(k).getHeadNameList().size() > j && Objects.equals( headList.get(k).getHeadNameList().get(j), headName) - && !alreadyRangeSet.contains(key)) { + && !alreadyRangeSet.contains(key) + && sameParentContext) { alreadyRangeSet.add(key); lastCol = k; } else { @@ -180,7 +191,7 @@ public List headCellRangeList(HeaderMergeStrategy mergeStrategy) { || mergeStrategy == HeaderMergeStrategy.AUTO) { outer: for (int k = j + 1; k < headNameList.size(); k++) { - // For FULL_RECTANGLE and AUTO, verify all cells in the row + // Verify all cells in the row boolean canMerge = true; for (int l = i; l <= lastCol; l++) { String key = l + "-" + k; @@ -193,11 +204,6 @@ public List headCellRangeList(HeaderMergeStrategy mergeStrategy) { } } - // For AUTO strategy, also check context consistency - if (canMerge && mergeStrategy == HeaderMergeStrategy.AUTO) { - canMerge = canMergeVertically(headList, j, k, i, lastCol); - } - if (canMerge) { for (int l = i; l <= lastCol; l++) { String key = l + "-" + k; @@ -232,42 +238,6 @@ public List headCellRangeList(HeaderMergeStrategy mergeStrategy) { return cellRangeList; } - /** - * Check if two rows can be merged vertically based on context consistency - * - * @param headList The list of heads - * @param row1 First row index - * @param row2 Second row index - * @param startCol Start column index - * @param endCol End column index - * @return true if the rows can be merged - */ - private boolean canMergeVertically(List headList, int row1, int row2, int startCol, int endCol) { - // Check if there's a row above that provides context - if (row1 > 0) { - // Check if all cells in the range have the same context above - for (int col = startCol; col <= endCol; col++) { - boolean hasUpper1 = headList.get(col).getHeadNameList().size() > row1; - boolean hasUpper2 = headList.get(col).getHeadNameList().size() > row2; - - // If one row has upper context but the other doesn't, don't merge - if (hasUpper1 != hasUpper2) { - return false; - } - - if (hasUpper1) { - String upper1 = headList.get(col).getHeadNameList().get(row1 - 1); - String upper2 = headList.get(col).getHeadNameList().get(row2 - 1); - // If context (upper cells) is different, don't merge - if (!Objects.equals(upper1, upper2)) { - return false; - } - } - } - } - return true; - } - /** * Verify if a rectangle region is valid (all cells exist and have the same name) * diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/head/AutoHeaderMergeContextTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/head/AutoHeaderMergeContextTest.java new file mode 100644 index 000000000..eabadbb32 --- /dev/null +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/head/AutoHeaderMergeContextTest.java @@ -0,0 +1,105 @@ +/* + * 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.head; + +import java.io.File; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +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.poi.ss.usermodel.Sheet; +import org.apache.poi.ss.usermodel.Workbook; +import org.apache.poi.ss.usermodel.WorkbookFactory; +import org.apache.poi.ss.util.CellRangeAddress; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +/** + * The AUTO merge strategy must not join same-name header cells that live under different parent groups, and it must + * keep merging equal cells vertically below the first header row. + */ +@Tag(Tags.ROUND_TRIP) +@Tag(Tags.WRITE) +public class AutoHeaderMergeContextTest extends AbstractExcelTest { + + @Test + public void autoDoesNotMergeSameNameCellsUnderDifferentParents() throws Exception { + File file = createTempFile("autoMergeIssue666", ExcelFormat.XLSX); + + List> head = new ArrayList<>(); + head.add(Arrays.asList("head10")); + head.add(Arrays.asList("head20", "head21")); + head.add(Arrays.asList("head30", "head31")); + head.add(Arrays.asList("head40", "head31")); + head.add(Arrays.asList("head40", "head41")); + FesodSheet.write(file).head(head).sheet().doWrite(dataRow()); + + // "head31" appears in the columns below "head30" and below "head40": they describe different groups and must + // stay separate (issue #666), while the plain first-row run "head40/head40" still merges. + Assertions.assertFalse( + containsRegion(file, 2, 3, 1, 1), "same-name cells under different parents must not merge"); + Assertions.assertTrue(containsRegion(file, 3, 4, 0, 0), "a first-row run of equal names still merges"); + } + + @Test + public void autoMergesVerticalRunsBelowTheFirstHeaderRow() throws Exception { + File file = createTempFile("autoMergeVertical", ExcelFormat.XLSX); + + List> head = new ArrayList<>(); + head.add(Arrays.asList("G", "A", "A")); + head.add(Arrays.asList("G", "B", "y")); + head.add(Arrays.asList("G", "B", "z")); + FesodSheet.write(file).head(head).sheet().doWrite(dataRow()); + + // "A"/"A" share the parent "G" and must merge vertically; "B"/"B" share the merged parent "G" run and must + // merge horizontally; the first-row "G" run merges as before. + Assertions.assertTrue( + containsRegion(file, 0, 0, 1, 2), "equal cells under the same parent must merge vertically"); + Assertions.assertTrue( + containsRegion(file, 1, 2, 1, 1), "equal cells with equal parents still merge horizontally"); + Assertions.assertTrue(containsRegion(file, 0, 2, 0, 0), "a first-row run of equal names still merges"); + } + + private boolean containsRegion(File file, int firstCol, int lastCol, int firstRow, int lastRow) throws Exception { + try (Workbook workbook = WorkbookFactory.create(file)) { + Sheet sheet = workbook.getSheetAt(0); + for (int i = 0; i < sheet.getNumMergedRegions(); i++) { + CellRangeAddress region = sheet.getMergedRegion(i); + if (region.getFirstColumn() == firstCol + && region.getLastColumn() == lastCol + && region.getFirstRow() == firstRow + && region.getLastRow() == lastRow) { + return true; + } + } + } + return false; + } + + private List> dataRow() { + List> data = new ArrayList<>(); + data.add(Arrays.asList("a", "b", "c", "d", "e")); + return data; + } +}