Ejegg has uploaded a new change for review.
https://gerrit.wikimedia.org/r/313622
Change subject: Remove bogus 'inflight' store
......................................................................
Remove bogus 'inflight' store
Would write things to /tmp and never do anything with them
Change-Id: I8eaf21444d1a127f2ed95a562c2188a33083b66a
---
M Core/DataStores/NullDataStore.php
M Core/Listeners/ListenerBase.php
M Core/Listeners/RestListener.php
M Core/Listeners/SoapListener.php
M PaymentProviders/Adyen/AdyenListener.php
M PaymentProviders/AstroPay/ExpatriatedMessages/AstroPayMessage.php
M SmashPig.yaml
7 files changed, 7 insertions(+), 30 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/wikimedia/fundraising/SmashPig
refs/changes/22/313622/1
diff --git a/Core/DataStores/NullDataStore.php
b/Core/DataStores/NullDataStore.php
index 36d3068..156d3cc 100644
--- a/Core/DataStores/NullDataStore.php
+++ b/Core/DataStores/NullDataStore.php
@@ -6,8 +6,7 @@
* Class NullDataStore
*
* Stub datastore to use when a datastore is not desirable to
- * be used. E.g. this can be subbed into the inflight data
- * store if that redundancy is not desired.
+ * be used.
*
* @package SmashPig\Core\DataStores
*/
diff --git a/Core/Listeners/ListenerBase.php b/Core/Listeners/ListenerBase.php
index 98a95e9..b3304ef 100644
--- a/Core/Listeners/ListenerBase.php
+++ b/Core/Listeners/ListenerBase.php
@@ -20,12 +20,8 @@
/** @var Configuration object - stores all listener configuration */
protected $c;
- /** @var KeyedOpaqueDataStore for placing messages that are in flight */
- protected $inflightStore;
-
public function __construct() {
$this->c = Context::get()->getConfiguration();
- $this->inflightStore = $this->c->object( 'data-store/inflight'
);
}
public function execute( Request $request, Response $response ) {
@@ -114,8 +110,7 @@
*
* @param ListenerMessage $msg Message object to operate on
*
- * @return bool True if the message was successfully processed.
Returning false will keep the
- * message in the bogus inflight queue.
+ * @return bool True if the message was successfully processed.
*/
protected function processMessage( ListenerMessage $msg ) {
try {
diff --git a/Core/Listeners/RestListener.php b/Core/Listeners/RestListener.php
index c007928..ff597ca 100644
--- a/Core/Listeners/RestListener.php
+++ b/Core/Listeners/RestListener.php
@@ -17,13 +17,7 @@
if ( is_array( $msgs ) ) {
foreach ( $msgs as $msg ) {
- // FIXME: this looks like an elaborate
try-catch. If there's
- // a fatal exception, the remaining
messages are toast anyway,
- // so we should... do something
different here.
- $this->inflightStore->addObject( $msg );
- if ( $this->processMessage( $msg ) ) {
-
$this->inflightStore->removeObjects( $msg );
- }
+ $this->processMessage( $msg );
}
}
$this->ackEnvelope();
diff --git a/Core/Listeners/SoapListener.php b/Core/Listeners/SoapListener.php
index 3f2f08c..9b55db1 100644
--- a/Core/Listeners/SoapListener.php
+++ b/Core/Listeners/SoapListener.php
@@ -43,8 +43,7 @@
/* --- Unfortunately because of how PHP handles SOAP
requests we cannot do the fully wrapped
loop like we could in the REST
listener. Instead it is up to the listener itself to
- do the required calls to
$this->inflightStore->addObject( $msg ), $this->processMessage( $msg ),
- and $this->inflightStore->removeObject(
$msg ).
+ do the required call to
$this->processMessage( $msg ).
It is also expected that inside the
handle() context that an exception will throw a SOAP
fault through $this->server->fault()
instead of doing a $response->kill_response() call.
diff --git a/PaymentProviders/Adyen/AdyenListener.php
b/PaymentProviders/Adyen/AdyenListener.php
index 7bdbb87..ba7d7dd 100644
--- a/PaymentProviders/Adyen/AdyenListener.php
+++ b/PaymentProviders/Adyen/AdyenListener.php
@@ -94,10 +94,9 @@
// Now process each message to the best of our ability
foreach ( $messages as $msg ) {
if ( $this->processMessage( $msg ) ) {
- Logger::debug( "Message successfully
processed, removing from inflight store." );
- $this->inflightStore->removeObjects(
$msg );
+ Logger::debug( "Message successfully
processed. Moving along..." );
} else {
- Logger::error( "Message was not
successfully processed. Leaving in inflight store!", $msg );
+ Logger::error( "Message was not
successfully processed!", $msg );
}
}
@@ -124,8 +123,7 @@
return false;
} else {
$className = get_class( $msg );
- Logger::info( "Listener message of type $className
created - adding to inflight store." );
- $this->inflightStore->addObject( $msg );
+ Logger::info( "Listener message of type $className
created." );
}
return $msg;
}
diff --git a/PaymentProviders/AstroPay/ExpatriatedMessages/AstroPayMessage.php
b/PaymentProviders/AstroPay/ExpatriatedMessages/AstroPayMessage.php
index 1dfd322..7b050d8 100644
--- a/PaymentProviders/AstroPay/ExpatriatedMessages/AstroPayMessage.php
+++ b/PaymentProviders/AstroPay/ExpatriatedMessages/AstroPayMessage.php
@@ -47,8 +47,6 @@
foreach ( $this->fields as $key ) {
$this->$key = ( array_key_exists( $key, $values ) ?
$values[$key] : '' );
}
- // Need to set the correlationId during construction
- // or inflight message store will get confused
$this->correlationId = "astropay-{$this->x_document}";
}
diff --git a/SmashPig.yaml b/SmashPig.yaml
index cd115cc..6b62643 100644
--- a/SmashPig.yaml
+++ b/SmashPig.yaml
@@ -12,12 +12,6 @@
host: 127.0.0.1
port: 6379
- # Store definitions
- inflight:
- class: SmashPig\Core\DataStores\DiskFileDataStore
- constructor-parameters:
- - /tmp/
-
payments-antifraud:
class: PHPQueue\Backend\Predis
constructor-parameters:
--
To view, visit https://gerrit.wikimedia.org/r/313622
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8eaf21444d1a127f2ed95a562c2188a33083b66a
Gerrit-PatchSet: 1
Gerrit-Project: wikimedia/fundraising/SmashPig
Gerrit-Branch: master
Gerrit-Owner: Ejegg <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
