iberflow / laravel-model-generator

Laravel 5 model generator for an existing schema
264 stars 76 forks source link

Include relations #9

Closed tomsowerby closed 7 years ago

tomsowerby commented 8 years ago

This isn't really an issue but I thought I'd raise the idea and open discussion.

It would be excellent if this libary added relations to the models for you.

The sql to see these:

SELECT
  `TABLE_SCHEMA`,                          -- Foreign key schema
  `TABLE_NAME`,                            -- Foreign key table
  `COLUMN_NAME`,                           -- Foreign key column
  `REFERENCED_TABLE_SCHEMA`,               -- Origin key schema
  `REFERENCED_TABLE_NAME`,                 -- Origin key table
  `REFERENCED_COLUMN_NAME`                 -- Origin key column
FROM
  `INFORMATION_SCHEMA`.`KEY_COLUMN_USAGE`  -- Will fail if user don't have privilege
WHERE
  `TABLE_SCHEMA` = SCHEMA()                -- Detect current schema in USE
  AND `REFERENCED_TABLE_NAME` IS NOT NULL; -- Only tables with foreign keys

I personally feel that to keep the scope small, assuming a one (from origin) to many (on foreign) would be ok. Perhaps it should only do this is a certain flag is provided?

Thoughts?

iberflow commented 8 years ago

Hi there, Yeah I've been thinking about this for a while but the whole thing needs quite a bit of refactoring in my opinion and at this point I don't have much time take it on. It's definitely a great feature to have as well as PostgreSQL support. I'd love to see a PR for this, or in any case I'll try to jump in, in a couple of weeks.

One thing that popped into my mind was that maybe it would be possible to generate models based on migrations rather then the schema itself, that way this package wouldn't be bound to MySQL but rather to Laravel's Schema builder which in general has most of the features that can be used/mapped in the models.

To answer your question, yeah the flag seems like a proper option in this case, but I'd probably have it enabled by default since I've never had a case where I didn't need to include the relationships:)

What do you think?

ghost commented 8 years ago

To me the real benefit of this package is using it with existing database schemas that don't conform to the Laravel style db.

For instance in my case, I'm using it to convert a large shopping cart application over to Eloquent. The existing db doesn't have id primary keys, or foreign keys or migrations for that matter. The primary keys are using the convention of table_id.

While this package falls short of that task, it still does a pretty good job and will save a bunch of time, especially considering I have about 130 tables to model.

I don't think switching it to use migrations would be a good idea. Maybe a --use-migrations flag as an option?

+1 for relationships though, but that does seem like a huge job to tackle.

iberflow commented 8 years ago

I've started rewriting the package from scratch, therefore I'm thinking about implementing this feature. I think the main issue is that I need to analyze the full schema at once rather than go through each table and then render the model. I think a simple solution would be gather and loop through all tables, cache the foreign keys and then render the relationships as such (pseudo code).

// collect all tables, all foreign keys
// and then do the following
$tables['table_a'] = [
    'foreign_keys' => [
        'some_foreign_column' => 'table_b'
    ],
    'class_name'   => 'App\Models\Table'
];

$currentTable = 'table_a';
foreach ($tables as $table => $properties) {
    if ($currentTable === $table) {
        foreach ($properties['foreign_keys'] as $key => $foreignTable) {
            if(isset($tables[$foreignTable])) {
                $this->addRelationship('n:n', $tables[$foreignTable]['class_name']);
            }
        }
    }
}

Any ideas how to figure out what kind of relationship that is? as in 1:1, 1:n, n:n?

serjgamover commented 7 years ago

You need to check the currentTable and foreignTable key. If current is a primaryKey - than it's 1:, else it's n: If foreign is a primaryKey - than it's :1, else it's :n