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; + } +}