Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions system/web/services/ModuleService.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -452,11 +452,26 @@ component extends="coldbox.system.web.services.BaseService" accessors="true" {
// Store module configuration in main modules configuration
modulesConfiguration[ modName ] = mConfig;

// Link aliases by reference in both modules list and config cache
for ( var thisAlias in mConfig.aliases ) {
modulesConfiguration[ thisAlias ] = modulesConfiguration[ modName ];
variables.mConfigCache[ thisAlias ] = variables.mConfigCache[ modName ];
}
// If module name contains ForgeBox username (@username), create a canonical alias
// This allows DSL injection like inject="coldbox:moduleSettings:modulename"
// to work even when the module is installed as modulename@username
if ( find( "@", modName ) ) {
var canonicalName = listFirst( modName, "@" );
// Only create alias if it doesn't conflict with an existing module
if ( !structKeyExists( modulesConfiguration, canonicalName ) ) {
modulesConfiguration[ canonicalName ] = modulesConfiguration[ modName ];
variables.mConfigCache[ canonicalName ] = variables.mConfigCache[ modName ];
if ( variables.logger.canDebug() ) {
variables.logger.debug(
"Created canonical alias [#canonicalName#] for ForgeBox module [#modName#]"
);
}
} else if ( variables.logger.canWarn() ) {
variables.logger.warn(
"Cannot create canonical alias [#canonicalName#] for ForgeBox module [#modName#] - name conflict with existing module"
);
}
}

// Update the paths according to conventions
mConfig.handlerInvocationPath &= ".#replace(
Expand Down
115 changes: 115 additions & 0 deletions tests/specs/web/services/ModuleServiceTest.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,121 @@ component extends="tests.resources.BaseIntegrationTest" {
expect( variables.moduleService.getModuleRegistry() ).toHaveKey( "test-module" );
} )
} );

describe( "ModuleService ForgeBox Alias Creation", function(){

it( "should create canonical alias when module name contains @", function(){
// Arrange
var moduleService = createMock( "coldbox.system.web.services.ModuleService" );
var modules = {};
var mConfigCache = {};

// Mock the logger
var mockLogger = createStub().$( "canDebug", true ).$( "debug" );
moduleService.$property( "logger", "variables", mockLogger );
moduleService.$property( "mConfigCache", "variables", mConfigCache );

// Simulate module config
var mConfig = {
name : "testmodule@testuser",
settings : { apiKey : "test-123" },
aliases : []
};

// Act - simulate what happens in ModuleService.cfc after line 453
var modName = "testmodule@testuser";
modules[ modName ] = mConfig;

// The fix code
if ( find( "@", modName ) ) {
var canonicalName = listFirst( modName, "@" );
if ( !structKeyExists( modules, canonicalName ) ) {
modules[ canonicalName ] = modules[ modName ];
mConfigCache[ canonicalName ] = mConfigCache[ modName ];
}
}

// Assert
expect( modules ).toHaveKey( "testmodule@testuser", "Full name should exist" );
expect( modules ).toHaveKey( "testmodule", "Canonical alias should be created" );
expect( modules[ "testmodule" ] ).toBe( modules[ "testmodule@testuser" ], "Alias should reference same config" );
});

it( "should NOT create alias when module name has no @", function(){
// Arrange
var modules = {};
var mConfigCache = {};
var mConfig = { name : "regularmodule", settings : {} };

// Act
var modName = "regularmodule";
modules[ modName ] = mConfig;

if ( find( "@", modName ) ) {
var canonicalName = listFirst( modName, "@" );
if ( !structKeyExists( modules, canonicalName ) ) {
modules[ canonicalName ] = modules[ modName ];
}
}

// Assert
expect( modules ).toHaveKey( "regularmodule" );
expect( structCount( modules ) ).toBe( 1, "Should only have one entry" );
Comment on lines +52 to +107
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test is duplicating the production code logic (lines 74-80) instead of testing the actual ModuleService.registerModule method. This makes the test fragile and doesn't verify the actual implementation. Consider calling the real registerModule method with a test module configuration to verify the alias creation behavior works end-to-end in the actual service.

Suggested change
// Arrange
var moduleService = createMock( "coldbox.system.web.services.ModuleService" );
var modules = {};
var mConfigCache = {};
// Mock the logger
var mockLogger = createStub().$( "canDebug", true ).$( "debug" );
moduleService.$property( "logger", "variables", mockLogger );
moduleService.$property( "mConfigCache", "variables", mConfigCache );
// Simulate module config
var mConfig = {
name : "testmodule@testuser",
settings : { apiKey : "test-123" },
aliases : []
};
// Act - simulate what happens in ModuleService.cfc after line 453
var modName = "testmodule@testuser";
modules[ modName ] = mConfig;
// The fix code
if ( find( "@", modName ) ) {
var canonicalName = listFirst( modName, "@" );
if ( !structKeyExists( modules, canonicalName ) ) {
modules[ canonicalName ] = modules[ modName ];
mConfigCache[ canonicalName ] = mConfigCache[ modName ];
}
}
// Assert
expect( modules ).toHaveKey( "testmodule@testuser", "Full name should exist" );
expect( modules ).toHaveKey( "testmodule", "Canonical alias should be created" );
expect( modules[ "testmodule" ] ).toBe( modules[ "testmodule@testuser" ], "Alias should reference same config" );
});
it( "should NOT create alias when module name has no @", function(){
// Arrange
var modules = {};
var mConfigCache = {};
var mConfig = { name : "regularmodule", settings : {} };
// Act
var modName = "regularmodule";
modules[ modName ] = mConfig;
if ( find( "@", modName ) ) {
var canonicalName = listFirst( modName, "@" );
if ( !structKeyExists( modules, canonicalName ) ) {
modules[ canonicalName ] = modules[ modName ];
}
}
// Assert
expect( modules ).toHaveKey( "regularmodule" );
expect( structCount( modules ) ).toBe( 1, "Should only have one entry" );
// Arrange: use the real ModuleService from beforeAll()
var moduleService = variables.moduleService;
var mConfig = {
name : "testmodule@testuser",
settings : { apiKey : "test-123" },
aliases : []
};
// Act: register the module and let ModuleService handle alias creation
moduleService.registerModule( mConfig.name, mConfig );
var modules = moduleService.getModules();
// Assert
expect( modules ).toHaveKey( "testmodule@testuser", "Full name should exist" );
expect( modules ).toHaveKey( "testmodule", "Canonical alias should be created" );
expect( modules[ "testmodule" ] ).toBe(
modules[ "testmodule@testuser" ],
"Alias should reference same config"
);
});
it( "should NOT create alias when module name has no @", function(){
// Arrange: snapshot current modules
var moduleService = variables.moduleService;
var beforeModules = duplicate( moduleService.getModules() );
var beforeCount = structCount( beforeModules );
var mConfig = {
name : "regularmodule_nocanonical_test",
settings : {}
};
// Act: register a module with no @ in its name
moduleService.registerModule( mConfig.name, mConfig );
var modules = moduleService.getModules();
// Assert: only one new module entry should have been added
expect( modules ).toHaveKey( "regularmodule_nocanonical_test" );
expect( structCount( modules ) ).toBe(
beforeCount + 1,
"Should only add one module entry when there is no @ in the name"
);

Copilot uses AI. Check for mistakes.
});
Comment on lines +88 to +108
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the first test, this test duplicates the production code logic (lines 98-103) rather than testing the actual ModuleService implementation. Consider using an integration test approach that calls the real service methods.

Copilot uses AI. Check for mistakes.

it( "should NOT create alias if canonical name already exists (conflict)", function(){
// Arrange
var modules = {};
var mConfigCache = {};
var mockLogger = createStub().$( "canWarn", true ).$( "warn" );

// Canonical module exists
modules[ "mymodule" ] = { name : "mymodule", settings : { source : "canonical" } };

// Act - try to register ForgeBox module with same canonical name
var modName = "mymodule@testuser";
modules[ modName ] = { name : modName, settings : { source : "forgebox" } };

if ( find( "@", modName ) ) {
var canonicalName = listFirst( modName, "@" );
if ( !structKeyExists( modules, canonicalName ) ) {
modules[ canonicalName ] = modules[ modName ];
mConfigCache[ canonicalName ] = mConfigCache[ modName ];
} else if ( !isNull( mockLogger ) && mockLogger.canWarn() ) {
mockLogger.warn( "Cannot create canonical alias" );
}
}

// Assert
expect( modules ).toHaveKey( "mymodule" );
expect( modules ).toHaveKey( "mymodule@testuser" );
expect( modules[ "mymodule" ].settings.source ).toBe( "canonical", "Original should remain unchanged" );
expect( modules[ "mymodule@testuser" ].settings.source ).toBe( "forgebox", "ForgeBox module should be separate" );
expect( mockLogger.$once( "warn" ) ).toBeTrue( "Should log warning about conflict" );
});
Comment on lines +110 to +139
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test also duplicates the production code logic (lines 123-131) rather than testing the actual ModuleService. Additionally, the test creates a mockLogger but never properly wires it into the execution context, so the logger warning behavior isn't actually being tested against the real implementation.

Copilot uses AI. Check for mistakes.

it( "should handle multiple @ symbols correctly", function(){
// Arrange
var modules = {};
var mConfigCache = {};

// Act
var modName = "my-module@user@extra";
modules[ modName ] = { name : modName, settings : {} };

if ( find( "@", modName ) ) {
var canonicalName = listFirst( modName, "@" );
if ( !structKeyExists( modules, canonicalName ) ) {
modules[ canonicalName ] = modules[ modName ];
}
}

// Assert
expect( modules ).toHaveKey( "my-module@user@extra" );
expect( modules ).toHaveKey( "my-module", "Should extract name before first @" );
});
Comment on lines +141 to +160
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test also duplicates the production code logic (lines 150-155) instead of testing the actual ModuleService implementation. Consider consolidating these unit tests into integration tests that verify the actual behavior of the ModuleService.registerModule method.

Suggested change
it( "should handle multiple @ symbols correctly", function(){
// Arrange
var modules = {};
var mConfigCache = {};
// Act
var modName = "my-module@user@extra";
modules[ modName ] = { name : modName, settings : {} };
if ( find( "@", modName ) ) {
var canonicalName = listFirst( modName, "@" );
if ( !structKeyExists( modules, canonicalName ) ) {
modules[ canonicalName ] = modules[ modName ];
}
}
// Assert
expect( modules ).toHaveKey( "my-module@user@extra" );
expect( modules ).toHaveKey( "my-module", "Should extract name before first @" );
});

Copilot uses AI. Check for mistakes.

});
}

}
Loading