From a5259627651ce4decfec634b243cd82ff4451a89 Mon Sep 17 00:00:00 2001 From: Shumoapp Date: Tue, 9 Jun 2015 02:20:23 +0300 Subject: [PATCH 01/11] php 5.4 compatibility --- application/Initialize.php | 2 + application/LiveCart.php | 4 ++ .../model/session/BaseSessionHandler.php | 55 +++++++++++++++++++ .../model/session/DatabaseSessionHandler.php | 6 +- installdata/sql/state/all.sql | 3 + 5 files changed, 67 insertions(+), 3 deletions(-) create mode 100644 application/model/session/BaseSessionHandler.php diff --git a/application/Initialize.php b/application/Initialize.php index e75aaedfe..cc39c7682 100644 --- a/application/Initialize.php +++ b/application/Initialize.php @@ -63,4 +63,6 @@ function __autoload($className) ClassLoader::import('application.model.system.*'); ClassLoader::import('application.LiveCart'); +date_default_timezone_set('Europe/Riga'); + ?> \ No newline at end of file diff --git a/application/LiveCart.php b/application/LiveCart.php index c42ede0fb..298601fd7 100644 --- a/application/LiveCart.php +++ b/application/LiveCart.php @@ -166,6 +166,10 @@ public function __construct() { error_reporting(E_ALL & ~E_DEPRECATED); } + elseif(phpversion() >= '5.4') + { + error_reporting(E_ALL & ~E_DEPRECATED & ~E_STRICT); + } else { error_reporting(E_ALL); diff --git a/application/model/session/BaseSessionHandler.php b/application/model/session/BaseSessionHandler.php new file mode 100644 index 000000000..a40fc53b4 --- /dev/null +++ b/application/model/session/BaseSessionHandler.php @@ -0,0 +1,55 @@ +getID(); + + if ($id != $this->userID) + { + $this->userID = $id; + $this->forceUpdate = true; + } + } + + public function updateCacheTimestamp() + { + $this->cacheUpdated = time(); + $this->forceUpdate = true; + } + + public function getCacheUpdateTime() + { + return $this->cacheUpdated; + } +} + +?> diff --git a/application/model/session/DatabaseSessionHandler.php b/application/model/session/DatabaseSessionHandler.php index 402569a0d..e18b38ce7 100644 --- a/application/model/session/DatabaseSessionHandler.php +++ b/application/model/session/DatabaseSessionHandler.php @@ -1,6 +1,6 @@ Date: Tue, 9 Jun 2015 21:22:41 +0300 Subject: [PATCH 02/11] Fixing getTemplatePath() method in the Email model class to properly return the customized template path. When editing module email templates from admin>customize>module>moduleName, the resulting templates are stored in livecartRoot/storage/customize/view/module/moduleName/email/en/ This fix allows the mailing scripts to get the customized template instead of the one from livecartRoot/module/moduleName/application/view/email/en/ --- application/model/Email.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/application/model/Email.php b/application/model/Email.php index 8eb6b87c8..fe8b7c11a 100755 --- a/application/model/Email.php +++ b/application/model/Email.php @@ -211,8 +211,10 @@ protected function getTemplatePath($templateFile) $paths = array( 'storage.customize.view.email.' . $locale . '.' . $templateFile, + 'storage.customize.view.module.' . $module .'.email.'. $locale . '.' . $path, 'module.' . $module . '.application.view.email.' . $locale . '.' . $path, 'storage.customize.view.email.en.' . $templateFile, + 'storage.customize.view.module.' . $module .'.email.en.' . '.' . $path, 'module.' . $module . '.application.view.email.en.' . $path, ); } @@ -377,4 +379,4 @@ public function send() } } -?> \ No newline at end of file +?> From a585399322ea009503992a571dc7ab1948fa1b63 Mon Sep 17 00:00:00 2001 From: Shumoapp Date: Wed, 10 Jun 2015 00:54:37 +0300 Subject: [PATCH 03/11] Workaround for the issue in tabTitleFromTemplateId() method from Backend.Template js class. For templates that are deep into the templates tree (admin>customize>edit templates), the dhtmlXTreeObject.parentObject.id returned an array, while a string was expected. As a result tabTitleFromTemplateId would die and not allow to edit the template. dhtmlxtree is some proprietary library, so I won't try to fix it. --- public/javascript/backend/Template.js | 1 + 1 file changed, 1 insertion(+) diff --git a/public/javascript/backend/Template.js b/public/javascript/backend/Template.js index 6910e60ab..33e6761ff 100644 --- a/public/javascript/backend/Template.js +++ b/public/javascript/backend/Template.js @@ -171,6 +171,7 @@ Backend.Template.prototype = tabTitleFromTemplateId: function(id) { + if (id instanceof Array) id= id.pop(); return id.split("/").pop(); }, From 9f726980573f30cdecbe206c9c84e8dc36d15fbc Mon Sep 17 00:00:00 2001 From: Shumoapp Date: Thu, 11 Jun 2015 03:18:49 +0300 Subject: [PATCH 04/11] Setting up test environment. Fixing an SQL error that prevented some test from running. --- .../EavSpecificationManagerCommon.php | 6 +-- test/test/Initialize.php | 51 +++++++++---------- test/test/LiveCartTest.php | 6 +++ test/test/locale/MakeTextTest.php | 2 +- 4 files changed, 34 insertions(+), 31 deletions(-) diff --git a/application/model/eavcommon/EavSpecificationManagerCommon.php b/application/model/eavcommon/EavSpecificationManagerCommon.php index 1bd875c53..cf860d3e0 100644 --- a/application/model/eavcommon/EavSpecificationManagerCommon.php +++ b/application/model/eavcommon/EavSpecificationManagerCommon.php @@ -760,11 +760,11 @@ private static function fetchRawSpecificationData($class, $objectIDs, $fullSpeci $group = $groupClass . '.position AS SpecFieldGroupPosition, ' . $groupClass . '.name AS SpecFieldGroupName, '; $query = ' - SELECT ' . $dateClass . '.*, NULL AS valueID, NULL AS specFieldValuePosition, ' . $group . $fieldClass . '.* as valueID FROM ' . $dateClass . ' ' . $cond . ' + SELECT ' . $dateClass . '.*, NULL AS valueID, NULL AS specFieldValuePosition, ' . $group . $fieldClass . '.* FROM ' . $dateClass . ' ' . $cond . ' UNION - SELECT ' . $stringClass . '.*, NULL, NULL AS specFieldValuePosition, ' . $group . $fieldClass . '.* as valueID FROM ' . $stringClass . ' ' . $cond . ' + SELECT ' . $stringClass . '.*, NULL, NULL AS specFieldValuePosition, ' . $group . $fieldClass . '.* FROM ' . $stringClass . ' ' . $cond . ' UNION - SELECT ' . $numericClass . '.*, NULL, NULL AS specFieldValuePosition, ' . $group . $fieldClass . '.* as valueID FROM ' . $numericClass . ' ' . $cond . ' + SELECT ' . $numericClass . '.*, NULL, NULL AS specFieldValuePosition, ' . $group . $fieldClass . '.* FROM ' . $numericClass . ' ' . $cond . ' UNION SELECT ' . $valueItemClass . '.' . $objectColumn . ', ' . $valueItemClass . '.' . $fieldColumn . ', ' . $valueClass . '.value, ' . $valueClass . '.ID, ' . $valueClass . '.position, ' . $group . $fieldClass . '.* FROM ' . $valueItemClass . ' diff --git a/test/test/Initialize.php b/test/test/Initialize.php index eb3db812f..ed7dff706 100644 --- a/test/test/Initialize.php +++ b/test/test/Initialize.php @@ -10,47 +10,44 @@ if (!defined('TEST_INITIALIZED')) { - // load classes and mount paths - $cd = getcwd(); + // load classes and mount paths + $cd = getcwd(); - chdir(dirname(dirname(__FILE__))); - chdir('..'); + chdir(dirname(dirname(__FILE__))); + chdir('..'); - include_once('application/Initialize.php'); + include_once('application/Initialize.php'); - $arPath = realpath(getcwd() . DIRECTORY_SEPARATOR . 'library' . DIRECTORY_SEPARATOR . 'activerecord' . DIRECTORY_SEPARATOR . 'ActiveRecord.php'); - include_once($arPath); - ActiveRecord::setDSN('mysql://root@server/livecart'); + $arPath = realpath(getcwd() . DIRECTORY_SEPARATOR . 'library' . DIRECTORY_SEPARATOR . 'activerecord' . DIRECTORY_SEPARATOR . 'ActiveRecord.php'); + include_once($arPath); + ActiveRecord::setDSN('mysql://testerer@localhost/livecart'); - // set unittest and simpletest library root directory - $libDir = dirname(dirname(__FILE__)) . '/_library/'; - ClassLoader::mountPath('phpunit', realpath($libDir . 'phpunit/')); - ClassLoader::mountPath('unittest', realpath($libDir . 'unittest') . '/'); - ClassLoader::mountPath('testdir', dirname(__FILE__).'/'); - ClassLoader::import("phpunit.*"); - ClassLoader::import("unittest.*"); - ClassLoader::import("testdir.*"); + // set unittest and simpletest library root directory + $libDir = dirname(dirname(__FILE__)) . '/_library/'; + ClassLoader::mountPath('unittest', realpath($libDir . 'unittest') . '/'); + ClassLoader::mountPath('testdir', dirname(__FILE__).'/'); + ClassLoader::import("unittest.*"); + ClassLoader::import("testdir.*"); - //ClassLoader::load('unit_tester'); - //ClassLoader::load('mock_objects'); - //ClassLoader::load('reporter'); + //ClassLoader::load('unit_tester'); + //ClassLoader::load('mock_objects'); + //ClassLoader::load('reporter'); - ClassLoader::import('unittest.UnitTest'); - chdir($cd); + ClassLoader::import('unittest.UnitTest'); + chdir($cd); - define('TEST_INITIALIZED', true); + define('TEST_INITIALIZED', true); - ClassLoader::import('application.LiveCart'); - UnitTest::setApplication(new LiveCart); - UnitTest::getApplication()->getConfig()->setAutoSave(false); - UnitTest::getApplication()->getConfig()->set('EMAIL_METHOD', 'FAKE'); + ClassLoader::import('application.LiveCart'); + UnitTest::setApplication(new LiveCart); + UnitTest::getApplication()->getConfig()->setAutoSave(false); + UnitTest::getApplication()->getConfig()->set('EMAIL_METHOD', 'FAKE'); } ClassLoader::import('application.system.*'); ClassLoader::import('library.locale.Locale'); ClassLoader::import('test.mock.Swift_Connection_Fake'); -require_once('UTStandalone.php'); require_once('LiveCartTest.php'); ?> diff --git a/test/test/LiveCartTest.php b/test/test/LiveCartTest.php index 5bc8cce80..eb140f55f 100644 --- a/test/test/LiveCartTest.php +++ b/test/test/LiveCartTest.php @@ -9,6 +9,12 @@ abstract class LiveCartTest extends PHPUnit_Framework_TestCase public function setUp() { + # Do not convert warnings to exceptions. + PHPUnit_Framework_Error_Warning::$enabled = FALSE; + + # Do not convert notice and strict to exceptions + PHPUnit_Framework_Error_Notice::$enabled = FALSE; + parent::setUp(); $this->config = ActiveRecordModel::getApplication()->getConfig(); diff --git a/test/test/locale/MakeTextTest.php b/test/test/locale/MakeTextTest.php index 5d8e67c88..0a7b82de2 100644 --- a/test/test/locale/MakeTextTest.php +++ b/test/test/locale/MakeTextTest.php @@ -1,6 +1,6 @@ Date: Wed, 17 Jun 2015 01:39:01 +0300 Subject: [PATCH 05/11] Fixing the Product->getProductsPurchasedTogether() model method. It was not returning products when the queried product was a product with variations. Apparently the variations are added as child products to a parent product, and the child ID was recorded in the OrderedItem table. Also added is a PHPUnit test to show the problem. --- application/model/product/Product.php | 18 ++++++++++- .../application/model/product/ProductTest.php | 30 +++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/application/model/product/Product.php b/application/model/product/Product.php index d618169bb..f556af7ae 100644 --- a/application/model/product/Product.php +++ b/application/model/product/Product.php @@ -1395,6 +1395,22 @@ public function getProductsPurchasedTogether($limit = null, $enabledOnly = false $limit = 0; } + $sql = 'select ID from Product where isEnabled=1 and (ID ='.$this->getID().' or parentID='.$this->getID().')'; + $products = ActiveRecord::getDataBySql($sql); + $childProducts = array(); + if (count($products>0)) + { + foreach ($products as $key => $prod) + { + $childProducts[]=$prod['ID']; + } + } + else + { + $childProducts[] = $this->getID(); + } + + $sql = 'SELECT COUNT(*) AS cnt, COALESCE(ParentProduct.ID, OtherItem.productID) AS ID FROM OrderedItem LEFT JOIN @@ -1406,7 +1422,7 @@ public function getProductsPurchasedTogether($limit = null, $enabledOnly = false LEFT JOIN Product AS ParentProduct ON Product.parentID=ParentProduct.ID WHERE - CustomerOrder.isFinalized=1 AND OrderedItem.productID=' . $this->getID() . ' AND OtherItem.productID!=' . $this->getID() . ($enabledOnly? ' AND Product.isEnabled=1' : '') . ' + CustomerOrder.isFinalized=1 AND OrderedItem.productID in (' . implode(',',$childProducts) . ') AND OtherItem.productID not in (' . implode(',',$childProducts).')' . ($enabledOnly? ' AND Product.isEnabled=1' : '') . ' GROUP BY OtherItem.productID ORDER BY diff --git a/test/test/application/model/product/ProductTest.php b/test/test/application/model/product/ProductTest.php index 180c3a864..b5c7c4ea0 100644 --- a/test/test/application/model/product/ProductTest.php +++ b/test/test/application/model/product/ProductTest.php @@ -821,6 +821,36 @@ public function testVariationInventory() $this->assertEquals(20, $this->product->stockCount->get()); } + /** + * Testing if getProductsPurchasedTogether() returns correct results when variations are purchased + */ + public function testVariationsPurchasedTogether() + { + $this->initOrder(); + + $parentProduct = $this->product; + + $childProduct = $parentProduct->createChildProduct(); + $childProduct->isEnabled->set(true); + $childProduct->stockCount->set(1); + $childProduct->save(); + + $product = Product::getInstanceByID($childProduct->getID(), true); + + $this->order->addProduct($this->products[0], 1); + $this->order->addProduct($this->products[1], 2); + $this->order->addProduct($product, 1); + $this->order->save(); + $this->order->finalize(); + + /* + * Because variations are added as child products, if an order contains a child product and regular products, + * querying getProductsPurchasedTogether() of the parent product should return the regular products. + * */ + $purchasedTogetherProducts = $parentProduct->getProductsPurchasedTogether(2); + $this->assertEquals(2, count($purchasedTogetherProducts)); + } + function test_SuiteTearDown() { ActiveRecordModel::rollback(); From c0473c23c749ef2492c281cf9f83c12368b61f1c Mon Sep 17 00:00:00 2001 From: Shumoapp Date: Fri, 19 Jun 2015 01:45:42 +0300 Subject: [PATCH 06/11] Fixed the address delete feature. Up till now if you delete the default address, the users could not login, because the default address associated with the user didn't change. (when there is custom field attached to the address - some error in EavSpecificationManagerCommon) Now the default addresses are set to the first of the remaining or null, and the users can login, even if they have no addresses at all. Also added are two PHPUnit tests that show the problem, and LiveCartTest is fixed to work with controller tests. --- application/controller/UserController.php | 18 +++++- test/test/LiveCartTest.php | 41 ++++++++---- .../controller/UserControllerTest.php | 64 +++++++++++++++++++ 3 files changed, 108 insertions(+), 15 deletions(-) diff --git a/application/controller/UserController.php b/application/controller/UserController.php index cda6b585d..51e572bab 100644 --- a/application/controller/UserController.php +++ b/application/controller/UserController.php @@ -785,7 +785,14 @@ public function deleteShippingAddress() { try { - return $this->deleteAddress(ShippingAddress::getUserAddress($this->request->get('id'), $this->user)); + $res = $this->deleteAddress(ShippingAddress::getUserAddress($this->request->get('id'), $this->user)); + + $remainingAddresses = $this->user->getShippingAddressSet(); + //ARSet->get() returns null if there is no such record index + $this->user->defaultShippingAddress->set($remainingAddresses->get(0)); + $this->user->save(); + + return $res; } catch (ARNotFoundException $e) { @@ -800,7 +807,14 @@ public function deleteBillingAddress() { try { - return $this->deleteAddress(BillingAddress::getUserAddress($this->request->get('id'), $this->user)); + $res = $this->deleteAddress(BillingAddress::getUserAddress($this->request->get('id'), $this->user)); + + $remainingAddresses = $this->user->getBillingAddressSet(); + //ARSet->get() returns null if there is no such record index + $this->user->defaultBillingAddress->set($remainingAddresses->get(0)); + $this->user->save(); + + return $res; } catch (ARNotFoundException $e) { diff --git a/test/test/LiveCartTest.php b/test/test/LiveCartTest.php index eb140f55f..d48fe946b 100644 --- a/test/test/LiveCartTest.php +++ b/test/test/LiveCartTest.php @@ -6,6 +6,8 @@ abstract class LiveCartTest extends PHPUnit_Framework_TestCase { protected $config; + protected $userEmail = 'test@test.com'; + protected $userPassword = 'testerer'; public function setUp() { @@ -29,6 +31,11 @@ public function setUp() ActiveRecordModel::executeUpdate('DELETE FROM DeliveryZone'); $this->getApplication()->clearCachedVars(); + + if ($this instanceof ControllerTestCase) + { + $this->request = self::getApplication()->getRequest(); + } } public function tearDown() @@ -66,17 +73,12 @@ protected function setUpCurrency() } } - protected function initOrder() + protected function initUser() { - // set up currency - $this->setUpCurrency(); - $this->usd->decimalCount->set(2); - $this->usd->clearRoundingRules(); - $this->usd->save(); - - // initialize order - ActiveRecordModel::executeUpdate('DELETE FROM User WHERE email="test@test.com"'); - $user = User::getNewInstance('test@test.com'); + ActiveRecordModel::executeUpdate('DELETE FROM User WHERE email="'.$this->userEmail.'"'); + $user = User::getNewInstance($this->userEmail); + $user->setPassword($this->userPassword); + $user->isEnabled->set(true); $user->save(); $this->user = $user; @@ -93,10 +95,23 @@ protected function initOrder() $address->save(); $shipping = ShippingAddress::getNewInstance($user, $address); $shipping->save(); + } - $this->order = CustomerOrder::getNewInstance($user); - $this->order->shippingAddress->set($shipping->userAddress->get()); - $this->order->billingAddress->set($billing->userAddress->get()); + protected function initOrder() + { + // set up currency + $this->setUpCurrency(); + $this->usd->decimalCount->set(2); + $this->usd->clearRoundingRules(); + $this->usd->save(); + + // initialize user + $this->initUser(); + + // initialize order + $this->order = CustomerOrder::getNewInstance($this->user); + $this->order->shippingAddress->set($this->user->defaultShippingAddress->get()->userAddress->get()); + $this->order->billingAddress->set($this->user->defaultBillingAddress->get()->userAddress->get()); // set up products $product = Product::getNewInstance(Category::getInstanceById(Category::ROOT_ID), 'test1'); diff --git a/test/test/application/controller/UserControllerTest.php b/test/test/application/controller/UserControllerTest.php index 26ee7e881..2a6f75d7b 100644 --- a/test/test/application/controller/UserControllerTest.php +++ b/test/test/application/controller/UserControllerTest.php @@ -168,6 +168,70 @@ public function testUserCheckoutWithDifferentAddresses() } + public function testDeleteDefaultShippingAddress() + { + //add another address + $defaultShippingAddress = $this->user->defaultShippingAddress->get()->userAddress->get(); + $address = clone $defaultShippingAddress; + $address->save(); + $shipping = ShippingAddress::getNewInstance($this->user, $address); + $shipping->save(); + + //delete the default shipping address + $defaultShippingAddressId = $this->user->defaultShippingAddress->get()->getID(); + $this->request->set('id', $defaultShippingAddressId); + $this->controller->deleteShippingAddress(); + + //verify that the address was deleted + $this->assertEquals(1, count($this->user->getShippingAddressArray())); + + //verify that the stored defaultShippingAddressID has changed + if (null!=$this->user->defaultShippingAddress->get()) + { + $this->assertNotEquals($defaultShippingAddressId, $this->user->defaultShippingAddress->get()->getID()); + } + + //delete all addresses and verify that the stored defaultShippingAddressID is null + foreach ($this->user->getShippingAddressArray() as $address) + { + $this->request->set('id', $address['ID']); + $this->controller->deleteShippingAddress(); + } + $this->assertEquals(null, $this->user->defaultShippingAddress->get()); + } + + public function testDeleteDefaultBillingAddress() + { + //add another address + $defaultBillingAddress = $this->user->defaultBillingAddress->get()->userAddress->get(); + $address = clone $defaultBillingAddress; + $address->save(); + $billing = BillingAddress::getNewInstance($this->user, $address); + $billing->save(); + + //delete the default billing address + $defaultBillingAddressId = $this->user->defaultBillingAddress->get()->getID(); + $this->request->set('id', $defaultBillingAddressId); + $this->controller->deleteBillingAddress(); + + //verify that the address was deleted + $this->assertEquals(1, count($this->user->getBillingAddressArray())); + + //verify that the stored defaultBillingAddressID has changed + if (null!=$this->user->defaultBillingAddress->get()) + { + $this->assertNotEquals($defaultBillingAddressId, $this->user->defaultBillingAddress->get()->getID()); + } + + //delete all addresses and verify that the stored defaultBillingAddressID is null + foreach ($this->user->getBillingAddressArray() as $address) + { + $this->request->set('id', $address['ID']); + $this->controller->deleteBillingAddress(); + } + $this->assertEquals(null, $this->user->defaultBillingAddress->get()); + } + private function setUpController(FrontendController $controller) { $this->initOrder(); From 88e2798cd1b7498e407797be085cbc462041133a Mon Sep 17 00:00:00 2001 From: Shumoapp Date: Fri, 26 Jun 2015 01:14:37 +0300 Subject: [PATCH 07/11] Fixed currency related problem in CheckoutController->notify() When finalizing an order in non-default currency, the prices of the items in the order were wrongly re-calculated. A test is added to show the problem. Also added is a second currency (EUR) in the setup, and PHPUnit_Framework_Error_Deprecated is disabled. --- application/controller/CheckoutController.php | 10 ++ test/mock/FakePaymentMethod.php | 75 +++++++++++++++ test/test/LiveCartTest.php | 15 +++ .../controller/CheckoutControllerTest.php | 95 +++++++++++++++++++ 4 files changed, 195 insertions(+) create mode 100644 test/mock/FakePaymentMethod.php create mode 100644 test/test/application/controller/CheckoutControllerTest.php diff --git a/application/controller/CheckoutController.php b/application/controller/CheckoutController.php index dc2caa318..9d9b7eac8 100644 --- a/application/controller/CheckoutController.php +++ b/application/controller/CheckoutController.php @@ -1098,6 +1098,7 @@ public function notify() } $order = CustomerOrder::getInstanceById($orderId, CustomerOrder::LOAD_DATA); + $originalCurrency = $order->getCurrency()->getID();//Get the original shopping cart currency $order->setPaymentMethod(get_class($handler)); $order->loadAll(); $this->order = $order; @@ -1105,6 +1106,15 @@ public function notify() $result = $handler->notify($this->request->toArray()); + if ($this->order->getCurrency()->getID()!=$originalCurrency) + { + //Update the order currency to match the currency the user made the purchase with + $this->order->changeCurrency(Currency::getInstanceByID($originalCurrency)); + + //Recalculate all prices, as changing currency does not apply business rules/discounts/etc. + $this->order->getTotal(true); + } + if ($result instanceof TransactionResult) { $this->registerPayment($result, $handler); diff --git a/test/mock/FakePaymentMethod.php b/test/mock/FakePaymentMethod.php new file mode 100644 index 000000000..86e18e508 --- /dev/null +++ b/test/mock/FakePaymentMethod.php @@ -0,0 +1,75 @@ +gatewayTransactionID->set($requestArray['transactionId']); + $result->amount->set($requestArray['amount']); + $result->currency->set($requestArray['currency']); + $result->rawResponse->set($requestArray); + $result->setTransactionType(TransactionResult::TYPE_SALE); + + return $result; + } + + /** + * Extract order ID from payment gateway response data + */ + public function getOrderIdFromRequest($requestArray) + { + return $requestArray['orderID']; + } + + /** + * Determine if HTML output is required as post-notification response + * @return bool + */ + public function isHtmlResponse() + { + return false; + } + + public function getValidCurrency($currentCurrencyCode) + { + $currentCurrencyCode = strtoupper($currentCurrencyCode); + $defCurrency = $this->getConfigValue('DEF_CURRENCY'); + if (!$defCurrency) + { + $defCurrency = 'USD'; + } + return in_array($currentCurrencyCode, self::getSupportedCurrencies()) ? $currentCurrencyCode : $defCurrency; + } + + public static function getSupportedCurrencies() + { + return array('USD', 'CAD', 'EUR', 'GBP', 'JPY', 'AUD', 'NZD', 'CHF', 'HKD', 'SGD', 'SEK', 'DKK', 'PLN', 'NOK', 'HUF', 'CZK', 'MXN', 'ILS'); + } + + public function isVoidable() + { + return false; + } +} diff --git a/test/test/LiveCartTest.php b/test/test/LiveCartTest.php index d48fe946b..737657db7 100644 --- a/test/test/LiveCartTest.php +++ b/test/test/LiveCartTest.php @@ -8,6 +8,7 @@ abstract class LiveCartTest extends PHPUnit_Framework_TestCase protected $config; protected $userEmail = 'test@test.com'; protected $userPassword = 'testerer'; + protected $usd2eru =2; public function setUp() { @@ -17,6 +18,9 @@ public function setUp() # Do not convert notice and strict to exceptions PHPUnit_Framework_Error_Notice::$enabled = FALSE; + # Do not convert deprecated notices to exceptions + PHPUnit_Framework_Error_Deprecated::$enabled = FALSE; + parent::setUp(); $this->config = ActiveRecordModel::getApplication()->getConfig(); @@ -71,6 +75,17 @@ protected function setUpCurrency() $this->usd->setAsDefault(); $this->usd->save(); } + + if (ActiveRecord::objectExists('Currency', 'EUR')) + { + $this->eur = Currency::getInstanceByID('EUR', Currency::LOAD_DATA); + } + else + { + $this->eur = Currency::getNewInstance('EUR'); + $this->eur->rate->set($this->usd2eru); + $this->eur->save(); + } } protected function initUser() diff --git a/test/test/application/controller/CheckoutControllerTest.php b/test/test/application/controller/CheckoutControllerTest.php new file mode 100644 index 000000000..524554ea3 --- /dev/null +++ b/test/test/application/controller/CheckoutControllerTest.php @@ -0,0 +1,95 @@ +controller = new CheckoutController(self::getApplication()); + $this->initOrder(); + $this->controller->setOrder($this->order); + $this->controller->setUser($this->user); + } + + /** + * Testing notify() method when the order was assembled in non-default currency. + */ + public function testNotifyNonDefaultCurrency() + { + $this->order->addProduct($this->products[0], 1); + $this->order->addProduct($this->products[1], 2); + $this->order->save(); + + //Get the total in the default currency + $total = $this->order->getTotal(true); + + //However the customer is a foreigner, and prefers to see the amounts in his own currency + $this->order->changeCurrency($this->eur); + + //Simulate a postback from the Payment Processor. Because it could be a background post, the user selected currency is not passed + $this->request->set('id', 'FakePaymentMethod'); + $this->request->set('orderID', $this->order->getID()); + $this->request->set('transactionId', time()); + $this->request->set('amount', $total); + $this->request->set('currency', 'USD'); + + //Process the transaction using the mock FakePaymentMethod class + $this->controller->notify(); + //$transactions = $this->order->getTransactions(); + + //Verify that the order prices match the product prices + foreach ($this->order->getPurchasedItems() as $orderedItem) + { + $product = $this->getProductByID($orderedItem->product->get()->getID()); + if (null!=$product) $this->assertEquals($product->getPrice($orderedItem->getCurrency()), $orderedItem->getItemPrice()); + } + } + + /** + * Find the product in the $this->products array, and return it + * + * @param $productID + * @return Product|null + */ + private function getProductByID($productID) + { + foreach ($this->products as $product) + { + if ($product->getID()==$productID) return $product; + } + + return null; + } +} + +?> From 251a1fe7761222029797d96403e89cffdf8115ee Mon Sep 17 00:00:00 2001 From: Shumoapp Date: Thu, 10 Sep 2015 14:58:44 +0300 Subject: [PATCH 08/11] Fixing currency conversion in CustomerOrder::changeCurrency() Added a line to reset the cached $toArrayData after each OrderedItem update, because otherwise OrderedItem::toArray() returns the price in the old currency. This resulted in incorrect prices in the shopping cart when alternating between currencies. It showed the old currency price as discounted, and with the wrong currency sign. A test was added to show the problem. --- application/model/order/CustomerOrder.php | 5 +++- .../application/model/order/OrderTest.php | 28 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/application/model/order/CustomerOrder.php b/application/model/order/CustomerOrder.php index 2c151e9c7..4a335d97b 100644 --- a/application/model/order/CustomerOrder.php +++ b/application/model/order/CustomerOrder.php @@ -1482,7 +1482,10 @@ public function changeCurrency(Currency $currency) { $item->price->set($item->getProduct()->getItemPrice($item, true, $currency)); $item->setItemPrice($item->price->get()); - $item->save(); + $item->save(true); + + //force update cache array ($toArrayData), otherwise $item->toArray()['price'] returns the price in the old currency + $item->resetArrayData(); } $this->save(); diff --git a/test/test/application/model/order/OrderTest.php b/test/test/application/model/order/OrderTest.php index 8f0282f30..f3a200d6f 100644 --- a/test/test/application/model/order/OrderTest.php +++ b/test/test/application/model/order/OrderTest.php @@ -1893,6 +1893,34 @@ public function testFreeShipping() } + /** + * Testing that alternating between currencies correctly change the prices of each OrderedItem + */ + public function testChangeCurrency() + { + $this->order->addProduct($this->products[0], 1); + $this->order->currency->set($this->usd); + $this->order->save(true); + + //force ActiveReccord to generate $toArrayData for each OrderedItem + $this->order->toArray(); + + //change the currency, cached $toArrayData should be updated for each OrderedItem + $this->order->changeCurrency($this->eur); + + //All prices should match because we don't have any discounts or taxes applied + $prices = array('itemBasePrice', 'displayPrice', 'displayPriceWithoutTax', 'itemPrice'); + + foreach ($this->order->getOrderedItems() as $item) + { + $itemArray = $item->toArray(); + $itemArray = array_intersect_key($item->toArray(),array_flip($prices)); + $pricesArray = array_fill_keys($prices, (float)$item->getProduct()->getPrice($this->eur)); + + $this->assertSame($itemArray, $pricesArray); + } + } + private function createOrderWithZone(DeliveryZone $zone = null) { if (is_null($zone)) From 929fbeb147d6e2b78754f4bb7447c5b920f5738a Mon Sep 17 00:00:00 2001 From: Shumoapp Date: Tue, 19 Jan 2016 21:57:56 +0200 Subject: [PATCH 09/11] Update listPrices for products with variations. Up till now if a user had a discount, the products with variations showed realPrice and listPrice, as the listPrice was shown with strikethrough formatting. The problem was that when changing the selected variation, the realPrice was changed, but the listPrice was not. This update fixes this problem, and now both prices are changed accordingly. --- public/javascript/frontend/Frontend.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/public/javascript/frontend/Frontend.js b/public/javascript/frontend/Frontend.js index a48b257c6..25e0d3fc3 100644 --- a/public/javascript/frontend/Frontend.js +++ b/public/javascript/frontend/Frontend.js @@ -231,7 +231,9 @@ Product.Variations = function(container, variations, options) if ($('productPrice')) { this.priceContainer = $('productPrice').down('.price').down('.realPrice'); + this.listPriceContainer = $('productPrice').down('.price').down('.listPrice'); this.defaultPrice = this.priceContainer.innerHTML; + if (this.listPriceContainer) this.defaultListPrice = this.listPriceContainer.innerHTML; } $H(this.variations.variations).each(function(value) @@ -409,6 +411,7 @@ Product.Variations.prototype = if (this.isPriceChanged(product)) { this.updatePrice(this.getProductPrice(product)); + this.updateListPrice(this.getProductListPrice(product)); } if (product.DefaultImage && product.DefaultImage.paths) @@ -442,6 +445,11 @@ Product.Variations.prototype = return (product.formattedPrice && product.formattedPrice[this.options.currency]) ? product.formattedPrice[this.options.currency] : this.defaultPrice; }, + getProductListPrice: function(product) + { + return (product.formattedListPrice && product.formattedListPrice[this.options.currency]) ? product.formattedListPrice[this.options.currency] : this.defaultListPrice; + }, + isPriceChanged: function(product) { return parseInt(product['price_' + this.options.currency]) > 0; @@ -455,6 +463,14 @@ Product.Variations.prototype = } }, + updateListPrice: function(price) + { + if (this.listPriceContainer) + { + this.listPriceContainer.innerHTML = price ? price : this.defaultListPrice; + } + }, + showDefaultInfo: function() { this.showDefaultImage(); From 29d2edc5c12c8e30713dfdefaedaca7a1596159b Mon Sep 17 00:00:00 2001 From: Shumoapp Date: Mon, 25 Jan 2016 21:05:46 +0200 Subject: [PATCH 10/11] Fixing list prices on product details page in backend. Even though only one currency was defined in a product, if the product had list price defined, all other currencies also calculated the list price, but not the base price. As a result only the list price for a currency was filled, and when the form was submitted (presumably editing something else), the base price was saved as 0. The base price is what the customer pays when adds the product in his shopping cart! This fix makes sure that the list price is not filled if a currency was not defined for a product. This way Livecart automatically calculates the prices in the other currencies, based on the currency exchange rate. --- application/controller/backend/ProductController.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/application/controller/backend/ProductController.php b/application/controller/backend/ProductController.php index 96c27745a..ea8b8cc89 100644 --- a/application/controller/backend/ProductController.php +++ b/application/controller/backend/ProductController.php @@ -907,7 +907,9 @@ private function productForm(Product $product) foreach ($pricesData['calculated'] as $currency => $price) { $pricesData['price_' . $currency] = isset($pricesData['defined'][$currency]) ? $pricesData['defined'][$currency] : ''; + $pricesData['listPrice_' . $currency] = isset($pricesData['defined'][$currency]) ? $pricesData['listPrice_' . $currency] : ''; $productFormData['price_' . $currency] = $pricesData['price_' . $currency]; + $productFormData['listPrice_' . $currency] = $pricesData['listPrice_' . $currency]; } } From 99e16cf3199863be39bddd0ca9f2315575811240 Mon Sep 17 00:00:00 2001 From: Shumoapp Date: Wed, 22 Jun 2016 22:44:33 +0300 Subject: [PATCH 11/11] Added mail queues for multi-threaded email sending. Currently supports only MySQL and Semaphore queue, but can be extended to support all kind of queues. You just need to add WhateverQueue.php file that extends QueueInterface.php inside model/queue folder. All emails will be added to a Queue, and a cronjob(or many cronjobs) will get the queued emails, and send them with the configured email transport system. This results in faster newsletter sending, and the clients are not waiting on checkout/complete for the server to connect to smtp server and send the emails. Add as many threads as you need to your crontab like this: * * * * * /usr/bin/php /public_html/storage/processQueues.php > /dev/null 2>&1 The more threads you have, the faster the emails will be sent, and the higher the CPU load will be. So please use responsibly! --- .../language/en/backend/Settings.lng | 6 +- .../configuration/registry/12-technical.ini | 5 + application/model/Email.php | 235 ++++++++++++------ application/model/email/EmailQueue.php | 32 +++ .../model/email/SimpleEmailMessage.php | 59 +++++ .../model/newsletter/NewsletterMessage.php | 4 +- application/model/queue/MysqlQueue.php | 130 ++++++++++ application/model/queue/NoQueue.php | 50 ++++ application/model/queue/ObjectQueueBase.php | 65 +++++ application/model/queue/QueueException.php | 15 ++ application/model/queue/QueueFactory.php | 82 ++++++ application/model/queue/QueueInterface.php | 39 +++ application/model/queue/SemaphoreQueue.php | 62 +++++ application/processQueues.php | 63 +++++ framework | 2 +- installdata/sql/create.sql | 18 ++ .../model/email/SimpleEmailMessageTest.php | 49 ++++ .../model/queue/MysqlQueueTest.php | 71 ++++++ .../model/queue/ObjectQueueBaseTest.php | 64 +++++ .../model/queue/QueueFactoryTest.php | 39 +++ .../model/queue/SemaphoreQueueTest.php | 39 +++ 21 files changed, 1046 insertions(+), 83 deletions(-) create mode 100644 application/model/email/EmailQueue.php create mode 100644 application/model/email/SimpleEmailMessage.php create mode 100644 application/model/queue/MysqlQueue.php create mode 100644 application/model/queue/NoQueue.php create mode 100644 application/model/queue/ObjectQueueBase.php create mode 100644 application/model/queue/QueueException.php create mode 100644 application/model/queue/QueueFactory.php create mode 100644 application/model/queue/QueueInterface.php create mode 100644 application/model/queue/SemaphoreQueue.php create mode 100644 application/processQueues.php create mode 100644 test/test/application/model/email/SimpleEmailMessageTest.php create mode 100644 test/test/application/model/queue/MysqlQueueTest.php create mode 100644 test/test/application/model/queue/ObjectQueueBaseTest.php create mode 100644 test/test/application/model/queue/QueueFactoryTest.php create mode 100644 test/test/application/model/queue/SemaphoreQueueTest.php diff --git a/application/configuration/language/en/backend/Settings.lng b/application/configuration/language/en/backend/Settings.lng index 01d37e348..d815d1ffb 100644 --- a/application/configuration/language/en/backend/Settings.lng +++ b/application/configuration/language/en/backend/Settings.lng @@ -517,7 +517,11 @@ SSL_DOMAIN=Use a different domain name for secure pages (e.g. secure.mysite.com) # Technical _technical=Technical _cache=Caching +_queue=Message Queue CACHE_METHOD=Caching method +QUEUE_METHOD=Queue method +CACHE_TIMEOUT=Cache timeout in seconds +CACHE_PREFIX=Cache prefix (chars and underscores only). Normally blank, but set to 'dev_' in case you have dev machine on the same server (so caches for live and dev won't interfere) _proxy=Use proxy server for outgoing connections PROXY_HOST=Proxy server host name PROXY_PORT=Proxy server port @@ -592,4 +596,4 @@ UPDATE_FTP_DIRECTORY=FTP directory _update_repositories=Module/update repositories UPDATE_REPO_1=Repository URL -UPDATE_REPO_2=Add new repository \ No newline at end of file +UPDATE_REPO_2=Add new repository diff --git a/application/configuration/registry/12-technical.ini b/application/configuration/registry/12-technical.ini index 3709d2cd6..c6708199c 100644 --- a/application/configuration/registry/12-technical.ini +++ b/application/configuration/registry/12-technical.ini @@ -2,6 +2,11 @@ [_cache] CACHE_METHOD = "application)) . '/'; ?>" +CACHE_TIMEOUT = +CACHE_PREFIX = + +[_queue] +QUEUE_METHOD = "application)) . '/'; ?>" [_proxy] PROXY_HOST = diff --git a/application/model/Email.php b/application/model/Email.php index fe8b7c11a..7999e862b 100755 --- a/application/model/Email.php +++ b/application/model/Email.php @@ -7,6 +7,8 @@ ClassLoader::ignoreMissingClasses(false); ClassLoader::import('application.model.template.EditedCssFile'); +ClassLoader::import('application.model.email.SimpleEmailMessage'); +ClassLoader::import('application.model.email.EmailQueue'); /** * E-mail handler @@ -42,47 +44,20 @@ class Email private $message; + private $simpleMessage; + + private $config; + public function __construct(LiveCart $application) { $this->application = $application; $this->set('request', $application->getRequest()->toArray()); - $config = $this->application->getConfig(); - - ClassLoader::ignoreMissingClasses(); - - if ('SMTP' == $config->get('EMAIL_METHOD')) - { - $server = $config->get('SMTP_SERVER'); - if (!$server) - { - $server = ini_get('SMTP'); - } - - $this->connection = Swift_SmtpTransport::newInstance($server, $config->get('SMTP_PORT')); - - if ($config->get('SMTP_USERNAME')) - { - $this->connection->setUsername($config->get('SMTP_USERNAME')); - $this->connection->setPassword($config->get('SMTP_PASSWORD')); - } - } - else if ('FAKE' == $config->get('EMAIL_METHOD')) - { - $this->connection = Swift_Connection_Fake::newInstance(); - } - else - { - $this->connection = Swift_MailTransport::newInstance(); - } - - $this->swiftInstance = Swift_Mailer::newInstance($this->connection); - - $this->message = Swift_Message::newInstance(); + $this->config = $this->application->getConfig(); - $this->setFrom($config->get('MAIN_EMAIL'), $config->get('STORE_NAME')); + $this->simpleMessage = new SimpleEmailMessage(); - ClassLoader::ignoreMissingClasses(false); + $this->setFrom($this->config->get('MAIN_EMAIL'), $this->config->get('STORE_NAME')); } public function getMessage() @@ -145,6 +120,11 @@ public function setHTML($html) $this->html = $html; } + public function getHTML() + { + return $this->html; + } + public function setFrom($email, $name) { if ($name) @@ -152,16 +132,17 @@ public function setFrom($email, $name) $name = '"' . $name . '"'; } - $this->message->setFrom(array($email => $name)); + $this->simpleMessage->setFrom(array($email => $name)); } - public function resetRecipients() - { - $headers = $this->message->getHeaders(); - $headers->remove('To'); - $headers->remove('Cc'); - $headers->remove('Bcc'); - } +// ?? not used... +// public function resetRecipients() +// { +// $headers = $this->message->getHeaders(); +// $headers->remove('To'); +// $headers->remove('Cc'); +// $headers->remove('Bcc'); +// } public function setTo($emailAddresses, $name = null) { @@ -172,18 +153,18 @@ public function setTo($emailAddresses, $name = null) $name = '"' . $name . '"'; } - $this->message->addTo($email, $name); + $this->simpleMessage->addTo($email, $name); } } public function setCc($email, $name) { - $this->message->addCc($email, $name); + $this->simpleMessage->addCc($email, $name); } public function setBcc($email, $name) { - $this->message->addBcc($email, $name); + $this->simpleMessage->addBcc($email, $name); } public function setTemplate($templateFile) @@ -278,58 +259,143 @@ public function getLocale() return $this->locale ? $this->locale : $this->application->getLocaleCode(); } - public function send() + /** + * Used to strip application and config references before this object is serialized. + */ + public function __sleep() { - ClassLoader::ignoreMissingClasses(); + $skipAttributes = array('config', 'application', 'swiftInstance'); + return array_diff(array_keys(get_object_vars($this)), $skipAttributes); + } - $this->application->processInstancePlugins('email-prepare-send', $this); - $this->application->processInstancePlugins('email-prepare-send/' . $this->relativeTemplatePath, $this); + /** + * Use after unserializing to restore the $application reference + * @param $application + */ + public function setApplication($application) + { + $this->application = $application; + } - if ($this->template) + /** + * Use after unserializing to restore the $config reference + * @param $config + */ + public function setConfig($config) + { + $this->config = $config; + } + + public function generateEmailBody() + { + $originalLocale = $this->application->getLocale(); + $emailLocale = Locale::getInstance($this->getLocale()); + $this->application->setLocale($emailLocale); + $this->application->getLocale()->translationManager()->loadFile('User'); + $this->application->loadLanguageFiles(); + + $smarty = $this->application->getRenderer()->getSmartyInstance(); + + foreach ($this->values as $key => $value) { - $originalLocale = $this->application->getLocale(); - $emailLocale = Locale::getInstance($this->getLocale()); - $this->application->setLocale($emailLocale); - $this->application->getLocale()->translationManager()->loadFile('User'); - $this->application->loadLanguageFiles(); + $smarty->assign($key, $value); + } - $smarty = $this->application->getRenderer()->getSmartyInstance(); + //?$router = $this->application->getRouter(); - foreach ($this->values as $key => $value) - { - $smarty->assign($key, $value); - } + $smarty->assign('html', false); + + $smarty->disableTemplateLocator(); + $text = $smarty->fetch($this->template); + $smarty->enableTemplateLocator(); + + $parts = explode("\n", $text, 2); + $this->subject = array_shift($parts); + $this->setText(array_shift($parts)); + + // fix URLs + $this->text = str_replace('&', '&', $this->text); + + if ($this->application->getConfig()->get('HTML_EMAIL')) + { + $smarty->assign('html', true); + $html = array_pop(explode("\n", $smarty->fetch($this->template), 2)); - $router = $this->application->getRouter(); + $css = new EditedCssFile('email'); + $smarty->assign('cssStyle', str_replace("\n", ' ', $css->getCode())); - $smarty->assign('html', false); + $smarty->assign('messageHtml', $html); + $html = $smarty->fetch($this->getTemplatePath('htmlWrapper')); - $smarty->disableTemplateLocator(); - $text = $smarty->fetch($this->template); - $smarty->enableTemplateLocator(); + $this->setHtml($html); + } - $parts = explode("\n", $text, 2); - $this->subject = array_shift($parts); - $this->setText(array_shift($parts)); + $this->application->setLocale($originalLocale); + } - // fix URLs - $this->text = str_replace('&', '&', $this->text); + /** + * This method is used to queue or send an email. If the $instant parameter is true, the mail will not be queued, but sent directly. + * + * @param bool $instant Set to true to send the email instantly + * @param int $priority The higher the value, the more likely the email will be retrieved from the queue with the next pull. + * @return bool|int + */ + public function send($instant = false, $priority = 10) + { + ClassLoader::ignoreMissingClasses(); - if ($this->application->getConfig()->get('HTML_EMAIL')) + //If not an instant message and there is a queue, add it to the mail queue + if(!$instant && 'NoQueue' != $this->config->get('QUEUE_METHOD')) + { + try { - $smarty->assign('html', true); - $html = array_pop(explode("\n", $smarty->fetch($this->template), 2)); + //If all is right with the queue, return without sending the message. + $queue = new EmailQueue($this->application->getConfig()); + $queue->send($this, $priority); + return; + } + catch(Exception $e) + { + //Continue with the sending, obviously the queue has some problems. + } + } + + $this->message = Swift_Message::newInstance(); + $this->simpleMessage->populateMessage($this->message); - $css = new EditedCssFile('email'); - $smarty->assign('cssStyle', str_replace("\n", ' ', $css->getCode())); + if ('SMTP' == $this->config->get('EMAIL_METHOD')) + { + $server = $this->config->get('SMTP_SERVER'); + if (!$server) + { + $server = ini_get('SMTP'); + } - $smarty->assign('messageHtml', $html); - $html = $smarty->fetch($this->getTemplatePath('htmlWrapper')); + $this->connection = Swift_SmtpTransport::newInstance($server, $this->config->get('SMTP_PORT')); - $this->setHtml($html); + if ($this->config->get('SMTP_USERNAME')) + { + $this->connection->setUsername($this->config->get('SMTP_USERNAME')); + $this->connection->setPassword($this->config->get('SMTP_PASSWORD')); } + } + else if ('FAKE' == $this->config->get('EMAIL_METHOD')) + { + $this->connection = Swift_Connection_Fake::newInstance(); + } + else + { + $this->connection = Swift_MailTransport::newInstance(); + } + + $this->swiftInstance = Swift_Mailer::newInstance($this->connection); - $this->application->setLocale($originalLocale); + $this->application->processInstancePlugins('email-prepare-send', $this); + $this->application->processInstancePlugins('email-prepare-send/' . $this->relativeTemplatePath, $this); + + if ($this->template) + { + $this->generateEmailBody(); } $this->application->processInstancePlugins('email-before-send', $this); @@ -359,6 +425,12 @@ public function send() return false; } + if ($this->application->isDevMode()) + { + $mailLogger = new \Swift_Plugins_Loggers_ArrayLogger(); + $this->swiftInstance->registerPlugin(new \Swift_Plugins_LoggerPlugin($mailLogger)); + } + try { $res = $this->swiftInstance->send($this->message); @@ -372,6 +444,11 @@ public function send() return false; } + if ($this->application->isDevMode() && !$res) + { + echo "Switmailer error:".$mailLogger->dump(); + } + $this->application->processInstancePlugins('email-after-send/' . $this->relativeTemplatePath, $this); $this->application->processInstancePlugins('email-after-send', $this); diff --git a/application/model/email/EmailQueue.php b/application/model/email/EmailQueue.php new file mode 100644 index 000000000..dc6d436e3 --- /dev/null +++ b/application/model/email/EmailQueue.php @@ -0,0 +1,32 @@ + + */ +ClassLoader::import('application.model.queue.QueueFactory'); +ClassLoader::import('application.model.queue.ObjectQueueBase'); +ClassLoader::import('application.model.Email'); + +ClassLoader::ignoreMissingClasses(); +ClassLoader::import('library.swiftmailer.lib.swift_required', true); +ClassLoader::ignoreMissingClasses(false); + +/** + * This class communicates with the configured queue to send and receive emails from the queue. + * + * Class EmailQueue + */ +class EmailQueue extends ObjectQueueBase +{ + /** + * The constructor. Sets the queue name (244566 in this case), and builds the queue. + * + * @param Config $config + */ + public function __construct(Config &$config) + { + parent::__construct($config, 244566); + } +} \ No newline at end of file diff --git a/application/model/email/SimpleEmailMessage.php b/application/model/email/SimpleEmailMessage.php new file mode 100644 index 000000000..859c000c1 --- /dev/null +++ b/application/model/email/SimpleEmailMessage.php @@ -0,0 +1,59 @@ + + */ +class SimpleEmailMessage +{ + private $headers = array(); + + public function addTo($email, $name) + { + $this->headers['addTo'][] = array($email, $name); + } + + public function addCc($email, $name) + { + $this->headers['addCc'][] = array($email, $name); + } + + public function addBcc($email, $name) + { + $this->headers['addBcc'][] = array($email, $name); + } + + public function setReplyTo($email, $name) + { + $this->headers['setReplyTo'][] = array($email, $name); + } + + public function setFrom($array) + { + $this->headers['setFrom'] = $array; + } + + /** + * Get a Swift_Message object, and populate it with the values in this class. + * + * @param Swift_Message $message + */ + public function populateMessage(Swift_Message &$message) + { + foreach($this->headers as $function=>$parameters) + { + if($function=='setFrom') + { + $message->setFrom($parameters); + } + else + { + foreach($parameters as $email) + { + $message->$function($email[0], $email[1]); + } + } + } + } +} \ No newline at end of file diff --git a/application/model/newsletter/NewsletterMessage.php b/application/model/newsletter/NewsletterMessage.php index 8d5197b0b..a5f285f47 100644 --- a/application/model/newsletter/NewsletterMessage.php +++ b/application/model/newsletter/NewsletterMessage.php @@ -94,7 +94,7 @@ private function sendMessage(NewsletterSentMessage $sent, LiveCart $application) $this->save(); } - return $email->send(); + return $email->send(false, $priority = 0); } public function markAsSent() @@ -105,4 +105,4 @@ public function markAsSent() } } -?> \ No newline at end of file +?> diff --git a/application/model/queue/MysqlQueue.php b/application/model/queue/MysqlQueue.php new file mode 100644 index 000000000..c06872558 --- /dev/null +++ b/application/model/queue/MysqlQueue.php @@ -0,0 +1,130 @@ + + */ +ClassLoader::import('application.model.ActiveRecordModel'); +ClassLoader::import('application.model.queue.QueueInterface'); +ClassLoader::import('application.model.queue.QueueException'); + +class MysqlQueue extends ActiveRecordModel implements QueueInterface +{ + /** + * The id of the currently processed row. + * @var int + */ + private $id = 0; + + public static function defineSchema($className = __CLASS__) + { + $schema = self::getSchemaInstance($className); + $schema->setName(__CLASS__); + + $schema->registerField(new ARPrimaryKeyField("ID", ARInteger::instance())); + $schema->registerField(new ARField("name", ARInteger::instance())); + $schema->registerField(new ARField("message", ARText::instance(1))); + $schema->registerField(new ARField("added", ARDateTime::instance())); + $schema->registerField(new ARField("consumed", ARDateTime::instance())); + $schema->registerField(new ARField("priority", ARInteger::instance())); + $schema->registerField(new ARField("isProcessed", ARInteger::instance(1))); + } + + public static function getNewInstance() + { + return ActiveRecord::getNewInstance(__CLASS__); + } + + /** + * Set the queue name. + * @param $queueName + */ + public function setQueueName($queueName) + { + $this->name->set($queueName); + } + + /** + * Adds a message to the queue. If priority parameter is passed, it will be assigned to the message. + * @param $message + * @param $priority + * @throws QueueException + * @return void + */ + public function send($message, $priority) + { + if (function_exists('gzcompress')) + { + $message = gzcompress($message); + } + $this->insertMessage($message, $priority); + } + + /** + * Consume a message from the queue. The messages with hither priority are fetched first. + * @return mixed + */ + public function receive() + { + $startTime = time(); + $timeout = 1*60;//1 minute + + //Loop for up to one minute in order to find a message not being processed by other worker thread. + while (time()-$startTime<$timeout) + { + //Get the first available row + $firstRow = $this->db->executeQuery("SELECT * from `".__CLASS__."` where isProcessed IS NULL ORDER BY priority DESC, ID ASC LIMIT 0,1"); + //If no result, it means no more rows for processing, so break the while + if(!$firstRow->next()) break; + $row = $firstRow->getRow(); + $this->id = $row['ID']; + if (!$this->id) break; + + //Update isProcessed for this row + $this->db->executeQuery("UPDATE `".__CLASS__."` set isProcessed=1, consumed=now() where ID=".$this->id); + + //check affected rows - if 0 - start all over. It would mean that another thread is already processing this message, so we should move to the next one. + if (!$this->db->getUpdateCount()) continue; + + if (function_exists('gzuncompress')) + { + $row['message'] = gzuncompress($row['message']); + } + + return $row['message']; + } + } + + /** + * Insert the message, along with the specified priority. Write to the php error_log on failure. + * @param $message + * @throws QueueException + * @param $priority + */ + private function insertMessage($message, $priority) + { + $sql = 'INSERT INTO `'.__CLASS__.'` SET name=' . (int)$this->name->get() .', ' . 'message="' . addslashes($message).'", priority='.(int)$priority; + try + { + $this->db->executeQuery($sql); + } + catch (Exception $e) + { + $message = "Failed to queue message: MysqlQueue->insertMessage(). QueueName=".(int)$this->name->get().'. ExceptionMe->getMessage()'.substr($e->getMessage(), 0, 150); + error_log($message); + throw new QueueException(__CLASS__, $message); + } + } + + /** + * Delete a message from the queue upon successfully consuming the message. + * @return mixed + */ + public function remove() + { + //Delete the currently processed row + $affectedRows = $this->db->executeQuery("DELETE from `".__CLASS__."` where ID=".$this->id); + } +} \ No newline at end of file diff --git a/application/model/queue/NoQueue.php b/application/model/queue/NoQueue.php new file mode 100644 index 000000000..2564a5046 --- /dev/null +++ b/application/model/queue/NoQueue.php @@ -0,0 +1,50 @@ + + */ +ClassLoader::import('application.model.queue.QueueInterface'); + +/** + * Class NoQueue + */ +class NoQueue implements QueueInterface{ + + /** + * @param $queueName + */ + public function setQueueName($queueName) + { + + } + + /** + * Does nothing. + * @param $message + * @param $priority + * @return void + */ + public function send($message, $priority) + { + + } + + /** + * Does nothing. + */ + public function receive() + { + + } + + /** + * Does nothing. + */ + public function remove() + { + + } + +} \ No newline at end of file diff --git a/application/model/queue/ObjectQueueBase.php b/application/model/queue/ObjectQueueBase.php new file mode 100644 index 000000000..a7cca42e9 --- /dev/null +++ b/application/model/queue/ObjectQueueBase.php @@ -0,0 +1,65 @@ + + */ +ClassLoader::import('application.model.queue.QueueFactory'); + +/** + * Base class for communicating with the configured queue to send and receive objects from the queue. + * + * Class ObjectQueue + */ +class ObjectQueueBase +{ + /** + * Queue instance. It could be any supported type from @package application.model.queue + * + * @var MysqlQueue|NoQueue|SemaphoreQueue + */ + private $queue; + + /** + * The constructor. Sets the queue name, and builds the queue. + * + * @param Config $config + * @param String $queueName + */ + public function __construct(Config &$config, $queueName) + { + $this->queueName = $queueName; + $this->queue = QueueFactory::getQueue($config, $this->queueName); + } + + /** + * Sends a message to the queue. If priority is sent, the message with higher priority will be consumed first. + * @param $object + * @param $priority + */ + public function send($object, $priority) + { + $this->queue->send(serialize($object), $priority); + } + + /** + * Consumes a message from the queue. + * + * @return mixed + */ + public function receive() + { + $message = $this->queue->receive(); + return unserialize($message); + } + + /** + * Use this to delete the message from the queue upon successfully processing the message. + */ + public function remove() + { + $this->queue->remove(); + } + +} \ No newline at end of file diff --git a/application/model/queue/QueueException.php b/application/model/queue/QueueException.php new file mode 100644 index 000000000..56a14f7a6 --- /dev/null +++ b/application/model/queue/QueueException.php @@ -0,0 +1,15 @@ + + */ +class QueueException extends Exception +{ + public function __construct($className, $message) + { + parent::__construct('Exception in '.$className.': '.$message); + } +} \ No newline at end of file diff --git a/application/model/queue/QueueFactory.php b/application/model/queue/QueueFactory.php new file mode 100644 index 000000000..cad4586dc --- /dev/null +++ b/application/model/queue/QueueFactory.php @@ -0,0 +1,82 @@ + + */ + +class QueueFactory +{ + /** + * Factory method that returns the currently selected queue method object. + * @param Config $config + * @param $queueName + * @return MysqlQueue|SemaphoreQueue|NoQueue + * @throws QueueException + */ + public static function getQueue(Config &$config, $queueName) + { + //Verify if name is integer, because some queues accept only integer names. + if (!is_int($queueName)) throw new QueueException(__CLASS__, 'Queue name must be integer value'); + + $queue = null; + $class = $config->get('QUEUE_METHOD'); + ClassLoader::import("application.model.queue." . $class); + + //Try to initialize the set queue method, fallback to NoQueue + try + { + if ('MysqlQueue' == $class) + { + $queue = MysqlQueue::getNewInstance(); + } + else + { + $queue = new $class(); + } + } + catch(Exception $e) + { + ClassLoader::import("application.model.queue.NoQueue"); + $queue = new NoQueue($queueName); + } + + $queue->setQueueName($queueName); + + return $queue; + } + + /** + * Returns the currently available queue methods (in the application/method/queue folder) + * @param LiveCart $application + * @return array + */ + public static function getQueueMethods(LiveCart &$application) + { + $ret = array(); + + $translationHandler = $application->getLocale()->translationManager(); + + foreach (new DirectoryIterator(dirname(__file__) . '/') as $method) + { + if (substr($method->getFileName(), 0, 1) != '.') + { + $class = substr($method->getFileName(), 0, -4); + + if ((substr($class, -5) == 'Queue') && (file_exists($method->getPathname()))) + { + include_once $method->getPathname(); + $translationHandler->setDefinition($class, $class); + $ret[] = $class; + } + } + } + + $ret = array_merge(array('NoQueue'), array_diff($ret, array('NoQueue'))); + + return $ret; + } +} \ No newline at end of file diff --git a/application/model/queue/QueueInterface.php b/application/model/queue/QueueInterface.php new file mode 100644 index 000000000..effcfc36d --- /dev/null +++ b/application/model/queue/QueueInterface.php @@ -0,0 +1,39 @@ + + */ +interface QueueInterface +{ + /** + * Set the queue name + * @param $queueName + */ + public function setQueueName($queueName); + + /** + * Adds a message to the queue. If priority parameter is passed, it will be assigned to the message. + * Must throw QueueException on failure. + * + * @param $message + * @param $priority + * @throws QueueException + * @return mixed + */ + public function send($message, $priority); + + /** + * Consume a message from the queue. The messages with hither priority are fetched first. + * @return mixed + */ + public function receive(); + + /** + * Delete a message from the queue upon successfully consuming the message. + * @return mixed + */ + public function remove(); + +} \ No newline at end of file diff --git a/application/model/queue/SemaphoreQueue.php b/application/model/queue/SemaphoreQueue.php new file mode 100644 index 000000000..b1322cf58 --- /dev/null +++ b/application/model/queue/SemaphoreQueue.php @@ -0,0 +1,62 @@ + + */ +ClassLoader::import('application.model.queue.QueueInterface'); +ClassLoader::import('application.model.queue.QueueException'); + +class SemaphoreQueue implements QueueInterface +{ + private $name; + private $queue; + private $msgType = NULL; + private $maxMsgSize = 1000000;//1Mb + + /** + * @param $queueName + */ + public function setQueueName($queueName) + { + $this->name = $queueName; + $this->queue = msg_get_queue($this->name); + } + + /** + * Adds a message to the queue. priority parameter is not supported by this engine. + * + * @param $message + * @param $priority + * @throws QueueException + * @return void + */ + public function send($message, $priority) + { + if (!msg_send($this->queue, 1, $message)) + { + throw new QueueException(__CLASS__, var_export(msg_stat_queue($this->queue), true)); + } + } + + /** + * Consume a message from the queue. The messages with hither priority are fetched first. + * + * @return mixed + */ + public function receive() + { + $message = null; + msg_receive($this->queue, 1, $this->msgType, $this->maxMsgSize, $message); + + return $message; + } + + public function remove() + { + // no alternative + } +} \ No newline at end of file diff --git a/application/processQueues.php b/application/processQueues.php new file mode 100644 index 000000000..b658237db --- /dev/null +++ b/application/processQueues.php @@ -0,0 +1,63 @@ + /dev/null 2>&1 + * + * The more threads you have, the faster the emails will be sent, and the higher the + * CPU load will be. So please use responsibly! + */ + +if (isset($_SERVER['REQUEST_URI'])) +{ + $_SERVER['REQUEST_URI'] = dirname($_SERVER['REQUEST_URI']) . '/'; +} +else +{ + // retrieve base URL from configuration + $url = include dirname(__file__) . '/../storage/configuration/url.php'; + $parsed = parse_url($url['url']); + $_SERVER['HTTP_HOST'] = $parsed['host']; + $_SERVER['REQUEST_URI'] = $parsed['path']; + $_SERVER['REWRITE'] = $url['rewrite']; +} + +if (isset($_SERVER['REWRITE']) && !$_SERVER['REWRITE']) +{ + //$this->request->set('noRewrite', true); +// $app->getRouter()->enableURLRewrite($_SERVER['rewrite']); +} + +include dirname(__file__) . '/../application/Initialize.php'; +$app = new LiveCart(); +$app->setDevMode(true); +ClassLoader::import('library.activerecord.ActiveRecord'); +ClassLoader::import('application.model.user.User'); +ClassLoader::import('application.model.newsletter.*'); +ClassLoader::import("application.model.*"); +ClassLoader::import('application.model.email.SimpleEmailMessage'); +ClassLoader::import('application.model.email.EmailQueue'); + +$app->loadLanguageFile('Base'); +$app->loadLanguageFile('Product'); +$app->loadLanguageFile('User'); +$app->loadLanguageFile('Order'); +$app->loadLanguageFile('Customize'); +$app->loadLanguageFile('Frontend'); +$app->loadLanguageFile('Custom'); + +$conn = ActiveRecord::getDBConnection(); +$queue = new EmailQueue($app->getConfig()); + +$startTime = time(); +$timeout = 1*60;//1 minute + +while (time()-$startTime<$timeout) +{ + $emailObject = $queue->receive(); + if (!$emailObject) break; + $emailObject->setApplication($app); + $emailObject->setConfig($app->getConfig()); + $res = $emailObject->send(true); + if ($res) $queue->remove(); +} \ No newline at end of file diff --git a/framework b/framework index fd01b9e41..3123e35c4 160000 --- a/framework +++ b/framework @@ -1 +1 @@ -Subproject commit fd01b9e417661051e132c008515514919c1fe5cf +Subproject commit 3123e35c4925f4f47faff5eb1dc3a897f9cd69a0 diff --git a/installdata/sql/create.sql b/installdata/sql/create.sql index 616fa8422..67afdc6c2 100644 --- a/installdata/sql/create.sql +++ b/installdata/sql/create.sql @@ -777,6 +777,24 @@ CREATE TABLE DeliveryZoneCityMask ( ) ENGINE = INNODB CHARACTER SET utf8 COLLATE utf8_general_ci; + +# ---------------------------------------------------------------------- # +# Add table "MysqlQueue" # +# ---------------------------------------------------------------------- # + +CREATE TABLE IF NOT EXISTS `MysqlQueue` ( + `ID` int(10) unsigned NOT NULL AUTO_INCREMENT, + `name` int(10) DEFAULT NULL COMMENT 'Queue name', + `message` longblob COMMENT 'The queued message', + `added` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP COMMENT 'When the message was added to the queue', + `consumed` TIMESTAMP NULL DEFAULT NULL COMMENT 'When the message was consumed for the last time', + `priority` INT NOT NULL DEFAULT '0' COMMENT 'The higher the value, the sooner the message will be consumed', + `isProcessed` tinyint(1) DEFAULT NULL COMMENT 'Determines if the message is being processed', + PRIMARY KEY (`ID`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE utf8_general_ci; +CREATE INDEX IDX_name ON MysqlQueue (name); +CREATE INDEX IDX_priority ON MysqlQueue (priority); + # ---------------------------------------------------------------------- # # Add table "DeliveryZoneZipMask" # # ---------------------------------------------------------------------- # diff --git a/test/test/application/model/email/SimpleEmailMessageTest.php b/test/test/application/model/email/SimpleEmailMessageTest.php new file mode 100644 index 000000000..e464bf9d1 --- /dev/null +++ b/test/test/application/model/email/SimpleEmailMessageTest.php @@ -0,0 +1,49 @@ + + */ +class SimpleEmailMessageTest extends LiveCartTest +{ + /** + * @var SimpleEmailMessage + */ + private $simpleMessage; + /** + * @var Swift_Message + */ + private $message; + + /** + * Test storing email parameters in SimpleEmailMessage, and transferring them to Swift_Message + */ + public function testStoreAndRestore() + { + $email = 'test@yahoo.com'; + $name = 'John Doe'; + + $this->simpleMessage = new SimpleEmailMessage(); + $this->simpleMessage->addTo($email, $name); + $this->simpleMessage->addCc($email, $name); + $this->simpleMessage->addBcc($email, $name); + $this->simpleMessage->setReplyTo($email, $name); + $this->simpleMessage->setFrom(array($email => $name)); + + $this->message = Swift_Message::newInstance(); + $this->simpleMessage->populateMessage($this->message); + + $this->assertEqual($this->message->getTo(), array($email => $name)); + $this->assertEqual($this->message->getCc(), array($email => $name)); + $this->assertEqual($this->message->getBcc(), array($email => $name)); + $this->assertEqual($this->message->getReplyTo(), array($email => $name)); + $this->assertEqual($this->message->getFrom(), array($email => $name)); + } + +} \ No newline at end of file diff --git a/test/test/application/model/queue/MysqlQueueTest.php b/test/test/application/model/queue/MysqlQueueTest.php new file mode 100644 index 000000000..2da7e4f96 --- /dev/null +++ b/test/test/application/model/queue/MysqlQueueTest.php @@ -0,0 +1,71 @@ + + */ +class MysqlQueueTest extends LiveCartTest +{ + /** + * @var + */ + private $queue; + + /** + * @return array + */ + public function getUsedSchemas() + { + return array( + 'MysqlQueue' + ); + } + + /** + * Setup the queue and clear it in case it's not empty + */ + public function setUp() + { + parent::setUp(); + ActiveRecordModel::executeUpdate('DELETE FROM MysqlQueue'); + $this->queue = MysqlQueue::getNewInstance(); + $this->queue->setQueueName(2534756); + + //Empty the queue in case there was some leftovers. + while(!is_null($this->queue->receive())){}; + } + + /** + * Test the MysqlQueue for sending and retrieval in the correct order + */ + public function testSendAndReceive() + { + $message1 = "SomeRandomMessage".rand(); + $message2 = "SomeRandomMessage".rand(); + $this->queue->send($message1, 1); + $this->queue->send($message2, 1); + + $this->assertEqual($this->queue->receive(), $message1); + $this->assertEqual($this->queue->receive(), $message2); + } + + /** + * Test the MysqlQueue for sending and retrieval messages with different priority + */ + public function testQueuePriority() + { + $message1 = "SomeRandomMessage".rand(); + $message2 = "SomeRandomMessage".rand(); + $this->queue->send($message1, 1); + $this->queue->send($message2, 2); + + //The latter message should be received first, as it has a higher priority. + $this->assertEqual($this->queue->receive(), $message2); + $this->assertEqual($this->queue->receive(), $message1); + } + +} \ No newline at end of file diff --git a/test/test/application/model/queue/ObjectQueueBaseTest.php b/test/test/application/model/queue/ObjectQueueBaseTest.php new file mode 100644 index 000000000..84929b79a --- /dev/null +++ b/test/test/application/model/queue/ObjectQueueBaseTest.php @@ -0,0 +1,64 @@ + + */ +class ObjectQueueBaseTest extends LiveCartTest +{ + /** + * @var + */ + private $queue; + + /** + * @return array + */ + public function getUsedSchemas() + { + return array( + 'MysqlQueue' + ); + } + + /** + * Setup the queue and clear it in case it's not empty + */ + public function setUp() + { + parent::setUp(); + ActiveRecordModel::executeUpdate('DELETE FROM MysqlQueue'); + $this->getApplication()->getConfig()->set('QUEUE_METHOD', 'MysqlQueue'); + $this->queue = new ObjectQueueBase($this->getApplication()->getConfig(), 754754); + + //Empty the queue in case there was some leftovers. + while($this->queue->receive()){}; + } + + public function testSendAndReceiveObjects() + { + $bar = new foo; + + $this->queue->send($bar, 1); + $message = $this->queue->receive(); + + $this->assertEquals($bar, $message); + } + +} + +class foo +{ + public $pub = 'pub'; + private $priv = 'priv'; + protected $prot = 'prot'; + + function do_foo() + { + echo "Doing foo."; + } +} \ No newline at end of file diff --git a/test/test/application/model/queue/QueueFactoryTest.php b/test/test/application/model/queue/QueueFactoryTest.php new file mode 100644 index 000000000..2f4e56d57 --- /dev/null +++ b/test/test/application/model/queue/QueueFactoryTest.php @@ -0,0 +1,39 @@ + + */ +class QueueFactoryTest extends LiveCartTest +{ + private $queueName = 244566; + private $queue; + + public function testGetQueueMethods() + { + $queueMethods = QueueFactory::getQueueMethods($this->getApplication()); + + $this->assertContains('NoQueue', $queueMethods); + $this->assertContains('MysqlQueue', $queueMethods); + $this->assertContains('SemaphoreQueue', $queueMethods); + } + + public function testGetQueue() + { + $queueMethods = QueueFactory::getQueueMethods($this->getApplication()); + $this->queueName = 244566; + + foreach ($queueMethods as $method) + { + $this->getApplication()->getConfig()->set('QUEUE_METHOD', $method); + $this->queue = QueueFactory::getQueue($this->getApplication()->getConfig(), $this->queueName); + + $this->assertIsA($this->queue, $method); + } + + } +} \ No newline at end of file diff --git a/test/test/application/model/queue/SemaphoreQueueTest.php b/test/test/application/model/queue/SemaphoreQueueTest.php new file mode 100644 index 000000000..5669827c7 --- /dev/null +++ b/test/test/application/model/queue/SemaphoreQueueTest.php @@ -0,0 +1,39 @@ + + */ +class SemaphoreQueueTest extends LiveCartTest +{ + /** + * Test the SemaphoreQueue for sending and retrieval in the correct order + */ + public function testSendAndReceive() + { + $extensions = get_loaded_extensions(); + + //If the extension is not compiled into php, no need to test + if (!in_array('sysvsem', $extensions)) return; + + $queue = new SemaphoreQueue(); + //Semaphore queue names should be integers + $queueName = 234767; + $queue->setQueueName($queueName); + + //Empty the queue in case there was some leftovers. + while(!is_null($queue->receive())){}; + + $message1 = "SomeRandomMessage1:"; + $message2 = "SomeRandomMessage2:"; + $queue->send($message1, 1); + $queue->send($message2, 1); + + $this->assertEqual($queue->receive(), $message2); + $this->assertEqual($queue->receive(), $message1); + } +} \ No newline at end of file