drupal_container() is deprecated, and all calls in the system module need to be replaced with Drupal::service(), except for where the module_handler service is requested, which needs to be replaced with Drupal::moduleHandler() (see #1957154: Replace calls to drupal_container()->get('module_handler') service with Drupal::moduleHandler())

This task a part of #2001206: Replace drupal_container() with Drupal::service()

Files: 
CommentFileSizeAuthor
#73 replace-drupal-container-with-drupal-service-2014017-73.patch6.42 KBLinL
PASSED: [[SimpleTest]]: [MySQL] 58,568 pass(es).
[ View ]
#64 replace-drupal-container-with-drupal-service-2014017-64.patch7.62 KBjlbellido
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch replace-drupal-container-with-drupal-service-2014017-64.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#64 interdiff.txt1.86 KBjlbellido
#62 replace-drupal-container-with-drupal-service-2014017-62.patch7.71 KBjlbellido
PASSED: [[SimpleTest]]: [MySQL] 59,234 pass(es).
[ View ]
#59 replace-drupal-container-with-drupal-service-2014017-59.patch7.61 KBjlbellido
PASSED: [[SimpleTest]]: [MySQL] 58,696 pass(es).
[ View ]
#59 interdiff.txt859 bytesjlbellido
#56 replace-drupal-container-with-drupal-service-2014017-56.patch7.55 KBjlbellido
PASSED: [[SimpleTest]]: [MySQL] 58,347 pass(es).
[ View ]
#49 patch-interdiff.txt4.28 KBpenyaskito
#49 replace-drupal-container-with-drupal-service-2014017-49.patch8.24 KBpenyaskito
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch replace-drupal-container-with-drupal-service-2014017-49.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#45 replace-drupal-container-with-drupal-service-2014017-45.patch8.97 KBjlbellido
PASSED: [[SimpleTest]]: [MySQL] 57,939 pass(es).
[ View ]
#45 interdiff.txt5.66 KBjlbellido
#44 replace-drupal-container-with-drupal-service-2014017-44.patch8.91 KBjlbellido
PASSED: [[SimpleTest]]: [MySQL] 57,607 pass(es).
[ View ]
#37 replace-drupal-container-with-drupal-service-2014017-38.patch10.67 KBjlbellido
PASSED: [[SimpleTest]]: [MySQL] 57,589 pass(es).
[ View ]
#33 replace-drupal-container-with-drupal-service-2014017-33.patch10.71 KBjlbellido
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch replace-drupal-container-with-drupal-service-2014017-33.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#33 interdiff.txt763 bytesjlbellido
#28 replace-drupal-container-with-drupal-service-2014017-28.patch10.71 KBjlbellido
PASSED: [[SimpleTest]]: [MySQL] 57,552 pass(es).
[ View ]
#26 replace-drupal-container-with-drupal-service-2014017-26.patch10.66 KBjlbellido
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch replace-drupal-container-with-drupal-service-2014017-26.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#22 replace-drupal-container-with-drupal-service-2014017-22.patch14.71 KBalvar0hurtad0
FAILED: [[SimpleTest]]: [MySQL] 56,443 pass(es), 33 fail(s), and 32 exception(s).
[ View ]
#20 replace-drupal-container-with-drupal-service-2014017-20.patch14.23 KBalvar0hurtad0
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch replace-drupal-container-with-drupal-service-2014017-20.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#16 replace-drupal-container-with-drupal-service-2014017-16.patch14.3 KBalvar0hurtad0
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch replace-drupal-container-with-drupal-service-2014017-16.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#14 2014017-14.patch2 MBalvar0hurtad0
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2014017-14_0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#13 replace-drupal-container-with-drupal-service-2014017-13.patch12.86 KBjlbellido
PASSED: [[SimpleTest]]: [MySQL] 56,710 pass(es).
[ View ]
#8 replace-drupal-container-with-drupal-service-2014017-8.patch12.86 KBjlbellido
PASSED: [[SimpleTest]]: [MySQL] 58,096 pass(es).
[ View ]
#6 replace-drupal-container-with-drupal-service-2014017-7.patch12.85 KBundertext
PASSED: [[SimpleTest]]: [MySQL] 57,836 pass(es).
[ View ]
#4 replace-drupal-container-with-drupal-service-2014017-2.patch13.33 KBundertext
PASSED: [[SimpleTest]]: [MySQL] 58,741 pass(es).
[ View ]
#3 replace-drupal-container-with-drupal-service-2014017-2.patch13.33 KBundertext
PASSED: [[SimpleTest]]: [MySQL] 57,132 pass(es).
[ View ]
#1 replace-drupal-container-with-drupal-service-2014017-1.patch12.84 KBbenjy
FAILED: [[SimpleTest]]: [MySQL] 55,305 pass(es), 4 fail(s), and 0 exception(s).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new12.84 KB
FAILED: [[SimpleTest]]: [MySQL] 55,305 pass(es), 4 fail(s), and 0 exception(s).
[ View ]

Initial patch to get things started. Pretty much a find and replace however I changed Drupal::service('request') for Drupal::request()

Status:Needs review» Needs work

The last submitted patch, replace-drupal-container-with-drupal-service-2014017-1.patch, failed testing.

StatusFileSize
new13.33 KB
PASSED: [[SimpleTest]]: [MySQL] 57,132 pass(es).
[ View ]

Use statements added.

Status:Needs work» Needs review
StatusFileSize
new13.33 KB
PASSED: [[SimpleTest]]: [MySQL] 58,741 pass(es).
[ View ]

Status:Needs review» Needs work

+++ b/core/modules/system/lib/Drupal/system/Tests/Path/UrlAlterFunctionalTest.phpundefined
@@ -7,6 +7,7 @@
+use Drupal;
@@ -120,7 +121,7 @@ protected function assertUrlOutboundAlter($original, $final) {
-    $result = drupal_container()->get('path.alias_manager')->getSystemPath($original);
+    $result = Drupal::service('path.alias_manager')->getSystemPath($original);

Don't use Drupal, use \Drupal. Same for those below.

Status:Needs work» Needs review
StatusFileSize
new12.85 KB
PASSED: [[SimpleTest]]: [MySQL] 57,836 pass(es).
[ View ]

Ok. As I understand from https://drupal.org/node/1353118

Classes and interfaces without a backslash \ inside their fully-qualified name (for example, the built-in PHP Exception class) must be fully qualified when used in a namespaced file. For example: new \Exception();.

Status:Needs review» Needs work

It looks like there are a lot of calls in test files that need to be converted to this->container->get().

StatusFileSize
new12.86 KB
PASSED: [[SimpleTest]]: [MySQL] 58,096 pass(es).
[ View ]

re-roll

Status:Needs work» Needs review

I was looking for more instances of drupal_container()->get instances and i didn't find one (i applied #8 patch before search). This is the instances list of drupal_container that i found in system module:

lib/Drupal/system/Tests/Plugin/CacheDecoratorTest.php:    drupal_container()->set("cache.$this->cacheBin", new MemoryBackend($this->cacheBin));
lib/Drupal/system/Tests/Bundle/BundleTest.php:    $this->assertTrue(drupal_container()->getDefinition('file.usage')->getClass() == 'Drupal\\bundle_test\\TestFileUsage', 'Class has been changed');
lib/Drupal/system/Tests/Bundle/BundleTest.php:    $this->assertTrue(drupal_container()->has('bundle_test_class'), 'The bundle_test_class service has been registered to the DIC');
lib/Drupal/system/Tests/Bundle/BundleTest.php:    $this->assertFalse(drupal_container()->has('bundle_test_class'), 'The bundle_test_class service does not exist in the DIC.');
lib/Drupal/system/Tests/Bundle/BundleTest.php:    $this->assertTrue(drupal_container()->has('bundle_test_class'), 'The bundle_test_class service exists in the DIC.');
lib/Drupal/system/Tests/Upgrade/UpgradePathTestBase.php:    $this->containerBuild(drupal_container());
lib/Drupal/system/Tests/Upgrade/UpgradePathTestBase.php:      $container = drupal_container();
system.module:  if (drupal_container()->isScopeActive('request')) {

Status:Needs review» Needs work
Issue tags:+Needs reroll

error: patch failed: core/modules/system/language.api.php:191
error: core/modules/system/language.api.php: patch does not apply

needs reroll.

Status:Needs work» Needs review
Issue tags:-Needs reroll

Rerolled #8

StatusFileSize
new12.86 KB
PASSED: [[SimpleTest]]: [MySQL] 56,710 pass(es).
[ View ]

Sorry, attached .patch

StatusFileSize
new2 MB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2014017-14_0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Rerolled and tested (I hope)

Status:Needs review» Needs work

The last submitted patch, 2014017-14.patch, failed testing.

StatusFileSize
new14.3 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch replace-drupal-container-with-drupal-service-2014017-16.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Re-trying upload the patch now following the steps at https://drupal.org/patch/reroll

+++ b/core/modules/system/system.moduleundefined
@@ -1027,13 +1027,45 @@ function system_plugin_ui_form($form, &$form_state, $plugin, $facet = NULL) {
+/**
+>>>>>>> Applying patch from https://drupal.org/node/2014017#comment-7608725
  * Checks access for a given plugin using the plugin's access() method.
  *
  * @todo This needs more explanation, some @see, and parameter documentation.

Should this be there?

Where?

This line

+++ b/core/modules/system/system.module
@@ -1027,13 +1027,45 @@ function system_plugin_ui_form($form, &$form_state, $plugin, $facet = NULL) {
+>>>>>>> Applying patch from https://drupal.org/node/2014017#comment-7608725

Status:Needs work» Needs review
StatusFileSize
new14.23 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch replace-drupal-container-with-drupal-service-2014017-20.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Ups,

You're right, Sorry and thank you very much :)

Trying again.

Status:Needs review» Needs work

The last submitted patch, replace-drupal-container-with-drupal-service-2014017-20.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new14.71 KB
FAILED: [[SimpleTest]]: [MySQL] 56,443 pass(es), 33 fail(s), and 32 exception(s).
[ View ]

Ok,

I know it's an easy task but it's my first patch.

Trying again

Status:Needs review» Needs work

The last submitted patch, replace-drupal-container-with-drupal-service-2014017-22.patch, failed testing.

@alvar0hurtad0 not to worry everyone is here to help.

Do you have a another patch applied in this? It seems there is a new test added and some other stuff?

I have reviewed #22 and i think that this are the mistakes :

- This must be deleted , because in HEAD version it isn't:

+
+    // Check in a rendered page.
+    $this->drupalGet('common-test/query-string');
+    $this->assertPattern('@<script>.+drupalSettings.+"currentPath":"common-test\\\/query-string"@s', 'currentPath is in the JS settings');
+    $path = array('source' => 'common-test/query-string', 'alias' => 'common-test/currentpath-check');
+    \Drupal::service('path.crud')->save($path['source'], $path['alias']);
+    $this->drupalGet('common-test/currentpath-check');
+    $this->assertPattern('@<script>.+drupalSettings.+"currentPath":"common-test\\\/query-string"@s', 'currentPath is in the JS settings for an aliased path');

- This code :

-    $user1->pass = drupal_container()->get('password')->hash(trim($user1->pass_raw));
-    db_query("UPDATE {users} SET pass = :pass WHERE uid = :uid", array(':pass' => $user1->pass, ':uid' => $user1->id()));
+    $user1->pass = \Drupal::service('password')->hash(trim($user1->pass_raw));
+    db_query("UPDATE {users} SET pass = :pass WHERE uid = :uid", array(':pass' => $user1->pass, ':uid' => $user1->uid));

must be :
-  $user1->pass = drupal_container()->get('password')->hash(trim($user1->pass_raw));
+ $user1->pass = \Drupal::service('password')->hash(trim($user1->pass_raw));

Because db_query function hasn't drupal_container function and we must preserve HEAD version.

- This code must be deleted too because this code isn't in HEAD version:

+    $config_to_import['views.view.frontpage'] = 'node';
+  }
+  foreach ($config_to_import as $config_name => $module) {
+    $module_config_path = drupal_get_path('module', $module) . '/config';
+    $module_filestorage = new FileStorage($module_config_path);
+    $config_storage = Drupal::service('config.storage');
+    // If this file already exists, something in the upgrade path went
+    // completely wrong and we want to know.
+    if ($config_storage->exists($config_name)) {
+      throw new ConfigException(format_string('Default configuration file @name of @module module unexpectedly exists already before the module was installed.', array(
+        '@module' => $module,
+        '@name' => $config_name,
+      )));
+    }
+    $config_storage->write($config_name, $module_filestorage->read($config_name));

- This code must be deleted too for the same reason:

+ * Page callback: Autocompletes any plugin system tied to a plugin UI plugin.
+ *
+ * The passed plugin_id indicates the specific plugin_ui plugin that is in use
+ * here. The documentation within the annotation of that plugin will contain a
+ * manager for the plugins that need to be autocompleted allowing this function
+ * to autocomplete plugins for any plugin type.
+ *
+ * @param $plugin_id
+ *   The plugin id for the calling plugin.
+ *
+ * @return object JsonResponse
+ */
+function system_plugin_autocomplete($plugin_id) {
+  $string_typed = Drupal::request()->query->get('q');
+  $string_typed = drupal_explode_tags($string_typed);
+  $string = drupal_strtolower(array_pop($string_typed));
+  $matches = array();
+  if ($string) {
+    $plugin_ui = Drupal::service('plugin.manager.system.plugin_ui')->getDefinition($plugin_id);
+    $manager = Drupal::service($plugin_ui['manager']);
+    $titles = array();
+    foreach($manager->getDefinitions() as $plugin_id => $plugin) {
+      $titles[$plugin_id] = $plugin[$plugin_ui['title_attribute']];
+    }
+    $matches = preg_grep("/\b". $string . "/i", $titles);
+  }
+
+  return new JsonResponse($matches);
+}
+
+/**

Don't worry,this reroll is very dificult to start. Try it one more time with this changes, i hope that it works.

Regards!

Status:Needs work» Needs review
StatusFileSize
new10.66 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch replace-drupal-container-with-drupal-service-2014017-26.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Rerolled and run test.

Status:Needs review» Needs work

The last submitted patch, replace-drupal-container-with-drupal-service-2014017-26.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new10.71 KB
PASSED: [[SimpleTest]]: [MySQL] 57,552 pass(es).
[ View ]

I did #26 reroll past Tuesday , it seems that something has changed again.Trying one more time with a new reroll, this can be applied locally

@Jlbellido you are my heroe.

Status:Needs review» Fixed

It applyes ok

Status:Fixed» Needs review

Status:Needs review» Needs work

+++ b/core/modules/system/lib/Drupal/system/Tests/Path/UrlAlterFunctionalTest.phpundefined
@@ -112,7 +112,7 @@ protected function assertUrlOutboundAlter($original, $final) {
+   *   Drupal::service('path.alias_manager')->getSystemPath().

This should be absolute as per: https://drupal.org/node/1353118

Status:Needs work» Needs review
StatusFileSize
new763 bytes
new10.71 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch replace-drupal-container-with-drupal-service-2014017-33.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Changed "Drupal::service('path.alias_manager')->getSystemPath()" to "\Drupal::service('path.alias_manager')->getSystemPath()"

Status:Needs review» Reviewed & tested by the community

Reviewed with emphasis on "Drupal" vs "\Drupal" and IMHO looks nice now.

Status:Reviewed & tested by the community» Needs work
Issue tags:+Novice

The last submitted patch, replace-drupal-container-with-drupal-service-2014017-33.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new10.67 KB
PASSED: [[SimpleTest]]: [MySQL] 57,589 pass(es).
[ View ]

Rerolled #33 and run test.

Status:Needs review» Needs work
Issue tags:-Novice

The last submitted patch, replace-drupal-container-with-drupal-service-2014017-38.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+Novice

Unrelated failure, retesting.

Status:Needs review» Reviewed & tested by the community

Back to RTBC.

Looks good to me.

Status:Reviewed & tested by the community» Needs work

+++ b/core/modules/system/lib/Drupal/system/Plugin/PluginUIBase.phpundefined
@@ -27,7 +27,7 @@ public function form($form, &$form_state) {
+      $manager = \Drupal::service($plugin_definition['manager']);

All OO code should have their dependencies injected.

+++ b/core/modules/system/lib/Drupal/system/Tests/Path/UrlAlterFunctionalTest.phpundefined
@@ -112,7 +112,7 @@ protected function assertUrlOutboundAlter($original, $final) {
+   *   \Drupal::service('path.alias_manager')->getSystemPath().

In tests we can use $this->container->get($service_id);

Status:Needs work» Needs review
StatusFileSize
new8.91 KB
PASSED: [[SimpleTest]]: [MySQL] 57,607 pass(es).
[ View ]

It needs be rerolled first, atached the rerolled patch again and running tests

StatusFileSize
new5.66 KB
new8.97 KB
PASSED: [[SimpleTest]]: [MySQL] 57,939 pass(es).
[ View ]

Changed "\Drupal::service($service_id)" to "$this->container->get($service_id)" in tests.
However i think that : All OO code should have their dependencies injected. is out of scope.

Status:Needs review» Needs work

We don't really want \Drupal::service() calls in our OO code at all because it couples the class to the global Drupal class.

We should either fix the OO stuff here or remove the OO changes (outside of tests) in this patch and create a follow up to do so.

Let's do it here, please :-)
@jlbellido: if you need help ping me on IRC.

Issue tags:+Needs reroll
  1. @@ -27,7 +27,7 @@ public function form($form, &$form_state) {
    -      $manager = drupal_container()->get($plugin_definition['manager']);
    +      $manager = \Drupal::service($plugin_definition['manager']);

    no more

  2. @@ -963,7 +963,6 @@ function system_menu() {
    -

    out of scope

StatusFileSize
new8.24 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch replace-drupal-container-with-drupal-service-2014017-49.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
new4.28 KB

Rerolled pairing phisically with @jlbellido, still needs DI.

Status:Needs work» Needs review

Test!

DI was only needed in the already removed class, so this should be ready for review.

Issue tags:-Novice, -Needs reroll+WSCCI

Clearing tags and tagging with WSCCI.

Status:Needs review» Reviewed & tested by the community

I don't think this is strictly WSCCI, but it's RTBC either way. :-)

Status:Reviewed & tested by the community» Needs work
Issue tags:+WSCCI

The last submitted patch, replace-drupal-container-with-drupal-service-2014017-49.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new7.55 KB
PASSED: [[SimpleTest]]: [MySQL] 58,347 pass(es).
[ View ]

Rerolled #49 and running tests

Status:Needs review» Reviewed & tested by the community

Perfect

Status:Reviewed & tested by the community» Needs work

+++ b/core/modules/system/lib/Drupal/system/Tests/Path/UrlAlterFunctionalTest.php
@@ -112,7 +112,7 @@ protected function assertUrlOutboundAlter($original, $final) {
    * @param $original
    *   A string with the aliased or un-normal path that is run through
-   *   drupal_container()->get('path.alias_manager')->getSystemPath().
+   *   $this->container->get('path.alias_manager')->getSystemPath($original).

Lets fix this comment. It is not helpful. How about

    * @param string $original
    *   The original path before it has been altered by inbound URL processing.

Status:Needs work» Needs review
StatusFileSize
new859 bytes
new7.61 KB
PASSED: [[SimpleTest]]: [MySQL] 58,696 pass(es).
[ View ]

Updated the comment in core/modules/system/lib/Drupal/system/Tests/Path/UrlAlterFunctionalTest.php with Alexpott's suggestion.

Thanks Alexpott! :D

Status:Needs review» Reviewed & tested by the community

Back over to you, Alex.

Status:Reviewed & tested by the community» Needs work
Issue tags:+Needs reroll

Patch no longer applies.

Status:Needs work» Needs review
StatusFileSize
new7.71 KB
PASSED: [[SimpleTest]]: [MySQL] 59,234 pass(es).
[ View ]

Relloled #59

Status:Needs review» Needs work

+++ b/core/modules/system/system.module
@@ -2201,7 +2201,7 @@ function system_page_build(&$page) {
+    $request = Drupal::request();
+++ b/core/modules/system/tests/modules/design_test/design_test.module
@@ -95,8 +95,8 @@ function design_test_menu_local_tasks_alter(&$data, $router_item, $root_path) {
-    $default_theme = \Drupal::config('system.theme')->get('default');
+    $selected_theme = Drupal::request()->query->get('theme');
+    $default_theme = Drupal::config('system.theme')->get('default');
@@ -129,5 +129,5 @@ function design_test_menu_local_tasks_alter(&$data, $router_item, $root_path) {
-  return drupal_container()->get('request')->query->get('theme');
+  return Drupal::request()->query->get('theme');

This should be \Drupal instead

Status:Needs work» Needs review
StatusFileSize
new1.86 KB
new7.62 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch replace-drupal-container-with-drupal-service-2014017-64.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Updated with suggested changes of dawehner , thanks!

Status:Needs review» Needs work

The last submitted patch, replace-drupal-container-with-drupal-service-2014017-64.patch, failed testing.

Status:Needs work» Needs review

Retesting

Status:Needs review» Reviewed & tested by the community

And back up.

Issue tags:-Needs reroll

Tags

Status:Reviewed & tested by the community» Needs work
Issue tags:+WSCCI

The last submitted patch, replace-drupal-container-with-drupal-service-2014017-64.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new6.42 KB
PASSED: [[SimpleTest]]: [MySQL] 58,568 pass(es).
[ View ]

Rerolled without the core/modules/system/tests/modules/design_test/design_test.module part as this module has now been deleted from core #2037569: Remove design_test module

Status:Needs review» Reviewed & tested by the community

Sigh.

Status:Reviewed & tested by the community» Fixed

I think there's an emerging standard to use \Drupal instead of $this->container in simpletest, but not sure if that's documented yet - at least this makes it easier to find all the $this->container so I went ahead and committed/pushed to 8.x, thanks!

I think there's an emerging standard to use \Drupal instead of $this->container in simpletest, but not sure if that's documented yet - at least this makes it easier to find all the $this->container so I went ahead and committed/pushed to 8.x, thanks!

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