This Bugzilla instance is deprecated, and most Eclipse projects now use GitHub or Eclipse GitLab. Please see the deprecation plan for details.
Bug 413149 - [DI] InjectorImpl.findExtendedSupplier does not use correct PrimaryObjectSupplier
Summary: [DI] InjectorImpl.findExtendedSupplier does not use correct PrimaryObjectSupp...
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Runtime (show other bugs)
Version: 4.3   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: 4.4 M1   Edit
Assignee: Paul Webster CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-07-17 05:33 EDT by Christoph Keimel CLA
Modified: 2013-08-07 13:30 EDT (History)
3 users (show)

See Also:

Attachments
Patch based on R4_3_maintenance (1.39 KB, patch)
2013-07-17 05:35 EDT, Christoph Keimel CLA 		no flags Details | Diff
Patch based on R4_3_maintenance (eclipse.platform.ui) (1.77 KB, patch)
2013-07-18 04:08 EDT, Christoph Keimel CLA 		no flags Details | Diff
patch setting the defaultSupplier earlier during the creation of the E4 workbench (1.76 KB, patch)
2013-07-19 05:29 EDT, Christoph Keimel CLA 		no flags Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Christoph Keimel CLA 2013-07-17 05:33:05 EDT 
InjectorImpl.findExtendedSupplier uses its defaultSupplier to inject values into the ExtendedObjectSupplier. It is possible that defaultSupplier is NULL, in which case no injection happens. This is unnesessary, since a valid PrimaryObjectSupplier gets passed to findExtendedSupplier, which can be used for the injection in this case.
Comment 1 Christoph Keimel CLA 2013-07-17 05:35:56 EDT 
Created attachment 233549 [details]
Patch based on R4_3_maintenance

Attached is a patch which uses the defaultSupplier, if it has been set and the passed-in objectSupplier if not. The patch is directed at the R_4_3_maintenance branch.
Comment 2 Christoph Keimel CLA 2013-07-17 05:38:57 EDT 
Forum Discussion:
http://www.eclipse.org/forums/index.php/mv/msg/490174/1070130/#msg_1070130
Comment 3 Christoph Keimel CLA 2013-07-17 08:09:40 EDT 
Remark: The defaultSupplier is set to be the application context during the startup of the E4Workbench. So, if an ExtendedSupplier is to be used before the creation of the application context (i.e. during @PostContextCreate in the LifeCyclemanager) it will not get processed by DI.
Comment 4 Christoph Keimel CLA 2013-07-18 04:08:17 EDT 
Created attachment 233585 [details]
Patch based on R4_3_maintenance (eclipse.platform.ui)

Alternative Solution setting the defaultSupplier earlier during the startup of the e4 workbench
Comment 5 Thomas Schindl CLA 2013-07-18 06:33:11 EDT 
to fix this problem in 4.3.x only patch 2 can be considered although patch 1 is interesting in itself but needs more care what would be changed to the DI system.
Comment 6 Paul Webster CLA 2013-07-18 14:20:01 EDT 
(In reply to comment #4)
> Created attachment 233585 [details]
> Patch based on R4_3_maintenance (eclipse.platform.ui)

I'd move this up even higher, to just after the appContext.set(IApplicationContext.class, applicationContext);

PW
Comment 7 Christoph Keimel CLA 2013-07-19 05:29:12 EDT 
Created attachment 233618 [details]
patch setting the defaultSupplier earlier during the creation of the E4 workbench

patch modified according to Pauls suggestion
Comment 8 Christoph Keimel CLA 2013-07-23 03:17:56 EDT 
I pushed the second patch to gerrit for review.
https://git.eclipse.org/r/14766
Comment 9 Paul Webster CLA 2013-07-23 10:56:33 EDT 
I've released patch2 as http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=758c1cbfcf8ce44ea27d48ff0c9ff4d3fa479238

Thanks Christoph.

Setting the defaultSupplier in the E4Application was introduced by bug 389863.  It explains why we don't use the primaryObjectSupplier anymore.

PW
Comment 10 Paul Webster CLA 2013-08-07 13:30:12 EDT 
In 4.4.0.I20130806-2000

PW