Community
Participate
Working Groups
Opening a dialog in a handler to get a username and password and put it into prefs (part of a tutorial by Lars) by using this code: public class EnterCredentialsHandler { @Inject @Preference(nodePath = "com.example.e4.rcp.todo", value = "user") String userPref; @Inject @Preference(nodePath = "com.example.e4.rcp.todo", value = "password") String passwordPref; @Execute public void execute(Shell shell, @Preference(nodePath = "com.example.e4.rcp.todo") IEclipsePreferences prefs) { PasswordDialog dialog = new PasswordDialog(shell); if (userPref != null) { dialog.setUser(userPref); } if (passwordPref != null) { dialog.setPassword(passwordPref); } if (dialog.open() == Window.OK) { System.out.println("change user now"); prefs.put("user", dialog.getUser()); System.out.println("change password now"); prefs.put("password", dialog.getPassword()); try { System.out.println("flush now"); prefs.flush(); } catch (BackingStoreException e) { } } } } And listening to it by using this code (in a Part): @Inject @Optional public void trackUser( @Preference(nodePath = "com.example.e4.rcp.todo", value = "user") String user) { System.out.println("User = " + user); } @Inject @Optional public void trackPassword( @Preference(nodePath = "com.example.e4.rcp.todo", value = "password") String password) { System.out.println("Pwd = " + password); } The previously persisted values of user/password are aa/bb. Now, by means of the dialog, I set them to aaa/bbb This results in the following output: change user now User = aaa Pwd = bb change password now User = aaa Pwd = bbb flush now To me that is quite unexpected. It seems that pwd is incorrectly injected when the user is changed. pwd did not change. The same when pwd is changed but now for the user. I would have expected the following output: change user now User = aaa change password now Pwd = bbb flush now IMHO this is (at least) a performance issue.
PS. I'm on Juno SR1.
ping?
No one is looking at this component at the moment, but I'd accept patches. http://wiki.eclipse.org/Platform_UI/How_to_Contribute My guess would be that it's done in the org.eclipse.e4.core.di.internal.extensions.PreferencesObjectSupplier in org.eclipse.e4.core.di.extensions PW
Perhaps Paul Elder could take a look at this when we gets back to looking into DI issues...
Moving target milestone since M3 has been delivered.
Removing target milestone since no one seems to be interested in fixing this at the moment.
OMG. A year and a half and still not fixed. Does anybody even use preference injection? The internal problem is the object getting injected is the class, not the method. The class injector has no way of knowing that a 'preference' is getting injected, and cannot refer to the nodePath or the key to know what methods to call; so every time a preference changes, every method and member gets reinjected. Performance is exponential as more methods are added. Dealing with intermediate values is difficult to say the least. My solution was to add a callback interface and a new method in IRequestor. static public interface AllowInjectionCallback { boolean allowInjection(Object location, Object userObject, Object actualArgs); } public Object execute(AllowInjectionCallback allowInjectionCallback) throws InjectionException; I updated all IRequestor implementations to call execute(null) from execute. A null callback has the original implementation, so nothing changes unless an actual handler is sent into the method. The end result is that PreferencesObjectSupplier can now call execute on the object, and veto all annotated methods that do not match the nodePath/key. We can extend this to do the same for members, constructor, etc. I will post the patch as soon as I confirm it does not break anything.
https://git.eclipse.org/r/#/c/25095/
It looks like the problem is we add a listener per (nodePath,requestor) instead of a listener per (nodePath, key, requestor). Why not add the key to the org.eclipse.e4.core.di.internal.extensions.PreferencesObjectSupplier.PrefInjectionListener and allow that to filter the PreferenceChangeEvent? PW
(In reply to Paul Webster from comment #9) > It looks like the problem is we add a listener per (nodePath,requestor) > instead of a listener per (nodePath, key, requestor). > > Why not add the key to the > org.eclipse.e4.core.di.internal.extensions.PreferencesObjectSupplier. > PrefInjectionListener and allow that to filter the PreferenceChangeEvent? > > PW That would result in more listeners, but certainly is a viable option. I'll git to it. Are you OK with having a hash table for each nodePath, or a hash of nodepath/key and a single table?
Released as http://git.eclipse.org/c/platform/eclipse.platform.runtime.git/commit/?id=13042ba9d212d3eae79459c631953f8d9f03b73d Thanks Steven. PW
(In reply to Paul Webster from comment #11) > Released as > http://git.eclipse.org/c/platform/eclipse.platform.runtime.git/commit/ > ?id=13042ba9d212d3eae79459c631953f8d9f03b73d > > Thanks Steven. > > PW thanks guys. nice solution
In 4.4.0.I20140428-2000 Ferry, could you please verify it works for your scenario as well? Thanks. PW
(In reply to Paul Webster from comment #13) > In 4.4.0.I20140428-2000 > > Ferry, could you please verify it works for your scenario as well? Thanks. > > PW Well, if you ran the reproducer given in comment 1 then I'm ok with it. I'm seriously low on time at the moment. sorry. If you tell me where to get a build with the fix in it, I'll try to squeeze in a check. no promises :-(
If you use an SDK, http://download.eclipse.org/eclipse/downloads/drops4/I20140429-2000/ If you use one of the EPP from http://www.eclipse.org/downloads/index-developer.php it would have to wait for another 1.5 weeks. It's OK if it takes 2 weeks, I've already verified it works at this end, but the confirmation from the reporter is still important. Thanks for what you can do. PW
(In reply to Ferry Huberts from comment #14) > (In reply to Paul Webster from comment #13) > > In 4.4.0.I20140428-2000 > > > > Ferry, could you please verify it works for your scenario as well? Thanks. > > > > PW > > Well, if you ran the reproducer given in comment 1 then I'm ok with it. > > I'm seriously low on time at the moment. sorry. > > If you tell me where to get a build with the fix in it, I'll try to squeeze > in a check. no promises :-( I pulled down latest integration and confirmed expected result from comment #1.
OK, thanks guys. PW
Thanks Steven, you saved me some time :-)
