Community
Participate
Working Groups
The task list data model is dag of objects of type AbstractTaskContainer and its subclasses. While the presentation (task list view) is meant to be extensible the primary data model and its structure is managed in the task list. The task list exposes methods such add addTask() and moveTask() to manipulate the structure but ensures that certain invariants hold when modifications are executed (e.g. no cycles, local tasks only have one parent). It also guarantees thread-safety if these methods are invoked concurrently. AbstractTaskContainer is currently part of the public API and connectors need to extend it to add tasks to the task list. Most connectors overwrite very few methods and store very little custom state on tasks objects. Since the task class is primarily intended as a data container it does not implement any behavior. This API can be slightly confusing to clients since it requires implementation overhead such as implementing a factory or management of task life-cycle without getting the benefits of inheritance, i.e. modification of behavior. In addition AbstractTaskContainer and AbstractTask have methods that are prefixed with "internal" and are not intended to be invoked by clients. Still these methods are part of the public API and there is slight risk that a client could corrupt the task list model by invoking these methods. In order to separate the concerns of the primary data model and the presentation better, to simplify the intended extensibility and to better guard against undesired structural changes the management of task objects should entirely handled by the task list. All objects that are part of the task list should be constructed through the task list and their life cycle should be managed in the task list. Only interfaces should be exposed to clients that allow limited extensibility, e.g. by allowing clients to store key/value pairs of Strings on task list elements. All changes to a task and to the task list structure should be properly synchronized and change notifications sent through the task list.
Created attachment 98591 [details] whiteboard capture
Now party done with the ITask and ITaskElement extraction...
This involved a considerable amount of manual work and I haven't been able to finish it yet. Will plan to check in early tomorrow, please avoid making changes in the meantime since just about everything in ..tasks.ui gets touched.
Steffen: First pass at extracting an ITask and ITaskElement interface from AbstractTask and AbstractTaskContainer is done. I managed to localize all the problematic casts to AbstractTask in TaskList, and we will need to fix those up. This pass at the refactoring is not quite done yet because there is a considerable number of tests failing in AllTasksTests. Could you take a look at those? You have the lock on this data structure, I will not touch it until I hear back.
Due to the scale of the refactoring, before committing I tagged HEAD as R_3_0_RC0_prerefactoring.
Created attachment 98670 [details] mylyn/context/zip
Cool. I'll take a look at the tests. They might have been failing before the refactoring already.
Remaining items for RC0: - rename AbstractTask to Task or create a new Task class that extends AbstractTask - remove deprecated methods from ITask and ITaskElement - remove internal methods from ITask (e.g. setSynchronizationState()) - extract a public interface for AbstractRepositoryQuery - extract a public interface for TaskCategory - create a factory for tasks, queries and categories - support arbitrary attributes on tasks and queries - clean up leakage of internal types - split ITask into ISharedState and IPersonalState
The more I look at ITaskList the more I am thinking that it should be internal until we have a better understanding of the API. It still exposes to much implementation detail, e.g. notifyTaskChanged(). Even the listeners have challenging properties: events are not guaranteed to arrive in order, events occur concurrently, the deltas don't have information what changed (just that something changed). Here is the list of methods that I think should be API of the task model: ITaskModel { public abstract void addQuery(IRepositoryQuery query); public abstract void addTask(ITask task); // does not add task to task list, e.g. for search results public abstract ITask createTask(String connectorKind, String repositoryUrl, String taskId); public abstract ITask getTask(String connectorKind, String repositoryUrl, String taskId); public abstract void deleteQuery(IRepositoryQuery query); public abstract void deleteTask(ITask task); } This API would not allow clients to modify the structure but that is one of the parts that I am struggling with at the moment. It seems odd to have an addTask(ITask, ITask) and addTask(ITaskCategry, ITask) methods. I would expect an addTask() method on ITask and ITaskCategory instead or a separate ITaskContainer interface that allows structural modifications. Are there other methods that are used by clients building on Mylyn?
Created attachment 99223 [details] data model
Created ITasksModel which exposes factory methods for creating tasks. The ITaskList class has been moved to an internal package until we get a better idea what API is needed for manipulation of the task list structure.
