Follow-up of #1937600: Determine what services to register in the new Drupal class

Replace all calls to typed_data() with Drupal::typedData() and then remove the typed_data() function.

Files: 
CommentFileSizeAuthor
#17 fix-to-use-typeddata-1957158-17.patch12.04 KBJacobSanford
PASSED: [[SimpleTest]]: [MySQL] 57,036 pass(es).
[ View ]
#13 1957158-fix-to-use-typedData.patch12.51 KBabghosh82
PASSED: [[SimpleTest]]: [MySQL] 54,324 pass(es).
[ View ]
#10 1957158-fix-to-use-typedData.patch12.44 KBabghosh82
PASSED: [[SimpleTest]]: [MySQL] 54,211 pass(es).
[ View ]
#8 1957158-fix-to-use-typedData.patch14.03 KBabghosh82
PASSED: [[SimpleTest]]: [MySQL] 54,060 pass(es).
[ View ]
#5 replace_typed_data_with_DrupaltypedData-issue#1957158-comment#7265174.patch14.03 KBabghosh82
FAILED: [[SimpleTest]]: [MySQL] Fetch test patch: failed to retrieve [replace_typed_data_with_DrupaltypedData-issue#1957158-comment#7265174.patch] from [drupal.org].
[ View ]

Comments

Title:Replace calls of typed_data() with Drupal::typedData()Replace typed_data() with Drupal::typedData()
Issue tags:+Novice

Instructions:

Search & replace all calls to typed_data() with the mentioned method. Note that calls within namespaced classes (have a namespace line and are in the lib/ folder) needs to be "\Drupal".

Status:Active» Postponed

Postponing on the main issue.

Status:Postponed» Active

Assigned:Unassigned» abghosh82

Status:Active» Needs review
StatusFileSize
new14.03 KB
FAILED: [[SimpleTest]]: [MySQL] Fetch test patch: failed to retrieve [replace_typed_data_with_DrupaltypedData-issue#1957158-comment#7265174.patch] from [drupal.org].
[ View ]

Posting the patch file.

Status:Needs review» Needs work

You need to re-upload that patch with special charaters or whitespaces, the testbot doesn't like that. Also you probably want to use a name that's a bit shorter :)

Status:Needs work» Needs review
StatusFileSize
new14.03 KB
PASSED: [[SimpleTest]]: [MySQL] 54,060 pass(es).
[ View ]

Thanks for the suggestion, I am new to contribution, was not sure about the patch file name.

Status:Needs review» Needs work

+++ b/core/lib/Drupal/Core/Entity/EntityNG.phpundefined
@@ -7,6 +7,7 @@
+use Drupal;
@@ -170,7 +171,7 @@ protected function getTranslatedField($property_name, $langcode) {
+        $this->fields[$property_name][$langcode] = Drupal::typedData()->getPropertyInstance($this, $property_name, $value);
@@ -316,7 +317,7 @@ public function getTranslation($langcode, $strict = TRUE) {
+    $translation = Drupal::typedData()->create($translation_definition, $fields);

Per our coding standards, we don't add use's for classes without a namespace but just reference them inline as \Drupal.

+++ b/core/lib/Drupal/Core/TypedData/TypedDataManager.phpundefined
@@ -130,7 +130,7 @@ public function createInstance($plugin_id, array $configuration, $name = NULL, $
-   * @see typed_data()
+   * @see Drupal::typedData()

I guess the @see should also be \Drupal::typedData()

This is a special case as it seems we only have cases that should be injected and we will hopefully get there but for now, let's do this simple conversion. We could still get usages of this in procedural code later on.

Status:Needs work» Needs review
StatusFileSize
new12.44 KB
PASSED: [[SimpleTest]]: [MySQL] 54,211 pass(es).
[ View ]

Implemented the code review comments.

Assigned:abghosh82» Unassigned
Status:Needs review» Reviewed & tested by the community

Thanks, I think this looks good now. We'll have to try and inject these calls, but let's get this conversion in first.

Status:Reviewed & tested by the community» Needs work

We already have the container injected into tests... lets us it :)

+++ b/core/modules/system/lib/Drupal/system/Tests/Entity/EntityFieldTest.phpundefined
@@ -357,7 +357,7 @@ protected function assertIntrospection($entity_type) {
+    $wrapped_entity = \Drupal::typedData()->create($definition);

Should be $this->container->get('typed_data')->create($definition);

+++ b/core/modules/system/lib/Drupal/system/Tests/Entity/EntityFieldTest.phpundefined
@@ -477,7 +477,7 @@ protected function assertDataStructureInterfaces($entity_type) {
+    $wrapped_entity = \Drupal::typedData()->create($entity_definition, $entity);

Should be $this->container->get('typed_data')->create($entity_definition, $entity);

+++ b/core/modules/system/lib/Drupal/system/Tests/Entity/EntityFieldTest.phpundefined
@@ -536,7 +536,7 @@ public function testEntityConstraintValidation() {
+    $wrapped_entity = \Drupal::typedData()->create($entity_definition, $entity);

Should be $this->container->get('typed_data')->create($entity_definition, $entity);

+++ b/core/modules/system/lib/Drupal/system/Tests/Entity/EntityFieldTest.phpundefined
@@ -564,7 +564,7 @@ public function testEntityConstraintValidation() {
+    $wrapped_entity = \Drupal::typedData()->create($entity_definition, $node);

Should be $this->container->get('typed_data')->create($entity_definition, $node);

+++ b/core/modules/system/lib/Drupal/system/Tests/TypedData/TypedDataTest.phpundefined
@@ -42,7 +42,7 @@ public function setUp() {
+    $this->typedData = \Drupal::typedData();

Should be $this->container->get('typed_data');

+++ b/core/modules/system/lib/Drupal/system/Tests/TypedData/TypedDataTest.phpundefined
@@ -54,7 +54,7 @@ protected function createTypedData($definition, $value = NULL, $name = NULL) {
+    $data = \Drupal::typedData()->create($definition, $value, $name);

Not sure why this is not $this->typedData->create($definition, $value, $name); given that the snippet above this one is from the same test.

Status:Needs work» Needs review
StatusFileSize
new12.51 KB
PASSED: [[SimpleTest]]: [MySQL] 54,324 pass(es).
[ View ]

Thanks for pointing this out, I was really not sure of the injected containers. Implemented the suggestions.

Not too sure about using $this->container in tests. I already used \Drupal::something() in various other patches, e.g. Guzzle conversions. With $this->container, we don't get autocomplete for methods and there are cases where $this->container and Drupal::getContainer() aren't the same object anymore, due to rebuilds or so. Might not affect this but was a problem for config/state cache clears in other issues.

Assigned:Unassigned» abghosh82

@alexpott Please review the changes.

StatusFileSize
new12.04 KB
PASSED: [[SimpleTest]]: [MySQL] 57,036 pass(es).
[ View ]

Reroll!

Status:Needs review» Reviewed & tested by the community

Looks good, back to RTBC.

Status:Reviewed & tested by the community» Fixed

Committed 521e1e5 and pushed to 8.x. Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.