Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#803 closed Bug (fixed)

Remove unnecessary require_once from codebase

Reported by: vipsoft Owned by: vipsoft
Priority: normal Milestone: Piwik 0.4.2
Component: Core Keywords:
Cc: Sensitive:

Description

In #620, we implemented an autoloader. In this ticket, we clean up the code by removing unnecessary require_once statements. This will require some analysis.

  • core/Piwik.php - add this as a special case to the autoloader or rename it to Helper.php (class Piwik_Helper)?
  • some files contain multiple class definitions, e.g., core/Auth.php contains Piwik_Auth and Piwik_Auth_Result
  • ./piwik.php - keep it as fast as possible; autoloading for a cache miss is ok

Don't change:

  • naming of "core" & "plugins" folders
  • FrontController.php and PluginsManager.php - they have their own autoloaders; don't change this

Change History (19)

comment:1 Changed 5 years ago by vipsoft (robocoder)

  • obviously, where we require_once a file that doesn't contain a class, keep it, but turn it into an absolute path (i.e., prefix it with PIWIK_INCLUDE_PATH)

comment:2 Changed 5 years ago by vipsoft (robocoder)

  • don't remove the require_once from Zend folder until it has been updated (#497)

comment:3 Changed 5 years ago by vipsoft (robocoder)

(In [1224]) refs #620, refs #803 - refactor autoloader into core/Loader.php (class
Piwik_Loader); remove unnecessary require_once in core/Common.php

comment:4 Changed 5 years ago by vipsoft (robocoder)

(In [1225]) refs #803 - add autoloader to tests

comment:5 Changed 5 years ago by matt (mattab)

  • concerning when two classes are in the same file, are you planning to lookup the class name without last part (to look core/Auth.php for the Auth_Result class) or do you plan to create the new file?

comment:6 Changed 5 years ago by vipsoft (robocoder)

My thought is to leave subclassed Exceptions where they are. We can add code to the autoloader to handle this, assuming I got the renaming right.

Something like Auth_Result should be in its own file for speed and consistency. An alternative is to continue to use require_once for these cases.

comment:7 Changed 5 years ago by vipsoft (robocoder)

Actually, Piwik_Auth_Result isn't a good example as it and Piwik_Auth are currently always used in the same context. (core/Access.php, core/Auth.php, plugins/Login/Auth.php)

comment:8 Changed 5 years ago by vipsoft (robocoder)

(In [1228]) refs #803 - auto-discovery algorithm to handle class Piwik, files with
multiple class definitions (e.g., Piwik_Auth_Result), and fix typo
(reference to Piwik_Api_Proxy).

comment:9 Changed 5 years ago by vipsoft (robocoder)

(In [1229]) refs #803 - temp fix: PluginsManager defines functions in global scope

comment:10 Changed 5 years ago by vipsoft (robocoder)

(In [1231]) refs #803 - temp fix; Translate.php contains global scope functions

comment:11 Changed 5 years ago by vipsoft (robocoder)

(In [1232]) refs #803 - Log/Exception contains classes that won't be found by
autoloader's discovery algorithm; FrontController.php and Common.php
use classes with global scope functions (i.e., this needs to be
refactored)

comment:12 Changed 5 years ago by vipsoft (robocoder)

(In [1233]) refs #803 - Auth is loaded by piwik.php?

comment:13 Changed 5 years ago by vipsoft (robocoder)

Ok, files containing global scope functions will continue to be manually included (via require_once).

comment:14 Changed 5 years ago by vipsoft (robocoder)

(In [1248]) refs #803 comment:11 - rename Log Formatter classes for consistency

comment:15 Changed 5 years ago by vipsoft (robocoder)

  • Milestone changed from 4- Stable release to 2- DigitalVibes
  • Owner set to vipsoft
  • Status changed from new to assigned

comment:16 Changed 5 years ago by matt (mattab)

  • Milestone changed from 2- DigitalVibes to 1 - Piwik 0.4.2

increasing priority - now that the loader is in place, maybe we should aim to close the ticket and remove all require_once - vipsoft if you disagree please reset the milestone

comment:17 Changed 5 years ago by vipsoft (robocoder)

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [1296]) fixes #803 - remove unnecessary require_once from core, plugins, and parts of
libs. (I didn't touch: open-flash-chart, Zend Framework, and PEAR HTML.)

comment:18 Changed 5 years ago by vipsoft (robocoder)

(In [1299]) refs #803, refs #735 - lower default to 200; remove unnecessary require_once; standardize setting of include_path

comment:19 Changed 5 years ago by alivenk

Note: See TracTickets for help on using tickets.