Bug 194475 - [dstore][api][breaking] Review "Discouraged Access" warnings in RSE
Summary: [dstore][api][breaking] Review "Discouraged Access" warnings in RSE
Status: RESOLVED FIXED
Alias: None
Product: Target Management
Classification: Tools
Component: RSE (show other bugs)
Version: 2.0   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 3.0 M3   Edit
Assignee: David McKnight CLA
QA Contact: Martin Oberhuber CLA
URL:
Whiteboard:
Keywords: api
Depends on:
Blocks:
 
Reported: 2007-06-26 14:43 EDT by Martin Oberhuber CLA
Modified: 2008-04-03 04:33 EDT (History)
0 users

See Also:


Attachments
Fix expected friend access between dstore components (3.02 KB, patch)
2007-06-27 05:20 EDT, Martin Oberhuber CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Oberhuber CLA 2007-06-26 14:43:24 EDT
In an RSE workspace, there are currently 368 "Discouraged Access" warnings. These fall in two categories:

(a) 103 "Discouraged Access" in the Platform. These should be replaced by
    official API usage where possible, or bugs need to be reported against 
    those other packages if this is not possible.

(b) 265 "Discouraged Access" inside RSE itself. These should be carefully
    reviewed; either they are deemed OK and an "x-friend" statement should be
    added to the Manifest; or, official API should be used where possible; or,
    non-API classes need to be made API for public consumption.

Issues (b) can be found with following filter in Problems View:
3x "due to restriction on required project org.eclipse.dstore"
  subsystems.files.dstore --> dstore.extra: 
    DomainNotifier.addDomainListener(), DomainNotifier.removeDomainListener()
    - DaveM: This is a dstore-specific thing and should not be public API

262x "due to restriction on required project org.eclipse.rse"
  Typically due to re-using RESID_ constants or similar in files.dstore,
  processes.dstore, shells.dstore or useractions

Issues (a) can be found with
103x "due to restriction on required library"
  These need to be addressed.
Comment 1 Martin Oberhuber CLA 2007-06-27 05:20:34 EDT
Created attachment 72567 [details]
Fix expected friend access between dstore components

Attached patch fixes the false positives (expected internal friend access warnings) between dstore components:
   subsystems.files.dstore --> dstore.extra
   subsystems.(files,processes.shells).dstore --> services.dstore
   connectorservice.dstore --> rse.ui
   processes.ui --> rse.ui

I'm releasing this patch for 2.0 based on our discussion that we should review "friend" declarations every now and then.
Comment 2 Martin Oberhuber CLA 2007-06-27 05:28:32 EDT
The patch reduces case (b) - "restriction on required project" from 265 occurrences to 12 occurrences, all of which are about OutputRefreshJob.

OutputRefreshJob should either be API itself or get accessible through a factory method in API rather than exposing its constructor.

The 103 warnings for case (a) should still be reviewed.
Comment 3 David McKnight CLA 2007-06-29 10:59:48 EDT
Martin, do you want to commit that patch?
Comment 4 Martin Oberhuber CLA 2007-06-29 11:07:37 EDT
I did commit it already :-) since it all seemed to make sense and is a releng only aspect of RSE that does not have any effect on the actual operation of it.

But I'd be happy if you reviewed it to confirm my assumptions were correct.

At any rate, the bug should remain open to deal with the other issues.
Comment 5 David McKnight CLA 2007-06-29 11:37:59 EDT
I agree with your changes, I just wasn't sure if the patch had been committed to cvs yet.
Comment 6 Martin Oberhuber CLA 2007-10-01 07:57:06 EDT
Bulk update target milestone 2.0.1 -> 3.0
Comment 7 David McKnight CLA 2007-11-05 16:47:09 EST
I don't see anymore dstore discouraged access warnings (after committing a couple changes to remove a few remaining warnings) so I'm going to mark this one as fixed.
Comment 8 Martin Oberhuber CLA 2008-04-03 04:33:45 EDT
This is an API breaking change, because
   DataStore#getDomainNotifier()
which used to return an (internal) DomainNotifier class, now returns
   IDomainNotifier DataStore#getDomainNotifier()