This is an automated email from the ASF dual-hosted git repository.

desruisseaux pushed a commit to branch geoapi-4.0
in repository https://gitbox.apache.org/repos/asf/sis.git

commit 771861937c67acac9f0372a0d97a4253a5f85a4f
Author: Martin Desruisseaux <martin.desruisse...@geomatys.com>
AuthorDate: Fri Nov 8 19:42:19 2019 +0100

    If a feature contains multi-valued properties, make possible to see their 
values on demand.
---
 .../org/apache/sis/gui/dataset/ExpandableList.java | 425 +++++++++++++++++++++
 .../apache/sis/gui/dataset/ExpandedFeature.java    | 214 +++++++++++
 .../org/apache/sis/gui/dataset/FeatureList.java    |  28 +-
 .../org/apache/sis/gui/dataset/FeatureTable.java   | 168 +++++++-
 .../sis/internal/gui/IdentityValueFactory.java     |  74 ++++
 .../java/org/apache/sis/internal/gui/Styles.java   |   5 +
 6 files changed, 887 insertions(+), 27 deletions(-)

diff --git 
a/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ExpandableList.java
 
b/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ExpandableList.java
new file mode 100644
index 0000000..3d07ca8
--- /dev/null
+++ 
b/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ExpandableList.java
@@ -0,0 +1,425 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.sis.gui.dataset;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.LinkedHashMap;
+import java.util.Collection;
+import javafx.util.Callback;
+import javafx.event.EventHandler;
+import javafx.scene.input.MouseEvent;
+import javafx.scene.control.TableCell;
+import javafx.scene.control.TableColumn;
+import javafx.scene.text.TextAlignment;
+import javafx.collections.ListChangeListener;
+import javafx.collections.transformation.TransformationList;
+import javafx.scene.layout.Background;
+import javafx.scene.layout.BackgroundFill;
+import org.apache.sis.internal.util.UnmodifiableArrayList;
+import org.apache.sis.internal.gui.Styles;
+import org.opengis.feature.FeatureType;
+import org.opengis.feature.Feature;
+
+
+/**
+ * Wraps a {@link FeatureList} with the capability to expand the multi-valued 
properties of
+ * a selected {@link Feature}. The expansion appears as additional rows below 
the feature.
+ * This view is used only if the feature type contains at least one property 
type with a
+ * maximum number of occurrence greater than 1.
+ *
+ * @author  Martin Desruisseaux (Geomatys)
+ * @version 1.1
+ * @since   1.1
+ * @module
+ */
+final class ExpandableList extends TransformationList<Feature,Feature>
+        implements Callback<TableColumn<Feature,Feature>, 
TableCell<Feature,Feature>>,
+                   EventHandler<MouseEvent>
+{
+    /**
+     * The background of {@linkplain #expansion} rows header.
+     */
+    private static final Background EXPANSION_HEADER =
+            new Background(new BackgroundFill(Styles.EXPANDED_ROW, null, 
null));
+
+    /**
+     * The icon for rows that can be expanded.
+     * Set to the Unicode supplementary character U+1F5C7 "Empty Note Pad".
+     */
+    private static final String EXPANDABLE = "\uD83D\uDDC7";
+
+    /**
+     * The icon for rows that are expanded.
+     * Set to the Unicode supplementary character U+1F5CA "Note Pad".
+     */
+    private static final String EXPANDED = "\uD83D\uDDCA";
+
+    /**
+     * Mapping from property names to index in the {@link 
ExpandedFeature#values} array.
+     * This map is shared by all {@link ExpandedFeature} instances contained 
in this list.
+     * It shall be modified only if this list has been cleared first.
+     */
+    private final Map<String,Integer> nameToIndex;
+
+    /**
+     * View index of the first row of the currently expanded feature, {@link 
Integer#MAX_VALUE} if none.
+     * Expansion rows will be added or removed <em>below</em> this row; this 
row itself will not be removed.
+     */
+    private int indexOfExpanded;
+
+    /**
+     * The rows of the expanded feature, or {@code null} if none. If non-null, 
this array shall never be empty.
+     * The reason why we do not allow empty arrays is because we will insert 
{@code expansion.length - 1} rows
+     * below the expanded feature. Consequently empty arrays cause negative 
indices that are more difficult to
+     * debug than {@link NullPointerException}, because they happen later.
+     *
+     * <p>If non-null, they array should have at least 2 elements. An array of 
only 1 element is not wrong,
+     * but is useless since it has no additional rows to show below the first 
row.</p>
+     */
+    private ExpandedFeature[] expansion;
+
+    /**
+     * Creates a new expandable list for the given features.
+     *
+     * @param  features  the {@link FeatureList} list to wrap.
+     */
+    ExpandableList(final FeatureList features) {
+        super(features);
+        nameToIndex     = new LinkedHashMap<>();
+        indexOfExpanded = Integer.MAX_VALUE;
+    }
+
+    /**
+     * Specifies the names of properties that may be multi-valued. This method 
needs to be invoked
+     * only if the {@link FeatureType} changed. This method shall not be 
invoked if there is any
+     * {@link #expansion} rows. Normally this list will be empty at invocation 
time.
+     *
+     * @param  columnNames  names of properties that may contain multi-values.
+     */
+    final void setMultivaluedColumns(final List<String> columnNames) {
+        assert expansion == null : indexOfExpanded;
+        nameToIndex.clear();
+        final int size = columnNames.size();
+        for (int i=0; i<size; i++) {
+            nameToIndex.putIfAbsent(columnNames.get(i), i);
+        }
+    }
+
+    /**
+     * Removes the expanded rows. This method does not fire change event;
+     * it is caller's responsibility to perform those tasks.
+     *
+     * <div class="note"><b>Note:</b> we return {@code null} instead than an 
empty list if
+     * there is no removed element because we want to force callers to perform 
a null check.
+     * The reason is that if there was no expansion rows, then {@link 
#indexOfExpanded} has an
+     * invalid value and using that value in {@link #nextRemove(int, List)} 
may be dangerous.
+     * A {@link NullPointerException} would intercept that error sooner.</div>
+     *
+     * @return the removed rows, or {@code null} if none.
+     */
+    private List<Feature> shrink() {
+        final List<Feature> removed = (expansion == null) ? null
+                                    : UnmodifiableArrayList.wrap(expansion, 1, 
expansion.length);
+        expansion       = null;
+        indexOfExpanded = Integer.MAX_VALUE;
+        return removed;
+    }
+
+    /**
+     * Clears all elements from this list. This method removes the expanded 
rows before to
+     * remove the rest of the list because otherwise, the {@code 
sourceChanged(…)} method in
+     * this class would have to expand the whole feature list for inserting 
removed elements.
+     */
+    @Override
+    public void clear() {
+        final int removeAfter = indexOfExpanded;
+        final List<Feature> removed = shrink();
+        if (removed != null) {
+            beginChange();
+            nextUpdate(removeAfter);
+            nextRemove(removeAfter + 1, removed);
+            endChange();
+        }
+        getSource().clear();
+    }
+
+    /**
+     * Invoked when user clicked on the icon on the left of a row.
+     * The method sets the expanded rows to the ones containing the clicked 
cell.
+     * If that row is the currently expanded one, then it will be reduced to a 
single row.
+     */
+    @Override
+    public void handle(final MouseEvent event) {
+        /*
+         * Remove the additional rows from this list. Before doing so, we need 
to remember
+         * what we are removing from this list view in order to send 
notification later.
+         */
+        final IconCell cell = (IconCell) event.getSource();
+        final int index = getSourceIndex(cell.getIndex());      // Must be 
invoked before `shrink()`.
+        final int removeAfter = indexOfExpanded;
+        final List<Feature> removed = shrink();
+//      index = getViewIndex(index);                // Not needed for current 
single-selection model.
+        /*
+         * If a new row is selected, extract now all properties. We need at 
least the number
+         * of properties anyway for determining the number of additional rows. 
 But we store
+         * also the property values in arrays for convenience because we can 
not use indices
+         * on arbitrary collections (they may not be lists).  This is okay on 
the assumption
+         * that the number of elements is not large.
+         */
+        if (index != indexOfExpanded) {
+            expansion = ExpandedFeature.create(cell.getItem(), nameToIndex);
+            if (expansion != null) {
+                indexOfExpanded = index;
+                final int limit = Integer.MAX_VALUE - getSource().size();
+                if (expansion.length > limit) {
+                    if (limit > 1) {
+                        expansion = Arrays.copyOf(expansion, limit);    // 
Drop last rows for avoiding integer overflow.
+                    } else {
+                        expansion = null;                               // 
Drop completely for avoiding integer overflow.
+                        indexOfExpanded = Integer.MAX_VALUE;
+                    }
+                }
+            }
+        }
+        /*
+         * Send change notifications only after all states have been updated.
+         */
+        beginChange();
+        if (removed != null) {
+            nextUpdate(removeAfter);
+            nextRemove(removeAfter + 1, removed);
+        }
+        if (expansion != null) {
+            // An ArithmeticException below would be a bug in above limit 
adjustment.
+            nextAdd(indexOfExpanded + 1, Math.addExact(indexOfExpanded, 
expansion.length));
+        }
+        endChange();
+    }
+
+    /**
+     * Returns {@code true} if the given feature contains more than one row.
+     */
+    private boolean isExpandable(final Feature feature) {
+        if (feature != null) {
+            for (final String name : nameToIndex.keySet()) {
+                final Object value = feature.getPropertyValue(name);
+                if (value instanceof Collection<?>) {
+                    final int size = ((Collection<?>) value).size();
+                    if (size >= 2) {
+                        return true;
+                    }
+                }
+            }
+        }
+        return false;
+    }
+
+    /**
+     * Returns the number of elements in this list.
+     */
+    @Override
+    public int size() {
+        int size = getSource().size();
+        if (expansion != null) {
+            size += expansion.length - 1;
+        }
+        return size;
+    }
+
+    /**
+     * Returns the feature at the given index. This method forwards the 
request to the source,
+     * except if the given index is for an expanded row.
+     */
+    @Override
+    public Feature get(int index) {
+        final int i = index - indexOfExpanded;
+        if (i >= 0) {
+            final int n = expansion.length;     // A NullPointerException here 
would be an ExpandableList bug.
+            if (i < n) return expansion[i];
+            index -= n;
+        }
+        return getSource().get(index);
+    }
+
+    /**
+     * Given an index in this expanded list, returns the index of 
corresponding element in the feature list.
+     * All indices from {@link #indexOfExpanded} inclusive to 
<code>{@linkplain #indexOfExpanded} +
+     * {@linkplain #expansion}.length</code> exclusive map to the same {@link 
Feature} instance.
+     *
+     * @param  index  index in this expandable list.
+     * @return index of the corresponding element in {@link FeatureList}.
+     */
+    @Override
+    public int getSourceIndex(int index) {
+        if (index > indexOfExpanded) {
+            index = Math.max(indexOfExpanded, index - (expansion.length - 1));
+            // A NullPointerException above would be an ExpandableList bug.
+        }
+        return index;
+    }
+
+    /**
+     * Given an index in the feature list, returns the index in this 
expandable list.
+     * If the given index maps the expanded feature, then the returned index 
will be
+     * for the first row. This is okay if the lower index inclusive or for 
upper index
+     * <em>exclusive</em> (it would not be okay for upper index inclusive).
+     *
+     * @param  index  index in the wrapped {@link FeatureList}.
+     * @return index of the corresponding element in this list.
+     */
+    @Override
+    public int getViewIndex(int index) {
+        if (index > indexOfExpanded) {
+            index += expansion.length - 1;
+            // A NullPointerException above would be an ExpandableList bug.
+        }
+        return index;
+    }
+
+    /**
+     * Notifies all listeners that the list changed. This method expects an 
event from the wrapped
+     * {@link FeatureList} and converts source indices to indices of this 
expandable list.
+     */
+    @Override
+    protected void sourceChanged(final ListChangeListener.Change<? extends 
Feature> c) {
+        fireChange(new ListChangeListener.Change<Feature>(this) {
+            @Override public void     reset()               {c.reset();}
+            @Override public boolean  next()                {return c.next();}
+            @Override public boolean  wasAdded()            {return 
c.wasAdded();}
+            @Override public boolean  wasRemoved()          {return 
c.wasRemoved();}
+            @Override public boolean  wasReplaced()         {return 
c.wasReplaced();}
+            @Override public boolean  wasUpdated()          {return 
c.wasUpdated();}
+            @Override public boolean  wasPermutated()       {return 
c.wasPermutated();}
+            @Override protected int[] getPermutation()      {return null;}  // 
Not invoked since we override the method below.
+            @Override public    int   getPermutation(int i) {return 
getViewIndex(c.getPermutation(getSourceIndex(i)));}
+            @Override public    int   getFrom()             {return 
getViewIndex(c.getFrom());}
+            @Override public    int   getTo() {
+                // If remove only, must be where removed elements were 
positioned in the list.
+                return (wasAdded() || !wasRemoved()) ? getViewIndex(c.getTo()) 
: getFrom();
+            }
+
+            @Override
+            public int getRemovedSize() {
+                int removedSize = c.getRemovedSize();
+                if (overlapExpanded(c.getFrom(), removedSize)) {
+                    removedSize += expansion.length - 1;
+                }
+                return removedSize;
+            }
+
+            @Override
+            @SuppressWarnings("unchecked")
+            public List<Feature> getRemoved() {
+                return (List<Feature>) expandRemoved(c.getFrom(), 
c.getRemoved());
+            }
+        });
+    }
+
+    /**
+     * Returns {@code true} if the given range of removed rows overlaps the 
expanded rows.
+     */
+    private boolean overlapExpanded(final int sourceFrom, final int 
removedSize) {
+        return (sourceFrom <= indexOfExpanded && sourceFrom > indexOfExpanded 
- removedSize);   // Use - for avoiding overflow.
+    }
+
+    /**
+     * If the range of removed elements overlaps the range of expanded rows, 
inserts values in the
+     * {@code removed} list for the expanded rows. Actually this insertion 
should never happens in
+     * the way we use {@link ExpandableList}, but we check as a safety.
+     *
+     * @param  sourceFrom  index of the first removed element in the source 
list.
+     * @param  removed     the removed elements provided by the {@link 
FeatureList}.
+     * @return the removed elements as seen by this {@code ExpandableList}.
+     */
+    private List<? extends Feature> expandRemoved(final int sourceFrom, final 
List<? extends Feature> removed) {
+        if (!overlapExpanded(sourceFrom, removed.size())) {
+            return removed;
+        }
+        final int s = indexOfExpanded;
+        final int n = expansion.length;         // A NullPointerException here 
would be an ExpandableList bug.
+        final Feature[] features = removed.toArray(new Feature[removed.size() 
+ (n - 1)]);
+        System.arraycopy(features,  s+1, features, s + n, features.length - 
(s+1));
+        System.arraycopy(expansion, 0,   features, s,  n);
+        return Arrays.asList(features);
+    }
+
+    /**
+     * Creates a new cell for an icon to show at the beginning of a row.
+     * This method is provided for allowing {@code ExpandableList} to be
+     * given to {@link TableColumn#setCellFactory(Callback)}.
+     *
+     * @param  column  the column where the cell will be shown.
+     */
+    @Override
+    public TableCell<Feature,Feature> call(final TableColumn<Feature,Feature> 
column) {
+        return new IconCell();
+    }
+
+    /**
+     * The cell which represents whether a row is expandable or expanded.
+     * If visible, this is the first column in the table.
+     */
+    private final class IconCell extends TableCell<Feature,Feature>  {
+        /**
+         * Whether this cell is listening to mouse click events.
+         */
+        private boolean isListening;
+
+        /**
+         * Creates a new cell for feature property value.
+         */
+        IconCell() {
+            setTextAlignment(TextAlignment.CENTER);
+        }
+
+        /**
+         * Invoked when a new feature needs to be show. This method sets an 
icon depending on
+         * whether there is multi-valued properties, and whether the current 
row is expanded.
+         * The call will have a listener only if it has an icon.
+         */
+        @Override
+        protected void updateItem(final Feature value, final boolean empty) {
+            super.updateItem(value, empty);
+            Background b = null;
+            String  text = null;
+            if (value instanceof ExpandedFeature) {
+                /*
+                 * If this is the selected row, put an icon only on the first 
row,
+                 * not on additional rows showing the other collection 
elements.
+                 */
+                if (((ExpandedFeature) value).index == 0) {
+                    text = EXPANDED;
+                } else {
+                    b = EXPANSION_HEADER;
+                }
+            } else if (isExpandable(value)) {
+                text = EXPANDABLE;
+            }
+            setBackground(b);
+            setText(text);
+            if (isListening != (isListening = (text != null))) {        // 
Check whether `isListening` changed.
+                if (isListening) {
+                    addEventFilter(MouseEvent.MOUSE_CLICKED, 
ExpandableList.this);
+                } else {
+                    removeEventFilter(MouseEvent.MOUSE_CLICKED, 
ExpandableList.this);
+                }
+            }
+        }
+    }
+}
diff --git 
a/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ExpandedFeature.java
 
b/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ExpandedFeature.java
new file mode 100644
index 0000000..536e6d8
--- /dev/null
+++ 
b/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ExpandedFeature.java
@@ -0,0 +1,214 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.sis.gui.dataset;
+
+import java.util.Arrays;
+import java.util.Map;
+import java.util.Collection;
+import java.util.Collections;
+import org.opengis.feature.Feature;
+import org.opengis.feature.FeatureType;
+import org.opengis.feature.Property;
+
+
+/**
+ * A feature where property values are specific elements of multi-valued 
properties of other features.
+ * The element is identified by an index. The same index value is used for all 
multi-valued properties
+ * of the {@linkplain #source} feature. Each property may have a different 
number of elements, so the
+ * {@link #index} value can be between 0 (inclusive) and the maximal 
collection size (exclusive).
+ * If a property is requested for which {@link #index} is too large, {@code 
null} is returned.
+ *
+ * @author  Martin Desruisseaux (Geomatys)
+ * @version 1.1
+ * @since   1.1
+ * @module
+ */
+final class ExpandedFeature implements Feature {
+    /**
+     * The array for properties having no value.
+     * This is the fill value for {@link #values} array.
+     */
+    private static final Object[] EMPTY = new Object[0];
+
+    /**
+     * The feature from which to get type and property values.
+     * This feature may contain a mix of single-valued and multi-valued 
properties.
+     * Values of multi-valued properties are copied in the {@link #values} 
array.
+     */
+    private final Feature source;
+
+    /**
+     * Mapping from property names to index in the {@link #values} array.
+     * <strong>Do not modify,</strong> because all {@link ExpandedFeature} 
instances created
+     * after a call to {@link FeatureTable#setFeatureType(FeatureType)} share 
the same map.
+     */
+    private final Map<String,Integer> nameToIndex;
+
+    /**
+     * All values from the {@linkplain #source} feature for properties named 
in the {@link #nameToIndex} keys.
+     * The {@code values[i]} elements may be empty arrays but never null. This 
{@code ExpandedFeature} instance
+     * uses only the values at {@code values[…][index]}. Other {@code 
ExpandedFeature} instances will share the
+     * same arrays but with different {@link #index} values.
+     */
+    private final Object[][] values;
+
+    /**
+     * Index of the element to extract in each {@link #values} sub-array.
+     *
+     * @see #getPropertyValue(String)
+     */
+    final int index;
+
+    /**
+     * Creates a new feature wrapping the given source.
+     */
+    private ExpandedFeature(final Feature source, final Map<String,Integer> 
nameToIndex,
+                            final Object[][] values, final int index)
+    {
+        this.source      = source;
+        this.nameToIndex = nameToIndex;
+        this.values      = values;
+        this.index       = index;
+    }
+
+    /**
+     * Creates the wrappers for all elements in the properties of the given 
feature.
+     * If non-null, the returned array is guaranteed to have at least two 
elements.
+     * See {@link ExpandableList#expansion} for an explanation about why we do 
not
+     * allow empty arrays.
+     *
+     * @param  source       the "real" feature to wrap, or {@code null} if 
none.
+     * @param  nameToIndex  mapping from property names to index in the {@code 
values} array.
+     * @return pseudo-features for property elements at all indices, or {@code 
null} if none.
+     */
+    static ExpandedFeature[] create(final Feature source, final 
Map<String,Integer> nameToIndex) {
+        if (source != null) {
+            final Object[][] values = new Object[nameToIndex.size()][];
+            Arrays.fill(values, EMPTY);
+            int count = 0;
+            for (final Map.Entry<String,Integer> entry : 
nameToIndex.entrySet()) {
+                final Object value = source.getPropertyValue(entry.getKey());
+                if (value != null) {                                    // 
Should not be null, but be paranoiac.
+                    final int i = entry.getValue();
+                    if (value instanceof Collection<?>) {
+                        final Object[] elements = ((Collection<?>) 
value).toArray();
+                        if (elements != null) {                         // 
Should not be null, but be paranoiac.
+                            values[i] = elements;
+                        }
+                    } else {
+                        values[i] = new Object[] {value};               // 
Should not happen, but be paranoiac.
+                    }
+                    final int n = values[i].length;
+                    if (n > count) count = n;
+                }
+            }
+            if (count > 1) {                                            // 
Required by this method contract.
+                final ExpandedFeature[] features = new ExpandedFeature[count];
+                for (int i=0; i<count; i++) {
+                    features[i] = new ExpandedFeature(source, nameToIndex, 
values, i);
+                }
+                return features;
+            }
+        }
+        return null;
+    }
+
+    /**
+     * Returns the source feature type verbatim.
+     */
+    @Override
+    public FeatureType getType() {
+        return source.getType();
+    }
+
+    /**
+     * Returns the property of the given name.
+     * This method is not used by {@link FeatureTable} so we just delegate to 
the source.
+     */
+    @Override
+    public Property getProperty(final String name) {
+        return source.getProperty(name);
+    }
+
+    /**
+     * Sets the property of the given name.
+     * This method is not used by {@link FeatureTable} so we just forward to 
the source.
+     */
+    @Override
+    public void setProperty(final Property property) {
+        source.setProperty(property);
+    }
+
+    /**
+     * Returns a single value for the property at the given name.
+     * If the property is multi-valued, only the value at the index managed by 
this instance is returned.
+     */
+    @Override
+    public Object getPropertyValue(final String name) {
+        final Integer i = nameToIndex.get(name);
+        if (i == null) {
+            // Not a multi-valued property.
+            return source.getPropertyValue(name);
+        }
+        final Object[] elements = values[i];
+        return (index < elements.length)
+               ? Collections.singletonList(elements[index])
+               : Collections.emptyList();
+    }
+
+    /**
+     * Unsupported operation.
+     */
+    @Override
+    public void setPropertyValue(String name, Object value) {
+        throw new UnsupportedOperationException();
+    }
+
+    /**
+     * Returns a hash code value for this feature.
+     * Defined consistently with {@link #equals(Object)}.
+     */
+    @Override
+    public int hashCode() {
+        return source.hashCode() + index;
+    }
+
+    /**
+     * Compares this feature with the specified object for equality.
+     * The comparison ignores {@link #nameToIndex} and {@link #values}
+     * because they are derived from {@link #source} and {@link #index}.
+     */
+    @Override
+    public boolean equals(final Object obj) {
+        if (this == obj) {
+            return true;
+        }
+        if (obj instanceof ExpandedFeature) {
+            final ExpandedFeature other = (ExpandedFeature) obj;
+            return index == other.index && source.equals(other.source);
+        }
+        return false;
+    }
+
+    /**
+     * Returns the feature string representation for debugging purpose.
+     */
+    @Override
+    public String toString() {
+        return source.toString();
+    }
+}
diff --git 
a/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/FeatureList.java
 
b/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/FeatureList.java
index 1842ff3..f333939 100644
--- 
a/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/FeatureList.java
+++ 
b/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/FeatureList.java
@@ -34,9 +34,10 @@ import org.apache.sis.util.ArraysExt;
  * When an element is requested, if that element has not yet been read, the 
reading is done
  * in a background thread.
  *
- * <p>This list is unmodifiable through public API. It can be modified only 
through package-private API.
- * This restriction exists for preventing uncontrolled modifications to 
introduce inconsistencies with
- * the modifications to be applied by the loader running in background 
thread.</p>
+ * <p>This list is unmodifiable through public API except for the {@link 
#clear()} method.
+ * This list should be modified only through package-private API.
+ * The intent is to prevents uncontrolled modifications to introduce 
inconsistencies
+ * with the modifications to be applied by the loader running in background 
thread.</p>
  *
  * <p>This list does not accept null elements; any attempt to add a null 
feature is silently ignored.
  * The null value is reserved for meaning that the element is in process of 
being loaded.</p>
@@ -61,9 +62,9 @@ final class FeatureList extends ObservableListBase<Feature> {
     private static final long NUM_PENDING_ROWS = 10;
 
     /**
-     * Maximum number of rows that this list will allow.
+     * Maximum number of rows that this list will allow. Must be smaller than 
{@link Integer#MAX_VALUE}.
      */
-    private static final int MAXIMUM_ROWS = Integer.MAX_VALUE;
+    private static final int MAXIMUM_ROWS = Integer.MAX_VALUE - 1;
 
     /**
      * The {@link #elements} value when this list is empty.
@@ -118,10 +119,13 @@ final class FeatureList extends 
ObservableListBase<Feature> {
     }
 
     /**
-     * Clears the content of this list. This method shall be invoked when no 
loader is running
-     * in a background thread, or when the loaded decided itself to invoke 
this method.
+     * Clears the content of this list. While this method can be invoked from 
public API,
+     * it should be reserved to {@link FeatureList} and {@link FeatureTable} 
internal usage.
+     * This method should be invoked only when no loader is running in a 
background thread,
+     * or when the loaded decided itself to invoke this method.
      */
-    final void clearUnsafe() {
+    @Override
+    public void clear() {
         final List<Feature> removed = validElements();
         elements = EMPTY;
         estimatedSize = 0;
@@ -140,7 +144,7 @@ final class FeatureList extends ObservableListBase<Feature> 
{
      * @param  features  the features to show in the table, or {@code null} if 
none.
      * @return whether a background process has been scheduled.
      */
-    final boolean setFeatures(final FeatureTable table, final FeatureSet 
features) {
+    final boolean startFeaturesLoading(final FeatureTable table, final 
FeatureSet features) {
         assert Platform.isFxApplicationThread();
         final FeatureLoader previous = nextPageLoader;
         if (previous != null) {
@@ -152,7 +156,7 @@ final class FeatureList extends ObservableListBase<Feature> 
{
             BackgroundThreads.execute(nextPageLoader);
             return true;
         } else {
-            clearUnsafe();
+            clear();
             return false;
         }
     }
@@ -236,7 +240,7 @@ final class FeatureList extends ObservableListBase<Feature> 
{
             }
             beginChange();
             nextReplace(validCount, replaceTo, removed);
-            nextAdd(replaceTo, validCount = newValidCount);
+            nextAdd(replaceTo, replaceTo + (validCount = newValidCount));
             endChange();
             checkOverflow();
         }
@@ -246,7 +250,7 @@ final class FeatureList extends ObservableListBase<Feature> 
{
      * If we can not load more features stop the reading process.
      *
      * @todo Add some message in the widget for warning the user.
-     *       Proposal: set MAXIMUM_ROWS to MAX_INTEGER - 1 and reserve the 
last table row for a message.
+     *       Proposal: set MAXIMUM_ROWS to MAX_INTEGER - 2 and reserve the 
last table row for a message.
      *       That row would span all columns. That row could also be used for 
exception message when the
      *       exception did not happened at the file beginning.
      */
diff --git 
a/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/FeatureTable.java
 
b/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/FeatureTable.java
index 6b2c219..309427c 100644
--- 
a/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/FeatureTable.java
+++ 
b/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/FeatureTable.java
@@ -20,22 +20,28 @@ import java.util.Locale;
 import java.util.List;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Iterator;
 import javafx.scene.control.TableColumn;
 import javafx.scene.control.TableView;
-import javafx.beans.property.ReadOnlyObjectWrapper;
-import javafx.beans.value.ObservableValue;
+import javafx.scene.control.TableCell;
 import javafx.scene.layout.StackPane;
 import javafx.scene.layout.Region;
+import javafx.beans.property.ReadOnlyObjectWrapper;
+import javafx.beans.value.ObservableValue;
+import javafx.collections.ObservableList;
 import javafx.geometry.Pos;
 import javafx.util.Callback;
 import org.opengis.feature.Feature;
 import org.opengis.feature.FeatureType;
 import org.opengis.feature.PropertyType;
+import org.opengis.feature.AttributeType;
+import org.opengis.feature.FeatureAssociationRole;
 import org.opengis.util.InternationalString;
 import org.apache.sis.internal.util.Strings;
 import org.apache.sis.internal.system.Modules;
 import org.apache.sis.util.logging.Logging;
 import org.apache.sis.storage.FeatureSet;
+import org.apache.sis.internal.gui.IdentityValueFactory;
 import org.apache.sis.internal.gui.ExceptionReporter;
 
 
@@ -48,6 +54,12 @@ import org.apache.sis.internal.gui.ExceptionReporter;
  * <p>If this view is removed from scene graph, then {@link #interrupt()} 
should be called
  * for stopping any loading process that may be under progress.</p>
  *
+ * @todo This class does not yet handle {@link FeatureAssociationRole}. We 
could handle them with
+ *       {@link javafx.scene.control.SplitPane} with the main feature table in 
the upper part and
+ *       the feature table of selected cell in the bottom part. Bottom part 
could put tables in a
+ *       {@link javafx.scene.control.Accordion} since there is possibly 
different tables to show
+ *       depending on the column of selected cell.
+ *
  * @author  Johann Sorel (Geomatys)
  * @author  Smaniotto Enzo (GSoC)
  * @author  Martin Desruisseaux (Geomatys)
@@ -81,7 +93,7 @@ public class FeatureTable extends TableView<Feature> {
     public FeatureTable() {
         textLocale = Locale.getDefault(Locale.Category.DISPLAY);
         dataLocale = Locale.getDefault(Locale.Category.FORMAT);
-        setColumnResizePolicy(TableView.UNCONSTRAINED_RESIZE_POLICY);
+        setColumnResizePolicy(CONSTRAINED_RESIZE_POLICY);
         setTableMenuButtonVisible(true);
         setItems(new FeatureList());
     }
@@ -91,7 +103,25 @@ public class FeatureTable extends TableView<Feature> {
      * All methods on the returned list shall be invoked from JavaFX thread.
      */
     final FeatureList getFeatureList() {
-        return (FeatureList) getItems();
+        final ObservableList<Feature> items = getItems();
+        if (items instanceof FeatureList) {
+            return (FeatureList) items;
+        } else {
+            return (FeatureList) ((ExpandableList) getItems()).getSource();
+        }
+    }
+
+    /**
+     * Returns the list where to add features when some properties may be 
multi-valued.
+     * This method wraps the {@link FeatureList} into an {@link 
ExpandableList} if needed.
+     */
+    private ExpandableList getExpandableList() {
+        final ObservableList<Feature> items = getItems();
+        if (items instanceof ExpandableList) {
+            return (ExpandableList) items;
+        } else {
+            return new ExpandableList((FeatureList) items);
+        }
     }
 
     /**
@@ -109,7 +139,7 @@ public class FeatureTable extends TableView<Feature> {
      */
     public void setFeatures(final FeatureSet features) {
         final FeatureList items = getFeatureList();
-        if (!items.setFeatures(this, features)) {
+        if (!items.startFeaturesLoading(this, features)) {
             featureType = null;
             getColumns().clear();
             setPlaceholder(null);
@@ -123,34 +153,85 @@ public class FeatureTable extends TableView<Feature> {
      */
     final void setFeatureType(final FeatureType type) {
         setPlaceholder(null);
-        getFeatureList().clearUnsafe();
+        getItems().clear();
         if (type != null && !type.equals(featureType)) {
             final Collection<? extends PropertyType> properties = 
type.getProperties(true);
             final List<TableColumn<Feature,?>> columns = new 
ArrayList<>(properties.size());
+            final List<String> multiValued = new ArrayList<>(columns.size());
             for (final PropertyType pt : properties) {
+                /*
+                 * Get localized text to show in column header. Also remember
+                 * the plain property name; it will be needed for ValueGetter.
+                 */
                 final String name = pt.getName().toString();
                 String title = string(pt.getDesignation());
                 if (title == null) {
                     title = string(pt.getName().toInternationalString());
                     if (title == null) title = name;
                 }
-                final TableColumn<Feature, Object> column = new 
TableColumn<>(title);
+                /*
+                 * If the property may contain more than one value, we will
+                 * need a specialized cell getter.
+                 *
+                 * TODO: we should also handle FeatureAssociationRole here.
+                 *       See comment in class javadoc.
+                 */
+                boolean isMultiValued = false;
+                if (pt instanceof AttributeType<?>) {
+                    isMultiValued = ((AttributeType<?>) pt).getMaximumOccurs() 
> 1;
+                }
+                if (isMultiValued) {
+                    multiValued.add(name);
+                }
+                /*
+                 * Create and configure the column. For multi-valued 
properties, ValueGetter always
+                 * gives the whole collection. Fetching a particular element 
in that collection will
+                 * be ElementCell's work.
+                 */
+                final TableColumn<Feature,Object> column = new 
TableColumn<>(title);
                 column.setCellValueFactory(new ValueGetter(name));
+                column.setCellFactory(isMultiValued ? ElementCell::new : 
ValueCell::new);
                 columns.add(column);
             }
+            /*
+             * If there is at least one multi-valued property, insert a column 
which will contain
+             * an icon in front of rows having a property with more than one 
value.
+             */
+            if (multiValued.isEmpty()) {
+                setItems(getFeatureList());         // Will fire a change 
event only if the list is not the same.
+            } else {
+                final ExpandableList list = getExpandableList();
+                list.setMultivaluedColumns(multiValued);
+                final TableColumn<Feature,Feature> column = new 
TableColumn<>();
+                column.setCellValueFactory(IdentityValueFactory.instance());
+                column.setCellFactory(list);
+                column.setReorderable(false);
+                column.setSortable   (false);
+                column.setResizable  (false);
+                column.setMinWidth(20);
+                column.setMaxWidth(20);
+                columns.add(0, column);
+                setItems(list);
+            }
             getColumns().setAll(columns);       // Change columns in an all or 
nothing operation.
         }
         featureType = type;
     }
 
+
+
+
     /**
-     * Fetch values to show in the table cells.
+     * Given a {@link Feature}, returns the value of the property having the 
name specified at construction time.
+     * Note that if the property is multi-valued, then this getter returns the 
whole collection since we have no
+     * easy way to know the current row number. Fetching a particular element 
in that collection will be done by
+     * {@link ExpandedFeature}.
      */
     private static final class ValueGetter implements 
Callback<TableColumn.CellDataFeatures<Feature,Object>, ObservableValue<Object>> 
{
         /**
          * The name of the feature property for which to fetch values.
          */
-        final String name;
+        private final String name;
 
         /**
          * Creates a new getter of property values.
@@ -163,7 +244,7 @@ public class FeatureTable extends TableView<Feature> {
 
         /**
          * Returns the value of the feature property wrapped by the given 
argument.
-         * This method is invoked by JavaFX when a new cell needs to be 
rendered.
+         * This method is invoked by JavaFX when a cell needs to be rendered 
with a new value.
          */
         @Override
         public ObservableValue<Object> call(final 
TableColumn.CellDataFeatures<Feature, Object> cell) {
@@ -171,15 +252,72 @@ public class FeatureTable extends TableView<Feature> {
             final Feature feature = cell.getValue();
             if (feature != null) {
                 value = feature.getPropertyValue(name);
-                if (value instanceof Collection<?>) {
-                    value = "collection";               // TODO
-                }
             }
             return new ReadOnlyObjectWrapper<>(value);
         }
     }
 
     /**
+     * A cell displaying a value in {@link FeatureTable}. This base class 
expects single values.
+     * If the property values are collections, then {@link ElementCell} should 
be used instead.
+     */
+    private static class ValueCell extends TableCell<Feature,Object> {
+        /**
+         * Creates a new cell for feature property value.
+         *
+         * @param  column  the column where the cell will be shown.
+         */
+        ValueCell(final TableColumn<Feature,Object> column) {
+            // Column not used at this time, but we need it in method 
signature.
+        }
+
+        /**
+         * Invoked when a new value needs to be show.
+         *
+         * @todo Needs to check for object type (number, date, etc.).
+         */
+        @Override
+        protected void updateItem(final Object value, final boolean empty) {
+            if (value == getItem()) return;
+            super.updateItem(value, empty);
+            String text = null;
+            if (value != null) {
+                text = value.toString();
+            }
+            setText(text);
+        }
+    }
+
+    /**
+     * Fetch single elements from multi-valued properties.
+     */
+    private static final class ElementCell extends ValueCell {
+        /**
+         * Creates a new cell for multi-values feature property value.
+         *
+         * @param  column  the column where the cell will be shown.
+         */
+        ElementCell(final TableColumn<Feature,Object> column) {
+            super(column);
+        }
+
+        /**
+         * Invoked when a new value needs to be show.
+         */
+        @Override
+        protected void updateItem(Object value, final boolean empty) {
+            if (value instanceof List<?>) {
+                final List<?> c = (List<?>) value;
+                value = c.isEmpty() ? null : c.get(0);
+            } else if (value instanceof Iterable<?>) {
+                final Iterator<?> c = ((Iterable<?>) value).iterator();
+                value = c.hasNext() ? c.next() : null;
+            }
+            super.updateItem(value, empty);
+        }
+    }
+
+    /**
      * Returns the given international string as a non-empty localized string, 
or {@code null} if none.
      */
     private String string(final InternationalString i18n) {
@@ -187,7 +325,7 @@ public class FeatureTable extends TableView<Feature> {
     }
 
     /**
-     * If a loading process was under way, interrupts it and close the feature 
stream.
+     * If a loading process was under way, interrupts it and closes the 
feature stream.
      * This method returns immediately; the release of resources happens in a 
background thread.
      */
     public void interrupt() {
@@ -199,7 +337,7 @@ public class FeatureTable extends TableView<Feature> {
      * This method is invoked after a loading process failed.
      */
     final void setException(final Throwable exception) {
-        getFeatureList().clearUnsafe();
+        getItems().clear();
         final Region trace = new ExceptionReporter(exception).getView();
         StackPane.setAlignment(trace, Pos.TOP_LEFT);
         setPlaceholder(trace);
diff --git 
a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/IdentityValueFactory.java
 
b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/IdentityValueFactory.java
new file mode 100644
index 0000000..9082b86
--- /dev/null
+++ 
b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/IdentityValueFactory.java
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.sis.internal.gui;
+
+import javafx.beans.property.ReadOnlyObjectWrapper;
+import javafx.beans.value.ObservableValue;
+import javafx.scene.control.TableColumn;
+import javafx.util.Callback;
+
+
+/**
+ * An value for {@link TableColumn#setCellValueFactory(Callback)} which just 
forward
+ * the values as-is (ignoring the observable wrapper).
+ *
+ * @author  Martin Desruisseaux (Geomatys)
+ * @version 1.1
+ *
+ * @param <S>  the type of objects that represent rows in the table.
+ * @param <T>  the type of values in table cells.
+ *
+ * @since 1.1
+ * @module
+ */
+public final class IdentityValueFactory<S extends T, T>
+        implements Callback<TableColumn.CellDataFeatures<S,T>, 
ObservableValue<T>>
+{
+    /**
+     * The singleton instance.
+     */
+    private static final IdentityValueFactory<?,?> INSTANCE = new 
IdentityValueFactory<>();
+
+    /**
+     * Returns the factory instance.
+     *
+     * @param  <S>  the type of objects that represent rows in the table.
+     * @param  <T>  the type of values in table cells.
+     * @return the factory instance.
+     */
+    @SuppressWarnings("unchecked")
+    public static <S extends T, T> IdentityValueFactory<S,T> instance() {
+        return (IdentityValueFactory<S,T>) INSTANCE;
+    }
+    /**
+     * For the singleton constructor only.
+     */
+    private IdentityValueFactory() {
+    }
+
+    /**
+     * Wraps the value in an observable.
+     * The argument are the return values are non-null, but the wrapped value 
may be null.
+     *
+     * @param  cell  the cell containing a value.
+     * @return the wrapped value.
+     */
+    @Override
+    public ObservableValue<T> call(final TableColumn.CellDataFeatures<S, T> 
cell) {
+        return new ReadOnlyObjectWrapper<>(cell.getValue());
+    }
+}
diff --git 
a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/Styles.java 
b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/Styles.java
index 18a2770..3dfdbad 100644
--- 
a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/Styles.java
+++ 
b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/Styles.java
@@ -56,6 +56,11 @@ public final class Styles {
     public static final Color ERROR_TEXT = Color.RED;
 
     /**
+     * Color for header of expanded rows in {@link 
org.apache.sis.gui.dataset.FeatureTable}.
+     */
+    public static final Color EXPANDED_ROW = Color.GAINSBORO;
+
+    /**
      * Do not allow instantiation of this class.
      */
     private Styles() {

Reply via email to