patternphpMinor
Large Zend_Db query
Viewed 0 times
zend_dblargequery
Problem
Is there a better way to accomplish the following?
/**
* Performs an access check given a user.
*
* @param Cas_Acl_Sid $user The user or SID being checked.
* @param Cas_Acl_Privilege $privilege The privilege to check.
* @return int|null 1 if user access allowed, 2 if group access allowed, false if access is denied, null if access cannot be determined.
*/
public function accessCheck(Cas_Acl_Sid $user, Cas_Acl_Privilege $privilege)
{
$db = Zend_Db_Table_Abstract::getDefaultAdapter();
$usersQuery = $db->select()->from('AccessControlEntries', array('Allowed', new Zend_Db_Expr('1 AS Type')))
->where('Acl = ?', $this->_id)
->where('Sid = ?', $user->GetGuid())
->where('Privilege = ?', $privilege->GetId());
$groupsQuery = $db->select()->from('AccessControlEntries', array('Allowed', new Zend_Db_Expr('2 AS Type')))
->join('GroupMembers', $db->quoteIdentifier(array('GroupMembers', 'Group')) . ' = ' .
$db->quoteIdentifier(array('AccessControlEntries', 'Sid')), array())
->where('Acl = ?', $this->_id)
->where($db->quoteIdentifier(array('GroupMembers', 'User')) . ' = ?', $user->GetGuid())
->where('Privilege = ?', $privilege->GetId());
$query = $db->select()
->union(array($usersQuery, $groupsQuery), Zend_Db_Select::SQL_UNION_ALL)
->order('Type')
->order('Allowed')
->limit(1);
$dbResult = $db->fetchAll($query);
if (!count($dbResult))
{
return null;
}
else {
if ($dbResult[0]['Allowed'])
{
return (int)$dbResult[0]['Type'];
}
else
{
return false;
}
}
}Solution
Stored procedure
The query above is enough big to move it from the application layer to the database in the form of a stored procedure. It will be clearer and faster, the only disadvantage is that an SP has a hard dependency on the database type (MySQL, MSSQL, other).
Return value(s)
You are returning 3 types in your method: null, integer and boolean.
Keep your logic clean, and return always one type if you can (yes, PHP has it's type juggling "issue" but still, clean code talks).
The query above is enough big to move it from the application layer to the database in the form of a stored procedure. It will be clearer and faster, the only disadvantage is that an SP has a hard dependency on the database type (MySQL, MSSQL, other).
Return value(s)
You are returning 3 types in your method: null, integer and boolean.
Keep your logic clean, and return always one type if you can (yes, PHP has it's type juggling "issue" but still, clean code talks).
public function accessCheck(Cas_Acl_Sid $user, Cas_Acl_Privilege $privilege)
{
//execute the query in a stored procedure
$dbResult = // stored procedure result
if (empty($dbResult) || !$dbResult[0]['Allowed'])
{
return 0;
}
return (int)$dbResult[0]['Type'];
}Code Snippets
public function accessCheck(Cas_Acl_Sid $user, Cas_Acl_Privilege $privilege)
{
//execute the query in a stored procedure
$dbResult = // stored procedure result
if (empty($dbResult) || !$dbResult[0]['Allowed'])
{
return 0;
}
return (int)$dbResult[0]['Type'];
}Context
StackExchange Code Review Q#676, answer score: 2
Revisions (0)
No revisions yet.