From c6ed429d8bb7485854d6c1ed3b2d4ff362917279 Mon Sep 17 00:00:00 2001 From: Yenfry Herrera Feliz Date: Wed, 17 Jan 2024 15:16:26 -0800 Subject: [PATCH 1/5] feat: add Symfony-7 support & deprecates old PHPvs We have done the following changes: - Add support for symfony 7 - Deprecates PHP versions below than 7.2.5 - Add support for phpunit 9 - Refactor code for making the current implementation compatible with the new versions that we are starting to support for the different used packages. --- .github/workflows/tests.yml | 85 ++++++++++++++++--- composer.json | 14 +-- src/DependencyInjection/AwsExtension.php | 25 +++--- src/DependencyInjection/Configuration.php | 10 +-- .../DependencyInjection/AwsExtensionTest.php | 40 +++++---- .../DependencyInjection/ConfigurationTest.php | 4 +- tests/fixtures/AppKernel.php | 6 +- tests/fixtures/TestService.php | 8 +- 8 files changed, 128 insertions(+), 64 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 671a5a8..3a5e730 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -7,32 +7,95 @@ on: pull_request: branches: [ master ] +permissions: + contents: read + jobs: + verify-no-models-changes: + runs-on: ubuntu-20.04 + name: Check for model changes + if: github.event_name == 'pull_request' + steps: + - name: Checkout codebase + uses: actions/checkout@v4 + with: + fetch-depth: 0 + - run: | + BASE_REPO="${{ github.event.pull_request.base.repo.clone_url }}" + git fetch $BASE_REPO master:master -q + CHANGED_FILES=$(git diff --name-only FETCH_HEAD...HEAD -- src/data/) + if [ ! -z "$CHANGED_FILES" ]; then + echo "Changes detected in the following models:" + echo "$CHANGED_FILES" + exit 1 + fi run: - runs-on: ubuntu-18.04 + runs-on: ubuntu-20.04 strategy: #for each of the following versions of PHP, with and without --prefer-lowest matrix: - php-versions: ['5.5', '5.6', '7.0', '7.1', '7.2', '7.3', '7.4'] - + php-versions: ['7.2.5', '7.2', '7.3', '7.4', '8.0', '8.1', '8.2'] + composer-options: ['', '--prefer-lowest'] #set the name for each job - name: PHP ${{ matrix.php-versions }} + name: PHP ${{ matrix.php-versions }} ${{ matrix.composer-options }} + #set up environment variables used by unit tests + env: + AWS_ACCESS_KEY_ID: foo + AWS_SECRET_ACCESS_KEY: bar + AWS_CSM_ENABLED: false + AWS_SUPPRESS_PHP_DEPRECATION_WARNING: true steps: #sets up the correct version of PHP with necessary config options - - name: Setup PHP + - name: Setup PHP with Xdebug uses: shivammathur/setup-php@v2 with: + coverage: xdebug php-version: ${{ matrix.php-versions }} - ini-values: memory_limit=4G, phar.readonly=false + ini-values: xdebug.overload_var_dump=0, memory_limit=4G, phar.readonly=false #checkout the codebase from github - name: Checkout codebase - uses: actions/checkout@v3 + uses: actions/checkout@v4 + + #validate composer files + - name: Validate composer.json and composer.lock + run: composer validate - #run composer - - name: Composer install - run: composer install + #get dependencies + - name: Install dependencies + run: composer update ${{ matrix.composer-options }} --no-interaction --prefer-source + + #php 8.x requirements + - if: ${{ matrix.php-versions >= '8.0' && matrix.composer-options != '' }} + name: PHP 8.x + run: composer require --dev phpunit/phpunit "^9.5" --no-interaction --prefer-source --with-all-dependencies + + #php 8.1+ requirements + - if: ${{ matrix.php-versions >= '8.1' && matrix.composer-options != '' }} + name: PHP 8.1+ + run: composer require --dev guzzlehttp/guzzle "^7.4.5" --no-interaction --prefer-source --with-all-dependencies #run tests - name: Run test suite - run: vendor/bin/phpunit \ No newline at end of file + run: make test + + #static analysis + - if: ${{ matrix.php-versions < '8.0' && matrix.composer-options == '' }} + name: Static analysis + run: | + composer require --dev nette/neon "^3.0" + composer require --dev phpstan/phpstan "0.12.45" + vendor/bin/phpstan analyse src + + #generate package + - if: ${{ matrix.composer-options == '' }} + name: Package generation + run: | + composer require guzzlehttp/psr7 "^1.9.1" -W + composer update + make package + + #generate code coverage + - if: ${{ (matrix.php-versions == '7.2.5' || matrix.php-versions == '8.0') && matrix.composer-options == '' }} + name: Code coverage + run: bash <(curl -s https://codecov.io/bash) diff --git a/composer.json b/composer.json index 9f9240d..c0403cb 100644 --- a/composer.json +++ b/composer.json @@ -12,16 +12,16 @@ } ], "require": { - "php": ">=5.5", + "php": ">=7.2.5", "aws/aws-sdk-php": "^3.2.6", - "symfony/config": "~2.3|~3.0|~4.0|~5.0|~6.0", - "symfony/dependency-injection": "~2.3|~3.0|~4.0|~5.0|~6.0", - "symfony/http-kernel": "~2.3|~3.0|~4.0|~5.0|~6.0" + "symfony/config": "~5.0|~6.0|~7.0", + "symfony/dependency-injection": "~5.0|~6.0|~7.0", + "symfony/http-kernel": "~5.0|~6.0|~7.0" }, "require-dev": { - "phpunit/phpunit": "^4.8.35|^5.6.3|^6.0.0", - "symfony/framework-bundle": "~2.3|~3.0|~4.0|~5.0|~6.0", - "symfony/yaml": "~2.3|~3.0|~4.0|~5.0|~6.0" + "phpunit/phpunit": "^8.5|^9.5", + "symfony/framework-bundle": "~5.0|~6.0|~7.0", + "symfony/yaml": "~5.0|~6.0|~7.0" }, "autoload": { "psr-4": { diff --git a/src/DependencyInjection/AwsExtension.php b/src/DependencyInjection/AwsExtension.php index c51ac76..b496eb5 100644 --- a/src/DependencyInjection/AwsExtension.php +++ b/src/DependencyInjection/AwsExtension.php @@ -15,7 +15,10 @@ class AwsExtension extends Extension { - public function load(array $configs, ContainerBuilder $container) + /** + * @throws \Exception + */ + public function load(array $configs, ContainerBuilder $container): void { $loader = new YamlFileLoader( $container, @@ -44,7 +47,7 @@ public function load(array $configs, ContainerBuilder $container) } - private function createServiceDefinition($name) + private function createServiceDefinition($name): Definition { $clientClass = "Aws\\{$name}\\{$name}Client"; $serviceDefinition = new Definition( @@ -53,21 +56,13 @@ class_exists($clientClass) ? $clientClass : AwsClient::class $serviceDefinition->setLazy(true); - // Handle Symfony >= 2.6 - if (method_exists($serviceDefinition, 'setFactory')) { - return $serviceDefinition->setFactory([ - new Reference('aws_sdk'), - 'createClient', - ])->setArguments([$name]); - } - - return $serviceDefinition - ->setFactoryService('aws_sdk') - ->setFactoryMethod('createClient') - ->setArguments([$name]); + return $serviceDefinition->setFactory([ + new Reference('aws_sdk'), + 'createClient', + ])->setArguments([$name]); } - private function inflateServicesInConfig(array &$config) + private function inflateServicesInConfig(array &$config): void { array_walk($config, function (&$value) { if (is_array($value)) { diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 6dc866f..2a13fbe 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -8,7 +8,7 @@ class Configuration implements ConfigurationInterface { - public function getConfigTreeBuilder() + public function getConfigTreeBuilder(): TreeBuilder { // Maintain backwars compatibility, only merge when AWS_MERGE_CONFIG is set $mergeConfig = $this->shouldMergeConfig(); @@ -70,8 +70,7 @@ public function getConfigTreeBuilder() ->variableNode('ua_append')->end() ->variableNode('validate')->end() ->scalarNode('version')->end() - ->end() - ; + ->end(); //Setup config trees for each of the services foreach (array_column(Aws\manifest(), 'namespace') as $awsService) { @@ -111,14 +110,13 @@ public function getConfigTreeBuilder() ->scalarNode('version')->end() ->end() ->end() - ->end() - ; + ->end(); } return $treeBuilder; } - protected function shouldMergeConfig() + protected function shouldMergeConfig(): ?string { # works with symfony/dotenv if (isset($_ENV['AWS_MERGE_CONFIG'])) { diff --git a/tests/DependencyInjection/AwsExtensionTest.php b/tests/DependencyInjection/AwsExtensionTest.php index 6c9fd1b..7db8c61 100644 --- a/tests/DependencyInjection/AwsExtensionTest.php +++ b/tests/DependencyInjection/AwsExtensionTest.php @@ -10,6 +10,7 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\ContainerInterface; +use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Reference; class AwsExtensionTest extends TestCase @@ -24,7 +25,7 @@ class AwsExtensionTest extends TestCase */ protected $container; - public function setUp() + public function setUp(): void { $this->kernel = new AppKernel('test', true); $this->kernel->boot(); @@ -38,9 +39,9 @@ public function setUp() public function sdk_config_should_be_passed_directly_to_the_constructor_and_resolved_by_the_sdk() { $config = $this->kernel->getTestConfig()['aws']; - $s3Region = isset($config['S3']['region']) ? $config['S3']['region'] : $config['region']; - $lambdaRegion = isset($config['Lambda']['region']) ? $config['Lambda']['region'] : $config['region']; - $codeDeployRegion = isset($config['CodeDeploy']['region']) ? $config['CodeDeploy']['region'] : $config['region']; + $s3Region = $config['S3']['region'] ?? $config['region']; + $lambdaRegion = $config['Lambda']['region'] ?? $config['region']; + $codeDeployRegion = $config['CodeDeploy']['region'] ?? $config['region']; $testService = $this->container->get('test_service'); @@ -76,13 +77,16 @@ public function extension_should_escape_strings_that_begin_with_at_sign() 'secret' => '@@secret' ]]; $container = $this->getMockBuilder(ContainerBuilder::class) - ->setMethods(['getDefinition', 'replaceArgument']) + ->onlyMethods(['getDefinition']) + ->getMock(); + $definition = $this->getMockBuilder(Definition::class) + ->onlyMethods(['replaceArgument']) ->getMock(); $container->expects($this->once()) ->method('getDefinition') ->with('aws_sdk') - ->willReturnSelf(); - $container->expects($this->once()) + ->willReturn($definition); + $definition->expects($this->once()) ->method('replaceArgument') ->with(0, $this->callback(function ($arg) { return is_array($arg) @@ -104,13 +108,16 @@ public function extension_should_expand_service_references() $extension = new AwsExtension; $config = ['credentials' => '@aws_sdk']; $container = $this->getMockBuilder(ContainerBuilder::class) - ->setMethods(['getDefinition', 'replaceArgument']) + ->onlyMethods(['getDefinition']) + ->getMock(); + $definition = $this->getMockBuilder(Definition::class) + ->onlyMethods(['replaceArgument']) ->getMock(); $container->expects($this->once()) ->method('getDefinition') ->with('aws_sdk') - ->willReturnSelf(); - $container->expects($this->once()) + ->willReturn($definition); + $definition->expects($this->once()) ->method('replaceArgument') ->with(0, $this->callback(function ($arg) { return is_array($arg) @@ -137,7 +144,6 @@ public function extension_should_validate_and_merge_configs() 'stats' => [ 'http' => true ], - 'retries' => 5, 'endpoint' => 'http://localhost:8000', 'endpoint_discovery' => [ 'enabled' => true, @@ -181,13 +187,16 @@ public function extension_should_validate_and_merge_configs() 'validate' => true, ]; $container = $this->getMockBuilder(ContainerBuilder::class) - ->setMethods(['getDefinition', 'replaceArgument']) + ->onlyMethods(['getDefinition']) + ->getMock(); + $definition = $this->getMockBuilder(Definition::class) + ->onlyMethods(['replaceArgument']) ->getMock(); $container->expects($this->once()) ->method('getDefinition') ->with('aws_sdk') - ->willReturnSelf(); - $container->expects($this->once()) + ->willReturn($definition); + $definition->expects($this->once()) ->method('replaceArgument') ->with(0, $this->callback(function ($arg) { return is_array($arg) @@ -230,9 +239,8 @@ public function extension_should_error_merging_unknown_config_options() 'foo' => 'baz' ]; $container = $this->getMockBuilder(ContainerBuilder::class) - ->setMethods(['getDefinition', 'replaceArgument']) + ->onlyMethods(['getDefinition']) ->getMock(); - try { $extension->load([$config, $configDev], $container); $this->fail('Should have thrown an Error or RuntimeException'); diff --git a/tests/DependencyInjection/ConfigurationTest.php b/tests/DependencyInjection/ConfigurationTest.php index 49e2454..ef66fdc 100644 --- a/tests/DependencyInjection/ConfigurationTest.php +++ b/tests/DependencyInjection/ConfigurationTest.php @@ -10,7 +10,7 @@ class ConfigurationTest extends TestCase { - public function setUp() + public function setUp(): void { (new Filesystem) ->remove(implode(DIRECTORY_SEPARATOR, [ @@ -39,7 +39,7 @@ public function container_should_compile_and_load($format) $this->assertNotInstanceOf(DynamoDbClient::class, $testService->getCodeDeployClient()); } - public function formatProvider() + public function formatProvider(): array { return [ ['yml'], diff --git a/tests/fixtures/AppKernel.php b/tests/fixtures/AppKernel.php index 733d86d..b3f79f6 100644 --- a/tests/fixtures/AppKernel.php +++ b/tests/fixtures/AppKernel.php @@ -16,7 +16,7 @@ public function __construct($env, $debug, $extension = 'yml') parent::__construct($env, $debug); } - public function registerBundles() + public function registerBundles(): iterable { return [ new FrameworkBundle(), @@ -24,7 +24,7 @@ public function registerBundles() ]; } - public function registerContainerConfiguration(LoaderInterface $loader) + public function registerContainerConfiguration(LoaderInterface $loader): void { $loader->load($this->getTestConfigFile($this->extension)); } @@ -35,7 +35,7 @@ public function getTestConfig() } - private function getTestConfigFile($extension) + private function getTestConfigFile($extension): string { return __DIR__ . '/config.' . $extension; } diff --git a/tests/fixtures/TestService.php b/tests/fixtures/TestService.php index 8f48792..2a7868b 100644 --- a/tests/fixtures/TestService.php +++ b/tests/fixtures/TestService.php @@ -35,7 +35,7 @@ public function __construct( /** * @return S3Client */ - public function getS3Client() + public function getS3Client(): S3Client { return $this->s3Client; } @@ -43,7 +43,7 @@ public function getS3Client() /** * @return LambdaClient */ - public function getLambdaClient() + public function getLambdaClient(): LambdaClient { return $this->lambdaClient; } @@ -51,7 +51,7 @@ public function getLambdaClient() /** * @return CodeDeployClient */ - public function getCodeDeployClient() + public function getCodeDeployClient(): CodeDeployClient { return $this->codeDeployClient; } @@ -59,7 +59,7 @@ public function getCodeDeployClient() /** * @return array */ - public function getClients() + public function getClients(): array { return [ $this->s3Client, From 8c1274b3f23ca09910c2508e92389fa310cde0e3 Mon Sep 17 00:00:00 2001 From: Sean O'Brien Date: Tue, 30 Jan 2024 14:41:07 -0500 Subject: [PATCH 2/5] remove unused job, update sdk dependency --- .github/workflows/tests.yml | 18 ------------------ composer.json | 2 +- 2 files changed, 1 insertion(+), 19 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 3a5e730..29fde26 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -11,24 +11,6 @@ permissions: contents: read jobs: - verify-no-models-changes: - runs-on: ubuntu-20.04 - name: Check for model changes - if: github.event_name == 'pull_request' - steps: - - name: Checkout codebase - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - run: | - BASE_REPO="${{ github.event.pull_request.base.repo.clone_url }}" - git fetch $BASE_REPO master:master -q - CHANGED_FILES=$(git diff --name-only FETCH_HEAD...HEAD -- src/data/) - if [ ! -z "$CHANGED_FILES" ]; then - echo "Changes detected in the following models:" - echo "$CHANGED_FILES" - exit 1 - fi run: runs-on: ubuntu-20.04 strategy: diff --git a/composer.json b/composer.json index c0403cb..2c37c14 100644 --- a/composer.json +++ b/composer.json @@ -13,7 +13,7 @@ ], "require": { "php": ">=7.2.5", - "aws/aws-sdk-php": "^3.2.6", + "aws/aws-sdk-php": "^3.279.0", "symfony/config": "~5.0|~6.0|~7.0", "symfony/dependency-injection": "~5.0|~6.0|~7.0", "symfony/http-kernel": "~5.0|~6.0|~7.0" From 48492a4f9d5b3e32c8cb4f65b0820c9600c9a2e7 Mon Sep 17 00:00:00 2001 From: Sean O'Brien Date: Tue, 30 Jan 2024 14:47:59 -0500 Subject: [PATCH 3/5] remove 7.2 from CI --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 29fde26..29d7f78 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -16,7 +16,7 @@ jobs: strategy: #for each of the following versions of PHP, with and without --prefer-lowest matrix: - php-versions: ['7.2.5', '7.2', '7.3', '7.4', '8.0', '8.1', '8.2'] + php-versions: ['7.2.5', '7.3', '7.4', '8.0', '8.1', '8.2'] composer-options: ['', '--prefer-lowest'] #set the name for each job name: PHP ${{ matrix.php-versions }} ${{ matrix.composer-options }} From 1dac44f3ebd330e8e0e3a2240460691ba819cf52 Mon Sep 17 00:00:00 2001 From: Sean O'Brien Date: Tue, 30 Jan 2024 14:52:56 -0500 Subject: [PATCH 4/5] remove unnecessary steps --- .github/workflows/tests.yml | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 29d7f78..0030e07 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -61,23 +61,3 @@ jobs: - name: Run test suite run: make test - #static analysis - - if: ${{ matrix.php-versions < '8.0' && matrix.composer-options == '' }} - name: Static analysis - run: | - composer require --dev nette/neon "^3.0" - composer require --dev phpstan/phpstan "0.12.45" - vendor/bin/phpstan analyse src - - #generate package - - if: ${{ matrix.composer-options == '' }} - name: Package generation - run: | - composer require guzzlehttp/psr7 "^1.9.1" -W - composer update - make package - - #generate code coverage - - if: ${{ (matrix.php-versions == '7.2.5' || matrix.php-versions == '8.0') && matrix.composer-options == '' }} - name: Code coverage - run: bash <(curl -s https://codecov.io/bash) From 8df8065001da2f99a922a06ec1e9ce5a16d45c85 Mon Sep 17 00:00:00 2001 From: Sean O'Brien Date: Tue, 30 Jan 2024 15:04:47 -0500 Subject: [PATCH 5/5] add 8.3 support --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 0030e07..c58fede 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -16,7 +16,7 @@ jobs: strategy: #for each of the following versions of PHP, with and without --prefer-lowest matrix: - php-versions: ['7.2.5', '7.3', '7.4', '8.0', '8.1', '8.2'] + php-versions: ['7.2.5', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3'] composer-options: ['', '--prefer-lowest'] #set the name for each job name: PHP ${{ matrix.php-versions }} ${{ matrix.composer-options }}