You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by sv...@apache.org on 2014/09/24 22:05:31 UTC
git commit: WICKET-5350 reintroduce wildcards for repeater over
models, otherwise subclasses is hindered
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 <sv...@meiers.net>
Authored: Wed Sep 24 22:05:04 2014 +0200
Committer: svenmeier <sv...@meiers.net>
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);
- }
-
- /**
* @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
Posted by Sven Meier <sv...@meiers.net>.
Great!
Sven
On 23.06.2015 10:35, Martijn Dashorst wrote:
> It's there:
>
> https://issues.apache.org/jira/browse/WICKET-5931
>
> I'm happy to resolve this. The code in our apps is much cleaner now
> without the wildcards.
>
> Martijn
>
>
> On Mon, Jun 22, 2015 at 9:50 PM, Sven Meier <sv...@meiers.net> wrote:
>>> Please create a new issue for this improvement.
>> ... before applying your patch - IMHO WICKET-5350 is already confusing
>> enough with my changes ;)
>>
>> Thanks
>> Sven
>>
>>
>>
>>
>> On 22.06.2015 16:44, Sven Meier wrote:
>>> I don't mind the change. Please create a new issue for this improvement.
>>>
>>> Many thanks
>>> Sven
>>>
>>>
>>> On 22.06.2015 16:05, Martijn Dashorst wrote:
>>>> This patch https://gist.github.com/dashorst/eb84199f86e109728dce fixes
>>>> the API.
>>>>
>>>> All in all it took for our 1.2M lines of code project roughly 10
>>>> minutes to fix the breakages, improving the API considerably,
>>>> discovering and fixing 1 wrongly typed listview and in total 16
>>>> compile errors. This is with about 550 ListView occurrences in total
>>>> across the whole application.
>>>>
>>>> I'm +1 for this patch.
>>>>
>>>> Martijn
>>>>
>>>>
>>>> On Mon, Jun 22, 2015 at 3:46 PM, Emond Papegaaij
>>>> <em...@topicus.nl> wrote:
>>>>> Hi,
>>>>>
>>>>> As Martijn said: the current form limits the user in a way that is not
>>>>> needed.
>>>>> The returned List is not read-only, forcing users to keep a reference to
>>>>> the
>>>>> IModel to be able to update the List. Also, it's inconsistent.
>>>>> populateItem
>>>>> still uses ListItem<T>, not <? extends T>. Changing the constructors,
>>>>> setList,
>>>>> getList and getModelObject to List<T> and IModel<? extends List<T>>
>>>>> seems a
>>>>> minor API to me, making the API much more consistent and flexible.
>>>>>
>>>>> Best regards,
>>>>> Emond
>>>>>
>>>>> On Monday 22 June 2015 15:36:25 Sven Meier wrote:
>>>>>> Hi,
>>>>>>
>>>>>> this is the relevant discussion why I reverted the ListView constructor
>>>>>> to that of Wicket 6:
>>>>>>
>>>>>>
>>>>>> http://mail-archives.apache.org/mod_mbox/wicket-dev/201409.mbox/%3CCAJmbs8gD
>>>>>> a5mJgwbkoOZS3oH5TYZZ-Ap3_SFDjBHs5SYpn4zTkg%40mail.gmail.com%3E
>>>>>>
>>>>>> Changing it would mean an API break for existing applications. I don't
>>>>>> mind
>>>>>>
>>>>>>> ListView exposes a read-write view on the contents of the list, via
>>>>>>> ListView.getModelObject(), but also ListItem.setModelObject
>>>>>> I wanted ListItem to be read-only, but Martin an I agreed on keeping it
>>>>>> writeable for backwards-compatibility. Is this really a problem we have
>>>>>> to
>>>>>> fix in our API?
>>>>>>
>>>>>> Regards
>>>>>> Sven
>>>>>>
>>>>>> On 22.06.2015 14:31, Emond Papegaaij wrote:
>>>>>>> Hi Sven,
>>>>>>>
>>>>>>> It's easy to change the testcase to:
>>>>>>> class NumberListView<N extends Number> extends ListView<N>
>>>>>>>
>>>>>>> and
>>>>>>>
>>>>>>> new NumberListView<Integer>("integers", integers)
>>>>>>>
>>>>>>> I think the constructor in Wicket 6 is wrong. It should be IModel<?
>>>>>>> extends
>>>>>>> List<T>> model: we don't care what type the list is, but we do care
>>>>>>> about
>>>>>>> the type of the contents of the list. ListView exposes a read-write
>>>>>>> view
>>>>>>> on the contents of the list, via ListView.getModelObject(), but also
>>>>>>> ListItem.setModelObject. IMHO it should either be read-only with ?
>>>>>>> extends
>>>>>>> T or read-write with T, but mixed. The first is a major API break, the
>>>>>>> second is not, so I prefer the second.
>>>>>>>
>>>>>>> Best regards,
>>>>>>> Emond
>>>>>>>
>>>>>>> On Monday 22 June 2015 14:22:25 Sven Meier wrote:
>>>>>>>> Hi Martijn,
>>>>>>>>
>>>>>>>> the ListView constructor is now as it has been in Wicket 6:
>>>>>>>> public ListView(final String id, final IModel<? extends List<?
>>>>>>>>
>>>>>>>> extends T>> model)
>>>>>>>>
>>>>>>>> ListViewTest#generics() shows a valid use case, that is not possible
>>>>>>>> otherwise.
>>>>>>>>
>>>>>>>> Regards
>>>>>>>> Sven
>>>>>>>>
>>>>>>>> On 22.06.2015 13:42, Martijn Dashorst wrote:
>>>>>>>>> I'm not sure I'm fan of this change. Using these wildcards breaks
>>>>>>>>> all
>>>>>>>>> kinds of code. What is the benefit?
>>>>>>>>>
>>>>>>>>> The way it is implemented currently is also inconsistent: ListItem
>>>>>>>>> is
>>>>>>>>> typed as ListItem<T> but it should be ListItem<? extends T>. This
>>>>>>>>> gist: https://gist.github.com/dashorst/4ee7ab1696321f290a24 shows
>>>>>>>>> how
>>>>>>>>> this should be implemented.
>>>>>>>>>
>>>>>>>>> HOWEVER: I don't actually propose such a change, but rather have
>>>>>>>>> adcb7a632af8225e86e09e398b8fb5430b143b18 be reverted. The linked
>>>>>>>>> patch
>>>>>>>>> will break the world and for little to no benefit.
>>>>>>>>> adcb7a632af8225e86e09e398b8fb5430b143b18 breaks API in a couple of
>>>>>>>>> places, but I don't see the benefit of those breaks as well.
>>>>>>>>>
>>>>>>>>> Martijn
>>>>
>>>>
>
>
Re: git commit: WICKET-5350 reintroduce wildcards for repeater over
models, otherwise subclasses is hindered
Posted by Martijn Dashorst <ma...@gmail.com>.
It's there:
https://issues.apache.org/jira/browse/WICKET-5931
I'm happy to resolve this. The code in our apps is much cleaner now
without the wildcards.
Martijn
On Mon, Jun 22, 2015 at 9:50 PM, Sven Meier <sv...@meiers.net> wrote:
>>Please create a new issue for this improvement.
>
> ... before applying your patch - IMHO WICKET-5350 is already confusing
> enough with my changes ;)
>
> Thanks
> Sven
>
>
>
>
> On 22.06.2015 16:44, Sven Meier wrote:
>>
>> I don't mind the change. Please create a new issue for this improvement.
>>
>> Many thanks
>> Sven
>>
>>
>> On 22.06.2015 16:05, Martijn Dashorst wrote:
>>>
>>> This patch https://gist.github.com/dashorst/eb84199f86e109728dce fixes
>>> the API.
>>>
>>> All in all it took for our 1.2M lines of code project roughly 10
>>> minutes to fix the breakages, improving the API considerably,
>>> discovering and fixing 1 wrongly typed listview and in total 16
>>> compile errors. This is with about 550 ListView occurrences in total
>>> across the whole application.
>>>
>>> I'm +1 for this patch.
>>>
>>> Martijn
>>>
>>>
>>> On Mon, Jun 22, 2015 at 3:46 PM, Emond Papegaaij
>>> <em...@topicus.nl> wrote:
>>>>
>>>> Hi,
>>>>
>>>> As Martijn said: the current form limits the user in a way that is not
>>>> needed.
>>>> The returned List is not read-only, forcing users to keep a reference to
>>>> the
>>>> IModel to be able to update the List. Also, it's inconsistent.
>>>> populateItem
>>>> still uses ListItem<T>, not <? extends T>. Changing the constructors,
>>>> setList,
>>>> getList and getModelObject to List<T> and IModel<? extends List<T>>
>>>> seems a
>>>> minor API to me, making the API much more consistent and flexible.
>>>>
>>>> Best regards,
>>>> Emond
>>>>
>>>> On Monday 22 June 2015 15:36:25 Sven Meier wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> this is the relevant discussion why I reverted the ListView constructor
>>>>> to that of Wicket 6:
>>>>>
>>>>>
>>>>> http://mail-archives.apache.org/mod_mbox/wicket-dev/201409.mbox/%3CCAJmbs8gD
>>>>> a5mJgwbkoOZS3oH5TYZZ-Ap3_SFDjBHs5SYpn4zTkg%40mail.gmail.com%3E
>>>>>
>>>>> Changing it would mean an API break for existing applications. I don't
>>>>> mind
>>>>>
>>>>>> ListView exposes a read-write view on the contents of the list, via
>>>>>> ListView.getModelObject(), but also ListItem.setModelObject
>>>>>
>>>>> I wanted ListItem to be read-only, but Martin an I agreed on keeping it
>>>>> writeable for backwards-compatibility. Is this really a problem we have
>>>>> to
>>>>> fix in our API?
>>>>>
>>>>> Regards
>>>>> Sven
>>>>>
>>>>> On 22.06.2015 14:31, Emond Papegaaij wrote:
>>>>>>
>>>>>> Hi Sven,
>>>>>>
>>>>>> It's easy to change the testcase to:
>>>>>> class NumberListView<N extends Number> extends ListView<N>
>>>>>>
>>>>>> and
>>>>>>
>>>>>> new NumberListView<Integer>("integers", integers)
>>>>>>
>>>>>> I think the constructor in Wicket 6 is wrong. It should be IModel<?
>>>>>> extends
>>>>>> List<T>> model: we don't care what type the list is, but we do care
>>>>>> about
>>>>>> the type of the contents of the list. ListView exposes a read-write
>>>>>> view
>>>>>> on the contents of the list, via ListView.getModelObject(), but also
>>>>>> ListItem.setModelObject. IMHO it should either be read-only with ?
>>>>>> extends
>>>>>> T or read-write with T, but mixed. The first is a major API break, the
>>>>>> second is not, so I prefer the second.
>>>>>>
>>>>>> Best regards,
>>>>>> Emond
>>>>>>
>>>>>> On Monday 22 June 2015 14:22:25 Sven Meier wrote:
>>>>>>>
>>>>>>> Hi Martijn,
>>>>>>>
>>>>>>> the ListView constructor is now as it has been in Wicket 6:
>>>>>>> public ListView(final String id, final IModel<? extends List<?
>>>>>>>
>>>>>>> extends T>> model)
>>>>>>>
>>>>>>> ListViewTest#generics() shows a valid use case, that is not possible
>>>>>>> otherwise.
>>>>>>>
>>>>>>> Regards
>>>>>>> Sven
>>>>>>>
>>>>>>> On 22.06.2015 13:42, Martijn Dashorst wrote:
>>>>>>>>
>>>>>>>> I'm not sure I'm fan of this change. Using these wildcards breaks
>>>>>>>> all
>>>>>>>> kinds of code. What is the benefit?
>>>>>>>>
>>>>>>>> The way it is implemented currently is also inconsistent: ListItem
>>>>>>>> is
>>>>>>>> typed as ListItem<T> but it should be ListItem<? extends T>. This
>>>>>>>> gist: https://gist.github.com/dashorst/4ee7ab1696321f290a24 shows
>>>>>>>> how
>>>>>>>> this should be implemented.
>>>>>>>>
>>>>>>>> HOWEVER: I don't actually propose such a change, but rather have
>>>>>>>> adcb7a632af8225e86e09e398b8fb5430b143b18 be reverted. The linked
>>>>>>>> patch
>>>>>>>> will break the world and for little to no benefit.
>>>>>>>> adcb7a632af8225e86e09e398b8fb5430b143b18 breaks API in a couple of
>>>>>>>> places, but I don't see the benefit of those breaks as well.
>>>>>>>>
>>>>>>>> Martijn
>>>
>>>
>>>
>>
>
--
Become a Wicket expert, learn from the best: http://wicketinaction.com
Re: git commit: WICKET-5350 reintroduce wildcards for repeater over
models, otherwise subclasses is hindered
Posted by Sven Meier <sv...@meiers.net>.
>Please create a new issue for this improvement.
... before applying your patch - IMHO WICKET-5350 is already confusing
enough with my changes ;)
Thanks
Sven
On 22.06.2015 16:44, Sven Meier wrote:
> I don't mind the change. Please create a new issue for this improvement.
>
> Many thanks
> Sven
>
>
> On 22.06.2015 16:05, Martijn Dashorst wrote:
>> This patch https://gist.github.com/dashorst/eb84199f86e109728dce
>> fixes the API.
>>
>> All in all it took for our 1.2M lines of code project roughly 10
>> minutes to fix the breakages, improving the API considerably,
>> discovering and fixing 1 wrongly typed listview and in total 16
>> compile errors. This is with about 550 ListView occurrences in total
>> across the whole application.
>>
>> I'm +1 for this patch.
>>
>> Martijn
>>
>>
>> On Mon, Jun 22, 2015 at 3:46 PM, Emond Papegaaij
>> <em...@topicus.nl> wrote:
>>> Hi,
>>>
>>> As Martijn said: the current form limits the user in a way that is
>>> not needed.
>>> The returned List is not read-only, forcing users to keep a
>>> reference to the
>>> IModel to be able to update the List. Also, it's inconsistent.
>>> populateItem
>>> still uses ListItem<T>, not <? extends T>. Changing the
>>> constructors, setList,
>>> getList and getModelObject to List<T> and IModel<? extends List<T>>
>>> seems a
>>> minor API to me, making the API much more consistent and flexible.
>>>
>>> Best regards,
>>> Emond
>>>
>>> On Monday 22 June 2015 15:36:25 Sven Meier wrote:
>>>> Hi,
>>>>
>>>> this is the relevant discussion why I reverted the ListView
>>>> constructor
>>>> to that of Wicket 6:
>>>>
>>>> http://mail-archives.apache.org/mod_mbox/wicket-dev/201409.mbox/%3CCAJmbs8gD
>>>>
>>>> a5mJgwbkoOZS3oH5TYZZ-Ap3_SFDjBHs5SYpn4zTkg%40mail.gmail.com%3E
>>>>
>>>> Changing it would mean an API break for existing applications. I
>>>> don't mind
>>>>
>>>>> ListView exposes a read-write view on the contents of the list, via
>>>>> ListView.getModelObject(), but also ListItem.setModelObject
>>>> I wanted ListItem to be read-only, but Martin an I agreed on
>>>> keeping it
>>>> writeable for backwards-compatibility. Is this really a problem we
>>>> have to
>>>> fix in our API?
>>>>
>>>> Regards
>>>> Sven
>>>>
>>>> On 22.06.2015 14:31, Emond Papegaaij wrote:
>>>>> Hi Sven,
>>>>>
>>>>> It's easy to change the testcase to:
>>>>> class NumberListView<N extends Number> extends ListView<N>
>>>>>
>>>>> and
>>>>>
>>>>> new NumberListView<Integer>("integers", integers)
>>>>>
>>>>> I think the constructor in Wicket 6 is wrong. It should be IModel<?
>>>>> extends
>>>>> List<T>> model: we don't care what type the list is, but we do
>>>>> care about
>>>>> the type of the contents of the list. ListView exposes a
>>>>> read-write view
>>>>> on the contents of the list, via ListView.getModelObject(), but also
>>>>> ListItem.setModelObject. IMHO it should either be read-only with ?
>>>>> extends
>>>>> T or read-write with T, but mixed. The first is a major API break,
>>>>> the
>>>>> second is not, so I prefer the second.
>>>>>
>>>>> Best regards,
>>>>> Emond
>>>>>
>>>>> On Monday 22 June 2015 14:22:25 Sven Meier wrote:
>>>>>> Hi Martijn,
>>>>>>
>>>>>> the ListView constructor is now as it has been in Wicket 6:
>>>>>> public ListView(final String id, final IModel<? extends
>>>>>> List<?
>>>>>>
>>>>>> extends T>> model)
>>>>>>
>>>>>> ListViewTest#generics() shows a valid use case, that is not possible
>>>>>> otherwise.
>>>>>>
>>>>>> Regards
>>>>>> Sven
>>>>>>
>>>>>> On 22.06.2015 13:42, Martijn Dashorst wrote:
>>>>>>> I'm not sure I'm fan of this change. Using these wildcards
>>>>>>> breaks all
>>>>>>> kinds of code. What is the benefit?
>>>>>>>
>>>>>>> The way it is implemented currently is also inconsistent:
>>>>>>> ListItem is
>>>>>>> typed as ListItem<T> but it should be ListItem<? extends T>. This
>>>>>>> gist: https://gist.github.com/dashorst/4ee7ab1696321f290a24
>>>>>>> shows how
>>>>>>> this should be implemented.
>>>>>>>
>>>>>>> HOWEVER: I don't actually propose such a change, but rather have
>>>>>>> adcb7a632af8225e86e09e398b8fb5430b143b18 be reverted. The linked
>>>>>>> patch
>>>>>>> will break the world and for little to no benefit.
>>>>>>> adcb7a632af8225e86e09e398b8fb5430b143b18 breaks API in a couple of
>>>>>>> places, but I don't see the benefit of those breaks as well.
>>>>>>>
>>>>>>> Martijn
>>
>>
>
Re: git commit: WICKET-5350 reintroduce wildcards for repeater over
models, otherwise subclasses is hindered
Posted by Sven Meier <sv...@meiers.net>.
I don't mind the change. Please create a new issue for this improvement.
Many thanks
Sven
On 22.06.2015 16:05, Martijn Dashorst wrote:
> This patch https://gist.github.com/dashorst/eb84199f86e109728dce fixes the API.
>
> All in all it took for our 1.2M lines of code project roughly 10
> minutes to fix the breakages, improving the API considerably,
> discovering and fixing 1 wrongly typed listview and in total 16
> compile errors. This is with about 550 ListView occurrences in total
> across the whole application.
>
> I'm +1 for this patch.
>
> Martijn
>
>
> On Mon, Jun 22, 2015 at 3:46 PM, Emond Papegaaij
> <em...@topicus.nl> wrote:
>> Hi,
>>
>> As Martijn said: the current form limits the user in a way that is not needed.
>> The returned List is not read-only, forcing users to keep a reference to the
>> IModel to be able to update the List. Also, it's inconsistent. populateItem
>> still uses ListItem<T>, not <? extends T>. Changing the constructors, setList,
>> getList and getModelObject to List<T> and IModel<? extends List<T>> seems a
>> minor API to me, making the API much more consistent and flexible.
>>
>> Best regards,
>> Emond
>>
>> On Monday 22 June 2015 15:36:25 Sven Meier wrote:
>>> Hi,
>>>
>>> this is the relevant discussion why I reverted the ListView constructor
>>> to that of Wicket 6:
>>>
>>> http://mail-archives.apache.org/mod_mbox/wicket-dev/201409.mbox/%3CCAJmbs8gD
>>> a5mJgwbkoOZS3oH5TYZZ-Ap3_SFDjBHs5SYpn4zTkg%40mail.gmail.com%3E
>>>
>>> Changing it would mean an API break for existing applications. I don't mind
>>>
>>>> ListView exposes a read-write view on the contents of the list, via
>>>> ListView.getModelObject(), but also ListItem.setModelObject
>>> I wanted ListItem to be read-only, but Martin an I agreed on keeping it
>>> writeable for backwards-compatibility. Is this really a problem we have to
>>> fix in our API?
>>>
>>> Regards
>>> Sven
>>>
>>> On 22.06.2015 14:31, Emond Papegaaij wrote:
>>>> Hi Sven,
>>>>
>>>> It's easy to change the testcase to:
>>>> class NumberListView<N extends Number> extends ListView<N>
>>>>
>>>> and
>>>>
>>>> new NumberListView<Integer>("integers", integers)
>>>>
>>>> I think the constructor in Wicket 6 is wrong. It should be IModel<?
>>>> extends
>>>> List<T>> model: we don't care what type the list is, but we do care about
>>>> the type of the contents of the list. ListView exposes a read-write view
>>>> on the contents of the list, via ListView.getModelObject(), but also
>>>> ListItem.setModelObject. IMHO it should either be read-only with ? extends
>>>> T or read-write with T, but mixed. The first is a major API break, the
>>>> second is not, so I prefer the second.
>>>>
>>>> Best regards,
>>>> Emond
>>>>
>>>> On Monday 22 June 2015 14:22:25 Sven Meier wrote:
>>>>> Hi Martijn,
>>>>>
>>>>> the ListView constructor is now as it has been in Wicket 6:
>>>>> public ListView(final String id, final IModel<? extends List<?
>>>>>
>>>>> extends T>> model)
>>>>>
>>>>> ListViewTest#generics() shows a valid use case, that is not possible
>>>>> otherwise.
>>>>>
>>>>> Regards
>>>>> Sven
>>>>>
>>>>> On 22.06.2015 13:42, Martijn Dashorst wrote:
>>>>>> I'm not sure I'm fan of this change. Using these wildcards breaks all
>>>>>> kinds of code. What is the benefit?
>>>>>>
>>>>>> The way it is implemented currently is also inconsistent: ListItem is
>>>>>> typed as ListItem<T> but it should be ListItem<? extends T>. This
>>>>>> gist: https://gist.github.com/dashorst/4ee7ab1696321f290a24 shows how
>>>>>> this should be implemented.
>>>>>>
>>>>>> HOWEVER: I don't actually propose such a change, but rather have
>>>>>> adcb7a632af8225e86e09e398b8fb5430b143b18 be reverted. The linked patch
>>>>>> will break the world and for little to no benefit.
>>>>>> adcb7a632af8225e86e09e398b8fb5430b143b18 breaks API in a couple of
>>>>>> places, but I don't see the benefit of those breaks as well.
>>>>>>
>>>>>> Martijn
>
>
Re: git commit: WICKET-5350 reintroduce wildcards for repeater over
models, otherwise subclasses is hindered
Posted by Martijn Dashorst <ma...@gmail.com>.
This patch https://gist.github.com/dashorst/eb84199f86e109728dce fixes the API.
All in all it took for our 1.2M lines of code project roughly 10
minutes to fix the breakages, improving the API considerably,
discovering and fixing 1 wrongly typed listview and in total 16
compile errors. This is with about 550 ListView occurrences in total
across the whole application.
I'm +1 for this patch.
Martijn
On Mon, Jun 22, 2015 at 3:46 PM, Emond Papegaaij
<em...@topicus.nl> wrote:
> Hi,
>
> As Martijn said: the current form limits the user in a way that is not needed.
> The returned List is not read-only, forcing users to keep a reference to the
> IModel to be able to update the List. Also, it's inconsistent. populateItem
> still uses ListItem<T>, not <? extends T>. Changing the constructors, setList,
> getList and getModelObject to List<T> and IModel<? extends List<T>> seems a
> minor API to me, making the API much more consistent and flexible.
>
> Best regards,
> Emond
>
> On Monday 22 June 2015 15:36:25 Sven Meier wrote:
>> Hi,
>>
>> this is the relevant discussion why I reverted the ListView constructor
>> to that of Wicket 6:
>>
>> http://mail-archives.apache.org/mod_mbox/wicket-dev/201409.mbox/%3CCAJmbs8gD
>> a5mJgwbkoOZS3oH5TYZZ-Ap3_SFDjBHs5SYpn4zTkg%40mail.gmail.com%3E
>>
>> Changing it would mean an API break for existing applications. I don't mind
>>
>> > ListView exposes a read-write view on the contents of the list, via
>> > ListView.getModelObject(), but also ListItem.setModelObject
>>
>> I wanted ListItem to be read-only, but Martin an I agreed on keeping it
>> writeable for backwards-compatibility. Is this really a problem we have to
>> fix in our API?
>>
>> Regards
>> Sven
>>
>> On 22.06.2015 14:31, Emond Papegaaij wrote:
>> > Hi Sven,
>> >
>> > It's easy to change the testcase to:
>> > class NumberListView<N extends Number> extends ListView<N>
>> >
>> > and
>> >
>> > new NumberListView<Integer>("integers", integers)
>> >
>> > I think the constructor in Wicket 6 is wrong. It should be IModel<?
>> > extends
>> > List<T>> model: we don't care what type the list is, but we do care about
>> > the type of the contents of the list. ListView exposes a read-write view
>> > on the contents of the list, via ListView.getModelObject(), but also
>> > ListItem.setModelObject. IMHO it should either be read-only with ? extends
>> > T or read-write with T, but mixed. The first is a major API break, the
>> > second is not, so I prefer the second.
>> >
>> > Best regards,
>> > Emond
>> >
>> > On Monday 22 June 2015 14:22:25 Sven Meier wrote:
>> >> Hi Martijn,
>> >>
>> >> the ListView constructor is now as it has been in Wicket 6:
>> >> public ListView(final String id, final IModel<? extends List<?
>> >>
>> >> extends T>> model)
>> >>
>> >> ListViewTest#generics() shows a valid use case, that is not possible
>> >> otherwise.
>> >>
>> >> Regards
>> >> Sven
>> >>
>> >> On 22.06.2015 13:42, Martijn Dashorst wrote:
>> >>> I'm not sure I'm fan of this change. Using these wildcards breaks all
>> >>> kinds of code. What is the benefit?
>> >>>
>> >>> The way it is implemented currently is also inconsistent: ListItem is
>> >>> typed as ListItem<T> but it should be ListItem<? extends T>. This
>> >>> gist: https://gist.github.com/dashorst/4ee7ab1696321f290a24 shows how
>> >>> this should be implemented.
>> >>>
>> >>> HOWEVER: I don't actually propose such a change, but rather have
>> >>> adcb7a632af8225e86e09e398b8fb5430b143b18 be reverted. The linked patch
>> >>> will break the world and for little to no benefit.
>> >>> adcb7a632af8225e86e09e398b8fb5430b143b18 breaks API in a couple of
>> >>> places, but I don't see the benefit of those breaks as well.
>> >>>
>> >>> Martijn
>
--
Become a Wicket expert, learn from the best: http://wicketinaction.com
Re: git commit: WICKET-5350 reintroduce wildcards for repeater over
models, otherwise subclasses is hindered
Posted by Sven Meier <sv...@meiers.net>.
I'd say changing the new constructor will limit the user in a way that
is not needed (as the Maxim's mail has shown).
>The returned List is not read-only, forcing users to keep a reference
You mean 'read-only'?
> Also, it's inconsistent.
Yes, that's right.
Have fun
Sven
On 22.06.2015 15:46, Emond Papegaaij wrote:
> Hi,
>
> As Martijn said: the current form limits the user in a way that is not needed.
> The returned List is not read-only, forcing users to keep a reference to the
> IModel to be able to update the List. Also, it's inconsistent. populateItem
> still uses ListItem<T>, not <? extends T>. Changing the constructors, setList,
> getList and getModelObject to List<T> and IModel<? extends List<T>> seems a
> minor API to me, making the API much more consistent and flexible.
>
> Best regards,
> Emond
>
> On Monday 22 June 2015 15:36:25 Sven Meier wrote:
>> Hi,
>>
>> this is the relevant discussion why I reverted the ListView constructor
>> to that of Wicket 6:
>>
>> http://mail-archives.apache.org/mod_mbox/wicket-dev/201409.mbox/%3CCAJmbs8gD
>> a5mJgwbkoOZS3oH5TYZZ-Ap3_SFDjBHs5SYpn4zTkg%40mail.gmail.com%3E
>>
>> Changing it would mean an API break for existing applications. I don't mind
>>
>>> ListView exposes a read-write view on the contents of the list, via
>>> ListView.getModelObject(), but also ListItem.setModelObject
>> I wanted ListItem to be read-only, but Martin an I agreed on keeping it
>> writeable for backwards-compatibility. Is this really a problem we have to
>> fix in our API?
>>
>> Regards
>> Sven
>>
>> On 22.06.2015 14:31, Emond Papegaaij wrote:
>>> Hi Sven,
>>>
>>> It's easy to change the testcase to:
>>> class NumberListView<N extends Number> extends ListView<N>
>>>
>>> and
>>>
>>> new NumberListView<Integer>("integers", integers)
>>>
>>> I think the constructor in Wicket 6 is wrong. It should be IModel<?
>>> extends
>>> List<T>> model: we don't care what type the list is, but we do care about
>>> the type of the contents of the list. ListView exposes a read-write view
>>> on the contents of the list, via ListView.getModelObject(), but also
>>> ListItem.setModelObject. IMHO it should either be read-only with ? extends
>>> T or read-write with T, but mixed. The first is a major API break, the
>>> second is not, so I prefer the second.
>>>
>>> Best regards,
>>> Emond
>>>
>>> On Monday 22 June 2015 14:22:25 Sven Meier wrote:
>>>> Hi Martijn,
>>>>
>>>> the ListView constructor is now as it has been in Wicket 6:
>>>> public ListView(final String id, final IModel<? extends List<?
>>>>
>>>> extends T>> model)
>>>>
>>>> ListViewTest#generics() shows a valid use case, that is not possible
>>>> otherwise.
>>>>
>>>> Regards
>>>> Sven
>>>>
>>>> On 22.06.2015 13:42, Martijn Dashorst wrote:
>>>>> I'm not sure I'm fan of this change. Using these wildcards breaks all
>>>>> kinds of code. What is the benefit?
>>>>>
>>>>> The way it is implemented currently is also inconsistent: ListItem is
>>>>> typed as ListItem<T> but it should be ListItem<? extends T>. This
>>>>> gist: https://gist.github.com/dashorst/4ee7ab1696321f290a24 shows how
>>>>> this should be implemented.
>>>>>
>>>>> HOWEVER: I don't actually propose such a change, but rather have
>>>>> adcb7a632af8225e86e09e398b8fb5430b143b18 be reverted. The linked patch
>>>>> will break the world and for little to no benefit.
>>>>> adcb7a632af8225e86e09e398b8fb5430b143b18 breaks API in a couple of
>>>>> places, but I don't see the benefit of those breaks as well.
>>>>>
>>>>> Martijn
Re: git commit: WICKET-5350 reintroduce wildcards for repeater over models, otherwise subclasses is hindered
Posted by Emond Papegaaij <em...@topicus.nl>.
Hi,
As Martijn said: the current form limits the user in a way that is not needed.
The returned List is not read-only, forcing users to keep a reference to the
IModel to be able to update the List. Also, it's inconsistent. populateItem
still uses ListItem<T>, not <? extends T>. Changing the constructors, setList,
getList and getModelObject to List<T> and IModel<? extends List<T>> seems a
minor API to me, making the API much more consistent and flexible.
Best regards,
Emond
On Monday 22 June 2015 15:36:25 Sven Meier wrote:
> Hi,
>
> this is the relevant discussion why I reverted the ListView constructor
> to that of Wicket 6:
>
> http://mail-archives.apache.org/mod_mbox/wicket-dev/201409.mbox/%3CCAJmbs8gD
> a5mJgwbkoOZS3oH5TYZZ-Ap3_SFDjBHs5SYpn4zTkg%40mail.gmail.com%3E
>
> Changing it would mean an API break for existing applications. I don't mind
>
> > ListView exposes a read-write view on the contents of the list, via
> > ListView.getModelObject(), but also ListItem.setModelObject
>
> I wanted ListItem to be read-only, but Martin an I agreed on keeping it
> writeable for backwards-compatibility. Is this really a problem we have to
> fix in our API?
>
> Regards
> Sven
>
> On 22.06.2015 14:31, Emond Papegaaij wrote:
> > Hi Sven,
> >
> > It's easy to change the testcase to:
> > class NumberListView<N extends Number> extends ListView<N>
> >
> > and
> >
> > new NumberListView<Integer>("integers", integers)
> >
> > I think the constructor in Wicket 6 is wrong. It should be IModel<?
> > extends
> > List<T>> model: we don't care what type the list is, but we do care about
> > the type of the contents of the list. ListView exposes a read-write view
> > on the contents of the list, via ListView.getModelObject(), but also
> > ListItem.setModelObject. IMHO it should either be read-only with ? extends
> > T or read-write with T, but mixed. The first is a major API break, the
> > second is not, so I prefer the second.
> >
> > Best regards,
> > Emond
> >
> > On Monday 22 June 2015 14:22:25 Sven Meier wrote:
> >> Hi Martijn,
> >>
> >> the ListView constructor is now as it has been in Wicket 6:
> >> public ListView(final String id, final IModel<? extends List<?
> >>
> >> extends T>> model)
> >>
> >> ListViewTest#generics() shows a valid use case, that is not possible
> >> otherwise.
> >>
> >> Regards
> >> Sven
> >>
> >> On 22.06.2015 13:42, Martijn Dashorst wrote:
> >>> I'm not sure I'm fan of this change. Using these wildcards breaks all
> >>> kinds of code. What is the benefit?
> >>>
> >>> The way it is implemented currently is also inconsistent: ListItem is
> >>> typed as ListItem<T> but it should be ListItem<? extends T>. This
> >>> gist: https://gist.github.com/dashorst/4ee7ab1696321f290a24 shows how
> >>> this should be implemented.
> >>>
> >>> HOWEVER: I don't actually propose such a change, but rather have
> >>> adcb7a632af8225e86e09e398b8fb5430b143b18 be reverted. The linked patch
> >>> will break the world and for little to no benefit.
> >>> adcb7a632af8225e86e09e398b8fb5430b143b18 breaks API in a couple of
> >>> places, but I don't see the benefit of those breaks as well.
> >>>
> >>> Martijn
Re: git commit: WICKET-5350 reintroduce wildcards for repeater over
models, otherwise subclasses is hindered
Posted by Sven Meier <sv...@meiers.net>.
Hi,
this is the relevant discussion why I reverted the ListView constructor
to that of Wicket 6:
http://mail-archives.apache.org/mod_mbox/wicket-dev/201409.mbox/%3CCAJmbs8gDa5mJgwbkoOZS3oH5TYZZ-Ap3_SFDjBHs5SYpn4zTkg%40mail.gmail.com%3E
Changing it would mean an API break for existing applications. I don't mind
> ListView exposes a read-write view on the contents of the list, via ListView.getModelObject(),
> but also ListItem.setModelObject
I wanted ListItem to be read-only, but Martin an I agreed on keeping it writeable for backwards-compatibility.
Is this really a problem we have to fix in our API?
Regards
Sven
On 22.06.2015 14:31, Emond Papegaaij wrote:
> Hi Sven,
>
> It's easy to change the testcase to:
> class NumberListView<N extends Number> extends ListView<N>
> and
> new NumberListView<Integer>("integers", integers)
>
> I think the constructor in Wicket 6 is wrong. It should be IModel<? extends
> List<T>> model: we don't care what type the list is, but we do care about the
> type of the contents of the list. ListView exposes a read-write view on the
> contents of the list, via ListView.getModelObject(), but also
> ListItem.setModelObject. IMHO it should either be read-only with ? extends T
> or read-write with T, but mixed. The first is a major API break, the second is
> not, so I prefer the second.
>
> Best regards,
> Emond
>
> On Monday 22 June 2015 14:22:25 Sven Meier wrote:
>> Hi Martijn,
>>
>> the ListView constructor is now as it has been in Wicket 6:
>>
>> public ListView(final String id, final IModel<? extends List<?
>> extends T>> model)
>>
>> ListViewTest#generics() shows a valid use case, that is not possible
>> otherwise.
>>
>> Regards
>> Sven
>>
>> On 22.06.2015 13:42, Martijn Dashorst wrote:
>>> I'm not sure I'm fan of this change. Using these wildcards breaks all
>>> kinds of code. What is the benefit?
>>>
>>> The way it is implemented currently is also inconsistent: ListItem is
>>> typed as ListItem<T> but it should be ListItem<? extends T>. This
>>> gist: https://gist.github.com/dashorst/4ee7ab1696321f290a24 shows how
>>> this should be implemented.
>>>
>>> HOWEVER: I don't actually propose such a change, but rather have
>>> adcb7a632af8225e86e09e398b8fb5430b143b18 be reverted. The linked patch
>>> will break the world and for little to no benefit.
>>> adcb7a632af8225e86e09e398b8fb5430b143b18 breaks API in a couple of
>>> places, but I don't see the benefit of those breaks as well.
>>>
>>> Martijn
Re: git commit: WICKET-5350 reintroduce wildcards for repeater over models, otherwise subclasses is hindered
Posted by Emond Papegaaij <em...@topicus.nl>.
Hi Sven,
It's easy to change the testcase to:
class NumberListView<N extends Number> extends ListView<N>
and
new NumberListView<Integer>("integers", integers)
I think the constructor in Wicket 6 is wrong. It should be IModel<? extends
List<T>> model: we don't care what type the list is, but we do care about the
type of the contents of the list. ListView exposes a read-write view on the
contents of the list, via ListView.getModelObject(), but also
ListItem.setModelObject. IMHO it should either be read-only with ? extends T
or read-write with T, but mixed. The first is a major API break, the second is
not, so I prefer the second.
Best regards,
Emond
On Monday 22 June 2015 14:22:25 Sven Meier wrote:
> Hi Martijn,
>
> the ListView constructor is now as it has been in Wicket 6:
>
> public ListView(final String id, final IModel<? extends List<?
> extends T>> model)
>
> ListViewTest#generics() shows a valid use case, that is not possible
> otherwise.
>
> Regards
> Sven
>
> On 22.06.2015 13:42, Martijn Dashorst wrote:
> > I'm not sure I'm fan of this change. Using these wildcards breaks all
> > kinds of code. What is the benefit?
> >
> > The way it is implemented currently is also inconsistent: ListItem is
> > typed as ListItem<T> but it should be ListItem<? extends T>. This
> > gist: https://gist.github.com/dashorst/4ee7ab1696321f290a24 shows how
> > this should be implemented.
> >
> > HOWEVER: I don't actually propose such a change, but rather have
> > adcb7a632af8225e86e09e398b8fb5430b143b18 be reverted. The linked patch
> > will break the world and for little to no benefit.
> > adcb7a632af8225e86e09e398b8fb5430b143b18 breaks API in a couple of
> > places, but I don't see the benefit of those breaks as well.
> >
> > Martijn
Re: git commit: WICKET-5350 reintroduce wildcards for repeater over
models, otherwise subclasses is hindered
Posted by Sven Meier <sv...@meiers.net>.
Hi Martijn,
the ListView constructor is now as it has been in Wicket 6:
public ListView(final String id, final IModel<? extends List<?
extends T>> model)
ListViewTest#generics() shows a valid use case, that is not possible
otherwise.
Regards
Sven
On 22.06.2015 13:42, Martijn Dashorst wrote:
> I'm not sure I'm fan of this change. Using these wildcards breaks all
> kinds of code. What is the benefit?
>
> The way it is implemented currently is also inconsistent: ListItem is
> typed as ListItem<T> but it should be ListItem<? extends T>. This
> gist: https://gist.github.com/dashorst/4ee7ab1696321f290a24 shows how
> this should be implemented.
>
> HOWEVER: I don't actually propose such a change, but rather have
> adcb7a632af8225e86e09e398b8fb5430b143b18 be reverted. The linked patch
> will break the world and for little to no benefit.
> adcb7a632af8225e86e09e398b8fb5430b143b18 breaks API in a couple of
> places, but I don't see the benefit of those breaks as well.
>
> Martijn
Re: git commit: WICKET-5350 reintroduce wildcards for repeater over
models, otherwise subclasses is hindered
Posted by Martijn Dashorst <ma...@gmail.com>.
I'm not sure I'm fan of this change. Using these wildcards breaks all
kinds of code. What is the benefit?
The way it is implemented currently is also inconsistent: ListItem is
typed as ListItem<T> but it should be ListItem<? extends T>. This
gist: https://gist.github.com/dashorst/4ee7ab1696321f290a24 shows how
this should be implemented.
HOWEVER: I don't actually propose such a change, but rather have
adcb7a632af8225e86e09e398b8fb5430b143b18 be reverted. The linked patch
will break the world and for little to no benefit.
adcb7a632af8225e86e09e398b8fb5430b143b18 breaks API in a couple of
places, but I don't see the benefit of those breaks as well.
Martijn
Re: git commit: WICKET-5350 reintroduce wildcards for repeater over
models, otherwise subclasses is hindered
Posted by Martin Grigorov <mg...@apache.org>.
On Thu, Sep 25, 2014 at 2:57 PM, Sven Meier <sv...@meiers.net> wrote:
> 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 makes it
> more safer than before!
>
> Indeed.
> On second thought we could keep the setter on ListItemModel though: Put a
> warning in its javadoc and mark it deprecated.
> This way users will have more time to improve their code.
>
+1
>
> 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 <sv...@apache.org>
>> 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, <sv...@apache.org> 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 <sv...@meiers.net>
>>>>> Authored: Wed Sep 24 22:05:04 2014 +0200
>>>>> Committer: svenmeier <sv...@meiers.net>
>>>>> 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
Posted by Sven Meier <sv...@meiers.net>.
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 makes
it more safer than before!
Indeed.
On second thought we could keep the setter on ListItemModel though: Put
a 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 <sv...@apache.org> 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, <sv...@apache.org> 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 <sv...@meiers.net>
>>>> Authored: Wed Sep 24 22:05:04 2014 +0200
>>>> Committer: svenmeier <sv...@meiers.net>
>>>> 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
Posted by Martin Grigorov <mg...@apache.org>.
Hi Sven,
On Wed, Sep 24, 2014 at 11:11 PM, Sven Meier <sv...@apache.org> 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, <sv...@apache.org> 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 <sv...@meiers.net>
>>> Authored: Wed Sep 24 22:05:04 2014 +0200
>>> Committer: svenmeier <sv...@meiers.net>
>>> 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
Posted by Sven Meier <sv...@apache.org>.
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.
Regards
Sven
On 09/24/2014 10:19 PM, Martin Grigorov wrote:
> Hi Sven,
>
> On Wed, Sep 24, 2014 at 10:05 PM, <sv...@apache.org> 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 <sv...@meiers.net>
>> Authored: Wed Sep 24 22:05:04 2014 +0200
>> Committer: svenmeier <sv...@meiers.net>
>> 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
Posted by Martin Grigorov <mg...@apache.org>.
Hi Sven,
On Wed, Sep 24, 2014 at 10:05 PM, <sv...@apache.org> 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 <sv...@meiers.net>
> Authored: Wed Sep 24 22:05:04 2014 +0200
> Committer: svenmeier <sv...@meiers.net>
> 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)
> {
>
>