Bug 33162 - Allow to listen on subtrees
Allow to listen on subtrees
Status: RESOLVED FIXED
Product: platform
Classification: Unclassified
Component: Filesystems
3.x
All All
: P3 with 1 vote (vote)
: 6.x
Assigned To: Jiri Skrivanek
issues@platform
: API, API_REVIEW_FAST
: 42021 125739 148960 (view as bug list)
Depends on:
Blocks: 156753 156754 156755 156758 49776 55651 156756 156757 156769 168237
  Show dependency treegraph
 
Reported: 2003-04-23 13:29 UTC by Vitezslav Stejskal
Modified: 2009-08-20 19:45 UTC (History)
15 users (show)

See Also:
Issue Type: ENHANCEMENT
:


Attachments
Suggested unit test (just a start of what it should test, of course) (2.36 KB, text/plain)
2004-02-04 04:58 UTC, Jesse Glick
Details
FileTree - API proposal (intentionally without impl. details) (2.81 KB, text/plain)
2004-05-07 16:40 UTC, rmatous
Details
Suggested API (10.94 KB, patch)
2007-01-31 17:42 UTC, rmatous
Details | Diff
preliminary impl. + test (33.56 KB, patch)
2007-01-31 17:59 UTC, rmatous
Details | Diff
API testing - #1, #2, #3a, #3b (but no external changes because this test is not just for masterfs) (5.41 KB, patch)
2007-02-09 11:08 UTC, rmatous
Details | Diff
Listen to File patch. (25.25 KB, text/plain)
2009-01-06 14:52 UTC, Jiri Skrivanek
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Vitezslav Stejskal 2003-04-23 13:29:49 UTC
The Filesystem allows to atach listener which
receives notifications about all changes happening
on this fs. In new projects when classpath !=
filesystems, however, modules need to listen on
changes occuring in some folder and all its
subfolders recursively. It would be nice to have
some support for this in filesystem API; otherwise
various pieces of code will have to listen on the
filsesystem and filter out a lot of events they
aren't interested in.
Comment 1 Jesse Glick 2004-02-04 04:57:41 UTC
I just realized I could really use this too. I was unhappy to find
that FileObject.addFileChangeListener *doesn't* already do this.
FileSystem/Repository.aFCL is nice and all, but in 4.0 a FileSystem is
maybe a whole disk root, and you may be interested in only a little piece.

Also the current documentation is very vague on the fact that
listening to a folder only gives you events occurring immediately in
it; I actually had to run code through a debugger and write some unit
tests and examine the source code to find this out.

Attached is the beginnings of a unit test showing how I expected it to
work.

Impl might involve changing:

- AbstractFileObject.create* to not check hasListeners directly (may
be different in parent)

- AbstractFolder.fileCreated0 should unconditionally delegate to
parent (if not null)

- fileCreated0 can check hasListeners
Comment 2 Jesse Glick 2004-02-04 04:58:42 UTC
Created attachment 13232 [details]
Suggested unit test (just a start of what it should test, of course)
Comment 3 Jesse Glick 2004-04-02 15:43:35 UTC
Sounds like web apps could use this too.
Comment 4 Petr Jiricka 2004-04-02 15:49:45 UTC
Yes, we could definitely use this, e.g. for the "deploy on save"
feature: when a file under the document base changes (is saved in the
editor), we'd like to automatically copy it to the staging area
('build' directory).
Comment 5 rmatous 2004-04-05 08:36:33 UTC
You can naturally listen on FileSystem and then use method
FileUtil.isParentOf. Isn't it enough ?
Comment 6 Milan Kuchtiak 2004-04-05 17:25:01 UTC
It doesn't seem too much efficient to listen on the whole filesystem
as Jesse also noticed.

For web-apps stuff we also need to listen on changes under the
src folder in order to change the deployment descriptor when servlet
file is removed/renamed or moved.

The src folder can be miles away from the filesystem root.
Comment 7 rmatous 2004-04-06 09:12:27 UTC
Efficiency in this case just depends on impl. of method isParentOf.
This method isn't currently efficcient enough. I suggest to
reimplement such method to be more efficient for promo-D. There should
be easy to compare  path for both FileObjects (at least for
MasterFileObjects). 

OK, I could add method addFileChangeListener (FileChangeListener fcl,
boolean recursively) and deprecate the current one method a
addFileChangeListener (FileChangeListener fcl) and the same for
removeFileChangeListenr.  First impl. version could actually listen on
FileSystem and call improved method isParentOf and then eventually
notify listeners. 

So, if everybody agrees I can provide patches and put it into dev rev
queue. Not sure if this is P1 but, so decreased to P2 but its up to you. 

Comment 8 Milan Kuchtiak 2004-04-06 12:47:31 UTC
Thank You,
The issue is quite blocking us to avoid the regresion since 3.6.
Comment 9 Jesse Glick 2004-04-07 00:32:02 UTC
I don't think speeding up isParentOf is enough. If the filesystem
contains a million files, and you do something which changes a
thousand of them, two of which are inside a subtree of interest, do
you want to get a thousand change events and then discard 998 of them?
Or do you want to get the two events you are interested in? There is
quite a difference.

Anyway I don't see why it should be difficult to fire changes to
listeners on subtrees. You already create an event and then propagate
it to the parent folder; just do the same thing recursively.
Comment 10 rmatous 2004-04-07 08:40:08 UTC
Instantiating 998 FileEvents and then discarding  of them means  in
other words that we create one FileEvent after one relatively slow I/O
operation. But if there was called current impl. isParentOf  then it
could mean that we must traverse (million * 998) FileObjects in the
worst case. As I mentioned there won't be probably neccesary to
traverse FileObjects for MasterFileSystem impl. at all beacuse I can
just compare paths but for AbstractFileSystem implementations get path
means traverse to root anyway (so impl. for AbstractFileSystem won't
be probably efficient enough - at least I don't have any idea how to
achieve it).  

So, I'm willing add new method, implement it somehow with respect to
performance (impl. based on isParentOf isn't dogma at all but it could
be the most simplest implementation). 
Comment 11 rmatous 2004-04-07 08:58:48 UTC
Please forget completely about my notes about isParentOf it was just
evolution of my mind. I'll attach diff with impl. ASAP.
Comment 12 Jesse Glick 2004-04-23 17:55:47 UTC
Note that such an API would probably be unusable for e.g. MDR because
they need to find changes made on disk to FileObject's which are not
necessarily loaded at the time. There is no way to capture these
except (1) with a manual periodic timestamp check, or (2) forcing all
such FO's to stay in memory. GlobFileBuiltQuery suffers from the same
issue - the only way I know of to find changed
build/classes/**/*.class files (created from the Ant process) is to be
listening on the nearest existing FileObject (and change that target
FO every time some intermediate dirs are created or deleted). This is
really cumbersome and probably not efficient; would prefer some way to
e.g. specify a URL or URL prefix and just be notified when such a file
is created, deleted, or modified. But that would be a whole different
API with complex performance implications.
Comment 13 Petr Nejedly 2004-04-23 19:51:06 UTC
> OK, I could add method addFileChangeListener (FileChangeListener
> fcl, boolean recursively) and deprecate the current one method a
> addFileChangeListener (FileChangeListener fcl) and the same for
> removeFileChangeListenr.  First impl. version could actually listen
> on FileSystem and call improved method isParentOf and then
> eventually notify listeners. 

Carefully with the remove method.
1. There is no reason to specify _how_ I'd like to remove it
2. WeakListeners need the usual pattern, it won't find the two-arg
   variant, so there may be WL stubs cumulating in your listener list.
Comment 14 Jesse Glick 2004-04-30 23:15:56 UTC
See also issue #13238.
Comment 15 rmatous 2004-05-03 16:23:04 UTC
I'm not sure if I understand how does this issue relates to #13238. 

Seems to me that #13238 request solution of problem how to catch
extenal changes:
if you listen on folder and there is internally modified content of
one child then you get fileChange event. If this modification was done
as external change then it isn't enough to call refresh on folder but
you must call refresh also on its children because refresh on folder
doesn't compare children timestamps   and just fires events that
content of folder was modified fileCreated, fileDeleted .

Comment 16 Jesse Glick 2004-05-03 17:16:14 UTC
There are several separate parameters to what different modules need,
I think; currently *any* combination is very difficult to write and
make efficient:

1. Do you need to know about modifications (timestamp changes), or
just creation and deletion of files?

2. Do you need to know about any file within a given root (e.g. URL
prefix or ancestor directory), or just a particular file/folder?

3. Do you need to handle changes made by external edits on disk
(assuming there is some refresh trigger), or only via Filesystems API?

Some scenarios:

- GlobFileBuiltQuery (ant/project): #1 yes (need to compare
timestamps), #2 probably no (name of .class file is known), #3 yes (a
refresh occurs when Ant exits)

- web app "deploy on save": #1 yes (check for save), #2 yes (any file
in the docroot), #3 no (only care about saves from IDE)

- MDR metadata cache: #1 yes (check against cache), #2 yes (any file
in source dir), #3 maybe

- ClassPath roots listener (java/api): #1 no (just need to know if the
dir exists), #2 no (just care about root), #3 probably no

- external source root updates (SourcesHelper in ant/project): #1 no,
#2 no, #3 probably no

The ideal API would let you specify your needs w.r.t. #1-3 and then
add a global listener (not tied to a specific FileSystem or FileObject
- just give a URL or File), and you would be notified if the relevant
events occur at any time thereafter. Whether this is feasible to
implement, I don't know.

Sounds like the originally requested API (recursive propagation of
file events) would satisfy #2 but not #1 (I guess?) nor #3.
Comment 17 Jesse Glick 2004-05-03 17:43:38 UTC
Forgotten use case:

- project logical view needs to listen to existence of each package
root: #1 no, #2 no, #3 perhaps (*)

(*) - since the root FileObject is presumably always held in memory
anyway, any deletion on disk will be notified when you next do File ->
Refresh All Files; however a newly created package root might not be
noticed if it is externally located, i.e. not directly inside project
directory, since its parent might not have a live FO
Comment 18 rmatous 2004-05-04 10:03:16 UTC
I think that originally requested API would satisfy #1, #2. I expect
that #1 means internal modification.  If so then there are overloaded
methods close on stream implementations in org.openide.filesystems
that fire fileChangeEvent and new API should just deliver this event
to all registered listenerers recursively.

#3 is problem
------------
new API doesn't care about refresh triggering. There isn't a way how
to refresh FileObjects recursively (just only FileSystem.refresh which
refreshes the whole hierarchy starting on root) - there is missing
method like FileObject.refreshRecursively. But FileSystem.refresh is
just something like soft refresh which means that only FileObjects
kept in memory (hard referenced FileObjects) will be refreshed.
Refresh means:
- fileCreated, fileDeleted events are fired for FileObject.isFolder ()
== true (but no fileChange)
- fileChange event is fired for FileOject.isData () == true (but no
fileCreated nor fileDeleted)

If this way of soft refresh isn't sufficient (hardly can be for all
use cases) then there isn't any easy solution for it (you must create
new FileObjects recursively which may cause storms of FileObjects,
timestamps can't be stored inside FileObjects cause FileObject
instances can be garbage collected and so on). Moreover seems to be
that something like that is necessary mainly for filesystem impl.
adressing java.io.File. Providing API that allows to listen on
java.io.File and then use conversion to FileObject could be 
sufficient and relatively lightweight.  There would be probably
necessary to be able:
- add and remove listener (create some cache with subfiles and their
timestamps)
- invoke refresh on arbitrary java.io.File (create new cache and
compare it with the old one)

This can be done relatively quickly and would be solution for #1, #2
and #3.



Comment 19 Jesse Glick 2004-05-04 18:32:58 UTC
I agree that it should be possible to solve #3 independently as a
separate utility of some kind. Just trying to collect the different
kinds of use cases in one place to be clear.

Re. the fact that only create/delete (of children) fired on folders,
and only change fired on files - probably OK, though it means that for
many use cases (#1's) you would need to listen on both a file and its
parent folder, which is a little awkward. I guess that is what issue
#13238 is about.
Comment 20 rmatous 2004-05-07 16:40:52 UTC
Created attachment 14764 [details]
FileTree - API proposal (intentionally without impl. details)
Comment 21 rmatous 2004-05-07 16:43:16 UTC
I attached new API proposal that should hopefully covers all use cases
defined by Jesse. 

I think that method "public Refresh getRefresh (boolean intrusive)"
could be controversial then here is my explanation why I suggest it
just this way.

Refresh of subtree doesn't need to be supported for all filesystems
that's why there should be some return value and not just void. This
method may block current thread for a long time especially if
intrusive (agressive) method is chosen. There is important to know if
it is supported before you call refresh without blocking (if not
implemented then there is just useless to post it in other thread). 

Why such strange FileTree.Refresh ?
- simplifies (but not necessary) to call refresh as AtomicAction which
postpones FileEvents and then there is also possible to find out if
FileEvent notification was caused by calling this refresh (see method
FileEvent.isFiredFrom)
- also convenient for calling RequestProcessor.post which might be
used for not blocking current thread

Comment 22 rmatous 2004-05-07 18:13:08 UTC
Can I ask for API review ? Intended API is in attachment.
Comment 23 Jesse Glick 2004-05-07 19:09:40 UTC
FileTree.getInstance should throw IAE if not a folder, rather than
return null.

I suppose for performance reasons you would want to cache FileTree
instances per root, and also if FT2 has a root inside FT1 and you
refresh FT1, that should automatically send changes to FT2.

I'm not sure that FileTree.getInstance(FileObject) will really satisfy
some of the use cases I had in mind. In particular, several pieces of
code (e.g. checking for existence of external source roots) need to
know whether a given file (or folder) is created or deleted. But at
the time you start listening, there might not be the parent folder, or
its parent folder, or anything. Or there might be, but then they are
removed. And maybe recreated later. What I really want for these cases
is to be able to specify a URL for a file/folder which may or may not
exist right now, and be told if at any time such a file/folder is
either created or deleted.

Of course I could use getInstance on the disk root but then I would be
listening to everything on disk, which is unacceptable for performance
reasons.

Is it possible to have getInstance(URL) instead? (With getRoot() ->
URL of course.)

Re. calling explicit refresh - I don't know who would use this
exactly. For the Ant-oriented stuff at least, when the Ant script
finishes I want to call refresh on all files generally, with no
knowledge of what FileTree's might exist. The listener does not know
when to refresh; it just expects to get some events. This means that
existing FileObject's should be refreshed as now, plus any potential
but nonexistent FO's inside FileTree's with requests for intrusive
refresh would be refreshed from disk.

I.e. for the use cases I have in mind, the proposed API could not
work. Not sure about other use cases. I would suggest something more like

public FileTree {
    private FileTree() {/* no instances */}
    public static boolean addFileChangeListener(FileChangeListener l,
URL root, boolean recursive, boolean guarantee);
    public static void removeFileChangeListener(l);
}

Semantics of adding a listener: If a file or folder named by the given
URL (or by any descendant URL, if recursive is true) is created or
deleted or modified, you are notified.

If guarantee is false, this is only guaranteed to happen if a
FileObject exists for that URL and the change is made through the
Filesystems API, or during a global refresh and the FO happened to exist.

If guarantee is true, you will be notified even if no one was holding
a hard reference to the relevant FileObject(s) - if an external change
was made and there is a global refresh, the utility class will check
on disk to see if there was a change (acc. to timestamps). TBD how
that would work concretely; might need to actually hold FileObject
refs internally. (From disk root down to requested root for
!recursive, and also for all contained files/folders for recursive.)

I don't think this is suitable for fast-track. I have yet to see a
proposal that actually covers all the known use cases or which
explicitly solves some and rejects others.
Comment 24 rmatous 2004-05-10 12:42:15 UTC
OK. 
Comment 25 Jaroslav Tulach 2004-07-23 09:54:14 UTC
This issue looks like being forgotten and assigned to apireviews,
without the actual will to go thru the review. Radek, if you want to
go thru API review, please prepare the documentation sumarrizing your
desired changes, and submit it once again please. Btw. do you really
want standard review?
Comment 26 Jesse Glick 2004-07-23 15:31:48 UTC
By the way I've already written a utility in
ant/project/src/org/netbeans/modules/project/ant/FileChangeSupport.java
which handles #1 and #2 (not #3). We should consider something like
this (hopefully a more efficient impl) for a future convenience API.
Comment 27 Jesse Glick 2004-10-04 18:00:36 UTC
Needs work. IMHO should be targeted for E. See FileChangeSupport in
ant/project for a sketch of the API needed for requirement #1 (but not
#2 nor #3), though that impl is not particularly efficient.
Comment 28 rmatous 2004-10-05 09:51:31 UTC
OK, I'll take a look at  FileChangeSupport API and impl. and then
probably will migrate it into filesystems throught apireview.
Comment 29 Andrei Badea 2006-06-21 14:59:52 UTC
Any plans to provide at least #1 and #2 in filesystems in 6.0? Apart from deploy
on save there are also a few places in the J2EE area where #2 is needed. I have
just copied FileChangeSupport to j2ee/persistenceapi (needed to listen on the
creation and deletion of persistence.xml files).
Comment 30 Jesse Glick 2006-06-21 17:29:34 UTC
Pretty much every project type needs this API. It is very common for various
nodes, editors, etc. for the project to react when certain files are created or
deleted (incl. renames), or modified. Writing such code using the current
Filesystems API is cumbersome and tricky to test, and the hand-written code is
usually wrong in some corner case. Memory leaks are also a danger whenever you
are manually attaching and detaching listeners. This issue has been inactive for
two years. I think it needs to be given priority.
Comment 31 Jesse Glick 2006-11-20 17:53:23 UTC
*** Issue 42021 has been marked as a duplicate of this issue. ***
Comment 32 Antonin Nebuzelsky 2006-12-19 10:16:51 UTC
Planned for 6.0.
Comment 33 rmatous 2006-12-20 14:14:14 UTC
Jesse, do you know any reason why not to reuse FileChangeSupport from
org.netbeans.modules.project.ant module. I would just use FileChangeListener
instead of FileChangeSupportListener and FileEvent instead of
FileChangeSupportEvent.
Comment 34 Jesse Glick 2006-12-20 16:19:06 UTC
I know of several reasons why not to reuse FileChangeSupport as is. :-) It

- only works on disk files (should really operate on URIs or something?)

- is probably quite inefficient when you have many listeners operating on the
same area, as multiple parent folder listeners are created

- forces creation of FileObject's for everything in a tree, which can be
expensive, and in many cases is totally unnecessary

Maybe it is possible to tweak FCS to be a bit more efficient and robust, and add
it to the API as a short-term workaround. So long as it is just a convenience
API on top of the existing FS API code it probably cannot be very good.

IMHO: I think the entire OO style of the Filesystems API (i.e. one FileObject =
one existing file) was a mistake from the beginning and we should consider
keeping the current API only as a wrapper around a simplified, lower-level API
with a fresh design based on file paths (e.g. java.net.URI). Besides JAR access,
locks, and VCS behavior hooks, all of which can be provided as services without
a FileObject class, the main purpose of the Filesystems API is to fire events.
So it seems like the basic implementation should follow directly from the known
requirements, which do not match the current FS API very well. Cf. my comment of
7 May 2004 for the best suggestion I have so far. A fresh implementation of such
an API could not only be much more efficient for large trees (in both memory and
CPU), it could probably permit modules such as java/source to ask for
information about changes in files across JVM restarts (cf. the Eclipse API
which does this).
Comment 35 rmatous 2007-01-31 17:42:44 UTC
Created attachment 37890 [details]
Suggested API
Comment 36 rmatous 2007-01-31 17:59:51 UTC
Created attachment 37891 [details]
preliminary impl. + test
Comment 37 Jaroslav Tulach 2007-02-01 09:56:41 UTC
Y01 why there is a need for FileModificationEvent? Imho it would be ok to just 
reuse existing FileEvent & co. That removes the need for two new classes from 
the API.

Y02 does the new api work on system file system? Imho it would be pretty 
useful and it should. In fact it should work on any filesystem, otherwise it 
will just lead to duplicated (write code that can use FileObject listeners as 
well as the new style) or broken (use just the new style and ignore if it does 
not work) client code.

Y03 Why is FileChangeManager<ListenerType extends EventListener> parametrized 
if its API contains only static methods?

Comment 38 rmatous 2007-02-02 15:38:37 UTC
Y01  my motivation: hypotheticaly there would be possible to implement provider
for some scheme without need to implement also FileSystem  and thus FileObjects
that you get from FileEvent. It was decline of filesystems in
openide/filesystems. Definitely impl. for MasterFS can fire  FileEvents.

Y02  not considered yet - just MasterFS. Generally this API could delegate on
FileChangeSupport like fall-back impl. for any filesystem that has ensured url
to FO mapping by URLMapper whereas their URIs are non-opaque. 

Y03 just impl. detail but agree - might be confusing
Comment 39 Jesse Glick 2007-02-08 22:22:28 UTC
I agree it is better not to use FileEvent, at least because that would force
creation of FileObject's for affected parts of a tree, which might not otherwise
be needed.


I don't foresee any reason to implement for the system filesystem. AFAIK all the
use cases are for disk files.


FileModificationEvent's constructor should request a particular source, not just
Object.


FME's second constructor should presumably assert that TYPE == RENAMED.


It might be preferable to have four event classes and four corresponding methods
in FileModificationListener.


It may be useful to permit a listener to receive batch notification of changes
in many files at once.


FileModificationEventSource.forScheme(String) might not be powerful enough. For
example, a provider might handle jar:file: URIs but not jar:http: URIs.


Do you plan to implement support for both my #2 and #3? Just #2? Neither?

The API implies that support for #2 can be written, but whether or not the
attached impl does it is not clear to me. #2 means that you can get events from
anywhere in a subtree just by listening to the top. This is not possible using
FileChangeListener today on a folder. It probably works (albeit inefficiently,
since you need to filter out uninteresting events) using FCL on the whole
filesystem *if* you do not care about external changes (but see #3b below).

The API is silent about #3 (disk changes not done thru FS API). There are really
three aspects of support for #3:

#3a (not assuming #2). I ask to listen to changes in
"file:/prjs/foo/nbproject/something.properties". It is changed somehow, or even
created or deleted, not necessarily using FS API. I want to be notified. Today
this is possible only using FileChangeSupport from ant/project, which is
inefficient and probably unreliable, since it needs to keep a chain of
references to FileObject's all the way down to this file from the root, in order
to prevent parents from being garbage collected, since then they would no longer
be monitored for changes.

#3b (assuming #2). I ask to listen on "file:/prjs/foo/" recursively. Somehow a
file /prjs/foo/nbproject/something.properties gets created - not necessarily
using the Filesystems API, maybe by Ant. A general file refresh is triggered
(e.g. main window exposed). I expect to get a CREATED event. Extending
FileChangeSupport to support this would be terribly inefficient as you would
need to create and hold onto FileObject's for the entire tree! So it can only be
supported efficiently using a non-FS-API-based implementation, such as keeping a
compact data structure listing known file paths together with timestamps and
sizes (say), and doing a bulk compare of snapshots to compute changes.

#3c. I ask to listen on "file:/prjs/foo/". At some point the IDE is shut down.
Later when it is restarted, I want to get a list of events corresponding to
changes made while the IDE was not running. This probably requires some
specialized API such as is found in Eclipse.
Comment 40 rmatous 2007-02-09 09:51:49 UTC
Y01: FileChangeListener(FCL) or FileModificationListener ? 

For this decision is important, I think:
a/ who is going to implement SPI - these events can be fired even without
FileSystem  impl.
b/ where this API will be located - openide/filesystems(FS) like, keeps current
FS concept, especially if this API is part of FS
c/ features (impossible batch notification)

Y02: as a proof of concept I've implemented fall-back impl. that handles also
other schemes (including nbfs) that can be somehow mapped to FS instance. 

JG:
- forScheme(String) might not be powerful enough (jar:file: URIs but not jar:http:) 
you are right although "jar:" is not  good example because jars are readonly, so
there is nothing to notify about 

- batch notification of changes
makes me sense for other SPI impl than FS. In FS I don't know when to trigger
collecting of events to deliver them together as batch notification. Atomic
actions or extend FS API? Use cases? 

-#1,#2,#3 
I don't expect to cover #3c. Others should be supported. I'll attach reworked a
few lines test that should give you overview what can be expected. 

Currently I have impl. for "file:" in MasterFS and fall-bask impl. in FS for others.

MasterFS impl. ("file:")
-----------------------
Listeners are not attached to FileObjects, so there aren't instantiated all
parent FOs just because event is going to be delivered to subtree-listeners.
This event delivery is triggered either  when changes are caused by using FS API
or when FS.refresh reveals changes. #3c or other sophisticated methods (which
would be probaably time consuming) ar not supported
 
FS impl. (fall-back)
------------
Less efficient (more sophisticated impl. than the current one can improve it).
On behind of API-user this impl. registers its own impl. of listener on FS
instance, uses filtering and redirects events to individual listeners registered
by API-user





 




Comment 41 rmatous 2007-02-09 11:08:23 UTC
Created attachment 38283 [details]
API testing - #1, #2, #3a, #3b (but no external changes because this test is not just for masterfs)
Comment 42 Jaroslav Tulach 2007-02-09 15:58:38 UTC
Jesse's comment made me think more about the issues discussed in this issue.

Imho there are two choices - either solve this issue as part of existing 
filesystems API or start NFS - new filesystem API that will have the features 
requested by Jesse:
> So it seems like the basic implementation should follow directly from 
> the known requirements, which do not match the current FS API very well.

If you want to start from scratch, then I think starting from zero in a new 
module is appropriate.

However I do believe that this is a matter of usecases. If the API is suppose 
to handle #3c - then we most likely better start from scratch and place this 
API somewhere into projects as it is very IDE specific anyway.

If we agree that it is ok to support #1, #2 we should stick with what we 
have - e.g. FileObjects, FileChangeListener, etc. It makes no sense to add new 
(completely unrelated) interfaces for events and listeners into an API if 
they'd completely independent of already existing ones.

That is why for now I'd suggest to stick with FileChangeListener & co. as the 
implementation anyway requires FileObject to be created and exists as the 
events are delivered to filesystems as well.

If we choose this direction, then indeed having this support working on SFS 
and in fact all filesystems is necessary as the implementation is done and 
usecases that would benefit from listening recursively exists (MenuFolder, 
ToolbarFolder, FolderLookup, Windows2, etc.)

On the other hand if the strong requirement is #3c (or similar), then the 
current implementation is unsufficient, the API is unsufficient and we should 
rather start designing NewFS API free of FileObject in a separate module.
Comment 43 Jesse Glick 2007-02-09 18:26:59 UTC
I think it is not just #3c but #3b that requires a non-FileObject back end for
efficient implementation. The tests I have seen so far do not touch on this
because they use FS APIs to make the changes, whereas support for #3b means that
changes from external processes deep inside a file tree would be fired after a
general refresh. For example, I would expect something like this to work:

File d = getWorkDir();
FileChangeManager.addFileChangeListener(d.toURI(), true, listener);
File f = new File(d, "subdir/subsubdir/somefile");
new FileOutputStream(f).close();
FileSystem.refreshAll(); // or something; separately filed RFE #?
listener.assertReceived(DATA_CREATED, f.toURI());

It is not possible (AFAIK) to support this efficiently unless you have a
compact, non-object-oriented cache of the last known state of the directory
tree, retaining path names (or their CRC-32s?) and for leaves (files) also
timestamps and sizes; keeping all FileObject's inside the tree would waste a lot
of memory. Also, while you could fire a o.o.f.FileChangeEvent, this would
require creating intermediate FileObject's which the listener might not even want.
Comment 44 Jaroslav Tulach 2007-02-09 20:16:39 UTC
If this deep hierarchy change test is supposed to be supported, then I guess Radek will have 
few funny days of coding ahead of himself. This really requires a cache somewhere (preferably 
$userdir/var/cache/something) and current impl has no signs of this.

Again I see two choices: mount this into existing FS API (and then I would prefer existing 
events, temporary creation of few FileObject shall cost nothing compared to the previous 
intesive I/O check), or completely independent API (then I would relate it to projects APIs 
somehow, as it does not seem much useful outside of project oriented world, imho).
Comment 45 Jaroslav Tulach 2007-02-12 11:37:29 UTC
So it seems to me that we need to know which usecases we are solving. 
Personally I am against trying to do anything with #3c now, as that smells too 
close to MDR. And we all pretend we are happy to get rid of it.

I am sure full support for #3b could be troublesome. However there is an 
interesting are relatively easy implementable subset of the requirement - e.g. 
be able to notify changes done by Ant in the same Java VM. That is something 
masterfs could handle with a little help of 
http://java.sun.com/javase/6/docs/api/java/lang/SecurityManager.html#checkWrite(java.lang.String)
Maybe we could start with it and leave the rest for future.


Comment 46 Jesse Glick 2007-02-12 23:40:05 UTC
Catching only in-VM java.io.File writes is not all that helpful. Even if the
writes are made from an Ant build, it is quite common for them to be made from a
forked subprocess. Anyway notification of changes to files made by Ant
processes, while nice, is probably not a primary use case.

Support for #3b and #3c may be optional, depending on the use cases. Without
#3a, use of the new API would actually be a functional regression from the
existing FileChangeSupport, since external changes to project metadata (e.g.
after doing a CVS update) would not be caught.
Comment 47 Antonin Nebuzelsky 2007-03-20 12:50:39 UTC
Just adjusting priority accordingly (P1->P3).
Comment 48 Jesse Glick 2008-01-22 19:40:10 UTC
*** Issue 125739 has been marked as a duplicate of this issue. ***
Comment 49 Jesse Glick 2008-02-21 16:24:01 UTC
My workaround utility copied to another module :-( We really need this in a supported API...

http://hg.netbeans.org/main?cmd=changeset;node=c132a3995ece
Comment 50 Antonin Nebuzelsky 2008-04-15 17:14:25 UTC
Reassigning to new module owner jskrivanek.
Comment 51 Jesse Glick 2008-10-02 18:57:06 UTC
*** Issue 148960 has been marked as a duplicate of this issue. ***
Comment 52 Jiri Skrivanek 2008-12-02 13:10:11 UTC
Based on discussion in this issue, the FileChangeSupport class by Jesse and patches by Radek, I would like to implement
this feature. For existing FileObject it remains

        fileObject.addFileChangeListener(fileChangeListener);
        fileObject.removeFileChangeListener(fileChangeListener);

For File (even not existing) would be added

        FileUtil.addFileChangeListener(fileChangeListener, File file);
        FileUtil.removeFileChangeListener(fileChangeListener, File file);

Which can be used for example this way (expects only /dir exists)

        FileObject dirFO = FileUtil.toFileObject(new File("/dir"));
        dirFO.addFileChangeListener(fileChangeListener);
        FileUtil.addFileChangeListener(fileChangeListener, new File("/dir/subdir/subsubdir/file"));

In case of internal (via Filesystems API) creation/deletion/changes of the file, fileChangeListener will be always
notified, in case of external change and subsequent filesystems refresh, fileChangeListener will be notified only when
subsubdir FileObject exists (was created and not garbage collected yet).

Implementation in masterfs would store listeners in FileName classes instead of in FileObjects. So, listeners survive
FileObjects until there is a reference to them.

Please, comment if you have some complains or ideas. Thanks.
Comment 53 Jesse Glick 2008-12-02 20:16:12 UTC
First, I don't understand what you are doing with dirFO in your example. It seems useless.

Second, "in case of external change and subsequent filesystems refresh, fileChangeListener will be notified only when
subsubdir FileObject exists (was created and not garbage collected yet)" effectively makes the new API useless for
clients like GlobFileBuiltQuery (i.e. my #3 requirement). If I could assure that, I would just listen to subsubdirFO to
begin with. The point is that as a client I generally have no idea what parent FileObject's might exist; I just want to
be notified if anything happens to that file, including after a refresh of external changes. The implementation (in the
absence of native file notifications) could be as simple as a Map<File,Long> retaining last known timestamps for various
files, to be rechecked after a global refresh.

I don't understand what "listeners survive FileObjects until there is a reference to them" means, but I will note that
project.ant's current impl assumes all such listeners are weak: i.e. you need to hold a strong ref to the listener for
as long as you expect to get changes. If FileUtil's methods do not behave this way, then clients would need to use
FileUtil.weakFileChangeListener in most cases. For example, a File-based PropertyProvider needs to listen to an
arbitrarily specified File so long as the PP exists, so it should hold a strong ref to the real FCL; it would never call
any remove method.

My #2 requirement may not be necessary, I am not sure. project.ant does not use it anyway. Would really need to consult
with Retouche/Parsing API developers as the most likely use case is detecting changes to files in a source tree.
Currently RepositoryUpdater seems to use FU.aFCL(FCL) to be notified of any file changes anywhere, which seems wasteful.
Comment 54 Jiri Skrivanek 2008-12-03 09:20:34 UTC
> First, I don't understand what you are doing with dirFO in your example. It seems useless.

It is a FileObject I want to listen to. Just a random choice.

> Second, "in case of external change and subsequent filesystems refresh, fileChangeListener will be notified only when
> subsubdir FileObject exists (was created and not garbage collected yet)" effectively makes the new API useless for
> clients like GlobFileBuiltQuery (i.e. my #3 requirement).

Yes, it doesn't solve external changes out of scope of existing FileObjects. My proposal just provides the same
functionality as org.netbeans.modules.project.ant.FileChangeSupport.

> If I could assure that, I would just listen to subsubdirFO to begin with. The point is that as a client I generally 
> have no idea what parent FileObject's might exist; I just want to be notified if anything happens to that file, 
> including after a refresh of external changes. The implementation (in the absence of native file notifications) could 
> be as simple as a Map<File,Long> retaining last known timestamps for various files, to be rechecked after a global 
> refresh.

It might be memory and time consuming. java.io.File holds entire path and refresh would cause a lot of disk touches. If
we want to support "reliable" refresh including external changes, we will have to use something similar like structure
of FileName instances in masterfs and not fire FileEvent to not create FileObjects for all new files.

> I don't understand what "listeners survive FileObjects until there is a reference to them" means, but I will note that
> project.ant's current impl assumes all such listeners are weak: i.e. you need to hold a strong ref to the listener for
> as long as you expect to get changes.

I also expect this behaviour. My note just emphasize the difference from the current behaviour. If a FileObject is
garbage collected, listeners added to it are also lost.

> My #2 requirement may not be necessary, I am not sure. project.ant does not use it anyway. Would really need to 
> consult with Retouche/Parsing API developers as the most likely use case is detecting changes to files in a source 
> tree. Currently RepositoryUpdater seems to use FU.aFCL(FCL) to be notified of any file changes anywhere, which seems 
> wasteful.

For me seems to be useful to listen to for example src folder and be notified about changes in all sub packages.
Comment 55 Jesse Glick 2008-12-03 17:54:04 UTC
"It is a FileObject I want to listen to." - I see that, but it doesn't make sense as an example usage for this API.
There is no FO for a parent dir that I know exists, and even if there were, adding a FCL directly to it would only
report changes in its direct children, which is irrelevant here.


"My proposal just provides the same functionality as org.netbeans.modules.project.ant.FileChangeSupport." - that does
not seem to be true. FCS supports my #3 requirement, by retaining a tree of existing FileObject's down to the listened
file, so that any disk change will be reported after a full refresh, without requiring you to hold onto any particular
FO instances manually. I don't think this is an _efficient_ way to support that requirement - an impl inside masterfs
could perhaps do better, as I suggested.


"My note just emphasize the difference from the current behaviour." - I still don't understand what your note means -
rephrase it grammatically and precisely, as this is going to be an API contract.


Maybe my #3 use case was not stated clearly enough. So to reiterate: I want to call e.g.

FileUtil.addFileChangeListener(fileChangeListener, new File("/dir/subdir/subsubdir/file"));

and (so long as I hold a strong reference to fileChangeListener) that should suffice to notify me in case a file named
/dir/subdir/subsubdir/file is ever created, deleted, or modified, using either Filesystems API calls or (after a full
refresh) disk file changes, whether or not /dir, /dir/subdir, or /dir/subdir/subsubdir exist at the time of the call.
This is what FileChangeSupport provides, and what several clients in (at least) project.ant require.
Comment 56 Jiri Skrivanek 2008-12-08 16:49:58 UTC
> "It is a FileObject I want to listen to." - I see that, but it doesn't make sense as an example usage for this API.
> There is no FO for a parent dir that I know exists, and even if there were, adding a FCL directly to it would only
> report changes in its direct children, which is irrelevant here.

I forgot to say that new implementation would meet original requirement of this issue (Allow to listen on subtrees). A
FCL on folder should then get events from all descendants.

> "My proposal just provides the same functionality as org.netbeans.modules.project.ant.FileChangeSupport." - that does
> not seem to be true. FCS supports my #3 requirement, by retaining a tree of existing FileObject's down to the listened
> file, so that any disk change will be reported after a full refresh, without requiring you to hold onto any particular
> FO instances manually. I don't think this is an _efficient_ way to support that requirement - an impl inside masterfs
> could perhaps do better, as I suggested.

You are right. I didn't completely realize the trick in FileChangeSupport and thought it may happen that some ancestor
FileObject can be garbage collected and then events are not delivered. Your implementation seems to be appropriate for
current FileObjects' refresh implementation. I don't know how to use timestamps or Map<File,Long> for refresh. To create
FileEvent I need FileObject. There has to be some completely different refresh model.

> "My note just emphasize the difference from the current behaviour." - I still don't understand what your note means -
> rephrase it grammatically and precisely, as this is going to be an API contract.

As you said, "you need to hold a strong ref to the listener for as long as you expect to get changes."

> #3

Do you excect recursive handling of events? Will the listener like this 
FileUtil.addFileChangeListener(fcl, new File("/dir/subdir")); be notified about all changes under the subdir? E.g.
creation of /dir/subdir/subsubdir/file?
Comment 57 Jesse Glick 2008-12-09 21:49:03 UTC
"a FCL on folder should then get events from all descendants" - I see. So that would provide feature #2 (without #3)
_if_ you could be assured that the root folder would never be deleted. Possibly useful in the Parsing API, but I don't
know - it is generally better to have the listening still work in case the root folder is deleted but later recreated.
This would probably have to be considered an incompatible change if you used the same FileObject.addFCL method as
currently exists and does not deliver recursive events.

"I don't know how to use timestamps or Map<File,Long> for refresh. To create FileEvent I need FileObject." - well, if we
wanted to use FileChangeListener/FileEvent for this purpose, then you would need to create the FileObject on demand
(FileEvent.getSource I guess), e.g. using FileUtil.toFileObject. A client might or might not need a FileObject (instead
of a File) from the event, so a new listener/event based on File could make sense; or FileEvent could be extended with a
public File getDiskFile() method which would be lower overhead in this situation, with getFile() just being a
convenience for toFileObject.

#3 _without_ #2 would not need any recursive notification, and in fact would only be meaningful when called on a file
path (not a directory path). This is the use case needed by project.ant. There may be use cases for #3 _plus_ #2, but I
don't know of them offhand.
Comment 58 Jiri Skrivanek 2008-12-10 10:01:56 UTC
I tend to abandon recursive file event firing. It doesn't seems to be useful without proper refresh of external changes.
I will try to prepare a patch for review adding FileUtil.addFileChangeListener(fcl, File). It should have the same
functionality as your FileChangeSupport but I will try to reuse existing FileEvent and FileChangeListener.
Comment 59 Jesse Glick 2008-12-10 22:35:23 UTC
"recursive file event firing [...] doesn't seem to be useful without proper refresh of external changes" - perhaps not.
Probably depends on the use case. For example, if you want to listen to changes in $prjdir/src/**/*.java, this would be
OK - you can expect $prjdir/src is not normally deleted. (If it is, you need to refire bigger changes anyway.) But if
you want to listen to $prjdir/build/classes/**/*.class then you need the listener to survive recreation of $prjdir/build.
Comment 60 Jiri Skrivanek 2009-01-06 14:50:54 UTC
Please, review attached patch. It is based on Jesse's FileChangeSupport but it uses existing FileEvent and
FileChangeListener. It adds FileUtil.addFileChangeListener(FileChangeListener listener, File path) and
FileUtil.removeFileChangeListener(listener, path). Behaviour is described at javadoc and tests.
Comment 61 Jiri Skrivanek 2009-01-06 14:52:17 UTC
Created attachment 75496 [details]
Listen to File patch.
Comment 62 Jesse Glick 2009-01-13 01:50:51 UTC
[JG01] Delete FileUtilTest.suite().


[JG02] Try to replace FileChangeListenerSupport in project.ant with the new API and check that tests still pass. Also
try to use the new API in the other places around our codebase that this has gotten copied to.
Comment 63 Jiri Skrivanek 2009-01-13 12:34:29 UTC
> [JG01] Delete FileUtilTest.suite().

Why? It is helpful when you want to run just a single test case.

> [JG02] Try to replace...

Tests in project.ant passes. With other tests in FileUtil I think it is enough to prove its function.

If there are no other objections I will integrate the patch tomorrow.
Comment 64 Jesse Glick 2009-01-13 14:02:19 UTC
JG01 - this should be an IDE (or harness) function - I thought we were getting it in 6.5 but not quite yet (vote for it
please). We do not want to have suite() methods littering all our test sources. Also there is the danger that when
adding a new test you will forget to add it to the "suite".

JG02 - you need to at least identify all the other places using this duplicated code, check that they are not using
customized versions with different functionality or newer bug fixes, and file issues for them to use the new API.
Comment 65 Jiri Skrivanek 2009-01-14 10:26:06 UTC
Fixed. It adds FileUtil.addFileChangeListener(FileChangeListener listener, File path). It doesn't allow to listen to
subtrees because we cannot reflect external changes.

http://hg.netbeans.org/core-main/rev/90ebee7f1fb3

> JG01 - OK.
> JG02 - I will file issues for the following modules:

j2ee.common - bugfix already covered in FileUtil
j2ee.persistenceapi, java.source, maven, ruby.rakeproject, spring.beans - unmodified
Comment 66 Quality Engineering 2009-01-15 07:16:29 UTC
Integrated into 'main-golden', will be available in build *200901150201* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/90ebee7f1fb3
User: Jiri Skrivanek <jskrivanek@netbeans.org>
Log: #33162 - Added FileUtil.addFileChangeListener(fileChangeListener, File). It allows you to listen to a file which does not yet exist, or continue listening to it after it is deleted and recreated, etc.
Comment 67 Erno Mononen 2009-01-21 15:05:08 UTC
Late for the review, but as agreed with Jirka I'm adding my comments here.

I think the method shouldn't throw an IAE when you add a listener to a path that is already being listened, more so 
since there is no way to check whether the path already has a listener. In addition, why not allow more than one 
listener for a path? What if another module has already added a listener to a path that you'd be interested in?

Similarly, I think the removeFileChangeListener(FileChangeListener, File) method should also not throw an IAE, maybe 
just return false instead.
Comment 68 Erno Mononen 2009-01-21 15:34:45 UTC
> In addition, why not allow more than one listener for a path? What if another module has already added a listener to 
a path that you'd be interested in?

I think I misread the JavaDoc - a different listener instance seems to be allowed for the same path, so ignore the 
above.


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo