Optimizing and avoiding NPE in RowId and ReadOnlyRowId toString(#10410).

Change-Id: I6f16b9c55f661f5f75628ff627a01f8ec35e714e

diff --git a/server/src/com/vaadin/data/util/sqlcontainer/ReadOnlyRowId.java b/server/src/com/vaadin/data/util/sqlcontainer/ReadOnlyRowId.java
index dcad8f7c5d18ce044113495a1f5b2144ddad25a5..c845cadc7a2b6411ce2aacfb48fb1d9673dea3f9 100644 (file)
--- a/server/src/com/vaadin/data/util/sqlcontainer/ReadOnlyRowId.java
+++ b/server/src/com/vaadin/data/util/sqlcontainer/ReadOnlyRowId.java
@@ -26,18 +26,23 @@ public class ReadOnlyRowId extends RowId {
 
     @Override
     public int hashCode() {
-        return rowNum.hashCode();
+        return getRowNum();
     }
 
     @Override
     public boolean equals(Object obj) {
-        if (obj == null || !(obj instanceof ReadOnlyRowId)) {
+        if (obj == null || !(ReadOnlyRowId.class.equals(obj.getClass()))) {
             return false;
         }
-        return rowNum.equals(((ReadOnlyRowId) obj).rowNum);
+        return getRowNum() == (((ReadOnlyRowId) obj).getRowNum());
     }
 
     public int getRowNum() {
         return rowNum;
     }
+
+    @Override
+    public String toString() {
+        return String.valueOf(getRowNum());
+    }
 }

diff --git a/server/src/com/vaadin/data/util/sqlcontainer/RowId.java b/server/src/com/vaadin/data/util/sqlcontainer/RowId.java
index 8674b9dca04e09737faad7e15db496825e052ebe..79c16b0f6097de5e06650fbbfef808a8f1b2f523 100644 (file)
--- a/server/src/com/vaadin/data/util/sqlcontainer/RowId.java
+++ b/server/src/com/vaadin/data/util/sqlcontainer/RowId.java
@@ -16,6 +16,7 @@
 package com.vaadin.data.util.sqlcontainer;
 
 import java.io.Serializable;
+import java.util.Arrays;
 
 /**
  * RowId represents identifiers of a single database result set row.
@@ -47,47 +48,30 @@ public class RowId implements Serializable {
 
     @Override
     public int hashCode() {
-        int result = 31;
-        if (id != null) {
-            for (Object o : id) {
-                if (o != null) {
-                    result += o.hashCode();
-                }
-            }
-        }
-        return result;
+        return Arrays.hashCode(getId());
     }
 
     @Override
     public boolean equals(Object obj) {
-        if (obj == null || !(obj instanceof RowId)) {
-            return false;
-        }
-        Object[] compId = ((RowId) obj).getId();
-        if (id == null && compId == null) {
-            return true;
-        }
-        if (id.length != compId.length) {
+        if (obj == null || !(RowId.class.equals(obj.getClass()))) {
             return false;
         }
-        for (int i = 0; i < id.length; i++) {
-            if ((id[i] == null && compId[i] != null)
-                    || (id[i] != null && !id[i].equals(compId[i]))) {
-                return false;
-            }
-        }
-        return true;
+        return Arrays.equals(getId(), ((RowId) obj).getId());
     }
 
     @Override
     public String toString() {
-        StringBuffer s = new StringBuffer();
-        for (int i = 0; i < id.length; i++) {
-            s.append(id[i]);
-            if (i < id.length - 1) {
-                s.append("/");
-            }
+        if (getId() == null) {
+            return "";
+        }
+        StringBuilder builder = new StringBuilder();
+        for (Object id : getId()) {
+            builder.append(id);
+            builder.append('/');
+        }
+        if (builder.length() > 0) {
+            return builder.substring(0, builder.length() - 1);
         }
-        return s.toString();
+        return builder.toString();
     }
 }

diff --git a/server/src/com/vaadin/data/util/sqlcontainer/TemporaryRowId.java b/server/src/com/vaadin/data/util/sqlcontainer/TemporaryRowId.java
index 6c1e07756feb7f077ceeb67fe33abb045c0d2e49..ca2f25963e85aff69925f3240ee4b5cf9d3a1edd 100644 (file)
--- a/server/src/com/vaadin/data/util/sqlcontainer/TemporaryRowId.java
+++ b/server/src/com/vaadin/data/util/sqlcontainer/TemporaryRowId.java
@@ -29,7 +29,7 @@ public class TemporaryRowId extends RowId {
 
     @Override
     public boolean equals(Object obj) {
-        if (obj == null || !(obj instanceof TemporaryRowId)) {
+        if (obj == null || !(TemporaryRowId.class.equals(obj.getClass()))) {
             return false;
         }
         Object[] compId = ((TemporaryRowId) obj).getId();

diff --git a/server/tests/src/com/vaadin/data/util/sqlcontainer/ReadOnlyRowIdTest.java b/server/tests/src/com/vaadin/data/util/sqlcontainer/ReadOnlyRowIdTest.java
index 248dc62d93251ec83663e04cef7dce8513c66cde..29968ecf94771c69ca9be398d33a281f6367349a 100644 (file)
--- a/server/tests/src/com/vaadin/data/util/sqlcontainer/ReadOnlyRowIdTest.java
+++ b/server/tests/src/com/vaadin/data/util/sqlcontainer/ReadOnlyRowIdTest.java
@@ -44,4 +44,12 @@ public class ReadOnlyRowIdTest {
         ReadOnlyRowId rid2 = new ReadOnlyRowId(42);
         Assert.assertFalse(rid.equals(rid2));
     }
+
+    @Test
+    public void toString_rowNumberIsReturned() {
+        int i = 1;
+        ReadOnlyRowId rowId = new ReadOnlyRowId(i);
+        Assert.assertEquals("Unexpected toString value", String.valueOf(i),
+                rowId.toString());
+    }
 }

diff --git a/server/tests/src/com/vaadin/data/util/sqlcontainer/RowIdTest.java b/server/tests/src/com/vaadin/data/util/sqlcontainer/RowIdTest.java
index e4ee28ba9ed499e23bfa6f75c0739ee27ace021d..73f7be9fb282939308da44c15032b3fa4393ac7e 100644 (file)
--- a/server/tests/src/com/vaadin/data/util/sqlcontainer/RowIdTest.java
+++ b/server/tests/src/com/vaadin/data/util/sqlcontainer/RowIdTest.java
@@ -50,4 +50,11 @@ public class RowIdTest {
         Assert.assertFalse(id.equals("Tudiluu"));
         Assert.assertFalse(id.equals(new Integer(1337)));
     }
+
+    @Test
+    public void toString_defaultCtor_noException() {
+        RowId rowId = new RowId();
+        Assert.assertTrue("Unexpected to string for empty Row Id", rowId
+                .toString().isEmpty());
+    }
 }