Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 361613

Summary:

Resource#isConflicting is not symmetric

Product:

[Eclipse Project] Platform

Reporter:

Szymon Ptaszkiewicz <sptaszkiewicz>

Component:

Resources

Assignee:

Szymon Ptaszkiewicz <sptaszkiewicz>

Status:

RESOLVED FIXED

QA Contact:

Severity:

major

Priority:

CC:

john.arthorne

Version:

3.8

Target Milestone:

3.8 M3

Hardware:

All

OS:

All

Whiteboard:

Bug Depends on:

Bug Blocks:

254948, 415847

Attachments:

Description	Flags
Patch v1	none
Patch v2	none

Description Szymon Ptaszkiewicz

2011-10-21 04:02:27 EDT

Current implementation of Resource#isConflicting is not symmetric as it should be if we use multi-rule. For a project with two folders a and b we get the following results:

multi = MultiRule.combine(a, b);
a.isConflicting(multi); => false but should be true
multi.isConflicting(a); => true

Patch will follow.

Comment 1 Szymon Ptaszkiewicz

2011-10-21 04:06:47 EDT

Created attachment 205691 [details]
Patch v1

Comment 2 John Arthorne

2011-10-21 09:18:32 EDT

You could just do something like this:

if (rule instanceof MultiRule)
  return rule.isConflicting(this);

Comment 3 Szymon Ptaszkiewicz

2011-10-21 09:39:23 EDT

(In reply to comment #2)
> You could just do something like this:
> 
> if (rule instanceof MultiRule)
>   return rule.isConflicting(this);

True. I just wanted to keep the order of the arguments unchanged when calling subsequent #isConflicting. If we switch the order, it can be hard to debug further enhancements (like bug 254948).

Comment 4 Szymon Ptaszkiewicz

2011-10-21 10:42:12 EDT

Created attachment 205732 [details]
Patch v2

Adjusted to comment 2.

Comment 5 John Arthorne

2011-10-21 10:43:28 EDT

Looks good.

Comment 6 John Arthorne

2011-10-21 10:44:01 EDT

And that was a good find! Thanks for tracking this down.

Comment 7 Szymon Ptaszkiewicz

2011-10-21 10:47:43 EDT

Shall I release this for M3?

Comment 8 John Arthorne

2011-10-21 11:01:08 EDT

Assuming all core resource tests pass, go for it.

Comment 9 Szymon Ptaszkiewicz

2011-10-21 17:40:58 EDT

Thanks! Fixed in master.