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.
domain_path currently loads aliases for all nodes and domains once and caches the results. I believe we should be loading only the paths that are necessary for the individual page load and store accordingly . To do this I have created a patch that replaces domain_path_paths with domain_path_lookup_path which mimics drupal_lookup_path.
This patch also will handle loading the language specific paths when necessary.
Comment | File | Size | Author |
---|---|---|---|
#9 | 1482208-domain-path-query.patch | 12.05 KB | agentrickard |
#8 | domain_path-domain_path_lookup_path-1482208-8.patch | 11.9 KB | grndlvl |
#7 | domain_path-domain_path_lookup_path-1482208-7.patch | 11.89 KB | grndlvl |
#5 | domain_path-domain_path_lookup_path-1482208-5.patch | 11.88 KB | grndlvl |
domain_path-domain_path_lookup_path--0.patch | 11.76 KB | grndlvl |
Comments
Comment #1
grndlvl CreditAttribution: grndlvl commentedIf accepted we should probably call this 7.x-2.x as it changes around the core of domain_path.
Comment #2
agentrickardI dunno about that, we only have a beta release, so changing the inner workings shouldn't be an issue.
Comment #3
agentrickardIt doesn't seem to be working for me against 7.x.3-dev. To test:
* Enable the Domain Switcher block.
* Edit a node
* Give the node a different path for some domains.
* Make the node visible on 'all affiliates'
* Save the node.
* From the node view page, check the links in the Domain Switcher block
Those links should be aliased properly, but are not.
Comment #4
agentrickardReverting the patch and the links work as expected.
Comment #5
grndlvl CreditAttribution: grndlvl commentedChanged out domain_path_domain_path() to use the original_path when looking up an alias && return Drupal alias if we don't have one as it did before.
Comment #6
grndlvl CreditAttribution: grndlvl commentedComment #7
grndlvl CreditAttribution: grndlvl commentedForgot about the pathlanguage for the call to drupal_get_path_alias.
Comment #8
grndlvl CreditAttribution: grndlvl commentedHad a notice on node/add for new nodes.
Comment #9
agentrickardSeems to be working. This is likely the best approach, though I suspect a load test would be nice.
I did add one bit, to skip the lookup if $options['absolute'] = TRUE, because if that's set, then we are dealing with an external link that doesn't need a path lookup.
Also standardized the comments.
Comment #10
agentrickardI also wonder if we can cluster these queries by the source, so that we don't have to run an individual query for each domain.
Comment #11
grndlvl CreditAttribution: grndlvl commentedMaybe, but in most cases I would not think that an alias would be loaded for multiple domains on a page so it has the potential to load unnecessary data. With the domain switcher module && admin pages it will query the same path multiple times to get for each domain's alias and on node/edit but in other cases it should only be looking @ one domain -- right?
- edit
Thinking about this more it really would not be much more data assuming that only a subset of the domain paths are set.
Comment #12
agentrickardLet's move forward without that premature optimization, then.
Comment #13
agentrickardYou now have commit access.
Comment #14
grndlvl CreditAttribution: grndlvl commentedLooking @ this further we should include the whitelist mechanism from drupal_lookup_path to account for system paths we should not care about.
Comment #15
agentrickardAgreed. Should we commit this first and add that as a follow-up? I'm a big fan of incremental improvements.
Comment #16
grndlvl CreditAttribution: grndlvl commentedCommitted in fdf786a
Comment #17
agentrickardShould probably be backported to d6, if anyone cares.
Comment #18
i-trokhanenkoClosing a Drupal 6 issue. It's time. Thanks!