Problem/Motivation

Drupal's EntityBase class has a method `toLink()`.

The signature of this method is:

<?php
public function toLink($text = NULL, $rel = 'canonical', array $options = []) {...
?>

As you can see, the default $rel value is 'canonical', meaning that every Entity Type must implement the canonical link template (if not, this parameter should not have a default value).

Drupal's File entity does not implement the 'canonical' template, meaning that if you call a `$file->toLink('link title')` you will get an exception.

<?php
$etm = \Drupal::service('entity_type.manager');
$file = $etm->getStorage('file')->load(169751);
$file->getEntityType()->getLinkTemplates();
= [
    "delete-form" => "/file/{file}/delete",
    "devel-load" => "/devel/file/{file}",
    "devel-load-with-references" => "/devel/load-with-references/file/{file}",
    "devel-path-alias" => "/devel/path-alias/file/{file}",
    "devel-definition" => "/devel/definition/file",
    "token-devel" => "/devel/token/file/{file}",
  ]
?>

As you can see, there is no 'canonical' link template for the File entity. Calling `$file->toLink()` (as you should be able to with any Entity, since they all extend EntityBase, causes an Exception, which is not appropriate for a default call of a method. It would be appropriate if you passed a template that did not exist,

Steps to reproduce

1. Load a file entity.
2. Attempt to render an entity link for that file using the `toLink()` method, using default parameters.

<?php
$etm = \Drupal::service('entity_type.manager');
$file = $etm->getStorage('file')->load(169751);
$file->toLink('link title');
?>
Drupal\Core\Entity\Exception\UndefinedLinkTemplateException No link template 'canonical' found for the 'file' entity type.

Proposed resolution

Option 1:
Core File module should supply a canonical link template.

Option 2:
Core File module's File entity should override EntityBase::toLink() and provide a default template that actually exists ('delete-form' in this case).

Remaining tasks

1. Decide on solution.
2. Create a merge request for the solution.

User interface changes

None.

Introduced terminology

None.

API changes

If option 2 is pursued, File::toLink() will have a different value for the default link template than other entities.

Data model changes

None.

Release notes snippet

TK

Comments

apotek created an issue. See original summary.

apotek’s picture

Title: File->toLink() throws exception due to missing 'canonical' link template » File->toLink() throws exception due to missing 'canonical' link template, which is the default template for the method
quietone’s picture

Version: 11.2.x-dev » 11.x-dev
Assigned: apotek » Unassigned

Hi, in Drupal core changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to the Core change policies. Thanks.

Un-assigning per Assigning ownership of a Drupal core issue. @apotek, if that is incorrect, add a comment and re-assign to yourself.

idebr’s picture

I believe it is intentionally that file entities don't have canonical route.

As @berdir mentioned in comment https://www.drupal.org/project/drupal/issues/2345761#comment-13180034

files will IMHO remain second-class citizens and we will not introduce an actual canonical route in core, and we decided against having one that points to the physical file.

See also #3406715: File delete form throws UndefinedLinkTemplateException without destination parameter

kim.pepper’s picture

Status: Active » Postponed (maintainer needs more info)

Leaving this open but marking PMNMI for any more feedback. Otherwise this will be closed as Won't Fix as per #4 in the near future.

apotek’s picture

> I believe it is intentionally that file entities don't have canonical route.

Yes, I believe it is intentional too :). My point here is that a class that is inheriting from a parent should be able to call the default version of its parent without an exception being thrown. If 'canonical' is not a safe default, then it should not be a default parameter. That's what I am trying to address.

There are a few ways to solve this:

1. Change the base implementation to not provide a default template.

This would end up being a breaking change for any module that relies on the `canonical` link template being the default option. But it would also be the most "correct" in that the base implementation no longer decides to make assumptions about what its inheritors need to do with regard to whether to provide a canonical link template or not.

2. Requiring that any inheriting class also provide the default link template (ie, force File module to implement the default `canonical` link template).

This would be a non-breaking change. We update documentation and base method declaration to properly document that you must provide a `canonical` link template in your implementation, and the file module would have to follow suit and provide one.

However, as @idebr points out, the maintainers of the File module don't want to have to provide a `canonical` link template. This is why solution 1 is more "correct," in that implementors of the base class should be able to do so without having something about that implementation forced upon them (in this case a requirement for a canonical link template).

3. Override the base implementation in the File module so that when you call `File->toLink()` it does what is intended by the method without triggering an exception.

The point is that a developer should be able to call File->toLink() without triggering an exception since the base class implementation allows for it, and the signature indicates that it should be possible to do.

Given all the above, while I think the best thing would be to re-engineer the base class, that is also the most disruptive. It would require a deprecation notice and a migration plan. So the short term solution would be what I have called Option 2 above, which would be for File to implement `toLink()` in a way that makes it callable without additional parameters and without triggering an exception.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

apotek’s picture

Status: Postponed (maintainer needs more info) » Active