r43377 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r43376‎ | r43377 | r43378 >
Date:01:07, 11 November 2008
Author:david
Status:old (Comments)
Tags:
Comment:
Factored wfInvokeFancyCallback() out of wfRunHooks(). Allows flexible dynamic invocation to be used in other contexts.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/includes/Hooks.php (modified) (history)

Diff [purge]

Index: trunk/phase3/RELEASE-NOTES
@@ -202,6 +202,8 @@
203203 * (bug 15835) Added Content-Style-Type meta tag
204204 * Add class="disambiguationpage" to <body> tag for disambiguations
205205 * (bug 11027) Add parameter to MW:Randompage-nopages so that user can see the namespace.
 206+* New global function wfInvokeFancyCallback() can be used to dynamically
 207+ invoke functions with a flexible interface, like wfRunHooks does.
206208
207209 === Bug fixes in 1.14 ===
208210
Index: trunk/phase3/includes/Hooks.php
@@ -22,7 +22,106 @@
2323 * @file
2424 */
2525
 26+/* Private */
 27+function _wfInternalFancyCallbackGoop($hook) {
 28+ $object = NULL;
 29+ $method = NULL;
 30+ $func = NULL;
 31+ $data = NULL;
 32+ $have_data = false;
2633
 34+ if (is_array($hook)) {
 35+ if (count($hook) < 1) {
 36+ throw new MWException("Empty array in hooks for " . $event . "\n");
 37+ } else if (is_object($hook[0])) {
 38+ $object = $hook[0];
 39+ if (count($hook) < 2) {
 40+ $method = "on" . $event;
 41+ } else {
 42+ $method = $hook[1];
 43+ if (count($hook) > 2) {
 44+ $data = $hook[2];
 45+ $have_data = true;
 46+ }
 47+ }
 48+ } else if (is_string($hook[0])) {
 49+ $func = $hook[0];
 50+ if (count($hook) > 1) {
 51+ $data = $hook[1];
 52+ $have_data = true;
 53+ }
 54+ } else {
 55+ var_dump( $wgHooks );
 56+ throw new MWException("Unknown datatype in hooks for " . $event . "\n");
 57+ }
 58+ } else if (is_string($hook)) { # functions look like strings, too
 59+ $func = $hook;
 60+ } else if (is_object($hook)) {
 61+ $object = $hook;
 62+ $method = "on" . $event;
 63+ } else {
 64+ throw new MWException("Unknown datatype in hooks for " . $event . "\n");
 65+ }
 66+
 67+ if ( isset( $object ) ) {
 68+ $func = get_class( $object ) . '::' . $method;
 69+ $callback = array( $object, $method );
 70+ } elseif ( false !== ( $pos = strpos( $func, '::' ) ) ) {
 71+ $callback = array( substr( $func, 0, $pos ), substr( $func, $pos + 2 ) );
 72+ } else {
 73+ $callback = $func;
 74+ }
 75+
 76+ return array($callback, $func, $data);
 77+}
 78+
 79+/* Return a string describing the hook for debugging purposes. */
 80+function wfFormatFancyCallback($hook) {
 81+ list($callback, $func, $data) = _wfInternalFancyCallbackGoop($hook, $args);
 82+
 83+ if( is_array( $callback ) ) {
 84+ if( is_object( $callback[0] ) ) {
 85+ $prettyClass = get_class( $callback[0] );
 86+ } else {
 87+ $prettyClass = strval( $callback[0] );
 88+ }
 89+ $prettyFunc = $prettyClass . '::' . strval( $callback[1] );
 90+ } else {
 91+ $prettyFunc = strval( $callback );
 92+ }
 93+ return $prettyFunc;
 94+}
 95+
 96+
 97+/*
 98+ * Invoke a function dynamically.
 99+ * $hook can be: a function, an object, an array of $function and $data,
 100+ * an array of just a function, an array of object and method, or an
 101+ * array of object, method, and data.
 102+ * If arguments are provided both as part of the hook itself, and when
 103+ * calling wfCallFancyCallback, the two arrays are merged.
 104+ */
 105+function wfInvokeFancyCallback($hook, $args = array()) {
 106+ list($callback, $func, $data) = _wfInternalFancyCallbackGoop($hook, $args);
 107+
 108+ /* We put the first data element on, if needed. */
 109+ if ($data) {
 110+ $hook_args = array_merge(array($data), $args);
 111+ } else {
 112+ $hook_args = $args;
 113+ }
 114+
 115+ // Run autoloader (workaround for call_user_func_array bug)
 116+ is_callable( $callback );
 117+
 118+ /* Call the hook. */
 119+ wfProfileIn( $func );
 120+ $retval = call_user_func_array( $callback, $hook_args );
 121+ wfProfileOut( $func );
 122+ return $retval;
 123+}
 124+
 125+
27126 /**
28127 * Because programmers assign to $wgHooks, we need to be very
29128 * careful about its contents. So, there's a lot more error-checking
@@ -48,75 +147,8 @@
49148
50149 foreach ($wgHooks[$event] as $index => $hook) {
51150
52 - $object = NULL;
53 - $method = NULL;
54 - $func = NULL;
55 - $data = NULL;
56 - $have_data = false;
 151+ $retval = wfInvokeFancyCallback($hook, $args);
57152
58 - /* $hook can be: a function, an object, an array of $function and $data,
59 - * an array of just a function, an array of object and method, or an
60 - * array of object, method, and data.
61 - */
62 -
63 - if (is_array($hook)) {
64 - if (count($hook) < 1) {
65 - throw new MWException("Empty array in hooks for " . $event . "\n");
66 - } else if (is_object($hook[0])) {
67 - $object = $wgHooks[$event][$index][0];
68 - if (count($hook) < 2) {
69 - $method = "on" . $event;
70 - } else {
71 - $method = $hook[1];
72 - if (count($hook) > 2) {
73 - $data = $hook[2];
74 - $have_data = true;
75 - }
76 - }
77 - } else if (is_string($hook[0])) {
78 - $func = $hook[0];
79 - if (count($hook) > 1) {
80 - $data = $hook[1];
81 - $have_data = true;
82 - }
83 - } else {
84 - var_dump( $wgHooks );
85 - throw new MWException("Unknown datatype in hooks for " . $event . "\n");
86 - }
87 - } else if (is_string($hook)) { # functions look like strings, too
88 - $func = $hook;
89 - } else if (is_object($hook)) {
90 - $object = $wgHooks[$event][$index];
91 - $method = "on" . $event;
92 - } else {
93 - throw new MWException("Unknown datatype in hooks for " . $event . "\n");
94 - }
95 -
96 - /* We put the first data element on, if needed. */
97 -
98 - if ($have_data) {
99 - $hook_args = array_merge(array($data), $args);
100 - } else {
101 - $hook_args = $args;
102 - }
103 -
104 - if ( isset( $object ) ) {
105 - $func = get_class( $object ) . '::' . $method;
106 - $callback = array( $object, $method );
107 - } elseif ( false !== ( $pos = strpos( $func, '::' ) ) ) {
108 - $callback = array( substr( $func, 0, $pos ), substr( $func, $pos + 2 ) );
109 - } else {
110 - $callback = $func;
111 - }
112 -
113 - // Run autoloader (workaround for call_user_func_array bug)
114 - is_callable( $callback );
115 -
116 - /* Call the hook. */
117 - wfProfileIn( $func );
118 - $retval = call_user_func_array( $callback, $hook_args );
119 - wfProfileOut( $func );
120 -
121153 /* String return is an error; false return means stop processing. */
122154
123155 if (is_string($retval)) {
@@ -124,16 +156,7 @@
125157 $wgOut->showFatalError($retval);
126158 return false;
127159 } elseif( $retval === null ) {
128 - if( is_array( $callback ) ) {
129 - if( is_object( $callback[0] ) ) {
130 - $prettyClass = get_class( $callback[0] );
131 - } else {
132 - $prettyClass = strval( $callback[0] );
133 - }
134 - $prettyFunc = $prettyClass . '::' . strval( $callback[1] );
135 - } else {
136 - $prettyFunc = strval( $callback );
137 - }
 160+ $prettyFunc = wfFormatFancyCallback($hook);
138161 throw new MWException( "Detected bug in an extension! " .
139162 "Hook $prettyFunc failed to return a value; " .
140163 "should return true to continue hook processing or false to abort." );

Follow-up revisions

RevisionCommit summaryAuthorDate
r43383Remove RELEASE-NOTES entry added in r43377 (Factored wfInvokeFancyCallback() ...catrope13:27, 11 November 2008
r43416Roll back r43377, r43379 "Factored wfInvokeFancyCallback() out of wfRunHooks(...brion18:06, 12 November 2008

Comments

#Comment by Brion VIBBER (talk | contribs)   18:06, 12 November 2008

Reverted in r43416 -- fails on existing hooks including the Chinese converter.

Notice: Undefined variable: event in /Library/WebServer/Documents/trunk/includes/Hooks.php on line 61
Warning: call_user_func_array(): First argument is expected to be a valid callback, 'ZhConverter::on' was given in /Library/WebServer/Documents/trunk/includes/Hooks.php on line 119
Notice: Undefined variable: args in /Library/WebServer/Documents/trunk/includes/Hooks.php on line 80
Notice: Undefined variable: event in /Library/WebServer/Documents/trunk/includes/Hooks.php on line 61
Detected bug in an extension! Hook ZhConverter::on failed to return a value; should return true to continue hook processing or false to abort.
Backtrace:
#0 /Library/WebServer/Documents/trunk/includes/Article.php(1684): wfRunHooks('ArticleSaveComp...', Array)
#1 /Library/WebServer/Documents/trunk/includes/Article.php(1354): Article->doEdit('Article for spe...', '', 97)
#2 /Library/WebServer/Documents/trunk/maintenance/parserTests.inc(998): Article->insertNewArticle('Article for spe...', '', false, false)
#3 /Library/WebServer/Documents/trunk/maintenance/parserTests.inc(307): ParserTest->addArticle('Xyzzyx', 'Article for spe...', 5168)
#4 /Library/WebServer/Documents/trunk/maintenance/parserTests.inc(273): ParserTest->runFile('/Library/WebSer...')
#5 /Library/WebServer/Documents/trunk/maintenance/parserTests.php(73): ParserTest->runTestsFromFiles(Array)
#6 {main}

Status & tagging log