Feature/clean up join messups (#73)

* Somewhere we forgot this. re-adding.

* $model->set('blah')  will set value to title field.

* save() can now combine set/save() by taking argument

* off-load insert() functionality to set()

* insert will return "id" instead of full model.

Because cloned models are not very reliable now.

* counter is part of dsql.

* Applied fixes from StyleCI

* fix some tests

* typo

* more tests

* style

* Add is_numeric

* More field validations

* Add more numeric field check

* style

* Implement hasOne() relationship ID binding.

* Applied fixes from StyleCI

* Implements intelligent model reloading after save()

* No need to complicate things.

* Add documentation

* Applied fixes from StyleCI

* Added afterUpdateQuery / afterInsertQuery

* Implement and document how to prevent default operations on active record

* remove outdated test (since it's no longer on readme)

* Added some really nice advanced topics.

* allow hasOne()->addField('foo'); (instead of saying foo twice)

* Applied fixes from StyleCI

* allow hasOne()->addField('foo'); (instead of saying foo twice)

* Applied fixes from StyleCI

* Improve exception catching in Persistence_SQL

* Use single-table update and single-table delete with where id=X

$m->save() invokes update with a fixed ID. Also it updates joined
tables separately. Because the record has been loaded successfully
before and will be reloaded after updates (in some cases) at a risk of
failed transaction, we don't have to invoke action here.

Action is more suited for all-set update and if you use joins with
SQLite you're on your own, as it does not support join with update.

Same goes for deletion.

* Remove commented out code.

* Expanded documentation.

* merge feature/allow-updating-id

* Revert "merge feature/allow-updating-id"

This reverts commit 77733ae.

* First we need ability to avoid field persistence

If model is saving field and join is saving on top, we get problem.

* Remove misleading test.

No manual tweaking of related values (until we can figure out how to do
it safely, through join)

* avoid clash between join and field save.

* Refactor SQL persistence to use save_buffer()

* implement afterUnload hook.

* make use of afterUnload correctly.

* still call update() on persistence if joins are dirty.

* do everything except actual query if no fields are changed.

* adjust test to verify unloading and changing only joined field.

docs/model.rst

@@ -235,7 +235,7 @@ Hooks
 How to verify Updates
 ---------------------
 
-The model is only being saved if any fields have been changed (dirty).
+The model is only beind saved if any fields have been changed (dirty).
 Sometimes it's possible that the record in the database is no longer
 available and your update() may not actually update anything. This
 does not normally generate an error, however if you want to actually

src/Field.php

@@ -54,6 +54,13 @@ class Field
      */
     public $editable = true;
 
+    /**
+     * Setting this to true will never actually store
+     * the field in the database. It will action as normal,
+     * but will be skipped by update/insert.
+     */
+    public $never_persist = false;
+
     /**
      * Constructor. You can pass field properties as array.
      *

src/Join.php

@@ -159,6 +159,8 @@ public function init()
                 $this->foreign_field = 'id';
             }
         }
+
+        $this->owner->addHook('afterUnload', $this);
     }
 
     /**
@@ -326,4 +328,10 @@ public function set($field, $value)
     {
         $this->save_buffer[$field] = $value;
     }
+
+    public function afterUnload()
+    {
+        $this->id = null;
+        $this->save_buffer = [];
+    }
 }

src/Join_Array.php

@@ -54,11 +54,6 @@ public function afterLoad($model)
         $model->data = array_merge($data, $model->data);
     }
 
-    public function afterUnload()
-    {
-        $this->id = null;
-    }
-
     public function beforeInsert($model, &$data)
     {
         if ($this->weak) {

src/Join_SQL.php

@@ -32,11 +32,6 @@ class Join_SQL extends Join implements \atk4\dsql\Expressionable
      */
     protected $dsql = null;
 
-    /**
-     * calling set() will make this dirty.
-     */
-    protected $dirty = false;
-
     /**
      * Will use either foreign_alias or create #join_<table>.
      */
@@ -83,6 +78,13 @@ public function init()
             $this->owner->addHook('beforeDelete', [$this, 'doDelete'], null, -5);
             $this->owner->addHook('afterLoad', $this);
         } else {
+
+            // If table already has the field corresponding to the "join" then mark it as "do-not-persist"
+            $e = $this->owner->hasElement($this->master_field);
+            if ($e) {
+                $e->never_persist = true;
+            }
+
             $this->owner->addHook('beforeInsertQuery', $this);
             $this->owner->addHook('beforeUpdateQuery', $this);
             $this->owner->addHook('afterDelete', [$this, 'doDelete']);
@@ -140,12 +142,6 @@ public function afterLoad($model)
         unset($model->data[$this->short_name]);
     }
 
-    public function afterUnload($model)
-    {
-        $this->id = null;
-        $this->dirty = false;
-    }
-
     public function beforeInsertQuery($model, $query)
     {
         if ($this->weak) {
@@ -157,7 +153,10 @@ public function beforeInsertQuery($model, $query)
             return;
         }
 
-        $insert = $this->dsql()->set($this->foreign_field, null);
+        $insert = $this->dsql();
+        $insert->mode('insert');
+        $insert->set($this->save_buffer);
+        $insert->set($this->foreign_field, null);
         $insert->insert();
         $this->id = $insert->connection->lastInsertID();
 
@@ -175,6 +174,7 @@ public function afterInsert($model, $id)
         }
 
         $insert = $this->dsql();
+        $insert->set($this->save_buffer);
         $insert
             ->set(
                 $this->foreign_field,
@@ -190,11 +190,12 @@ public function beforeUpdateQuery($model, $query)
             return;
         }
 
-        if (!$this->dirty) {
+        if (!$this->save_buffer) {
             return;
         }
 
         $update = $this->dsql();
+        $update->set($this->save_buffer);
         $update->where($this->foreign_field, $this->id);
         $update->update();
     }
@@ -211,10 +212,4 @@ public function doDelete($model, $id)
 
         $delete->delete()->execute();
     }
-
-    public function set($field, $value)
-    {
-        $this->dirty = true;
-        $this->dsql->set($field, $value);
-    }
 }

src/Model.php

@@ -703,9 +703,11 @@ public function loaded()
      */
     public function unload()
     {
+        $this->hook('beforeUnload');
         $this->id = null;
         $this->data = [];
         $this->dirty = [];
+        $this->hook('afterUnload');
 
         return $this;
     }
@@ -907,6 +909,7 @@ public function save($data = [])
         $is_update = $this->loaded();
         if ($is_update) {
             $data = [];
+            $dirty_join = false;
             foreach ($this->dirty as $name => $junk) {
                 $field = $this->hasElement($name);
                 if (!$field) {
@@ -920,6 +923,7 @@ public function save($data = [])
                 $value = $this->get($name);
 
                 if (isset($field->join)) {
+                    $dirty_join = true;
                     // storing into a different table join
                     $field->join->set($actual, $value);
                 } else {
@@ -928,7 +932,7 @@ public function save($data = [])
             }
 
             // No save needed, nothing was changed
-            if (!$data) {
+            if (!$data && !$dirty_join) {
                 return $this;
             }
 

src/Persistence_SQL.php

@@ -571,7 +571,7 @@ public function insert(Model $m, $data)
         // apply all fields we got from get
         foreach ($data as $field => $value) {
             $f = $m->getElement($field);
-            if (!$f->editable) {
+            if (!$f->editable || $f->never_persist) {
                 continue;
             }
             $insert->set($f->actual ?: $f->short_name, $value);
@@ -649,20 +649,22 @@ public function update(Model $m, $id, $data)
         $cnt = 0;
         foreach ($data as $field => $value) {
             $f = $m->getElement($field);
+            if ($f->never_persist) {
+                continue;
+            }
             $update->set($f->actual ?: $f->short_name, $value);
             $cnt++;
         }
-        if (!$cnt) {
-            return;
-        }
         $update->where($m->getElement($m->id_field), $id);
 
 
         $st = null;
 
         try {
             $m->hook('beforeUpdateQuery', [$update]);
-            $st = $update->execute();
+            if ($cnt) {
+                $st = $update->execute();
+            }
         } catch (\PDOException $e) {
             throw new Exception([
                 'Unable to update due to query error',
@@ -675,7 +677,7 @@ public function update(Model $m, $id, $data)
         $m->hook('afterUpdateQuery', [$update, $st]);
 
         // if any rows were updated in database, and we had expressions, reload
-        if ($m->reload_after_save === true && $st->rowCount()) {
+        if ($m->reload_after_save === true && (!$st || $st->rowCount())) {
             $d = $m->dirty;
             $m->reload();
             $m->_dirty_after_reload = $m->dirty;

tests/JoinSQLTest.php

@@ -77,24 +77,11 @@ public function testJoinSaving1()
 
         $m_u->save();
 
-        $this->assertEquals([
-            'user'    => [1 => ['id' => 1, 'name' => 'John', 'contact_id' => 1]],
-            'contact' => [1 => ['id' => 1, 'contact_phone' => '+123']],
-        ], $this->getDB('user,contact'));
-
-        $m_u->unload();
-        $m_u['name'] = 'Peter';
-        $m_u['contact_id'] = 1;
-        $m_u->save();
         $m_u->unload();
 
         $this->assertEquals([
-            'user' => [
-                1 => ['id' => 1, 'name' => 'John', 'contact_id' => 1],
-                2 => ['id' => 2, 'name' => 'Peter', 'contact_id' => 1],
-            ], 'contact' => [
-                1 => ['id' => 1, 'contact_phone' => '+123'],
-            ],
+            'user'    => [1 => ['id' => 1, 'name' => 'John', 'contact_id' => 1]],
+            'contact' => [1 => ['id' => 1, 'contact_phone' => '+123']],
         ], $this->getDB('user,contact'));
 
         $m_u['name'] = 'Joe';
@@ -104,8 +91,7 @@ public function testJoinSaving1()
         $this->assertEquals([
             'user' => [
                 1 => ['id' => 1, 'name' => 'John', 'contact_id' => 1],
-                2 => ['id' => 2, 'name' => 'Peter', 'contact_id' => 1],
-                3 => ['id' => 3, 'name' => 'Joe', 'contact_id' => 2],
+                2 => ['id' => 2, 'name' => 'Joe', 'contact_id' => 2],
             ], 'contact' => [
                 1 => ['id' => 1, 'contact_phone' => '+123'],
                 2 => ['id' => 2, 'contact_phone' => '+321'],
@@ -272,9 +258,11 @@ public function testJoinUpdate()
             ], ], $this->getDB()
         );
 
-        $m_u->load(3);
+        $m_u->load(1);
         $m_u['name'] = 'XX';
         $m_u['contact_phone'] = '+999';
+        $m_u->load(3);
+        $m_u['name'] = 'XX';
         $m_u->save();
 
 
@@ -285,10 +273,23 @@ public function testJoinUpdate()
                 3 => ['id' => 3, 'name' => 'XX', 'contact_id' => 2],
             ], 'contact' => [
                 1 => ['id' => 1, 'contact_phone' => '+555'],
-                2 => ['id' => 2, 'contact_phone' => '+999'],
+                2 => ['id' => 2, 'contact_phone' => '+321'],
             ], ], $this->getDB()
         );
 
+        $m_u['contact_phone'] = '+999';
+        $m_u->save();
+
+        $this->assertEquals([
+            'user' => [
+                1 => ['id' => 1, 'name' => 'John 2', 'contact_id' => 1],
+                2 => ['id' => 2, 'name' => 'Peter', 'contact_id' => 1],
+                3 => ['id' => 3, 'name' => 'XX', 'contact_id' => 2],
+            ], 'contact' => [
+                1 => ['id' => 1, 'contact_phone' => '+555'],
+                2 => ['id' => 2, 'contact_phone' => '+999'],
+            ], ], $this->getDB()
+        );
 
         $m_u->tryLoad(4);
         $m_u['name'] = 'YYY';