Completed
Push — master ( bc3587...b308f6 )
by Angus
03:03
created

MY_Controller   A

Complexity

Total Complexity 11

Size/Duplication

Total Lines 59
Duplicated Lines 0 %

Coupling/Cohesion

Components 2
Dependencies 7

Test Coverage

Coverage 64.71%

Importance

Changes 0
Metric Value
dl 0
loc 59
ccs 22
cts 34
cp 0.6471
rs 10
c 0
b 0
f 0
wmc 11
lcom 2
cbo 7

3 Methods

Rating   Name   Duplication   Size   Complexity  
A __construct() 0 20 2
A _render_json() 0 6 2
A _render_content() 0 8 2
1
<?php defined('BASEPATH') OR exit('No direct script access allowed');
2
3
class MY_Controller extends CI_Controller {
4
	protected $header_data = array();
5
	protected $body_data   = array();
6
	protected $footer_data = array();
7
	public    $global_data = array();
8
9 42
	public function __construct(){
10 42
		parent::__construct();
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class CI_Controller as the method __construct() does only exist in the following sub-classes of CI_Controller: AJAX_Controller, About, AdminCLI, AdminPanel, Admin_Controller, Auth_Controller, CLI_Controller, Favourites, Forgot_Password, GetKey, Help, History, Import_AMR, IndexC, Login, Logout, MY_Controller, No_Auth_Controller, Options, PublicList, ReportBug, Signup, Stats, TitleHistory, TrackerInline, UpdateStatus, User_Controller, UsernameCheck, Userscript, Welcome. Maybe you want to instanceof check for one of these explicitly?

Let’s take a look at an example:

abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
11
12 42
		$this->load->driver('cache', array('adapter' => 'file', 'backup' => 'apc')); //Sadly we can't autoload this with params
13
		//FIXME: We would just use APC caching, but it's a bit more tricky to manage with multiple site setups..
14
15
		//FIXME: This is pretty much a phpUnit hack. Without it phpUnit fails here. We need a proper way to fake user/admin testing.
16 42
		$this->global_data['user'] = ($this->ion_auth->user() ? $this->ion_auth->user()->row() : ['username' => '']);
17 42
		$this->global_data['username'] = $this->User->username;
18
19
		//TODO: Move this to a lib or something.
20 42
		$this->global_data['analytics_tracking_id'] = $this->config->item('tracking_id');
21
22 42
		$this->global_data['theme'] = $this->User_Options->get('theme');
23 42
		$css_path = "css/main.{$this->User_Options->get('theme')}";
24 42
		$this->global_data['complied_css_path'] = asset_url()."{$css_path}.".filemtime(APPPATH . "../public/assets/{$css_path}.css").".css";
25
26 42
		$js_path = 'js/compiled.min';
27 42
		$this->global_data['complied_js_path']  = asset_url()."{$js_path}.".filemtime(APPPATH . "../public/assets/{$js_path}.js").".js";
28 42
	}
29
30 21
	public function _render_page(/*(array) $paths*/) : void {
31
		//We could just use global, but this is the only var we need in both header+footer
32 21
		$this->footer_data['page'] = $this->header_data['page'];
33
34 21
		$this->header_data['show_header'] = (array_key_exists('show_header', $this->header_data) ? $this->header_data['show_header'] : TRUE);
35 21
		$this->footer_data['show_footer'] = (array_key_exists('show_footer', $this->footer_data) ? $this->footer_data['show_footer'] : TRUE);
36
37 21
		$this->load->view('common/header', ($this->global_data + $this->header_data));
38 21
		foreach(func_get_args() as $path) {
39 21
			view_exists($path) or show_404(); //TODO (FIXME): This seems bad performance wise in the long run. Is there any reason to have it in production?
40
41 21
			$this->load->view($path, ($this->global_data + $this->body_data));
42
		}
43
		//using the union operator + makes sure global_data always takes priority
44
		//SEE: http://stackoverflow.com/a/2140094/1168377
45 21
		$this->load->view('common/footer', ($this->global_data + $this->footer_data));
46 21
	}
47
	public function _render_json($json_input, bool $download = FALSE, string $filenamePrefix = 'tracker') : void {
48
		$json = is_array($json_input) ? json_encode($json_input) : $json_input;
49
50
		$this->output->set_content_type('application/json', 'utf-8');
51
		$this->_render_content($json,'json', $download, $filenamePrefix);
52
	}
53
	public function _render_content(string $content, string $filenameExt, bool $download = FALSE, string $filenamePrefix = 'tracker') : void {
54
		if($download) {
55
			$date = date('Ymd_Hi', time());
56
			$this->output->set_header('Content-Disposition: attachment; filename="'.$filenamePrefix.'-'.$date.'.'.$filenameExt.'"');
57
			$this->output->set_header('Content-Length: '.strlen($content));
58
		}
59
		$this->output->set_output($content);
60
	}
61
}
62
63
class CLI_Controller extends CI_Controller {
64 1
	public function __construct() {
65 1
		parent::__construct();
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class CI_Controller as the method __construct() does only exist in the following sub-classes of CI_Controller: AJAX_Controller, About, AdminCLI, AdminPanel, Admin_Controller, Auth_Controller, CLI_Controller, Favourites, Forgot_Password, GetKey, Help, History, Import_AMR, IndexC, Login, Logout, MY_Controller, No_Auth_Controller, Options, PublicList, ReportBug, Signup, Stats, TitleHistory, TrackerInline, UpdateStatus, User_Controller, UsernameCheck, Userscript, Welcome. Maybe you want to instanceof check for one of these explicitly?

Let’s take a look at an example:

abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
66
67
		//NOTE: This should fail, assuming routes.php does handles things properly.
68
		//      It's good to have "just in case" fallbacks though.
69 1
		is_cli() or exit("ERROR: This controller can only be called via command line: php index.php ...");
70 1
	}
71
}
72
73
/**** AUTH CONTROLLERS ****/
74
class User_Controller extends MY_Controller {
75 41
	public function __construct() {
76 41
		parent::__construct();
77
78 41
		$this->load->database();
79 41
	}
80
}
81
82
class Auth_Controller extends User_Controller {
83 8
	public function __construct(bool $redirect = TRUE) {
84 8
		parent::__construct();
85
86 8
		if(!$this->ion_auth->logged_in()) {
0 ignored issues
show
Bug introduced by
The method logged_in() does not seem to exist on object<Ion_auth_model>.

This check looks for calls to methods that do not seem to exist on a given type. It looks for the method on the type itself as well as in inherited classes or implemented interfaces.

This is most likely a typographical error or the method has been renamed.

Loading history...
87 8
			if($redirect || $_SERVER['REQUEST_METHOD'] === 'GET') {
88 2
				$this->User->login_redirect();
89 2
				redirect('user/login');
90
			} else {
91 6
				$this->output->set_status_header(401, 'Session has expired, please re-log to continue.');
92 6
				exit_ci();
93
			}
94
		}
95
	}
96
}
97
98
class No_Auth_Controller extends User_Controller {
99 30
	public function __construct() {
100 30
		parent::__construct();
101
102 30
		if($this->ion_auth->logged_in()) redirect('/');
0 ignored issues
show
Bug introduced by
The method logged_in() does not seem to exist on object<Ion_auth_model>.

This check looks for calls to methods that do not seem to exist on a given type. It looks for the method on the type itself as well as in inherited classes or implemented interfaces.

This is most likely a typographical error or the method has been renamed.

Loading history...
103 27
	}
104
}
105
106
class Admin_Controller extends Auth_Controller {
107
	public function __construct() {
108
		parent::__construct();
109
110
		if(!$this->ion_auth->is_admin()) {
0 ignored issues
show
Bug introduced by
The method is_admin() does not seem to exist on object<Ion_auth_model>.

This check looks for calls to methods that do not seem to exist on a given type. It looks for the method on the type itself as well as in inherited classes or implemented interfaces.

This is most likely a typographical error or the method has been renamed.

Loading history...
111
			//user is not an admin, redirect them to front page
112
			//TODO (CHECK): Should we note that "you must be an admin to view this page"?
113
114
			redirect('/');
115
		}
116
	}
117
}
118
119
/**** AJAX CONTROLLERS ****/
120
class AJAX_Controller extends CI_Controller {
121 4
	public function __construct() {
122 4
		parent::__construct();
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class CI_Controller as the method __construct() does only exist in the following sub-classes of CI_Controller: AJAX_Controller, About, AdminCLI, AdminPanel, Admin_Controller, Auth_Controller, CLI_Controller, Favourites, Forgot_Password, GetKey, Help, History, Import_AMR, IndexC, Login, Logout, MY_Controller, No_Auth_Controller, Options, PublicList, ReportBug, Signup, Stats, TitleHistory, TrackerInline, UpdateStatus, User_Controller, UsernameCheck, Userscript, Welcome. Maybe you want to instanceof check for one of these explicitly?

Let’s take a look at an example:

abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
123
124
		//TODO: general security stuff
125 4
	}
126
}
127