laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.54k stars 11.02k forks source link

[Proposal] Mapping functionality for Eloquent #7398

Closed andyfleming closed 9 years ago

andyfleming commented 9 years ago

Consider the following example.

<?php namespace Example\Code;

use Illuminate\Database\Eloquent\Model;

class Profile extends Model {

    protected $table = 'profiles';

    protected $appends = [
        'email'
    ];

    /**
     * Returns email_primary as email
     * 
     * @return mixed
     */
    public function getEmailAttribute()
    {
        return $this->attributes['email_primary'];
    }

}

It is rather clunky and potentially inefficient for such a simple issue. A use-case would be if you have a legacy table that you are providing an API for and want specific control over naming conventions.

The following would be the proposed syntax for mapping.

<?php namespace Example\Code;

use Illuminate\Database\Eloquent\Model;

class Profile extends Model {

    protected $table = 'profiles';

    /**
     * @var bool (optional) default false
     */
    protected $mappedOnly = false;

    /**
     * @var array (optional) specifies columns to alias
     */
    protected $map = [
        'email_primary' => 'email'
    ];

}

It is similar to how doctrine works in ways while keeping the simplicity of eloquent.

The $map variable would take a key, value and treat it as column, alias.

The $mappedOnly boolean would allow for, if true, to only select the mapped columns. This could help in certain situations where you need to load part of a table efficiently.

All in all, it adds power with a clean opt-in approach.

Thoughts?

ghost commented 9 years ago

Definitely cleaner. +1

ottowayne commented 9 years ago

+1 Would come in handy in an application I am building right now.

franzliedke commented 9 years ago

It's clearer, but less powerful. Is this supposed to replace, or augment the accessor methods?

andyfleming commented 9 years ago

It is supposed to augment (and really be independent).

Here's an example:


profiles table

id primary_email first_name last_name favorite_color favorite_animal
1 jsmith@example.com John Smith blue tiger
2 sbanks@example.com Susan Banks red griaffe

Profile.php

<?php

use Illuminate\Database\Eloquent\Model;

class Profile extends Model {

    protected $table = 'profiles';

    protected $map = [
        'primary_email' => 'email'
    ];

    protected $appends = [
        'full_name'
    ];

    public function getFullNameAttribute()
    {
        return $this->attributes['first_name'].' '.$this->attributes['last_name'];
    }

}

LightProfile.php

<?php

use Illuminate\Database\Eloquent\Model;

class LightProfile extends Model {

    protected $table = 'profiles';

    protected $mappedOnly = true;
    protected $map = [
        'primary_email' => 'email',
        'first_name',
        'last_name'
    ];

    protected $hidden = [
        'first_name',
        'last_name'
    ];

    protected $appends = [
        'name'
    ];

    public function getNameAttribute()
    {
        return $this->attributes['first_name'].' '.$this->attributes['last_name'];
    }

}

routes.php

<?php

Route::get('/profiles', function() {
    return Profile::all();
});

Route::get('/light-profiles', function() {
    return LightProfile::all();
});

GET: /profiles

[
    {
        "id": 1,
        "email": "jsmith@example.com",
        "first_name": "John",
        "last_name": "Smith",
        "full_name": "John Smith",
        "favorite_color": "blue",
        "favorite_animal": "tiger"
    },
    {
        "id": 2,
        "email": "sbanks@example.com",
        "first_name": "Susan",
        "last_name": "Banks",
        "full_name": "Susan Banks",
        "favorite_color": "red",
        "favorite_animal": "giraffe"
    }
]

GET: /light-profiles

[
    {
        "id": 1,
        "email": "jsmith@example.com",
        "full_name": "John Smith"
    },
    {
        "id": 2,
        "email": "sbanks@example.com",
        "full_name": "Susan Banks"
    }
]

For the "light profile", it isn't just "hiding" the extra properties, as you would do with $hidden. It is actually only selecting the "mapped" fields.

Anahkiasen commented 9 years ago

So, kind of like $visible then no?

JoostK commented 9 years ago

@Anahkiasen well now the select will be constrained I'm thinking.

Anyway, I don't think this should be in the core. By using packages such as Fractal you have far more control over how your objects are sent as JSON and limiting the fields you retrieve from the database is already easily done.

The mapping in itself is something different, but I guess also easily implemented in a base superclass.

andyfleming commented 9 years ago

@JoostK — Is there really a simple way to limit the fields you retreieve the from the database? What if you want to have the contraint on all queries against that model?

On another note, it isn't really about the formatting for JSON. Its purpose is two-fold.

  1. Allowing for selection of only the necessary columns for an object (not just hidden/visible, but actual select constraints).
  2. Mapping/aliasing columns easily without needing an iteration through a collection and run an accessor/getter.
JoostK commented 9 years ago
  1. I'm not sure how easy it would be to apply it on all queries, because it is very likely to conflict with lots of Eloquent internals (where by default * is used in many places). Easiest way would be to define a VIEW in your database that only selects a subset of columns, although I don't know how this would impact performance.
  2. As I said, the mapping is essentially unrelated from limiting selected columns. I don't think they should actually be combined, if at all implemented.
andyfleming commented 9 years ago

On 2, they both affect the select part of the query. That said, it does have deeper implications with how queries are built.

Though, this new approach would be (theoretically) more performant than any other approach mentioned.

JoostK commented 9 years ago

Oh, I see, you meant to rename columns directly from the SQL. I had in mind that it was a mapping somewhere in Eloquent while setting attributes (much like your attribute setter but without that extra code)

andyfleming commented 9 years ago

Yeah, exactly. I'm trying to minimize the transformations done on the php side. There are, of course, places where it makes sense to transform a value with an accessor/getter, but it shouldn't have to be that way for a simple renaming of a column.

ardeay commented 9 years ago

+1 It would definitely be nice to not need a custom DB query to specificy the columns to select on an Eloquent model.

heisian commented 9 years ago

@ardeay well you can do this, right?

Profile::all()->select('primary_email','first_name')->get();

but the purpose of this proposal seems to be more aimed towards the renaming of columns. seems useful, +1

ibrasho commented 9 years ago

:+1:

martinbean commented 9 years ago

I like the idea of specifying aliases (similar to virtual fields in CakePHP), i.e. full_name may be the first_name and last_name attributes concatenated.

However, I don’t like this “mapped only” flag. This sounds like something that should be in a service class, and not the model class for an ORM. The ORM should fetch and persist data to a data source, it’s then up to you what you do with those records once they’ve been converted to objects.

montogeek commented 9 years ago

+1

andyfleming commented 9 years ago

@martinbean — Why would it be in a service? Isn't it the responsibility of the ORM? The hidden/visible approach is really just superficial mapping minus the ability to actually alias/map fields (which has to be done with getters/accessors).

martinbean commented 9 years ago

@andyfleming Because the hidden/visible approach works well if you only need to access different attributes depending on two contexts, but fails when you have more than two contexts. Wrap it in an appropriate class that returns the attributes for that particular use case.

andyfleming commented 9 years ago

@martinbean — Yeah. I've done that in practice before, but it doesn't change that all the columns are being loaded behind the scenes.

martinbean commented 9 years ago

@andyfleming Then that’s when you would specify the columns to load.

andyfleming commented 9 years ago

@martinbean But you need to do that on every query, right? What if I want a specific object to always load just those columns?

2bj commented 9 years ago

:+1:

KevinCreel commented 9 years ago

+1 been. Exact same situation except it's the entire database!

garygreen commented 9 years ago

-1 Not a fan of this. It adds a lot of complication into Eloquent for the sake of supporting 'legacy' applications to, essentially, prettify field names. With this, you now have 2 different field names in your application; one legacy/database one which may be used throughout your app and one that is used in eloquent and your models. If your aim is to make your application more simple and unify the field names, you've ironically done the opposite -- you've created another field name, adding another layer of complexity to your app. That is not good.

To me, if I was that fussed about changing field names in a legacy app, I would change them all by some kind of grep search / through migrations. Or, if it's not practical, use getter / setter attributes, which we already have. If your use case if for an API, format your response explicitly and just don't simply do toArray().

Also is this just for just 'getters' or should it work the other way with setting attributes, so when it comes to saving email in the database it get's saved as primary_email ?

martinbean commented 9 years ago

Completely agree, @garygreen, and you explained it far better than me when I tried.

If you need to refer to columns by different names, then that’s what the Adapter pattern is for. Don’t stick it in your ORM for the sake of legacy. It just makes things convoluted and non-uniform.

willrowe commented 9 years ago

If you need this functionality for a specific project it seems like it would be better if it was a separate package. Doesn't seem like it's something that would be needed on every project and thus included in Eloquent.

KevinCreel commented 9 years ago

My use case is creating searchable json APIs for ugly, legacy dbs that I can't change. Something to this effect would be a useful package.

martinbean commented 9 years ago

@KevinCreel Again: Adapter pattern.

FoxxMD commented 9 years ago

I know this is closed but +1

thiagomata commented 9 years ago

I know this is closed but +1. If you are using laravel 5 this can solve your problems https://github.com/jarektkaczyk/eloquence/wiki/Mappable.

odahcam commented 7 years ago

I thikn that by the name ORM (Object Relational Mapper) everything should be mappable by the tool.

Maybe some concepts of Propel or Doctrine could be used by Eloquent too.

mkrell commented 6 years ago

+1. I have an existing Laravel application that planned to have all 40+ tables migrated from an older schema, until we decided that was untenable in the near future. Making a translation layer through repositories and accessors works, but it's kind of a pain.

odahcam commented 6 years ago

I would like to see some mature evolution in Eloquent, not some software I tryied to built at 17's with PHP 5.4. I'm sadly migrating to Doctrine right now.

Or then Laravel could be more like Symfony guys and use and help another mature ORM folk, could be Propel, which needs a lot more community and has awesome features.

mkrell commented 6 years ago

You can use the packaged listed above for the mapping. I'm using it, and it works well. Still, it would be nice to have this in the core like other ORMs.

martinbean commented 6 years ago

@mkrell If other ORMs do what you want, then use them.

odahcam commented 6 years ago

@martinbean we are just dicussing here, looking for a better Eloquent. If you have nothing to add to the conversation, then please don't. Thanks for the adapter pattern btw.

garygreen commented 6 years ago

vrf4z1oz21j9e

3og0iuiibwjcexf1hg

mkrell commented 1 year ago

Has anyone found another solution besides sofa/eloquence? The package appears to be abandoned and has not been updated for php8 (You can, however, get it working for Laravel 8 and PHP7.3 if you require the dev-master branch). I'll have to update my column names if I can't find a better solution.

martinbean commented 1 year ago

Has anyone found another solution besides sofa/eloquence? The package appears to be abandoned and has not been updated for php8 (You can, however, get it working for Laravel 8 and PHP7.3 if you require the dev-master branch). I'll have to update my column names if I can't find a better solution.

Use Eloquent API resources to present a different representation on your object.

mkrell commented 1 year ago

Use Eloquent API resources to present a different representation on your object.

Unless there's something about API resources I don't understand, that doesn't help. What I need to translate the eloquent queries themselves, example ->where('customer_id', 1) to WHERE JobID = 1. That package was able to do this, and I'm guessing Doctrine can do this, but not Eloquent.

MtDalPizzol commented 11 months ago

Here's a good reason I think Eloquent should support this out of the box:

I'm using Filament, and thousands of functions on the codebase expect to receive Builder $query and to return an instance of Builder, which is Illuminate\Database\Eloquent\Builder.

However, if we depend on somethign like sofa/eloquence-mappable, the Builder on that is a completelly separate thing that doesn't extent Illuminate\Database\Eloquent\Builder.

This causes us to end up facing things like this:

Filament\Resources\Pages\ListRecords::Filament\Tables\Concerns\{closure}(): Argument #1 ($query) must be of type Illuminate\Contracts\Database\Eloquent\Builder, Sofa\Eloquence\Query\Builder given, called in /var/www/mamute/bignews-livewire/vendor/laravel/framework/src/Illuminate/Database/Query/Builder.php on line 356

So now, we need to write our own adapter for something that is pretty much the same for everyone needing this.