Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

uri callback needs complete removal or documentation #5641

Open
argiepiano opened this issue Jun 2, 2022 · 5 comments
Open

uri callback needs complete removal or documentation #5641

argiepiano opened this issue Jun 2, 2022 · 5 comments

Comments

@argiepiano
Copy link

argiepiano commented Jun 2, 2022

In #164, callback definitions were removed from hook_entity_info() in favor of adding methods to entity classes. This included uri callback, a legacy from D7 that was used by the entity controller to build the url. Entities now are supposed to override Entity::uri() instead.

The problem is that uri callback is still being used in entity.tokens.inc - entity_token_info_alter() checks if uri callback exists when deciding if the 'url' token should be defined for the entity type. (As a side note: the token fulfillment function uses Entity::uri() rather than the uri callback to look up the entity uri).

The only core entity still providing a uri callback is file.

We should either remove these last uses of uri callback or document it properly in entity.api.php.

Removal presents a problem as uri callback is still a useful way to know if the entity "wants" a uri token to be defined: when you are defining tokens, you don't have an entity against which to check if the uri method returns a valid uri. We could either create a dummy entity (ugly solution, but done with create access for some entities), or provide uri tokens for ALL entities, but this is problematic if entities do not want to support a uri for viewing or editing (there are examples of this in Ubercart and OG). In those cases, you don't want the site builder to see url tokens for those entities.

@indigoxela
Copy link
Member

The File entity lags behind in many ways. The uri callback might just be another outdated thing.

The callback only makes sense if an instance exists, as it currently returns 'file/' . $file->fid;. I didn't dig deep, so one question: what's the use case for a token containing the uri, if there's no entity (instance of File)? I don't get why the dummy entity should be necessary.

@argiepiano
Copy link
Author

argiepiano commented Jun 3, 2022

The callback only makes sense if an instance exists, as it currently returns 'file/' . $file->fid;. I didn't dig deep, so one question: what's the use case for a token containing the uri, if there's no entity (instance of File)? I don't get why the dummy entity should be necessary.

This has to do with the ways tokens are defined and fulfilled. They are completely separate processes. It's ancient code that really needs updating:

  • Tokens are defined abstractly by hook_token_info(). The definitions are created without the need of an actual entity. This hook simply returns a multilevel associative array with two keys: types and tokens. These are simply definitions of types and tokens.
  • For the fulfillment/replacement, you use hook_tokens() and do pass entities and other data to it. This hook basically returns the replaced values as strings

So, to your question: uri callback was used in ancient times to decide whether to create a token DEFINITION for a core entity. If the entity info contained uri callback, then hook_token_info created a definition for the token [ENTITY_TYPE][uri]. There wasn't a need for an actual entity instance to create that definition.

Without the availability of uri callback in most entity info arrays, then this check is not possible to perform anymore. Without uri callback the only way to know if the entity "wants" a uri token is by creating a dummy entity and invoking its uri() method, then see what it returns. If it returns something valid, it means that there is a valid uri and we should define a uri token. If it returns NULL (like some entities in OG, Ubercart, etc) then we assume the entity doesn't "want" a uri token, and therefore we don't define it.

So, without uri callback there is no way for an entity to "tell" system_token_info() to not define a uri token for it.

[EDIT: corrected many typos - gosh my brain is still asleep]

@indigoxela
Copy link
Member

indigoxela commented Jun 3, 2022

the only way to know if the entity "wants" a uri token is by creating a dummy entity and invoking its uri() method

I see... sort of. 🤔

system_token_info() doesn't define any uri tokens, though.

$types['file'] has a 'needs-data' => 'file' set. So file tokens for path, url, size, mime ... always have an instance available.

I suspect, you're not actually talking about the tokens defined by core (system), but about tokens via module entity_tokens. Is that correct?

@argiepiano
Copy link
Author

you're not actually talking about the tokens defined by core (system), but about tokens via module entity_tokens. Is that correct

I was referring to core tokens, but I typed the wrong function in my last response to you - I meant entity_token_info_alter(), which is where the uri token is defined.

@indigoxela
Copy link
Member

Sorry, yes, it's in the "alter" hook.

Following your explanation this all makes sense, but could you also provide an actual situation, where this either breaks functionality or causes problems? Not sure yet about the best approach (remove / restore...).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants