🇺🇸United States @davidwbarratt

I'm closing this because a more comprehensive solution is needed. I've documented the problem more clearly at 🐛 QueryInterface::accessCheck does nothing Active

I think the "use permissions to influence the query" is a solved problem? That's essentially implemented by existing query alters that should already be performing permission checks together with the query that's being altered.

heh. that is what brought me here and is better explained by 🐛 QueryInterface::accessCheck does nothing Active . Effectively there isn't a way to do this other than directly editing the SQL itself (which means it only applies to content entity queries that use Sql Storage), but at that point the query will be run (no opportunity to no-op the query), and you no longer get to work with the EntityQuery, you have to work with the SqlQuery. The only work-around I can find is by directly attaching to the access logic into the query when it is being built, which is what core does. Which I guess is the way I'm going to implement for now, which feels gross.

This would already be impossible to write as a simple entity condition in the current entity API because there's no path from the content entity (Node in the example, but could be any entity) to the membership table

You can use the conditions across entity references even with deep references, so as long as they are connected with entity reference, this isn't a problem (as far as I can tell). Of course, it does not allow one to make an arbitrary entity join which is somewhat annoying, but that seems like a separate issue.

Is another way to solve this problem by attaching scopes to the entity query? Wouldn't that express the intent without having to actually know anything about the query itself (other than the entity type and the scope)?

Oh. I think I understand now. Thank you for walking me through that. I didn't get that you could already revoke permissions with the Access Policy API. Now I see why we were talking past each other.

So riddle me this... given that we have this in NodeAccessControlHandler::checkViewAccess

protected function checkViewAccess(NodeInterface $node, AccountInterface $account, CacheableMetadata $cacheability) : ?AccessResultInterface {
    // If the node status changes, so does the outcome of the check below, so
    // we need to add the node as a cacheable dependency.
    $cacheability->addCacheableDependency($node);
    if ($node->isPublished()) {
        return NULL;
    }
    $cacheability->addCacheContexts([
        'user.permissions',
    ]);
    if (!$account->hasPermission('view own unpublished content')) {
        return NULL;
    }
    $cacheability->addCacheContexts([
        'user.roles:authenticated',
    ]);
    // The "view own unpublished content" permission must not be granted
    // to anonymous users for security reasons.
    if (!$account->isAuthenticated()) {
        return NULL;
    }
    $cacheability->addCacheContexts([
        'user',
    ]);
    if ($account->id() != $node->getOwnerId()) {
        return NULL;
    }
    return AccessResult::allowed()->addCacheableDependency($cacheability);
}

Are you saying the Access Policy API could allow you to bypass that bit at the end by revoking the user's view own unpublished content permission?

If that is the case, then....

What if, we had this hypothetical method / implementation for a query?

  protected function queryAccess(callable $conditionFactory, ?AccountInterface $account = NULL): AccessResultInterface {
    $cacheability = new CacheableMetadata();

    $cacheability->addCacheContexts(['user.permissions']);

    if (!$account->hasPermission('view own unpublished content')) {
      return AccessResult::neutral()->addCacheableDependency($cacheability);
    }

    $cacheability->addCacheContexts(['user']);

    $condition = $conditionFactory('OR')
        ->condition('status', 1)
        ->condition(
          $conditionFactory('AND')
            ->condition('status', 0)
            ->condition('uid', $account->id())
        );

    return AccessResult::allowedWithCondition($condition)->addCacheableDependency($cacheability);
  }

I assume you would also be able to prevent the hypothetical allowedWithCondition to be invoked by likewise revoking the user's view own unpublished content?

I realize I'm overly simplifying the example, you would also have other conditions that would/could be added based on the permission.

I'm confused, are you saying a module can undo a EntityAccessControlHandler? I know about hook_entity_access but if the former returns forbidden can the latter override that decision? It looks like AccessResult::orIf would prevent that?

If that is the case, then why would a query be any different at all?

If we take one of your suggestions, we would have to make them an editor on the entire website and then make sure they can't do anything in fashion, politics, economics, etc. which is a lot of extra work.

I'm still confused on how that any extra work than determining the intent of a query or trying to modify things? It seems like it would be like a single hook alter (to disable the existing permission) implementation and one additional permission conditional? Am I missing something really basic?

This would solve all of the problems I explained above because it would be crystal clear whether a status check is a filter or an access rule.

Providing a mechanism (a hook?) to either enable or disable an existing permission in core would fix the same problem without having to express intent at all, which is why I'm talking about the design decisions.

Would it be useful if you could take over permissions from other modules? For instance, what if you could just adjust which roles have a permission and "disable" permissions? In this case you could effectively grant all users the ability to access unpublish content globally, but then you can have your own "ungrouped" permissions?

That would basically alter the way the access result in the entity (in this case, node) is handled, while still giving you full control over how it should be handled?

It would be an absolute ball ache to achieve per-domain editor access if it meant you first had to grant general node edit access for the entire website.

Could you expand on this? Why would it be a ball ache and for whom? A lot of systems have access controls like this so I'm confused why it would be such a problem?

I do admit that it shifts the responsability from the developer to the site builder, but on the flip side it gives them a lot more control?

Effectively it's making each module responsible for it's own context and the access controls that goes along with it rather than spreading that context across multiple modules. The only context that gets shared is the permissions themselves based on the access result.

In your case you would need to provide an additional permission to prevent access to ungrouped content. In other words, you're aware of the gorup context, and therefore can (and should) perform access restrictions based on that context alone, without reaching into the context of the underlying entity.

I added some clarity to the title/description. You are correct that it does something it just doesn't do what I think most people expect it to do.

I'm going to take a stab and making an interface for entity query access that is at least somewhat consistent with our existing entity access patterns (for better or worse). I think there is perhaps a way to solve this problem for contrib/custom entities in 11.x and start being more strict in core in 12.x.

I agree though, there seems to be some fundamental disagreements on how access should or shouldn't work.

This problem actually creates some really weird behavior like in DefaultMenuLinkTreeManipulators::checkNodeAccess where the node access query gets modified outside of the context of node

NodeHooks1::queryNodeAccessAlter née node_query_node_access_alter exists and is tested by NodeQueryAlterTest. Also, among others #3452449: Grant query level access to own/any unpublished nodes has the opposite claim. So I think more details are needed here on how to reproduce this.

The implementation you mention performs a no-op if no modules implement hook_node_grants

if (!\Drupal::moduleHandler()->hasImplementations('node_grants')) {
  return;
}

https://git.drupalcode.org/project/drupal/-/blob/401a157fc120a0883964670...

As far as I can tell, there are no modules in core that implements that hook other than in a test for node and a test for path. Therefore, node_grants only works if you have a contrib module that utilizes it, otherwise it doesn't do anything.

Regardless, this issue isn't specific to node, the same problem happens with media where all media is returned/exposed regardless of whether you have the view media permission or not, despite that being a hypothetical query alter.

My assumption in ✨ Grant query level access to own unpublished nodes Active , based on the conversation in ✨ Add an entity query access API and deprecate hook_query_ENTITY_TYPE_access_alter() Needs work is that the reporter has some sort of module that is "providing context" (read: removing the permission context) to the query. As I have argued multiple times in ✨ Add an entity query access API and deprecate hook_query_ENTITY_TYPE_access_alter() Needs work , modules should only be restricting access, not granting additional access. Or it's possible that a contrib module isn't being granular enough when it is restricting access (i.e. the access is being restricted further than it ought to be and there is no way to undo that).

Yes, it's a problem, there are plenty of open issues about it, it's essentially about core not implementing its own access systems "for performance reasons".

That is wild to me. There are many cases (i.e. view media) where a query could simply no-op because the user doesn't have permission to view the entity in the first place. Likewise we don't have a system for adding cacheability metadata to acccess alterations on queries so node does some weird things.

When Node ids are collected via an entity query or database query with node_access tag in place, unpublished contents by the (current) user are not returned.

Could you help me replicate this? I'm finding precisely the opposite in 🐛 QueryInterface::accessCheck does nothing Active

To be clear, I'm not arguing that permissions don't depend on some sort of context, I'm saying that part of that context is the user's existing permissions.

In other words, a permission does not infer some sort of right. The context window therefore, ought to get narrower rather than broader.

Let's take your example:

I think this is what Kristiaan is trying to frame: there is a difference between filtering which is a matter of user intent (I want to find green apples) and access control which is a matter of context specific rules (anyone can find green apples on the tree outside, but you can only see the apples in a secret apple club if you're a member of that club).

I'm saying that must have permission to view apples (regardless of the variety) in order to view secret green apples. The access system we have right now says that it doesn't actually matter if I have permission to view apples or not, a member of the secret green apples club has a right to view secret green apples. If that's the case, we aren't talking about permissions, we are talking about rights.

For a more concreate example, if you don't have access to view any unpublished node how could you possibly view an unpublished node (that isn't yours) in a group? A simple way to solve the contextual problem therefore is by removing the notion of a right by allowing modules to remove access rather than grant access. In this example, Group could add a view any ungrouped unpublished nodes in which a user could (or could not) do just that, but it still resides within the view any unpublished node context rather than overriding (or ignoring) that context.

I've come to the conclusion that the problem that this issue is aiming to solve is way too broad and is leading to a level of complexity that isn't easily achievable.

For that reason, I've opened 🐛 QueryInterface::accessCheck does nothing Active which more clearly defines the problem that I ran into and I think perhaps has a simpler solution.

would you like to review the API i pioneered some time ago in Entity Unified Access, how it resonates with your vision? (Feel free to PM me on this.)

This feels very comprehensive, but way outside of the scope of the problem I'm looking to solve.

This statement from #256 is going to keep me up at night:

which comes from a published check and a module that allows you to bypass any entity access if you have a particular permission.

Effectively we have an access control system that, allows a user to enable a module, with the express intention of performing an access bypass. I think that is fundamentally the wrong way we should be thinking about access controls. I don't think a user should ever be able to loosen access simply by enabling a module. If we need to have more granular permissions in core to prevent that, then so be it, but that seems incredibly dangerous to me.

davidwbarratt → created an issue.

Just to check my understanding of the problem, the problem is that an Entity's permissions could be more restrictive, and modules should be able to loosen those restrictions. Does that sound correct?

Here are two proposals to solve that problem without haivng to know anything about the intent of the query.

Proposal A

Node adds a view any unpblished content permission and Group adds a view ungrouped unpublished content permission.

In this way, the permissions always flow downwards and Group doesn't have to undo anything. It puts a small amount of burdon on site builders because permissions from the entity's module would always take precidence over any other module. This makes sense to me from a security standpoint becausae it seems really odd (and somewhat dangourous?) that you could install a module and all of the sudden the permissions are less restrictive rather than more restrictive.

Proposal B

If we really want to allow modules to loosen permissions (which I don't think is a good idea, but fine), then what if we didn't allow folks to inspect/modify the query at all? What if we created an AccessResult that added a ConditionInterface? It seems like there isn't actually a need to inspect or mutate the query, just a need to remove access results? Does that sound right?

For instance, it could look something like this:

public function queryAccess(?AccountInterface $account = NULL): AccessQueryResultInterface

and the hook:

function hook_entity_query_access(string $entity_type_id, ?AccountInterface $account = NULL): AccessQueryResultInterface

and the AccessQueryResultInterface would be something like this:

interface AccessQueryResultInterface extends AccessResultInterface {
  public function condition(): Condition;
}

and you'd create one like this:

   AccessQueryResult::allowedWithCondotion($condition);

Then we could just merge all the access results together, or we could provide a hook to alter them (or the list of them).

Would either of those solve the problem?

Good luck digging through all of the joins and conditions finding the thing you need to undo and adjust.

I'm trying to understand why you would need to undo a condition from another module? Wouldn't that imply that you're trying to override an access restriction put in place by another module? If you're trying to add a restriction, simply adding the condition seems totally fine from what I can tell.

we need to make some, we might end up joining the same table 5 times because everyone and their dog wanted to get some info from it.

We already dedupe joins in Tables::addField. Indeed the code has this comment:

    // This variable ensures grouping works correctly. For example, given the
    // following conditions:
    // ->condition('tags', 2, '>')
    // ->condition('tags', 20, '<')
    // ->condition('node_reference.nid.entity.tags', 2)
    // The first two should use the same table but the last one needs to be a
    // new table. So for the first two, the table array index will be 'tags'
    // while the third will be 'node_reference.nid.tags'.

It doesn't seem like it's a problem that two modules could add conditions on the same field? It seems like that would only compile to a single JOIN?

Which is why I'd rather have a system where we can first allow modules to specify simple conditions, which get compiled in the end, and then figure out a way for more complex alters to step in before it's too late.

I'm really trying to understand how this isn't exactly what I'm proposing.

In #245 I proposed a hook_entity_query_access which could look something like this:

function hook_entity_query_access(\Drupal\Core\Entity\Query\QueryInterface\QueryInterface $query, ?AccountInterface $account = NULL): AccessResultInterface

I'm trying to understand how that wouldn't suit your needs? The only problem with this that I can see is that it doesn't cover Views which feels like a completely separate beast, but perhaps a similar approach could be taken there.

If you put it on an entity storage handler, you imply that it only serves to define access over that entity type's queries. This is not always the case and thus it completely rules out anyone trying to provide entity query access for multiple entity types. You should not put user query logic into a NodeAccessControlHandler, for instance.

And what about any module trying to leverage entity query access across multiple entity types?

It allows you to hook into all EntityQueries (for both content and config) that have QueryInterface::accessCheck set to TRUE.

As we know, entity handlers are a nightmare to extend because only one module can swap out the original (something I also fixed in Group). This is exactly why I abandoned the approach in Entity API in favor of a custom one in Group.

Why would you need to do that? The AccessResults should get merged from the entity access handler as well as any hook implementations. Likewise the query gets modified by the hook implementations regardless of final AccessResult.

We need a system that can take instructions from various places, one of them could be an entity type handler for the default behavior, and compile those into an efficient query.

That is what the hook does? I think there is a separate issue that EntityQuery should better support subqueries, but that's already a problem with node grants.

This again assumes you are dealing with the entity type table.

I guess that is a broader question if we should apply access controls to any SELECT query or only to EntityQueries and Views?

Either way heavy -1 on relying solely on entity handlers to deal with this problem space. They are inflexible and almost non-extensible. You'd make access modules dead in the water from the get go.

I'm having trouble understanding how/why the proposed hook(s) wouldn't work for you?

So rather than try to pigeon hole all access logic into conditions, we need a system where you can choose to use simple conditions but at the same time have the freedom to alter the query more thoroughly if necessary.

Can you help me understand how the interface I proposed in #245 does not allow you to modify the query more thoroughly? I would imagine you would also need that in order to handle core's view own unpublished content?

We could perhaps come up with a few custom value objects such as OwnerCheck, but again it needs instructions on where to find that owner. is it on the same table and, if so, under what column? Or is it in a different table and how do we join that one?

If you take a look at EntityOwnerTrait it exclusively relies on the owner key on the entity. You should be able to modify the Entity Query with that key, which is how I would handle view own unpublished content. The table that it is on doesn't matter because EntityQuery deals with entity fields rather than database columns.

I think this task needs to be broken down, first into a "low-level" API that can handle the existing permissions in core that we are currently completely ignoring, and then maybe a higher level API that can replace node grants.

I don't understand how your handler proposal helps it.

More specifically, AccessResult implements RefinableCacheableDependencyInterface so where we handle the query execution we could collect the cacheable metadata from the access result.

In principle having this on the handler seems good. But I see it as a small addition to the current system, not a simplification

Perhaps I don't understand the problem being described in the issue summary? From what I understand it is mostly to resolve this note:

Note that this hook is not called for listings (e.g., from entity queries and Views).

and provide a hook that is called for listings, is it not?

I'm interested in the cacheability question you raise, but I don't understand how your handler proposal helps it. Can you say more?

For instance, if you have a query like this:

$query = \Drupal::entityQuery('node')
  ->accessCheck(TRUE);

You would expect to at least have the user.permissions cache context. But what if the user does not have bypass node access OR administer nodes but does have view own unpublished content? In that case, you should get the user cache context, even if no results are returned.

As far as I can tell, we don't properly handle this case with the recommended approach of hook_query_TAG_alter. Then again, in core, we completely ignore the the QueryInterface::accessCheck and return everything anyways! Which is not only a huge security flaw, that at least to this engineer did not expect, but is also a fairly bad performance problem. We are effectively querying for entities that the user has no ability to access in the first place.

We could persue a more comprehensive solution, which I've documented in #777578-245: Add an entity query access API and deprecate hook_query_ENTITY_TYPE_access_alter() →

Here's my proposal to add to EntityAccessControlHandlerInterface

use Drupal\Core\Entity\Query\QueryInterface;

/**
 * Checks access to query an entity.
 */
public function queryAccess(QueryInterface $query, ?AccountInterface $account = NULL): AccessResultInterface

I think this should be simple enough to handle simple permissions like view media and access user profiles, but flexible enough to handle view own unpublished content which would require modifying the $query. This also allows appropriate cacheability metadata to be added and would only be executed if QueryInterface::accessCheck is TRUE

It does not expand node grants outside of nodes, but it should allow us to simplify the query entity access system? We can add hooks similar to hook_entity_access, maybe hook_entity_query_access ? Then modules can take further action.

What do you all think?

I was doing some research ✨ Add a getter for Entity\Query\QueryBase::$accessCheck Active on where we use hook_query_TAG_alter in core for ✨ Add a getter for Entity\Query\QueryBase::$accessCheck Active and I was surprised that it's only really used in NodeHooks1::queryNodeAccessAlter, which is probably why this issue is so long because that method is a beast and I think we need to find a better solution for all of the other entities first and then it might be more straightforward how to solve this problem.

For instance, if you run an entity query with accessCheck for a list of users, I would expect that query to return no results unless the requesting user has access user profiles (or one of the administrative privlidges). But right now, from what I can tell, it returns all of the results!

The same can be said for media and the view media permission. There are probably many more examples.

I'm starting to think we should indeed add a method to EntityAccessControlHandlerInterface that forces/allows for altering the EntityQuery and the logic is similar to EntityAccessControlHandlerInterface::access. Might also be nice to add a method to AlterableInterface that forces the query to immediatly return zero results. Lastly, we need some way to handle cacheability metadata that doens't weirdly rely on the rendererer.

All of that feels like a lot and has nothing to do with node grants, but, imho, is needed for a more comprehensive solution. Like if you don't have access to view media do more granualar permissions beyond that even matter? We aren't even handling the permissions we have right now let alone more granualar ones.

It seems outside of the scope of this issue, but we could add a hook_entity_query_ENTITY_TYPE_alter for an entity type like user? For instance, if a user doesn't have the access user profiles permission and the query hasAccessCheck than the query should immediately return zero results right?

I'm kind of shocked that hook_query_TAG_alter, at least for the purposes of access, is only used by node in core with NodeHooks1::queryNodeAccessAlter which is kind of a beast.

Ironically, the example for hook_query_TAG_alter is a hypothetical for the media module that would be greatly simplified by this change, which wouldn't use that hook in the first place.

I'm not sure where a test should therefore belong? This change is effectively only useful for contrib modules to utilize until someone wants to take on the work of refactoring node access, in which this change I imagine would be helpful.

Any guidance you can provide would be helpful!

davidwbarratt → created an issue.

Honestly if we want to be stricter about it, we could add a method to EntityAccessControlHandlerInterface that forces entities to alter the entity query (or pass). Which makes it clearer when implementing a custom entity that you need to handle access controlled queries.

You can kind of do this now with hook_entity_query_ENTITY_TYPE_alter. The problem is that you cannot access the accessCheck property without using reflection → .

A very simple implementation could be to either provide access to that property, or by adding an `alterTag` for `access`.

I think that mostly solves the problem described in the issue? I imagine it could be a more comprehensive API, but it's a good first step?

Just ran into this trying to access the accessCheck property.

davidwbarratt → created an issue.

I haven't seen this again since I wrote it so I think a cache clear did do the trick but if it comes back up I'll let ya know!

I believe the module has to be installed with Composer → .

davidwbarratt → created an issue.

I think moving all of
https://git.drupalcode.org/project/drupal/-/blob/38d18a4f1f172a12374228c...
into
https://git.drupalcode.org/project/drupal/-/blob/38d18a4f1f172a12374228c...

would basically fix this?

I figured it out, the prebuilt Docker images from https://hub.docker.com/_/drupal have Drupal baked in. If you're doing something like this:
https://github.com/davidbarratt/davidwbarratt/blob/8558d6d1e0be3d797bc3a...
then you may end up with an older version of Drupal that is baked in, which would normally be fine since you're probably copying everything over anyways:
https://github.com/davidbarratt/davidwbarratt/blob/8558d6d1e0be3d797bc3a...
but Docker's COPY does not remove files that are not in the source.

To get around this I could either specify the same version in the image (i.e. 10.4) or I can remove the baked image:
https://github.com/davidbarratt/davidwbarratt/commit/8558d6d1e0be3d797bc...

@cilefen hmm, this is odd, it does not exist when I blow out the folder and do a composer install, but it does exist in the docker container which does the same thing... this might be a caching problem I suppose.

I'm having this problem, and I upgraded using Composer....

I can confirm that the attached patch fixes. the issue. I am unable to re-open the issue, but this should be RTBC!

I installed this module in a fresh Drupal 11 site and I ran into the same issue.

davidwbarratt → created an issue. See original summary → .

davidwbarratt → changed the visibility of the branch 3094343-queue-confusion-on-replicated-databases to active.

davidwbarratt → changed the visibility of the branch 3094343-queue-confusion-on-replicated-databases to hidden.

The fix for this is in 🐛 Queue confusion on replicated databases (auto_increment_offset) RTBC

I have fixed it!

[DEBUG] [purge] [2024-08-23T22:32:11] purger_cloudflare_worker_0b7da3a883: Received Purge Request for node_list, node_list:article, node:49 | uid: 1 | request-uri: http://localhost:8888/node/49/edit?destination=%2Fadmin%2Fcontent | refer: http://localhost:8888/node/49/edit?destination=/admin/content | ip:  127.0.0.1 | link:
[DEBUG] [purge] [2024-08-23T22:32:11] purger_cloudflare_worker_0b7da3a883: Executing purge request for node_list, node_list:article, node:49 | uid: 1 | request-uri: http://localhost:8888/node/49/edit?destination=%2Fadmin%2Fcontent | refer: http://localhost:8888/node/49/edit?destination=/admin/content | ip:  127.0.0.1 | link:

The code basically relies on the indexing of the array to always match. Instead of making that assumption, we'll just use whatever is passed in and ensure the indexing matches up.

I did try switching from mod_php to php-fpm. I thought it might since the former holds on to the response and perhaps I was hitting a timeout or something, but it didn't make a difference.

I added repo steps to 🐛 Late Runtime Processor purges incorrect tags Active it happens every time I use Late Runtime Processor, if I use the drush or cron processor the issue goes away.

I didn't test it on any other database, but I am using SQLite, I assumed since the problem happens consistantly that that could be a clue, but it might not be.

I figured this out because I was writting a Cloudflare Worker that receives the purge requests and I noticed it was missing a tag. Upon further investigation, the tag isn't being sent.

To clarify, the patch clearly fixes a problem, but I'm not sure if that's the problem I'm having 🐛 Late Runtime Processor purges incorrect tags Active or if that's the same problem described in this issue. From the comment thread, it looks like the patch does fix the issue for some folks so it's possible there is more than one problem and the patch isn't a comprehensive solution.

I tested the patch and it does not fix the issue I had in 🐛 Late Runtime Processor purges incorrect tags Active so I'm re-opening it.

I accidentally created a duplicate of this issue at 🐛 Late Runtime Processor purges incorrect tags Active . This problem seems exceptionally bad with SQLite.

This appears to be a duplicate of 🐛 Queue confusion on replicated databases (auto_increment_offset) RTBC .

I edited a node and I get this debug message:

[DEBUG] [purge] [2024-08-21T23:17:42] queue: claimed 3 items: node_list, node_list:article, node_list | uid: 1 | request-uri: http://localhost:8888/node/49/edit?destination=%2Fadmin%2Fcontent | refer: http://localhost:8888/node/49/edit?destination=/admin/content | ip:  127.0.0.1 | link:

However, if I disable the Late Runtime Processor, I get the correct tags if I inspect the queue

What might cause the Late Runtime Processor to have incorrect tags?

davidwbarratt → created an issue.

This is a breaking change, so I opened up a 2.x branch.

thejimbirch → credited davidwbarratt → .

davidwbarratt → created an issue.

davidwbarratt → created an issue.

sourojeetpaul → credited davidwbarratt → .

Or we figure out that we don't actually need to declare these things as completely uncachable anymore.

For instance, what if we decide to rely on SameSite=Lax rather than using our custom CSRF protection? This is what other projects like Next.js do and maybe that would be fine for us as well?

i think you won't be able to solve such problems without client side javascript code that updates part of the page outside the context of the whole page caching .

That seems fine?

In my mind, undercaching is always prefered to overcaching. In an ideal world, it would be perfectly cached, but if the form (or whatever) declares that the whole page is uncachable because of it's inclusion, then.... it is what it is. The only thing you can do is refactor that element to be cacheable (i.e. generate the dynamic bits with JS and/or WebAssembly)

I'm still subscribed to this issue 8 years later and I re-read the duplicate I created → and I'm still thinking about this:

There are existing issues for this, it's mostly by design and I'm not sure if it can be changed. It would result in many pages no longer being cacheable that currently are, for example as forms declare themself as uncacheable.

#2732129-4: FinishResponseSubscriber::setResponseCacheable() does not respect Cacheable Metadata for Cache-Control header →

And now I'm wondering.... so what? Like if a bunch of things become uncachable in Drupal 11 and we go and fix those things (individually) in 11.1+ that seems... fine? Am I missing something here?

Yeah sorry about that. Something on Azure is borken so I'm migrating back to my Pi which is cheaper anyways.

Rebased

I created a new merge request that is more incremental, and I think should solve my problem without having to add the Transport to the DI container.
https://git.drupalcode.org/project/drupal/-/merge_requests/5847

Please take a look and let me know what you think!

Would you accept a middle ground? Perhaps a TransportFactoryManager that just collects tagged transports, but doesn't create the Symfony Transport object?

I don't understand these test failures, if anyone could help explain them I would be much appreciated!

davidwbarratt → created an issue.

davidwbarratt → created an issue.

davidwbarratt → created an issue.

Adding attribution

davidwbarratt → created an issue.