Skip to content

MNT Fix link test to not pre-cache link owner before test starts.#392

Merged
emteknetnz merged 1 commit intosilverstripe:5from
creative-commoners:pulls/5/fix-link-tests
Jul 2, 2025
Merged

MNT Fix link test to not pre-cache link owner before test starts.#392
emteknetnz merged 1 commit intosilverstripe:5from
creative-commoners:pulls/5/fix-link-tests

Conversation

@GuySartorelli
Copy link
Copy Markdown
Member

@GuySartorelli GuySartorelli commented Jun 27, 2025

With the new caching, this will fail tests that add versioned to the class. That's because of a combination of factors:

  • extension instances are stored on instances instead of accessed statically (see Extension instances are singletons, but are stored on the instance silverstripe-framework#11772) which means a record cached before versioned is applied doesn't have a versioned extension instance
  • in DataObject::get_one(), the getUniqueKeyComponents() method is used to determine the cache key, which just happens to be called on a singleton, and we just happen to have not generated a singleton of the class until now, which means it has the versioned extension freshly added to it and that is counted in the cache key where before that part of the cache key was empty (and therefore the key doesn't match the original unversioned record's cache key with get_one)
  • In DataList when caching is enabled, getUniqueKeyComponents() isn't taken into account - instead the resulting SQL is used. This includes versioning (see new tests in MNT Test new cache doesn't bleed across stages silverstripe-versioned#517) but the versioned state is draft which uses the base table, so the SQL matches the original query and returns the originally cached record (which is correct behaviour).

This would be extremely unlikely to occur in non-test code, and I think our response to a bug report would be "reset your cache after applying the extension, or just don't fiddle around with extensions at runtime except inside _config.php".

Important

Relies on silverstripe/silverstripe-framework#11768
DO NOT MERGE until that PR has been merged in

Issue

With the new caching, this will fail tests that add versioned to the
class. That's because of a combination of factors:
- extension instances are stored on instances instead of accessed
  statically
- in `DataObject::get_one()`, the `getUniqueKeyComponents()` method is
  used to determine the cache key, which just _happens_ to be called on
a singleton, and we just _happen_ to have not generated a singleton of
the class until now, which means it has the versioned extension freshly
added to it.

This would be extremely unlikely to occur in real code, and I think our
response to a bug report would be "reset your cache after applying the
extension, or just don't fiddle around with extensions at runtime except
inside `_config.php`".
@emteknetnz emteknetnz merged commit b2215d9 into silverstripe:5 Jul 2, 2025
12 of 15 checks passed
@emteknetnz emteknetnz deleted the pulls/5/fix-link-tests branch July 2, 2025 03:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants