Bug 482534 - [UCOSP] persist task review mappings
Summary: [UCOSP] persist task review mappings
Status: RESOLVED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Mylyn (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows 7
: P3 enhancement (vote)
Target Milestone: 2.10   Edit
Assignee: Blaine Lewis CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks: 477635
  Show dependency tree
 
Reported: 2015-11-18 16:23 EST by Blaine Lewis CLA
Modified: 2016-01-11 16:11 EST (History)
0 users

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Blaine Lewis CLA 2015-11-18 16:23:50 EST 
Make the TaskReviewMappingStore serialize all mappings to disk using JSON.
Comment 1 Eclipse Genie CLA 2015-11-18 16:25:42 EST 
New Gerrit change created: https://git.eclipse.org/r/60754
Comment 2 Eclipse Genie CLA 2015-11-20 02:11:11 EST 
New Gerrit change created: https://git.eclipse.org/r/60862
Comment 3 Eclipse Genie CLA 2015-11-23 23:38:03 EST 
New Gerrit change created: https://git.eclipse.org/r/61096
Comment 4 Eclipse Genie CLA 2015-11-27 14:54:18 EST 
New Gerrit change created: https://git.eclipse.org/r/61495
Comment 5 Sam Davis CLA 2015-11-27 16:00:38 EST 
Blaine, I think I owe you an apology. You've implemented exactly what I asked for here, but now that I have the implementation in front of me I'm realizing my design for this wasn't the best. I think we can simplify the persistence by storing the URL of each review's associated task as an attribute on the review's ITask rather than writing JSON files to disk. At startup, instead of reading in a lot of JSON files to construct the mapping, we can build the Mulitmap from [task URL] to [list of review URLs] just by iterating over all reviews in the task list. This will have a number of benefits:

* we don't need to keep a second map for the reverse mapping from reviews to tasks, because given a review we can just get the task URL from the ITask
* we don't need to worry about serialization and deserialization, or scheduling too many jobs
* the ReviewConnector can put the task URL in the ITask in updateTaskFromTaskData, so that your listener doesn't have to load the task data or do the URL parsing. This would be better for performance because the listener gets called many times each time a task changes.

I've pushed a review to show you what I mean. It probably needs a few null checks and the tests need to be updated. Can you take a look?
61495: 482534: [UCOSP] store task review mapping using on review ITasks [I621893f4]
https://git.eclipse.org/r/#/c/61495/
Comment 6 Sam Davis CLA 2015-11-27 17:23:45 EST 
(In reply to comment #5)
> * the ReviewConnector can put the task URL in the ITask in
> updateTaskFromTaskData, so that your listener doesn't have to load the task data
> or do the URL parsing. This would be better for performance because the listener
> gets called many times each time a task changes.

This isn't quite right. updateTaskFromTaskData gets called the same number of times as the listener, the difference is that it already has the task data from the connector, so we aren't loading it from disk over and over again.