ClassInfo::hasTable could be cached on dev/build

lekoala commented 3 months ago

Description

For context, I realized this on a website using Fluent that is using hasTable in its middleware. The consequence of that, since the cache is a static cache per request, a SHOW FULL TABLES WHERE Table_Type != 'VIEW' is made on each request. It's not super expensive, but still, it feels this could be avoided entirely.

Currently, there are two ways to check if a table exists:

ClassInfo::hasTable
DBSchemaManager::hasTable

It feels like the one from ClassInfo could be cached entirely on dev build. If you really need to make a live query against the database, the DBSchemaManager is still there.

This would benefit to classes like ModelAsController or RootURLController that have a

if (!DB::is_active() || !ClassInfo::hasTable('SiteTree')) {

statement, triggering the query each time.

Additional context or points of discussion

No response

Validations

[X] You intend to implement the feature yourself
[X] You have read the contributing guide
[X] You strongly believe this feature should be in core, rather than being its own community module
[X] You have checked for existing issues or pull requests related to this feature (and didn't find any)

PRs

https://github.com/silverstripe/silverstripe-framework/pull/11183

GuySartorelli commented 3 months ago

This is definitely a good idea. It is already cached in memory per request so I think it's safe to cache more persistently in a minor release without breaking expectations. I'd support a pull request that implements this.

lekoala commented 3 months ago

@GuySartorelli i've made a first attempt. at first glance, it seems to work really well but I'd be happy to have your feedback on this

GuySartorelli commented 3 months ago

PR merged. This will be included in the October minor release.

silverstripe / silverstripe-framework