'as link' formatters need a generic way to build the url of an 'entity'

oh totally, i hope this doesn't end up totally tied to the fields API because we really need similar functionality for the files table. over on #353458: hook_file_references() was not designed for a highly flexible field storage i'm trying to kill hook_file_references() and move it to a table so that user, nodes, comments, what ever can hold a reference to a file and this would be a great way to build a link to the them for administrative purposes ala #322287: Add file.module to provide UI for managing files.

Subscribing as #16161: Move Read More link to end of node content would benefit from this too.

I'm new to the whole Fields API story, but as I understand it, one of the main benefits of fields is their flexibility. In that light I think a hook or callback solution (with default fallback {type}/{id}) is preferred over the pseudo token string.

Suggestion:

Let's add a "view path" to entity declarations in hook_entity_info. This could then contain an arbitrary URL, with a wildcard for the ID (or, even better, for an arbitrary field of the entity).

So e.g in node_entity_info() you'd had

$return['node']['view path'] = 'node/%nid';

and to create the link to an entity, you'd match the wildcard and replace it with the identically named property of the actual entity object.

This would provide a very flexible solution also for not-so-standard entity types, and could also be altered by contrib through hook_entity_info_alter().

The attached patch adds a function to create the URL to an arbitrary entity, following the suggestion I outlined above.

It's working well. I think this solution should be flexible enough.

Note that this is not yet integrated in field API. I didn't find a spot so I'm leaving this to the field api guys.

re-roll to chase head, but didn't include all of yched's comments from #7, thus leaving as 'needs work'.
In particular, still need to address

+++ includes/common.inc
@@ -5165,6 +5165,37 @@ function entity_get_controller($entity_type) {
+    preg_match('!%([^/]+)(/|$)!', $info['view path'], $matches);
+    // Get the property of the entity that has the name of the wildcard.
+    $property = $entity->{$matches[1]};

We'd possibly need to support several replacements ?

and

+ We'd also need comment/%cid for comments

.

re-roll to incorporate yched's:

• supporting multiple replacements
  • I'm not sure how one might go about handling recursive regex substitutions, so I explod()ed and foreach'ed it. There's probably a more elegant solution.
• comment/%cid for comments

Re-rolled with better tools, and fixed an error I had made.

Looks proper to me .. .since file module is in core now, do we have a use case yet? regardless, this is ready.

Very elegant.

+    $pieces = explode('/', $path);
+    // Replace wildcards in the view path.
+    foreach ($pieces as $piece) {
+      // Match the wildcard
+      if (preg_match('!(%[^/]+)(/|$)!', $piece, $matches)) {

The regexp looks too complex. $pieces is exploded on /, so $piece will never contain a slash. And (/|$) is redundant AFAICT, because the + quantifier is greedy, so it will always match until the end of the of the string (or to a slash if there was one anyway).

+        $path = str_replace($matches[1], $property, $info['view path']);

If there are more %foo placeholders in $info['view path'], only the latter is replaced, because $path is overwritten inside the loop.

I suggest something like this instead (not tested):

foreach ($pieces as &$piece) {
  if (substr($piece, 0, 1) == '%') {
    $piece = $entity->{substr($piece, 1)};
  }
}
return implode('/', $pieces);

Also, some tests would be great.

Imagefield just went in without this. While this looks good, it's probably Drupal 8 material at this point.

I agree this seems important, and I mostly agree with the comments in #14. But since code freeze is passed, we could actually do this in a much simpler way, by assuming the link is [view path]/[id] (untested):

function field_create_path($entity_type, $entity) {
  $info = entity_get_info($entity_type);
  list($id) = field_extract_ids($entity_type, $entity);
  return $info['view path'] . '/' . $id;
}

We keep the part of the patch that adds 'view path' to entity info.

If we actually do #606994: Move entity handling out of Field API, which is just renaming (or copying to preserve the API) field_extract_id() to entity_extract_id(), then this can be used system-wide.

The only other thing this should support is an optional $vid

Can't find anything to complain about, we removed hook_term_path() in favour of hook_url_alter(), however this would be a more performant solution for modules wanting to recreate what we had in D6, since hook_url_alter() runs on everything. It's a small API change for modules which expose their own entity types, but these are very few, if at all, at the moment.

Correct me if I'm wrong, but don't we lose functionality here? How does Taxonomy Rewrite module change the path returned by taxonomy entities with this approach?

1) These are internal paths, not URLs - URL rewriting happens later.

2) A hypothetical taxonomy_entity_rewrite.module would implement hook_entity_info_alter() to replace the path callback function with its own.

Ok, committed to HEAD. Thanks!

Oops. Needs documentation.

Holy crap, I love you all. This is a great addition to entity API. And a huuuuuge helper for Pathauto D7.

Oh, I guess I was assuming somewhere we had some nice documentation about how to create entities, which would need to be adjusted to take this change into account.

HAHAHA I KILL ME. :(

Ok, setting back to fixed then.

Oops, we're missing the path callback in user_entity_info().

Committed. But you know what that means, right? :P

As discussed with catch in IRC, this approach does not fully support the case of the comment entity. The comment path callback value cannot be reused in the theme layer because it's a path internal to Drupal and it cannot include a fragment, as a result, in template_preprocess_comment() we end replicating (twice!) the logic of what comment_path() is supposed to do:

  $variables['title']     = l($comment->subject, 'comment/' . $comment->cid, array('fragment' => "comment-$comment->cid"));
  $variables['permalink'] = l('#', 'comment/' . $comment->cid, array('fragment' => "comment-$comment->cid"));

Also, the code above costs us an extra call to url() via l(): this is to be multiplied by 50 comments / page or more. We should allow the entity path callback functions to return a structure containing the path but also optional options compatible with url():

/**
 * Entity path callback.
 */
function comment_path($comment) {
  return array(
    'path' => 'comment/' . $comment->cid,
    'options' => array('fragment' => 'comment-' . $comment->cid),
  );
}

EDIT: ignore the extra call to l(), this will be dealt in a different issue, which will leverage the full support or url friendly entity path callback and make it cacheable by entitycache.

and here is the patch

Just in case 'path' isn't the first item in the array, current(entity_path($obj_type, $object)) take the array key directly instead of using current.

Reading #699440: Add bundle support to entity_uri callback to remove performance overhead of forum_url_outbound_alter() (regression from D6) might help to understand better what's the reason behind the patch #35.

@catch: I was trying to do that in one line but afaik PHP doesn't allow you to do that :(

@scor, yeah that's a two-line job :(

On terminology, hmm. Doesn't seem ideal, but can't think of anything better.

the terminology needs to be changed, but could not find something appropriate. we need something broader than path. how about url_structure or url_elements?

or simply url? entity_url(), 'url callback', [entity_type]_url()

Would url give the impression that it's a fully formed url, rather than a url() friendly array though?

solving #36:

Just in case 'path' isn't the first item in the array, current(entity_path($obj_type, $object)) take the array key directly instead of using current.

Also do we need to update the docs for hook_entity_info()? It currently says "path callback: A function taking an entity as argument and returning the path to the entity. "

patch addressing #44 and #45.

ignore patch above, this is the right one. I still think we need to have a better name than 'path' for the path and options elements, lines such as this are confusing:

    $path = $variables['path']['path'];

The key '#path_options' in image_field_formatter_view() does not get passed to theme_image_formatter().

I've made the path 'options' key default to array().

Discussed this with catch on IRC. The 'path' term is too specific here. I'd like to suggest the broader term 'uri' which includes the 'path' and the various 'options' like fragment and query which can be used to build the uri of an entity (comment entities have a fragment in their uri for example).

uri callback: A function taking an entity as argument and returning the uri elements of the entity, e.g. 'path' and 'options'. The actual entity uri can be constructed by passing these elements to url().

The '#path' key in the formatter would need to be aligned with the fact that we can have a uri instead of a path only.

just realizing I forgot to attach the patch in the previous comment... :/

#51: 525622_comment_path_50.patch queued for re-testing.

This looks great. I can't think of anything other than 'uri' which describes this somewhat accurately, and the url() friendly array is something we've done in a lot of functions. It's a teensy API change, but the path callback only went in a couple of weeks ago, isn't documented yet, there's barely any entity modules floating around, and it qualifies as a proper bug since it's actually the wrong thing for comments in HEAD - so we should get this in quick and /then/ document it.

Subcribing.

Committed to HEAD.

Oooh, thank you. Back to cnw for docs.

Follow-up issue: #823428: Impossible to alter node URLs efficiently

For people who worked on this issue, I would really appreciate your input on #823428: Impossible to alter node URLs efficiently. Dries and chx are questioning that patch, and therefore, I'm asking the community for how to make sense of the entity_uri() function. Thanks!

As per #57 - can you clarify what docs need to be written?

If it's an entry in the module/theme update guide that's needed, please change the tag to "Needs Update Documentation". If it's something else, you can leave the tag at "Needs Documentation", but please explain what is needed.