[feature/avatars] Make avatars generic

Adding a cleaning function for data coming from the users/groups
tables so drivers only deal with clean data (ideally). Refactored
get_user_avatar() and get_group_avatar() to use an underlying
get_avatar() function.

PHPBB3-10018
This commit is contained in:
Cullen Walsh 2011-06-18 22:50:52 -07:00
parent d0bb14ded1
commit a06380c69a
6 changed files with 141 additions and 78 deletions

View file

@ -44,6 +44,12 @@ abstract class phpbb_avatar_driver
* @type phpbb_cache_driver_interface * @type phpbb_cache_driver_interface
*/ */
protected $cache; protected $cache;
/**
* @TODO
*/
const FROM_USER = 0;
const FROM_GROUP = 1;
/** /**
* This flag should be set to true if the avatar requires a nonstandard image * This flag should be set to true if the avatar requires a nonstandard image
@ -71,12 +77,12 @@ abstract class phpbb_avatar_driver
/** /**
* Get the avatar url and dimensions * Get the avatar url and dimensions
* *
* @param $ignore_config Whether this function should respect the users/board * @param $ignore_config Whether this function should respect the users prefs
* configuration option, or should just render the avatar anyways. * and board configuration configuration option, or should just render
* Useful for the ACP. * the avatar anyways. Useful for the ACP.
* @return array Avatar data * @return array Avatar data
*/ */
public function get_data($user_row, $ignore_config = false) public function get_data($row, $ignore_config = false)
{ {
return array( return array(
'src' => '', 'src' => '',
@ -89,12 +95,12 @@ abstract class phpbb_avatar_driver
* Returns custom html for displaying this avatar. * Returns custom html for displaying this avatar.
* Only called if $custom_html is true. * Only called if $custom_html is true.
* *
* @param $ignore_config Whether this function should respect the users/board * @param $ignore_config Whether this function should respect the users prefs
* configuration option, or should just render the avatar anyways. * and board configuration configuration option, or should just render
* Useful for the ACP. * the avatar anyways. Useful for the ACP.
* @return string HTML * @return string HTML
*/ */
public function get_custom_html($user_row, $ignore_config = false) public function get_custom_html($row, $ignore_config = false)
{ {
return ''; return '';
} }
@ -102,7 +108,7 @@ abstract class phpbb_avatar_driver
/** /**
* @TODO * @TODO
**/ **/
public function prepare_form($template, $user_row, &$error, &$override_focus) public function prepare_form($template, $row, &$error, &$override_focus)
{ {
return false; return false;
} }
@ -110,7 +116,7 @@ abstract class phpbb_avatar_driver
/** /**
* @TODO * @TODO
**/ **/
public function process_form($template, $user_row, &$error) public function process_form($template, $row, &$error)
{ {
return false; return false;
} }
@ -118,8 +124,49 @@ abstract class phpbb_avatar_driver
/** /**
* @TODO * @TODO
**/ **/
public function delete($user_row) public function delete($row)
{ {
return true; return true;
} }
/**
* @TODO
**/
public static function clean_row($row, $src = phpbb_avatar_driver::FROM_USER)
{
$return = array();
$prefix = false;
if ($src == phpbb_avatar_driver::FROM_USER)
{
$prefix = 'user_';
}
else if ($src == phpbb_avatar_driver::FROM_GROUP)
{
$prefix = 'group_';
}
if ($prefix)
{
$len = strlen($prefix);
foreach ($row as $key => $val)
{
$sub = substr($key, 0, $len);
if ($sub == $prefix)
{
$return[substr($key, $len)] = $val;
}
else
{
$return[$key] = $val;
}
}
}
else
{
$return = $row;
}
return $return;
}
} }

View file

@ -24,14 +24,14 @@ class phpbb_avatar_driver_local extends phpbb_avatar_driver
/** /**
* @inheritdoc * @inheritdoc
*/ */
public function get_data($user_row, $ignore_config = false) public function get_data($row, $ignore_config = false)
{ {
if ($ignore_config || $this->config['allow_avatar_local']) if ($ignore_config || $this->config['allow_avatar_local'])
{ {
return array( return array(
'src' => $this->phpbb_root_path . $this->config['avatar_gallery_path'] . '/' . $user_row['user_avatar'], 'src' => $this->phpbb_root_path . $this->config['avatar_gallery_path'] . '/' . $row['avatar'],
'width' => $user_row['user_avatar_width'], 'width' => $row['avatar_width'],
'height' => $user_row['user_avatar_height'], 'height' => $row['avatar_height'],
); );
} }
else else
@ -47,7 +47,7 @@ class phpbb_avatar_driver_local extends phpbb_avatar_driver
/** /**
* @inheritdoc * @inheritdoc
*/ */
public function prepare_form($template, $user_row, &$error) public function prepare_form($template, $row, &$error)
{ {
$avatar_list = $this->get_avatar_list(); $avatar_list = $this->get_avatar_list();
$category = request_var('av_local_cat', ''); $category = request_var('av_local_cat', '');
@ -83,7 +83,7 @@ class phpbb_avatar_driver_local extends phpbb_avatar_driver
/** /**
* @inheritdoc * @inheritdoc
*/ */
public function process_form($template, $user_row, &$error) public function process_form($template, $row, &$error)
{ {
$avatar_list = $this->get_avatar_list(); $avatar_list = $this->get_avatar_list();
$category = request_var('av_local_cat', ''); $category = request_var('av_local_cat', '');
@ -96,9 +96,9 @@ class phpbb_avatar_driver_local extends phpbb_avatar_driver
} }
return array( return array(
'user_avatar' => $category . '/' . $file, 'avatar' => $category . '/' . $file,
'user_avatar_width' => $avatar_list[$category][urldecode($file)]['width'], 'avatar_width' => $avatar_list[$category][urldecode($file)]['width'],
'user_avatar_height' => $avatar_list[$category][urldecode($file)]['height'], 'avatar_height' => $avatar_list[$category][urldecode($file)]['height'],
); );
} }

View file

@ -24,14 +24,14 @@ class phpbb_avatar_driver_remote extends phpbb_avatar_driver
/** /**
* @inheritdoc * @inheritdoc
*/ */
public function get_data($user_row, $ignore_config = false) public function get_data($row, $ignore_config = false)
{ {
if ($ignore_config || $this->config['allow_avatar_remote']) if ($ignore_config || $this->config['allow_avatar_remote'])
{ {
return array( return array(
'src' => $user_row['user_avatar'], 'src' => $row['avatar'],
'width' => $user_row['user_avatar_width'], 'width' => $row['avatar_width'],
'height' => $user_row['user_avatar_height'], 'height' => $row['avatar_height'],
); );
} }
else else
@ -47,12 +47,12 @@ class phpbb_avatar_driver_remote extends phpbb_avatar_driver
/** /**
* @inheritdoc * @inheritdoc
*/ */
public function prepare_form($template, $user_row, &$error) public function prepare_form($template, $row, &$error)
{ {
$template->assign_vars(array( $template->assign_vars(array(
'AV_REMOTE_WIDTH' => (($user_row['user_avatar_type'] == AVATAR_REMOTE || $user_row['user_avatar_type'] == 'remote') && $user_row['user_avatar_width']) ? $user_row['user_avatar_width'] : request_var('av_local_width', 0), 'AV_REMOTE_WIDTH' => (($row['avatar_type'] == AVATAR_REMOTE || $row['avatar_type'] == 'remote') && $row['avatar_width']) ? $row['avatar_width'] : request_var('av_local_width', 0),
'AV_REMOTE_HEIGHT' => (($user_row['user_avatar_type'] == AVATAR_REMOTE || $user_row['user_avatar_type'] == 'remote') && $user_row['user_avatar_height']) ? $user_row['user_avatar_height'] : request_var('av_local_width', 0), 'AV_REMOTE_HEIGHT' => (($row['avatar_type'] == AVATAR_REMOTE || $row['avatar_type'] == 'remote') && $row['avatar_height']) ? $row['avatar_height'] : request_var('av_local_width', 0),
'AV_REMOTE_URL' => (($user_row['user_avatar_type'] == AVATAR_REMOTE || $user_row['user_avatar_type'] == 'remote') && $user_row['user_avatar']) ? $user_row['user_avatar'] : '', 'AV_REMOTE_URL' => (($row['avatar_type'] == AVATAR_REMOTE || $row['avatar_type'] == 'remote') && $row['avatar']) ? $row['avatar'] : '',
)); ));
return true; return true;
@ -61,7 +61,7 @@ class phpbb_avatar_driver_remote extends phpbb_avatar_driver
/** /**
* @inheritdoc * @inheritdoc
*/ */
public function process_form($template, $user_row, &$error) public function process_form($template, $row, &$error)
{ {
$url = request_var('av_remote_url', ''); $url = request_var('av_remote_url', '');
$width = request_var('av_remote_width', 0); $width = request_var('av_remote_width', 0);
@ -155,9 +155,9 @@ class phpbb_avatar_driver_remote extends phpbb_avatar_driver
} }
return array( return array(
'user_avatar' => $url, 'avatar' => $url,
'user_avatar_width' => $width, 'avatar_width' => $width,
'user_avatar_height' => $height, 'avatar_height' => $height,
); );
} }
} }

View file

@ -24,14 +24,14 @@ class phpbb_avatar_driver_upload extends phpbb_avatar_driver
/** /**
* @inheritdoc * @inheritdoc
*/ */
public function get_data($user_row, $ignore_config = false) public function get_data($row, $ignore_config = false)
{ {
if ($ignore_config || $this->config['allow_avatar_upload']) if ($ignore_config || $this->config['allow_avatar_upload'])
{ {
return array( return array(
'src' => $this->phpbb_root_path . 'download/file.' . $this->phpEx . '?avatar=' . $user_row['user_avatar'], 'src' => $this->phpbb_root_path . 'download/file.' . $this->phpEx . '?avatar=' . $row['avatar'],
'width' => $user_row['user_avatar_width'], 'width' => $row['avatar_width'],
'height' => $user_row['user_avatar_height'], 'height' => $row['avatar_height'],
); );
} }
else else
@ -47,7 +47,7 @@ class phpbb_avatar_driver_upload extends phpbb_avatar_driver
/** /**
* @inheritdoc * @inheritdoc
*/ */
public function prepare_form($template, $user_row, &$error) public function prepare_form($template, $row, &$error)
{ {
if (!$this->can_upload()) if (!$this->can_upload())
{ {
@ -65,7 +65,7 @@ class phpbb_avatar_driver_upload extends phpbb_avatar_driver
/** /**
* @inheritdoc * @inheritdoc
*/ */
public function process_form($template, $user_row, &$error) public function process_form($template, $row, &$error)
{ {
if (!$this->can_upload()) if (!$this->can_upload())
{ {
@ -88,7 +88,7 @@ class phpbb_avatar_driver_upload extends phpbb_avatar_driver
} }
$prefix = $this->config['avatar_salt'] . '_'; $prefix = $this->config['avatar_salt'] . '_';
$file->clean_filename('avatar', $prefix, $user_row['user_id']); $file->clean_filename('avatar', $prefix, $row['id']);
$destination = $this->config['avatar_path']; $destination = $this->config['avatar_path'];
@ -115,19 +115,19 @@ class phpbb_avatar_driver_upload extends phpbb_avatar_driver
} }
return array( return array(
'user_avatar' => $user_row['user_id'] . '_' . time() . '.' . $file->get('extension'), 'avatar' => $row['id'] . '_' . time() . '.' . $file->get('extension'),
'user_avatar_width' => $file->get('width'), 'avatar_width' => $file->get('width'),
'user_avatar_height' => $file->get('height'), 'avatar_height' => $file->get('height'),
); );
} }
/** /**
* @inheritdoc * @inheritdoc
*/ */
public function delete($user_row) public function delete($row)
{ {
$ext = substr(strrchr($user_row['user_avatar'], '.'), 1); $ext = substr(strrchr($row['avatar'], '.'), 1);
$filename = $this->phpbb_root_path . $this->config['avatar_path'] . '/' . $this->config['avatar_salt'] . '_' . $user_row['user_id'] . '.' . $ext; $filename = $this->phpbb_root_path . $this->config['avatar_path'] . '/' . $this->config['avatar_salt'] . '_' . $row['id'] . '.' . $ext;
if (file_exists($filename)) if (file_exists($filename))
{ {

View file

@ -1279,6 +1279,36 @@ function get_user_rank($user_rank, $user_posts, &$rank_title, &$rank_img, &$rank
* @return string Avatar html * @return string Avatar html
*/ */
function get_user_avatar($user_row, $alt = 'USER_AVATAR', $ignore_config = false) function get_user_avatar($user_row, $alt = 'USER_AVATAR', $ignore_config = false)
{
$row = phpbb_avatar_driver::clean_row($user_row, phpbb_avatar_driver::FROM_USER);
return get_avatar($row, $alt, $ignore_config);
}
/**
* Get group avatar
*
* @param array $group_row Row from the groups table
* @param string $alt Optional language string for alt tag within image, can be a language key or text
* @param bool $ignore_config Ignores the config-setting, to be still able to view the avatar in the UCP
*
* @return string Avatar html
*/
function get_group_avatar($user_row, $alt = 'GROUP_AVATAR', $ignore_config = false)
{
$row = phpbb_avatar_driver::clean_row($user_row, phpbb_avatar_driver::FROM_GROUP);
return get_avatar($row, $alt, $ignore_config);
}
/**
* Get avatar
*
* @param array $row Row cleaned by phpbb_avatar_driver::clean_row
* @param string $alt Optional language string for alt tag within image, can be a language key or text
* @param bool $ignore_config Ignores the config-setting, to be still able to view the avatar in the UCP
*
* @return string Avatar html
*/
function get_avatar($row, $alt, $ignore_config = false)
{ {
global $user, $config, $cache, $phpbb_root_path, $phpEx; global $user, $config, $cache, $phpbb_root_path, $phpEx;
@ -1290,12 +1320,12 @@ function get_user_avatar($user_row, $alt = 'USER_AVATAR', $ignore_config = false
} }
$avatar_data = array( $avatar_data = array(
'src' => $user_row['user_avatar'], 'src' => $row['avatar'],
'width' => $user_row['user_avatar_width'], 'width' => $row['avatar_width'],
'height' => $user_row['user_avatar_height'], 'height' => $row['avatar_height'],
); );
switch ($user_row['user_avatar_type']) switch ($row['avatar_type'])
{ {
case AVATAR_UPLOAD: case AVATAR_UPLOAD:
// Compatibility with old avatars // Compatibility with old avatars
@ -1341,16 +1371,16 @@ function get_user_avatar($user_row, $alt = 'USER_AVATAR', $ignore_config = false
$avatar_manager = new phpbb_avatar_manager($phpbb_root_path, $phpEx, $config, $cache->get_driver()); $avatar_manager = new phpbb_avatar_manager($phpbb_root_path, $phpEx, $config, $cache->get_driver());
} }
$avatar = $avatar_manager->get_driver($user_row['user_avatar_type']); $avatar = $avatar_manager->get_driver($row['avatar_type']);
if ($avatar) if ($avatar)
{ {
if ($avatar->custom_html) if ($avatar->custom_html)
{ {
return $avatar->get_html($user_row, $ignore_config); return $avatar->get_html($row, $ignore_config);
} }
$avatar_data = $avatar->get_data($user_row, $ignore_config); $avatar_data = $avatar->get_data($row, $ignore_config);
} }
else else
{ {
@ -1372,25 +1402,3 @@ function get_user_avatar($user_row, $alt = 'USER_AVATAR', $ignore_config = false
return $html; return $html;
} }
/**
* Get group avatar
*
* @param array $group_row Row from the groups table
* @param string $alt Optional language string for alt tag within image, can be a language key or text
* @param bool $ignore_config Ignores the config-setting, to be still able to view the avatar in the UCP
*
* @return string Avatar html
*/
function get_group_avatar($group_row, $alt = 'GROUP_AVATAR', $ignore_config = false)
{
// Kind of abusing this functionality...
$avatar_row = array(
'user_avatar' => $group_row['group_avatar'],
'user_avatar_type' => $group_row['group_avatar_type'],
'user_avatar_width' => $group_row['group_avatar_width'],
'user_avatar_height' => $group_row['group_avatar_height'],
);
return get_user_avatar($group_row, $alt, $ignore_config);
}

View file

@ -556,6 +556,9 @@ class ucp_profile
$avatar_drivers = $avatar_manager->get_valid_drivers(); $avatar_drivers = $avatar_manager->get_valid_drivers();
sort($avatar_drivers); sort($avatar_drivers);
// This is normalised data, without the user_ prefix
$avatar_data = phpbb_avatar_driver::clean_row($user->data, phpbb_avatar_driver::FROM_USER);
if ($submit) if ($submit)
{ {
@ -565,12 +568,17 @@ class ucp_profile
if (in_array($driver, $avatar_drivers) && $config["allow_avatar_$driver"]) if (in_array($driver, $avatar_drivers) && $config["allow_avatar_$driver"])
{ {
$avatar = $avatar_manager->get_driver($driver); $avatar = $avatar_manager->get_driver($driver);
$result = $avatar->process_form($template, $user->data, $error); $result = $avatar->process_form($template, $avatar_data, $error);
if ($result && empty($error)) if ($result && empty($error))
{ {
// Success! Lets save the result in the database // Success! Lets save the result in the database
$result['user_avatar_type'] = $driver; $result = array(
'user_avatar_type' => $driver,
'user_avatar' => $result['avatar'],
'user_avatar_width' => $result['avatar_width'],
'user_avatar_height' => $result['avatar_height'],
);
$sql = 'UPDATE ' . USERS_TABLE . ' $sql = 'UPDATE ' . USERS_TABLE . '
SET ' . $db->sql_build_array('UPDATE', $result) . ' SET ' . $db->sql_build_array('UPDATE', $result) . '
@ -588,7 +596,7 @@ class ucp_profile
// They are removing their avatar or are trying to play games with us // They are removing their avatar or are trying to play games with us
if ($avatar = $avatar_manager->get_driver($user->data['user_avatar_type'])) if ($avatar = $avatar_manager->get_driver($user->data['user_avatar_type']))
{ {
$avatar->delete($user->data); $avatar->delete($avatar_data);
} }
$result = array( $result = array(
@ -628,7 +636,7 @@ class ucp_profile
$avatar = $avatar_manager->get_driver($driver); $avatar = $avatar_manager->get_driver($driver);
if ($avatar->prepare_form($template, $user->data, $error)) if ($avatar->prepare_form($template, $avatar_data, $error))
{ {
$driver_u = strtoupper($driver); $driver_u = strtoupper($driver);