Fix TreeTable declarative support (#16368)

Also fix some small issues in Table declarative:
* Write null property values as empty strings instead of NPEing
* Read/write item ids from/to <tr item-id="...">

Change-Id: Ieccc3f49c5021f8a4a50d4ea671f9086ad8f997c

4 files changed, 127 insertions, 25 deletions

diff --git a/server/src/com/vaadin/ui/AbstractSelect.java b/server/src/com/vaadin/ui/AbstractSelect.java
index 0e3cfef4ce..237cdbddb6 100644
--- a/server/src/com/vaadin/ui/AbstractSelect.java
+++ b/server/src/com/vaadin/ui/AbstractSelect.java
@@ -2231,7 +2231,7 @@ public abstract class AbstractSelect extends AbstractField<Object> implements
      *             if the tag name of the {@code child} element is not
      *             {@code option}.
      */
-    protected String readItem(Element child, Set<String> selected,
+    protected Object readItem(Element child, Set<String> selected,
             DesignContext context) {
         if (!"option".equals(child.tagName())) {
             throw new DesignException("Unrecognized child element in "
diff --git a/server/src/com/vaadin/ui/OptionGroup.java b/server/src/com/vaadin/ui/OptionGroup.java
index ff2e384285..10c263aca0 100644
--- a/server/src/com/vaadin/ui/OptionGroup.java
+++ b/server/src/com/vaadin/ui/OptionGroup.java
@@ -257,9 +257,9 @@ public class OptionGroup extends AbstractSelect implements
     }
 
     @Override
-    protected String readItem(Element child, Set<String> selected,
+    protected Object readItem(Element child, Set<String> selected,
             DesignContext context) {
-        String itemId = super.readItem(child, selected, context);
+        Object itemId = super.readItem(child, selected, context);
 
         if (child.hasAttr("disabled")) {
             setItemEnabled(itemId, false);
diff --git a/server/src/com/vaadin/ui/Table.java b/server/src/com/vaadin/ui/Table.java
index eb3d35be3e..cb61fa31ec 100644
--- a/server/src/com/vaadin/ui/Table.java
+++ b/server/src/com/vaadin/ui/Table.java
@@ -6042,7 +6042,7 @@ public class Table extends AbstractSelect implements Action.Container,
 
         readColumns(design);
         readHeader(design);
-        readBody(design);
+        readBody(design, context);
         readFooter(design);
     }
 
@@ -6150,31 +6150,44 @@ public class Table extends AbstractSelect implements Action.Container,
         }
     }
 
-    private void readBody(Element design) {
+    protected void readBody(Element design, DesignContext context) {
         Element tbody = design.select("> table > tbody").first();
-        if (tbody != null) {
-            for (Element row : tbody.children()) {
-                Elements cells = row.children();
-                if (visibleColumns.size() != cells.size()) {
-                    throw new DesignException(
-                            "Wrong number of columns in a row of a Table. Expected "
-                                    + visibleColumns.size() + ", was "
-                                    + cells.size() + ".");
-                }
-                Object[] data = new String[cells.size()];
-                for (int c = 0; c < cells.size(); ++c) {
-                    data[c] = cells.get(c).html();
-                }
-                Object itemId = addItem(data, null);
-                if (itemId == null) {
-                    throw new DesignException(
-                            "A row of a Table could not be read");
-                }
-            }
+        if (tbody == null) {
+            return;
+        }
+
+        Set<String> selected = new HashSet<String>();
+        for (Element tr : tbody.children()) {
+            readItem(tr, selected, context);
         }
     }
 
     @Override
+    protected Object readItem(Element tr, Set<String> selected,
+            DesignContext context) {
+        Elements cells = tr.children();
+        if (visibleColumns.size() != cells.size()) {
+            throw new DesignException(
+                    "Wrong number of columns in a Table row. Expected "
+                            + visibleColumns.size() + ", was " + cells.size()
+                            + ".");
+        }
+        Object[] data = new String[cells.size()];
+        for (int c = 0; c < cells.size(); ++c) {
+            data[c] = cells.get(c).html();
+        }
+
+        Object itemId = addItem(data,
+                tr.hasAttr("item-id") ? tr.attr("item-id") : null);
+
+        if (itemId == null) {
+            throw new DesignException("Failed to add a Table row: " + data);
+        }
+
+        return itemId;
+    }
+
+    @Override
     public void writeDesign(Element design, DesignContext context) {
         Table def = context.getDefaultInstance(this);
 
@@ -6260,6 +6273,9 @@ public class Table extends AbstractSelect implements Action.Container,
 
     @Override
     protected void writeItems(Element design, DesignContext context) {
+        if (getVisibleColumns().length == 0) {
+            return;
+        }
         Element tbody = design.child(0).appendElement("tbody");
         super.writeItems(tbody, context);
     }
@@ -6268,10 +6284,12 @@ public class Table extends AbstractSelect implements Action.Container,
     protected Element writeItem(Element tbody, Object itemId,
             DesignContext context) {
         Element tr = tbody.appendElement("tr");
+        tr.attr("item-id", String.valueOf(itemId));
         Item item = getItem(itemId);
         for (Object id : getVisibleColumns()) {
             Element td = tr.appendElement("td");
-            td.html(item.getItemProperty(id).getValue().toString());
+            Object value = item.getItemProperty(id).getValue();
+            td.html(value != null ? value.toString() : "");
         }
         return tr;
     }
diff --git a/server/src/com/vaadin/ui/TreeTable.java b/server/src/com/vaadin/ui/TreeTable.java
index 63b54a6ced..254d8774a9 100644
--- a/server/src/com/vaadin/ui/TreeTable.java
+++ b/server/src/com/vaadin/ui/TreeTable.java
@@ -23,9 +23,13 @@ import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
+import java.util.Stack;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import org.jsoup.nodes.Element;
+
 import com.vaadin.data.Collapsible;
 import com.vaadin.data.Container;
 import com.vaadin.data.Container.Hierarchical;
@@ -41,6 +45,9 @@ import com.vaadin.ui.Tree.CollapseEvent;
 import com.vaadin.ui.Tree.CollapseListener;
 import com.vaadin.ui.Tree.ExpandEvent;
 import com.vaadin.ui.Tree.ExpandListener;
+import com.vaadin.ui.declarative.DesignAttributeHandler;
+import com.vaadin.ui.declarative.DesignContext;
+import com.vaadin.ui.declarative.DesignException;
 
 /**
  * TreeTable extends the {@link Table} component so that it can also visualize a
@@ -888,4 +895,81 @@ public class TreeTable extends Table implements Hierarchical {
         }
         return itemIds;
     }
+
+    @Override
+    protected void readBody(Element design, DesignContext context) {
+        Element tbody = design.select("> table > tbody").first();
+        if (tbody == null) {
+            return;
+        }
+
+        Set<String> selected = new HashSet<String>();
+        Stack<Object> parents = new Stack<Object>();
+        int lastDepth = -1;
+
+        for (Element tr : tbody.children()) {
+            int depth = DesignAttributeHandler.readAttribute("depth",
+                    tr.attributes(), 0, int.class);
+
+            if (depth < 0 || depth > lastDepth + 1) {
+                throw new DesignException(
+                        "Malformed TreeTable item hierarchy at " + tr
+                                + ": last depth was " + lastDepth);
+            } else if (depth <= lastDepth) {
+                for (int d = depth; d <= lastDepth; d++) {
+                    parents.pop();
+                }
+            }
+
+            Object itemId = readItem(tr, selected, context);
+            setParent(itemId, !parents.isEmpty() ? parents.peek() : null);
+            parents.push(itemId);
+            lastDepth = depth;
+        }
+    }
+
+    @Override
+    protected Object readItem(Element tr, Set<String> selected,
+            DesignContext context) {
+        Object itemId = super.readItem(tr, selected, context);
+
+        if (tr.hasAttr("collapsed")) {
+            boolean collapsed = DesignAttributeHandler.readAttribute(
+                    "collapsed", tr.attributes(), boolean.class);
+            setCollapsed(itemId, collapsed);
+        }
+
+        return itemId;
+    }
+
+    @Override
+    protected void writeItems(Element design, DesignContext context) {
+        if (getVisibleColumns().length == 0) {
+            return;
+        }
+        Element tbody = design.child(0).appendElement("tbody");
+        writeItems(tbody, rootItemIds(), 0, context);
+    }
+
+    protected void writeItems(Element tbody, Collection<?> itemIds, int depth,
+            DesignContext context) {
+        for (Object itemId : itemIds) {
+            Element tr = writeItem(tbody, itemId, context);
+            DesignAttributeHandler.writeAttribute("depth", tr.attributes(),
+                    depth, 0, int.class);
+
+            if (getChildren(itemId) != null) {
+                writeItems(tbody, getChildren(itemId), depth + 1, context);
+            }
+        }
+    }
+
+    @Override
+    protected Element writeItem(Element tbody, Object itemId,
+            DesignContext context) {
+        Element tr = super.writeItem(tbody, itemId, context);
+        DesignAttributeHandler.writeAttribute("collapsed", tr.attributes(),
+                isCollapsed(itemId), true, boolean.class);
+        return tr;
+    }
 }