Updated: Comment #7

Problem/Motivation

Path system was initially converted to OOP by simply moving procedural code into classes. While this is a logical first step it needs follow-up issues that will re-organize code, clean it up, make it more readable and understandable.

Proposed resolution

This task consists out of many subtasks (see below).

Remaining tasks

\Drupal\Core\Path\Path
- #2199381: Add \Drupal\Core\Path\PathInterface
- #2208631: Rename \Drupal\Core\Path\Path to \Drupal\Core\Path\AliasStorage
- #2209145: Move all path alias SQL queries to a single storage controller
- #2233595: Deprecate the custom path alias storage

\Drupal\Core\Path\AliasManager
- use "path" for non-aliased URL and "alias"
- #2233619: Merge lookup functions in AliasManager
- #2233623: Merge AliasManagerCacheDecorator into AliasManager

path.inc:
- #2233607: Kill path_load() and use \Drupal::service('path.alias_storage')->load($conditions) instead

User interface changes

N/A.

API changes

Original report by @dawehner

If you look at #1893730: Provide an in-memory mock implementation of Path\AliasManager for tests and update.php you might think that an alias manager is a simple thing but the interface is pretty complicated compared to what it would need it be.

Session: https://portland2013.drupal.org/node/3893

Comments

dawehner’s picture

Issue tags: -VDC +WSCCI

This has nothing to do with VDC

slashrsm’s picture

Issue summary: View changes
slashrsm’s picture

My suggestions related to this. Some of things are from @msonnabaum's session and I added some more:

\Drupal\Core\Path\Path
- Rename Path to AliasStorage (+ interface) #2208631: Rename \Drupal\Core\Path\Path to \Drupal\Core\Path\AliasStorage, #2199381: Add \Drupal\Core\Path\PathInterface
- move all storage-dependednt code into AliasStorage #2209145: Move all path alias SQL queries to a single storage controller
- change load($conditions) to load($pid) and introduce two other functions: loadByPath($path, $langcode) and loadByAlias($alias, $langcode)
- change delete($conditions) to delete($pid) and introduce two other functions: deleteByPath($path, $language) and deleteByAlias($alias, $langcode)

\Drupal\Core\Path\AliasManager
- use "path" for non-aliased URL and "alias"
- merge getSystemPath() and lookupPathSource() into getPathByAlias()
- merge getPathALias() and lookupPathAlias() into getAliasByPath()
- refactor and move cacheClear(), getPathLookups(), preloadPathLookups() and pathAliasWhitelistRebuild() into AliasManagerCacheDecorator()

path.inc:
- kill path_load() function, which is only a procedural wrapper around \Drupal::service('path.crud')->load($conditions);

We should probably do this stuff in sub-issues as it will make it much more manageable.

andypost’s picture

sun’s picture

All of that sounds good to me. Rock on :-)

slashrsm’s picture

Issue summary: View changes
slashrsm’s picture

Title: Refactor the alias manager according to msonnabaum session » Refactor the alias subsystem (classes in \Drupal\Core\Path)
Issue summary: View changes
slashrsm’s picture

Issue summary: View changes
slashrsm’s picture

Issue summary: View changes
slashrsm’s picture

Issue summary: View changes
slashrsm’s picture

Issue summary: View changes
Crell’s picture

Title: Refactor the alias subsystem (classes in \Drupal\Core\Path) » [meta] Refactor the alias subsystem (classes in \Drupal\Core\Path)

Refiling as a meta. Not closing as there's still 2 open child issues.

Crell’s picture

Status: Active » Fixed

We closed one of the child issues as no longer relevant, and the other is now pushed to 8.1. So closing this meta. Yay.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.