From: Artur Signell Date: Mon, 15 Dec 2014 06:41:45 +0000 (+0200) Subject: Do not create default instances while reading designs (#7749) X-Git-Tag: 7.4.0.beta1~12^2^2~21 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=8eafe7aee31b3e2f80c87b39c4e42e260a86a5eb;p=vaadin-framework.git Do not create default instances while reading designs (#7749) Change-Id: I26cb6d8d43200d10ebf8c0ac22c538b4272f5ecd --- diff --git a/common.xml b/common.xml index 6e28ab6a60..edd941c5e3 100644 --- a/common.xml +++ b/common.xml @@ -386,6 +386,10 @@ + + + + diff --git a/server/src/com/vaadin/ui/AbstractComponent.java b/server/src/com/vaadin/ui/AbstractComponent.java index 3c68fde9b2..0b9f8bd244 100644 --- a/server/src/com/vaadin/ui/AbstractComponent.java +++ b/server/src/com/vaadin/ui/AbstractComponent.java @@ -920,11 +920,13 @@ public abstract class AbstractComponent extends AbstractClientConnector @Override public void readDesign(Element design, DesignContext designContext) { Attributes attr = design.attributes(); - AbstractComponent def = designContext.getDefaultInstance(this - .getClass()); // handle default attributes for (String attribute : getDefaultAttributes()) { - DesignAttributeHandler.readAttribute(this, attribute, attr, def); + if (!design.hasAttr(attribute)) { + continue; + } + + DesignAttributeHandler.readAttribute(this, attribute, attr); } // handle immediate if (attr.hasKey("immediate")) { @@ -941,14 +943,12 @@ public abstract class AbstractComponent extends AbstractClientConnector setLocale(null); } // handle width and height - readSize(attr, def); + readSize(attr); // handle component error if (attr.hasKey("error")) { UserError error = new UserError(attr.get("error"), ContentMode.HTML, ErrorLevel.ERROR); setComponentError(error); - } else { - setComponentError(def.getComponentError()); } // handle responsive setResponsive(attr.hasKey("responsive") @@ -1045,8 +1045,7 @@ public abstract class AbstractComponent extends AbstractClientConnector * @param defaultInstance * instance of the class that has default sizing. */ - private void readSize(Attributes attributes, - AbstractComponent defaultInstance) { + private void readSize(Attributes attributes) { // read width if (attributes.hasKey("width-auto") || attributes.hasKey("size-auto")) { this.setWidth(null); @@ -1055,9 +1054,6 @@ public abstract class AbstractComponent extends AbstractClientConnector this.setWidth("100%"); } else if (attributes.hasKey("width")) { this.setWidth(attributes.get("width")); - } else { - this.setWidth(defaultInstance.getWidth(), - defaultInstance.getWidthUnits()); } // read height @@ -1068,9 +1064,6 @@ public abstract class AbstractComponent extends AbstractClientConnector this.setHeight("100%"); } else if (attributes.hasKey("height")) { this.setHeight(attributes.get("height")); - } else { - this.setHeight(defaultInstance.getHeight(), - defaultInstance.getHeightUnits()); } } diff --git a/server/src/com/vaadin/ui/AbstractField.java b/server/src/com/vaadin/ui/AbstractField.java index ba518000d6..c3e2036936 100644 --- a/server/src/com/vaadin/ui/AbstractField.java +++ b/server/src/com/vaadin/ui/AbstractField.java @@ -1767,15 +1767,16 @@ public abstract class AbstractField extends AbstractComponent implements @Override public void readDesign(Element design, DesignContext designContext) { super.readDesign(design, designContext); - AbstractField def = designContext.getDefaultInstance(this.getClass()); Attributes attr = design.attributes(); - boolean readOnly = DesignAttributeHandler.readAttribute("readonly", - attr, def.isReadOnly(), Boolean.class); - setReadOnly(readOnly); + if (design.hasAttr("readonly")) { + setReadOnly(DesignAttributeHandler.readAttribute("readonly", attr, + Boolean.class)); + } // tabindex - int tabIndex = DesignAttributeHandler.readAttribute("tabindex", attr, - def.getTabIndex(), Integer.class); - setTabIndex(tabIndex); + if (design.hasAttr("tabindex")) { + setTabIndex(DesignAttributeHandler.readAttribute("tabindex", attr, + Integer.class)); + } } /* diff --git a/server/src/com/vaadin/ui/AbstractOrderedLayout.java b/server/src/com/vaadin/ui/AbstractOrderedLayout.java index e99641a9ab..cd5609f091 100644 --- a/server/src/com/vaadin/ui/AbstractOrderedLayout.java +++ b/server/src/com/vaadin/ui/AbstractOrderedLayout.java @@ -477,18 +477,10 @@ public abstract class AbstractOrderedLayout extends AbstractLayout implements public void readDesign(Element design, DesignContext designContext) { // process default attributes super.readDesign(design, designContext); - // remove current children - removeAllComponents(); // handle margin - AbstractOrderedLayout def = designContext.getDefaultInstance(this - .getClass()); if (design.hasAttr("margin")) { - String value = design.attr("margin"); - setMargin(value.isEmpty() || value.equalsIgnoreCase("true")); - - } else { - // we currently support only on-off margins - setMargin(def.getMargin().getBitMask() != 0); + setMargin(DesignAttributeHandler.readAttribute("margin", + design.attributes(), Boolean.class)); } // handle children for (Element childComponent : design.children()) { diff --git a/server/src/com/vaadin/ui/AbstractSplitPanel.java b/server/src/com/vaadin/ui/AbstractSplitPanel.java index 66d01084d1..9c33665c1c 100644 --- a/server/src/com/vaadin/ui/AbstractSplitPanel.java +++ b/server/src/com/vaadin/ui/AbstractSplitPanel.java @@ -566,41 +566,31 @@ public abstract class AbstractSplitPanel extends AbstractComponentContainer { super.readDesign(design, designContext); // handle custom attributes, use default values if no explicit value // set - AbstractSplitPanel def = designContext.getDefaultInstance(this - .getClass()); // There is no setter for reversed, so it will be handled using // setSplitPosition. - boolean reversed = DesignAttributeHandler.readAttribute("reversed", - design.attributes(), def.getSplitterState().positionReversed, - Boolean.class); + boolean reversed = false; + if (design.hasAttr("reversed")) { + reversed = DesignAttributeHandler.readAttribute("reversed", + design.attributes(), Boolean.class); + setSplitPosition(getSplitPosition(), reversed); + } if (design.hasAttr("split-position")) { SizeWithUnit splitPosition = SizeWithUnit.parseStringSize( - design.attr("split-position"), def.getSplitPositionUnit()); + design.attr("split-position"), Unit.PERCENTAGE); setSplitPosition(splitPosition.getSize(), splitPosition.getUnit(), reversed); - } else { // default value for split position - setSplitPosition(def.getSplitPosition(), - def.getSplitPositionUnit(), reversed); } if (design.hasAttr("min-split-position")) { SizeWithUnit minSplitPosition = SizeWithUnit.parseStringSize( - design.attr("min-split-position"), - def.getMinSplitPositionUnit()); + design.attr("min-split-position"), Unit.PERCENTAGE); setMinSplitPosition(minSplitPosition.getSize(), minSplitPosition.getUnit()); - } else { // default value for min-split-position - setMinSplitPosition(def.getMinSplitPosition(), - def.getMinSplitPositionUnit()); } if (design.hasAttr("max-split-position")) { SizeWithUnit maxSplitPosition = SizeWithUnit.parseStringSize( - design.attr("max-split-position"), - def.getMaxSplitPositionUnit()); + design.attr("max-split-position"), Unit.PERCENTAGE); setMaxSplitPosition(maxSplitPosition.getSize(), maxSplitPosition.getUnit()); - } else { // default value for max-split-position - setMaxSplitPosition(def.getMaxSplitPosition(), - def.getMaxSplitPositionUnit()); } // remove current children removeAllComponents(); diff --git a/server/src/com/vaadin/ui/AbstractTextField.java b/server/src/com/vaadin/ui/AbstractTextField.java index 79492ecf5e..8bca74936a 100644 --- a/server/src/com/vaadin/ui/AbstractTextField.java +++ b/server/src/com/vaadin/ui/AbstractTextField.java @@ -772,12 +772,11 @@ public abstract class AbstractTextField extends AbstractField implements @Override public void readDesign(Element design, DesignContext designContext) { super.readDesign(design, designContext); - AbstractTextField def = designContext.getDefaultInstance(this - .getClass()); Attributes attr = design.attributes(); - int maxLength = DesignAttributeHandler.readAttribute("maxlength", attr, - def.getMaxLength(), Integer.class); - setMaxLength(maxLength); + if (attr.hasKey("maxlength")) { + setMaxLength(DesignAttributeHandler.readAttribute("maxlength", + attr, Integer.class)); + } } /* diff --git a/server/src/com/vaadin/ui/Button.java b/server/src/com/vaadin/ui/Button.java index 2d0a0cf8da..9b50b5eebc 100644 --- a/server/src/com/vaadin/ui/Button.java +++ b/server/src/com/vaadin/ui/Button.java @@ -675,22 +675,26 @@ public class Button extends AbstractComponent implements @Override public void readDesign(Element design, DesignContext designContext) { super.readDesign(design, designContext); - Button def = designContext.getDefaultInstance(this.getClass()); Attributes attr = design.attributes(); String content = design.html(); setCaption(content); // tabindex - setTabIndex(DesignAttributeHandler.readAttribute("tabindex", attr, - def.getTabIndex(), Integer.class)); + if (attr.hasKey("tabindex")) { + setTabIndex(DesignAttributeHandler.readAttribute("tabindex", attr, + Integer.class)); + } // plain-text (default is html) - setHtmlContentAllowed(!DesignAttributeHandler.readAttribute( - DESIGN_ATTR_PLAIN_TEXT, attr, false, Boolean.class)); + Boolean plain = DesignAttributeHandler.readAttribute( + DESIGN_ATTR_PLAIN_TEXT, attr, Boolean.class); + if (plain == null || !plain) { + setHtmlContentAllowed(true); + } setIconAlternateText(DesignAttributeHandler.readAttribute("icon-alt", - attr, def.getIconAlternateText(), String.class)); + attr, String.class)); // click-shortcut removeClickShortcut(); ShortcutAction action = DesignAttributeHandler.readAttribute( - "click-shortcut", attr, null, ShortcutAction.class); + "click-shortcut", attr, ShortcutAction.class); if (action != null) { setClickShortcut(action.getKeyCode(), action.getModifiers()); } diff --git a/server/src/com/vaadin/ui/Panel.java b/server/src/com/vaadin/ui/Panel.java index 5112a732ac..b0242cc32d 100644 --- a/server/src/com/vaadin/ui/Panel.java +++ b/server/src/com/vaadin/ui/Panel.java @@ -348,9 +348,8 @@ public class Panel extends AbstractSingleComponentContainer implements public void readDesign(Element design, DesignContext designContext) { super.readDesign(design, designContext); // handle tabindex - Panel def = designContext.getDefaultInstance(this.getClass()); int tabIndex = DesignAttributeHandler.readAttribute("tabindex", - design.attributes(), def.getTabIndex(), Integer.class); + design.attributes(), Integer.class); setTabIndex(tabIndex); } diff --git a/server/src/com/vaadin/ui/PasswordField.java b/server/src/com/vaadin/ui/PasswordField.java index 5f27eb59c9..974a1c2c10 100644 --- a/server/src/com/vaadin/ui/PasswordField.java +++ b/server/src/com/vaadin/ui/PasswordField.java @@ -91,12 +91,11 @@ public class PasswordField extends AbstractTextField { @Override public void readDesign(Element design, DesignContext designContext) { super.readDesign(design, designContext); - AbstractTextField def = designContext.getDefaultInstance(this - .getClass()); Attributes attr = design.attributes(); - String value = DesignAttributeHandler.readAttribute("value", attr, - def.getValue(), String.class); - setValue(value); + if (attr.hasKey("value")) { + setValue(DesignAttributeHandler.readAttribute("value", attr, + String.class)); + } } /* diff --git a/server/src/com/vaadin/ui/TabSheet.java b/server/src/com/vaadin/ui/TabSheet.java index 94bcb80279..330acacfd0 100644 --- a/server/src/com/vaadin/ui/TabSheet.java +++ b/server/src/com/vaadin/ui/TabSheet.java @@ -1464,12 +1464,8 @@ public class TabSheet extends AbstractComponentContainer implements Focusable, @Override public void readDesign(Element design, DesignContext designContext) { super.readDesign(design, designContext); - Attributes attr = design.attributes(); - TabSheet def = designContext.getDefaultInstance(this.getClass()); // handle tab index - int tabIndex = DesignAttributeHandler.readAttribute("tabindex", attr, - def.getTabIndex(), Integer.class); - setTabIndex(tabIndex); + readTabIndex(design); // clear old tabs removeAllComponents(); // create new tabs @@ -1482,6 +1478,15 @@ public class TabSheet extends AbstractComponentContainer implements Focusable, } } + private void readTabIndex(Element design) { + // Could be in AbstractComponent as if (this implements Focusable) + if (design.hasAttr("tabindex")) { + setTabIndex(DesignAttributeHandler.readAttribute("tabindex", + design.attributes(), Integer.class)); + } + + } + /** * Reads the given tab element from design * @@ -1503,28 +1508,48 @@ public class TabSheet extends AbstractComponentContainer implements Focusable, Element content = tabElement.child(0); Component child = designContext.createChild(content); Tab tab = this.addTab(child); - tab.setVisible(DesignAttributeHandler.readAttribute("visible", attr, - tab.isVisible(), Boolean.class)); - tab.setClosable(DesignAttributeHandler.readAttribute("closable", attr, - tab.isClosable(), Boolean.class)); - tab.setCaption(DesignAttributeHandler.readAttribute("caption", attr, - tab.getCaption(), String.class)); - tab.setEnabled(DesignAttributeHandler.readAttribute("enabled", attr, - tab.isEnabled(), Boolean.class)); - tab.setIcon(DesignAttributeHandler.readAttribute("icon", attr, - tab.getIcon(), Resource.class)); - tab.setIconAlternateText(DesignAttributeHandler.readAttribute( - "icon-alt", attr, tab.getIconAlternateText(), String.class)); - tab.setDescription(DesignAttributeHandler.readAttribute("description", - attr, tab.getDescription(), String.class)); - tab.setStyleName(DesignAttributeHandler.readAttribute("style-name", - attr, tab.getStyleName(), String.class)); - tab.setId(DesignAttributeHandler.readAttribute("id", attr, tab.getId(), - String.class)); - boolean selected = DesignAttributeHandler.readAttribute("selected", - attr, false, Boolean.class); - if (selected) { - this.setSelectedTab(tab.getComponent()); + if (attr.hasKey("visible")) { + tab.setVisible(DesignAttributeHandler.readAttribute("visible", + attr, Boolean.class)); + } + if (attr.hasKey("closable")) { + tab.setClosable(DesignAttributeHandler.readAttribute("closable", + attr, Boolean.class)); + } + if (attr.hasKey("caption")) { + tab.setCaption(DesignAttributeHandler.readAttribute("caption", + attr, String.class)); + } + if (attr.hasKey("enabled")) { + tab.setEnabled(DesignAttributeHandler.readAttribute("enabled", + attr, Boolean.class)); + } + if (attr.hasKey("icon")) { + tab.setIcon(DesignAttributeHandler.readAttribute("icon", attr, + Resource.class)); + } + if (attr.hasKey("icon-alt")) { + tab.setIconAlternateText(DesignAttributeHandler.readAttribute( + "icon-alt", attr, String.class)); + } + if (attr.hasKey("description")) { + tab.setDescription(DesignAttributeHandler.readAttribute( + "description", attr, String.class)); + } + if (attr.hasKey("style-name")) { + tab.setStyleName(DesignAttributeHandler.readAttribute("style-name", + attr, String.class)); + } + if (attr.hasKey("id")) { + tab.setId(DesignAttributeHandler.readAttribute("id", attr, + String.class)); + } + if (attr.hasKey("selected")) { + boolean selected = DesignAttributeHandler.readAttribute("selected", + attr, Boolean.class); + if (selected) { + this.setSelectedTab(tab.getComponent()); + } } } diff --git a/server/src/com/vaadin/ui/TextField.java b/server/src/com/vaadin/ui/TextField.java index 03bbfd2d8f..cd919ded9c 100644 --- a/server/src/com/vaadin/ui/TextField.java +++ b/server/src/com/vaadin/ui/TextField.java @@ -113,12 +113,11 @@ public class TextField extends AbstractTextField { @Override public void readDesign(Element design, DesignContext designContext) { super.readDesign(design, designContext); - AbstractTextField def = designContext.getDefaultInstance(this - .getClass()); Attributes attr = design.attributes(); - String value = DesignAttributeHandler.readAttribute("value", attr, - def.getValue(), String.class); - setValue(value); + if (attr.hasKey("value")) { + setValue(DesignAttributeHandler.readAttribute("value", attr, + String.class)); + } } /* diff --git a/server/src/com/vaadin/ui/declarative/DesignAttributeHandler.java b/server/src/com/vaadin/ui/declarative/DesignAttributeHandler.java index b410fd0001..0a58fc549f 100644 --- a/server/src/com/vaadin/ui/declarative/DesignAttributeHandler.java +++ b/server/src/com/vaadin/ui/declarative/DesignAttributeHandler.java @@ -99,16 +99,12 @@ public class DesignAttributeHandler implements Serializable { * the attribute map. If the attributes does not contain the * requested attribute, the value is retrieved from the * defaultInstance - * @param defaultInstance - * the default instance of the class for fetching the default - * values * @return true on success */ public static boolean readAttribute(Component component, String attribute, - Attributes attributes, Component defaultInstance) { + Attributes attributes) { String value = null; - if (component == null || attribute == null || attributes == null - || defaultInstance == null) { + if (component == null || attribute == null || attributes == null) { throw new IllegalArgumentException( "Parameters with null value not allowed"); } @@ -129,13 +125,10 @@ public class DesignAttributeHandler implements Serializable { setter.invoke(component, param); success = true; } else { - // otherwise find the getter for the attribute - Method getter = findGetterForAttribute(component.getClass(), - attribute); - // read the default value from defaults - Object defaultValue = getter.invoke(defaultInstance); - setter.invoke(component, defaultValue); - success = true; + getLogger().log( + Level.WARNING, + "Attribute value for " + attribute + + " is null, this should not happen"); } } catch (Exception e) { getLogger().log(Level.WARNING, @@ -256,13 +249,13 @@ public class DesignAttributeHandler implements Serializable { */ @SuppressWarnings("unchecked") public static T readAttribute(String attribute, Attributes attributes, - T defaultValue, Class outputType) { + Class outputType) { if (!isSupported(outputType)) { throw new IllegalArgumentException("output type: " + outputType.getName() + " not supported"); } if (!attributes.hasKey(attribute)) { - return defaultValue; + return null; } else { try { String value = attributes.get(attribute); diff --git a/server/tests/src/com/vaadin/tests/design/designroot/DesignRootTest.java b/server/tests/src/com/vaadin/tests/design/designroot/DesignRootTest.java index 0d390ba184..300d293e66 100644 --- a/server/tests/src/com/vaadin/tests/design/designroot/DesignRootTest.java +++ b/server/tests/src/com/vaadin/tests/design/designroot/DesignRootTest.java @@ -16,11 +16,8 @@ package com.vaadin.tests.design.designroot; import org.junit.Assert; -import org.junit.Ignore; import org.junit.Test; -// This test will not pass until default instance creation is changed. Will leave it ignored for now. -@Ignore public class DesignRootTest { @Test public void designAnnotationWithoutFilename() { diff --git a/server/tests/src/com/vaadin/tests/server/component/abstractsplitpanel/TestReadDesign.java b/server/tests/src/com/vaadin/tests/server/component/abstractsplitpanel/TestReadDesign.java index c57f18cbf9..db30f05c5f 100644 --- a/server/tests/src/com/vaadin/tests/server/component/abstractsplitpanel/TestReadDesign.java +++ b/server/tests/src/com/vaadin/tests/server/component/abstractsplitpanel/TestReadDesign.java @@ -60,21 +60,6 @@ public class TestReadDesign extends TestCase { assertEquals(Unit.PIXELS, sp.getMaxSplitPositionUnit()); assertEquals(true, sp.isLocked()); checkReversed(sp, true); - // check that the properties get the default values if the design - // does not have attributes corresponding to those properties - design = createDesign(true, true, true, true); - sp.readDesign(design, ctx); - HorizontalSplitPanel def = new HorizontalSplitPanel(); - assertEquals(def.getSplitPosition(), sp.getSplitPosition()); - assertEquals(def.getSplitPositionUnit(), sp.getSplitPositionUnit()); - assertEquals(def.getMinSplitPosition(), sp.getMinSplitPosition()); - assertEquals(def.getMinSplitPositionUnit(), - sp.getMinSplitPositionUnit()); - assertEquals(def.getMaxSplitPosition(), sp.getMaxSplitPosition()); - assertEquals(def.getMaxSplitPositionUnit(), - sp.getMaxSplitPositionUnit()); - assertEquals(def.isLocked(), sp.isLocked()); - checkReversed(sp, false); } public void testWithNoChildren() {