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

[5.3] Error handling: Adding new shouldUseException() #44098

Open
wants to merge 5 commits into
base: 5.3-dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -596,6 +596,10 @@ public function getTransitions()
$this->cache[$store] = $transitions;
}
} catch (\RuntimeException $e) {
if ($this->shouldUseExceptions()) {
throw $e;
}

$this->setError($e->getMessage());

return false;
Expand Down
40 changes: 20 additions & 20 deletions administrator/components/com_content/src/View/Articles/HtmlView.php
Original file line number Diff line number Diff line change
Expand Up @@ -111,28 +111,28 @@ public function display($tpl = null)
{
/** @var ArticlesModel $model */
$model = $this->getModel();
$model->setUseExceptions(true);

try {
$this->items = $model->getItems();
$this->pagination = $model->getPagination();
$this->state = $model->getState();
$this->filterForm = $model->getFilterForm();
$this->activeFilters = $model->getActiveFilters();
$this->vote = PluginHelper::isEnabled('content', 'vote');
$this->hits = ComponentHelper::getParams('com_content')->get('record_hits', 1) == 1;

if (!\count($this->items) && $this->isEmptyState = $model->getIsEmptyState()) {
$this->setLayout('emptystate');
}

$this->items = $model->getItems();
$this->pagination = $model->getPagination();
$this->state = $model->getState();
$this->filterForm = $model->getFilterForm();
$this->activeFilters = $model->getActiveFilters();
$this->vote = PluginHelper::isEnabled('content', 'vote');
$this->hits = ComponentHelper::getParams('com_content')->get('record_hits', 1) == 1;

if (!\count($this->items) && $this->isEmptyState = $model->getIsEmptyState()) {
$this->setLayout('emptystate');
}

if (ComponentHelper::getParams('com_content')->get('workflow_enabled')) {
PluginHelper::importPlugin('workflow');

$this->transitions = $model->getTransitions();
}
if (ComponentHelper::getParams('com_content')->get('workflow_enabled')) {
PluginHelper::importPlugin('workflow');

// Check for errors.
if (\count($errors = $model->getErrors()) || $this->transitions === false) {
throw new GenericDataException(implode("\n", $errors), 500);
$this->transitions = $model->getTransitions();
}
} catch (\Exception $e) {
throw new GenericDataException($e->getMessage(), 500, $e);
}

// We don't need toolbar in the modal window.
Expand Down
4 changes: 4 additions & 0 deletions libraries/src/MVC/Model/BaseDatabaseModel.php
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,10 @@ public function getTable($name = '', $prefix = '', $options = [])
}

if ($table = $this->_createTable($name, $prefix, $options)) {
if ($this->shouldUseExceptions()) {
$table->setUseExceptions(true);
}

return $table;
}

Expand Down
8 changes: 8 additions & 0 deletions libraries/src/MVC/Model/ListModel.php
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,10 @@ public function getItems()
// Load the list items and add the items to the internal cache.
$this->cache[$store] = $this->_getList($this->_getListQuery(), $this->getStart(), $this->getState('list.limit'));
} catch (\RuntimeException $e) {
if ($this->shouldUseExceptions()) {
throw $e;
}

$this->setError($e->getMessage());

return false;
Expand Down Expand Up @@ -360,6 +364,10 @@ public function getTotal()
// Load the total and add the total to the internal cache.
$this->cache[$store] = (int) $this->_getListCount($this->_getListQuery());
} catch (\RuntimeException $e) {
if ($this->shouldUseExceptions()) {
throw $e;
}

$this->setError($e->getMessage());

return false;
Expand Down
51 changes: 46 additions & 5 deletions libraries/src/Object/LegacyErrorHandlingTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
*
* @since 4.3.0
*
* @deprecated 4.3 will be removed in 6.0
* @deprecated 4.3 will be removed in 7.0
* Will be removed without replacement
* Throw an Exception instead of setError
*/
Expand All @@ -36,6 +36,15 @@ trait LegacyErrorHandlingTrait
protected $_errors = [];
// phpcs:enable PSR2.Classes.PropertyDeclaration

/**
* Use exceptions rather than getError/setError.
*
* @var boolean
* @since __DEPLOY_VERSION__
* @deprecated 7.0
*/
private bool $useExceptions = false;

/**
* Get the most recent error message.
*
Expand All @@ -46,7 +55,7 @@ trait LegacyErrorHandlingTrait
*
* @since 1.7.0
*
* @deprecated 3.1.4 will be removed in 6.0
* @deprecated 3.1.4 will be removed in 7.0
* Will be removed without replacement
* Catch thrown Exceptions instead of getError
*/
Expand Down Expand Up @@ -78,7 +87,7 @@ public function getError($i = null, $toString = true)
*
* @since 1.7.0
*
* @deprecated 3.1.4 will be removed in 6.0
* @deprecated 3.1.4 will be removed in 7.0
* Will be removed without replacement
* Catch thrown Exceptions instead of getErrors
*/
Expand All @@ -96,12 +105,44 @@ public function getErrors()
*
* @since 1.7.0
*
* @deprecated 3.1.4 will be removed in 6.0
* @deprecated 3.1.4 will be removed in 7.0
* Will be removed without replacement
* Throw an Exception instead of using setError
*/
public function setError($error)
{
$this->_errors[] = $error;
if (!$this->useExceptions || $error === '') {
$this->_errors[] = $error;
} else {
throw new \Exception($error, 500);
}
}

/**
* If true then subclasses should throw exceptions rather than use getError and setError.
*
* @return boolean
*
* @since __DEPLOY_VERSION__
* @deprecated 7.0
*/
public function shouldUseExceptions(): bool
{
return $this->useExceptions;
}

/**
* If true then subclasses should throw exceptions rather than use getError and setError.
*
* @param boolean $value The value to set for the field.
*
* @return void
*
* @since __DEPLOY_VERSION__
* @deprecated 7.0
*/
public function setUseExceptions(bool $value): void
{
$this->useExceptions = $value;
}
}
1 change: 1 addition & 0 deletions tests/Unit/Libraries/Cms/Object/CMSObjectTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ public function testGetProperties()
'_privateproperty1' => 'valuep1',
'property1' => 'value1',
'property2' => 5,
'useExceptions' => false,
],
$object->getProperties(false),
'Should get all properties, including private ones'
Expand Down