From f800b4911913690ecea477c606f8375a61d68baf Mon Sep 17 00:00:00 2001 From: Pierre Pouchin Date: Thu, 27 Jul 2023 15:02:40 +0200 Subject: [PATCH 1/4] Check table columns before adding rows --- .../igred/omero/annotations/TableWrapper.java | 50 +++++++++++++++---- 1 file changed, 39 insertions(+), 11 deletions(-) diff --git a/src/main/java/fr/igred/omero/annotations/TableWrapper.java b/src/main/java/fr/igred/omero/annotations/TableWrapper.java index a445f64b..5531e8e2 100644 --- a/src/main/java/fr/igred/omero/annotations/TableWrapper.java +++ b/src/main/java/fr/igred/omero/annotations/TableWrapper.java @@ -164,7 +164,9 @@ public TableWrapper(Client client, ResultsTable results, Long imageId, List 0) offset++; + if (roiColumn.length > 0) { + hasROICol = true; + offset++; + } String[] headings = rt.getHeadings(); String[] shortHeadings = rt.getHeadingsAsVariableNames(); @@ -187,14 +193,14 @@ public TableWrapper(Client client, ResultsTable results, Long imageId, List 0) { + if (hasImageId) { createColumn(0, IMAGE, ImageData.class); data[0] = new ImageData[rowCount]; Arrays.fill(data[0], image.asDataObject()); } - if (offset > 1) { + if (hasROICol) { createColumn(1, roiProperty, ROIData.class); - data[1] = roiColumn; + data[offset - 1] = roiColumn; } for (int i = 0; i < nColumns; i++) { Variable[] col = rt.getColumnAsVariables(headings[i]); @@ -440,6 +446,22 @@ private static ROIData[] createROIColumn(ResultsTable results, } + /** + * Checks if the new columns match the existing ones. + * + * @param offset The offset in the current Results Table. + * @param nColumns The number of columns in the current Results Table. + * @param hasImageCol Whether an Image column is present. + * @param hasROICol Whether a ROI column is present. + */ + private boolean checkColumns(int offset, int nColumns, boolean hasImageCol, boolean hasROICol) { + boolean match = offset + nColumns == columnCount; + if (hasImageCol && !columns[0].getType().equals(ImageData.class)) match = false; + if (hasROICol && !columns[offset - 1].getType().equals(ROIData.class)) match = false; + return match; + } + + /** * Sets the information about a given column. * @@ -509,28 +531,34 @@ public void addRows(Client client, ResultsTable results, Long imageId, List rois = new ArrayList<>(0); - int offset = 0; + int offset = 0; + boolean hasImageId = false; + boolean hasROICol = false; if (imageId != null) { image = client.getImage(imageId); rois = image.getROIs(client); + hasImageId = true; offset++; renameImageColumn(rt); } ROIData[] roiColumn = createROIColumn(rt, rois, ijRois, roiProperty); - if (roiColumn.length > 0) offset++; + if (roiColumn.length > 0) { + hasROICol = true; + offset++; + } String[] headings = rt.getHeadings(); int nColumns = headings.length; - if (nColumns + offset != columnCount) { - throw new IllegalArgumentException("Number of columns mismatch"); + if (!checkColumns(offset, nColumns, hasImageId, hasROICol)) { + throw new IllegalArgumentException("Number or type of columns mismatch"); } int n = rt.size(); setRowCount(rowCount + n); - if (offset > 0) Arrays.fill(data[0], row, row + n, image.asDataObject()); - if (offset > 1) System.arraycopy(roiColumn, 0, data[1], row, n); + if (hasImageId) Arrays.fill(data[0], row, row + n, image.asDataObject()); + if (hasROICol) System.arraycopy(roiColumn, 0, data[offset - 1], row, n); for (int i = 0; i < nColumns; i++) { if (columns[offset + i].getType().equals(String.class)) { for (int j = 0; j < n; j++) { From 390464801a16aae8e4c3ed72d7d4960b2f61de8e Mon Sep 17 00:00:00 2001 From: Pierre Pouchin Date: Thu, 27 Jul 2023 16:06:21 +0200 Subject: [PATCH 2/4] Fix ROI column creation when ROI list is empty --- .../igred/omero/annotations/TableWrapper.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/main/java/fr/igred/omero/annotations/TableWrapper.java b/src/main/java/fr/igred/omero/annotations/TableWrapper.java index 5531e8e2..ac28581e 100644 --- a/src/main/java/fr/igred/omero/annotations/TableWrapper.java +++ b/src/main/java/fr/igred/omero/annotations/TableWrapper.java @@ -43,6 +43,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.ExecutionException; @@ -400,18 +401,18 @@ private static ROIData[] createROIColumn(ResultsTable results, safeParseLong(r.getProperty(roiIdProperty)))) .filter(p -> p.getKey() != null) .filter(p -> p.getValue() != null) - .collect(toMap(SimpleEntry::getKey, - p -> id2roi.get(p.getValue()), - (x1, x2) -> x1)); + .collect(HashMap::new, + (m, v) -> m.put(v.getKey(), id2roi.get(v.getValue())), + HashMap::putAll); Map shape2roi = ijRois.stream() .map(r -> new SimpleEntry<>(r.getName(), safeParseLong(r.getProperty(roiIdProperty)))) .filter(p -> p.getKey() != null) .filter(p -> p.getValue() != null) - .collect(toMap(SimpleEntry::getKey, - p -> id2roi.get(p.getValue()), - (x1, x2) -> x1)); + .collect(HashMap::new, + (m, v) -> m.put(v.getKey(), id2roi.get(v.getValue())), + HashMap::putAll); String colToDelete = ""; if (results.columnExists(roiIdProperty)) { Variable[] roiCol = results.getColumnAsVariables(roiIdProperty); @@ -535,16 +536,16 @@ public void addRows(Client client, ResultsTable results, Long imageId, List 0) { - hasROICol = true; offset++; + hasROICol = true; } String[] headings = rt.getHeadings(); From ec087e92de2f4018d9a28818d05211cdf3c3e8d2 Mon Sep 17 00:00:00 2001 From: Pierre Pouchin Date: Thu, 27 Jul 2023 16:06:43 +0200 Subject: [PATCH 3/4] Add test --- .../omero/annotations/ImageJTableTest.java | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/test/java/fr/igred/omero/annotations/ImageJTableTest.java b/src/test/java/fr/igred/omero/annotations/ImageJTableTest.java index e689aacd..172e4fe5 100644 --- a/src/test/java/fr/igred/omero/annotations/ImageJTableTest.java +++ b/src/test/java/fr/igred/omero/annotations/ImageJTableTest.java @@ -45,6 +45,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeFalse; @@ -722,4 +723,41 @@ void testSaveTableAs() throws Exception { Files.deleteIfExists(file.toPath()); } + + @Test + void testAddRowsWithMismatch() throws Exception { + List rois = new ArrayList<>(2); + rois.add(new ROIWrapper()); + rois.add(new ROIWrapper()); + + rois.get(0).setImage(image); + rois.get(1).setImage(image); + + for (int i = 0; i < 4; i++) { + RectangleWrapper rectangle = new RectangleWrapper(); + rectangle.setText(String.valueOf(10 + i % 2)); + rectangle.setCoordinates(i * 2, i * 2, 10, 10); + rectangle.setZ(i); + rectangle.setT(0); + rectangle.setC(0); + + if (i % 2 == 1) rois.get(0).addShape(rectangle); + else rois.get(1).addShape(rectangle); + } + + List newROIs = image.saveROIs(client, rois); + List ijRois = ROIWrapper.toImageJ(newROIs); + + String label1 = image.getName() + ":" + newROIs.get(0).getShapes().get(0).getText() + ":4"; + String label2 = image.getName() + ":" + newROIs.get(1).getShapes().get(0).getText() + ":10"; + + ResultsTable results1 = createOneRowResultsTable(label1, volume1, unit1); + ResultsTable results2 = createOneRowResultsTable(label2, volume2, unit2); + + TableWrapper table = new TableWrapper(client, results1, IMAGE1.id, ijRois); + + assertThrows(IllegalArgumentException.class, + () -> table.addRows(client, results2, null, ijRois)); + } + } \ No newline at end of file From 4a097258f4f441820d810121bb0f60a56d5d7016 Mon Sep 17 00:00:00 2001 From: Pierre Pouchin Date: Thu, 27 Jul 2023 16:42:57 +0200 Subject: [PATCH 4/4] Revert some changes --- .../igred/omero/annotations/TableWrapper.java | 44 +++++++------------ 1 file changed, 15 insertions(+), 29 deletions(-) diff --git a/src/main/java/fr/igred/omero/annotations/TableWrapper.java b/src/main/java/fr/igred/omero/annotations/TableWrapper.java index ac28581e..2526262c 100644 --- a/src/main/java/fr/igred/omero/annotations/TableWrapper.java +++ b/src/main/java/fr/igred/omero/annotations/TableWrapper.java @@ -165,9 +165,7 @@ public TableWrapper(Client client, ResultsTable results, Long imageId, List 0) { - hasROICol = true; - offset++; - } + if (roiColumn.length > 0) offset++; String[] headings = rt.getHeadings(); String[] shortHeadings = rt.getHeadingsAsVariableNames(); @@ -194,14 +188,14 @@ public TableWrapper(Client client, ResultsTable results, Long imageId, List 0) { createColumn(0, IMAGE, ImageData.class); data[0] = new ImageData[rowCount]; Arrays.fill(data[0], image.asDataObject()); } - if (hasROICol) { + if (offset > 1) { createColumn(1, roiProperty, ROIData.class); - data[offset - 1] = roiColumn; + data[1] = roiColumn; } for (int i = 0; i < nColumns; i++) { Variable[] col = rt.getColumnAsVariables(headings[i]); @@ -450,15 +444,13 @@ private static ROIData[] createROIColumn(ResultsTable results, /** * Checks if the new columns match the existing ones. * - * @param offset The offset in the current Results Table. - * @param nColumns The number of columns in the current Results Table. - * @param hasImageCol Whether an Image column is present. - * @param hasROICol Whether a ROI column is present. + * @param nColumns The number of columns in the current Results Table. + * @param offset The offset in the current Results Table. */ - private boolean checkColumns(int offset, int nColumns, boolean hasImageCol, boolean hasROICol) { + private boolean checkColumns(int nColumns, int offset) { boolean match = offset + nColumns == columnCount; - if (hasImageCol && !columns[0].getType().equals(ImageData.class)) match = false; - if (hasROICol && !columns[offset - 1].getType().equals(ROIData.class)) match = false; + if (offset > 0 && !columns[0].getType().equals(ImageData.class)) match = false; + if (offset > 1 && !columns[1].getType().equals(ROIData.class)) match = false; return match; } @@ -532,34 +524,28 @@ public void addRows(Client client, ResultsTable results, Long imageId, List rois = new ArrayList<>(0); - int offset = 0; - boolean hasImageId = false; - boolean hasROICol = false; + int offset = 0; if (imageId != null) { - hasImageId = true; image = client.getImage(imageId); rois = image.getROIs(client); offset++; renameImageColumn(rt); } ROIData[] roiColumn = createROIColumn(rt, rois, ijRois, roiProperty); - if (roiColumn.length > 0) { - offset++; - hasROICol = true; - } + if (roiColumn.length > 0) offset++; String[] headings = rt.getHeadings(); int nColumns = headings.length; - if (!checkColumns(offset, nColumns, hasImageId, hasROICol)) { + if (!checkColumns(nColumns, offset)) { throw new IllegalArgumentException("Number or type of columns mismatch"); } int n = rt.size(); setRowCount(rowCount + n); - if (hasImageId) Arrays.fill(data[0], row, row + n, image.asDataObject()); - if (hasROICol) System.arraycopy(roiColumn, 0, data[offset - 1], row, n); + if (offset > 0) Arrays.fill(data[0], row, row + n, image.asDataObject()); + if (offset > 1) System.arraycopy(roiColumn, 0, data[1], row, n); for (int i = 0; i < nColumns; i++) { if (columns[offset + i].getType().equals(String.class)) { for (int j = 0; j < n; j++) {