Do not return the same object in {Model, Persistence}::add methods

README.md

@@ -31,7 +31,7 @@ $vip_clients = (new Client($db))->addCondition('is_vip', true);
 // express total for all VIP client invoices. The value of the variable is an object
 $total_due = $vip_clients->ref('Invoice')->action('fx', ['sum', 'total']);
 
-// Single database query is executed here, but not before!
+// single database query is executed here, but not before!
 echo $total_due->getOne();
 ```
 
@@ -42,7 +42,7 @@ In other ORM the similar implementation would be either [slow, clumsy, limited o
 Agile Toolkit is a low-code framework. Once you have defined your business object, it can be associated with a UI widget:
 
 ``` php
-$app->add(new Crud())->setModel(new Client($db), ['name', 'surname'], ['edit', 'archive']);
+Crud::addTo($app)->setModel(new Client($db), ['name', 'surname'], ['edit', 'archive']);
 ```
 
 or with an API end-point:
@@ -126,41 +126,44 @@ This next example builds a complex "Job Profitability Report" by only relying on
 
 ``` php
 class JobReport extends Job {
-  function init(): void {
-    parent::init();
+    function init(): void {
+        parent::init();
 
-    // Invoice contains Lines that may relevant to this job
-    $invoice = new Invoice($this->persistence);
+        // Invoice contains Lines that may relevant to this job
+        $invoice = new Invoice($this->persistence);
 
-    // We need to ignore draft invoices
-    $invoice->addCondition('status', '!=', 'draft');
+        // we need to ignore draft invoices
+        $invoice->addCondition('status', '!=', 'draft');
 
-    // Each invoice may have multiple lines, which is what we want
-    $invoice_lines = $invoice->ref('Lines');
+        // each invoice may have multiple lines, which is what we want
+        $invoice_lines = $invoice->ref('Lines');
 
-    // Build relation between job and invoice line
-    $this->hasMany('InvoiceLines', ['model' => $invoice_lines])
-      ->addField('invoiced', ['aggregate' => 'sum', 'field' => 'total', 'type' => 'atk4_money']);
+        // build relation between job and invoice line
+        $this->hasMany('InvoiceLines', ['model' => $invoice_lines])
+            ->addField('invoiced', ['aggregate' => 'sum', 'field' => 'total', 'type' => 'atk4_money']);
 
-    // Next we need to see how much is reported through timesheets
-    $timesheet = new Timesheet($this->persistence);
+        // next we need to see how much is reported through timesheets
+        $timesheet = new Timesheet($this->persistence);
 
-    // Timesheet relates to client. Import client.hourly_rate as expression.
-    $timesheet->getRef('client_id')->addField('hourly_rate');
+        // timesheet relates to client, import client.hourly_rate as expression
+        $timesheet->getRef('client_id')->addField('hourly_rate');
 
-    // Calculate timesheet cost expression
-    $timesheet->addExpression('cost', '[hours]*[hourly_rate]');
+        // calculate timesheet cost expression
+        $timesheet->addExpression('cost', '[hours]*[hourly_rate]');
 
-    // Build relation between Job and Timesheets
-    $this->hasMany('Timesheets', ['model' => $timesheet])
-      ->addField('reported', ['aggregate' => 'sum', 'field' => 'cost', 'type' => 'atk4_money']);
+        // build relation between Job and Timesheets
+        $this->hasMany('Timesheets', ['model' => $timesheet])
+            ->addField(
+                'reported',
+                ['aggregate' => 'sum', 'field' => 'cost', 'type' => 'atk4_money']
+            );
 
-	// Finally lets calculate profit
-    $this->addExpression('profit', '[invoiced]-[reported]');
+        // finally lets calculate profit
+        $this->addExpression('profit', '[invoiced]-[reported]');
 
-    // Profit margin could be also useful
-    $this->addExpression('profit_margin', 'coalesce([profit] / [invoiced], 0)');
-  }
+        // profit margin could be also useful
+        $this->addExpression('profit_margin', 'coalesce([profit] / [invoiced], 0)');
+    }
 }
 ```
 
@@ -190,7 +193,7 @@ $data->addExpression('month', 'month([date])');
 $aggregate = new \Atk4\Report\GroupModel($data);
 $aggregate->groupBy('month', ['profit_margin' => 'sum']);
 
-// Associate presentation with data
+// associate presentation with data
 $chart->setModel($aggregate, ['month', 'profit_margin']);
 $html = $chart->html();
 ```
@@ -294,16 +297,16 @@ You get to manipulate your objects first before query is invoked. The next code
 ``` php
 $m = new Client($db);
 echo $m->addCondition('vip', true)
-  ->ref('Order')->ref('Line')->action('fx', ['sum', 'total'])->getOne();
+    ->ref('Order')->ref('Line')->action('fx', ['sum', 'total'])->getOne();
 ```
 
 Resulting Query will always use parametric variables if vendor driver supports them (such as PDO):
 
 ``` sql
 select sum(`price`*`qty`) from `order_line` `O_L` where `order_id` in (
-  select `id` from `order` `O` where `client_id` in (
-    select `id` from `client` where `vip` = :a
-  )
+    select `id` from `order` `O` where `client_id` in (
+        select `id` from `client` where `vip` = :a
+    )
 )
 
 // :a is "Y"
@@ -319,11 +322,11 @@ My next example demonstrates how simple and clean your code looks when you store
 $m = new Client($db);
 $m->loadBy('name', 'Pear Company');
 $m->ref('Order')
-   ->save(['ref' => 'TBL1', 'delivery' => new DateTime('+1 month')])
-   ->ref('Lines')->import([
-      ['Table', 'category' => 'furniture', 'qty' => 2, 'price' => 10.50],
-      ['Chair', 'category' => 'furniture', 'qty' => 10, 'price' => 3.25],
-]);
+    ->save(['ref' => 'TBL1', 'delivery' => new DateTime('+1 month')])
+    ->ref('Lines')->import([
+        ['Table', 'category' => 'furniture', 'qty' => 2, 'price' => 10.50],
+        ['Chair', 'category' => 'furniture', 'qty' => 10, 'price' => 3.25],
+    ]);
 ```
 
 Resulting queries (I have removed back-ticks and parametric variables for readability) use a consise syntax and demonstrate some of the "behind-the-scenes" logic:
@@ -502,7 +505,7 @@ foreach ($client->ref('Project') as $project) {
 }
 
 // $project refers to same object at all times, but $project's active data
-// is re-populated on each iteration.
+// is re-populated on each iteration
 ```
 
 Nothing unnecessary is pre-fetched. Only requested columns are queried. Rows are streamed and never ever we will try to squeeze a large collection of IDs into a variable or a query.
@@ -575,7 +578,8 @@ class User extends \Atk4\Data\Model
         parent::init();
 
         $this->addFields(['email', 'name', 'password']);
-        // use your table fields here
+
+        // add your table fields here
     }
 }
 ```
@@ -584,7 +588,9 @@ Next create `console.php`:
 
 ``` php
 <?php
-include'vendor/autoload.php';
+
+require __DIR__ . '/vendor/autoload.php';
+
 $db = \Atk4\Data\Persistence::connect(PDO_DSN, USER, PASS);
 eval(\Psy\sh());
 ```
@@ -625,41 +631,39 @@ DSQL Is Simple and Powerful
 
 ``` php
 $query = new Atk4\Data\Persistence\Sql\Query();
-$query  ->table('employees')
-        ->where('birth_date','1961-05-02')
-        ->field('count(*)')
-        ;
-echo "Employees born on May 2, 1961: ".$query->getOne();
+$query->table('employees')
+    ->where('birth_date','1961-05-02')
+    ->field('count(*)');
+echo 'Employees born on May 2, 1961: ' . $query->getOne();
 ```
 
 If the basic query is not fun, how about more complex one?
 
 ``` php
-// Establish a query looking for a maximum salary
+// establish a query looking for a maximum salary
 $salary = new Atk4\Data\Persistence\Sql\Query(['connection' => $pdo]);
 
-// Create few expression objects
+// create few expression objects
 $e_ms = $salary->expr('max(salary)');
 $e_df = $salary->expr('TimeStampDiff(month, from_date, to_date)');
 
-// Configure our basic query
+// configure our basic query
 $salary
     ->table('salary')
     ->field(['emp_no', 'max_salary' => $e_ms, 'months' => $e_df])
     ->group('emp_no')
     ->order('-max_salary')
 
-// Define sub-query for employee "id" with certain birth-date
+// define sub-query for employee "id" with certain birth-date
 $employees = $salary->dsql()
     ->table('employees')
     ->where('birth_date','1961-05-02')
-    ->field('emp_no')
-    ;
+    ->field('emp_no');
 
 // use sub-select to condition salaries
 $salary->where('emp_no', $employees);
 
-// Join with another table for more data
+// join with another table for more data
 $salary
     ->join('employees.emp_id','emp_id')
     ->field('employees.first_name');

src/Model.php

@@ -541,15 +541,15 @@ private function initEntityIdHooks(): void
         $this->onHookShort(self::HOOK_AFTER_SAVE, $fx, [], -10);
     }
 
-    public function add(object $obj, array $defaults = []): object
+    public function add(object $obj, array $defaults = []): void
     {
         $this->assertIsModel();
 
         if ($obj instanceof Field) {
             throw new Exception('Field can be added using addField() method only');
         }
 
-        return $this->_add($obj, $defaults);
+        $this->_add($obj, $defaults);
     }
 
     public function _addIntoCollection(string $name, object $item, string $collection): object

src/Model/JoinsTrait.php

@@ -41,7 +41,9 @@ public function join(string $foreignTable, array $defaults = []): Join
                 ->addMoreInfo('foreignTable', $foreignTable);
         }
 
-        return $this->add($join);
+        $this->add($join);
+
+        return $join;
     }
 
     /**

src/Model/ReferencesTrait.php

@@ -65,7 +65,9 @@ protected function _addRef(array $seed, string $link, array $defaults = []): Ref
                 ->addMoreInfo('link', $link);
         }
 
-        return $this->add($reference);
+        $this->add($reference);
+
+        return $reference;
     }
 
     /**

src/Persistence.php

@@ -78,29 +78,23 @@ public function disconnect(): void
     /**
      * Associate model with the data driver.
      */
-    public function add(Model $m, array $defaults = []): Model
+    public function add(Model $model, array $defaults = []): void
     {
-        $m = Factory::factory($m, $defaults);
+        Factory::factory($model, $defaults);
 
-        if ($m->persistence) {
-            if ($m->persistence === $this) {
-                return $m;
-            }
-
-            throw new Exception('Model is already related to another persistence');
+        if ($model->persistence !== null) {
+            throw new Exception('Persistence already set');
         }
 
-        $m->persistence = $this;
-        $m->persistence_data = [];
-        $this->initPersistence($m);
+        $model->persistence = $this;
+        $model->persistence_data = [];
+        $this->initPersistence($model);
 
         // invokes Model::init()
         // model is not added to elements as it does not implement TrackableTrait trait
-        $m = $this->_add($m);
-
-        $this->hook(self::HOOK_AFTER_ADD, [$m]);
+        $this->_add($model);
 
-        return $m;
+        $this->hook(self::HOOK_AFTER_ADD, [$model]);
     }
 
     /**

src/Persistence/Array_.php

@@ -156,10 +156,7 @@ private function saveRow(Model $model, array $rowData, $id): void
         }
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function add(Model $model, array $defaults = []): Model
+    public function add(Model $model, array $defaults = []): void
     {
         if (isset($defaults[0])) {
             $model->table = $defaults[0];
@@ -170,7 +167,7 @@ public function add(Model $model, array $defaults = []): Model
             '_default_seed_join' => [Array_\Join::class],
         ], $defaults);
 
-        $model = parent::add($model, $defaults);
+        parent::add($model, $defaults);
 
         // if there is no model table specified, then create fake one named 'data'
         // and put all persistence data in there 2/2
@@ -186,8 +183,6 @@ public function add(Model $model, array $defaults = []): Model
         }
 
         $this->seedData($model);
-
-        return $model;
     }
 
     private function filterRowDataOnlyModelFields(Model $model, array $rowData): array

src/Persistence/Sql.php

@@ -140,10 +140,7 @@ public function getDatabasePlatform(): AbstractPlatform
         return $this->connection->getDatabasePlatform();
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function add(Model $model, array $defaults = []): Model
+    public function add(Model $model, array $defaults = []): void
     {
         // Use our own classes for fields, references and expressions unless
         // $defaults specify them otherwise.
@@ -155,7 +152,7 @@ public function add(Model $model, array $defaults = []): Model
             '_default_seed_join' => $this->_default_seed_join,
         ], $defaults);
 
-        $model = parent::add($model, $defaults);
+        parent::add($model, $defaults);
 
         if ($model->table === null) {
             throw (new Exception('Property $table must be specified for a model'))
@@ -170,8 +167,6 @@ public function add(Model $model, array $defaults = []): Model
             // SQL databases use ID of int by default
             //$m->getField($m->id_field)->type = 'integer';
         }
-
-        return $model;
     }
 
     /**

src/Persistence/Sql/Expression.php

@@ -7,6 +7,7 @@
 use Atk4\Core\WarnDynamicPropertyTrait;
 use Doctrine\DBAL\Connection as DbalConnection;
 use Doctrine\DBAL\Exception as DbalException;
+use Doctrine\DBAL\Platforms\OraclePlatform;
 use Doctrine\DBAL\Platforms\PostgreSQL94Platform;
 use Doctrine\DBAL\Result as DbalResult;
 
@@ -178,7 +179,7 @@ public function offsetUnset($offset): void
     public function expr($properties = [], $arguments = null)
     {
         if ($this->connection !== null) {
-            // TODO - condition above always satisfied when connection is set - adjust tests,
+            // TODO condition above always satisfied when connection is set - adjust tests,
             // so connection is always set and remove the code below
             return $this->connection->expr($properties, $arguments);
         }
@@ -595,7 +596,7 @@ private function getCastValue($v): ?string
 
         // for Oracle CLOB/BLOB datatypes and PDO driver
         if (is_resource($v) && get_resource_type($v) === 'stream'
-                && $this->connection->getDatabasePlatform() instanceof \Doctrine\DBAL\Platforms\OraclePlatform) {
+                && $this->connection->getDatabasePlatform() instanceof OraclePlatform) {
             $v = stream_get_contents($v);
         }