Add an EntityStorageControllerBase

Yeah, I agree that would be a good idea.

I don't think we should try to do too much here tbh. And Tim, I still think we should not try to make config entities fieldable :) Here is an initial patch moving over some basic properties and cache methods to a base class. Let's get the ball rolling atleast.

I haven't touched any caching logic in the COnfigStorageController class, as it seems we should tackle that in the summary related issue.

whooops

Doh, this is the actual correct patch. sorry for the noise.

Really, don't roll patches when you first wake up!

Great, thanks.

+++ b/core/lib/Drupal/Core/Entity/EntityStorageControllerBase.phpundefined
@@ -0,0 +1,133 @@
+  public function __construct($entityType) {
+    $this->entityType = $entityType;
+    $this->entityInfo = entity_get_info($entityType);
+    // Check if the entity type supports static caching of loaded entities.
+    $this->cache = !empty($this->entityInfo['static_cache']);
+  }

The parent method has $entityType, so we maybe fix it now, or in a follow up

Yeah, that is being tackled in #1909418: Allow Entity Controllers to have their dependencies injected

#1893820: Manage entity field definitions in the entity manager takes care of moving the fieldDefinitions part out - so this one would by solved with that then.

I'm not sure of the validity of this patch given the discussion about config context and entity static caching. Tempted to set as won't fix.

Needs re-roll :(

Rerolled.

I think we need this either way.

Suppose caching should be implemented properly

+++ b/core/lib/Drupal/Core/Config/Entity/ConfigStorageController.phpundefined
@@ -136,14 +102,6 @@ public static function createInstance(ContainerInterface $container, $entity_typ
-  public function resetCache(array $ids = NULL) {
-    // The configuration system is fast enough and/or implements its own
-    // (advanced) caching mechanism already.

@@ -183,15 +141,6 @@ public function load(array $ids = NULL) {
-    $this->resetCache(array($id));

+++ b/core/lib/Drupal/Core/Entity/DatabaseStorageController.phpundefined
@@ -175,23 +124,6 @@ public function __construct($entity_type, array $entity_info, Connection $databa
-    // Check if the entity type supports static caching of loaded entities.
-    $this->cache = !empty($this->entityInfo['static_cache']);
...
-        unset($this->entityCache[$id]);

@@ -262,15 +194,6 @@ public function load(array $ids = NULL) {
-    $this->resetCache(array($id));

@@ -444,34 +367,6 @@ protected function attachLoad(&$queried_entities, $load_revision = FALSE) {
-    if (!empty($this->entityCache)) {
...
-  protected function cacheSet($entities) {
-    $this->entityCache += $entities;

+++ b/core/lib/Drupal/Core/Entity/EntityStorageControllerBase.phpundefined
@@ -0,0 +1,133 @@
+    // Check if the entity type supports static caching of loaded entities.
+    $this->cache = !empty($this->entityInfo['static_cache']);
...
+  public function resetCache(array $ids = NULL) {
+    if (isset($ids)) {
...
+    if (!empty($this->entityCache)) {

Usage of $this->cache was broken for db entities?

#18: 1978870-18.patch queued for re-testing.

needs re-roll

Rerole and a minor thing.

Nice!

Amazingly, #22 still applies despite being 4 days old, so removing the "Needs reroll" tag.

Committed 1745bd2 and pushed to 8.x. Thanks!