Memo: code refactoring

[virtualtam]

Motivation

Refactoring serves multiple purposes:

• remove the logic from index.php
• spread the logic over meaningful blocks: classes, functions
• ease contribution
  • improve readability
  • improve stability / reduce the risk of regressions

See also: #71, #95, #130

Tasks

Remove dead code

• json_encode removed #391 - json_encode
• small code cleanup #946 - get_magic_quotes_gpc, stripslashes_deep

Date formatting

#270 / #486 - Use DateTime to format dates and timestamps

• linkdate2timestamp
• linkdate2rfc822
• linkdate2iso8601

Feed

#273, #515 - use templates for Atom & RSS feeds

• showATOM
• showRSS
• showDailyRSS

HTML generation

#397 #560 - move PageBuilder to a proper file

• buildLinkList
• pageBuilder (class)
• renderPage (the one chunk)
• showDaily

HTTP

#332 - Detect https protocol by reverse-proxy rfc7239 : X-Forwarded-Proto
#333 - Refactor HTTP / Url functions => #334, #346

• serverUrl
• indexUrl
• pageUrl
• http_parse_headers_shaarli
• getHTTP

Images

#345 - Refactor thumbnail functions
#687 - Use web-thumbnailer to retrieve thumbnails

• computeThumbnail
• genThumbnail
• lazyThumbnail
• resizeImage
• thumbnail

Import & export

#493, #607 - Netscape import
#538 - Netscape export

• importFile
• hardcoded Netscape export generation

Links

• html_extract_title

Shaarli utilities

#372 - Introduce a Shaarli class to group install/compatibility/update checks
#40 - Require write permissions only for relevant directories
#396 - application: refactor the install form generation

• checkUpdate
• checkPHPVersion
• install
• file permissions & ownership

Session & login management

#1005 - Refactor session management utilities
#587 - Improve IP ban file format
#1008 - Refactor login / ban management

• setup_login_state
• allIPs
• fillSessionInfo
• check_auth
• isLoggedIn
• logout
• ban_loginFailed
• ban_loginOk
• ban_canLogin
• getToken
• tokenOk

Utilities

#437 - Logging: move logm() from index.php to application/Utils.php
#800 - Fix a warning generated in return_bytes and refactor it
#1122 - Replace logm() with a PSR-3 logger

• logm
• return_bytes
• getMaxFileSize

Contributing

If there's a topic you'd like to work on, please say so in the comments or have a chat on Gitter before starting to code ;-)

Topics related to active issues will get extra attention.

I'll keep this issue updated as cleanup is progressing.

[nicolasdanelon]

@virtualtam I change the class name Auth (the idea came from Laravel) to Authentication but I have some doubt about the unit tests.. can you (or anybody) help with that?

[virtualtam]

Sure :)

Here is a suggestion for grouping and renaming existing methods into new Authentication and Session classes, acknowledging:

• Authentication handles user login operations
• Session handles the management of PHP $_SESSION variables

Authentication:

• allIPs => getClientIp
• check_auth => checkCredentials
• fillSessionInfo => setSessionVariables
• logout
• setup_login_state & isLoggedIn => isLoggedIn

#1008 - LoginManager:

• ban_loginOk => login
• ban_loginFailed => banIp
• ban_canLogin => checkIpBans

#1005 - Session:

• getToken => generateToken
• tokenOk => checkToken
• is_session_id_valid (from Utils.php) => checkId

Now regarding how to proceed (& split work into sub-tasks):

• refactoring should start with session methods (easy, low risk of side-effects),
• authentication comes next,
  • starting with IP/ban management,
  • then session vars,
  • then login, logout & credential checks,
• whenever possible, usage of globals should be avoided:
  • globals will very likely mess up with tests (side-effects due to variable scope)
  • any external dependency should be injected as a parameter, which also enables testing without being affected by the environment

[virtualtam]

Closing as stale / obsolete.