Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Model::{asModel, saveAs, newInstance} methods #856

Merged
merged 6 commits into from
Apr 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 1 addition & 20 deletions docs/model.rst
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ and even perform operations on multiple records (See `Persistence Actions` below
$m->addCondition('expired', true);

$m->action('delete')->execute(); // performs mass delete, hooks are not executed

$m->each(function () use ($m) { $m->delete(); }); // deletes each record, hooks are executed

When data is loaded from associated Persistence, it is automatically converted into
Expand Down Expand Up @@ -434,17 +434,6 @@ This introduces a new business object, which is a sub-set of User. The new class
inherit all the fields, methods and actions of "User" class but will introduce one new
action - `send_gift`.

There are some advanced techniques like "SubTypes" or class substitution,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this was quite a nice feature we used in one project :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See code below, it tries to return different class from load method.

Notice, that a lot of custom code is needed already, you can adjust the code for the removed method easily. Usually, in the case below, you want to retain ID!

for example, this hook may be placed in the "User" class init()::

$this->onHookShort(Model::HOOK_AFTER_LOAD, function() {
if ($this->get('purchases') > 1000) {
$this->breakHook($this->asModel(VipUser::class);
}
});

See also :php:class:`Field\\SubTypeSwitch`


Associating Model with Database
===============================
Expand Down Expand Up @@ -498,14 +487,6 @@ Creates a duplicate of a current model and associate new copy with a specified
persistence. This method is useful for moving model data from one persistence
to another.

.. php:method:: asModel($class, $options = [])

Casts current model into another class. The new model class should be compatible
with $this - you can do `$user->asModel(VipUser::class)` but converting `$user`
into `Invoice::class` is a bad idea.

Although class is switched, the new model will retain current record data, replace all
fields/actions and will combine conditions (avoiding identical conditions).

Populating Data
===============
Expand Down
79 changes: 5 additions & 74 deletions docs/persistence.rst
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ take values of 123 and write it on top of 124?

Here is how::

$m->load(123)->duplicate(124)->replace();
$m->load(123)->duplicate()->setId(124)->save();

Now the record 124 will be replaced with the data taken from record 123.
For SQL that means calling 'replace into x'.
Expand Down Expand Up @@ -482,7 +482,7 @@ Start by creating a beforeSave handler for Order::
if ($this->isDirty('ref')) {

if (
$this->newInstance()
(new static())
->addCondition('client_id', $this->get('client_id')) // same client
->addCondition($this->id_field, '!=', $this->getId()) // has another order
->tryLoadBy('ref', $this->get('ref')) // with same ref
Expand Down Expand Up @@ -537,7 +537,7 @@ The other, more appropriate option is to re-use a vanilla Order record::
function archive() {
$this->save(); // just to be sure, no dirty stuff is left over

$archive = $this->newInstance();
$archive = (new static());
$archive->load($this->getId());
$archive->set('is_archived', true);

Expand All @@ -546,75 +546,6 @@ The other, more appropriate option is to re-use a vanilla Order record::
return $archive;
}

This method may still not work if you extend and use "ActiveOrder" as your
model. In this case you should pass the class to newInstance()::

$archive = $this->newInstance('Order');
// or
$archive = $this->newInstance(new Order());
// or with passing some default properties:
$archive = $this->newInstance([new Order(), 'audit'=>true]);


In this case newInstance() would just associate passed class with the
persistence pretty much identical to::

$archive = new Order($this->persistence);

The use of newInstance() however requires you to load the model which is
an extra database query.

Using Model casting and saveAs
------------------------------

There is another method that can help with escaping the DataSet that does not
involve record loading:

.. php:method:: asModel($class = null, $options = [])

Changes the class of a model, while keeping all the loaded and dirty
values.

The above example would then work like this::

function archive() {
$this->save(); // just to be sure, no dirty stuff is left over

$archive = $o->asModel('Order');
$archive->set('is_archived', true);

$this->unload(); // active record is no longer accessible.

return $archive;
}

Note that after saving 'Order' it may attempt to :ref:`load_after_save` just
to ensure that stored model is a valid 'Order'.

.. php:method:: saveAs($class = null, $options= [])

Save record into the database, using a different class for a model.

As in my archiving example, here is how we can eliminate need of archive()
method altogether::

$o = new ActiveOrder($db);
$o->load(123);

$o->set('is_arhived', true)->saveAs('Order');

Currently the implementation of saveAs is rather trivial, but in the future
versions of Agile Data you may be able to do this::

// MAY NOT WORK YET
$o = new ActiveOrder($db);
$o->load(123);

$o->saveAs('ArchivedOrder');

Of course - instead of using 'Order' you can also specify the object
with `new Order()`.


Working with Multiple Persistences
==================================
Expand Down Expand Up @@ -659,7 +590,7 @@ application::
$m = $this->sql->add(clone $class)->load($id);

// store into MemCache too
$m = $m->withPersistence($this->mdb)->replace();
$m = $m->withPersistence($this->mdb)->save();
}

$m->onHook(Model::HOOK_BEFORE_SAVE, function($m){
Expand Down Expand Up @@ -697,7 +628,7 @@ cache::
$m = $this->sql->add(clone $class)->load($id);

// store into MemCache too
$m = $m->withPersistence($this->mdb)->replace();
$m = $m->withPersistence($this->mdb)->save();
}

Load the record from the SQL database and store it into $m. Next, save $m into
Expand Down
57 changes: 1 addition & 56 deletions src/Model.php
Original file line number Diff line number Diff line change
Expand Up @@ -1287,23 +1287,6 @@ public function duplicate()
return $duplicate;
}

/**
* Saves the current record by using a different
* model class. This is similar to:.
*
* $m2 = $m->newInstance($class);
* $m2->load($m->getId());
* $m2->set($m->get());
* $m2->save();
*
* but will assume that both models are compatible,
* therefore will not perform any loading.
*/
public function saveAs(string $class, array $options = []): self
{
return $this->asModel($class, $options)->save();
}

/**
* Store the data into database, but will never attempt to
* reload the data. Additionally any data will be unloaded.
Expand All @@ -1327,47 +1310,9 @@ public function saveAndUnload(array $data = [])
return $this;
}

/**
* This will cast Model into another class without
* loosing state of your active record.
*/
public function asModel(string $class, array $options = []): self
{
$m = $this->newInstance($class, $options);

foreach ($this->data as $field => $value) {
if ($value !== null && $value !== $this->getField($field)->default && $field !== $this->id_field) {
// Copying only non-default value
$m->set($field, $value);
}
}

// next we need to go over fields to see if any system
// values have changed and mark them as dirty

return $m;
}

/**
* Create new model from the same base class
* as $this.
*
* @return static
*/
public function newInstance(string $class = null, array $options = [])
{
$model = (self::class)::fromSeed([$class ?? static::class], $options);

if ($this->persistence) {
return $this->persistence->add($model); // @phpstan-ignore-line
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the essence of newInstance(). At least for me.
It creates new model object and already links it with same persistence of old object.

Copy link
Member Author

@mvorisek mvorisek Apr 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am aware, I can not be fixed in the constructor as construtor is not aware of the previous instance/persistence.

The solution is to call new $class($model->persistence), eg. pass the persistence manually when needed.

Safety is handled, all DB operations throws if persistence is not set.

}

return $model;
}

/**
* Create new model from the same base class
* as $this. If you omit $id,then when saving
* as $this. If you omit $id then when saving
* a new record will be created with default ID.
* If you specify $id then it will be used
* to save/update your record. If set $id
Expand Down
22 changes: 0 additions & 22 deletions tests/PersistentArrayTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,28 +54,6 @@ public function testLoadArray()
$this->assertSame('Smith', $mm->get('surname'));
}

public function testSaveAs()
{
$p = new Persistence\Array_([
'person' => [
1 => ['name' => 'John', 'surname' => 'Smith', 'gender' => 'M'],
2 => ['name' => 'Sarah', 'surname' => 'Jones', 'gender' => 'F'],
],
]);

$m = new Male($p);
$m->load(1);
$m->saveAs(Female::class);
$m->delete();

$this->assertEquals([
'person' => [
2 => ['name' => 'Sarah', 'surname' => 'Jones', 'gender' => 'F'],
3 => ['name' => 'John', 'surname' => 'Smith', 'gender' => 'F'],
],
], $this->getInternalPersistenceData($p));
}

public function testSaveAndUnload()
{
$p = new Persistence\Array_([
Expand Down
14 changes: 0 additions & 14 deletions tests/RandomTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -480,20 +480,6 @@ public function testExport()
], $m2->export(['code', 'name'], 'code'));
}

public function testNewInstance()
{
// model without persistence
$m = new Model(null, ['table' => 'order']);
$a = $m->newInstance();
$this->assertFalse(isset($a->persistence));

// model with persistence
$db = new Persistence\Array_();
$m = new Model($db, ['table' => 'order']);
$a = $m->newInstance();
$this->assertTrue(isset($a->persistence));
}

public function testDuplicateSaveNew()
{
$this->setDb([
Expand Down
6 changes: 3 additions & 3 deletions tests/ReferenceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,11 @@ public function testRefName3()
$db = new Persistence\Array_();
$order = new Model($db, ['table' => 'order']);
$order->addRef('archive', ['model' => function ($m) {
return $m->newInstance(null, ['table' => $m->table . '_archive']);
return new $m(null, ['table' => $m->table . '_archive']);
}]);
$this->expectException(Exception::class);
$order->addRef('archive', ['model' => function ($m) {
return $m->newInstance(null, ['table' => $m->table . '_archive']);
return new $m(null, ['table' => $m->table . '_archive']);
}]);
}

Expand All @@ -112,7 +112,7 @@ public function testCustomRef()

$m = new Model($p, ['table' => 'user']);
$m->addRef('archive', ['model' => function ($m) {
return $m->newInstance(null, ['table' => $m->table . '_archive']);
return new $m(null, ['table' => $m->table . '_archive']);
}]);

$this->assertSame('user_archive', $m->ref('archive')->table);
Expand Down
6 changes: 3 additions & 3 deletions tests/TypecastingTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public function testType()
$this->assertSame([1, 2, 3], $mm->get('array'));
$this->assertSame(8.202343, $mm->get('float'));

$m/*->duplicate()*/ ->setMulti(array_diff_key($mm->get(), ['id' => true]))->save();
$m->setMulti(array_diff_key($mm->get(), ['id' => true]))->save();

$dbData = [
'types' => [
Expand Down Expand Up @@ -198,7 +198,7 @@ public function testEmptyValues()
$mm->save();
$this->assertEquals($dbData, $this->getDb());

$m/*->duplicate()*/ ->setMulti(array_diff_key($mm->get(), ['id' => true]))->save();
$m->setMulti(array_diff_key($mm->get(), ['id' => true]))->save();

$dbData['types'][2] = [
'id' => 2,
Expand Down Expand Up @@ -293,7 +293,7 @@ public function testTypeCustom1()
$this->assertTrue($mm->get('b1'));
$this->assertFalse($mm->get('b2'));

(clone $m)/*->duplicate()*/ ->setMulti(array_diff_key($mm->get(), ['id' => true]))->save();
(clone $m)->setMulti(array_diff_key($mm->get(), ['id' => true]))->save();
$m->delete(1);

unset($dbData['types'][0]);
Expand Down