silverqx / TinyORM

Modern C++ ORM library
https://www.tinyorm.org
MIT License
242 stars 25 forks source link

Let TomApplication accept a parent/delegator migration class #47

Closed Redmanacac closed 2 months ago

Redmanacac commented 3 months ago

Hi,

with seeders its possible to go

.seeders<DatabaseSeeder>()

Which contains following code in my project

struct DatabaseSeeder : Seeder {
   /*! Run the database seeders. */
   void
   run() override {
      call<DatabaseCoreSeeder, DatabaseProjectSeeder>();
   }
};

As you can see, with this approach I am able to seed project specific data and at the same time keep my core data which will always provided nicely seperated.

Can the same be implemented for the .migration<> part of TomApplication?

Right now I got this

.migrations<CreateRolesTable, CreatePermissionsTable,
              CreateRolespermissionsTable, CreateUsersTable, CreateLoginsTable,
              CreateGeneralsTable, CreateSortingClassTable>

But would like to have something like this

.migrations<DatabaseMigration>

Which follows same principle as the seeder

silverqx commented 3 months ago

Hi, I understand what you mean and why you would want to have something like this, but it's not possible as all migrations logic counts with migrations to be in separate files, the reason is that migrations also contain timestamps and they must be migrated in the specific order, it could be possible to do it like that but it would be a lot of work.

silverqx commented 3 months ago

The order is decided by these timestamps in the migrations' filenames.

silverqx commented 3 months ago

Also, there is a tom make:migration CreateXyzTable command, which you can use to generate 20 migration files in 1 minute, but in the main branch, it contains a bug in the Utils::String::snake() method.

It generates a filename like 2024_08_14_005032_create_xy_ztable.hpp instead of 2024_08_14_005032_create_xyz_table.hpp.

This bug was already fixed in the develop branch. Also be aware that support for Qt v5.15 is dropped in the develop branch and in the next release I'm working on (v0.38.0).

silverqx commented 3 months ago

I have a little time now and am still thinking about this.

There is a reason why seeders have this feature and that is we can group and categorize seeders and then apply them based on the environment, so you can use different seeders for staging, testing, or development areas, or even invoke different seeders in unit tests before testing some feature.

But this isn't true for migrations, there is nothing like I will have different columns or tables based on env., all environments will use the same migrations.

I think this is the reason why I designed it this way.

Can you explain why you would need this for migration as well? Thx

Redmanacac commented 2 months ago

In our software we have different dlls. Two of these are "core" and "project". "Core" contains functionality that our very basic application consists of, in later stages we port back functionality from project that we deem usefull in further projects .

"Project" contains project specific functionality for the current customer project. This includes database adaptions and many other things. So in our case, we do almost always have some migration/models/seeders specific to a project.

Since we strictly seperate our core code from project code, we gain the benefit of "simply deleting" the project source code and we start with a fresh core application, ready for the next customer.

With the current implementation of tinyorm we have to make adjustments in our core for project database adjuments. It's not very clean and easy to look over.

I've seen this practice a lot in the industry, thats why we also decided to do it like this. Could be, that I'm the only one in need for this.

silverqx commented 2 months ago

Whether you would use this functionality in the core or project sources is unclear, but it doesn't matter. I understand what you want to achieve.

The solution is simple for you, you can create your own base Migration class eg. RMingration or Red::Migration that will extend the Tom::Migration something like class RMigration : public Tom::Migration and then you can copy all the call() related methods from the Seeder class.

Now I looked at it and it's not a good idea, it wouldn't work, migrations have up/down virtual methods and it looks like it will not even be possible to do it this way. The problem is that I don't remember how these migrations are implemented behind the scenes 😁, so I'm not able to propose another solution. It's 2 years ago when I coded it and that's a long time to remember implementation details. 🤯

I'm not going to implement it because it's non-standard. Sry 😞