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

[5.5] Add a "qualifyColumn" method to the Eloquent Model #22577

Merged
merged 2 commits into from
Jan 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 11 additions & 0 deletions src/Illuminate/Database/Eloquent/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -1222,6 +1222,17 @@ public function setModel(Model $model)
return $this;
}

/**
* Qualify the given column name by the model's table.
*
* @param string $column
* @return string
*/
public function qualifyColumn($column)
{
return $this->model->qualifyColumn($column);
}

/**
* Get the given macro by name.
*
Expand Down
17 changes: 16 additions & 1 deletion src/Illuminate/Database/Eloquent/Model.php
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,21 @@ public function forceFill(array $attributes)
});
}

/**
* Qualify the given column name by the model's table.
*
* @param string $column
* @return string
*/
public function qualifyColumn($column)
{
if (Str::contains($column, '.')) {
return $column;
}

return $this->getTable().'.'.$column;
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this is missing quotations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uh, what?

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean quotations of the table name and the column name. We already have this available via Grammar::wrap().

Copy link
Contributor

Choose a reason for hiding this comment

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

Why would this suddenly be an issue? I mean I understand your idea, but this PR is only refactoring existing code doing exactly what this line does, nothing else.

I think if grammar shall be involved in this, it should be a separate PR anyway.

}

/**
* Remove the table name from a given key.
*
Expand Down Expand Up @@ -1206,7 +1221,7 @@ public function setKeyName($key)
*/
public function getQualifiedKeyName()
{
return $this->getTable().'.'.$this->getKeyName();
return $this->qualifyColumn($this->getKeyName());
}

/**
Expand Down
6 changes: 3 additions & 3 deletions src/Illuminate/Database/Eloquent/Relations/BelongsTo.php
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ public function getRelationExistenceQuery(Builder $query, Builder $parentQuery,
}

return $query->select($columns)->whereColumn(
$this->getQualifiedForeignKey(), '=', $query->getModel()->getTable().'.'.$this->ownerKey
$this->getQualifiedForeignKey(), '=', $query->qualifyColumn($this->ownerKey)
);
}

Expand Down Expand Up @@ -327,7 +327,7 @@ public function getForeignKey()
*/
public function getQualifiedForeignKey()
{
return $this->child->getTable().'.'.$this->foreignKey;
return $this->child->qualifyColumn($this->foreignKey);
}

/**
Expand All @@ -347,7 +347,7 @@ public function getOwnerKey()
*/
public function getQualifiedOwnerKeyName()
{
return $this->related->getTable().'.'.$this->ownerKey;
return $this->related->qualifyColumn($this->ownerKey);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -959,7 +959,7 @@ public function getQualifiedRelatedPivotKeyName()
*/
public function getQualifiedParentKeyName()
{
return $this->parent->getTable().'.'.$this->parentKey;
return $this->parent->qualifyColumn($this->parentKey);
}

/**
Expand Down
8 changes: 4 additions & 4 deletions src/Illuminate/Database/Eloquent/Relations/HasManyThrough.php
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ protected function performJoin(Builder $query = null)
*/
public function getQualifiedParentKeyName()
{
return $this->parent->getTable().'.'.$this->secondLocalKey;
return $this->parent->qualifyColumn($this->secondLocalKey);
}

/**
Expand Down Expand Up @@ -495,7 +495,7 @@ public function getQualifiedFarKeyName()
*/
public function getQualifiedFirstKeyName()
{
return $this->throughParent->getTable().'.'.$this->firstKey;
return $this->throughParent->qualifyColumn($this->firstKey);
}

/**
Expand All @@ -505,7 +505,7 @@ public function getQualifiedFirstKeyName()
*/
public function getQualifiedForeignKeyName()
{
return $this->related->getTable().'.'.$this->secondKey;
return $this->related->qualifyColumn($this->secondKey);
}

/**
Expand All @@ -515,6 +515,6 @@ public function getQualifiedForeignKeyName()
*/
public function getQualifiedLocalKeyName()
{
return $this->farParent->getTable().'.'.$this->localKey;
return $this->farParent->qualifyColumn($this->localKey);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ public function getParentKey()
*/
public function getQualifiedParentKeyName()
{
return $this->parent->getTable().'.'.$this->localKey;
return $this->parent->qualifyColumn($this->localKey);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Illuminate/Database/Eloquent/SoftDeletes.php
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,6 @@ public function getDeletedAtColumn()
*/
public function getQualifiedDeletedAtColumn()
{
return $this->getTable().'.'.$this->getDeletedAtColumn();
return $this->qualifyColumn($this->getDeletedAtColumn());
}
}
15 changes: 13 additions & 2 deletions tests/Database/DatabaseEloquentBuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\Eloquent\Collection;
use Illuminate\Support\Collection as BaseCollection;
use Illuminate\Database\Query\Builder as BaseBuilder;

class DatabaseEloquentBuilderTest extends TestCase
{
Expand Down Expand Up @@ -126,6 +127,16 @@ public function testFirstMethod()
$this->assertEquals('bar', $result);
}

public function testQualifyColumn()
{
$builder = new Builder(m::mock(BaseBuilder::class));
$builder->shouldReceive('from')->with('stub');

$builder->setModel(new EloquentModelStub);

$this->assertEquals('stub.column', $builder->qualifyColumn('column'));
}

public function testGetMethodLoadsModelsAndHydratesEagerRelations()
{
$builder = m::mock('Illuminate\Database\Eloquent\Builder[getModels,eagerLoadRelations]', [$this->getMockQueryBuilder()]);
Expand Down Expand Up @@ -366,7 +377,7 @@ public function testPluckWithoutModelGetterJustReturnTheAttributesFoundInDatabas
public function testLocalMacrosAreCalledOnBuilder()
{
unset($_SERVER['__test.builder']);
$builder = new \Illuminate\Database\Eloquent\Builder(new \Illuminate\Database\Query\Builder(
$builder = new Builder(new BaseBuilder(
m::mock('Illuminate\Database\ConnectionInterface'),
m::mock('Illuminate\Database\Query\Grammars\Grammar'),
m::mock('Illuminate\Database\Query\Processors\Processor')
Expand Down Expand Up @@ -987,7 +998,7 @@ protected function getMockModel()

protected function getMockQueryBuilder()
{
$query = m::mock('Illuminate\Database\Query\Builder');
$query = m::mock(BaseBuilder::class);
$query->shouldReceive('from')->with('foo_table');

return $query;
Expand Down
2 changes: 1 addition & 1 deletion tests/Database/DatabaseEloquentHasOneTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ public function testRelationCountQueryCanBeBuilt()
$builder->shouldReceive('getQuery')->once()->andReturn($parentQuery);

$builder->shouldReceive('select')->once()->with(m::type('Illuminate\Database\Query\Expression'))->andReturnSelf();
$relation->getParent()->shouldReceive('getTable')->andReturn('table');
$relation->getParent()->shouldReceive('qualifyColumn')->andReturn('table.id');
$builder->shouldReceive('whereColumn')->once()->with('table.id', '=', 'table.foreign_key')->andReturn($baseQuery);
$baseQuery->shouldReceive('setBindings')->once()->with([], 'select');

Expand Down
7 changes: 7 additions & 0 deletions tests/Database/DatabaseEloquentModelTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -866,6 +866,13 @@ public function testFillable()
$this->assertEquals('bar', $model->age);
}

public function testQualifyColumn()
{
$model = new EloquentModelStub;

$this->assertEquals('stub.column', $model->qualifyColumn('column'));
}

public function testForceFillMethodFillsGuardedAttributes()
{
$model = (new EloquentModelSaveStub)->forceFill(['id' => 21]);
Expand Down