PURL doesn't integrate well with all modules

Another thing that puzzles me a bit and might be related.

Purl is making a redirect instead of internally changing the url. So with the above example:

A/node will result in a redirect to node. This removes the context from the url, not like what's done in spaces, where the active space will be shown in the url. Also since a redirect is used, all static variables is deleted (a full new request is made).

If I have understood purl correctly, the url should be preserved and there shouldn't be made any redirect, so the above problem with the redirect loop, might really be a bug that purl shouldn't be redirecting at all.

I won't mind spending some time on this my self helping to find the cause of this and fix it. However since I'm still very new to purl, I would like some feedback to make sure that I have understood the module correctly and isn't going out on a wild goose chase. It might be my implementation that's completely wrong. I have tried to mimic what's done in spaces_og so it shouldn't be that far off.

Sure, I'll post my code.

This first part is snippets containing the code to save the slug, to purl. I left out all the code that didn't relate to purl. I also left out the update function, which only save the slug value if there is changes to it, or it wasn't set. The node type's name is branch

/**
 * Implementation of hook_form().
 */
function module_form(&$node) {
...
$form['slug_settings']['slug'] = array(
    '#type' => 'textfield',
    '#title' => t('Slug'),
    '#description' => t('Short branch name, used in URLs. Allowed characters are a-z, 0-9, and .-_'),
    '#default_value' => $node->slug,
    '#required' => TRUE,
  );
...
}


/**
 * Implementation of hook_insert().
 */
function module_insert($node) {
  module_save_slug($node->nid, $node->slug);
  ...
}

/**
 * Save/update the slug for a branch node.
 */
function module_save_slug($nid, $slug) {
  return purl_save(array(
    'id' => $nid,
    'provider' => 'module',
    'value' => $slug,
  ));
}

This next part is the actual integration with purl. A note: atm I'm storing the active branch in the $_SESSION. This is a quick way to fix the redirect problem, but I would probably prefer to just store it as a static variable. I'm not sure yet, if I want to save the active branch for several requests. Users when logged in, will be associated with a center, so I probably want to use that as a fall back instead of using a session variable.

/**
 * Implementation of hook_purl_provider().
 */
function module_purl_provider() {
  return array(
    'module' => array(
      'name' => t('Fitness center branch'),
      'description' => t('Determines the related fitness center branch.'),
      'callback' => 'module_set_active_branch',
      'example' => 'my-branch',
    ),
  );
}

/**
 * Set the active branch, stored in the user's session.
 *
 * @param (int) $nid
 *    The node ID for the branch to activate
 *
 * @return node ID for the active branch
 */
function module_set_active_branch($nid=NULL) {
  // I don't know why this is needed, but if purl_disable is not set to true we get a redirect loop.
  purl_disable(TRUE);

  // Store active center in the user's session. This makes it possible to save
  // The users choice between sessions.
  if (!empty($nid)) {
    $_SESSION['module_active_branch'] = $nid;
  }
  return $_SESSION['module_active_branch'] ? $_SESSION['module_active_branch'] : FALSE;
}

/**
 * Get the active branch.
 *
 * @return node ID for the active branch, FALSE if no branch in active.
 */
function module_get_active_branch() {
  return module_set_active_branch();
}

Different places in the code I use module_get_active_branch to get the active branch. Atm I use it for blocks and page callbacks. Also for now I don't set the method used like it's done in spaces_og, but instead I've set it to path in the admin interface. At some point this stuff will probably be added in the install hook or we might just set it up in the interface, since it should only be done once anyways.

I think all of this should be the relevant code, if something is missing or you need clarification, let me know.

I'm using beta12. At the time it was the most recent release.

However, I'm not using numbers in my purl aliases. It's possible, but for the test data I've used, only a-z values have been used.

so in the purl table:

value = [a-z]
provider = 'module'
id = [0-9]

I updated to the CSV version, and it seems to have fixed the issue, so must have been related to #847786: Numbers in purl aliases causing problems.

No redirects is made now, so url don't change and I can use static variables and I don't need to use purl_disable(TRUE)

Thanx for the help.

Setting version to beta-13 to indicate that the fix applies for this version.

I'm sorry, I was a bit to quick to resolve this issue.

I downloaded the CVS-HEAD version, I thought that would be the most resent version, but when I was about to commit the changes I noticed that HEAD is from 2009/03/08.

It seems that the old version from 2009 actually works as it should, while the new version DRUPAL-6--1 does not work. The symptoms are exactly the same. So it seems somewhere from the old version and the current CVS version, the bug must have been introduced.

So having a version that worked vs one that didn't work, helped quite a lot to debug the problem.

I tracked down the issue to drupal_init_path()

In the old code you did

$_REQUEST['q'] = $_GET['q'] = _purl_get_normal_path($q);

In the new code you did

drupal_init_path();

function drupal_init_path() {
  if (!empty($_GET['q'])) {
    $_GET['q'] = drupal_get_normal_path(trim($_GET['q'], '/'));
  }
  else {
    $_GET['q'] = drupal_get_normal_path(variable_get('site_frontpage', 'node'));
  }
}

So the only difference is that $_REQUEST is not changed in the new version. I did a search on my Drupal setup, and it turns out that globalredirect was causing the some of the problems. In it's init function it uses $_REQUEST instead of $_GET. Since modules can use either, the best thing to do, is to set both to the new value.

I added a patch that solves the issue.

The global redirect modules vs purl setup is pretty simple.

Purl with weight -20 will have first op, and change $_GET['q']

globalredirect will compare $_GET['q'] and $_REQUEST['q'], if they are different perform redirect to $_REQUEST['q'] (the un altered request)

This is essentially the loop that I ran in to.

If you truely don't want to change $_REQUEST['q'] but want purl working with globalredirect redirect, an alternative option would be to alter the weight of globalredirect to have it perform it's init before purl does it's alteration.

I don't like this option, as it only will be a fix vs one module, and can be broken by changing db values. Also, as mikl writes, $_REQUEST should be related to $_GET, according to PHP docs, so the best option would be to save the original request with a different name, so it still is accessible if needed.

@yhahn I've taken a bit closer look to globalredirect. It seems for the language rewrites, it makes a special case for that, by getting the language prefix and appending it to the url alias. I think you are right that the issue should be raised at globalredirect instead of here thus I'm closing this issue.

Thanx for the assistance on debugging this.

Updated close reason.