Fix ODS row removal and replacement to match XLS/XLSX

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

@@ -37,11 +37,12 @@ public OdsRow(OdsSheet sheet, int rowNum) {
 	}
 
 	/**
-	 * Clears all cells in this row, setting their SODS range values to null.
+	 * Clears value, formula, annotation and style of every cell in this row.
 	 */
-	public void clear() {
-		for (OdsCell cell : cells.values()) {
-			cell.getSodsRange().setValue(null);
+	public synchronized void clear() {
+		com.github.miachm.sods.Sheet sodsSheet = sheet.getSodsSheet();
+		if (rowNum < sodsSheet.getMaxRows() && sodsSheet.getMaxColumns() > 0) {
+			sodsSheet.getRange(rowNum, 0, 1, sodsSheet.getMaxColumns()).clear();
 		}
 		cells.clear();
 	}
@@ -95,40 +96,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;
@@ -217,10 +210,10 @@ public Iterator<Cell> iterator() {
 		return cellIterator();
 	}
 	@Override
-	public void removeCell(Cell cell) {
+	public synchronized void removeCell(Cell cell) {
 		if (cell instanceof OdsCell) {
 			int col = cell.getColumnIndex();
-			((OdsCell) cell).getSodsRange().setValue(null);
+			((OdsCell) cell).getSodsRange().clear();
 			cells.remove(col);
 		}
 	}

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<>();
 
@@ -75,10 +83,12 @@ public synchronized Row createRow(int rownum) {
 			throw new IllegalArgumentException("Row number must be >= 0");
 		}
 		int currentRows = sodsSheet.getMaxRows();
+		OdsRow row = new OdsRow(this, rownum);
 		if (rownum >= currentRows) {
 			sodsSheet.appendRows(rownum - currentRows + 1);
+		} else {
+			resetRow(row); // replaces the existing row, as in POI
 		}
-		OdsRow row = new OdsRow(this, rownum);
 		rows.put(rownum, row);
 		return row;
 	}
@@ -89,7 +99,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;
@@ -98,64 +111,101 @@ public synchronized Row getRow(int rownum) {
 	}
 
 	@Override
-	public void removeRow(Row row) {
+	public synchronized void removeRow(Row row) {
 		if (row instanceof OdsRow) {
 			int rowNum = row.getRowNum();
-			OdsRow odsRow = (OdsRow) row;
-			odsRow.clear();
+			resetRow((OdsRow) row);
 			rows.remove(rowNum);
+			// Narrow the cached range so getRow returns null for the removed edge row.
+			if (contentScanned) {
+				scanContentRows();
+			}
 		}
 	}
 
-	private boolean hasData() {
-		if (!rows.isEmpty()) {
-			return true;
+	/** Leaves the row as POI has it after the row is dropped: no cells, default height, visible. */
+	private void resetRow(OdsRow row) {
+		row.clear();
+		int rowNum = row.getRowNum();
+		if (rowNum < sodsSheet.getMaxRows()) {
+			sodsSheet.setRowHeight(rowNum, null);
+			sodsSheet.showRow(rowNum);
 		}
-		com.github.miachm.sods.Range dataRange = sodsSheet.getDataRange();
-		if (dataRange == null) {
+	}
+
+	/**
+	 * 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;
+		}
+		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;
+	private boolean inContentRange(int row) {
+		if (!contentScanned) {
+			scanContentRows();
 		}
-		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;
-		}
-		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