Untangle Cache\DatabaseBackend from procedural database.inc functions to make it available in early bootstrap

Looks simple enough.

Why getting the connection each time instead of having it as a class property?

So the issue to me looks like:

- we want to use dependency injection if possible.
- the cache instance is a 'global' class - i.e. we only want one per request per bin except for very rare exceptions.
- the database connection is also global and can be changed (outside of the DIC)
- we don't want to instantiate a new cache class each call just in case the database connection changed.

To me it feels like #0 is RTBC, but that we might want to eventually jiggle around so that the connection can be injected via the DIC later on (or inject the DIC as a service locator so it can always return the correct connection).

Assigning to Crell since there's going to be several places we need to do similar with the database connection.

Sorry, these are all wrong. :-) The answer is to put the DB connection into the DIC, and then make the cache object pull it from there when it too is instantiated from the DIC. The installer then needs its own DIC. The Routing patch has the first part done, because I'm doing exactly that. I'll split off the "DB in the DIC" part shortly in a separate patch.

(Note: And then we can move all of our DB-cache tests to unit tests, not integration tests, for better performance and more reliable tests.)

#1759152: Add a database service to the DIC - The issue, courtesy katbailey.

beejeebus: When using a DIC in this fashion, each implementation would have a different constructor (the constructor is not part of the interface) and the DIC would be wired up accordingly. Eg, cache.page would be wired to the database backend, with the DB connection as a dependency. If you wire cache.page to memcache, you'd instead specify that class with the memcache connection as a dependency. Etc.

I wasn't aware of this issue and went ahead and created #1762388: Use Dependency Injection for FileStorage, which deals with removing the constructor from the StorageInterface and injecting the DB connection into the DatabaseStorage. So I think it's only a partial solution to what's being discussed here..?

sun: #0 just replaces db_* with Database::getConnection()->*(). That does not reduce the dependencies at all. There's no useful benefit from it. Even if you kept one Database:: call in the constructor and saved that to a property to use later (i.e., #4), that would be a marginal improvement that makes switching to a properly injected connection later a little easier, but #0 doesn't do anything useful at present.

The wacky "everything's broken in the installer" problem is something we should fix, not keep hacking around.

I agree with #19, I don't see any benefit in commiting this as-is. It sounds like there is a deeper chicken and egg dependency resolution problem, and it is architectural and lies in the config system, not in the details of the cache backend.

I agree, sun's fix will get us moving on what is a critical issue for CMI, and we can move larger issues into the followup beejeebus created.

Even if you kept one Database:: call in the constructor and saved that to a property to use later (i.e., #4), that would be a marginal improvement that makes switching to a properly injected connection later a little easier, but #0 doesn't do anything useful at present.

See #8, we can't rely on the database connection not changing under our feet with a db_set_active() call before a cache()->get(), which is why I pinged Crell for this issue in the first place :)

Since #1764474: Make Cache interface and backends use the DIC is major, I'm OK committing this as an interim measure to unblock CMI, so I've gone ahead and done that.