Skip to content

Commit

Permalink
Remove getFromJson from Translator
Browse files Browse the repository at this point in the history
These changes merge the getFromJson behavior into the get method of the Translator class. What it basically does is make the JSON file lookup take precedence over the directory based PHP translation files. The __ helper becomes an alias of the trans helper.

It basically doesn't breaks much. The get call lookup will now first look at the JSON files to find a translation. If it doesn't finds it, it applies the previous get behavior and performs a lookup in the PHP translation files.

These changes make the public api of the Translator class easier and don't require us to add the getFromJson method to the Translator contract.

See #23162
See #29030
  • Loading branch information
driesvints committed Jul 4, 2019
1 parent 822ea2e commit 697b898
Show file tree
Hide file tree
Showing 10 changed files with 68 additions and 71 deletions.
10 changes: 5 additions & 5 deletions src/Illuminate/Auth/Notifications/ResetPassword.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,11 @@ public function toMail($notifiable)
}

return (new MailMessage)
->subject(Lang::getFromJson('Reset Password Notification'))
->line(Lang::getFromJson('You are receiving this email because we received a password reset request for your account.'))
->action(Lang::getFromJson('Reset Password'), url(config('app.url').route('password.reset', ['token' => $this->token, 'email' => $notifiable->getEmailForPasswordReset()], false)))
->line(Lang::getFromJson('This password reset link will expire in :count minutes.', ['count' => config('auth.passwords.users.expire')]))
->line(Lang::getFromJson('If you did not request a password reset, no further action is required.'));
->subject(Lang::get('Reset Password Notification'))
->line(Lang::get('You are receiving this email because we received a password reset request for your account.'))
->action(Lang::get('Reset Password'), url(config('app.url').route('password.reset', ['token' => $this->token, 'email' => $notifiable->getEmailForPasswordReset()], false)))
->line(Lang::get('This password reset link will expire in :count minutes.', ['count' => config('auth.passwords.users.expire')]))
->line(Lang::get('If you did not request a password reset, no further action is required.'));
}

/**
Expand Down
8 changes: 4 additions & 4 deletions src/Illuminate/Auth/Notifications/VerifyEmail.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ public function toMail($notifiable)
}

return (new MailMessage)
->subject(Lang::getFromJson('Verify Email Address'))
->line(Lang::getFromJson('Please click the button below to verify your email address.'))
->action(Lang::getFromJson('Verify Email Address'), $verificationUrl)
->line(Lang::getFromJson('If you did not create an account, no further action is required.'));
->subject(Lang::get('Verify Email Address'))
->line(Lang::get('Please click the button below to verify your email address.'))
->action(Lang::get('Verify Email Address'), $verificationUrl)
->line(Lang::get('If you did not create an account, no further action is required.'));
}

/**
Expand Down
8 changes: 4 additions & 4 deletions src/Illuminate/Foundation/helpers.php
Original file line number Diff line number Diff line change
Expand Up @@ -899,14 +899,14 @@ function trans_choice($key, $number, array $replace = [], $locale = null)
/**
* Translate the given message.
*
* @param string $key
* @param string|null $key
* @param array $replace
* @param string|null $locale
* @return string|array|null
* @return \Illuminate\Contracts\Translation\Translator|string|array|null
*/
function __($key, $replace = [], $locale = null)
function __($key = null, $replace = [], $locale = null)
{
return app('translator')->getFromJson($key, $replace, $locale);
return trans($key, $replace, $locale);
}
}

Expand Down
52 changes: 16 additions & 36 deletions src/Illuminate/Translation/Translator.php
Original file line number Diff line number Diff line change
Expand Up @@ -111,38 +111,6 @@ public function trans($key, array $replace = [], $locale = null)
* @return string|array
*/
public function get($key, array $replace = [], $locale = null, $fallback = true)
{
[$namespace, $group, $item] = $this->parseKey($key);

// Here we will get the locale that should be used for the language line. If one
// was not passed, we will use the default locales which was given to us when
// the translator was instantiated. Then, we can load the lines and return.
$locales = $fallback ? $this->localeArray($locale)
: [$locale ?: $this->locale];

foreach ($locales as $locale) {
if (! is_null($line = $this->getLine(
$namespace, $group, $locale, $item, $replace
))) {
break;
}
}

// If the line doesn't exist, we will return back the key which was requested as
// that will be quick to spot in the UI if language keys are wrong or missing
// from the application's language files. Otherwise we can return the line.
return $line ?? $key;
}

/**
* Get the translation for a given key from the JSON translation files.
*
* @param string $key
* @param array $replace
* @param string|null $locale
* @return string|array
*/
public function getFromJson($key, array $replace = [], $locale = null)
{
$locale = $locale ?: $this->locale;

Expand All @@ -157,13 +125,25 @@ public function getFromJson($key, array $replace = [], $locale = null)
// using the typical translation file. This way developers can always just use a
// helper such as __ instead of having to pick between trans or __ with views.
if (! isset($line)) {
$fallback = $this->get($key, $replace, $locale);

if ($fallback !== $key) {
return $fallback;
[$namespace, $group, $item] = $this->parseKey($key);

// Here we will get the locale that should be used for the language line. If one
// was not passed, we will use the default locales which was given to us when
// the translator was instantiated. Then, we can load the lines and return.
$locales = $fallback ? $this->localeArray($locale) : [$locale];

foreach ($locales as $locale) {
if (! is_null($line = $this->getLine(
$namespace, $group, $locale, $item, $replace
))) {
break;
}
}
}

// If the line doesn't exist, we will return back the key which was requested as
// that will be quick to spot in the UI if language keys are wrong or missing
// from the application's language files. Otherwise we can return the line.
return $this->makeReplacements($line ?: $key, $replace);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ protected function compileLang($expression)
return "<?php \$__env->startTranslation{$expression}; ?>";
}

return "<?php echo app('translator')->getFromJson{$expression}; ?>";
return "<?php echo app('translator')->get{$expression}; ?>";
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Illuminate/View/Concerns/ManagesTranslations.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public function startTranslation($replacements = [])
*/
public function renderTranslation()
{
return $this->container->make('translator')->getFromJson(
return $this->container->make('translator')->get(
trim(ob_get_clean()), $this->translationReplacements
);
}
Expand Down
47 changes: 32 additions & 15 deletions tests/Translation/TranslationTranslatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,36 +34,50 @@ public function testHasMethodReturnsFalseWhenReturnedTranslationIsNull()
$t->expects($this->once())->method('get')->with($this->equalTo('foo'), $this->equalTo([]), $this->equalTo('bar'), false)->will($this->returnValue('foo'));
$this->assertFalse($t->hasForLocale('foo', 'bar'));

$t = $this->getMockBuilder(Translator::class)->setMethods(['load', 'getLine'])->setConstructorArgs([$this->getLoader(), 'en'])->getMock();
$t->expects($this->any())->method('load')->with($this->equalTo('*'), $this->equalTo('foo'), $this->equalTo('en'))->will($this->returnValue(null));
$t->expects($this->once())->method('getLine')->with($this->equalTo('*'), $this->equalTo('foo'), $this->equalTo('en'), null, $this->equalTo([]))->will($this->returnValue('bar'));
$t = new Translator($this->getLoader(), 'en');
$t->getLoader()->shouldReceive('load')->once()->with('en', '*', '*')->andReturn([]);
$t->getLoader()->shouldReceive('load')->once()->with('en', 'foo', '*')->andReturn(['foo' => 'bar']);
$this->assertTrue($t->hasForLocale('foo'));

$t = $this->getMockBuilder(Translator::class)->setMethods(['load', 'getLine'])->setConstructorArgs([$this->getLoader(), 'en'])->getMock();
$t->expects($this->any())->method('load')->with($this->equalTo('*'), $this->equalTo('foo'), $this->equalTo('en'))->will($this->returnValue(null));
$t->expects($this->once())->method('getLine')->with($this->equalTo('*'), $this->equalTo('foo'), $this->equalTo('en'), null, $this->equalTo([]))->will($this->returnValue('foo'));
$t = new Translator($this->getLoader(), 'en');
$t->getLoader()->shouldReceive('load')->once()->with('en', '*', '*')->andReturn([]);
$t->getLoader()->shouldReceive('load')->once()->with('en', 'foo', '*')->andReturn([]);
$this->assertFalse($t->hasForLocale('foo'));
}

public function testGetMethodProperlyLoadsAndRetrievesItem()
{
$t = new Translator($this->getLoader(), 'en');
$t->getLoader()->shouldReceive('load')->once()->with('en', '*', '*')->andReturn([]);
$t->getLoader()->shouldReceive('load')->once()->with('en', 'bar', 'foo')->andReturn(['foo' => 'foo', 'baz' => 'breeze :foo', 'qux' => ['tree :foo', 'breeze :foo']]);
$this->assertEquals(['tree bar', 'breeze bar'], $t->get('foo::bar.qux', ['foo' => 'bar'], 'en'));
$this->assertEquals('breeze bar', $t->get('foo::bar.baz', ['foo' => 'bar'], 'en'));
$this->assertEquals('foo', $t->get('foo::bar.foo'));
}

public function testGetMethodForNonExistingReturnsSameKey()
{
$t = new Translator($this->getLoader(), 'en');
$t->getLoader()->shouldReceive('load')->once()->with('en', '*', '*')->andReturn([]);
$t->getLoader()->shouldReceive('load')->once()->with('en', 'bar', 'foo')->andReturn(['foo' => 'foo', 'baz' => 'breeze :foo', 'qux' => ['tree :foo', 'breeze :foo']]);
$t->getLoader()->shouldReceive('load')->once()->with('en', 'unknown', 'foo')->andReturn([]);
$this->assertEquals('foo::unknown', $t->get('foo::unknown', ['foo' => 'bar'], 'en'));
$this->assertEquals('foo::bar.unknown', $t->get('foo::bar.unknown', ['foo' => 'bar'], 'en'));
$this->assertEquals('foo::unknown.bar', $t->get('foo::unknown.bar'));
}

public function testTransMethodProperlyLoadsAndRetrievesItemWithHTMLInTheMessage()
{
$t = new Translator($this->getLoader(), 'en');
$t->getLoader()->shouldReceive('load')->once()->with('en', '*', '*')->andReturn([]);
$t->getLoader()->shouldReceive('load')->once()->with('en', 'foo', '*')->andReturn(['bar' => 'breeze <p>test</p>']);
$this->assertSame('breeze <p>test</p>', $t->trans('foo.bar', [], 'en'));
}

public function testGetMethodProperlyLoadsAndRetrievesItemWithCapitalization()
{
$t = $this->getMockBuilder(Translator::class)->setMethods(null)->setConstructorArgs([$this->getLoader(), 'en'])->getMock();
$t->getLoader()->shouldReceive('load')->once()->with('en', '*', '*')->andReturn([]);
$t->getLoader()->shouldReceive('load')->once()->with('en', 'bar', 'foo')->andReturn(['foo' => 'foo', 'baz' => 'breeze :Foo :BAR']);
$this->assertEquals('breeze Bar FOO', $t->get('foo::bar.baz', ['foo' => 'bar', 'bar' => 'foo'], 'en'));
$this->assertEquals('foo', $t->get('foo::bar.foo'));
Expand All @@ -72,6 +86,7 @@ public function testGetMethodProperlyLoadsAndRetrievesItemWithCapitalization()
public function testGetMethodProperlyLoadsAndRetrievesItemWithLongestReplacementsFirst()
{
$t = new Translator($this->getLoader(), 'en');
$t->getLoader()->shouldReceive('load')->once()->with('en', '*', '*')->andReturn([]);
$t->getLoader()->shouldReceive('load')->once()->with('en', 'bar', 'foo')->andReturn(['foo' => 'foo', 'baz' => 'breeze :foo :foobar']);
$this->assertEquals('breeze bar taylor', $t->get('foo::bar.baz', ['foo' => 'bar', 'foobar' => 'taylor'], 'en'));
$this->assertEquals('breeze foo bar baz taylor', $t->get('foo::bar.baz', ['foo' => 'foo bar baz', 'foobar' => 'taylor'], 'en'));
Expand All @@ -82,6 +97,7 @@ public function testGetMethodProperlyLoadsAndRetrievesItemForFallback()
{
$t = new Translator($this->getLoader(), 'en');
$t->setFallback('lv');
$t->getLoader()->shouldReceive('load')->once()->with('en', '*', '*')->andReturn([]);
$t->getLoader()->shouldReceive('load')->once()->with('en', 'bar', 'foo')->andReturn([]);
$t->getLoader()->shouldReceive('load')->once()->with('lv', 'bar', 'foo')->andReturn(['foo' => 'foo', 'baz' => 'breeze :foo']);
$this->assertEquals('breeze bar', $t->get('foo::bar.baz', ['foo' => 'bar'], 'en'));
Expand All @@ -91,6 +107,7 @@ public function testGetMethodProperlyLoadsAndRetrievesItemForFallback()
public function testGetMethodProperlyLoadsAndRetrievesItemForGlobalNamespace()
{
$t = new Translator($this->getLoader(), 'en');
$t->getLoader()->shouldReceive('load')->once()->with('en', '*', '*')->andReturn([]);
$t->getLoader()->shouldReceive('load')->once()->with('en', 'foo', '*')->andReturn(['bar' => 'breeze :foo']);
$this->assertEquals('breeze bar', $t->get('foo.bar', ['foo' => 'bar']));
}
Expand Down Expand Up @@ -119,64 +136,64 @@ public function testChoiceMethodProperlyCountsCollectionsAndLoadsAndRetrievesIte
$t->choice('foo', $values, ['replace']);
}

public function testGetJsonMethod()
public function testGetJson()
{
$t = new Translator($this->getLoader(), 'en');
$t->getLoader()->shouldReceive('load')->once()->with('en', '*', '*')->andReturn(['foo' => 'one']);
$this->assertEquals('one', $t->getFromJson('foo'));
$this->assertEquals('one', $t->get('foo'));
}

public function testGetJsonReplaces()
{
$t = new Translator($this->getLoader(), 'en');
$t->getLoader()->shouldReceive('load')->once()->with('en', '*', '*')->andReturn(['foo :i:c :u' => 'bar :i:c :u']);
$this->assertEquals('bar onetwo three', $t->getFromJson('foo :i:c :u', ['i' => 'one', 'c' => 'two', 'u' => 'three']));
$this->assertEquals('bar onetwo three', $t->get('foo :i:c :u', ['i' => 'one', 'c' => 'two', 'u' => 'three']));
}

public function testGetJsonReplacesForAssociativeInput()
{
$t = new Translator($this->getLoader(), 'en');
$t->getLoader()->shouldReceive('load')->once()->with('en', '*', '*')->andReturn(['foo :i :c' => 'bar :i :c']);
$this->assertEquals('bar eye see', $t->getFromJson('foo :i :c', ['i' => 'eye', 'c' => 'see']));
$this->assertEquals('bar eye see', $t->get('foo :i :c', ['i' => 'eye', 'c' => 'see']));
}

public function testGetJsonPreservesOrder()
{
$t = new Translator($this->getLoader(), 'en');
$t->getLoader()->shouldReceive('load')->once()->with('en', '*', '*')->andReturn(['to :name I give :greeting' => ':greeting :name']);
$this->assertEquals('Greetings David', $t->getFromJson('to :name I give :greeting', ['name' => 'David', 'greeting' => 'Greetings']));
$this->assertEquals('Greetings David', $t->get('to :name I give :greeting', ['name' => 'David', 'greeting' => 'Greetings']));
}

public function testGetJsonForNonExistingJsonKeyLooksForRegularKeys()
{
$t = new Translator($this->getLoader(), 'en');
$t->getLoader()->shouldReceive('load')->once()->with('en', '*', '*')->andReturn([]);
$t->getLoader()->shouldReceive('load')->once()->with('en', 'foo', '*')->andReturn(['bar' => 'one']);
$this->assertEquals('one', $t->getFromJson('foo.bar'));
$this->assertEquals('one', $t->get('foo.bar'));
}

public function testGetJsonForNonExistingJsonKeyLooksForRegularKeysAndReplace()
{
$t = new Translator($this->getLoader(), 'en');
$t->getLoader()->shouldReceive('load')->once()->with('en', '*', '*')->andReturn([]);
$t->getLoader()->shouldReceive('load')->once()->with('en', 'foo', '*')->andReturn(['bar' => 'one :message']);
$this->assertEquals('one two', $t->getFromJson('foo.bar', ['message' => 'two']));
$this->assertEquals('one two', $t->get('foo.bar', ['message' => 'two']));
}

public function testGetJsonForNonExistingReturnsSameKey()
{
$t = new Translator($this->getLoader(), 'en');
$t->getLoader()->shouldReceive('load')->once()->with('en', '*', '*')->andReturn([]);
$t->getLoader()->shouldReceive('load')->once()->with('en', 'Foo that bar', '*')->andReturn([]);
$this->assertEquals('Foo that bar', $t->getFromJson('Foo that bar'));
$this->assertEquals('Foo that bar', $t->get('Foo that bar'));
}

public function testGetJsonForNonExistingReturnsSameKeyAndReplaces()
{
$t = new Translator($this->getLoader(), 'en');
$t->getLoader()->shouldReceive('load')->once()->with('en', '*', '*')->andReturn([]);
$t->getLoader()->shouldReceive('load')->once()->with('en', 'foo :message', '*')->andReturn([]);
$this->assertEquals('foo baz', $t->getFromJson('foo :message', ['message' => 'baz']));
$this->assertEquals('foo baz', $t->get('foo :message', ['message' => 'baz']));
}

protected function getLoader()
Expand Down
4 changes: 2 additions & 2 deletions tests/View/Blade/BladeExpressionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ class BladeExpressionTest extends AbstractBladeTestCase
{
public function testExpressionsOnTheSameLine()
{
$this->assertEquals('<?php echo app(\'translator\')->getFromJson(foo(bar(baz(qux(breeze()))))); ?> space () <?php echo app(\'translator\')->getFromJson(foo(bar)); ?>', $this->compiler->compileString('@lang(foo(bar(baz(qux(breeze()))))) space () @lang(foo(bar))'));
$this->assertEquals('<?php echo app(\'translator\')->get(foo(bar(baz(qux(breeze()))))); ?> space () <?php echo app(\'translator\')->get(foo(bar)); ?>', $this->compiler->compileString('@lang(foo(bar(baz(qux(breeze()))))) space () @lang(foo(bar))'));
}

public function testExpressionWithinHTML()
{
$this->assertEquals('<html <?php echo e($foo); ?>>', $this->compiler->compileString('<html {{ $foo }}>'));
$this->assertEquals('<html<?php echo e($foo); ?>>', $this->compiler->compileString('<html{{ $foo }}>'));
$this->assertEquals('<html <?php echo e($foo); ?> <?php echo app(\'translator\')->getFromJson(\'foo\'); ?>>', $this->compiler->compileString('<html {{ $foo }} @lang(\'foo\')>'));
$this->assertEquals('<html <?php echo e($foo); ?> <?php echo app(\'translator\')->get(\'foo\'); ?>>', $this->compiler->compileString('<html {{ $foo }} @lang(\'foo\')>'));
}
}
4 changes: 2 additions & 2 deletions tests/View/Blade/BladeLangTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ class BladeLangTest extends AbstractBladeTestCase
public function testStatementThatContainsNonConsecutiveParenthesisAreCompiled()
{
$string = "Foo @lang(function_call('foo(blah)')) bar";
$expected = "Foo <?php echo app('translator')->getFromJson(function_call('foo(blah)')); ?> bar";
$expected = "Foo <?php echo app('translator')->get(function_call('foo(blah)')); ?> bar";
$this->assertEquals($expected, $this->compiler->compileString($string));
}

public function testLanguageAndChoicesAreCompiled()
{
$this->assertEquals('<?php echo app(\'translator\')->getFromJson(\'foo\'); ?>', $this->compiler->compileString("@lang('foo')"));
$this->assertEquals('<?php echo app(\'translator\')->get(\'foo\'); ?>', $this->compiler->compileString("@lang('foo')"));
$this->assertEquals('<?php echo app(\'translator\')->choice(\'foo\', 1); ?>', $this->compiler->compileString("@choice('foo', 1)"));
}
}
2 changes: 1 addition & 1 deletion tests/View/ViewFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ public function testTranslation()
{
$container = new Container;
$container->instance('translator', $translator = m::mock(stdClass::class));
$translator->shouldReceive('getFromJson')->with('Foo', ['name' => 'taylor'])->andReturn('Bar');
$translator->shouldReceive('get')->with('Foo', ['name' => 'taylor'])->andReturn('Bar');
$factory = $this->getFactory();
$factory->setContainer($container);
$factory->startTranslation(['name' => 'taylor']);
Expand Down

0 comments on commit 697b898

Please sign in to comment.