Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Drupal/Core/Url::getInternalPath()
is marked as @deprecated for removal before 8.0.0.
However, it will not be removed for 8.0.0, and may not even be removed for 9.0.0
Proposed resolution
Simply remove deprecation comment from Drupal/Core/Url::getInternalPath()
.
Remaining tasks
none
User interface changes
n/a
API changes
n/a
Data model changes
n/a
Comment | File | Size | Author |
---|---|---|---|
#26 | 2367753-url-26.patch | 561 bytes | tim.plunkett |
#20 | remove_usage_of-2367753-20.patch | 10.06 KB | gauravjeet |
#15 | interdiff-15.txt | 1.5 KB | RavindraSingh |
#15 | 2367753-remove-usage-of-getinternalPath.patch | 14.07 KB | RavindraSingh |
#13 | remove_usage_of-2367753-13.patch | 14.05 KB | JeroenT |
Comments
Comment #1
tim.plunkettComment #2
ianthomas_ukI think this depends on #2339219: [meta] Finalize URL generation API (naming, docs, deprecation) being sorted
Comment #3
Mile23Url->getInternalPath()
seems to be used 21 times in core.Should it really be deprecated?
Comment #4
Mile23Comment #5
Mile23Changing parent issue.
Comment #6
Mile23Comment #7
JeroenTStarted working on this issue.
Should it be something like this or is the fix too simple?
Comment #10
JeroenTI removed unrelated changes + fixed a lot of failures, but I'm not sure this is the right way..
Patch attached.
Comment #13
JeroenTI know RC1 is out, but there is still a small change we can remove this function. See #2205673-147: [META] Remove all @deprecated functions marked "remove before 8.0"
Comment #15
RavindraSingh CreditAttribution: RavindraSingh as a volunteer and at Material commented$url->toString() should be trimmed.
I am not sure if this function also require tests?
Need tests.
Needs tests.
We are ending system_path with '/' and someplaces we are not using. why?
Need tests.
Also, we would require to think about getInternalPath was not terurning the paths ending with slash whereas. OLD Code
I have done some minor changes in existing patch and submitting here to review.
Main thing, this issue should be considered as a Major .
Comment #16
RavindraSingh CreditAttribution: RavindraSingh as a volunteer and at Material commentedComment #17
RavindraSingh CreditAttribution: RavindraSingh as a volunteer and at Material commentedComment #20
gauravjeet CreditAttribution: gauravjeet as a volunteer and at Material commentedIn the patch above, the function toString() does not give any results. That has to be replaced with specific values that include RouteName and the RouteParameters. Following are the Test files where changes have to be made in addition to the changes made in this attached patch, and then we can completely remove the getInternalPath() function usage from /core/lib/Drupal/Core/Url.php.
Test Paths :
/core/modules/aggregator/src/Tests/AggregatorRenderingTest.php
/core/modules/system/src/Tests/Menu/BreadcrumbTest.php
/core/tests/Drupal/Tests/Core/UnroutedUrlTest.php
/core/tests/Drupal/Tests/Core/UrlTest.php
Comment #21
gauravjeet CreditAttribution: gauravjeet as a volunteer and at Material commentedPutting this issue in needs review state
Comment #25
pwolanin CreditAttribution: pwolanin at Acquia commentedSince this is just a wrapper on UrlGenerator::getPathFromRoute() I don't think there is much gain if we are just swapping the one for the other.
I think we just need to change or remove the deprecation notice now.
Comment #26
tim.plunkettUntil every system that relies on an internal path can instead use a Url object, we need this.
One example: \Drupal\Core\Path\AliasStorageInterface::save().
Comment #27
pwolanin CreditAttribution: pwolanin at Acquia commentedI don't think using a Url object or not in method calls matters here, I think the real issue is that path processing and aliases are coupled to the "internal" path so we cannot remove that capability from the generator if we want to be able to e.g. create a path alias for a given route.
If we are not removing it from the generator, there is no point in removing the wrapper method from Url.
Comment #28
pwolanin CreditAttribution: pwolanin at Acquia commentedComment #29
pwolanin CreditAttribution: pwolanin at Acquia commentedComment #30
pwolanin CreditAttribution: pwolanin at Acquia commentedThis only affects phpdoc, not code
Comment #31
pwolanin CreditAttribution: pwolanin at Acquia commentedCould use an issue summary update, and a check for affected change records
Comment #35
webchickHm. Remove @deprecated entirely? Or target for 9 instead?
Comment #36
webchickTagging rc eligible since this is just about docs now.
Comment #37
jhodgdonPlease update the issue summary.
Comment #38
pwolanin CreditAttribution: pwolanin at Acquia commentedComment #39
alexpottCommitted b219839 and pushed to 8.0.x. Thanks!