We can register a views.exectuable service in the container that uses a primitive factory class to return the a ViewExecutable instance. This could then be easily swapped out if people want to do that. For example, you could create a wrapper around the current ViewExectuble class to monitor and collect more performance data.

CommentFileSizeAuthor
#7 1898926-7.patch4.79 KBdamiankloip
#7 interdiff.txt1.17 KBdamiankloip
#6 1898926-6.patch4.8 KBdamiankloip
#6 interdiff.txt1.23 KBdamiankloip
#5 1898926-5.patch3.56 KBdamiankloip
#3 1898926-3.patch3.56 KBdamiankloip
#3 interdiff.txt522 bytesdamiankloip
d8.view-exec-factory-service.patch3.75 KBdamiankloip

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

This is a great idea. It's nice to get rid of that unnecessary hard dependency, and I can imagine this being useful in custom code.

Status: Needs review » Needs work

The last submitted patch, d8.view-exec-factory-service.patch, failed testing.

damiankloip’s picture

damiankloip commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt522 bytes
new 1898926-3.patch3.56 KB

Oops, thought I added that back in, must have forgot to save!

dawehner’s picture

dawehner
Primary language German
commented 
+++ b/core/modules/views/lib/Drupal/views/ViewExecutableFactory.phpundefined
@@ -0,0 +1,30 @@
+   * @return Drupal\views\ViewExecutable

Nitpick: Missing "\"

damiankloip’s picture

damiankloip commented
StatusFileSize
new 1898926-5.patch3.56 KB

Damn you, dawehner ;)

damiankloip’s picture

damiankloip commented
StatusFileSize
new interdiff.txt1.23 KB
new 1898926-6.patch4.8 KB

With a couple of unit tests too. What do you think?

damiankloip’s picture

damiankloip commented
StatusFileSize
new interdiff.txt1.17 KB
new 1898926-7.patch4.79 KB

What a noisy issue, sorry. Now using $this->container in the test and fixed the typos.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community 
+++ b/core/modules/views/lib/Drupal/views/Tests/ViewExecutableTest.phpundefined
@@ -86,10 +86,10 @@ protected function setUp() {
-    $this->assertTrue($factory instanceof ViewExecutableFactory, 'An ViewExecutableFactory instance was returned from the container.');
...
+    $this->assertTrue($factory instanceof ViewExecutableFactory, 'A ViewExecutableFactory instance was returned from the container.');
...
-    $this->assertTrue($factory->get($view) instanceof ViewExecutable, 'An ViewExecutable instance was returned from the factory.');
+    $this->assertTrue($factory->get($view) instanceof ViewExecutable, 'A ViewExecutable instance was returned from the factory.');

I trust english people here, even they have some disagreements with the americans about certain aspects of the language :)

+++ b/core/modules/views/lib/Drupal/views/ViewsBundle.phpundefined
@@ -35,6 +35,8 @@ public function build(ContainerBuilder $container) {
+    $container->register('views.executable', 'Drupal\views\ViewExecutableFactory');

There should be some kind of naming standard for factories in the container (config.factory vs. queue) but yeah this is no critique on that patch.

+++ b/core/modules/views/lib/Drupal/views/Tests/ViewExecutableTest.phpundefined
@@ -86,10 +86,10 @@ protected function setUp() {
-    $this->assertTrue($factory instanceof ViewExecutableFactory, 'An ViewExecutableFactory instance was returned from the container.');
...
+    $this->assertTrue($factory instanceof ViewExecutableFactory, 'A ViewExecutableFactory instance was returned from the container.');
...
-    $this->assertTrue($factory->get($view) instanceof ViewExecutable, 'An ViewExecutable instance was returned from the factory.');
+    $this->assertTrue($factory->get($view) instanceof ViewExecutable, 'A ViewExecutable instance was returned from the factory.');

I trust english people here, even they have some disagreements with the americans about certain aspects of the language :)

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

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