Skip to content

Commit 218394b

Browse files
committed
Fix: keep WireBox mappings registered when first metadata processing fails
WireBox removed a mapping when its first mapping.process() call failed. The first caller received the original error. Later lookups failed with Injector.InstanceNotFoundException until the application was reinitialized. This turned a temporary load error into a lasting outage for explicit binder.map().to() mappings because WireBox could not recreate them. WireBox now keeps the failed mapping registered and unprocessed. The next lookup retries processing. The original caller still receives the original error. Retrying is safe because Mapping.process() marks the mapping as discovered only after processing succeeds. An exclusive lock prevents concurrent processing. The dependency injection methods also skip names that are already registered. Updated both places that removed failed mappings: - Injector.getInstance() - Binder.processMappings() Added four specs to InjectorLiveTest.cfc. The full WireBox test suite passes on Adobe ColdFusion 2023, BoxLang 1.15, and Lucee 5.4.
1 parent 3bfdbfa commit 218394b

3 files changed

Lines changed: 113 additions & 13 deletions

File tree

system/ioc/Injector.cfc

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -575,16 +575,9 @@ component serializable="false" accessors="true" {
575575

576576
// Check if the mapping has been discovered yet, and if it hasn't it must be autowired enabled in order to process.
577577
if ( NOT mapping.isDiscovered() ) {
578-
try {
579-
// process inspection of instance
580-
mapping.process( binder = variables.binder, injector = this );
581-
} catch ( any e ) {
582-
// Remove bad mapping
583-
var mappings = variables.binder.getMappings();
584-
mappings.delete( name );
585-
// rethrow
586-
throw( object = e );
587-
}
578+
// Read the mapped object's metadata.
579+
// Keep the mapping after an error so the next lookup can try again.
580+
mapping.process( binder = variables.binder, injector = this );
588581
}
589582

590583
// Request object from scope now, we now have it from the scope created, initialized and wired

system/ioc/config/Binder.cfc

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1364,11 +1364,10 @@ component accessors="true" {
13641364
} )
13651365
.each( function( key, thisMapping ){
13661366
try {
1367-
// process the metadata
1367+
// Read the mapped object's metadata.
13681368
arguments.thisMapping.process( binder = this, injector = variables.injector );
13691369
} catch ( any e ) {
1370-
// Remove bad mapping
1371-
variables.mappings.delete( key );
1370+
// Keep the mapping so the next lookup can try again.
13721371
mappingError = e;
13731372
}
13741373
} );

tests/specs/ioc/InjectorLiveTest.cfc

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,114 @@ component extends="tests.resources.BaseIntegrationTest" {
130130
} );
131131
} );
132132
} );
133+
134+
feature( "Keep mappings after a processing error (COLDBOX-1420)", function(){
135+
beforeEach( function( currentSpec ){
136+
variables.injector1420 = new coldbox.system.ioc.Injector();
137+
variables.ghostPath1420 = expandPath( "/tests/resources/Ghost1420.cfc" );
138+
} );
139+
140+
afterEach( function( currentSpec ){
141+
if ( fileExists( variables.ghostPath1420 ) ) {
142+
fileDelete( variables.ghostPath1420 );
143+
}
144+
} );
145+
146+
story( "Keep explicit mappings after a processing error", function(){
147+
given( "a mapping for a component that does not exist", function(){
148+
then( "each lookup reports a component error and keeps the mapping", function(){
149+
injector1420
150+
.getBinder()
151+
.map( "ghost1420@demo" )
152+
.to( "tests.resources.DoesNotExist1420" );
153+
154+
var firstErrorType = "NONE";
155+
try {
156+
injector1420.getInstance( "ghost1420@demo" );
157+
} catch ( any e ) {
158+
firstErrorType = e.type;
159+
}
160+
expect( firstErrorType ).notToBe( "NONE", "The first lookup should report an error" );
161+
expect( firstErrorType ).notToBe( "Injector.InstanceNotFoundException" );
162+
163+
// The failed lookup must not remove the mapping.
164+
expect( injector1420.getBinder().mappingExists( "ghost1420@demo" ) ).toBeTrue();
165+
166+
// The second lookup must try to read the component again.
167+
var secondErrorType = "NONE";
168+
try {
169+
injector1420.getInstance( "ghost1420@demo" );
170+
} catch ( any e ) {
171+
secondErrorType = e.type;
172+
}
173+
expect( secondErrorType ).notToBe( "NONE", "The second lookup should report an error" );
174+
expect( secondErrorType ).notToBe( "Injector.InstanceNotFoundException" );
175+
} );
176+
} );
177+
178+
given( "a mapped component added after the first lookup", function(){
179+
then( "the second lookup creates the instance", function(){
180+
injector1420
181+
.getBinder()
182+
.map( "ghostFile1420@demo" )
183+
.to( "tests.resources.Ghost1420" );
184+
185+
// The first lookup fails because the component file does not exist.
186+
var firstErrorType = "NONE";
187+
try {
188+
injector1420.getInstance( "ghostFile1420@demo" );
189+
} catch ( any e ) {
190+
firstErrorType = e.type;
191+
}
192+
expect( firstErrorType ).notToBe( "NONE", "The first lookup should report an error" );
193+
expect( injector1420.getBinder().mappingExists( "ghostFile1420@demo" ) ).toBeTrue();
194+
195+
// Add the component file and try the same mapping again.
196+
fileWrite( variables.ghostPath1420, "component {}" );
197+
var instance = injector1420.getInstance( "ghostFile1420@demo" );
198+
expect( isObject( instance ) ).toBeTrue();
199+
} );
200+
} );
201+
202+
given( "one missing component mapped under two names", function(){
203+
then( "a failed lookup keeps both names", function(){
204+
injector1420
205+
.getBinder()
206+
.map( [ "aliasA1420", "aliasB1420" ] )
207+
.to( "tests.resources.DoesNotExist1420" );
208+
209+
try {
210+
injector1420.getInstance( "aliasA1420" );
211+
} catch ( any e ) {
212+
// The missing component error is expected.
213+
}
214+
215+
expect( injector1420.getBinder().mappingExists( "aliasA1420" ) ).toBeTrue();
216+
expect( injector1420.getBinder().mappingExists( "aliasB1420" ) ).toBeTrue();
217+
} );
218+
} );
219+
} );
220+
221+
story( "Keep failed mappings during processMappings()", function(){
222+
given( "a mapping for a component that does not exist", function(){
223+
then( "processMappings() reports the error and keeps the mapping", function(){
224+
injector1420
225+
.getBinder()
226+
.map( "bad1420" )
227+
.to( "tests.resources.DoesNotExist1420" );
228+
229+
var errorType = "NONE";
230+
try {
231+
injector1420.getBinder().processMappings();
232+
} catch ( any e ) {
233+
errorType = e.type;
234+
}
235+
expect( errorType ).notToBe( "NONE", "processMappings() should report an error" );
236+
expect( injector1420.getBinder().mappingExists( "bad1420" ) ).toBeTrue();
237+
} );
238+
} );
239+
} );
240+
} );
133241
}
134242

135243
}

0 commit comments

Comments
 (0)