From c3744cb7553e3d56d0b17edd1045b90c4a56fb06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Thu, 18 Jan 2018 19:27:13 -0200 Subject: [PATCH] Refactor PhpMyAdmin\BrowseForeigners class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Static methods have been replaced with instance methods Signed-off-by: Maurício Meneghini Fauth --- browse_foreigners.php | 16 ++-- libraries/classes/BrowseForeigners.php | 86 ++++++++++-------- test/classes/BrowseForeignersTest.php | 119 ++++++++++++------------- 3 files changed, 116 insertions(+), 105 deletions(-) diff --git a/browse_foreigners.php b/browse_foreigners.php index 6b31352799..8412321cc9 100644 --- a/browse_foreigners.php +++ b/browse_foreigners.php @@ -38,10 +38,15 @@ $header->setBodyId('body_browse_foreigners'); /** * Displays the frame */ - $foreigners = Relation::getForeigners($db, $table); -$foreign_limit = BrowseForeigners::getForeignLimit( +$browseForeigners = new BrowseForeigners( + $GLOBALS['cfg']['LimitChars'], $GLOBALS['cfg']['MaxRows'], + $GLOBALS['cfg']['RepeatCells'], + $GLOBALS['cfg']['ShowAll'], + $GLOBALS['pmaThemeImage'] +); +$foreign_limit = $browseForeigners->getForeignLimit( isset($_REQUEST['foreign_showAll']) ? $_REQUEST['foreign_showAll'] : null ); @@ -55,12 +60,7 @@ $foreignData = Relation::getForeignData( ); // HTML output -$html = BrowseForeigners::getHtmlForRelationalFieldSelection( - $GLOBALS['cfg']['RepeatCells'], - $GLOBALS['pmaThemeImage'], - $GLOBALS['cfg']['MaxRows'], - $GLOBALS['cfg']['ShowAll'], - $GLOBALS['cfg']['LimitChars'], +$html = $browseForeigners->getHtmlForRelationalFieldSelection( $db, $table, $_REQUEST['field'], diff --git a/libraries/classes/BrowseForeigners.php b/libraries/classes/BrowseForeigners.php index 2ca1746673..b41ed78f2a 100644 --- a/libraries/classes/BrowseForeigners.php +++ b/libraries/classes/BrowseForeigners.php @@ -18,6 +18,35 @@ use PhpMyAdmin\Util; */ class BrowseForeigners { + private $limitChars; + private $maxRows; + private $repeatCells; + private $showAll; + private $themeImage; + + /** + * Constructor + * + * @param int $limitChars Maximum number of characters to show + * @param int $maxRows Number of rows to display + * @param int $repeatCells Repeat the headers every X cells, or 0 to deactivate + * @param boolean $showAll Shows the 'Show all' button or not + * @param string $themeImage Theme image path + */ + public function __construct( + $limitChars, + $maxRows, + $repeatCells, + $showAll, + $themeImage + ) { + $this->limitChars = (int) $limitChars; + $this->maxRows = (int) $maxRows; + $this->repeatCells = (int) $repeatCells; + $this->showAll = (bool) $showAll; + $this->themeImage = $themeImage; + } + /** * Function to get html for one relational key * @@ -31,10 +60,7 @@ class BrowseForeigners * * @return string $html the generated html */ - public static function getHtmlForOneKey( - $repeatCells, - $pmaThemeImage, - $limitChars, + private function getHtmlForOneKey( $horizontal_count, $header, array $keys, @@ -50,7 +76,7 @@ class BrowseForeigners $rightKeynameIsSelected = false; $leftKeynameIsSelected = false; - if ($repeatCells > 0 && $horizontal_count > $repeatCells) { + if ($this->repeatCells > 0 && $horizontal_count > $this->repeatCells) { $output .= $header; $horizontal_count = 0; } @@ -61,10 +87,7 @@ class BrowseForeigners list( $leftDescription, $leftDescriptionTitle - ) = self::getDescriptionAndTitle( - $limitChars, - $descriptions[$indexByKeyname] - ); + ) = $this->getDescriptionAndTitle($descriptions[$indexByKeyname]); // key names and descriptions for the right section, // sorted by descriptions @@ -72,10 +95,7 @@ class BrowseForeigners list( $rightDescription, $rightDescriptionTitle - ) = self::getDescriptionAndTitle( - $limitChars, - $descriptions[$indexByDescription] - ); + ) = $this->getDescriptionAndTitle($descriptions[$indexByDescription]); $indexByDescription++; @@ -102,7 +122,7 @@ class BrowseForeigners ]); $output .= '' - . ''; $output .= Template::get('table/browse_foreigners/column_element')->render([ @@ -137,12 +157,7 @@ class BrowseForeigners * * @return string */ - public static function getHtmlForRelationalFieldSelection( - $repeatCells, - $pmaThemeImage, - $maxRows, - $showAll, - $limitChars, + public function getHtmlForRelationalFieldSelection( $db, $table, $field, @@ -150,11 +165,11 @@ class BrowseForeigners $fieldkey, $current_value ) { - $gotopage = self::getHtmlForGotoPage($maxRows, $foreignData); + $gotopage = $this->getHtmlForGotoPage($foreignData); $foreignShowAll = Template::get('table/browse_foreigners/show_all')->render([ 'foreign_data' => $foreignData, - 'show_all' => $showAll, - 'max_rows' => $maxRows, + 'show_all' => $this->showAll, + 'max_rows' => $this->maxRows, ]); $output = '
getHtmlForOneKey( $horizontal_count, $header, $keys, @@ -259,9 +271,9 @@ class BrowseForeigners * * @return array the new description and title */ - public static function getDescriptionAndTitle($limitChars, $description) + private function getDescriptionAndTitle($description) { - if (mb_strlen($description) <= $limitChars) { + if (mb_strlen($description) <= $this->limitChars) { $description = htmlspecialchars( $description ); @@ -272,7 +284,7 @@ class BrowseForeigners ); $description = htmlspecialchars( mb_substr( - $description, 0, $limitChars + $description, 0, $this->limitChars ) . '...' ); @@ -287,7 +299,7 @@ class BrowseForeigners * * @return string */ - public static function getHtmlForGotoPage($maxRows, $foreignData) + private function getHtmlForGotoPage($foreignData) { $gotopage = ''; isset($_REQUEST['pos']) ? $pos = $_REQUEST['pos'] : $pos = 0; @@ -295,13 +307,13 @@ class BrowseForeigners return $gotopage; } - $pageNow = @floor($pos / $maxRows) + 1; - $nbTotalPage = @ceil($foreignData['the_total'] / $maxRows); + $pageNow = @floor($pos / $this->maxRows) + 1; + $nbTotalPage = @ceil($foreignData['the_total'] / $this->maxRows); - if ($foreignData['the_total'] > $maxRows) { + if ($foreignData['the_total'] > $this->maxRows) { $gotopage = Util::pageselector( 'pos', - $maxRows, + $this->maxRows, $pageNow, $nbTotalPage, 200, @@ -323,12 +335,12 @@ class BrowseForeigners * * @return string */ - public static function getForeignLimit($maxRows, $foreignShowAll) + public function getForeignLimit($foreignShowAll) { if (isset($foreignShowAll) && $foreignShowAll == __('Show all')) { return null; } isset($_REQUEST['pos']) ? $pos = $_REQUEST['pos'] : $pos = 0; - return 'LIMIT ' . $pos . ', ' . intval($maxRows) . ' '; + return 'LIMIT ' . $pos . ', ' . $this->maxRows . ' '; } } diff --git a/test/classes/BrowseForeignersTest.php b/test/classes/BrowseForeignersTest.php index 4d0bc8b52d..9dcb7eee5e 100644 --- a/test/classes/BrowseForeignersTest.php +++ b/test/classes/BrowseForeignersTest.php @@ -9,6 +9,7 @@ namespace PhpMyAdmin\Tests; use PhpMyAdmin\BrowseForeigners; use PHPUnit\Framework\TestCase; +use ReflectionClass; /** * Tests for PhpMyAdmin\BrowseForeigners @@ -17,18 +18,36 @@ use PHPUnit\Framework\TestCase; */ class BrowseForeignersTest extends TestCase { + private $browseForeigners; + /** * Setup for test cases * * @return void */ - public function setUp() + protected function setUp() { - $GLOBALS['cfg']['LimitChars'] = 50; - $GLOBALS['cfg']['MaxRows'] = 25; - $GLOBALS['cfg']['RepeatCells'] = 100; - $GLOBALS['cfg']['ShowAll'] = false; - $GLOBALS['pmaThemeImage'] = ''; + $this->browseForeigners = new BrowseForeigners(50, 25, 100, false, ''); + } + + /** + * Call protected functions by setting visibility to public. + * + * @param string $name method name + * @param array $params parameters for the invocation + * @param BrowseForeigners $object BrowseForeigners instance object + * + * @return mixed the output from the protected method. + */ + private function callProtectedMethod($name, $params, BrowseForeigners $object = null) + { + $class = new ReflectionClass(BrowseForeigners::class); + $method = $class->getMethod($name); + $method->setAccessible(true); + return $method->invokeArgs( + $object !== null ? $object : $this->browseForeigners, + $params + ); } /** @@ -39,46 +58,37 @@ class BrowseForeignersTest extends TestCase function testGetForeignLimit() { $this->assertNull( - BrowseForeigners::getForeignLimit( - $GLOBALS['cfg']['MaxRows'], - 'Show all' - ) + $this->browseForeigners->getForeignLimit('Show all') ); $this->assertEquals( 'LIMIT 0, 25 ', - BrowseForeigners::getForeignLimit( - $GLOBALS['cfg']['MaxRows'], - null - ) + $this->browseForeigners->getForeignLimit(null) ); $_REQUEST['pos'] = 10; $this->assertEquals( 'LIMIT 10, 25 ', - BrowseForeigners::getForeignLimit( - $GLOBALS['cfg']['MaxRows'], - null - ) + $this->browseForeigners->getForeignLimit(null) ); - $GLOBALS['cfg']['MaxRows'] = 50; - - $this->assertEquals( - 'LIMIT 10, 50 ', - BrowseForeigners::getForeignLimit( - $GLOBALS['cfg']['MaxRows'], - null - ) + $browseForeigners = new BrowseForeigners( + 50, + 50, + 100, + false, + '' ); $this->assertEquals( 'LIMIT 10, 50 ', - BrowseForeigners::getForeignLimit( - $GLOBALS['cfg']['MaxRows'], - 'xyz' - ) + $browseForeigners->getForeignLimit(null) + ); + + $this->assertEquals( + 'LIMIT 10, 50 ', + $browseForeigners->getForeignLimit('xyz') ); } @@ -91,9 +101,9 @@ class BrowseForeignersTest extends TestCase { $this->assertEquals( '', - BrowseForeigners::getHtmlForGotoPage( - $GLOBALS['cfg']['MaxRows'], - null + $this->callProtectedMethod( + 'getHtmlForGotoPage', + [null] ) ); @@ -104,16 +114,16 @@ class BrowseForeignersTest extends TestCase $this->assertEquals( '', - BrowseForeigners::getHtmlForGotoPage( - $GLOBALS['cfg']['MaxRows'], - $foreignData + $this->callProtectedMethod( + 'getHtmlForGotoPage', + [$foreignData] ) ); $foreignData['the_total'] = 30; - $result = BrowseForeigners::getHtmlForGotoPage( - $GLOBALS['cfg']['MaxRows'], - $foreignData + $result = $this->callProtectedMethod( + 'getHtmlForGotoPage', + [$foreignData] ); $this->assertStringStartsWith( @@ -150,24 +160,24 @@ class BrowseForeignersTest extends TestCase */ function testGetDescriptionAndTitle() { - $GLOBALS['cfg']['LimitChars'] = 30; $desc = 'foobarassertEquals( array('foobar<baz', ''), - BrowseForeigners::getDescriptionAndTitle( - $GLOBALS['cfg']['LimitChars'], - $desc + $this->callProtectedMethod( + 'getDescriptionAndTitle', + [$desc] ) ); - $GLOBALS['cfg']['LimitChars'] = 5; + $browseForeigners = new BrowseForeigners(5, 25, 100, false, ''); $this->assertEquals( array('fooba...', 'foobar<baz'), - BrowseForeigners::getDescriptionAndTitle( - $GLOBALS['cfg']['LimitChars'], - $desc + $this->callProtectedMethod( + 'getDescriptionAndTitle', + [$desc], + $browseForeigners ) ); } @@ -188,12 +198,7 @@ class BrowseForeignersTest extends TestCase $current_value = ''; $_REQUEST['rownumber'] = 1; $_REQUEST['foreign_filter'] = '5'; - $result = BrowseForeigners::getHtmlForRelationalFieldSelection( - $GLOBALS['cfg']['RepeatCells'], - $GLOBALS['pmaThemeImage'], - $GLOBALS['cfg']['MaxRows'], - $GLOBALS['cfg']['ShowAll'], - $GLOBALS['cfg']['LimitChars'], + $result = $this->browseForeigners->getHtmlForRelationalFieldSelection( $db, $table, $field, @@ -264,13 +269,7 @@ class BrowseForeignersTest extends TestCase $foreignData['disp_row'] = array(); $foreignData['the_total'] = 5; - $GLOBALS['cfg']['ShowAll'] = false; - $result = BrowseForeigners::getHtmlForRelationalFieldSelection( - $GLOBALS['cfg']['RepeatCells'], - $GLOBALS['pmaThemeImage'], - $GLOBALS['cfg']['MaxRows'], - $GLOBALS['cfg']['ShowAll'], - $GLOBALS['cfg']['LimitChars'], + $result = $this->browseForeigners->getHtmlForRelationalFieldSelection( $db, $table, $field,