tattersoftware / codeigniter4-schemas

Database schema management, for CodeIgniter 4
MIT License
23 stars 10 forks source link

Load models outside of 'Models' folders. #18

Closed sfadschm closed 3 years ago

sfadschm commented 3 years ago

This is related to #17 and might also be connected to #7 when it gets implemented.

Not sure if the current behaviour is desired, but the ModelHandler currently only explicitly loads classes contained in 'Models' folders in the namespaces. https://github.com/tattersoftware/codeigniter4-schemas/blob/4b7fc16e73c2dee2cfcf468a7eff8935fd592920/src/Drafter/Handlers/ModelHandler.php#L150 This might ignore some model classes that are stored in other subfolders (e.g., Myth\Auth\Authorization).

As explained in #17, the ModelHandler uses get_declared_classes to collect class names. Therefore some of those not explicitly loaded models will still be collected, if they have been loaded from the system before the draft() (e.g. 'Myth\Auth\Authorization\GroupModel' is group filters are used for the current route).

A workaround might be, to not only load files in Models subdirectories but also file ending with Model.php or something like this. As all file are required_once, this should not be a problem but ensure, that all (or at least, more) models are considered.

MGatner commented 3 years ago

Automated discovery has to balance thoroughness with efficiency. A serious project implementing Schemas will likely want to provide explicit definitions instead of relying on the discovery, but the truth is that most projects will not, nor will they pre-cache, so we have to keep the discovery efficient. I think a full filesystem scan could definitely catch more, but it will be costly.

It might be a good addition as a CLI argument or something, where we could guarantee the full load was only happening in a setting where some user was waiting for a response.

sfadschm commented 3 years ago

You're right, the goal should be explicit definitions eventually, but thats not gonna happen for everyone. For me, I've turned off automatic drafting totally and set the cache lifetime of the schema to never expire, as I don't really see any use in having it autoamtically redrafted every x hours or so, except that drafting might fail for some reason at sometime and crash the whole site. Therefore I only redraft the schema from a specific controller manually, by cleaning schemas chache beforehand. However, with the potential new model drafter using Factories, autodiscovery might become a completely different topic anyways :)

MGatner commented 3 years ago

I think that's a brilliant way to do it. I would probably do it via Commands myself but more of a person preference.

I also added migrate triggers to the framework a little while ago, so at some point I will probably add a listeners option to this repo to auto-regenerate.

sfadschm commented 3 years ago

I can't ssh into that specific server, so commands are a little complicated, unfortunately. Migration triggers sounds pretty clever. Might also be a good way to have people deal with migration more (should maybe do that myself, too...

Closing as this will likely be covered or deprecated in the refactor.

MGatner commented 3 years ago

Once codeigniter4-tasks is ready that would be another good way to activate schema generation, assuming you have access to the cron (e.g. from CPanel).

All you ideas and work are appreciated! Please don't balk from PRs just because "somewhere down the road this might change". I'm sharing some of my ideas without a specific roadmap, but the reality is I'm strapped for time for much module development.

sfadschm commented 3 years ago

That would be feasible. In gerneral, I like the idea of decoupling such actions from the app and its interface.

I will see what I think is really useful changes in the end, if I'm confident with a solution, I will happily reinstate the PR :)