Set the Drupal\views\ViewsData::$fullyLoaded property to TRUE only when the data is really loaded.

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.

Thanks for opening.

MRs need to point to 11.x as the development branch. Kinda seems like a feature request but will leave as a bug, regardless will need test coverage and submaintainer sign off

I have added test coverage for this. Please review.
The Needs subsystem maintainer review is still open.
Removing the Needs tests Tag.

Sorry, we did not see this issue when creating the other
https://www.drupal.org/project/drupal/issues/3569624

We will revert our commit 2d9613522980faf9c168f0e39585ae10be6a8e4f, the changes made on this issue are a better approximation

We added some improvements suggested on MR https://git.drupalcode.org/project/drupal/-/merge_requests/14497#note_66... from @godotislate

While not finalized, the general trend for core tests is not to use custom assertion messages: https://www.drupal.org/project/drupal/issues/3131946.

And added the race conditions tests from Fibers.

Failed https://git.drupalcode.org/issue/drupal-3565020/-/jobs/8228022
152.839s Drupal\Tests\ckeditor5\FunctionalJavascript\CKEditor5Test 7 passed, 1 failed, exit code 1
But should not be related to the issue

Tests stabilized

We did a rebase, and it broke the JavaScript tests, but we are not sure if it is related to the changes; it is a CKEditor problem, so probably not.
https://git.drupalcode.org/issue/drupal-3565020/-/jobs/8235625

126.895s Drupal\Tests\ckeditor5\FunctionalJavascript\CKEditor5MarkupTest           3 passed, 1 failed, exit code 1

Re-ran the failing tests, and they passed. I also ran the test-only job, and it failed as expected, with the views data arrays being empty: https://git.drupalcode.org/issue/drupal-3565020/-/jobs/8241893

I also followed the reproduction steps in the IS (though against 11.x), and I saw a similar, but not identical error:
Drupal\Core\Database\DatabaseExceptionWrapper: Exception in Blocks[blocks]: SQLSTATE[HY000]: General error: 1 near "FROM": syntax error: SELECT FROM "block_content_field_data" "block_content_field_data" LIMIT 5 OFFSET 0; Array ( ) in main() (line 20 of index.php).

Switching to the MR branch, this error does not appear.

The MR is still targeted against 11.x though (somehow missed by the bulk update), so either the MR creator @dench0 or a maintainer will need to retarget against main, and then the MR will need to be rebased. Alternatively another MR can be opened against main.

Needs subsystem maintainer review is still outstanding. I'm not sure it needs one, but I'll leave the tag.

Back to NW for MR retarget.

Copied MR 14185branch to a new branch and opened new MR 14609 against main. Test only job still fails as expected: https://git.drupalcode.org/issue/drupal-3565020/-/jobs/8332482

Commits are otherwise the same, so moving to RTBC.

I can confirm this issue affected our site after upgrading core from 11.2.10 to 11.3.3.

Site context:
- Multilingual Drupal site (EN + translated routes).
- Multiple Views block displays on node pages.
- Logs confirmed runtime failures during page render.

What we observed:
1. After the core upgrade, translated (non-EN) node pages intermittently failed.
2. Main error pattern:
- `SQLSTATE[42000] ... near 'FROM "node_field_data" ...': SELECT FROM "node_field_data"...`
3. On reloads / subsequent requests, we also got:
- `InvalidArgumentException: A valid cache entry key is required. Use getAll() to get all table data.`
4. Once we worked around one failing view display, another view would fail in the same request lifecycle (symptom of partially initialized Views data).

Why this looked like this issue:
- Failures were inconsistent between first request and reload.
- Behavior was strongly tied to render flow and language-specific page paths.
- Symptoms match Views data being marked fully loaded while data is still incomplete.

Validation:
- Applying the patch from this issue resolved the production-like failures in our environment.
- After patching, pages that previously failed (especially translated routes) rendered consistently.

So from our side, this patch fixes a real regression introduced by the 11.3.x upgrade path.

Can confirm this. We had a very similar issue in the LocalTaskManager. There we went one step further and explicitly check for a fiber and suspend once on detecting a re-entry. See \Drupal\Core\Menu\LocalTaskManager::getLocalTasks().

This is possibly a considerable performance performance improvement, because right now, with the current implementation, what happens is that the full views data is going to be built twice. With the check as we have it there, we give the fiber that entered this first a chance to complete the work before doing it twice.

Given this is a functional bug I'm OK merging a fix without the optimization in #28, but we should either do it here or open a follow-up for it.

Added optimization per #28/30. Back to NR.

The optimization looks good to me, exactly like the local task manager. Fewer changes required to test than I expected.

I also created #3573878: Add a cache builder/manager that helps protect against fibers, cache stampedes and simplify DX to standardize on stuff like this.

Fewer changes required to test than I expected.

The test changes are just to remove $this->any() usages because it's deprecated. I didn't set up testing to show the loading only runs once. Do we need that?

I wasn't sure, didn't really mean to say anything with that comment, just that I expected that more adjustments would be necessary.

But.. why not? I think we could just update the assertions that invokeAll() for views data is called exactly once instead of at least once in those tests and then that should be covered?

I had looked at testing that, but it made mocking cacheGet() and cacheSet() complicated and decided not to. I can revisit, especially since it seems I missed some $this->any() use that needs to be cleaned up.

OK, made the test changes. Back to NR for that.

Improvements look good to me now, back to RTBC.

Committed/pushed to main, 11.x, and 11.3.x, thanks!