Ejegg has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/402905 )
Change subject: CRM-16819 improve on CRM_Utils_Request::retrieve
......................................................................
CRM-16819 improve on CRM_Utils_Request::retrieve
I'm proposing this as an improved (but not perfect) way to retrieve data from
http requests.
The biggest issue IMHO with the CRM_Utils_Request::retrieve method is that the
third
parameter is & - and if any of the subsequent parameters need to be passed a
NULL
array must be generated. The practice of passing a specific static NULL array
is known to
have caused errors due to contamination. This commit offers an alternate
function without that
parameter (my suggestion is to create a new function again where we really do
want to
work but in general it seems flakey to me). I have also re-ordered the
parameters as I perceive
as being more commonly required than (renamed from 'abort').
I have also made it so the new function will throw an exception rather than use
the deprecated
fatal - as a new function I feel we can enforce this change and I have made
some changes
to the error message thrown in the validate function so it returns a list of
permissable
values for the string as the existing error is hard for developers to work
with, as it
can be hard to realise why 'int' is wrong (casing) or what should have been
passed.
Finally I made the visibility on CRM_Utils_Retrieve::getValues protected. This
function
is not called from within core & is downright unsafe as it does no validation
so we should
not permit anyone to accidentally use it (I have no reason to think they have
Change-Id: I5531ed061cf3b7f486aa57d073b425d0874c4df0
---
M CRM/Utils/Request.php
M CRM/Utils/Type.php
2 files changed, 77 insertions(+), 14 deletions(-)
Approvals:
jenkins-bot: Verified
Ejegg: Looks good to me, approved
diff --git a/CRM/Utils/Request.php b/CRM/Utils/Request.php
index 2748e07..ad6bf09 100644
--- a/CRM/Utils/Request.php
+++ b/CRM/Utils/Request.php
@@ -79,17 +79,15 @@
* Default value of the variable if not present.
* @param string $method
* Where to look for the variable - 'GET', 'POST' or 'REQUEST'.
+ * @param bool $isThrowException
+ * Should a an exception be thrown rather than a fatal.
*
* @return mixed
* The value of the variable
+ *
+ * @throws \CRM_Core_Exception
*/
- public static function retrieve($name, $type, &$store = NULL, $abort =
FALSE, $default = NULL, $method = 'REQUEST') {
-
- // hack to detect stuff not yet converted to new style
- if (!is_string($type)) {
- CRM_Core_Error::backtrace();
- CRM_Core_Error::fatal(ts("Please convert retrieve call to use new
function signature"));
- }
+ public static function retrieve($name, $type, &$store = NULL, $abort =
FALSE, $default = NULL, $method = 'REQUEST', $isThrowException = FALSE) {
$value = NULL;
switch ($method) {
@@ -117,6 +115,9 @@
}
if (!isset($value) && $abort) {
+ if ($isThrowException) {
+ throw new CRM_Core_Exception(ts("Could not find valid value for %1",
array(1 => $name)));
+ }
CRM_Core_Error::fatal(ts("Could not find valid value for %1", array(1 =>
$name)));
}
@@ -145,7 +146,7 @@
* @return mixed
* The value of the variable
*/
- public static function getValue($name, $method) {
+ protected static function getValue($name, $method) {
if (isset($method[$name])) {
return $method[$name];
}
@@ -165,6 +166,10 @@
}
/**
+ * @deprecated
+ *
+ * We should use a function that checks url values.
+ *
* This is a replacement for $_REQUEST which includes $_GET/$_POST
* but excludes $_COOKIE / $_ENV / $_SERVER.
*
@@ -186,4 +191,32 @@
return $result;
}
+ /**
+ * Retrieve a variable from the http request.
+ *
+ * @param string $name
+ * Name of the variable to be retrieved.
+ * @param string $type
+ * Type of the variable (see CRM_Utils_Type for details).
+ * Most common options are:
+ * - 'Integer'
+ * - 'Positive'
+ * - 'CommaSeparatedIntegers'
+ * - 'Boolean'
+ * - 'String'
+ *
+ * @param mixed $defaultValue
+ * Default value of the variable if not present.
+ * @param bool $isRequired
+ * Is the variable required for this function to proceed without an
exception.
+ * @param string $method
+ * Where to look for the value - GET|POST|REQUEST
+ *
+ * @return mixed
+ */
+ public static function retrieveValue($name, $type, $defaultValue = NULL,
$isRequired = FALSE, $method = 'REQUEST') {
+ $null = NULL;
+ return CRM_Utils_Request::retrieve((string) $name, (string) $type, $null,
(bool) $isRequired, $defaultValue, $method, TRUE);
+ }
+
}
diff --git a/CRM/Utils/Type.php b/CRM/Utils/Type.php
index fbe1c93..48e575e 100644
--- a/CRM/Utils/Type.php
+++ b/CRM/Utils/Type.php
@@ -369,13 +369,44 @@
* The type to validate against.
* @param bool $abort
* If TRUE, the operation will CRM_Core_Error::fatal() on invalid data.
- * @name string $name
+ * @param string $name
* The name of the attribute
+ * @param bool $isThrowException
+ * Should an exception be thrown rather than a using a deprecated fatal
error.
*
* @return mixed
* The data, escaped if necessary
+ *
+ * @throws \CRM_Core_Exception
*/
- public static function validate($data, $type, $abort = TRUE, $name = 'One of
parameters ') {
+ public static function validate($data, $type, $abort = TRUE, $name = 'One of
parameters ', $isThrowException = FALSE) {
+
+ $possibleTypes = array(
+ 'Integer',
+ 'Int',
+ 'Positive',
+ 'CommaSeparatedIntegers',
+ 'Boolean',
+ 'Float',
+ 'Money',
+ 'Text',
+ 'String',
+ 'Link',
+ 'Memo',
+ 'Date',
+ 'Timestamp',
+ 'ContactReference',
+ 'MysqlColumnNameOrAlias',
+ 'MysqlOrderByDirection',
+ 'MysqlOrderBy',
+ 'ExtensionKey',
+ );
+ if (!in_array($type, $possibleTypes)) {
+ if ($isThrowException) {
+ throw new CRM_Core_Exception(ts('Invalid type, must be one of : ' .
implode($possibleTypes)));
+ }
+ CRM_Core_Error::fatal(ts('Invalid type, must be one of : ' .
implode($possibleTypes)));
+ }
switch ($type) {
case 'Integer':
case 'Int':
@@ -471,14 +502,13 @@
return $data;
}
break;
-
- default:
- CRM_Core_Error::fatal("Cannot recognize $type for $data");
- break;
}
if ($abort) {
$data = htmlentities($data);
+ if ($isThrowException) {
+ throw new CRM_Core_Exception("$name (value: $data) is not of the type
$type");
+ }
CRM_Core_Error::fatal("$name (value: $data) is not of the type $type");
}
--
To view, visit https://gerrit.wikimedia.org/r/402905
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I5531ed061cf3b7f486aa57d073b425d0874c4df0
Gerrit-PatchSet: 2
Gerrit-Project: wikimedia/fundraising/crm/civicrm
Gerrit-Branch: master
Gerrit-Owner: Eileen <emcnaugh...@wikimedia.org>
Gerrit-Reviewer: Ejegg <ej...@ejegg.com>
Gerrit-Reviewer: Mepps <me...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
