From 7a8c24b2efd9d2e59bf9efac016a9e656b0b4067 Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Thu, 14 Jun 2018 09:48:26 +0200 Subject: [PATCH 1/4] Fix typehint of customer-data updateSectionId parameters The typehint wrongly indicates the parameter should be a number, where in fact it is a boolean. It is passed through to the server side to the controller action \Magento\Customer\Controller\Section\Load::execute() where it is cast to a boolean and passed as the $updateIds parameter to \Magento\Customer\CustomerData\SectionPool::getSectionsData() and from there as the boolean parameter $forceUpdate to the \Magento\Customer\CustomerData\Section\Identifier::initMark() method. This PR introduces no backward incompatibilities, it only fixes the type hint to make the code more readable and improve compatibility with static code analysis tools. --- .../Magento/Customer/view/frontend/web/js/customer-data.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Customer/view/frontend/web/js/customer-data.js b/app/code/Magento/Customer/view/frontend/web/js/customer-data.js index 15df80f360bf7..56ee437d9704f 100644 --- a/app/code/Magento/Customer/view/frontend/web/js/customer-data.js +++ b/app/code/Magento/Customer/view/frontend/web/js/customer-data.js @@ -74,7 +74,7 @@ define([ /** * @param {Object} sectionNames - * @param {Number} updateSectionId + * @param {Boolean} updateSectionId * @return {*} */ getFromServer: function (sectionNames, updateSectionId) { @@ -324,7 +324,7 @@ define([ /** * @param {Array} sectionNames - * @param {Number} updateSectionId + * @param {Boolean} updateSectionId * @return {*} */ reload: function (sectionNames, updateSectionId) { From fc1c1d2ab5d774d527ff97630f75a5072fa1bf18 Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Sat, 16 Jun 2018 20:51:24 +0200 Subject: [PATCH 2/4] Rename parameter variable to make intent more easy to understand. --- .../Customer/view/frontend/web/js/customer-data.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/code/Magento/Customer/view/frontend/web/js/customer-data.js b/app/code/Magento/Customer/view/frontend/web/js/customer-data.js index 56ee437d9704f..27c8230f9b9af 100644 --- a/app/code/Magento/Customer/view/frontend/web/js/customer-data.js +++ b/app/code/Magento/Customer/view/frontend/web/js/customer-data.js @@ -74,17 +74,17 @@ define([ /** * @param {Object} sectionNames - * @param {Boolean} updateSectionId + * @param {Boolean} forceNewSectionTimestamp * @return {*} */ - getFromServer: function (sectionNames, updateSectionId) { + getFromServer: function (sectionNames, forceNewSectionTimestamp) { var parameters; sectionNames = sectionConfig.filterClientSideSections(sectionNames); parameters = _.isArray(sectionNames) ? { sections: sectionNames.join(',') } : []; - parameters['update_section_id'] = updateSectionId; + parameters['update_section_id'] = forceNewSectionTimestamp; return $.getJSON(options.sectionLoadUrl, parameters).fail(function (jqXHR) { throw new Error(jqXHR); @@ -324,11 +324,11 @@ define([ /** * @param {Array} sectionNames - * @param {Boolean} updateSectionId + * @param {Boolean} forceNewSectionTimestamp * @return {*} */ - reload: function (sectionNames, updateSectionId) { - return dataProvider.getFromServer(sectionNames, updateSectionId).done(function (sections) { + reload: function (sectionNames, forceNewSectionTimestamp) { + return dataProvider.getFromServer(sectionNames, forceNewSectionTimestamp).done(function (sections) { $(document).trigger('customer-data-reload', [sectionNames]); buffer.update(sections); }); From 73b02f8c0b10adb38580cd55d51256f75cc6d1fa Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Fri, 22 Jun 2018 08:47:36 -0400 Subject: [PATCH 3/4] Rename request parameter and variables to make them more expressive --- .../Customer/Controller/Section/Load.php | 8 +++---- .../CustomerData/Section/Identifier.php | 14 +++++------ .../Customer/CustomerData/SectionPool.php | 4 ++-- .../CustomerData/SectionPoolInterface.php | 4 ++-- .../Test/Unit/Controller/Section/LoadTest.php | 24 +++++++++---------- .../Unit/CustomerData/SectionPoolTest.php | 2 +- .../view/frontend/web/js/customer-data.js | 2 +- 7 files changed, 29 insertions(+), 29 deletions(-) diff --git a/app/code/Magento/Customer/Controller/Section/Load.php b/app/code/Magento/Customer/Controller/Section/Load.php index 7a2345c91750c..e37461d20f5de 100644 --- a/app/code/Magento/Customer/Controller/Section/Load.php +++ b/app/code/Magento/Customer/Controller/Section/Load.php @@ -70,11 +70,11 @@ public function execute() $sectionNames = $this->getRequest()->getParam('sections'); $sectionNames = $sectionNames ? array_unique(\explode(',', $sectionNames)) : null; - $updateSectionId = $this->getRequest()->getParam('update_section_id'); - if ('false' === $updateSectionId) { - $updateSectionId = false; + $forceNewSectionTimestamp = $this->getRequest()->getParam('force_new_section_timestamp'); + if ('false' === $forceNewSectionTimestamp) { + $forceNewSectionTimestamp = false; } - $response = $this->sectionPool->getSectionsData($sectionNames, (bool)$updateSectionId); + $response = $this->sectionPool->getSectionsData($sectionNames, (bool)$forceNewSectionTimestamp); } catch (\Exception $e) { $resultJson->setStatusHeader( \Zend\Http\Response::STATUS_CODE_400, diff --git a/app/code/Magento/Customer/CustomerData/Section/Identifier.php b/app/code/Magento/Customer/CustomerData/Section/Identifier.php index 2a770925d1c37..54d7cee2d90bd 100644 --- a/app/code/Magento/Customer/CustomerData/Section/Identifier.php +++ b/app/code/Magento/Customer/CustomerData/Section/Identifier.php @@ -43,12 +43,12 @@ public function __construct( /** * Init mark(identifier) for sections * - * @param bool $forceUpdate + * @param bool $forceNewTimestamp * @return int */ - public function initMark($forceUpdate) + public function initMark($forceNewTimestamp) { - if ($forceUpdate) { + if ($forceNewTimestamp) { $this->markId = time(); return $this->markId; } @@ -68,18 +68,18 @@ public function initMark($forceUpdate) * * @param array $sectionsData * @param null $sectionNames - * @param bool $updateIds + * @param bool $forceNewTimestamp * @return array */ - public function markSections(array $sectionsData, $sectionNames = null, $updateIds = false) + public function markSections(array $sectionsData, $sectionNames = null, $forceNewTimestamp = false) { if (!$sectionNames) { $sectionNames = array_keys($sectionsData); } - $markId = $this->initMark($updateIds); + $markId = $this->initMark($forceNewTimestamp); foreach ($sectionNames as $name) { - if ($updateIds || !array_key_exists(self::SECTION_KEY, $sectionsData[$name])) { + if ($forceNewTimestamp || !array_key_exists(self::SECTION_KEY, $sectionsData[$name])) { $sectionsData[$name][self::SECTION_KEY] = $markId; } } diff --git a/app/code/Magento/Customer/CustomerData/SectionPool.php b/app/code/Magento/Customer/CustomerData/SectionPool.php index 26e9140c63df5..be5ea09c0db33 100644 --- a/app/code/Magento/Customer/CustomerData/SectionPool.php +++ b/app/code/Magento/Customer/CustomerData/SectionPool.php @@ -55,10 +55,10 @@ public function __construct( /** * {@inheritdoc} */ - public function getSectionsData(array $sectionNames = null, $updateIds = false) + public function getSectionsData(array $sectionNames = null, $forceNewTimestamp = false) { $sectionsData = $sectionNames ? $this->getSectionDataByNames($sectionNames) : $this->getAllSectionData(); - $sectionsData = $this->identifier->markSections($sectionsData, $sectionNames, $updateIds); + $sectionsData = $this->identifier->markSections($sectionsData, $sectionNames, $forceNewTimestamp); return $sectionsData; } diff --git a/app/code/Magento/Customer/CustomerData/SectionPoolInterface.php b/app/code/Magento/Customer/CustomerData/SectionPoolInterface.php index c308804fd0f8d..ad73b9722b133 100644 --- a/app/code/Magento/Customer/CustomerData/SectionPoolInterface.php +++ b/app/code/Magento/Customer/CustomerData/SectionPoolInterface.php @@ -14,8 +14,8 @@ interface SectionPoolInterface * Get section data by section names. If $sectionNames is null then return all sections data * * @param array $sectionNames - * @param bool $updateIds + * @param bool $forceNewTimestamp * @return array */ - public function getSectionsData(array $sectionNames = null, $updateIds = false); + public function getSectionsData(array $sectionNames = null, $forceNewTimestamp = false); } diff --git a/app/code/Magento/Customer/Test/Unit/Controller/Section/LoadTest.php b/app/code/Magento/Customer/Test/Unit/Controller/Section/LoadTest.php index 2552beeca463d..6e145e1228778 100644 --- a/app/code/Magento/Customer/Test/Unit/Controller/Section/LoadTest.php +++ b/app/code/Magento/Customer/Test/Unit/Controller/Section/LoadTest.php @@ -81,13 +81,13 @@ protected function setUp() } /** - * @param $sectionNames - * @param $updateSectionID - * @param $sectionNamesAsArray - * @param $updateIds + * @param string $sectionNames + * @param bool $forceNewSectionTimestamp + * @param string[] $sectionNamesAsArray + * @param bool $forceNewTimestamp * @dataProvider executeDataProvider */ - public function testExecute($sectionNames, $updateSectionID, $sectionNamesAsArray, $updateIds) + public function testExecute($sectionNames, $forceNewSectionTimestamp, $sectionNamesAsArray, $forceNewTimestamp) { $this->resultJsonFactoryMock->expects($this->once()) ->method('create') @@ -101,12 +101,12 @@ public function testExecute($sectionNames, $updateSectionID, $sectionNamesAsArra $this->httpRequestMock->expects($this->exactly(2)) ->method('getParam') - ->withConsecutive(['sections'], ['update_section_id']) - ->willReturnOnConsecutiveCalls($sectionNames, $updateSectionID); + ->withConsecutive(['sections'], ['force_new_section_timestamp']) + ->willReturnOnConsecutiveCalls($sectionNames, $forceNewSectionTimestamp); $this->sectionPoolMock->expects($this->once()) ->method('getSectionsData') - ->with($sectionNamesAsArray, $updateIds) + ->with($sectionNamesAsArray, $forceNewTimestamp) ->willReturn([ 'message' => 'some message', 'someKey' => 'someValue' @@ -128,15 +128,15 @@ public function executeDataProvider() return [ [ 'sectionNames' => 'sectionName1,sectionName2,sectionName3', - 'updateSectionID' => 'updateSectionID', + 'forceNewSectionTimestamp' => 'forceNewSectionTimestamp', 'sectionNamesAsArray' => ['sectionName1', 'sectionName2', 'sectionName3'], - 'updateIds' => true + 'forceNewTimestamp' => true ], [ 'sectionNames' => null, - 'updateSectionID' => null, + 'forceNewSectionTimestamp' => null, 'sectionNamesAsArray' => null, - 'updateIds' => false + 'forceNewTimestamp' => false ], ]; } diff --git a/app/code/Magento/Customer/Test/Unit/CustomerData/SectionPoolTest.php b/app/code/Magento/Customer/Test/Unit/CustomerData/SectionPoolTest.php index 98fee70e335f7..2b67df1aee292 100644 --- a/app/code/Magento/Customer/Test/Unit/CustomerData/SectionPoolTest.php +++ b/app/code/Magento/Customer/Test/Unit/CustomerData/SectionPoolTest.php @@ -63,7 +63,7 @@ public function testGetSectionsDataAllSections() $this->identifierMock->expects($this->once()) ->method('markSections') - //check also default value for $updateIds = false + //check also default value for $forceTimestamp = false ->with($allSectionsData, $sectionNames, false) ->willReturn($identifierResult); $modelResult = $this->model->getSectionsData($sectionNames); diff --git a/app/code/Magento/Customer/view/frontend/web/js/customer-data.js b/app/code/Magento/Customer/view/frontend/web/js/customer-data.js index 27c8230f9b9af..ac28084218ddc 100644 --- a/app/code/Magento/Customer/view/frontend/web/js/customer-data.js +++ b/app/code/Magento/Customer/view/frontend/web/js/customer-data.js @@ -84,7 +84,7 @@ define([ parameters = _.isArray(sectionNames) ? { sections: sectionNames.join(',') } : []; - parameters['update_section_id'] = forceNewSectionTimestamp; + parameters['force_new_section_timestamp'] = forceNewSectionTimestamp; return $.getJSON(options.sectionLoadUrl, parameters).fail(function (jqXHR) { throw new Error(jqXHR); From 3275cd7e2c2184cb2263d60203c40be2da1a4ccd Mon Sep 17 00:00:00 2001 From: Vinai Kopp Date: Sat, 23 Jun 2018 17:40:12 +0200 Subject: [PATCH 4/4] Update query parameter name in performance toolkit to match new name in code --- setup/performance-toolkit/benchmark.jmx | 76 ++++++++++---------- setup/performance-toolkit/benchmark_2015.jmx | 36 +++++----- 2 files changed, 56 insertions(+), 56 deletions(-) diff --git a/setup/performance-toolkit/benchmark.jmx b/setup/performance-toolkit/benchmark.jmx index 8f8a72673d858..e5a4485aef0c3 100644 --- a/setup/performance-toolkit/benchmark.jmx +++ b/setup/performance-toolkit/benchmark.jmx @@ -2876,12 +2876,12 @@ vars.put("customer_email", customerUser); true sections - + true false = true - update_section_id + force_new_section_timestamp true @@ -4901,12 +4901,12 @@ vars.put("totalProductsAdded", String.valueOf(productsAdded)); true sections - + true true = true - update_section_id + force_new_section_timestamp true @@ -5267,12 +5267,12 @@ vars.put("totalProductsAdded", String.valueOf(productsAdded)); true sections - + true true = true - update_section_id + force_new_section_timestamp true @@ -5561,12 +5561,12 @@ vars.put("customer_email", customerUser); true sections - + true false = true - update_section_id + force_new_section_timestamp true @@ -5734,12 +5734,12 @@ vars.put("product_sku", product.get("sku")); true sections - + true false = true - update_section_id + force_new_section_timestamp true @@ -6166,12 +6166,12 @@ vars.put("totalProductsAdded", String.valueOf(productsAdded)); true sections - + true false = true - update_section_id + force_new_section_timestamp true @@ -6344,12 +6344,12 @@ vars.put("totalProductsAdded", String.valueOf(productsAdded)); true sections - + true false = true - update_section_id + force_new_section_timestamp true @@ -6782,12 +6782,12 @@ vars.put("totalProductsAdded", String.valueOf(productsAdded)); true sections - + true true = true - update_section_id + force_new_section_timestamp true @@ -7148,12 +7148,12 @@ vars.put("totalProductsAdded", String.valueOf(productsAdded)); true sections - + true true = true - update_section_id + force_new_section_timestamp true @@ -7864,12 +7864,12 @@ vars.put("customer_email", customerUser); true sections - + true false = true - update_section_id + force_new_section_timestamp true @@ -8100,12 +8100,12 @@ vars.put("totalProductsAdded", String.valueOf(productsAdded)); true sections - + true true = true - update_section_id + force_new_section_timestamp true @@ -8466,12 +8466,12 @@ vars.put("totalProductsAdded", String.valueOf(productsAdded)); true sections - + true true = true - update_section_id + force_new_section_timestamp true @@ -9141,12 +9141,12 @@ vars.put("customer_email", customerUser); true sections - + true false = true - update_section_id + force_new_section_timestamp true @@ -9325,12 +9325,12 @@ vars.put("product_sku", product.get("sku")); true sections - + true false = true - update_section_id + force_new_section_timestamp true @@ -9657,12 +9657,12 @@ vars.put("customer_email", customerUser); true sections - + true false = true - update_section_id + force_new_section_timestamp true @@ -9924,12 +9924,12 @@ vars.put("totalProductsAdded", String.valueOf(productsAdded)); true sections - + true true = true - update_section_id + force_new_section_timestamp true @@ -10290,12 +10290,12 @@ vars.put("totalProductsAdded", String.valueOf(productsAdded)); true sections - + true true = true - update_section_id + force_new_section_timestamp true @@ -10487,12 +10487,12 @@ vars.put("item_id", vars.get("cart_items_qty_inputs_" + id)); true sections - + true true = true - update_section_id + force_new_section_timestamp true @@ -10808,12 +10808,12 @@ vars.put("customer_email", customerUser); true sections - + true false = true - update_section_id + force_new_section_timestamp true diff --git a/setup/performance-toolkit/benchmark_2015.jmx b/setup/performance-toolkit/benchmark_2015.jmx index 8be7749a08bc4..e58e610304199 100644 --- a/setup/performance-toolkit/benchmark_2015.jmx +++ b/setup/performance-toolkit/benchmark_2015.jmx @@ -2981,12 +2981,12 @@ vars.put("loadType", "Guest"); true sections - + true true = true - update_section_id + force_new_section_timestamp false @@ -3217,12 +3217,12 @@ vars.put("loadType", "Guest"); sections true - + true true = true - update_section_id + force_new_section_timestamp true @@ -3565,12 +3565,12 @@ vars.put("loadType", "Guest"); sections true - + true true = true - update_section_id + force_new_section_timestamp true @@ -4039,12 +4039,12 @@ vars.put("loadType", "Guest"); true sections - + true true = true - update_section_id + force_new_section_timestamp false @@ -4275,12 +4275,12 @@ vars.put("loadType", "Guest"); sections true - + true true = true - update_section_id + force_new_section_timestamp true @@ -4623,12 +4623,12 @@ vars.put("loadType", "Guest"); sections true - + true true = true - update_section_id + force_new_section_timestamp true @@ -5637,12 +5637,12 @@ vars.put("loadType", "Customer"); true sections - + true true = true - update_section_id + force_new_section_timestamp false @@ -5873,12 +5873,12 @@ vars.put("loadType", "Customer"); sections true - + true true = true - update_section_id + force_new_section_timestamp true @@ -6221,12 +6221,12 @@ vars.put("loadType", "Customer"); sections true - + true true = true - update_section_id + force_new_section_timestamp true