From 738ca487ac7ebc037d9bff9d2f5b9b417797df26 Mon Sep 17 00:00:00 2001
From: Guy Sartorelli <36352093+GuySartorelli@users.noreply.github.com>
Date: Wed, 1 Feb 2023 16:05:54 +1300
Subject: [PATCH] FIX Allow public extension getter methods to work (#10676)

Accidentally broke this in #10670
---
 src/Core/CustomMethods.php                    | 10 +++++++-
 src/View/ViewableData.php                     |  5 +++-
 tests/php/View/ViewableDataTest.php           | 20 ++++++++++++++--
 .../ViewableDataTestObject.php                |  2 +-
 .../ViewableDataTextExtension.php             | 24 +++++++++++++++++++
 5 files changed, 56 insertions(+), 5 deletions(-)
 rename tests/php/View/{ => ViewableDataTest}/ViewableDataTestObject.php (92%)
 create mode 100644 tests/php/View/ViewableDataTest/ViewableDataTextExtension.php

diff --git a/src/Core/CustomMethods.php b/src/Core/CustomMethods.php
index ed247b333..465cb5b07 100644
--- a/src/Core/CustomMethods.php
+++ b/src/Core/CustomMethods.php
@@ -147,7 +147,15 @@ trait CustomMethods
      */
     public function hasMethod($method)
     {
-        return method_exists($this, $method ?? '') || $this->getExtraMethodConfig($method);
+        return method_exists($this, $method ?? '') || $this->hasCustomMethod($method);
+    }
+
+    /**
+     * Determines if a custom method with this name is defined.
+     */
+    protected function hasCustomMethod($method): bool
+    {
+        return $this->getExtraMethodConfig($method) !== null;
     }
 
     /**
diff --git a/src/View/ViewableData.php b/src/View/ViewableData.php
index 9dcea9267..77d25ae4f 100644
--- a/src/View/ViewableData.php
+++ b/src/View/ViewableData.php
@@ -247,11 +247,14 @@ class ViewableData implements IteratorAggregate
     private function isAccessibleMethod(string $method): bool
     {
         if (!method_exists($this, $method)) {
-            return false;
+            // Methods added via extensions are accessible
+            return $this->hasCustomMethod($method);
         }
+        // All methods defined on ViewableData are accessible to ViewableData
         if (static::class === self::class) {
             return true;
         }
+        // Private methods defined on subclasses are not accessible to ViewableData
         $reflectionMethod = new ReflectionMethod($this, $method);
         return !$reflectionMethod->isPrivate();
     }
diff --git a/tests/php/View/ViewableDataTest.php b/tests/php/View/ViewableDataTest.php
index c536b9ad1..5ddfb0399 100644
--- a/tests/php/View/ViewableDataTest.php
+++ b/tests/php/View/ViewableDataTest.php
@@ -7,8 +7,9 @@ use SilverStripe\ORM\FieldType\DBField;
 use SilverStripe\Dev\SapphireTest;
 use SilverStripe\View\ArrayData;
 use SilverStripe\View\SSViewer;
+use SilverStripe\View\Tests\ViewableDataTest\ViewableDataTestExtension;
+use SilverStripe\View\Tests\ViewableDataTest\ViewableDataTestObject;
 use SilverStripe\View\ViewableData;
-use SilverStripe\View\Tests\ViewableDataTestObject;
 
 /**
  * See {@link SSViewerTest->testCastingHelpers()} for more tests related to casting and ViewableData behaviour,
@@ -16,6 +17,11 @@ use SilverStripe\View\Tests\ViewableDataTestObject;
  */
 class ViewableDataTest extends SapphireTest
 {
+    protected static $required_extensions = [
+        ViewableDataTestObject::class => [
+            ViewableDataTestExtension::class,
+        ],
+    ];
 
     public function testCasting()
     {
@@ -213,6 +219,7 @@ class ViewableDataTest extends SapphireTest
         $reflectionMethod = new ReflectionMethod(ViewableData::class, 'isAccessibleMethod');
         $reflectionMethod->setAccessible(true);
         $object = new ViewableDataTestObject();
+        $viewableData = new ViewableData();
 
         $output = $reflectionMethod->invokeArgs($object, ['privateMethod']);
         $this->assertFalse($output, 'Method should not be accessible');
@@ -226,8 +233,17 @@ class ViewableDataTest extends SapphireTest
         $output = $reflectionMethod->invokeArgs($object, ['missingMethod']);
         $this->assertFalse($output, 'Method should not be accessible');
 
-        $output = $reflectionMethod->invokeArgs(new ViewableData(), ['isAccessibleProperty']);
+        $output = $reflectionMethod->invokeArgs($viewableData, ['isAccessibleProperty']);
         $this->assertTrue($output, 'Method should be accessible');
+
+        $output = $reflectionMethod->invokeArgs($object, ['publicMethodFromExtension']);
+        $this->assertTrue($output, 'Method should be accessible');
+
+        $output = $reflectionMethod->invokeArgs($object, ['protectedMethodFromExtension']);
+        $this->assertFalse($output, 'Method should not be accessible');
+
+        $output = $reflectionMethod->invokeArgs($object, ['privateMethodFromExtension']);
+        $this->assertFalse($output, 'Method should not be accessible');
     }
 
     public function testIsAccessibleProperty()
diff --git a/tests/php/View/ViewableDataTestObject.php b/tests/php/View/ViewableDataTest/ViewableDataTestObject.php
similarity index 92%
rename from tests/php/View/ViewableDataTestObject.php
rename to tests/php/View/ViewableDataTest/ViewableDataTestObject.php
index 90878d80f..9aa422a04 100644
--- a/tests/php/View/ViewableDataTestObject.php
+++ b/tests/php/View/ViewableDataTest/ViewableDataTestObject.php
@@ -1,6 +1,6 @@
 <?php
 
-namespace SilverStripe\View\Tests;
+namespace SilverStripe\View\Tests\ViewableDataTest;
 
 use SilverStripe\Dev\TestOnly;
 use SilverStripe\ORM\DataObject;
diff --git a/tests/php/View/ViewableDataTest/ViewableDataTextExtension.php b/tests/php/View/ViewableDataTest/ViewableDataTextExtension.php
new file mode 100644
index 000000000..edeb27d32
--- /dev/null
+++ b/tests/php/View/ViewableDataTest/ViewableDataTextExtension.php
@@ -0,0 +1,24 @@
+<?php
+
+namespace SilverStripe\View\Tests\ViewableDataTest;
+
+use SilverStripe\Core\Extension;
+use SilverStripe\Dev\TestOnly;
+
+class ViewableDataTestExtension extends Extension implements TestOnly
+{
+    private function privateMethodFromExtension(): string
+    {
+        return 'Private function';
+    }
+
+    protected function protectedMethodFromExtension(): string
+    {
+        return 'Protected function';
+    }
+
+    public function publicMethodFromExtension(): string
+    {
+        return 'Public function';
+    }
+}