Skip to content

Commit f8078de

Browse files
author
Łukasz Bajsarowicz
committed
Refactor: Replace equality to identity comparison (Refer: phpbench.com), removing try ... catch blocks throwing just caught exception, make use of imports and extract method initializing Subscription, use short-return structure to reduce complexity.
1 parent bb3cede commit f8078de

File tree

1 file changed

+100
-102
lines changed
  • lib/internal/Magento/Framework/Mview

1 file changed

+100
-102
lines changed

‎lib/internal/Magento/Framework/Mview/View.php

Lines changed: 100 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,19 @@
66

77
namespace Magento\Framework\Mview;
88

9+
use InvalidArgumentException;
10+
use Magento\Framework\DataObject;
911
use Magento\Framework\Mview\View\ChangelogTableNotExistsException;
1012
use Magento\Framework\Mview\View\SubscriptionFactory;
13+
use Exception;
14+
use Magento\Framework\Mview\View\SubscriptionInterface;
1115

1216
/**
1317
* Mview
1418
*
1519
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
1620
*/
17-
class View extends \Magento\Framework\DataObject implements ViewInterface
21+
class View extends DataObject implements ViewInterface
1822
{
1923
/**
2024
* Default batch size for partial reindex
@@ -52,7 +56,7 @@ class View extends \Magento\Framework\DataObject implements ViewInterface
5256
protected $subscriptionFactory;
5357

5458
/**
55-
* @var \Magento\Framework\Mview\View\StateInterface
59+
* @var View\StateInterface
5660
*/
5761
protected $state;
5862

@@ -113,7 +117,7 @@ public function setId($id)
113117
/**
114118
* Id field name setter
115119
*
116-
* @param string $name
120+
* @param string $name
117121
* @return $this
118122
*/
119123
public function setIdFieldName($name)
@@ -167,13 +171,13 @@ public function getSubscriptions()
167171
*
168172
* @param string $viewId
169173
* @return ViewInterface
170-
* @throws \InvalidArgumentException
174+
* @throws InvalidArgumentException
171175
*/
172176
public function load($viewId)
173177
{
174178
$view = $this->config->getView($viewId);
175-
if (empty($view) || empty($view['view_id']) || $view['view_id'] != $viewId) {
176-
throw new \InvalidArgumentException("{$viewId} view does not exist.");
179+
if (empty($view) || empty($view['view_id']) || $view['view_id'] !== $viewId) {
180+
throw new InvalidArgumentException("{$viewId} view does not exist.");
177181
}
178182

179183
$this->setId($viewId);
@@ -185,37 +189,21 @@ public function load($viewId)
185189
/**
186190
* Create subscriptions
187191
*
188-
* @throws \Exception
189192
* @return ViewInterface
193+
* @throws Exception
190194
*/
191195
public function subscribe()
192196
{
193-
if ($this->getState()->getMode() != View\StateInterface::MODE_ENABLED) {
194-
try {
195-
// Create changelog table
196-
$this->getChangelog()->create();
197-
198-
// Create subscriptions
199-
foreach ($this->getSubscriptions() as $subscriptionConfig) {
200-
/** @var \Magento\Framework\Mview\View\SubscriptionInterface $subscription */
201-
$subscriptionInstance = $this->subscriptionFactory->create(
202-
[
203-
'view' => $this,
204-
'tableName' => $subscriptionConfig['name'],
205-
'columnName' => $subscriptionConfig['column'],
206-
'subscriptionModel' => !empty($subscriptionConfig['subscription_model'])
207-
? $subscriptionConfig['subscription_model']
208-
: SubscriptionFactory::INSTANCE_NAME,
209-
]
210-
);
211-
$subscriptionInstance->create();
212-
}
197+
if ($this->getState()->getMode() !== View\StateInterface::MODE_ENABLED) {
198+
// Create changelog table
199+
$this->getChangelog()->create();
213200

214-
// Update view state
215-
$this->getState()->setMode(View\StateInterface::MODE_ENABLED)->save();
216-
} catch (\Exception $e) {
217-
throw $e;
201+
foreach ($this->getSubscriptions() as $subscriptionConfig) {
202+
$this->initSubscriptionInstance($subscriptionConfig)->create();
218203
}
204+
205+
// Update view state
206+
$this->getState()->setMode(View\StateInterface::MODE_ENABLED)->save();
219207
}
220208

221209
return $this;
@@ -224,34 +212,19 @@ public function subscribe()
224212
/**
225213
* Remove subscriptions
226214
*
227-
* @throws \Exception
228215
* @return ViewInterface
216+
* @throws Exception
229217
*/
230218
public function unsubscribe()
231219
{
232220
if ($this->getState()->getMode() != View\StateInterface::MODE_DISABLED) {
233-
try {
234-
// Remove subscriptions
235-
foreach ($this->getSubscriptions() as $subscriptionConfig) {
236-
/** @var \Magento\Framework\Mview\View\SubscriptionInterface $subscription */
237-
$subscriptionInstance = $this->subscriptionFactory->create(
238-
[
239-
'view' => $this,
240-
'tableName' => $subscriptionConfig['name'],
241-
'columnName' => $subscriptionConfig['column'],
242-
'subscriptionModel' => !empty($subscriptionConfig['subscriptionModel'])
243-
? $subscriptionConfig['subscriptionModel']
244-
: SubscriptionFactory::INSTANCE_NAME,
245-
]
246-
);
247-
$subscriptionInstance->remove();
248-
}
249-
250-
// Update view state
251-
$this->getState()->setMode(View\StateInterface::MODE_DISABLED)->save();
252-
} catch (\Exception $e) {
253-
throw $e;
221+
// Remove subscriptions
222+
foreach ($this->getSubscriptions() as $subscriptionConfig) {
223+
$this->initSubscriptionInstance($subscriptionConfig)->remove();
254224
}
225+
226+
// Update view state
227+
$this->getState()->setMode(View\StateInterface::MODE_DISABLED)->save();
255228
}
256229

257230
return $this;
@@ -261,64 +234,68 @@ public function unsubscribe()
261234
* Materialize view by IDs in changelog
262235
*
263236
* @return void
264-
* @throws \Exception
237+
* @throws Exception
265238
*/
266239
public function update()
267240
{
268-
if ($this->getState()->getStatus() == View\StateInterface::STATUS_IDLE) {
269-
try {
270-
$currentVersionId = $this->getChangelog()->getVersion();
271-
} catch (ChangelogTableNotExistsException $e) {
272-
return;
273-
}
274-
$lastVersionId = (int) $this->getState()->getVersionId();
275-
$action = $this->actionFactory->get($this->getActionClass());
276-
277-
try {
278-
$this->getState()->setStatus(View\StateInterface::STATUS_WORKING)->save();
279-
280-
$versionBatchSize = self::$maxVersionQueryBatch;
281-
$batchSize = isset($this->changelogBatchSize[$this->getChangelog()->getViewId()])
282-
? $this->changelogBatchSize[$this->getChangelog()->getViewId()]
283-
: self::DEFAULT_BATCH_SIZE;
284-
285-
for ($vsFrom = $lastVersionId; $vsFrom < $currentVersionId; $vsFrom += $versionBatchSize) {
286-
// Don't go past the current version for atomicy.
287-
$versionTo = min($currentVersionId, $vsFrom + $versionBatchSize);
288-
$ids = $this->getChangelog()->getList($vsFrom, $versionTo);
289-
290-
// We run the actual indexer in batches.
291-
// Chunked AFTER loading to avoid duplicates in separate chunks.
292-
$chunks = array_chunk($ids, $batchSize);
293-
foreach ($chunks as $ids) {
294-
$action->execute($ids);
295-
}
296-
}
241+
if ($this->getState()->getStatus() !== View\StateInterface::STATUS_IDLE) {
242+
return;
243+
}
244+
245+
try {
246+
$currentVersionId = $this->getChangelog()->getVersion();
247+
} catch (ChangelogTableNotExistsException $e) {
248+
return;
249+
}
250+
251+
$lastVersionId = (int)$this->getState()->getVersionId();
252+
$action = $this->actionFactory->get($this->getActionClass());
297253

298-
$this->getState()->loadByView($this->getId());
299-
$statusToRestore = $this->getState()->getStatus() == View\StateInterface::STATUS_SUSPENDED
300-
? View\StateInterface::STATUS_SUSPENDED
301-
: View\StateInterface::STATUS_IDLE;
302-
$this->getState()->setVersionId($currentVersionId)->setStatus($statusToRestore)->save();
303-
} catch (\Exception $exception) {
304-
$this->getState()->loadByView($this->getId());
305-
$statusToRestore = $this->getState()->getStatus() == View\StateInterface::STATUS_SUSPENDED
306-
? View\StateInterface::STATUS_SUSPENDED
307-
: View\StateInterface::STATUS_IDLE;
308-
$this->getState()->setStatus($statusToRestore)->save();
309-
throw $exception;
254+
try {
255+
$this->getState()->setStatus(View\StateInterface::STATUS_WORKING)->save();
256+
257+
$versionBatchSize = self::$maxVersionQueryBatch;
258+
$batchSize = isset($this->changelogBatchSize[$this->getChangelog()->getViewId()])
259+
? $this->changelogBatchSize[$this->getChangelog()->getViewId()]
260+
: self::DEFAULT_BATCH_SIZE;
261+
262+
for ($vsFrom = $lastVersionId; $vsFrom < $currentVersionId; $vsFrom += $versionBatchSize) {
263+
// Don't go past the current version for atomicy.
264+
$versionTo = min($currentVersionId, $vsFrom + $versionBatchSize);
265+
$ids = $this->getChangelog()->getList($vsFrom, $versionTo);
266+
267+
// We run the actual indexer in batches.
268+
// Chunked AFTER loading to avoid duplicates in separate chunks.
269+
$chunks = array_chunk($ids, $batchSize);
270+
foreach ($chunks as $ids) {
271+
$action->execute($ids);
272+
}
310273
}
274+
275+
$this->getState()->loadByView($this->getId());
276+
$statusToRestore = $this->getState()->getStatus() === View\StateInterface::STATUS_SUSPENDED
277+
? View\StateInterface::STATUS_SUSPENDED
278+
: View\StateInterface::STATUS_IDLE;
279+
$this->getState()->setVersionId($currentVersionId)->setStatus($statusToRestore)->save();
280+
} catch (Exception $exception) {
281+
$this->getState()->loadByView($this->getId());
282+
$statusToRestore = $this->getState()->getStatus() === View\StateInterface::STATUS_SUSPENDED
283+
? View\StateInterface::STATUS_SUSPENDED
284+
: View\StateInterface::STATUS_IDLE;
285+
$this->getState()->setStatus($statusToRestore)->save();
286+
throw $exception;
311287
}
312288
}
313289

314290
/**
315291
* Suspend view updates and set version ID to changelog's end
316292
*
317293
* @return void
294+
* @throws Exception
318295
*/
319296
public function suspend()
320297
{
321-
if ($this->getState()->getMode() == View\StateInterface::MODE_ENABLED) {
298+
if ($this->getState()->getMode() === View\StateInterface::MODE_ENABLED) {
322299
$state = $this->getState();
323300
$state->setVersionId($this->getChangelog()->getVersion());
324301
$state->setStatus(View\StateInterface::STATUS_SUSPENDED);
@@ -330,11 +307,12 @@ public function suspend()
330307
* Resume view updates
331308
*
332309
* @return void
310+
* @throws Exception
333311
*/
334312
public function resume()
335313
{
336314
$state = $this->getState();
337-
if ($state->getStatus() == View\StateInterface::STATUS_SUSPENDED) {
315+
if ($state->getStatus() === View\StateInterface::STATUS_SUSPENDED) {
338316
$state->setStatus(View\StateInterface::STATUS_IDLE);
339317
$state->save();
340318
}
@@ -347,7 +325,7 @@ public function resume()
347325
*/
348326
public function clearChangelog()
349327
{
350-
if ($this->getState()->getMode() == View\StateInterface::MODE_ENABLED) {
328+
if ($this->getState()->getMode() === View\StateInterface::MODE_ENABLED) {
351329
$this->getChangelog()->clear($this->getState()->getVersionId());
352330
}
353331
}
@@ -384,7 +362,7 @@ public function setState(View\StateInterface $state)
384362
*/
385363
public function isEnabled()
386364
{
387-
return $this->getState()->getMode() == View\StateInterface::MODE_ENABLED;
365+
return $this->getState()->getMode() === View\StateInterface::MODE_ENABLED;
388366
}
389367

390368
/**
@@ -394,7 +372,7 @@ public function isEnabled()
394372
*/
395373
public function isIdle()
396374
{
397-
return $this->getState()->getStatus() == \Magento\Framework\Mview\View\StateInterface::STATUS_IDLE;
375+
return $this->getState()->getStatus() === View\StateInterface::STATUS_IDLE;
398376
}
399377

400378
/**
@@ -404,7 +382,7 @@ public function isIdle()
404382
*/
405383
public function isWorking()
406384
{
407-
return $this->getState()->getStatus() == \Magento\Framework\Mview\View\StateInterface::STATUS_WORKING;
385+
return $this->getState()->getStatus() === View\StateInterface::STATUS_WORKING;
408386
}
409387

410388
/**
@@ -414,7 +392,7 @@ public function isWorking()
414392
*/
415393
public function isSuspended()
416394
{
417-
return $this->getState()->getStatus() == \Magento\Framework\Mview\View\StateInterface::STATUS_SUSPENDED;
395+
return $this->getState()->getStatus() === View\StateInterface::STATUS_SUSPENDED;
418396
}
419397

420398
/**
@@ -439,4 +417,24 @@ public function getChangelog()
439417
}
440418
return $this->changelog;
441419
}
420+
421+
/**
422+
* Initializes Subscription instance
423+
*
424+
* @param array $subscriptionConfig
425+
* @return SubscriptionInterface
426+
*/
427+
private function initSubscriptionInstance(array $subscriptionConfig): SubscriptionInterface
428+
{
429+
return $this->subscriptionFactory->create(
430+
[
431+
'view' => $this,
432+
'tableName' => $subscriptionConfig['name'],
433+
'columnName' => $subscriptionConfig['column'],
434+
'subscriptionModel' => !empty($subscriptionConfig['subscription_model'])
435+
? $subscriptionConfig['subscription_model']
436+
: SubscriptionFactory::INSTANCE_NAME,
437+
]
438+
);
439+
}
442440
}

0 commit comments

Comments
 (0)