Bug 482534

Summary: [UCOSP] persist task review mappings
Product: z_Archived Reporter: Blaine Lewis <Blaine1>
Component: MylynAssignee: Blaine Lewis <Blaine1>
Status: RESOLVED FIXED QA Contact:
Severity: enhancement    
Priority: P3 Keywords: contributed
Version: unspecified   
Target Milestone: 2.10   
Hardware: PC   
OS: Windows 7   
See Also: https://git.eclipse.org/r/60754
https://git.eclipse.org/r/60862
https://git.eclipse.org/r/61096
https://git.eclipse.org/r/61495
https://git.eclipse.org/c/mylyn/org.eclipse.mylyn.reviews.git/commit/?id=7ec7690d253aeafe01625501da8c054cbabbae33
Whiteboard:
Bug Depends on:    
Bug Blocks: 477635    

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.