Extract a base class from typed NativeSelect

This patch adds some helpers for extensions aimed at Listing components.

Change-Id: I7ac2ee56ca7e44ac0300c94d02d30533aea11f9a

server/src/main/java/com/vaadin/server/ListingExtension.java

@@ -0,0 +1,23 @@
+/*
+ * Copyright 2000-2014 Vaadin Ltd.
+ * 
+ * Licensed 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 com.vaadin.server;
+
+import com.vaadin.ui.components.Listing;
+
+public interface ListingExtension<T> extends Extension {
+
+    public void extend(Listing<T> listing);
+}

server/src/main/java/com/vaadin/server/communication/data/typed/AbstractSelectionModel.java

@@ -16,10 +16,10 @@
 package com.vaadin.server.communication.data.typed;
 
 import com.vaadin.server.AbstractClientConnector;
-import com.vaadin.server.AbstractExtension;
 import com.vaadin.server.Extension;
 import com.vaadin.shared.data.typed.DataProviderConstants;
 import com.vaadin.ui.Component;
+import com.vaadin.ui.components.AbstractListing.AbstractListingExtension;
 import com.vaadin.ui.components.Listing;
 
 import elemental.json.JsonObject;
@@ -30,8 +30,8 @@ import elemental.json.JsonObject;
  * @param <T>
  *            type of selected data
  */
-public abstract class AbstractSelectionModel<T> extends AbstractExtension
-        implements SelectionModel<T>, TypedDataGenerator<T> {
+public abstract class AbstractSelectionModel<T> extends
+        AbstractListingExtension<T> implements SelectionModel<T> {
 
     private Listing<T> parent;
 
@@ -47,42 +47,6 @@ public abstract class AbstractSelectionModel<T> extends AbstractExtension
         }
     }
 
-    // TODO: Following methods should be somewhere else being less weird.
-
-    protected final Listing<T> getParentListing() {
-        return parent;
-    }
-
-    protected final DataProvider<T> getDataProvider() {
-        for (Extension e : ((Component) parent).getExtensions()) {
-            if (e instanceof DataProvider) {
-                return ((DataProvider<T>) e);
-            }
-        }
-        return null;
-    }
-
-    protected final DataKeyMapper<T> getKeyMapper() {
-        for (Extension e : ((Component) parent).getExtensions()) {
-            if (e instanceof DataProvider) {
-                return ((DataProvider<T>) e).getKeyMapper();
-            }
-        }
-        return null;
-    }
-
-    protected final T getData(String key) {
-        DataKeyMapper<T> keyMapper = getKeyMapper();
-        if (keyMapper != null) {
-            return keyMapper.get(key);
-        }
-        return null;
-    }
-
-    protected final void refresh(T value) {
-        getDataProvider().refresh(value);
-    }
-
     @Override
     public void generateData(T data, JsonObject jsonObject) {
         if (getSelected().contains(data)) {

server/src/main/java/com/vaadin/server/communication/data/typed/DataProvider.java

@@ -46,11 +46,6 @@ public abstract class DataProvider<T> extends AbstractExtension {
      * Creates the appropriate type of DataProvider based on the type of
      * Collection provided to the method.
      * <p>
-     * <strong>Note:</strong> this method will also extend the given component
-     * with the newly created DataProvider. The user should <strong>not</strong>
-     * call the {@link #extend(com.vaadin.server.AbstractClientConnector)}
-     * method explicitly.
-     * <p>
      * TODO: Actually use different DataProviders and provide an API for the
      * back end to inform changes back.
      * 
@@ -63,7 +58,6 @@ public abstract class DataProvider<T> extends AbstractExtension {
     public static <V> SimpleDataProvider<V> create(DataSource<V> data,
             AbstractComponent component) {
         SimpleDataProvider<V> dataProvider = new SimpleDataProvider<V>(data);
-        dataProvider.extend(component);
         return dataProvider;
     }
 

server/src/main/java/com/vaadin/server/communication/data/typed/SelectionModel.java

@@ -22,7 +22,7 @@ import java.util.List;
 
 import com.vaadin.event.handler.Handler;
 import com.vaadin.event.handler.Registration;
-import com.vaadin.server.Extension;
+import com.vaadin.server.ListingExtension;
 import com.vaadin.ui.Component;
 import com.vaadin.ui.components.HasValue;
 import com.vaadin.ui.components.Listing;
@@ -34,7 +34,7 @@ import com.vaadin.ui.components.Listing;
  * @param <T>
  *            type of selected values
  */
-public interface SelectionModel<T> extends Serializable, Extension {
+public interface SelectionModel<T> extends Serializable, ListingExtension<T> {
 
     /**
      * Selection model for selection a single value.

server/src/main/java/com/vaadin/ui/components/AbstractListing.java

@@ -0,0 +1,206 @@
+/*
+ * Copyright 2000-2014 Vaadin Ltd.
+ * 
+ * Licensed 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 com.vaadin.ui.components;
+
+import java.util.LinkedHashSet;
+import java.util.Set;
+
+import com.vaadin.server.AbstractClientConnector;
+import com.vaadin.server.AbstractExtension;
+import com.vaadin.server.ListingExtension;
+import com.vaadin.server.communication.data.typed.AbstractSelectionModel;
+import com.vaadin.server.communication.data.typed.DataProvider;
+import com.vaadin.server.communication.data.typed.SelectionModel;
+import com.vaadin.server.communication.data.typed.TypedDataGenerator;
+import com.vaadin.ui.AbstractComponent;
+
+/**
+ * Base class for Listing components. Provides common handling for
+ * {@link DataProvider}, {@link SelectionModel} and {@link TypedDataGenerator}s.
+ * 
+ * @param <T>
+ *            listing data type
+ */
+public abstract class AbstractListing<T> extends AbstractComponent implements
+        Listing<T> {
+
+    /**
+     * Helper base class for creating extensions for Listing components. This
+     * class provides helpers for accessing the underlying parts of the
+     * component and it's communicational mechanism.
+     *
+     * @param <T>
+     *            listing data type
+     */
+    public abstract static class AbstractListingExtension<T> extends
+            AbstractExtension implements ListingExtension<T>,
+            TypedDataGenerator<T> {
+
+        /**
+         * {@inheritDoc}
+         * <p>
+         * Note: AbstractListingExtensions need parent to be of type
+         * AbstractListing.
+         * 
+         * @throws IllegalArgument
+         *             if parent is not an AbstractListing
+         */
+        @Override
+        public void extend(Listing<T> listing) {
+            if (listing instanceof AbstractListing) {
+                AbstractListing<T> parent = (AbstractListing<T>) listing;
+                super.extend(parent);
+                parent.addDataGenerator(this);
+            } else {
+                throw new IllegalArgumentException(
+                        "Parent needs to extend AbstractListing");
+            }
+        }
+
+        @Override
+        public void remove() {
+            getParent().removeDataGenerator(this);
+
+            super.remove();
+        }
+
+        /**
+         * Gets a data object based on it's client-side identifier key.
+         * 
+         * @param key
+         *            key for data object
+         * @return the data object
+         */
+        protected T getData(String key) {
+            DataProvider<T> dataProvider = getParent().getDataProvider();
+            if (dataProvider != null) {
+                return dataProvider.getKeyMapper().get(key);
+            }
+            return null;
+        }
+
+        @Override
+        @SuppressWarnings("unchecked")
+        public AbstractListing<T> getParent() {
+            return (AbstractListing<T>) super.getParent();
+        }
+
+        /**
+         * Helper method for refreshing a single data object.
+         * 
+         * @param data
+         *            data object to refresh
+         */
+        protected void refresh(T data) {
+            DataProvider<T> dataProvider = getParent().getDataProvider();
+            if (dataProvider != null) {
+                dataProvider.refresh(data);
+            }
+        }
+    }
+
+    /* DataProvider for this Listing component */
+    private DataProvider<T> dataProvider;
+    /* TypedDataGenerators used by this Listing */
+    private Set<TypedDataGenerator<T>> generators = new LinkedHashSet<>();
+    /* SelectionModel for this Listing */
+    private SelectionModel<T> selectionModel;
+
+    /**
+     * Adds a {@link TypedDataGenerator} for the {@link DataProvider} of this
+     * Listing component.
+     * 
+     * @param generator
+     *            typed data generator
+     */
+    protected void addDataGenerator(TypedDataGenerator<T> generator) {
+        generators.add(generator);
+
+        if (dataProvider != null) {
+            dataProvider.addDataGenerator(generator);
+        }
+    }
+
+    /**
+     * Removed a {@link TypedDataGenerator} from the {@link DataProvider} of
+     * this Listing component.
+     * 
+     * @param generator
+     *            typed data generator
+     */
+    protected void removeDataGenerator(TypedDataGenerator<T> generator) {
+        generators.remove(generator);
+
+        if (dataProvider != null) {
+            dataProvider.removeDataGenerator(generator);
+        }
+    }
+
+    /**
+     * Extends this listing component with a data provider. This method
+     * reapplies all data generators to the new data provider.
+     * 
+     * @param dataProvider
+     *            new data provider
+     */
+    protected void setDataProvider(DataProvider<T> dataProvider) {
+        if (this.dataProvider == dataProvider) {
+            return;
+        }
+
+        if (this.dataProvider != null) {
+            this.dataProvider.remove();
+        }
+
+        this.dataProvider = dataProvider;
+        if (dataProvider != null) {
+            addExtension(dataProvider);
+
+            if (dataProvider != null) {
+                // Reapply all data generators to the new data provider.
+                for (TypedDataGenerator<T> generator : generators) {
+                    dataProvider.addDataGenerator(generator);
+                }
+            }
+        }
+    }
+
+    /**
+     * Get the {@link DataProvider} of this Listing component.
+     * 
+     * @return data provider
+     */
+    protected DataProvider<T> getDataProvider() {
+        return dataProvider;
+    }
+
+    @SuppressWarnings("unchecked")
+    @Override
+    public void setSelectionModel(SelectionModel<T> model) {
+        if (selectionModel != null) {
+            selectionModel.remove();
+        }
+        selectionModel = model;
+        if (model != null) {
+            model.extend(this);
+        }
+    }
+
+    @Override
+    public SelectionModel<T> getSelectionModel() {
+        return selectionModel;
+    }
+}

server/src/main/java/com/vaadin/ui/components/nativeselect/NativeSelect.java

@@ -17,62 +17,46 @@ package com.vaadin.ui.components.nativeselect;
 
 import java.util.function.Function;
 
-import com.vaadin.server.Extension;
 import com.vaadin.server.communication.data.typed.DataProvider;
 import com.vaadin.server.communication.data.typed.DataSource;
-import com.vaadin.server.communication.data.typed.SelectionModel;
 import com.vaadin.server.communication.data.typed.SingleSelection;
 import com.vaadin.server.communication.data.typed.TypedDataGenerator;
-import com.vaadin.ui.AbstractComponent;
-import com.vaadin.ui.components.Listing;
+import com.vaadin.ui.components.AbstractListing;
 
 import elemental.json.JsonObject;
 
-public class NativeSelect<T> extends AbstractComponent implements Listing<T> {
+public class NativeSelect<T> extends AbstractListing<T> {
 
     private DataSource<T> dataSource;
-    private DataProvider<T> dataProvider;
-    private SelectionModel<T> selectionModel;
     private Function<T, String> nameProvider = T::toString;
 
     public NativeSelect() {
-        internalSetSelectionModel(new SingleSelection<>());
+        setSelectionModel(new SingleSelection<>());
+        addDataGenerator(new TypedDataGenerator<T>() {
+
+            @Override
+            public void generateData(T data, JsonObject jsonObject) {
+                jsonObject.put("n", nameProvider.apply(data));
+            }
+
+            @Override
+            public void destroyData(T data) {
+            }
+        });
     }
 
     public NativeSelect(DataSource<T> dataSource) {
         this();
-        internalSetDataSource(dataSource);
+        setDataSource(dataSource);
     }
 
     @Override
     public void setDataSource(DataSource<T> data) {
-        internalSetDataSource(data);
-    }
-
-    private void internalSetDataSource(DataSource<T> data) {
-        if (dataProvider != null) {
-            dataProvider.remove();
-            dataProvider = null;
-        }
         dataSource = data;
         if (dataSource != null) {
-            dataProvider = DataProvider.create(dataSource, this);
-            dataProvider.addDataGenerator(new TypedDataGenerator<T>() {
-
-                @Override
-                public void generateData(T data, JsonObject jsonObject) {
-                    jsonObject.put("n", nameProvider.apply(data));
-                }
-
-                @Override
-                public void destroyData(T data) {
-                }
-            });
-            for (Extension e : getExtensions()) {
-                if (e instanceof TypedDataGenerator) {
-                    dataProvider.addDataGenerator((TypedDataGenerator<T>) e);
-                }
-            }
+            setDataProvider(DataProvider.create(dataSource, this));
+        } else {
+            setDataProvider(null);
         }
     }
 
@@ -80,43 +64,4 @@ public class NativeSelect<T> extends AbstractComponent implements Listing<T> {
     public DataSource<T> getDataSource() {
         return dataSource;
     }
-
-    @Override
-    public SelectionModel<T> getSelectionModel() {
-        return selectionModel;
-    }
-
-    @Override
-    public void setSelectionModel(SelectionModel<T> model) {
-        internalSetSelectionModel(model);
-    }
-
-    private void internalSetSelectionModel(SelectionModel<T> model) {
-        if (selectionModel != null) {
-            selectionModel.remove();
-        }
-        selectionModel = model;
-        if (model != null) {
-            model.setParentListing(this);
-        }
-    }
-
-    @Override
-    protected void addExtension(Extension extension) {
-        super.addExtension(extension);
-
-        if (dataProvider != null && extension instanceof TypedDataGenerator) {
-            dataProvider.addDataGenerator((TypedDataGenerator<T>) extension);
-        }
-    }
-
-    @Override
-    public void removeExtension(Extension extension) {
-        super.removeExtension(extension);
-
-        if (dataProvider != null && extension instanceof TypedDataGenerator) {
-            dataProvider.removeDataGenerator((TypedDataGenerator<T>) extension);
-        }
-    }
-
 }

server/src/test/java/com/vaadin/ui/components/AbstractListingTest.java

@@ -0,0 +1,89 @@
+package com.vaadin.ui.components;
+
+import static org.junit.Assert.assertEquals;
+
+import java.util.Arrays;
+import java.util.Iterator;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import com.vaadin.server.communication.data.typed.AbstractDataSource;
+import com.vaadin.server.communication.data.typed.DataProvider;
+import com.vaadin.server.communication.data.typed.DataSource;
+import com.vaadin.ui.components.AbstractListing.AbstractListingExtension;
+
+import elemental.json.JsonObject;
+
+public class AbstractListingTest {
+
+    private final class CountGenerator extends AbstractListingExtension<String> {
+
+        int callCount = 0;
+
+        @Override
+        public void generateData(String data, JsonObject jsonObject) {
+            ++callCount;
+        }
+
+        @Override
+        public void destroyData(String data) {
+        }
+    }
+
+    AbstractListing<String> testComponent = new AbstractListing<String>() {
+
+        DataSource<String> data;
+
+        @Override
+        public void setDataSource(DataSource<String> data) {
+            this.data = data;
+            setDataProvider(DataProvider.create(data, this));
+        }
+
+        @Override
+        public DataSource<String> getDataSource() {
+            return data;
+        }
+    };
+
+    @Before
+    public void setUp() {
+        testComponent.setDataSource(new AbstractDataSource<String>() {
+
+            @Override
+            public void save(String data) {
+            }
+
+            @Override
+            public void remove(String data) {
+            }
+
+            @Override
+            public Iterator<String> iterator() {
+                return Arrays.asList("Foo").iterator();
+            }
+        });
+    }
+
+    @Test
+    public void testAddDataGenerator() {
+        CountGenerator countGenerator = new CountGenerator();
+
+        countGenerator.extend(testComponent);
+        testComponent.getDataProvider().beforeClientResponse(true);
+
+        assertEquals("Generator was not called.", 1, countGenerator.callCount);
+    }
+
+    @Test
+    public void testAddAndRemoveDataGenerator() {
+        CountGenerator countGenerator = new CountGenerator();
+
+        countGenerator.extend(testComponent);
+        countGenerator.remove();
+        testComponent.getDataProvider().beforeClientResponse(true);
+
+        assertEquals("Generator was called.", 0, countGenerator.callCount);
+    }
+}