Performance optimisation

OpenBuildings · Jan 28, 2014 · 6d4570c · 6d4570c
1 parent 62f119c
commit 6d4570c
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 50 deletions.
diff --git a/classes/Kohana/Jam/Behavior/Promotable/Store/Purchase.php b/classes/Kohana/Jam/Behavior/Promotable/Store/Purchase.php
@@ -2,7 +2,7 @@
 
 /**
  * This behavior adds filter_items and update_items events to work with promotions for Model_Store_Purchase
- * 
+ *
  * @package    openbuildings\promotions
  * @author     Ivan Kerin <ikerin@gmail.com>
  * @copyright  (c) 2013 OpenBuildings Ltd.
@@ -13,7 +13,7 @@ class Kohana_Jam_Behavior_Promotable_Store_Purchase extends Jam_Behavior {
 	/**
 	 * @codeCoverageIgnore
 	 */
-	public function initialize(Jam_Meta $meta, $name) 
+	public function initialize(Jam_Meta $meta, $name)
 	{
 		parent::initialize($meta, $name);
 
@@ -26,15 +26,15 @@ public function initialize(Jam_Meta $meta, $name)
 	/**
 	 * Add a "promotion" filter:
 	 *
-	 * 	array('promotion' => 'promocode_giftcard') 
+	 * 	array('promotion' => 'promocode_giftcard')
 	 *
-	 * This will return only items that are "promotion_promocode_giftcard" models. 
+	 * This will return only items that are "promotion_promocode_giftcard" models.
 	 * Can be an array too.
-	 * 
-	 * @param  Model_Store_Purchase $store_purchase 
-	 * @param  Jam_Event_Data       $data           
-	 * @param  array                $items          
-	 * @param  array                $filter         
+	 *
+	 * @param  Model_Store_Purchase $store_purchase
+	 * @param  Jam_Event_Data       $data
+	 * @param  array                $items
+	 * @param  array                $filter
 	 */
 	public function filter_promotion_items(Model_Store_Purchase $store_purchase, Jam_Event_Data $data, array $items, array $filter)
 	{
@@ -63,8 +63,8 @@ public function filter_promotion_items(Model_Store_Purchase $store_purchase, Jam
 
 	/**
 	 * Convert promotion name to model names
-	 * @param  string|array $name 
-	 * @return array       
+	 * @param  string|array $name
+	 * @return array
 	 */
 	public static function promotion_model_names($name)
 	{
@@ -75,9 +75,9 @@ public static function promotion_model_names($name)
 
 	/**
 	 * Check if purchase item's reference is one of the given promotions
-	 * @param  Model_Purchase_Item $item      
-	 * @param  string|array              $promotion 
-	 * @return boolean                         
+	 * @param  Model_Purchase_Item $item
+	 * @param  string|array              $promotion
+	 * @return boolean
 	 */
 	public static function purchase_item_is_promotion(Model_Purchase_Item $item, $promotion)
 	{
@@ -88,20 +88,22 @@ public static function purchase_item_is_promotion(Model_Purchase_Item $item, $pr
 
 	/**
 	 * Iterate through available promotions and update its status on the store purchase (does it apply or not)
-	 * 
-	 * @param  Model_Store_Purchase $store_purchase 
+	 *
+	 * @param  Model_Store_Purchase $store_purchase
 	 */
 	public function update_promotion_items(Model_Store_Purchase $store_purchase)
 	{
-		foreach ($this->available_promotions() as $promotion) 
+		$items = $store_purchase->items->as_array();
+		foreach ($this->available_promotions() as $promotion)
 		{
-			$promotion->update_store_purchase($store_purchase);
+			$promotion->update_store_purchase_items($promotion->applies_to($store_purchase), $items);
 		}
+		$store_purchase->items = $items;
 	}
 
 	/**
 	 * Return available (non expired) promotions
-	 * @return Jam_Array_Model 
+	 * @return Jam_Array_Model
 	 */
 	public function available_promotions()
 	{

diff --git a/classes/Kohana/Model/Promotion.php b/classes/Kohana/Model/Promotion.php
@@ -81,20 +81,17 @@ public function currency()
 	 * If the promotion applies to the store_purchase - add a purchase_item for this promotion, otherwise remove the associated purchase item
 	 * @param  Model_Store_Purchase $store_purchase
 	 */
-	public function update_store_purchase(Model_Store_Purchase $store_purchase)
+	public function update_store_purchase_items($applies, & $items)
 	{
-		$items = $store_purchase->items->as_array();
-		$promo_item = $this->build_purchase_item();
+		$promo_item = Jam::build('purchase_item_promotion', array('reference' => $this));
 
 		$items = array_filter($items, function($item) use ($promo_item) {
 			return ! $item->is_same($promo_item);
 		});
 
-		if ($this->applies_to($store_purchase))
+		if ($applies)
 		{
 			$items []= $promo_item;
 		}
-
-		$store_purchase->items = $items;
 	}
 }
diff --git a/tests/tests/Jam/Behavior/Promotable/Store/PurchaseTest.php b/tests/tests/Jam/Behavior/Promotable/Store/PurchaseTest.php
@@ -34,12 +34,22 @@ public function test_update_promotion_items()
 	{
 		$store_purchase = Jam::build('store_purchase');
 
-		$promotion = $this->getMock('Model_Promotion', array('update_store_purchase'), array('promotion'));
+		$promotion = $this->getMock('Model_Promotion', array('update_store_purchase_items', 'applies_to'), array('promotion'));
+
+		$promotion
+			->expects($this->at(1))
+			->method('update_store_purchase_items')
+			->with($this->equalTo(TRUE), $this->equalTo(array()));
+
+		$promotion
+			->expects($this->at(3))
+			->method('update_store_purchase_items')
+			->with($this->equalTo(FALSE), $this->equalTo(array()));
 
 		$promotion
 			->expects($this->exactly(2))
-			->method('update_store_purchase')
-			->with($this->identicalTo($store_purchase));
+			->method('applies_to')
+			->will($this->onConsecutiveCalls(TRUE, FALSE));
 
 		$behavior = $this->getMock('Jam_Behavior_Promotable_Store_Purchase', array('available_promotions'));
 

diff --git a/tests/tests/Model/PromotionTest.php b/tests/tests/Model/PromotionTest.php
@@ -74,40 +74,31 @@ public function test_update_store_purchase()
 			array('id' => 15, 'model' => 'purchase_item_product'),
 		);
 
-		$promotion = $this->getMock('Model_Promotion', array(
-			'applies_to'
-		), array(
-			'promotion'
-		));
-
-		$promotion->load_fields(array('id' => 10));
+		$promotion = Jam::build('promotion')->load_fields(array('id' => 10));
 
-		$promotion
-			->expects($this->exactly(4))
-			->method('applies_to')
-			->will($this->onConsecutiveCalls(TRUE, TRUE, FALSE, FALSE));
+		$items = $store_purchase->items->as_array();
 
 		// applies_to = TRUE, offset = 0
-		$promotion->update_store_purchase($store_purchase);
+		$promotion->update_store_purchase_items(TRUE, $items);
 
-		$this->assertCount(3, $store_purchase->items);
-		$this->assertSame($promotion, $store_purchase->items[2]->reference);
+		$this->assertCount(3, $items);
+		$this->assertSame($promotion, $items[2]->reference);
 
 		// applies_to = TRUE, offset = NULL
-		$promotion->update_store_purchase($store_purchase);
+		$promotion->update_store_purchase_items(TRUE, $items);
 
-		$this->assertCount(3, $store_purchase->items);
-		$this->assertSame($promotion, $store_purchase->items[2]->reference);
+		$this->assertCount(3, $items);
+		$this->assertSame($promotion, $items[2]->reference);
 
 		// applies_to = FALSE, offset = 1
-		$promotion->update_store_purchase($store_purchase);
+		$promotion->update_store_purchase_items(FALSE, $items);
 
-		$this->assertCount(2, $store_purchase->items);
-		$this->assertNull($store_purchase->items[2]);
+		$this->assertCount(2, $items);
+		$this->assertFalse(isset($items[2]));
 
 		// applies_to = FALSE, offset = NULL
-		$promotion->update_store_purchase($store_purchase);
+		$promotion->update_store_purchase_items(FALSE, $items);
 
-		$this->assertCount(2, $store_purchase->items);
+		$this->assertCount(2, $items);
 	}
 }