Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 358392 - Compare editor does not show author if opened from Synchronize view (NPE)
Summary: Compare editor does not show author if opened from Synchronize view (NPE)
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Team (show other bugs)
Version: 3.8   Edit
Hardware: All All
: P3 major (vote)
Target Milestone: 3.8 M3   Edit
Assignee: Tomasz Zarna CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-21 07:01 EDT by Dani Megert CLA
Modified: 2011-09-28 08:20 EDT (History)
4 users (show)

See Also:

Attachments
Fix v01 (2.80 KB, patch)
2011-09-23 10:03 EDT, Tomasz Zarna CLA 		no flags Details | Diff
mylyn/context/zip (50.46 KB, application/octet-stream)
2011-09-23 10:04 EDT, Tomasz Zarna CLA 		no flags Details
Show Obsolete (1) View All 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 Dani Megert CLA 2011-09-21 07:01:42 EDT 
1.1.0.201109151100-r.

A compare editor opened from the Synchronize view does not show the right author on the right side if
    Team > [x] Show the file author in compare editors
is enabled.

Instead it throws and NPE:


!ENTRY org.eclipse.core.jobs 4 2 2011-09-21 12:51:49.678
!MESSAGE An internal error occurred during: "Updating Compare Editor".
!STACK 0
java.lang.NullPointerException
	at org.eclipse.team.internal.ui.synchronize.LocalResourceTypedElement.fetchAuthor(LocalResourceTypedElement.java:383)
	at org.eclipse.team.internal.ui.mapping.ResourceDiffCompareInput.updateAuthorInfo(ResourceDiffCompareInput.java:357)
	at org.eclipse.team.internal.ui.mapping.ResourceCompareInputChangeNotifier.fetchAuthors(ResourceCompareInputChangeNotifier.java:327)
	at org.eclipse.team.internal.ui.mapping.ResourceCompareInputChangeNotifier$3.run(ResourceCompareInputChangeNotifier.java:321)
	at org.eclipse.team.internal.core.BackgroundEventHandler$RunnableEvent.run(BackgroundEventHandler.java:176)
	at org.eclipse.team.internal.ui.mapping.CompareInputChangeNotifier$InputChangeEventHandler.executeRunnableNow(CompareInputChangeNotifier.java:146)
	at org.eclipse.team.internal.ui.mapping.CompareInputChangeNotifier$InputChangeEventHandler.doDispatchEvents(CompareInputChangeNotifier.java:101)
	at org.eclipse.team.internal.core.BackgroundEventHandler.dispatchEvents(BackgroundEventHandler.java:394)
	at org.eclipse.team.internal.core.BackgroundEventHandler.processEvents(BackgroundEventHandler.java:374)
	at org.eclipse.team.internal.core.BackgroundEventHandler$1.run(BackgroundEventHandler.java:203)
	at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54)
Comment 1 Dani Megert CLA 2011-09-21 07:02:00 EDT 
Might be a Platform Team bug.
Comment 2 Remy Suen CLA 2011-09-21 17:41:22 EDT 
(In reply to comment #1)
> Might be a Platform Team bug.

Confirmed. The API for withAllProperties(*) specs that 'null' is a valid return value. As an aside, it doesn't spec when a CoreException should be thrown.

------------------

revision= revision.withAllProperties(monitor);

author= revision.getAuthor();
Comment 3 Szymon Brandys CLA 2011-09-22 08:06:20 EDT 
Tomek, could you or Gosia look at it?
Comment 4 Tomasz Zarna CLA 2011-09-22 11:38:22 EDT 
It looks like this statement "NOTE: Must not check for revision#isPropertyMissing() as this will always return true for the workspace file revision"[1] is not quite for local revisions used in EGit. I would fix it on both sides: in Team, to be prepared for nulls and in EGit to return this when #withAllProperties is called for local revisions. What do you think Remy?

[1] org.eclipse.team.internal.ui.synchronize.LocalResourceTypedElement.fetchAuthor(IProgressMonitor)
Comment 5 Remy Suen CLA 2011-09-22 14:04:31 EDT 
(In reply to comment #4)
> I would fix it
> on both sides: in Team, to be prepared for nulls and in EGit to return this
> when #withAllProperties is called for local revisions.

Yes, I also noticed this problem while looking at this code yesterday. It's likely we should return something valid instead of 'null'. There might be a reason for this but I'd have to check the history in more detail.
Comment 6 Tomasz Zarna CLA 2011-09-23 10:03:39 EDT 
Created attachment 203903 [details]
Fix v01

On second thought, EGit seems to be fine, it returns false when asked if any property is missing (#isPropertyMissing), meaning the revision is complete. So Team should not ask for other properties (#withAllProperties).
Comment 7 Tomasz Zarna CLA 2011-09-23 10:04:00 EDT 
Created attachment 203904 [details]
mylyn/context/zip
Comment 8 Tomasz Zarna CLA 2011-09-26 12:29:03 EDT 
Fixed with http://git.eclipse.org/c/platform/eclipse.platform.team.git/commit/?id=8afabc748e2b34711690564a9b390a43c3f5a295, a slightly modified Fix v01. 

Available in builds >= N20110926-2000.
Comment 9 Dani Megert CLA 2011-09-27 08:11:08 EDT 
> Available in builds >= N20110926-2000.

Not really ;-).
Comment 10 Tomasz Zarna CLA 2011-09-28 06:54:18 EDT 
Right, my bad, it's in I20110927-0800. I forgot to update the map file for said n-build (bug 359030). I've also filed bug 359071 against EGit for the missing author.
Comment 11 Dani Megert CLA 2011-09-28 08:20:28 EDT 
Verified that the NPE is gone.