From c51fae92dc7d3080def81a2797e0d683b3e6d82a Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Tue, 23 Feb 2016 19:21:14 +0100 Subject: [PATCH] Allow crossed search between terms and tags * Partial fix of #449 * Current use case: search term + click on tag. * LinkFilter now returns all links if no filter is given. * Unit tests. --- application/LinkDB.php | 3 +- application/LinkFilter.php | 27 +++++++-- inc/shaarli.css | 4 ++ index.php | 120 ++++++++++++++++++++++++------------- tests/LinkFilterTest.php | 51 +++++++++++++++- tpl/linklist.html | 38 ++++++------ 6 files changed, 178 insertions(+), 65 deletions(-) diff --git a/application/LinkDB.php b/application/LinkDB.php index 9488ac45..1b505620 100644 --- a/application/LinkDB.php +++ b/application/LinkDB.php @@ -353,8 +353,7 @@ public function getLinkFromUrl($url) public function filter($type = '', $request = '', $casesensitive = false, $privateonly = false) { $linkFilter = new LinkFilter($this->_links); - $requestFilter = is_array($request) ? implode(' ', $request) : $request; - return $linkFilter->filter($type, trim($requestFilter), $casesensitive, $privateonly); + return $linkFilter->filter($type, $request, $casesensitive, $privateonly); } /** diff --git a/application/LinkFilter.php b/application/LinkFilter.php index 17594e8f..3fd803cb 100644 --- a/application/LinkFilter.php +++ b/application/LinkFilter.php @@ -55,16 +55,25 @@ public function filter($type, $request, $casesensitive = false, $privateonly = f switch($type) { case self::$FILTER_HASH: return $this->filterSmallHash($request); - break; + case self::$FILTER_TAG | self::$FILTER_TEXT: + if (!empty($request)) { + $filtered = $this->links; + if (isset($request[0])) { + $filtered = $this->filterTags($request[0], $casesensitive, $privateonly); + } + if (isset($request[1])) { + $lf = new LinkFilter($filtered); + $filtered = $lf->filterFulltext($request[1], $privateonly); + } + return $filtered; + } + return $this->noFilter($privateonly); case self::$FILTER_TEXT: return $this->filterFulltext($request, $privateonly); - break; case self::$FILTER_TAG: return $this->filterTags($request, $casesensitive, $privateonly); - break; case self::$FILTER_DAY: return $this->filterDay($request); - break; default: return $this->noFilter($privateonly); } @@ -138,6 +147,10 @@ private function filterSmallHash($smallHash) */ private function filterFulltext($searchterms, $privateonly = false) { + if (empty($searchterms)) { + return $this->links; + } + $filtered = array(); $search = mb_convert_case(html_entity_decode($searchterms), MB_CASE_LOWER, 'UTF-8'); $exactRegex = '/"([^"]+)"/'; @@ -219,6 +232,12 @@ private function filterFulltext($searchterms, $privateonly = false) */ public function filterTags($tags, $casesensitive = false, $privateonly = false) { + // Implode if array for clean up. + $tags = is_array($tags) ? trim(implode(' ', $tags)) : $tags; + if (empty($tags)) { + return $this->links; + } + $searchtags = self::tagsStrToArray($tags, $casesensitive); $filtered = array(); if (empty($searchtags)) { diff --git a/inc/shaarli.css b/inc/shaarli.css index 8a7409b2..2e41988e 100644 --- a/inc/shaarli.css +++ b/inc/shaarli.css @@ -33,6 +33,10 @@ h1 { margin-bottom: 20px; } +em { + font-style: italic; +} + /* Buttons */ .bigbutton { background-color: #c0c0c0; diff --git a/index.php b/index.php index 5bd9cac4..c2bec1db 100644 --- a/index.php +++ b/index.php @@ -623,7 +623,7 @@ private function initialize() if (!empty($_GET['searchtags'])) { $searchcrits .= '&searchtags=' . urlencode($_GET['searchtags']); } - elseif (!empty($_GET['searchterm'])) { + if (!empty($_GET['searchterm'])) { $searchcrits .= '&searchterm=' . urlencode($_GET['searchterm']); } $this->tpl->assign('searchcrits', $searchcrits); @@ -709,11 +709,19 @@ function showRSS() // Read links from database (and filter private links if user it not logged in). // Optionally filter the results: - if (!empty($_GET['searchterm'])) { - $linksToDisplay = $LINKSDB->filter(LinkFilter::$FILTER_TEXT, $_GET['searchterm']); + $searchtags = !empty($_GET['searchtags']) ? escape($_GET['searchtags']) : ''; + $searchterm = !empty($_GET['searchterm']) ? escape($_GET['searchterm']) : ''; + if (! empty($searchtags) && ! empty($searchterm)) { + $linksToDisplay = $LINKSDB->filter( + LinkFilter::$FILTER_TAG | LinkFilter::$FILTER_TEXT, + array($searchtags, $searchterm) + ); } - elseif (!empty($_GET['searchtags'])) { - $linksToDisplay = $LINKSDB->filter(LinkFilter::$FILTER_TAG, trim($_GET['searchtags'])); + elseif ($searchtags) { + $linksToDisplay = $LINKSDB->filter(LinkFilter::$FILTER_TAG, $searchtags); + } + elseif ($searchterm) { + $linksToDisplay = $LINKSDB->filter(LinkFilter::$FILTER_TEXT, $searchterm); } else { $linksToDisplay = $LINKSDB; @@ -807,11 +815,19 @@ function showATOM() ); // Optionally filter the results: - if (!empty($_GET['searchterm'])) { - $linksToDisplay = $LINKSDB->filter(LinkFilter::$FILTER_TEXT, $_GET['searchterm']); + $searchtags = !empty($_GET['searchtags']) ? escape($_GET['searchtags']) : ''; + $searchterm = !empty($_GET['searchterm']) ? escape($_GET['searchterm']) : ''; + if (! empty($searchtags) && ! empty($searchterm)) { + $linksToDisplay = $LINKSDB->filter( + LinkFilter::$FILTER_TAG | LinkFilter::$FILTER_TEXT, + array($searchtags, $searchterm) + ); } - else if (!empty($_GET['searchtags'])) { - $linksToDisplay = $LINKSDB->filter(LinkFilter::$FILTER_TAG, trim($_GET['searchtags'])); + elseif ($searchtags) { + $linksToDisplay = $LINKSDB->filter(LinkFilter::$FILTER_TAG, $searchtags); + } + elseif ($searchterm) { + $linksToDisplay = $LINKSDB->filter(LinkFilter::$FILTER_TEXT, $searchterm); } else { $linksToDisplay = $LINKSDB; @@ -1165,11 +1181,19 @@ function renderPage() if ($targetPage == Router::$PAGE_PICWALL) { // Optionally filter the results: - if (!empty($_GET['searchterm'])) { - $links = $LINKSDB->filter(LinkFilter::$FILTER_TEXT, $_GET['searchterm']); + $searchtags = !empty($_GET['searchtags']) ? escape($_GET['searchtags']) : ''; + $searchterm = !empty($_GET['searchterm']) ? escape($_GET['searchterm']) : ''; + if (! empty($searchtags) && ! empty($searchterm)) { + $links = $LINKSDB->filter( + LinkFilter::$FILTER_TAG | LinkFilter::$FILTER_TEXT, + array($searchtags, $searchterm) + ); } - elseif (! empty($_GET['searchtags'])) { - $links = $LINKSDB->filter(LinkFilter::$FILTER_TAG, trim($_GET['searchtags'])); + elseif ($searchtags) { + $links = $LINKSDB->filter(LinkFilter::$FILTER_TAG, $searchtags); + } + elseif ($searchterm) { + $links = $LINKSDB->filter(LinkFilter::$FILTER_TEXT, $searchterm); } else { $links = $LINKSDB; @@ -1963,29 +1987,46 @@ function importFile() // This function fills all the necessary fields in the $PAGE for the template 'linklist.html' function buildLinkList($PAGE,$LINKSDB) { - // ---- Filter link database according to parameters - $search_type = ''; - $search_crits = ''; + // Filter link database according to parameters. + $searchtags = !empty($_GET['searchtags']) ? escape($_GET['searchtags']) : ''; + $searchterm = !empty($_GET['searchterm']) ? escape(trim($_GET['searchterm'])) : ''; $privateonly = !empty($_SESSION['privateonly']) ? true : false; - // Fulltext search - if (isset($_GET['searchterm'])) { - $search_crits = escape(trim($_GET['searchterm'])); - $search_type = LinkFilter::$FILTER_TEXT; - $linksToDisplay = $LINKSDB->filter($search_type, $search_crits, false, $privateonly); + // Search tags + fullsearch. + if (! empty($searchtags) && ! empty($searchterm)) { + $linksToDisplay = $LINKSDB->filter( + LinkFilter::$FILTER_TAG | LinkFilter::$FILTER_TEXT, + array($searchtags, $searchterm), + false, + $privateonly + ); } - // Search by tag - elseif (isset($_GET['searchtags'])) { - $search_crits = explode(' ', escape(trim($_GET['searchtags']))); - $search_type = LinkFilter::$FILTER_TAG; - $linksToDisplay = $LINKSDB->filter($search_type, $search_crits, false, $privateonly); + // Search by tags. + elseif (! empty($searchtags)) { + $linksToDisplay = $LINKSDB->filter( + LinkFilter::$FILTER_TAG, + $searchtags, + false, + $privateonly + ); + } + // Fulltext search. + elseif (! empty($searchterm)) { + $linksToDisplay = $LINKSDB->filter( + LinkFilter::$FILTER_TEXT, + $searchterm, + false, + $privateonly + ); } // Detect smallHashes in URL. - elseif (isset($_SERVER['QUERY_STRING']) - && preg_match('/[a-zA-Z0-9-_@]{6}(&.+?)?/', $_SERVER['QUERY_STRING'])) { - $search_type = LinkFilter::$FILTER_HASH; - $search_crits = substr(trim($_SERVER["QUERY_STRING"], '/'), 0, 6); - $linksToDisplay = $LINKSDB->filter($search_type, $search_crits); + elseif (! empty($_SERVER['QUERY_STRING']) + && preg_match('/[a-zA-Z0-9-_@]{6}(&.+?)?/', $_SERVER['QUERY_STRING']) + ) { + $linksToDisplay = $LINKSDB->filter( + LinkFilter::$FILTER_HASH, + substr(trim($_SERVER["QUERY_STRING"], '/'), 0, 6) + ); if (count($linksToDisplay) == 0) { $PAGE->render404('The link you are trying to reach does not exist or has been deleted.'); @@ -2041,21 +2082,18 @@ function buildLinkList($PAGE,$LINKSDB) } // Compute paging navigation - $searchterm = empty($_GET['searchterm']) ? '' : '&searchterm=' . $_GET['searchterm']; - $searchtags = empty($_GET['searchtags']) ? '' : '&searchtags=' . $_GET['searchtags']; + $searchtagsUrl = empty($searchtags) ? '' : '&searchtags=' . urlencode($searchtags); + $searchtermUrl = empty($searchterm) ? '' : '&searchterm=' . urlencode($searchterm); $previous_page_url = ''; if ($i != count($keys)) { - $previous_page_url = '?page=' . ($page+1) . $searchterm . $searchtags; + $previous_page_url = '?page=' . ($page+1) . $searchtermUrl . $searchtagsUrl; } $next_page_url=''; if ($page>1) { - $next_page_url = '?page=' . ($page-1) . $searchterm . $searchtags; + $next_page_url = '?page=' . ($page-1) . $searchtermUrl . $searchtagsUrl; } - $token = ''; - if (isLoggedIn()) { - $token = getToken(); - } + $token = isLoggedIn() ? getToken() : ''; // Fill all template fields. $data = array( @@ -2065,8 +2103,8 @@ function buildLinkList($PAGE,$LINKSDB) 'page_current' => $page, 'page_max' => $pagecount, 'result_count' => count($linksToDisplay), - 'search_type' => $search_type, - 'search_crits' => $search_crits, + 'search_term' => $searchterm, + 'search_tags' => $searchtags, 'redirector' => empty($GLOBALS['redirector']) ? '' : $GLOBALS['redirector'], // Optional redirector URL. 'token' => $token, 'links' => $linkDisp, diff --git a/tests/LinkFilterTest.php b/tests/LinkFilterTest.php index 31fd4cf4..ef1cc10a 100644 --- a/tests/LinkFilterTest.php +++ b/tests/LinkFilterTest.php @@ -12,6 +12,8 @@ class LinkFilterTest extends PHPUnit_Framework_TestCase */ protected static $linkFilter; + protected static $NB_LINKS_REFDB = 7; + /** * Instanciate linkFilter with ReferenceLinkDB data. */ @@ -27,7 +29,7 @@ public static function setUpBeforeClass() public function testFilter() { $this->assertEquals( - 7, + self::$NB_LINKS_REFDB, count(self::$linkFilter->filter('', '')) ); @@ -36,6 +38,16 @@ public function testFilter() 2, count(self::$linkFilter->filter('', '', false, true)) ); + + $this->assertEquals( + self::$NB_LINKS_REFDB, + count(self::$linkFilter->filter(LinkFilter::$FILTER_TAG, '')) + ); + + $this->assertEquals( + self::$NB_LINKS_REFDB, + count(self::$linkFilter->filter(LinkFilter::$FILTER_TEXT, '')) + ); } /** @@ -341,4 +353,41 @@ public function testTagFilterWithExclusion() count(self::$linkFilter->filter(LinkFilter::$FILTER_TAG, '-free')) ); } + + /** + * Test crossed search (terms + tags). + */ + public function testFilterCrossedSearch() + { + $terms = '"Free Software " stallman "read this" @website stuff'; + $tags = 'free'; + $this->assertEquals( + 1, + count(self::$linkFilter->filter( + LinkFilter::$FILTER_TAG | LinkFilter::$FILTER_TEXT, + array($tags, $terms) + )) + ); + $this->assertEquals( + 2, + count(self::$linkFilter->filter( + LinkFilter::$FILTER_TAG | LinkFilter::$FILTER_TEXT, + array('', $terms) + )) + ); + $this->assertEquals( + 1, + count(self::$linkFilter->filter( + LinkFilter::$FILTER_TAG | LinkFilter::$FILTER_TEXT, + array($tags, '') + )) + ); + $this->assertEquals( + self::$NB_LINKS_REFDB, + count(self::$linkFilter->filter( + LinkFilter::$FILTER_TAG | LinkFilter::$FILTER_TEXT, + '' + )) + ); + } } diff --git a/tpl/linklist.html b/tpl/linklist.html index ca91699e..c0d42006 100644 --- a/tpl/linklist.html +++ b/tpl/linklist.html @@ -11,16 +11,16 @@