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.

Sign up for GitHub

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

[6.x] Allow explicit Model definitions in database rules #30653

Merged
merged 4 commits into from
Nov 22, 2019
Merged

[6.x] Allow explicit Model definitions in database rules #30653

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
57306b7
Allow explicit model definitions in database rules
ahinkle Nov 22, 2019
24c73db
Add few more test to support explicit models in Database Rule
ahinkle Nov 22, 2019
758f93a
Style CI
ahinkle Nov 22, 2019
0ced00e
Style CI
ahinkle Nov 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion src/Illuminate/Validation/Rules/DatabaseRule.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
namespace Illuminate\Validation\Rules;

use Closure;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Support\Str;

trait DatabaseRule
{
Expand Down Expand Up @@ -43,10 +45,31 @@ trait DatabaseRule
*/
public function __construct($table, $column = 'NULL')
{
$this->table = $table;
$this->table = $this->resolveTableName($table);
$this->column = $column;
}

/**
* Resolves the name of the table from the given string.
*
* @param string $table
* @return string
*/
public function resolveTableName($table)
{
if (! Str::contains($table, '\\') || ! class_exists($table)) {
return $table;
}

$model = new $table;

if (! $model instanceof Model) {
return $table;
}

return $model->getTable();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Feel this could have been cleaner by checking the opposite, i.e. checking if the argument is a Model instance rather than not; checking the string is a class and exists rather than not.

public function resolveTableName($table)
{
    if (is_object($table) && $table instanceof Model) {
        return $table->getTable();
    }

    if (class_exists($table)) {
        return (new $table)->getTable();
    }

    return $table;
}


/**
* Set a "where" constraint on the query.
*
Expand Down
48 changes: 35 additions & 13 deletions tests/Validation/ValidationExistsRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,20 +39,36 @@ public function testItCorrectlyFormatsAStringVersionOfTheRule()
$rule->where('foo', 'bar');
$this->assertSame('exists:table,NULL,foo,"bar"', (string) $rule);

$rule = new Exists(User::class);
$rule->where('foo', 'bar');
$this->assertSame('exists:users,NULL,foo,"bar"', (string) $rule);

$rule = new Exists('table', 'column');
$rule->where('foo', 'bar');
$this->assertSame('exists:table,column,foo,"bar"', (string) $rule);

$rule = new Exists(User::class, 'column');
$rule->where('foo', 'bar');
$this->assertSame('exists:users,column,foo,"bar"', (string) $rule);

$rule = new Exists('Illuminate\Tests\Validation\User', 'column');
$rule->where('foo', 'bar');
$this->assertSame('exists:users,column,foo,"bar"', (string) $rule);

$rule = new Exists(NoTableNameModel::class, 'column');
$rule->where('foo', 'bar');
$this->assertSame('exists:no_table_name_models,column,foo,"bar"', (string) $rule);
}

public function testItChoosesValidRecordsUsingWhereInRule()
{
$rule = new Exists('users', 'id');
$rule->whereIn('type', ['foo', 'bar']);

EloquentTestUser::create(['id' => '1', 'type' => 'foo']);
EloquentTestUser::create(['id' => '2', 'type' => 'bar']);
EloquentTestUser::create(['id' => '3', 'type' => 'baz']);
EloquentTestUser::create(['id' => '4', 'type' => 'other']);
User::create(['id' => '1', 'type' => 'foo']);
User::create(['id' => '2', 'type' => 'bar']);
User::create(['id' => '3', 'type' => 'baz']);
User::create(['id' => '4', 'type' => 'other']);

$trans = $this->getIlluminateArrayTranslator();
$v = new Validator($trans, [], ['id' => $rule]);
Expand All @@ -73,10 +89,10 @@ public function testItChoosesValidRecordsUsingWhereNotInRule()
$rule = new Exists('users', 'id');
$rule->whereNotIn('type', ['foo', 'bar']);

EloquentTestUser::create(['id' => '1', 'type' => 'foo']);
EloquentTestUser::create(['id' => '2', 'type' => 'bar']);
EloquentTestUser::create(['id' => '3', 'type' => 'baz']);
EloquentTestUser::create(['id' => '4', 'type' => 'other']);
User::create(['id' => '1', 'type' => 'foo']);
User::create(['id' => '2', 'type' => 'bar']);
User::create(['id' => '3', 'type' => 'baz']);
User::create(['id' => '4', 'type' => 'other']);

$trans = $this->getIlluminateArrayTranslator();
$v = new Validator($trans, [], ['id' => $rule]);
Expand All @@ -97,10 +113,10 @@ public function testItChoosesValidRecordsUsingWhereNotInAndWhereNotInRulesTogeth
$rule = new Exists('users', 'id');
$rule->whereIn('type', ['foo', 'bar', 'baz'])->whereNotIn('type', ['foo', 'bar']);

EloquentTestUser::create(['id' => '1', 'type' => 'foo']);
EloquentTestUser::create(['id' => '2', 'type' => 'bar']);
EloquentTestUser::create(['id' => '3', 'type' => 'baz']);
EloquentTestUser::create(['id' => '4', 'type' => 'other']);
User::create(['id' => '1', 'type' => 'foo']);
User::create(['id' => '2', 'type' => 'bar']);
User::create(['id' => '3', 'type' => 'baz']);
User::create(['id' => '4', 'type' => 'other']);

$trans = $this->getIlluminateArrayTranslator();
$v = new Validator($trans, [], ['id' => $rule]);
Expand Down Expand Up @@ -175,9 +191,15 @@ public function getIlluminateArrayTranslator()
/**
* Eloquent Models.
*/
class EloquentTestUser extends Eloquent
class User extends Eloquent
{
protected $table = 'users';
protected $guarded = [];
public $timestamps = false;
}

class NoTableNameModel extends Eloquent
{
protected $guarded = [];
public $timestamps = false;
}
24 changes: 24 additions & 0 deletions tests/Validation/ValidationUniqueRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,28 @@ public function testItCorrectlyFormatsAStringVersionOfTheRule()
$rule->where('foo', 'bar');
$this->assertSame('unique:table,NULL,NULL,id,foo,"bar"', (string) $rule);

$rule = new Unique(EloquentModelStub::class);
$rule->where('foo', 'bar');
$this->assertSame('unique:table,NULL,NULL,id,foo,"bar"', (string) $rule);

$rule = new Unique(NoTableName::class);
$rule->where('foo', 'bar');
$this->assertSame('unique:no_table_names,NULL,NULL,id,foo,"bar"', (string) $rule);

$rule = new Unique('Illuminate\Tests\Validation\NoTableName');
$rule->where('foo', 'bar');
$this->assertSame('unique:no_table_names,NULL,NULL,id,foo,"bar"', (string) $rule);

$rule = new Unique('table', 'column');
$rule->ignore('Taylor, Otwell', 'id_column');
$rule->where('foo', 'bar');
$this->assertSame('unique:table,column,"Taylor, Otwell",id_column,foo,"bar"', (string) $rule);

$rule = new Unique(EloquentModelStub::class, 'column');
$rule->ignore('Taylor, Otwell', 'id_column');
$rule->where('foo', 'bar');
$this->assertSame('unique:table,column,"Taylor, Otwell",id_column,foo,"bar"', (string) $rule);

$rule = new Unique('table', 'column');
$rule->ignore('Taylor, Otwell"\'..-"', 'id_column');
$rule->where('foo', 'bar');
Expand Down Expand Up @@ -51,6 +68,13 @@ public function testItCorrectlyFormatsAStringVersionOfTheRule()

class EloquentModelStub extends Model
{
protected $table = 'table';
protected $primaryKey = 'id_column';
protected $guarded = [];
}

class NoTableName extends Model
{
protected $guarded = [];
public $timestamps = false;
}