From 75381af7422cd74c07af29b2cfdb2153baf0f887 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 24 Nov 2025 09:22:27 +0100 Subject: [PATCH] fix: convert Stringable to plain strings in applicationParser for strict comparisons and collection lookups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- bootstrap/helpers/parsers.php | 6 +- .../Unit/ApplicationParserStringableTest.php | 182 ++++++++++++++++++ 2 files changed, 185 insertions(+), 3 deletions(-) create mode 100644 tests/Unit/ApplicationParserStringableTest.php diff --git a/bootstrap/helpers/parsers.php b/bootstrap/helpers/parsers.php index 8286b643a..dfcc3e190 100644 --- a/bootstrap/helpers/parsers.php +++ b/bootstrap/helpers/parsers.php @@ -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(); diff --git a/tests/Unit/ApplicationParserStringableTest.php b/tests/Unit/ApplicationParserStringableTest.php new file mode 100644 index 000000000..dfb1870db --- /dev/null +++ b/tests/Unit/ApplicationParserStringableTest.php @@ -0,0 +1,182 @@ +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()"); +});