Inter domain linking
ariflukito - August 27, 2008 - 05:58
| Project: | Domain Access |
| Version: | 5.x-1.10 |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | patch (to be ported) |
Description
Hi I would like to have an option where DA rewrites all outbound links to other affiliate nodes but not rewriting links to nodes that are published to all affiliates.
To be short and simple, something like SEO but minus rewriting all affiliate links to root domain.
Also it would be nice to have DA checks if url_alias table is prefixed when rewriting links. In case the node has a different alias in other subdomain.
#1
Hi I would like to have an option where DA rewrites all outbound links to other affiliate nodes but not rewriting links to nodes that are published to all affiliates.This already exists. Look under the Advanced Settings for "Default source domain" -- see attached.
Checking the url_alias for prefixing would be possible, but take a little code trickery. I would like to see a patch for that.
#2
ah I dont have that option, is that D6 only?
what is the best way to alter path aliasing, as far as I know there is no hook for that.
The only way you can do it is by patching core. Am I correct on this one?
#3
You must enable Domain Source, even if you do not use it.
[edit: I said "Domain Strict" originally, which is incorrect.
#4
oh I tracked this down
looks like the d5 port was wrong
compare this
D6 http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/domain/doma...
D5 http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/domain/doma...
#5
What am I supposed to be seeing?
#6
This feature is in 5.x.1.5 and higher. It does not require Domain Source.
#7
sorry I was in hurry yesterday
drupal 5 doesn't have the option "Do not change domain"
D6 -> $options = array('-1' => t('Do not change domain'));
D5 -> $options = array();
edit: attached patch for D5 head
also I edited my comment above after you posted
what is the best way to alter path aliasing, as far as I know there is no hook for that.The only way you can do it is by patching core. Am I correct on this one?
#8
Now I see. The patch is appropriate, but you should compare the custom_url_rewrite_outbound() function in both versions as well, since we act based on that setting in that function.
You can (in D5) alter path aliasing by using custom_url_rewrite(). DA includes a patch that backports custom_url_rewrite_outbound() and custom_url_rewrite_inbound() from D6. With some modification to custom_url_rewrite_outbound(), you could do domain-specific path lookups.
You would have to determine the target domain for the link, then switch database tables, and then write the link. It is possible, but I do not have time at the moment.
#9
yes my patch has already included the change on custom_url_rewrite_outbound()
custom_url_rewrite_outbound() on D5 is called before drupal_get_path_alias() so it is a bit tricky here.
#10
custom_url_rewrite_outbound() on D5 is called before drupal_get_path_alias()The included patch does that? If so, we should rewrite the patch.
#11
ok here is my first attempt
patch for drupal 5
patched common.inc to have the same execution path as drupal 6 so that the custom_url_rewrite_outbound() is called after drupal_get_path_alias()
the alias lookup codes mostly borrowed from drupal_lookup_path()
#12
Looks good, just from reading through the code. This will need to go into the 5.x.2 / 6.x.2 release, since it changes the patch that needs to be applied.
Are your tests working as expected?
The only other thing I can think is how to make this optional, so that we only use this routine in certain cases. Maybe we provide a separate include with Domain Prefix so that, if you want this feature, you place a different include file in settings.php.
#13
Status change!
#14
it's not perfect yet, I haven't handled certain cases. I roll out a new one later.
#15
ok here is the new one
I separate it into new functions
domain_prefix_get_path_alias() and domain_prefix_lookup_path()
they work the same way like drupal except the domain_prefix_lookup_path() only does aliasing.
I have done a little bit of testing and it is working so far with my setup.
#16
patch for D6
I've just found one problem unrelated to the patch but semi-related to the issue
If we have different url alias in different domain for the same node, domain switcher and domain nav will not work.
They will give page not found error.
#17
That makes sense. Take a look at the domain_uri() function, it may need to be extneded with your path logic.
#18
ok here is the patch for domain_get_uri() function.
now we just need to decide where to put those new functions, you still want to make it optional?
#19
I don't know. I am in Budapest after DrupalCON Szeged, and will not get a chance to really think about this until Monday.
#20
Just took a look. The one part that I don't like is the direct call to domain_prefix_get_path_alias() and the function_exists() that goes with it.
It would be more Drupal if we used a hook here instead.
So I suppose we would need hook_domainpath(), which would let other modules modify the path.
<?phphook_domainpath($domain_id, $path, $path_language = '') {
// code here
}
?>
Then we can just invoke the hook without having to use the function_exists() bit in our code. In this case, we do not need a separate settings.php file for Domain Prefix.
#21
#22
I don't think letting multiple modules modify the path is a good idea. I don't see any benefit doing that.
If module A and module B implement hook_domainpath(). Module A alter the $path to $new_path then what would module B do?
or if both module A and module B return different path based on original path, which one would we choose?
note that using hook indirectly call function_exists()
well I know it's a little bit ugly but I don't see using hook is an option.
what do you think?
#23
Using a hook is how we do things. Look at hook_domainload() or hook_nodeapi().
Will there be cases where modules compete? Possibly, but that can be fixed at the module level. Our task is to build a modular framework that supports extension. Running module-specific functions calls (exceptions, really) goes against that goal.
#24
Ok here is the new patch
drupal 5 needs new common.inc patch from #11
domain_get_uri() patch is merged into this patch
can you please commit the patch from #7
edit: patch updated with same file name, make sure you don't get the cached one
#25
OK, the patch from #7 has been committed to the d5 branch -- it is easy to lose track of multiple patches in an issue.
#26
This got confused with another patch. #7 has been committed. #24 still needs review.
#27
Updated d5 patch
d6 head looks unstable now.
#28
D6 HEAD is working. Domain Views is incomplete, but the rest should be fine. See http://drupal.org/node/320388
This should actually apply to D6 pretty cleanly.
#29
This has been committed to HEAD (D6). Note that I took the opportunity to optimize the way this code works.
Two special considerations:
-- Any implementation of this hook will now run on _all_ path lookups, not just when DA rewrites the path. This is by design, since there are paths other than node paths that might need to be rewritten.
-- Because of this, the hook could be _very_ resource intensive. So I broke it out into a special file that is only loaded IFF we know that a prefixed {url_alias} table is present. This saves a ton of potential calls to the function that might not be needed.
For details, see domain_prefix_init() and domain_prefix.path.inc.
Updated patch attached for D5 reference -- though the patch is for D6.
#30
Wasn't that already the case before? The hook is only needed to run when DA changes the base_url or else the lookup will be handled by drupal_get_path_alias().
<?phpif ($path_rewrite > 0) {
$path = domain_path($_domain['domain_id'], $original_path, isset($options['language']) ? $options['language']->language : '');
}
?>
This wouldn't work. The first parameter $_domain['domain_id] causes the lookup on active domain's url_alias table. What we want here is the target domain_id.
In the domain_get_uri(), you added this.
<?phpif ($string == $alias) {
$alias = drupal_get_path_alias($path);
}
?>
What is it for? It doesn't seem to do anything. $string is the aliased $path, so drupal_get_path_alias($path) will give you $string. But $alias is already equal to $string based on the condition. I am confused.
#31
I took that last bit out last night, so check the newest version of HEAD.
The code in custom_url_rewrite.inc is correct. Because of how the Domain Switcher block works, we have to run some checks against the default 'url_alias' table.
And no, custom_url_rewrite.inc is designed to _only_ rewrite node paths -- see the check for if ($nid) near the top. So it would have had no effect if you wanted to change path aliases for a path like 'user,' so we have to run the multi-domain path lookup on _every_ path. The result, if you have multiple domains, is that you might end up running dozens or hundreds of path lookups per page.
The code seems to be as optimized as it can be, though.
Please test before claiming that something doesn't work.
#32
I'm sorry I should say I have already tested that and it didn't work. The first paramater is telling to do the path lookup on this domain.
I think it doesn't matter on how many domains you have, there is only one lookup per path.
I think you may have a different use case to mine. My use case is I want to have a same alias for 2 different nodes. First node is assigned to domain A and the other to domain B. I want to link both nodes from root domain. So I have something like this:
a.example.com/products -> node/1
b.example.com/products -> node/2
the alias on root domain
example.com/products-a -> node/1
example.com/products-b -> node/2
without the rewrite I get this
a.example.com/products-a and
b.example.com/products-b
and as a result I get page not found error.
edit: this is unrelated I noticed on Node Link Patterns you have this "node\%n\outline". It uses forward slashes instead of back slashes like other patterns.
#33
performance improvement, avoid calling db_table_exists multiple times
#34
Yes, you were correct. I had tested it in the Domain Switcher, but not in custom_url_rewrite().
Patches committed to HEAD.
The problem that remains -- which I am not sure how to solve -- is what to do if you have two separate aliases for a path that is not a node. The Domain Switcher handles this through the use of domain_get_uri(). But normal links do not, and I do not see any way to handle that without some very nasty regex code that I don't think we need.
In most cases, this will not be an issue, though I could see problems with menu links that span multiple domains -- unless they are node paths, they will not be aliased correctly. So perhaps just a documentation note about the limitations of this approach.
#35
Couple things:
- $domain[$nid]['domain_id'] is not always set, so I introduce a new variable $target_domain_id default to $_domain['domain_id']. And if we use default source, the target would be default source domain id.
- I added check $path != '' because I saw '' and '' (empty string) were being passed around.
- Added drupal_is_front_page() check in domain_get_uri(), so we don't see something like http://example.com/node, http://a.example.com/node in the domain switcher when we are in the front page.
One more thing that I am thinking now. In the domain_prefix_domainpath(), it's not needed to do the path lookup if the target domain id is equal to the active domain id (ie: $domain_id == $_domain['domain_id']). Because it is just duplicating the job of drupal_get_path_alias(). We just need to return the original aliased path but then we need to pass it to the function.
I am not really getting the problem you mentioned. How can you get two separate aliases, you have 2 separate aliases for the path in same url_alias table? Mind giving me some examples.
#36
I mean 'two separate aliases (on two different prefixed domains) for the same path that is not a node path.'
New patch committed. Nice fixes.
#37
hmm right but that's a rare case unless you construct the URL yourself, it won't be any problem. I believe if you want to have menu links that span multiple domains, you need to give the absolute URLs there and those URLs don't get passed to url() function.
One more thing that I am thinking now. In the domain_prefix_domainpath(), it's not needed to do the path lookup if the target domain id is equal to the active domain id (ie: $domain_id == $_domain['domain_id']). Because it is just duplicating the job of drupal_get_path_alias(). We just need to return the original aliased path but then we need to pass it to the function.what do you think about this one? Just to clarify the last part, add the original alias to the hook's argument.
#38
I think that needs to be accounted for. Committed to HEAD.
<?php
function domain_prefix_domainpath($domain_id, &$path, $path_language = NULL) {
global $language, $_domain;
// $map[$domain_id] keys are Drupal paths and the values are the corresponding aliases
static $map = array(), $count = array(), $status = array(), $tablename = array();
// If the alias is for the current domain, do nothing.
if ($domain_id == $_domain['domain_id']) {
return;
}
// ... rest of function ...
?>
#39
The problem with that is it will just return the original path (node/$nid) not the original aliased path.
<?php$path = domain_path($target_domain_id, $original_path, isset($options['language']) ? $options['language']->language : '');
?>
See we only pass the $original_path to the hook.
#40
new custom_url_rewrite_outbound.patch for d5
#41
So you mean:
<?php// If the alias is for the current domain, do nothing.
if ($domain_id == $_domain['domain_id']) {
$path = drupal_get_path_alias($path);
return;
}
?>
#42
no something like this
<?php
$path = domain_path($target_domain_id, $original_path, $original_alias, isset($options['language']) ? $options['language']->language : '');
function domain_prefix_domainpath($domain_id, &$path, $original_alias, $path_language = NULL) {
//snip..
// If the alias is for the current domain, do nothing.
if ($domain_id == $_domain['domain_id']) {
return $original_alias;
}
//snip..
}
?>
#43
Ah. I didn't see the extra parameter in the function. This will have to wait until I get home.
#44
I added the extra paramater, it is not in the original function.
#45
If we do this check inside domain_path() itself, do we need to worry about the $original_path var?
#46
If we do the check inside domain_path() then there will be no path altering for all paths to current domain. That's not a problem for domain prefix but I don't know if other modules will need it.
Also I think probably it's not a good way to pass the path by reference, the other hook implementations will get the altered path instead of the original one.
#47
here is patch for D5 port and small fix for D6 (wrong table naming when we have global prefix)
note: in domain_prefix_table_exists(), we don't need to call db_escape_table() explicitly since db_table_exists() is going to do it for us.
#48
The small D6 patch has been committed. Marking as to be ported.
#49
I just committed a change to HEAD, since we were accidentally dropping language paths.
So this needs a retest.
#50
This will need to be tested post-release, it appears.
#51
this hasn't been ported to D5 and custom_url_rewrite_outbound.patch from #40 is needed to make sure custom_url_rewrite_outbound is called after drupal_get_path_alias.
#52
#53
Here's the patch from #51 revised for 5.x-dev.
I am confused about the note referring to #40.
Can you simply update the patch file with the changes to the custom_url patch?
#54
Hi Ken, in your revised patch it doesn't include the added new file domain_prefix.path.inc
The patch on #40 is meant to replace the custom_url patch. What the patch actually does is change the code flow so that the custom_url_rewrite_outbound function is called after drupal_get_path_alias since both functions modify the $path.
#55
Can you re-roll?
#56
The patch on #51 still applies cleanly but I reroll anyway
I attach 2 custom_url_rewrite_outbound.patch
- the first one is a reroll of #40, it's basically a backport from D6
- the second one is smaller patch, it works as well based on my test
#57
#58