Jing,
A few comments:
Documentation:
We need to document when old row key can be null (CollectionModel
doesn't have access to the old key for whatever reason) and what, if
anything, the listener should do about it. Also, in what cases (row was
deleted?) would the new row key ever be null? Is the listener called if
rows are newly inserted or deleted? If so, should the event contain the
reason for the change (INSERT, UPDATE, DELETE)?
We also need documentation regarding how we expect that the listener and
model should handle the fact that the listener (if it is associated with
a component) and the model could have different lifecycles. For
example, do we expect that a component listener will attach and detach
itself on each request?
Implementation-wise, we need to consider what we want the Serialization
behavior of the event to be since we are a subclass of
java.util.EventObject, which implements Serializable. In particular, we
need to consider the case where the CollectionModel that we have a
reference to isn't Serializable. We should also have a serialization id
if we will be using default serialization.
Finally, the event should implement equals() hashCode() and toString().
On the CollectionModel, it isn't clear that we actually need a
getRowKeyChangeListeners(). Also, we can definitely use a smaller
default size for the set of listeners. Considering the small number of
listeners likely to be registered, using a List to simulate the Set by
checking if the Listener is already registered is likely faster and smaller.
-- Blake Sullivan
jing wu said the following On 10/28/2009 10:14 AM PT:
> RowKey is used in collection model to identify a row, it is safer to
> use than row indicies because RowKeys are unaffected by mutations to
> the collection. But there are cases where a RowKey instance can be
> changed. The proposal is to define a clear way to raise and broadcast
> such event. The proposal contains the following parts:
>
> 1. RowKeyChangeEvent - This newly introduced event is delivered when
> row key is changed, the event should contain the value for both old
> row key and the new row key.
> 2. RowKeyChangeListener - This is the listener that listens to
> RowKeyChangeEvent, it has method onRowKeyChange() defined. Every time
> when there is an RowKeyChangeEvent event, the implementor of
> RowKeyChangeListener will get notified. 3. CollectionModel - The
> source that detects row key change and raises RowKeyChangeEvent and
> invokes registered listeners.
>
> Every module that holds reference to rowKey needs to implement
> RowKeyChangeListener, register the listener with CollectionModel. When
> collectionModel detects row key change and there are registered
> listeners, it fires RowKeyChangeEvent to notify all the registered
> listeners, and the affected module will react to the event, update the
> row key references.
>
>
> /**
> * Event that is generated when RowKey changes.
> */
> public class RowKeyChangeEvent extends java.util.EventObject
> {
> /**
> * Creates a new RowKeyChangeEvent
> * @param source the source of the event
> * @param oldRowKey the old row key.
> * @param newRowKey the new row key.
> */
> public RowKeyChangeEvent(
> CollectionModel source, Object oldRowKey,
> Object newRowKey)
> {
> super(source);
> _oldRowKey = oldRowKey;
> _newRowKey = newRowKey;
> }
>
> /**
> * retrieve the old key from the event
> * @return the old key of the event.
> */
> public Object getOldRowKey()
> {
> return _oldRowKey;
> }
>
> /**
> * retrieve the new key from the event
> * @return the new key of the event.
> */
> public Object getNewRowKey()
> {
> return _newRowKey;
> }
>
> private final Object _oldRowKey;
> private final Object _newRowKey;
> }
>
>
> /**
> * Listener for RowKeyChangeEvent.
> */
> public interface RowKeyChangeListener
> {
> public void onRowKeyChange(RowKeyChangeEvent event);
> }
>
>
> public abstract class CollectionModel extends DataModel
> implements RowKeyIndex
> {
> ...
>
> /**
> * register the specified listener with the collection model.
> * @param listener The listener to register with the collection model
> */
> public void addRowKeyChangeListener(RowKeyChangeListener listener)
> {
> _rowKeyChangeListeners.add(listener);
> }
>
> /**
> * unregister the specified listener with the collection model.
> * @param listener The listener to unregister with the collection model
> */
> public void removeRowKeyChangeListener(RowKeyChangeListener listener)
> {
> _rowKeyChangeListeners.remove(listener);
> }
>
> /**
> * retrieve the registered listeners
> * @return the registered listeners
> */
> public void Set<RowKeyChangeListener> getRowKeyChangeListeners()
> {
> return Collections.unmodifiableSet(_rowKeyChangeListeners);
> }
>
> ...
>
> private Set<RowKeyChangeListener> _rowKeyChangeListeners = new
> HashSet<RowKeyChangeListener>();
> }
>
> Notes regarding the proposal:
>
> (1) For better performance, if a module holds references to row key,
> but knows that the row key will not change, e.g. read only table, the
> module should not register itself as listener to collection model.
>
> (2) If a module has difference lifecycle than the collection model,
> the module needs to re-register with collection model whenever
> necessarily.
> -Jing
>
