From d2d8bee0db5a6bc00d732f02aaeb49ab6cbafe5b Mon Sep 17 00:00:00 2001 From: Andrew H Date: Wed, 18 Nov 2020 14:48:51 -0500 Subject: [PATCH 01/19] Update RevisionScope query for better execution plan --- src/Scopes/RevisionScope.php | 47 +++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/src/Scopes/RevisionScope.php b/src/Scopes/RevisionScope.php index 3f3b90e..9ddba23 100644 --- a/src/Scopes/RevisionScope.php +++ b/src/Scopes/RevisionScope.php @@ -5,7 +5,6 @@ use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Scope; -use Plank\Checkpoint\Models\Checkpoint; use Plank\Checkpoint\Models\Revision; class RevisionScope implements Scope @@ -55,32 +54,42 @@ public function extend(Builder $builder) */ protected function addTemporal(Builder $builder) { - $builder->macro('temporal', function (Builder $builder, $upper = null, $lower = null) { + // Constrain the scope to a specific window of time using valid checkpoints, carbon objects or datetime strings + $builder->macro('temporal', function (Builder $builder, $until = null, $since = null) { $model = $builder->getModel(); - $revision = config('checkpoint.revision_model', Revision::class); + $revision = new Revision(); $builder->withoutGlobalScope($this); // worst case execution plan : #reads = all rows in your table * all checkpoints * all revisions - // the timestamps subquery is the most expensive, reading all revisions from disk, try to build an index to fit it. + // avg case execution plan: all rows in your table * 1 revision * 1 revision * 0..max amount of checkpoints - //TODO: watch out for hardcoded columns names and relation names + // METHOD 1: Join current table on revisions, join the result on closest subquery - // METHOD 1: Join current table on revisions, join the result on timestamps subquery - /* $builder->join('revisions', $model->getQualifiedKeyName(), '=', 'revisionable_id') - ->joinSub( - $revision::latestIds($upper, $lower), - 'temporal', 'temporal.closest', '=', - (new $revision)->getQualifiedKeyName() - )->where('revisionable_type', '=', get_class($model));*/ + // METHOD 2 : Join current table on revisions, filter out by original and type index, use a where in for the closest ids subquery +/* $builder->join("{$revision->getTable()} as _r", $model->getQualifiedKeyName(), '=', 'revisionable_id') + ->whereIn("_r.{$revision->getKeyName()}", + $revision::latestIds($until, $since) + ->whereColumn('original_revisionable_id', '_r.original_revisionable_id') + ->whereType($model) + )->where('revisionable_type', '=', get_class($model)); - // METHOD 2 : Join current table on revisions, filter out by revisionable_type, use a whereIn subquery for timestamps - /* $builder->join('revisions', $model->getQualifiedKeyName(), '=', 'revisionable_id') - ->whereIn((new $revision)->getQualifiedKeyName(), $revision::latestIds($upper, $lower)) - ->where('revisionable_type', '=', get_class($model));*/ + // when using a join, you need to specific the select manually to avoid grabbing all the columns + // but when using addSelect and giving it a subquery, laravel automatically selects the main table + // causing duplicate columns + $select = $model->getTable().'.*'; + $columns = $builder->getQuery()->columns; + if ($columns === null || !in_array($select, $columns, true)) { + $builder->addSelect($select); + } + $builder->addSelect(['_r.metadata']);*/ - // METHOD 3 : Uses a where exists wrapper, laravel handles the revisionable id/type, use a whereIn subquery for timestamps - $builder->whereHas('revision', function (Builder $query) use ($revision, $upper, $lower) { - $query->whereIn((new $revision)->getQualifiedKeyName(), $revision::latestIds($upper, $lower)); + // METHOD 3 : Uses a where exists wrapper on a where in subquery for closest timestamps + $builder->whereHas('revision', function (Builder $query) use ($model, $until, $since) { + $query->whereIn($query->getModel()->getQualifiedKeyName(), + $query->newModelInstance()->setTable('_r')->from($query->getModel()->getTable(), '_r') + ->latestIds($until, $since) + ->whereColumn($query->qualifyColumn('original_revisionable_id'), '_r.original_revisionable_id') + ->whereType($model)); }); return $builder; From efd7a48d86cbbb7444140327a36eef0ed6789293 Mon Sep 17 00:00:00 2001 From: Andrew H Date: Wed, 18 Nov 2020 14:55:23 -0500 Subject: [PATCH 02/19] Update Checkpoint relations, Revision relations and latestIds scope, clean up RevisionScope temporal extension --- src/Models/Checkpoint.php | 117 +++++++++++++++++++++++++++-------- src/Models/Revision.php | 57 +++++++++-------- src/Scopes/RevisionScope.php | 34 +++------- 3 files changed, 132 insertions(+), 76 deletions(-) diff --git a/src/Models/Checkpoint.php b/src/Models/Checkpoint.php index 8c4dfcd..d551a63 100644 --- a/src/Models/Checkpoint.php +++ b/src/Models/Checkpoint.php @@ -1,11 +1,11 @@ getCheckpointDateColumn(); - return $query->where($column, '<', $checkpoint->getCheckpointDate())->latest($column); - } - - /** - * Apply a scope to filter for checkpoints newer than the one provided ordered by checkpoint date asc - * - * @param \Illuminate\Database\Eloquent\Builder $query - * @param Checkpoint $checkpoint + * Return current checkpoint at this moment in time * - * @return \Illuminate\Database\Eloquent\Builder + * @return Checkpoint|Model|null */ - public function scopeNewerThan(Builder $query, Checkpoint $checkpoint) - { - $column = $checkpoint->getCheckpointDateColumn(); - return $query->where($column, '>', $checkpoint->getCheckpointDate())->oldest($column); + public static function current() { + return static::olderThan(now())->first(); } /** @@ -117,7 +105,7 @@ public function scopeNewerThan(Builder $query, Checkpoint $checkpoint) */ public function previous() { - return self::olderThan($this)->first(); + return static::olderThan($this)->first(); } /** @@ -127,7 +115,7 @@ public function previous() */ public function next() { - return self::newerThan($this)->first(); + return static::newerThan($this)->first(); } /** @@ -166,4 +154,79 @@ public function models(): Collection { return $this->revisions()->with('revisionable')->get()->pluck('revisionable'); } + + /** + * Apply a scope to filter for checkpoints closest to the one provided based on an operator + * when less than, orders by checkpoint_date desc + * when greater than, orders checkpoint_date asc + * + * @param Builder $query + * @param Checkpoint|\Illuminate\Support\Carbon|string $moment + * @param string $operator + * @return Builder + */ + public function scopeClosestTo(Builder $query, $moment, $operator = '<=') + { + $column = $this->getCheckpointDateColumn(); + if ($moment instanceof static) { + $moment = $moment->getCheckpointDate(); + } + $query->where($column, $operator, $moment); + if ($operator === '<' || $operator === '<=') { + $query->latest($column); + } elseif ($operator === '>' || $operator === '>=') { + $query->oldest($column); + } + return $query; + } + + /** + * Apply a scope to filter for checkpoints older than the one provided ordered by checkpoint date desc + * + * @param Builder $query + * @param Checkpoint|\Illuminate\Support\Carbon|string $moment + * @param bool $strict + * @return Builder + */ + public function scopeOlderThan(Builder $query, $moment, $strict = true) + { + return $query->closestTo($moment, $strict ? '<' : '<='); + } + + /** + * Apply a scope to filter for checkpoints older or equal to the one provided ordered by checkpoint date desc + * + * @param Builder $query + * @param Checkpoint|\Illuminate\Support\Carbon|string $moment + * @return Builder + */ + public function scopeOlderThanEquals(Builder $query, $moment) + { + return $query->olderThan($moment, false); + } + + /** + * Apply a scope to filter for checkpoints newer than the one provided ordered by checkpoint date asc + * + * @param Builder $query + * @param Checkpoint|\Illuminate\Support\Carbon|string $moment + * @param bool $strict + * @return Builder + */ + public function scopeNewerThan(Builder $query, $moment, $strict = true) + { + return $query->closestTo($moment, $strict ? '>' : '>='); + } + + /** + * Apply a scope to filter for checkpoints newer or equal to the one provided ordered by checkpoint date asc + * + * @param Builder $query + * @param Checkpoint|\Illuminate\Support\Carbon|string $moment + * @return Builder + */ + public function scopeNewerThanEquals(Builder $query, $moment) + { + return $query->newerThan($moment, false); + } } diff --git a/src/Models/Revision.php b/src/Models/Revision.php index 22cbd8b..262d7ce 100644 --- a/src/Models/Revision.php +++ b/src/Models/Revision.php @@ -1,14 +1,14 @@ belongsTo(get_class($this), 'previous_revision_id', $this->primaryKey); + return $this->belongsTo(static::class, 'previous_revision_id', $this->primaryKey); } /** @@ -133,7 +133,7 @@ public function previous(): BelongsTo */ public function next(): HasOne { - return $this->hasOne(get_class($this), 'previous_revision_id', $this->primaryKey); + return $this->hasOne(static::class, 'previous_revision_id', $this->primaryKey); } /** @@ -188,7 +188,7 @@ public function isUpdatedAt(Checkpoint $moment): bool */ public function allRevisions(): HasMany { - return $this->hasMany(get_class($this), 'revisionable_type', 'revisionable_type') + return $this->hasMany(static::class, 'revisionable_type', 'revisionable_type') ->where('original_revisionable_id', $this->original_revisionable_id); } @@ -199,7 +199,7 @@ public function allRevisions(): HasMany */ public function otherRevisions(): HasMany { - return $this->allRevisions()->where('id', '!=', $this->id); + return $this->allRevisions()->where('id', '!=', $this->getKey()); } /** @@ -210,33 +210,40 @@ public function otherRevisions(): HasMany */ public function scopeLatestIds(Builder $q, $until = null, $since = null) { - $q->withoutGlobalScopes()->selectRaw("max({$this->getQualifiedKeyName()}) as closest") - ->groupBy(['original_revisionable_id', 'revisionable_type'])->orderBy(DB::raw('NULL')); - + $q->withoutGlobalScopes()->selectRaw("max({$this->getKeyName()}) as closest") + ->groupBy(['original_revisionable_id', 'revisionable_type'])->orderByDesc('previous_revision_id'); $checkpoint = config('checkpoint.checkpoint_model', Checkpoint::class); - $checkpointDateColumn = $checkpoint::CHECKPOINT_DATE; if ($until instanceof $checkpoint) { - // where in this checkpoint or one of the previous ones - $q->whereIn( - $this->getCheckpointIdColumn(), - $checkpoint::select('id')->where($checkpointDateColumn, '<=', $until->$checkpointDateColumn) - ); + // where in given checkpoint or one of the previous ones + $q->whereIn($this->getCheckpointIdColumn(), $checkpoint::olderThanEquals($until)->select('id')); } elseif ($until !== null) { - $q->where($this->getQualifiedCreatedAtColumn(), '<=', Carbon::parse($until)); + $q->where($this->getQualifiedCreatedAtColumn(), '<=', $until); } if ($since instanceof $checkpoint) { - // where in this checkpoint or one of the following ones - $q->whereIn( - $this->getCheckpointIdColumn(), - $checkpoint::select('id')->where($checkpointDateColumn, '>', $since->$checkpointDateColumn) - ); + // where in one of the newer checkpoints than given + $q->whereIn($this->getCheckpointIdColumn(), $checkpoint::newerThan($since)->select('id')); } elseif ($since !== null) { - $q->where($this->getQualifiedCreatedAtColumn(), '>', Carbon::parse($since)); + $q->where($this->getQualifiedCreatedAtColumn(), '>', $since); } return $q; } + + /** + * @param Builder $q + * @param Model|string|null $type + * @return Builder + */ + public function scopeWhereType(Builder $q, $type = null) + { + if (is_string($type)) { + $q->where('revisionable_type', $type); + } elseif ($type instanceof Model) { + $q->where('revisionable_type', get_class($type)); + } + return $q; + } } diff --git a/src/Scopes/RevisionScope.php b/src/Scopes/RevisionScope.php index 9ddba23..d3bcc4b 100644 --- a/src/Scopes/RevisionScope.php +++ b/src/Scopes/RevisionScope.php @@ -5,7 +5,6 @@ use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Scope; -use Plank\Checkpoint\Models\Revision; class RevisionScope implements Scope { @@ -30,7 +29,7 @@ class RevisionScope implements Scope */ public function apply(Builder $builder, Model $model) { - $builder->at(); + $builder->at(); // show the latest available revisions by default } /** @@ -48,7 +47,11 @@ public function extend(Builder $builder) /** - * Allows to scope the query using dates or + * Add the temporal extension to the builder + * + * worst case execution plan : #reads = all rows in your table * all checkpoints * all revisions + * avg case execution plan: all rows in your table * 1 revision * 1 revision * 0..max amount of checkpoints + * * @param \Illuminate\Database\Eloquent\Builder $builder * @return void */ @@ -57,33 +60,13 @@ protected function addTemporal(Builder $builder) // Constrain the scope to a specific window of time using valid checkpoints, carbon objects or datetime strings $builder->macro('temporal', function (Builder $builder, $until = null, $since = null) { $model = $builder->getModel(); - $revision = new Revision(); $builder->withoutGlobalScope($this); - // worst case execution plan : #reads = all rows in your table * all checkpoints * all revisions - // avg case execution plan: all rows in your table * 1 revision * 1 revision * 0..max amount of checkpoints - // METHOD 1: Join current table on revisions, join the result on closest subquery // METHOD 2 : Join current table on revisions, filter out by original and type index, use a where in for the closest ids subquery -/* $builder->join("{$revision->getTable()} as _r", $model->getQualifiedKeyName(), '=', 'revisionable_id') - ->whereIn("_r.{$revision->getKeyName()}", - $revision::latestIds($until, $since) - ->whereColumn('original_revisionable_id', '_r.original_revisionable_id') - ->whereType($model) - )->where('revisionable_type', '=', get_class($model)); - // when using a join, you need to specific the select manually to avoid grabbing all the columns - // but when using addSelect and giving it a subquery, laravel automatically selects the main table - // causing duplicate columns - $select = $model->getTable().'.*'; - $columns = $builder->getQuery()->columns; - if ($columns === null || !in_array($select, $columns, true)) { - $builder->addSelect($select); - } - $builder->addSelect(['_r.metadata']);*/ - - // METHOD 3 : Uses a where exists wrapper on a where in subquery for closest timestamps + // METHOD 3 : Uses a where exists wrapper on a where in subquery for closest ids $builder->whereHas('revision', function (Builder $query) use ($model, $until, $since) { $query->whereIn($query->getModel()->getQualifiedKeyName(), $query->newModelInstance()->setTable('_r')->from($query->getModel()->getTable(), '_r') @@ -97,6 +80,7 @@ protected function addTemporal(Builder $builder) } /** + * Add the at extension to the builder. * * @param \Illuminate\Database\Eloquent\Builder $builder * @return void @@ -109,6 +93,7 @@ protected function addAt(Builder $builder) } /** + * Add the since extension to the builder. * * @param \Illuminate\Database\Eloquent\Builder $builder * @return void @@ -122,6 +107,7 @@ protected function addSince(Builder $builder) /** * Shortcut to clearing scope from query + * * @param \Illuminate\Database\Eloquent\Builder $builder * @return void */ From 7f5c61f29e2a7b7439292e3c7cb36c35d76beb21 Mon Sep 17 00:00:00 2001 From: Andrew H Date: Wed, 18 Nov 2020 16:13:29 -0500 Subject: [PATCH 03/19] clean up starting initial revisions and creating new revisions --- src/Commands/StartRevisioning.php | 2 +- src/Concerns/HasRevisions.php | 170 +++++++++++-------------- src/Observers/RevisionableObserver.php | 8 +- 3 files changed, 81 insertions(+), 99 deletions(-) diff --git a/src/Commands/StartRevisioning.php b/src/Commands/StartRevisioning.php index 754b45f..807d10e 100644 --- a/src/Commands/StartRevisioning.php +++ b/src/Commands/StartRevisioning.php @@ -67,7 +67,7 @@ public function handle() foreach ($models as $class) { $records = $class::withoutGlobalScopes()->chunk(100, function ($results) use ($checkpoint, &$timeDelta) { foreach ($results as $item) { - $item->updateOrCreateRevision(); + $item->startRevision(); $revision = $item->revision; // TODO: shouldn't be required for global query anymore. $revision->created_at = $item->freshRevisionCreatedAt(); diff --git a/src/Concerns/HasRevisions.php b/src/Concerns/HasRevisions.php index a03cbc0..6ced810 100755 --- a/src/Concerns/HasRevisions.php +++ b/src/Concerns/HasRevisions.php @@ -7,6 +7,7 @@ use ReflectionClass; use Plank\Checkpoint\Models\Revision; use Plank\Checkpoint\Models\Checkpoint; +use Illuminate\Database\Eloquent\Builder; use Plank\Checkpoint\Scopes\RevisionScope; use Plank\Checkpoint\Helpers\RelationHelper; use Plank\Checkpoint\Observers\RevisionableObserver; @@ -24,17 +25,6 @@ trait HasRevisions use HasCheckpointRelations; use StoresRevisionMeta; - /** - * Make sure that revisioning should be done before proceeding. - * Override and add any conditions your use cases may require. - * - * @return bool - */ - public function shouldRevision(): bool - { - return true; - } - /** * Boot has revisions trait for a model. * @@ -52,9 +42,9 @@ public static function bootHasRevisions(): void * * @return void */ - public function initializeHasRevisions() + public function initializeHasRevisions(): void { - // + $this->addObservableEvents('revisioning', 'revisioned'); } /** @@ -200,29 +190,35 @@ public function getExcludedRelations(): array return array_merge($default, $this->excludedRelations ?? []); } + /** + * Make sure that revisioning should be done before proceeding. + * Override and add any conditions your use cases may require. + * + * @return bool + */ + public function shouldRevision(): bool + { + return true; + } + /** * Update or Create the revision for this model. * * @param array $values * - * @return \Illuminate\Database\Eloquent\Model + * @return bool|\Illuminate\Database\Eloquent\Model */ - public function updateOrCreateRevision($values = []) + public function startRevision($values = []) { - if ($this->revision()->exists()) { - $search = ['id' => $this->revision->id]; - } else { - $search = [ - 'revisionable_id' => $this->id, - 'revisionable_type' => self::class, - 'original_revisionable_id' => $this->id, - 'created_at' => $this->freshRevisionCreatedAt(), - ]; + if (!$this->shouldRevision() || $this->revision()->exists()) { + return false; } - // when values is empty, we want to write in the data as used for lookup - $values = empty($values) ? $search : $values; - return $this->revision()->updateOrCreate($search, $values); + return $this->revision()->create(array_merge([ + 'revisionable_id' => $this->id, + 'revisionable_type' => static::class, + 'original_revisionable_id' => $this->id, + ], $values)); } /** @@ -231,85 +227,73 @@ public function updateOrCreateRevision($values = []) * @return bool * @throws \Throwable */ - public function saveAsRevision() + public function performRevision() { - if ($this->shouldRevision()) { - try { - if ($this->fireModelEvent('revisioning') === false) { - return false; - } - - $this->getConnection()->transaction(function () { - - // Ensure that the original model has a revision - if ($this->revision()->doesntExist()) { - $this->updateOrCreateRevision(); - } + if (!$this->shouldRevision() || $this->fireModelEvent('revisioning') === false) { + return false; + } - // Replicate the current object - $copy = $this->withoutRelations()->replicate($this->getExcludedColumns()); - $copy->save(); - $copy->refresh(); - - // Reattach relations to this object - $excludedRelations = $this->getExcludedRelations(); - foreach (RelationHelper::getModelRelations($this) as $relation => $attributes) { - if (!in_array($relation, $excludedRelations, true)) { - if (RelationHelper::isChild($attributes['type'])) { - logger(self::class . " {$this->getKey()}: duplicating children via $relation ({$attributes['type']})"); - foreach ($this->$relation()->get() as $child) { - if (method_exists($child, 'bootHasRevisions')) { - // Revision the child model by attaching it to our new copy - $child->setRawAttributes(array_merge($child->getOriginal(), [ - $this->$relation()->getForeignKeyName() => $copy->getKey() - ])); - $child->setRevisionUnwatched(); - $child->save(); - } else { - $copy->$relation()->save($child->replicate()); - } - } - } elseif (RelationHelper::isPivoted($attributes['type'])) { - logger(self::class . " {$this->getKey()}: duplicating pivots via $relation ({$attributes['type']})"); - $copy->$relation()->syncWithoutDetaching($this->$relation()->get()); + $this->getConnection()->transaction(function () { + + // Ensure that the original model has a revision + $this->startRevision(); + + // Replicate the current object + $copy = $this->withoutRelations()->replicate($this->getExcludedColumns()); + $copy->save(); + $copy->refresh(); + + // Reattach relations to the copied object + $excludedRelations = $this->getExcludedRelations(); + foreach (RelationHelper::getModelRelations($this) as $relation => $attributes) { + if (!in_array($relation, $excludedRelations, true)) { + if (RelationHelper::isChild($attributes['type'])) { + logger(self::class . " {$this->getKey()}: duplicating children via $relation ({$attributes['type']})"); + foreach ($this->$relation()->get() as $child) { + if (method_exists($child, 'bootHasRevisions')) { + // Revision the child model by attaching it to our new copy + $child->setRawAttributes(array_merge($child->getOriginal(), [ + $this->$relation()->getForeignKeyName() => $copy->getKey() + ])); + $child->setRevisionUnwatched(); + $child->save(); } else { - logger(self::class . " {$this->getKey()}: skipping duplication of $relation ({$attributes['type']})"); + $copy->$relation()->save($child->replicate()); } } + } elseif (RelationHelper::isPivoted($attributes['type'])) { + logger(self::class . " {$this->getKey()}: duplicating pivots via $relation ({$attributes['type']})"); + $copy->$relation()->syncWithoutDetaching($this->$relation()->get()); + } else { + logger(self::class . " {$this->getKey()}: skipping duplication of $relation ({$attributes['type']})"); } + } + } - // Reset the current model instance to original data - $this->setRawAttributes($this->getOriginal()); + // Reset the current model instance to original data + $this->setRawAttributes($this->getOriginal()); - // Handle unique columns by storing them as meta on the revision itself - $this->moveMetaToRevision(); + // Handle unique columns by storing them as meta on the revision itself + $this->moveMetaToRevision(); - // Update the revision of the original item - $this->updateOrCreateRevision([ - 'latest' => false, - ]); + // Update the revision of the original item + $this->revision()->update([ 'latest' => false ]); - // Update the revision of the duplicate with the correct data. - $copy->updateOrCreateRevision([ - 'original_revisionable_id' => $this->revision->original_revisionable_id, - 'previous_revision_id' => $this->revision->id, - 'created_at' => $copy->freshRevisionCreatedAt(), - ]); + // Update the revision of the duplicate with the correct data. + $copy->revision()->update([ + 'original_revisionable_id' => $this->revision->original_revisionable_id, + 'previous_revision_id' => $this->revision->id, + ]); - // Point $this to the duplicate, unload its relations and refresh the object - $this->setRawAttributes($copy->getAttributes()); - $this->unsetRelations(); - $this->refresh(); - }); + // Point $this to the duplicate, unload its relations and refresh the object + $this->setRawAttributes($copy->getAttributes()); + $this->unsetRelations(); + $this->refresh(); + }); - $this->fireModelEvent('revisioned', false); + $this->fireModelEvent('revisioned', false); - return true; - } catch (Exception $e) { - throw $e; - } - } - return false; + return true; } /** diff --git a/src/Observers/RevisionableObserver.php b/src/Observers/RevisionableObserver.php index 2cd7efb..838bb85 100644 --- a/src/Observers/RevisionableObserver.php +++ b/src/Observers/RevisionableObserver.php @@ -48,9 +48,7 @@ public function creating($model) */ public function created($model) { - if ($model->shouldRevision()) { - $model->updateOrCreateRevision(); - } + $model->startRevision(); } /** @@ -63,7 +61,7 @@ public function updating($model) { // Check if any column is dirty and filter out the unwatched fields if(!empty(array_diff(array_keys($model->getDirty()), $model->getRevisionUnwatched()))) { - $model->saveAsRevision(); + $model->performRevision(); } } @@ -87,7 +85,7 @@ public function updated($model) public function deleting($model) { if (method_exists($model, 'bootSoftDeletes') && !$model->isForceDeleting()) { - $model->saveAsRevision(); + $model->performRevision(); } } From d8fe05b0d5ae034c7ed1289f334a42186751dba1 Mon Sep 17 00:00:00 2001 From: Andrew H Date: Wed, 18 Nov 2020 16:13:50 -0500 Subject: [PATCH 04/19] remove created_at handling, let the projects handle it --- src/Commands/StartRevisioning.php | 2 -- src/Concerns/HasRevisions.php | 10 ---------- 2 files changed, 12 deletions(-) diff --git a/src/Commands/StartRevisioning.php b/src/Commands/StartRevisioning.php index 807d10e..6b5977f 100644 --- a/src/Commands/StartRevisioning.php +++ b/src/Commands/StartRevisioning.php @@ -69,8 +69,6 @@ public function handle() foreach ($results as $item) { $item->startRevision(); $revision = $item->revision; - // TODO: shouldn't be required for global query anymore. - $revision->created_at = $item->freshRevisionCreatedAt(); $revision->checkpoint()->associate($checkpoint); $revision->save(); } diff --git a/src/Concerns/HasRevisions.php b/src/Concerns/HasRevisions.php index 6ced810..84f6e3b 100755 --- a/src/Concerns/HasRevisions.php +++ b/src/Concerns/HasRevisions.php @@ -133,16 +133,6 @@ public function isNewAt(Checkpoint $moment): bool return $this->revision->isNewAt($moment); } - /** - * Override to change what is stored in a new revision's created at timestamp - * - * @return string - */ - protected function freshRevisionCreatedAt(): string - { - return $this->freshTimestampString(); - } - /** * Return the columns to ignore when creating a copy of a model. * Gets passed to replicate() in saveAsRevision(). From 3834d93f645672b74d28e6e7bf12fc8287f4f86b Mon Sep 17 00:00:00 2001 From: Andrew H Date: Wed, 18 Nov 2020 16:17:11 -0500 Subject: [PATCH 05/19] cast revision metadata column directly to an array --- src/Concerns/StoresRevisionMeta.php | 2 +- src/Models/Revision.php | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Concerns/StoresRevisionMeta.php b/src/Concerns/StoresRevisionMeta.php index c10e9d7..d8fe4f3 100644 --- a/src/Concerns/StoresRevisionMeta.php +++ b/src/Concerns/StoresRevisionMeta.php @@ -32,7 +32,7 @@ public function initializeStoresRevisionMeta() public function getMetadataAttribute() { if ($this->revision !== null) { - return json_decode($this->revision->metadata, true); + return $this->revision->metadata; } return []; } diff --git a/src/Models/Revision.php b/src/Models/Revision.php index 262d7ce..97121c0 100644 --- a/src/Models/Revision.php +++ b/src/Models/Revision.php @@ -71,6 +71,15 @@ class Revision extends MorphPivot 'metadata' ]; + /** + * The attributes that should be cast to native types. + * + * @var array + */ + protected $casts = [ + 'metadata' => 'array' + ]; + /** * Get the name of the "checkpoint id" column. * From 20d5eb9d134414bae39b3d15337e3c74f11579e1 Mon Sep 17 00:00:00 2001 From: Andrew H Date: Wed, 18 Nov 2020 16:17:41 -0500 Subject: [PATCH 06/19] improve checkpoint relations --- src/Concerns/HasCheckpointRelations.php | 53 +++++++------ src/Concerns/HasRevisions.php | 98 +++++++++++++++++++++++-- 2 files changed, 117 insertions(+), 34 deletions(-) diff --git a/src/Concerns/HasCheckpointRelations.php b/src/Concerns/HasCheckpointRelations.php index d1f35d6..653ff84 100755 --- a/src/Concerns/HasCheckpointRelations.php +++ b/src/Concerns/HasCheckpointRelations.php @@ -3,18 +3,13 @@ namespace Plank\Checkpoint\Concerns; use Illuminate\Database\Eloquent\Relations\HasOne; -use Plank\Checkpoint\Models\Revision; -use Plank\Checkpoint\Models\Checkpoint; -use Illuminate\Database\Eloquent\Relations\MorphTo; use Illuminate\Database\Eloquent\Relations\MorphOne; use Illuminate\Database\Eloquent\Relations\HasOneThrough; use Illuminate\Database\Eloquent\Relations\HasManyThrough; use Illuminate\Database\Eloquent\Relations\MorphOneOrMany; /** - * @package Plank\Versionable\Concerns - * - * @mixin \Illuminate\Database\Eloquent\Model + * @mixin HasRevisions */ trait HasCheckpointRelations { @@ -24,34 +19,34 @@ trait HasCheckpointRelations */ public function checkpoint(): HasOneThrough { - $revision = config('checkpoint.revision_model', Revision::class); - $checkpoint = config('checkpoint.checkpoint_model', Checkpoint::class); + $revision = config('checkpoint.revision_model'); + $checkpoint = config('checkpoint.checkpoint_model'); return $this->hasOneThrough( $checkpoint, $revision, 'revisionable_id', - (new $checkpoint)->getKeyName(), - (new $revision)->getKeyName(), + $checkpoint::getModel()->getKeyName(), + $this->getKeyName(), $revision::CHECKPOINT_ID - )->where('revisionable_type', self::class); + )->where('revisionable_type', static::class); } /** * Return the checkpoints associated with all the revisions of this model * @return HasManyThrough */ - public function checkpoints() + public function checkpoints(): HasManyThrough { - $revision = config('checkpoint.revision_model', Revision::class); - $checkpoint = config('checkpoint.checkpoint_model', Checkpoint::class); + $revision = config('checkpoint.revision_model'); + $checkpoint = config('checkpoint.checkpoint_model'); return $this->hasManyThrough( $checkpoint, $revision, 'original_revisionable_id', - (new $checkpoint)->getKeyName(), - 'first_revision_id', + $checkpoint::getModel()->getKeyName(), + 'initial_id', $revision::CHECKPOINT_ID - )->where('revisionable_type', self::class); + )->where('revisionable_type', static::class); } /** @@ -61,8 +56,7 @@ public function checkpoints() */ public function revision(): MorphOne { - $model = config('checkpoint.revision_model', Revision::class); - return $this->morphOne($model, 'revisionable'); + return $this->morphOne(config('checkpoint.revision_model'), 'revisionable'); } /** @@ -72,18 +66,23 @@ public function revision(): MorphOne */ public function revisions() { - return $this->morphMany(config('checkpoint.revision_model', Revision::class), - 'revisionable', 'revisionable_type', 'original_revisionable_id', 'first_revision_id'); + return $this->morphMany( + config('checkpoint.revision_model'), + 'revisionable', + 'revisionable_type', + 'original_revisionable_id', + 'initial_id' + ); } /** - * Get the model at its first revision + * Get the model at its initial revision * * @return HasOne */ public function initial(): HasOne { - return $this->hasOne(self::class, 'id', 'first_revision_id')->withoutRevisions(); + return $this->hasOne(static::class, 'id', 'initial_id')->withoutRevisions(); } /** @@ -93,17 +92,17 @@ public function initial(): HasOne */ public function older(): HasOne { - return $this->hasOne(self::class, 'id', 'previous_revision_id')->withoutRevisions(); + return $this->hasOne(static::class, 'id', 'previous_id')->withoutRevisions(); } /** * Get the model at the next revision * - * @return MorphTo + * @return HasOne */ - public function newer(): MorphTo + public function newer(): HasOne { - return $this->revision->next->revisionable(); + return $this->hasOne(static::class, 'id', 'next_id')->withoutRevisions(); } } diff --git a/src/Concerns/HasRevisions.php b/src/Concerns/HasRevisions.php index 84f6e3b..03e9f7a 100755 --- a/src/Concerns/HasRevisions.php +++ b/src/Concerns/HasRevisions.php @@ -70,26 +70,110 @@ public static function revisioned($callback): void } /** - * Get the id of the original revisioned item - * todo: change to subSelect + * Add the previous and next scopes loading their respective relations * + * @param Builder $builder * @return mixed */ - public function getFirstRevisionIdAttribute() + public function scopeWithRevisions(Builder $builder) { - return $this->revision->original_revisionable_id; + return $builder->withPrevious()->withNext(); } + + /** + * Get the id of the initial revisioned item + * + * @param Builder $builder + * @return mixed + */ + public function scopeWithInitial(Builder $builder) + { + return $builder->addSelect([ + 'initial_id' => Revision::select('original_revisionable_id') + ->whereColumn('revisionable_id', $this->getQualifiedKeyName()) + ->whereType(self::class) + ])->with('initial'); + } + /** * Get the id of the previous revisioned item - * todo: change to subSelect * + * @param Builder $builder * @return mixed */ - public function getPreviousRevisionIdAttribute() + public function scopeWithPrevious(Builder $builder) { - return $this->revision->previous_revision_id; + return $builder->addSelect([ + 'previous_id' => Revision::select('revisionable_id') + ->whereIn('id', Revision::select('previous_revision_id') + ->whereColumn('revisionable_id', $this->getQualifiedKeyName()) + ->whereType($this)) + ])->with('older'); } + /** + * Get the id of the next revisioned item + * + * @param Builder $builder + * @return mixed + */ + public function scopeWithNext(Builder $builder) + { + return $builder->addSelect([ + 'next_id' => Revision::from('revisions as r') + ->whereColumn('revisionable_id', $this->getQualifiedKeyName()) + ->whereType($this) + ->selectSub( + Revision::select('id') + ->whereColumn('previous_revision_id', 'r.id') + ->whereType($this), 'sub' + ) + ])->with('newer'); + } + + /** + * Get the id of the initial revisionable + * + * @param $value + * @return int + */ + public function getInitialIdAttribute($value) + { + if ($value !== null || array_key_exists('initial_id', $this->attributes)) { + return $value; + } + return $this->revision->original_revisionable_id ?? null; + } + + /** + * Get the id of the previous revisionable + * + * @param $value + * @return int + */ + public function getPreviousIdAttribute($value) + { + if ($value !== null || array_key_exists('previous_id', $this->attributes)) { + return $value; + } + return $this->revision->previous->revisionable_id ?? null; + } + + /** + * Get the id of the next revisionable + * + * @param $value + * @return int + */ + public function getNextIdAttribute($value) + { + if ($value !== null || array_key_exists('next_id', $this->attributes)) { + return $value; + } + return $this->revision->next->revisionable_id ?? null; + } + + /** * Is this model the first revision * From e4bb05bb987550f8ada56c87e3772f6d8fa39e4a Mon Sep 17 00:00:00 2001 From: Andrew H Date: Wed, 18 Nov 2020 16:18:42 -0500 Subject: [PATCH 07/19] bump test coverage --- tests/Feature/RevisionObservablesTest.php | 19 ++++++++- tests/Feature/RevisionScopesTest.php | 46 ++++++++++++++++++++ tests/Feature/RevisionableRelationsTest.php | 47 ++++++++++++++++++--- 3 files changed, 105 insertions(+), 7 deletions(-) diff --git a/tests/Feature/RevisionObservablesTest.php b/tests/Feature/RevisionObservablesTest.php index e1a7e87..334ff8d 100644 --- a/tests/Feature/RevisionObservablesTest.php +++ b/tests/Feature/RevisionObservablesTest.php @@ -20,6 +20,7 @@ public function created_post_has_revision(): void $revisions = Revision::latest()->get(); $this->assertCount(1, $revisions); $this->assertEquals($post->id, $revisions->first()->revisionable_id); + $this->assertEquals($post->id, $post->revision->revisionable_id); } /** @@ -55,7 +56,23 @@ public function deleted_posts_are_revisioned(): void /** * @test */ - public function forced_deleted_posts_remove_item_and_revision(): void + public function restored_posts_are_revisioned(): void + { + $post = factory(Post::class)->create(); + $this->assertCount(1, Post::all()); + + $post->delete(); + $post->restore(); + + $this->assertCount(1, Post::all()); + $this->assertEquals(2, Post::withoutRevisions()->count()); + $this->assertEquals(3, Post::withTrashed()->withoutRevisions()->count()); + } + + /** + * @test + */ + public function force_deleted_posts_remove_item_and_revision(): void { $post = factory(Post::class)->create(); $this->assertCount(1, Post::all()); diff --git a/tests/Feature/RevisionScopesTest.php b/tests/Feature/RevisionScopesTest.php index 971d057..9f23d54 100644 --- a/tests/Feature/RevisionScopesTest.php +++ b/tests/Feature/RevisionScopesTest.php @@ -4,12 +4,27 @@ use Plank\Checkpoint\Models\Checkpoint; use Plank\Checkpoint\Models\Revision; +use Plank\Checkpoint\Scopes\RevisionScope; use Plank\Checkpoint\Tests\Support\Post; use Plank\Checkpoint\Tests\TestCase; class RevisionScopesTest extends TestCase { + /** @test */ + public function revision_global_scope_is_applied() + { + $post = new Post(); + $this->assertContains(RevisionScope::class, array_keys($post->getGlobalScopes())); + } + + /** @test */ + public function revision_global_scope_can_be_disabled() + { + $this->assertNotContains(RevisionScope::class, array_keys(Post::removedScopes())); + $this->assertContains(RevisionScope::class, array_keys(Post::withoutRevisions()->removedScopes())); + } + /** * @test */ @@ -37,6 +52,22 @@ public function lookup_visible_posts_since_a_date(): void $this->assertEquals(0, Post::since($after)->count()); } + /** + * @test + */ + public function lookup_visible_posts_between_two_dates(): void + { + $before = now()->startOfMinute(); + $post = factory(Post::class)->create(); + $after = now()->endOfMinute(); + + $this->assertEquals(1, Post::since($before)->count()); + $this->assertEquals(1, Post::at($after)->count()); + $this->assertEquals(1, Post::temporal($after, $before)->count()); + $this->assertEquals(0, Post::since($after)->count()); + $this->assertEquals(0, Post::at($before)->count()); + } + /** * @test */ @@ -73,4 +104,19 @@ public function lookup_visible_posts_since_a_checkpoint(): void $this->assertEquals(1, Post::since($before)->count()); $this->assertEquals(0, Post::since($after)->count()); } + + /** + * @test + */ + public function lookup_visible_posts_between_two_checkpoints(): void + { + $before = factory(Checkpoint::class)->create(['checkpoint_date' => now()->startOfDay()]); + $post = factory(Post::class)->create(); + $after = factory(Checkpoint::class)->create(['checkpoint_date' => now()->endOfMinute()]); + + $post->revision->checkpoint_id = $after->id; + $post->push(); + + $this->assertEquals(1, Post::temporal($after, $before)->count()); + } } diff --git a/tests/Feature/RevisionableRelationsTest.php b/tests/Feature/RevisionableRelationsTest.php index b1fd18c..dd48c0f 100644 --- a/tests/Feature/RevisionableRelationsTest.php +++ b/tests/Feature/RevisionableRelationsTest.php @@ -9,7 +9,6 @@ class RevisionableRelationsTest extends TestCase { - /** * @test */ @@ -23,6 +22,16 @@ public function new_revisionable_has_revision(): void $this->assertEquals($revision->original_revisionable_id, $post->id); } + /** + * @test + */ + public function all_revisionables_have_revision(): void + { + $posts = factory(Post::class, 5)->create(); + + $this->assertCount($posts->count(), $posts->filter(function ($post) { return $post->revision()->exists(); })); + } + /** * @test */ @@ -30,7 +39,7 @@ public function retrieve_initial_revisionable_from_any_revision(): void { $post = factory(Post::class)->create(); $revision1 = $post->revision; - $post->saveAsRevision(); + $post->performRevision(); $revision2 = $post->revision; $this->assertNotEquals($revision1->id, $revision2->id); @@ -45,13 +54,39 @@ public function retrieve_previous_revisionable_from_any_revision(): void { $post = factory(Post::class)->create(); $revision1 = $post->revision; - $post->saveAsRevision(); + $original_id = $post->id; + + $post->performRevision(); $revision2 = $post->revision; + $this->assertTrue($post->older()->exists()); + $this->assertEquals($original_id, Post::withPrevious()->find($post->id)->older->id); + $this->assertEquals($post->id, $revision2->revisionable_id); $this->assertEquals($post->older->id, $revision1->revisionable_id); } + /** + * @test + */ + public function retrieve_next_revisionable_if_available(): void + { + $post = factory(Post::class)->create(); + $revision1 = $post->revision; + $original = clone $post; + + $this->assertFalse($original->newer()->exists()); + $this->assertFalse($post->newer()->exists()); + + $post->performRevision(); + $revision2 = $post->revision; + + $this->assertEquals($post->id, Post::withoutRevisions()->withNext()->find($original->id)->newer->id); + + $this->assertEquals($original->id, $revision1->revisionable_id); + $this->assertEquals($post->id, $revision2->revisionable_id); + } + /** * @test */ @@ -59,7 +94,7 @@ public function retrieve_all_revisions_from_revisionable(): void { $post = factory(Post::class)->create(); $revision1 = $post->revision; - $post->saveAsRevision(); + $post->performRevision(); $revision2 = $post->revision; $this->assertEquals(2, $post->revisions()->count()); @@ -75,7 +110,7 @@ public function retrieve_checkpoint_from_revisionable(): void $checkpoint = factory(Checkpoint::class)->create(); $post = factory(Post::class)->create(); $revision1 = $post->revision; - $post->saveAsRevision(); + $post->performRevision(); $revision2 = $post->revision; $original_post = $post->initial; @@ -98,7 +133,7 @@ public function retrieve_all_checkpoints_where_revisionable_was_modified(): void $revision1->save(); $c2 = factory(Checkpoint::class)->create(); - $post->saveAsRevision(); + $post->performRevision(); $original_post = $post->initial; $revision2 = $post->revision; $revision2->checkpoint_id = $c2->id; From 695029cb9aebbfcab0b1b61a1cfbf8efe2e8aa8b Mon Sep 17 00:00:00 2001 From: Andrew H Date: Wed, 18 Nov 2020 16:19:14 -0500 Subject: [PATCH 08/19] preserve revision history on delete --- src/Observers/RevisionableObserver.php | 6 ++++++ tests/Feature/RevisionObservablesTest.php | 24 +++++++++++++++++------ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/Observers/RevisionableObserver.php b/src/Observers/RevisionableObserver.php index 838bb85..e6f02ea 100644 --- a/src/Observers/RevisionableObserver.php +++ b/src/Observers/RevisionableObserver.php @@ -98,6 +98,12 @@ public function deleting($model) public function deleted($model) { if (!method_exists($model, 'bootSoftDeletes') || $model->isForceDeleting()) { + $revision = $model->revision; + // if newer revision exists, point its previous_revision_id to the previous revision of this item + if ($revision !== null && $revision->next()->exists()) { + $revision->next->previous_revision_id = $revision->previous_revision_id; + $revision->next->save(); + } $model->revision()->delete(); } } diff --git a/tests/Feature/RevisionObservablesTest.php b/tests/Feature/RevisionObservablesTest.php index 334ff8d..4581689 100644 --- a/tests/Feature/RevisionObservablesTest.php +++ b/tests/Feature/RevisionObservablesTest.php @@ -88,16 +88,28 @@ public function force_deleted_posts_remove_item_and_revision(): void /** * @test */ - public function restored_posts_are_revisioned(): void + public function force_deleted_posts_preserves_revision_history(): void { $post = factory(Post::class)->create(); - $this->assertCount(1, Post::all()); - $post->delete(); - $post->restore(); + $original = clone $post; + $post->title .= ' v2'; + $post->save(); + $to_delete = clone $post; + + $post->title .= ' v3'; + $post->save(); $this->assertCount(1, Post::all()); - $this->assertEquals(2, Post::withoutRevisions()->count()); - $this->assertEquals(3, Post::withTrashed()->withoutRevisions()->count()); + $this->assertCount(3, Revision::all()); + + $to_delete->forceDelete(); + $r1 = $original->revision()->first(); + $r3 = $post->revision()->first(); + + $this->assertCount(2, Post::withoutRevisions()->get()); + $this->assertCount(2, Revision::all()); + $this->assertEquals($r1->id, $r3->previous_revision_id); } + } From 6dde594fad572082f4adf0fce3d79e7705ccae53 Mon Sep 17 00:00:00 2001 From: Andrew H Date: Wed, 18 Nov 2020 16:30:29 -0500 Subject: [PATCH 09/19] add more doc blocks, type hinting, comments and update readme --- README.md | 19 ++++++------ src/Concerns/HasRevisions.php | 14 ++++----- src/Concerns/StoresRevisionMeta.php | 5 ---- src/Models/Checkpoint.php | 43 ++++++++++++++++++++------- src/Models/Revision.php | 46 +++++++++++++++++++++++++---- 5 files changed, 89 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index 22e7951..c31768b 100755 --- a/README.md +++ b/README.md @@ -113,8 +113,7 @@ This query scope will limit the query to return the *Model* whose ```Revision``` the ```Revision``` was created at or before the given moment. The moment can either be an instance of a ```Checkpoint``` -using its ```checkpoint_date``` field, or a string representation of a date compatible with ```Carbon::parse```, or a -```Carbon``` instance. +using its ```checkpoint_date``` field, a string representation of a date or a ```Carbon``` instance. #### since($moment) ```php @@ -126,8 +125,8 @@ since($moment = null) This query scope will limit the query to return the *Model* whose ```Revision``` has the max primary key, where the ```Revision``` was created after the given moment. -The moment can either be an instance of a ```Checkpoint``` using its ```checkpoint_date``` field, or a string -representation of a date compatible with ```Carbon::parse```, or a ```Carbon``` instance. +The moment can either be an instance of a ```Checkpoint``` using its ```checkpoint_date``` field, a string +representation of a date or a ```Carbon``` instance. #### temporal($upper, $lower) ```php @@ -135,15 +134,15 @@ representation of a date compatible with ```Carbon::parse```, or a ```Carbon``` * @param $upper Checkpoint|Carbon|string * @param $upper Checkpoint|Carbon|string */ -temporal($upper = null, $lower = null) +temporal($until = null, $since = null) ``` This query scope will limit the query to return the *Model* whose ```Revision``` has the max primary key created at -or before ```$upper```. This method can also limit the query to the *Model* whose revision has the max primary key -created after ```$lower```. +or before ```$until```. This method can also limit the query to the *Model* whose revision has the max primary key +created after ```$since```. -Each argument operates independently of each other and ```$upper``` and ```$lower``` can -either be an instance of a ```Checkpoint``` using its ```checkpoint_date``` field, or a string representation of a date -compatible with ```Carbon::parse```, or a ```Carbon``` instance. +Each argument operates independently of each other and ```$until``` and ```$since``` can +either be an instance of a ```Checkpoint``` using its ```checkpoint_date``` field, a string representation of a +date or a ```Carbon``` instance. #### withoutRevisions() ```php diff --git a/src/Concerns/HasRevisions.php b/src/Concerns/HasRevisions.php index 03e9f7a..6493ff6 100755 --- a/src/Concerns/HasRevisions.php +++ b/src/Concerns/HasRevisions.php @@ -13,12 +13,12 @@ use Plank\Checkpoint\Observers\RevisionableObserver; /** - * @method static static|\Illuminate\Database\Eloquent\Builder|\Illuminate\Database\Query\Builder at() - * @method static static|\Illuminate\Database\Eloquent\Builder|\Illuminate\Database\Query\Builder since() - * @method static static|\Illuminate\Database\Eloquent\Builder|\Illuminate\Database\Query\Builder temporal() + * @method static static|\Illuminate\Database\Eloquent\Builder|\Illuminate\Database\Query\Builder at($until) + * @method static static|\Illuminate\Database\Eloquent\Builder|\Illuminate\Database\Query\Builder since($since) + * @method static static|\Illuminate\Database\Eloquent\Builder|\Illuminate\Database\Query\Builder temporal($until, $since) * @method static static|\Illuminate\Database\Eloquent\Builder|\Illuminate\Database\Query\Builder withoutRevisions() * - * @mixin \Illuminate\Database\Eloquent\Model + * @mixin \Eloquent */ trait HasRevisions { @@ -229,8 +229,7 @@ public function getExcludedColumns(): array } /** - * Set a protected unwatched array on your model - * to skip revisioning on specific columns. + * Set a protected unwatched array on your model to skip revisioning on specific columns * * @return array */ @@ -251,8 +250,7 @@ public function setRevisionUnwatched($unwatched = null): void } /** - * Set the relationships to be ignored during duplication. - * Supply an array of relation method names. + * Get an array of the relationships to be ignored during duplication * * @return array */ diff --git a/src/Concerns/StoresRevisionMeta.php b/src/Concerns/StoresRevisionMeta.php index d8fe4f3..768ef85 100644 --- a/src/Concerns/StoresRevisionMeta.php +++ b/src/Concerns/StoresRevisionMeta.php @@ -2,12 +2,7 @@ namespace Plank\Checkpoint\Concerns; -use Plank\Checkpoint\Observers\RevisionableObserver; -use Plank\Checkpoint\Scopes\RevisionScope; - /** - * Trait StoresMeta - * * Allows saving specific columns onto the revision metadata field and ensures the * model can retrieve those columns from meta if they don't exist in the main table. * Mainly used for columns that cannot contain duplicate entries (ex: ordering) diff --git a/src/Models/Checkpoint.php b/src/Models/Checkpoint.php index d551a63..aba5a2a 100644 --- a/src/Models/Checkpoint.php +++ b/src/Models/Checkpoint.php @@ -1,4 +1,5 @@ morphedByMany($type, 'revisionable', 'revisions', 'checkpoint_id') @@ -146,7 +169,7 @@ public function modelsOf($type): MorphToMany /** * Retrieve all models directly associated with this checkpoint - * More expensive than just calling + * More expensive than just calling modelsOf() with specific class / type * * @return Collection */ diff --git a/src/Models/Revision.php b/src/Models/Revision.php index 97121c0..a1a7a05 100644 --- a/src/Models/Revision.php +++ b/src/Models/Revision.php @@ -10,6 +10,40 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Database\Eloquent\Relations\MorphPivot; +/** + * @property int $id + * @property string $revisionable_type + * @property int $revisionable_id + * @property int $original_revisionable_id + * @property int|null $previous_revision_id + * @property int|null $checkpoint_id + * @property mixed|null $metadata + * @property \Illuminate\Support\Carbon|null $created_at + * @property \Illuminate\Support\Carbon|null $updated_at + * @property-read \Illuminate\Database\Eloquent\Model|\Eloquent $revisionable + * @property-read \Illuminate\Database\Eloquent\Model|\Eloquent $initialRevisionable + * @property-read \Illuminate\Database\Eloquent\Collection|Revision[] $allRevisions + * @property-read int|null $all_revisions_count + * @property-read \Illuminate\Database\Eloquent\Collection|Revision[] $otherRevisions + * @property-read int|null $other_revisions_count + * @property-read Revision|null $next + * @property-read Revision|null $previous + * @property-read Checkpoint|null $checkpoint + * @method static Builder|Revision newModelQuery() + * @method static Builder|Revision newQuery() + * @method static Builder|Revision query() + * @method static Builder|Revision whereId($value) + * @method static Builder|Revision whereRevisionableId($value) + * @method static Builder|Revision whereRevisionableType($value) + * @method static Builder|Revision whereOriginalRevisionableId($value) + * @method static Builder|Revision wherePreviousRevisionId($value) + * @method static Builder|Revision whereCheckpointId($value) + * @method static Builder|Revision whereMetadata($value) + * @method static Builder|Revision whereCreatedAt($value) + * @method static Builder|Revision whereUpdatedAt($value) + * @method static Builder|Revision latestIds($until = null, $since = null) + * @mixin \Eloquent + */ class Revision extends MorphPivot { /** @@ -34,7 +68,7 @@ class Revision extends MorphPivot protected $keyType = 'int'; /** - * Prevent Eloquent from overriding uuid with `lastInsertId`. + * Indicates if the IDs are auto-incrementing. * * @var bool */ @@ -202,9 +236,9 @@ public function allRevisions(): HasMany } /** - * Return all the revisions sibling that share the same item + * Return all the revisions sibling that share the same item except the current one * - * @return + * @return HasMany */ public function otherRevisions(): HasMany { @@ -212,9 +246,11 @@ public function otherRevisions(): HasMany } /** + * Retrieve the latest revision ids within the boundary window given valid checkpoints, carbon or datetime strings + * * @param Builder $q - * @param Checkpoint|Carbon|string|null $until - * @param Checkpoint|Carbon|string|null $since + * @param Checkpoint|\Illuminate\Support\Carbon|string|null $until valid checkpoint, carbon or datetime string + * @param Checkpoint|\Illuminate\Support\Carbon|string|null $since valid checkpoint, carbon or datetime string * @return Builder */ public function scopeLatestIds(Builder $q, $until = null, $since = null) From 0294a46d1b5938b02e3458ed9f8dbd4023e30ef8 Mon Sep 17 00:00:00 2001 From: Andrew H Date: Wed, 18 Nov 2020 17:55:58 -0500 Subject: [PATCH 10/19] set default checkpoint.run_migrations config option --- config/checkpoint.php | 6 ++++++ src/CheckpointServiceProvider.php | 3 +-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/config/checkpoint.php b/config/checkpoint.php index 07ff07e..ee8e408 100755 --- a/config/checkpoint.php +++ b/config/checkpoint.php @@ -5,6 +5,12 @@ */ return [ + /* + | Should Checkpoint run the default migrations + | + */ + 'run_migrations' => env('RUN_CHECKPOINT_MIGRATIONS', true), + /* | | The full namespace to your User model class. diff --git a/src/CheckpointServiceProvider.php b/src/CheckpointServiceProvider.php index d32f9f3..be20840 100755 --- a/src/CheckpointServiceProvider.php +++ b/src/CheckpointServiceProvider.php @@ -23,7 +23,6 @@ public function boot() foreach (File::glob(__DIR__ . '/../database/migrations/*') as $migration) { $basename = strstr($migration, 'create'); if (empty(File::glob(database_path('migrations/*' . $basename)))) { - config()->set('checkpoint.runs_migrations', true); $this->publishes([ $migration => database_path("migrations/". date('Y_m_d_His') . "_" . $basename) ], 'migrations'); @@ -31,7 +30,7 @@ public function boot() } // Load default migrations if the runs_migrations toggle is true in the config (or if any of the expected files is missing) - if (config('checkpoint.runs_migrations', false)) { + if (config('checkpoint.run_migrations')) { $this->loadMigrationsFrom(__DIR__ . '/../database/migrations'); } From 6544cf9b7c5cefe55395229873a987a8d7d63978 Mon Sep 17 00:00:00 2001 From: Andrew H Date: Mon, 23 Nov 2020 16:00:56 -0500 Subject: [PATCH 11/19] wip: cleaning up column and config references + phpdocs --- src/Concerns/HasCheckpointRelations.php | 4 ++-- src/Concerns/HasRevisions.php | 32 ++++++++++++++----------- src/Models/Revision.php | 7 +++--- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/Concerns/HasCheckpointRelations.php b/src/Concerns/HasCheckpointRelations.php index 653ff84..06a3db7 100755 --- a/src/Concerns/HasCheckpointRelations.php +++ b/src/Concerns/HasCheckpointRelations.php @@ -82,7 +82,7 @@ public function revisions() */ public function initial(): HasOne { - return $this->hasOne(static::class, 'id', 'initial_id')->withoutRevisions(); + return $this->hasOne(static::class, $this->getKeyName(), 'initial_id')->withoutRevisions(); } /** @@ -92,7 +92,7 @@ public function initial(): HasOne */ public function older(): HasOne { - return $this->hasOne(static::class, 'id', 'previous_id')->withoutRevisions(); + return $this->hasOne(static::class, $this->getKeyName(), 'previous_id')->withoutRevisions(); } /** diff --git a/src/Concerns/HasRevisions.php b/src/Concerns/HasRevisions.php index 6493ff6..f6c6b37 100755 --- a/src/Concerns/HasRevisions.php +++ b/src/Concerns/HasRevisions.php @@ -50,7 +50,7 @@ public function initializeHasRevisions(): void /** * Register a revisioning model event with the dispatcher. * - * @param Closure|string $callback + * @param Closure|string $callback * @return void */ public static function revisioning($callback): void @@ -61,7 +61,7 @@ public static function revisioning($callback): void /** * Register a revisioned model event with the dispatcher. * - * @param Closure|string $callback + * @param Closure|string $callback * @return void */ public static function revisioned($callback): void @@ -88,10 +88,11 @@ public function scopeWithRevisions(Builder $builder) */ public function scopeWithInitial(Builder $builder) { + $revision = config('checkpoint.models.revision'); return $builder->addSelect([ - 'initial_id' => Revision::select('original_revisionable_id') + 'initial_id' => $revision::select('original_revisionable_id') ->whereColumn('revisionable_id', $this->getQualifiedKeyName()) - ->whereType(self::class) + ->whereType($this) ])->with('initial'); } @@ -103,11 +104,13 @@ public function scopeWithInitial(Builder $builder) */ public function scopeWithPrevious(Builder $builder) { + $revision = config('checkpoint.models.revision'); return $builder->addSelect([ - 'previous_id' => Revision::select('revisionable_id') - ->whereIn('id', Revision::select('previous_revision_id') + 'previous_id' => $revision::select('revisionable_id') + ->whereIn('id', $revision::select('previous_revision_id') ->whereColumn('revisionable_id', $this->getQualifiedKeyName()) - ->whereType($this)) + ->whereType($this) + ) ])->with('older'); } @@ -119,15 +122,14 @@ public function scopeWithPrevious(Builder $builder) */ public function scopeWithNext(Builder $builder) { + $revision = config('checkpoint.models.revision'); return $builder->addSelect([ - 'next_id' => Revision::from('revisions as r') + 'next_id' => $revision::selectSub($revision::select('id') + ->whereColumn('previous_revision_id', 'r.id') + ->whereType($this), 'sub' + )->from('revisions as r') ->whereColumn('revisionable_id', $this->getQualifiedKeyName()) ->whereType($this) - ->selectSub( - Revision::select('id') - ->whereColumn('previous_revision_id', 'r.id') - ->whereType($this), 'sub' - ) ])->with('newer'); } @@ -142,6 +144,7 @@ public function getInitialIdAttribute($value) if ($value !== null || array_key_exists('initial_id', $this->attributes)) { return $value; } + // when value isn't set by extra subselect scope, fetch from relation return $this->revision->original_revisionable_id ?? null; } @@ -156,6 +159,7 @@ public function getPreviousIdAttribute($value) if ($value !== null || array_key_exists('previous_id', $this->attributes)) { return $value; } + // when value isn't set by extra subselect scope, fetch from relation return $this->revision->previous->revisionable_id ?? null; } @@ -170,6 +174,7 @@ public function getNextIdAttribute($value) if ($value !== null || array_key_exists('next_id', $this->attributes)) { return $value; } + // when value isn't set by extra subselect scope, fetch from relation return $this->revision->next->revisionable_id ?? null; } @@ -277,7 +282,6 @@ public function shouldRevision(): bool * Update or Create the revision for this model. * * @param array $values - * * @return bool|\Illuminate\Database\Eloquent\Model */ public function startRevision($values = []) diff --git a/src/Models/Revision.php b/src/Models/Revision.php index a1a7a05..688cca1 100644 --- a/src/Models/Revision.php +++ b/src/Models/Revision.php @@ -33,6 +33,7 @@ * @method static Builder|Revision newQuery() * @method static Builder|Revision query() * @method static Builder|Revision whereId($value) + * @method static Builder|Revision whereType($value) * @method static Builder|Revision whereRevisionableId($value) * @method static Builder|Revision whereRevisionableType($value) * @method static Builder|Revision whereOriginalRevisionableId($value) @@ -166,7 +167,7 @@ public function checkpoint(): BelongsTo */ public function previous(): BelongsTo { - return $this->belongsTo(static::class, 'previous_revision_id', $this->primaryKey); + return $this->belongsTo(static::class, 'previous_revision_id', $this->getKeyName()); } /** @@ -176,7 +177,7 @@ public function previous(): BelongsTo */ public function next(): HasOne { - return $this->hasOne(static::class, 'previous_revision_id', $this->primaryKey); + return $this->hasOne(static::class, 'previous_revision_id', $this->getKeyName()); } /** @@ -255,7 +256,7 @@ public function otherRevisions(): HasMany */ public function scopeLatestIds(Builder $q, $until = null, $since = null) { - $q->withoutGlobalScopes()->selectRaw("max({$this->getKeyName()}) as closest") + $q->withoutGlobalScopes()->selectRaw("max({$this->getKeyName()})") ->groupBy(['original_revisionable_id', 'revisionable_type'])->orderByDesc('previous_revision_id'); $checkpoint = config('checkpoint.checkpoint_model', Checkpoint::class); From a7d40563bee7fbfef7e541c1f8e92b35e91d6465 Mon Sep 17 00:00:00 2001 From: Andrew H Date: Mon, 23 Nov 2020 16:01:49 -0500 Subject: [PATCH 12/19] chore: clean up redundant static arrays in RelationHelper --- src/Helpers/RelationHelper.php | 31 ++----------------------------- 1 file changed, 2 insertions(+), 29 deletions(-) diff --git a/src/Helpers/RelationHelper.php b/src/Helpers/RelationHelper.php index e37caac..d55ded0 100644 --- a/src/Helpers/RelationHelper.php +++ b/src/Helpers/RelationHelper.php @@ -48,20 +48,6 @@ class RelationHelper 'morphToMany', ]; - /** - * All available Laravel's direct relations. - * - * @var array - */ - protected static $directRelations = [ - HasOne::class, - MorphOne::class, - HasMany::class, - MorphMany::class, - BelongsTo::class, - MorphTo::class, - ]; - /** * All available Laravel's pivoted relations. * @@ -81,19 +67,6 @@ class RelationHelper BelongsTo::class, MorphTo::class, ]; - - /** - * All available Laravel's direct child relations. - * - * @var array - */ - protected static $childRelations = [ - HasOne::class, - MorphOne::class, - HasMany::class, - MorphMany::class, - ]; - /** * All available Laravel's direct single child relations. * @@ -122,7 +95,7 @@ class RelationHelper */ public static function isDirect(string $relation): bool { - return in_array($relation, static::$directRelations); + return self::isChild($relation) || self::isParent($relation); } /** @@ -155,7 +128,7 @@ public static function isParent(string $relation): bool */ public static function isChild(string $relation): bool { - return in_array($relation, static::$childRelations); + return self::isChildSingle($relation) || self::isChildMultiple($relation); } /** From 05e138907f78b3925ec38233dd798e117635b4a1 Mon Sep 17 00:00:00 2001 From: Andrew H Date: Mon, 23 Nov 2020 16:02:41 -0500 Subject: [PATCH 13/19] test: added a Checkpoint model test --- tests/Unit/CheckpointModelTest.php | 107 +++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 tests/Unit/CheckpointModelTest.php diff --git a/tests/Unit/CheckpointModelTest.php b/tests/Unit/CheckpointModelTest.php new file mode 100644 index 0000000..2f05c8b --- /dev/null +++ b/tests/Unit/CheckpointModelTest.php @@ -0,0 +1,107 @@ +create(); + $from_query = Checkpoint::latest('checkpoint_date')->first(); + $current = Checkpoint::current(); + + $this->assertEquals($from_query->id, $current->id); + + $tomorrow = factory(Checkpoint::class)->create(['checkpoint_date' => strtotime('tomorrow')]); + $current = Checkpoint::current(); + + $this->assertNotEquals($tomorrow->id, $current->id); + $this->assertGreaterThan($current->checkpoint_date, $tomorrow->checkpoint_date); + } + + /** + * @test + */ + public function previous_checkpoint_older_than_current(): void + { + $checkpoints = factory(Checkpoint::class, random_int(5, 25))->create(); + $from_query = Checkpoint::latest('checkpoint_date')->limit(2)->get()->last(); + $previous = Checkpoint::current()->previous(); + + $this->assertEquals($from_query->id, $previous->id); + } + + /** + * @test + */ + public function next_checkpoint_newer_than_current(): void + { + $checkpoints = factory(Checkpoint::class, random_int(5, 25))->create(); + $current = Checkpoint::current(); + + $this->assertNull($current->next()); + + $tomorrow = factory(Checkpoint::class)->create(['checkpoint_date' => strtotime('tomorrow')]); + $next = $current->next(); + + $this->assertNotNull($next); + $this->assertEquals($tomorrow->id, $next->id); + $this->assertGreaterThan($current->checkpoint_date, $next->checkpoint_date); + } + + /** + * @test + */ + public function current_checkpoint_is_older_or_equals_to_now(): void + { + factory(Checkpoint::class, random_int(1,5))->create(); + + $this->assertEquals(Checkpoint::current()->id, Checkpoint::olderThanEquals(now())->first()->id); + } + + /** + * @test + */ + public function current_checkpoint_date_equals_to_itself(): void + { + factory(Checkpoint::class, random_int(1,5))->create(); + + $current = Checkpoint::current(); + + $this->assertEquals($current->id, Checkpoint::newerThanEquals($current)->first()->id); + $this->assertEquals($current->id, Checkpoint::olderThanEquals($current)->first()->id); + } + + /** + * @test + */ + public function checkpoint_has_revisions(): void + { + $checkpoint = factory(Checkpoint::class)->create(); + $post = factory(Post::class)->create(); + + $this->assertFalse($post->revision->checkpoint()->exists()); + $this->assertFalse($checkpoint->revisions()->exists()); + $this->assertFalse($checkpoint->modelsOf(Post::class)->exists()); + $this->assertEmpty($checkpoint->models()); + + $post->revision->checkpoint_id = $checkpoint->id; + $post->push(); + //$checkpoint->checkpoint_date = now(); + //$checkpoint->save(); + + $this->assertTrue($post->revision->checkpoint()->exists()); + $this->assertTrue($checkpoint->revisions()->exists()); + $this->assertTrue($checkpoint->modelsOf(Post::class)->exists()); + $this->assertCount(1, $checkpoint->models()); + + } + +} From 106824293cfaf3c0e01661bcd03aa0bd49fee6de Mon Sep 17 00:00:00 2001 From: Andrew H Date: Mon, 23 Nov 2020 16:05:55 -0500 Subject: [PATCH 14/19] fix: metadata cast to array + new withMetadata scope query removed collection->toJson call --- src/Concerns/StoresRevisionMeta.php | 53 ++++++++++++++--- tests/Feature/RevisionMetadataTest.php | 82 ++++++++++++++++++++++++++ 2 files changed, 127 insertions(+), 8 deletions(-) create mode 100644 tests/Feature/RevisionMetadataTest.php diff --git a/src/Concerns/StoresRevisionMeta.php b/src/Concerns/StoresRevisionMeta.php index 768ef85..dcee432 100644 --- a/src/Concerns/StoresRevisionMeta.php +++ b/src/Concerns/StoresRevisionMeta.php @@ -2,6 +2,8 @@ namespace Plank\Checkpoint\Concerns; +use Illuminate\Database\Eloquent\Builder; + /** * Allows saving specific columns onto the revision metadata field and ensures the * model can retrieve those columns from meta if they don't exist in the main table. @@ -21,15 +23,32 @@ public function initializeStoresRevisionMeta() // $this->appends[] = 'metadata'; } + /** + * Get the id of the previous revisioned item + * + * @param Builder $builder + * @return mixed + */ + public function scopeWithMetadata(Builder $builder) + { + $revision = config('checkpoint.models.revision'); + return $builder->addSelect([ + 'metadata' => $revision::select('revisionable_id') + ->where('revisionable_id', $this->getKey()) + ->whereType($this) + ]); + } + /** * @return array */ - public function getMetadataAttribute() + public function getMetadataAttribute($value) { - if ($this->revision !== null) { - return $this->revision->metadata; + if ($value !== null || array_key_exists('metadata', $this->attributes)) { + return $value; } - return []; + // when value isn't set by extra subselect scope, fetch from relation + return $this->revision->metadata ?? null; } /** @@ -50,14 +69,14 @@ public function moveMetaToRevision(&$revision = null) { $metaColumns = $this->getRevisionMeta(); //if (!empty($metaColumns)) { - $meta = collect(); + $meta = []; foreach ($metaColumns as $attribute) { $meta[$attribute] = $this->$attribute; // TODO: check column is nullable or put empty string $this->$attribute = null; } $revision = $revision ?? $this->revision; - $revision->metadata = $meta->toJson(); + $revision->metadata = $meta; $this->withoutEvents(function () use ($revision) { $revision->save(); $this->save(); // modified attributes, make sure this is saved without events @@ -66,8 +85,8 @@ public function moveMetaToRevision(&$revision = null) } /** - * Get a plain attribute (not a relationship). **Override of base eloquent model** - * This allows a user to access an attribute that is stored in the meta data instead of the model. + * Get a plain attribute (not a relationship). + * Overrides default eloquent method by adding support for pulling data from revision metadata. * * @param string $key * @return mixed @@ -80,6 +99,24 @@ public function getAttributeValue($key) } return $value; + } + + /** + * Convert the model's attributes to an array. + * + * @return array + */ + public function attributesToArray() + { + $attributes = parent::attributesToArray(); + + // go over columns stored in the revision metadata and add them to the returned array + if ($this->revision()->exists()) { + foreach ($this->getRevisionMeta() as $key) { + $attributes[$key] = $this->metadata[$key] ?? null; + } + } + return $attributes; } } diff --git a/tests/Feature/RevisionMetadataTest.php b/tests/Feature/RevisionMetadataTest.php new file mode 100644 index 0000000..59b29ec --- /dev/null +++ b/tests/Feature/RevisionMetadataTest.php @@ -0,0 +1,82 @@ +create(); + + $this->assertEmpty($post->revision->metadata); + $this->assertEmpty($post->metadata); + + $post->title .= ' v2'; + $post->save(); + + // stays empty even after a new revision is created + $this->assertEmpty($post->revision->metadata); + $this->assertEmpty($post->metadata); + } + + /** + * @test + */ + public function revisions_store_metadata(): void + { + $post = factory(Post::class)->create(); + $post->title .= ' v2'; + $post->save(); + + // the metadata on the revision matches the mutator on the revisionable + $this->assertEquals($post->older->metadata, $post->revision->previous->metadata); + + // verify entries in the metadata are available as attributes on revisionable + foreach ($post->getRevisionMeta() as $key) { + $this->assertEquals($post->$key, $post->older->$key); + $this->assertEquals($post->older->$key, $post->revision->previous->metadata[$key]); + $this->assertContains($post->revision->previous->metadata[$key], $post->older->toArray()); + } + } + + /** + * @test + */ + public function revisonable_without_revision_returns_empty_metadata() + { + $post = factory(Post::class)->create(); + + $this->assertTrue($post->revision()->exists()); + + $post->revision()->delete(); + + $this->assertFalse($post->revision()->exists()); + $this->assertNull($post->revision); + + // stays the same, doesn't crash + $this->assertEmpty($post->metadata); + } + + /** + * @test + */ + public function revisionable_with_metadata_scope() + { + $post = factory(Post::class)->create(); + + $this->assertArrayNotHasKey('metadata', $post->toArray()); + + $post_with_meta = Post::withMetadata()->find($post->id); + + $this->assertArrayHasKey('metadata', $post_with_meta->toArray()); + } +} From 09bfc6c4083f06fb4e26adeb82f7b598d5f1ea5e Mon Sep 17 00:00:00 2001 From: Andrew H Date: Tue, 24 Nov 2020 11:14:39 -0500 Subject: [PATCH 15/19] fix: test for dynamically removing the global scope test --- README.md | 17 ++++++++--------- composer.json | 13 ++++++++----- tests/Feature/RevisionScopesTest.php | 4 ++-- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index c31768b..d63e572 100755 --- a/README.md +++ b/README.md @@ -1,8 +1,7 @@ # Laravel Checkpoint [![Latest Version on Packagist](https://img.shields.io/packagist/v/plank/laravel-checkpoint.svg?style=flat-square)](https://packagist.org/packages/plank/versionable) -[![Build Status](https://img.shields.io/travis/plank/laravel-checkpoint/master.svg?style=flat-square)](https://travis-ci.org/plank/versionable) -[![Quality Score](https://img.shields.io/scrutinizer/g/plank/laravel-checkpoint.svg?style=flat-square)](https://scrutinizer-ci.com/g/plank/versionable) +[![GitHub Tests Action Status](https://img.shields.io/github/workflow/status/plank/laravel-checkpoint/tests?label=tests)](https://github.com/plank/laravel-checkpoint/actions?query=workflow%3Atests+branch%3Amaster) [![Total Downloads](https://img.shields.io/packagist/dt/plank/laravel-checkpoint.svg?style=flat-square)](https://packagist.org/packages/plank/versionable) ## Table of Contents @@ -27,10 +26,10 @@ - [Should Revision](#should-revision) - [Excluded Columns](#excluded-columns) - [Excluded Relations](#excluded-relations) - - [Testing](#testing) - - [Changelog](#changelog) + - [Testing](#testing) + - [Changelog](#changelog) - [Contributing](#contributing) - - [Security](#security) + - [Security](#security) - [Credits](#credits) - [License](#license) @@ -185,21 +184,21 @@ cases you can add the names of the relations to the``` protected $excludedRelati revisioning. Excluding all relations to the ```Checkpoint```s and other related ```Revision```s are handled by the package. -### Testing +## Testing ``` bash composer test ``` -### Changelog +## Changelog Please see [CHANGELOG](CHANGELOG.md) for more information what has changed recently. ## Contributing -Please see [CONTRIBUTING](CONTRIBUTING.md) for details. +Please see [CONTRIBUTING](.github/CONTRIBUTING.md) for details. -### Security +## Security If you discover any security related issues, please email massimo@plankdesign.com instead of using the issue tracker. diff --git a/composer.json b/composer.json index cc0dbfd..850cfa6 100755 --- a/composer.json +++ b/composer.json @@ -30,8 +30,9 @@ "illuminate/support": "5.8.*|^6.0|^7.0|^8.0" }, "require-dev": { - "orchestra/testbench": "^4.0", - "phpunit/phpunit": "^8.0" + "orchestra/testbench": "^4.8 || ^5.2", + "phpunit/phpunit": "^9.0", + "vimeo/psalm": "^3.11" }, "autoload": { "psr-4": { @@ -44,9 +45,9 @@ } }, "scripts": { - "test": "vendor/bin/phpunit", + "psalm": "vendor/bin/psalm", + "test": "vendor/bin/phpunit --colors=always", "test-coverage": "vendor/bin/phpunit --coverage-html coverage" - }, "config": { "sort-packages": true @@ -57,5 +58,7 @@ "Plank\\Checkpoint\\CheckpointServiceProvider" ] } - } + }, + "minimum-stability": "dev", + "prefer-stable": true } diff --git a/tests/Feature/RevisionScopesTest.php b/tests/Feature/RevisionScopesTest.php index 9f23d54..afdd9ea 100644 --- a/tests/Feature/RevisionScopesTest.php +++ b/tests/Feature/RevisionScopesTest.php @@ -21,8 +21,8 @@ public function revision_global_scope_is_applied() /** @test */ public function revision_global_scope_can_be_disabled() { - $this->assertNotContains(RevisionScope::class, array_keys(Post::removedScopes())); - $this->assertContains(RevisionScope::class, array_keys(Post::withoutRevisions()->removedScopes())); + $this->assertNotContains(RevisionScope::class, Post::removedScopes()); + $this->assertContains(RevisionScope::class, Post::withoutRevisions()->removedScopes()); } /** From c1e49e0cc1c1552619a3aa98905fa0be71205422 Mon Sep 17 00:00:00 2001 From: Andrew H Date: Tue, 24 Nov 2020 11:29:36 -0500 Subject: [PATCH 16/19] chore: update :package: skeleton --- .editorconfig | 3 +++ .gitattributes | 4 ++++ CONTRIBUTING.md => .github/CONTRIBUTING.md | 0 .gitignore | 15 +++++++++------ psalm.xml.dist | 16 ++++++++++++++++ 5 files changed, 32 insertions(+), 6 deletions(-) rename CONTRIBUTING.md => .github/CONTRIBUTING.md (100%) create mode 100644 psalm.xml.dist diff --git a/.editorconfig b/.editorconfig index cd8eb86..08fd287 100755 --- a/.editorconfig +++ b/.editorconfig @@ -13,3 +13,6 @@ trim_trailing_whitespace = true [*.md] trim_trailing_whitespace = false + +[*.{yml,yaml}] +indent_size = 2 \ No newline at end of file diff --git a/.gitattributes b/.gitattributes index bb6265e..c29b1b5 100755 --- a/.gitattributes +++ b/.gitattributes @@ -9,3 +9,7 @@ /.scrutinizer.yml export-ignore /tests export-ignore /.editorconfig export-ignore +/.php_cs.dist export-ignore +/.github export-ignore +/psalm.xml export-ignore +/psalm.xml.dist export-ignore \ No newline at end of file diff --git a/CONTRIBUTING.md b/.github/CONTRIBUTING.md similarity index 100% rename from CONTRIBUTING.md rename to .github/CONTRIBUTING.md diff --git a/.gitignore b/.gitignore index d3fc4b5..73ab74c 100755 --- a/.gitignore +++ b/.gitignore @@ -1,9 +1,12 @@ -build -composer.lock +.idea +.DS_Store docs -vendor +build coverage -.idea +vendor +composer.lock +.php_cs .php_cs.cache -.DS_Store -/.phpunit* +.phpunit.result.cache +phpunit.xml +psalm.xml \ No newline at end of file diff --git a/psalm.xml.dist b/psalm.xml.dist new file mode 100644 index 0000000..506c359 --- /dev/null +++ b/psalm.xml.dist @@ -0,0 +1,16 @@ + + + + + + + + + \ No newline at end of file From d0f158994df89e401116c93050b91fa641e19ac4 Mon Sep 17 00:00:00 2001 From: Andrew H Date: Tue, 24 Nov 2020 11:54:28 -0500 Subject: [PATCH 17/19] reorganize checkpoint config file --- config/checkpoint.php | 58 +++++++++++++------------ src/Commands/StartRevisioning.php | 2 +- src/Concerns/HasCheckpointRelations.php | 12 ++--- src/Concerns/HasRevisions.php | 6 ++- src/Models/Checkpoint.php | 5 +-- src/Models/Revision.php | 10 ++--- src/Scopes/RevisionScope.php | 4 +- 7 files changed, 52 insertions(+), 45 deletions(-) diff --git a/config/checkpoint.php b/config/checkpoint.php index ee8e408..85e6a83 100755 --- a/config/checkpoint.php +++ b/config/checkpoint.php @@ -1,40 +1,44 @@ env('RUN_CHECKPOINT_MIGRATIONS', true), + * When true, checkpoint will automatically hook its observer to the + * revisionable model events in order to create revisions when needed + */ + 'enabled' => env('CHECKPOINT_ENABLED', true), /* - | - | The full namespace to your User model class. - | - | If your application doesn't have a user class, the value should be "NULL". - | - */ - 'user_model' => '\App\User', + * When true, checkpoint will automatically apply the global revision scope + * onto revisionables for filtering out relevant content based on time + * + * ***warning*** + * disabling after using the package will result in query results containing + * duplicate items as they won't automatically be filtered by time + */ + 'apply_global_scope' => env('REVISIONS_GLOBAL_SCOPE', env('CHECKPOINT_ENABLED', true)), /* - | - | Concrete implementation for the "version model". - | To extend or replace this functionality, change the value below with your full "version model" FQCN. - | - */ - 'checkpoint_model' => \Plank\Checkpoint\Models\Checkpoint::class, + * Should checkpoint run its default migrations + */ + 'run_migrations' => env('RUN_CHECKPOINT_MIGRATIONS', true), - /* - | - | Concrete implementation for the "version model". - | To extend or replace this functionality, change the value below with your full "version model" FQCN. - | - */ - 'revision_model' => \Plank\Checkpoint\Models\Revision::class, + 'models' => [ + + /* + * When using the "HasRevisions" trait from this package, we need to know which model + * should be used to retrieve your revisions. To extend or replace this functionality, + * change the value below with your full "revision model" class name. + */ + 'revision' => Plank\Checkpoint\Models\Revision::class, + + /* + * When using the "HasRevisions" trait from this package, we need to know which model + * should be used to retrieve your checkpoints. To extend or replace this functionality, + * change the value below with your full "checkpoint model" class name. + */ + 'checkpoint' => Plank\Checkpoint\Models\Checkpoint::class, + ], ]; diff --git a/src/Commands/StartRevisioning.php b/src/Commands/StartRevisioning.php index 6b5977f..39ec273 100644 --- a/src/Commands/StartRevisioning.php +++ b/src/Commands/StartRevisioning.php @@ -45,7 +45,7 @@ public function handle() { $checkpoint = null; if ($this->option('with-checkpoint')) { - $checkpointClass = config('checkpoint.checkpoint_model'); + $checkpointClass = config('checkpoint.models.checkpoint'); $checkpoint = $checkpointClass::first() ?? new $checkpointClass(); $checkpoint->save(); $checkpoint->refresh(); diff --git a/src/Concerns/HasCheckpointRelations.php b/src/Concerns/HasCheckpointRelations.php index 06a3db7..3d02863 100755 --- a/src/Concerns/HasCheckpointRelations.php +++ b/src/Concerns/HasCheckpointRelations.php @@ -19,8 +19,8 @@ trait HasCheckpointRelations */ public function checkpoint(): HasOneThrough { - $revision = config('checkpoint.revision_model'); - $checkpoint = config('checkpoint.checkpoint_model'); + $revision = config('checkpoint.models.revision'); + $checkpoint = config('checkpoint.models.checkpoint'); return $this->hasOneThrough( $checkpoint, $revision, @@ -37,8 +37,8 @@ public function checkpoint(): HasOneThrough */ public function checkpoints(): HasManyThrough { - $revision = config('checkpoint.revision_model'); - $checkpoint = config('checkpoint.checkpoint_model'); + $revision = config('checkpoint.models.revision'); + $checkpoint = config('checkpoint.models.checkpoint'); return $this->hasManyThrough( $checkpoint, $revision, @@ -56,7 +56,7 @@ public function checkpoints(): HasManyThrough */ public function revision(): MorphOne { - return $this->morphOne(config('checkpoint.revision_model'), 'revisionable'); + return $this->morphOne(config('checkpoint.models.revision'), 'revisionable'); } /** @@ -67,7 +67,7 @@ public function revision(): MorphOne public function revisions() { return $this->morphMany( - config('checkpoint.revision_model'), + config('checkpoint.models.revision'), 'revisionable', 'revisionable_type', 'original_revisionable_id', diff --git a/src/Concerns/HasRevisions.php b/src/Concerns/HasRevisions.php index f6c6b37..4ea34c5 100755 --- a/src/Concerns/HasRevisions.php +++ b/src/Concerns/HasRevisions.php @@ -33,8 +33,10 @@ trait HasRevisions public static function bootHasRevisions(): void { static::addGlobalScope(new RevisionScope); - // hook onto all relevant events: On Create, Update, Delete, Restore : make new revisions... - static::observe(RevisionableObserver::class); + if (config('checkpoint.enabled')) { + // hook onto all relevant events: On Create, Update, Delete, Restore : make new revisions... + static::observe(RevisionableObserver::class); + } } /** diff --git a/src/Models/Checkpoint.php b/src/Models/Checkpoint.php index aba5a2a..ade650f 100644 --- a/src/Models/Checkpoint.php +++ b/src/Models/Checkpoint.php @@ -148,7 +148,7 @@ public function next() */ public function revisions(): HasMany { - $model = config('checkpoint.revision_model', Revision::class); + $model = config('checkpoint.models.revision'); return $this->hasMany($model, $model::CHECKPOINT_ID); } @@ -161,10 +161,9 @@ public function revisions(): HasMany */ public function modelsOf(string $type): MorphToMany { - $rev = config('checkpoint.revision_model', Revision::class); return $this->morphedByMany($type, 'revisionable', 'revisions', 'checkpoint_id') ->withPivot('metadata', 'previous_revision_id', 'original_revisionable_id')->withTimestamps() - ->using($rev); + ->using(config('checkpoint.models.revision')); } /** diff --git a/src/Models/Revision.php b/src/Models/Revision.php index 688cca1..c9d3b04 100644 --- a/src/Models/Revision.php +++ b/src/Models/Revision.php @@ -156,8 +156,7 @@ public function initialRevisionable(): MorphTo */ public function checkpoint(): BelongsTo { - $model = config('checkpoint.checkpoint_model', Checkpoint::class); - return $this->belongsTo($model, $this->getCheckpointIdColumn()); + return $this->belongsTo(config('checkpoint.models.checkpoint'), $this->getCheckpointIdColumn()); } /** @@ -259,18 +258,19 @@ public function scopeLatestIds(Builder $q, $until = null, $since = null) $q->withoutGlobalScopes()->selectRaw("max({$this->getKeyName()})") ->groupBy(['original_revisionable_id', 'revisionable_type'])->orderByDesc('previous_revision_id'); - $checkpoint = config('checkpoint.checkpoint_model', Checkpoint::class); + $checkpoint = config('checkpoint.models.checkpoint'); + $checkpoint_key = Checkpoint::getModel()->getKeyName(); if ($until instanceof $checkpoint) { // where in given checkpoint or one of the previous ones - $q->whereIn($this->getCheckpointIdColumn(), $checkpoint::olderThanEquals($until)->select('id')); + $q->whereIn($this->getCheckpointIdColumn(), $checkpoint::olderThanEquals($until)->select($checkpoint_key)); } elseif ($until !== null) { $q->where($this->getQualifiedCreatedAtColumn(), '<=', $until); } if ($since instanceof $checkpoint) { // where in one of the newer checkpoints than given - $q->whereIn($this->getCheckpointIdColumn(), $checkpoint::newerThan($since)->select('id')); + $q->whereIn($this->getCheckpointIdColumn(), $checkpoint::newerThan($since)->select($checkpoint_key)); } elseif ($since !== null) { $q->where($this->getQualifiedCreatedAtColumn(), '>', $since); } diff --git a/src/Scopes/RevisionScope.php b/src/Scopes/RevisionScope.php index d3bcc4b..3b9d262 100644 --- a/src/Scopes/RevisionScope.php +++ b/src/Scopes/RevisionScope.php @@ -29,7 +29,9 @@ class RevisionScope implements Scope */ public function apply(Builder $builder, Model $model) { - $builder->at(); // show the latest available revisions by default + if (config('checkpoint.apply_global_scope', true)) { + $builder->at(); // show the latest available revisions by default + } } /** From d1add014ec0c4287118031d0b584d407fc01c541 Mon Sep 17 00:00:00 2001 From: Andrew H Date: Tue, 24 Nov 2020 11:54:45 -0500 Subject: [PATCH 18/19] ci: added github actions --- .github/workflows/psalm.yml | 33 +++++++++++++++++++++++++++ .github/workflows/tests.yml | 45 +++++++++++++++++++++++++++++++++++++ composer.json | 2 +- phpunit.xml.dist | 24 +++++++++++--------- 4 files changed, 92 insertions(+), 12 deletions(-) create mode 100644 .github/workflows/psalm.yml create mode 100644 .github/workflows/tests.yml diff --git a/.github/workflows/psalm.yml b/.github/workflows/psalm.yml new file mode 100644 index 0000000..25de931 --- /dev/null +++ b/.github/workflows/psalm.yml @@ -0,0 +1,33 @@ +name: Static Analysis + +on: + push: + paths: + - '**.php' + - 'psalm.xml.dist' + +jobs: + psalm: + name: psalm + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + + - name: Setup PHP + uses: shivammathur/setup-php@v2 + with: + php-version: '7.4' + extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, sqlite, pdo_sqlite, bcmath, soap, intl, gd, exif, iconv, imagick + coverage: none + + - name: Cache composer dependencies + uses: actions/cache@v2 + with: + path: vendor + key: composer-${{ hashFiles('composer.lock') }} + + - name: Run composer install + run: composer install -n --prefer-dist + + - name: Run psalm + run: ./vendor/bin/psalm --output-format=github \ No newline at end of file diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml new file mode 100644 index 0000000..cacf379 --- /dev/null +++ b/.github/workflows/tests.yml @@ -0,0 +1,45 @@ +name: Tests + +on: [push, pull_request] + +jobs: + test: + runs-on: ${{ matrix.os }} + strategy: + fail-fast: true + matrix: + os: [ubuntu-latest, windows-latest] + php: [7.4] + laravel: [6.*, 7.*] + stability: [prefer-lowest, prefer-stable] + include: + - laravel: 7.* + testbench: 5.* + - laravel: 6.* + testbench: 4.* + + name: P${{ matrix.php }} - L${{ matrix.laravel }} - ${{ matrix.stability }} - ${{ matrix.os }} + + steps: + - name: Checkout code + uses: actions/checkout@v2 + + - name: Setup PHP + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php }} + extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, sqlite, pdo_sqlite, bcmath, soap, intl, gd, exif, iconv, imagick + coverage: none + + - name: Setup problem matchers + run: | + echo "::add-matcher::${{ runner.tool_cache }}/php.json" + echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json" + + - name: Install dependencies + run: | + composer require "laravel/framework:${{ matrix.laravel }}" "orchestra/testbench:${{ matrix.testbench }}" --no-interaction --no-update + composer update --${{ matrix.stability }} --prefer-dist --no-interaction + + - name: Execute tests + run: vendor/bin/phpunit \ No newline at end of file diff --git a/composer.json b/composer.json index 850cfa6..eaefb84 100755 --- a/composer.json +++ b/composer.json @@ -31,7 +31,7 @@ }, "require-dev": { "orchestra/testbench": "^4.8 || ^5.2", - "phpunit/phpunit": "^9.0", + "phpunit/phpunit": "^9.3", "vimeo/psalm": "^3.11" }, "autoload": { diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 3d3a63d..290cba1 100755 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -8,7 +8,8 @@ convertNoticesToExceptions="true" convertWarningsToExceptions="true" processIsolation="false" - stopOnFailure="false"> + stopOnFailure="false" +> ./tests/Unit @@ -17,16 +18,17 @@ ./tests/Feature - - - src/ - - + + + ./src + + + + + + + - - - - - + From 16818c22e043e2998867133cb72d450b3e852d44 Mon Sep 17 00:00:00 2001 From: Andrew H Date: Tue, 24 Nov 2020 12:13:51 -0500 Subject: [PATCH 19/19] ci: fix run test action --- .github/workflows/psalm.yml | 33 --------------------------------- .github/workflows/tests.yml | 10 ++++++++-- 2 files changed, 8 insertions(+), 35 deletions(-) delete mode 100644 .github/workflows/psalm.yml diff --git a/.github/workflows/psalm.yml b/.github/workflows/psalm.yml deleted file mode 100644 index 25de931..0000000 --- a/.github/workflows/psalm.yml +++ /dev/null @@ -1,33 +0,0 @@ -name: Static Analysis - -on: - push: - paths: - - '**.php' - - 'psalm.xml.dist' - -jobs: - psalm: - name: psalm - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - - - name: Setup PHP - uses: shivammathur/setup-php@v2 - with: - php-version: '7.4' - extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, sqlite, pdo_sqlite, bcmath, soap, intl, gd, exif, iconv, imagick - coverage: none - - - name: Cache composer dependencies - uses: actions/cache@v2 - with: - path: vendor - key: composer-${{ hashFiles('composer.lock') }} - - - name: Run composer install - run: composer install -n --prefer-dist - - - name: Run psalm - run: ./vendor/bin/psalm --output-format=github \ No newline at end of file diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index cacf379..c66984f 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -8,10 +8,10 @@ jobs: strategy: fail-fast: true matrix: - os: [ubuntu-latest, windows-latest] + os: [ubuntu-latest] php: [7.4] laravel: [6.*, 7.*] - stability: [prefer-lowest, prefer-stable] + stability: [prefer-stable] include: - laravel: 7.* testbench: 5.* @@ -36,6 +36,12 @@ jobs: echo "::add-matcher::${{ runner.tool_cache }}/php.json" echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json" + - name: Cache dependencies + uses: actions/cache@v2 + with: + path: ~/.composer/cache/files + key: dependencies-php-${{ matrix.php }}-laravel-${{ matrix.laravel }}-stability-${{ matrix.stability }}-composer-${{ hashFiles('composer.json') }} + - name: Install dependencies run: | composer require "laravel/framework:${{ matrix.laravel }}" "orchestra/testbench:${{ matrix.testbench }}" --no-interaction --no-update