Passed
Pull Request — master (#70)
by Pierre-Henry
03:15
created

ReadHandler   B

Complexity

Total Complexity 48

Size/Duplication

Total Lines 336
Duplicated Lines 0 %

Importance

Changes 0
Metric Value
dl 0
loc 336
rs 8.5599
c 0
b 0
f 0
wmc 48

12 Methods

Rating   Name   Duplication   Size   Complexity  
A __construct() 0 9 4
A setReadCookie() 0 6 2
A setReadDb() 0 21 5
A setRead() 0 11 3
A isReadItemsCookie() 0 11 3
A getRead() 0 10 3
A getReadDb() 0 16 4
A clearGarbage() 0 25 5
B isReadItemsDb() 0 30 6
A getReadCookie() 0 8 2
B clearDuplicate() 0 53 8
A isReadItems() 0 14 3

How to fix   Complexity   

Complex Class

Complex classes like ReadHandler often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use ReadHandler, and based on these observations, apply Extract Interface, too.

1
<?php namespace XoopsModules\Newbb;
0 ignored issues
show
Coding Style Compatibility introduced by
For compatibility and reusability of your code, PSR1 recommends that a file should introduce either new symbols (like classes, functions, etc.) or have side-effects (like outputting something, or including other files), but not both at the same time. The first symbol is defined on line 49 and the first side effect is on line 35.

The PSR-1: Basic Coding Standard recommends that a file should either introduce new symbols, that is classes, functions, constants or similar, or have side effects. Side effects are anything that executes logic, like for example printing output, changing ini settings or writing to a file.

The idea behind this recommendation is that merely auto-loading a class should not change the state of an application. It also promotes a cleaner style of programming and makes your code less prone to errors, because the logic is not spread out all over the place.

To learn more about the PSR-1, please see the PHP-FIG site on the PSR-1.

Loading history...
2
3
//
4
//  ------------------------------------------------------------------------ //
5
//                XOOPS - PHP Content Management System                      //
6
//                  Copyright (c) 2000-2016 XOOPS.org                        //
7
//                       <https://xoops.org/>                             //
8
//  ------------------------------------------------------------------------ //
9
//  This program is free software; you can redistribute it and/or modify     //
10
//  it under the terms of the GNU General Public License as published by     //
11
//  the Free Software Foundation; either version 2 of the License, or        //
12
//  (at your option) any later version.                                      //
13
//                                                                           //
14
//  You may not change or alter any portion of this comment or credits       //
15
//  of supporting developers from this source code or any supporting         //
16
//  source code which is considered copyrighted (c) material of the          //
17
//  original comment or credit authors.                                      //
18
//                                                                           //
19
//  This program is distributed in the hope that it will be useful,          //
20
//  but WITHOUT ANY WARRANTY; without even the implied warranty of           //
21
//  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the            //
22
//  GNU General Public License for more details.                             //
23
//                                                                           //
24
//  You should have received a copy of the GNU General Public License        //
25
//  along with this program; if not, write to the Free Software              //
26
//  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA //
27
//  ------------------------------------------------------------------------ //
28
//  Author: phppp (D.J., [email protected])                                  //
29
//  URL: https://xoops.org                                                    //
30
//  Project: Article Project                                                 //
31
//  ------------------------------------------------------------------------ //
32
33
// defined('XOOPS_ROOT_PATH') || die('Restricted access');
0 ignored issues
show
Unused Code Comprehensibility introduced by
70% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
34
35
defined('NEWBB_FUNCTIONS_INI') || include $GLOBALS['xoops']->path('modules/newbb/include/functions.ini.php');
36
37
/**
38
 * A handler for read/unread handling
39
 *
40
 * @package       newbb
41
 *
42
 * @author        D.J. (phppp, http://xoopsforge.com)
43
 * @copyright     copyright (c) 2005 XOOPS.org
44
 */
45
46
/**
47
 * Class ReadHandler
48
 */
49
class ReadHandler extends \XoopsPersistableObjectHandler
0 ignored issues
show
Bug introduced by
The type XoopsPersistableObjectHandler was not found. Maybe you did not declare it correctly or list all dependencies?

The issue could also be caused by a filter entry in the build configuration. If the path has been excluded in your configuration, e.g. excluded_paths: ["lib/*"], you can move it to the dependency path list as follows:

filter:
    dependency_paths: ["lib/*"]

For further information see https://scrutinizer-ci.com/docs/tools/php/php-scrutinizer/#list-dependency-paths

Loading history...
50
{
51
    /**
52
     * Object type.
53
     * <ul>
54
     *  <li>forum</li>
55
     *  <li>topic</li>
56
     * </ul>
57
     *
58
     * @var string
59
     */
60
    public $type;
61
62
    /**
63
     * seconds records will persist.
64
     * assigned from $GLOBALS['xoopsModuleConfig']["read_expire"]
65
     * <ul>
66
     *  <li>positive days = delete all read records exist in the tables before expire time // irmtfan add comment</li>
67
     *  <li>0 = never expires // irmtfan change comment</li>
68
     *  <li>-1 or any negative days = never records // irmtfan change comment</li>
69
     * </ul>
70
     *
71
     * @var integer
72
     */
73
    public $lifetime;
74
75
    /**
76
     * storage mode for records.
77
     * assigned from $GLOBALS['xoopsModuleConfig']["read_mode"]
78
     * <ul>
79
     *  <li>0 = never records</li>
80
     *  <li>1 = uses cookie</li>
81
     *  <li>2 = stores in database</li>
82
     * </ul>
83
     *
84
     * @var integer
85
     */
86
    public $mode;
87
88
    /**
89
     * @param \XoopsDatabase     $db
90
     * @param                    $type
91
     */
92
    public function __construct(\XoopsDatabase $db, $type)
0 ignored issues
show
Bug introduced by
The type XoopsDatabase was not found. Maybe you did not declare it correctly or list all dependencies?

The issue could also be caused by a filter entry in the build configuration. If the path has been excluded in your configuration, e.g. excluded_paths: ["lib/*"], you can move it to the dependency path list as follows:

filter:
    dependency_paths: ["lib/*"]

For further information see https://scrutinizer-ci.com/docs/tools/php/php-scrutinizer/#list-dependency-paths

Loading history...
93
    {
94
        $type = ('forum' === $type) ? 'forum' : 'topic';
95
        parent::__construct($db, 'newbb_reads_' . $type, Read::class . $type, 'read_id', 'post_id');
96
        $this->type  = $type;
97
        $newbbConfig = newbbLoadConfig();
98
        // irmtfan if read_expire = 0 dont clean
99
        $this->lifetime = isset($newbbConfig['read_expire']) ? (int)$newbbConfig['read_expire'] * 24 * 3600 : 30 * 24 * 3600;
100
        $this->mode     = isset($newbbConfig['read_mode']) ? $newbbConfig['read_mode'] : 2;
101
    }
102
103
    /**
104
     * Clear garbage
105
     *
106
     * Delete all expired and duplicated records
107
     */
108
    // START irmtfan rephrase function to 1- add clearDuplicate and 2- dont clean when read_expire = 0
109
    public function clearGarbage()
110
    {
111
        // irmtfan clear duplicaed rows
112
        if (!$result = $this->clearDuplicate()) {
0 ignored issues
show
Unused Code introduced by
The assignment to $result is dead and can be removed.
Loading history...
113
            return false;
114
        }
115
116
        $sql = 'DELETE bb FROM ' . $this->table . ' AS bb' . ' LEFT JOIN ' . $this->table . ' AS aa ON bb.read_item = aa.read_item ' . ' WHERE aa.post_id > bb.post_id';
117
        if (!$result = $this->db->queryF($sql)) {
118
            //xoops_error($this->db->error());
0 ignored issues
show
Unused Code Comprehensibility introduced by
73% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
119
            return false;
120
        }
121
        // irmtfan if read_expire = 0 dont clean
122
        if (empty($this->lifetime)) {
123
            return true;
124
        }
125
        // irmtfan move here and rephrase
126
        $expire = time() - (int)$this->lifetime;
127
        $sql    = 'DELETE FROM ' . $this->table . ' WHERE read_time < ' . $expire;
128
        if (!$result = $this->db->queryF($sql)) {
129
            //xoops_error($this->db->error());
0 ignored issues
show
Unused Code Comprehensibility introduced by
73% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
130
            return false;
131
        }
132
133
        return true;
134
    }
135
136
    // END irmtfan rephrase function to 1- add clearDuplicate and 2- dont clean when read_expire = 0
137
138
    /**
139
     * @param                  $read_item
140
     * @param  null            $uid
0 ignored issues
show
Documentation Bug introduced by
Are you sure the doc-type for parameter $uid is correct as it would always require null to be passed?
Loading history...
141
     * @return bool|mixed|null
142
     */
143
    public function getRead($read_item, $uid = null)
144
    {
145
        if (empty($this->mode)) {
146
            return null;
147
        }
148
        if (1 == $this->mode) {
149
            return $this->getReadCookie($read_item);
150
        }
151
152
        return $this->getReadDb($read_item, $uid);
153
    }
154
155
    /**
156
     * @param $item_id
157
     * @return mixed
158
     */
159
    public function getReadCookie($item_id)
160
    {
161
        $cookie_name = ('forum' === $this->type) ? 'LF' : 'LT';
162
        $cookie_var  = $item_id;
163
        // irmtfan set true to return array
164
        $lastview = newbbGetCookie($cookie_name, true);
165
166
        return @$lastview[$cookie_var];
167
    }
168
169
    /**
170
     * @param $read_item
171
     * @param $uid
172
     * @return bool|null
173
     */
174
    public function getReadDb($read_item, $uid)
175
    {
176
        if (empty($uid)) {
177
            if (is_object($GLOBALS['xoopsUser'])) {
178
                $uid = $GLOBALS['xoopsUser']->getVar('uid');
179
            } else {
180
                return false;
181
            }
182
        }
183
        $sql = 'SELECT post_id ' . ' FROM ' . $this->table . ' WHERE read_item = ' . (int)$read_item . '     AND uid = ' . (int)$uid;
184
        if (!$result = $this->db->queryF($sql, 1)) {
185
            return null;
186
        }
187
        list($post_id) = $this->db->fetchRow($result);
188
189
        return $post_id;
190
    }
191
192
    /**
193
     * @param                  $read_item
194
     * @param                  $post_id
195
     * @param  null            $uid
0 ignored issues
show
Documentation Bug introduced by
Are you sure the doc-type for parameter $uid is correct as it would always require null to be passed?
Loading history...
196
     * @return bool|mixed|void
197
     */
198
    public function setRead($read_item, $post_id, $uid = null)
199
    {
200
        if (empty($this->mode)) {
201
            return true;
202
        }
203
204
        if (1 == $this->mode) {
205
            return $this->setReadCookie($read_item, $post_id);
0 ignored issues
show
Bug introduced by
Are you sure the usage of $this->setReadCookie($read_item, $post_id) targeting XoopsModules\Newbb\ReadHandler::setReadCookie() seems to always return null.

This check looks for function or method calls that always return null and whose return value is used.

class A
{
    function getObject()
    {
        return null;
    }

}

$a = new A();
if ($a->getObject()) {

The method getObject() can return nothing but null, so it makes no sense to use the return value.

The reason is most likely that a function or method is imcomplete or has been reduced for debug purposes.

Loading history...
206
        }
207
208
        return $this->setReadDb($read_item, $post_id, $uid);
209
    }
210
211
    /**
212
     * @param $read_item
213
     * @param $post_id
214
     */
215
    public function setReadCookie($read_item, $post_id)
216
    {
217
        $cookie_name          = ('forum' === $this->type) ? 'LF' : 'LT';
218
        $lastview             = newbbGetCookie($cookie_name, true);
219
        $lastview[$read_item] = time();
220
        newbbSetCookie($cookie_name, $lastview);
221
    }
222
223
    /**
224
     * @param $read_item
225
     * @param $post_id
226
     * @param $uid
227
     * @return bool|mixed
228
     */
229
    public function setReadDb($read_item, $post_id, $uid)
230
    {
231
        if (empty($uid)) {
232
            if (is_object($GLOBALS['xoopsUser'])) {
233
                $uid = $GLOBALS['xoopsUser']->getVar('uid');
234
            } else {
235
                return false;
236
            }
237
        }
238
239
        $sql = 'UPDATE ' . $this->table . ' SET post_id = ' . (int)$post_id . ',' . '     read_time =' . time() . ' WHERE read_item = ' . (int)$read_item . '     AND uid = ' . (int)$uid;
240
        if ($this->db->queryF($sql) && $this->db->getAffectedRows()) {
241
            return true;
242
        }
243
        $object = $this->create();
244
        $object->setVar('read_item', $read_item);
245
        $object->setVar('post_id', $post_id);
246
        $object->setVar('uid', $uid);
247
        $object->setVar('read_time', time());
248
249
        return parent::insert($object);
250
    }
251
252
    /**
253
     * @param             $items
254
     * @param  null       $uid
0 ignored issues
show
Documentation Bug introduced by
Are you sure the doc-type for parameter $uid is correct as it would always require null to be passed?
Loading history...
255
     * @return array|null
256
     */
257
    public function isReadItems(&$items, $uid = null)
258
    {
259
        $ret = null;
260
        if (empty($this->mode)) {
261
            return $ret;
262
        }
263
264
        if (1 == $this->mode) {
265
            $ret = $this->isReadItemsCookie($items);
266
        } else {
267
            $ret = $this->isReadItemsDb($items, $uid);
268
        }
269
270
        return $ret;
271
    }
272
273
    /**
274
     * @param $items
275
     * @return array
276
     */
277
    public function isReadItemsCookie(&$items)
278
    {
279
        $cookie_name = ('forum' === $this->type) ? 'LF' : 'LT';
280
        $cookie_vars = newbbGetCookie($cookie_name, true);
281
282
        $ret = [];
283
        foreach ($items as $key => $last_update) {
284
            $ret[$key] = (max(@$GLOBALS['last_visit'], @$cookie_vars[$key]) >= $last_update);
285
        }
286
287
        return $ret;
288
    }
289
290
    /**
291
     * @param $items
292
     * @param $uid
293
     * @return array
294
     */
295
    public function isReadItemsDb(&$items, $uid)
296
    {
297
        $ret = [];
298
        if (empty($items)) {
299
            return $ret;
300
        }
301
302
        if (empty($uid)) {
303
            if (is_object($GLOBALS['xoopsUser'])) {
304
                $uid = $GLOBALS['xoopsUser']->getVar('uid');
305
            } else {
306
                return $ret;
307
            }
308
        }
309
310
        $criteria = new \CriteriaCompo(new \Criteria('uid', $uid));
0 ignored issues
show
Bug introduced by
The type CriteriaCompo was not found. Maybe you did not declare it correctly or list all dependencies?

The issue could also be caused by a filter entry in the build configuration. If the path has been excluded in your configuration, e.g. excluded_paths: ["lib/*"], you can move it to the dependency path list as follows:

filter:
    dependency_paths: ["lib/*"]

For further information see https://scrutinizer-ci.com/docs/tools/php/php-scrutinizer/#list-dependency-paths

Loading history...
Bug introduced by
The type Criteria was not found. Maybe you did not declare it correctly or list all dependencies?

The issue could also be caused by a filter entry in the build configuration. If the path has been excluded in your configuration, e.g. excluded_paths: ["lib/*"], you can move it to the dependency path list as follows:

filter:
    dependency_paths: ["lib/*"]

For further information see https://scrutinizer-ci.com/docs/tools/php/php-scrutinizer/#list-dependency-paths

Loading history...
311
        $criteria->add(new \Criteria('read_item', '(' . implode(', ', array_map('intval', array_keys($items))) . ')', 'IN'));
312
        $itemsObject = $this->getAll($criteria, ['read_item', 'post_id']);
313
314
        $items_list = [];
315
        foreach (array_keys($itemsObject) as $key) {
316
            $items_list[$itemsObject[$key]->getVar('read_item')] = $itemsObject[$key]->getVar('post_id');
317
        }
318
        unset($itemsObject);
319
320
        foreach ($items as $key => $last_update) {
321
            $ret[$key] = (@$items_list[$key] >= $last_update);
322
        }
323
324
        return $ret;
325
    }
326
327
    // START irmtfan add clear duplicated rows function
328
329
    /**
330
     * @return bool
331
     */
332
    public function clearDuplicate()
333
    {
334
        /**
335
         * This is needed for the following query GROUP BY clauses to work in MySQL 5.7.
336
         * This is a TEMPORARY fix. Needing this function is bad in the first place, but
337
         * needing sloppy SQL to make it work is worse.
338
         * @todo The schema itself should preclude the duplicates
339
         */
340
        $sql = "SET sql_mode=(SELECT REPLACE(@@sql_mode, 'ONLY_FULL_GROUP_BY', ''))";
341
        $this->db->queryF($sql);
342
343
        $sql = 'CREATE TABLE ' . $this->table . '_duplicate LIKE ' . $this->table . '; ';
344
        if (!$result = $this->db->queryF($sql)) {
0 ignored issues
show
Unused Code introduced by
The assignment to $result is dead and can be removed.
Loading history...
345
            xoops_error($this->db->error() . '<br>' . $sql);
0 ignored issues
show
Bug introduced by
The function xoops_error was not found. Maybe you did not declare it correctly or list all dependencies? ( Ignorable by Annotation )

If this is a false-positive, you can also ignore this issue in your code via the ignore-call  annotation

345
            /** @scrutinizer ignore-call */ 
346
            xoops_error($this->db->error() . '<br>' . $sql);
Loading history...
346
347
            return false;
348
        }
349
        $sql = 'INSERT ' . $this->table . '_duplicate SELECT * FROM ' . $this->table . ' GROUP BY read_item, uid; ';
350
        if (!$result = $this->db->queryF($sql)) {
351
            xoops_error($this->db->error() . '<br>' . $sql);
352
353
            return false;
354
        }
355
        $sql = 'RENAME TABLE ' . $this->table . ' TO ' . $this->table . '_with_duplicate; ';
356
        if (!$result = $this->db->queryF($sql)) {
357
            xoops_error($this->db->error() . '<br>' . $sql);
358
359
            return false;
360
        }
361
        $sql = 'RENAME TABLE ' . $this->table . '_duplicate TO ' . $this->table . '; ';
362
        if (!$result = $this->db->queryF($sql)) {
363
            xoops_error($this->db->error() . '<br>' . $sql);
364
365
            return false;
366
        }
367
        $sql    = 'SHOW INDEX FROM ' . $this->table . " WHERE KEY_NAME = 'read_item_uid'";
368
        $result = $this->db->queryF($sql);
369
        if (empty($result)) {
370
            $sql .= 'ALTER TABLE ' . $this->table . ' ADD INDEX read_item_uid ( read_item, uid ); ';
371
            if (!$result = $this->db->queryF($sql)) {
372
                xoops_error($this->db->error() . '<br>' . $sql);
373
374
                return false;
375
            }
376
        }
377
        $sql = 'DROP TABLE ' . $this->table . '_with_duplicate; ';
378
        if (!$result = $this->db->queryF($sql)) {
379
            xoops_error($this->db->error() . '<br>' . $sql);
380
381
            return false;
382
        }
383
384
        return true;
385
    }
386
    // END irmtfan add clear duplicated rows function
387
}
388