This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 243265 - Remove (deprecate) FileSytem.getActions()
Summary: Remove (deprecate) FileSytem.getActions()
Status: RESOLVED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Filesystems (show other bugs)
Version: 8.0.1
Hardware: PC Mac OS X
: P2 normal (vote)
Assignee: Jaroslav Tulach
URL:
Keywords: API_REVIEW
Depends on:
Blocks:
 
Reported: 2014-03-25 16:07 UTC by Tomas Zezula
Modified: 2014-07-22 02:33 UTC (History)
5 users (show)

See Also:
Issue Type: TASK
Exception Reporter:


Attachments
Deprecations and an XXX comment showing the problematic place (9.15 KB, patch)
2014-04-18 10:46 UTC, Jaroslav Tulach
Details | Diff
Introducing FileSystem.findExtrasFor instead of getActions (17.12 KB, patch)
2014-05-07 12:41 UTC, Jaroslav Tulach
Details | Diff
Final patch with changes in masterfs and versioning.masterfs (27.44 KB, patch)
2014-05-12 13:56 UTC, Jaroslav Tulach
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tomas Zezula 2014-03-25 16:07:49 UTC
Remove (deprecate) FileSytem.getActions().
Discussed offline with Jarda.
Comment 1 Jaroslav Tulach 2014-03-31 09:54:17 UTC
Analysis: The method is used in called from
http://hg.netbeans.org/releases/file/35b2705af4c1/openide.loaders/src/org/openide/actions/FileSystemAction.java
however it is not widely used. The only related usage is "Refresh" action when scan on focus is disabled. However that one is explicitly added by openide.loaders:
http://hg.netbeans.org/releases/file/35b2705af4c1/openide.loaders/src/org/openide/actions/FileSystemAction.java#l155

The method in question is implemented in masterfs where it delegates to various version control systems, but my feeling is that none of the standard ones provides any actions anymore.
Comment 2 Jaroslav Tulach 2014-04-18 10:46:20 UTC
Created attachment 146817 [details]
Deprecations and an XXX comment showing the problematic place

Turns out I was wrong. The FileSystemAction is heavily used by versioning (see the XXX comment in my diff). As our EOL rules say, we should not deprecate something that is being used.

What to do now?

#1 - e.g. leave the work on Sváťa and his server_split branch - e.g. move getActions into the part of filesystem that will be deprecated.

#2 - change versioning to register the actions without the help of FileSystemAction.
Comment 3 Jaroslav Tulach 2014-04-18 10:48:18 UTC
I think we need a review. I suggest Tomáš Zezula, Sváťa Dědic, Tomáš Stupka and Jarda Havlín to be my reviewers. Can you decide during next week how to proceed?
Comment 4 Tomas Zezula 2014-04-18 12:13:47 UTC
Both possibilities are OK for me.
Mostly question for VCS.
Comment 5 Jaroslav Tulach 2014-04-22 07:30:18 UTC
According to Tomáš Stupka versioning modules won't be needed on server side. As such #1 is viable option. However, if masterfs is needed, we need to split the module into non-UI and UI part as well.
Comment 6 Svata Dedic 2014-04-24 12:33:24 UTC
masterfs will be needed AFAIK, since it contains the _real_ and tested implementation of local filesystem (while LocalFileSystem is only used as a writable config layer + in tests)
Comment 7 Svata Dedic 2014-04-24 12:36:31 UTC
(In reply to Jaroslav Tulach from comment #2)
> What to do now?
> 
> #1 - e.g. leave the work on Sváťa and his server_split branch - e.g. move
> getActions into the part of filesystem that will be deprecated.
> 
> #2 - change versioning to register the actions without the help of
> FileSystemAction.

I think both: as we probably release some 8.x minor/micro version in the future, we may do #2 and deprecate the getActions(). According to EOL process, we will be able to remove getActions in the next major version (#1) - the branch should not be merged into a minor version anyway.
Comment 8 Jaroslav Tulach 2014-04-24 12:45:11 UTC
(In reply to Svata Dedic from comment #7)
> (In reply to Jaroslav Tulach from comment #2)
> > What to do now?
> > 
> > #1 - e.g. leave the work on Sváťa and his server_split branch - e.g. move
> > getActions into the part of filesystem that will be deprecated.
> > 
> > #2 - change versioning to register the actions without the help of
> > FileSystemAction.
> 

> future, we may do #2 and deprecate the getActions(). 

Well, that is not my #2. My #2 requested complete rewrite of versioning modules. I am not sure we can do just your #2 - NetBeans EOL policy asks us to eliminate all usages before marking an element deprecated.
Comment 9 Jaroslav Tulach 2014-04-28 12:28:56 UTC
The decision of the review meeting is to replace the getActions(Set<FileObject> fos) method with something like

public Lookup getActions(Set<FileObject> fos);

and change FileSystemAction to extract actions from here in provided method. Thanks. I'll prepare new version of the patch and let you know.
Comment 10 Jaroslav Tulach 2014-05-07 12:41:28 UTC
Created attachment 147158 [details]
Introducing FileSystem.findExtrasFor instead of getActions

Please approve this API change. I'll use it in masterfs and versioning and integrate by middle of next week. Unless there are objections.
Comment 11 Jaroslav Tulach 2014-05-12 13:56:36 UTC
Created attachment 147232 [details]
Final patch with changes in masterfs and versioning.masterfs
Comment 12 Jaroslav Tulach 2014-05-13 08:46:24 UTC
Unless there are objections I integrate tomorrow.
Comment 13 Tomas Zezula 2014-05-13 09:10:15 UTC
Seems good to me.
Just two small comments:
TZ01: FileBasedFileSystem:311

final Lookup lkp = ap.findExtrasFor(foSet);
while (retVal == null && it.hasNext()) {
if (lkp != null) {
AnnotationProvider ap = it.next();
arr.add(lkp);
retVal = ap.actions(foSet);
}

shouldn't be better to return Lookup.EMPTY and not check for null.

TZ02:
arr.toArray(new Lookup[0]) should be arr.toArray(new Lookup[arr.size()]) for better performance.
Comment 14 Jaroslav Tulach 2014-05-14 09:22:12 UTC
Integrated as ergonomics#d68c68d54fcd

I fixed TZ02. As far TZ01 - the method is SPI and it is easier to leave more flexibility for the providers and check for null in the infrastructure. It maches the original behavior of actions. I left it as it was in the original patch.
Comment 15 Quality Engineering 2014-07-22 02:33:09 UTC
Integrated into 'main-silver', will be available in build *201407220001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)

Changeset: http://hg.netbeans.org/main-silver/rev/d68c68d54fcd
User: Jaroslav Tulach <jtulach@netbeans.org>
Log: #243265: Deprecating getActions and replacing the method with univeral bag-of-things - e.g. Lookup - in order to make filesystems independent on AWT in the future.