From c6c8e371e1c2053204ef5510294128e40faefc3b Mon Sep 17 00:00:00 2001
From: Laurent Laffont <llaffont@afi-sa.fr>
Date: Tue, 14 Dec 2021 11:44:56 +0100
Subject: [PATCH] Implements Storm_Model_Loader::updateAll

Storm_Model_Loader now handles update request like:

Video::updateAllBy(['support' => 'vhs',
                    'where' => 'year < 2000'],
                   ['support' => 'dvd']);

Also handle like and negative clauses in updateAll and deleteBy
---
 src/Storm/Model/Loader.php                    | 18 ++++
 .../Model/PersistenceStrategy/Abstract.php    |  7 +-
 src/Storm/Model/PersistenceStrategy/Db.php    | 55 ++++++------
 .../Model/PersistenceStrategy/Volatile.php    | 86 +++++++++++--------
 tests/Storm/Test/LoaderTest.php               | 38 ++++++--
 tests/Storm/Test/LoaderVolatileTest.php       | 31 ++++++-
 6 files changed, 161 insertions(+), 74 deletions(-)

diff --git a/src/Storm/Model/Loader.php b/src/Storm/Model/Loader.php
index 05a73fbf..1a7a9660 100644
--- a/src/Storm/Model/Loader.php
+++ b/src/Storm/Model/Loader.php
@@ -139,6 +139,7 @@ class Storm_Model_Loader {
     return $this->_id_field;
   }
 
+
   /**
    * @param mixed $select
    * @return array
@@ -414,6 +415,23 @@ class Storm_Model_Loader {
     return  isset($filter) ? array_filter($result, $filter) : $result;
   }
 
+
+  /**
+   * Execute an update query on all rows matching $where
+   * and apply new $values
+   * @param array $where update where condition
+   * @param array $set_values update set statement
+   * @return int number of affected rows
+   * @example
+   * Video::updateAllBy(['support' => 'vhs',
+   *                     'where' => 'year < 2000'],
+   *                    ['support' => 'dvd']);
+   */
+  public function updateAll(array $where, array $set_values) : int {
+    return $this->getPersistenceStrategy()->updateAll($where, $set_values);
+  }
+
+
   /**
    * Sends a count request and returns the value
    * @see findAllBy
diff --git a/src/Storm/Model/PersistenceStrategy/Abstract.php b/src/Storm/Model/PersistenceStrategy/Abstract.php
index 6ef3b814..c99ba05c 100644
--- a/src/Storm/Model/PersistenceStrategy/Abstract.php
+++ b/src/Storm/Model/PersistenceStrategy/Abstract.php
@@ -42,4 +42,9 @@ class Storm_Model_PersistenceStrategy_Abstract  {
   public function fetchAllBy($fields, $params) {
     return [];
   }
-}
\ No newline at end of file
+
+
+  public function updateAll(array $where, array $set_values) : int {
+    return 0;
+  }
+}
diff --git a/src/Storm/Model/PersistenceStrategy/Db.php b/src/Storm/Model/PersistenceStrategy/Db.php
index c68c7eb2..4dad81e9 100644
--- a/src/Storm/Model/PersistenceStrategy/Db.php
+++ b/src/Storm/Model/PersistenceStrategy/Db.php
@@ -66,6 +66,12 @@ class Storm_Model_PersistenceStrategy_Db extends Storm_Model_PersistenceStrategy
   }
 
 
+  public function updateAll(array $where, array $set_values) : int {
+    return $this->getTable()->update($set_values,
+                                     $this->_generateWhereClause($where));
+  }
+
+
   public function delete($model) {
     return $this->deleteBy([$this->_loader->getIdField() => $model->getId()]);
   }
@@ -75,23 +81,36 @@ class Storm_Model_PersistenceStrategy_Db extends Storm_Model_PersistenceStrategy
     if (!is_array($where_or_clauses))
       return $this->getTable()->delete($where_or_clauses);
 
+    return $this->getTable()
+                ->delete($this->_generateWhereClause($where_or_clauses));
+  }
+
+
+  protected function _generateWhereClause(array $where_or_clauses) : string {
     $where = [];
     foreach($where_or_clauses as $key => $value)
       $where []= $this->_generateWhereClauseForKeyAndValue($key, $value);
-
-    return $this->getTable()->delete(implode(' and ', $where));
+    return implode(' and ', $where);
   }
 
 
-  protected function _generateWhereClauseForKeyAndValue($key, $value_or_array) {
+  protected function _generateWhereClauseForKeyAndValue(string $key, $value_or_array) : string {
     if ('where' == $key)
       return '(' . $value_or_array . ')';
 
-    $operator = is_array($value_or_array)
-      ? ' in (?)'
-      : '=?';
+    if (' like' == substr($key, -5))
+      return $this->quoteInto($key . ' ?', $value_or_array);
+
+    $negated = (' not' == substr($key, -4));
+    $key = $negated ? substr($key, 0, strlen($key) - 4) : $key;
+
+    if (is_array($value_or_array))
+      return $this->quoteInto($key . ($negated ? ' not': '') . ' in (?)', $value_or_array);
 
-    return $this->quoteInto($key . $operator, $value_or_array);
+    if (null === $value_or_array)
+      return $key . ' is' . ($negated ? ' not' : '') . ' null';
+
+    return $this->quoteInto($key . ($negated ? '!': '') . '=?', $value_or_array);
   }
 
 
@@ -183,27 +202,7 @@ class Storm_Model_PersistenceStrategy_Db extends Storm_Model_PersistenceStrategy
         continue;
       }
 
-
-      if (' like' == substr($field, -5)) {
-        $select->where($field . ' ?', $value);
-        continue;
-      }
-
-
-      $negated = (' not' == substr($field, -4));
-      $field = $negated ? substr($field, 0, strlen($field) - 4): $field;
-
-      if (is_array($value)) {
-        $select->where($field . ($negated ? ' not': '') . ' in (?)', $value);
-        continue;
-      }
-
-      if (null === $value) {
-        $select->where($field . ' is' . ($negated ? ' not' : '') . ' null');
-        continue;
-      }
-
-      $select->where($field . ($negated ? '!': '') . '=?', $value);
+      $select->where($this->_generateWhereClauseForKeyAndValue($field, $value));
     }
 
     return $select;
diff --git a/src/Storm/Model/PersistenceStrategy/Volatile.php b/src/Storm/Model/PersistenceStrategy/Volatile.php
index 96f0b032..b6466809 100644
--- a/src/Storm/Model/PersistenceStrategy/Volatile.php
+++ b/src/Storm/Model/PersistenceStrategy/Volatile.php
@@ -52,7 +52,6 @@ class Storm_Model_PersistenceStrategy_Volatile  extends Storm_Model_PersistenceS
     $order = isset($filtered_select['order']) ? $filtered_select['order'] : '';
     $limit = isset($filtered_select['limit']) ? $filtered_select['limit'] : '';
 
-    $instances = $this->getInstancesArray();
     $page_size=0;
     if (isset($filtered_select['limitpage'])) {
       list($page, $page_size) = $filtered_select['limitpage'];
@@ -62,27 +61,12 @@ class Storm_Model_PersistenceStrategy_Volatile  extends Storm_Model_PersistenceS
     foreach($this->special_select_fields as $field)
       unset($filtered_select[$field]);
 
-    $negations = [];
-
-    foreach($filtered_select as $k => $v) {
-      if (false !== strpos($k, ' not')) {
-        $negations[str_replace(' not', '', $k)] = $v;
-        unset($filtered_select[$k]);
-      }
-    }
+    $values = [];
+    $this->_allMatchingInstancesDo($filtered_select,
+                                   function($model) use (&$values) {
+                                     $values []= $model;
+                                   });
 
-    $values = array_values(
-      array_filter(
-        $instances,
-        function($instance) use($filtered_select) {
-          return $this->containsAllAttributes($instance, $filtered_select);
-        }));
-
-    if (!empty($negations))
-      $values = array_values(array_filter($values,
-                                          function($instance) use($negations) {
-                                            return $this->containsNoneAttributes($instance, $negations);
-                                          }));
 
     $values = $this->ordered($values, $order);
     $values = $this->limited($values, $limit);
@@ -153,7 +137,7 @@ class Storm_Model_PersistenceStrategy_Volatile  extends Storm_Model_PersistenceS
 
 
 
-  protected function getInstancesArray() {
+  protected function getInstancesArray() : array {
     return
       array_values(
         array_map(
@@ -161,7 +145,8 @@ class Storm_Model_PersistenceStrategy_Volatile  extends Storm_Model_PersistenceS
           $this->_instances));
   }
 
-  public function findAllBy($args) {
+
+  public function findAllBy($args) : array {
     return $this->findAll($args);
   }
 
@@ -199,7 +184,7 @@ class Storm_Model_PersistenceStrategy_Volatile  extends Storm_Model_PersistenceS
   }
 
 
-  public function containsAllAttributes($model, $select) {
+  public function containsAllAttributes(array $model, array $select) : bool {
     foreach ($select as $key => $value)
       if (!$this->containsAttribute($model, $key, $value))
         return false;
@@ -208,7 +193,7 @@ class Storm_Model_PersistenceStrategy_Volatile  extends Storm_Model_PersistenceS
   }
 
 
-  public function containsNoneAttributes($model, $select) {
+  public function containsNoneAttributes(array $model, array $select) : bool {
     foreach ($select as $key => $value)
       if ($this->containsAttribute($model, $key, $value))
         return false;
@@ -217,7 +202,7 @@ class Storm_Model_PersistenceStrategy_Volatile  extends Storm_Model_PersistenceS
   }
 
 
-  public function containsAttribute($model, $key, $value) {
+  public function containsAttribute(array $model, string $key, $value) : bool {
     if (preg_match('/left\((.+),(.+)\)/', $key, $matches)) {
       $key = trim($matches[1]);
       $left_len = (int)$matches[2];
@@ -298,28 +283,55 @@ class Storm_Model_PersistenceStrategy_Volatile  extends Storm_Model_PersistenceS
   }
 
 
-  public function deleteBy($clauses) {
-    $delete_count = 0;
+  public function updateAll(array $where, array $set_values) : int {
+    return $this->_allMatchingInstancesDo($where,
+                                          function($model) use ($set_values) {
+                                            $this->_instances[$model['id']]->initializeAttributes($set_values);
+                                          });
+  }
+
+
+  public function deleteBy(array $clauses) : int {
+    return $this->_allMatchingInstancesDo($clauses,
+                                          function($model) {
+                                            unset($this->_instances[$model['id']]);
+                                          },
+                                          $this->getInstancesArray());
+  }
+
 
-    if (!is_array($clauses))
-      return $delete_count;
+  protected function _allMatchingInstancesDo(array $clauses, callable $callback) : int {
+    $negations = [];
 
-    foreach($this->getInstancesArray() as $id => $model) {
-      if ($this->containsAllAttributes($model, $clauses)) {
-        unset($this->_instances[$model['id']]);
-        $delete_count += 1;
+    foreach($clauses as $k => $v) {
+      if (false !== strpos($k, ' not')) {
+        $negations[str_replace(' not', '', $k)] = $v;
+        unset($clauses[$k]);
       }
     }
 
-    return $delete_count;
+    $matching_instances = array_filter($this->getInstancesArray(),
+                                       function($model) use ($clauses, $negations) {
+                                         return $this->_modelMatchesClausesAndNegations($model, $clauses, $negations);
+                                       });
+
+    return count(array_map($callback, $matching_instances));
   }
 
 
-  public function countBy($args) {
+  protected function _modelMatchesClausesAndNegations(array $model, array $clauses, array $negations) : bool {
+    return
+      $this->containsAllAttributes($model, $clauses)
+      && (empty($negations) || $this->containsNoneAttributes($model, $negations));
+  }
+
+
+  public function countBy(array $args) : int {
     return sizeof($this->findAll($args));
   }
 
-  public function isVolatile() {
+
+  public function isVolatile() : bool {
     return true;
   }
 }
diff --git a/tests/Storm/Test/LoaderTest.php b/tests/Storm/Test/LoaderTest.php
index cdcab35d..30b51530 100644
--- a/tests/Storm/Test/LoaderTest.php
+++ b/tests/Storm/Test/LoaderTest.php
@@ -157,11 +157,11 @@ class Storm_Test_LoaderBasicTest extends Storm_Test_LoaderTestCase {
   public function findAllByGeneratedSelects() {
     return
       [
-       [ ['name not' => 'Harlock'] , ['name!=?', 'Harlock'] ],
-       [ ['name not' => ['Harlock', 'Nausicaa']] , ['name not in (?)', ['Harlock', 'Nausicaa']] ],
+       [ ['name not' => 'Harlock'] , ['name!=\'Harlock\''] ],
+       [ ['name not' => ['Harlock', 'Nausicaa']] , ['name not in (\'Harlock\',\'Nausicaa\')'] ],
        [ ['name not' => null] , ['name is not null'] ],
-       [ ['login like' => '%aus%'] , ['login like ?', '%aus%'] ],
-       [ ['login not like' => '%aus%'] , ['login not like ?', '%aus%'] ]
+       [ ['login like' => '%aus%'] , ['login like \'%aus%\''] ],
+       [ ['login not like' => '%aus%'] , ['login not like \'%aus%\''] ]
       ];
   }
 
@@ -232,6 +232,32 @@ class Storm_Test_LoaderBasicTest extends Storm_Test_LoaderTestCase {
 
     $this->assertEquals('nom = "dupont"', $this->_table->getFirstAttributeForLastCallOn('delete'));
   }
+
+
+  /** @test */
+  public function updateAllWithNomArrayBondLupinToTypeFictiveShouldGenerateUpdateQuery() {
+    $this->_table->whenCalled('update')->answers(2);
+    $this->_loader->updateAll(['nom' => ['bond', 'lupin'],
+                               'type' => 'real'],
+                              ['type' => 'fictive']);
+
+    $this->assertEquals([['type' => 'fictive'],
+                         "nom in ('bond','lupin') and type='real'"],
+                        $this->_table->getAttributesForLastCallOn('update'));
+  }
+
+
+  /** @test */
+  public function updateAllWithNomNotBondShouldGenerateUpdateQueryWithNegations() {
+    $this->_table->whenCalled('update')->answers(1);
+    $this->_loader->updateAll(['nom not' => 'bond'],
+                              ['type' => 'fictive']);
+
+    $this->assertEquals([['type' => 'fictive'],
+                         "nom!='bond'"],
+                        $this->_table->getAttributesForLastCallOn('update'));
+  }
+
 }
 
 
@@ -247,7 +273,7 @@ class Storm_Test_LoaderSaveWithIdTest extends Storm_Test_LoaderTestCase {
       ->answers($this->_select = $this->mock());
 
     $this->_select
-      ->whenCalled('where')->with('id=?', '4')->answers($this->_select)
+      ->whenCalled('where')->with('id=\'4\'')->answers($this->_select)
       ->whenCalled('from')->with($this->_table, ['count(*) as numberof'])->answers($this->_select);
 
   }
@@ -289,4 +315,4 @@ class Storm_Test_LoaderSaveWithIdTest extends Storm_Test_LoaderTestCase {
     $this->assertArraySubset(['id' => 4],
                              $this->_table->getFirstAttributeForLastCallOn('update'));
   }
-}
\ No newline at end of file
+}
diff --git a/tests/Storm/Test/LoaderVolatileTest.php b/tests/Storm/Test/LoaderVolatileTest.php
index 2b8c1495..a42e61b9 100644
--- a/tests/Storm/Test/LoaderVolatileTest.php
+++ b/tests/Storm/Test/LoaderVolatileTest.php
@@ -698,8 +698,11 @@ class Storm_Test_LoaderVolatileTest extends Storm_Test_ModelTestCase {
   }
 
 
-  /** @test */
-  public function basicDeleteByWithWhereStringShouldIgnoreIt() {
+  /**
+   * @expectedException TypeError
+   * @test
+   */
+  public function basicDeleteByWithWhereStringShouldRaiseError() {
     Storm_Test_VolatileUser::basicDeleteBy('login = "albert"');
     $this->assertEquals(3, Storm_Test_VolatileUser::count());
   }
@@ -750,5 +753,29 @@ class Storm_Test_LoaderVolatileTest extends Storm_Test_ModelTestCase {
                    ['id' => 764,
                     'name' => 'dog']);
   }
+
+
+
+  /** @test */
+  public function updateAllToUsersByLevelInviteToHeroShouldSetHubertAndMaxToHero() {
+    $this->max->save();
+    $this->assertEquals(2, Storm_Test_VolatileUser::updateAll(['level' => 'invite'],
+                                                              ['level' => 'hero']));
+    $this->assertEquals(['hubert', 'max'],
+                        (new Storm_Model_Collection(Storm_Test_VolatileUser::findAllBy(['level' => 'hero'])))
+                        ->collect('login')
+                        ->getArrayCopy());
+  }
+
+
+  /** @test */
+  public function updateAllToUsersByNotLevelInviteToHeroShouldSetAlbertAndZoeToHero() {
+    $this->assertEquals(2, Storm_Test_VolatileUser::updateAll(['level not' => 'invite'],
+                                                              ['level' => 'hero']));
+    $this->assertEquals(['albert', 'zoe'],
+                        (new Storm_Model_Collection(Storm_Test_VolatileUser::findAllBy(['level' => 'hero'])))
+                        ->collect('login')
+                        ->getArrayCopy());
+  }
 }
 ?>
-- 
GitLab