Decorator pattern
Closes #2347867
Merge request reports
Activity
added 20 commits
-
e205a11d...20a080c2 - 17 commits from branch
project:11.x
- 50c73fcb - First stab at decorator pattern
- c4c19999 - Fix CS
- 4f8d1c5c - Update replica/kill-switch logic
Toggle commit list-
e205a11d...20a080c2 - 17 commits from branch
added 1 commit
- 09d5d3fc - Enable database perf on non-transactional connection as well.
added 7 commits
Toggle commit listadded 1 commit
- d954b6aa - Address return type incompatibility in decorator
125 125 * 126 126 * The general format for the $databases array is as follows: 127 127 * @code 128 * $databases['default']['default'] = $info_array; 129 * $databases['default']['replica'][] = $info_array; 130 * $databases['default']['replica'][] = $info_array; 131 * $databases['extra']['default'] = $info_array; 128 * $databases[\Drupal\Core\Database\Database::DEFAULT_KEY][\Drupal\Core\Database\Database::DEFAULT_TARGET] = $info_array; 129 * $databases[\Drupal\Core\Database\Database::DEFAULT_KEY][\Drupal\Core\Database\Database::REPLICA_TARGET][] = $info_array; 130 * $databases[\Drupal\Core\Database\Database::DEFAULT_KEY][\Drupal\Core\Database\Database::REPLICA_TARGET][] = $info_array; 131 * $databases['extra'][\Drupal\Core\Database\Database::DEFAULT_TARGET] = $info_array; The FQCN is not super readable, no, but this is also a sample code snippet that kinda needs to exist on its own. I don't feel strongly about this here, more so in other contexts where you for instance are using
default
as both a key and a target in the same code snippet. I think we could keep this file as-is while still using the constant where it helps improve readability and you can import the shorter alias.I tried to use this pattern in a
contrib
module recently. Regrettably module code is not loaded at the point in time when the settings file is loaded. I think that more people will try to start using this pattern if core is introducing constants as array keys insettings.php
. And consequently more people will fail and will have to debug the same problem as myself. For this reason, I'd prefer if we'd stick to string keys insettings.php
.Yeah, I'm getting enough push-back on this that I think it'll have to get backed out. I generally disagree in the case of core, but it's not the main thrust of the issue/MR and so not worth bikeshedding. Agree that it would be difficult for contrib to do the same. I would love to move most all of this configuration out of settings and into DI parameters but that's still a way off.
changed this line in version 48 of the diff
1 <?php 2 3 declare(strict_types=1); 4 5 namespace Drupal\Core\Database\Exception; 6 7 use Drupal\Core\Database\DatabaseException; 8 9 /** 10 * Exception thrown when a connection does not implement transactions. 11 */ 12 final class TransactionsNotSupportedException extends \RuntimeException implements DatabaseException {} changed this line in version 42 of the diff
2 3 declare(strict_types=1); 4 5 namespace Drupal\Core\Database; 6 7 use Drupal\Core\Database\Event\DatabaseEvent; 8 use Drupal\Core\Database\Exception\TransactionsNotSupportedException; 9 use Drupal\Core\Database\Transaction\TransactionManagerInterface; 10 use Drupal\Core\Pager\PagerManagerInterface; 11 12 /** 13 * A decorator class wrapping a connection, not supporting transactions. 14 * 15 * @internal 16 */ 17 final class NonTransactionalConnection extends Connection { I wish. The problem is that Connection is an abstract class that basically acts as an interface. So there are
Connection
typehints everywhere, and there is no underlying interface. Otherwise yes this would be as simple as a__call()
implementation. Alas, no can do if we choose to use a decorator.I think we could do that in some places around core but it doesn't solve the underlying problem that we can't control extending code expecting only a
Connection
. The better solution would be to typehint on an interface, but again, that's a bigger lift and would be a separate issue. The problem here is, I think, that the original authors didn't anticipate something like this and so an interface wasn't in the design.changed this line in version 30 of the diff
32 33 protected Connection $connection, 33 34 protected TimeInterface $time, 34 35 ) { 36 if (!($connection instanceof NonTransactionalConnection)) { 37 @trigger_error('Calling ' . __METHOD__ . '() without an explicitly non-transactional database connection is deprecated in drupal:10.4.0 and will be required in drupal:12.0.0. See https://www.drupal.org/node/3310017', E_USER_DEPRECATED); changed this line in version 15 of the diff
- Resolved by Brad Jones
added 126 commits
-
88f14090...2e311a8d - 116 commits from branch
project:11.x
- a8b964b5 - First stab at decorator pattern
- 4dfd3543 - Fix CS
- 44aa3fdd - Update replica/kill-switch logic
- fe92daa5 - Test tweaks
- f5d57597 - Enable database perf on non-transactional connection as well.
- fb0924aa - Add docblock
- e69bd6ad - Fix CS
- e8804354 - Address return type incompatibility in decorator
- c51eb297 - Fix cspell
- fba1e669 - Delete duplicate test
Toggle commit list-
88f14090...2e311a8d - 116 commits from branch
added 1 commit
- b5f6354e - Don't use non-transactional service with sqlite
added 1 commit
- 74fc55bd - Fix tests which install non-active database drivers
added 1 commit
- 8107ebb5 - More narrowly tailor backend-overridable tag
added 1 commit
- c38c4b45 - Handle original public value when registering proxy services
added 1 commit
- f85242b9 - Replace overridden services, don't alias them.
added 22 commits
-
1cfca5e1...e700aa81 - 3 commits from branch
project:11.x
- e700aa81...c6500891 - 9 earlier commits
- 0f2e0c7b - Delete duplicate test
- 6cf654f8 - Don't use non-transactional service with sqlite
- 75117123 - Fix tests which install non-active database drivers
- 938f19c1 - Revert "Fix tests which install non-active database drivers"
- 32a8f653 - Fix backend override for lazy service
- aab5b6a1 - More narrowly tailor backend-overridable tag
- da11f437 - Handle original public value when registering proxy services
- e8bf1b27 - Replace overridden services, don't alias them.
- e8580b69 - Fix CS
- 0958ecb0 - Include now-legacy check for existing aliases.
Toggle commit list-
1cfca5e1...e700aa81 - 3 commits from branch
added 1 commit
- a080cabe - Make sure replaced service is still lazy and fix comment
added 32 commits
-
a080cabe...9f3ee1ff - 11 commits from branch
project:11.x
- 9f3ee1ff...3af7008b - 11 earlier commits
- 8d63a81f - Fix tests which install non-active database drivers
- d320f622 - Revert "Fix tests which install non-active database drivers"
- 3d52b188 - Fix backend override for lazy service
- 63b2e680 - More narrowly tailor backend-overridable tag
- a4628da3 - Handle original public value when registering proxy services
- 43770eec - Replace overridden services, don't alias them.
- d1bd195b - Fix CS
- 8dce5054 - Include now-legacy check for existing aliases.
- 3027483a - Make sure replaced service is still lazy and fix comment
- 732661a0 - Move backend-swap pass to be first
Toggle commit list-
a080cabe...9f3ee1ff - 11 commits from branch
added 24 commits
-
c1708b32 - 1 commit from branch
project:11.x
- c1708b32...2b0d91dc - 13 earlier commits
- 3b997189 - Fix backend override for lazy service
- bda5b722 - More narrowly tailor backend-overridable tag
- ec5aa249 - Handle original public value when registering proxy services
- 56587157 - Replace overridden services, don't alias them.
- 725a8d63 - Fix CS
- e9d88078 - Include now-legacy check for existing aliases.
- f8412e69 - Make sure replaced service is still lazy and fix comment
- e6eb7f9d - Move backend-swap pass to be first
- da306ae1 - Clone definitions while overriding
- 2212a137 - Fix override-check test
Toggle commit list-
c1708b32 - 1 commit from branch
added 1 commit
- 2a875ed4 - Ensure locations of db-driver backend overrides are consistent
added 28 commits
-
2a875ed4...5a3ca127 - 3 commits from branch
project:11.x
- 5a3ca127...38dba452 - 15 earlier commits
- 0d9f51f9 - Handle original public value when registering proxy services
- c62b1fe6 - Replace overridden services, don't alias them.
- 95398a2d - Fix CS
- dbc4e0b0 - Include now-legacy check for existing aliases.
- 1cbeb219 - Make sure replaced service is still lazy and fix comment
- 9ef1e50c - Move backend-swap pass to be first
- a0a2993a - Clone definitions while overriding
- 1caa2ee2 - Fix override-check test
- bc9b3f21 - Fix PHPCS
- f705fbcf - Ensure locations of db-driver backend overrides are consistent
Toggle commit list-
2a875ed4...5a3ca127 - 3 commits from branch
added 459 commits
-
f705fbcf...49f6e6bb - 434 commits from branch
project:11.x
- 49f6e6bb...a6e460c1 - 15 earlier commits
- 2c3c0aa9 - Handle original public value when registering proxy services
- 84ccb7a5 - Replace overridden services, don't alias them.
- be12f6a6 - Fix CS
- 46a0515d - Include now-legacy check for existing aliases.
- 05c7c250 - Make sure replaced service is still lazy and fix comment
- e673a096 - Move backend-swap pass to be first
- 5c3d66ad - Clone definitions while overriding
- 04819a8e - Fix override-check test
- 3d4d4f36 - Fix PHPCS
- 2a7e4657 - Ensure locations of db-driver backend overrides are consistent
Toggle commit list-
f705fbcf...49f6e6bb - 434 commits from branch
added 44 commits
-
2a7e4657...94bc9615 - 18 commits from branch
project:11.x
- 94bc9615...796aa1af - 16 earlier commits
- a310fffe - Replace overridden services, don't alias them.
- 1307110e - Fix CS
- e1d76363 - Include now-legacy check for existing aliases.
- ea11852c - Make sure replaced service is still lazy and fix comment
- 3a88bd3b - Move backend-swap pass to be first
- 091ce607 - Clone definitions while overriding
- 47dacac6 - Fix override-check test
- 9514d77e - Fix PHPCS
- b1c22ac5 - Ensure locations of db-driver backend overrides are consistent
- 53c0251e - Appease phpstan on return types
Toggle commit list-
2a7e4657...94bc9615 - 18 commits from branch
added 1 commit
- 53c0cdda - Adjust typehint in the persistent db lock backend.
added 1 commit
- a08ecf33 - Additional typehint updates for non-transactional services
added 1 commit
- dac5e1e3 - Fix tests with unnecessary container overrides
added 342 commits
-
f4d38b57...17b0403e - 300 commits from branch
project:11.x
- 17b0403e...fda28636 - 32 earlier commits
- d54983f5 - Additional typehint updates for non-transactional services
- bff51448 - Remove overzealous deprecation notice
- 31fd765b - Fix tests with unnecessary container overrides
- 9269397b - Unused use
- a39ce1c9 - Handle more hard-coded tests
- a8757689 - Fix CS
- 81f7f0d8 - Add method to flag transactions support
- 3d7cc896 - Add method to flag transactions support
- 1b77d064 - Complete interface
- 82a4423d - Update docs updated upstream not captured in rebase (methods moved)
Toggle commit list-
f4d38b57...17b0403e - 300 commits from branch