Skip to content

[5.2] Model casting to any object #11225

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

Closed
wants to merge 1 commit into from
Closed

[5.2] Model casting to any object #11225

wants to merge 1 commit into from

Conversation

bkuhl
Copy link
Contributor

@bkuhl bkuhl commented Dec 7, 2015

The intended functionality of this PR is to provide the ability to cast to any object. A great example of where this would be useful is user settings when they are stored as json in a users.settings column. Having the json converted to an object allows one to easily define the default values for settings and a streamlined method of accessing these settings.

Here's an example:

class User extends Model {
    protected $casts = [
        'settings' => Settings::class
    ];
}
class Settings extends Fluent
{
    public function __construct($attributes = [])
    {
        if (!is_null($attributes)) {
            parent::__construct($attributes);
        }
    }

    public function timeszone()
    {
        return $this->get('timezone', 'America/Kentucky/Louisville');
    }
}

Currently I'm experiencing an issue with a unit test am I'm unsure how to mock app(). Thus a test is failing:

PHP Fatal error:  Call to a member function getBindings() on null in /Users/bkuhl/Personal/framework/src/Illuminate/Database/Eloquent/Model.php on line 2825

I'd like to add another assertion or two once I can get passed this issue.

@GrahamCampbell GrahamCampbell changed the title Pushing code up to share [5.2] Pushing code up to share Dec 7, 2015
@bkuhl bkuhl changed the title [5.2] Pushing code up to share [5.2] Model casting to any object Dec 7, 2015
class_exists($this->casts[$key]) ||
in_array($key, app()->getBindings())
)
) {

Choose a reason for hiding this comment

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

Expected 1 newline after opening brace; 2 found

@JosephSilber
Copy link
Contributor

Wouldn't it be better to just have a default block in that switch statement that calls castTo{type}?

@bkuhl
Copy link
Contributor Author

bkuhl commented Dec 7, 2015

Currently if it's null, it bypasses object mapping altogether. In the use case I provided, I'd still want to get an instance of Settings when it's null so that the object can decide what to do.

@AdenFraser
Copy link
Contributor

I think the ability to extend castAttribute with custom cast types would be very helpful, and perhaps a better route to acheive the functionality you are after and offer greater flexibility for others?

If a check was made for a custom attribute cast type before both the check of a null value and the getCastType switch, it would be possible to create custom cast types for objects (including null objects), serialzed objects (which are not currenty supported) and any other implementations (binary, CSV, bson etc.)

@@ -3,6 +3,7 @@
use Mockery as m;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\Relation;
use Illuminate\Support\Fluent;
Copy link
Member

Choose a reason for hiding this comment

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

please sort by length

@GrahamCampbell
Copy link
Member

👎

@GrahamCampbell
Copy link
Member

This involves using the application object that eloquent shouldn't know about.

@taylorotwell
Copy link
Member

Yeah, we probably won't be able to have access to the container from within Eloquent.

Probably the only option I can think of is to simply pass the raw database value into the object just using like new Settings($rawValue). And you can do whatever you want in the Settings constructor.

One thing I'm wondering about is how do you cast back for storage... like if I do:

$model = Model::find(1);
$model->settings = new Settings('something');
$model->save();

How do I know what to make settings in the database?

@bkuhl
Copy link
Contributor Author

bkuhl commented Dec 7, 2015

I appreciate the feedback. I did miss this in the implementation, but my thought was that casted objects would need to implement Jsonable. I see the challenges with the IOC container... maybe this functionality is best staying in a Laravel package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants