Community
Participate
Working Groups
While AbstractRepositoryTask's and AbstractQueryHit's are not the same thing because the latter does not have Task Data, we should probably provide a common interface to them so that clients don't have to repeatedly check which they are working with and create special rules.
I also wonder if there should be common mechanism to mark "read" status. I.e. for bugzilla you can tell if task is read because query is using "changed since", then you can just record lastread timestamp. So, you won't have to create task for query only to mark it read.
We also need to consider hits adapting to repository tasks, and repository tasks adapting to repositories.
We also discussed while ago that query sync should not remove query hits but mark them as "removed" (similar to incoming deletes in Team Synchronize view). So, user could use "mark as read" or open them in the editor to clear change marker and actually remove hit from query results. Another issue also discussed several times was to clear incoming markers after task editor is closed (and not before like it is now). Anyways, should this go into 2.0 plan?
Mik, you mentioned that we could possible get rid of hits entirely and replace them by tasks? That would work well for Trac and JIRA since these repositories return almost all task information when queried and it would potentially simplify the API.
*** Bug 187310 has been marked as a duplicate of this bug. ***
Rob, consider adding a call to QueryHitCollector that allows connectors (such as JIRA) to submit a RepositoryTaskData object if available: public void addHit(RepositoryTaskData hit); public void addHit(AbstractRepositoryTask hit); The framework could create the corresponding task through the (yet to be implemented) task factory and update it from the task data. The task data could be stored in the offline repository for instant opening of tasks and offline support. We discussed this before but I can't find a corresponding bug.
Created attachment 69646 [details] hits removed (alpha) Here is a first pass at removal of hits form the code base. Use of the ITaskFactory is questionalble so suggestions welcome.
Created attachment 69647 [details] mylar/context/zip
Could you give a quick overview what the patch does in terms of task list migration? Remarks from a first glance: * QueryHitCollector.accept() should throw an IllegalArgumentException in case of a null argument * Why is ITaskFactory needed? QueryHitCollector could invoke this when accepting task data: AbstractRepositoryTask task = connector.makeTask(repository.getUrl(), taskData.getId(), taskData.getSummary()); updateTaskFromTaskData(repository, task, taskData, false); * Rename AbstractRepositoryConnector.makeTask () to createTask() * Why is there still a BugzillaQueryHit class? * grep for System.*.println() before the release :) Rob, is this patch good for applying to my bootstrapped workspace?
(In reply to comment #9) > Could you give a quick overview what the patch does in terms of task list > migration? Since query hits wrapped existing tasks, we should be okay but all queries will need to be re-synched up. Once this is done query hits (now tasks) are externalized much like sub tasks are now, by handle. > * QueryHitCollector.accept() should throw an IllegalArgumentException in case > of a null argument > * Why is ITaskFactory needed? QueryHitCollector could invoke this when > accepting task data: > AbstractRepositoryTask task = connector.makeTask(repository.getUrl(), > taskData.getId(), taskData.getSummary()); > updateTaskFromTaskData(repository, task, taskData, false); The problem is (as usual) synchronization and the saving of task data. Under certain circumstances we need the data returned to just be saved offline and in others to be saved via the synchronization manager so that if there are incoming upon task data update the proper state results on the task. That's why I need to pass in a TaskFactory that is capable of this. Unfortunately the synchronization related code is in UI so this all can't simply be done in the connector (hence the use of an interface then passing in TaskFactory from ui). Thoughts? > * Rename AbstractRepositoryConnector.makeTask () to createTask() Done. > * Why is there still a BugzillaQueryHit class? Deleted. > * grep for System.*.println() before the release :) Yup, will do. > Rob, is this patch good for applying to my bootstrapped workspace? I'll test on my workspace and let you know.
Rob: I am not able to review this today but can review it with you on Monday.
Created attachment 69734 [details] Updated patch
I get merge conflicts due to recent changes in JIRA.
Created attachment 69789 [details] Updated patch
Created attachment 69828 [details] updated patchnew hit icons restored, concurrency bug fix
If you boot strapped on what I committed earlier you may need to again and resynch your queries if you find upon opening hits they don't have all the regular actions available.
Even with the latest changes that allow connectors to create task data instead of query hits JIRA still downloads all tasks three times: 1. Query is performed 2. JiraRepositoryConnector.getChangedSinceLastSync() is invoked which downloads all tasks including the ones that were already received in the previous step and returns a list of changed tasks 3. Tasks returned by step 2 are synchronized Here is my suggestion to fix this: Narrow the query in step 2 as good as possible by filtering by creation date only taking the tasks into account that were not already downloaded in step 1. Filtering by issue key/id would be preferable but as far as I know JIRA can not do that. Always return an empty list in step 2 since at that point the synchronization has already been completed.
Steffen, what do you mean by narrowing "changed since" query? Can we make getChangedSinceLastSync() call to use same signature as performQuery()? Currently that call don't receive monitor and there is no way to pass retrieved task data. Later we can also use repositoryQuery argument to get only changed hits for given query in order to avoid full query sync every time.
(In reply to comment #18) > Steffen, what do you mean by narrowing "changed since" query? The query that retrieves a list of changed tasks in getChangedSinceLastSync(). I am making a few assumptions here: * Most tasks that change are recently created tasks. These changes are picked up by queries or the user is not interested. * Tasks that are in the archive category are old. * If we narrow the query by creation date and modification timestamp we should only pick up few tasks and still be able to notify about changes in the archive category. This is a lot better than the current implementation that downloads all changes for all tasks. > Can we make getChangedSinceLastSync() call to use same signature as > performQuery()? Not really, since getChangedSinceLastSync() works across the whole repository whereas performQuery() is for a single query only. Why that help anyhow? > Currently that call don't receive monitor and there is no way to > pass retrieved task data. That needs to be fixed. Rob? > Later we can also use repositoryQuery argument to get > only changed hits for given query in order to avoid full query sync every time. We can't avoid running the query and at that point we have the task data already. What worries me is that we don't get the complete task data from queries. This could be tricky to synchronize with the existing task data correctly.
(In reply to comment #19) > I am making a few assumptions here: > > * Most tasks that change are recently created tasks. These changes are picked > up by queries or the user is not interested. > * Tasks that are in the archive category are old. I don't think those assumptions can be made. Let's take simple example - status changed to closed (so, task don't match query anymore) and people keep commenting. > * If we narrow the query by creation date and modification timestamp we should > only pick up few tasks and still be able to notify about changes in the archive > category. This is a lot better than the current implementation that downloads > all changes for all tasks. It not supposed to. There is filter by time already: FilterDefinition changedFilter = new FilterDefinition("Changed Tasks"); changedFilter.setUpdatedDateFilter(new RelativeDateRangeFilter(RangeType.MINUTE, minutes)); changedFilter.setOrdering(new Order[] { new Order(Order.Field.UPDATED, false) }); > > Can we make getChangedSinceLastSync() call to use same signature as > > performQuery()? > Not really, since getChangedSinceLastSync() works across the whole repository > whereas performQuery() is for a single query only. Why that help anyhow? It won't help for full repository tasks but it could help to get only updated tasks for given query. see below. Also, right now I am not sure how can we pass the task data for those changed tasks, so we won't have to read or synchronize them again. > > Later we can also use repositoryQuery argument to get only changed > > hits for given query in order to avoid full query sync every time. > We can't avoid running the query and at that point we have the task data already. There are two things: -- if we get updated tasks from getChangedSinceLastSync() then there is no need to synchronize query (your step 1 can be skipped all together) -- on the other hand, if we only need to synchronize single query, its parameters can be updated (in some cases) to take into account tasks that changed since last sync within that query only. > What worries me is that we don't get the complete task data from > queries. This could be tricky to synchronize with the existing task data > correctly. I am not sure what you mean.
(In reply to comment #20) > > * Most tasks that change are recently created tasks. These changes are picked > > up by queries or the user is not interested. > > * Tasks that are in the archive category are old. > I don't think those assumptions can be made. Let's take simple example - status > changed to closed (so, task don't match query anymore) and people keep > commenting. True, but would assume that that is not a common case. > > * If we narrow the query by creation date and modification timestamp we should > > only pick up few tasks and still be able to notify about changes in the > archive > > category. This is a lot better than the current implementation that downloads > > all changes for all tasks. > It not supposed to. There is filter by time already: > FilterDefinition changedFilter = new FilterDefinition("Changed Tasks"); > changedFilter.setUpdatedDateFilter(new RelativeDateRangeFilter(RangeType.MINUTE, > minutes)); This will essentially pick up all changed tickets since the last synchronization. Since we frequently synchronize this means we currently pick up almost all changes ever made in the repository. Additionally filtering by creation time could optimize this query. > changedFilter.setOrdering(new Order[] { new Order(Order.Field.UPDATED, false) > }); > > > Can we make getChangedSinceLastSync() call to use same signature as > > > performQuery()? > > Not really, since getChangedSinceLastSync() works across the whole repository > > whereas performQuery() is for a single query only. Why that help anyhow? > It won't help for full repository tasks but it could help to get only updated > tasks for given query. see below. > Also, right now I am not sure how can we pass the task data for those changed > tasks, so we won't have to read or synchronize them again. > > > Later we can also use repositoryQuery argument to get only changed > > > hits for given query in order to avoid full query sync every time. > > We can't avoid running the query and at that point we have the task data > already. > There are two things: > -- if we get updated tasks from getChangedSinceLastSync() then there is no need > to synchronize query (your step 1 can be skipped all together) You don't know how that affects the query results unless you replicate the query engine. Also you download information about all created tasks. Consider a repository such as Apache where you might only be interested in a single project out of hundreds. > -- on the other hand, if we only need to synchronize single query, its > parameters can be updated (in some cases) to take into account tasks that > changed since last sync within that query only. What about tasks that are not part of a query? Like the ones in the archive category or manually imported tasks? > > What worries me is that we don't get the complete task data from > > queries. This could be tricky to synchronize with the existing task data > > correctly. > I am not sure what you mean. How is that currently handled if the editor is opened and information about the operations and editable fields has not been downloaded, yet?
(In reply to comment #19) > > Currently that call don't receive monitor and there is no way to > > pass retrieved task data. > > That needs to be fixed. Rob? We could change the getChanged call to return a suitable AbstractRepositoryQuery that is then passed to performQuery which handles task data properly?
(In reply to comment #21) > True, but would assume that that is not a common case. Sorry, I am not following your logic. We need to know "all" tasks had been changed and not "some" tasks. > > It not supposed to. There is filter by time already: > > FilterDefinition changedFilter = new FilterDefinition("Changed Tasks"); > > changedFilter.setUpdatedDateFilter(new RelativeDateRangeFilter(RangeType.MINUTE, > > minutes)); > > This will essentially pick up all changed tickets since the last > synchronization. Right it will pickup the only changed tickets since last synchronization. > Since we frequently synchronize this means we currently pick > up almost all changes ever made in the repository. No. We only pickup what had been changed since last synchronization, but not all. > Additionally filtering by creation time could optimize this query. I am not sure how we can scope by creation date. I would think that number of tasks that could have chanced since last synchronization is usually less then number of tasks since some arbitrary creation date. What am I missing? > > > We can't avoid running the query and at that point we have the task data > > already. > > There are two things: > > -- if we get updated tasks from getChangedSinceLastSync() then there is no need > > to synchronize query (your step 1 can be skipped all together) > > You don't know how that affects the query results unless you replicate the > query engine. True. It would miss added and removed tasks. > Also you download information about all created tasks. Consider a > repository such as Apache where you might only be interested in a single > project out of hundreds. That is why I thought of passing query and adding "changed since" condition. > > -- on the other hand, if we only need to synchronize single query, its > > parameters can be updated (in some cases) to take into account tasks that > > changed since last sync within that query only. > > What about tasks that are not part of a query? Like the ones in the archive > category or manually imported tasks? Those haven't been asked to be synchronized in this case. It is about synchronizing individual query. > > > What worries me is that we don't get the complete task data from > > > queries. This could be tricky to synchronize with the existing task data > > > correctly. > > I am not sure what you mean. > How is that currently handled if the editor is opened and information > about the operations and editable fields has not been downloaded, yet? It is always downloaded when task data is updated.
(In reply to comment #22) > We could change the getChanged call to return a suitable > AbstractRepositoryQuery that is then passed to performQuery which handles task > data properly? Interesting idea, though getChanged need to filter out tasks that aren't in the task list and should not download their task data. So some logic is connector specific: 1 run query 2 filter/skip entries that aren't in the task list 3 create task data (in jira we can create most of the task data from query hit, so we need that hit around) you can do 1 and 2 if you get the query, but it is unclear how would you pass query hit from step 1 to 3.
(In reply to comment #23) > > Since we frequently synchronize this means we currently pick > > up almost all changes ever made in the repository. > No. We only pickup what had been changed since last synchronization, but not > all. Yes, but for a repository that has hundreds of projects this can be an enormous amount of data. > > Additionally filtering by creation time could optimize this query. > I am not sure how we can scope by creation date. I would think that number of > tasks that could have chanced since last synchronization is usually less then > number of tasks since some arbitrary creation date. What am I missing? All I am saying that we should additionally scope by creation date. > > > -- if we get updated tasks from getChangedSinceLastSync() then there is no > need > > > to synchronize query (your step 1 can be skipped all together) > > > > You don't know how that affects the query results unless you replicate the > > query engine. > True. It would miss added and removed tasks. But that's not acceptable, is it? > > Also you download information about all created tasks. Consider a > > repository such as Apache where you might only be interested in a single > > project out of hundreds. > That is why I thought of passing query and adding "changed since" condition. Got your point now. Seems like we are trying to solve this problem from different angles. I am trying to keep the current logic and API whereas you want to change the way queries are synchronized. > > > > What worries me is that we don't get the complete task data from > > > > queries. This could be tricky to synchronize with the existing task data > > > > correctly. > > > I am not sure what you mean. > > How is that currently handled if the editor is opened and information > > about the operations and editable fields has not been downloaded, yet? > It is always downloaded when task data is updated. I'd be great if that could be optimized to download it when the editor is opened for the first time.
(In reply to comment #24) > > We could change the getChanged call to return a suitable > > AbstractRepositoryQuery that is then passed to performQuery which handles task > > data properly? > Interesting idea, though getChanged need to filter out tasks that aren't in the > task list and should not download their task data. Doesn't work well for Trac since it's not a query but an API call but I could work around that. I like the idea though: 1 Keep the current logic of synchronizing each query. 2 Then pass all tasks to the connector to create a query to determine the changed tasks. 3 The connector picks out the ones that haven't been synchronized and returns a query. 4 The query is run. > So some logic is connector > specific: > 1 run query > 2 filter/skip entries that aren't in the task list > 3 create task data (in jira we can create most of the task data from query hit, > so we need that hit around) > you can do 1 and 2 if you get the query, but it is unclear how would you pass > query hit from step 1 to 3. Could the QueryHitCollector take care of that?
(In reply to comment #25) > Yes, but for a repository that has hundreds of projects this can be an enormous > amount of data. I think it is not related to number of projects but to number of changes between synchronizations. > All I am saying that we should additionally scope by creation date. Sorry, I still don't understand. Where we get that date from? And why we only should scope by date and not by some field X? > > That is why I thought of passing query and adding "changed since" condition. > Got your point now. Seems like we are trying to solve this problem from > different angles. I am trying to keep the current logic and API whereas you want > to change the way queries are synchronized. He he. > I'd be great if that could be optimized to download it when the editor is opened > for the first time. What if editor is opened for the first time when user went offline? For JIRA there are several options, such as cache some of that info per repository (assuming user have same permissions), or improve JIRA remoting API.
(In reply to comment #26) > Doesn't work well for Trac since it's not a query but an API call but I could > work around that. I like the idea though: > > 1 Keep the current logic of synchronizing each query. > 2 Then pass all tasks to the connector to create a query to determine the > changed tasks. > 3 The connector picks out the ones that haven't been synchronized and > returns a query. > 4 The query is run. Something is missing there. For JIRA we can only construct "changed since" query, get hits and filter them out using passed list of tasks. After that for we need to get task data for hits that left. Each hit associated with jira's Issue object that has most of the task data and I am not sure how do we pass that object, so JIRA connector won't have to retrieve it again. The only option I see there is to construct "changed since" query and pass it along with the list of tasks to performQuery() method, so it could filter query results.
> (In reply to comment #25) > > Yes, but for a repository that has hundreds of projects this can be an > enormous > > amount of data. > I think it is not related to number of projects but to number of changes between > synchronizations. My point is that I am probably only interested in a small portion of the repository and want to limit the queries to these tasks as good as possible. > > All I am saying that we should additionally scope by creation date. > Sorry, I still don't understand. Where we get that date from? And why we only > should scope by date and not by some field X? I was trying to optimize the current implementation that only has a list of tasks. > > I'd be great if that could be optimized to download it when the editor is > opened > > for the first time. > What if editor is opened for the first time when user went offline? Display whatever data there is. > For JIRA there are several options, such as cache some of that info per > repository (assuming user have same permissions), or improve JIRA remoting API. Yes, caching would be good. I am afraid I have not completely understood how the automatic query synchronization process works with the changes you have suggested. In particular how is the current functionality ensured: 1 Display accurate results when queries are synchronized 2 Get change notifications for tasks not part of a query
(In reply to comment #28) > Something is missing there. For JIRA we can only construct "changed since" > query, get hits and filter them out using passed list of tasks. After that for > we need to get task data for hits that left. Not sure I understand. We return the query and it is the responsibility of the QueryHitCollector in that case to throw away hits that are not in the task list. > The only option I see there is to construct "changed since" query and pass it > along with the list of tasks to performQuery() method, so it could filter query > results. The caller of getChangedQuery() has that information and can construct a QueryHitCollector that does the filtering.
(In reply to comment #29) > My point is that I am probably only interested in a small portion of the > repository and want to limit the queries to these tasks as good as possible. But the only criteria you have is the list of tasks that need to be checked and the date of last sync. > > > All I am saying that we should additionally scope by creation date. > > Sorry, I still don't understand. Where we get that date from? And why we only > > should scope by date and not by some field X? > I was trying to optimize the current implementation that only has a list of tasks. So where you were planning to get that date? > > What if editor is opened for the first time when user went offline? > Display whatever data there is. I don't think it is a good idea because you won't be edit this task. > > For JIRA there are several options, such as cache some of that info per > > repository (assuming user have same permissions), or improve JIRA remoting > > API. > Yes, caching would be good. Hmm. Few days ago you said it isn't. > I am afraid I have not completely understood how the automatic query > synchronization process works with the changes you have suggested. In particular > how is the current functionality ensured: > 1 Display accurate results when queries are synchronized > 2 Get change notifications for tasks not part of a query See below. (In reply to comment #30) > Not sure I understand. We return the query and it is the responsibility of the > QueryHitCollector in that case to throw away hits that are not in the task list. Sure, but it would also throw away Issue instance > > The only option I see there is to construct "changed since" query and pass it > > along with the list of tasks to performQuery() method, so it could filter > > query results. > > The caller of getChangedQuery() has that information and can construct a > QueryHitCollector that does the filtering. Yes, but results of that filtering need to be processed by connector. Here is JIRA code (I've changed current stuff into using task data instead of task): final List<Issue> issues = new ArrayList<Issue>(); JiraIssueCollector collector = new JiraIssueCollector(monitor, issues, 500); client.search(changedFilter, collector); Set<AbstractRepositoryTask> changedTasks = new HashSet<AbstractRepositoryTask>(); for (Issue issue : issues) { // here is the filtering, practically same thing as searching trough list of issues passed ITask task = TasksUiPlugin.getTaskListManager().getTaskList().getTask(repository.getUrl(), issue.getId()); if (task instanceof AbstractRepositoryTask) { RepositoryTaskData taskData = offlineHandler.createTaskData(repository, client, issue); queryHitCollector.add(taskData); } if (issue.getUpdated() != null && issue.getUpdated().after(lastSyncDate)) { lastSyncDate = issue.getUpdated(); } }
> > > > All I am saying that we should additionally scope by creation date. > > > Sorry, I still don't understand. Where we get that date from? And why we > only > > > should scope by date and not by some field X? > > I was trying to optimize the current implementation that only has a list of > tasks. > So where you were planning to get that date? We can retrieve it from the list of tasks passed to the method. The reason for using the creation date is that it is the only searchable stable property a task has. > > > For JIRA there are several options, such as cache some of that info per > > > repository (assuming user have same permissions), or improve JIRA remoting > > > API. > > Yes, caching would be good. > Hmm. Few days ago you said it isn't. Please reread bug 190662#c9 . > > Not sure I understand. We return the query and it is the responsibility of the > > QueryHitCollector in that case to throw away hits that are not in the task > list. > Sure, but it would also throw away Issue instance The QueryHitCollector receives a task data object so I don't see how that is a problem. I don't think it matters much where the filtering is done. > Yes, but results of that filtering need to be processed by connector. Here is > JIRA code (I've changed current stuff into using task data instead of task): [...] That could work.
(In reply to comment #32) > Please reread bug 190662#c9 . I don't think it is a good idea to make caching conditional. > The QueryHitCollector receives a task data object so I don't see how > that is a problem. I don't think it matters much where the filtering is done. The thing is that we don't want to create full task data before we'll know if issue is included or not. See code from my previous comment. So, filtering seem to have to stay inside getChanged... method, or we need to add list of tasks to performQuery().
IProgressMonitor parameter added to getChangedSinceLastSync(). I've created bug#191575 to track optimization of Jira as discussed above.
