From 31303fa66690df0e203daf7f6e2dfb057aa31415 Mon Sep 17 00:00:00 2001 From: Jeffrey Wong Date: Fri, 24 Jan 2025 15:23:07 -0800 Subject: [PATCH 01/11] add support for symfony 6.0; update CI config --- .github/workflows/ci.yml | 29 +++++++++++++++++------------ composer.json | 2 +- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 514ac6a..cad8e64 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,22 +5,27 @@ on: jobs: test: - runs-on: ubuntu-22.04 + runs-on: ubuntu-22.04 strategy: fail-fast: false matrix: php: - - '7.4' +# - '7.4' - '8.0' - '8.1' - '8.2' - composer: - - '' - - '--prefer-lowest' - + - '8.3' + - '8.4' + deps: + - 'highest' + - 'lowest' + exclude: + - php: '8.4' + deps: 'lowest' + steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Use PHP uses: shivammathur/setup-php@v2 @@ -38,7 +43,7 @@ jobs: - name: cache dependencies id: cache-dependencies - uses: actions/cache@v3 + uses: actions/cache@v4 with: path: ${{ steps.composer-cache.outputs.dir }} key: ${{ runner.os }}-${{ matrix.php }}-${{ matrix.composer }}-composer-${{ hashFiles('**/composer.lock') }} @@ -50,10 +55,10 @@ jobs: working-directory: ./ - name: Install dependencies - env: - COMPOSER_FLAGS: ${{ matrix.composer }} - run: composer update ${COMPOSER_FLAGS} --prefer-source - working-directory: ./ + uses: ramsey/composer-install@v3 + with: + dependency-versions: '${{ matrix.deps }}' + working-directory: ./ - name: Run Tests run: composer run-script ci-test diff --git a/composer.json b/composer.json index 2a0bd59..07c72ba 100644 --- a/composer.json +++ b/composer.json @@ -9,7 +9,7 @@ "type": "symfony-bundle", "license": "MIT", "require": { - "symfony/framework-bundle": "^4.4|^5.0" + "symfony/framework-bundle": "^4.4|^5.0|^6.0" }, "require-dev": { From 1c6becbcf4f246ae4a194e4c97fbff4e4d115f0e Mon Sep 17 00:00:00 2001 From: Jeffrey Wong Date: Mon, 27 Jan 2025 09:24:50 -0800 Subject: [PATCH 02/11] run rector to clean up deprecations from symfony 5.x --- .../DayspringLoggingExtension.php | 6 ++--- Logger/SessionRequestProcessor.php | 12 +++++----- .../DependencyInjection/ConfigurationTest.php | 6 ++--- Tests/DependencyInjection/ExtensionTest.php | 10 ++++---- composer.json | 3 ++- rector.php | 24 +++++++++++++++++++ 6 files changed, 43 insertions(+), 18 deletions(-) create mode 100644 rector.php diff --git a/DependencyInjection/DayspringLoggingExtension.php b/DependencyInjection/DayspringLoggingExtension.php index 9fbde4d..8dcdd41 100644 --- a/DependencyInjection/DayspringLoggingExtension.php +++ b/DependencyInjection/DayspringLoggingExtension.php @@ -26,12 +26,12 @@ public function load(array $configs, ContainerBuilder $container) foreach ($config['session_request_processor_handlers'] as $handler) { $definition = new Definition(SessionRequestProcessor::class); - $definition->addTag('monolog.processor', array('handler' => $handler)); + $definition->addTag('monolog.processor', ['handler' => $handler]); $definition->setAutowired(true); - $container->addDefinitions(array( + $container->addDefinitions([ 'dayspring_logging.session_request_processor.'.$handler => $definition - )); + ]); } } } diff --git a/Logger/SessionRequestProcessor.php b/Logger/SessionRequestProcessor.php index 9988b4b..202a9b7 100644 --- a/Logger/SessionRequestProcessor.php +++ b/Logger/SessionRequestProcessor.php @@ -35,7 +35,7 @@ public function __construct(SessionInterface $session, RequestStack $requestStac protected function getServerVar($var) { - return isset($_SERVER[$var]) ? $_SERVER[$var] : null; + return $_SERVER[$var] ?? null; } public function setExtraField($key, $value) @@ -67,13 +67,13 @@ public function __invoke(array $record) } } $this->requestId = substr(uniqid(), -8); - $this->_server = array( + $this->_server = [ 'http.url' => ($this->getServerVar('HTTP_HOST')).'/'.($this->getServerVar('REQUEST_URI')), 'http.method' => $this->getServerVar('REQUEST_METHOD'), 'http.useragent' => $this->getServerVar('HTTP_USER_AGENT'), 'http.referer' => $this->getServerVar('HTTP_REFERER'), 'http.x_forwarded_for' => $this->getServerVar('HTTP_X_FORWARDED_FOR') - ); + ]; $this->_post = $this->clean($_POST); $this->_get = $this->clean($_GET); } @@ -97,7 +97,7 @@ public function __invoke(array $record) } else { $parameters = $this->matcher->match($request->getPathInfo()); } - $context['route'] = isset($parameters['_route']) ? $parameters['_route'] : 'n/a'; + $context['route'] = $parameters['_route'] ?? 'n/a'; $context['route_parameters'] = $parameters; } catch (Exception $e) { } @@ -122,14 +122,14 @@ protected function getMainRequest() if (method_exists(RequestStack::class, 'getMainRequest')) { return $this->requestStack->getMainRequest(); } else { - return $this->requestStack->getMasterRequest(); + return $this->requestStack->getMainRequest(); } } protected function clean($array) { - $toReturn = array(); + $toReturn = []; foreach (array_keys($array) as $key) { if (false !== strpos($key, 'password')) { // Do not add diff --git a/Tests/DependencyInjection/ConfigurationTest.php b/Tests/DependencyInjection/ConfigurationTest.php index 2ab3676..2fecd89 100644 --- a/Tests/DependencyInjection/ConfigurationTest.php +++ b/Tests/DependencyInjection/ConfigurationTest.php @@ -17,11 +17,11 @@ class ConfigurationTest extends TestCase public function testConfiguration() { - $config = array(); + $config = []; $processor = new Processor(); - $configuration = new Configuration(array()); - $config = $processor->processConfiguration($configuration, array($config)); + $configuration = new Configuration([]); + $config = $processor->processConfiguration($configuration, [$config]); $this->assertEquals([ 'session_request_processor_handlers' => [] diff --git a/Tests/DependencyInjection/ExtensionTest.php b/Tests/DependencyInjection/ExtensionTest.php index d6e91df..186b848 100644 --- a/Tests/DependencyInjection/ExtensionTest.php +++ b/Tests/DependencyInjection/ExtensionTest.php @@ -20,7 +20,7 @@ public function testLoadEmptyConfiguration() { $container = $this->createContainer(); $extension = new DayspringLoggingExtension(); - $extension->load(array(), $container); + $extension->load([], $container); $container->registerExtension($extension); $this->compileContainer($container); @@ -32,19 +32,19 @@ public function testLoadEmptyConfiguration() private function createContainer() { - $container = new ContainerBuilder(new ParameterBag(array( + $container = new ContainerBuilder(new ParameterBag([ 'kernel.cache_dir' => __DIR__, 'kernel.charset' => 'UTF-8', 'kernel.debug' => false, - ))); + ])); return $container; } private function compileContainer(ContainerBuilder $container) { - $container->getCompilerPassConfig()->setOptimizationPasses(array()); - $container->getCompilerPassConfig()->setRemovingPasses(array()); + $container->getCompilerPassConfig()->setOptimizationPasses([]); + $container->getCompilerPassConfig()->setRemovingPasses([]); $container->compile(); } } diff --git a/composer.json b/composer.json index 07c72ba..0b6f6dc 100644 --- a/composer.json +++ b/composer.json @@ -15,7 +15,8 @@ "require-dev": { "php": ">=7.4", "phpunit/phpunit": "~8.5.33|^9.0", - "monolog/monolog": "^1.22 || ^2.0" + "monolog/monolog": "^1.22 || ^2.0", + "rector/rector": "^2.0" }, "authors": [ { diff --git a/rector.php b/rector.php new file mode 100644 index 0000000..58205de --- /dev/null +++ b/rector.php @@ -0,0 +1,24 @@ +withSets([ + LevelSetList::UP_TO_PHP_74, + SymfonySetList::SYMFONY_50, + SymfonySetList::SYMFONY_51, + SymfonySetList::SYMFONY_52, + SymfonySetList::SYMFONY_53, + SymfonySetList::SYMFONY_54, + ]) + ->withSkip([ + __DIR__ . '/vendor', + __DIR__ . '/var/cache', + OptionalParametersAfterRequiredRector::class, // this re-orders parameters and requires updating calls to those functions. this would produce backward-incompatible changes + ]) + ->withPaths([__DIR__]); From a7e44c553e141c74cc0e6356d635903aeffad649 Mon Sep 17 00:00:00 2001 From: Jeffrey Wong Date: Mon, 27 Jan 2025 09:43:08 -0800 Subject: [PATCH 03/11] restore compatibility with 4.4, fix php8.0+ --- DependencyInjection/Configuration.php | 2 +- Logger/SessionRequestProcessor.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index a6eff6f..9e27373 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -15,7 +15,7 @@ class Configuration implements ConfigurationInterface /** * {@inheritdoc} */ - public function getConfigTreeBuilder() + public function getConfigTreeBuilder(): TreeBuilder { $treeBuilder = new TreeBuilder('dayspring_logging'); $rootNode = $treeBuilder->getRootNode(); diff --git a/Logger/SessionRequestProcessor.php b/Logger/SessionRequestProcessor.php index 202a9b7..489cd5e 100644 --- a/Logger/SessionRequestProcessor.php +++ b/Logger/SessionRequestProcessor.php @@ -122,7 +122,7 @@ protected function getMainRequest() if (method_exists(RequestStack::class, 'getMainRequest')) { return $this->requestStack->getMainRequest(); } else { - return $this->requestStack->getMainRequest(); + return $this->requestStack->getMasterRequest(); } } From 88cab41e2eb8bb5a52157787484ed37d6251de29 Mon Sep 17 00:00:00 2001 From: Jeffrey Wong Date: Tue, 11 Feb 2025 19:54:58 -0800 Subject: [PATCH 04/11] test specific symfony versions --- .github/workflows/ci.yml | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cad8e64..000f2ad 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,25 +5,38 @@ on: jobs: test: - runs-on: ubuntu-22.04 + strategy: fail-fast: false matrix: php: -# - '7.4' - '8.0' - '8.1' - '8.2' - '8.3' - '8.4' - deps: - - 'highest' - - 'lowest' + symfony_version: + - '5.4.*' + - '6.0.*' + - '6.1.*' + - '6.2.*' + - '6.3.*' + - '6.4.*' exclude: - - php: '8.4' - deps: 'lowest' - + # symfony 6.1+ requires PHP 8.1+ + - php: '8.0' + symfony_version: '6.1.*' + - php: '8.0' + symfony_version: '6.2.*' + - php: '8.0' + symfony_version: '6.3.*' + - php: '8.0' + symfony_version: '6.4.*' + # deps: + # - 'highest' + # - 'lowest' + steps: - uses: actions/checkout@v4 @@ -42,7 +55,7 @@ jobs: working-directory: ./ - name: cache dependencies - id: cache-dependencies + id: angular-dependencies uses: actions/cache@v4 with: path: ${{ steps.composer-cache.outputs.dir }} @@ -50,6 +63,11 @@ jobs: restore-keys: | ${{ runner.os }}-${{ matrix.php }}-${{ matrix.composer }}-composer- + - name: Set Symfony minor version + run: | + sed -i 's|\("symfony/.*"\): ".*"|\1: "${{ matrix.symfony_version }}"|' composer.json + working-directory: ./ + - name: Validate composer.json and composer.lock run: composer validate working-directory: ./ @@ -57,7 +75,7 @@ jobs: - name: Install dependencies uses: ramsey/composer-install@v3 with: - dependency-versions: '${{ matrix.deps }}' + # dependency-versions: '${{ matrix.deps }}' working-directory: ./ - name: Run Tests From 5f4d96de54dbb210047d587c5a7d0f831f7f467d Mon Sep 17 00:00:00 2001 From: Jeffrey Wong Date: Tue, 11 Feb 2025 20:02:56 -0800 Subject: [PATCH 05/11] remove SessionInterface arg --- .github/workflows/ci.yml | 1 + Logger/SessionRequestProcessor.php | 10 +++------- Tests/Logger/SessionRequestProcessorTest.php | 3 +-- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 000f2ad..0a0d33b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,6 +17,7 @@ jobs: - '8.3' - '8.4' symfony_version: + - '4.4.*' - '5.4.*' - '6.0.*' - '6.1.*' diff --git a/Logger/SessionRequestProcessor.php b/Logger/SessionRequestProcessor.php index 489cd5e..8bc73e2 100644 --- a/Logger/SessionRequestProcessor.php +++ b/Logger/SessionRequestProcessor.php @@ -4,15 +4,12 @@ use Exception; use Symfony\Component\HttpFoundation\RequestStack; -use Symfony\Component\HttpFoundation\Session\SessionInterface; use Symfony\Component\Routing\Matcher\RequestMatcherInterface; use Symfony\Component\Routing\Matcher\UrlMatcherInterface; class SessionRequestProcessor { - /** @var SessionInterface $session */ - private $session; /** @var RequestStack $requestStack */ private $requestStack; /** @var UrlMatcherInterface|RequestMatcherInterface $matcher */ @@ -26,9 +23,8 @@ class SessionRequestProcessor protected $extraFields = []; - public function __construct(SessionInterface $session, RequestStack $requestStack, UrlMatcherInterface $matcher) + public function __construct(RequestStack $requestStack, UrlMatcherInterface $matcher) { - $this->session = $session; $this->requestStack = $requestStack; $this->matcher = $matcher; } @@ -60,8 +56,8 @@ public function __invoke(array $record) $this->sessionId = getmypid(); } else { try { - $this->session->start(); - $this->sessionId = $this->session->getId(); + $this->requestStack->getSession()->start(); + $this->sessionId = $this->requestStack->getSession()->getId(); } catch (\RuntimeException $e) { $this->sessionId = '????????'; } diff --git a/Tests/Logger/SessionRequestProcessorTest.php b/Tests/Logger/SessionRequestProcessorTest.php index d0d715f..2b44b26 100644 --- a/Tests/Logger/SessionRequestProcessorTest.php +++ b/Tests/Logger/SessionRequestProcessorTest.php @@ -17,7 +17,6 @@ class SessionRequestProcessorTest extends TestCase { public function testProcessor() { - $session = new Session(); $requestStack = new RequestStack(); $requestStack->push(Request::create('/', 'GET')); $router = $this->createPartialMock(Router::class, ['matchRequest']); @@ -25,7 +24,7 @@ public function testProcessor() ->method('matchRequest') ->willReturn(['_route' => 'test']); - $processor = new SessionRequestProcessor($session, $requestStack, $router); + $processor = new SessionRequestProcessor($requestStack, $router); $handler = new TestHandler(); From 405766a9dedeb21d2b45c0a0dbc886dba592402c Mon Sep 17 00:00:00 2001 From: Jeffrey Wong Date: Wed, 12 Feb 2025 10:39:02 -0800 Subject: [PATCH 06/11] add container-based tests; limit monolog to <3.0 --- .gitignore | 2 + Tests/Logger/SessionRequestProcessorTest.php | 3 - Tests/Resources/config/config.yml | 35 ++++++ Tests/Resources/config/config_jenkins.yml | 6 + Tests/Resources/config/config_test.yml | 3 + Tests/Resources/config/routing.yml | 0 Tests/Service/LoggerTest.php | 111 +++++++++++++++++++ Tests/TestKernel.php | 49 ++++++++ Tests/autoload.php | 1 - composer.json | 9 +- 10 files changed, 211 insertions(+), 8 deletions(-) create mode 100644 Tests/Resources/config/config.yml create mode 100644 Tests/Resources/config/config_jenkins.yml create mode 100644 Tests/Resources/config/config_test.yml create mode 100644 Tests/Resources/config/routing.yml create mode 100644 Tests/Service/LoggerTest.php create mode 100644 Tests/TestKernel.php diff --git a/.gitignore b/.gitignore index 98b01ef..8ba9b08 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,5 @@ /build/ /composer.lock /.phpunit.result.cache + +var/cache/ diff --git a/Tests/Logger/SessionRequestProcessorTest.php b/Tests/Logger/SessionRequestProcessorTest.php index 2b44b26..636d9d7 100644 --- a/Tests/Logger/SessionRequestProcessorTest.php +++ b/Tests/Logger/SessionRequestProcessorTest.php @@ -8,10 +8,7 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RequestStack; -use Symfony\Component\HttpFoundation\Session\Session; -use Symfony\Component\HttpFoundation\Session\SessionFactory; use Symfony\Component\Routing\Router; -use function var_dump; class SessionRequestProcessorTest extends TestCase { diff --git a/Tests/Resources/config/config.yml b/Tests/Resources/config/config.yml new file mode 100644 index 0000000..afd3572 --- /dev/null +++ b/Tests/Resources/config/config.yml @@ -0,0 +1,35 @@ +imports: + +framework: + test: ~ + secret: xxxxxxxxxx + router: { resource: "%kernel.project_dir%/Resources/config/routing.yml" } + #serializer: { enable_annotations: true } + #templating: { engines: ['twig', 'php'] } + session: + storage_factory_id: session.storage.factory.mock_file + profiler: + collect: false + +services: + Monolog\Handler\TestHandler: + public: true + + test_logger: + alias: logger + public: true + + test_session_request_processor: + alias: dayspring_logging.session_request_processor.main + public: true + +monolog: + handlers: + main: + type: service + id: Monolog\Handler\TestHandler + level: debug + +dayspring_logging: + session_request_processor_handlers: + - main diff --git a/Tests/Resources/config/config_jenkins.yml b/Tests/Resources/config/config_jenkins.yml new file mode 100644 index 0000000..9afaa7f --- /dev/null +++ b/Tests/Resources/config/config_jenkins.yml @@ -0,0 +1,6 @@ +propel: + dbal: + driver: mysql + user: symfonydevuser + password: symfonydevpass + dsn: mysql:host=lydia.dayspring.office;dbname=securitybundle;charset=UTF8 diff --git a/Tests/Resources/config/config_test.yml b/Tests/Resources/config/config_test.yml new file mode 100644 index 0000000..9e928d8 --- /dev/null +++ b/Tests/Resources/config/config_test.yml @@ -0,0 +1,3 @@ +imports: + - { resource: config.yml } + diff --git a/Tests/Resources/config/routing.yml b/Tests/Resources/config/routing.yml new file mode 100644 index 0000000..e69de29 diff --git a/Tests/Service/LoggerTest.php b/Tests/Service/LoggerTest.php new file mode 100644 index 0000000..ed99f60 --- /dev/null +++ b/Tests/Service/LoggerTest.php @@ -0,0 +1,111 @@ +getContainer()->get('test_logger'); + + $logger->info('This is a test log message'); + + $testHandler = static::$kernel->getContainer()->get(TestHandler::class); + $records = $testHandler->getRecords(); + + $this->assertGreaterThan(0, count($records)); + + $record = array_pop($records); + + $this->assertArrayHasKey('message', $record); + $this->assertArrayHasKey('http.session_id', $record); + $this->assertArrayHasKey('http.request_id', $record); + } + + public function testExtraFields() + { + $logger = static::$kernel->getContainer()->get('test_logger'); + + /** @var SessionRequestProcessor $sessionRequestProcessor */ + $sessionRequestProcessor = static::$kernel->getContainer()->get('test_session_request_processor'); + $sessionRequestProcessor->setExtraField('test', 'value'); + + $logger->info('This is a test log message'); + + $testHandler = static::$kernel->getContainer()->get(TestHandler::class); + $records = $testHandler->getRecords(); + + $this->assertGreaterThan(0, count($records)); + + $record = array_pop($records); + $this->assertArrayHasKey('test', $record); + $this->assertEquals('value', $record['test']); + } + + public function testUnsetExtraFields() + { + $logger = static::$kernel->getContainer()->get('test_logger'); + + /** @var SessionRequestProcessor $sessionRequestProcessor */ + $sessionRequestProcessor = static::$kernel->getContainer()->get('test_session_request_processor'); + $sessionRequestProcessor->setExtraField('test', 'value'); + + $logger->info('This is a test log message'); + + $sessionRequestProcessor->unsetExtraField('test'); + + $logger->info('This is a test log message without extra field'); + + $testHandler = static::$kernel->getContainer()->get(TestHandler::class); + $records = $testHandler->getRecords(); + + $this->assertGreaterThan(0, count($records)); + + $record = array_pop($records); + $this->assertEquals('This is a test log message without extra field', $record['message']); + $this->assertArrayNotHasKey('test', $record); + } + + public function testClearExtraFields() + { + $logger = static::$kernel->getContainer()->get('test_logger'); + + /** @var SessionRequestProcessor $sessionRequestProcessor */ + $sessionRequestProcessor = static::$kernel->getContainer()->get('test_session_request_processor'); + $sessionRequestProcessor->setExtraField('test', 'value'); + + $logger->info('This is a test log message'); + + $sessionRequestProcessor->clearExtraFields(); + + $logger->info('This is a test log message without extra field'); + + $testHandler = static::$kernel->getContainer()->get(TestHandler::class); + $records = $testHandler->getRecords(); + + $this->assertGreaterThan(0, count($records)); + + $record = array_pop($records); + $this->assertEquals('This is a test log message without extra field', $record['message']); + $this->assertArrayNotHasKey('test', $record); + } + +} diff --git a/Tests/TestKernel.php b/Tests/TestKernel.php new file mode 100644 index 0000000..7faf4e6 --- /dev/null +++ b/Tests/TestKernel.php @@ -0,0 +1,49 @@ +getEnvironment(), ['dev', 'test'], true)) { + } + + return $bundles; + } + public function registerContainerConfiguration(LoaderInterface $loader) + { + $loader->load(__DIR__.'/Resources/config/config_test.yml'); + if (class_exists(\Symfony\Component\Asset\Package::class)) { + $loader->load(function (ContainerBuilder $container) { + $container->loadFromExtension('framework', ['assets' => []]); + }); + } + } +} diff --git a/Tests/autoload.php b/Tests/autoload.php index 8c22ff7..9692e32 100644 --- a/Tests/autoload.php +++ b/Tests/autoload.php @@ -1,6 +1,5 @@ =7.4", "phpunit/phpunit": "~8.5.33|^9.0", - "monolog/monolog": "^1.22 || ^2.0", - "rector/rector": "^2.0" + "rector/rector": "^2.0", + "symfony/yaml": "^6.4", + "symfony/monolog-bundle": "^3.10" }, "authors": [ { From 11fc7b7297bf1513c00ece92adf4abdea0f7eb72 Mon Sep 17 00:00:00 2001 From: Jeffrey Wong Date: Wed, 12 Feb 2025 10:51:07 -0800 Subject: [PATCH 07/11] update ci config --- .github/workflows/ci.yml | 2 +- composer.json | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0a0d33b..0238e13 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -66,7 +66,7 @@ jobs: - name: Set Symfony minor version run: | - sed -i 's|\("symfony/.*"\): ".*"|\1: "${{ matrix.symfony_version }}"|' composer.json + sed -i 's|\("symfony/.*"\): ".*^6\.0"|\1: "${{ matrix.symfony_version }}"|' composer.json working-directory: ./ - name: Validate composer.json and composer.lock diff --git a/composer.json b/composer.json index 0e00a07..b0daaef 100644 --- a/composer.json +++ b/composer.json @@ -16,8 +16,8 @@ "php": ">=7.4", "phpunit/phpunit": "~8.5.33|^9.0", "rector/rector": "^2.0", - "symfony/yaml": "^6.4", - "symfony/monolog-bundle": "^3.10" + "symfony/yaml": "^4.4|^5.0|^6.0", + "symfony/monolog-bundle": "^3.0" }, "authors": [ { From 4b1e6d690f165f471c33f1940a093cfd295860ca Mon Sep 17 00:00:00 2001 From: Jeffrey Wong Date: Wed, 12 Feb 2025 10:59:25 -0800 Subject: [PATCH 08/11] fix tests on 4.4 --- Tests/Resources/config/config.yml | 4 ++-- Tests/Service/LoggerTest.php | 1 - composer.json | 1 + 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Tests/Resources/config/config.yml b/Tests/Resources/config/config.yml index afd3572..c812292 100644 --- a/Tests/Resources/config/config.yml +++ b/Tests/Resources/config/config.yml @@ -6,8 +6,8 @@ framework: router: { resource: "%kernel.project_dir%/Resources/config/routing.yml" } #serializer: { enable_annotations: true } #templating: { engines: ['twig', 'php'] } - session: - storage_factory_id: session.storage.factory.mock_file +# session: +# storage_factory_id: session.storage.factory.mock_file profiler: collect: false diff --git a/Tests/Service/LoggerTest.php b/Tests/Service/LoggerTest.php index ed99f60..b7dccb1 100644 --- a/Tests/Service/LoggerTest.php +++ b/Tests/Service/LoggerTest.php @@ -6,7 +6,6 @@ use Dayspring\LoggingBundle\Tests\TestKernel; use Monolog\Handler\TestHandler; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; -use function array_unshift; class LoggerTest extends WebTestCase { diff --git a/composer.json b/composer.json index b0daaef..c1df7de 100644 --- a/composer.json +++ b/composer.json @@ -17,6 +17,7 @@ "phpunit/phpunit": "~8.5.33|^9.0", "rector/rector": "^2.0", "symfony/yaml": "^4.4|^5.0|^6.0", + "symfony/monolog-bridge": "^4.4|^5.0|^6.0", "symfony/monolog-bundle": "^3.0" }, "authors": [ From c5e13554cb3f816b8dca159c57e658cf560ad597 Mon Sep 17 00:00:00 2001 From: Jeffrey Wong Date: Fri, 21 Feb 2025 14:19:40 -0800 Subject: [PATCH 09/11] change detection of running as CLI --- Logger/SessionRequestProcessor.php | 4 +- Tests/Logger/SessionRequestProcessorTest.php | 62 +++++++++++++++++++- Tests/Service/LoggerTest.php | 9 +++ 3 files changed, 72 insertions(+), 3 deletions(-) diff --git a/Logger/SessionRequestProcessor.php b/Logger/SessionRequestProcessor.php index 8bc73e2..6c02b78 100644 --- a/Logger/SessionRequestProcessor.php +++ b/Logger/SessionRequestProcessor.php @@ -47,12 +47,12 @@ public function unsetExtraField($key) public function clearExtraFields() { $this->extraFields = []; - } + } public function __invoke(array $record) { if (null === $this->requestId) { - if ('cli' === php_sapi_name()) { + if (!array_key_exists('SERVER_NAME', $_SERVER)) { $this->sessionId = getmypid(); } else { try { diff --git a/Tests/Logger/SessionRequestProcessorTest.php b/Tests/Logger/SessionRequestProcessorTest.php index 636d9d7..fdc0d80 100644 --- a/Tests/Logger/SessionRequestProcessorTest.php +++ b/Tests/Logger/SessionRequestProcessorTest.php @@ -8,14 +8,26 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RequestStack; +use Symfony\Component\HttpFoundation\Session\Session; +use Symfony\Component\HttpFoundation\Session\Storage\MockFileSessionStorage; use Symfony\Component\Routing\Router; class SessionRequestProcessorTest extends TestCase { + protected function setUp(): void + { + parent::setUp(); + + $_SERVER['SERVER_NAME'] = 'test'; + } + public function testProcessor() { + $request = Request::create('/', 'GET'); + $request->setSession(new Session(new MockFileSessionStorage())); + $requestStack = new RequestStack(); - $requestStack->push(Request::create('/', 'GET')); + $requestStack->push($request); $router = $this->createPartialMock(Router::class, ['matchRequest']); $router->expects($this->once()) ->method('matchRequest') @@ -35,5 +47,53 @@ public function testProcessor() $record = $records[0]; $this->assertEquals('test', $record['context']['route']); $this->assertEquals('test', $record['context']['route_parameters']['_route']); + $this->assertArrayHasKey('http.session_id', $record); + } + + public function testProcessorCli() + { + unset($_SERVER['SERVER_NAME']); + + $requestStack = new RequestStack(); + $router = $this->createPartialMock(Router::class, ['matchRequest']); + + $processor = new SessionRequestProcessor($requestStack, $router); + + $handler = new TestHandler(); + + $logger = new Logger('test', [$handler], [$processor]); + + $logger->info('test'); + + $records = $handler->getRecords(); + + $this->assertCount(1, $records); + $record = $records[0]; + $this->assertArrayHasKey('http.session_id', $record); + $this->assertEquals($record['http.session_id'], getmypid()); + + } + + public function testProcessorNoRequest() + { + $this->markTestSkipped('This test is not working as expected'); + + $requestStack = new RequestStack(); + $router = $this->createPartialMock(Router::class, ['matchRequest']); + + $processor = new SessionRequestProcessor($requestStack, $router); + + $handler = new TestHandler(); + + $logger = new Logger('test', [$handler], [$processor]); + + $logger->info('test'); + + $records = $handler->getRecords(); + + $this->assertCount(1, $records); + $record = $records[0]; + $this->assertArrayNotHasKey('route', $record['context']); + $this->assertArrayNotHasKey('http.session_id', $record); } } diff --git a/Tests/Service/LoggerTest.php b/Tests/Service/LoggerTest.php index b7dccb1..e6d4985 100644 --- a/Tests/Service/LoggerTest.php +++ b/Tests/Service/LoggerTest.php @@ -6,6 +6,9 @@ use Dayspring\LoggingBundle\Tests\TestKernel; use Monolog\Handler\TestHandler; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Session\Session; +use Symfony\Component\HttpFoundation\Session\Storage\MockFileSessionStorage; class LoggerTest extends WebTestCase { @@ -14,6 +17,12 @@ protected function setUp(): void parent::setUp(); self::bootKernel(); + + $request = Request::create('/', 'GET'); + $request->setSession(new Session(new MockFileSessionStorage())); + + $requestStack = static::$kernel->getContainer()->get('request_stack'); + $requestStack->push($request); } protected static function getKernelClass(): string From e2e6ea7af72183ab0771c023c5a37df930d63c72 Mon Sep 17 00:00:00 2001 From: Jeffrey Wong Date: Fri, 21 Feb 2025 14:25:19 -0800 Subject: [PATCH 10/11] fix handling when request/session are not present --- Logger/SessionRequestProcessor.php | 21 +++++------ Tests/Logger/SessionRequestProcessorTest.php | 38 ++++++++++++++++++-- 2 files changed, 46 insertions(+), 13 deletions(-) diff --git a/Logger/SessionRequestProcessor.php b/Logger/SessionRequestProcessor.php index 6c02b78..a3eb2b6 100644 --- a/Logger/SessionRequestProcessor.php +++ b/Logger/SessionRequestProcessor.php @@ -52,16 +52,6 @@ public function clearExtraFields() public function __invoke(array $record) { if (null === $this->requestId) { - if (!array_key_exists('SERVER_NAME', $_SERVER)) { - $this->sessionId = getmypid(); - } else { - try { - $this->requestStack->getSession()->start(); - $this->sessionId = $this->requestStack->getSession()->getId(); - } catch (\RuntimeException $e) { - $this->sessionId = '????????'; - } - } $this->requestId = substr(uniqid(), -8); $this->_server = [ 'http.url' => ($this->getServerVar('HTTP_HOST')).'/'.($this->getServerVar('REQUEST_URI')), @@ -73,6 +63,17 @@ public function __invoke(array $record) $this->_post = $this->clean($_POST); $this->_get = $this->clean($_GET); } + if (null === $this->sessionId) { + if (!array_key_exists('SERVER_NAME', $_SERVER)) { + $this->sessionId = getmypid(); + } elseif ($this->requestStack->getMainRequest() && $this->requestStack->getMainRequest()->getSession()) { + try { + $this->sessionId = $this->requestStack->getSession()->getId(); + } catch (\RuntimeException $e) { + $this->sessionId = '????????'; + } + } + } $record['http.request_id'] = $this->requestId; $record['http.session_id'] = $this->sessionId; $record['http.url'] = $this->_server['http.url']; diff --git a/Tests/Logger/SessionRequestProcessorTest.php b/Tests/Logger/SessionRequestProcessorTest.php index fdc0d80..b3ce476 100644 --- a/Tests/Logger/SessionRequestProcessorTest.php +++ b/Tests/Logger/SessionRequestProcessorTest.php @@ -11,6 +11,7 @@ use Symfony\Component\HttpFoundation\Session\Session; use Symfony\Component\HttpFoundation\Session\Storage\MockFileSessionStorage; use Symfony\Component\Routing\Router; +use function var_dump; class SessionRequestProcessorTest extends TestCase { @@ -76,8 +77,6 @@ public function testProcessorCli() public function testProcessorNoRequest() { - $this->markTestSkipped('This test is not working as expected'); - $requestStack = new RequestStack(); $router = $this->createPartialMock(Router::class, ['matchRequest']); @@ -94,6 +93,39 @@ public function testProcessorNoRequest() $this->assertCount(1, $records); $record = $records[0]; $this->assertArrayNotHasKey('route', $record['context']); - $this->assertArrayNotHasKey('http.session_id', $record); + $this->assertNull($record['http.session_id']); + } + + + public function testProcessorRequestAddedLater() + { + $requestStack = new RequestStack(); + $router = $this->createPartialMock(Router::class, ['matchRequest']); + + $processor = new SessionRequestProcessor($requestStack, $router); + + $handler = new TestHandler(); + + $logger = new Logger('test', [$handler], [$processor]); + + $logger->info('test'); + + $request = Request::create('/', 'GET'); + $request->setSession(new Session(new MockFileSessionStorage())); + $requestStack->push($request); + + $logger->info('test with session'); + + $records = $handler->getRecords(); + + $this->assertCount(2, $records); + $record = $records[0]; + $this->assertArrayNotHasKey('route', $record['context']); + $this->assertNull($record['http.session_id']); + + $record2 = $records[1]; + $this->assertArrayHasKey('route', $record2['context']); + $this->assertNotNull($record2['http.session_id']); + $this->assertEquals($record['http.request_id'], $record2['http.request_id']); } } From 5d57c21c007a49b620705cd50917ef39e7cd614f Mon Sep 17 00:00:00 2001 From: Jeffrey Wong Date: Mon, 24 Feb 2025 21:02:00 -0800 Subject: [PATCH 11/11] use hasSession -- getSession will throw an exception --- Logger/SessionRequestProcessor.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Logger/SessionRequestProcessor.php b/Logger/SessionRequestProcessor.php index a3eb2b6..7f635ae 100644 --- a/Logger/SessionRequestProcessor.php +++ b/Logger/SessionRequestProcessor.php @@ -66,7 +66,7 @@ public function __invoke(array $record) if (null === $this->sessionId) { if (!array_key_exists('SERVER_NAME', $_SERVER)) { $this->sessionId = getmypid(); - } elseif ($this->requestStack->getMainRequest() && $this->requestStack->getMainRequest()->getSession()) { + } elseif ($this->requestStack->getMainRequest() && $this->requestStack->getMainRequest()->hasSession()) { try { $this->sessionId = $this->requestStack->getSession()->getId(); } catch (\RuntimeException $e) {