Comment 1 Eric Rizzo 2007-07-06 13:27:38 EDT

Here's my comment from the newsgroup thread, as an opening to further discussion:
It is probably worth discussing the merits of keeping as similar as possible to how nested beans are typically accessed in JSP, which is just dot-separated Strings: "person.address.street"
There is also accommodation for referring to individual instances in Maps, Lists, and arrays: "person[Smith].children[1].age"

This notation is widely familiar to a large number of JEE developers - familiarity and consistency shouldn't be ignored. In addition, there are general-purpose libraries (from Jakarta) that support it already.

I'm not saying Eclipse Databinding absolutely must use this kind of notation, just that the notation should be carefully considered and discussed.

Comment 2 Brad Reynolds 2007-07-07 14:07:17 EDT

Adding David to the CC...

I'd be OK with the JSP[1]/JSF[2] EL syntax (this is also how JSR 295[3] is approaching this for Beans Binding and I know bean shell supported "dot notation" as well) but I want to make a couple of points that I think are important.  Feel free to agree/disagree.

1.  Don't diverge from the standard.  I'm OK with doing this at times but since we'd be relying on just a String which doesn't allow for versioning of the expected syntax my opinion is to be conservative.  So we shouldn't try to get cute with this.
2.  This is only for Java Beans.  One of the reasons we have static factories for the creation of observables is that we want to allow those who are familiar with a domain to be able to create observables with the familiarity they have of that domain.  So if EMF has their own expression notation or other API they shouldn't have to feel like they need to conform to a JFace Data Binding expression language.

[1] http://jcp.org/en/jsr/detail?id=245
[2] http://jcp.org/en/jsr/detail?id=252
[1] http://jcp.org/en/jsr/detail?id=295

Comment 3 Brad Reynolds 2007-07-10 15:51:03 EDT

BTW, there's been some disagreements on using EL syntax in Beans Bindings[1].

[1] http://www.javalobby.org/java/forums/t98543.html

Comment 4 Thomas Schindl 2007-07-12 06:53:06 EDT

Created attachment 73649 [details]
Just for reference this is my current solution

Comment 5 Thomas Schindl 2007-07-16 03:32:40 EDT

Just that we don't forget it. BeansObservables.observeMaps() should also be able to work with nested attributes.

Comment 6 Frank Gerhardt 2007-07-17 06:03:29 EDT

Note: Peter Centgraf posted a similar solution in bug #127381

Comment 7 Eclipse Webmaster 2007-07-29 09:23:18 EDT

Changing OS from Mac OS to Mac OS X as per bug 185991

Comment 8 Matthew Hall 2008-09-04 19:58:51 EDT

Anyone interested in nested properties, please review the most recent patches in bug 194734 (PropertyObservables).

With the mentioned patch you can chain properties to observe properties several levels deep into an object graph.  For example, given the following bean class (with getter and setter methods implied for each property):

class Person {
  Person mother;
  Person father;
  List<Person> children;
  String name;
}

A number of relationships can be observed very easily:

IValueProperty fatherProperty = BeanProperties.valueProperty(Person.class, "father");
IValueProperty motherProperty = BeanProperies.valueProperty(Person.class, "mother");
IValueProperty nameProperty = BeanProperties.valueProperty(Person.class, "name");
IListProperty childrenProperty = BeanProperties.listProperty(Person.class, "children");

IObservableValue fatherObservable = PropertyObservables.observeValue(person, fatherProperty);
IObservableValue paternalGrandFatherObservable = MasterDetailObservables.detailValue(
  fatherObservable, PropertyObservables.valueFactory(fatherProperty), Person.class);

Properties can be combined in master-detail fashion to observe more complex relationship without having to create observables at each step:

IValueProperty paternalGrandFatherProperty = MasterDetailProperties.detailValue(fatherProperty, fatherProperty);

IObservableValue paternalGrandFatherObservable = PropertyObservables.observeValue(person, paternalGrandFatherProperty);

IListProperty childrenNamesProperty = MasterDetailProperties.detailList(childrenProperty, nameProperty);
IObservableList childrenNamesObservable = PropertyObservables.observeList(person, childrenNamesProperty);

Also, please tell me if you think I'm close enough to the mark for this bug to mark it as a dupe of 194734.  If it is it would be nice to consolidate everybody's attention on a single bug.

Comment 9 Thomas Schindl 2008-09-05 03:17:31 EDT

I'm not sure how this simplies the case of observing father.name. How would that look like? 

Another thing that came to my mind was (I don't know if this is possible) if you have a scenario like this:

Person {
   parents: List<Parents>
   name: String
}

Parents {
   person: Person
   mother: boolean
}

and I'd like to bind the name of the Persons mother I currently have to add a virtual attribute (in case of Beans it is a set/get method for get/setMother):

Person {
   parents: List<Parents>
   name: String
   mother: Parent (transient)
   father: Parent (transient)
}

Now in my dreams I'd like to write something like this:

BeanObservable.observeValue(person,"parents[@mother=true]/name");

Could you imagine something like this is possible at all (my wild guess is not but I wanted to mention it).

Comment 10 Thomas Schindl 2008-09-05 03:23:54 EDT

(In reply to comment #9)
> 
> BeanObservable.observeValue(person,"parents[@mother=true]/name");
> 

Should be BeanObservables.observeValue(person,"parents[@mother=true]/person/name");

Comment 11 Matthew Hall 2008-09-05 12:30:53 EDT

(In reply to comment #9)
> ..I'd like to bind the name of the Persons mother I currently have to add a
> virtual attribute (in case of Beans it is a set/get method for get/setMother):
> 
> Person {
>    parents: List<Parents>
>    name: String
>    mother: Parent (transient)
>    father: Parent (transient)
> }
> 
> Now in my dreams I'd like to write something like this:
> 
> BeanObservable.observeValue(person,"parents[@mother=true]/name");
> 
> Could you imagine something like this is possible at all (my wild guess is not
> but I wanted to mention it).

I think we could achieve the conditional selection / filtering as in your example, albeit probably not with the fancy string parsing it implies.  e.g.

IListProperty parentsProperty = BeanProperties.listProperty(Person.class, "parents");
IPropertyFilter motherFilter = PropertyFilters.valuePropertyEqualsFilter(
    BeanProperties.valueProperty(Parents.class, "mother"), Boolean.TRUE);
IValueProperty motherParentProperty = FilteredProperties.firstMatch(parentsProperty, motherFilter);
IValueProperty parentPersonProperty = BeanProperties.valueProperty(Parents.class, "person");
IValueProperty motherProperty = MasterDetailProperties.detailValue(motherParentProperty, parentPersonProperty);
IValueProperty personNameProperty = BeanProperties.valueProperty(Person.class, "name");
IValueProperty motherNameProperty = MaterDetailProperties.detailValue(motherProperty, personNameProperty);

Keeping in mind that PropertyFilters and FilteredProperties are theoretical at this point.

Comment 12 Thomas Schindl 2008-09-05 12:38:58 EDT

You'd be my personal hero if you could make this work. Still I can't see how a simply nesting is solved with the new API can I write something easy like "address.street" with it? Nonetheless amazing work!

Comment 13 Matthew Hall 2008-09-05 13:08:04 EDT

(In reply to comment #12)
> You'd be my personal hero if you could make this work. Still I can't see how a
> simply nesting is solved with the new API can I write something easy like
> "address.street" with it? Nonetheless amazing work!

You're already my personal hero for your ViewerColumn + CellLabelProvider + EditingSupport work, so if I do my job too well it might cause some sort of feedback loop.  :)

The problem with simple dot-separated property names is that we have four different types of properties (value, set, list, map) that can be combined in various ways, and it would be very difficult if not impossible to predict how people want to combine those ahead of time.

I just had a thought that a builder API might solve this:

class BeanValuePropertyBuilder {
  static BeanValuePropertyBuilder value(Class beanClass, String propertyName)

  IValueProperty build()
  BeanValuePropertyBuilder ofValue(Class beanClass, String propertyName)
  BeanMapPropertyBuilder ofSet(Class beanClass, String propertyName)
  BeanListPropertyBuilder ofList(Class beanClass, String propertyName)
  BeanMapPropertyBuilder ofMap(Class beanClass, String propertyName)
}

class BeanSetPropertyBuilder {
  static BeanSetPropertyBuilder set(Class beanClass, String propertyName)

  ISetProperty build()
  BeanSetPropertyBuilder ofValue(Class beanClass, String propertyName)
}

class BeanListPropertyBuilder {
  static BeanListPropertyBuilder list(Class beanClass, String propertyName)

  IListProperty build()
  BeanListPropertyBuilder ofValue(Class beanClass, String propertyName)
}

class BeanMapPropertyBuilder {
  static BeanMapPropertyBuilder map(Class beanClass, String propertyName)

  IMapProperty build()
  BeanMapPropertyBuilder ofValue(Class beanClass, String propertyName)
}

So taking your "street.address" example, assuming the given bean classes:

class Contact {
  Address address
}

class Address {
  String street
}

Using these theoretical builders you could get the "address.street" property like this:

IValueProperty contactAddressStreetProperty =
    BeanValuePropertyBuilder
      .value(Address.class, "street")
      .ofValue(Contact.class, "address")
      .build();

Comment 14 Boris Bokowski 2008-09-05 13:18:32 EDT

(In reply to comment #13)
> Using these theoretical builders you could get the "address.street" property
> like this:
> 
> IValueProperty contactAddressStreetProperty =
>     BeanValuePropertyBuilder
>       .value(Address.class, "street")
>       .ofValue(Contact.class, "address")
>       .build();

This looks very promising, I like it!

Btw, if the model has good enough meta information (like e.g. in EMF), you could parse queries like Tom's from comment 10 and build a structure like the one in comment 11. I would hope that somewhere in the Modeling project, a parser for queries in some kind of standard language already exists.

Comment 15 Matthew Hall 2008-09-19 16:42:29 EDT

Boris, I'll take this bug if it's all right with you.

Comment 16 Boris Bokowski 2008-09-19 21:31:40 EDT

(In reply to comment #15)
> Boris, I'll take this bug if it's all right with you.

Sure, please do!

Comment 17 Matthew Hall 2008-09-22 11:27:20 EDT

Taking ownership

Comment 18 Matthew Hall 2008-10-16 19:50:01 EDT

Boris convinced me to keep the order of method calls similar to what you'd expect in regular nested method calls.  e.g. if I want to observe person.getAddress().getStreet() the code should look like:

IValueProperty personAddressStreet = BeanPropertyBuilder.value(Person.class, "address").value(Address.class, "street").build();

or:

IValueProperty personAddressStreet = BeanPropertyBuilder.value(Person.class, "address").value("street").build();

or even:

IValueProperty objectAddressStreet = BeanPropertyBuilder.value("address").value("street").build();

This latter example presumes support for generic bean properties (bug 247997).  In this case the builder will try to infer the bean class from the value type / element type of the previous property.  If the bean class can be inferred it is used; otherwise a generic bean property will be used.

This leads to interesting possibilities:

IListProperty friendsFathersFirstNames = BeanPropertyBuilder.list(Person.class, "friends").value("father").value("firstName").build();
IObservableList friendsFathersFirstNamesObservable = PropertyObservables.observeList(person, friendsFathersFirstNames);

Comment 19 Jacek Furmankiewicz 2008-12-05 14:39:46 EST

Will this solution provide functionality equivalent to the EL abilities of the Swing BeansBinding? In particular, the ability to build binding expressions comprised of various properties, e.g.

"My name is ${employee.firstName} ${employee.lastName}".

or boolean conditional statements

"${employee.firstName} != null"

You can do this in BeansBinding (JSR295) using the ELProperty object they offer. 

More code samples here:

http://weblogs.java.net/blog/shan_man/archive/2007/09/beans_binding_1.html

Comment 20 Matthew Hall 2008-12-05 16:30:49 EST

(In reply to comment #19)
> Will this solution provide functionality equivalent to the EL abilities of the
> Swing BeansBinding? In particular, the ability to build binding expressions
> comprised of various properties, e.g.
> 
> "My name is ${employee.firstName} ${employee.lastName}".
> 
> or boolean conditional statements
> 
> "${employee.firstName} != null"

Yes and no.  The API proposed here will simplify the creation of chained bean properties.  Using BeanPropertyBuilder your "employee.firstName" example would be coded as:

IValueProperty employeeFirstName = BeanPropertyBuilder.value("employee").value("firstName").build();

Building properties through string parsing will come later, and there are some unanswered questions to work out, such as:
* Should we support list, set and map properties in addition to value properties?
* If so, how do we tell the parser what kind of property it is looking at?  Should we use a suffix notation, e.g. [] suffix for lists, {} suffix for sets, {:} (or something) suffix for maps?

As for an ELProperty equivalent, unless somebody contributes this I don't know when this would be added.

Comment 21 Matthew Hall 2008-12-09 15:41:04 EST

Moving to M5

Comment 22 Matthew Hall 2008-12-17 18:16:12 EST

Bug 194734 (properties API) is nearing completion, so we've committed the current state of work to branch "mhall_bug194734_properties".  If anyone cares to to start submitting patches for this bug, please do so against this branch.

Comment 23 Matthew Hall 2008-12-29 10:02:35 EST

I've been working on a prototype for this which I will attach soon, but I wanted to share my thoughts so far:

* The BeanPropertyBuilder API essentially duplicates that of BeanProperties.  This seems a waste of effort.
* I'm considering adding extension interfaces (i.e. IBeanProperty, IBeanValueProperty, IBeanListProperty, etc) and moving the BeanPropertyBuilder methods into these interfaces instead.  By having BeanProperties and PojoProperties return these extension interfaces we can support property chaining directly through the properties, so instead of:

IValueProperty personMotherName =
   BeanPropertyBuilder.value(Person.class, "mother").value("name").build();

the code would be:

IValueProperty personMotherName =
   BeanProperties.value(Person.class, "mother").value("name");

Comment 24 Matthew Hall 2008-12-29 21:51:58 EST

Created attachment 121321 [details]
Patch

Changes to properties API in this patch:

Moved getValueType() from SimpleValueProperty to IValueProperty

Moved getElementType() from SimpleSetProperty to ISetProperty

Moved getElementType() from SimpleListProperty to IListProperty

Moved getKeyType() and getValueType() from SimpleMapProperty to IMapProperty

Renamed all chain() methods in properties to value(), list(), set(), map() or values() depending on the context.  This made property chaining code much more readable--for example a property observing the names of maternal siblings would now be code as:
	person.value(mother).list(children).values(name)
instead of:
	person.chain(mother).chain(children).chain(name)

Removed most methods from Properties as they are redundant to the chaining methods in each property interface

Renamed all observe<Value|List|Set|Map> methods to just observe() since the type of observable is implied by the type of property

Renamed all observeDetail<Value|List|Set|Map|Values> methods to just observeDetail() for the same reasons

Introduced IBeanProperty, IBeanValueProperty, IBeanListProperty, IBeanSetProperty and IBeanMapProperty.  IBeanProperty provides getPropertyDescriptor().  The other interfaces extend it and provide convenience methods for creating nested properties, using similar method signatures to those of BeanProperties and PojoProperties.  So the following is now possible:
	IListProperty materialSiblingNames = BeanProperties
		.value(Person.class, "mother")
		.list(Person.class, "children")
		.values(Person.class, "name");

Additionally there are methods that exclude the bean class argument--the bean class is implied by the master property.  So the above example could be rewritten as:
	IValueProperty materialSiblingNames = BeanProperties
		.value(Person.class, "mother")
		.list("children")
		.values("name");
This will work if Person implements the children property as a Person array, but not if it is implemented as a List property.  This is because the list property cannot discover the element through reflection.  This can be overcome by specifying the element type on the children property:
	IValueProperty materialSiblingNames = BeanProperties
		.value(Person.class, "mother")
		.list("children", Person.class)
		.values("name");
or by specifying the bean class on the name property:
	IValueProperty materialSiblingNames = BeanProperties
		.value(Person.class, "mother")
		.list("children")
		.values(Person.class, "name");

And finally, the one everyone's been waiting for: nested properties can now be defined using period-delimited property names:
	IValueProperty paternalGrandmotherName =
		BeanProperties.value(Person.class, "father.mother.name");
Note that this is only supported for value properties, i.e. the above code is just an alias for:
	IValueProperty paternalGrandmotherName = BeanProperties
		.value(Person.class, "father")
		.value("mother")
		.value("name");
Note that period-delimited nested property names are also supported in all relevant API in BeansObservables/PojoObservables (including the observeMap(IObservableSet, String) and observeMaps(IObservableSet, String[]) methods).

I've updated Snippet025 in this patch to demonstrate period-delimited property names.  You have to be checked out on the mhall_bug194734_properties branch to apply this patch.

Enjoy

Comment 25 Matthew Hall 2008-12-29 21:52:09 EST

Created attachment 121322 [details]
mylyn/context/zip

Comment 26 Matthew Hall 2008-12-29 22:15:43 EST

Note: all the maternalSiblingNames should have been written as IListProperty instances, not IValueProperty.  Just in case that caused any confusion..

Comment 27 Matthew Hall 2009-01-02 21:00:41 EST

Created attachment 121448 [details]
Replacing some FIXMEs with javadoc

Comment 28 Matthew Hall 2009-01-02 21:00:49 EST

Created attachment 121449 [details]
mylyn/context/zip

Comment 29 Matthew Hall 2009-01-03 09:40:46 EST

Created attachment 121463 [details]
More javadoc

Boris, could you take a look at the javadoc so far for all methods in the IBean*Property interfaces?  I'm not sure if the explanations are clear enough.

Comment 30 Matthew Hall 2009-01-03 09:40:53 EST

Created attachment 121464 [details]
mylyn/context/zip

Comment 31 Matthew Hall 2009-01-05 10:21:08 EST

Created attachment 121528 [details]
Finished javadoc in new interface methods

Comment 32 Matthew Hall 2009-01-05 10:31:30 EST

Created attachment 121531 [details]
Finished writing javadoc for new interfaces

Comment 33 Matthew Hall 2009-01-05 10:31:39 EST

Created attachment 121532 [details]
mylyn/context/zip

Comment 34 Matthew Hall 2009-01-06 01:21:47 EST

Released last patch to mhall_bug194734_properties branch > 20090106

Comment 35 Matthew Hall 2009-01-07 20:02:34 EST

Results from discussion with Boris on IRC:
* IBean*Property interfaces have too many overloaded methods.  In particular having the method IBeanValueProperty.value(Class, String) and value(String, Class) is confusing.  So we will drop the (String, Class) methods in all IBean*Property interfaces
* Make IBean*Property interfaces @noextend and @noimplement so that we are free to add new methods in the future
* Explain dot notation for nested value properties in javadoc.  This in cludes BeanProperties, PojoProperties, BeansObservables, PojoObservables, and IBean*Property interfaces

Comment 36 Matthew Hall 2009-01-08 23:17:13 EST

Upon reflection I think it would be better to exclude the methods with beanClass argument rather than the elementType / keyType / valueType arguments.  My reasoning behind this is that 1) we want the element / key / value type to be known as often as possible so that the observables will inherit these; and 2) if the element / value type is always (or at least usually) provided, then that type can be automatically used as the bean class for nested properties.

Comment 37 Matthew Hall 2009-01-09 02:50:11 EST

Created attachment 122068 [details]
Patch

Revisions described in comment #35 and comment #36.

Boris, please review

Comment 38 Matthew Hall 2009-01-09 02:50:19 EST

Created attachment 122069 [details]
mylyn/context/zip

Comment 39 Matthew Hall 2009-01-12 12:26:14 EST

Committed latest attached patch to mhall_bug194734_properties branch

Comment 40 Matthew Hall 2009-01-12 13:15:21 EST

Similar to the work committed earlier on SWTObservables and ViewersObservables, I've committed changes to BeansObservables and PojoObservables so that all four factory classes delegate entirely to properties.  Thus there should be no observable (ha ha) difference in behavior when creating observables through WidgetProperties vs SWTObservables.

Comment 41 Matthew Hall 2009-01-12 13:19:47 EST

Whoops, the previous comment was intended for 194734.

Work on this feature is complete for all intents and purposes.  All that remains is for the mhall_bug194734_properties branch to be merged into HEAD.  Anyone wanting to follow this feature should CC themselves to bug 194734 in order to follow the progress of that bug.

Comment 42 Matthew Hall 2009-01-20 18:24:54 EST

Released to HEAD (as part of bug 194734) > 20090120

Comment 43 Matthew Hall 2009-01-29 23:33:46 EST

Verified in I20090129-0100 by running Snippet025TableViewerWithPropertyDerivedColumns