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

PHPORM-206 Add model schema version feature #3021

Merged
merged 14 commits into from
Jul 9, 2024

Conversation

florianJacques
Copy link
Contributor

@florianJacques florianJacques commented Jul 1, 2024

Added a feature for managing mongoDB document versioning.
The default key is "schema_version"
If the version is not specified, t is automatically set to 1

Usage

use MongoDB\Laravel\Eloquent\HasDocumentVersion;
use MongoDB\Laravel\Eloquent\Model as Eloquent;

/** @property int $schema_version */
class SchemaVersionned extends Eloquent
{
    use HasSchemaVersion;

    public const int SCHEMA_VERSION = 1;

    protected $connection       = 'mongodb';
    protected $collection       = 'documentVersion';
    protected static $unguarded = true;
}

For overwrite version key:

/** @property int $__v */
class DocumentVersion extends Eloquent
{
    use HasSchemaVersion;

    protected $connection       = 'mongodb';
    protected $collection       = 'documentVersion';
    protected static $unguarded = true;

    public static function getSchemaVersionKey(): string
    {
       return '__v';
    }
}

For update document version:

/** @property int $schema_version */
class DocumentVersion extends Eloquent
{
    use HasSchemaVersion;

    public const int SCHEMA_VERSION = 2;

    protected $connection       = 'mongodb';
    protected $collection       = 'documentVersion';
    protected static $unguarded = true;
}
  • Add tests and ensure they pass
  • Add an entry to the CHANGELOG.md file
  • Update documentation for new features

@florianJacques florianJacques requested a review from a team as a code owner July 1, 2024 10:43
@florianJacques florianJacques requested a review from GromNaN July 1, 2024 10:43
Copy link
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

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

Thanks for proposing this feature.

The ability to store documents with different schemas in the same collection is undoubtedly one of the great strengths of the MongoDB model. We have a tutorial on this subject.

I think there's a piece missing: the code to automatically update documents when they're read from the DB. This would probably be done in a listener of the retrieved event.

In the trait:

        static::retrieved(function (self $model) {
            $model->upgradeDocumentVersion($model, $model->{static::getDocumentVersionKey()} ?? 0);
        });

In the code would look like this (example for the docs):

    public function upgradeDocumentVersion(Model $document, int $fromVersion): void
    {
        switch (true) {
            case $fromVersion < 2:
                // Field renamed
                $document->newField = $document->oldField;
            // No break to apply all changes
            case $fromVersion < 3:
                // Field type updated
                $document->price = (float) $document->price;
            // ...
        }
    }

tests/ModelTest.php Outdated Show resolved Hide resolved
src/Eloquent/HasDocumentVersion.php Outdated Show resolved Hide resolved
tests/Models/DocumentVersion.php Outdated Show resolved Hide resolved
src/Eloquent/HasDocumentVersion.php Outdated Show resolved Hide resolved
src/Eloquent/HasDocumentVersion.php Outdated Show resolved Hide resolved
src/Eloquent/HasDocumentVersion.php Outdated Show resolved Hide resolved
src/Eloquent/HasDocumentVersion.php Outdated Show resolved Hide resolved
src/Eloquent/HasDocumentVersion.php Outdated Show resolved Hide resolved
src/Eloquent/HasDocumentVersion.php Outdated Show resolved Hide resolved
@GromNaN GromNaN added this to the 4.6 milestone Jul 2, 2024
src/Eloquent/HasSchemaVersion.php Outdated Show resolved Hide resolved
* migrate schema and set current model version
* @return void
*/
public function upgradeSchemaVersion(): void
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need this public method? All this code can be safely in the "retrieved" closure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's a good idea to let the user choose which method to use, between migrating + upgrading the version or migrating and upgrading the version later.

tests/SchemaVersionTest.php Outdated Show resolved Hide resolved
@GromNaN GromNaN added the feature label Jul 5, 2024
@GromNaN GromNaN force-pushed the feat-documentVersion branch from c56af3b to 09c95b4 Compare July 9, 2024 12:19
Copy link
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

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

@florianJacques I updated the implementation so that

  • the SCHEMA_VERSION constant is required
  • the tests ensure the documents are updated when retrieved
  • adding documentation in the trait description

src/Eloquent/HasSchemaVersion.php Show resolved Hide resolved
tests/SchemaVersionTest.php Show resolved Hide resolved
@GromNaN GromNaN force-pushed the feat-documentVersion branch from 09c95b4 to a8ca8aa Compare July 9, 2024 12:41
@GromNaN GromNaN requested a review from alcaeus July 9, 2024 12:41

trait HasSchemaVersion
{
public $currentSchemaVersion = 1;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this is the best solution, do you have any ideas?

});

static::retrieved(function ($model) {
$model->upgradeSchemaVersion();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we need to find a way to give control over the call to this function

* migrate schema and set current model version
* @return void
*/
public function upgradeSchemaVersion(): void
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's a good idea to let the user choose which method to use, between migrating + upgrading the version or migrating and upgrading the version later.

tests/SchemaVersionTest.php Outdated Show resolved Hide resolved
@florianJacques
Copy link
Contributor Author

Thanks for your work !

@GromNaN GromNaN merged commit 179c6a6 into mongodb:4.6 Jul 9, 2024
26 checks passed
@GromNaN GromNaN changed the title Add document version feature Add model schema version feature Jul 9, 2024
@GromNaN
Copy link
Member

GromNaN commented Jul 9, 2024

Thank you @florianJacques

@GromNaN GromNaN changed the title Add model schema version feature PHPORM-206 Add model schema version feature Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants