r44702 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r44701‎ | r44702 | r44703 >
Date:07:00, 17 December 2008
Author:david
Status:reverted (Comments)
Tags:
Comment:
Factored wfInvoke out of wfRunHooks. This time without breakage. (I think.)
Modified paths:
  • /trunk/phase3/includes/Hooks.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Hooks.php
@@ -22,7 +22,106 @@
2323 * @file
2424 */
2525
 26+/* Private */
 27+function _wfInvokeInternalGoop($event, $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 wfFormatInvocation($event, $hook, $args = array()) {
 81+ list($callback, $func, $data) = _wfInvokeInternalGoop($event, $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 wfInvoke($event, $hook, $args = array()) {
 106+ list($callback, $func, $data) = _wfInvokeInternalGoop($event, $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 = wfInvoke($event, $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 = wfFormatInvocation($event, $hook, $args);
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
r44960Revert r44702, r44703, r44704 (wfInvoke and UserMailer refactor based on it) ...brion18:08, 23 December 2008

Comments

#Comment by Tim Starling (talk | contribs)   07:28, 17 December 2008

Please revert this, and convert UserMailer.php to use ordinary PHP callbacks. The interpretation of $wgHooks was a terrible design mistake, probably introduced because the author of Hooks.php was not aware of how PHP callbacks work. It is only kept for backwards compatibility and I would like to see it phased out, and replaced by a hook registration method which uses plain old PHP callbacks.

#Comment by IAlex (talk | contribs)   11:56, 17 December 2008

This also caused the following error: Strict Standards: Non-static method EmailNotification::notify() should not be called statically in /includes/UserMailer.php on line 403

#Comment by IAlex (talk | contribs)   11:57, 17 December 2008

sorry, this was for r44704.

#Comment by Brion VIBBER (talk | contribs)   18:09, 23 December 2008

Reverted in r44960

Status & tagging log