Editor framework should generate specific code for primitive values to handle nulls and report errors

[dankurka]

Originally reported on Google Code with ID 7783

Issue Report Summary: Editor framework raises an NullPointerException when ValueBox
of primitive types are used as properties on a object being edited.

Found in GWT Release (e.g. 1.5.3, 1.6 RC):

GWT 2.5

Encountered on OS / Browser (e.g. WinXP, IE6-7 | Fedora 10, FF3):

Windows 7 x64, Chrome, Firefox. Hosted mode and Super dev mode

Detailed description (please be as specific as possible):

When use a DoubleBox in a Editor to edit a double property, if user clear the value
of the box a null pointer exception is raised.

Example:

public double getTasaImpuesto();

<g:DoubleBox ui:field="tasaImpuesto"></g:DoubleBox>
@UiField DoubleBox tasaImpuesto;

If we call the driver.flush() in this scenarios:

Diferent value than number we get on the List<EditorError> the bad value.

Leave blank the field raises a null pointer exception.

The line i get my atention is this one:

Caused by: com.google.gwt.core.client.JavaScriptException: (TypeError) : Cannot read
property 'value_0' of null
    at Unknown.$doubleValue(http://localhost:9876/proj/C6E66C9FC3DCB1FC08DFFFC07FE049E0.cache.js@21:38447)

Seems that DoubleBox cannot handle the null value when we use primity types.

You can reproduce this issue by doing the following:

1) Create a POJO with a double property.

2) Create a editor of this POJO and then use a DoubleBox to edit that property.

3) On the Double box clear all text and then flush the driver.

You will get a NPE.

I think would be nice if the editor generate some code to handle this case and report
like when the user use some bad input and then this can be reported in to the getErrors()
so the NPE wont be raised.

Like when you use bad values
(1a) Bad value(1a)
(empty) bad value(null)

Workaround if you have one:

Dont't use primitive types on the POJOS and use wrapper Clasess Double, Integer, ...

Links to relevant GWT Developer Forum posts:

https://groups.google.com/forum/?fromgroups=#!topic/google-web-toolkit/IiPJhHcDajU%5B1-25%5D

Reported by ceo.lion.dev on 2012-11-12 04:48:30

[dankurka]

Wondering if we shouldn't instead add an option to DoubleBox and the like to return
a specific value instead of null when the field is empty.

Or use the default value for primitives (i.e. 0.0 for a double) when the editor returns
null, rather trying to unbox it (and instead of reporting an error as proposed above;
if people want a different behavior, then they can use another editor than DoubleBox
–possibly extending DoubleBox and overriding getValueOrThrow–, or wrap DoubleBox inside
an editor of their own).

Reported by t.broyer on 2012-11-12 08:37:34

• Status changed: Accepted

[dankurka]

I think the default value 0.0 for DoubleBox and 0 for IntegerBox should be fine. Since
we can use JSR303 validation now it's easy to validate non 0 values.

Reported by ceo.lion.dev on 2012-11-12 18:21:56

[dankurka]

Do you mind proposing a patch, see: http://www.gwtproject.org/makinggwtbetter.html

Reported by dankurka@google.com on 2013-05-27 19:36:37

• Status changed: NeedsInfo

[dankurka]

This issue renders the validation framework useless. At least the exception should be
caught and passed down as some violation that can be shown to the user.

I am experiencing the problem with IntegerBox when the field is empty or contains letters.

Reported by robert.hoffmann.phd on 2014-09-13 18:07:56

[dankurka]

That's what I learned and how I solved the issue for me... maybe useful to others:

1) The  problem:

If IntegerBox contains non-digits, then the ValueBoxEditor passes an error to a delegate
(EditorDelegate). The reason is that there is no Integer.NOT_AVAILABLE (or similar)
it could pass to the validator.
>>
ValueBoxEditor
...
  @Override
  public T getValue() {
    try {
      value = peer.getValueOrThrow();
    } catch (ParseException e) {
      // TODO i18n
      getDelegate().recordError("Bad value (" + peer.getText() + ")",
          peer.getText(), e);
    }
    return value;
  }
<<

2) The ValueBoxEditorDecorator is used to display error messages to the user. However
it does not pass an EditorDelegate to the Editor it wraps. So getDelegate().recordError(..)
from above fails.

3) ValueBoxEditorDecorator is very limited because it only handles ValueBoxEditor,
so I created my own that would accept TakesValueEditor and thus be more general. Then
I pass the EditorDelegate to the wrapped Editor with:
>>
if(editor instanceof HasEditorDelegate) {
 ((HasEditorDelegate<T>) editor).setDelegate(delegate);
}
<< 

4) Finally, showing the error message to the user. Here I learned that order matters:

>>
Validator validator = Validation.buildDefaultValidatorFactory().getValidator();
Set<?> violations = validator.validate(settings);
driver.setConstraintViolations((Set<ConstraintViolation<?>>) violations);
<<

...this shows validation errors in the ValueDecorator (see 3) but clears error messages
that have been shown during a call to driver.flush() 

5) The error message in ValueBoxEditor is hardcoded (see TODO i18n above in under 3)
and I do not know how to fix this. So I just wrapped the EditorError messages to change
the message for something I can control and feed them directly to the Editor referenced
by errors:
>>
                if(e.getEditor() instanceof HasEditorErrors) {
                    HasEditorErrors hasEditorErrors= (HasEditorErrors) e.getEditor();
                    hasEditorErrors.showErrors(nes);
                }
<<

Reported by robert.hoffmann.phd on 2014-09-16 12:47:02

[dankurka]

Note: there's no need to "pass an EditorDelegate to the Editor it wraps"; if it implements
HasEditorDelegate then the Editor framework will call setDelegate on it. If that's
not the case, then it's a bug.

Reported by t.broyer on 2014-09-18 10:13:14

[dankurka]

Thomas, you are right. The problem is that com.google.gwt.editor.ui.client.ValueBoxEditorDecorator
does not implement HasEditorDelegate:

See ValueBoxEditorDecorator:
>>
  /**
   * Set the widget that the EditorPanel will display. This method will
   * automatically call {@link #setEditor}.
   * 
   * @param widget a {@link ValueBoxBase} widget
   */
  @UiChild(limit = 1, tagname = "valuebox")
  public void setValueBox(ValueBoxBase<T> widget) {
    contents.add(widget);
    setEditor(widget.asEditor());
  }
<<

In the browser this leads then to a NullPointer
>>
Thu Sep 18 12:48:41 GMT+200 2014 com.apsburg.gwt.prototype.gwt.main.client.AppEntryPointDesktop
SEVERE: Uncaught exceptioncom.google.gwt.event.shared.UmbrellaException: Exception
caught: (TypeError) 
 __gwt$exception: <skipped>: Cannot read property 'recordError' of undefined
<<

when getDelegate().recordError(...) is called.

In conclusion, ValueBoxEditorDecorator is probably not production ready.

I am using a derivate:
>>
public class InputValueDecorator<T> extends Composite implements
        HasEditorErrors<T>, IsEditor<TakesValueEditor<T>>, HasWidgets, HasEditorDelegate<T>
<<

which is more general in that it can handle other Input elements apart from ValueBox
and passes the delegator down to the editor it wraps. See the source code in my next
post.

Reported by robert.hoffmann.phd on 2014-09-18 11:08:41

[dankurka]

import com.google.gwt.core.client.GWT;
import com.google.gwt.dom.client.DivElement;
import com.google.gwt.dom.client.Style.Display;
import com.google.gwt.editor.client.*;
import com.google.gwt.editor.client.adapters.TakesValueEditor;
import com.google.gwt.resources.client.ClientBundle;
import com.google.gwt.resources.client.GssResource;
import com.google.gwt.uibinder.client.UiBinder;
import com.google.gwt.uibinder.client.UiConstructor;
import com.google.gwt.uibinder.client.UiField;
import com.google.gwt.user.client.ui.*;

import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.logging.Logger;

public class InputValueDecorator<T> extends Composite implements
        HasEditorErrors<T>, IsEditor<TakesValueEditor<T>>, HasWidgets, HasEditorDelegate<T>
{

    private List<EditorDelegate<T>> delegates=new ArrayList<EditorDelegate<T>>();

    private Logger log = Logger.getLogger(this.getClass().getName());

    @UiField
    SimplePanel contents;

    @UiField
    DivElement errorLabel;

    private TakesValueEditor<T> editor;
    @SuppressWarnings("UnusedDeclaration")
    private ResourceBundle.Gss gss;

    public void setDelegate(EditorDelegate<T> delegate) {
        delegates.add(delegate);
        //
        setDelegate(editor);
    }

    private void setDelegate(TakesValueEditor<T> editor) {

        if(delegates.size()>0 && editor!=null) {

            //
            if(editor instanceof HasEditorDelegate) {

                for(EditorDelegate<T> delegate : delegates) {
                    //noinspection unchecked
                    ((HasEditorDelegate<T>) editor).setDelegate(delegate);
                }
                //
                delegates.clear();
            }
        }
    }

    public interface ResourceBundle extends ClientBundle {

        public static final ResourceBundle INSTANCE = GWT.create(ResourceBundle.class);

        public static final String DEFAULT_GSS = "com/apsburg/gwt/lib/client/ui/desktop/widget/InputValueDecorator.gss";

        // !!! Note: workaround to make Intellij find/consider the gss
        @Source({DEFAULT_GSS, "InputValueDecorator.gss"})
        // @GssResource.NotStrict
        public Gss gss();

        public interface Gss extends GssResource {

            String InputValueDecoratorContent();

            String InputValueDecoratorError();
        }
    }

    public void add(Widget w) {
        if (contents.getWidget() != null) {
            throw new IllegalStateException("SimplePanel can only contain one child
widget");
        }

        //
        contents.add(w);
        //noinspection unchecked

        setEditor(((IsEditor<TakesValueEditor<T>>) w).asEditor());
    }

    public void clear() {

    }

    public Iterator<Widget> iterator() {
        return null;
    }

    public boolean remove(Widget w) {
        return false;
    }

    interface Binder extends UiBinder<Widget, InputValueDecorator<?>> {
        Binder BINDER = GWT.create(Binder.class);
    }

    /**
     * Constructs a ValueBoxEditorDecorator.
     */
    @UiConstructor
    public InputValueDecorator() {
        this(null);
    }

    public InputValueDecorator(ResourceBundle.Gss gss) {

        if (gss == null) gss = ResourceBundle.INSTANCE.gss();
        gss.ensureInjected();
        this.gss = gss;

        //
        initWidget(Binder.BINDER.createAndBindUi(this));

        //
        errorLabel.getStyle().setDisplay(Display.NONE);
    }

    /**
     * Constructs a ValueBoxEditorDecorator using a {@link com.google.gwt.user.client.ui.HasValue}
     * widget and a {@link com.google.gwt.editor.ui.client.adapters.ValueBoxEditor}
editor.
     *
     * @param widget the widget
     * @param editor the editor
     */
    @SuppressWarnings("UnusedDeclaration")
    public InputValueDecorator(IsWidget widget,
                          TakesValueEditor<T> editor) {
        this();
        contents.add(widget);
        //
        setEditor(editor);
    }

    /**
     * Returns the associated {@link com.google.gwt.editor.ui.client.adapters.ValueBoxEditor}.
     *
     * @return a {@link com.google.gwt.editor.ui.client.adapters.ValueBoxEditor} instance
     * @see #setEditor(TakesValueEditor)
     */
    public TakesValueEditor<T> asEditor() {
        return editor;
    }

    /**
     * Sets the associated {@link com.google.gwt.editor.ui.client.adapters.ValueBoxEditor}.
     *
     * @param editor a {@link com.google.gwt.editor.ui.client.adapters.ValueBoxEditor}
instance
     * @see #asEditor()
     */
    public void setEditor(TakesValueEditor<T> editor) {
        this.editor = editor;
        setDelegate(this.editor);
    }

    /**
     * Set the widget that the EditorPanel will display. This method will
     * automatically call {@link #setEditor}.
     *
     * @param widget a {@link com.google.gwt.user.client.ui.HasValue} widget
     */
/*
    @UiChild(limit = 1, tagname = "value")
    public <S extends IsWidget & IsEditor<TakesValueEditor<T>>>void add(S widget) {
        contents.add(widget);
        setEditor(widget.asEditor());
    }
*/

    /**
     * The default implementation will display, but not consume, received errors
     * whose {@link com.google.gwt.editor.client.EditorError#getEditor() getEditor()}
method returns the Editor
     * passed into {@link #setEditor}.
     *
     * @param errors a List of {@link com.google.gwt.editor.client.EditorError} instances
     */
    public void showErrors(List<EditorError> errors) {
        StringBuilder sb = new StringBuilder();
        for (EditorError error : errors) {

            // !!! RH: always false: if (error.getEditor().equals(editor))
            sb.append("\n").append(error.getMessage());
        }

        if (sb.length() == 0) {
            errorLabel.setInnerText("");
            errorLabel.getStyle().setDisplay(Display.NONE);
            return;
        }

        errorLabel.setInnerText(sb.substring(1));
        errorLabel.getStyle().setDisplay(Display.INLINE_BLOCK);
    }
}

Reported by robert.hoffmann.phd on 2014-09-18 11:10:02

[dankurka]

ValueBoxEditorDecorator does not need to implement HasEditorDelegate; the wrapped editor
needs to, to be able to report errors, and ValueBoxEditor implements it.

Use the EditorHierarchyPrinter (or just call toString() on the EditorDriver in DevMode)
to debug your editor hierarchy.

Also, let's discuss this on the forum if you don't mind (particularly as it's not directly
related to the bug initially reported here).

Reported by t.broyer on 2014-09-22 09:13:52