Skip to content

Commit

Permalink
Improve JInstallerAdapterComponent::_buildAdminMenus
Browse files Browse the repository at this point in the history 
Changes:
* Type hinting for $table
* There can be more than one root menu items for a single component if the installation previously failed without rollback (e.g. timeout error)
* The extension ID must always be fetched from the #__extensions table, not the old menu items for the same reasons as above
* Just do not create the menu if $menuElement not exist (it was a TODO item for as long as I remember developing for Joomla!)
* Unnecessary code duplication creating the root menu item
* Refactored the code, moving the actual menu item creation in the _createAdminMenuItem method
* If a submenu already existed with the same parent item the installation would fail. Now we try removing it first, just like we do for the root menu item.
  • Loading branch information
Nicholas K. Dionysopoulos committed Dec 23, 2014
1 parent 6c2f0cc commit 2841b1b
Showing 1 changed file with 115 additions and 113 deletions.
228 changes: 115 additions & 113 deletions libraries/cms/installer/adapter/component.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1166,7 +1166,10 @@ public function uninstall($id)
protected function _buildAdminMenus()
{
$db = $this->parent->getDbo();

/** @var JTableMenu $table */
$table = JTable::getInstance('menu');

$option = $this->get('element');

// If a component exists with this option in the table then we don't need to add menus
Expand All @@ -1179,52 +1182,62 @@ protected function _buildAdminMenus()
->where('e.element = ' . $db->quote($option));

$db->setQuery($query);
$componentrow = $db->loadObject();

// In case of a failed installation (e.g. timeout error) we may have duplicate menu item and extension records.
$componentrows = $db->loadObjectList();

// Check if menu items exist
if ($componentrow)
if (!empty($componentrows))
{
// Don't do anything if overwrite has not been enabled
if (!$this->parent->isOverwrite())
{
return true;
}

// Remove existing menu items if overwrite has been enabled
if ($option)
// Remove all menu items
foreach ($componentrows as $componentrow)
{
// If something goes wrong, there's no way to rollback TODO: Search for better solution
$this->_removeAdminMenus($componentrow->extension_id);
// Remove existing menu items if overwrite has been enabled
if ($option)
{
// If something goes wrong, there's no way to rollback TODO: Search for better solution
$this->_removeAdminMenus($componentrow->extension_id);
}
}

$component_id = $componentrow->extension_id;
}
else
{
// Lets find the extension id
$query->clear()
->select('e.extension_id')
->from('#__extensions AS e')
->where('e.type = ' . $db->quote('component'))
->where('e.element = ' . $db->quote($option));

$db->setQuery($query);
// Lets find the extension id
$query->clear()
->select('e.extension_id')
->from('#__extensions AS e')
->where('e.type = ' . $db->quote('component'))
->where('e.element = ' . $db->quote($option));

// @TODO: Find Some better way to discover the component_id
$component_id = $db->loadResult();
}
$db->setQuery($query);
$component_id = $db->loadResult();

// Ok, now its time to handle the menus. Start with the component root menu, then handle submenus.
$menuElement = $this->manifest->administration->menu;

// @TODO: Just do not create the menu if $menuElement not exist
// Just do not create the menu if $menuElement not exist
if (!$menuElement)
{
return true;
}

// If the menu item is hidden do nothing more, just return
if (in_array((string) $menuElement['hidden'], array('true', 'hidden')))
{
return true;
}
elseif ($menuElement)

// Let's figure out what the menu item data should look like
$data = array();

if ($menuElement)
{
$data = array();
// I have a menu element, use this information
$data['menutype'] = 'main';
$data['client_id'] = 1;
$data['title'] = (string) trim($menuElement);
Expand All @@ -1236,73 +1249,10 @@ protected function _buildAdminMenus()
$data['component_id'] = $component_id;
$data['img'] = ((string) $menuElement->attributes()->img) ? (string) $menuElement->attributes()->img : 'class:component';
$data['home'] = 0;

try
{
$table->setLocation(1, 'last-child');
}
catch (InvalidArgumentException $e)
{
JLog::add($e->getMessage(), JLog::WARNING, 'jerror');

return false;
}

if (!$table->bind($data) || !$table->check() || !$table->store())
{
// The menu item already exists. Delete it and retry instead of throwing an error.
$query->clear()
->select('id')
->from('#__menu')
->where('menutype = ' . $db->quote('main'))
->where('client_id = 1')
->where('link = ' . $db->quote('index.php?option=' . $option))
->where('type = ' . $db->quote('component'))
->where('parent_id = 1')
->where('home = 0');

$db->setQuery($query);
$menu_id = $db->loadResult();

if (!$menu_id)
{
// Oops! Could not get the menu ID. Go back and rollback changes.
JError::raiseWarning(1, $table->getError());

return false;
}
else
{
// Remove the old menu item
$query->clear()
->delete('#__menu')
->where('id = ' . (int) $menu_id);

$db->setQuery($query);
$db->query();

// Retry creating the menu item
$table->setLocation(1, 'last-child');

if (!$table->bind($data) || !$table->check() || !$table->store())
{
// Install failed, warn user and rollback changes
JError::raiseWarning(1, $table->getError());

return false;
}
}
}

/*
* Since we have created a menu item, we add it to the installation step stack
* so that if we have to rollback the changes we can undo it.
*/
$this->parent->pushStep(array('type' => 'menu', 'id' => $component_id));
}
// No menu element was specified, Let's make a generic menu item
else
{
// No menu element was specified, Let's make a generic menu item
$data = array();
$data['menutype'] = 'main';
$data['client_id'] = 1;
Expand All @@ -1315,31 +1265,25 @@ protected function _buildAdminMenus()
$data['component_id'] = $component_id;
$data['img'] = 'class:component';
$data['home'] = 0;
}

try
{
$table->setLocation(1, 'last-child');
}
catch (InvalidArgumentException $e)
{
JLog::add($e->getMessage(), JLog::WARNING, 'jerror');

return false;
}
// Try to create the menu item in the database
try
{
$table->setLocation(1, 'last-child');
}
catch (InvalidArgumentException $e)
{
JLog::add($e->getMessage(), JLog::WARNING, 'jerror');

if (!$table->bind($data) || !$table->check() || !$table->store())
{
// Install failed, warn user and rollback changes
JLog::add($table->getError(), JLog::WARNING, 'jerror');
return false;
}

return false;
}
$result = $this->_createAdminMenuItem($table, $data);

/*
* Since we have created a menu item, we add it to the installation step stack
* so that if we have to rollback the changes we can undo it.
*/
$this->parent->pushStep(array('type' => 'menu', 'id' => $component_id));
if ($result === false)
{
return $result;
}

/*
Expand All @@ -1348,6 +1292,7 @@ protected function _buildAdminMenus()

if (!$this->manifest->administration->submenu)
{
// No submenu? We're done.
return true;
}

Expand Down Expand Up @@ -1410,8 +1355,6 @@ protected function _buildAdminMenus()
$data['link'] = 'index.php?option=' . $option . $qstring;
}

$table = JTable::getInstance('menu');

try
{
$table->setLocation($parent_id, 'last-child');
Expand All @@ -1421,10 +1364,11 @@ protected function _buildAdminMenus()
return false;
}

if (!$table->bind($data) || !$table->check() || !$table->store())
$result = $this->_createAdminMenuItem($table, $data);

if ($result === false)
{
// Install failed, rollback changes
return false;
return $result;
}

/*
Expand Down Expand Up @@ -1863,6 +1807,64 @@ public function refreshManifestCache()
return false;
}
}

/**
* Creates the menu item in the database. If the item already exists it tries to remove it and create it afresh.
*
* @param JTableMenu &$table The menu table on which to create the menu item
* @param array &$data The menu item data to create
*
* @return bool True on success
*/
protected function _createAdminMenuItem(JTableMenu &$table, array &$data)
{
$db = $this->parent->getDbo();

if ( !$table->bind($data) || !$table->check() || !$table->store())
{
// The menu item already exists. Delete it and retry instead of throwing an error.
$query = $db->getQuery(true)
->select('id')
->from('#__menu')
->where('menutype = ' . $db->q($data['menutype']))
->where('client_id = 1')
->where('link = ' . $db->q($data['link']))
->where('type = ' . $db->q($data['type']))
->where('parent_id = ' . $db->q($data['parent_id']))
->where('home = ' . $db->q($data['home']));

$db->setQuery($query);
$menu_id = $db->loadResult();

if ( !$menu_id)
{
// Oops! Could not get the menu ID. Go back and rollback changes.
JError::raiseWarning(1, $table->getError());

return false;
}
else
{
/** @var JTableMenu $temporaryTable */
$temporaryTable = JTable::getInstance('menu');
$temporaryTable->delete($menu_id, true);
$temporaryTable->rebuild($data['parent_id']);

// Retry creating the menu item
$table->setLocation(1, 'last-child');

if ( !$table->bind($data) || !$table->check() || !$table->store())
{
// Install failed, warn user and rollback changes
JError::raiseWarning(1, $table->getError());

return false;
}
}
}

return true;
}
}

/**
Expand Down

0 comments on commit 2841b1b

Please sign in to comment.