r106259 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r106258‎ | r106259 | r106260 >
Date:22:16, 14 December 2011
Author:khorn
Status:deferred
Tags:fundraising 
Comment:
Lots of comments for DonationData.php (and protected some of the functions).
Modified paths:
  • /trunk/extensions/DonationInterface/gateway_common/DonationData.php (modified) (history)

Diff [purge]

Index: trunk/extensions/DonationInterface/gateway_common/DonationData.php
@@ -1,8 +1,16 @@
22 <?php
33
44 /**
5 - * Description of DonationData
6 - *
 5+ * DonationData
 6+ * This class is responsible for pulling all the data used by DonationInterface
 7+ * from various sources. Once pulled, DonationData will then normalize and
 8+ * sanitize the data for use by the various gateway adapters which connect to
 9+ * the payment gateways, and through those gateway adapters, the forms that
 10+ * provide the user interface.
 11+ *
 12+ * DonationData was not written to be instantiated by anything other than a
 13+ * gateway adapter (or class descended from GatewayAdapter).
 14+ *
715 * @author khorn
816 */
917 class DonationData {
@@ -10,14 +18,38 @@
1119 protected $normalized = array( );
1220 public $boss;
1321
 22+ /**
 23+ * DonationData constructor
 24+ * @param string $owning_class The name of the class that instantiated this
 25+ * instance of DonationData. This is used to grab gateway-specific functions
 26+ * and values, such as the logging function and gateway-specific global
 27+ * variables.
 28+ * @param boolean $test Indicates if DonationData has been instantiated in
 29+ * testing mode. Default is false.
 30+ * @param mixed $data An optional array of donation data that will, if
 31+ * present, circumvent the usual process of gathering the data from various
 32+ * places in $wgRequest, or 'false' to gather the data the usual way.
 33+ * Default is false.
 34+ */
1435 function __construct( $owning_class, $test = false, $data = false ) {
15 - //TODO: Actually think about this bit.
16 - // ...and keep in mind we can re-populate if it's a test or whatever. (But that may not be a good idea either)
1736 $this->boss = $owning_class;
1837 $this->gatewayID = $this->getGatewayIdentifier();
1938 $this->populateData( $test, $data );
2039 }
2140
 41+ /**
 42+ * populateData, called on construct, pulls donation data from various
 43+ * sources. Once the data has been pulled, it will handle any session data
 44+ * if present, normalize the data regardless of the source, and handle the
 45+ * caching variables.
 46+ * @global Webrequest $wgRequest
 47+ * @param boolean $test Indicates if DonationData has been instantiated in
 48+ * testing mode. Default is false.
 49+ * @param mixed $external_data An optional array of donation data that will,
 50+ * if present, circumvent the usual process of gathering the data from
 51+ * various places in $wgRequest, or 'false' to gather the data the usual way.
 52+ * Default is false.
 53+ */
2254 protected function populateData( $test = false, $external_data = false ) {
2355 global $wgRequest;
2456 $this->normalized = array( );
@@ -162,6 +194,18 @@
163195 return $this->normalized;
164196 }
165197
 198+ /**
 199+ * populateData helper function.
 200+ * If there is no external data provided upon DonationData construct, and
 201+ * the object was instantiated in test mode, populateData_Test in intended
 202+ * to provide a baseline minimum of data with which to run tests without
 203+ * exploding.
 204+ * Populates $this->normalized.
 205+ * TODO: Implement an override for the test data, in the event that a
 206+ * partial data array is provided when DonationData is instantiated.
 207+ * @param array $testdata Intended to implement an override for any values
 208+ * that may be provided on instantiation.
 209+ */
166210 protected function populateData_Test( $testdata = false ) {
167211 // define arrays of cc's and cc #s for random selection
168212 $cards = array( 'american' );
@@ -232,9 +276,11 @@
233277 }
234278
235279 /**
236 - * Tells you if a value is something or not.
237 - * @param string $key The field you would like to determine if it exists or not.
238 - * @return boolean true if the field is something. False if it is null, or an empty string.
 280+ * Tells you if a value in $this->normalized is something or not.
 281+ * @param string $key The field you would like to determine if it exists in
 282+ * a usable way or not.
 283+ * @return boolean true if the field is something. False if it is null, or
 284+ * an empty string.
239285 */
240286 public function isSomething( $key ) {
241287 if ( array_key_exists( $key, $this->normalized ) ) {
@@ -249,7 +295,8 @@
250296
251297 /**
252298 * getVal_Escaped
253 - * @param string $key The data field you would like to retrieve.
 299+ * @param string $key The data field you would like to retrieve. Pulls the
 300+ * data from $this->normalized if it is found to be something.
254301 * @return mixed The normalized and escaped value of that $key.
255302 */
256303 public function getVal_Escaped( $key ) {
@@ -265,7 +312,8 @@
266313 /**
267314 * getVal
268315 * For Internal Use Only! External objects should use getVal_Escaped.
269 - * @param string $key The data field you would like to retrieve.
 316+ * @param string $key The data field you would like to retrieve directly
 317+ * from $this->normalized.
270318 * @return mixed The normalized value of that $key.
271319 */
272320 protected function getVal( $key ) {
@@ -278,14 +326,23 @@
279327
280328 /**
281329 * Sets a key in the normalized data array, to a new value.
 330+ * This function should only ever be used for keys that are not listed in
 331+ * DonationData::getCalculatedFields().
 332+ * TODO: If the $key is listed in DonationData::getCalculatedFields(), use
 333+ * DonationData::addData() instead. Or be a jerk about it and throw an
 334+ * exception. (Personally I like the second one)
282335 * @param string $key The key you want to set.
283336 * @param string $val The value you'd like to assign to the key.
284337 */
285 - function setVal( $key, $val ) {
 338+ public function setVal( $key, $val ) {
286339 $this->normalized[$key] = $val;
287340 }
288341
289 - function expunge( $key ) {
 342+ /**
 343+ * Removes a value from $this->normalized.
 344+ * @param type $key
 345+ */
 346+ public function expunge( $key ) {
290347 if ( array_key_exists( $key, $this->normalized ) ) {
291348 unset( $this->normalized[$key] );
292349 }
@@ -294,11 +351,12 @@
295352 /**
296353 * Returns an array of all the fields that get re-calculated during a
297354 * normalize.
298 - * This will most likely be used on the outside when in the process of
299 - * adding data.
 355+ * This can be used on the outside when in the process of changing data,
 356+ * particularly if any of the recalculted fields need to be restaged by the
 357+ * gateway adapter.
300358 * @return array An array of values matching all recauculated fields.
301359 */
302 - function getCalculatedFields() {
 360+ public function getCalculatedFields() {
303361 $fields = array(
304362 'utm_source',
305363 'amount',
@@ -317,9 +375,13 @@
318376
319377 /**
320378 * Normalizes the current set of data, just after it's been
321 - * pulled (or re-pulled) from a source.
 379+ * pulled (or re-pulled) from a data source.
 380+ * Care should be taken in the normalize helper functions to write code in
 381+ * such a way that running them multiple times on the same array won't cause
 382+ * the data to stroll off into the sunset: Normalize will definitely need to
 383+ * be called multiple times against the same array.
322384 */
323 - function normalize() {
 385+ protected function normalize() {
324386 if ( !empty( $this->normalized ) ) {
325387 $this->setUtmSource();
326388 $this->setNormalizedAmount();
@@ -337,8 +399,17 @@
338400 /**
339401 * normalize helper function
340402 * Sets the form class we will be using.
 403+ * In the case that we are using forms, form_name will be harvested from
 404+ * $wgRequest by populateData. If we are coming from somewhere that does not
 405+ * use a form interface (like an api call), this logic should be skipped.
 406+ *
 407+ * For any specified form, if it is enabled and available, the class would
 408+ * have been autoloaded at this point. If it is not enabled and available,
 409+ * we will check the default for the calling gateway, and failing that,
 410+ * throw an exception.
 411+ *
341412 */
342 - function setFormClass(){
 413+ protected function setFormClass(){
343414 //don't actually try to load the forms here... but do determine if what we've got in there will load or not.
344415 //Elsewise, set it to the default.
345416
@@ -362,8 +433,10 @@
363434 /**
364435 * normalize helper function
365436 * Setting the country correctly.
 437+ * If we have no country, we try to get something rational through GeoIP
 438+ * lookup.
366439 */
367 - function setCountry() {
 440+ protected function setCountry() {
368441 global $wgRequest;
369442 if ( !$this->isSomething('country') ){
370443 // If no country was passed, try to do GeoIP lookup
@@ -381,8 +454,10 @@
382455 /**
383456 * normalize helper function
384457 * Setting the currency code correctly.
 458+ * Historically, this value could come in through 'currency' or
 459+ * 'currency_code'. After this fires, we will only have 'currency_code'.
385460 */
386 - function setCurrencyCode() {
 461+ protected function setCurrencyCode() {
387462 global $wgRequest;
388463
389464 //at this point, we can have either currency, or currency_code.
@@ -413,7 +488,7 @@
414489 * If a contribution tracking id is already present, no new rows will be
415490 * assigned.
416491 */
417 - function handleContributionTrackingID(){
 492+ protected function handleContributionTrackingID(){
418493 if ( !$this->isSomething( 'contribution_tracking_id' ) &&
419494 ( !$this->isCaching() ) ){
420495 $this->saveContributionTracking();
@@ -426,7 +501,7 @@
427502 * calculate it from the data fields more than once.
428503 * @return boolean true if we are going to be caching, false if we aren't.
429504 */
430 - function isCaching(){
 505+ public function isCaching(){
431506
432507 static $cache = null;
433508
@@ -455,7 +530,7 @@
456531 * Takes all possible sources for the intended donation amount, and
457532 * normalizes them into the 'amount' field.
458533 */
459 - function setNormalizedAmount() {
 534+ protected function setNormalizedAmount() {
460535 if ( !($this->isSomething( 'amount' )) || !(preg_match( '/^\d+(\.(\d+)?)?$/', $this->getVal( 'amount' ) ) ) ) {
461536 if ( $this->isSomething( 'amountGiven' ) && preg_match( '/^\d+(\.(\d+)?)?$/', $this->getVal( 'amountGiven' ) ) ) {
462537 $this->setVal( 'amount', number_format( $this->getVal( 'amountGiven' ), 2, '.', '' ) );
@@ -473,7 +548,7 @@
474549 * comes in populated or not, and where it came from.
475550 * @return null
476551 */
477 - function setNormalizedOrderIDs() {
 552+ protected function setNormalizedOrderIDs() {
478553 //basically, we need a new order_id every time we come through here, but if there's an internal already there,
479554 //we want to use that one internally. So.
480555 //Exception: If we pass in an order ID in the querystring: Don't mess with it.
@@ -493,7 +568,7 @@
494569 /**
495570 * Generate an order id exactly once for this go-round.
496571 */
497 - static function generateOrderId() {
 572+ protected static function generateOrderId() {
498573 static $order_id = null;
499574 if ( $order_id === null ) {
500575 $order_id = ( double ) microtime() * 1000000 . mt_rand( 1000, 9999 );
@@ -515,6 +590,12 @@
516591 $value = htmlspecialchars( $value, $flags, 'UTF-8', $double_encode );
517592 }
518593
 594+ /**
 595+ * log: This grabs the adapter class that instantiated DonationData, and
 596+ * uses its log function.
 597+ * @param string $message The message to log.
 598+ * @param type $log_level
 599+ */
519600 protected function log( $message, $log_level=LOG_INFO ) {
520601 $c = $this->getAdapterClass();
521602 if ( $c && is_callable( array( $c, 'log' ) )){
@@ -522,6 +603,14 @@
523604 }
524605 }
525606
 607+ /**
 608+ * getGatewayIdentifier
 609+ * This grabs the adapter class that instantiated DonationData, and returns
 610+ * the result of its 'getIdentifier' function. Used for normalizing the
 611+ * 'gateway' value, and stashing and retrieving the edit token (and other
 612+ * things, where needed) in the session.
 613+ * @return type
 614+ */
526615 protected function getGatewayIdentifier() {
527616 $c = $this->getAdapterClass();
528617 if ( $c && is_callable( array( $c, 'getIdentifier' ) ) ){
@@ -531,6 +620,16 @@
532621 }
533622 }
534623
 624+ /**
 625+ * getGatewayGlobal
 626+ * This grabs the adapter class that instantiated DonationData, and returns
 627+ * the result of its 'getGlobal' function for the $varname passed in. Used
 628+ * to determine gateway-specific configuration settings.
 629+ * @param string $varname the global variable (minus prefix) that we want to
 630+ * check.
 631+ * @return mixed The value of the gateway global if it exists. Else, the
 632+ * value of the Donation Interface global if it exists. Else, null.
 633+ */
535634 protected function getGatewayGlobal( $varname ) {
536635 $c = $this->getAdapterClass();
537636 if ( $c && is_callable( array( $c, 'getGlobal' ) ) ){
@@ -611,8 +710,18 @@
612711 }
613712 }
614713
 714+ /**
 715+ * getAnnoyingOrderIDLogLinePrefix
 716+ * Constructs and returns the annoying order ID log line prefix.
 717+ * This has moved from being annoyingly all over the place in the edit token
 718+ * logging code before it was functionalized, to being annoying to look at
 719+ * in the logs because the two numbers in the prefix are frequently
 720+ * identical (and large).
 721+ * TODO: Determine if anything actually looks at both of those numbers, in
 722+ * order to make this less annoying. Rename on success.
 723+ * @return string Annoying Order ID Log Line Prefix in all its dubious glory.
 724+ */
615725 protected function getAnnoyingOrderIDLogLinePrefix() {
616 - //TODO: ...aww. But it's so descriptive.
617726 return $this->getVal( 'order_id' ) . ' ' . $this->getVal( 'i_order_id' ) . ': ';
618727 }
619728
@@ -649,8 +758,10 @@
650759 }
651760
652761 /**
 762+ * token_refreshAllTokenEverything
653763 * In the case where we have an expired session (token mismatch), we go
654 - * ahead and fix it for 'em for their next post.
 764+ * ahead and fix it for 'em for their next post. We do this by refreshing
 765+ * everything that has to do with the edit token.
655766 */
656767 protected function token_refreshAllTokenEverything(){
657768 $unsalted = self::token_generateToken();
@@ -661,6 +772,13 @@
662773 $this->setVal( 'token', $salted );
663774 }
664775
 776+ /**
 777+ * token_applyMD5AndSalt
 778+ * Takes a clear-text token, and returns the MD5'd result of the token plus
 779+ * the configured gateway salt.
 780+ * @param string $clear_token The original, unsalted, unencoded edit token.
 781+ * @return string The salted and MD5'd token.
 782+ */
665783 protected function token_applyMD5AndSalt( $clear_token ){
666784 $salt = $this->getGatewayGlobal( 'Salt' );
667785
@@ -674,9 +792,10 @@
675793
676794
677795 /**
678 - * Generate a token string
679 - *
680 - * @var mixed $padding
 796+ * token_generateToken
 797+ * Generate a random string to be used as an edit token.
 798+ * @var string $padding A string with which we could pad out the random hex
 799+ * further.
681800 * @return string
682801 */
683802 public static function token_generateToken( $padding = '' ) {
@@ -685,7 +804,11 @@
686805 }
687806
688807 /**
689 - * Determine the validity of a token
 808+ * token_matchEditToken
 809+ * Determine the validity of a token by checking it against the salted
 810+ * version of the clear-text token we have already stored in the session.
 811+ * On failure, it resets the edit token both in the session and in the form,
 812+ * so they will match on the user's next load.
690813 *
691814 * @var string $val
692815 * @return bool
@@ -702,10 +825,14 @@
703826 }
704827
705828 /**
 829+ * ensureSession
706830 * Ensure that we have a session set for the current user.
707 - *
708831 * If we do not have a session set for the current user,
709832 * start the session.
 833+ * BE CAREFUL with this one, as creating sessions willy-nilly will break
 834+ * squid caching for reasons that are not immediately obvious.
 835+ * (See DonationData::doCacheStuff, and basically everything about setting
 836+ * headers in $wgOut)
710837 */
711838 protected static function ensureSession() {
712839 // if the session is already started, do nothing
@@ -717,6 +844,7 @@
718845 }
719846
720847 /**
 848+ * sessionExists
721849 * Checks to see if the session exists without actually creating one.
722850 * @return bool true if we have a session, otherwise false.
723851 */
@@ -726,16 +854,30 @@
727855 return false;
728856 }
729857
 858+ /**
 859+ * token_checkTokens
 860+ * The main function to check the salted and MD5'd token we should have
 861+ * saved and gathered from $wgRequest, against the clear-text token we
 862+ * should have saved to the user's session.
 863+ * token_getSaltedSessionToken() will start off the process if this is a
 864+ * first load, and there's no saved token in the session yet.
 865+ * @global Webrequest $wgRequest
 866+ * @staticvar string $match
 867+ * @return type
 868+ */
730869 public function token_checkTokens() {
731870 global $wgRequest;
732 - static $match = null;
 871+ static $match = null; //because we only want to do this once per load.
733872
734873 if ( $match === null ) {
735874 if ( $this->isCaching() ){
736875 //This makes sense.
737876 //If all three conditions for caching are currently true, the
738877 //last thing we want to do is screw it up by setting a session
739 - //token before the page loads.
 878+ //token before the page loads, because sessions break caching.
 879+ //The API will set the session and form token values immediately
 880+ //after that first page load, which is all we care about saving
 881+ //in the cache anyway.
740882 return true;
741883 }
742884
@@ -815,6 +957,10 @@
816958 * because the form elements for comment anonymization and email opt-out
817959 * are backwards (they are really opt-in) relative to contribution_tracking
818960 * (which is opt-out), we need to reverse the values.
 961+ * Difficulty here is compounded by the fact that these values come from
 962+ * checkboxes on forms, which simply don't make it to $wgRequest if they are
 963+ * not checked... or not present in the form at all. In other words, this
 964+ * situation is painful and you probably want to leave it alone.
819965 * NOTE: If you prune here, and there is a paypal redirect, you will have
820966 * problems with the email-opt/optout and comment-option/anonymous.
821967 */
@@ -945,6 +1091,15 @@
9461092 }
9471093 }
9481094
 1095+ /**
 1096+ * addDonorDataToSession
 1097+ * Adds all the fields that are required to make a well-formed stomp
 1098+ * message, to the user's session for later use. This mechanism is used by gateways that
 1099+ * have a user being directed somewhere out of our control, and then coming
 1100+ * back to complete a transaction. (Globalcollect Hosted Credit Card, for
 1101+ * example)
 1102+ *
 1103+ */
9491104 public function addDonorDataToSession() {
9501105 self::ensureSession();
9511106 $donordata = $this->getStompMessageFields();
@@ -1011,6 +1166,15 @@
10121167 session_destroy(); //killed on the server.
10131168 }
10141169
 1170+ /**
 1171+ * addData
 1172+ * Adds an array of data to the normalized array, and then re-normalizes it.
 1173+ * NOTE: If any gateway is using this function, it should then immediately
 1174+ * repopulate its own data set with the DonationData source, and then
 1175+ * re-stage values as necessary.
 1176+ * @param array $newdata An array of data to integrate with the existing
 1177+ * data held by the DonationData object.
 1178+ */
10151179 public function addData( $newdata ) {
10161180 if ( is_array( $newdata ) && !empty( $newdata ) ) {
10171181 foreach ( $newdata as $key => $val ) {
@@ -1022,6 +1186,11 @@
10231187 $this->normalize();
10241188 }
10251189
 1190+ /**
 1191+ * incrementNumAttempt
 1192+ * Adds one to the 'numAttempt' field we use to keep track of how many times
 1193+ * a donor has tried to do something.
 1194+ */
10261195 public function incrementNumAttempt() {
10271196 if ( $this->isSomething( 'numAttempt' ) ) {
10281197 $attempts = $this->getVal( 'numAttempt' );
@@ -1034,6 +1203,10 @@
10351204 }
10361205 }
10371206
 1207+ /**
 1208+ * Gets the name of the adapter class that instantiated DonationData.
 1209+ * @return mixed The name of the class if it exists, or false.
 1210+ */
10381211 protected function getAdapterClass(){
10391212 if ( class_exists( $this->boss ) ) {
10401213 return $this->boss;

Follow-up revisions

RevisionCommit summaryAuthorDate
r112287MFT r101785, r105938, r105941, r105953, r106109, r106158, r106259, r106366, r...khorn01:29, 24 February 2012

Status & tagging log