Do not leave transaction open if remove fails (#17858)

Change-Id: Iae9243bd0dc90e130e2866adef472a4d09c4a16f

2 files changed, 10 insertions, 13 deletions

diff --git a/server/src/com/vaadin/data/util/sqlcontainer/SQLContainer.java b/server/src/com/vaadin/data/util/sqlcontainer/SQLContainer.java
index c0c660c084..f07b7ecc58 100644
--- a/server/src/com/vaadin/data/util/sqlcontainer/SQLContainer.java
+++ b/server/src/com/vaadin/data/util/sqlcontainer/SQLContainer.java
@@ -1012,9 +1012,15 @@ public class SQLContainer implements Container, Container.Filterable,
             queryDelegate.beginTransaction();
             /* Perform buffered deletions */
             for (RowItem item : removedItems.values()) {
-                if (!queryDelegate.removeRow(item)) {
+                try {
+                    if (!queryDelegate.removeRow(item)) {
+                        throw new SQLException(
+                                "Removal failed for row with ID: "
+                                        + item.getId());
+                    }
+                } catch (IllegalArgumentException e) {
                     throw new SQLException("Removal failed for row with ID: "
-                            + item.getId());
+                            + item.getId(), e);
                 }
             }
             /* Perform buffered modifications */
diff --git a/server/tests/src/com/vaadin/data/util/sqlcontainer/SQLContainerTableQueryTest.java b/server/tests/src/com/vaadin/data/util/sqlcontainer/SQLContainerTableQueryTest.java
index 92d0c49205..b2cc9a5d0c 100644
--- a/server/tests/src/com/vaadin/data/util/sqlcontainer/SQLContainerTableQueryTest.java
+++ b/server/tests/src/com/vaadin/data/util/sqlcontainer/SQLContainerTableQueryTest.java
@@ -99,23 +99,14 @@ public class SQLContainerTableQueryTest {
         assertTrue(container.removeItem(container.lastItemId()));
     }
 
-    @Test
+    @Test(expected = SQLException.class)
     public void itemWithNonExistingVersionColumnCannotBeRemoved()
             throws SQLException {
         query.setVersionColumn("version");
 
         container.removeItem(container.lastItemId());
 
-        // FIXME Remove try-catch when https://dev.vaadin.com/ticket/17858 is
-        // fixed
-        try {
-            container.commit();
-            Assert.fail("Commit should not succeed when version column does not exist");
-        } catch (IllegalArgumentException e) {
-            // This should not be here at all as commit() should not leave the
-            // transaction open!
-            container.getQueryDelegate().rollback();
-        }
+        container.commit();
     }
 
     @Test