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

Improved calculating version hash for the js-translation.json file. #10378

Merged
merged 9 commits into from
Aug 18, 2017
10 changes: 10 additions & 0 deletions app/code/Magento/Translation/Block/Js.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,14 @@ public function getTranslationFilePath()
{
return $this->fileManager->getTranslationFilePath();
}

/**
* Gets current version of the translation file.
*
* @return string
*/
public function getTranslationFileVersion()
{
return $this->fileManager->getTranslationFileVersion();
}
}
32 changes: 29 additions & 3 deletions app/code/Magento/Translation/Model/FileManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
namespace Magento\Translation\Model;

use Magento\Framework\App\Filesystem\DirectoryList;
use Magento\Framework\App\ObjectManager;
use Magento\Translation\Model\Inline\File as TranslationFile;

/**
* A service for handling Translation config files
Expand All @@ -32,19 +34,27 @@ class FileManager
*/
private $driverFile;

/**
* @var TranslationFile
*/
private $translationFile;

/**
* @param \Magento\Framework\View\Asset\Repository $assetRepo
* @param \Magento\Framework\App\Filesystem\DirectoryList $directoryList,
* @param \Magento\Framework\Filesystem\Driver\File $driverFile,
* @param \Magento\Framework\App\Filesystem\DirectoryList $directoryList
* @param \Magento\Framework\Filesystem\Driver\File $driverFile
* @param TranslationFile $translationFile
*/
public function __construct(
\Magento\Framework\View\Asset\Repository $assetRepo,
\Magento\Framework\App\Filesystem\DirectoryList $directoryList,
\Magento\Framework\Filesystem\Driver\File $driverFile
\Magento\Framework\Filesystem\Driver\File $driverFile,
\Magento\Translation\Model\Inline\File $translationFile = null
) {
$this->assetRepo = $assetRepo;
$this->directoryList = $directoryList;
$this->driverFile = $driverFile;
$this->translationFile = $translationFile ?: ObjectManager::getInstance()->get(TranslationFile::class);
}

/**
Expand Down Expand Up @@ -110,4 +120,20 @@ public function updateTranslationFileContent($content)
}
$this->driverFile->filePutContents($this->getTranslationFileFullPath(), $content);
}

/**
* Calculate translation file version hash.
*
* @return string
*/
public function getTranslationFileVersion()
{
$translationFile = $this->getTranslationFileFullPath();
if (!$this->driverFile->isExists($translationFile)) {
$this->updateTranslationFileContent($this->translationFile->getTranslationFileContent());
}

$translationFileHash = sha1_file($translationFile);
return sha1($translationFileHash . $this->getTranslationFilePath());
}
}
60 changes: 60 additions & 0 deletions app/code/Magento/Translation/Model/Inline/File.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
namespace Magento\Translation\Model\Inline;

use Magento\Framework\Translate\ResourceInterface;
use Magento\Framework\Locale\ResolverInterface;
use Magento\Framework\Serialize\Serializer\Json;

/**
* Prepares content of inline translations file.
*/
class File
{
/**
* @var ResourceInterface
*/
private $translateResource;

/**
* @var ResolverInterface
*/
private $localeResolver;

/**
* @var Json
*/
private $jsonSerializer;

/**
* Initialize dependencies
*
* @param ResourceInterface $translateResource
* @param ResolverInterface $localeResolver
* @param Json $jsonSerializer
*/
public function __construct(
ResourceInterface $translateResource,
ResolverInterface $localeResolver,
Json $jsonSerializer
) {
$this->translateResource = $translateResource;
$this->localeResolver = $localeResolver;
$this->jsonSerializer = $jsonSerializer;
}

/**
* Generate translation file content for the current locale.
*
* @return string
*/
public function getTranslationFileContent()
{
$translations = $this->translateResource->getTranslationArray(null, $this->localeResolver->getLocale());
$translations = $this->jsonSerializer->serialize($translations);
return $translations;
}
}
10 changes: 10 additions & 0 deletions app/code/Magento/Translation/Test/Unit/Block/JsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,14 @@ public function testGetTranslationFilePath()
->willReturn('frontend/Magento/luma/en_EN');
$this->assertEquals('frontend/Magento/luma/en_EN', $this->model->getTranslationFilePath());
}

public function testGetTranslationFileVersion()
{
$version = sha1('translationFile');

$this->fileManagerMock->expects($this->once())
->method('getTranslationFileVersion')
->willReturn($version);
$this->assertEquals($version, $this->model->getTranslationFileVersion());
}
}
60 changes: 60 additions & 0 deletions app/code/Magento/Translation/Test/Unit/Model/Inline/FileTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
namespace Magento\Translation\Test\Unit\Model\Inline;

class FileTest extends \PHPUnit\Framework\TestCase
{
/**
* @var \Magento\Translation\Model\Inline\File
*/
private $model;

/**
* @var \Magento\Framework\Translate\ResourceInterface|\PHPUnit_Framework_MockObject_MockObject
*/
private $translateResourceMock;

/**
* @var \Magento\Framework\Locale\ResolverInterface|\PHPUnit_Framework_MockObject_MockObject
*/
private $localeResolverMock;

/**
* @var \Magento\Framework\Serialize\Serializer\Json
*/
private $jsonSerializer;

protected function setUp()
{
$this->translateResourceMock = $this->getMockBuilder(\Magento\Framework\Translate\ResourceInterface::class)
->disableOriginalConstructor()
->getMock();
$this->localeResolverMock = $this->getMockBuilder(\Magento\Framework\Locale\ResolverInterface::class)
->disableOriginalConstructor()
->getMock();
$this->jsonSerializer = new \Magento\Framework\Serialize\Serializer\Json();

$this->model = new \Magento\Translation\Model\Inline\File(
$this->translateResourceMock,
$this->localeResolverMock,
$this->jsonSerializer
);
}

public function testGetTranslationFileContent()
{
$translations = ['string' => 'translatedString'];

$this->localeResolverMock->expects($this->atLeastOnce())->method('getLocale')->willReturn('en_US');
$this->translateResourceMock->expects($this->atLeastOnce())->method('getTranslationArray')
->willReturn($translations);

$this->assertEquals(
$this->jsonSerializer->serialize($translations),
$this->model->getTranslationFileContent()
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
$.initNamespaceStorage('mage-translation-file-version');
versionObj = $.localStorage.get('mage-translation-file-version');

<?php $version = sha1($block->getTranslationFileTimestamp() . $block->getTranslationFilePath()); ?>
<?php $version = $block->getTranslationFileVersion(); ?>

if (versionObj.version !== '<?= /* @noEscape */ $version ?>') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shoudn't we escape version there? Previously it was always sha1, but from now it uses raw value from public method, that could be overridden by plugin.
@ishakhsuvarov i think it should be escaped. what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds completely reasonable to me

Copy link
Contributor

Choose a reason for hiding this comment

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

@ishakhsuvarov pull request prepared #10904

dependencies.push(
Expand Down