Skip to content
Open
Original file line number Diff line number Diff line change
Expand Up @@ -19,33 +19,123 @@

package org.apache.fesod.sheet.analysis.v07.handlers;

import lombok.extern.slf4j.Slf4j;
import org.apache.fesod.sheet.constant.ExcelXmlConstants;
import org.apache.fesod.sheet.context.xlsx.XlsxReadContext;
import org.apache.fesod.sheet.metadata.data.FormulaData;
import org.apache.fesod.sheet.read.metadata.holder.xlsx.XlsxReadSheetHolder;
import org.apache.fesod.sheet.util.StringUtils;
import org.apache.poi.ss.SpreadsheetVersion;
import org.apache.poi.ss.formula.FormulaParser;
import org.apache.poi.ss.formula.FormulaRenderer;
import org.apache.poi.ss.formula.FormulaType;
import org.apache.poi.ss.formula.SharedFormula;
import org.apache.poi.ss.formula.ptg.Ptg;
import org.xml.sax.Attributes;

/**
* Cell Handler
*
*/
@Slf4j
public class CellFormulaTagHandler extends AbstractXlsxTagHandler {

@Override
public void startElement(XlsxReadContext xlsxReadContext, String name, Attributes attributes) {
XlsxReadSheetHolder xlsxReadSheetHolder = xlsxReadContext.xlsxReadSheetHolder();
xlsxReadSheetHolder.setTempFormula(new StringBuilder());

String formulaType = attributes.getValue(ExcelXmlConstants.ATTRIBUTE_T);
String sharedIndex = attributes.getValue(ExcelXmlConstants.ATTRIBUTE_SI);

xlsxReadSheetHolder.setTempFormulaType(formulaType);
xlsxReadSheetHolder.setTempFormulaSharedIndex(sharedIndex != null ? Integer.parseInt(sharedIndex) : null);
}
Comment on lines +52 to 53
Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding error handling for Integer.parseInt(sharedIndex). If the shared index attribute contains an invalid integer value, this will throw a NumberFormatException that will propagate up the call stack. You should either wrap this in a try-catch block or validate the value before parsing.

Suggested change
xlsxReadSheetHolder.setTempFormulaSharedIndex(sharedIndex != null ? Integer.parseInt(sharedIndex) : null);
}
if (sharedIndex != null) {
try {
xlsxReadSheetHolder.setTempFormulaSharedIndex(Integer.parseInt(sharedIndex));
} catch (NumberFormatException e) {
// Optionally log the error or handle it as needed
xlsxReadSheetHolder.setTempFormulaSharedIndex(null);
}
} else {
xlsxReadSheetHolder.setTempFormulaSharedIndex(null);
}

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion! I checked the codebase and found that Integer.parseInt is used without try-catch in similar XML parsing contexts such as CellTagHandler.java:66. Meanwhile, according to the Office Open XML standard, the si attribute is defined as:

<attribute name="si" type="xsd:unsignedInt" use="optional"/>

The spec guarantees si is always an unsigned integer in valid Excel files. If it contains an invalid value, the Excel file is malformed and the NumberFormatException should propagate to fail fast rather than silently swallowing the error.


@Override
public void endElement(XlsxReadContext xlsxReadContext, String name) {
XlsxReadSheetHolder xlsxReadSheetHolder = xlsxReadContext.xlsxReadSheetHolder();
String formulaText = xlsxReadSheetHolder.getTempFormula().toString();
String formulaType = xlsxReadSheetHolder.getTempFormulaType();
Integer sharedIndex = xlsxReadSheetHolder.getTempFormulaSharedIndex();

if (ExcelXmlConstants.ATTRIBUTE_SHARED.equals(formulaType) && sharedIndex != null) {
formulaText = handleSharedFormula(xlsxReadSheetHolder, formulaText, sharedIndex);
}

FormulaData formulaData = new FormulaData();
formulaData.setFormulaValue(xlsxReadSheetHolder.getTempFormula().toString());
formulaData.setFormulaValue(formulaText);
xlsxReadSheetHolder.getTempCellData().setFormulaData(formulaData);
}

@Override
public void characters(XlsxReadContext xlsxReadContext, char[] ch, int start, int length) {
xlsxReadContext.xlsxReadSheetHolder().getTempFormula().append(ch, start, length);
}

/**
* Handles shared formula when reading from Excel
*/
private String handleSharedFormula(XlsxReadSheetHolder xlsxReadSheetHolder, String formulaText, int sharedIndex) {
Integer currentRow = xlsxReadSheetHolder.getRowIndex();
Integer currentCol = xlsxReadSheetHolder.getColumnIndex();

if (currentRow == null || currentCol == null) {
return formulaText;
}

// If formula text exists then this is the master cell
if (!StringUtils.isEmpty(formulaText)) {
xlsxReadSheetHolder
.getSharedFormulaMap()
.put(sharedIndex, new XlsxReadSheetHolder.SharedFormulaInfo(formulaText, currentRow, currentCol));
return formulaText;
} else {
// No formula text means this is a shared reference cell
XlsxReadSheetHolder.SharedFormulaInfo masterInfo =
xlsxReadSheetHolder.getSharedFormulaMap().get(sharedIndex);

if (masterInfo == null) {
log.warn(
"Master formula not found for shared index {} at row {}, col {}",
sharedIndex,
currentRow,
currentCol);
return "";
}

return convertSharedFormula(masterInfo, currentRow, currentCol);
}
}

/**
* Converts shared formula based on row and column offset
*/
private String convertSharedFormula(
XlsxReadSheetHolder.SharedFormulaInfo masterInfo, int currentRow, int currentCol) {
try {
// Parse the master formula text into tokens
Ptg[] masterPtgs = FormulaParser.parse(masterInfo.getFormulaText(), null, FormulaType.CELL, 0);

// Calculate offset from the master cell position
int rowOffset = currentRow - masterInfo.getFirstRow();
int colOffset = currentCol - masterInfo.getFirstCol();

// Use POI SharedFormula to convert with offset
SharedFormula sharedFormula = new SharedFormula(SpreadsheetVersion.EXCEL2007);
Ptg[] convertedPtgs = sharedFormula.convertSharedFormulas(masterPtgs, rowOffset, colOffset);

// Convert tokens back to formula string
return FormulaRenderer.toFormulaString(null, convertedPtgs);
} catch (Exception e) {
// If conversion fails, return the master formula as-is
// This handles cases like volatile functions with no cell references
// where the formula should be identical across all cells anyway
log.warn(
"Failed to convert shared formula at row {}, col {}. Using master formula instead.",
currentRow,
currentCol);
return masterInfo.getFormulaText();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,14 @@ public class ExcelXmlConstants {
* t attribute
*/
public static final String ATTRIBUTE_T = "t";
/**
* si attribute
*/
public static final String ATTRIBUTE_SI = "si";
/**
* shared attribute
*/
public static final String ATTRIBUTE_SHARED = "shared";
/**
* location attribute
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
package org.apache.fesod.sheet.read.metadata.holder.xlsx;

import java.util.Deque;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.Map;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.Setter;
Expand Down Expand Up @@ -54,6 +56,18 @@ public class XlsxReadSheetHolder extends ReadSheetHolder {
* Formula for current label.
*/
private StringBuilder tempFormula;
/**
* Formula type for current label.
*/
private String tempFormulaType;
/**
* Formula shared index for current label.
*/
private Integer tempFormulaSharedIndex;
/**
* Map to store master shared formulas by their shared index.
*/
private Map<Integer, SharedFormulaInfo> sharedFormulaMap;
Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getSharedFormulaMap exposes the internal representation stored in field sharedFormulaMap. The value may be modified after this call to getSharedFormulaMap.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion! However, this class uses @Getter at the class level, which already exposes all mutable fields directly including Deque<String> tagDeque. XlsxReadSheetHolder is an internal holder class designed to store temporary parsing state where direct map access is consistent with how other mutable fields in this class are used. The map is only accessed by CellFormulaTagHandler during the parsing process.

/**
* excel Relationship
*/
Expand All @@ -62,8 +76,35 @@ public class XlsxReadSheetHolder extends ReadSheetHolder {
public XlsxReadSheetHolder(ReadSheet readSheet, ReadWorkbookHolder readWorkbookHolder) {
super(readSheet, readWorkbookHolder);
this.tagDeque = new LinkedList<String>();
this.sharedFormulaMap = new HashMap<>();
packageRelationshipCollection = ((XlsxReadWorkbookHolder) readWorkbookHolder)
.getPackageRelationshipCollectionMap()
.get(readSheet.getSheetNo());
}

/**
* Information about a shared formula master cell.
*/
@Getter
@Setter
public static class SharedFormulaInfo {
/**
* The master formula text.
*/
private String formulaText;
/**
* Row index of the master formula cell.
*/
private int firstRow;
/**
* Column index of the master formula cell.
*/
private int firstCol;

public SharedFormulaInfo(String formulaText, int firstRow, int firstCol) {
this.formulaText = formulaText;
this.firstRow = firstRow;
this.firstCol = firstCol;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* 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.formula;

import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.Setter;
import org.apache.fesod.sheet.annotation.ExcelProperty;
import org.apache.fesod.sheet.metadata.data.ReadCellData;

/**
* Test data for shared formula reading
*/
@Getter
@Setter
@EqualsAndHashCode
public class FormulaData {
@ExcelProperty("date")
private String date;

@ExcelProperty("value1")
private Integer value1;

@ExcelProperty("value2")
private Integer value2;

@ExcelProperty("volatileFormula")
private ReadCellData<?> volatileFormula;

@ExcelProperty("relativeFormula")
private ReadCellData<?> relativeFormula;

@ExcelProperty("columnAbsoluteFormula")
private ReadCellData<?> columnAbsoluteFormula;

@ExcelProperty("mixedAbsoluteFormula")
private ReadCellData<?> mixedAbsoluteFormula;

@ExcelProperty("additionFormula")
private ReadCellData<?> additionFormula;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/*
* 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.formula;

import lombok.extern.slf4j.Slf4j;
import org.apache.fesod.sheet.context.AnalysisContext;
import org.apache.fesod.sheet.event.AnalysisEventListener;
import org.junit.jupiter.api.Assertions;

/**
* Listener for formula data testing
*/
@Slf4j
public class FormulaDataListener extends AnalysisEventListener<FormulaData> {

@Override
public void invoke(FormulaData data, AnalysisContext context) {
// Verify that shared formulas are read correctly
if (data.getVolatileFormula() != null && data.getVolatileFormula().getFormulaData() != null) {
String formula = data.getVolatileFormula().getFormulaData().getFormulaValue();
Assertions.assertNotNull(formula);
log.info("volatileFormula: {}", formula);
}

if (data.getRelativeFormula() != null && data.getRelativeFormula().getFormulaData() != null) {
String formula = data.getRelativeFormula().getFormulaData().getFormulaValue();
Assertions.assertNotNull(formula);
log.info("relativeFormula: {}", formula);
}

if (data.getColumnAbsoluteFormula() != null
&& data.getColumnAbsoluteFormula().getFormulaData() != null) {
String formula = data.getColumnAbsoluteFormula().getFormulaData().getFormulaValue();
Assertions.assertNotNull(formula);
log.info("columnAbsoluteFormula: {}", formula);
}

if (data.getMixedAbsoluteFormula() != null
&& data.getMixedAbsoluteFormula().getFormulaData() != null) {
String formula = data.getMixedAbsoluteFormula().getFormulaData().getFormulaValue();
Assertions.assertNotNull(formula);
log.info("mixedAbsoluteFormula: {}", formula);
}

if (data.getAdditionFormula() != null && data.getAdditionFormula().getFormulaData() != null) {
String formula = data.getAdditionFormula().getFormulaData().getFormulaValue();
Assertions.assertNotNull(formula);
log.info("additionFormula: {}", formula);
}
}

@Override
public void doAfterAllAnalysed(AnalysisContext context) {
log.info("All formula data analysed");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* 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.formula;

import java.io.File;
import org.apache.fesod.sheet.FesodSheet;
import org.apache.fesod.sheet.util.TestFileUtil;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

/**
* Test for shared formula reading
*/
public class FormulaDataTest {

private static File file07;

@BeforeAll
public static void init() {
file07 = TestFileUtil.readFile("formula" + File.separator + "shared_formula.xlsx");
}

@Test
public void t01ReadSharedFormula07() throws Exception {
FesodSheet.read(file07, FormulaData.class, new FormulaDataListener())
.sheet()
.doRead();
}
}
Binary file not shown.