Re: git commit: WICKET-5350 reintroduce wildcards for repeater over models, otherwise subclasses is hindered

Sven Meier Thu, 25 Sep 2014 05:58:17 -0700

Hi Martin,

> And I have the feeling that there are users of this in 1.5.x/6.x.

> But now the application developer has to do the cast and this makesit more safer than before!


Indeed.

On second thought we could keep the setter on ListItemModel though: Puta warning in its javadoc and mark it deprecated.

This way users will have more time to improve their code.

Regards
Sven


On 09/25/2014 02:47 PM, Martin Grigorov wrote:

Hi Sven,

On Wed, Sep 24, 2014 at 11:11 PM, Sven Meier <[email protected]> wrote:

Hi Martin,

ListView's works on a list with wildcards now (as it did before
WICKET-5350):
- for ListView it doesn't matter whether ther are wildcards or not
- users can use subtypes in their listView subclasses

But #getModelObjet() can return a wildcard list only (in Wicket 6.x it
erroneously returns List<T>):

   public final List<? extends T> getModelObject()
   {
     return (List<? extends T>)getDefaultModelObject();
   }

Otherwise the following could fail with a runtime error:
   listView = new ListView<Number>("id", new ArrayList<Integer>());
   listView.getModelObject().add(new Double()); // now we have a Double
inside an integer-list :/

A wildcard on a collection makes it read-only effectively, so
ListItemModel can no longer change anything in the list.

I don't think anyone would need ListItemModl#setObject() anyway, at least
I never used it and neither does Wicket. So I just made ListItemModel a
read-only model too.

Maybe there are not many users of ListItemModl#setObject()...
But now listView.getModelObject().add(anInteger); won't work too, because
as you said List<? extends T> is a read-only list (while List<? super T> is
a write-only).
And I have the feeling that there are users of this in 1.5.x/6.x.
But now the application developer has to do the cast and this makes it more
safer than before!

Thanks!

Regards
Sven



On 09/24/2014 10:19 PM, Martin Grigorov wrote:

Hi Sven,

On Wed, Sep 24, 2014 at 10:05 PM, <[email protected]> wrote:

  Repository: wicket

Updated Branches:
    refs/heads/master fb8db6ce6 -> adcb7a632


WICKET-5350 reintroduce wildcards for repeater over models, otherwise
subclasses is hindered

Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/adcb7a63
Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/adcb7a63
Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/adcb7a63

Branch: refs/heads/master
Commit: adcb7a632af8225e86e09e398b8fb5430b143b18
Parents: fb8db6c
Author: svenmeier <[email protected]>
Authored: Wed Sep 24 22:05:04 2014 +0200
Committer: svenmeier <[email protected]>
Committed: Wed Sep 24 22:05:04 2014 +0200

----------------------------------------------------------------------
   .../wicket/markup/html/list/ListItemModel.java  | 13 ++------
   .../wicket/markup/html/list/ListView.java       | 20 ++++++------
   .../markup/html/list/PageableListView.java      |  6 ++--
   .../markup/html/list/PropertyListView.java      |  6 ++--
   .../wicket/markup/html/panel/FeedbackPanel.java |  2 +-
   .../markup/repeater/AbstractPageableView.java   |  4 +--
   .../wicket/markup/repeater/RefreshingView.java  |  4 +--
   .../wicket/markup/html/list/ListViewTest.java   | 33
++++++++++++++++++++
   .../markup/html/form/select/SelectOptions.java  |  7 +++--
   9 files changed, 60 insertions(+), 35 deletions(-)
----------------------------------------------------------------------



http://git-wip-us.apache.org/repos/asf/wicket/blob/
adcb7a63/wicket-core/src/main/java/org/apache/wicket/markup/
html/list/ListItemModel.java
----------------------------------------------------------------------
diff --git
a/wicket-core/src/main/java/org/apache/wicket/markup/html/
list/ListItemModel.java
b/wicket-core/src/main/java/org/apache/wicket/markup/html/
list/ListItemModel.java
index 9bed7bb..0b8192b 100644
---
a/wicket-core/src/main/java/org/apache/wicket/markup/html/
list/ListItemModel.java
+++
b/wicket-core/src/main/java/org/apache/wicket/markup/html/
list/ListItemModel.java
@@ -16,7 +16,7 @@
    */
   package org.apache.wicket.markup.html.list;

-import org.apache.wicket.model.IModel;
+import org.apache.wicket.model.AbstractReadOnlyModel;

   /**
    * Model for list items.
@@ -26,7 +26,7 @@ import org.apache.wicket.model.IModel;
    *            Model object type
    *
    */
-public class ListItemModel<T> implements IModel<T>
+public class ListItemModel<T> extends AbstractReadOnlyModel<T>
   {
          private static final long serialVersionUID = 1L;

@@ -60,15 +60,6 @@ public class ListItemModel<T> implements IModel<T>
          }

          /**
-        * @see org.apache.wicket.model.IModel#setObject(java.lang.
Object)
-        */
-       @Override
-       public void setObject(T object)
-       {
-               listView.getModelObject().set(index, object);
-       }

  Why did you make this change ?

-       /**
           * @see org.apache.wicket.model.IDetachable#detach()
           */
          @Override


http://git-wip-us.apache.org/repos/asf/wicket/blob/
adcb7a63/wicket-core/src/main/java/org/apache/wicket/markup/
html/list/ListView.java
----------------------------------------------------------------------
diff --git
a/wicket-core/src/main/java/org/apache/wicket/markup/html/
list/ListView.java
b/wicket-core/src/main/java/org/apache/wicket/markup/html/
list/ListView.java
index 54b3015..57f9d84 100644
---
a/wicket-core/src/main/java/org/apache/wicket/markup/html/
list/ListView.java
+++
b/wicket-core/src/main/java/org/apache/wicket/markup/html/
list/ListView.java
@@ -100,7 +100,7 @@ import
org.apache.wicket.util.collections.ReadOnlyIterator;
    * @author Eelco Hillenius
    *
    * @param <T>
- *            Model object type
+ *            type of elements contained in the model's list
    */
   public abstract class ListView<T> extends AbstractRepeater
   {
@@ -130,11 +130,11 @@ public abstract class ListView<T> extends
AbstractRepeater
          }

          /**
-        * @param id
-        * @param model
+        * @param id component id
+        * @param model model containing a list of
           * @see org.apache.wicket.Component#Component(String, IModel)
           */
-       public ListView(final String id, final IModel<? extends List<T>>
model)
+       public ListView(final String id, final IModel<? extends List<?
extends T>> model)
          {
                  super(id, model);

@@ -156,7 +156,7 @@ public abstract class ListView<T> extends
AbstractRepeater
           *            List to cast to Serializable
           * @see org.apache.wicket.Component#Component(String, IModel)
           */
-       public ListView(final String id, final List<T> list)
+       public ListView(final String id, final List<? extends T> list)
          {
                  this(id, Model.ofList(list));
          }
@@ -169,9 +169,9 @@ public abstract class ListView<T> extends
AbstractRepeater
           * @return The list of items in this list view.
           */
          @SuppressWarnings("unchecked")
-       public final List<T> getList()
+       public final List<? extends T> getList()
          {
-               final List<T> list = (List<T>)getDefaultModelObject();
+               final List<? extends T> list = (List<? extends
T>)getDefaultModelObject();
                  if (list == null)
                  {
                          return Collections.emptyList();
@@ -364,7 +364,7 @@ public abstract class ListView<T> extends
AbstractRepeater
           *            The list for the new model. The list must
implement
{@link Serializable}.
           * @return This for chaining
           */
-       public ListView<T> setList(List<T> list)
+       public ListView<T> setList(List<? extends T> list)
          {
                  setDefaultModel(Model.ofList(list));
                  return this;
@@ -638,9 +638,9 @@ public abstract class ListView<T> extends
AbstractRepeater
           * @return model object
           */
          @SuppressWarnings("unchecked")
-       public final List<T> getModelObject()
+       public final List<? extends T> getModelObject()
          {
-               return (List<T>)getDefaultModelObject();
+               return (List<? extends T>)getDefaultModelObject();
          }

          /**


http://git-wip-us.apache.org/repos/asf/wicket/blob/
adcb7a63/wicket-core/src/main/java/org/apache/wicket/markup/
html/list/PageableListView.java
----------------------------------------------------------------------
diff --git
a/wicket-core/src/main/java/org/apache/wicket/markup/html/
list/PageableListView.java
b/wicket-core/src/main/java/org/apache/wicket/markup/html/
list/PageableListView.java
index 7c717dd..7690db2 100644
---
a/wicket-core/src/main/java/org/apache/wicket/markup/html/
list/PageableListView.java
+++
b/wicket-core/src/main/java/org/apache/wicket/markup/html/
list/PageableListView.java
@@ -29,7 +29,7 @@ import org.apache.wicket.model.IModel;
    *
    * @author Jonathan Locke
    * @param <T>
- *            Model object type
+ *            type of elements contained in the model's list
    */
   public abstract class PageableListView<T> extends ListView<T>
implements
IPageableItems
   {
@@ -51,7 +51,7 @@ public abstract class PageableListView<T> extends
ListView<T> implements IPageab
           * @param itemsPerPage
           *            Number of rows to show on a page
           */
-       public PageableListView(final String id, final IModel<? extends
List<T>> model,
+       public PageableListView(final String id, final IModel<? extends
List<? extends T>> model,
                  int itemsPerPage)
          {
                  super(id, model);
@@ -70,7 +70,7 @@ public abstract class PageableListView<T> extends
ListView<T> implements IPageab
           *            Number of rows to show on a page
           * @see ListView#ListView(String, List)
           */
-       public PageableListView(final String id, final List<T> list,
final
int itemsPerPage)
+       public PageableListView(final String id, final List<? extends T>
list, final int itemsPerPage)
          {
                  super(id, list);
                  this.itemsPerPage = itemsPerPage;


http://git-wip-us.apache.org/repos/asf/wicket/blob/
adcb7a63/wicket-core/src/main/java/org/apache/wicket/markup/
html/list/PropertyListView.java
----------------------------------------------------------------------
diff --git
a/wicket-core/src/main/java/org/apache/wicket/markup/html/
list/PropertyListView.java
b/wicket-core/src/main/java/org/apache/wicket/markup/html/
list/PropertyListView.java
index 464a14e..b458ed0 100644
---
a/wicket-core/src/main/java/org/apache/wicket/markup/html/
list/PropertyListView.java
+++
b/wicket-core/src/main/java/org/apache/wicket/markup/html/
list/PropertyListView.java
@@ -29,7 +29,7 @@ import org.apache.wicket.model.IModel;
    * @author Nathan Hamblen
    *
    * @param <T>
- *            Model object type
+ *            type of elements contained in the model's list
    */
   public abstract class PropertyListView<T> extends ListView<T>
   {
@@ -57,7 +57,7 @@ public abstract class PropertyListView<T> extends
ListView<T>
           * @param model
           *            wrapping a List
           */
-       public PropertyListView(final String id, final IModel<? extends
List<T>> model)
+       public PropertyListView(final String id, final IModel<? extends
List<? extends T>> model)
          {
                  super(id, model);
          }
@@ -71,7 +71,7 @@ public abstract class PropertyListView<T> extends
ListView<T>
           * @param list
           *            unmodeled List
           */
-       public PropertyListView(final String id, final List<T> list)
+       public PropertyListView(final String id, final List<? extends T>
list)
          {
                  super(id, list);
          }


http://git-wip-us.apache.org/repos/asf/wicket/blob/
adcb7a63/wicket-core/src/main/java/org/apache/wicket/markup/
html/panel/FeedbackPanel.java
----------------------------------------------------------------------
diff --git
a/wicket-core/src/main/java/org/apache/wicket/markup/html/
panel/FeedbackPanel.java
b/wicket-core/src/main/java/org/apache/wicket/markup/html/
panel/FeedbackPanel.java
index 50fe999..d17ba9f 100644
---
a/wicket-core/src/main/java/org/apache/wicket/markup/html/
panel/FeedbackPanel.java
+++
b/wicket-core/src/main/java/org/apache/wicket/markup/html/
panel/FeedbackPanel.java
@@ -327,7 +327,7 @@ public class FeedbackPanel extends Panel implements
IFeedback
           */
          protected final List<FeedbackMessage> getCurrentMessages()
          {
-               final List<FeedbackMessage> messages =
messageListView.getModelObject();
+               final List<? extends FeedbackMessage> messages =
messageListView.getModelObject();
                  return Collections.unmodifiableList(messages);
          }



http://git-wip-us.apache.org/repos/asf/wicket/blob/
adcb7a63/wicket-core/src/main/java/org/apache/wicket/markup/
repeater/AbstractPageableView.java
----------------------------------------------------------------------
diff --git
a/wicket-core/src/main/java/org/apache/wicket/markup/
repeater/AbstractPageableView.java
b/wicket-core/src/main/java/org/apache/wicket/markup/
repeater/AbstractPageableView.java
index 297ee8c..8b98077 100644
---
a/wicket-core/src/main/java/org/apache/wicket/markup/
repeater/AbstractPageableView.java
+++
b/wicket-core/src/main/java/org/apache/wicket/markup/
repeater/AbstractPageableView.java
@@ -41,7 +41,7 @@ import org.apache.wicket.model.IModel;
    * @author Igor Vaynberg (ivaynberg)
    *
    * @param <T>
- *            Model object type
+ *            type of elements contained in the model's list
    */
   public abstract class AbstractPageableView<T> extends RefreshingView<T>
implements IPageableItems
   {
@@ -73,7 +73,7 @@ public abstract class AbstractPageableView<T> extends
RefreshingView<T> implemen
           * @param model
           * @see org.apache.wicket.Component#Component(String, IModel)
           */
-       public AbstractPageableView(String id, IModel<? extends
Collection<T>> model)
+       public AbstractPageableView(String id, IModel<? extends
Collection<? extends T>> model)
          {
                  super(id, model);
                  clearCachedItemCount();


http://git-wip-us.apache.org/repos/asf/wicket/blob/
adcb7a63/wicket-core/src/main/java/org/apache/wicket/markup/
repeater/RefreshingView.java
----------------------------------------------------------------------
diff --git
a/wicket-core/src/main/java/org/apache/wicket/markup/
repeater/RefreshingView.java
b/wicket-core/src/main/java/org/apache/wicket/markup/
repeater/RefreshingView.java
index 3e18d88..e9a11fc 100644
---
a/wicket-core/src/main/java/org/apache/wicket/markup/
repeater/RefreshingView.java
+++
b/wicket-core/src/main/java/org/apache/wicket/markup/
repeater/RefreshingView.java
@@ -46,7 +46,7 @@ import org.apache.wicket.util.lang.Generics;
    * @author Igor Vaynberg (ivaynberg)
    *
    * @param <T>
- *            Model object type
+ *            type of elements contained in the model's list
    */
   public abstract class RefreshingView<T> extends RepeatingView
   {
@@ -79,7 +79,7 @@ public abstract class RefreshingView<T> extends
RepeatingView
           * @param model
           *            model
           */
-       public RefreshingView(String id, IModel<? extends Collection<T>>
model)
+       public RefreshingView(String id, IModel<? extends Collection<?
extends T>> model)
          {
                  super(id, model);
          }


http://git-wip-us.apache.org/repos/asf/wicket/blob/
adcb7a63/wicket-core/src/test/java/org/apache/wicket/markup/
html/list/ListViewTest.java
----------------------------------------------------------------------
diff --git
a/wicket-core/src/test/java/org/apache/wicket/markup/html/
list/ListViewTest.java
b/wicket-core/src/test/java/org/apache/wicket/markup/html/
list/ListViewTest.java
index c4caead..8ebf50f 100644
---
a/wicket-core/src/test/java/org/apache/wicket/markup/html/
list/ListViewTest.java
+++
b/wicket-core/src/test/java/org/apache/wicket/markup/html/
list/ListViewTest.java
@@ -17,8 +17,11 @@
   package org.apache.wicket.markup.html.list;

   import java.util.ArrayList;
+import java.util.List;

   import org.apache.wicket.WicketTestCase;
+import org.apache.wicket.markup.html.basic.Label;
+import org.apache.wicket.model.IModel;
   import org.apache.wicket.model.util.ListModel;
   import org.junit.Test;

@@ -57,6 +60,36 @@ public class ListViewTest extends WicketTestCase
          }

          /**
+        */
+       @Test
+       public void generics() {
+               // a listView for numbers
+               class NumberListView extends ListView<Number> {
+
+                       private static final long serialVersionUID = 1L;
+
+                       // since the given list is not changed actually,
we can safely
+                       // accept lists accepting subtypes of numbers
only
+                       public NumberListView(String id, IModel<? extends
List<? extends Number>> model)
+                       {
+                               super(id, model);
+                       }
+
+                       @Override
+                       protected void populateItem(ListItem<Number>
item)
+                       {
+                               // non-fancy display of the number
+                               add(new Label("label", item.getModel()));
+                       }
+               };
+
+               IModel<List<Integer>> integers = new ListModel<>(new
ArrayList<Integer>());
+
+               // pass list of integers to the number listView
+               new NumberListView("integers", integers);
+       }
+
+       /**
           *
           */
          @Test


http://git-wip-us.apache.org/repos/asf/wicket/blob/
adcb7a63/wicket-extensions/src/main/java/org/apache/
wicket/extensions/markup/html/form/select/SelectOptions.java
----------------------------------------------------------------------
diff --git
a/wicket-extensions/src/main/java/org/apache/wicket/
extensions/markup/html/form/select/SelectOptions.java
b/wicket-extensions/src/main/java/org/apache/wicket/
extensions/markup/html/form/select/SelectOptions.java
index c02f058..12cfed3 100644
---
a/wicket-extensions/src/main/java/org/apache/wicket/
extensions/markup/html/form/select/SelectOptions.java
+++
b/wicket-extensions/src/main/java/org/apache/wicket/
extensions/markup/html/form/select/SelectOptions.java
@@ -39,6 +39,7 @@ import org.apache.wicket.model.util.CollectionModel;
    * </pre>
    *
    * @param <T>
+ *            type of elements contained in the model's collection
    * @author Igor Vaynberg (ivaynberg)
    */
   public class SelectOptions<T> extends RepeatingView
@@ -56,7 +57,7 @@ public class SelectOptions<T> extends RepeatingView
           * @param model
           * @param renderer
           */
-       public SelectOptions(final String id, final IModel<? extends
Collection<T>> model,
+       public SelectOptions(final String id, final IModel<? extends
Collection<? extends T>> model,
                  final IOptionRenderer<T> renderer)
          {
                  super(id, model);
@@ -71,7 +72,7 @@ public class SelectOptions<T> extends RepeatingView
           * @param elements
           * @param renderer
           */
-       public SelectOptions(final String id, final Collection<T>
elements,
+       public SelectOptions(final String id, final Collection<? extends
T> elements,
                  final IOptionRenderer<T> renderer)
          {
                  this(id, new CollectionModel<>(elements), renderer);
@@ -107,7 +108,7 @@ public class SelectOptions<T> extends RepeatingView
                          // populate this repeating view with
SelectOption
components
                          removeAll();

-                       Collection<T> modelObject =
(Collection<T>)getDefaultModelObject();
+                       Collection<? extends T> modelObject =
(Collection<? extends T>)getDefaultModelObject();

                          if (modelObject != null)
                          {

Re: git commit: WICKET-5350 reintroduce wildcards for repeater over models, otherwise subclasses is hindered

Reply via email to