Allow ViewExecutable to be pluggable [#1898926]

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.

Comment	File	Size	Author
#7	1898926-7.patch	4.79 KB	damiankloip
#7
#7	interdiff.txt	1.17 KB	damiankloip
#6	1898926-6.patch	4.8 KB	damiankloip
#6
#6	interdiff.txt	1.23 KB	damiankloip
#5	1898926-5.patch	3.56 KB	damiankloip
#5
#3	1898926-3.patch	3.56 KB	damiankloip
#3
#3	interdiff.txt	522 bytes	damiankloip
	d8.view-exec-factory-service.patch	3.75 KB	damiankloip

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett commented 25 January 2013 at 15:46

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.

Comment #2

25 January 2013 at 16:02

Status:

Needs review

» Needs work

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

Comment #3

damiankloip CreditAttribution: damiankloip commented 25 January 2013 at 16:15

Status:

Needs work

» Needs review

File	Size
interdiff.txt	522 bytes
1898926-3.patch	3.56 KB

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

Comment #4

dawehner

German

CreditAttribution: dawehner commented 25 January 2013 at 16:33

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

Nitpick: Missing "\"

Comment #5

damiankloip CreditAttribution: damiankloip commented 25 January 2013 at 16:38

File	Size
1898926-5.patch	3.56 KB

Damn you, dawehner ;)

Comment #6

damiankloip CreditAttribution: damiankloip commented 25 January 2013 at 16:57

File	Size
interdiff.txt	1.23 KB
1898926-6.patch	4.8 KB

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

Comment #7

damiankloip CreditAttribution: damiankloip commented 25 January 2013 at 17:03

File	Size
interdiff.txt	1.17 KB
1898926-7.patch	4.79 KB

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

Comment #8

dawehner

German

CreditAttribution: dawehner commented 28 January 2013 at 13:29

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 :)