Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Symfony ^4.0|^5.0 + fix various deprecations + fix travis CI #271

Merged
merged 39 commits into from
Jan 14, 2020
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
f935b51
Rename all Twig_Environment to Twig 3.0 namespacing
Dec 6, 2019
6d93524
Update composer to require Symfony 4.4
Dec 6, 2019
939defb
Remove obsolete Use statement
Dec 6, 2019
3a62801
TreeBuilder requires a name
Dec 6, 2019
e7f398b
Alter node configuration to use new syntax
Dec 6, 2019
d526100
Add exception_controller option to get rid of Twig deprecation warning
Dec 6, 2019
6934557
Set composer fixed to 4.4.*
Dec 6, 2019
cddffec
Change composer json versioning
Dec 9, 2019
87b0d0d
Merge branch 'master' into 4.4
vanputten Dec 9, 2019
002ae95
Make composer compatible for symfony ^4.0 + add 4.4 and 5.0 to travis…
Dec 9, 2019
25c7d2f
Remove symfony 2 and 3 from travis
Dec 9, 2019
8ae0e3d
Remove 5.* PHP versions from travis
Dec 9, 2019
43ac603
Fix incompatible tearDown method
Dec 9, 2019
e145321
Restore support for symfony 4.1
Dec 9, 2019
9fe2bfd
Restore support for symfony 4.1
Dec 9, 2019
5f00c5e
Restore support for Symfony <4
Dec 11, 2019
8005a1e
Readd symfony 3 and 2 to travis.yml
Dec 11, 2019
eb27d83
Drop php <5.6 from travis.yml since travis no longer supports these
Dec 11, 2019
82d4c4f
Allow phpunit 6
Dec 11, 2019
2f76b4b
Drop symfony < 3.3 to allow 5.0 support (root_dir is deprecated in 5.…
Dec 11, 2019
b2b4782
Fix broken paths due to project_dir
Dec 11, 2019
df54a46
Add support for both legacy and 5.0 translator interface
Dec 11, 2019
77d87b9
Remove php 5.6 support since symfony 5.0 is incompatible with 5.6
Dec 11, 2019
846f51e
Revert "Remove php 5.6 support since symfony 5.0 is incompatible with…
Dec 11, 2019
75c239f
Drop php 5.6
Dec 11, 2019
ad11a99
Remove hard dependencies for phpunit
Dec 11, 2019
e6a7bbe
Allow php 7.1
Dec 11, 2019
aac2513
Remove file extension for phpunit file
Dec 11, 2019
fd975a6
Restore old phpunit style
Dec 11, 2019
a3338b6
Increase travis composer memory limit to prevent CI from failing to d…
Dec 11, 2019
a40d370
Restore versioning for phpunit and phpunit bridge
Dec 11, 2019
d795253
Alter Controller to return an actual 404 response instead of throwing…
Dec 11, 2019
1ec6eab
use NotFoundHttpException in special case for symfony < 4.4
monteiro Jan 14, 2020
8d7295a
fix composer typo
monteiro Jan 14, 2020
7faa3d5
remove deprecation message for symfony 5.1 related with framework.rou…
monteiro Jan 14, 2020
76a22bf
remove deprecation warnings for symfony 5.1
monteiro Jan 14, 2020
0cde017
fix
monteiro Jan 14, 2020
2ece4d8
twig namespace fix
monteiro Jan 14, 2020
d1fe77a
fix spacing
monteiro Jan 14, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
language: php

php:
- 5.4
- 5.5
- 5.6
- 7.1
- 7.2

Expand All @@ -14,6 +11,7 @@ cache:
- $HOME/.composer/cache/files

before_install:
- COMPOSER_MEMORY_LIMIT=-1
- composer self-update
- if [ "$STABILITY" = "dev" ]; then composer config minimum-stability dev; fi;
- if [ "$SYMFONY_VERSION" != "" ]; then composer require symfony/symfony:${SYMFONY_VERSION}; fi;
Expand All @@ -29,11 +27,11 @@ script:
matrix:
fast_finish: true
include:
- php: 5.6
env: SYMFONY_VERSION='~2.7.0'
- php: 5.6
env: SYMFONY_VERSION='~2.8.0'
- php: 5.6
env: SYMFONY_VERSION='~3.4.0'
- php: 7.2
env: SYMFONY_VERSION='^3.4.0'
- php: 7.2
env: SYMFONY_VERSION='^4.0'
- php: 7.2
env: SYMFONY_VERSION='^5.0'
- php: 7.2
env: STABILITY=dev
4 changes: 2 additions & 2 deletions Command/DumpCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ class DumpCommand extends Command
* @param TranslationDumper $dumper
* @param $kernelRootDir
*/
public function __construct(TranslationDumper $dumper, $kernelRootDir)
public function __construct(TranslationDumper $dumper, $projectDir)
{
$this->dumper = $dumper;
$this->kernelRootDir = $kernelRootDir;
$this->kernelRootDir = $projectDir . '/app';

parent::__construct();
}
Expand Down
40 changes: 25 additions & 15 deletions Controller/Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@
namespace Bazinga\Bundle\JsTranslationBundle\Controller;

use Bazinga\Bundle\JsTranslationBundle\Finder\TranslationFinder;
use Symfony\Component\Translation\TranslatorInterface;
use Symfony\Component\Translation\TranslatorInterface as LegacyTranslatorInterface;
use Symfony\Contracts\Translation\TranslatorInterface;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Config\ConfigCache;
use Symfony\Component\Config\Resource\FileResource;
use Symfony\Component\Filesystem\Exception\IOException;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
use Twig_Environment;
use Twig\Environment;

/**
* @author William DURAND <william.durand1@gmail.com>
Expand All @@ -23,7 +24,7 @@ class Controller
private $translator;

/**
* @var Twig_Environment
* @var Environment|Twig_Environment
*/
private $twig;

Expand Down Expand Up @@ -62,25 +63,34 @@ class Controller
private $httpCacheTime;

/**
* @param TranslatorInterface $translator The translator.
* @param Twig_Environment $twig The twig environment.
* @param TranslationFinder $translationFinder The translation finder.
* @param string $cacheDir
* @param boolean $debug
* @param string $localeFallback
* @param string $defaultDomain
* @param int $httpCacheTime
* @param TranslatorInterface $translator The translator.
* @param Environment|Twig_Environment $twig The twig environment.
* @param TranslationFinder $translationFinder The translation finder.
* @param string $cacheDir
* @param boolean $debug
* @param string $localeFallback
* @param string $defaultDomain
* @param int $httpCacheTime
* @throws InvalidArgumentException
*/
public function __construct(
TranslatorInterface $translator,
Twig_Environment $twig,
$translator,
$twig,
TranslationFinder $translationFinder,
$cacheDir,
$debug = false,
$localeFallback = '',
$defaultDomain = '',
$httpCacheTime = 86400
) {
if (!$twig instanceof \Twig_Environment && !$twig instanceof Environment) {
monteiro marked this conversation as resolved.
Show resolved Hide resolved
throw new \InvalidArgumentException(sprintf('Providing an instance of "%s" as twig is not supported.', get_class($twig)));
}

if (!$translator instanceof TranslatorInterface && !$translator instanceof LegacyTranslatorInterface) {
throw new \InvalidArgumentException(sprintf('Providing an instance of "%s" as translator is not supported.', get_class($translator)));
}

$this->translator = $translator;
$this->twig = $twig;
$this->translationFinder = $translationFinder;
Expand Down Expand Up @@ -109,7 +119,7 @@ public function getTranslationsAction(Request $request, $domain, $_format)
$locales = $this->getLocales($request);

if (0 === count($locales)) {
throw new NotFoundHttpException();
return (new Response())->setStatusCode(Response::HTTP_NOT_FOUND);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be reverted. What blocker do you have ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ControllerTest.php testGetTranslationsWithPathTraversalAttack and testGetTranslationsWithLocaleInjection expect a Response with status 404, but get a NotFoundException.

This works fine in Symfony ^4.0 and ^5.0, but in Symfony 3.4 this fails (probably because the 3.4 exception handler does not turn it into a 404 Response object, whereas the 4.0 and 5.0 handlers do).

I thought it made more sense to return an actual 404 response, since according to the ControllerTest that is what we expect to get anyway (line 184 and 201 of ControllerTest.php).

I would say either return an actual 404 response
OR change the ControllerTest to test for an uncaught NotFoundException instead of 404 response

If you want to reproduce the error:

  • Revert these changes
  • Symfony 3.4 on PHP 7.2 -> run tests

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now I think it is ok. Symfony 3.4 will be a pain to support always so when it gets deprecated on November 2020 we can change this piece of code.

}

$cache = new ConfigCache(sprintf('%s/%s.%s.%s',
Expand Down Expand Up @@ -160,7 +170,7 @@ public function getTranslationsAction(Request $request, $domain, $_format)
try {
$cache->write($content, $resources);
} catch (IOException $e) {
throw new NotFoundHttpException();
return (new Response())->setStatusCode(Response::HTTP_NOT_FOUND);
}
}

Expand Down
20 changes: 12 additions & 8 deletions Dumper/TranslationDumper.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use Bazinga\Bundle\JsTranslationBundle\Finder\TranslationFinder;
use Symfony\Component\Config\Loader\LoaderInterface;
use Symfony\Component\Filesystem\Filesystem;
use Twig_Environment;
use Twig\Environment;

/**
* @author Adrien Russo <adrien.russo.qc@gmail.com>
Expand All @@ -16,7 +16,7 @@ class TranslationDumper
const DEFAULT_TRANSLATION_PATTERN = '/translations/{domain}.{_format}';

/**
* @var \Twig_Environment
* @var Environment|Twig_Environment
*/
private $twig;

Expand Down Expand Up @@ -56,21 +56,25 @@ class TranslationDumper
private $defaultDomain;

/**
* @param Twig_Environment $twig The twig environment.
* @param TranslationFinder $finder The translation finder.
* @param FileSystem $filesystem The file system.
* @param string $localeFallback
* @param string $defaultDomain
* @param Environment|Twig_Environment $twig The twig environment.
* @param TranslationFinder $finder The translation finder.
* @param FileSystem $filesystem The file system.
* @param string $localeFallback
* @param string $defaultDomain
*/
public function __construct(
Twig_Environment $twig,
$twig,
TranslationFinder $finder,
Filesystem $filesystem,
$localeFallback = '',
$defaultDomain = '',
array $activeLocales = array(),
array $activeDomains = array()
) {
if (!$twig instanceof \Twig_Environment && !$twig instanceof Environment) {
monteiro marked this conversation as resolved.
Show resolved Hide resolved
throw new \InvalidArgumentException(sprintf('Providing an instance of "%s" as twig is not supported.', get_class($twig)));
}

$this->twig = $twig;
$this->finder = $finder;
$this->filesystem = $filesystem;
Expand Down
2 changes: 0 additions & 2 deletions Finder/TranslationFinder.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

namespace Bazinga\Bundle\JsTranslationBundle\Finder;

use Symfony\Component\Finder\Finder;

/**
* @author William DURAND <william.durand1@gmail.com>
* @author Markus Poerschke <markus@eluceo.de>
Expand Down
2 changes: 1 addition & 1 deletion Resources/config/services.xml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
</service>
<service id="bazinga.jstranslation.dump_command" class="Bazinga\Bundle\JsTranslationBundle\Command\DumpCommand" public="true">
<argument type="service" id="bazinga.jstranslation.translation_dumper" />
<argument>%kernel.root_dir%</argument>
<argument>%kernel.project_dir%</argument>
<tag name="console.command" command="bazinga:js-translation:dump" />
</service>
</services>
Expand Down
2 changes: 1 addition & 1 deletion Tests/Dumper/TranslationDumperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ public function setUp()
$this->filesystem->mkdir($this->target);
}

public function tearDown()
public function tearDown(): void
{
if (is_dir($this->target)) {
$this->filesystem->remove($this->target);
Expand Down
7 changes: 4 additions & 3 deletions Tests/Fixtures/app/config/base_config.yml
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
framework:
secret: test
router: { resource: "%kernel.root_dir%/config/routing.yml" }
router: { resource: "%kernel.project_dir%/app/config/routing.yml" }
test: ~
translator: { enabled: true }
validation: { enabled: false }

# Twig Configuration
twig:
debug: "%kernel.debug%"
strict_variables: "%kernel.debug%"
debug: "%kernel.debug%"
strict_variables: "%kernel.debug%"
exception_controller: null
monteiro marked this conversation as resolved.
Show resolved Hide resolved

# Traduction dans JS
bazinga_js_translation:
Expand Down
2 changes: 1 addition & 1 deletion Tests/Fixtures/app/config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ framework:
translator:
fallbacks: ['en']
paths:
- "%kernel.root_dir%/Infrastructure/Ui/Translations"
- "%kernel.project_dir%/app/Infrastructure/Ui/Translations"
25 changes: 13 additions & 12 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,21 @@
}
],
"require": {
"symfony/framework-bundle": "~2.7|~3.1|~4.0",
"symfony/finder": "~2.7|~3.1|~4.0",
"symfony/console": "~2.7|~3.1|~4.0",
"symfony/intl": "~2.7|~3.1|~4.0",
"symfony/translation": "~2.7|~3.1|~4.0",
"symfony/twig-bundle": "~2.7|~3.1|~4.0"
"php": "^7.1",
"symfony/framework-bundle": "~3.4|~4.0|~5.0",
"symfony/finder": "~3.4|~4.0|~5.0",
"symfony/console": "~3.4|~4.0|~5.0",
"symfony/intl": "~3.4|~4.0|~5.0",
"symfony/translation": "~3.4|~4.0|~5.0",
"symfony/twig-bundle": "~3.4|~4.0|~5.0"
},
"require-dev": {
"symfony/asset": "~2.7|~3.1|~4.0",
"symfony/yaml": "~2.7|~3.1|~4.0",
"symfony/browser-kit": "~2.7|~3.1|~4.0",
"symfony/twig-bundle": "~2.7|~3.1|~4.0",
"symfony/phpunit-bridge": "~2.7|~3.1|~4.0",
"phpunit/phpunit": "^4.8|~5.7"
"symfony/asset": "~3.4|~4.0|~5.0",
"symfony/yaml": "~3.4|~4.0|~5.0",
"symfony/browser-kit": "~3.4|~4.0|~5.0",
"symfony/twig-bundle": "~3.4|~4.0|~5.0",
"symfony/phpunit-bridge": "^3.4.24|^4.0|^5.0",
monteiro marked this conversation as resolved.
Show resolved Hide resolved
"phpunit/phpunit": "^4.8|~5.7|~6.5"
},
"replace": {
"willdurand/expose-translation-bundle": "2.5.*"
Expand Down