You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by "Sergiy Yevtushenko (JIRA)" <ji...@apache.org> on 2008/02/07 20:51:08 UTC

[jira] Created: (WICKET-1331) getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly

getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly
-------------------------------------------------------------------------------------------------------------------

                 Key: WICKET-1331
                 URL: https://issues.apache.org/jira/browse/WICKET-1331
             Project: Wicket
          Issue Type: Improvement
          Components: wicket
    Affects Versions: 1.3.0-final, 1.3.1
         Environment: Suse 10.3, JRE 1.6 (1.6.0_04-b12), jetty 6.1.7
            Reporter: Sergiy Yevtushenko


AbstractSingleSelectChoice.getModelValue() implementation uses List.indexOf() to find the key and this causes problems if list of choices contains complex values rather than simple list of String instances. In this case indexOf() returns -1 and this can't be resolved by overriding equals() for list elements. This happens because internally AbstractList.indexOf() invokes equals() method of passed key value passing it list items as a parameter. Also, current implementation may pass key returned by getModelObject() to IChoiceRender, while it expects values from list of items. Correct implementation of this method may look so:
-------------------------
	public String getModelValue()
	{
		// @@ Modified by SIY
		Object object = getModelObject();
		if (object != null)
		{
			// compare choice items with given keys and pass down
			// to IChoiceRenderer list item rather than key
			Iterator iter = getChoices().iterator();

			int i = 0;

			while (iter.hasNext())
			{
				Object obj = iter.next();

				if (obj != null && obj.equals(object))
				{
					object = obj;
					break;
				}
				i++;
			}

			if (i > getChoices().size())
				i = -1;

			return getChoiceRenderer().getIdValue(object, i);
		}
		return NO_SELECTION_VALUE;
	}
-----------------------------------
Similar issues also present in ListMultipleChoice.getModelValue(), but they can't be resolved by overriding this method in subclass because this method declared final.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (WICKET-1331) getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly

Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/WICKET-1331?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Igor Vaynberg resolved WICKET-1331.
-----------------------------------

    Resolution: Won't Fix

i dont see how indexof() is broken for complex types, especially since i rarely use choice components with collections of strings. please provide a working example of where this fails for you in a form of a quickstart. thanks.

> getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: WICKET-1331
>                 URL: https://issues.apache.org/jira/browse/WICKET-1331
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.3.0-final, 1.3.1
>         Environment: Suse 10.3, JRE 1.6 (1.6.0_04-b12), jetty 6.1.7
>            Reporter: Sergiy Yevtushenko
>            Assignee: Igor Vaynberg
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> AbstractSingleSelectChoice.getModelValue() implementation uses List.indexOf() to find the key and this causes problems if list of choices contains complex values rather than simple list of String instances. In this case indexOf() returns -1 and this can't be resolved by overriding equals() for list elements. This happens because internally AbstractList.indexOf() invokes equals() method of passed key value passing it list items as a parameter. Also, current implementation may pass key returned by getModelObject() to IChoiceRender, while it expects values from list of items. Correct implementation of this method may look so:
> -------------------------
> 	public String getModelValue()
> 	{
> 		// @@ Modified by SIY
> 		Object object = getModelObject();
> 		if (object != null)
> 		{
> 			// compare choice items with given keys and pass down
> 			// to IChoiceRenderer list item rather than key
> 			Iterator iter = getChoices().iterator();
> 			int i = 0;
> 			while (iter.hasNext())
> 			{
> 				Object obj = iter.next();
> 				if (obj != null && obj.equals(object))
> 				{
> 					object = obj;
> 					break;
> 				}
> 				i++;
> 			}
> 			if (i > getChoices().size())
> 				i = -1;
> 			return getChoiceRenderer().getIdValue(object, i);
> 		}
> 		return NO_SELECTION_VALUE;
> 	}
> -----------------------------------
> Similar issues also present in ListMultipleChoice.getModelValue(), but they can't be resolved by overriding this method in subclass because this method declared final.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WICKET-1331) getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly

Posted by "Sergiy Yevtushenko (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-1331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12567814#action_12567814 ] 

Sergiy Yevtushenko commented on WICKET-1331:
--------------------------------------------

I'm in situation when model object contains values convenient for processing and storing, while user should see more verbose, human readable choice items. Also, I'm about to implement another similar case - model object contains numbers from predefined set of non-sequential values while for user they are presented as descriptive text strings. In all these situations it is convenient to let DropDownChoice maintain and automatically handle conversion, especially if to take into account that it already support almost all necessary functionality. At present this will require custom ChoiceRenderer for each such property, while it can be done is one piece of generic code.

> getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: WICKET-1331
>                 URL: https://issues.apache.org/jira/browse/WICKET-1331
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.3.0-final, 1.3.1
>         Environment: Suse 10.3, JRE 1.6 (1.6.0_04-b12), jetty 6.1.7
>            Reporter: Sergiy Yevtushenko
>            Assignee: Igor Vaynberg
>         Attachments: testproject.zip
>
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> AbstractSingleSelectChoice.getModelValue() implementation uses List.indexOf() to find the key and this causes problems if list of choices contains complex values rather than simple list of String instances. In this case indexOf() returns -1 and this can't be resolved by overriding equals() for list elements. This happens because internally AbstractList.indexOf() invokes equals() method of passed key value passing it list items as a parameter. Also, current implementation may pass key returned by getModelObject() to IChoiceRender, while it expects values from list of items. Correct implementation of this method may look so:
> -------------------------
> 	public String getModelValue()
> 	{
> 		// @@ Modified by SIY
> 		Object object = getModelObject();
> 		if (object != null)
> 		{
> 			// compare choice items with given keys and pass down
> 			// to IChoiceRenderer list item rather than key
> 			Iterator iter = getChoices().iterator();
> 			int i = 0;
> 			while (iter.hasNext())
> 			{
> 				Object obj = iter.next();
> 				if (obj != null && obj.equals(object))
> 				{
> 					object = obj;
> 					break;
> 				}
> 				i++;
> 			}
> 			if (i > getChoices().size())
> 				i = -1;
> 			return getChoiceRenderer().getIdValue(object, i);
> 		}
> 		return NO_SELECTION_VALUE;
> 	}
> -----------------------------------
> Similar issues also present in ListMultipleChoice.getModelValue(), but they can't be resolved by overriding this method in subclass because this method declared final.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WICKET-1331) getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly

Posted by "Sergiy Yevtushenko (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-1331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12567597#action_12567597 ] 

Sergiy Yevtushenko commented on WICKET-1331:
--------------------------------------------

I've attached test project based on quickstart which exposes the issue - attempt to access home page fails with following exception (relevant part):

org.apache.wicket.WicketRuntimeException: No get method defined for class: class java.lang.Long expression: id
     at org.apache.wicket.util.lang.PropertyResolver.getGetAndSetter(PropertyResolver.java:433)
     at org.apache.wicket.util.lang.PropertyResolver.getObjectAndGetSetter(PropertyResolver.java:275)
     at org.apache.wicket.util.lang.PropertyResolver.getValue(PropertyResolver.java:84)
     at org.apache.wicket.markup.html.form.ChoiceRenderer.getIdValue(ChoiceRenderer.java:140)
     at org.apache.wicket.markup.html.form.AbstractSingleSelectChoice.getModelValue(AbstractSingleSelectChoice.java:144)
     at org.apache.wicket.markup.html.form.FormComponent.getValue(FormComponent.java:744)
     at org.apache.wicket.markup.html.form.AbstractChoice.onComponentTagBody(AbstractChoice.java:344)
...

If in HomePage.java make following changes:

...
        // default AbstractSingleSelectChoice.getModelValue():
        //form.add(new DropDownChoice("option", getList(), new ChoiceRenderer("name", "id"))); <-----

        // Overridden AbstractSingleSelectChoice.getModelValue();
        form.add(new ModifiedDropDownChoice("option", getList(), new ChoiceRenderer("name", "id"))); //<-----
...

then home page is rendered properly. Also, this test application seems expose another issue which I didn't see before - attempt to submit for traps. Debugging shows that in this case entire choice list item is passed without conversion. I've not digged deeper.

> getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: WICKET-1331
>                 URL: https://issues.apache.org/jira/browse/WICKET-1331
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.3.0-final, 1.3.1
>         Environment: Suse 10.3, JRE 1.6 (1.6.0_04-b12), jetty 6.1.7
>            Reporter: Sergiy Yevtushenko
>            Assignee: Igor Vaynberg
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> AbstractSingleSelectChoice.getModelValue() implementation uses List.indexOf() to find the key and this causes problems if list of choices contains complex values rather than simple list of String instances. In this case indexOf() returns -1 and this can't be resolved by overriding equals() for list elements. This happens because internally AbstractList.indexOf() invokes equals() method of passed key value passing it list items as a parameter. Also, current implementation may pass key returned by getModelObject() to IChoiceRender, while it expects values from list of items. Correct implementation of this method may look so:
> -------------------------
> 	public String getModelValue()
> 	{
> 		// @@ Modified by SIY
> 		Object object = getModelObject();
> 		if (object != null)
> 		{
> 			// compare choice items with given keys and pass down
> 			// to IChoiceRenderer list item rather than key
> 			Iterator iter = getChoices().iterator();
> 			int i = 0;
> 			while (iter.hasNext())
> 			{
> 				Object obj = iter.next();
> 				if (obj != null && obj.equals(object))
> 				{
> 					object = obj;
> 					break;
> 				}
> 				i++;
> 			}
> 			if (i > getChoices().size())
> 				i = -1;
> 			return getChoiceRenderer().getIdValue(object, i);
> 		}
> 		return NO_SELECTION_VALUE;
> 	}
> -----------------------------------
> Similar issues also present in ListMultipleChoice.getModelValue(), but they can't be resolved by overriding this method in subclass because this method declared final.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (WICKET-1331) getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly

Posted by "Sergiy Yevtushenko (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/WICKET-1331?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sergiy Yevtushenko updated WICKET-1331:
---------------------------------------

    Attachment: testproject.zip

Test project which exposes the issue

> getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: WICKET-1331
>                 URL: https://issues.apache.org/jira/browse/WICKET-1331
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.3.0-final, 1.3.1
>         Environment: Suse 10.3, JRE 1.6 (1.6.0_04-b12), jetty 6.1.7
>            Reporter: Sergiy Yevtushenko
>            Assignee: Igor Vaynberg
>         Attachments: testproject.zip
>
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> AbstractSingleSelectChoice.getModelValue() implementation uses List.indexOf() to find the key and this causes problems if list of choices contains complex values rather than simple list of String instances. In this case indexOf() returns -1 and this can't be resolved by overriding equals() for list elements. This happens because internally AbstractList.indexOf() invokes equals() method of passed key value passing it list items as a parameter. Also, current implementation may pass key returned by getModelObject() to IChoiceRender, while it expects values from list of items. Correct implementation of this method may look so:
> -------------------------
> 	public String getModelValue()
> 	{
> 		// @@ Modified by SIY
> 		Object object = getModelObject();
> 		if (object != null)
> 		{
> 			// compare choice items with given keys and pass down
> 			// to IChoiceRenderer list item rather than key
> 			Iterator iter = getChoices().iterator();
> 			int i = 0;
> 			while (iter.hasNext())
> 			{
> 				Object obj = iter.next();
> 				if (obj != null && obj.equals(object))
> 				{
> 					object = obj;
> 					break;
> 				}
> 				i++;
> 			}
> 			if (i > getChoices().size())
> 				i = -1;
> 			return getChoiceRenderer().getIdValue(object, i);
> 		}
> 		return NO_SELECTION_VALUE;
> 	}
> -----------------------------------
> Similar issues also present in ListMultipleChoice.getModelValue(), but they can't be resolved by overriding this method in subclass because this method declared final.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WICKET-1331) getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly

Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-1331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12567834#action_12567834 ] 

Igor Vaynberg commented on WICKET-1331:
---------------------------------------

well, that is your opinion. my opinion is that the rendering of human-readable values should be in the renderer. getmodelvalue() is not final, so you can create your own subclasses that work the way you want - this is the power of wicket. of course if you feel like your way is much better feel free to start a thread on the dev list.

> getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: WICKET-1331
>                 URL: https://issues.apache.org/jira/browse/WICKET-1331
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.3.0-final, 1.3.1
>         Environment: Suse 10.3, JRE 1.6 (1.6.0_04-b12), jetty 6.1.7
>            Reporter: Sergiy Yevtushenko
>            Assignee: Igor Vaynberg
>         Attachments: testproject.zip
>
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> AbstractSingleSelectChoice.getModelValue() implementation uses List.indexOf() to find the key and this causes problems if list of choices contains complex values rather than simple list of String instances. In this case indexOf() returns -1 and this can't be resolved by overriding equals() for list elements. This happens because internally AbstractList.indexOf() invokes equals() method of passed key value passing it list items as a parameter. Also, current implementation may pass key returned by getModelObject() to IChoiceRender, while it expects values from list of items. Correct implementation of this method may look so:
> -------------------------
> 	public String getModelValue()
> 	{
> 		// @@ Modified by SIY
> 		Object object = getModelObject();
> 		if (object != null)
> 		{
> 			// compare choice items with given keys and pass down
> 			// to IChoiceRenderer list item rather than key
> 			Iterator iter = getChoices().iterator();
> 			int i = 0;
> 			while (iter.hasNext())
> 			{
> 				Object obj = iter.next();
> 				if (obj != null && obj.equals(object))
> 				{
> 					object = obj;
> 					break;
> 				}
> 				i++;
> 			}
> 			if (i > getChoices().size())
> 				i = -1;
> 			return getChoiceRenderer().getIdValue(object, i);
> 		}
> 		return NO_SELECTION_VALUE;
> 	}
> -----------------------------------
> Similar issues also present in ListMultipleChoice.getModelValue(), but they can't be resolved by overriding this method in subclass because this method declared final.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WICKET-1331) getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly

Posted by "Sergiy Yevtushenko (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-1331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12567757#action_12567757 ] 

Sergiy Yevtushenko commented on WICKET-1331:
--------------------------------------------

Sorry, I haven't mentioned it, but attached example does show both these issues: 
1) causes a trap "No get method defined for class: class java.lang.Long expression: id" cited above (because ChoiceRenderer tries to resolve property name for simple type)
2) can be seen if replace ChoiceRenderer with custom one (as you have suggested) or just by stepping through getDisplayValue() in debugger - value taken from model does not match any choice item (the List indexOf() is responsible for this, as I've mentioned).


> getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: WICKET-1331
>                 URL: https://issues.apache.org/jira/browse/WICKET-1331
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.3.0-final, 1.3.1
>         Environment: Suse 10.3, JRE 1.6 (1.6.0_04-b12), jetty 6.1.7
>            Reporter: Sergiy Yevtushenko
>            Assignee: Igor Vaynberg
>         Attachments: testproject.zip
>
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> AbstractSingleSelectChoice.getModelValue() implementation uses List.indexOf() to find the key and this causes problems if list of choices contains complex values rather than simple list of String instances. In this case indexOf() returns -1 and this can't be resolved by overriding equals() for list elements. This happens because internally AbstractList.indexOf() invokes equals() method of passed key value passing it list items as a parameter. Also, current implementation may pass key returned by getModelObject() to IChoiceRender, while it expects values from list of items. Correct implementation of this method may look so:
> -------------------------
> 	public String getModelValue()
> 	{
> 		// @@ Modified by SIY
> 		Object object = getModelObject();
> 		if (object != null)
> 		{
> 			// compare choice items with given keys and pass down
> 			// to IChoiceRenderer list item rather than key
> 			Iterator iter = getChoices().iterator();
> 			int i = 0;
> 			while (iter.hasNext())
> 			{
> 				Object obj = iter.next();
> 				if (obj != null && obj.equals(object))
> 				{
> 					object = obj;
> 					break;
> 				}
> 				i++;
> 			}
> 			if (i > getChoices().size())
> 				i = -1;
> 			return getChoiceRenderer().getIdValue(object, i);
> 		}
> 		return NO_SELECTION_VALUE;
> 	}
> -----------------------------------
> Similar issues also present in ListMultipleChoice.getModelValue(), but they can't be resolved by overriding this method in subclass because this method declared final.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WICKET-1331) getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly

Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-1331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12567689#action_12567689 ] 

Igor Vaynberg commented on WICKET-1331:
---------------------------------------

form.add(new DropDownChoice("option", getList(), new ChoiceRenderer("name", "id"))); is not a proper use of ddc if your choices are of type Long. you are using an invalid choice renderer. this choice renderer expects to work on a pojo that has a getId() and getMethod() defined. that is whats causing your error - when the choice renderer tries to call getId() on a member of getList(). instead try a choice renderer like this:

new ichoicerenderer() {
  string getidvalue(Object o) { return ""+o; }
  string getdisplayvalue(Object o) { return ""+o; }
}


> getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: WICKET-1331
>                 URL: https://issues.apache.org/jira/browse/WICKET-1331
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.3.0-final, 1.3.1
>         Environment: Suse 10.3, JRE 1.6 (1.6.0_04-b12), jetty 6.1.7
>            Reporter: Sergiy Yevtushenko
>            Assignee: Igor Vaynberg
>         Attachments: testproject.zip
>
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> AbstractSingleSelectChoice.getModelValue() implementation uses List.indexOf() to find the key and this causes problems if list of choices contains complex values rather than simple list of String instances. In this case indexOf() returns -1 and this can't be resolved by overriding equals() for list elements. This happens because internally AbstractList.indexOf() invokes equals() method of passed key value passing it list items as a parameter. Also, current implementation may pass key returned by getModelObject() to IChoiceRender, while it expects values from list of items. Correct implementation of this method may look so:
> -------------------------
> 	public String getModelValue()
> 	{
> 		// @@ Modified by SIY
> 		Object object = getModelObject();
> 		if (object != null)
> 		{
> 			// compare choice items with given keys and pass down
> 			// to IChoiceRenderer list item rather than key
> 			Iterator iter = getChoices().iterator();
> 			int i = 0;
> 			while (iter.hasNext())
> 			{
> 				Object obj = iter.next();
> 				if (obj != null && obj.equals(object))
> 				{
> 					object = obj;
> 					break;
> 				}
> 				i++;
> 			}
> 			if (i > getChoices().size())
> 				i = -1;
> 			return getChoiceRenderer().getIdValue(object, i);
> 		}
> 		return NO_SELECTION_VALUE;
> 	}
> -----------------------------------
> Similar issues also present in ListMultipleChoice.getModelValue(), but they can't be resolved by overriding this method in subclass because this method declared final.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WICKET-1331) getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly

Posted by "Sergiy Yevtushenko (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-1331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12567787#action_12567787 ] 

Sergiy Yevtushenko commented on WICKET-1331:
--------------------------------------------

As you may notice in test application, choice list contains complex items. Also, updated getDisplayValue() works properly in this case, although everything else remains unchanged. Just try step through both versions of getDisplayValue() in debugger and compare values passed to choice renderer.


> getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: WICKET-1331
>                 URL: https://issues.apache.org/jira/browse/WICKET-1331
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.3.0-final, 1.3.1
>         Environment: Suse 10.3, JRE 1.6 (1.6.0_04-b12), jetty 6.1.7
>            Reporter: Sergiy Yevtushenko
>            Assignee: Igor Vaynberg
>         Attachments: testproject.zip
>
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> AbstractSingleSelectChoice.getModelValue() implementation uses List.indexOf() to find the key and this causes problems if list of choices contains complex values rather than simple list of String instances. In this case indexOf() returns -1 and this can't be resolved by overriding equals() for list elements. This happens because internally AbstractList.indexOf() invokes equals() method of passed key value passing it list items as a parameter. Also, current implementation may pass key returned by getModelObject() to IChoiceRender, while it expects values from list of items. Correct implementation of this method may look so:
> -------------------------
> 	public String getModelValue()
> 	{
> 		// @@ Modified by SIY
> 		Object object = getModelObject();
> 		if (object != null)
> 		{
> 			// compare choice items with given keys and pass down
> 			// to IChoiceRenderer list item rather than key
> 			Iterator iter = getChoices().iterator();
> 			int i = 0;
> 			while (iter.hasNext())
> 			{
> 				Object obj = iter.next();
> 				if (obj != null && obj.equals(object))
> 				{
> 					object = obj;
> 					break;
> 				}
> 				i++;
> 			}
> 			if (i > getChoices().size())
> 				i = -1;
> 			return getChoiceRenderer().getIdValue(object, i);
> 		}
> 		return NO_SELECTION_VALUE;
> 	}
> -----------------------------------
> Similar issues also present in ListMultipleChoice.getModelValue(), but they can't be resolved by overriding this method in subclass because this method declared final.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Issue Comment Edited: (WICKET-1331) getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly

Posted by "Sergiy Yevtushenko (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-1331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12567729#action_12567729 ] 

evsi edited comment on WICKET-1331 at 2/11/08 10:03 AM:
----------------------------------------------------------------------

Initially I met the same issue with POJO which had two string members and symptoms were the same - attempt to resolve property for simple type (that time is was String). Also, I've tried to replace ChoiceRenderer with custom one. This approach resolves a trap, but does not resolve two other issues: 
1. under some circumstances IChoiceRenderer.getDisplayValue() can be invoked with key (field model value) rather than choice list item (this happens when key can't be found in choice list).
2. Initial value set incorrectly (for the same reason - selected item can't be determined).

Proposed version of getModelValue() resolves these issue if choice list item has proper equals() method.

      was (Author: evsi):
    Initially I met the same issue with POJO which has two string members and symptoms were the same - attempt to resolve property for simple type (that time is was String). Also, I've tried to replace ChoiceRenderer with custom one. This approach resolves a trap, but does not resolve two other issues: 
1. under some circumstances IChoiceRenderer.getDisplayValue() can be invoked with key (field model value) rather than choice list item (this happens when key can't be found in choice list).
2. Initial value set incorrectly (for the same reason - selected item can't be determined).

Proposed version of getModelValue() resolves these issue if choice list item has proper equals() method.
  
> getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: WICKET-1331
>                 URL: https://issues.apache.org/jira/browse/WICKET-1331
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.3.0-final, 1.3.1
>         Environment: Suse 10.3, JRE 1.6 (1.6.0_04-b12), jetty 6.1.7
>            Reporter: Sergiy Yevtushenko
>            Assignee: Igor Vaynberg
>         Attachments: testproject.zip
>
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> AbstractSingleSelectChoice.getModelValue() implementation uses List.indexOf() to find the key and this causes problems if list of choices contains complex values rather than simple list of String instances. In this case indexOf() returns -1 and this can't be resolved by overriding equals() for list elements. This happens because internally AbstractList.indexOf() invokes equals() method of passed key value passing it list items as a parameter. Also, current implementation may pass key returned by getModelObject() to IChoiceRender, while it expects values from list of items. Correct implementation of this method may look so:
> -------------------------
> 	public String getModelValue()
> 	{
> 		// @@ Modified by SIY
> 		Object object = getModelObject();
> 		if (object != null)
> 		{
> 			// compare choice items with given keys and pass down
> 			// to IChoiceRenderer list item rather than key
> 			Iterator iter = getChoices().iterator();
> 			int i = 0;
> 			while (iter.hasNext())
> 			{
> 				Object obj = iter.next();
> 				if (obj != null && obj.equals(object))
> 				{
> 					object = obj;
> 					break;
> 				}
> 				i++;
> 			}
> 			if (i > getChoices().size())
> 				i = -1;
> 			return getChoiceRenderer().getIdValue(object, i);
> 		}
> 		return NO_SELECTION_VALUE;
> 	}
> -----------------------------------
> Similar issues also present in ListMultipleChoice.getModelValue(), but they can't be resolved by overriding this method in subclass because this method declared final.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WICKET-1331) getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly

Posted by "Sergiy Yevtushenko (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-1331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12567842#action_12567842 ] 

Sergiy Yevtushenko commented on WICKET-1331:
--------------------------------------------

Initially I did as you suggesting - subclassed DropDownChoice. But then I hit similar issue in ListMultipleChoice and found that getModelValue() is final there. At present I'm ought to maintain local copy of Wicket sources.
And, actually, I'm not completely understand why you don't like proposed change. It provides more flexibility and functionality for free, without affecting existing applications or performance penalties.


> getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: WICKET-1331
>                 URL: https://issues.apache.org/jira/browse/WICKET-1331
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.3.0-final, 1.3.1
>         Environment: Suse 10.3, JRE 1.6 (1.6.0_04-b12), jetty 6.1.7
>            Reporter: Sergiy Yevtushenko
>            Assignee: Igor Vaynberg
>         Attachments: testproject.zip
>
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> AbstractSingleSelectChoice.getModelValue() implementation uses List.indexOf() to find the key and this causes problems if list of choices contains complex values rather than simple list of String instances. In this case indexOf() returns -1 and this can't be resolved by overriding equals() for list elements. This happens because internally AbstractList.indexOf() invokes equals() method of passed key value passing it list items as a parameter. Also, current implementation may pass key returned by getModelObject() to IChoiceRender, while it expects values from list of items. Correct implementation of this method may look so:
> -------------------------
> 	public String getModelValue()
> 	{
> 		// @@ Modified by SIY
> 		Object object = getModelObject();
> 		if (object != null)
> 		{
> 			// compare choice items with given keys and pass down
> 			// to IChoiceRenderer list item rather than key
> 			Iterator iter = getChoices().iterator();
> 			int i = 0;
> 			while (iter.hasNext())
> 			{
> 				Object obj = iter.next();
> 				if (obj != null && obj.equals(object))
> 				{
> 					object = obj;
> 					break;
> 				}
> 				i++;
> 			}
> 			if (i > getChoices().size())
> 				i = -1;
> 			return getChoiceRenderer().getIdValue(object, i);
> 		}
> 		return NO_SELECTION_VALUE;
> 	}
> -----------------------------------
> Similar issues also present in ListMultipleChoice.getModelValue(), but they can't be resolved by overriding this method in subclass because this method declared final.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WICKET-1331) getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly

Posted by "Matej Knopp (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-1331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12567856#action_12567856 ] 

Matej Knopp commented on WICKET-1331:
-------------------------------------

What you do is just wrong. There is a contract specified on the equals method and you just violate it.

# public boolean equals(Object obj)  
#         {  
#             if (obj instanceof Long)  
#                 return id.equals(obj);  
#               

Your equals method is not symmetric. I really don't see why we should clutter wicket code with something like that just because you can't be bothered to write a proper choice renderer.


> getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: WICKET-1331
>                 URL: https://issues.apache.org/jira/browse/WICKET-1331
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.3.0-final, 1.3.1
>         Environment: Suse 10.3, JRE 1.6 (1.6.0_04-b12), jetty 6.1.7
>            Reporter: Sergiy Yevtushenko
>            Assignee: Igor Vaynberg
>         Attachments: testproject.zip
>
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> AbstractSingleSelectChoice.getModelValue() implementation uses List.indexOf() to find the key and this causes problems if list of choices contains complex values rather than simple list of String instances. In this case indexOf() returns -1 and this can't be resolved by overriding equals() for list elements. This happens because internally AbstractList.indexOf() invokes equals() method of passed key value passing it list items as a parameter. Also, current implementation may pass key returned by getModelObject() to IChoiceRender, while it expects values from list of items. Correct implementation of this method may look so:
> -------------------------
> 	public String getModelValue()
> 	{
> 		// @@ Modified by SIY
> 		Object object = getModelObject();
> 		if (object != null)
> 		{
> 			// compare choice items with given keys and pass down
> 			// to IChoiceRenderer list item rather than key
> 			Iterator iter = getChoices().iterator();
> 			int i = 0;
> 			while (iter.hasNext())
> 			{
> 				Object obj = iter.next();
> 				if (obj != null && obj.equals(object))
> 				{
> 					object = obj;
> 					break;
> 				}
> 				i++;
> 			}
> 			if (i > getChoices().size())
> 				i = -1;
> 			return getChoiceRenderer().getIdValue(object, i);
> 		}
> 		return NO_SELECTION_VALUE;
> 	}
> -----------------------------------
> Similar issues also present in ListMultipleChoice.getModelValue(), but they can't be resolved by overriding this method in subclass because this method declared final.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WICKET-1331) getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly

Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-1331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12567735#action_12567735 ] 

Igor Vaynberg commented on WICKET-1331:
---------------------------------------

well, i would like to see the code snippet that demonstrates (1) and (2).

in fact (2) sounds like a programmer error....

> getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: WICKET-1331
>                 URL: https://issues.apache.org/jira/browse/WICKET-1331
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.3.0-final, 1.3.1
>         Environment: Suse 10.3, JRE 1.6 (1.6.0_04-b12), jetty 6.1.7
>            Reporter: Sergiy Yevtushenko
>            Assignee: Igor Vaynberg
>         Attachments: testproject.zip
>
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> AbstractSingleSelectChoice.getModelValue() implementation uses List.indexOf() to find the key and this causes problems if list of choices contains complex values rather than simple list of String instances. In this case indexOf() returns -1 and this can't be resolved by overriding equals() for list elements. This happens because internally AbstractList.indexOf() invokes equals() method of passed key value passing it list items as a parameter. Also, current implementation may pass key returned by getModelObject() to IChoiceRender, while it expects values from list of items. Correct implementation of this method may look so:
> -------------------------
> 	public String getModelValue()
> 	{
> 		// @@ Modified by SIY
> 		Object object = getModelObject();
> 		if (object != null)
> 		{
> 			// compare choice items with given keys and pass down
> 			// to IChoiceRenderer list item rather than key
> 			Iterator iter = getChoices().iterator();
> 			int i = 0;
> 			while (iter.hasNext())
> 			{
> 				Object obj = iter.next();
> 				if (obj != null && obj.equals(object))
> 				{
> 					object = obj;
> 					break;
> 				}
> 				i++;
> 			}
> 			if (i > getChoices().size())
> 				i = -1;
> 			return getChoiceRenderer().getIdValue(object, i);
> 		}
> 		return NO_SELECTION_VALUE;
> 	}
> -----------------------------------
> Similar issues also present in ListMultipleChoice.getModelValue(), but they can't be resolved by overriding this method in subclass because this method declared final.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Issue Comment Edited: (WICKET-1331) getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly

Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-1331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12567854#action_12567854 ] 

ivaynberg edited comment on WICKET-1331 at 2/11/08 2:40 PM:
----------------------------------------------------------------

you are using the component in a manner we consider incorrect.

in 1.4 the component will be generified as:

DropDownChoice<T>(String id, IModel<T> model, IModel<List<T>> choices, IChoiceRenderer<T> renderer)

how will your change work than??? last time i checked your ChoiceOption doesnt not extend Long.

anyways, this is the last time i am replying here, as i have mentioned you are free to start an email thread on our dev list, that is much better for discussions.

====================================================================

also looking at your code, your implementation of ChoiceOption.equlas() is horribly broken and would be considered as an ugly hack by any experienced developer

one of key equals contracts is symmetry. that meas if a.equals(b) is true, then so is b.equals(a)

in your case choiceoption.equals(long(5)) might be true, but never 5l.equals(choiceoption)

thus your equals implementation is flawed, i urge you to read Object.equals() javadoc.

      was (Author: ivaynberg):
    you are using the component in a manner we consider incorrect.

in 1.4 the component will be generified as:

DropDownChoice<T>(String id, IModel<T> model, IModel<List<T>> choices, IChoiceRenderer<T> renderer)

how will your change work than??? last time i checked your ChoiceOption doesnt not extend Long.

anyways, this is the last time i am replying here, as i have mentioned you are free to start an email thread on our dev list, that is much better for discussions.
  
> getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: WICKET-1331
>                 URL: https://issues.apache.org/jira/browse/WICKET-1331
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.3.0-final, 1.3.1
>         Environment: Suse 10.3, JRE 1.6 (1.6.0_04-b12), jetty 6.1.7
>            Reporter: Sergiy Yevtushenko
>            Assignee: Igor Vaynberg
>         Attachments: testproject.zip
>
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> AbstractSingleSelectChoice.getModelValue() implementation uses List.indexOf() to find the key and this causes problems if list of choices contains complex values rather than simple list of String instances. In this case indexOf() returns -1 and this can't be resolved by overriding equals() for list elements. This happens because internally AbstractList.indexOf() invokes equals() method of passed key value passing it list items as a parameter. Also, current implementation may pass key returned by getModelObject() to IChoiceRender, while it expects values from list of items. Correct implementation of this method may look so:
> -------------------------
> 	public String getModelValue()
> 	{
> 		// @@ Modified by SIY
> 		Object object = getModelObject();
> 		if (object != null)
> 		{
> 			// compare choice items with given keys and pass down
> 			// to IChoiceRenderer list item rather than key
> 			Iterator iter = getChoices().iterator();
> 			int i = 0;
> 			while (iter.hasNext())
> 			{
> 				Object obj = iter.next();
> 				if (obj != null && obj.equals(object))
> 				{
> 					object = obj;
> 					break;
> 				}
> 				i++;
> 			}
> 			if (i > getChoices().size())
> 				i = -1;
> 			return getChoiceRenderer().getIdValue(object, i);
> 		}
> 		return NO_SELECTION_VALUE;
> 	}
> -----------------------------------
> Similar issues also present in ListMultipleChoice.getModelValue(), but they can't be resolved by overriding this method in subclass because this method declared final.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WICKET-1331) getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly

Posted by "Sergiy Yevtushenko (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-1331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12567981#action_12567981 ] 

Sergiy Yevtushenko commented on WICKET-1331:
--------------------------------------------

All of you are right, proposed change is incorrect and equals() is broken. The need of such a ugly hack may mean that for my purposes I need a version of DropDownChoice written from scratch, because existing one is inconvenient.


> getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: WICKET-1331
>                 URL: https://issues.apache.org/jira/browse/WICKET-1331
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.3.0-final, 1.3.1
>         Environment: Suse 10.3, JRE 1.6 (1.6.0_04-b12), jetty 6.1.7
>            Reporter: Sergiy Yevtushenko
>            Assignee: Igor Vaynberg
>         Attachments: testproject.zip
>
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> AbstractSingleSelectChoice.getModelValue() implementation uses List.indexOf() to find the key and this causes problems if list of choices contains complex values rather than simple list of String instances. In this case indexOf() returns -1 and this can't be resolved by overriding equals() for list elements. This happens because internally AbstractList.indexOf() invokes equals() method of passed key value passing it list items as a parameter. Also, current implementation may pass key returned by getModelObject() to IChoiceRender, while it expects values from list of items. Correct implementation of this method may look so:
> -------------------------
> 	public String getModelValue()
> 	{
> 		// @@ Modified by SIY
> 		Object object = getModelObject();
> 		if (object != null)
> 		{
> 			// compare choice items with given keys and pass down
> 			// to IChoiceRenderer list item rather than key
> 			Iterator iter = getChoices().iterator();
> 			int i = 0;
> 			while (iter.hasNext())
> 			{
> 				Object obj = iter.next();
> 				if (obj != null && obj.equals(object))
> 				{
> 					object = obj;
> 					break;
> 				}
> 				i++;
> 			}
> 			if (i > getChoices().size())
> 				i = -1;
> 			return getChoiceRenderer().getIdValue(object, i);
> 		}
> 		return NO_SELECTION_VALUE;
> 	}
> -----------------------------------
> Similar issues also present in ListMultipleChoice.getModelValue(), but they can't be resolved by overriding this method in subclass because this method declared final.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WICKET-1331) getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly

Posted by "Johan Compagner (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-1331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12567855#action_12567855 ] 

Johan Compagner commented on WICKET-1331:
-----------------------------------------

this is useless, at the moment we generify everything you will see it moe clearly. And generics will force you to do it right:

DropDownChoice<T>(String id, IModel<T> model, List<T> choices, IChoiceRendere<T> renderer)

> getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: WICKET-1331
>                 URL: https://issues.apache.org/jira/browse/WICKET-1331
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.3.0-final, 1.3.1
>         Environment: Suse 10.3, JRE 1.6 (1.6.0_04-b12), jetty 6.1.7
>            Reporter: Sergiy Yevtushenko
>            Assignee: Igor Vaynberg
>         Attachments: testproject.zip
>
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> AbstractSingleSelectChoice.getModelValue() implementation uses List.indexOf() to find the key and this causes problems if list of choices contains complex values rather than simple list of String instances. In this case indexOf() returns -1 and this can't be resolved by overriding equals() for list elements. This happens because internally AbstractList.indexOf() invokes equals() method of passed key value passing it list items as a parameter. Also, current implementation may pass key returned by getModelObject() to IChoiceRender, while it expects values from list of items. Correct implementation of this method may look so:
> -------------------------
> 	public String getModelValue()
> 	{
> 		// @@ Modified by SIY
> 		Object object = getModelObject();
> 		if (object != null)
> 		{
> 			// compare choice items with given keys and pass down
> 			// to IChoiceRenderer list item rather than key
> 			Iterator iter = getChoices().iterator();
> 			int i = 0;
> 			while (iter.hasNext())
> 			{
> 				Object obj = iter.next();
> 				if (obj != null && obj.equals(object))
> 				{
> 					object = obj;
> 					break;
> 				}
> 				i++;
> 			}
> 			if (i > getChoices().size())
> 				i = -1;
> 			return getChoiceRenderer().getIdValue(object, i);
> 		}
> 		return NO_SELECTION_VALUE;
> 	}
> -----------------------------------
> Similar issues also present in ListMultipleChoice.getModelValue(), but they can't be resolved by overriding this method in subclass because this method declared final.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WICKET-1331) getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly

Posted by "Sergiy Yevtushenko (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-1331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12567729#action_12567729 ] 

Sergiy Yevtushenko commented on WICKET-1331:
--------------------------------------------

Initially I met the same issue with POJO which has two string members and symptoms were the same - attempt to resolve property for simple type (that time is was String). Also, I've tried to replace ChoiceRenderer with custom one. This approach resolves a trap, but does not resolve two other issues: 
1. under some circumstances IChoiceRenderer.getDisplayValue() can be invoked with key (field model value) rather than choice list item (this happens when key can't be found in choice list).
2. Initial value set incorrectly (for the same reason - selected item can't be determined).

Proposed version of getModelValue() resolves these issue if choice list item has proper equals() method.

> getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: WICKET-1331
>                 URL: https://issues.apache.org/jira/browse/WICKET-1331
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.3.0-final, 1.3.1
>         Environment: Suse 10.3, JRE 1.6 (1.6.0_04-b12), jetty 6.1.7
>            Reporter: Sergiy Yevtushenko
>            Assignee: Igor Vaynberg
>         Attachments: testproject.zip
>
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> AbstractSingleSelectChoice.getModelValue() implementation uses List.indexOf() to find the key and this causes problems if list of choices contains complex values rather than simple list of String instances. In this case indexOf() returns -1 and this can't be resolved by overriding equals() for list elements. This happens because internally AbstractList.indexOf() invokes equals() method of passed key value passing it list items as a parameter. Also, current implementation may pass key returned by getModelObject() to IChoiceRender, while it expects values from list of items. Correct implementation of this method may look so:
> -------------------------
> 	public String getModelValue()
> 	{
> 		// @@ Modified by SIY
> 		Object object = getModelObject();
> 		if (object != null)
> 		{
> 			// compare choice items with given keys and pass down
> 			// to IChoiceRenderer list item rather than key
> 			Iterator iter = getChoices().iterator();
> 			int i = 0;
> 			while (iter.hasNext())
> 			{
> 				Object obj = iter.next();
> 				if (obj != null && obj.equals(object))
> 				{
> 					object = obj;
> 					break;
> 				}
> 				i++;
> 			}
> 			if (i > getChoices().size())
> 				i = -1;
> 			return getChoiceRenderer().getIdValue(object, i);
> 		}
> 		return NO_SELECTION_VALUE;
> 	}
> -----------------------------------
> Similar issues also present in ListMultipleChoice.getModelValue(), but they can't be resolved by overriding this method in subclass because this method declared final.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WICKET-1331) getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly

Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-1331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12567854#action_12567854 ] 

Igor Vaynberg commented on WICKET-1331:
---------------------------------------

you are using the component in a manner we consider incorrect.

in 1.4 the component will be generified as:

DropDownChoice<T>(String id, IModel<T> model, IModel<List<T>> choices, IChoiceRenderer<T> renderer)

how will your change work than??? last time i checked your ChoiceOption doesnt not extend Long.

anyways, this is the last time i am replying here, as i have mentioned you are free to start an email thread on our dev list, that is much better for discussions.

> getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: WICKET-1331
>                 URL: https://issues.apache.org/jira/browse/WICKET-1331
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.3.0-final, 1.3.1
>         Environment: Suse 10.3, JRE 1.6 (1.6.0_04-b12), jetty 6.1.7
>            Reporter: Sergiy Yevtushenko
>            Assignee: Igor Vaynberg
>         Attachments: testproject.zip
>
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> AbstractSingleSelectChoice.getModelValue() implementation uses List.indexOf() to find the key and this causes problems if list of choices contains complex values rather than simple list of String instances. In this case indexOf() returns -1 and this can't be resolved by overriding equals() for list elements. This happens because internally AbstractList.indexOf() invokes equals() method of passed key value passing it list items as a parameter. Also, current implementation may pass key returned by getModelObject() to IChoiceRender, while it expects values from list of items. Correct implementation of this method may look so:
> -------------------------
> 	public String getModelValue()
> 	{
> 		// @@ Modified by SIY
> 		Object object = getModelObject();
> 		if (object != null)
> 		{
> 			// compare choice items with given keys and pass down
> 			// to IChoiceRenderer list item rather than key
> 			Iterator iter = getChoices().iterator();
> 			int i = 0;
> 			while (iter.hasNext())
> 			{
> 				Object obj = iter.next();
> 				if (obj != null && obj.equals(object))
> 				{
> 					object = obj;
> 					break;
> 				}
> 				i++;
> 			}
> 			if (i > getChoices().size())
> 				i = -1;
> 			return getChoiceRenderer().getIdValue(object, i);
> 		}
> 		return NO_SELECTION_VALUE;
> 	}
> -----------------------------------
> Similar issues also present in ListMultipleChoice.getModelValue(), but they can't be resolved by overriding this method in subclass because this method declared final.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Issue Comment Edited: (WICKET-1331) getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly

Posted by "Sergiy Yevtushenko (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-1331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12567757#action_12567757 ] 

evsi edited comment on WICKET-1331 at 2/11/08 11:04 AM:
----------------------------------------------------------------------

Sorry, I haven't mentioned it, but attached example does show both these issues: 
1) causes a trap "No get method defined for class: class java.lang.Long expression: id" cited above (because ChoiceRenderer tries to resolve property name for simple type)
2) can be seen if replace ChoiceRenderer with custom one (as you have suggested) or just by stepping through getDisplayValue() in debugger - value taken from model does not match any choice item (the List indexOf() is responsible for this, as I've mentioned). Also, with updated getModelValue() this issue disappears, so this is not a programmer error. 

      was (Author: evsi):
    Sorry, I haven't mentioned it, but attached example does show both these issues: 
1) causes a trap "No get method defined for class: class java.lang.Long expression: id" cited above (because ChoiceRenderer tries to resolve property name for simple type)
2) can be seen if replace ChoiceRenderer with custom one (as you have suggested) or just by stepping through getDisplayValue() in debugger - value taken from model does not match any choice item (the List indexOf() is responsible for this, as I've mentioned).

  
> getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: WICKET-1331
>                 URL: https://issues.apache.org/jira/browse/WICKET-1331
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.3.0-final, 1.3.1
>         Environment: Suse 10.3, JRE 1.6 (1.6.0_04-b12), jetty 6.1.7
>            Reporter: Sergiy Yevtushenko
>            Assignee: Igor Vaynberg
>         Attachments: testproject.zip
>
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> AbstractSingleSelectChoice.getModelValue() implementation uses List.indexOf() to find the key and this causes problems if list of choices contains complex values rather than simple list of String instances. In this case indexOf() returns -1 and this can't be resolved by overriding equals() for list elements. This happens because internally AbstractList.indexOf() invokes equals() method of passed key value passing it list items as a parameter. Also, current implementation may pass key returned by getModelObject() to IChoiceRender, while it expects values from list of items. Correct implementation of this method may look so:
> -------------------------
> 	public String getModelValue()
> 	{
> 		// @@ Modified by SIY
> 		Object object = getModelObject();
> 		if (object != null)
> 		{
> 			// compare choice items with given keys and pass down
> 			// to IChoiceRenderer list item rather than key
> 			Iterator iter = getChoices().iterator();
> 			int i = 0;
> 			while (iter.hasNext())
> 			{
> 				Object obj = iter.next();
> 				if (obj != null && obj.equals(object))
> 				{
> 					object = obj;
> 					break;
> 				}
> 				i++;
> 			}
> 			if (i > getChoices().size())
> 				i = -1;
> 			return getChoiceRenderer().getIdValue(object, i);
> 		}
> 		return NO_SELECTION_VALUE;
> 	}
> -----------------------------------
> Similar issues also present in ListMultipleChoice.getModelValue(), but they can't be resolved by overriding this method in subclass because this method declared final.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WICKET-1331) getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly

Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-1331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12567777#action_12567777 ] 

Igor Vaynberg commented on WICKET-1331:
---------------------------------------

but (1) is programmer error...you shouldnt use that choice renderer implementation with primitives!

> getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: WICKET-1331
>                 URL: https://issues.apache.org/jira/browse/WICKET-1331
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.3.0-final, 1.3.1
>         Environment: Suse 10.3, JRE 1.6 (1.6.0_04-b12), jetty 6.1.7
>            Reporter: Sergiy Yevtushenko
>            Assignee: Igor Vaynberg
>         Attachments: testproject.zip
>
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> AbstractSingleSelectChoice.getModelValue() implementation uses List.indexOf() to find the key and this causes problems if list of choices contains complex values rather than simple list of String instances. In this case indexOf() returns -1 and this can't be resolved by overriding equals() for list elements. This happens because internally AbstractList.indexOf() invokes equals() method of passed key value passing it list items as a parameter. Also, current implementation may pass key returned by getModelObject() to IChoiceRender, while it expects values from list of items. Correct implementation of this method may look so:
> -------------------------
> 	public String getModelValue()
> 	{
> 		// @@ Modified by SIY
> 		Object object = getModelObject();
> 		if (object != null)
> 		{
> 			// compare choice items with given keys and pass down
> 			// to IChoiceRenderer list item rather than key
> 			Iterator iter = getChoices().iterator();
> 			int i = 0;
> 			while (iter.hasNext())
> 			{
> 				Object obj = iter.next();
> 				if (obj != null && obj.equals(object))
> 				{
> 					object = obj;
> 					break;
> 				}
> 				i++;
> 			}
> 			if (i > getChoices().size())
> 				i = -1;
> 			return getChoiceRenderer().getIdValue(object, i);
> 		}
> 		return NO_SELECTION_VALUE;
> 	}
> -----------------------------------
> Similar issues also present in ListMultipleChoice.getModelValue(), but they can't be resolved by overriding this method in subclass because this method declared final.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (WICKET-1331) getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly

Posted by "Sergiy Yevtushenko (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/WICKET-1331?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sergiy Yevtushenko updated WICKET-1331:
---------------------------------------

    Issue Type: Bug  (was: Improvement)

> getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: WICKET-1331
>                 URL: https://issues.apache.org/jira/browse/WICKET-1331
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.3.0-final, 1.3.1
>         Environment: Suse 10.3, JRE 1.6 (1.6.0_04-b12), jetty 6.1.7
>            Reporter: Sergiy Yevtushenko
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> AbstractSingleSelectChoice.getModelValue() implementation uses List.indexOf() to find the key and this causes problems if list of choices contains complex values rather than simple list of String instances. In this case indexOf() returns -1 and this can't be resolved by overriding equals() for list elements. This happens because internally AbstractList.indexOf() invokes equals() method of passed key value passing it list items as a parameter. Also, current implementation may pass key returned by getModelObject() to IChoiceRender, while it expects values from list of items. Correct implementation of this method may look so:
> -------------------------
> 	public String getModelValue()
> 	{
> 		// @@ Modified by SIY
> 		Object object = getModelObject();
> 		if (object != null)
> 		{
> 			// compare choice items with given keys and pass down
> 			// to IChoiceRenderer list item rather than key
> 			Iterator iter = getChoices().iterator();
> 			int i = 0;
> 			while (iter.hasNext())
> 			{
> 				Object obj = iter.next();
> 				if (obj != null && obj.equals(object))
> 				{
> 					object = obj;
> 					break;
> 				}
> 				i++;
> 			}
> 			if (i > getChoices().size())
> 				i = -1;
> 			return getChoiceRenderer().getIdValue(object, i);
> 		}
> 		return NO_SELECTION_VALUE;
> 	}
> -----------------------------------
> Similar issues also present in ListMultipleChoice.getModelValue(), but they can't be resolved by overriding this method in subclass because this method declared final.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WICKET-1331) getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly

Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-1331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12567791#action_12567791 ] 

Igor Vaynberg commented on WICKET-1331:
---------------------------------------

once again, you are using the dropdown choice incorrectly

your model object is of type Long, while your choice objects are ChoiceOption. dropdown choice expects the collection of choices and the model object be of the same type!

if you are in this situation - where your collection is a list of primitives, then you should convert it to a user-facing string inside choicerenderer.

> getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: WICKET-1331
>                 URL: https://issues.apache.org/jira/browse/WICKET-1331
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.3.0-final, 1.3.1
>         Environment: Suse 10.3, JRE 1.6 (1.6.0_04-b12), jetty 6.1.7
>            Reporter: Sergiy Yevtushenko
>            Assignee: Igor Vaynberg
>         Attachments: testproject.zip
>
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> AbstractSingleSelectChoice.getModelValue() implementation uses List.indexOf() to find the key and this causes problems if list of choices contains complex values rather than simple list of String instances. In this case indexOf() returns -1 and this can't be resolved by overriding equals() for list elements. This happens because internally AbstractList.indexOf() invokes equals() method of passed key value passing it list items as a parameter. Also, current implementation may pass key returned by getModelObject() to IChoiceRender, while it expects values from list of items. Correct implementation of this method may look so:
> -------------------------
> 	public String getModelValue()
> 	{
> 		// @@ Modified by SIY
> 		Object object = getModelObject();
> 		if (object != null)
> 		{
> 			// compare choice items with given keys and pass down
> 			// to IChoiceRenderer list item rather than key
> 			Iterator iter = getChoices().iterator();
> 			int i = 0;
> 			while (iter.hasNext())
> 			{
> 				Object obj = iter.next();
> 				if (obj != null && obj.equals(object))
> 				{
> 					object = obj;
> 					break;
> 				}
> 				i++;
> 			}
> 			if (i > getChoices().size())
> 				i = -1;
> 			return getChoiceRenderer().getIdValue(object, i);
> 		}
> 		return NO_SELECTION_VALUE;
> 	}
> -----------------------------------
> Similar issues also present in ListMultipleChoice.getModelValue(), but they can't be resolved by overriding this method in subclass because this method declared final.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Assigned: (WICKET-1331) getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly

Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/WICKET-1331?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Igor Vaynberg reassigned WICKET-1331:
-------------------------------------

    Assignee: Igor Vaynberg

> getModelValue() in AbstractSingleSelectChoice and ListMultipleChoice can't handle complex list items type correctly
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: WICKET-1331
>                 URL: https://issues.apache.org/jira/browse/WICKET-1331
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.3.0-final, 1.3.1
>         Environment: Suse 10.3, JRE 1.6 (1.6.0_04-b12), jetty 6.1.7
>            Reporter: Sergiy Yevtushenko
>            Assignee: Igor Vaynberg
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> AbstractSingleSelectChoice.getModelValue() implementation uses List.indexOf() to find the key and this causes problems if list of choices contains complex values rather than simple list of String instances. In this case indexOf() returns -1 and this can't be resolved by overriding equals() for list elements. This happens because internally AbstractList.indexOf() invokes equals() method of passed key value passing it list items as a parameter. Also, current implementation may pass key returned by getModelObject() to IChoiceRender, while it expects values from list of items. Correct implementation of this method may look so:
> -------------------------
> 	public String getModelValue()
> 	{
> 		// @@ Modified by SIY
> 		Object object = getModelObject();
> 		if (object != null)
> 		{
> 			// compare choice items with given keys and pass down
> 			// to IChoiceRenderer list item rather than key
> 			Iterator iter = getChoices().iterator();
> 			int i = 0;
> 			while (iter.hasNext())
> 			{
> 				Object obj = iter.next();
> 				if (obj != null && obj.equals(object))
> 				{
> 					object = obj;
> 					break;
> 				}
> 				i++;
> 			}
> 			if (i > getChoices().size())
> 				i = -1;
> 			return getChoiceRenderer().getIdValue(object, i);
> 		}
> 		return NO_SELECTION_VALUE;
> 	}
> -----------------------------------
> Similar issues also present in ListMultipleChoice.getModelValue(), but they can't be resolved by overriding this method in subclass because this method declared final.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.