From b56f0e7a1e7bd925c9f8ee6a293965b5cffa3768 Mon Sep 17 00:00:00 2001 From: Reuben RJ Date: Tue, 25 Aug 2020 19:05:00 +1000 Subject: [PATCH 1/3] added support for packages which register language files via loadTransanlationsFrom with override ability from local language vendor folder --- .../Generators/LangJsGenerator.php | 91 ++++++++++++------- .../LaravelJsLocalizationServiceProvider.php | 6 +- .../packages/nonameinc/lang/en/messages.php | 8 ++ tests/fixtures/resources/lang/en/messages.php | 15 +++ .../lang/vendor/nonameinc/en/messages.php | 6 ++ tests/specs/LangJsCommandTest.php | 33 ++++++- tests/specs/LangJsServiceProviderTest.php | 76 +++++++++++++++- tests/specs/NonameIncProvider.php | 15 +++ 8 files changed, 212 insertions(+), 38 deletions(-) create mode 100644 tests/fixtures/packages/nonameinc/lang/en/messages.php create mode 100644 tests/fixtures/resources/lang/en/messages.php create mode 100644 tests/fixtures/resources/lang/vendor/nonameinc/en/messages.php create mode 100644 tests/specs/NonameIncProvider.php diff --git a/src/Mariuzzo/LaravelJsLocalization/Generators/LangJsGenerator.php b/src/Mariuzzo/LaravelJsLocalization/Generators/LangJsGenerator.php index d756989..a460565 100644 --- a/src/Mariuzzo/LaravelJsLocalization/Generators/LangJsGenerator.php +++ b/src/Mariuzzo/LaravelJsLocalization/Generators/LangJsGenerator.php @@ -6,6 +6,7 @@ use Illuminate\Filesystem\Filesystem as File; use Illuminate\Support\Str; use JShrink\Minifier; +use Illuminate\Support\Arr; /** * The LangJsGenerator class. @@ -35,6 +36,12 @@ class LangJsGenerator */ protected $messagesIncluded = []; + /** + * List of namespace index of packages language folder + * @var array + */ + protected $packagesMessages = []; + /** * Name of the domain in which all string-translation should be stored under. * More about string-translation: https://laravel.com/docs/master/localization#retrieving-translation-strings @@ -46,14 +53,17 @@ class LangJsGenerator /** * Construct a new LangJsGenerator instance. * - * @param File $file The file service instance. + * @param File $file The file service instance. * @param string $sourcePath The source path of the language files. + * @param array $messagesIncluded + * @param array $packagesMessages */ - public function __construct(File $file, $sourcePath, $messagesIncluded = []) + public function __construct(File $file, $sourcePath, $messagesIncluded = [], $packagesMessages = []) { $this->file = $file; $this->sourcePath = $sourcePath; $this->messagesIncluded = $messagesIncluded; + $this->packagesMessages = $packagesMessages; } /** @@ -125,37 +135,17 @@ protected function getMessages($noSort) throw new \Exception("${path} doesn't exists!"); } - foreach ($this->file->allFiles($path) as $file) { - $pathName = $file->getRelativePathName(); - $extension = $this->file->extension($pathName); - if ($extension != 'php' && $extension != 'json') { - continue; - } - - if ($this->isMessagesExcluded($pathName)) { - continue; - } - - $key = substr($pathName, 0, -4); - $key = str_replace('\\', '.', $key); - $key = str_replace('/', '.', $key); - - if (Str::startsWith($key, 'vendor')) { - $key = $this->getVendorKey($key); + foreach($this->packagesMessages as $namespace => $langPath) + { + foreach ($this->file->allFiles($langPath) as $file) + { + $this->processFile($messages, $file, $namespace); } + } - $fullPath = $path.DIRECTORY_SEPARATOR.$pathName; - if ($extension == 'php') { - $messages[$key] = include $fullPath; - } else { - $key = $key.$this->stringsDomain; - $fileContent = file_get_contents($fullPath); - $messages[$key] = json_decode($fileContent, true); - - if (json_last_error() !== JSON_ERROR_NONE) { - throw new InvalidArgumentException('Error while decode ' . basename($fullPath) . ': ' . json_last_error_msg()); - } - } + foreach ($this->file->allFiles($path) as $file) + { + $this->processFile($messages, $file); } if (!$noSort) @@ -214,4 +204,43 @@ private function getVendorKey($key) return $keyParts[2] .'.'. $keyParts[1] . '::' . $keyParts[3]; } + + private function processFile(&$messages, $file, $namespace = null) + { + $pathName = $file->getRelativePathName(); + $extension = $file->getExtension(); + if ($extension != 'php' && $extension != 'json') { + return; + } + + if ($this->isMessagesExcluded($pathName)) { + return; + } + + $key = substr($pathName, 0, -4); + $key = str_replace('\\', '.', $key); + $key = str_replace('/', '.', $key); + + if ($namespace !== null) + { + $key = 'vendor.' . $namespace . '.'. $key; + } + + if (Str::startsWith($key, 'vendor')) { + $key = $this->getVendorKey($key); + } + + $fullPath = $file->getRealPath(); + if ($extension == 'php') { + $messages[$key] = array_merge(Arr::get($messages,$key,[]), include $fullPath); + } else { + $key = $key.$this->stringsDomain; + $fileContent = file_get_contents($fullPath); + $messages[$key] = json_decode($fileContent, true); + + if (json_last_error() !== JSON_ERROR_NONE) { + throw new InvalidArgumentException('Error while decode ' . basename($fullPath) . ': ' . json_last_error_msg()); + } + } + } } diff --git a/src/Mariuzzo/LaravelJsLocalization/LaravelJsLocalizationServiceProvider.php b/src/Mariuzzo/LaravelJsLocalization/LaravelJsLocalizationServiceProvider.php index eab4548..0661edf 100644 --- a/src/Mariuzzo/LaravelJsLocalization/LaravelJsLocalizationServiceProvider.php +++ b/src/Mariuzzo/LaravelJsLocalization/LaravelJsLocalizationServiceProvider.php @@ -68,6 +68,7 @@ public function register() $app = $this->app; $laravelMajorVersion = (int) $app::VERSION; + $translator = $app->make('Illuminate\Translation\Translator'); $files = $app['files']; if ($laravelMajorVersion === 4) { @@ -75,8 +76,9 @@ public function register() } elseif ($laravelMajorVersion >= 5) { $langs = $app['path.base'].'/resources/lang'; } - $messages = $app['config']->get('localization-js.messages'); - $generator = new Generators\LangJsGenerator($files, $langs, $messages); + $messages = $app['config']->get('localization-js.messages', []); + $packagesLanguages = $translator->getLoader()->namespaces(); + $generator = new Generators\LangJsGenerator($files, $langs, $messages, $packagesLanguages); return new Commands\LangJsCommand($generator); }); diff --git a/tests/fixtures/packages/nonameinc/lang/en/messages.php b/tests/fixtures/packages/nonameinc/lang/en/messages.php new file mode 100644 index 0000000..b73f02e --- /dev/null +++ b/tests/fixtures/packages/nonameinc/lang/en/messages.php @@ -0,0 +1,8 @@ + 'package important data should be overridden with lang/vendor/nonameinc/en/ declarations', + 'another_important_data' => 'this is from the package lang', + +); diff --git a/tests/fixtures/resources/lang/en/messages.php b/tests/fixtures/resources/lang/en/messages.php new file mode 100644 index 0000000..75eb85b --- /dev/null +++ b/tests/fixtures/resources/lang/en/messages.php @@ -0,0 +1,15 @@ + 'Home', + 'family' => array( + 'father' => 'Dad', + 'mother' => 'Mom', + 'children' => array( + 'son' => 'Son', + ), + ), + 'plural' => 'one apple|a million apples', + 'random' => 'gm8ft2hrrlq1u6m54we9udi', +); diff --git a/tests/fixtures/resources/lang/vendor/nonameinc/en/messages.php b/tests/fixtures/resources/lang/vendor/nonameinc/en/messages.php new file mode 100644 index 0000000..a6e81e2 --- /dev/null +++ b/tests/fixtures/resources/lang/vendor/nonameinc/en/messages.php @@ -0,0 +1,6 @@ + 'should have replaced packages value for this key', + 'new_key' => 'this is a new key added ontop of the packages', +); diff --git a/tests/specs/LangJsCommandTest.php b/tests/specs/LangJsCommandTest.php index 4465677..a102f28 100644 --- a/tests/specs/LangJsCommandTest.php +++ b/tests/specs/LangJsCommandTest.php @@ -163,6 +163,37 @@ public function testAllFilesShouldBeConverted() $this->cleanupOutputDirectory(); } + /** + */ + public function testPackageFilesShouldBeConverted() + { + $langPath = "{$this->testPath}/fixtures/resources/lang"; + $packageFiles = ["nonameinc" => "{$this->testPath}/fixtures/packages/nonameinc/lang"]; + + $generator = new LangJsGenerator(new File(), $langPath, [], $packageFiles); + + $command = new LangJsCommand($generator); + $command->setLaravel($this->app); + + $code = $this->runCommand($command, ['target' => $this->outputFilePath]); + $this->assertRunsWithSuccess($code); + $this->assertFileExists($this->outputFilePath); + + $contents = file_get_contents($this->outputFilePath); + + $this->_assertStringContainsString('gm8ft2hrrlq1u6m54we9udi', $contents); + + $this->_assertStringNotContainsString('vendor.nonameinc.en.messages', $contents); + + $this->_assertStringContainsString('en.nonameinc::messages', $contents); + + $this->_assertStringContainsString('should have replaced packages value for this key', $contents); + $this->_assertStringContainsString('this is a new key added ontop of the packages', $contents); + $this->_assertStringContainsString('another_important_data', $contents); + + $this->cleanupOutputDirectory(); + } + /** */ public function testFilesSelectedInConfigShouldBeConverted() @@ -240,7 +271,7 @@ public function testShouldIgnoreDefaultOutputPathFromConfigIfTargetArgumentExist $code = $this->runCommand($command, ['target' => $this->outputFilePath]); $this->assertRunsWithSuccess($code); $this->assertFileExists($this->outputFilePath); - $this->assertFileNotExists($customOutputFilePath); + $this->assertFileDoesNotExist($customOutputFilePath); $template = "$this->rootPath/src/Mariuzzo/LaravelJsLocalization/Generators/Templates/langjs_with_messages.js"; $this->assertFileExists($template); diff --git a/tests/specs/LangJsServiceProviderTest.php b/tests/specs/LangJsServiceProviderTest.php index 4da11dd..83e50df 100644 --- a/tests/specs/LangJsServiceProviderTest.php +++ b/tests/specs/LangJsServiceProviderTest.php @@ -2,6 +2,7 @@ namespace Mariuzzo\LaravelJsLocalization; +use Illuminate\Support\Facades\File as FileFacade; use Orchestra\Testbench\TestCase; /** @@ -11,17 +12,84 @@ */ class LangJsServiceProviderTest extends TestCase { + /** + * The base path of tests. + * + * @var string + */ + private $testPath; + + /** + * The root path of the project. + * + * @var string + */ + private $rootPath; + + /** + * The file path of the expected output. + * + * @var string + */ + private $outputFilePath; + + /** + * The base path of language files. + * + * @var string + */ + private $langPath; + + /** + * LangJsCommandTest constructor. + */ + public function __construct() + { + parent::__construct(); + + $this->testPath = __DIR__ . '/..'; + $this->rootPath = __DIR__ . '/../..'; + $this->outputFilePath = "$this->testPath/output/lang.js"; + $this->langPath = "$this->testPath/fixtures/lang"; + } + protected function getPackageProviders($app = null) { - return ['Mariuzzo\LaravelJsLocalization\LaravelJsLocalizationServiceProvider']; + return [ + 'Mariuzzo\LaravelJsLocalization\LaravelJsLocalizationServiceProvider', + 'Mariuzzo\LaravelJsLocalization\NonameIncProvider', + ]; } /** * Test the command. */ - public function testShouldRegisterProvider() + public function testShouldIncludePackageLangRegisteredInProvider() + { + + $this->app->setBasePath(__DIR__ . '/../fixtures/'); + + // this checks the LaravelJsLocalizationServiceProvider was loaded + $this->artisan("lang:js {$this->outputFilePath}") + ->assertExitCode(0); + $this->assertFileExists($this->outputFilePath); + + $expected = '"en.nonameinc::messages":{"another_important_data":"this is from the package lang","important_data":"should have replaced packages value for this key","new_key":"this is a new key added ontop of the packages"}'; + $result = file_get_contents($this->outputFilePath); + $this->assertStringContainsString($expected, $result); + + $this->cleanupOutputDirectory(); + + } + + /** + * Cleanup output directory after tests. + */ + protected function cleanupOutputDirectory() { - // TODO: Add some assertions. (however, this already test if this - // package can be provided with the method: getPackageProviders). + $files = FileFacade::files("{$this->testPath}/output"); + foreach ($files as $file) { + FileFacade::delete($file); + } } } diff --git a/tests/specs/NonameIncProvider.php b/tests/specs/NonameIncProvider.php new file mode 100644 index 0000000..2ec8ce8 --- /dev/null +++ b/tests/specs/NonameIncProvider.php @@ -0,0 +1,15 @@ +loadTranslationsFrom(__DIR__ . '/../fixtures/packages/nonameinc/lang', 'nonameinc'); + } + +} + From 68bc564a89a1967073d85427ad34a526820eb8c3 Mon Sep 17 00:00:00 2001 From: Reuben RJ Date: Fri, 28 Aug 2020 10:00:22 +1000 Subject: [PATCH 2/3] fix the reindexing of arrays with array_merge with array_replace --- .../LaravelJsLocalization/Generators/LangJsGenerator.php | 2 +- tests/fixtures/resources/lang/en/messages.php | 1 + tests/specs/LangJsCommandTest.php | 3 +++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Mariuzzo/LaravelJsLocalization/Generators/LangJsGenerator.php b/src/Mariuzzo/LaravelJsLocalization/Generators/LangJsGenerator.php index a460565..86b0f2d 100644 --- a/src/Mariuzzo/LaravelJsLocalization/Generators/LangJsGenerator.php +++ b/src/Mariuzzo/LaravelJsLocalization/Generators/LangJsGenerator.php @@ -232,7 +232,7 @@ private function processFile(&$messages, $file, $namespace = null) $fullPath = $file->getRealPath(); if ($extension == 'php') { - $messages[$key] = array_merge(Arr::get($messages,$key,[]), include $fullPath); + $messages[$key] = array_replace(Arr::get($messages,$key,[]), include $fullPath); } else { $key = $key.$this->stringsDomain; $fileContent = file_get_contents($fullPath); diff --git a/tests/fixtures/resources/lang/en/messages.php b/tests/fixtures/resources/lang/en/messages.php index 75eb85b..b82b7da 100644 --- a/tests/fixtures/resources/lang/en/messages.php +++ b/tests/fixtures/resources/lang/en/messages.php @@ -12,4 +12,5 @@ ), 'plural' => 'one apple|a million apples', 'random' => 'gm8ft2hrrlq1u6m54we9udi', + 0x045F => 'should still be error code 1119' ); diff --git a/tests/specs/LangJsCommandTest.php b/tests/specs/LangJsCommandTest.php index a102f28..98f7e9d 100644 --- a/tests/specs/LangJsCommandTest.php +++ b/tests/specs/LangJsCommandTest.php @@ -183,6 +183,9 @@ public function testPackageFilesShouldBeConverted() $this->_assertStringContainsString('gm8ft2hrrlq1u6m54we9udi', $contents); + # do not reindex error codes. + $this->_assertStringContainsString('"1119":"should still be error code 1119"', $contents); + $this->_assertStringNotContainsString('vendor.nonameinc.en.messages', $contents); $this->_assertStringContainsString('en.nonameinc::messages', $contents); From 19db22734d4c4c9e5558fd6887ebeab8594e068e Mon Sep 17 00:00:00 2001 From: Reuben RJ Date: Fri, 28 Aug 2020 10:00:51 +1000 Subject: [PATCH 3/3] updated unit test to support backward compat. --- tests/specs/LangJsServiceProviderTest.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/specs/LangJsServiceProviderTest.php b/tests/specs/LangJsServiceProviderTest.php index 83e50df..b751847 100644 --- a/tests/specs/LangJsServiceProviderTest.php +++ b/tests/specs/LangJsServiceProviderTest.php @@ -76,7 +76,7 @@ public function testShouldIncludePackageLangRegisteredInProvider() $expected = '"en.nonameinc::messages":{"another_important_data":"this is from the package lang","important_data":"should have replaced packages value for this key","new_key":"this is a new key added ontop of the packages"}'; $result = file_get_contents($this->outputFilePath); - $this->assertStringContainsString($expected, $result); + $this->_assertStringContainsString($expected, $result); $this->cleanupOutputDirectory(); @@ -92,4 +92,13 @@ protected function cleanupOutputDirectory() FileFacade::delete($file); } } + + public function _assertStringContainsString($needle, $haystack) + { + if (method_exists(get_parent_class($this), 'assertStringContainsString')) { + return $this->assertStringContainsString($needle, $haystack); + } + + return $this->assertContains($needle, $haystack); + } }