Bug 265810 - Change event unnecessarily sent for all linked resources
Summary: Change event unnecessarily sent for all linked resources
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Resources (show other bugs)
Version: 3.5   Edit
Hardware: PC All
: P3 normal (vote)
Target Milestone: 3.5 M6   Edit
Assignee: Szymon Brandys CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-02-23 06:06 EST by Szymon Brandys CLA
Modified: 2009-03-03 10:24 EST (History)
2 users (show)

See Also:

Attachments
Fix v01 (7.54 KB, patch)
2009-03-02 17:32 EST, Szymon Brandys CLA 		no flags Details | Diff
Additional fix & improved test (6.34 KB, patch)
2009-03-03 09:59 EST, Szymon Brandys CLA 		no flags Details | Diff
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 Szymon Brandys CLA 2009-02-23 06:06:38 EST 
Build id: I20090217-2200

Steps:
1) Create a project and linked resources inside, e.g. a.txt, b.txt, c.txt
2) Delete c.txt, you'll get an event with such a delta

/[*]: {}
/testProject1[*]: {DESCRIPTION}
/testProject1/.project[*]: {CONTENT}
/testProject1/c.txt[-]: {}

3) Recreate c.txt
4) Open .project and remove c.txt by editing it, you'll get another delta

/[*]: {}
/testProject1[*]: {DESCRIPTION}
/testProject1/.project[*]: {CONTENT}
/testProject1/a.txt[*]: {CONTENT | REPLACED}
/testProject1/b.txt[*]: {CONTENT | REPLACED}
/testProject1/c.txt[-]: {}

Actually any change to .project will trigger an event with CONTENT | REPLACED for all linked resources.
Comment 1 Szymon Brandys CLA 2009-03-02 17:32:45 EST 
Created attachment 127246 [details]
Fix v01
Comment 2 Szymon Brandys CLA 2009-03-02 18:21:59 EST 
Released to HEAD.
Comment 3 James Blackburn CLA 2009-03-03 08:18:19 EST 
-toLink.createLink(newLink.getLocationURI(), IResource.REPLACE | IResource.ALLOW_MISSING_LOCAL, null);
+if (!toLink.exists())
+toLink.createLink(newLink.getLocationURI(), IResource.REPLACE | IResource.ALLOW_MISSING_LOCAL, null);

Doesn't this introduce a bug when an oldLink is being replaced with a newLink (different location) and the link is actually masking a real resource in the filesystem?

In the first part of the reconcileLinks, the old link will be removed, as it's being replaced.  Then the creation of the new link will never occur because toLink.exists() will return true as there's an IResource there already. Should the check be something like: if (!toLink.exists() || toLink.isLinked())
Comment 4 James Blackburn CLA 2009-03-03 08:20:06 EST 
(In reply to comment #3)
> ... || !toLink.isLinked() ...
Comment 5 Szymon Brandys CLA 2009-03-03 09:06:09 EST 
(In reply to comment #3)
> In the first part of the reconcileLinks, the old link will be removed, as it's
> being replaced.  Then the creation of the new link will never occur because
> toLink.exists() will return true as there's an IResource there already. Should
> the check be something like: if (!toLink.exists() || toLink.isLinked())
> 

True. I'll fix it and add yet another test.
Comment 6 Szymon Brandys CLA 2009-03-03 09:59:39 EST 
Created attachment 127322 [details]
Additional fix & improved test
Comment 7 Szymon Brandys CLA 2009-03-03 10:24:18 EST 
Released to HEAD.