Provide a LoadableInterface for Typed Data objects

I discussed this already with fubhy and I think it's useful addition. It kind of depends on #1868004: Improve the TypedData API usage of EntityNG to really make sense, but applies to typed integration for all "entity" types anyway.

public static function load($id, $type, $definition) {

$type is duplicated here as it has to be part of $definition anyway.

+++ b/core/lib/Drupal/Core/Entity/Entity.php
@@ -75,6 +75,13 @@ public function __construct(array $values, $entity_type) {
+  public static function load(array $definition, $id) {

Should be $id, $definition so we could make $definition optional for Node::load() and remove node_load()?

Else, this looks good and obviously misses tests + implementation ;-)

A fix for the views fails.

In general I'm wondering why this patch does not follow the pattern we have for plugins and controllers and will have with entity controller: Have a method which get the container + additional metadata
as most dependencies will be part of the container.

If you have a look at patch you see the need for the container.

+++ b/core/lib/Drupal/Core/Entity/Entity.php
@@ -75,6 +75,19 @@ public function __construct(array $values, $entity_type) {
+    if ($entity = \Drupal::service('plugin.manager.entity')->getStorageController($definition['constraints']['EntityType'])->load(array($id))) {

We want to eventually get rid of this class, not use it in more places.

+++ b/core/lib/Drupal/Core/Entity/Entity.php
@@ -75,6 +75,19 @@ public function __construct(array $values, $entity_type) {
+    return FALSE;

This should return null: http://www.garfieldtech.com/blog/empty-return-values

+++ b/core/lib/Drupal/Core/TypedData/LoadableInterface.php
@@ -0,0 +1,28 @@
+   * @return \Drupal\Core\TypedData\TypedDataInterface|bool
+   *   The loaded typed data object or FALSE if it could not be loaded.

See above.

+++ b/core/lib/Drupal/Core/TypedData/LoadableInterface.phpundefined
@@ -0,0 +1,32 @@
+ * Contains LoadableInterface.

Let's use the fullpath here.

+++ b/core/lib/Drupal/Core/TypedData/LoadableInterface.phpundefined
@@ -0,0 +1,32 @@
+   *   The loaded typed data object or NULL if it could not be loaded.

+++ b/core/lib/Drupal/Core/TypedData/TypedDataManager.phpundefined
@@ -151,6 +154,51 @@ public function create(array $definition, $value = NULL, $name = NULL, $parent =
+   *   The loaded typed data object or FALSE if it could not be loaded.

That's odd, shouldn't there be just a single value return if someone couldn't be loaded?

+++ b/core/lib/Drupal/Core/TypedData/TypedDataManager.phpundefined
@@ -15,6 +16,7 @@
@@ -55,12 +57,13 @@ class TypedDataManager extends PluginManagerBase {

I guess if we depend on the container this should implement ContainerAwareInterface (see the common on top of that one).

+++ b/core/lib/Drupal/Core/TypedData/TypedDataManager.phpundefined
@@ -55,12 +57,13 @@ class TypedDataManager extends PluginManagerBase {
+    $this->container = $container;

We need documentation for the new property.

+++ b/core/lib/Drupal/Core/TypedData/TypedDataManager.phpundefined
@@ -151,6 +154,51 @@ public function create(array $definition, $value = NULL, $name = NULL, $parent =
+   * @param $id

so i guess it is mxed/

+++ b/core/lib/Drupal/Core/TypedData/TypedDataManager.phpundefined
@@ -151,6 +154,51 @@ public function create(array $definition, $value = NULL, $name = NULL, $parent =
+      throw new InvalidArgumentException(format_string('Invalid data type %plugin_id has been given.', array('%plugin_id' => $definition['type'])));
...
+      throw new PluginException(sprintf('The plugin (%s) did not specify an instance class.', $definition['type']));

format_string => String::format(). At least sprintf and format_string should not appear in the same class.

+++ b/core/lib/Drupal/Core/TypedData/TypedDataManager.phpundefined
@@ -151,6 +154,51 @@ public function create(array $definition, $value = NULL, $name = NULL, $parent =
+    // Allow per-data definition overrides of the used classes, i.e. take over
+    // classes specified in the data definition.
+    $key = empty($definition['list']) ? 'class' : 'list class';
+    if (isset($definition[$key])) {
+      $class = $definition[$key];

What are usecases for that? This seems to be to flexible?

+++ b/core/lib/Drupal/Core/Entity/EntityBCDecorator.php
@@ -68,6 +69,13 @@ function __construct(EntityNG $decorated, array &$definitions) {
+  public static function load(ContainerInterface $container, $id, array $definition) {

hm, it's quite unfortunate that we have to pass on the container.

Can we make it so that definition and container could be optional and you could do Node::load($id) if you care to re-define it?

I must say I dislike that we need to pass on the service container to load() but I see that we need to meet our dependencies somehow. However, what about introducing separate loader objects with proper dependency injection?
In the case of entities we already have them even -> the storage controllers. Could we re-use them somehow, e.g. by having one optional loader class per type which implements a container aware factory in which it gets the right storage controller?

Which information do we need in the plugins? Wouldn't it be enough to have the loader class in the data type plugin definition?

29: 1983100-29.patch queued for re-testing.

I think according to recent discussions with @fubhy, this is no longer required. Please confirm...