From ddf2ef5cc938ece538f5f2da71feb3dbeea9569b Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Fri, 14 Nov 2014 14:59:12 +0100 Subject: [PATCH] Remove $request parameter from Form::onSuccess and Form::onRequest fixes #7552 --- .../Config/Authentication/DbBackendForm.php | 3 +- .../Config/Authentication/LdapBackendForm.php | 3 +- .../AuthenticationBackendConfigForm.php | 9 ++- .../AuthenticationBackendReorderForm.php | 18 +++-- .../forms/Config/GeneralConfigForm.php | 5 +- .../forms/Config/Resource/DbResourceForm.php | 3 +- .../Config/Resource/LdapResourceForm.php | 3 +- .../Resource/LivestatusResourceForm.php | 3 +- .../forms/Config/ResourceConfigForm.php | 9 ++- application/forms/PreferenceForm.php | 6 +- library/Icinga/Web/Form.php | 65 +++++++++++-------- library/Icinga/Web/Wizard.php | 21 +++++- .../DisableNotificationsExpireCommandForm.php | 5 +- .../ToggleInstanceFeaturesCommandForm.php | 5 +- .../Object/AcknowledgeProblemCommandForm.php | 7 +- .../Command/Object/AddCommentCommandForm.php | 7 +- .../Command/Object/CheckNowCommandForm.php | 5 +- .../Object/DeleteCommentCommandForm.php | 5 +- .../Object/DeleteDowntimeCommandForm.php | 5 +- .../RemoveAcknowledgementCommandForm.php | 5 +- .../Object/ScheduleHostCheckCommandForm.php | 5 +- .../ScheduleHostDowntimeCommandForm.php | 6 +- .../ScheduleServiceCheckCommandForm.php | 4 +- .../ScheduleServiceDowntimeCommandForm.php | 4 +- .../ToggleObjectFeaturesCommandForm.php | 5 +- .../forms/Config/BackendConfigForm.php | 9 ++- .../forms/Config/InstanceConfigForm.php | 9 ++- .../forms/Config/SecurityConfigForm.php | 5 +- .../setup/application/forms/ModulePage.php | 5 +- test/php/library/Icinga/Web/FormTest.php | 22 +------ 30 files changed, 130 insertions(+), 136 deletions(-) diff --git a/application/forms/Config/Authentication/DbBackendForm.php b/application/forms/Config/Authentication/DbBackendForm.php index 14570b0ae..4cf015db8 100644 --- a/application/forms/Config/Authentication/DbBackendForm.php +++ b/application/forms/Config/Authentication/DbBackendForm.php @@ -7,7 +7,6 @@ namespace Icinga\Forms\Config\Authentication; use Exception; use Icinga\Application\Config; use Icinga\Web\Form; -use Icinga\Web\Request; use Icinga\Data\ResourceFactory; use Icinga\Authentication\Backend\DbUserBackend; @@ -89,7 +88,7 @@ class DbBackendForm extends Form * * @see Form::onSuccess() */ - public function onSuccess(Request $request) + public function onSuccess() { if (false === static::isValidAuthenticationBackend($this)) { return false; diff --git a/application/forms/Config/Authentication/LdapBackendForm.php b/application/forms/Config/Authentication/LdapBackendForm.php index b865395d9..3e5c2b305 100644 --- a/application/forms/Config/Authentication/LdapBackendForm.php +++ b/application/forms/Config/Authentication/LdapBackendForm.php @@ -7,7 +7,6 @@ namespace Icinga\Forms\Config\Authentication; use Exception; use Icinga\Application\Config; use Icinga\Web\Form; -use Icinga\Web\Request; use Icinga\Data\ResourceFactory; use Icinga\Exception\AuthenticationException; use Icinga\Authentication\Backend\LdapUserBackend; @@ -119,7 +118,7 @@ class LdapBackendForm extends Form * * @see Form::onSuccess() */ - public function onSuccess(Request $request) + public function onSuccess() { if (false === static::isValidAuthenticationBackend($this)) { return false; diff --git a/application/forms/Config/AuthenticationBackendConfigForm.php b/application/forms/Config/AuthenticationBackendConfigForm.php index 8038b5347..c21ccf8f2 100644 --- a/application/forms/Config/AuthenticationBackendConfigForm.php +++ b/application/forms/Config/AuthenticationBackendConfigForm.php @@ -5,7 +5,6 @@ namespace Icinga\Forms\Config; use InvalidArgumentException; -use Icinga\Web\Request; use Icinga\Forms\ConfigForm; use Icinga\Web\Notification; use Icinga\Application\Config; @@ -192,7 +191,7 @@ class AuthenticationBackendConfigForm extends ConfigForm * * @see Form::onSuccess() */ - public function onSuccess(Request $request) + public function onSuccess() { if (($el = $this->getElement('force_creation')) === null || false === $el->isChecked()) { $backendForm = $this->getBackendForm($this->getElement('type')->getValue()); @@ -202,7 +201,7 @@ class AuthenticationBackendConfigForm extends ConfigForm } } - $authBackend = $request->getQuery('auth_backend'); + $authBackend = $this->request->getQuery('auth_backend'); try { if ($authBackend === null) { // create new backend $this->add($this->getValues()); @@ -230,9 +229,9 @@ class AuthenticationBackendConfigForm extends ConfigForm * * @throws ConfigurationError In case the backend name is missing in the request or is invalid */ - public function onRequest(Request $request) + public function onRequest() { - $authBackend = $request->getQuery('auth_backend'); + $authBackend = $this->request->getQuery('auth_backend'); if ($authBackend !== null) { if ($authBackend === '') { throw new ConfigurationError(t('Authentication backend name missing')); diff --git a/application/forms/Config/AuthenticationBackendReorderForm.php b/application/forms/Config/AuthenticationBackendReorderForm.php index 90eeb6b90..07200827a 100644 --- a/application/forms/Config/AuthenticationBackendReorderForm.php +++ b/application/forms/Config/AuthenticationBackendReorderForm.php @@ -5,7 +5,6 @@ namespace Icinga\Forms\Config; use InvalidArgumentException; -use Icinga\Web\Request; use Icinga\Web\Notification; use Icinga\Forms\ConfigForm; @@ -30,17 +29,26 @@ class AuthenticationBackendReorderForm extends ConfigForm return $this->config->keys(); } + /** + * @see Form::createElements() + */ + public function createElements(array $formData) + { + // This adds just a dummy element to be able to utilize Form::getValue as part of onSuccess() + $this->addElement('hidden', 'backend_newpos'); + } + /** * Update the authentication backend order and save the configuration * * @see Form::onSuccess() */ - public function onSuccess(Request $request) + public function onSuccess() { - $formData = $this->getRequestData($request); - if (isset($formData['backend_newpos'])) { + $newPosData = $this->getValue('backend_newpos'); + if ($newPosData) { $configForm = $this->getConfigForm(); - list($backendName, $position) = explode('|', $formData['backend_newpos'], 2); + list($backendName, $position) = explode('|', $newPosData, 2); try { if ($configForm->move($backendName, $position)->save()) { diff --git a/application/forms/Config/GeneralConfigForm.php b/application/forms/Config/GeneralConfigForm.php index 70d131695..2cb72e3a1 100644 --- a/application/forms/Config/GeneralConfigForm.php +++ b/application/forms/Config/GeneralConfigForm.php @@ -4,7 +4,6 @@ namespace Icinga\Forms\Config; -use Icinga\Web\Request; use Icinga\Web\Notification; use Icinga\Forms\ConfigForm; use Icinga\Forms\Config\General\LoggingConfigForm; @@ -38,7 +37,7 @@ class GeneralConfigForm extends ConfigForm /** * @see Form::onSuccess() */ - public function onSuccess(Request $request) + public function onSuccess() { $sections = array(); foreach ($this->getValues() as $sectionAndPropertyName => $value) { @@ -62,7 +61,7 @@ class GeneralConfigForm extends ConfigForm /** * @see Form::onRequest() */ - public function onRequest(Request $request) + public function onRequest() { $values = array(); foreach ($this->config as $section => $properties) { diff --git a/application/forms/Config/Resource/DbResourceForm.php b/application/forms/Config/Resource/DbResourceForm.php index 46339fc6e..b1d54c704 100644 --- a/application/forms/Config/Resource/DbResourceForm.php +++ b/application/forms/Config/Resource/DbResourceForm.php @@ -7,7 +7,6 @@ namespace Icinga\Forms\Config\Resource; use Exception; use Icinga\Application\Config; use Icinga\Web\Form; -use Icinga\Web\Request; use Icinga\Data\ResourceFactory; use Icinga\Application\Platform; @@ -113,7 +112,7 @@ class DbResourceForm extends Form * * @see Form::onSuccess() */ - public function onSuccess(Request $request) + public function onSuccess() { if (false === static::isValidResource($this)) { return false; diff --git a/application/forms/Config/Resource/LdapResourceForm.php b/application/forms/Config/Resource/LdapResourceForm.php index 01dd16e80..6c29a09d6 100644 --- a/application/forms/Config/Resource/LdapResourceForm.php +++ b/application/forms/Config/Resource/LdapResourceForm.php @@ -7,7 +7,6 @@ namespace Icinga\Forms\Config\Resource; use Exception; use Icinga\Application\Config; use Icinga\Web\Form; -use Icinga\Web\Request; use Icinga\Data\ResourceFactory; /** @@ -94,7 +93,7 @@ class LdapResourceForm extends Form * * @see Form::onSuccess() */ - public function onSuccess(Request $request) + public function onSuccess() { if (false === static::isValidResource($this)) { return false; diff --git a/application/forms/Config/Resource/LivestatusResourceForm.php b/application/forms/Config/Resource/LivestatusResourceForm.php index 5766caaec..e4dae844b 100644 --- a/application/forms/Config/Resource/LivestatusResourceForm.php +++ b/application/forms/Config/Resource/LivestatusResourceForm.php @@ -7,7 +7,6 @@ namespace Icinga\Forms\Config\Resource; use Exception; use Icinga\Application\Config; use Icinga\Web\Form; -use Icinga\Web\Request; use Icinga\Application\Icinga; use Icinga\Data\ResourceFactory; @@ -57,7 +56,7 @@ class LivestatusResourceForm extends Form * * @see Form::onSuccess() */ - public function onSuccess(Request $request) + public function onSuccess() { if (false === static::isValidResource($this)) { return false; diff --git a/application/forms/Config/ResourceConfigForm.php b/application/forms/Config/ResourceConfigForm.php index 10a853874..d5faca1cd 100644 --- a/application/forms/Config/ResourceConfigForm.php +++ b/application/forms/Config/ResourceConfigForm.php @@ -5,7 +5,6 @@ namespace Icinga\Forms\Config; use InvalidArgumentException; -use Icinga\Web\Request; use Icinga\Web\Notification; use Icinga\Forms\ConfigForm; use Icinga\Forms\Config\Resource\DbResourceForm; @@ -128,7 +127,7 @@ class ResourceConfigForm extends ConfigForm * * @see Form::onSuccess() */ - public function onSuccess(Request $request) + public function onSuccess() { if (($el = $this->getElement('force_creation')) === null || false === $el->isChecked()) { $resourceForm = $this->getResourceForm($this->getElement('type')->getValue()); @@ -138,7 +137,7 @@ class ResourceConfigForm extends ConfigForm } } - $resource = $request->getQuery('resource'); + $resource = $this->request->getQuery('resource'); try { if ($resource === null) { // create new resource $this->add($this->getValues()); @@ -166,9 +165,9 @@ class ResourceConfigForm extends ConfigForm * * @throws ConfigurationError In case the backend name is missing in the request or is invalid */ - public function onRequest(Request $request) + public function onRequest() { - $resource = $request->getQuery('resource'); + $resource = $this->request->getQuery('resource'); if ($resource !== null) { if ($resource === '') { throw new ConfigurationError(t('Resource name missing')); diff --git a/application/forms/PreferenceForm.php b/application/forms/PreferenceForm.php index 40f42d628..fbc4808e0 100644 --- a/application/forms/PreferenceForm.php +++ b/application/forms/PreferenceForm.php @@ -12,10 +12,8 @@ use Icinga\User\Preferences; use Icinga\User\Preferences\PreferencesStore; use Icinga\Util\TimezoneDetect; use Icinga\Util\Translator; -use Icinga\Web\Controller\ControllerTabCollector; use Icinga\Web\Form; use Icinga\Web\Notification; -use Icinga\Web\Request; use Icinga\Web\Session; /** @@ -86,7 +84,7 @@ class PreferenceForm extends Form * * @see Form::onSuccess() */ - public function onSuccess(Request $request) + public function onSuccess() { $this->preferences = new Preferences($this->store->load()); @@ -122,7 +120,7 @@ class PreferenceForm extends Form * * @see Form::onRequest() */ - public function onRequest(Request $request) + public function onRequest() { $auth = Manager::getInstance(); $values = $auth->getUser()->getPreferences()->get('icingaweb'); diff --git a/library/Icinga/Web/Form.php b/library/Icinga/Web/Form.php index 5dcc1cc5c..20484c33e 100644 --- a/library/Icinga/Web/Form.php +++ b/library/Icinga/Web/Form.php @@ -37,6 +37,13 @@ class Form extends Zend_Form */ protected $created = false; + /** + * The request associated with this form + * + * @var Request + */ + protected $request; + /** * The callback to call instead of Form::onSuccess() * @@ -112,7 +119,7 @@ class Form extends Zend_Form * Create a new form * * Accepts an additional option `onSuccess' which is a callback that is called instead of this - * form's method. It is called using the following signature: (Request $request, Form $form). + * form's method. It is called using the following signature: (Form $form). * * @see Zend_Form::__construct() * @@ -356,11 +363,9 @@ class Form extends Zend_Form * * Intended to be implemented by concrete form classes. The base implementation returns always FALSE. * - * @param Request $request The valid request used to process this form - * * @return null|bool Return FALSE in case no redirect should take place */ - public function onSuccess(Request $request) + public function onSuccess() { return false; } @@ -369,10 +374,8 @@ class Form extends Zend_Form * Perform actions when no form dependent data was sent * * Intended to be implemented by concrete form classes. - * - * @param Request $request The current request */ - public function onRequest(Request $request) + public function onRequest() { } @@ -552,15 +555,17 @@ class Form extends Zend_Form { if ($request === null) { $request = $this->getRequest(); + } else { + $this->request = $request; } - $formData = $this->getRequestData($request); + $formData = $this->getRequestData(); if ($this->getUidDisabled() || $this->wasSent($formData)) { $this->populate($formData); // Necessary to get isSubmitted() to work if (! $this->getSubmitLabel() || $this->isSubmitted()) { if ($this->isValid($formData) - && (($this->onSuccess !== null && false !== call_user_func($this->onSuccess, $request, $this)) - || ($this->onSuccess === null && false !== $this->onSuccess($request)))) { + && (($this->onSuccess !== null && false !== call_user_func($this->onSuccess, $this)) + || ($this->onSuccess === null && false !== $this->onSuccess()))) { $this->getResponse()->redirectAndExit($this->getRedirectUrl()); } } else { @@ -568,7 +573,7 @@ class Form extends Zend_Form $this->isValidPartial($formData); } } else { - $this->onRequest($request); + $this->onRequest(); } return $request; @@ -694,29 +699,19 @@ class Form extends Zend_Form } /** - * Return the request data based on this form's request method + * Return the request associated with this form * - * @param Request $request The request to fetch the data from - * - * @return array - */ - public function getRequestData(Request $request) - { - if (strtolower($request->getMethod()) === $this->getMethod()) { - return $request->{'get' . ($request->isPost() ? 'Post' : 'Query')}(); - } - - return array(); - } - - /** - * Return the current request + * Returns the global request if none has been set for this form yet. * * @return Request */ public function getRequest() { - return Icinga::app()->getFrontController()->getRequest(); + if ($this->request === null) { + $this->request = Icinga::app()->getFrontController()->getRequest(); + } + + return $this->request; } /** @@ -729,6 +724,20 @@ class Form extends Zend_Form return Icinga::app()->getFrontController()->getResponse(); } + /** + * Return the request data based on this form's request method + * + * @return array + */ + protected function getRequestData() + { + if (strtolower($this->request->getMethod()) === $this->getMethod()) { + return $this->request->{'get' . ($this->request->isPost() ? 'Post' : 'Query')}(); + } + + return array(); + } + /** * Create a new element located in the Icinga Web 2 library * diff --git a/library/Icinga/Web/Wizard.php b/library/Icinga/Web/Wizard.php index faf950292..2cc4abe5a 100644 --- a/library/Icinga/Web/Wizard.php +++ b/library/Icinga/Web/Wizard.php @@ -207,7 +207,7 @@ class Wizard } $this->setupPage($page, $request); - $requestData = $page->getRequestData($request); + $requestData = $this->getRequestData($page, $request); if ($page->wasSent($requestData)) { if (($requestedPage = $this->getRequestedPage($requestData)) !== null) { $isValid = false; @@ -238,6 +238,23 @@ class Wizard return $request; } + /** + * Return the request data based on given form's request method + * + * @param Form $page The page to fetch the data for + * @param Request $request The request to fetch the data from + * + * @return array + */ + protected function getRequestData(Form $page, Request $request) + { + if (strtolower($request->getMethod()) === $page->getMethod()) { + return $request->{'get' . ($request->isPost() ? 'Post' : 'Query')}(); + } + + return array(); + } + /** * Return the name of the requested page * @@ -269,7 +286,7 @@ class Wizard $request = $currentPage->getRequest(); } - $requestData = $currentPage->getRequestData($request); + $requestData = $this->getRequestData($currentPage, $request); if (isset($requestData[static::BTN_NEXT])) { return static::FORWARD; } elseif (isset($requestData[static::BTN_PREV])) { diff --git a/modules/monitoring/application/forms/Command/Instance/DisableNotificationsExpireCommandForm.php b/modules/monitoring/application/forms/Command/Instance/DisableNotificationsExpireCommandForm.php index 2cb1b83ba..5620d4908 100644 --- a/modules/monitoring/application/forms/Command/Instance/DisableNotificationsExpireCommandForm.php +++ b/modules/monitoring/application/forms/Command/Instance/DisableNotificationsExpireCommandForm.php @@ -9,7 +9,6 @@ use DateInterval; use Icinga\Module\Monitoring\Command\Instance\DisableNotificationsExpireCommand; use Icinga\Module\Monitoring\Forms\Command\CommandForm; use Icinga\Web\Notification; -use Icinga\Web\Request; /** * Form for disabling host and service notifications w/ an optional expire date and time on an Icinga instance @@ -60,12 +59,12 @@ class DisableNotificationsExpireCommandForm extends CommandForm * (non-PHPDoc) * @see \Icinga\Web\Form::onSuccess() For the method documentation. */ - public function onSuccess(Request $request) + public function onSuccess() { $disableNotifications = new DisableNotificationsExpireCommand(); $disableNotifications ->setExpireTime($this->getElement('expire_time')->getValue()->getTimestamp()); - $this->getTransport($request)->send($disableNotifications); + $this->getTransport($this->request)->send($disableNotifications); Notification::success(mt('monitoring', 'Disabling host and service notifications..')); return true; } diff --git a/modules/monitoring/application/forms/Command/Instance/ToggleInstanceFeaturesCommandForm.php b/modules/monitoring/application/forms/Command/Instance/ToggleInstanceFeaturesCommandForm.php index ed2d7e719..291619b2f 100644 --- a/modules/monitoring/application/forms/Command/Instance/ToggleInstanceFeaturesCommandForm.php +++ b/modules/monitoring/application/forms/Command/Instance/ToggleInstanceFeaturesCommandForm.php @@ -7,7 +7,6 @@ namespace Icinga\Module\Monitoring\Forms\Command\Instance; use Icinga\Module\Monitoring\Command\Instance\ToggleInstanceFeatureCommand; use Icinga\Module\Monitoring\Forms\Command\CommandForm; use Icinga\Web\Notification; -use Icinga\Web\Request; /** * Form for enabling or disabling features of Icinga objects, i.e. hosts or services @@ -190,14 +189,14 @@ class ToggleInstanceFeaturesCommandForm extends CommandForm * (non-PHPDoc) * @see \Icinga\Web\Form::onSuccess() For the method documentation. */ - public function onSuccess(Request $request) + public function onSuccess() { foreach ($this->getValues() as $feature => $enabled) { $toggleFeature = new ToggleInstanceFeatureCommand(); $toggleFeature ->setFeature($feature) ->setEnabled($enabled); - $this->getTransport($request)->send($toggleFeature); + $this->getTransport($this->request)->send($toggleFeature); } Notification::success(mt('monitoring', 'Toggling feature..')); return true; diff --git a/modules/monitoring/application/forms/Command/Object/AcknowledgeProblemCommandForm.php b/modules/monitoring/application/forms/Command/Object/AcknowledgeProblemCommandForm.php index 34ac138b1..9123fddc1 100644 --- a/modules/monitoring/application/forms/Command/Object/AcknowledgeProblemCommandForm.php +++ b/modules/monitoring/application/forms/Command/Object/AcknowledgeProblemCommandForm.php @@ -8,7 +8,6 @@ use DateTime; use DateInterval; use Icinga\Module\Monitoring\Command\Object\AcknowledgeProblemCommand; use Icinga\Web\Notification; -use Icinga\Web\Request; /** * Form for acknowledging host or service problems @@ -143,7 +142,7 @@ class AcknowledgeProblemCommandForm extends ObjectsCommandForm * (non-PHPDoc) * @see \Icinga\Web\Form::onSuccess() For the method documentation. */ - public function onSuccess(Request $request) + public function onSuccess() { foreach ($this->objects as $object) { /** @var \Icinga\Module\Monitoring\Object\MonitoredObject $object */ @@ -151,14 +150,14 @@ class AcknowledgeProblemCommandForm extends ObjectsCommandForm $ack ->setObject($object) ->setComment($this->getElement('comment')->getValue()) - ->setAuthor($request->getUser()->getUsername()) + ->setAuthor($this->request->getUser()->getUsername()) ->setPersistent($this->getElement('persistent')->isChecked()) ->setSticky($this->getElement('sticky')->isChecked()) ->setNotify($this->getElement('notify')->isChecked()); if ($this->getElement('expire')->isChecked()) { $ack->setExpireTime($this->getElement('expire_time')->getValue()->getTimestamp()); } - $this->getTransport($request)->send($ack); + $this->getTransport($this->request)->send($ack); } Notification::success(mtp( 'monitoring', diff --git a/modules/monitoring/application/forms/Command/Object/AddCommentCommandForm.php b/modules/monitoring/application/forms/Command/Object/AddCommentCommandForm.php index 70b216da6..5d9a71d51 100644 --- a/modules/monitoring/application/forms/Command/Object/AddCommentCommandForm.php +++ b/modules/monitoring/application/forms/Command/Object/AddCommentCommandForm.php @@ -6,7 +6,6 @@ namespace Icinga\Module\Monitoring\Forms\Command\Object; use Icinga\Module\Monitoring\Command\Object\AddCommentCommand; use Icinga\Web\Notification; -use Icinga\Web\Request; /** * Form for adding host or service comments @@ -76,16 +75,16 @@ class AddCommentCommandForm extends ObjectsCommandForm * (non-PHPDoc) * @see \Icinga\Web\Form::onSuccess() For the method documentation. */ - public function onSuccess(Request $request) + public function onSuccess() { foreach ($this->objects as $object) { /** @var \Icinga\Module\Monitoring\Object\MonitoredObject $object */ $comment = new AddCommentCommand(); $comment->setObject($object); $comment->setComment($this->getElement('comment')->getValue()); - $comment->setAuthor($request->getUser()->getUsername()); + $comment->setAuthor($this->request->getUser()->getUsername()); $comment->setPersistent($this->getElement('persistent')->isChecked()); - $this->getTransport($request)->send($comment); + $this->getTransport($this->request)->send($comment); } Notification::success(mtp( 'monitoring', diff --git a/modules/monitoring/application/forms/Command/Object/CheckNowCommandForm.php b/modules/monitoring/application/forms/Command/Object/CheckNowCommandForm.php index b8f708de0..477624bb6 100644 --- a/modules/monitoring/application/forms/Command/Object/CheckNowCommandForm.php +++ b/modules/monitoring/application/forms/Command/Object/CheckNowCommandForm.php @@ -7,7 +7,6 @@ namespace Icinga\Module\Monitoring\Forms\Command\Object; use Icinga\Module\Monitoring\Command\Object\ScheduleHostCheckCommand; use Icinga\Module\Monitoring\Command\Object\ScheduleServiceCheckCommand; use Icinga\Web\Notification; -use Icinga\Web\Request; /** * Form for immediately checking hosts or services @@ -54,7 +53,7 @@ class CheckNowCommandForm extends ObjectsCommandForm * (non-PHPDoc) * @see \Icinga\Web\Form::onSuccess() For the method documentation. */ - public function onSuccess(Request $request) + public function onSuccess() { foreach ($this->objects as $object) { /** @var \Icinga\Module\Monitoring\Object\MonitoredObject $object */ @@ -67,7 +66,7 @@ class CheckNowCommandForm extends ObjectsCommandForm ->setObject($object) ->setForced() ->setCheckTime(time()); - $this->getTransport($request)->send($check); + $this->getTransport($this->request)->send($check); } Notification::success(mtp( 'monitoring', diff --git a/modules/monitoring/application/forms/Command/Object/DeleteCommentCommandForm.php b/modules/monitoring/application/forms/Command/Object/DeleteCommentCommandForm.php index d0edef274..3d85afcec 100644 --- a/modules/monitoring/application/forms/Command/Object/DeleteCommentCommandForm.php +++ b/modules/monitoring/application/forms/Command/Object/DeleteCommentCommandForm.php @@ -6,7 +6,6 @@ namespace Icinga\Module\Monitoring\Forms\Command\Object; use Icinga\Module\Monitoring\Command\Object\DeleteCommentCommand; use Icinga\Web\Notification; -use Icinga\Web\Request; /** * Form for deleting host or service comments @@ -67,7 +66,7 @@ class DeleteCommentCommandForm extends ObjectsCommandForm * (non-PHPDoc) * @see \Icinga\Web\Form::onSuccess() For the method documentation. */ - public function onSuccess(Request $request) + public function onSuccess() { foreach ($this->objects as $object) { /** @var \Icinga\Module\Monitoring\Object\MonitoredObject $object */ @@ -75,7 +74,7 @@ class DeleteCommentCommandForm extends ObjectsCommandForm $delComment ->setObject($object) ->setCommentId($this->getElement('comment_id')->getValue()); - $this->getTransport($request)->send($delComment); + $this->getTransport($this->request)->send($delComment); } $redirect = $this->getElement('redirect')->getValue(); if (! empty($redirect)) { diff --git a/modules/monitoring/application/forms/Command/Object/DeleteDowntimeCommandForm.php b/modules/monitoring/application/forms/Command/Object/DeleteDowntimeCommandForm.php index 3dced5072..1c7095b82 100644 --- a/modules/monitoring/application/forms/Command/Object/DeleteDowntimeCommandForm.php +++ b/modules/monitoring/application/forms/Command/Object/DeleteDowntimeCommandForm.php @@ -6,7 +6,6 @@ namespace Icinga\Module\Monitoring\Forms\Command\Object; use Icinga\Module\Monitoring\Command\Object\DeleteDowntimeCommand; use Icinga\Web\Notification; -use Icinga\Web\Request; /** * Form for deleting host or service downtimes @@ -67,7 +66,7 @@ class DeleteDowntimeCommandForm extends ObjectsCommandForm * (non-PHPDoc) * @see \Icinga\Web\Form::onSuccess() For the method documentation. */ - public function onSuccess(Request $request) + public function onSuccess() { foreach ($this->objects as $object) { /** @var \Icinga\Module\Monitoring\Object\MonitoredObject $object */ @@ -75,7 +74,7 @@ class DeleteDowntimeCommandForm extends ObjectsCommandForm $delDowntime ->setObject($object) ->setDowntimeId($this->getElement('downtime_id')->getValue()); - $this->getTransport($request)->send($delDowntime); + $this->getTransport($this->request)->send($delDowntime); } $redirect = $this->getElement('redirect')->getValue(); if (! empty($redirect)) { diff --git a/modules/monitoring/application/forms/Command/Object/RemoveAcknowledgementCommandForm.php b/modules/monitoring/application/forms/Command/Object/RemoveAcknowledgementCommandForm.php index 41cde9b43..189261c94 100644 --- a/modules/monitoring/application/forms/Command/Object/RemoveAcknowledgementCommandForm.php +++ b/modules/monitoring/application/forms/Command/Object/RemoveAcknowledgementCommandForm.php @@ -6,7 +6,6 @@ namespace Icinga\Module\Monitoring\Forms\Command\Object; use Icinga\Module\Monitoring\Command\Object\RemoveAcknowledgementCommand; use Icinga\Web\Notification; -use Icinga\Web\Request; /** * Form for removing host or service problem acknowledgements @@ -37,13 +36,13 @@ class RemoveAcknowledgementCommandForm extends ObjectsCommandForm * (non-PHPDoc) * @see \Icinga\Web\Form::onSuccess() For the method documentation. */ - public function onSuccess(Request $request) + public function onSuccess() { foreach ($this->objects as $object) { /** @var \Icinga\Module\Monitoring\Object\MonitoredObject $object */ $removeAck = new RemoveAcknowledgementCommand(); $removeAck->setObject($object); - $this->getTransport($request)->send($removeAck); + $this->getTransport($this->request)->send($removeAck); } Notification::success(mtp( 'monitoring', diff --git a/modules/monitoring/application/forms/Command/Object/ScheduleHostCheckCommandForm.php b/modules/monitoring/application/forms/Command/Object/ScheduleHostCheckCommandForm.php index 4e2bd62a9..82a9b2e52 100644 --- a/modules/monitoring/application/forms/Command/Object/ScheduleHostCheckCommandForm.php +++ b/modules/monitoring/application/forms/Command/Object/ScheduleHostCheckCommandForm.php @@ -6,7 +6,6 @@ namespace Icinga\Module\Monitoring\Forms\Command\Object; use Icinga\Module\Monitoring\Command\Object\ScheduleHostCheckCommand; use Icinga\Web\Notification; -use Icinga\Web\Request; /** * Form for scheduling host checks @@ -40,7 +39,7 @@ class ScheduleHostCheckCommandForm extends ScheduleServiceCheckCommandForm * (non-PHPDoc) * @see \Icinga\Web\Form::onSuccess() For the method documentation. */ - public function onSuccess(Request $request) + public function onSuccess() { foreach ($this->objects as $object) { /** @var \Icinga\Module\Monitoring\Object\Host $object */ @@ -48,7 +47,7 @@ class ScheduleHostCheckCommandForm extends ScheduleServiceCheckCommandForm $check ->setObject($object) ->setOfAllServices($this->getElement('all_services')->isChecked()); - $this->scheduleCheck($check, $request); + $this->scheduleCheck($check, $this->request); } Notification::success(mtp( 'monitoring', diff --git a/modules/monitoring/application/forms/Command/Object/ScheduleHostDowntimeCommandForm.php b/modules/monitoring/application/forms/Command/Object/ScheduleHostDowntimeCommandForm.php index db4262f68..beb0793ef 100644 --- a/modules/monitoring/application/forms/Command/Object/ScheduleHostDowntimeCommandForm.php +++ b/modules/monitoring/application/forms/Command/Object/ScheduleHostDowntimeCommandForm.php @@ -59,7 +59,7 @@ class ScheduleHostDowntimeCommandForm extends ScheduleServiceDowntimeCommandForm * (non-PHPDoc) * @see \Icinga\Web\Form::onSuccess() For the method documentation. */ - public function onSuccess(Request $request) + public function onSuccess() { foreach ($this->objects as $object) { /** @var \Icinga\Module\Monitoring\Object\Host $object */ @@ -79,12 +79,12 @@ class ScheduleHostDowntimeCommandForm extends ScheduleServiceDowntimeCommandForm foreach ($object->services as $service) { $serviceDowntime = new ScheduleServiceDowntimeCommand(); $serviceDowntime->setObject($service); - $this->scheduleDowntime($serviceDowntime, $request); + $this->scheduleDowntime($serviceDowntime, $this->request); } } } $hostDowntime->setObject($object); - $this->scheduleDowntime($hostDowntime, $request); + $this->scheduleDowntime($hostDowntime, $this->request); } Notification::success(mtp( 'monitoring', diff --git a/modules/monitoring/application/forms/Command/Object/ScheduleServiceCheckCommandForm.php b/modules/monitoring/application/forms/Command/Object/ScheduleServiceCheckCommandForm.php index 5641557d6..faba7a769 100644 --- a/modules/monitoring/application/forms/Command/Object/ScheduleServiceCheckCommandForm.php +++ b/modules/monitoring/application/forms/Command/Object/ScheduleServiceCheckCommandForm.php @@ -90,13 +90,13 @@ class ScheduleServiceCheckCommandForm extends ObjectsCommandForm * (non-PHPDoc) * @see \Icinga\Web\Form::onSuccess() For the method documentation. */ - public function onSuccess(Request $request) + public function onSuccess() { foreach ($this->objects as $object) { /** @var \Icinga\Module\Monitoring\Object\Service $object */ $check = new ScheduleServiceCheckCommand(); $check->setObject($object); - $this->scheduleCheck($check, $request); + $this->scheduleCheck($check, $this->request); } Notification::success(mtp( 'monitoring', diff --git a/modules/monitoring/application/forms/Command/Object/ScheduleServiceDowntimeCommandForm.php b/modules/monitoring/application/forms/Command/Object/ScheduleServiceDowntimeCommandForm.php index 93a45c5c4..18bbd0593 100644 --- a/modules/monitoring/application/forms/Command/Object/ScheduleServiceDowntimeCommandForm.php +++ b/modules/monitoring/application/forms/Command/Object/ScheduleServiceDowntimeCommandForm.php @@ -201,13 +201,13 @@ class ScheduleServiceDowntimeCommandForm extends ObjectsCommandForm * (non-PHPDoc) * @see \Icinga\Web\Form::onSuccess() For the method documentation. */ - public function onSuccess(Request $request) + public function onSuccess() { foreach ($this->objects as $object) { /** @var \Icinga\Module\Monitoring\Object\Service $object */ $downtime = new ScheduleServiceDowntimeCommand(); $downtime->setObject($object); - $this->scheduleDowntime($downtime, $request); + $this->scheduleDowntime($downtime, $this->request); } Notification::success(mtp( 'monitoring', diff --git a/modules/monitoring/application/forms/Command/Object/ToggleObjectFeaturesCommandForm.php b/modules/monitoring/application/forms/Command/Object/ToggleObjectFeaturesCommandForm.php index ebaad32c8..c33527895 100644 --- a/modules/monitoring/application/forms/Command/Object/ToggleObjectFeaturesCommandForm.php +++ b/modules/monitoring/application/forms/Command/Object/ToggleObjectFeaturesCommandForm.php @@ -7,7 +7,6 @@ namespace Icinga\Module\Monitoring\Forms\Command\Object; use Icinga\Module\Monitoring\Command\Object\ToggleObjectFeatureCommand; use Icinga\Module\Monitoring\Object\MonitoredObject; use Icinga\Web\Notification; -use Icinga\Web\Request; /** * Form for enabling or disabling features of Icinga objects, i.e. hosts or services @@ -106,7 +105,7 @@ class ToggleObjectFeaturesCommandForm extends ObjectsCommandForm * (non-PHPDoc) * @see \Icinga\Web\Form::onSuccess() For the method documentation. */ - public function onSuccess(Request $request) + public function onSuccess() { foreach ($this->objects as $object) { /** @var \Icinga\Module\Monitoring\Object\MonitoredObject $object */ @@ -117,7 +116,7 @@ class ToggleObjectFeaturesCommandForm extends ObjectsCommandForm ->setFeature($feature) ->setObject($object) ->setEnabled($enabled); - $this->getTransport($request)->send($toggleFeature); + $this->getTransport($this->request)->send($toggleFeature); } } } diff --git a/modules/monitoring/application/forms/Config/BackendConfigForm.php b/modules/monitoring/application/forms/Config/BackendConfigForm.php index e3c68fc67..1f31990cb 100644 --- a/modules/monitoring/application/forms/Config/BackendConfigForm.php +++ b/modules/monitoring/application/forms/Config/BackendConfigForm.php @@ -5,7 +5,6 @@ namespace Icinga\Module\Monitoring\Forms\Config; use InvalidArgumentException; -use Icinga\Web\Request; use Icinga\Web\Notification; use Icinga\Forms\ConfigForm; use Icinga\Application\Config; @@ -135,9 +134,9 @@ class BackendConfigForm extends ConfigForm * * @see Form::onSuccess() */ - public function onSuccess(Request $request) + public function onSuccess() { - $monitoringBackend = $request->getQuery('backend'); + $monitoringBackend = $this->request->getQuery('backend'); try { if ($monitoringBackend === null) { // create new backend $this->add($this->getValues()); @@ -165,9 +164,9 @@ class BackendConfigForm extends ConfigForm * * @throws ConfigurationError In case the backend name is missing in the request or is invalid */ - public function onRequest(Request $request) + public function onRequest() { - $monitoringBackend = $request->getQuery('backend'); + $monitoringBackend = $this->request->getQuery('backend'); if ($monitoringBackend !== null) { if ($monitoringBackend === '') { throw new ConfigurationError(mt('monitoring', 'Monitoring backend name missing')); diff --git a/modules/monitoring/application/forms/Config/InstanceConfigForm.php b/modules/monitoring/application/forms/Config/InstanceConfigForm.php index 8515c522e..302c0dec3 100644 --- a/modules/monitoring/application/forms/Config/InstanceConfigForm.php +++ b/modules/monitoring/application/forms/Config/InstanceConfigForm.php @@ -12,7 +12,6 @@ use Icinga\Module\Monitoring\Command\Transport\RemoteCommandFile; use Icinga\Module\Monitoring\Forms\Config\Instance\LocalInstanceForm; use Icinga\Module\Monitoring\Forms\Config\Instance\RemoteInstanceForm; use Icinga\Web\Notification; -use Icinga\Web\Request; /** * Form for modifying/creating monitoring instances @@ -132,9 +131,9 @@ class InstanceConfigForm extends ConfigForm * @see Form::onRequest() For the method documentation. * @throws ConfigurationError In case the instance name is missing or invalid */ - public function onRequest(Request $request) + public function onRequest() { - $instanceName = $request->getQuery('instance'); + $instanceName = $this->request->getQuery('instance'); if ($instanceName !== null) { if (! $instanceName) { throw new ConfigurationError(mt('monitoring', 'Instance name missing')); @@ -153,9 +152,9 @@ class InstanceConfigForm extends ConfigForm * (non-PHPDoc) * @see Form::onSuccess() For the method documentation. */ - public function onSuccess(Request $request) + public function onSuccess() { - $instanceName = $request->getQuery('instance'); + $instanceName = $this->request->getQuery('instance'); try { if ($instanceName === null) { // create new instance $this->add($this->getValues()); diff --git a/modules/monitoring/application/forms/Config/SecurityConfigForm.php b/modules/monitoring/application/forms/Config/SecurityConfigForm.php index 4d8230bfd..de4055802 100644 --- a/modules/monitoring/application/forms/Config/SecurityConfigForm.php +++ b/modules/monitoring/application/forms/Config/SecurityConfigForm.php @@ -4,7 +4,6 @@ namespace Icinga\Module\Monitoring\Forms\Config; -use Icinga\Web\Request; use Icinga\Web\Notification; use Icinga\Forms\ConfigForm; @@ -25,7 +24,7 @@ class SecurityConfigForm extends ConfigForm /** * @see Form::onSuccess() */ - public function onSuccess(Request $request) + public function onSuccess() { $this->config->security = $this->getValues(); @@ -39,7 +38,7 @@ class SecurityConfigForm extends ConfigForm /** * @see Form::onRequest() */ - public function onRequest(Request $request) + public function onRequest() { if (isset($this->config->security)) { $this->populate($this->config->security->toArray()); diff --git a/modules/setup/application/forms/ModulePage.php b/modules/setup/application/forms/ModulePage.php index 06d3cf621..5e8927dbc 100644 --- a/modules/setup/application/forms/ModulePage.php +++ b/modules/setup/application/forms/ModulePage.php @@ -45,7 +45,8 @@ class ModulePage extends Form public function handleRequest(Request $request = null) { - if ($this->wasSent($this->getRequestData($request))) { + $isPost = strtolower($request->getMethod()) === 'post'; + if ($isPost && $this->wasSent($request->getPost())) { if (($newModule = $request->getPost('module')) !== null) { $this->setCurrentModule($newModule); $this->getResponse()->redirectAndExit($this->getRedirectUrl()); @@ -58,7 +59,7 @@ class ModulePage extends Form $wizardPage = $wizard->getCurrentPage(); $wizard->handleRequest($request); - if ($wizard->isFinished() && $wizardPage->wasSent($wizardPage->getRequestData($request))) { + if ($isPost && $wizard->isFinished() && $wizardPage->wasSent($request->getPost())) { $wizards = $this->getWizards(); $newModule = null; diff --git a/test/php/library/Icinga/Web/FormTest.php b/test/php/library/Icinga/Web/FormTest.php index a2fc9c8b1..de880f070 100644 --- a/test/php/library/Icinga/Web/FormTest.php +++ b/test/php/library/Icinga/Web/FormTest.php @@ -11,7 +11,7 @@ use Icinga\Test\BaseTestCase; class SuccessfulForm extends Form { - public function onSuccess(Request $request) + public function onSuccess() { return true; } @@ -238,24 +238,6 @@ class FormTest extends BaseTestCase ); } - public function testWhetherGetRequestDataOnlyReturnsFormRelevantData() - { - $form = new Form(); - $form->setMethod('POST'); - - $expectedResult = array('expected_key' => 'expected_value'); - $request = $this->getRequestMock(); - $request->shouldReceive('getMethod')->andReturn('POST') - ->shouldReceive('isPost')->andReturn(true) - ->shouldReceive('getPost')->andReturn($expectedResult); - - $this->assertEquals( - $expectedResult, - $form->getRequestData($request), - 'Form::getRequestData() does not (only) return form relevant data' - ); - } - /** * @expectedException LogicException */ @@ -273,7 +255,7 @@ class FormTest extends BaseTestCase { $request = new Request(); $form = new Form(array( - 'onSuccess' => function ($req) { $req->setParam('test', 'tset'); return false; } + 'onSuccess' => function ($form) { $form->getRequest()->setParam('test', 'tset'); return false; } )); $form->setTokenDisabled(); $form->setUidDisabled();