From eb1e3174c60abad0fbe02274664df857da0a8fd7 Mon Sep 17 00:00:00 2001
From: Rdornier <64911638+Rdornier@users.noreply.github.com>
Date: Fri, 24 May 2024 14:53:16 +0200
Subject: [PATCH] Re-assign correct indices to emptyColumns at table creation
 (#89)

* Get correct indices when removing empty columns
* Ensure indices are sorted in reverse order before removing columns
* Add test for removing empty columns
---
 .../igred/omero/annotations/TableWrapper.java |  6 ++-
 .../fr/igred/omero/annotations/TableTest.java | 51 ++++++++++++++++++-
 2 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/src/main/java/fr/igred/omero/annotations/TableWrapper.java b/src/main/java/fr/igred/omero/annotations/TableWrapper.java
index 90c14e00..abf75031 100644
--- a/src/main/java/fr/igred/omero/annotations/TableWrapper.java
+++ b/src/main/java/fr/igred/omero/annotations/TableWrapper.java
@@ -42,6 +42,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -516,7 +517,7 @@ private void createColumn(int column, String columnName, Class<?> type) {
      */
     private Collection<Integer> getEmptyStringColumns() {
         Collection<Integer> emptyColumns = new ArrayList<>(0);
-        for (int j = 0; j < columns.length; j++) {
+        for (int j = columns.length - 1; j >= 0; j--) {
             TableDataColumn column = columns[j];
             if (column.getType().equals(String.class)) {
                 boolean empty = true;
@@ -881,7 +882,8 @@ public TableData createTable() {
             truncateRow();
         }
 
-        Collection<Integer> emptyColumns = getEmptyStringColumns();
+        List<Integer> emptyColumns = new ArrayList<>(getEmptyStringColumns());
+        emptyColumns.sort(Collections.reverseOrder());
         emptyColumns.forEach(this::removeColumn);
 
         return new TableData(columns, data);
diff --git a/src/test/java/fr/igred/omero/annotations/TableTest.java b/src/test/java/fr/igred/omero/annotations/TableTest.java
index c00a0b02..728d6770 100644
--- a/src/test/java/fr/igred/omero/annotations/TableTest.java
+++ b/src/test/java/fr/igred/omero/annotations/TableTest.java
@@ -62,7 +62,7 @@ void testCreateTable() throws Exception {
         }
 
         assertEquals(images.get(0).asDataObject(), table.getData(0, 0));
-        assertEquals(images.get(1).getName(), table.getData(0, 1));
+        assertEquals(images.get(1).getName(), table.getData(1, 1));
 
         dataset.addTable(client, table);
 
@@ -75,6 +75,55 @@ void testCreateTable() throws Exception {
     }
 
 
+    @Test
+    void testCreateTableWithEmptyColumns() throws Exception {
+        DatasetWrapper dataset = client.getDataset(DATASET1.id);
+
+        List<ImageWrapper> images = dataset.getImages(client);
+
+        TableWrapper table = new TableWrapper(4, "TableTest");
+
+        assertEquals(4, table.getColumnCount());
+
+        table.setColumn(0, "Image", ImageData.class);
+        table.setColumn(1, "Condition", String.class);
+        table.setColumn(2, "Name", String.class);
+        table.setColumn(3, "Phenotype", String.class);
+        assertEquals("Image", table.getColumnName(0));
+        assertEquals("Condition", table.getColumnName(1));
+        assertEquals("Name", table.getColumnName(2));
+        assertEquals("Phenotype", table.getColumnName(3));
+        assertSame(ImageData.class, table.getColumnType(0));
+
+        table.setRowCount(images.size());
+
+        assertEquals(images.size(), table.getRowCount());
+
+        for (ImageWrapper image : images) {
+            assertFalse(table.isComplete());
+            table.addRow(image.asDataObject(), "", image.getName(), "");
+        }
+
+        assertEquals(images.get(0).asDataObject(), table.getData(0, 0));
+        assertEquals("", table.getData(0, 1));
+        assertEquals(images.get(1).getName(), table.getData(1, 2));
+        assertEquals("", table.getData(0, 3));
+
+        dataset.addTable(client, table);
+
+        assertEquals(2, table.getColumnCount());
+        assertEquals(images.get(0).asDataObject(), table.getData(0, 0));
+        assertEquals(images.get(1).getName(), table.getData(1, 1));
+
+        List<TableWrapper> tables = dataset.getTables(client);
+        client.deleteTables(tables);
+        List<TableWrapper> noTables = dataset.getTables(client);
+
+        assertEquals(1, tables.size());
+        assertEquals(0, noTables.size());
+    }
+
+
     @Test
     void testReplaceTable() throws Exception {
         DatasetWrapper dataset = client.getDataset(DATASET1.id);