From 429ce557561f7d4fd648b73cb83d12c1f424cead Mon Sep 17 00:00:00 2001
From: Loz Calver <lozcalver@bigfork.co.uk>
Date: Fri, 3 Jun 2016 17:04:22 +0100
Subject: [PATCH] FIX: ViewableData::setFailover() didn't remove cached methods

---
 core/Object.php                 | 42 +++++++++++++++++++++++++++++++++
 tests/view/ViewableDataTest.php |  5 ++++
 view/ViewableData.php           |  5 ++++
 3 files changed, 52 insertions(+)

diff --git a/core/Object.php b/core/Object.php
index 4f3f990b1..1762011b7 100755
--- a/core/Object.php
+++ b/core/Object.php
@@ -845,6 +845,48 @@ abstract class Object {
 		}
 	}
 
+	/**
+	 * Add all the methods from an object property (which is an {@link Extension}) to this object.
+	 *
+	 * @param string $property the property name
+	 * @param string|int $index an index to use if the property is an array
+	 */
+	protected function removeMethodsFrom($property, $index = null) {
+		$extension = ($index !== null) ? $this->{$property}[$index] : $this->$property;
+
+		if(!$extension) {
+			throw new InvalidArgumentException (
+				"Object->removeMethodsFrom(): could not remove methods from {$this->class}->{$property}[$index]"
+			);
+		}
+
+		if(method_exists($extension, 'allMethodNames')) {
+			if ($extension instanceof Extension) $extension->setOwner($this);
+			$methods = $extension->allMethodNames(true);
+			if ($extension instanceof Extension) $extension->clearOwner();
+
+		} else {
+			if(!isset(self::$built_in_methods[$extension->class])) {
+				self::$built_in_methods[$extension->class] = array_map('strtolower', get_class_methods($extension));
+			}
+			$methods = self::$built_in_methods[$extension->class];
+		}
+
+		if($methods) {
+			foreach ($methods as $method) {
+				$methodInfo = self::$extra_methods[$this->class][$method];
+
+				if ($methodInfo['property'] === $property && $methodInfo['index'] === $index) {
+					unset(self::$extra_methods[$this->class][$method]);
+				}
+			}
+
+			if (empty(self::$extra_methods[$this->class])) {
+				unset(self::$extra_methods[$this->class]);
+			}
+		}
+	}
+
 	/**
 	 * Add a wrapper method - a method which points to another method with a different name. For example, Thumbnail(x)
 	 * can be wrapped to generateThumbnail(x)
diff --git a/tests/view/ViewableDataTest.php b/tests/view/ViewableDataTest.php
index 1e153fd13..7c85f6c6d 100644
--- a/tests/view/ViewableDataTest.php
+++ b/tests/view/ViewableDataTest.php
@@ -187,6 +187,11 @@ class ViewableDataTest extends SapphireTest {
 		// Ensure that defined methods detected from the failover aren't cached when setting a new failover
 		$container->setFailover(new ViewableDataTest_Failover);
 		$this->assertTrue($container->hasMethod('testMethod'));
+
+		// Test the reverse - that defined methods previously detected in a failover are removed if they no longer exist
+		$container->setFailover($failover);
+		$this->assertSame($failover, $container->getFailover(), 'getFailover() returned a different object');
+		$this->assertFalse($container->hasMethod('testMethod'), 'testMethod() incorrectly reported as existing');
 	}
 
 }
diff --git a/view/ViewableData.php b/view/ViewableData.php
index c65e88d26..59adb7eb7 100644
--- a/view/ViewableData.php
+++ b/view/ViewableData.php
@@ -134,6 +134,11 @@ class ViewableData extends Object implements IteratorAggregate {
 	 * @param ViewableData $failover
 	 */
 	public function setFailover(ViewableData $failover) {
+		// Ensure cached methods from previous failover are removed
+		if ($this->failover) {
+			$this->removeMethodsFrom('failover');
+		}
+
 		$this->failover = $failover;
 		$this->defineMethods();
 	}