Have a consistent API for confirming if a DataObject is ready for database operations

[GuySartorelli]

There are time when code that manipulates or even fetches data from some DataObject could be run during or before dev/build. For those situations, we need to confirm that the relevant table(s) exist in the database, and that all of the necessary database columns exist in those tables.

Currently in those situations we have a variety of different solutions. Max identified some of these in silverstripe/silverstripe-graphql#445 (comment) which I have included in a list further down in this description. We should have a unified way of performing these checks.

Reasons to making these checks unified:

• Easier maintenance
  • if we identify a problem with the way the checks are being made we just have to fix it in one place
  • if we need to add additional checks, we can just add those checks in the unified method
  • makes it easier to make larger architectural changes (e.g. --no-database schema build fails with various module configuration silverstripe-graphql#445)
• Provides a clear API for developers who need to do the same checks in their own projects.

Ways these checks are currently being performed

These have been found using github search, which I have noticed misses stuff sometimes, so this may not be a complete list. This does not include anything included inside tests or test classes.

• Security::database_is_ready()
  • Caches the ready state if it is indeed ready (so future calls in the same request for classes we know are ready don't have to query the db again)
  • Skips TestOnly classes
  • Checks the entire hierarchy of a class (so confirms that all superclasses have their tables/columns, and checks the same for subclasses, which may ultimately be injected to replace the class we'd normally expect)
  • Checks if the classes which should have tables do have tables
  • Checks if the those tables have all columns they should have (including those provided by Extension classes)
  • Used in the following relevant situations:
    • Avoid a permissions check if db isn't ready for one
    • Avoid checking for the current Member if the db isn't ready for that
    • Avoid another permissions check if db isn't ready
    • Not really sure what it's doing here... I suspect someone say "database_is_ready" and thought "That's what I need here"
• DataObjectSchema::classHasTable()
  • Is cached per request
  • Only checks if the given class has fields in its own table - this means subclasses which don't have their own db fields will return false for this even if the database is ready for fetching or manipulating data for that class.
  • Doesn't check if the table has all the columns it should have
  • Used in the following relevant place:
    • Check if the SiteConfig table exists before querying it - the method which calls this then separately confirms if all the relevant columns are present, though extensions may not have been applied by this point.
• ClassInfo::hasTable()
  • Is cached per request
  • Used to determine if a specific table exists (as opposed to whether a table for some class exists)
  • Doesn't confirm if that table currently has all of the requisite columns
  • Ultimately just a cached alias for DBSchemaManager::hasTable()
  • Used in the following situations that may be relevant:
    • Checking if dev/build should be run before handling a root request
    • The same check as above, but this time while handling that request (seems redundant)
    • Same as above, this time before and during handling a request on ModelForController.
    • To check if the database is in a ready state for storing hybrid session data
    • Part of the Security::database_is_ready() check
• DBSchemaManager::hasTable()
  • Not cached
  • Used to determine if a specific table exists (as opposed to whether a table for some class exists)
  • Doesn't confirm if that table currently has all of the requisite columns
  • Used in the following situations that may be relevant:
    • Getting the subsite ID for a given domain
    • Populating SiteConfig defaults in Translatable
    • An optional check to confirm the database is in a good state
• Catching a DatabaseException and selectively reading the message to see if a Database exists.
  • Not cached
  • Pretty brute force way around the issue rather than actually checking if the db is ready for the specific queries that will be made

Acceptance Criteria

• A preferred way of performing the checks is identified, with clearly documented reasoning.
• This works correctly regardless of any changes to code that will affect the database
  • Prior to the first dev/build
  • adding a new Extension to a DataObject after the first dev/build
  • adding new fields or relations to a DataObject after the first dev/build
  • changing the $table_name config for a DataObject after the first dev/build
• All identified areas that need this check use the new method
• Any old now-unused methods are deprecated

PRs

• NEW: Add method to check if a class is ready for db queries. #10276

[maxime-rainville]

Didn't have time to get this into CMS5.

[jakxnz]

This is great. Let's try and find the next most immediate minor to include this in!