TextField value change listener triggered twice - AbstractField bug?

I have implemented value change listener on TextField in may table. Code goes something like this:


        field.addListener(new ValueChangeListener() {
		@Override
		public void valueChange(ValueChangeEvent event) {
			log.trace("value changed to " + event.getProperty().getValue());
			// do some access to db....
		}
	});

The problem is that valueChange method is triggered twice so it limits my business logic in it, e.g. i can’t put logic for accessing db because of additional overhead of second call.
I detected that problem lies in AbstractField.setValue(…) protected method… or maybe I’m doing something wrong.

The first event is fired when dataSource.setValue(newValue) is called (bolded in code belowe).
The second one is called when fireValueChange(repaintIsNotNeeded) is called explicitly (bolded in code below).

Is this a bug? Shouldn’t explicit call be wrapped in conditional clause something like this:


if (!isWriteThrough()) {
    fireValueChange(repaintIsNotNeeded);
}

code from AbstractField class:



    /**
     * Sets the value of the field.
     * 
     * @param newValue
     *            the New value of the field.
     * @param repaintIsNotNeeded
     *            True iff caller is sure that repaint is not needed.
     * @throws Property.ReadOnlyException
     * @throws Property.ConversionException
     */
    protected void setValue(Object newValue, boolean repaintIsNotNeeded)
            throws Property.ReadOnlyException, Property.ConversionException {

        if ((newValue == null && value != null)
                || (newValue != null && !newValue.equals(value))) {

            // Read only fields can not be changed
            if (isReadOnly()) {
                throw new Property.ReadOnlyException();
            }

            // Repaint is needed even when the client thinks that it knows the
            // new state if validity of the component may change
            if (repaintIsNotNeeded && (isRequired() || getValidators() != null)) {
                repaintIsNotNeeded = false;
            }

            // If invalid values are not allowed, the value must be checked
            if (!isInvalidAllowed()) {
                final Collection<Validator> v = getValidators();
                if (v != null) {
                    for (final Iterator<Validator> i = v.iterator(); i
                            .hasNext();) {
                        (i.next()).validate(newValue);
                    }
                }
            }

            // Changes the value
            setInternalValue(newValue);
            modified = dataSource != null;

            // In write trough mode , try to commit
            if (isWriteThrough() && dataSource != null
                    && (isInvalidCommitted() || isValid())) {
                try {

                    // Commits the value to datasource
                    [b]
dataSource.setValue(newValue);
[/b]

                    // The buffer is now unmodified
                    modified = false;

                } catch (final Throwable e) {

                    // Sets the buffering state
                    currentBufferedSourceException = new Buffered.SourceException(
                            this, e);
                    requestRepaint();

                    // Throws the source exception
                    throw currentBufferedSourceException;
                }
            }

            // If successful, remove set the buffering state to be ok
            if (currentBufferedSourceException != null) {
                currentBufferedSourceException = null;
                requestRepaint();
            }

            // Fires the value change
            [b]
fireValueChange(repaintIsNotNeeded);
[/b]
        }
    }

Ok, I have done simple test case. Can anybody tell me why value change event is triggered twice? Is this a feature or bug :slight_smile:



	private void test() {
		Table table = new Table();
		table.setTableFieldFactory(new TableFieldFactory() {
			
			@Override
			public Field createField(Container container, Object itemId,
					Object propertyId, Component uiContext) {
				CheckBox checkBox = new CheckBox();
				checkBox.setImmediate(true);
				checkBox.addListener(new ValueChangeListener() {
					
					@Override
					public void valueChange(ValueChangeEvent event) {
						//log.info("value changed...");
						System.out.println("value changed...");
					}
				});
				return checkBox;
			}
		});
		table.setContainerDataSource(new BeanItemContainer<TestBean>(Arrays.asList(new TestBean())));
		table.setEditable(true);
		
		mainWindow.addComponent(table);
	}
	
	public class TestBean {
		Boolean testProperty = false;

		public Boolean getTestProperty() {
			return testProperty;
		}

		public void setTestProperty(Boolean testProperty) {
			this.testProperty = testProperty;
		}
	}

Ha, I figure it out! If I set “writeThrough” mode of checkbox to false value change event is triggered only once.


                        .........
                         public Field createField(Container container, Object itemId,
					Object propertyId, Component uiContext) {
				CheckBox checkBox = new CheckBox();
				checkBox.setImmediate(true);
				checkBox.setWriteThrough(false);
				checkBox.addListener(new ValueChangeListener() {
					
					@Override
					public void valueChange(ValueChangeEvent event) {
						//log.info("value changed...");
						System.out.println("value changed...");
					}
				});
				return checkBox;
			}
                         .........

But I don’t understand logic behind this concept. Can anybody explain me why field’s should have writeThrough mode set to false if you want value change event to work as expected (to be triggered only once per value change)?

The multiple value change events can be considered a bug, and are tracked in
ticket 4394
.

The read-through and write-through properties control buffering in fields. Their effects are roughly as follows:

If a field that has a property data source is in write-through mode, any changes made (from the UI or from code calling myfield.setValue(…)) to its value are immediately sent to the property.

In some situations, it might be preferable that the field simply buffers changes to its value, and only writes them to its property data source when commit() is called. To achieve this, write-through should be off.

Read-through controls whether the value of the field is updated from the data source whenever it is requested (unless it has been modified directly in the field). This has side-effects - the value of the field can change when its value is requested. However, this setting can be useful if the data behind the property might have been updated directly without triggering a notification from the property.

See the javadoc for the interface
Buffered
for more information.

Is it possible that this bug is appeared again in Vaadin 6.5.6?

I’m going crazy

it cames only with

form.setWriteThrough(true);

otherwise only once is called

I found what was wrong.

I was adding ValueChangeListener on FormFieldFactory like this:

    form.setFormFieldFactory(new MyFieldFactory() {
      @Override
      public Field createField(final Item item, final Object propertyId, final Component uiContext) {
        Field field = super.createField(item, propertyId, uiContext);

        if ("myFieldWithListener".equals(propertyId)) field.addListener(fieldValueChangeListener);
        ....
        ....
        ....

then I do:

    form.setItemDataSource(new BeanItem<MyBean>(myBeanInstance));
    form.setVisibleItemProperties(new String[] { "prop1", "prop2", "myFieldWithListener" });

Doing like this my FieldFactory was called twice so the ValueChangeListener, and when the field changes the listener was called twice.

My solution is simply replacing the code above with:

   form.setItemDataSource(new BeanItem<MyBean>(myBeanInstance), Arrays.asList(new String[] { "prop1", "prop2", "myFieldWithListener"  }));

Now it works, but i’m asking, is it correct that the first solution call the FieldFactory twice?

Note: it seems that CustomField does not has a patch with field suppressValueChangePropagation as tracked on
ticket 4394.
while I think it should has.

Regards