Strategy for Limpet property editors

[IanMayo]

We've got a hacked implementation of property descriptors.

I'd like us to use a tidier, neater strategy for them.

Here are the requirements/constraints:

• our core data objects are not RCP aware
• I'd quite like to define editable properties at the data object level - so we have the potential opportunity to use the defined properties in other UIs (web)
• I wish to define properties for Collections and Operations
• I'd like there to be some inheritance: so for a collection we have "top level" properties that associate with IOperation (such as isDynamic) and others that relate to child classes (such as "Window Size" for SimpleMovingAverage).

Have you done anything similar?

Can you suggest a strategy?

[dinkoivanov]

Can EMF be considered to define the core data objects in Limpet? Several jars needs to be present in the classpath (https://wiki.eclipse.org/EMF/FAQ#How_do_I_use_EMF_in_standalone_applications_.28such_as_an_ordinary_main.29.3F). I'm sure there'll be some more technical challenges related to dependency management, probably when the core project (info.limpet) is used in RCP. Besides serialization & event notification mechanism, EMF provides convenient introspection API, so that object properties can be queried dynamically (some sort of reflection, not the java native one). For example for certain object, you can list its properties with their property types and construct the corresponding UI (RCP or Web) accordingly. Another option would be to come up with an in-house version of IPropertyDescriptor interface and then use it to define the properties. Then this interface would be adapted to the RCP properties (or used to create a Web UI). Regarding the inheritance, here is what can be done in the current implementation (using the example with command properties):

1. Delegate the creation of CommandPropertySource to a Factory, instead of how they're directly created right now in the CommandWrapper class.
2. The factory would create a specialized CommandPropertySource for the moving average command (MovingAverageCommandPropertySource extends CommandPropertySource).
3. The MovingAverageCommandPropertySource will override getPropertyDescriptors, then call super.getPropertyDescriptors and add the window size property to the array of descriptors. I believe with EMF we can even get the property descriptos for free (auto-generated or some adapter implementation, I'm not sure)

[IanMayo]

a) I dont' think EMF is justified in this context. Its formal approach solves lots of problems, but I don't necessarily think we have those problems in our modest application. b) in-house IPropertyDescriptor. I guess we could re-use the Java FeatureDescriptor class (and children for properties & methods). Our RCP layer could translate these into RCP editor controls - as you describe. c) Yes, the factory approach looks great. The Java "SimpleBeanInfo" looks like a way of the UI-agnostic Command class to provide details on what can be edited. A factory class could then retrieve the properties for the current object, and assign RCP properties as appropriate. (we've done something similar in Debrief for 10 years :wink: )

@dinkoivanov - feedback welcome :smiley:

[dinkoivanov]

Another option would be to use Java Reflection API to obtain the fields of a class with their types and be able to get and set values. In that case Limpet Model object must adhere to the Java Beans conventions and have getter/setter methods. Then if we need some additional metadata on what can be edited, or how, we can use our own custom annotations to specify this. Like in the AbstractOperation we can have: @UIProperty(editable="false") final private String _description; @UIProperty private boolean _dynamic = true;

For SimpleMovingAverageCommand we can have: @UIProperty(rangeMin="1",rangeMax="20") private int winSize;

I see that the ICollection is actually wrapped in CollectionWrapper in the UI. So when user selects a collection wrapper in order to see the underlying collection properties, we can have something like:

public class CollectionWrapper implements IAdaptable, LimpetWrapper { @UIProperty(inline="true") private final ICollection _collection;

In CollectionPropertySource I see some dynamic properties, like if the collection is a singleton, then either provide a text field to edit the value. Modelling this using the annotation approach will be bit challenging, but still possible. Since there is no model property equivalent in the ICollection class, we'll need to add an additional property in the CollectionWrapper like this:

@UIProperty(visible="_collection.size() == 1") private String singletonValue;

There should be a getter/setter for the singleton value in the CollectionWrapper to do the singleton work now performed in the CollectionPropertySource.

What do you think?

[IanMayo]

Yes, good suggestion. I'd considered moving Debrief editors to annotations - but there's just so many places where it would need to be edited.

Using that practice in Limpet would be a great way for me to learn about it.

But, is it worth applying the annotations at the method (getter/setter) level?

Hmm, it would be great if we could remove the "wrapper" classes. They're really only in there to generate the IPropertySource adapted class.

I've just done some research. CollectionWrapper.getAdapter(IPropertySource) is called from "ViewsPlugin". I wonder if we can register an adapter here - one that will take a ICollection and dynamically produce a set of IPropertySource data items.

Oh, one last thought. I guess that instead of UIProperty, we could define UIReadProperty, UITextProperty, UIIntegerProperty, UIDoubleProperty, UIBooleanProperty - and so on.

Hmm, we're not mixing "model" with "view" are we here? Well actually, that's exactly what I'm hoping to do with the system here :-)

[dinkoivanov]

I imagine annotations can be applied on field or method level as well. Method level makes sense for certain derived properties, which lack corresponding member field.

If wrapper classes are only used to generate the IPropertySource adapted class, then I believe you can get rid of them by registering an adapter factory in the plugin.xml. Such solution is described here: https://waynebeaton.wordpress.com/2007/10/23/adapters-part-deux/

We can have UIProperty hierrachy based on the type, but my feeling is that a single class like UIProperty(type=UIPropertyType.TEXT) could be easier to code (autocompletion) + the type can be implied by the field type.

Regarding the last concern about mixing "model" and "view", we can still introduce another layer "view-model" that will hold the presentation properties of the model.

[IanMayo]

Hi there @dinkoivanov, this is an area that I'd still like to explore.

Specifically, I have this priority one aim:

• to use annotations so we can create property editors declaratively instead of programmatically

And this priority two aim:

• to remove the CollectionWrapper, GroupWrapper, etc tier of classes - so we use our actual model elements in the data model

I've just had a look at the code, and I know number two may be difficult. If we replace our xxxWrapper with the raw data model, then our data model would need to be able to return an instanceof of IPropertySource: https://github.com/debrief/limpet/blob/master/info.limpet.rcp/src/info/limpet/rcp/data_provider/data/CollectionWrapper.java#L58

But, I think there is the Eclipse concept of AdapterFactories, where we register some adapters, and they handle the task of transforming/decorating model objects.

Such an adapter factory could inspect annotations on the data object and produce a suitable IPropertySource object.

Would you be interested in exploring this further?

[dinkoivanov]

Hi @IanMayo, Yes, I'm interested in exploring this. My plan is to first make use of the adapter factories, then introduce a sample annotation and see how it works on a single domain class, then go further. What do you think?

[IanMayo]

Hi there - yes, that's a good plan.

I suggest you start on GroupWrapper - it's the least complex.

I may be online in one hour, for an hour. Otherwise it will have to be Monday.

[dinkoivanov]

OK, I'll spend few hours on this tomorrow and push in a separate branch for you to review.

[dinkoivanov]

After the initial implementation round I'm now quite convinced that we should keep the wrapper classes. They are actually needed since a model object can appear multiple times in different places in the tree. Wrappers are also useful to avoid recursion when a node appears second time in a tree path (force hasChildren = false).

[IanMayo]

Yes, I agree about the value of the wrapper classes. But, hopefully we can have a generic wrapper instead of the type-specific ones.

[IanMayo]

Thanks for the latest commit(s). I've done some refactoring, so you should do a "pull" before you start any changes.

1. More properties

I'd like you to continue replacing hard-coded properties with UI Property values please - just to explore the concept further, and give demonstrations of how we handle different challenges.

We have the "units" for quantity collections. As an interim step, this can be a read-only string property.

We also have "value" and "range" for quantity collection singletons. Oh, and Latitude and Longitude for singleton ILocations datasets.

2. Wrapper classes

We currently have a Wrapper for each data type (command wrapper, group wrapper, collection wrapper). The three classes all seem also identical. I think we should replace them with either a generic one (instantiated with the type of data item to be wrapped), one that just stores a IStoreItem, or one that is somehow responsive/reflective to the item being stored.