diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 514ac6a..0238e13 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,16 +11,35 @@ jobs: fail-fast: false matrix: php: - - '7.4' - '8.0' - '8.1' - '8.2' - composer: - - '' - - '--prefer-lowest' + - '8.3' + - '8.4' + symfony_version: + - '4.4.*' + - '5.4.*' + - '6.0.*' + - '6.1.*' + - '6.2.*' + - '6.3.*' + - '6.4.*' + exclude: + # 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@v3 + - uses: actions/checkout@v4 - name: Use PHP uses: shivammathur/setup-php@v2 @@ -37,23 +56,28 @@ jobs: working-directory: ./ - name: cache dependencies - id: cache-dependencies - uses: actions/cache@v3 + id: angular-dependencies + uses: actions/cache@v4 with: path: ${{ steps.composer-cache.outputs.dir }} key: ${{ runner.os }}-${{ matrix.php }}-${{ matrix.composer }}-composer-${{ hashFiles('**/composer.lock') }} restore-keys: | ${{ runner.os }}-${{ matrix.php }}-${{ matrix.composer }}-composer- + - name: Set Symfony minor version + run: | + sed -i 's|\("symfony/.*"\): ".*^6\.0"|\1: "${{ matrix.symfony_version }}"|' composer.json + working-directory: ./ + - name: Validate composer.json and composer.lock run: composer validate 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/.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/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/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..7f635ae 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,16 +23,15 @@ 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; } protected function getServerVar($var) { - return isset($_SERVER[$var]) ? $_SERVER[$var] : null; + return $_SERVER[$var] ?? null; } public function setExtraField($key, $value) @@ -51,32 +47,33 @@ public function unsetExtraField($key) public function clearExtraFields() { $this->extraFields = []; - } + } public function __invoke(array $record) { if (null === $this->requestId) { - if ('cli' === php_sapi_name()) { - $this->sessionId = getmypid(); - } else { - try { - $this->session->start(); - $this->sessionId = $this->session->getId(); - } catch (\RuntimeException $e) { - $this->sessionId = '????????'; - } - } $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); } + if (null === $this->sessionId) { + if (!array_key_exists('SERVER_NAME', $_SERVER)) { + $this->sessionId = getmypid(); + } elseif ($this->requestStack->getMainRequest() && $this->requestStack->getMainRequest()->hasSession()) { + 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']; @@ -97,7 +94,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) { } @@ -129,7 +126,7 @@ protected function 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/Tests/Logger/SessionRequestProcessorTest.php b/Tests/Logger/SessionRequestProcessorTest.php index d0d715f..b3ce476 100644 --- a/Tests/Logger/SessionRequestProcessorTest.php +++ b/Tests/Logger/SessionRequestProcessorTest.php @@ -9,23 +9,32 @@ 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\HttpFoundation\Session\Storage\MockFileSessionStorage; use Symfony\Component\Routing\Router; use function var_dump; class SessionRequestProcessorTest extends TestCase { + protected function setUp(): void + { + parent::setUp(); + + $_SERVER['SERVER_NAME'] = 'test'; + } + public function testProcessor() { - $session = new Session(); + $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') ->willReturn(['_route' => 'test']); - $processor = new SessionRequestProcessor($session, $requestStack, $router); + $processor = new SessionRequestProcessor($requestStack, $router); $handler = new TestHandler(); @@ -39,5 +48,84 @@ 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() + { + $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->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']); } } diff --git a/Tests/Resources/config/config.yml b/Tests/Resources/config/config.yml new file mode 100644 index 0000000..c812292 --- /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..e6d4985 --- /dev/null +++ b/Tests/Service/LoggerTest.php @@ -0,0 +1,119 @@ +setSession(new Session(new MockFileSessionStorage())); + + $requestStack = static::$kernel->getContainer()->get('request_stack'); + $requestStack->push($request); + } + + protected static function getKernelClass(): string + { + return TestKernel::class; + } + + public function testLogger() + { + $logger = static::$kernel->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", + "symfony/yaml": "^4.4|^5.0|^6.0", + "symfony/monolog-bridge": "^4.4|^5.0|^6.0", + "symfony/monolog-bundle": "^3.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__]);