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 133855 - wrong event order when deleting and creating files in AtomicAction
Summary: wrong event order when deleting and creating files in AtomicAction
Status: RESOLVED FIXED
Alias: None
Product: versioncontrol
Classification: Unclassified
Component: Code (show other bugs)
Version: 6.x
Hardware: All All
: P3 blocker (vote)
Assignee: Tomas Stupka
URL:
Keywords: API_REVIEW_FAST
Depends on:
Blocks:
 
Reported: 2008-04-24 17:54 UTC by Tomas Stupka
Modified: 2011-02-26 05:13 UTC (History)
6 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
Patch of masterfs. (17.06 KB, text/plain)
2008-06-24 13:51 UTC, Jiri Skrivanek
Details
Updated patch. (7.98 KB, text/plain)
2008-06-25 15:42 UTC, Jiri Skrivanek
Details
Updated patch with beforeMove, moveSuccess and moveFailure. (11.33 KB, text/plain)
2008-08-27 12:38 UTC, Jiri Skrivanek
Details
updated patch (11.47 KB, patch)
2010-06-02 11:43 UTC, Tomas Stupka
Details | Diff
updated patch (24.35 KB, patch)
2010-06-09 15:59 UTC, Tomas Stupka
Details | Diff
updated patch (22.09 KB, patch)
2010-06-10 09:40 UTC, Tomas Stupka
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tomas Stupka 2008-04-24 17:54:13 UTC
unit test in o.n.m.versionig/DeleteCreateTest.java
Comment 1 Jiri Skrivanek 2008-06-24 13:50:06 UTC
I will attach a patch which should reflect our discussion. Please, look at it. Previously
BaseFileObj.FileChangeListenerForVersioning triggered ProvidedExtensions (e.g. fileDeleted -> deleteSuccess). That's why
events were fired in different order in case of AtomicAction. I removed FileChangeListenerForVersioning and I call
methods from ProvidedExtensions directly.
I also added ProvidedExtensions.deletedExternally and createdExternally which are called when refresh in FileSystem
detects deleted or created files and it is about to fire events.
With suggested implementation order of events in the DeleteCreateTest is the following:

beforeDelete
getDeleteHandler
getDeleteHandler.delete
fileDeleted
deleteSuccess
beforeCreate
createSuccess
fileDataCreated

----- with AtomicAction --------------------------
beforeDelete
getDeleteHandler
getDeleteHandler.delete
deleteSuccess
beforeCreate
createSuccess
fileDeleted
fileDataCreated

FileChangeEvents are not fired until AtomicAction is finished but ProvidedExtensions methods instantly.

Comment 2 Jiri Skrivanek 2008-06-24 13:51:28 UTC
Created attachment 63334 [details]
Patch of masterfs.
Comment 3 Jaroslav Tulach 2008-06-24 14:31:37 UTC
Looks promising to me. If it really works for vcs guys, then please add:
1. tests
2. increment spec version of the module
3. @since
4. apichange note
Comment 4 Tomas Stupka 2008-06-24 15:38:31 UTC
looks like what we wanted
Comment 5 rmatous 2008-06-24 16:02:40 UTC
Maybe I'm mistaken, BUT createSuccess was originally called after AtomicAction and now as soon as method delete is
finished, is it? Not sure but (as I remember) could lead to to problems because then FS issues instanceS of FileObject
of newly created file before intended(empty file) => wrong data object assigned, mime type, .... 
Comment 6 Jiri Skrivanek 2008-06-25 09:51:12 UTC
Originally, createSuccess was triggered by FileChangeListenerForVersioning.fileDataCreated, i.e. after AtomicAction. But
FileObject for newly created file is always created immediatelly inside AtomicAction (FileObject.createData() returns
created FileObject). From FS point of view it seems transparent. But I don't know what versioning needs. If you want to
be informed immediatelly when file is created, use createSuccess. If you want to wait until AtomicAction finishes, use
FileChangeListener.fileDataCreated.
Comment 7 Jaroslav Tulach 2008-06-25 12:59:17 UTC
Radek refers to problem when VCS tried to access DataObjects for not yet finished FileObjects. Guys, please do not do 
that. Do not leak these FileObjects sent to you through masterfs API anywhere. If we can trust in this, then it is OK 
to notify the events as soon as Jirka knows about them.
Comment 8 Jiri Skrivanek 2008-06-25 15:40:54 UTC
I will attach new patch (apply it to masterfs/src) which adds ProvidedExtensions.fileChanged() method. It is also called
immediatelly regardless it is in AtomicAction or not. Let me know if it suites for you. If yes, I will add tests and
other things as Jarda requested.
Comment 9 Jiri Skrivanek 2008-06-25 15:42:47 UTC
Created attachment 63433 [details]
Updated patch.
Comment 10 Jiri Skrivanek 2008-07-03 08:22:57 UTC
Please, evaluate and reassign back to me if you want me to integrate proposed changes.
Comment 11 Maros Sandor 2008-07-30 12:32:06 UTC
We are missing one thing in the proposed patch: it is not possible NOT to handle a Move operation and have a callback
when it completes. For Delete operations it works fine because we have a deleteSuccess() callback but there is no
moveSuccess() callback.
Comment 12 Jiri Skrivanek 2008-08-27 12:37:09 UTC
I added beforeMove, moveSuccess and moveFailure to InterceptionListener. Reassigning back for evaluation.
Comment 13 Jiri Skrivanek 2008-08-27 12:38:17 UTC
Created attachment 68435 [details]
Updated patch with beforeMove, moveSuccess and moveFailure.
Comment 14 Lukas Hasik 2008-09-09 14:37:04 UTC
will it be fixed in 65? There is not much time.

Comment 15 Maros Sandor 2008-09-10 14:21:27 UTC
We will integrate it after 6.5 is out.
Comment 16 Marian Mirilovic 2008-10-27 16:11:10 UTC
6.5 is cloned ... proceed with integration please
Comment 17 Tomas Stupka 2010-06-02 11:43:54 UTC
Created attachment 99753 [details]
updated patch

last patch version became outdated in the meantime.
Comment 18 Tomas Stupka 2010-06-09 15:59:56 UTC
Created attachment 99943 [details]
updated patch

new patch version:

- added some corrections in o.n.m...FolderObj.refreshImpl(boolean, boolean) - deletedExternally wasn't called

- changed the beforeMove, moveSuccess, moveFailure signatures:
   beforeMove(FileObject from) to moveSuccess(FileObject from, File to)
   moveSuccess(FileObject from) to moveSuccess(FileObject from, File to)
   moveFailure(FileObject from) to moveSuccess(FileObject from, File to)

- added unit tests

- apichanges, javadoc, @since & co.
Comment 19 Jaroslav Tulach 2010-06-10 06:58:51 UTC
Y01 Adding new methods into InterceptionListener is not compatible. Maybe you want to create interface InterfaceptionListener.Move extends InterceptionListener and instanceof at places where you call the new methods?

Y02 Missing javadoc and @since in InterceptionListener new methods.
Comment 20 Tomas Stupka 2010-06-10 09:40:00 UTC
Created attachment 99971 [details]
updated patch

- removed the methods from InterceptionListener and left them only in ProvidedExtensions. handling the same way as e.g. createdExternally, deteledExternally, ... 

- adapted tests
Comment 21 Tomas Stupka 2010-06-21 09:35:13 UTC
will apply the last patch  (id=99971)
Comment 22 Tomas Stupka 2010-06-22 09:18:40 UTC
http://hg.netbeans.org/cdev/rev/7bb7fc36a3ef
fixed
Comment 23 Quality Engineering 2010-06-23 03:30:15 UTC
Integrated into 'main-golden', will be available in build *201006230001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/7bb7fc36a3ef
User: Tomas Stupka <tstupka@netbeans.org>
Log: Issue #133855 - wrong event order when deleting and creating files in AtomicAction
Comment 24 Quality Engineering 2011-02-26 05:13:15 UTC
Integrated into 'main-golden', will be available in build *201102260001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/d9521a9f5144
User: Tomas Stupka <tstupka@netbeans.org>
Log: missing implementation for createdExternaly - should have been added together with #9fd543e21a3e (issue #133855)