Problem/Motivation

I won't be possible to override any service that doesn't implement an interface so all core services should implement an interface.

Proposed resolution

  1. Create interfaces for all core services.
  2. Document any core services that for some reason can not implement an interface.
  3. Update references to all services which are injected in core to use the interface.

Remaining tasks

Check all core services for interface implementation.
Create sub tasks for any core services which fail to implement an interface.
Create interfaces or documentation to correct sub-tasks the issue for each case.

User interface changes

None

API changes

Injected dependencies will have to inject the interface instead of the implementation class directly.

Classes needing interfaces

\Drupal\Core\Access\CsrfTokenGenerator
\Drupal\Core\AppRootFactory
\Drupal\Core\Asset\LibraryDiscoveryParser
#2580757: Create an interface for \Drupal\Core\Cache\Context\CacheContextsManager
\Drupal\Core\Diff\DiffFormatter
#2580767: Create an interface for \Drupal\Core\Entity\EntityAutocompleteMatcher
#2580763: Create an interface for \Drupal\Core\Entity\EntityResolverManager
\Drupal\Core\Entity\Query\QueryFactory
\Drupal\Core\Http\ClientFactory
\Drupal\Core\Http\HandlerStackConfigurator
\Drupal\Core\Image\ImageFactory
\Drupal\Core\Language\LanguageDefault
\Drupal\Core\Menu\DefaultMenuLinkTreeManipulators
\Drupal\Core\Path\CurrentPathStack
\Drupal\Core\PrivateKey
\Drupal\Core\Queue\QueueDatabaseFactory
\Drupal\Core\Queue\QueueFactory
\Drupal\Core\Routing\AdminContext
\Drupal\Core\Routing\RequestContext
\Drupal\Core\SitePathFactory
\Drupal\Core\Theme\Registry
\Drupal\Core\Update\UpdateRegistry
\Drupal\Core\Update\UpdateRegistryFactory
\Drupal\Core\Utility\Token (WTF, why is a utility a service?)

\Drupal\Core\Site\Settings this is final, seems purposeful

The following do not have interfaces, but directly extend from upstream classes:
\Drupal\Core\Template\Loader\FilesystemLoader
\Drupal\Core\Template\Loader\ThemeRegistryLoader
\Drupal\Core\Template\TwigEnvironment
\Drupal\Core\Template\TwigExtension

Data model changes

None

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task because services work without a specific interface. It just hampers the service's extensibility.
Issue priority Major because contrib modules will have a hard if something just won't work in a service for the module's use case. Not critical because drupal 8 does not override services in core.
Disruption I don't think this would be disruptive for contrib because it only affects services that cannot currently be overridden but each service should be evaluated on it's own to determine possible disruptions.
CommentFileSizeAuthor
#8 2572181-core-services-8c.patch125.57 KBmarvin_b8
#4 2572181-core services-4.patch126.06 KBmarvin_b8

Comments

Jaesin created an issue. See original summary.

Jaesin’s picture

Jaesin commented

I think services will have to implement their own interface. For instance, if a service injects class ConditionManager extends DefaultPluginManager implements ExecutableManagerInterface, CategorizingPluginManagerInterface They will probably use public function __construct(\ConditionManager $condition_manager... . If that service is overridden, this constructor will error out.

marvin_b8’s picture

marvin_b8 commented

Only Extend some class

  1. Drupal\Core\Render\PlaceholderingRenderCache
  2. Drupal\Core\Template\TwigEnvironment
  3. Drupal\Core\Template\TwigExtension
  4. Drupal\Core\Session\MetadataBag
  5. Drupal\Core\Template\Loader\FilesystemLoader
  6. Drupal\Core\Template\Loader\ThemeRegistryLoader
  7. Drupal\Core\Asset\LibraryDiscoveryCollector
  8. Drupal\Core\ImageToolkit\ImageToolkitManager
  9. Drupal\Core\StreamWrapper\TemporaryStream
  10. Drupal\Core\StreamWrapper\PublicStream
  11. Drupal\Core\Transliteration\PhpTransliteration
  12. Drupal\Core\EventSubscriber\ExceptionJsonSubscriber
  13. Drupal\Core\EventSubscriber\CustomPageExceptionHtmlSubscriber
  14. Drupal\Core\EventSubscriber\Fast404ExceptionHtmlSubscriber
  15. Drupal\Core\EventSubscriber\ExceptionTestSiteSubscriber
  16. Drupal\Core\EventSubscriber\ExceptionJsonSubscriber
  17. Drupal\Core\EventSubscriber\ConfigImportSubscriber
  18. Drupal\Core\Render\MainContent\ModalRenderer
  19. Drupal\Core\Controller\HtmlFormController
  20. Drupal\Core\Entity\HtmlEntityFormController
  21. Drupal\Core\EventSubscriber\SpecialAttributesRouteSubscriber
  22. Drupal\Core\ParamConverter\AdminPathConfigEntityConverter
  23. Drupal\Core\EventSubscriber\ModuleRouteSubscriber
  24. Drupal\Core\Entity\Query\Sql\pgsql\QueryFactory
  25. Drupal\Core\Routing\UrlMatcher
  26. Drupal\Core\Validation\ConstraintManager
  27. Drupal\Core\Lock\DatabaseLockBackend
  28. Drupal\Core\Lock\PersistentDatabaseLockBackend
  29. Drupal\Core\Routing\RequestContext
  30. Drupal\Core\StringTranslation\Translator\CustomStrings
  31. Drupal\Core\Display\VariantManager
  32. Drupal\Core\Field\WidgetPluginManager
  33. Drupal\Core\Field\FormatterPluginManager
  34. Drupal\Core\Archiver\ArchiverManager
  35. Drupal\Core\Entity\Event\BundleConfigImportValidate
  36. Drupal\Core\Cache\Context\RouteNameCacheContext
  37. Drupal\Core\Cache\Context\SessionCacheContext
  38. Drupal\Core\Cache\Context\RequestFormatCacheContext

Implement no interface and don't extend another class

  1. Drupal\Core\Update\UpdateRegistry
  2. Drupal\Core\Asset\LibraryDiscoveryParser
  3. Drupal\Core\Datetime\DateFormatter
  4. Drupal\Core\Utility\Token
  5. Drupal\Core\Image\ImageFactory
  6. Drupal\Core\PrivateKey
  7. Drupal\Core\Access\CsrfTokenGenerator
  8. Drupal\Core\Entity\EntityResolverManager
  9. Drupal\Core\Routing\AdminContext
  10. Drupal\Core\Language\LanguageDefault
  11. Drupal\Core\Menu\DefaultMenuLinkTreeManipulators
  12. Drupal\Core\AppRootFactory
  13. Drupal\Core\SitePathFactory
  14. Drupal\Core\Entity\EntityAutocompleteMatcher
  15. Drupal\Core\Path\CurrentPathStack
  16. Drupal\Core\Http\HandlerStackConfigurator
  17. Drupal\Core\Http\ClientFactory
  18. Drupal\Core\Queue\QueueDatabaseFactory
  19. Drupal\Core\Site\Settings
  20. Drupal\Core\Config\FileStorageFactory
  21. Drupal\Core\Cache\Context\RouteCacheContext

a list of all services will follow if necessary

marvin_b8’s picture

marvin_b8 commented
Status: Active » Needs review
StatusFileSize
new 2572181-core services-4.patch126.06 KB

Only static methods so we don't need a interface for the following classes:

  1. Drupal\Core\Site\Settings
  2. Drupal\Core\Config\FileStorageFactory

By this classes i'm not sure if we need some interfaces:

  1. Drupal\Core\Template\TwigEnvironment
  2. Drupal\Core\Template\TwigExtension
  1. Drupal\Core\EventSubscriber\ExceptionJsonSubscriber
  2. Drupal\Core\EventSubscriber\CustomPageExceptionHtmlSubscriber
  3. Drupal\Core\EventSubscriber\Fast404ExceptionHtmlSubscriber
  4. Drupal\Core\EventSubscriber\ExceptionTestSiteSubscriber
  5. Drupal\Core\EventSubscriber\ExceptionJsonSubscriber
  6. Drupal\Core\EventSubscriber\ConfigImportSubscriber
  7. Drupal\Core\EventSubscriber\SpecialAttributesRouteSubscriber
  8. Drupal\Core\EventSubscriber\ModuleRouteSubscriber

In think all of this classes need a interface.
Because the have some method that are not in the interface of the extend class.

  1. Drupal\Core\Session\MetadataBag
  2. Drupal\Core\Template\Loader\FilesystemLoader
  3. Drupal\Core\ImageToolkit\ImageToolkitManager
  4. Drupal\Core\Validation\ConstraintManager
  5. Drupal\Core\Validation\ConstraintManager
  6. Drupal\Core\Routing\RequestContext
  7. Drupal\Core\Field\WidgetPluginManager extends
  8. Drupal\Core\Field\FormatterPluginManager
  9. Drupal\Core\Archiver\ArchiverManager

And this classes are ok

  1. Drupal\Core\Render\PlaceholderingRenderCache
  2. Drupal\Core\Template\Loader\ThemeRegistryLoader
  3. Drupal\Core\Asset\LibraryDiscoveryCollector
  4. Drupal\Core\StreamWrapper\TemporaryStream
  5. Drupal\Core\StreamWrapper\PublicStream
  6. Drupal\Core\Transliteration\PhpTransliteration
  7. Drupal\Core\Render\MainContent\ModalRenderer
  8. Drupal\Core\Controller\HtmlFormController
  9. Drupal\Core\Entity\HtmlEntityFormController
  10. Drupal\Core\ParamConverter\AdminPathConfigEntityConverter
  11. Drupal\Core\Routing\UrlMatcher
  12. Drupal\Core\Lock\DatabaseLockBackend
  13. Drupal\Core\Lock\PersistentDatabaseLockBackend
  14. Drupal\Core\StringTranslation\Translator\CustomStrings
  15. Drupal\Core\Display\VariantManager
  16. Drupal\Core\Entity\Event\BundleConfigImportValidate
  17. Drupal\Core\Cache\Context\RouteCacheContext
  18. Drupal\Core\Cache\Context\RouteNameCacheContext
  19. Drupal\Core\Cache\Context\SessionCacheContext
  20. Drupal\Core\Cache\Context\RequestFormatCacheContext

And as attachment the first patch.
It's only adds to all classes without a interface, a interface.

Status: Needs review » Needs work

The last submitted patch, 4: 2572181-core services-4.patch, failed testing.

The last submitted patch, 4: 2572181-core services-4.patch, failed testing.

Jaesin’s picture

Jaesin commented

@marvin_B8 Thanks for diving into this.

Here are a couple of tips about the patch.

  1. +++ b/core/lib/Drupal/Core/Access/CsrfTokenGeneratorInterface.php
@@ -0,0 +1,51 @@
\ No newline at end of file

    Let's make sure to add newlines to the end of files. 

  2. +++ b/core/lib/Drupal/Core/ParamConverter/AdminPathConfigEntityConverter.php
@@ -50,11 +50,11 @@ class AdminPathConfigEntityConverter extends EntityConverter {
+  public function __construct(EntityManagerInterface $entity_manager, ConfigFactoryInterface $config_factory, AdminContextInterface $admin_context) {

    perfect, This is exactly the kind of thing that gets us into trouble. 

  3. +++ b/core/lib/Drupal/Core/Routing/AdminContextInterface.php
@@ -0,0 +1,27 @@
+interface AdminContext

    "AdminContext" should be "AdminContextInterface" This looks like the primary reason the test is failing.

marvin_b8’s picture

marvin_b8 commented
Status: Needs work » Needs review
StatusFileSize
new 2572181-core-services-8c.patch125.57 KB

another try

Status: Needs review » Needs work

The last submitted patch, 8: 2572181-core-services-8c.patch, failed testing.

Anonymous’s picture

Anonymous (not verified) commented
Issue tags: +Needs beta evaluation

Creating an interface for the DateFormatter service has a separate issue which is RTBC: #2571647: Create DateFormatterInterface.

Keeping it there will make this patch less intrusive.

The last submitted patch, 8: 2572181-core-services-8c.patch, failed testing.

Jaesin’s picture

Jaesin commented

@pjonckiere Thanks for the note. I totally agree.

Create sub tasks for any core services which fail to implement an interface

All patches should go in a sub-task and be dealt with individually so their risks and validity can be evaluated separately.

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
commented

Let's convert this to a meta issue and create sub-issues for each service in need of an interface, that will result in less intrusive and reviewable patches which can be committed more easily.

Jaesin’s picture

Jaesin commented
Title: Make sure all core services implement an interface. » [META] Make sure all core services implement an interface.
Category: Bug report » Task
Issue summary: View changes
Issue tags: -Needs beta evaluation +epic, +meta

Adding [META] to the title and a beta evaluation.

mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
commented
Category: Task » Plan
Issue summary: View changes

Added Tim's findings from #2580739: [META] All core services should implement an interface

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.