You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@pivot.apache.org by Edvin Syse <ed...@sysedata.no> on 2011/06/22 21:25:24 UTC
No support for Map in BeanAdapter?
I have a TableView that I want to show the following column:
<TableView.Column name="data.group"/>
My model object contains a Map called data with an entry for 'group'.
This seems to fail in BeanAdapter#containsKey, because it only looks for
a getter method. I looks like it could be adapted to support maps
without too much hassle, but maybe there is another way to do this that
doesn't require a change to the BeanAdapter?
Would it be valuable to make BeanAdapter wrap Map properties?
-- Edvin
Re: No support for Map in BeanAdapter?
Posted by Edvin Syse <ed...@sysedata.no>.
Den 23.06.2011 11:03, skrev Sandro Martini:
> Hi all,
> so the proposed solution should be enough now, right ?
Absolutely :)
> And maybe for mid-term (2.1 and later) we could try to think if setting a
> custom implementation of BeanAdapter (defaulting with the current one) ...
> but not only for Tables, I think could be useful even on other cases,
> anywhere BeanAdapter is used.
>
> What do you think ?
> If you like it I could create a ticket for it.
> Sent from the Apache Pivot - Users mailing list archive at Nabble.com.
Maybe it is better to wait with creating a ticket until there is a
usecase for it. The next usecase that comes along might just involve a
trivial and logical evolvement of the current BeanAdapter (like this
time around). Pluggability sounds cool, but only if it is actually needed :)
Re: No support for Map in BeanAdapter?
Posted by Sandro Martini <sa...@gmail.com>.
Hi all,
so the proposed solution should be enough now, right ?
And maybe for mid-term (2.1 and later) we could try to think if setting a
custom implementation of BeanAdapter (defaulting with the current one) ...
but not only for Tables, I think could be useful even on other cases,
anywhere BeanAdapter is used.
What do you think ?
If you like it I could create a ticket for it.
In the meantime thanks to Edvin (and Chris) for the proposal, the patch, etc
...
Bye
--
View this message in context: http://apache-pivot-users.399431.n3.nabble.com/No-support-for-Map-in-BeanAdapter-tp3097003p3099156.html
Sent from the Apache Pivot - Users mailing list archive at Nabble.com.
Re: No support for Map in BeanAdapter?
Posted by Chris Bartlett <cb...@gmail.com>.
On 23 June 2011 15:23, Edvin Syse <ed...@sysedata.no> wrote:
> Den 23.06.2011 10:19, skrev Chris Bartlett:
>
> I haven't looked at the patch, but does it allow java.util.Map values to
> also be *set*? If not, the next question on the list will be - 'BeanAdapter
> can read value from a java.util.Map, but can't write them'
>
> Yes ofcourse it allows set as well :))
>
> Good stuff.
> Maybe a more flexible solution would be to allow users to control which
> class is used to process the TableView.Column.name property? (Defaulting
> to BeanAdapter of course).
>
>
> That would allow any custom Map<String, Object> implementation to be
> used, giving the user freedom to handle whatever notation they chose.
>
> I agree, but I think that JSON should support java.util.Map anyways, and
> the override for TableView.Colum.name processing could be held off until a
> real usecase comes along :)
>
> -- Edvin
>
Yep, if your patch sorts things out now, then that is perfect.
Re: No support for Map in BeanAdapter?
Posted by Edvin Syse <ed...@sysedata.no>.
Den 23.06.2011 10:19, skrev Chris Bartlett:
> I haven't looked at the patch, but does it allow java.util.Map values
> to also be *set*? If not, the next question on the list will be -
> 'BeanAdapter can read value from a java.util.Map, but can't write them'
Yes ofcourse it allows set as well :))
> Maybe a more flexible solution would be to allow users to control
> which class is used to process the TableView.Column.name
> <http://TableView.Column.name> property? (Defaulting to BeanAdapter
> of course).
>
> That would allow any custom Map<String, Object> implementation to be
> used, giving the user freedom to handle whatever notation they chose.
I agree, but I think that JSON should support java.util.Map anyways, and
the override for TableView.Colum.name processing could be held off until
a real usecase comes along :)
-- Edvin
Re: No support for Map in BeanAdapter?
Posted by Chris Bartlett <cb...@gmail.com>.
I haven't looked at the patch, but does it allow java.util.Map values to
also be *set*? If not, the next question on the list will be - 'BeanAdapter
can read value from a java.util.Map, but can't write them'
Maybe a more flexible solution would be to allow users to control which
class is used to process the TableView.Column.name property? (Defaulting to
BeanAdapter of course).
That would allow any custom Map<String, Object> implementation to be used,
giving the user freedom to handle whatever notation they chose.
If beneficial, this could be extended to cover all other Components which
use BeanAdapter internally, or even to have a system
property/ApplicationContext property/static Component property to specify a
global 'BeanAdapterFactory'.
Chris
On 23 June 2011 14:42, Sandro Martini <sa...@gmail.com> wrote:
> Hi Edvin,
> the patch seems good (I like this feature, improving interaction with
> domain
> objects is important), would you like to reopen the issue
> https://issues.apache.org/jira/browse/PIVOT-764
> and attach the patch there (maybe deleting the old one ...) ?
>
> So if Greg is not against this we can apply it.
>
> Bye
>
>
> --
> View this message in context:
> http://apache-pivot-users.399431.n3.nabble.com/No-support-for-Map-in-BeanAdapter-tp3097003p3098900.html
> Sent from the Apache Pivot - Users mailing list archive at Nabble.com.
>
Re: No support for Map in BeanAdapter?
Posted by Edvin Syse <ed...@sysedata.no>.
Thanks Sandro,
I've reopened the ticket and attached the patch :)
-- Edvin
Den 23.06.2011 09:42, skrev Sandro Martini:
> Hi Edvin,
> the patch seems good (I like this feature, improving interaction with domain
> objects is important), would you like to reopen the issue
> https://issues.apache.org/jira/browse/PIVOT-764
> and attach the patch there (maybe deleting the old one ...) ?
>
> So if Greg is not against this we can apply it.
>
> Bye
>
>
> --
> View this message in context: http://apache-pivot-users.399431.n3.nabble.com/No-support-for-Map-in-BeanAdapter-tp3097003p3098900.html
> Sent from the Apache Pivot - Users mailing list archive at Nabble.com.
Re: No support for Map in BeanAdapter?
Posted by Sandro Martini <sa...@gmail.com>.
Hi Edvin,
the patch seems good (I like this feature, improving interaction with domain
objects is important), would you like to reopen the issue
https://issues.apache.org/jira/browse/PIVOT-764
and attach the patch there (maybe deleting the old one ...) ?
So if Greg is not against this we can apply it.
Bye
--
View this message in context: http://apache-pivot-users.399431.n3.nabble.com/No-support-for-Map-in-BeanAdapter-tp3097003p3098900.html
Sent from the Apache Pivot - Users mailing list archive at Nabble.com.
Re: No support for Map in BeanAdapter?
Posted by Edvin Syse <ed...@sysedata.no>.
Den 23.06.2011 00:12, skrev Edvin Syse:
> Den 22.06.2011 23:58, skrev Greg Brown:
>> Ah, I see. java.util.Map is not supported. You need to use the Pivot
>> collections with BeanAdapter. Did you try wrapping your map in a
>> MapAdapter (or simply using a Pivot Map such as HashMap)?
>
> I don't see how I can possibly do that, since I can't (and shouldn't)
> change the domain object to hold an instance of Pivot Map. Example
> domain object:
>
> public class Person {
> String name;
> java.util.Map properties = new java.util.HashMap();
> }
>
> So I have a list of these set as the tableData for a TableView. Each
> Person contains another map inside properties, and I want to retrieve a
> value from that map to show in the TableView. Here is how one could
> manually construct one of the Person objects:
>
> Person p = new Person();
> p.name = "John Doe"
>
> java.util.Map data = new java.util.HashMap();
> data.put("group", "My Group")
>
> p.properties.put("data", "data")
So sorry, it's getting late, my example is off and I can't think
straight :) Will get back on the horse tomorrow :)
Re: No support for Map in BeanAdapter?
Posted by Edvin Syse <ed...@sysedata.no>.
Den 23.06.2011 00:25, skrev Greg Brown:
>> Do you think adding support for java.util.Map to BeanAdapter has little value?
>>
>> By the way, the patch is trivial, and it works great :)
> BeanAdapter is the wrong place to put this code. BeanAdapter makes a bean instance look like a Pivot Map. It is not meant to make a java.util.Map look like a Pivot map. That is the purpose of MapAdapter.
>
> The JSON class would be more appropriate for this kind of check
After looking at it closer (and sleeping a bit :), you are ofcourse
absolutely right as always :) Would you concider applying the attached
patch to add Java Map support to JSON? Local variable name "beanAdapter"
might be a misnomer with my change though.
-- Edvin
Re: No support for Map in BeanAdapter?
Posted by Sandro Martini <sa...@gmail.com>.
Hi all,
and a customized (application) class that extends BeanAdapter, reusing
most of its code, but adding support for standard Java Maps ?
And then the ability on our components to override the default
BeanAdapter if/where needed with a custom implementation like that ?
Bye
Re: No support for Map in BeanAdapter?
Posted by Edvin Syse <ed...@sysedata.no>.
Den 23.06.2011 00:25, skrev Greg Brown:
>> Do you think adding support for java.util.Map to BeanAdapter has little value?
>>
>> By the way, the patch is trivial, and it works great :)
>
> BeanAdapter is the wrong place to put this code. BeanAdapter makes a bean instance look like a Pivot Map. It is not meant to make a java.util.Map look like a Pivot map. That is the purpose of MapAdapter.
>
> The JSON class would be more appropriate for this kind of check.
OK, I'll try to make a patch for JSON tomorrow after I have gotten som
sleep :) Thank you so far.
-- Edvin
Re: No support for Map in BeanAdapter?
Posted by Greg Brown <gk...@verizon.net>.
> Do you think adding support for java.util.Map to BeanAdapter has little value?
>
> By the way, the patch is trivial, and it works great :)
BeanAdapter is the wrong place to put this code. BeanAdapter makes a bean instance look like a Pivot Map. It is not meant to make a java.util.Map look like a Pivot map. That is the purpose of MapAdapter.
The JSON class would be more appropriate for this kind of check.
Re: No support for Map in BeanAdapter?
Posted by Edvin Syse <ed...@sysedata.no>.
Den 22.06.2011 23:58, skrev Greg Brown:
> Ah, I see. java.util.Map is not supported. You need to use the Pivot collections with BeanAdapter. Did you try wrapping your map in a MapAdapter (or simply using a Pivot Map such as HashMap)?
I don't see how I can possibly do that, since I can't (and shouldn't)
change the domain object to hold an instance of Pivot Map. Example
domain object:
public class Person {
String name;
java.util.Map properties = new java.util.HashMap();
}
So I have a list of these set as the tableData for a TableView. Each
Person contains another map inside properties, and I want to retrieve a
value from that map to show in the TableView. Here is how one could
manually construct one of the Person objects:
Person p = new Person();
p.name = "John Doe"
java.util.Map data = new java.util.HashMap();
data.put("group", "My Group")
p.properties.put("data", "data")
My column is:
<TableView.Column name="data.group"/>
I can't see how I can make this work without using an intermediary
domain object, or simply adding support for java.util.Map to BeanAdapter.
Do you think adding support for java.util.Map to BeanAdapter has little
value?
By the way, the patch is trivial, and it works great :)
-- Edvin
Re: No support for Map in BeanAdapter?
Posted by Greg Brown <gk...@verizon.net>.
Ah, I see. java.util.Map is not supported. You need to use the Pivot collections with BeanAdapter. Did you try wrapping your map in a MapAdapter (or simply using a Pivot Map such as HashMap)?
On Jun 22, 2011, at 5:45 PM, Edvin Syse wrote:
> Den 22.06.2011 23:28, skrev Greg Brown:
>> Here's an example demonstrating how to use JSON.get() to access nested map properties by path:
>>
>> package org.apache.pivot.tests;
>>
>> import org.apache.pivot.collections.Map;
>> import org.apache.pivot.json.JSON;
>> import org.apache.pivot.json.JSONSerializer;
>>
>> public class JSONPathTest {
>> public static void main(String[] args) throws Exception {
>> Map<String, ?> map = JSONSerializer.parseMap("{data:{group:'My Group'}}");
>> System.out.println(JSON.get(map, "data.group"));
>> }
>> }
>>
>> When the program is executed, "My Group" is written to the console.
>
> Yes, but this works because JSONSerializer.parseMap returns pivots own Map class, not java.util.Map as in my example. I'll comment on the ticket and hold off the list to avoid double posting :)
Re: No support for Map in BeanAdapter?
Posted by Edvin Syse <ed...@sysedata.no>.
Den 22.06.2011 23:28, skrev Greg Brown:
> Here's an example demonstrating how to use JSON.get() to access nested map properties by path:
>
> package org.apache.pivot.tests;
>
> import org.apache.pivot.collections.Map;
> import org.apache.pivot.json.JSON;
> import org.apache.pivot.json.JSONSerializer;
>
> public class JSONPathTest {
> public static void main(String[] args) throws Exception {
> Map<String, ?> map = JSONSerializer.parseMap("{data:{group:'My Group'}}");
> System.out.println(JSON.get(map, "data.group"));
> }
> }
>
> When the program is executed, "My Group" is written to the console.
Yes, but this works because JSONSerializer.parseMap returns pivots own
Map class, not java.util.Map as in my example. I'll comment on the
ticket and hold off the list to avoid double posting :)
Re: No support for Map in BeanAdapter?
Posted by Greg Brown <gk...@verizon.net>.
Here's an example demonstrating how to use JSON.get() to access nested map properties by path:
package org.apache.pivot.tests;
import org.apache.pivot.collections.Map;
import org.apache.pivot.json.JSON;
import org.apache.pivot.json.JSONSerializer;
public class JSONPathTest {
public static void main(String[] args) throws Exception {
Map<String, ?> map = JSONSerializer.parseMap("{data:{group:'My Group'}}");
System.out.println(JSON.get(map, "data.group"));
}
}
When the program is executed, "My Group" is written to the console.
Hope this helps.
G
On Jun 22, 2011, at 5:19 PM, Edvin Syse wrote:
> I created an issue and attached a patch, I hope you will concider it. I think it is very in line with the "it just works as expected" feeling I get from using Pivot :)
>
> https://issues.apache.org/jira/browse/PIVOT-764
>
> -- Edvin
Re: No support for Map in BeanAdapter?
Posted by Edvin Syse <ed...@sysedata.no>.
I created an issue and attached a patch, I hope you will concider it. I
think it is very in line with the "it just works as expected" feeling I
get from using Pivot :)
https://issues.apache.org/jira/browse/PIVOT-764
-- Edvin
Re: No support for Map in BeanAdapter?
Posted by Edvin Syse <ed...@sysedata.no>.
>> Right. We already made that change. So what is the problem you are
>> running into? data.group seems like it should work as a column name.
>
> It doesn't work because BeanAdapter tries to find the getterMethod for
> group in data (which is a Map) and it fails, since it is looking for
> "getData", when it should be looking for "get" with an argument of
> "data". It would require a couple of checks in the lines of if
I'm sorry, I meant it's looking for "getGroup" when it should be looking
for "get" -> "group".
-- Edvin
Re: No support for Map in BeanAdapter?
Posted by Edvin Syse <ed...@sysedata.no>.
Den 22.06.2011 22:03, skrev Greg Brown:
>>> I think this has come up before. BeanAdapter wraps a single bean instance and exposes its (immediate) properties via map methods. You can access nested properties using JSON.get(), put(), etc.
>>>
>>> However, TableView does not currently expect paths in column names, only keys. It could be modified to use JSON.get() instead, but I'm on the fence about whether that is a good idea or not.
>>
>> I think it looks like TableView already supports paths in column names. TableView#load calls:
>>
>> JSON.get(context, tableDataKey)
>>
>> and JSON#get will call parse on the tableDataKey, which seems to create an object navigational graph.
>
> Right. We already made that change. So what is the problem you are running into? data.group seems like it should work as a column name.
It doesn't work because BeanAdapter tries to find the getterMethod for
group in data (which is a Map) and it fails, since it is looking for
"getData", when it should be looking for "get" with an argument of
"data". It would require a couple of checks in the lines of if
(Map.class.isAssignableFrom(beanClass)) to create a spesific getter
method for Maps, and some other small stuff, but it could be implemented
very cleanly without any performance hit.
-- Edvin
Re: No support for Map in BeanAdapter?
Posted by Greg Brown <gk...@verizon.net>.
>> I think this has come up before. BeanAdapter wraps a single bean instance and exposes its (immediate) properties via map methods. You can access nested properties using JSON.get(), put(), etc.
>>
>> However, TableView does not currently expect paths in column names, only keys. It could be modified to use JSON.get() instead, but I'm on the fence about whether that is a good idea or not.
>
> I think it looks like TableView already supports paths in column names. TableView#load calls:
>
> JSON.get(context, tableDataKey)
>
> and JSON#get will call parse on the tableDataKey, which seems to create an object navigational graph.
Right. We already made that change. So what is the problem you are running into? data.group seems like it should work as a column name.
Re: No support for Map in BeanAdapter?
Posted by Edvin Syse <ed...@sysedata.no>.
Den 22.06.2011 21:34, skrev Greg Brown:
> I think this has come up before. BeanAdapter wraps a single bean instance and exposes its (immediate) properties via map methods. You can access nested properties using JSON.get(), put(), etc.
>
> However, TableView does not currently expect paths in column names, only keys. It could be modified to use JSON.get() instead, but I'm on the fence about whether that is a good idea or not.
I think it looks like TableView already supports paths in column names.
TableView#load calls:
JSON.get(context, tableDataKey)
and JSON#get will call parse on the tableDataKey, which seems to create
an object navigational graph.
-- Edvin
Re: No support for Map in BeanAdapter?
Posted by Greg Brown <gk...@verizon.net>.
>> I think this has come up before. BeanAdapter wraps a single bean instance and exposes its (immediate) properties via map methods. You can access nested properties using JSON.get(), put(), etc.
>>
>> However, TableView does not currently expect paths in column names, only keys. It could be modified to use JSON.get() instead, but I'm on the fence about whether that is a good idea or not.
>
> What about just letting BeanAdapter understand Maps as well? It will only wrap explicilty used keys anyway, and it should be an easy fix.
No, that's what JSON#get() is for. BeanAdapter#get() is used by JSON#get() as it traverses the path.
Re: No support for Map in BeanAdapter?
Posted by Edvin Syse <ed...@sysedata.no>.
Den 22.06.2011 21:34, skrev Greg Brown:
> I think this has come up before. BeanAdapter wraps a single bean instance and exposes its (immediate) properties via map methods. You can access nested properties using JSON.get(), put(), etc.
>
> However, TableView does not currently expect paths in column names, only keys. It could be modified to use JSON.get() instead, but I'm on the fence about whether that is a good idea or not.
What about just letting BeanAdapter understand Maps as well? It will
only wrap explicilty used keys anyway, and it should be an easy fix.
Re: No support for Map in BeanAdapter?
Posted by Greg Brown <gk...@verizon.net>.
I think this has come up before. BeanAdapter wraps a single bean instance and exposes its (immediate) properties via map methods. You can access nested properties using JSON.get(), put(), etc.
However, TableView does not currently expect paths in column names, only keys. It could be modified to use JSON.get() instead, but I'm on the fence about whether that is a good idea or not.
On Jun 22, 2011, at 3:25 PM, Edvin Syse wrote:
> I have a TableView that I want to show the following column:
>
> <TableView.Column name="data.group"/>
>
> My model object contains a Map called data with an entry for 'group'. This seems to fail in BeanAdapter#containsKey, because it only looks for a getter method. I looks like it could be adapted to support maps without too much hassle, but maybe there is another way to do this that doesn't require a change to the BeanAdapter?
>
> Would it be valuable to make BeanAdapter wrap Map properties?
>
> -- Edvin