Completed
Pull Request — master (#1727)
by Ingo
02:21
created

SearchForm::getResults()   F

Complexity

Conditions 20
Paths 512

Size

Total Lines 69
Code Lines 36

Duplication

Lines 20
Ratio 28.99 %

Importance

Changes 0
Metric Value
c 0
b 0
f 0
dl 20
loc 69
rs 3.5855
cc 20
eloc 36
nc 512
nop 2

How to fix   Long Method    Complexity   

Long Method

Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.

For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.

Commonly applied refactorings include:

1
<?php
2
3
namespace SilverStripe\CMS\Search;
4
5
use SilverStripe\CMS\Model\SiteTree;
6
use SilverStripe\Control\Controller;
7
use SilverStripe\Forms\FieldList;
8
use SilverStripe\Forms\Form;
9
use SilverStripe\Forms\FormAction;
10
use SilverStripe\Forms\HiddenField;
11
use SilverStripe\Forms\TextField;
12
use SilverStripe\ORM\DB;
13
use SilverStripe\ORM\SS_List;
14
use Translatable;
15
16
/**
17
 * Standard basic search form which conducts a fulltext search on all {@link SiteTree}
18
 * objects.
19
 *
20
 * If multilingual content is enabled through the {@link Translatable} extension,
21
 * only pages the currently set language on the holder for this searchform are found.
22
 * The language is set through a hidden field in the form, which is prepoluated
23
 * with {@link Translatable::get_current_locale()} when then form is constructed.
24
 *
25
 * @see Use ModelController and SearchContext for a more generic search implementation based around DataObject
26
 */
27
class SearchForm extends Form
28
{
29
30
    /**
31
     * @var int $pageLength How many results are shown per page.
32
     * Relies on pagination being implemented in the search results template.
33
     */
34
    protected $pageLength = 10;
35
36
    /**
37
     * Classes to search
38
     */
39
    protected $classesToSearch = array(
40
        "SilverStripe\\CMS\\Model\\SiteTree",
41
        "SilverStripe\\Assets\\File"
42
    );
43
44
    private static $casting = array(
0 ignored issues
show
Comprehensibility introduced by
Consider using a different property name as you override a private property of the parent class.
Loading history...
45
        'SearchQuery' => 'Text'
46
    );
47
48
    /**
49
     *
50
     * @param Controller $controller
51
     * @param string $name The name of the form (used in URL addressing)
52
     * @param FieldList $fields Optional, defaults to a single field named "Search". Search logic needs to be customized
53
     *  if fields are added to the form.
54
     * @param FieldList $actions Optional, defaults to a single field named "Go".
55
     */
56
    public function __construct($controller, $name, $fields = null, $actions = null)
57
    {
58
        if (!$fields) {
59
            $fields = new FieldList(
60
                new TextField('Search', _t('SearchForm.SEARCH', 'Search'))
61
            );
62
        }
63
64
        if (class_exists('Translatable')
65
            && SiteTree::singleton()->hasExtension('Translatable')
66
        ) {
67
            $fields->push(new HiddenField('searchlocale', 'searchlocale', Translatable::get_current_locale()));
68
        }
69
70
        if (!$actions) {
71
            $actions = new FieldList(
72
                new FormAction("getResults", _t('SearchForm.GO', 'Go'))
73
            );
74
        }
75
76
        parent::__construct($controller, $name, $fields, $actions);
77
78
        $this->setFormMethod('get');
79
80
        $this->disableSecurityToken();
81
    }
82
83
    /**
84
     * Set the classes to search.
85
     * Currently you can only choose from "SiteTree" and "File", but a future version might improve this.
86
     *
87
     * @param array $classes
88
     */
89
    public function classesToSearch($classes)
90
    {
91
        $supportedClasses = array('SilverStripe\\CMS\\Model\\SiteTree', 'SilverStripe\\Assets\\File');
92
        $illegalClasses = array_diff($classes, $supportedClasses);
93
        if ($illegalClasses) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $illegalClasses of type array is implicitly converted to a boolean; are you sure this is intended? If so, consider using ! empty($expr) instead to make it clear that you intend to check for an array without elements.

This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent.

Consider making the comparison explicit by using empty(..) or ! empty(...) instead.

Loading history...
94
            user_error(
95
                "SearchForm::classesToSearch() passed illegal classes '" . implode("', '", $illegalClasses)
96
                . "'.  At this stage, only File and SiteTree are allowed",
97
                E_USER_WARNING
98
            );
99
        }
100
        $legalClasses = array_intersect($classes, $supportedClasses);
101
        $this->classesToSearch = $legalClasses;
102
    }
103
104
    /**
105
     * Get the classes to search
106
     *
107
     * @return array
108
     */
109
    public function getClassesToSearch()
110
    {
111
        return $this->classesToSearch;
112
    }
113
114
    /**
115
     * Return dataObjectSet of the results using $_REQUEST to get info from form.
116
     * Wraps around {@link searchEngine()}.
117
     *
118
     * @param int $pageLength DEPRECATED 2.3 Use SearchForm->pageLength
119
     * @param array $data Request data as an associative array. Should contain at least a key 'Search' with all searched keywords.
120
     * @return SS_List
121
     */
122
    public function getResults($pageLength = null, $data = null)
123
    {
124
        // legacy usage: $data was defaulting to $_REQUEST, parameter not passed in doc.silverstripe.org tutorials
125
        if (!isset($data) || !is_array($data)) {
126
            $data = $_REQUEST;
127
        }
128
129
        // set language (if present)
130 View Code Duplication
        if (class_exists('Translatable')) {
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
131
            if (SiteTree::singleton()->hasExtension('Translatable') && isset($data['searchlocale'])) {
132
                if ($data['searchlocale'] == "ALL") {
133
                    Translatable::disable_locale_filter();
134
                } else {
135
                    $origLocale = Translatable::get_current_locale();
136
137
                    Translatable::set_current_locale($data['searchlocale']);
138
                }
139
            }
140
        }
141
142
        $keywords = $data['Search'];
143
144
        $andProcessor = create_function('$matches', '
145
	 		return " +" . $matches[2] . " +" . $matches[4] . " ";
146
	 	');
147
        $notProcessor = create_function('$matches', '
148
	 		return " -" . $matches[3];
149
	 	');
150
151
        $keywords = preg_replace_callback('/()("[^()"]+")( and )("[^"()]+")()/i', $andProcessor, $keywords);
152
        $keywords = preg_replace_callback('/(^| )([^() ]+)( and )([^ ()]+)( |$)/i', $andProcessor, $keywords);
153
        $keywords = preg_replace_callback('/(^| )(not )("[^"()]+")/i', $notProcessor, $keywords);
154
        $keywords = preg_replace_callback('/(^| )(not )([^() ]+)( |$)/i', $notProcessor, $keywords);
155
156
        $keywords = $this->addStarsToKeywords($keywords);
157
158
        if (!$pageLength) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $pageLength of type integer|null is loosely compared to false; this is ambiguous if the integer can be zero. You might want to explicitly use === null instead.

In PHP, under loose comparison (like ==, or !=, or switch conditions), values of different types might be equal.

For integer values, zero is a special case, in particular the following results might be unexpected:

0   == false // true
0   == null  // true
123 == false // false
123 == null  // false

// It is often better to use strict comparison
0 === false // false
0 === null  // false
Loading history...
159
            $pageLength = $this->pageLength;
160
        }
161
        $start = isset($_GET['start']) ? (int)$_GET['start'] : 0;
162
163
        if (strpos($keywords, '"') !== false || strpos($keywords, '+') !== false || strpos($keywords, '-') !== false || strpos($keywords, '*') !== false) {
164
            $results = DB::get_conn()->searchEngine($this->classesToSearch, $keywords, $start, $pageLength, "\"Relevance\" DESC", "", true);
165
        } else {
166
            $results = DB::get_conn()->searchEngine($this->classesToSearch, $keywords, $start, $pageLength);
167
        }
168
169
        // filter by permission
170
        if ($results) {
171
            foreach ($results as $result) {
172
                if (!$result->canView()) {
173
                    $results->remove($result);
174
                }
175
            }
176
        }
177
178
        // reset locale
179 View Code Duplication
        if (class_exists('Translatable')) {
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
180
            if (SiteTree::singleton()->hasExtension('Translatable') && isset($data['searchlocale'])) {
181
                if ($data['searchlocale'] == "ALL") {
182
                    Translatable::enable_locale_filter();
183
                } else {
184
                    Translatable::set_current_locale($origLocale);
0 ignored issues
show
Bug introduced by
The variable $origLocale does not seem to be defined for all execution paths leading up to this point.

If you define a variable conditionally, it can happen that it is not defined for all execution paths.

Let’s take a look at an example:

function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}

In the above example, the variable $x is defined if you pass “foo” or “bar” as argument for $a. However, since the switch statement has no default case statement, if you pass any other value, the variable $x would be undefined.

Available Fixes

  1. Check for existence of the variable explicitly:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        if (isset($x)) { // Make sure it's always set.
            echo $x;
        }
    }
    
  2. Define a default value for the variable:

    function myFunction($a) {
        $x = ''; // Set a default which gets overridden for certain paths.
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        echo $x;
    }
    
  3. Add a value for the missing path:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
    
            // We add support for the missing case.
            default:
                $x = '';
                break;
        }
    
        echo $x;
    }
    
Loading history...
185
                }
186
            }
187
        }
188
189
        return $results;
190
    }
191
192
    protected function addStarsToKeywords($keywords)
193
    {
194
        if (!trim($keywords)) {
195
            return "";
196
        }
197
        // Add * to each keyword
198
        $splitWords = preg_split("/ +/", trim($keywords));
199
        $newWords = [];
200
        while (list($i,$word) = each($splitWords)) {
0 ignored issues
show
Unused Code introduced by
The assignment to $i is unused. Consider omitting it like so list($first,,$third).

This checks looks for assignemnts to variables using the list(...) function, where not all assigned variables are subsequently used.

Consider the following code example.

<?php

function returnThreeValues() {
    return array('a', 'b', 'c');
}

list($a, $b, $c) = returnThreeValues();

print $a . " - " . $c;

Only the variables $a and $c are used. There was no need to assign $b.

Instead, the list call could have been.

list($a,, $c) = returnThreeValues();
Loading history...
201
            if ($word[0] == '"') {
202
                while (list($i,$subword) = each($splitWords)) {
0 ignored issues
show
Unused Code introduced by
The assignment to $i is unused. Consider omitting it like so list($first,,$third).

This checks looks for assignemnts to variables using the list(...) function, where not all assigned variables are subsequently used.

Consider the following code example.

<?php

function returnThreeValues() {
    return array('a', 'b', 'c');
}

list($a, $b, $c) = returnThreeValues();

print $a . " - " . $c;

Only the variables $a and $c are used. There was no need to assign $b.

Instead, the list call could have been.

list($a,, $c) = returnThreeValues();
Loading history...
203
                    $word .= ' ' . $subword;
204
                    if (substr($subword, -1) == '"') {
205
                        break;
206
                    }
207
                }
208
            } else {
209
                $word .= '*';
210
            }
211
            $newWords[] = $word;
212
        }
213
        return implode(" ", $newWords);
214
    }
215
216
    /**
217
     * Get the search query for display in a "You searched for ..." sentence.
218
     *
219
     * @param array $data
220
     * @return string
221
     */
222
    public function getSearchQuery($data = null)
223
    {
224
        // legacy usage: $data was defaulting to $_REQUEST, parameter not passed in doc.silverstripe.org tutorials
225
        if (!isset($data)) {
226
            $data = $_REQUEST;
227
        }
228
229
        // The form could be rendered without the search being done, so check for that.
230
        if (isset($data['Search'])) {
231
            return $data['Search'];
232
        }
233
234
        return null;
235
    }
236
237
    /**
238
     * Set the maximum number of records shown on each page.
239
     *
240
     * @param int $length
241
     */
242
    public function setPageLength($length)
243
    {
244
        $this->pageLength = $length;
245
    }
246
247
    /**
248
     * @return int
249
     */
250
    public function getPageLength()
251
    {
252
        return $this->pageLength;
253
    }
254
}
255