Skip to content
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

Eloquent support #19

Open
AidasK opened this issue Sep 20, 2019 · 5 comments
Open

Eloquent support #19

AidasK opened this issue Sep 20, 2019 · 5 comments

Comments

@AidasK
Copy link

AidasK commented Sep 20, 2019

I would like to create a pull request to add these traits for eloquent models. I would change it a bit to use this lib instead of hashid directly, but you get the idea.

I use these two alot and think it would be great to include them to this lib

HashID.php

/**
 * Trait HashID
 * @package App\Traits
 * @property-read string $hash_id
 * @mixin Model
 */
trait HashID
{
    public function getHashIdAttribute()
    {
        return static::idToHash($this->getKey());
    }

    public static function idToHash($id)
    {
        return static::newHashId()->encode($id);
    }

    public static function hashToId($code)
    {
        $ids = static::newHashId()->decode($code);
        $id = empty($ids) ? null : $ids[0];
        return $id;
    }

    /**
     * @param $code
     * @param array $columns
     * @return static
     */
    public static function findByHash($code, $columns = null)
    {
        $id = static::hashToId($code);
        return static::find($id, $columns);
    }

    /**
     * @param $code
     * @param array $columns
     * @return static
     */
    public static function findOrFailByHash($code, $columns = null)
    {
        $id = static::hashToId($code);
        return static::findOrFail($id, $columns);
    }

    public static function newHashId()
    {
        return new Hashids(config('app.key'), 5, 'qwertyuiopasdfghjklzxcvbnm7894561230');
    }
}

HashIDRoutable.php

/**
 * Trait HashID
 * @package App\Traits
 * @mixin \Eloquent
 * @mixin HashID
 */
trait HashIDRoutable
{
    public function getRouteKeyName()
    {
        return 'hash_id';
    }

    public function resolveRouteBinding($value)
    {
        return (new static)->find(static::hashToId($value));
    }
}

Usage

class OrderReturn extends Model
{
    use HashID; 
    use HashIDRoutable; // hash id is used instead of id
}
@ElfSundae
Copy link
Owner

Thanks for your advice, and I also have several hashid traits in my projects. This package is just a manager for multiple connections and various encode drivers, I think it is better to let package users use it as they want, or define their own traits based on different requirements.

@AidasK
Copy link
Author

AidasK commented Sep 22, 2019

Trait implementation is the same in the every project, that's why I think it can be included to the package.
Ofcourse I would modify above code to be more customizable and I would include parameters for "key" name and for connection instead of "newHashId()" method.

@ElfSundae
Copy link
Owner

YES, I know your opinion. What I said "different requirements", such as multiple hashed columns, or keep the original value for internal use but expose the hashed value in URL or response data.

For obfuscating the primary key or other sensitive columns, I prefer a non-invasive way to do this. I mean do not change the way of retrieving or saving the Eloquent ORM, do not need to explicitly call such findByHash findOrFailByHash methods, the hashing work should be automatically done in the route model binding, toJson() etc. In fact, when I developed this package, I'd like to provide such a function, but there are some issues on Laravel route model binding at that time, these issues may be fixed now, I have not used Laravel for two years.

@AidasK
Copy link
Author

AidasK commented Sep 23, 2019

Well, this approach is quite suitable for Laravel 6.x. Though there is always more than one solution and this could be the start. If you don't have time for this, I could use this repo as dependency and add these traits to a new repo

@ElfSundae
Copy link
Owner

A new repo is great 👍

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

No branches or pull requests

2 participants