mirror of
https://github.com/coollabsio/coolify.git
synced 2025-12-28 05:34:50 +00:00
fix: convert Stringable to plain strings in applicationParser for strict comparisons and collection lookups
This fixes critical bugs where Stringable objects were used in strict comparisons and collection key lookups, causing service existence checks and domain lookups to fail. **Changes:** - Line 539: Added ->value() to $originalServiceName conversion - Line 541: Added ->value() to $serviceName normalization - Line 621: Removed redundant (string) cast now that $serviceName is a plain string **Impact:** - Service existence check now works correctly (line 606: $transformedServiceName === $serviceName) - Domain lookup finds existing domains (line 615: $domains->get($serviceName)) - Prevents duplicate domain entries in docker_compose_domains collection **Tests:** - Added comprehensive unit test suite in ApplicationParserStringableTest.php - 9 test cases covering type verification, strict comparisons, collection operations, and edge cases - All tests pass (24 tests, 153 assertions across related parser tests) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
64cbda0140
commit
75381af742
@ -536,9 +536,9 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
|
||||
}
|
||||
}
|
||||
|
||||
$originalServiceName = str($serviceName)->replace('_', '-');
|
||||
$originalServiceName = str($serviceName)->replace('_', '-')->value();
|
||||
// Always normalize service names to match docker_compose_domains lookup
|
||||
$serviceName = str($serviceName)->replace('-', '_')->replace('.', '_');
|
||||
$serviceName = str($serviceName)->replace('-', '_')->replace('.', '_')->value();
|
||||
|
||||
// Generate BOTH FQDN & URL
|
||||
$fqdn = generateFqdn(server: $server, random: "$originalServiceName-$uuid", parserVersion: $resource->compose_parsing_version);
|
||||
@ -618,7 +618,7 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
|
||||
$domainValue = $port ? $urlWithPort : $url;
|
||||
|
||||
if (is_null($domainExists)) {
|
||||
$domains->put((string) $serviceName, [
|
||||
$domains->put($serviceName, [
|
||||
'domain' => $domainValue,
|
||||
]);
|
||||
$resource->docker_compose_domains = $domains->toJson();
|
||||
|
||||
182
tests/Unit/ApplicationParserStringableTest.php
Normal file
182
tests/Unit/ApplicationParserStringableTest.php
Normal file
@ -0,0 +1,182 @@
|
||||
<?php
|
||||
|
||||
/**
|
||||
* Unit tests to verify that the applicationParser function in parsers.php
|
||||
* properly converts Stringable objects to plain strings to fix strict
|
||||
* comparison and collection key lookup issues.
|
||||
*
|
||||
* Related issue: Lines 539 and 541 in parsers.php were creating Stringable
|
||||
* objects which caused:
|
||||
* - Strict comparisons (===) to fail (line 606)
|
||||
* - Collection key lookups to fail (line 615)
|
||||
*/
|
||||
it('ensures service name normalization returns plain strings not Stringable objects', function () {
|
||||
// Test the exact transformations that happen in parsers.php lines 539-541
|
||||
|
||||
// Simulate what happens at line 520
|
||||
$parsed = parseServiceEnvironmentVariable('SERVICE_URL_my-service');
|
||||
$serviceName = $parsed['service_name']; // 'my-service'
|
||||
|
||||
// Line 539: $originalServiceName = str($serviceName)->replace('_', '-')->value();
|
||||
$originalServiceName = str($serviceName)->replace('_', '-')->value();
|
||||
|
||||
// Line 541: $serviceName = str($serviceName)->replace('-', '_')->replace('.', '_')->value();
|
||||
$serviceName = str($serviceName)->replace('-', '_')->replace('.', '_')->value();
|
||||
|
||||
// Verify both are plain strings, not Stringable objects
|
||||
expect(is_string($originalServiceName))->toBeTrue('$originalServiceName should be a plain string');
|
||||
expect(is_string($serviceName))->toBeTrue('$serviceName should be a plain string');
|
||||
expect($originalServiceName)->not->toBeInstanceOf(\Illuminate\Support\Stringable::class);
|
||||
expect($serviceName)->not->toBeInstanceOf(\Illuminate\Support\Stringable::class);
|
||||
|
||||
// Verify the transformations work correctly
|
||||
expect($originalServiceName)->toBe('my-service');
|
||||
expect($serviceName)->toBe('my_service');
|
||||
});
|
||||
|
||||
it('ensures strict comparison works with normalized service names', function () {
|
||||
// This tests the fix for line 606 where strict comparison failed
|
||||
|
||||
// Simulate service name from docker-compose services array (line 604-605)
|
||||
$serviceNameKey = 'my-service';
|
||||
$transformedServiceName = str($serviceNameKey)->replace('-', '_')->replace('.', '_')->value();
|
||||
|
||||
// Simulate service name from environment variable parsing (line 520, 541)
|
||||
$parsed = parseServiceEnvironmentVariable('SERVICE_URL_my-service');
|
||||
$serviceName = $parsed['service_name'];
|
||||
$serviceName = str($serviceName)->replace('-', '_')->replace('.', '_')->value();
|
||||
|
||||
// Line 606: if ($transformedServiceName === $serviceName)
|
||||
// This MUST work - both should be plain strings and match
|
||||
expect($transformedServiceName === $serviceName)->toBeTrue(
|
||||
'Strict comparison should work when both are plain strings'
|
||||
);
|
||||
expect($transformedServiceName)->toBe($serviceName);
|
||||
});
|
||||
|
||||
it('ensures collection key lookup works with normalized service names', function () {
|
||||
// This tests the fix for line 615 where collection->get() failed
|
||||
|
||||
// Simulate service name normalization (line 541)
|
||||
$parsed = parseServiceEnvironmentVariable('SERVICE_URL_app-name');
|
||||
$serviceName = $parsed['service_name'];
|
||||
$serviceName = str($serviceName)->replace('-', '_')->replace('.', '_')->value();
|
||||
|
||||
// Create a collection like $domains at line 614
|
||||
$domains = collect([
|
||||
'app_name' => [
|
||||
'domain' => 'https://example.com',
|
||||
],
|
||||
]);
|
||||
|
||||
// Line 615: $domainExists = data_get($domains->get($serviceName), 'domain');
|
||||
// This MUST work - $serviceName should be a plain string 'app_name'
|
||||
$domainExists = data_get($domains->get($serviceName), 'domain');
|
||||
|
||||
expect($domainExists)->toBe('https://example.com', 'Collection lookup should find the domain');
|
||||
expect($domainExists)->not->toBeNull('Collection lookup should not return null');
|
||||
});
|
||||
|
||||
it('handles service names with dots correctly', function () {
|
||||
// Test service names with dots (e.g., 'my.service')
|
||||
|
||||
$parsed = parseServiceEnvironmentVariable('SERVICE_URL_my.service');
|
||||
$serviceName = $parsed['service_name'];
|
||||
$serviceName = str($serviceName)->replace('-', '_')->replace('.', '_')->value();
|
||||
|
||||
expect(is_string($serviceName))->toBeTrue();
|
||||
expect($serviceName)->toBe('my_service');
|
||||
|
||||
// Verify it matches transformed service name from docker-compose
|
||||
$serviceNameKey = 'my.service';
|
||||
$transformedServiceName = str($serviceNameKey)->replace('-', '_')->replace('.', '_')->value();
|
||||
|
||||
expect($transformedServiceName === $serviceName)->toBeTrue();
|
||||
});
|
||||
|
||||
it('handles service names with underscores correctly', function () {
|
||||
// Test service names that already have underscores
|
||||
|
||||
$parsed = parseServiceEnvironmentVariable('SERVICE_URL_my_service');
|
||||
$serviceName = $parsed['service_name'];
|
||||
$serviceName = str($serviceName)->replace('-', '_')->replace('.', '_')->value();
|
||||
|
||||
expect(is_string($serviceName))->toBeTrue();
|
||||
expect($serviceName)->toBe('my_service');
|
||||
});
|
||||
|
||||
it('handles mixed special characters in service names', function () {
|
||||
// Test service names with mix of dashes, dots, underscores
|
||||
|
||||
$parsed = parseServiceEnvironmentVariable('SERVICE_URL_my-app.service_v2');
|
||||
$serviceName = $parsed['service_name'];
|
||||
$serviceName = str($serviceName)->replace('-', '_')->replace('.', '_')->value();
|
||||
|
||||
expect(is_string($serviceName))->toBeTrue();
|
||||
expect($serviceName)->toBe('my_app_service_v2');
|
||||
|
||||
// Verify collection operations work
|
||||
$domains = collect([
|
||||
'my_app_service_v2' => ['domain' => 'https://test.com'],
|
||||
]);
|
||||
|
||||
$found = $domains->get($serviceName);
|
||||
expect($found)->not->toBeNull();
|
||||
expect($found['domain'])->toBe('https://test.com');
|
||||
});
|
||||
|
||||
it('ensures originalServiceName conversion works for FQDN generation', function () {
|
||||
// Test line 539: $originalServiceName conversion
|
||||
|
||||
$parsed = parseServiceEnvironmentVariable('SERVICE_URL_my_service');
|
||||
$serviceName = $parsed['service_name']; // 'my_service'
|
||||
|
||||
// Line 539: Convert underscores to dashes for FQDN generation
|
||||
$originalServiceName = str($serviceName)->replace('_', '-')->value();
|
||||
|
||||
expect(is_string($originalServiceName))->toBeTrue();
|
||||
expect($originalServiceName)->not->toBeInstanceOf(\Illuminate\Support\Stringable::class);
|
||||
expect($originalServiceName)->toBe('my-service');
|
||||
|
||||
// Verify it can be used in string interpolation (line 544)
|
||||
$uuid = 'test-uuid';
|
||||
$random = "$originalServiceName-$uuid";
|
||||
expect($random)->toBe('my-service-test-uuid');
|
||||
});
|
||||
|
||||
it('prevents duplicate domain entries in collection', function () {
|
||||
// This tests that using plain strings prevents duplicate entries
|
||||
// (one with Stringable key, one with string key)
|
||||
|
||||
$parsed = parseServiceEnvironmentVariable('SERVICE_URL_webapp');
|
||||
$serviceName = $parsed['service_name'];
|
||||
$serviceName = str($serviceName)->replace('-', '_')->replace('.', '_')->value();
|
||||
|
||||
$domains = collect();
|
||||
|
||||
// Add domain entry (line 621)
|
||||
$domains->put($serviceName, [
|
||||
'domain' => 'https://webapp.com',
|
||||
]);
|
||||
|
||||
// Try to lookup the domain (line 615)
|
||||
$found = $domains->get($serviceName);
|
||||
|
||||
expect($found)->not->toBeNull('Should find the domain we just added');
|
||||
expect($found['domain'])->toBe('https://webapp.com');
|
||||
|
||||
// Verify only one entry exists
|
||||
expect($domains->count())->toBe(1);
|
||||
expect($domains->has($serviceName))->toBeTrue();
|
||||
});
|
||||
|
||||
it('verifies parsers.php has the ->value() calls', function () {
|
||||
// Ensure the fix is actually in the code
|
||||
$parsersFile = file_get_contents(__DIR__.'/../../bootstrap/helpers/parsers.php');
|
||||
|
||||
// Line 539: Check originalServiceName conversion
|
||||
expect($parsersFile)->toContain("str(\$serviceName)->replace('_', '-')->value()");
|
||||
|
||||
// Line 541: Check serviceName normalization
|
||||
expect($parsersFile)->toContain("str(\$serviceName)->replace('-', '_')->replace('.', '_')->value()");
|
||||
});
|
||||
Loading…
Reference in New Issue
Block a user