Skip to content

fix(sheet): apply AUTO header merge context on the right axis #1202

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.

Sign up for GitHub

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

Open
Sean-Walker0 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(sheet): apply AUTO header merge context on the right axis #1202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 13 additions & 43 deletions fesod-sheet/src/main/java/org/apache/fesod/sheet/write/property/ExcelWriteHeadProperty.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,21 @@ public List<CellRange> 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 {
Expand All @@ -180,7 +191,7 @@ public List<CellRange> 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;
Expand All @@ -193,11 +204,6 @@ public List<CellRange> 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;
Expand Down Expand Up @@ -232,42 +238,6 @@ public List<CellRange> 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<Head> 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)
*
Expand Down
105 changes: 105 additions & 0 deletions fesod-sheet/src/test/java/org/apache/fesod/sheet/head/AutoHeaderMergeContextTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<List<String>> 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<List<String>> 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<List<Object>> dataRow() {
List<List<Object>> data = new ArrayList<>();
data.add(Arrays.asList("a", "b", "c", "d", "e"));
return data;
}
}
Loading