Fix ODS read failing on empty rows written by LibreOffice

src/main/java/org/spdx/spreadsheetstore/ods/OdsRow.java

@@ -95,40 +95,32 @@ public synchronized Cell getCell(int cellnum) {
 		return null;
 	}
 
+	// A cell exists if it has a value, formula, annotation or non-default style (see getCell).
 	@Override
 	public short getFirstCellNum() {
-		com.github.miachm.sods.Range dataRange = sheet.getSodsSheet().getDataRange();
-		if (!cells.isEmpty()) {
-			int col = cells.firstKey();
-			if (dataRange != null && rowNum >= dataRange.getRow() && rowNum <= dataRange.getLastRow()) {
-				col = Math.min(col, dataRange.getColumn());
+		int maxCols = sheet.getSodsSheet().getMaxColumns();
+		for (int col = 0; col < maxCols; col++) {
+			if (getCell(col) != null) {
+				return toShort(col);
 			}
-			return col > Short.MAX_VALUE ? Short.MAX_VALUE : (short) col;
-		}
-		if (dataRange != null && rowNum >= dataRange.getRow() && rowNum <= dataRange.getLastRow()) {
-			int col = dataRange.getColumn();
-			return col > Short.MAX_VALUE ? Short.MAX_VALUE : (short) col;
 		}
 		return -1;
 	}
 
 	@Override
 	public short getLastCellNum() {
-		com.github.miachm.sods.Range dataRange = sheet.getSodsSheet().getDataRange();
-		if (!cells.isEmpty()) {
-			int nextCol = cells.lastKey() + 1;
-			if (dataRange != null && rowNum >= dataRange.getRow() && rowNum <= dataRange.getLastRow()) {
-				nextCol = Math.max(nextCol, dataRange.getLastColumn() + 1);
+		for (int col = sheet.getSodsSheet().getMaxColumns() - 1; col >= 0; col--) {
+			if (getCell(col) != null) {
+				return toShort(col + 1);
 			}
-			return nextCol > Short.MAX_VALUE ? Short.MAX_VALUE : (short) nextCol;
-		}
-		if (dataRange != null && rowNum >= dataRange.getRow() && rowNum <= dataRange.getLastRow()) {
-			int nextCol = dataRange.getLastColumn() + 1;
-			return nextCol > Short.MAX_VALUE ? Short.MAX_VALUE : (short) nextCol;
 		}
 		return -1;
 	}
 
+	private static short toShort(int col) {
+		return col > Short.MAX_VALUE ? Short.MAX_VALUE : (short) col;
+	}
+
 	@Override
 	public int getRowNum() {
 		return rowNum;

src/main/java/org/spdx/spreadsheetstore/ods/OdsSheet.java

@@ -46,6 +46,14 @@ public class OdsSheet implements Sheet {
 	private final OdsWorkbook workbook;
 	private final com.github.miachm.sods.Sheet sodsSheet;
 	private final NavigableMap<Integer, OdsRow> rows = new TreeMap<>();
+	/**
+	 * Lowest and highest SODS rows that may hold content, -1 if none.
+	 * Set by the first scan, then only narrowed.
+	 */
+	private int firstContentRow = -1;
+	private int lastContentRow = -1;
+	private boolean contentScanned = false;
+	private static final int PROBE_COLUMNS = 8;
 
 	private final List<CellRangeAddress> mergedRegions = new ArrayList<>();
 
@@ -89,7 +97,10 @@ public synchronized Row getRow(int rownum) {
 		if (row != null) {
 			return row;
 		}
-		if (rownum >= 0 && rownum < sodsSheet.getMaxRows()) {
+		// Null outside the content range, as in POI: SODS loads the empty rows LibreOffice
+		// writes around the data as real rows.
+		// Empty rows inside the range are returned; POI returns null for those not created.
+		if (rownum >= 0 && rownum < sodsSheet.getMaxRows() && inContentRange(rownum)) {
 			OdsRow newRow = new OdsRow(this, rownum);
 			rows.put(rownum, newRow);
 			return newRow;
@@ -107,55 +118,79 @@ public void removeRow(Row row) {
 		}
 	}
 
-	private boolean hasData() {
-		if (!rows.isEmpty()) {
-			return true;
+	/**
+	 * Finds the first and last rows with content.
+	 * First scan covers the whole sheet; later scans narrow the cached range.
+	 */
+	private void scanContentRows() {
+		if (contentScanned && firstContentRow < 0) {
+			return;
 		}
-		com.github.miachm.sods.Range dataRange = sodsSheet.getDataRange();
-		if (dataRange == null) {
+		int first = contentScanned ? firstContentRow : 0;
+		int last = contentScanned ? lastContentRow : sodsSheet.getMaxRows() - 1;
+		while (first <= last && !rowHasContent(first)) {
+			first++;
+		}
+		while (last >= first && !rowHasContent(last)) {
+			last--;
+		}
+		boolean hasContent = first <= last;
+		firstContentRow = hasContent ? first : -1;
+		lastContentRow = hasContent ? last : -1;
+		contentScanned = true;
+	}
+
+	/**
+	 * @param row SODS row index
+	 * @return true if any cell in the row has a value, formula or annotation (style alone is not content)
+	 */
+	private boolean rowHasContent(int row) {
+		int maxCols = sodsSheet.getMaxColumns();
+		// Probe leading cells first: content rows exit without a full-width read.
+		int probe = Math.min(maxCols, PROBE_COLUMNS);
+		for (int col = 0; col < probe; col++) {
+			com.github.miachm.sods.Range cell = sodsSheet.getRange(row, col);
+			if (cell.getValue() != null || cell.getFormula() != null || cell.getAnnotation() != null) {
+				return true;
+			}
+		}
+		if (maxCols <= probe) {
 			return false;
 		}
-		for (int r = dataRange.getRow(); r <= dataRange.getLastRow(); r++) {
-			for (int c = dataRange.getColumn(); c <= dataRange.getLastColumn(); c++) {
-				com.github.miachm.sods.Range range = sodsSheet.getRange(r, c);
-				if (range.getValue() != null || range.getFormula() != null || range.getAnnotation() != null) {
-					return true;
-				}
+		com.github.miachm.sods.Range rest = sodsSheet.getRange(row, probe, 1, maxCols - probe);
+		return anyNonNull(rest.getValues()[0]) || anyNonNull(rest.getFormulas()[0])
+				|| anyNonNull(rest.getAnnotations()[0]);
+	}
+
+	private static boolean anyNonNull(Object[] cells) {
+		for (Object cell : cells) {
+			if (cell != null) {
+				return true;
 			}
 		}
 		return false;
 	}
 
-	@Override
-	public int getFirstRowNum() {
-		if (!hasData()) {
-			return -1;
-		}
-		com.github.miachm.sods.Range dataRange = sodsSheet.getDataRange();
-		if (!rows.isEmpty()) {
-			int firstRow = rows.firstKey();
-			if (dataRange != null) {
-				return Math.min(firstRow, dataRange.getRow());
-			}
-			return firstRow;
+	private boolean inContentRange(int row) {
+		if (!contentScanned) {
+			scanContentRows();
 		}
-		return dataRange.getRow();
+		return row >= firstContentRow && row <= lastContentRow;
 	}
 
 	@Override
-	public int getLastRowNum() {
-		if (!hasData()) {
-			return -1;
-		}
-		com.github.miachm.sods.Range dataRange = sodsSheet.getDataRange();
-		if (!rows.isEmpty()) {
-			int lastRow = rows.lastKey();
-			if (dataRange != null) {
-				return Math.max(lastRow, dataRange.getLastRow());
-			}
-			return lastRow;
+	public synchronized int getFirstRowNum() {
+		scanContentRows();
+		if (rows.isEmpty()) {
+			return firstContentRow;
 		}
-		return dataRange.getLastRow();
+		return firstContentRow < 0 ? rows.firstKey() : Math.min(rows.firstKey(), firstContentRow);
+	}
+
+	@Override
+	public synchronized int getLastRowNum() {
+		scanContentRows();
+		return rows.isEmpty() ? lastContentRow : Math.max(rows.lastKey(), lastContentRow);
 	}
 
 	@Override

src/test/java/org/spdx/spreadsheetstore/OdsEmptyRowsTest.java

@@ -0,0 +1,177 @@
+/*
+ * SPDX-FileContributor: Arthit Suriyawongkul
+ * SPDX-FileCopyrightText: 2026 SPDX Contributors
+ * SPDX-FileType: SOURCE
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.spdx.spreadsheetstore;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+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.ss.usermodel.WorkbookFactory;
+import org.junit.Test;
+
+import org.spdx.core.InvalidSPDXAnalysisException;
+import org.spdx.library.ModelCopyManager;
+import org.spdx.library.SpdxModelFactory;
+import org.spdx.library.model.v2.SpdxConstantsCompatV2;
+import org.spdx.library.model.v2.SpdxDocument;
+import org.spdx.library.model.v2.SpdxElement;
+import org.spdx.spreadsheetstore.ods.OdsWorkbook;
+import org.spdx.storage.simple.InMemSpdxStore;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+
+/**
+ * ODS files from LibreOffice end every sheet with empty rows.
+ * Those rows must be ignored: content matches the XLSX example.
+ * Fixtures: <code>TestFiles/ods</code>.
+ */
+public class OdsEmptyRowsTest {
+
+	private static final String TEST_FILES = "TestFiles";
+	private static final String ODS_DIR = TEST_FILES + File.separator + "ods";
+	private static final String V2_3 = "SPDXSpreadsheetExample-v2.3";
+	private static final String[] EXAMPLES = new String[] {"SPDXSpreadsheetExample-2.0", "SPDXSpreadsheetExample-v2.2", V2_3};
+	/** v2.3 variants with extra empty rows that must load the same as the XLSX */
+	private static final String[] EMPTY_ROW_VARIANTS = new String[] {
+			V2_3 + "-trailing-repeated-5000", V2_3 + "-trailing-styled-blank",
+			V2_3 + "-trailing-many-columns", V2_3 + "-leading-empty-row"};
+
+	/** Element IDs and counts found in a spreadsheet */
+	private static class Contents {
+		List<String> files = new ArrayList<>();
+		List<String> snippets = new ArrayList<>();
+		List<String> packages = new ArrayList<>();
+		int relationships = 0;
+		int annotations = 0;
+	}
+
+	private Contents load(String path) throws InvalidSPDXAnalysisException, IOException {
+		SpreadsheetStore sst = new SpreadsheetStore(new InMemSpdxStore());
+		try (FileInputStream stream = new FileInputStream(path)) {
+			sst.deSerialize(stream, false);
+		}
+		ModelCopyManager cm = new ModelCopyManager();
+		String documentUri;
+		try (Stream<?> docs = SpdxModelFactory.getSpdxObjects(sst, cm,
+				SpdxConstantsCompatV2.CLASS_SPDX_DOCUMENT, null, null)) {
+			List<?> allDocs = docs.collect(Collectors.toList());
+			assertEquals(1, allDocs.size());
+			documentUri = ((SpdxDocument)allDocs.get(0)).getDocumentUri();
+		}
+		Contents retval = new Contents();
+		SpdxDocument doc = new SpdxDocument(sst, documentUri, cm, false);
+		retval.annotations = doc.getAnnotations().size();
+		retval.relationships = doc.getRelationships().size();
+		retval.files = ids(sst, cm, SpdxConstantsCompatV2.CLASS_SPDX_FILE, documentUri);
+		retval.snippets = ids(sst, cm, SpdxConstantsCompatV2.CLASS_SPDX_SNIPPET, documentUri);
+		retval.packages = ids(sst, cm, SpdxConstantsCompatV2.CLASS_SPDX_PACKAGE, documentUri);
+		return retval;
+	}
+
+	private List<String> ids(SpreadsheetStore sst, ModelCopyManager cm, String type, String documentUri)
+			throws InvalidSPDXAnalysisException {
+		List<String> retval = new ArrayList<>();
+		try (Stream<?> elements = SpdxModelFactory.getSpdxObjects(sst, cm, type, documentUri, documentUri + "#")) {
+			elements.forEach(e -> retval.add(((SpdxElement)e).getId()));
+		}
+		Collections.sort(retval);
+		return retval;
+	}
+
+	private void assertSameContents(Contents expected, Contents actual) {
+		assertEquals(expected.files, actual.files);
+		assertEquals(expected.snippets, actual.snippets);
+		assertEquals(expected.packages, actual.packages);
+		assertEquals(expected.relationships, actual.relationships);
+		assertEquals(expected.annotations, actual.annotations);
+	}
+
+	private void assertNoAnonymousSnippets(Contents contents) {
+		for (String id : contents.snippets) {
+			assertFalse("Phantom snippet from an empty row: " + id, id.contains("__anon__"));
+		}
+		assertEquals(contents.snippets.size(), new TreeSet<>(contents.snippets).size());
+	}
+
+	@Test
+	public void libreOfficeConvertedOdsMatchesXlsx() throws InvalidSPDXAnalysisException, IOException {
+		for (String example : EXAMPLES) {
+			Contents expected = load(TEST_FILES + File.separator + example + ".xlsx");
+			Contents ods = load(ODS_DIR + File.separator + example + ".ods");
+			assertNoAnonymousSnippets(ods);
+			assertSameContents(expected, ods);
+		}
+	}
+
+	@Test
+	public void emptyRowsAreIgnored() throws InvalidSPDXAnalysisException, IOException {
+		Contents expected = load(TEST_FILES + File.separator + V2_3 + ".xlsx");
+		assertFalse(expected.snippets.isEmpty());
+		for (String variant : EMPTY_ROW_VARIANTS) {
+			Contents ods = load(ODS_DIR + File.separator + variant + ".ods");
+			assertNoAnonymousSnippets(ods);
+			assertSameContents(expected, ods);
+		}
+	}
+
+	@Test
+	public void headerOnlySnippetSheetHasNoSnippets() throws InvalidSPDXAnalysisException, IOException {
+		Contents expected = load(TEST_FILES + File.separator + V2_3 + ".xlsx");
+		Contents ods = load(ODS_DIR + File.separator + V2_3 + "-snippets-header-only.ods");
+		assertTrue(ods.snippets.isEmpty());
+		assertEquals(expected.files, ods.files);
+		assertEquals(expected.packages, ods.packages);
+		assertEquals(expected.annotations, ods.annotations);
+	}
+
+	/** Row bounds of every sheet equal the XLSX's, shifted by the rows added before the data. */
+	private void assertRowBounds(String odsPath, int rowOffset) throws IOException {
+		try (FileInputStream xlsxStream = new FileInputStream(TEST_FILES + File.separator + V2_3 + ".xlsx");
+				FileInputStream odsStream = new FileInputStream(odsPath);
+				Workbook xlsx = WorkbookFactory.create(xlsxStream);
+				Workbook ods = new OdsWorkbook(odsStream)) {
+			assertEquals(xlsx.getNumberOfSheets(), ods.getNumberOfSheets());
+			for (int i = 0; i < xlsx.getNumberOfSheets(); i++) {
+				Sheet expected = xlsx.getSheetAt(i);
+				Sheet actual = ods.getSheet(expected.getSheetName());
+				String name = expected.getSheetName();
+				assertEquals(name, expected.getFirstRowNum() + rowOffset, actual.getFirstRowNum());
+				assertEquals(name, expected.getLastRowNum() + rowOffset, actual.getLastRowNum());
+				if (rowOffset > 0) {
+					assertNull(name, actual.getRow(0));
+				}
+				assertNull(name, actual.getRow(actual.getLastRowNum() + 1));
+				Row header = actual.getRow(actual.getFirstRowNum());
+				assertEquals(name, expected.getRow(expected.getFirstRowNum()).getFirstCellNum(), header.getFirstCellNum());
+				assertEquals(name, expected.getRow(expected.getFirstRowNum()).getLastCellNum(), header.getLastCellNum());
+			}
+		}
+	}
+
+	@Test
+	public void trailingEmptyRowsAreNotRows() throws IOException {
+		assertRowBounds(ODS_DIR + File.separator + V2_3 + ".ods", 0);
+	}
+
+	@Test
+	public void leadingEmptyRowIsNotARow() throws IOException {
+		assertRowBounds(ODS_DIR + File.separator + V2_3 + "-leading-empty-row.ods", 1);
+	}
+}