Fix NPE in case some items don't contain all properties of Grid.

This could occur in when parent is a different entity than its children
in hierarchical data.

Change-Id: Icd53b5b5e5544a3680d0cd99702ab78224b2dc08

server/src/main/java/com/vaadin/data/fieldgroup/FieldGroup.java

@@ -101,6 +101,13 @@ public class FieldGroup implements Serializable {
     public void setItemDataSource(Item itemDataSource) {
         this.itemDataSource = itemDataSource;
 
+        bindFields();
+    }
+
+    /**
+     * Binds all fields to the properties in the item in use.
+     */
+    protected void bindFields() {
         for (Field<?> f : fieldToPropertyId.keySet()) {
             bind(f, fieldToPropertyId.get(f));
         }
@@ -256,20 +263,7 @@ public class FieldGroup implements Serializable {
         fieldToPropertyId.put(field, propertyId);
         propertyIdToField.put(propertyId, field);
         if (itemDataSource == null) {
-            // Clear any possible existing binding to clear the field
-            field.setPropertyDataSource(null);
-            boolean fieldReadOnly = field.isReadOnly();
-            if (!fieldReadOnly) {
-                field.clear();
-            } else {
-                // Temporarily make the field read-write so we can clear the
-                // value. Needed because setPropertyDataSource(null) does not
-                // currently clear the field
-                // (https://dev.vaadin.com/ticket/14733)
-                field.setReadOnly(false);
-                field.clear();
-                field.setReadOnly(true);
-            }
+            clearField(field);
 
             // Will be bound when data source is set
             return;
@@ -280,6 +274,29 @@ public class FieldGroup implements Serializable {
         configureField(field);
     }
 
+    /**
+     * Clears field and any possible existing binding.
+     *
+     * @param field
+     *         The field to be cleared
+     */
+    protected void clearField(Field<?> field) {
+        // Clear any possible existing binding to clear the field
+        field.setPropertyDataSource(null);
+        boolean fieldReadOnly = field.isReadOnly();
+        if (!fieldReadOnly) {
+            field.clear();
+        } else {
+            // Temporarily make the field read-write so we can clear the
+            // value. Needed because setPropertyDataSource(null) does not
+            // currently clear the field
+            // (https://dev.vaadin.com/ticket/14733)
+            field.setReadOnly(false);
+            field.clear();
+            field.setReadOnly(true);
+        }
+    }
+
     /**
      * Wrap property to transactional property.
      */

server/src/main/java/com/vaadin/ui/Grid.java

@@ -104,7 +104,6 @@ import com.vaadin.shared.ui.grid.selection.MultiSelectionModelState;
 import com.vaadin.shared.ui.grid.selection.SingleSelectionModelServerRpc;
 import com.vaadin.shared.ui.grid.selection.SingleSelectionModelState;
 import com.vaadin.shared.util.SharedUtil;
-import com.vaadin.ui.Grid.SelectionModel;
 import com.vaadin.ui.declarative.DesignAttributeHandler;
 import com.vaadin.ui.declarative.DesignContext;
 import com.vaadin.ui.declarative.DesignException;
@@ -548,6 +547,36 @@ public class Grid extends AbstractFocusable implements SelectionNotifier,
             }
             return field;
         }
+
+        @Override
+        protected void bindFields() {
+            List<Field<?>> fields = new ArrayList<Field<?>>(getFields());
+            Item itemDataSource = getItemDataSource();
+
+            if (itemDataSource == null) {
+                unbindFields(fields);
+            } else {
+                bindFields(fields, itemDataSource);
+            }
+        }
+
+        private void unbindFields(List<Field<?>> fields) {
+            for (Field<?> field : fields) {
+                clearField(field);
+                unbind(field);
+                field.setParent(null);
+            }
+        }
+
+        private void bindFields(List<Field<?>> fields,
+                Item itemDataSource) {
+            for (Field<?> field : fields) {
+                if (itemDataSource.getItemProperty(getPropertyId(field))
+                        != null) {
+                    bind(field, getPropertyId(field));
+                }
+            }
+        }
     }
 
     /**
@@ -2197,8 +2226,9 @@ public class Grid extends AbstractFocusable implements SelectionNotifier,
             Renderer<?> renderer = column.getRenderer();
 
             Item item = cell.getItem();
-            Object modelValue = item.getItemProperty(cell.getPropertyId())
-                    .getValue();
+            Property itemProperty = item.getItemProperty(cell.getPropertyId());
+            Object modelValue =
+                    itemProperty == null ? null : itemProperty.getValue();
 
             data.put(columnKeys.key(cell.getPropertyId()), AbstractRenderer
                     .encodeValue(modelValue, renderer, converter, getLocale()));
@@ -4521,6 +4551,12 @@ public class Grid extends AbstractFocusable implements SelectionNotifier,
     private boolean editorSaving = false;
     private FieldGroup editorFieldGroup = new CustomFieldGroup();
 
+    /**
+     * Poperty ID to Field mapping that stores editor fields set by {@link
+     * #setEditorField(Object, Field)}.
+     */
+    private Map<Object, Field<?>> editorFields = new HashMap<Object, Field<?>>();
+
     private CellStyleGenerator cellStyleGenerator;
     private RowStyleGenerator rowStyleGenerator;
 
@@ -6805,6 +6841,15 @@ public class Grid extends AbstractFocusable implements SelectionNotifier,
 
         Field<?> editor = editorFieldGroup.getField(propertyId);
 
+        // If field group has no field for this property, see if we have it stored
+        if (editor == null) {
+            editor = editorFields.get(propertyId);
+            if (editor != null) {
+                editorFieldGroup.bind(editor, propertyId);
+            }
+        }
+
+        // Otherwise try to build one
         try {
             if (editor == null) {
                 editor = editorFieldGroup.buildAndBind(propertyId);
@@ -6860,8 +6905,9 @@ public class Grid extends AbstractFocusable implements SelectionNotifier,
         editorFieldGroup.setItemDataSource(item);
 
         for (Column column : getColumns()) {
-            column.getState().editorConnector = getEditorField(
-                    column.getPropertyId());
+            column.getState().editorConnector =
+                    item.getItemProperty(column.getPropertyId()) == null
+                            ? null : getEditorField(column.getPropertyId());
         }
 
         editorActive = true;
@@ -6890,6 +6936,9 @@ public class Grid extends AbstractFocusable implements SelectionNotifier,
             field.setParent(this);
             editorFieldGroup.bind(field, propertyId);
         }
+
+        // Store field for this property for future reference
+        editorFields.put(propertyId, field);
     }
 
     /**

server/src/test/java/com/vaadin/tests/server/component/grid/GridEditorMissingPropertyTest.java

@@ -0,0 +1,318 @@
+package com.vaadin.tests.server.component.grid;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import com.vaadin.data.Item;
+import com.vaadin.data.Property;
+import com.vaadin.data.fieldgroup.FieldGroup;
+import com.vaadin.data.util.AbstractInMemoryContainer;
+import com.vaadin.data.util.BeanItem;
+import com.vaadin.ui.Field;
+import com.vaadin.ui.PasswordField;
+import com.vaadin.ui.TextField;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+public class GridEditorMissingPropertyTest {
+
+    private static final String PROPERTY_NAME = "name";
+    private static final String PROPERTY_SIZE = "size";
+
+    private static final String FOLDER_NAME_BEFORE = "Folder name";
+    private static final String FOLDER_NAME_AFTER = "Modified folder name";
+    private static final String FILE_NAME_BEFORE = "File name";
+    private static final String FILE_NAME_AFTER = "Modified file name";
+    private static final String FILE_SIZE_BEFORE = "10kB";
+    private static final String FILE_SIZE_AFTER = "20MB";
+
+    private final Grid grid = new Grid();
+
+    // Test items
+    private final Folder folder = new Folder(FOLDER_NAME_BEFORE);
+    private final File file = new File(FILE_NAME_BEFORE, FILE_SIZE_BEFORE);
+
+    @Before
+    public void setup() throws SecurityException, NoSuchMethodException  {
+        final BeanItem<Entry> folderItem = new BeanItem<Entry>(folder);
+        final BeanItem<Entry> childItem = new BeanItem<Entry>(file);
+
+        @SuppressWarnings("unchecked")
+        TestContainer container = new TestContainer(
+                Arrays.asList(folderItem, childItem),
+                Arrays.asList(PROPERTY_NAME, PROPERTY_SIZE));
+
+        grid.setContainerDataSource(container);
+        grid.setSelectionMode(Grid.SelectionMode.SINGLE);
+        grid.setEditorEnabled(true);
+    }
+
+    @Test
+    public void testBindFields() {
+        FieldGroup fieldGroup = grid.getEditorFieldGroup();
+
+        // Item with incomplete property set
+        fieldGroup.setItemDataSource(
+                grid.getContainerDataSource().getItem(folder));
+        grid.getColumn(PROPERTY_NAME)
+                .getEditorField(); // called in grid.doEditItem
+        assertTrue("Properties in item should be bound",
+                fieldGroup.getBoundPropertyIds().contains(PROPERTY_NAME));
+        assertFalse("Properties not present in item should not be bound",
+                fieldGroup.getBoundPropertyIds().contains(PROPERTY_SIZE));
+        assertTrue("All of item's properties should be bound",
+                fieldGroup.getUnboundPropertyIds().isEmpty());
+
+        // Unbind all fields
+        fieldGroup.setItemDataSource(null);
+        assertTrue("No properties should be bound",
+                fieldGroup.getBoundPropertyIds().isEmpty());
+        assertTrue("No unbound properties should exist",
+                fieldGroup.getUnboundPropertyIds().isEmpty());
+
+        // Item with complete property set
+        fieldGroup
+                .setItemDataSource(grid.getContainerDataSource().getItem(file));
+        grid.getColumn(PROPERTY_NAME).getEditorField();
+        grid.getColumn(PROPERTY_SIZE).getEditorField();
+        assertTrue("Properties in item should be bound",
+                fieldGroup.getBoundPropertyIds().contains(PROPERTY_NAME));
+        assertTrue("Properties in item should be bound",
+                fieldGroup.getBoundPropertyIds().contains(PROPERTY_SIZE));
+        assertTrue("All of item's properties should be bound",
+                fieldGroup.getUnboundPropertyIds().isEmpty());
+
+        // Unbind all fields
+        fieldGroup.setItemDataSource(null);
+        assertTrue("No properties should be bound",
+                fieldGroup.getBoundPropertyIds().isEmpty());
+        assertTrue("No unbound properties should exist",
+                fieldGroup.getUnboundPropertyIds().isEmpty());
+    }
+
+    @Test
+    public void testSetEditorField() {
+        FieldGroup fieldGroup = grid.getEditorFieldGroup();
+        Field editorField = new PasswordField();
+
+        // Explicitly set editor field
+        fieldGroup.setItemDataSource(
+                grid.getContainerDataSource().getItem(folder));
+        grid.getColumn(PROPERTY_NAME).setEditorField(editorField);
+        assertTrue("Editor field should be the one that was previously set",
+                grid.getColumn(PROPERTY_NAME).getEditorField() == editorField);
+
+        // Reset item
+        fieldGroup.setItemDataSource(null);
+        fieldGroup
+                .setItemDataSource(grid.getContainerDataSource().getItem(file));
+        assertTrue("Editor field should be the one that was previously set",
+                grid.getColumn(PROPERTY_NAME).getEditorField() == editorField);
+    }
+
+    @Test
+    public void testEditCell() {
+        // Row with missing property
+        startEdit(folder);
+        assertEquals(folder, grid.getEditedItemId());
+        assertEquals(getEditedItem(),
+                grid.getEditorFieldGroup().getItemDataSource());
+
+        assertEquals(FOLDER_NAME_BEFORE,
+                grid.getColumn(PROPERTY_NAME).getEditorField().getValue());
+        try {
+            grid.getColumn(PROPERTY_SIZE).getEditorField();
+            fail("Grid.editorFieldGroup should throw BindException by default");
+        } catch (FieldGroup.BindException e) {
+            // BindException is thrown using the default FieldGroup
+        }
+        grid.cancelEditor();
+
+        // Row with all properties
+        startEdit(file);
+        assertEquals(file, grid.getEditedItemId());
+        assertEquals(getEditedItem(),
+                grid.getEditorFieldGroup().getItemDataSource());
+
+        assertEquals(FILE_NAME_BEFORE,
+                grid.getColumn(PROPERTY_NAME).getEditorField().getValue());
+        assertEquals(FILE_SIZE_BEFORE,
+                grid.getColumn(PROPERTY_SIZE).getEditorField().getValue());
+        grid.cancelEditor();
+    }
+
+    @Test
+    public void testCancelEditor() {
+        // Row with all properties
+        testCancel(file, PROPERTY_NAME, FILE_NAME_BEFORE, FILE_NAME_AFTER);
+        testCancel(file, PROPERTY_SIZE, FILE_SIZE_BEFORE, FILE_SIZE_AFTER);
+
+        // Row with missing property
+        testCancel(folder, PROPERTY_NAME, FOLDER_NAME_BEFORE, FOLDER_NAME_AFTER);
+    }
+
+    private void testCancel(Object itemId, String propertyId,
+            String valueBefore, String valueAfter) {
+        startEdit(itemId);
+
+        TextField field = (TextField) grid.getColumn(propertyId)
+                .getEditorField();
+        field.setValue(valueAfter);
+
+        Property<?> datasource = field.getPropertyDataSource();
+
+        grid.cancelEditor();
+        assertFalse(grid.isEditorActive());
+        assertNull(grid.getEditedItemId());
+        assertFalse(field.isModified());
+        assertEquals("", field.getValue());
+        assertEquals(valueBefore, datasource.getValue());
+        assertNull(field.getPropertyDataSource());
+        assertNull(grid.getEditorFieldGroup().getItemDataSource());
+    }
+
+    @Test
+    public void testSaveEditor() throws Exception {
+        // Row with all properties
+        testSave(file, PROPERTY_SIZE, FILE_SIZE_BEFORE, FILE_SIZE_AFTER);
+
+        // Row with missing property
+        testSave(folder, PROPERTY_NAME, FOLDER_NAME_BEFORE, FOLDER_NAME_AFTER);
+    }
+
+    private void testSave(Object itemId, String propertyId, String valueBefore,
+            String valueAfter) throws Exception {
+        startEdit(itemId);
+        TextField field = (TextField) grid.getColumn(propertyId)
+                .getEditorField();
+
+        field.setValue(valueAfter);
+        assertEquals(valueBefore, field.getPropertyDataSource().getValue());
+
+        grid.saveEditor();
+        assertTrue(grid.isEditorActive());
+        assertFalse(field.isModified());
+        assertEquals(valueAfter, field.getValue());
+        assertEquals(valueAfter, getEditedProperty(propertyId).getValue());
+        grid.cancelEditor();
+    }
+
+    private Item getEditedItem() {
+        assertNotNull(grid.getEditedItemId());
+        return grid.getContainerDataSource().getItem(grid.getEditedItemId());
+    }
+
+    private Property<?> getEditedProperty(Object propertyId) {
+        return getEditedItem().getItemProperty(propertyId);
+    }
+
+    private void startEdit(Object itemId) {
+        grid.setEditorEnabled(true);
+        grid.editItem(itemId);
+        // Simulate succesful client response to actually start the editing.
+        grid.doEditItem();
+    }
+
+    private class TestContainer extends
+            AbstractInMemoryContainer<Object, String, BeanItem> {
+
+        private final List<BeanItem<Entry>> items;
+        private final List<String> pids;
+
+        public TestContainer(List<BeanItem<Entry>> items, List<String> pids) {
+            this.items = items;
+            this.pids = pids;
+        }
+
+        @Override
+        protected List<Object> getAllItemIds() {
+            List<Object> ids = new ArrayList<Object>();
+            for (BeanItem<Entry> item : items) {
+                ids.add(item.getBean());
+            }
+            return ids;
+        }
+
+        @Override
+        protected BeanItem<Entry> getUnfilteredItem(Object itemId) {
+            for (BeanItem<Entry> item : items) {
+                if (item.getBean().equals(itemId)) {
+                    return item;
+                }
+            }
+            return null;
+        }
+
+        @Override
+        public Collection<?> getContainerPropertyIds() {
+            return pids;
+        }
+
+        @Override
+        public Property getContainerProperty(Object itemId, Object propertyId) {
+            return getItem(itemId).getItemProperty(propertyId);
+        }
+
+        @Override
+        public Class<?> getType(Object propertyId) {
+            return String.class;
+        }
+    }
+
+    public class Entry {
+        private String name;
+
+        public String getName() {
+            return name;
+        }
+
+        public void setName(String name) {
+            this.name = name;
+        }
+
+        public Entry(String name) {
+            this.name = name;
+        }
+    }
+
+    public class Folder extends Entry {
+
+        public Folder(String name) {
+            super(name);
+        }
+    }
+
+    public class File extends Entry {
+        private String size;
+
+        public File(String name, String size) {
+            super(name);
+            this.size = size;
+        }
+
+        public String getSize() {
+            return size;
+        }
+
+        public void setSize(String size) {
+            this.size = size;
+        }
+    }
+
+    private class Grid extends com.vaadin.ui.Grid {
+        @Override
+        protected void doEditItem() {
+            super.doEditItem();
+        }
+    }
+}

uitest/src/main/java/com/vaadin/tests/components/grid/GridMissingProperty.java

@@ -0,0 +1,129 @@
+package com.vaadin.tests.components.grid;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+
+import com.vaadin.data.Property;
+import com.vaadin.data.util.AbstractInMemoryContainer;
+import com.vaadin.data.util.BeanItem;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Grid;
+
+public class GridMissingProperty extends AbstractTestUI {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        final Grid grid = new Grid();
+
+        final Folder folder = new Folder("Folder name");
+        final BeanItem<Entry> folderItem = new BeanItem<Entry>(folder);
+
+        final File file = new File("File name", "10kB");
+        final BeanItem<Entry> fileItem = new BeanItem<Entry>(file);
+
+        @SuppressWarnings("unchecked")
+        TestContainer container = new TestContainer(
+                Arrays.asList(folderItem, fileItem),
+                Arrays.asList("name", "size"));
+
+        grid.setContainerDataSource(container);
+        grid.setSelectionMode(Grid.SelectionMode.SINGLE);
+        grid.setEditorEnabled(true);
+
+        addComponent(grid);
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Grid Editor should not throw exception even when items are missing properties.";
+    }
+
+    private class TestContainer extends
+            AbstractInMemoryContainer<Object, String, BeanItem> {
+
+        private final List<BeanItem<Entry>> items;
+        private final List<String> pids;
+
+        public TestContainer(List<BeanItem<Entry>> items, List<String> pids) {
+            this.items = items;
+            this.pids = pids;
+        }
+
+        @Override
+        protected List<Object> getAllItemIds() {
+            List<Object> ids = new ArrayList<Object>();
+            for (BeanItem<Entry> item : items) {
+                ids.add(item.getBean());
+            }
+            return ids;
+        }
+
+        @Override
+        protected BeanItem<Entry> getUnfilteredItem(Object itemId) {
+            for (BeanItem<Entry> item : items) {
+                if (item.getBean().equals(itemId)) {
+                    return item;
+                }
+            }
+            return null;
+        }
+
+        @Override
+        public Collection<?> getContainerPropertyIds() {
+            return pids;
+        }
+
+        @Override
+        public Property getContainerProperty(Object itemId, Object propertyId) {
+            return getItem(itemId).getItemProperty(propertyId);
+        }
+
+        @Override
+        public Class<?> getType(Object propertyId) {
+            return String.class;
+        }
+    }
+
+    public class Entry {
+        private String name;
+
+        public String getName() {
+            return name;
+        }
+
+        public void setName(String name) {
+            this.name = name;
+        }
+
+        public Entry(String name) {
+            this.name = name;
+        }
+    }
+
+    public class Folder extends Entry {
+
+        public Folder(String name) {
+            super(name);
+        }
+    }
+
+    public class File extends Entry {
+        private String size;
+
+        public File(String name, String size) {
+            super(name);
+            this.size = size;
+        }
+
+        public String getSize() {
+            return size;
+        }
+
+        public void setSize(String size) {
+            this.size = size;
+        }
+    }
+}

uitest/src/test/java/com/vaadin/tests/components/grid/GridMissingPropertyTest.java

@@ -0,0 +1,73 @@
+package com.vaadin.tests.components.grid;
+
+import org.junit.Test;
+
+import com.vaadin.testbench.elements.GridElement;
+import com.vaadin.testbench.elements.GridElement.GridEditorElement;
+import com.vaadin.testbench.elements.TextFieldElement;
+import com.vaadin.tests.tb3.SingleBrowserTest;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class GridMissingPropertyTest extends SingleBrowserTest {
+
+    @Test
+    public void testCellEditable() {
+        openTestURL();
+        GridElement grid = $(GridElement.class).first();
+
+        // Row with missing property
+        grid.getCell(0, 0).doubleClick();
+        GridEditorElement editor = grid.getEditor();
+
+        assertTrue("Cell with property should be editable",
+                editor.isEditable(0));
+        assertFalse("Cell without property should not be editable",
+                editor.isEditable(1));
+
+        editor.cancel();
+
+        // Row with all properties
+        grid.getCell(1, 0).doubleClick();
+        editor = grid.getEditor();
+
+        assertTrue("Cell with property should be editable",
+                editor.isEditable(0));
+        assertTrue("Cell with property should be editable",
+                editor.isEditable(1));
+
+        editor.cancel();
+    }
+
+    @Test
+    public void testEditCell() {
+        openTestURL();
+        GridElement grid = $(GridElement.class).first();
+
+        GridEditorElement editor;
+        TextFieldElement editorField;
+
+        grid.getCell(0, 0).doubleClick();
+        editor = grid.getEditor();
+        editorField = editor.getField(0).wrap(TextFieldElement.class);
+        editorField.setValue("New Folder Name");
+        editor.save();
+        assertEquals("New Folder Name", grid.getCell(0, 0).getText());
+
+        grid.getCell(1, 0).doubleClick();
+        editor = grid.getEditor();
+        editorField = editor.getField(1).wrap(TextFieldElement.class);
+        editorField.setValue("10 MB");
+        editor.save();
+        assertEquals("10 MB", grid.getCell(1, 1).getText());
+
+        grid.getCell(1, 0).doubleClick();
+        editor = grid.getEditor();
+        editorField = editor.getField(0).wrap(TextFieldElement.class);
+        editorField.setValue("New File Name");
+        editor.save();
+        assertEquals("New File Name", grid.getCell(1, 0).getText());
+    }
+}