Merge pull request #242 from tractorcow/pulls/validate-extensions

API Add tests for incorrectly applied extensions
tractorcow-farm · Nov 24, 2016 · 9d3c554 · 9d3c554
2 parents ab23171 + 26371d7
commit 9d3c554
Show file tree

Hide file tree

Showing 3 changed files with 100 additions and 0 deletions.
diff --git a/code/tasks/FluentValidateTask.php b/code/tasks/FluentValidateTask.php
@@ -45,6 +45,7 @@ public function validateConfig(Config $config = null)
 
         $this->validateLocales($config, $result);
         $this->validateDomains($config, $result);
+        $this->validateExtensions($config, $result);
 
         return $result;
     }
@@ -188,4 +189,59 @@ protected function validateDomains(Config $config, ValidationResult $result)
             }
         }
     }
+
+    /**
+     * @param Config $config
+     * @param ValidationResult $result
+     */
+    protected function validateExtensions(Config $config, $result)
+    {
+        // Check all classes
+        $dataClasses = ClassInfo::subclassesFor('DataObject');
+		array_shift($dataClasses);
+
+        foreach($dataClasses as $class) {
+            // If non-base class, fail if any fluent extension provided
+            $isBase = ClassInfo::baseDataClass($class) === $class;
+            $extensions = $this->getDirectExtensions($config, $class);
+            if ($isBase) {
+                // Base classes must have no more than 1 fluent extension applied
+                if (count($extensions) > 1) {
+                    $result->error(sprintf(
+                        "Class %s has multiple FluentExtension classes: %s",
+                        $class,
+                        implode(', ', $extensions)
+                    ));
+                }
+            } else {
+                // Non-base classes may not have this extension applied directly at all
+                if ($extensions) {
+                    $result->error(sprintf(
+                        "Class %s is not a base data class but has the following FluentExtensions: %s",
+                        $class,
+                        implode(', ', $extensions)
+                    ));
+                }
+            }
+        }
+    }
+
+    /**
+     * Gets all extensions directly on this class that extend FluentExtension
+     *
+     * @param Config $config
+     * @param string $class
+     * @return array
+     */
+    protected function getDirectExtensions($config, $class) {
+        $extensions = $config->get($class, 'extensions', Config::UNINHERITED);
+		$found = array();
+        if($extensions) foreach($extensions as $extension) {
+            $extensionClass = ClassInfo::class_name(Extension::get_classname_without_arguments($extension));
+            if ($extensionClass === 'FluentExtension' || is_subclass_of($extensionClass, 'FluentExtension')) {
+                $found[] = $extensionClass;
+            }
+        }
+        return $found;
+    }
 }
diff --git a/docs/en/configuration.md b/docs/en/configuration.md
@@ -16,6 +16,7 @@ there are any errors in your config. Examples of errors could include:
  * Invalid default locales
  * Duplicate locales in multiple domains
  * Locales missing from domains
+ * Incorrectly applied extensions
 
 To run this task you can run this on the CLI.
 

diff --git a/tests/FluentValidateTaskTest.php b/tests/FluentValidateTaskTest.php
@@ -0,0 +1,43 @@
+<?php
+
+class FluentValidateTaskTest extends SapphireTest
+{
+    public function setUp()
+    {
+        parent::setUp();
+
+        // Tweak configuration
+        Config::inst()->update('Fluent', 'locales', array('fr_CA', 'en_NZ', 'en_US', 'es_ES'));
+    }
+
+    public function testNonBaseClass() {
+        $config = Config::inst();
+        $config->update('Page', 'extensions', array('FluentExtension'));
+        $validator = new FluentValidateTask();
+        $result = $validator->validateConfig($config);
+        $this->assertFalse($result->valid());
+        $this->assertContains(
+            'Class Page is not a base data class but has the following FluentExtensions: FluentExtension',
+            $result->messageList()
+        );
+    }
+
+    public function testMultipleExtensions() {
+        $config = Config::inst();
+        $config->update('SiteTree', 'extensions', array('FluentExtension'));
+        $validator = new FluentValidateTask();
+        $result = $validator->validateConfig($config);
+        $this->assertFalse($result->valid());
+        $this->assertContains(
+            'Class SiteTree has multiple FluentExtension classes: FluentExtension, FluentSiteTree',
+            $result->messageList()
+        );
+    }
+
+    public function testDefaultConfigValid() {
+        $config = Config::inst();
+        $validator = new FluentValidateTask();
+        $result = $validator->validateConfig($config);
+        $this->assertTrue($result->valid());
+    }
+}