Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Currently, the call to uri() will return NULL if there is no uri callback specified for the bundle or entity. This is the case for the entity_test in D8.
However, if we want to use the @id key in JSON-LD to reference entities, we need a URI.
Proposed resolution
Default to a standard callback if the uri callback isn't defined on the bundle or entity.
Comment | File | Size | Author |
---|---|---|---|
#28 | 1803586-28-entity-uri.patch | 3.56 KB | linclark |
#28 | interdiff.txt | 1.91 KB | linclark |
#25 | 1803586-25-entity-uri.patch | 3.04 KB | linclark |
#25 | interdiff.txt | 2.88 KB | linclark |
#7 | 1803586-07-entity-uri.patch | 2.62 KB | linclark |
Comments
Comment #1
larowlanAdd vocabularies to the list (see taxonomy_entity_info).
Comment #2
scor CreditAttribution: scor commented+1, that makes sense to me. The default pattern could be
entity/[entity-type]/[entity-id]
, for exampleentity/bla/1
. Should we consider using UUID in these URIs and skip the entity type in the URI? (#1726734: Replace serial entity IDs with UUIDs in URLs, or even globally?).I'm assuming the route for this default path would be registered once by the Entity property API maybe, so that each new entity implementation does not have to define it?
For entity types which define their own uri() like node, it means the path
entity/node/1
would work and be a duplicate ofnode/1
. Is that a problem? I guess it's fine sinceentity/node/1
should not be used or nor published anywhere as long as we always use the uri() method to retrieve the URI. It is implied that entity type with a custom uri() should also register their route for it.Comment #3
rszrama CreditAttribution: rszrama commentedI'm not a huge fan of [entity-type] as an argument in the URL, but I suppose it's trivial enough for an entity type to define its own URI and never have to use this pattern. I suppose you add the entity type to ensure we don't have ID collisions between different entity types using the generic pattern, but then why not just make the default pattern
[entity-type]/[entity-id]
? You can't have entity type naming collisions, and even if such a path was already in use on the front end, the fact that an API request accept header specifies JSON-LD should be enough to differentiate it from any browser based request.If that's insufficient, I'd think it'd be better as you suggest to have a single entity collection in core based on UUIDs. The
entity
resource could be filtered by entity type with a simple query parameter, and I presume you'd never have a collision atentity/[entity-uuid]
. It just may be awkward for the default pattern to use UUIDs if every other entity URI pattern used entity IDs.Comment #4
Anonymous (not verified) CreditAttribution: Anonymous commentedIt is important to note this URI isn't specific to JSON-LD. It is the URI set in the entity system, which I believe is sometimes used to construct other paths which can be used for browser based requests.
That said, I doubt that we would have collisions if we use the [entity-type]/[entity-id] pattern, and would be fine with either that or entity/[entity-uuid].
Comment #5
moshe weitzman CreditAttribution: moshe weitzman commentedIs it a problem to offer two equivalent routes, one with a UUID and one with a local ID? Seems reasonable to me.
Comment #6
scor CreditAttribution: scor commented@moshe: could be a problem wrt SEO and duplicate content, so one of these should be chosen and set to be the canonical URI of the entity via a rel=canonical in a link element (related: #989032: Insert the canonical tag into all content, not just into nodes).
Comment #7
Anonymous (not verified) CreditAttribution: Anonymous commentedJust posting a patch so we have something to work from. This adds a whole new test for the URI handling because I couldn't figure out an existing test where it would fit.
Comment #8
fagoGenerally, this sounds like a good idea. But, if an entity has an URI, I'd assume I can link to it - what won't fly if there is no HTML representation of them available... :/
So, instead couldn't we just default to such a pattern for the json-ld representation only? Still, RDF probably has the same needs - so maybe we better add a helper function for that?
@uuid-vs-id: I agree we want do id by default, as we do elsewhere. But adding both - uuid and id paths - should be fine, the canonical url tag resolves the issue for HTML and for REST services it's fine also as long as you always use the *same* URI for referring to the entity.
Comment #9
Anonymous (not verified) CreditAttribution: Anonymous commentedThere might be other (non-RDF) serialization modules which need the URI, and we shouldn't mint different URIs based on serialization. Because of this, I don't think that we want to define the helper function in RDF module. We could potentially add a helper function to Entity.
Is the uri() function used by code to infer routes that should return HTML? Or does it just imply to the human coder that they should be able to use it as an HTML link?
Comment #10
fagoIt's mostly the human coder thinking that. But the same way it should be possible to only link to the entity if it has an URI, e.g. as the helpers from #1637342: Add entity_url() and entity_l() wrapper functions to simplify using EntityInterface::uri().
Comment #11
Anonymous (not verified) CreditAttribution: Anonymous commentedThere's a proposal in #1332058: Need a way to distinguish "public/final" URIs for entities from admin and redirected ones, which I callout in #10, which might address fago's concern.
Comment #12
Crell CreditAttribution: Crell commentedI'm fine with a default path for all entities of /entity/{entity_type}/{entity-id}. That gives us a standard to work from, which can be overridden for a particular entity if appropriate (eg, nodes, terms, users, all do so already.) I'm not sure if we should use UUIDs there or serial IDs. UUIDs are useful, but serial IDs would be consistent. 'course, non-content entities probably will have machine names, not UUIDs or serial IDs, so...
Comment #13
andypostClosely related #1783964: Allow entity types to provide menu items
Comment #14
yched CreditAttribution: yched commentedJust to be sure - are we talking about giving URIs to entities across all entity types ? Including config entities (i.e "a view", "a field", "a breakpoint"...) ?
Comment #15
Anonymous (not verified) CreditAttribution: Anonymous commentedIf we want to expose those entities in JSON-LD, then it makes sense to give them URIs. If a request to that URI used "Accept: text/html", they would either get a 415 Unsupported Media Type, or they would get human readable documentation of the config.
I'm fine with not exposing config entities, but others have use cases for it, such as MASt module.
Comment #16
Grayside CreditAttribution: Grayside commentedRegards to Serial vs. UUIDs, URI should consistently use one, can use querystring for lookups by other elements.
Comment #17
moshe weitzman CreditAttribution: moshe weitzman commentedThe most recent patch looks fine to me. Do we need to actually implement uri callback anywhere or is this good to go?
Comment #18
moshe weitzman CreditAttribution: moshe weitzman commentedAdding tag
Comment #19
chx CreditAttribution: chx commentedOne point for id: /entity/1234-adgf-gftrutyrnfgh is as ugly as it gets.
One point for uuid: <a href="/entity/1234-adgf-gftrutyrnfgh"> works across environments even if the id changes.
Both is moot when aliases are used but as this is about a default, I presumed they are not. Between the two, I think my preference goes to id, the second one should be a reference field.
Comment #20
chx CreditAttribution: chx commentedAlso? entity/$entity_type/$id the lookup time is O(1) but entity/$uuid is a black horse -- you would need to go over every entity type and query 'em. If you go entity/$entity_type anyways then doing entity/$entity_type/$uuid seems awkward and pointless.
entity/$entity_type/$id it is.
Comment #21
andypost+1 to entity/entity_type/id
having uuid as key seems ugly but could work
Comment #22
jibranRelated #1899816: Various config entity types are missing URI callbacks
Comment #23
smiletrl CreditAttribution: smiletrl commentedpatch #7 looks good, but I'm thinking can we force entity to have a URI callback at the beginning? Maybe URI isn't used for that entity-self, but for JsonLD, it could be useful. I also saw this info at WSCCI sprint
Comment #24
andypostI think that DrupalUnitTestBase is enough here
Is it needed at all? Anonym could be used
Comment #25
Anonymous (not verified) CreditAttribution: Anonymous commentedYeah, DrupalUnitBaseTest wasn't around when the first patch was posted. I've converted the test.
I also updated the URI structure to match the pattern everyone seems to agree on, entity/$entity_type/$id
Comment #26
tim.plunkettFunctionally, its great. Just some nitpicks on the coding standards.
This seems to change the expected return value of EntityInterface::uri(), implying that NULL should never be returned. If so, we should update that.
For some reason, the coding standard is to not give setUp a docblock.
The indentation is off here, and setUp should be protected
Tests that...
Comment #27
sunHm. Cross-linking: #1907208: Some config entities do not respect uri callbacks
Comment #28
Anonymous (not verified) CreditAttribution: Anonymous commentedThanks for the review and catching the interface change, Tim :) Fixed all.
Comment #29
chx CreditAttribution: chx commentedwell, that's great, but what happens if you actually visit that URL? is this question out of scope?
Comment #30
Anonymous (not verified) CreditAttribution: Anonymous commentedIf REST module is enabled, GET requests will be served a serialized entity in the data format that was requested if it is enabled. Otherwise, they get HTTP Error 415 Unsupported media type.
Comment #31
Crell CreditAttribution: Crell commentedSilly question: In this case, why do we even need uri callbacks? Just let a specific entity type subclass and override the method. Done.
Comment #32
klausiPatch looks good.
@chx: if you visit that URL and rest.module is not enabled you get a 404, because there are no routes registered for the path. If you enable rest.module you will still get a 404. If you then enable the specific entity resource and visit it with your browser you will get a 415 for entity_test and some other 5xx error for entities that do not implement EntityNG yet.
@Crell: offtopic for this issue, whether we want uri callbacks or not is a separate issue that should be opened if you feel like it.
Comment #33
webchickCommitted and pushed to 8.x. Thanks!
Comment #34
Dave ReidBumped #1332058: Need a way to distinguish "public/final" URIs for entities from admin and redirected ones to a major task and Contributed project blocker as adding URIs to all entity types prevents contrib modules from knowing which entity types are 'public' and meant to be visible to end users, and which ones are not and shouldn't get things like meta tags, redirects, etc.
Comment #36
dwwA) This conflicts with and breaks the bug fix we're trying to do at #1991464: user_uri() should not return an invalid path (user/0) for the anonymous user object (which might be duplicate with #1275902: Allow entity URI callbacks to indicate that the entity has no URI, and make the User module use that for anonymous users, which is really a bug). user/0 is a 404. Why should we be forced to return broken links? Or, is the theory that user/0 should return something about the anonymous user now, just since we support a bunch of other serialization formats? WTF? ;)
B) Shouldn't there be a change notice for this if we end up keeping it?
Comment #37
fagoI agree this needs a change notice.
Comment #38
BerdirI'm not sure if anything from here is actually left, it all changed 3 times anyway, so I don't think we need to keep this open.