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

• [x] #391 - json_encode
• [x] #946 - get_magic_quotes_gpc, stripslashes_deep

Date formatting

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

• [x] linkdate2timestamp
• [x] linkdate2rfc822
• [x] linkdate2iso8601

Feed

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

• [x] showATOM
• [x] showRSS
• [ ] showDailyRSS

HTML generation

397 #560 - move PageBuilder to a proper file

• [ ] buildLinkList
• [x] 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

• [x] serverUrl
• [x] indexUrl
• [x] pageUrl
• [x] http_parse_headers_shaarli
• [x] getHTTP

Images

345 - Refactor thumbnail functions

687 - Use web-thumbnailer to retrieve thumbnails

• [x] computeThumbnail
• [x] genThumbnail
• [x] lazyThumbnail
• [x] resizeImage
• [x] thumbnail

Import & export

493, #607 - Netscape import

538 - Netscape export

• [x] importFile
• [x] 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

• [x] checkUpdate
• [x] checkPHPVersion
• [ ] install
• [x] file permissions & ownership

Session & login management

1005 - Refactor session management utilities

587 - Improve IP ban file format

1008 - Refactor login / ban management

• [x] setup_login_state
• [x] allIPs
• [x] fillSessionInfo
• [x] check_auth
• [x] isLoggedIn
• [x] logout
• [x] ban_loginFailed
• [x] ban_loginOk
• [x] ban_canLogin
• [x] getToken
• [x] 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

• [x] logm
• [x] return_bytes
• [x] 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.