diff options
author | Ilia Motornyi <elmot@vaadin.com> | 2018-06-13 09:38:45 +0300 |
---|---|---|
committer | Teemu Suo-Anttila <tsuoanttila@users.noreply.github.com> | 2018-06-13 09:38:45 +0300 |
commit | e8521ae0ab2b7c9cc8e4d042aaf8864045a57b8e (patch) | |
tree | c377c62ecccb4dc9434f494537cfda3f488e09cf | |
parent | 16335d3abad58420013e261d4fdc57ee3e867a27 (diff) | |
download | vaadin-framework-e8521ae0ab2b7c9cc8e4d042aaf8864045a57b8e.tar.gz vaadin-framework-e8521ae0ab2b7c9cc8e4d042aaf8864045a57b8e.zip |
Fix temporal renderers serialization (#10929)
7 files changed, 349 insertions, 29 deletions
diff --git a/server/src/main/java/com/vaadin/data/util/BeanUtil.java b/server/src/main/java/com/vaadin/data/util/BeanUtil.java index ac59586d1b..4600451b4e 100644 --- a/server/src/main/java/com/vaadin/data/util/BeanUtil.java +++ b/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(); diff --git a/server/src/main/java/com/vaadin/ui/renderers/LocalDateRenderer.java b/server/src/main/java/com/vaadin/ui/renderers/LocalDateRenderer.java index 155867a695..ef962e7d8b 100644 --- a/server/src/main/java/com/vaadin/ui/renderers/LocalDateRenderer.java +++ b/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,12 +145,21 @@ 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, ""); } @@ -155,8 +167,56 @@ public class LocalDateRenderer extends AbstractRenderer<Object, LocalDate> { /** * 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); } + } diff --git a/server/src/main/java/com/vaadin/ui/renderers/LocalDateTimeRenderer.java b/server/src/main/java/com/vaadin/ui/renderers/LocalDateTimeRenderer.java index c0faae9dd9..9dc88b5cbc 100644 --- a/server/src/main/java/com/vaadin/ui/renderers/LocalDateTimeRenderer.java +++ b/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); } diff --git a/server/src/test/java/com/vaadin/data/provider/hierarchical/HierarchyMapperWithDataTest.java b/server/src/test/java/com/vaadin/data/provider/hierarchical/HierarchyMapperWithDataTest.java index 81ee908a52..e056b99c21 100644 --- a/server/src/test/java/com/vaadin/data/provider/hierarchical/HierarchyMapperWithDataTest.java +++ b/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++)); } } } diff --git a/server/src/test/java/com/vaadin/data/provider/hierarchical/Node.java b/server/src/test/java/com/vaadin/data/provider/hierarchical/Node.java index 95e961df52..1be5bfa0aa 100644 --- a/server/src/test/java/com/vaadin/data/provider/hierarchical/Node.java +++ b/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() { diff --git a/server/src/test/java/com/vaadin/tests/TestTemporalSerialization.java b/server/src/test/java/com/vaadin/tests/TestTemporalSerialization.java new file mode 100644 index 0000000000..51e4933de6 --- /dev/null +++ b/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); + } + } +} diff --git a/server/src/test/java/com/vaadin/tests/server/ClassesSerializableTest.java b/server/src/test/java/com/vaadin/tests/server/ClassesSerializableTest.java index 819f0530c1..4bc0dd2729 100644 --- a/server/src/test/java/com/vaadin/tests/server/ClassesSerializableTest.java +++ b/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); } |