From: Teemu Suo-Anttila Date: Wed, 19 Oct 2016 11:54:33 +0000 (+0300) Subject: Fix setValue() methods behavior null argument value + javadocs X-Git-Tag: 8.0.0.alpha6~47 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=63d218efc0ee5825633b4950e3355f4e8cc7bc29;p=vaadin-framework.git Fix setValue() methods behavior null argument value + javadocs Change-Id: I0000c1caf7c129634473161fe4876931f3c8dabf --- diff --git a/server/src/main/java/com/vaadin/data/HasValue.java b/server/src/main/java/com/vaadin/data/HasValue.java index 7a8bc7c296..aa130344b2 100644 --- a/server/src/main/java/com/vaadin/data/HasValue.java +++ b/server/src/main/java/com/vaadin/data/HasValue.java @@ -80,6 +80,7 @@ public interface HasValue extends Serializable { * {@code true} if this event originates from the client, * {@code false} otherwise. */ + public ValueChangeEvent(Component component, HasValue hasValue, boolean userOriginated) { super(hasValue); @@ -141,7 +142,6 @@ public interface HasValue extends Serializable { @FunctionalInterface public interface ValueChangeListener extends Consumer>, Serializable { - @Deprecated public static final Method VALUE_CHANGE_METHOD = ReflectTools .findMethod(ValueChangeListener.class, "accept", diff --git a/server/src/main/java/com/vaadin/ui/AbstractColorPicker.java b/server/src/main/java/com/vaadin/ui/AbstractColorPicker.java index f256636845..a537df0b21 100644 --- a/server/src/main/java/com/vaadin/ui/AbstractColorPicker.java +++ b/server/src/main/java/com/vaadin/ui/AbstractColorPicker.java @@ -164,10 +164,12 @@ public abstract class AbstractColorPicker extends AbstractField { /** * Sets the selected color of this color picker. If the new color is not - * equal to getValue(), fires a value change event. + * equal to getValue(), fires a {@link ValueChangeEvent}. * * @param color * the new selected color, not null + * @throws NullPointerException + * if {@code color} is {@code null} */ @Override public void setValue(Color color) { diff --git a/server/src/main/java/com/vaadin/ui/AbstractDateField.java b/server/src/main/java/com/vaadin/ui/AbstractDateField.java index 4735d155d9..c62c76ac9c 100644 --- a/server/src/main/java/com/vaadin/ui/AbstractDateField.java +++ b/server/src/main/java/com/vaadin/ui/AbstractDateField.java @@ -484,6 +484,13 @@ public abstract class AbstractDateField extends AbstractField return value; } + /** + * Sets the value of this object. If the new value is not equal to + * {@code getValue()}, fires a {@link ValueChangeEvent} . + * + * @param value + * the new value, may be {@code null} + */ @Override public void setValue(LocalDate value) { /* diff --git a/server/src/main/java/com/vaadin/ui/AbstractField.java b/server/src/main/java/com/vaadin/ui/AbstractField.java index 7d87053a22..c87a72e430 100644 --- a/server/src/main/java/com/vaadin/ui/AbstractField.java +++ b/server/src/main/java/com/vaadin/ui/AbstractField.java @@ -52,8 +52,8 @@ public abstract class AbstractField extends AbstractComponent implements HasValue, HasRequired, Focusable { @Deprecated - private static final Method VALUE_CHANGE_METHOD = ReflectTools - .findMethod(ValueChangeListener.class, "accept", ValueChangeEvent.class); + private static final Method VALUE_CHANGE_METHOD = ReflectTools.findMethod( + ValueChangeListener.class, "accept", ValueChangeEvent.class); @Override public void setValue(T value) { diff --git a/server/src/main/java/com/vaadin/ui/AbstractMultiSelect.java b/server/src/main/java/com/vaadin/ui/AbstractMultiSelect.java index 5374b7d608..5ab6951f8d 100644 --- a/server/src/main/java/com/vaadin/ui/AbstractMultiSelect.java +++ b/server/src/main/java/com/vaadin/ui/AbstractMultiSelect.java @@ -366,7 +366,7 @@ public abstract class AbstractMultiSelect * * @param value * the items to select, not {@code null} - * @throws IllegalArgumentException + * @throws NullPointerException * if the value is invalid */ @Override diff --git a/server/src/main/java/com/vaadin/ui/AbstractTextField.java b/server/src/main/java/com/vaadin/ui/AbstractTextField.java index 9c2720efd8..ee2ccc6604 100644 --- a/server/src/main/java/com/vaadin/ui/AbstractTextField.java +++ b/server/src/main/java/com/vaadin/ui/AbstractTextField.java @@ -79,9 +79,19 @@ public abstract class AbstractTextField extends AbstractField registerRpc(new AbstractTextFieldFocusAndBlurRpcImpl()); } + /** + * Sets the value of this text field. If the new value is not equal to + * {@code getValue()}, fires a {@link ValueChangeEvent}. Throws + * {@code NullPointerException} if the value is not null. + * + * @param value + * the new value, not {@code null} + * @throws NullPointerException + * if {@code value} is {@code null} + */ @Override public void setValue(String value) { - Objects.requireNonNull(value, "Null value not supported"); + Objects.requireNonNull(value, "value cannot be null"); setValue(value, false); } diff --git a/server/src/main/java/com/vaadin/ui/CheckBox.java b/server/src/main/java/com/vaadin/ui/CheckBox.java index c4bedbea0a..02d7b0db8b 100644 --- a/server/src/main/java/com/vaadin/ui/CheckBox.java +++ b/server/src/main/java/com/vaadin/ui/CheckBox.java @@ -17,6 +17,7 @@ package com.vaadin.ui; import java.util.Collection; +import java.util.Objects; import org.jsoup.nodes.Attributes; import org.jsoup.nodes.Element; @@ -115,21 +116,18 @@ public class CheckBox extends AbstractField } /** - * Sets the value of this ComboBox. If the new value is not equal to - * {@code getValue()}, fires a value change event. Throws - * {@code IllegalArgumentException} if the value is null. + * Sets the value of this CheckBox. If the new value is not equal to + * {@code getValue()}, fires a {@link ValueChangeEvent}. Throws + * {@code NullPointerException} if the value is null. * * @param value - * the new value - * @throws IllegalArgumentException - * if the value is null + * the new value, not {@code null} + * @throws NullPointerException + * if {@code value} is {@code null} */ @Override public void setValue(Boolean value) { - if (value == null) { - throw new IllegalArgumentException( - "CheckBox value must not be null"); - } + Objects.requireNonNull(value, "CheckBox value must not be null"); super.setValue(value); } diff --git a/server/src/main/java/com/vaadin/ui/ComboBox.java b/server/src/main/java/com/vaadin/ui/ComboBox.java index 9ebc035c12..5c40a744f7 100644 --- a/server/src/main/java/com/vaadin/ui/ComboBox.java +++ b/server/src/main/java/com/vaadin/ui/ComboBox.java @@ -548,6 +548,13 @@ public class ComboBox extends AbstractSingleSelect implements HasValue, this.filter = filter; } + /** + * Sets the value of this object. If the new value is not equal to + * {@code getValue()}, fires a {@link ValueChangeEvent}. + * + * @param value + * the new value, may be {@code null} + */ @Override public void setValue(T value) { getSelectionModel().setSelectedFromServer(value); diff --git a/server/src/main/java/com/vaadin/ui/RichTextArea.java b/server/src/main/java/com/vaadin/ui/RichTextArea.java index 545166d33a..96a639e285 100644 --- a/server/src/main/java/com/vaadin/ui/RichTextArea.java +++ b/server/src/main/java/com/vaadin/ui/RichTextArea.java @@ -16,6 +16,8 @@ package com.vaadin.ui; +import java.util.Objects; + import org.jsoup.nodes.Element; import com.vaadin.shared.ui.ValueChangeMode; @@ -100,13 +102,20 @@ public class RichTextArea extends AbstractField return (RichTextAreaState) super.getState(markAsDirty); } + /** + * Sets the value of this object. If the new value is not equal to + * {@code getValue()}, fires a {@link ValueChangeEvent}. Throws + * {@code NullPointerException} if the value is null. + * + * @param value + * the new value, not {@code null} + * @throws NullPointerException + * if {@code value} is {@code null} + */ @Override public void setValue(String value) { - if (value == null) { - setValue("", false); - } else { - setValue(value, false); - } + Objects.requireNonNull(value, "value cannot be null"); + setValue(value, false); } @Override diff --git a/server/src/main/java/com/vaadin/ui/Slider.java b/server/src/main/java/com/vaadin/ui/Slider.java index 99ab476e2a..397e33e7c3 100644 --- a/server/src/main/java/com/vaadin/ui/Slider.java +++ b/server/src/main/java/com/vaadin/ui/Slider.java @@ -17,6 +17,7 @@ package com.vaadin.ui; import java.util.Collection; +import java.util.Objects; import org.jsoup.nodes.Attributes; import org.jsoup.nodes.Element; @@ -289,6 +290,22 @@ public class Slider extends AbstractField { getState().value = trimmedValue; } + /** + * Sets the value of this object. If the new value is not equal to + * {@code getValue()}, fires a {@link ValueChangeEvent}. Throws + * {@code NullPointerException} if the value is null. + * + * @param value + * the new value, not {@code null} + * @throws NullPointerException + * if {@code value} is {@code null} + */ + @Override + public void setValue(Double value) { + Objects.requireNonNull(value, "color cannot be null"); + super.setValue(value); + } + @Override public Double getValue() { return getState().value; diff --git a/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerGradient.java b/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerGradient.java index ed8b12f80b..2e0b855ef9 100644 --- a/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerGradient.java +++ b/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerGradient.java @@ -15,6 +15,8 @@ */ package com.vaadin.ui.components.colorpicker; +import java.util.Objects; + import com.vaadin.shared.ui.colorpicker.Color; import com.vaadin.shared.ui.colorpicker.ColorPickerGradientServerRpc; import com.vaadin.shared.ui.colorpicker.ColorPickerGradientState; @@ -64,6 +66,22 @@ public class ColorPickerGradient extends AbstractField { this.converter = converter; } + /** + * Sets the value of this object. If the new value is not equal to + * {@code getValue()}, fires a {@link ValueChangeEvent}. Throws + * {@code NullPointerException} if the value is null. + * + * @param color + * the new color, not {@code null} + * @throws NullPointerException + * if {@code color} is {@code null} + */ + @Override + public void setValue(Color color) { + Objects.requireNonNull(color, "value must not be null"); + super.setValue(color); + } + @Override public Color getValue() { return color; diff --git a/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerGrid.java b/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerGrid.java index c03c865d6d..95229a2d6a 100644 --- a/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerGrid.java +++ b/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerGrid.java @@ -18,6 +18,7 @@ package com.vaadin.ui.components.colorpicker; import java.awt.Point; import java.util.HashMap; import java.util.Map; +import java.util.Objects; import com.vaadin.shared.ui.colorpicker.Color; import com.vaadin.shared.ui.colorpicker.ColorPickerGridServerRpc; @@ -174,6 +175,22 @@ public class ColorPickerGrid extends AbstractField { return new int[] { x, y }; } + /** + * Sets the value of this object. If the new value is not equal to + * {@code getValue()}, fires a {@link ValueChangeEvent}. Throws + * {@code NullPointerException} if the value is null. + * + * @param color + * the new value, not {@code null} + * @throws NullPointerException + * if {@code color} is {@code null} + */ + @Override + public void setValue(Color color) { + Objects.requireNonNull(color, "value cannot be null"); + super.setValue(color); + } + @Override public Color getValue() { return colorGrid[x][y]; diff --git a/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerPopup.java b/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerPopup.java index 4cf79d862d..ba00773fec 100644 --- a/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerPopup.java +++ b/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerPopup.java @@ -458,11 +458,19 @@ public class ColorPickerPopup extends Window implements HasValue { return history; } + /** + * Sets the value of this object. If the new value is not equal to + * {@code getValue()}, fires a {@link ValueChangeEvent}. Throws + * {@code NullPointerException} if the value is null. + * + * @param color + * the new value, not {@code null} + * @throws NullPointerException + * if {@code color} is {@code null} + */ @Override public void setValue(Color color) { - if (color == null) { - return; - } + Objects.requireNonNull(color, "color cannot be null"); selectedColor = color; diff --git a/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerPreview.java b/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerPreview.java index fd284b64b9..67724d134b 100644 --- a/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerPreview.java +++ b/server/src/main/java/com/vaadin/ui/components/colorpicker/ColorPickerPreview.java @@ -63,8 +63,19 @@ public class ColorPickerPreview extends CssLayout implements HasValue { setValue(color); } + /** + * Sets the value of this object. If the new value is not equal to + * {@code getValue()}, fires a {@link ValueChangeEvent}. Throws + * {@code NullPointerException} if the value is null. + * + * @param color + * the new value, not {@code null} + * @throws NullPointerException + * if {@code color} is {@code null} + */ @Override public void setValue(Color color) { + Objects.requireNonNull(color, "color cannot be null"); this.color = color; // Unregister listener @@ -160,7 +171,8 @@ public class ColorPickerPreview extends CssLayout implements HasValue { } oldValue = value; - fireEvent(new ValueChangeEvent<>(this, event.isUserOriginated())); + fireEvent( + new ValueChangeEvent<>(this, event.isUserOriginated())); } } catch (NumberFormatException nfe) { diff --git a/server/src/test/java/com/vaadin/tests/server/component/abstracttextfield/AbstractTextFieldTest.java b/server/src/test/java/com/vaadin/tests/server/component/abstracttextfield/AbstractTextFieldTest.java new file mode 100644 index 0000000000..5645c44970 --- /dev/null +++ b/server/src/test/java/com/vaadin/tests/server/component/abstracttextfield/AbstractTextFieldTest.java @@ -0,0 +1,36 @@ +/* + * Copyright 2000-2016 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.tests.server.component.abstracttextfield; + +import org.junit.Test; +import org.mockito.Mockito; + +import com.vaadin.ui.AbstractTextField; + +/** + * @author Vaadin Ltd + * + */ +public class AbstractTextFieldTest { + + @Test(expected = NullPointerException.class) + public void setValue_nullValue_throwsNPE() { + AbstractTextField field = Mockito.mock(AbstractTextField.class); + Mockito.doCallRealMethod().when(field).setValue(null); + + field.setValue(null); + } +} diff --git a/server/src/test/java/com/vaadin/tests/server/component/colorpicker/AbstractColorPickerTest.java b/server/src/test/java/com/vaadin/tests/server/component/colorpicker/AbstractColorPickerTest.java new file mode 100644 index 0000000000..f4663663d4 --- /dev/null +++ b/server/src/test/java/com/vaadin/tests/server/component/colorpicker/AbstractColorPickerTest.java @@ -0,0 +1,36 @@ +/* + * Copyright 2000-2016 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.tests.server.component.colorpicker; + +import org.junit.Test; +import org.mockito.Mockito; + +import com.vaadin.ui.AbstractColorPicker; + +/** + * @author Vaadin Ltd + * + */ +public class AbstractColorPickerTest { + + @Test(expected = NullPointerException.class) + public void setValue_nullValue_throwsNPE() { + AbstractColorPicker picker = Mockito.mock(AbstractColorPicker.class); + Mockito.doCallRealMethod().when(picker).setValue(null); + + picker.setValue(null); + } +} diff --git a/server/src/test/java/com/vaadin/tests/server/component/colorpicker/ColorPickerComponentsTest.java b/server/src/test/java/com/vaadin/tests/server/component/colorpicker/ColorPickerComponentsTest.java new file mode 100644 index 0000000000..a655a3b6f6 --- /dev/null +++ b/server/src/test/java/com/vaadin/tests/server/component/colorpicker/ColorPickerComponentsTest.java @@ -0,0 +1,58 @@ +/* + * Copyright 2000-2016 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.tests.server.component.colorpicker; + +import org.junit.Test; +import org.mockito.Mockito; + +import com.vaadin.shared.ui.colorpicker.Color; +import com.vaadin.ui.AbstractColorPicker.Coordinates2Color; +import com.vaadin.ui.components.colorpicker.ColorPickerGradient; +import com.vaadin.ui.components.colorpicker.ColorPickerGrid; +import com.vaadin.ui.components.colorpicker.ColorPickerPopup; +import com.vaadin.ui.components.colorpicker.ColorPickerPreview; + +/** + * @author Vaadin Ltd + * + */ +public class ColorPickerComponentsTest { + + @Test(expected = NullPointerException.class) + public void setValue_nullValue_colorPickerGradientThrowsNPE() { + ColorPickerGradient gradient = new ColorPickerGradient("foo", + Mockito.mock(Coordinates2Color.class)); + gradient.setValue(null); + } + + @Test(expected = NullPointerException.class) + public void setValue_nullValue_colorPickerGridThrowsNPE() { + ColorPickerGrid grid = new ColorPickerGrid(); + grid.setValue(null); + } + + @Test(expected = NullPointerException.class) + public void setValue_nullValue_colorPickerPopupThrowsNPE() { + ColorPickerPopup popup = new ColorPickerPopup(Color.WHITE); + popup.setValue(null); + } + + @Test(expected = NullPointerException.class) + public void setValue_nullValue_colorPickerPreviewThrowsNPE() { + ColorPickerPreview preview = new ColorPickerPreview(Color.WHITE); + preview.setValue(null); + } +} diff --git a/server/src/test/java/com/vaadin/tests/server/component/slider/SliderTest.java b/server/src/test/java/com/vaadin/tests/server/component/slider/SliderTest.java index 0ce40cad39..4d178e618f 100644 --- a/server/src/test/java/com/vaadin/tests/server/component/slider/SliderTest.java +++ b/server/src/test/java/com/vaadin/tests/server/component/slider/SliderTest.java @@ -126,4 +126,10 @@ public class SliderTest { assertThat(slider.getValue(), is(1.23)); } + + @Test(expected = NullPointerException.class) + public void setValue_nullValue_throwNPE() { + Slider slider = new Slider(); + slider.setValue(null); + } } diff --git a/server/src/test/java/com/vaadin/ui/CheckBoxTest.java b/server/src/test/java/com/vaadin/ui/CheckBoxTest.java index 04d626c7d1..ee0ed943eb 100644 --- a/server/src/test/java/com/vaadin/ui/CheckBoxTest.java +++ b/server/src/test/java/com/vaadin/ui/CheckBoxTest.java @@ -34,4 +34,10 @@ public class CheckBoxTest { Assert.assertFalse(cb.getValue()); } + @Test(expected = NullPointerException.class) + public void setValue_nullValue_throwsNPE() { + CheckBox cb = new CheckBox(); + cb.setValue(null); + } + } diff --git a/server/src/test/java/com/vaadin/ui/RichTextAreaTest.java b/server/src/test/java/com/vaadin/ui/RichTextAreaTest.java index 554e002848..6a4fd32173 100644 --- a/server/src/test/java/com/vaadin/ui/RichTextAreaTest.java +++ b/server/src/test/java/com/vaadin/ui/RichTextAreaTest.java @@ -88,11 +88,10 @@ public class RichTextAreaTest extends ComponentTest { Assert.assertEquals("bar", tf.getValue()); } - @Test - public void setValueNullBecomesEmptyString() { + @Test(expected = NullPointerException.class) + public void setValue_nullValue_throwsNPE() { RichTextArea tf = new RichTextArea(); tf.setValue(null); - Assert.assertEquals("", tf.getValue()); } @Test