Make \Drupal\Core\Path\AliasManager storage independent [#2136503]

Updated: Comment #0

Problem/Motivation

Path alias related queries currently appear in \Drupal\Core\Path\AliasManager and \Drupal\Core\Path\Path. We should move all storage-dependent code to one class to make storage swap-ability easier.

Proposed resolution

Move all queries from \Drupal\Core\Path\AliasManager to \Drupal\Core\Path\Path and make the latter storage controller for all path-related operations. This is currently impossible to do, since it results in DIC circular dependency. #2126421: Decouple \Drupal\Core\Path\AliasManager and \Drupal\Core\Path\Path fixes that, which means that we need to postpone this until #2126421: Decouple \Drupal\Core\Path\AliasManager and \Drupal\Core\Path\Path is committed.

I have some code ready. Will upload it later today.

Remaining tasks

- finish work on #2126421: Decouple \Drupal\Core\Path\AliasManager and \Drupal\Core\Path\Path
- move all storage-dependent code from AliasManager to Path

User interface changes

None.

API changes

None.

Comment	File	Size	Author
#9	interdiff.txt	808 bytes	slashrsm
#9	2136503_9.patch	13.34 KB	slashrsm
#9
#7	interdff.txt	960 bytes	slashrsm
#7	2136503_7.patch	13.4 KB	slashrsm
#7
#5	interdiff.txt	3.69 KB	slashrsm
#5	2136503_5.patch	12.47 KB	slashrsm
#5
#2	2136503_1.patch	12.45 KB	slashrsm
#2

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

slashrsm CreditAttribution: slashrsm commented 18 February 2014 at 12:50

Comment #2

slashrsm CreditAttribution: slashrsm commented 18 February 2014 at 13:15

Status:

Postponed

» Needs review

File	Size
2136503_1.patch	12.45 KB

Comment #3

18 February 2014 at 14:47

Status:

Needs review

» Needs work

The last submitted patch, 2: 2136503_1.patch, failed testing.

Comment #4

jibran

he/him

Urdu

Sydney, Australia

CreditAttribution: jibran commented 20 February 2014 at 19:00

+++ b/core/lib/Drupal/Core/Path/Path.php
@@ -141,4 +141,111 @@ public function delete($conditions) {
+   * @param $path
...
+   * @param $langcode
...
+   * @param $path
...
+   * @param $langcode
...
+   * @return
...
+   * @param $path
...
+   * @param $langcode
...
+   * @return

typhint missing.

+++ b/core/lib/Drupal/Core/Path/Path.php
@@ -141,4 +141,111 @@ public function delete($conditions) {
+    // Always get the language-specific alias before the language-neutral
+    // one. For example 'de' is less than 'und' so the order needs to be
+    // ASC, while 'xx-lolspeak' is more than 'und' so the order needs to
+    // be DESC. We also order by pid ASC so that fetchAllKeyed() returns
+    // the most recently created alias for each source. Subsequent queries
+    // using fetchField() must use pid DESC to have the same effect.
+    // For performance reasons, the query builder is not used here.

We can add more chars on all comment lines to make it 80 chars.

+++ b/core/lib/Drupal/Core/Path/Path.php
@@ -141,4 +141,111 @@ public function delete($conditions) {
+   * Given a path source, return its path alias if one exists. Otherwise,
+   * return FALSE.
...
+   * Given an alias, return its Drupal system URL if one exists. Otherwise,
+   * return FALSE.

Should be single line. It is not valid description IMO.

Comment #5

slashrsm CreditAttribution: slashrsm commented 21 February 2014 at 10:59

Status:

Needs work

» Needs review

File	Size
2136503_5.patch	12.47 KB

interdiff.txt	3.69 KB

Fixed #4 comments and some other minor thing.

It is not valid description IMO.

Not sure what you mean.

Comment #6

21 February 2014 at 12:33

Status:

Needs review

» Needs work

The last submitted patch, 5: 2136503_5.patch, failed testing.

Comment #7

slashrsm CreditAttribution: slashrsm commented 21 February 2014 at 13:18

Status:

Needs work

» Needs review

File	Size
2136503_7.patch	13.4 KB

interdff.txt	960 bytes

Let's try again.

Comment #8

jibran

he/him

Urdu

Sydney, Australia

CreditAttribution: jibran commented 21 February 2014 at 21:21

Thanks for fixing all the issues.
According to https://drupal.org/node/1354#functions

Functions that are easily described in one line may be documented by providing the function summary line only (omitting all parameters and return value)

So here are my suggestions.

+++ b/core/lib/Drupal/Core/Path/Path.php
@@ -141,4 +141,109 @@ public function delete($conditions) {
+   * Given a path source, return its path alias or FALSE is non-existing.

Returns an alias of Drupal system URL.

+++ b/core/lib/Drupal/Core/Path/Path.php
@@ -141,4 +141,109 @@ public function delete($conditions) {
+   * Given an alias, return its Drupal system URL or FALSE if non-existing.

Returns Drupal system URL of an alias.