Grid now uses ObjectRenderer by default (#15417)

Change-Id: I2aa8105c0eadbadb29f9aab9e3e3aeb21629f6f3

8 files changed, 252 insertions, 40 deletions

diff --git a/client/src/com/vaadin/client/connectors/ObjectRendererConnector.java b/client/src/com/vaadin/client/connectors/ObjectRendererConnector.java
new file mode 100644
index 0000000000..877eaaa569
--- /dev/null
+++ b/client/src/com/vaadin/client/connectors/ObjectRendererConnector.java
@@ -0,0 +1,38 @@
+/*
+ * Copyright 2000-2014 Vaadin Ltd.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.client.connectors;
+
+import com.vaadin.client.renderers.TextRenderer;
+import com.vaadin.shared.ui.Connect;
+
+/**
+ * A connector for {@link com.vaadin.ui.renderer.ObjectRenderer the server side
+ * ObjectRenderer}.
+ * <p>
+ * This uses a {@link TextRenderer} to actually render the contents, as the
+ * object is already converted into a string server-side.
+ * 
+ * @since
+ * @author Vaadin Ltd
+ */
+@Connect(com.vaadin.ui.renderer.ObjectRenderer.class)
+public class ObjectRendererConnector extends AbstractRendererConnector<String> {
+
+    @Override
+    public TextRenderer getRenderer() {
+        return (TextRenderer) super.getRenderer();
+    }
+}
diff --git a/client/src/com/vaadin/client/renderers/ObjectRenderer.java b/client/src/com/vaadin/client/renderers/ObjectRenderer.java
new file mode 100644
index 0000000000..a2c4e7bfc6
--- /dev/null
+++ b/client/src/com/vaadin/client/renderers/ObjectRenderer.java
@@ -0,0 +1,36 @@
+/*
+ * Copyright 2000-2014 Vaadin Ltd.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.client.renderers;
+
+import com.vaadin.client.widget.grid.RendererCellReference;
+
+/**
+ * A renderer for displaying an object to a string using the
+ * {@link Object#toString()} method.
+ * <p>
+ * If the object is <code>null</code>, then it is rendered as an empty string
+ * instead.
+ * 
+ * @since
+ * @author Vaadin Ltd
+ */
+public class ObjectRenderer implements Renderer<Object> {
+    @Override
+    public void render(RendererCellReference cell, Object data) {
+        String text = (data != null) ? data.toString() : "";
+        cell.getElement().setInnerText(text);
+    }
+}
diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java
index 9445ab77fb..93e94b8447 100644
--- a/client/src/com/vaadin/client/widgets/Grid.java
+++ b/client/src/com/vaadin/client/widgets/Grid.java
@@ -63,6 +63,7 @@ import com.vaadin.client.WidgetUtil;
 import com.vaadin.client.data.DataChangeHandler;
 import com.vaadin.client.data.DataSource;
 import com.vaadin.client.renderers.ComplexRenderer;
+import com.vaadin.client.renderers.ObjectRenderer;
 import com.vaadin.client.renderers.Renderer;
 import com.vaadin.client.renderers.WidgetRenderer;
 import com.vaadin.client.ui.SubPartAware;
@@ -2577,13 +2578,18 @@ public class Grid<T> extends ResizeComposite implements
     public static abstract class Column<C, T> {
 
         /**
-         * Default renderer for GridColumns. Renders everything into text
-         * through {@link Object#toString()}.
+         * The default renderer for grid columns.
+         * <p>
+         * The first time this renderer is called, a warning is displayed,
+         * informing the developer to use a manually defined renderer for their
+         * column.
          */
-        private final class DefaultTextRenderer implements Renderer<Object> {
+        private final class DefaultObjectRenderer extends ObjectRenderer {
             boolean warned = false;
-            private final String DEFAULT_RENDERER_WARNING = "This column uses a dummy default TextRenderer. "
-                    + "A more suitable renderer should be set using the setRenderer() method.";
+            private final String DEFAULT_RENDERER_WARNING = "This column uses "
+                    + "a dummy default ObjectRenderer. A more suitable "
+                    + "renderer should be set using the setRenderer() "
+                    + "method.";
 
             @Override
             public void render(RendererCellReference cell, Object data) {
@@ -2594,14 +2600,7 @@ public class Grid<T> extends ResizeComposite implements
                     warned = true;
                 }
 
-                final String text;
-                if (data == null) {
-                    text = "";
-                } else {
-                    text = data.toString();
-                }
-
-                cell.getElement().setInnerText(text);
+                super.render(cell, data);
             }
         }
 
@@ -2633,7 +2632,7 @@ public class Grid<T> extends ResizeComposite implements
          * Constructs a new column with a simple TextRenderer.
          */
         public Column() {
-            setRenderer(new DefaultTextRenderer());
+            setRenderer(new DefaultObjectRenderer());
         }
 
         /**
diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java
index ab27a141b7..2d45dfef61 100644
--- a/server/src/com/vaadin/ui/Grid.java
+++ b/server/src/com/vaadin/ui/Grid.java
@@ -87,8 +87,8 @@ import com.vaadin.shared.ui.grid.GridStaticSectionState.RowState;
 import com.vaadin.shared.ui.grid.HeightMode;
 import com.vaadin.shared.ui.grid.ScrollDestination;
 import com.vaadin.shared.util.SharedUtil;
+import com.vaadin.ui.renderer.ObjectRenderer;
 import com.vaadin.ui.renderer.Renderer;
-import com.vaadin.ui.renderer.TextRenderer;
 import com.vaadin.util.ReflectTools;
 
 import elemental.json.Json;
@@ -1810,7 +1810,25 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
             this.grid = grid;
             this.state = state;
             this.propertyId = propertyId;
-            internalSetRenderer(new TextRenderer());
+
+            internalSetRenderer(new ObjectRenderer() {
+                private boolean warned = false;
+                private final String DEFAULT_RENDERER_WARNING = "This column uses "
+                        + "a dummy default ObjectRenderer. A more suitable "
+                        + "renderer should be set using the setRenderer() "
+                        + "method.";
+
+                @Override
+                public JsonValue encode(Object value) {
+                    if (!warned && !(value instanceof String)) {
+                        getLogger().warning(
+                                Column.this.toString() + ": "
+                                        + DEFAULT_RENDERER_WARNING);
+                        warned = true;
+                    }
+                    return super.encode(value);
+                }
+            });
         }
 
         /**
@@ -1974,13 +1992,13 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
          * @see #setConverter(Converter)
          */
         public Column setRenderer(Renderer<?> renderer) {
-            if (!internalSetRenderer(renderer)) {
-                throw new IllegalArgumentException(
-                        "Could not find a converter for converting from the model type "
-                                + getModelType()
-                                + " to the renderer presentation type "
-                                + renderer.getPresentationType() + " (in "
-                                + toString() + ")");
+            boolean success = internalSetRenderer(renderer);
+            if (!success) {
+                throw new IllegalArgumentException("Could not find a "
+                        + "converter for converting from the model type "
+                        + getModelType() + " to the renderer presentation "
+                        + "type " + renderer.getPresentationType() + " (in "
+                        + toString() + ")");
             }
             return this;
         }
@@ -2033,20 +2051,19 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
             Class<?> modelType = getModelType();
             if (converter != null) {
                 if (!converter.getModelType().isAssignableFrom(modelType)) {
-                    throw new IllegalArgumentException(
-                            "The converter model type "
-                                    + converter.getModelType()
-                                    + " is not compatible with the property type "
-                                    + modelType + " (in " + toString() + ")");
+                    throw new IllegalArgumentException("The converter model "
+                            + "type " + converter.getModelType() + " is not "
+                            + "compatible with the property type " + modelType
+                            + " (in " + toString() + ")");
 
                 } else if (!getRenderer().getPresentationType()
                         .isAssignableFrom(converter.getPresentationType())) {
-                    throw new IllegalArgumentException(
-                            "The converter presentation type "
-                                    + converter.getPresentationType()
-                                    + " is not compatible with the renderer presentation type "
-                                    + getRenderer().getPresentationType()
-                                    + " (in " + toString() + ")");
+                    throw new IllegalArgumentException("The converter "
+                            + "presentation type "
+                            + converter.getPresentationType() + " is not "
+                            + "compatible with the renderer presentation "
+                            + "type " + getRenderer().getPresentationType()
+                            + " (in " + toString() + ")");
                 }
             }
 
@@ -2106,6 +2123,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
             return converter;
         }
 
+        @SuppressWarnings("unchecked")
         private <T> boolean internalSetRenderer(Renderer<T> renderer) {
 
             Converter<? extends T, ?> converter;
diff --git a/server/src/com/vaadin/ui/renderer/ObjectRenderer.java b/server/src/com/vaadin/ui/renderer/ObjectRenderer.java
new file mode 100644
index 0000000000..9f8b44162c
--- /dev/null
+++ b/server/src/com/vaadin/ui/renderer/ObjectRenderer.java
@@ -0,0 +1,46 @@
+/*
+ * Copyright 2000-2014 Vaadin Ltd.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.ui.renderer;
+
+import com.vaadin.ui.Grid.AbstractRenderer;
+
+import elemental.json.JsonValue;
+
+/**
+ * A renderer for displaying an object to a string using the
+ * {@link Object#toString()} method.
+ * <p>
+ * If the object is <code>null</code>, then it is rendered as an empty string
+ * instead.
+ * 
+ * @since
+ * @author Vaadin Ltd
+ */
+public class ObjectRenderer extends AbstractRenderer<Object> {
+
+    /**
+     * Creates a new <code>toString</code> renderer.
+     */
+    public ObjectRenderer() {
+        super(Object.class);
+    }
+
+    @Override
+    public JsonValue encode(Object value) {
+        String text = (value != null) ? value.toString() : "";
+        return super.encode(text);
+    }
+}
diff --git a/server/tests/src/com/vaadin/tests/server/renderer/RendererTest.java b/server/tests/src/com/vaadin/tests/server/renderer/RendererTest.java
index 464d409543..767c72f5d9 100644
--- a/server/tests/src/com/vaadin/tests/server/renderer/RendererTest.java
+++ b/server/tests/src/com/vaadin/tests/server/renderer/RendererTest.java
@@ -18,6 +18,7 @@ package com.vaadin.tests.server.renderer;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
 
 import java.util.Locale;
 
@@ -36,6 +37,7 @@ import com.vaadin.ui.ConnectorTracker;
 import com.vaadin.ui.Grid;
 import com.vaadin.ui.Grid.Column;
 import com.vaadin.ui.UI;
+import com.vaadin.ui.renderer.ObjectRenderer;
 import com.vaadin.ui.renderer.TextRenderer;
 
 import elemental.json.JsonValue;
@@ -44,6 +46,11 @@ public class RendererTest {
 
     private static class TestBean {
         int i = 42;
+
+        @Override
+        public String toString() {
+            return "TestBean [" + i + "]";
+        }
     }
 
     private static class ExtendedBean extends TestBean {
@@ -130,15 +137,16 @@ public class RendererTest {
 
     @Test
     public void testDefaultRendererAndConverter() throws Exception {
-        assertSame(TextRenderer.class, foo.getRenderer().getClass());
-        assertSame(StringToIntegerConverter.class, foo.getConverter()
-                .getClass());
+        assertTrue("Foo default renderer should be a type of ObjectRenderer",
+                foo.getRenderer() instanceof ObjectRenderer);
 
-        assertSame(TextRenderer.class, bar.getRenderer().getClass());
+        assertTrue("Bar default renderer should be a type of ObjectRenderer",
+                bar.getRenderer() instanceof ObjectRenderer);
         // String->String; converter not needed
         assertNull(bar.getConverter());
 
-        assertSame(TextRenderer.class, baz.getRenderer().getClass());
+        assertTrue("Baz default renderer should be a type of ObjectRenderer",
+                baz.getRenderer() instanceof ObjectRenderer);
         // MyBean->String; converter not found
         assertNull(baz.getConverter());
     }
@@ -166,6 +174,11 @@ public class RendererTest {
 
     @Test
     public void testEncoding() throws Exception {
+        /*
+         * For some strange reason, this test seems to fail locally, but not on
+         * TeamCity.
+         */
+
         assertEquals("42", render(foo, 42).asString());
         foo.setRenderer(renderer());
         assertEquals("renderer(42)", render(foo, 42).asString());
@@ -177,7 +190,7 @@ public class RendererTest {
 
     @Test
     public void testEncodingWithoutConverter() throws Exception {
-        assertEquals("", render(baz, new TestBean()).asString());
+        assertEquals("TestBean [42]", render(baz, new TestBean()).asString());
     }
 
     @Test
diff --git a/uitest/src/com/vaadin/tests/components/grid/GridUndefinedObjectConverter.java b/uitest/src/com/vaadin/tests/components/grid/GridUndefinedObjectConverter.java
new file mode 100644
index 0000000000..fba2bbf698
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/grid/GridUndefinedObjectConverter.java
@@ -0,0 +1,37 @@
+package com.vaadin.tests.components.grid;
+
+import com.vaadin.data.util.IndexedContainer;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Grid;
+
+public class GridUndefinedObjectConverter extends AbstractTestUI {
+    private static class Pojo {
+        private final String content;
+
+        public Pojo(String content) {
+            this.content = content;
+        }
+
+        @Override
+        public String toString() {
+            return "Pojo:" + content;
+        }
+    }
+
+    @Override
+    @SuppressWarnings("all")
+    protected void setup(VaadinRequest request) {
+        IndexedContainer container = new IndexedContainer();
+        container.addContainerProperty("pojo", Pojo.class, new Pojo("foo"));
+        container.addContainerProperty("pojo object ", Object.class, new Pojo(
+                "bar"));
+        container.addContainerProperty("int", Integer.class, 1);
+        container.addContainerProperty("int object", Object.class, 2);
+        container.addContainerProperty("string", String.class, "foo");
+        container.addContainerProperty("string object", Object.class, "bar");
+        container.addItem();
+
+        addComponent(new Grid(container));
+    }
+}
diff --git a/uitest/src/com/vaadin/tests/components/grid/GridUndefinedObjectConverterTest.java b/uitest/src/com/vaadin/tests/components/grid/GridUndefinedObjectConverterTest.java
new file mode 100644
index 0000000000..401bfda885
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/grid/GridUndefinedObjectConverterTest.java
@@ -0,0 +1,25 @@
+package com.vaadin.tests.components.grid;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.Test;
+
+import com.vaadin.testbench.elements.GridElement;
+import com.vaadin.tests.annotations.TestCategory;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+@TestCategory("grid")
+public class GridUndefinedObjectConverterTest extends MultiBrowserTest {
+    @Test
+    public void testDefaultToStringRendering() {
+        openTestURL();
+
+        GridElement grid = $(GridElement.class).first();
+        assertEquals("pojo", "Pojo:foo", grid.getCell(0, 0).getText());
+        assertEquals("pojo object", "Pojo:bar", grid.getCell(0, 1).getText());
+        assertEquals("int", "1", grid.getCell(0, 2).getText());
+        assertEquals("int object", "2", grid.getCell(0, 3).getText());
+        assertEquals("string", "foo", grid.getCell(0, 4).getText());
+        assertEquals("string object", "bar", grid.getCell(0, 5).getText());
+    }
+}