What the bug is: LMS looks up the list of "course" group types once and then never looks again. When a recipe installs LMS, Drupal core creates those group types only after the install step. So LMS remembers "there are no course types" and crashes as soon as the recipe creates a group. This affects every release since 1.2.0.
Problem/Motivation
If a recipe installs LMS and then creates groups in the same run, the recipe fails partway through with:
Drupal\Core\Entity\EntityStorageException: Query condition 'group_relationship_field_data.group_type IN ()' cannot be empty.
This affects 1.2.0 and later. I found it while applying LMS Demo Kit (a recipe I maintain), but it also happens with a minimal recipe that only installs lms and lms_classes and creates one class group (attached).
Cause. TrainingManager::getCourseBundleIds() stores its result in a property the first time it runs, and never recalculates it. When a recipe installs modules, core deliberately skips creating config entities (RecipeRunner::installModules() sets the config installer to syncing mode) and imports them afterwards. The router rebuild at the end of module install calls RouteSubscriber::alterRoutes(), which calls getCourseBundleIds() while no group types exist yet, so an empty list is stored. The recipe then imports the lms_course group type, but the same TrainingManager instance keeps returning the empty list for the rest of the process.
When the recipe then saves a group, the creator membership is validated, group permissions are calculated, and ClassPermissionCalculator calls loadGroupRelationshipsByCourseBundles(), which queries group_type IN (). That's the exception above.
Smaller side effect. Any router rebuild later in that same process saves the five course routes (lms.course.start, lms.course.reset_test, lms.group.answer_form, lms.group.results, lms.group.self_results) with bundle: []. Core's EntityConverter treats an empty list as "any bundle", so until the next cache rebuild these routes also match non-course groups. In my (ahem, Claude) testing, /course/{class id}/start returned 403 instead of 404. I didn't find anything worse than that.
Steps to reproduce
Automated: the attached patch includes a kernel test, CourseBundleIdsTest. On 1.2.x without the fix it fails with the same IN () exception.
Manually (Drupal 11.4.7, Group 3.3.5, LMS 1.2.x-dev; reproduced on SQLite, and first seen on MySQL/MariaDB):
- Install a fresh site:
drush site:install standard -y - Unzip the attached
lms-recipe-repro.zipintorecipes/. drush recipe /full/path/to/recipes/lms_min_repro- The recipe fails while creating content, with the exception above.
(The zip contains two recipes. group_base imports Group's group_roles field storage first. It's needed only because Group expects that field to exist when a group type is saved, and has nothing to do with this bug.)
Proposed resolution
- Stop caching in
getCourseBundleIds(). Core'sEntityTypeBundleInfoalready caches bundle info, so calculating the list again is just a loop over the group bundles. I measured about 0.4 µs per call uncached vs 0.03 µs cached. I counted 0–3 calls per page load (3 on a cold/coursespage, 0 with warm caches), so the cost is around a microsecond per request. - Return
[]early fromloadGroupRelationshipsByCourseBundles()when there are no course bundles, sinceIN ()isn't valid SQL. A site can have no course bundles at a given moment even without the caching problem, e.g. partway through an install.
Both changes are needed. I tested each on its own:
| Variant | Minimal recipe | Course route bundle afterwards |
New kernel test |
|---|---|---|---|
| Unpatched 1.2.x | fails (IN ()), 3/3 runs |
n/a | fails |
| Empty-list guard only | applies | [] (stale) |
fails |
| Cache removal only | applies | ["lms_course"] |
fails (IN () before any course type exists) |
| Full patch | applies, 3/3 runs | ["lms_course"] |
passes |
An alternative would be to keep the cache and clear it when a group type is created or deleted. I didn't go that way because it adds code to save a fraction of a microsecond per request.
Verified locally: LMS's existing unit and kernel tests pass with the patch; phpcs (the project's phpcs.xml, plus DrupalPractice), phpstan (the project's phpstan.neon, level 6) and cspell report no issues on the changed files. The patch applies cleanly to 1.2.x at 5666cfe. I haven't run the FunctionalJavascript test locally.
Remaining tasks
- Review.
- Maybe a follow-up: other
getCourseBundleIds()callers also pass the result intoINconditions or route options without checking for an empty list (UnpublishedParentConstraintValidator,LmsParentEntityFilter,CoursePermission,LmsEntityViewsDataProvider). I haven't seen them fail, so they're not changed here.
User interface changes
None.
API changes
None. The private $courseBundleIds property is removed; public method signatures are unchanged.
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| lms-course-bundle-ids-stale-cache.patch | 4.16 KB | markfelton | |
| lms-recipe-repro.zip | 1.62 KB | markfelton |
Comments
Comment #2
markfelton commentedDuplicate of https://www.drupal.org/project/lms/issues/3626214