reassign to platform|Options ... please evaluate ASAP!

Changing product:component. I came across "ide:ImportSettings" which seems more appropriate.

Changed product:component, but assigned-to didn't change. Changing that to generic.

Can't reproduce on Win XP. 

- I installed jvi 
 (7.0rc1/config/Preferences/org/netbeans/modules/jvi folder contains ONLY module.properties file)

-  changed some key options 
  (KeyBindings.properties is added to 7.0rc1/config/Preferences/org/netbeans/modules/jvi)

- imported "All" from rc1 to rc2
Result: 7.0rc2/config/Preferences/org/netbeans/modules/jvi contains two files: module.properties and KeyBindings.properties.

May be this is too simple testcase? err, could you please try this simple one? Do you still have this problem? 
According to the log you attached everything should be fine.
I'll proceed with testing, please feel free to reopen the bug as soon as you are able to reproduce. Thanks.

Anyway this is not P1, downgrading to P2

> not P1, downgrading to P2
I understand. (though there is lost data with no workaround (if you consider the preferences as data)

The jVi on sourceforge did not have the new layer.xml that references all the preferences for import; I just added that a few days ago. I appologise.  I have upload nbvi-1.3.1.beta6.2-bug.zip to sf https://sourceforge.net/projects/jvi/files/jvi/1.3-NB7.0/. (the SF state has been pending for 30 minutes, I can attach the jvi-zip, ~400k, if you want)

I reproduced the problem.

- starting with clean userdir

- start IDE-RC2 (don't import from 6.9 settings)
- install nbm's from nbvi-1.3.1.beta6.2-bug.zip, restart IDE
  NOTE: must completely uninstall any previous version, I haven't
        updated the module version numbers yet.
  In /erra/.netbeans/TEST/config/Preferences/org/netbeans/modules/
  only see "jvi/module.properties" as you noticed in your test.
- imoprt from an RC1 userdir
  The jVi entry should show 5 sub-categories
  Optins, Filemarks, Registers, Marks, Command/Search
- Check jVi only in dialog
- press OK
- uncheck "Restart IDE..."
- press Yes, See
    /modules/ $ ls jvi*
    jvi.properties*

    jvi:
    KeyBindings.properties*  filemarks/  module.properties*  search.properties*
    commands.properties*     marks/      registers/
- exit IDE, see
    /modules/ $ ls jvi*
    jvi.properties*

    jvi:
    KeyBindings.properties*  marks/  module.properties*


To insure there are the preferences in all the categories in the RC1 userdir, you can do something like:
- install a jVi in RC1
- open a file, caret on some line of the file that has text
- create a filemark, enter the two characters: mA
  that is lower case 'm' following by uppercase 'A'
  can then enter: :marks
  to see the that mark was created, this also puts something in
  commands.properties
- put something in register b, enter 4 characters: "byy
- put something in search.properties by searching, enter: /foo
  To search for foo in the file
- Exit the IDE, to persist the information into userdir preferences
So with these steps there is stuff to import

It took all day, but the download on sourceforge is finally available.

http://sourceforge.net/projects/jvi/files/jvi/1.3-NB7.0/nbvi-1.3.1.beta6.2-bug.zip/download

Any idea what's going on? This is a serious problem for jVi. I'll need to find a workaround.

I managed to reproduce. Evaluating.

Jarda, could you please take a look, can it be a side effect of the recent change in filesystem? It is strange that existing files disappear as soon as IDE is closed.

Well, I made search.properties file read only after Import, then closed the IDE and got the following exception:

Caused: org.openide.filesystems.FSException: C:\Documents and Settings\yn158264\.netbeans\7.0rc2\config\Preferences\org\netbeans\modules\jvi\search.properties is read-only.
	at org.openide.filesystems.LocalFileSystem.lock(LocalFileSystem.java:476)
	at org.netbeans.core.startup.layers.LocalFileSystemEx.lock(LocalFileSystemEx.java:159)
	at org.openide.filesystems.LocalFileSystem$Impl.lock(LocalFileSystem.java:671)
	at org.openide.filesystems.AbstractFileObject.lock(AbstractFileObject.java:261)
	at org.openide.filesystems.MultiFileObject$MfLock.<init>(MultiFileObject.java:1665)
	at org.openide.filesystems.MultiFileObject.lock(MultiFileObject.java:698)
	at org.openide.filesystems.MultiFileObject$MfLock.<init>(MultiFileObject.java:1665)
	at org.openide.filesystems.MultiFileObject.lock(MultiFileObject.java:698)
	at org.openide.filesystems.FileObject.delete(FileObject.java:363)
	at org.netbeans.core.startup.preferences.PropertiesStorage.removeNode(PropertiesStorage.java:185)
	at org.netbeans.core.startup.preferences.NbPreferences.removeNodeSpi(NbPreferences.java:187)
Caused: java.util.prefs.BackingStoreException
	at org.netbeans.core.startup.preferences.NbPreferences.removeNodeSpi(NbPreferences.java:189)
	at java.util.prefs.AbstractPreferences.removeNode2(AbstractPreferences.java:958)
	at java.util.prefs.AbstractPreferences.removeNode(AbstractPreferences.java:929)
	at org.netbeans.core.startup.preferences.NbPreferences.removeNode(NbPreferences.java:249)
[catch] at com.raelity.jvi.core.Misc.writeList(Misc.java:1222)
	at com.raelity.jvi.core.Misc.write_viminfo_search(Misc.java:1193)
	at com.raelity.jvi.core.Misc.access$700(Misc.java:76)
	at com.raelity.jvi.core.Misc$1.propertyChange(Misc.java:115)
	at java.beans.PropertyChangeSupport.firePropertyChange(PropertyChangeSupport.java:339)
	at java.beans.PropertyChangeSupport.firePropertyChange(PropertyChangeSupport.java:347)
	at java.beans.PropertyChangeSupport.firePropertyChange(PropertyChangeSupport.java:276)
	at com.raelity.jvi.manager.ViManager.firePropertyChange(ViManager.java:597)
	at com.raelity.jvi.manager.ViManager$2.run(ViManager.java:227)
	at org.netbeans.modules.jvi.Module.close(Module.java:274)
	at org.netbeans.core.startup.NbInstaller.close(NbInstaller.java:696)
	at org.netbeans.ModuleManager.shutDown(ModuleManager.java:1745)
	at org.netbeans.core.startup.ModuleSystem.shutDown(ModuleSystem.java:316)
	at org.netbeans.core.NbLifecycleManager.doExit(NbLifecycleManager.java:167)


ALL [null]: C:\Documents and Settings\yn158264\.netbeans\7.0rc2\config\Preferences\org\netbeans\modules\jvi\search.properties is read-only.

I didn't dig a lot into jvi code , but is it possible that the cause of the problem is here:
com.raelity.jvi.core.Misc.writeList(Misc.java:1222) ["prefs.removeNode();"]
com.raelity.jvi.core.Misc.write_viminfo_search(Misc.java:1193) 

err, could you please evaluate?

(In reply to comment #12)
> I didn't dig a lot into jvi code , but is it possible that the cause of the
> problem is here:
> com.raelity.jvi.core.Misc.writeList(Misc.java:1222) ["prefs.removeNode();

jVi persists a variety of of information when the IDE closes. It does this by removing and rewriting the data. Since there was no data when the IDE started...

I'll run some tests, if the import fires the appropriate preference change operations, then I should be able to work around the problem. 

I guess at least for the jVi case, it expects/requires a clean restart with the data base updated after it closes. I guess like the way some modules require an IDE restart when they are installed.

Sorry for the trouble. I'll close this after I run some test.

The import process does not fire any Preferences change events. (summary updated)

How can I detect which imports have taken place?

Back to P2.

I can provide the source and/or debug output that shows that the events are not being generated.

Do I understand correctly that NbPreferences don't deliver any changes when their underlaying storage file changes? They don't and they should. 

Write a unit test against NbPreferences to demonstrate the buggy behaviour. I am CC'ing Angelo, as I know his team had such test some time ago.

Created attachment 107756 [details]
monitors for pref subtree creation and/or changes

> unit test against NbPreferences
Actually, a unit test against OptionsImport is what's needed.

I'm guessing it simply removes/replaces files and doesn't use NbPreferences for the updates. The files should be replaced during the next restart, not while the IDE is running. (or NbPreferences should be used to udpate the preferences)

I'm attaching PreferencesChangeMonitor, which Preferences listeners, I put together if Yulia wants to use it in some test. It also has a "hack check" which doesn't depend on listeners, but this also doesn't work.

I believe http://hg.netbeans.org/releases/rev/1cdc8b3c5503 fixes this bug.

Fixed in both trunk and release701.

Can someone verify as soon as the fix appears in a new build?

(In reply to comment #18)
> Fixed in both trunk and release701.
> 
> Can someone verify as soon as the fix appears in a new build?

I'll do some checking over the next few days. I don't have a releases clone, but I see the change in main-silver.

Two approaches are
1) require a restart to replace the files
2) update the files while things are running.

The fix from comment 17 is a beginning for 2). I am concerned that when the events are working 2) might be destablizing. I don't know what 7.01 release timeframe is, but...

There seem to be several problems with the fix.
ER01 - No events are fired. Attached unit test patch fails (main-silver).
ER02 - NbPreferences should not implement ChangeListener; that changes its API.
ER03 - The fix listens to file changes. This means that normal operation
       will cause NbProperties.stateChanged() to be invoked. Huge overhead.
       I think only file removed/added should be used.
ER04 - how is the storage file updated when import? The PropertiesStorage
       class says that NbPreferences is responsible for synchonization.

I'll attached a patch to the unit test that first tests that 4 PreferenceChangeEvent and NodeChangeEvent are issued in normal circumstances (it does not test event parameters just that the events occur). Then it tests for a couple of PreferenceChangeEvent cases. 

A test for NodeChangeEvent should be implemented.

Notice in the patch there is a still a method under developement (its not used) method that replaces the file rather than modifies an existing file.

Created attachment 108884 [details]
test cases for events

simple test cases. incomplete and should be enhanced.

> ER01 - No events are fired. Attached unit test patch fails (main-silver).

No events are fired, correct. But NbPreferences is reloaded now when its underlaying storage file changes so it contains up to date information. From comment #16 I concluded it is what you expected as one of the possible way of fixing so closed the bug. So I'm afraid misunderstanding took place.
Isn't it possible to fix your problem now when all the updated preferences are loaded into NbPreferences after any file change in /Preferences folder? 

ER02 - NbPreferences should not implement ChangeListener; that changes its API.

Make sense. I'll discuss this with Jarda. But since NbPreferences implements method "void stateChanged(ChangeEvent e);" I don't think this is a big problem.

ER03 - The fix listens to file changes. This means that normal operation
       will cause NbProperties.stateChanged() to be invoked. Huge overhead.
       I think only file removed/added should be used.

It listens only changes in preferences storage folder and as a result NbPreferences contains up to date information. It is a general improvement to NbPreferences (not only concerning your situation).

ER04 - how is the storage file updated when import? The PropertiesStorage
       class says that NbPreferences is responsible for synchonization.

What exactly do you mean? While import state of "/Preferences" folder is changed and fileChanged event is fired. This event is listened by FileChangeAdapter in the PropertiesStorage class. When Adapter gets the fileChanged event it runs "NBPreferences.stateChanged" method. This method zeroize properties field in NbPreferences class that leads to the properties reload. 


About jVi form your e-mail:
-------------------------------------
>Here's what jVi does
>
>   1. During startup read data from preferences into internal structures
>   2. During  IDE shutdown remove the old data's preferences
>   3. and then write internal structures to newly created preferences node
>

--------------------------------------

I don't know what is the reason for such a strange implementation but please let me know If firing the event is the only fix that satisfies you. I'll try to implement it in that case but as enchancement not a bug.
Anyway if you only need to recognize if import took place then fileChanged event should be enough. Or you want to know what exactly has been added/changed by import?

(In reply to comment #23)
I assumed that the goal was to get NbPreferences to follow the java Preferences contract when an import happed; undocumented behavior, changing preferences without firing events, can easily cause other problems and be destablizing. If it is too expensive to make it correct, then I would suggest, from comment 20,
        > 1) require a restart to replace the files
This would be reliable.

> > ER01 - No events are fired. Attached unit test patch fails (main-silver).
> 
> No events are fired... NbPreferences reloaded now ...
> comment #16 I concluded it is what you expected as one of the possible ...
> Isn't it possible to fix your problem now ...

Sorry, I didn't mean to suggest that as a fix. Yes, I could make it work for my case, but I think it will create other problems in general, not for jVi.

> ER02 - NbPreferences should not implement ChangeListener; that changes its API.
> 
> Make sense ... [isn't] a big problem.

Using an inner or anonymous class takes care of the issue.

> 
> ER03 - The fix listens to file changes. This means that normal operation
>        will cause NbProperties.stateChanged() to be invoked. Huge overhead.
>        I think only file removed/added should be used.
> 
> It listens only changes in preferences storage folder and as a result
> NbPreferences contains up to date information.

With this change any time a preference is set, "pref.put(key, value)" all the preferences in the node will be reloaded and reparsed. That seems very expensive. If the import replaced the file, rather than overwrite it, then you could listen to file delete/add events and would not incur this additional overhead.

> It is a general improvement to
> NbPreferences (not only concerning your situation).

IIUC, except for the import issue, the only writer to the preferences file is NbPreferences. Since it is only an import issue, using a file replace protocol, rather than overwrite, seems better. There is no need for NbPreferences to support random file modifications.

> ER04 - how is the storage file updated when import? The PropertiesStorage
>        class says that NbPreferences is responsible for synchonization.
> 
> What exactly do you mean?

I have not looked at the import code, so this may not be an issue. Is it possible for both NbPreferences and the import code to be modifying the file at the "same" time?

> 
> 
> About jVi form your e-mail:
> -------------------------------------
> >Here's what jVi does
> >
> >   1. During startup read data from preferences into internal structures
> >   2. During  IDE shutdown remove the old data's preferences
> >   3. and then write internal structures to newly created preferences node
> >
> 
> --------------------------------------
> 
> I don't know what is the reason for such a strange implementation

The "internal structures" are extensive and work with old code. So I read them when the IDE starts and persist them when the IDE shuts down.

> let me know If firing the event is the only fix that satisfies you...

Actually, my main concern is that the current fix could cause problems throughout the platform. I think restart-replace is the way to go and forget the dynamic replace. I will work with whatever ends up happening.

I'll rollback the fix from release701 branch so NetBeans 7.0.1 won't contain one. In trunk I'll do more testing to be sure the fix is save or improve it in other case. 
As for current issue, I downgrade bug to P3 and proceed working on it considering the way "require a restart to replace the files".

Right, there is no need to backport this to a bug fix release like 7.0.1. If the old, broken behavior was good enough for 7.0, it is for sure good enough for 7.0.1.

But we need a fix in trunk and we need it soon. There is enough tests to show what is wrong and I think Ernie will be willing to write even more. As soon as the underlaying FileObject are changed (outside of NbPreferences.store()), they just have to reload and fire changes. Making P2 (for 7.1) again.

(In reply to comment #13)
> jVi persists a variety of of information when the IDE closes. It does this by
> removing and rewriting the data.

Well this is clearly a contributing cause of the bug. No module should be storing preference keys during shutdown - if and when the user makes a change to something, that one key should be saved. Nor should any module be loading a large set of preferences during startup.

Is this really P2 given that it is only known to affect a module not in the standard IDE distribution?

I tend to agree with Jesse's last comment.

If you think this is not a P2 I should downgrade it to P3 and handle it as proposed in comment 25. Otherwise, I think we should waive it as I believe it is too tricky to handle it now for 7.1.

A paying customer building an app on top of NetBeans Platform had to disable an important part of functionality (role switching) due to this bug. I promised to the customer this bug will be fixed in March 2011.

I believe this is P2 and it should have been fixed for 7.0. Rolling this problem in front of us for years is a sign of lack of interest in proper behavior of a standard API.

Who approved the waiver? What can I do "unapprove" it?

(I've just returned home after being away for over a month...)

Jarda succinctly stated my main point when he mentions

> proper behavior of a standard API

Recall that import offers the option to restart; this gives two cases to consider as mentioned in comment 20.

1) choosing restart
   This seems like a simple case to fix since NB already has an infrastructure to run code to change files during a restart.  If the files are changed while NB is shutdown then I would argue that no events are needed.

2) update while running
   Need the change events to be API compatible.


FYI, jVi currently uses an ugly workaround involving looking into the file system and parsing a preferences data base file to see if a well known preference has inexplicably changed.

> I promised to the customer this bug will be fixed in March 2011.

Jardo, would you be able to help resolve the issue now, before code-freeze?

With the change of the area owner few days ago, I see the only option of addressing the issue properly in the release after 7.1.

> Who approved the waiver? What can I do "unapprove" it?

Waiver process - the waiver request was sent to nbdev as usual.

> FYI, jVi currently uses an ugly workaround involving looking into the file
> system and parsing a preferences data base file to see if a well known
> preference has inexplicably changed.

Given the jVi works, though with a hacky workaround, I hope this is not a real stopper.

> Well this is clearly a contributing cause of the bug. No module should be
> storing preference keys during shutdown - if and when the user makes a change
> to something, that one key should be saved. Nor should any module be loading a
> large set of preferences during startup.

This comment above does indicate that there is *not* an agreement on how the API behavior should be changed and that "lack of interest in proper behavior of a standard API." is not the problem here.

(In reply to comment #31)
> 
> > Well this is clearly a contributing cause of the bug. No module should be
> > storing preference keys during shutdown - if and when the user makes a change
> > to something, that one key should be saved. Nor should any module be loading a
> > large set of preferences during startup.
> 
> This comment above does indicate that there is *not* an agreement on how the
> API behavior should be changed and that "lack of interest in proper behavior of
> a standard API." is not the problem here.

IMHO, Jesse's statement is about how things work best in NetBeans and a comment on "best practices". It does not address the *fact* that the NetBeans implementation, NbPreferences, does not adhere to the Preferences API due to the API violations introduced by the NB OptionsImport implementation.

I admit that jVi does not follow common practice, and explain why in comment 24 (about working with legacy data structures).  jVi works OK with  the Preferences API, but has a problem in NetBeans because of the OptionsImport API violations.

My point in comment #27 was that incorrect API behavior only known to affect one module which fails to use best practices for accessing preferences, and not included in the standard distribution, would not be considered a P2 bug. I do not know what comment #29 refers to but if there are other more critical use cases and someone knows how to fix this simply and safely in 7.1, then fine.

(In reply to comment #33)
> My point in comment #27 was that incorrect API behavior only known to affect
> one module which fails to use best practices for accessing preferences, and not
> included in the standard distribution, would not be considered a P2 bug.

I understand, but disagree; breaking a JDK/JRE API seems P2 (especially such a central one).

> I do
> not know what comment #29 refers to but if there are other more critical use
> cases and someone knows how to fix this simply and safely in 7.1, then fine.

The import dialog gives a warning if you don't restart. So the restart case seems like the most important to fix (but I don't know the role switching use case problem either). I'm guessing that the restart is relatively simple to fix as mentioned in comment 30.

Fixed. The approach to restart was adopted.

http://hg.netbeans.org/core-main/rev/946ccf853413

I took a quick look at the fix. I don't see how the preferences files are updated; how is it that the update occurs while the IDE's JVM is not running as mentioned in comment 30 point 1)?

Hello, I took the following steps.

1) Installed nbvi-1.3.0.x1 in netbeans 6.9.1 and then exported some options (see zip file)

2) Started netbeans dev with clean userdir and installed nbvi-1.4.1 and after restart I got the following

===
=== initial conditions
===
~/Desktop/userDir$ pwd
/home/theofanis/Desktop/userDir
~/Desktop/userDir$ ls config/Preferences/org/netbeans/modules/jvi*
commands.properties  filemarks.properties  marks.properties  registers.properties  search.properties

3) Used the Tools->Import dialog to import the previously exported options from 6.9.1 and after the forced ide restart I got the following

===
=== after full import, the ide forces a restart as part of import
=== The expected data is present
===
~/Desktop/userDir$ ls config/Preferences/org/netbeans/modules/jvi*
config/Preferences/org/netbeans/modules/jvi.properties

config/Preferences/org/netbeans/modules/jvi:
commands.properties  filemarks.properties    marks             registers             search.properties
filemarks            KeyBindings.properties  marks.properties  registers.properties

Isn't this what I should be seeing?

From your reply in comment #13 I thought that this is what solves your problem

> I guess at least for the jVi case, it expects/requires a clean restart with the
> data base updated after it closes.

Created attachment 115538 [details]
jvi nbvi-1.3.0.x1 options exported from 6.9.1

The test you ran is invalid. jVi has a workaround as mentioned at the end of comment 30. It was in nbvi-1.4.

It is unfortunate that no test was written as Yarda requested in comment 15.

The "restart option" requires the following steps
    - Shutdown IDE
    - change the files around
    - startup IDE
this is like what happens when a new module is installed that requires restart. Note that the preferences files should only be changed while the JVM is *not* running.

If the underlying preference files are changed while the IDE's JVM is running  (and no events are fired) then the Preferences API is violated.

I can't tell if your latest change does this.

Modified NbPreferences, PropertiesStorage and also wrote some tests.
Now it seems that the events are fired. 
Also, modified import to use FileUtil when folders or files are created but there might be some more deep problem with this so reassigning to Jardo.

All my changes: http://hg.netbeans.org/core-main/rev/eeab76e58c0a

Thanks

Integrated into 'main-golden', will be available in build *201203080400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/eeab76e58c0a
User: Theofanis Oikonomou <theofanis@netbeans.org>
Log: Issue #197594 - options>import fails to fire change events

With the latest build, I've seen some exceptions that may be related to this change.

See http://statistics.netbeans.org/analytics/exception.do?id=564579

Can you guys give me a summary of steps I should do to reproduce the problem in the IDE?

Guys, I see a lot of attachments, but I don't know how to use them to simulate the problem. Can one of you explain it here and reopen the bug, please?