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 30085 - FileObject move notification should be added in the Filesystem API
Summary: FileObject move notification should be added in the Filesystem API
Status: RESOLVED WONTFIX
Alias: None
Product: platform
Classification: Unclassified
Component: Filesystems (show other bugs)
Version: 3.x
Hardware: All All
: P1 blocker (vote)
Assignee: Jiri Skrivanek
URL:
Keywords: API
Depends on:
Blocks: 30369 47035
  Show dependency tree
 
Reported: 2003-01-14 15:08 UTC by Jan Pokorsky
Modified: 2008-12-22 10:09 UTC (History)
3 users (show)

See Also:
Issue Type: ENHANCEMENT
Exception Reporter:


Attachments
I implemented some way. A little different approach, then I announced. (2.97 KB, text/plain)
2003-02-19 15:50 UTC, rmatous
Details
I added simple impl. and I would like to know if it is acceptable. Here is simplified usage. (478 bytes, text/plain)
2003-02-19 16:00 UTC, rmatous
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Pokorsky 2003-01-14 15:08:43 UTC
I am missing some kind of notification about
moving of FileObjects in the FileSystem API. Even
though the FileObject provides support for the move.

I am not sure what is the best way to achieve
this. Maybe introducing FileMoveEvent with
getNewFileObject() in
FileChangeListener.fileDeleted()?
Comment 1 Jesse Glick 2003-01-14 16:13:45 UTC
Possible duplicate of issue #28497? IMHO without identity-retaining
proxies, a FileMoveEvent does not make much sense because you can do a
copy-delete move without invoking any special code in FS API (just
copy streams, then delete original) and of course no event would be
fired as a result.
Comment 2 Jan Pokorsky 2003-01-14 18:09:08 UTC
Will identity-retaining proxies handle a file manipulation  without
using FS API as Jesse mentioned (eg copy stream, then delete original)?

IMO FS API should notify at least about operations performed via that API.

BTW the issue #28497 and the thread on nbdev says nothing about event
notification.
Comment 3 rmatous 2003-01-15 10:04:46 UTC
I prefer being notified than retain identity.
I think that retain identity is almost the same as be notified about
change for user of API. Retain identity means to introduce new
FileSystem only because move should be  handled in different
way.Moreover identity-retaining complicates also slightly client code
to be correct. First you must call FileUtil.proxy (and mainly you must
know about such method) to get another instance of FileObject. Also if
you need instance of original FileSystem you must be aware that from
ProxyFileObject you can`t get it anymore. Next mentioned problem with
equals.
Jan asked the same question as I asked for myself. And the answer is,
that naturally also identity-retaining impl. is not able to handle all
obscure cases how to move FileObject (e.g.: original FileObject may be
deleted month after original FileObject was copied, while IDE was
restarted for umpteenth time). 
Comment 4 rmatous 2003-02-19 15:50:54 UTC
Created attachment 9039 [details]
I implemented some way. A little different approach, then I announced.
Comment 5 rmatous 2003-02-19 16:00:57 UTC
Created attachment 9041 [details]
I added simple impl. and I would like to know if it is acceptable. Here is simplified usage.
Comment 6 Jesse Glick 2003-02-19 16:21:33 UTC
1. Prefer ChangeListener to a PropertyChangeListener with a dummy
property.

2. I think "public final" is preferred to "final public" for
readability - consult Checkstyle.

3. Dislike that moveNotify is package private. IMHO there is a problem
that the filesystems package has API & SPI in the same package,
because there is always a temptation to make the "official" impls have
abilities that independent impls of FileSystem/FileObject cannot
possibly have. The use of instanceof in getFileObjectIdentity is an
obvious symptom. Suggest perhaps in FileObject:

protected boolean supportsFileObjectIdentity() {
    return false;
    // overridden in AbstractFolder to return true
}

protected void changeIdentity(FileObject nue) {
    FileObjectIdentity.moveNotify(this, nue);
}

Of course if changeIdentity is only ever called by FileUtil.move
anyway, then *all* file objects support it, and there is no need for
such methods in FileObject.
Comment 7 rmatous 2003-02-19 16:55:49 UTC
You are right in your comments, but I'm not sure that I understand the
last paragraph. Is it hypothetical, if everybody used
FileUtil.moveFile instead of FileObject.move ?
Comment 8 Jesse Glick 2003-02-19 18:26:47 UTC
Huh, sorry, did not see that FileObject.move() existed. So as to
comment #3: use of FileUtil.moveFile vs. FileObject.move() is
irrelevant, since FU.mF just delegates. Therefore, you *do* need the
protected SPI methods I suggested in FileObject: if someone is
directly extending FileObject, they may be overriding move(), and so
they will need access to the stuff in FOI, to have the same power as
AbstractFileSystem & MultiFileSystem. So suggest slightly different:

class FileObject {
    // ...
    /** override to return true if you call changeIdentity() in move() */
    protected boolean supportsFileObjectIdentity() {
        return false;
        // overridden in AbstractFileObject to return true
    }
    protected void changeIdentity(FileObject nue) {
        FileObjectIdentity.moveNotify(this, nue);
    }
    move(...) {
        // ... as before, but call changeIdentity in else-block
    }
}
class AbstractFileObject {
    // ...
    move(...) {
        // ...as before, but maybe call changeIdentity()
    }
}
class MultiFileObject {
    // ...
    move(...) {
        // ...as before, but maybe call changeIdentity()
    }
}

Now FOI should call supportsChangeIdentity rather than using instanceof.

Only compat problem here is that for an older FileSystem which extends
FileObject directly but does not override move(), sFOI() will return
false, though it could return true. But that is OK I think - not
really incompatible, since FOI was not supported before at all.
Comment 9 Jaroslav Tulach 2003-02-20 09:30:41 UTC
I would like to implement the solution without the need to add
protected (final) methods into FileObject. Enough there are those
fireFile*Event ones. But I have to admit, I do not see an easy way.

supportChangeIdentity is more likely attribute of all FileObjects on
one FileSystem. So it might be property of FileSystem. Either
protected method there (nearly as ugly as above) or which I think is
better - new argument to (protected) constructor. This would nearly
hide the feature for API users and left it there just for SPI writers.

As the main purpose for supportChangeIdentity and notifyIdentityChange
methods is to give writers extending directly FileSystem outside of
the api package (which not many people do) we do not need to make it
too simple. Thus I suggest:

class FileSystem {
  protected FileSystem (FileObjectIdentityFactory identity) {
     this.foif = identity;
  }

  /** package private */ FileObjectIdentityFactory getFOIF () {
     return foif;
  }
}

/** Just pure SPI */
interface FileObjectIdentityFactory {
   FileObjectIdentity createIdentity (FileObject fo);
}

implementation FileObjectIdentity as is now, just does not create new
identity itself, but asks the filesystem. Both AbstractFS and MultiFS
return their own identity.


Now a bit from API point of view: It is not very useful to allow null
as valid return value from getIdentity (). Imagine I am writing
CloneableEditorSupport and need the identity. How the code will look
like if null is allowed value? There will be field of type Object
either being assigned by FOIdentity or by FO. Actually what CES needs
is the simplest identity that guarantees as much as possible. That
means I can listen on the FOIdentity and be notified when the FO is
moved or renamed (update the window title) or is no longer valid
(close the window). Thus by default just provide the best impl
possible (no support for move, just support for delete). Ok?


Naming murmuring: Do not you guys find silly to start every class in
the package by "FileObject" prefix? If you really feel that Identity
should start with it than better make it innerclass of FileObject and
provide FileObject.getIdentity method. If you do not like the
innerclass why not call it just Identity?



Comment 10 Jesse Glick 2004-05-21 20:13:49 UTC
Probably this RFE should be reconsidered in light of masterfs, which
might make it easier to implement. Could be valuable in a number of
places, I think.
Comment 11 Antonin Nebuzelsky 2008-04-15 17:04:31 UTC
Reassigning to new module owner jskrivanek.
Comment 12 Jiri Skrivanek 2008-10-13 16:23:09 UTC
If there is still a need for a move notification, please reopen with some use case. It seems not for four years at least.