Hi,
I've encountered another issue with ProjectRegistryRefreshJob. This time when renaming a project.
Having a simple setup with project A (gav=test:a:1.0) depending on B (gav=test:b:1.0), when you rename B to B1 and autobuild is on. It is not related to my in-progress bug 445675 with autobuild.
What happens, is RefreshJob receives a PRE_DELETE(B) and ADDED(B1) resourceChange events in sequence. PRE_DELETE schedules a refresh on deleted B, but before it gets to actually run, B1 is getting built by MavenBuilder, which performs a refresh of B1. This results in an error, that test:a is missing a dependency test:b.
Scenario 1 (Autobuild=on)
1. ResourceChanged PRE_DELETE (B)
2. Refresh of B scheduled
3. ResourceChanged ADDED (B1) (ignored by ProjectRegistryRefreshJob)
4. Building B1
4.1. Refresh phase1 B1
4.2. Refresh phase2 B1
4.3. Refresh phase2 A
No errors yet
5. Refresh B
5.1. Refresh phase1 B
5.2. Refresh phase2 B
5.3. Refresh phase2 A (error marker added)
Result: B1(test:a) has an unresolved dependency test:b
To get rid of the error, one has to manually perform Update project on both A and B1.
If you have autobuild turned off, then such a rename will not cause a build of new B1 project. and refresh of deleted B gets to run as soon as possible.
Scenario 2 (Autobuild=off)
1. ResourceChanged PRE_DELETE (B)
2. Refresh of B scheduled
3. ResourceChanged ADDED (B1) (ignored by ProjectRegistryRefreshJob)
4. Refresh B
4.1. Refresh phase1 B
4.2. Refresh phase2 B
4.3. Refresh phase2 A
Error marker added
Autobuild gets turned on manually
5. Building B1
5.1. Refresh phase1 B1
5.2. Refresh phase2 B1
5.3. Refresh phase2 A
Result: no errors
At first I tried a workaround: since AutobuildJob's schedule delay is between 100 and 1000ms depending on circumstances, I put a delay of 10ms to RefreshJob for deleted projects. It did cause a refresh of B to start earlier, however a refresh of B1 (as called by MavenBuilder) ran in parallel before B got refreshed - this is because RefreshJob only locks applyMutableState with workspace rule (lines 101-102 ). Extending it to also include a call to refresh did the trick - Scenario 1 steps were the same as Scenario 2.
I was not entirely happy about such workaround, since under some weird circumstances, build could still run earlier than refresh job, so I finally traced the problem down to ProjectRegistry.
BasicProjectRegistry has a Map<ArtifactKey, IFile> workspaceArtifacts field which can only accomodate one project with the same ArtifactKey. Whenever there is a new project facade (import, gav change, etc.) an entry with corresponding ArtifactKey would get overwritten even if there was something else. All the other registry contents can cope with same-gav projects quite nicely.
So basically project registry inconsistencies will occur whenever there is more than one same-gav project present at the same time. For instance, with the initial setup importing another test:b:1.0 (as B1) project and then changing its version to, say, 1.1 will cause test:a to 'miss' dependency to test:b:1.0 in the presence of seemingly valid B (test:b:1.0).
I went and implemented a patch that turns this map into Map<ArtifactKey, Set<IFile>> (also migrating it from <ArtifactKey, IFile> after workspace state deserialization) that maintains a Set of poms for each ArtifactKey and that fixed all abovementioned scenarios.
Also, the Set there is an instance of LinkedHashSet that preserves the import order of projects and workspace dependency resolution always yields the first one.
Igor, I'd issue a bug and send code for review (when I have a bit more time to make proper tests) unless you have some comments against such change.
PS: could RefreshJob's call to refresh() not being covered by workspace root rule be a potential problem? There are checks against MutableProjectRegistry#isStale() in some places that re-schedule the job though.
--
