Fix temporal renderers serialization (#10929)

server/src/main/java/com/vaadin/data/util/BeanUtil.java

@@ -19,6 +19,8 @@ import java.beans.BeanInfo;
 import java.beans.IntrospectionException;
 import java.beans.Introspector;
 import java.beans.PropertyDescriptor;
+import java.io.ObjectOutputStream;
+import java.io.OutputStream;
 import java.io.Serializable;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
@@ -226,6 +228,38 @@ public final class BeanUtil implements Serializable {
         }
     }
 
+    /**
+     * Checks if the object is serializable or not. To be used in assertion checks only,
+     * since the check might be a bit heavyweight.
+     *
+     * @param obj to be checked
+     * @return {@code true}
+     * @throws AssertionError if the object is not serializable
+     */
+    public static boolean checkSerialization(Object obj) {
+        try {
+            ObjectOutputStream dummyObjectOutputStream = new ObjectOutputStream(new OutputStream() {
+                @Override
+                public void write(int b) {
+                }
+
+                @SuppressWarnings("NullableProblems")
+                @Override
+                public void write(byte[] ignored) {
+                }
+
+                @SuppressWarnings("NullableProblems")
+                @Override
+                public void write(byte[] b, int off, int len) {
+                }
+            });
+            dummyObjectOutputStream.writeObject(obj);
+        } catch (Throwable e) {
+            throw new AssertionError("Formatter supplier should be serializable", e);
+        }
+        return true;
+    }
+
     private static class LazyValidationAvailability implements Serializable {
         private static final boolean BEAN_VALIDATION_AVAILABLE = isAvailable();
 

server/src/main/java/com/vaadin/ui/renderers/LocalDateRenderer.java

@@ -20,6 +20,8 @@ import java.time.format.DateTimeFormatter;
 import java.time.format.FormatStyle;
 import java.util.Locale;
 
+import com.vaadin.data.util.BeanUtil;
+import com.vaadin.server.SerializableSupplier;
 import com.vaadin.shared.ui.grid.renderers.LocalDateRendererState;
 
 import elemental.json.JsonValue;
@@ -32,7 +34,7 @@ import elemental.json.JsonValue;
  */
 public class LocalDateRenderer extends AbstractRenderer<Object, LocalDate> {
 
-    private DateTimeFormatter formatter;
+    private SerializableSupplier<DateTimeFormatter> formatterSupplier;
     private boolean getLocaleFromGrid;
 
     /**
@@ -47,7 +49,7 @@ public class LocalDateRenderer extends AbstractRenderer<Object, LocalDate> {
      *      FormatStyle.LONG</a>
      */
     public LocalDateRenderer() {
-        this(DateTimeFormatter.ofLocalizedDate(FormatStyle.LONG), "");
+        this(() -> DateTimeFormatter.ofLocalizedDate(FormatStyle.LONG), "");
         getLocaleFromGrid = true;
     }
 
@@ -133,7 +135,8 @@ public class LocalDateRenderer extends AbstractRenderer<Object, LocalDate> {
             throw new IllegalArgumentException("locale may not be null");
         }
 
-        formatter = DateTimeFormatter.ofPattern(formatPattern, locale);
+        formatterSupplier = () -> DateTimeFormatter.ofPattern(formatPattern,
+                locale);
     }
 
     /**
@@ -142,21 +145,78 @@ public class LocalDateRenderer extends AbstractRenderer<Object, LocalDate> {
      * The renderer is configured to render with the given formatter, with an
      * empty string as its null representation.
      *
+     * <p>
+     * <b>Note</b> the {@code DateTimeFormatter} is not a serializable class, so
+     * using this method in an environment which requires session persistence
+     * may produce {@link java.io.NotSerializableException}.
+     *
      * @param formatter
      *            the formatter to use, not {@code null}
      *
      * @throws IllegalArgumentException
      *             if formatter is null
+     * @deprecated the method is unsafe for serialization, may produce troubles
+     *             in a cluster environment
+     * @see #LocalDateRenderer(SerializableSupplier)
      */
+    @Deprecated
     public LocalDateRenderer(DateTimeFormatter formatter) {
         this(formatter, "");
     }
 
+    /**
+     * Creates a new LocalDateRenderer.
+     * <p>
+     * The renderer is configured to render with the given formatterSupplier.
+     *
+     * @param formatterSupplier
+     *            the formatterSupplier supplier to use, not {@code null}, it
+     *            should not supply {@code null} either
+     * @param nullRepresentation
+     *            the textual representation of the {@code null} value
+     *
+     * @throws IllegalArgumentException
+     *             if formatterSupplier is null
+     */
+    public LocalDateRenderer(
+            SerializableSupplier<DateTimeFormatter> formatterSupplier,
+            String nullRepresentation) {
+        super(LocalDate.class, nullRepresentation);
+
+        if (formatterSupplier == null) {
+            throw new IllegalArgumentException(
+                    "formatterSupplier may not be null");
+        }
+        this.formatterSupplier = formatterSupplier;
+        assert BeanUtil.checkSerialization(formatterSupplier);
+    }
+
+    /**
+     * Creates a new LocalDateRenderer.
+     * <p>
+     * The renderer is configured to render with the given formatterSupplier.
+     *
+     * @param formatterSupplier
+     *            the formatterSupplier supplier to use, not {@code null}, it
+     *            should not supply {@code null} either
+     * @throws IllegalArgumentException
+     *             if formatterSupplier is null
+     */
+    public LocalDateRenderer(
+            SerializableSupplier<DateTimeFormatter> formatterSupplier) {
+        this(formatterSupplier, "");
+    }
+
     /**
      * Creates a new LocalDateRenderer.
      * <p>
      * The renderer is configured to render with the given formatter.
      *
+     * <p>
+     * <b>Note</b> the {@code DateTimeFormatter} is not a serializable class, so
+     * using this method in an environment which requires session persistence
+     * may produce {@link java.io.NotSerializableException}.
+     *
      * @param formatter
      *            the formatter to use, not {@code null}
      * @param nullRepresentation
@@ -164,7 +224,11 @@ public class LocalDateRenderer extends AbstractRenderer<Object, LocalDate> {
      *
      * @throws IllegalArgumentException
      *             if formatter is null
+     * @deprecated the method is unsafe for serialization, may produce troubles
+     *             in acluster environment
+     * @see #LocalDateRenderer(SerializableSupplier, String)
      */
+    @Deprecated
     public LocalDateRenderer(DateTimeFormatter formatter,
             String nullRepresentation) {
         super(LocalDate.class, nullRepresentation);
@@ -173,7 +237,7 @@ public class LocalDateRenderer extends AbstractRenderer<Object, LocalDate> {
             throw new IllegalArgumentException("formatter may not be null");
         }
 
-        this.formatter = formatter;
+        this.formatterSupplier = () -> formatter;
     }
 
     @Override
@@ -188,10 +252,10 @@ public class LocalDateRenderer extends AbstractRenderer<Object, LocalDate> {
                                 + "this renderer should either be attached to a grid "
                                 + "or constructed with locale information");
             }
-            dateString = value
-                    .format(formatter.withLocale(getParentGrid().getLocale()));
+            dateString = value.format(formatterSupplier.get()
+                    .withLocale(getParentGrid().getLocale()));
         } else {
-            dateString = value.format(formatter);
+            dateString = value.format(formatterSupplier.get());
         }
         return encode(dateString, String.class);
     }
@@ -205,4 +269,5 @@ public class LocalDateRenderer extends AbstractRenderer<Object, LocalDate> {
     protected LocalDateRendererState getState(boolean markAsDirty) {
         return (LocalDateRendererState) super.getState(markAsDirty);
     }
+
 }

server/src/main/java/com/vaadin/ui/renderers/LocalDateTimeRenderer.java

@@ -20,9 +20,11 @@ import java.time.format.DateTimeFormatter;
 import java.time.format.FormatStyle;
 import java.util.Locale;
 
+import com.vaadin.server.SerializableSupplier;
 import com.vaadin.shared.ui.grid.renderers.LocalDateTimeRendererState;
 
 import elemental.json.JsonValue;
+import static com.vaadin.data.util.BeanUtil.checkSerialization;
 
 /**
  * A renderer for presenting {@code LocalDateTime} objects.
@@ -33,7 +35,7 @@ import elemental.json.JsonValue;
 public class LocalDateTimeRenderer
         extends AbstractRenderer<Object, LocalDateTime> {
 
-    private DateTimeFormatter formatter;
+    private SerializableSupplier<DateTimeFormatter> formatterSupplier;
     private boolean getLocaleFromGrid;
 
     /**
@@ -52,7 +54,7 @@ public class LocalDateTimeRenderer
      *      FormatStyle.SHORT</a>
      */
     public LocalDateTimeRenderer() {
-        this(DateTimeFormatter.ofLocalizedDateTime(FormatStyle.LONG,
+        this(() -> DateTimeFormatter.ofLocalizedDateTime(FormatStyle.LONG,
                 FormatStyle.SHORT), "");
         getLocaleFromGrid = true;
     }
@@ -63,12 +65,21 @@ public class LocalDateTimeRenderer
      * The renderer is configured to render with the given formatter, with the
      * empty string as its null representation.
      *
+     * <p>
+     * <b>Note</b> the {@code DateTimeFormatter} is not a serializable class, so
+     * using this method in an environment which requires session persistence
+     * may produce {@link java.io.NotSerializableException}.
+     *
      * @param formatter
      *            the formatter to use, not {@code null}
      *
      * @throws IllegalArgumentException
      *             if formatter is null
+     * @deprecated the method is unsafe for serialization, may produce troubles
+     *             in a cluster environment
+     * @see #LocalDateTimeRenderer(SerializableSupplier)
      */
+    @Deprecated
     public LocalDateTimeRenderer(DateTimeFormatter formatter) {
         this(formatter, "");
     }
@@ -78,6 +89,12 @@ public class LocalDateTimeRenderer
      * <p>
      * The renderer is configured to render with the given formatter.
      *
+     * <p>
+     * <b>Note</b> the {@code DateTimeFormatter} is not a serializable class, so
+     * using this method in an environment which requires session persistence
+     * may produce {@link java.io.NotSerializableException}.
+     *
+     *
      * @param formatter
      *            the formatter to use, not {@code null}
      * @param nullRepresentation
@@ -85,7 +102,11 @@ public class LocalDateTimeRenderer
      *
      * @throws IllegalArgumentException
      *             if formatter is null
+     * @deprecated the method is unsafe for serialization, may produce troubles
+     *             in acluster environment
+     * @see #LocalDateTimeRenderer(SerializableSupplier, String)
      */
+    @Deprecated
     public LocalDateTimeRenderer(DateTimeFormatter formatter,
             String nullRepresentation) {
         super(LocalDateTime.class, nullRepresentation);
@@ -94,7 +115,7 @@ public class LocalDateTimeRenderer
             throw new IllegalArgumentException("formatter may not be null");
         }
 
-        this.formatter = formatter;
+        this.formatterSupplier = () -> formatter;
     }
 
     /**
@@ -179,7 +200,51 @@ public class LocalDateTimeRenderer
             throw new IllegalArgumentException("locale may not be null");
         }
 
-        formatter = DateTimeFormatter.ofPattern(formatPattern, locale);
+        formatterSupplier = () -> DateTimeFormatter.ofPattern(formatPattern,
+                locale);
+    }
+
+    /**
+     * Creates a new LocalDateTimeRenderer.
+     * <p>
+     * The renderer is configured to render with the given formatterSupplier.
+     *
+     * @param formatterSupplier
+     *            the formatterSupplier supplier to use, not {@code null}, it
+     *            should not supply {@code null} either
+     * @throws IllegalArgumentException
+     *             if formatterSupplier is null
+     */
+    public LocalDateTimeRenderer(
+            SerializableSupplier<DateTimeFormatter> formatterSupplier) {
+        this(formatterSupplier, "");
+    }
+
+    /**
+     * Creates a new LocalDateTimeRenderer.
+     * <p>
+     * The renderer is configured to render with the given formatterSupplier.
+     *
+     * @param formatterSupplier
+     *            the formatterSupplier supplier to use, not {@code null}, it
+     *            should not supply {@code null} either
+     * @param nullRepresentation
+     *            the textual representation of the {@code null} value
+     *
+     * @throws IllegalArgumentException
+     *             if formatterSupplier is null
+     */
+    public LocalDateTimeRenderer(
+            SerializableSupplier<DateTimeFormatter> formatterSupplier,
+            String nullRepresentation) {
+        super(LocalDateTime.class, nullRepresentation);
+
+        if (formatterSupplier == null) {
+            throw new IllegalArgumentException(
+                    "formatterSupplier may not be null");
+        }
+        this.formatterSupplier = formatterSupplier;
+        assert checkSerialization(formatterSupplier);
     }
 
     @Override
@@ -194,10 +259,10 @@ public class LocalDateTimeRenderer
                                 + "this renderer should either be attached to a grid "
                                 + "or constructed with locale information");
             }
-            dateString = value
-                    .format(formatter.withLocale(getParentGrid().getLocale()));
+            dateString = value.format(formatterSupplier.get()
+                    .withLocale(getParentGrid().getLocale()));
         } else {
-            dateString = value.format(formatter);
+            dateString = value.format(formatterSupplier.get());
         }
         return encode(dateString, String.class);
     }

server/src/test/java/com/vaadin/data/provider/hierarchical/HierarchyMapperWithDataTest.java

@@ -227,15 +227,16 @@ public class HierarchyMapperWithDataTest {
 
     static List<Node> generateTestData(int rootCount, int parentCount,
             int leafCount) {
+        int nodeCounter = 0;
         List<Node> nodes = new ArrayList<>();
         for (int i = 0; i < rootCount; ++i) {
-            Node root = new Node();
+            Node root = new Node(nodeCounter++);
             nodes.add(root);
             for (int j = 0; j < parentCount; ++j) {
-                Node parent = new Node(root);
+                Node parent = new Node(root, nodeCounter++);
                 nodes.add(parent);
                 for (int k = 0; k < leafCount; ++k) {
-                    nodes.add(new Node(parent));
+                    nodes.add(new Node(parent, nodeCounter++));
                 }
             }
         }

server/src/test/java/com/vaadin/data/provider/hierarchical/Node.java

@@ -4,18 +4,16 @@ import java.io.Serializable;
 
 public class Node implements Serializable {
 
-    private static int counter = 0;
-
     private final Node parent;
     private final int number;
 
-    public Node() {
-        this(null);
+    public Node(int number) {
+        this(null, number);
     }
 
-    public Node(Node parent) {
+    public Node(Node parent, int number) {
         this.parent = parent;
-        this.number = counter++;
+        this.number = number;
     }
 
     public Node getParent() {

server/src/test/java/com/vaadin/tests/TestTemporalSerialization.java

@@ -0,0 +1,155 @@
+package com.vaadin.tests;
+
+import javax.swing.text.DateFormatter;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+import java.io.Serializable;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Executable;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.lang.reflect.Parameter;
+import java.lang.reflect.ParameterizedType;
+import java.lang.reflect.Type;
+import java.time.ZoneId;
+import java.time.format.DateTimeFormatter;
+import java.util.Date;
+import java.util.Locale;
+
+import org.apache.commons.lang.SerializationUtils;
+import org.junit.Test;
+
+import com.vaadin.server.SerializableSupplier;
+import com.vaadin.ui.Grid;
+import com.vaadin.ui.renderers.LocalDateRenderer;
+import com.vaadin.ui.renderers.LocalDateTimeRenderer;
+
+import static java.lang.reflect.Modifier.isPublic;
+import static java.lang.reflect.Modifier.isStatic;
+import static junit.framework.TestCase.assertNotNull;
+
+public class TestTemporalSerialization {
+
+    @Test
+    public void smokeTestRendererSerialization()
+            throws IOException, ClassNotFoundException {
+        Grid<Object> grid = new Grid<>();
+        grid.addColumn(
+                o -> new Date(o.hashCode()).toInstant()
+                        .atZone(ZoneId.systemDefault()).toLocalDate(),
+                new LocalDateRenderer());
+        ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
+        ObjectOutputStream objectOutputStream = new ObjectOutputStream(
+                outputStream);
+        objectOutputStream.writeObject(grid);
+        ByteArrayInputStream inputStream = new ByteArrayInputStream(
+                outputStream.toByteArray());
+        Grid readGrid = (Grid) new ObjectInputStream(inputStream).readObject();
+        assertNotNull(readGrid.getColumns().get(0));
+    }
+
+    @Test
+    public void testLocalDateRenderer() throws IllegalAccessException,
+            InstantiationException, InvocationTargetException {
+        testSerialization(LocalDateRenderer.class);
+    }
+
+    @Test
+    public void testLocalDateTimeRenderer() throws IllegalAccessException,
+            InstantiationException, InvocationTargetException {
+        testSerialization(LocalDateTimeRenderer.class);
+    }
+
+    @Test(expected = AssertionError.class)
+    public void testAssertionFail() {
+        new LocalDateRenderer(new NonSerializableThing());
+    }
+
+    private static class NonSerializableThing
+            implements SerializableSupplier<DateTimeFormatter> {
+        public NonSerializableThing() {
+        }
+
+        private DateTimeFormatter useless = DateTimeFormatter.ofPattern("Y");
+
+        @Override
+        public DateTimeFormatter get() {
+            return useless;
+        }
+    }
+
+    private void testSerialization(Class<?> rendererClass)
+            throws IllegalAccessException, InvocationTargetException,
+            InstantiationException {
+        for (Constructor<?> constructor : rendererClass.getConstructors()) {
+            if (!isPublic(constructor.getModifiers()))
+                continue;
+            Object[] params = simulateParams(constructor);
+            if (params != null) {
+                Object o = constructor.newInstance(params);
+                checkSerialization(constructor, o);
+            }
+        }
+        for (Method method : rendererClass.getMethods()) {
+            if (!isPublic(method.getModifiers())
+                    || !isStatic(method.getModifiers())) {
+                continue;
+            }
+            if (!method.getReturnType().isAssignableFrom(rendererClass)) {
+                continue;
+            }
+            Object[] params = simulateParams(method);
+            if (params != null) {
+                Object o = method.invoke(simulateParams(method));
+                checkSerialization(method, o);
+            }
+        }
+    }
+
+    private Object[] simulateParams(Executable executable) {
+        Parameter[] parameters = executable.getParameters();
+        Object[] args = new Object[parameters.length];
+        for (int i = 0; i < parameters.length; i++) {
+            Parameter parameter = parameters[i];
+            Class<?> type = parameter.getType();
+            if (type.isAssignableFrom(String.class)) {
+                args[i] = "";
+            } else if (type.isAssignableFrom(Locale.class)) {
+                args[i] = Locale.US;
+            } else if (type.isAssignableFrom(SerializableSupplier.class)) {
+                Type genericType = ((ParameterizedType) parameter
+                        .getParameterizedType()).getActualTypeArguments()[0];
+                args[i] = (SerializableSupplier) () -> {
+                    try {
+                        return ((Class) genericType).newInstance();
+                    } catch (Exception e) {
+                        throw new AssertionError(e);
+                    }
+                };
+            } else if (type.isAssignableFrom(DateFormatter.class)
+                    || type.isAssignableFrom(DateTimeFormatter.class)) {
+                assertNotNull(
+                        "Non-deprecated code has non-serializable parameter: "
+                                + executable.toGenericString(),
+                        executable.getAnnotation(Deprecated.class));
+                return null;
+            } else {
+                throw new IllegalArgumentException(
+                        "Unsupported parameter type: " + type.getName());
+            }
+        }
+        return args;
+    }
+
+    private void checkSerialization(Executable method, Object o) {
+        try {
+            byte[] serialize = SerializationUtils.serialize((Serializable) o);
+            SerializationUtils.deserialize(serialize);
+        } catch (Throwable e) {
+            throw new AssertionError(method.toGenericString(), e);
+        }
+    }
+}

server/src/test/java/com/vaadin/tests/server/ClassesSerializableTest.java

@@ -69,8 +69,9 @@ public class ClassesSerializableTest {
             "com\\.vaadin\\.server\\.communication\\.PushHandler.*", // PushHandler
             "com\\.vaadin\\.server\\.communication\\.DateSerializer", //
             "com\\.vaadin\\.server\\.communication\\.JSONSerializer", //
+            "com\\.vaadin\\.ui\\.declarative\\.DesignContext", //
             // and its inner classes do not need to be serializable
-            "com\\.vaadin\\.util\\.SerializerHelper", // fully static
+            "com\\.vaadin\\.v7\\.util\\.SerializerHelper", // fully static
             // class level filtering, also affecting nested classes and
             // interfaces
             "com\\.vaadin\\.server\\.LegacyCommunicationManager.*", //
@@ -81,10 +82,11 @@ public class ClassesSerializableTest {
             "com\\.vaadin\\.data\\.provider\\.HierarchyMapper\\$TreeLevelQuery",
             "com\\.vaadin\\.data\\.util\\.ReflectTools.*", //
             "com\\.vaadin\\.data\\.util\\.JsonUtil.*", //
-            "com\\.vaadin\\.data\\.util.BeanItemContainerGenerator.*",
-            "com\\.vaadin\\.data\\.util\\.sqlcontainer\\.connection\\.MockInitialContextFactory",
-            "com\\.vaadin\\.data\\.util\\.sqlcontainer\\.DataGenerator",
-            "com\\.vaadin\\.data\\.util\\.sqlcontainer\\.FreeformQueryUtil",
+            "com\\.vaadin\\.v7\\.data\\.util.BeanItemContainerGenerator.*",
+            "com\\.vaadin\\.v7\\.data\\.util\\.sqlcontainer\\.connection\\.MockInitialContextFactory",
+            "com\\.vaadin\\.v7\\.data\\.util\\.sqlcontainer\\.DataGenerator",
+            "com\\.vaadin\\.v7\\.data\\.util\\.sqlcontainer\\.FreeformQueryUtil",
+            "com\\.vaadin\\.data\\.util\\.BeanUtil.*",
             // the JSR-303 constraint interpolation context
             "com\\.vaadin\\.data\\.validator\\.BeanValidator\\$1", //
             "com\\.vaadin\\.sass.*", //
@@ -142,7 +144,7 @@ public class ClassesSerializableTest {
                 continue;
             }
 
-            if (Component.class.isAssignableFrom(cls) && !cls.isInterface()
+            if (!cls.isInterface()
                     && !Modifier.isAbstract(cls.getModifiers())) {
                 serializeAndDeserialize(cls);
             }