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

Bug 315775

Summary:

IResource.accept(IResourceVisitior,...) should not check existence

Product:

[Eclipse Project] Platform

Reporter:

Dani Megert <daniel_megert>

Component:

Resources

Assignee:

Szymon Ptaszkiewicz <sptaszkiewicz>

Status:

RESOLVED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

markus.kell.r, Szymon.Brandys

Version:

3.6

Keywords:

api

Target Milestone:

3.8 M2

Hardware:

All

OS:

All

Whiteboard:

Attachments:

Description	Flags
Patch v1	none
Patch v2	none
Patch v3	none
Patch v4	none
Patch v5	none
Patch with updated Javadocs	none

Description Dani Megert

2010-06-04 10:48:56 EDT

3.6 RC4.

IResource.accept(IResourceVisitor,...) should not check the existence of the visited elements as this is not specified and should be left to the visitor itself. According to the spec it should only check the resource on which accept(...) is called. Note that the implementation already does this when using an IResourceProxyVisitor.

Personally I also find the spec wrong as it shouldn't even check the first resource because that only complicates the visitor implementation (it has be able to handle existing and non-existing elements). This applies to both, IResourceVisitor and IResourceProxyVisitor.


Changing this after several years is probably too dangerous, hence I suggest to
1. document this clearly
2. introduce a new member flag: IResource.DO_NOT_CHECK_EXISTENCE which would
   allow to disable throwing exceptions

Comment 1 Szymon Brandys

2011-03-30 08:24:14 EDT

Point 1) I would make IResource.accept(IResourceVisitor,...) and IResource.accept(IResourceProxyVisitor,...) consistent and would check the existence only for the resource on which accept is called. Since the change may affect callers, I would postpone this change to 3.8.

Point 2) IResource.DO_NOT_CHECK_EXISTENCE should be moved to 3.8.

Comment 2 Dani Megert

2011-03-30 11:29:56 EDT

(In reply to comment #1)
+1.

Comment 3 Szymon Ptaszkiewicz

2011-07-27 05:46:53 EDT

Created attachment 200435 [details]
Patch v1

Comment 4 Szymon Brandys

2011-08-16 05:05:16 EDT

Comments:

- increment the plug-in version, we have an API change here
- I would change the doc in IResource#accept(s) 

from 
@param memberFlags bit-wise or of member flag constants
	 *   ({@link IContainer#INCLUDE_PHANTOMS}, {@link IContainer#INCLUDE_TEAM_PRIVATE_MEMBERS},
	 *   {@link IContainer#INCLUDE_HIDDEN} and {@link IResource#DO_NOT_CHECK_EXISTENCE})
	 *   indicating which members are of interest and if the resource on which the method
	 *   is called should be checked for the existence
	 
to

@param memberFlags bit-wise or of member flag constants
	 *   ({@link IContainer#INCLUDE_PHANTOMS}, {@link IContainer#INCLUDE_TEAM_PRIVATE_MEMBERS},
	 *   {@link IContainer#INCLUDE_HIDDEN} indicating which members are of interest and {@link IResource#DO_NOT_CHECK_EXISTENCE})
	 *   if the resource on which the method is called should NOT be checked for the existence
	 
- I think Point 1) from comment 1 is not addressed
- update the migration guide about the change

Comment 5 Szymon Brandys

2011-08-16 05:09:36 EDT

(In reply to comment #4)
> - I think Point 1) from comment 1 is not addressed
Ignore this point.

Comment 6 Szymon Ptaszkiewicz

2011-08-16 08:28:48 EDT

Created attachment 201563 [details]
Patch v2

Comment 7 Szymon Ptaszkiewicz

2011-08-16 08:31:26 EDT

In Patch v2 there is also one small fix for a typo in 3.7 migration guide.

Comment 8 Szymon Ptaszkiewicz

2011-08-16 09:44:37 EDT

Created attachment 201565 [details]
Patch v3

Changed recommended.html according to Szymon's suggestions.

Comment 9 Szymon Ptaszkiewicz

2011-08-16 11:59:59 EDT

Created attachment 201589 [details]
Patch v4

Simplified test + changes in documentation.

Comment 10 Szymon Ptaszkiewicz

2011-08-17 05:36:22 EDT

Created attachment 201631 [details]
Patch v5

The new flag moved to IContainer.

Comment 11 Szymon Brandys

2011-08-17 06:07:17 EDT

Looks good. Once tests pass, release please.

Comment 12 Szymon Ptaszkiewicz

2011-08-17 06:28:21 EDT

Patch v5 released.

Comment 13 Markus Keller

2011-08-19 12:58:18 EDT

 * Member constant (bit mask value 16) indicating that a resource
 * should not be checked for the existence.

The "the" sounds wrong here. You would say that you "check for the existence *of* a certain property". But your usage is similar to "check a device for errors", where a "the" would also be wrong.

Please correct this in all the new Javadocs.

Comment 14 Szymon Ptaszkiewicz

2011-08-22 06:14:43 EDT

Created attachment 201904 [details]
Patch with updated Javadocs

Comment 15 Szymon Ptaszkiewicz

2011-08-24 08:15:05 EDT

Patch with updated Javadocs released. core.resources part released to Git, doc.isv part released to CVS. Thanks Markus!