Merge pull request #3 from ianaldrighetti/1.0.1

Issue #2 - LMMVC should throw PageNotFoundException when a method is private, protected or static.

LmMvc/Application.php

@@ -391,7 +391,8 @@ public function fixGetGlobal($queryString)
     }
 
     /**
-     * Returns a ReflectionMethod if the method exists in the controller.
+     * Returns a ReflectionMethod if the method exists (and is accessible [it must be a public method, it cannot be
+     * private, protected or static]) in the controller.
      *
      * @param BaseController $controller
      * @param string $methodName
@@ -402,19 +403,21 @@ public function getMethodObject(BaseController $controller, $methodName)
     {
         $reflector = new \ReflectionClass($controller);
 
+        $methodObject = null;
         try
         {
             $methodObject = $reflector->getMethod($methodName);
         }
         catch (\ReflectionException $ex)
         {
-            throw new PageNotFoundException(
-                sprintf(
-                    'The method "%s" was not found in the "%s" controller.',
-                    htmlspecialchars($methodName),
-                    htmlspecialchars(get_class($controller))
-                )
-            );
+            $this->throwPageNotFoundMethodException($controller, $methodName);
+        }
+
+        // If the method is not public (or if it is static), we're going to throw a PageNotFoundException as well.
+        if (!$methodObject->isPublic() || $methodObject->isStatic())
+        {
+            // We're going to pretend it doesn't exist at all.
+            $this->throwPageNotFoundMethodException($controller, $methodName);
         }
 
         return $methodObject;
@@ -705,4 +708,23 @@ public function cleanMethodName($methodName)
         // Remove the /.
         return substr($methodName, 0, strpos($methodName, '/'));
     }
+
+    /**
+     * Throws a PageNotFoundException with a message indicating that the method does not exist within the specified
+     * controller.
+     *
+     * @param BaseController $controller
+     * @param string $methodName
+     * @throws Exception\PageNotFoundException
+     */
+    private function throwPageNotFoundMethodException(BaseController $controller, $methodName)
+    {
+        throw new PageNotFoundException(
+            sprintf(
+                'The method "%s" was not found in the "%s" controller.',
+                htmlspecialchars($methodName),
+                htmlspecialchars(get_class($controller))
+            )
+        );
+    }
 }

LmMvcTests/ApplicationTest.php

@@ -598,5 +598,38 @@ public function testRunShowExceptionPageExternal()
         $this->assertInstanceOf('\\Exception', $mockExceptionHandler->getException());
         $this->assertFalse($mockExceptionHandler->getInternal());
     }
+
+    /**
+     * Tests to ensure that a PageNotFoundException is thrown when a method that is not accessible (not public) is
+     * attempted to be accessed.
+     *
+     * @param string $methodName The name of the method in the MockController.
+     * @dataProvider notAccessibleMethodProvider
+     */
+    public function testNotAccessibleMethodTypes($methodName)
+    {
+        $mockController = new MockController();
+
+        // We're expecting a PageNotFoundException with a specific message.
+        $this->setExpectedException(
+            '\LmMvc\Exception\PageNotFoundException',
+            sprintf('The method "%s" was not found in the "%s" controller.', $methodName, get_class($mockController))
+        );
+
+        // This should throw an Exception.
+        $this->application->getMethodObject($mockController, $methodName);
+    }
+
+    /**
+     * @return array
+     */
+    public function notAccessibleMethodProvider()
+    {
+        return array(
+            array('privateMethod'),
+            array('protectedMethod'),
+            array('staticMethod')
+        );
+    }
 }
 

LmMvcTests/Mock/MockController.php

@@ -80,4 +80,28 @@ public function throwException()
     {
         throw new \Exception('Just an Exception...');
     }
+
+    /**
+     * A private method for testing.
+     */
+    private function privateMethod()
+    {
+        // This is a private method and should cause LMMVC to throw a PageNotFoundException.
+    }
+
+    /**
+     * A protected method for testing.
+     */
+    protected function protectedMethod()
+    {
+        // The same goes for protected methods (should throw PageNotFoundException).
+    }
+
+    /**
+     * A public static method for testing.
+     */
+    public static function staticMethod()
+    {
+        // Just like with private and protected method, static methods shouldn't be callable either.
+    }
 }