Skip to content

Commit e55fa70

Browse files
authored
Issue 160: Avoid calling internal methods with null (#164)
* Fixes to avoid passing null values to qualiyColumn method * Fixed test * Removed debug code
1 parent 9ffa7d4 commit e55fa70

File tree

6 files changed

+39
-10
lines changed

6 files changed

+39
-10
lines changed

src/Mixins/RelationshipsExtraMethods.php

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
namespace Kirschbaum\PowerJoins\Mixins;
44

5+
use Stringable;
6+
use Illuminate\Support\Str;
57
use Kirschbaum\PowerJoins\StaticCache;
68
use Kirschbaum\PowerJoins\PowerJoinClause;
79
use Kirschbaum\PowerJoins\Tests\Models\Post;
@@ -256,8 +258,8 @@ protected function performJoinForEloquentPowerJoinsForMorphTo()
256258
"{$modelInstance->getTable()}.{$modelInstance->getKeyName()}"
257259
)->where("{$this->getModel()->getTable()}.{$this->getMorphType()}", '=', $modelInstance->getMorphClass());
258260

259-
if ($disableExtraConditions === false && $this->usesSoftDeletes($this->query->getModel())) {
260-
$join->whereNull($this->query->getModel()->getQualifiedDeletedAtColumn());
261+
if ($disableExtraConditions === false && $this->usesSoftDeletes($modelInstance)) {
262+
$join->whereNull($modelInstance->getQualifiedDeletedAtColumn());
261263
}
262264

263265
if ($disableExtraConditions === false) {
@@ -267,7 +269,7 @@ protected function performJoinForEloquentPowerJoinsForMorphTo()
267269
if ($callback && is_callable($callback)) {
268270
$callback($join);
269271
}
270-
}, $this->getModel());
272+
}, $modelInstance);
271273

272274
return $this;
273275
};
@@ -485,7 +487,13 @@ public function applyNestedCondition()
485487
public function shouldNotApplyExtraCondition()
486488
{
487489
return function ($condition) {
488-
$key = $this->getPowerJoinExistenceCompareKey();
490+
if (isset($condition['column']) && Str::endsWith($condition['column'], '.')) {
491+
return true;
492+
}
493+
494+
if (! $key = $this->getPowerJoinExistenceCompareKey()) {
495+
return true;
496+
}
489497

490498
if (isset($condition['query'])) {
491499
return false;
@@ -502,6 +510,10 @@ public function shouldNotApplyExtraCondition()
502510
public function getPowerJoinExistenceCompareKey()
503511
{
504512
return function () {
513+
if ($this instanceof MorphTo) {
514+
return [$this->getMorphType(), $this->getForeignKeyName()];
515+
}
516+
505517
if ($this instanceof BelongsTo) {
506518
return $this->getQualifiedOwnerKeyName();
507519
}

src/PowerJoinClause.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use Illuminate\Database\Eloquent\Model;
1010
use Illuminate\Database\Query\JoinClause;
1111
use Illuminate\Database\Eloquent\SoftDeletes;
12+
use Illuminate\Database\Eloquent\SoftDeletingScope;
1213

1314
class PowerJoinClause extends JoinClause
1415
{
@@ -96,6 +97,10 @@ public function withGlobalScopes(): self
9697
continue;
9798
}
9899

100+
if ($scope instanceof SoftDeletingScope) {
101+
continue;
102+
}
103+
99104
(new $scope())->apply($this, $this->model);
100105
}
101106

tests/JoinRelationshipExtraConditionsTest.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ public function test_extra_conditions_with_belongs_to_many()
141141
$this->assertCount(1, Group::joinRelationship('publishedPosts')->get());
142142

143143
$this->assertStringContainsString(
144-
'inner join "posts" on "posts"."id" = "post_groups"."post_id" and "posts"."published" = ?',
144+
'inner join "posts" on "posts"."id" = "post_groups"."post_id" and "posts"."deleted_at" is null and "posts"."published" = ?',
145145
Group::joinRelationship('publishedPosts')->toSql()
146146
);
147147
}
@@ -163,7 +163,7 @@ public function test_extra_conditions_in_pivot_with_belongs_to_many_in()
163163
$this->assertCount(2, Group::joinRelationship('recentPosts')->get());
164164

165165
$this->assertStringContainsString(
166-
'inner join "posts" on "posts"."id" = "post_groups"."post_id" and "post_groups"."assigned_at" >= ?',
166+
'inner join "posts" on "posts"."id" = "post_groups"."post_id" and "posts"."deleted_at" is null and "post_groups"."assigned_at" >= ?',
167167
Group::joinRelationship('recentPosts')->toSql()
168168
);
169169
}
@@ -238,7 +238,7 @@ public function test_extra_conditions_with_closure()
238238
User::joinRelationship('publishedOrReviewedPosts')->get();
239239

240240
$this->assertStringContainsString(
241-
'inner join "posts" on "posts"."user_id" = "users"."id" and ("published" = ? or "reviewed" = ?)',
241+
'inner join "posts" on "posts"."user_id" = "users"."id" and "posts"."deleted_at" is null and ("published" = ? or "reviewed" = ?)',
242242
$query
243243
);
244244
}

tests/JoinRelationshipTest.php

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -700,7 +700,7 @@ public function test_scope_inside_nested_where()
700700
})->toSql();
701701

702702
$this->assertStringContainsString(
703-
'inner join "posts" on "comments"."post_id" = "posts"."id" and ("posts"."published" = ?)',
703+
'inner join "posts" on "comments"."post_id" = "posts"."id" and "posts"."deleted_at" is null and ("posts"."published" = ?)',
704704
$sql
705705
);
706706
}
@@ -798,9 +798,16 @@ public function test_it_doesnt_fail_to_join_the_same_query_repeatedly()
798798

799799
public function test_join_morph_to_morphable_class()
800800
{
801-
factory(Image::class, 5)->state('owner:post')->create();
801+
$publishedPost = factory(Post::class)->state('published')->create();
802+
$unpublishedPost = factory(Post::class)->state('unpublished')->create();
803+
factory(Image::class, 3)->state('owner:post')->create(['imageable_id' => $unpublishedPost->id]);
804+
factory(Image::class, 2)->state('owner:post')->create(['imageable_id' => $publishedPost->id]);
802805
factory(Image::class, 4)->state('owner:user')->create();
803806

807+
$publishedPostImages = Image::query()
808+
->joinRelationship('imageable', callback: fn ($join) => $join->published(), morphable: Post::class)
809+
->get();
810+
804811
$postImages = Image::query()
805812
->joinRelationship('imageable', morphable: Post::class)
806813
->get();
@@ -810,10 +817,11 @@ public function test_join_morph_to_morphable_class()
810817
->toSql();
811818

812819
$this->assertStringContainsString(
813-
'inner join "posts" on "images"."imageable_id" = "posts"."id" and "images"."imageable_type" = ?',
820+
'inner join "posts" on "images"."imageable_id" = "posts"."id" and "images"."imageable_type" = ? and "posts"."deleted_at" is null',
814821
$sql
815822
);
816823

817824
$this->assertCount(5, $postImages);
825+
$this->assertCount(2, $publishedPostImages);
818826
}
819827
}

tests/Models/Post.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,12 @@
1010
use Illuminate\Database\Eloquent\Relations\BelongsTo;
1111
use Illuminate\Database\Eloquent\Relations\MorphMany;
1212
use Illuminate\Database\Eloquent\Relations\MorphToMany;
13+
use Illuminate\Database\Eloquent\SoftDeletes;
1314

1415
class Post extends Model
1516
{
1617
use PowerJoins;
18+
use SoftDeletes;
1719

1820
/** @var string */
1921
protected $table = 'posts';

tests/database/migrations/2020_03_16_000000_create_tables.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ public function up()
5454
$table->boolean('reviewed')->default(true);
5555
$table->boolean('published')->default(true);
5656
$table->timestamps();
57+
$table->softDeletes();
5758
});
5859

5960
Schema::create('post_groups', function (Blueprint $table) {
@@ -84,6 +85,7 @@ public function up()
8485
$table->morphs('imageable');
8586
$table->boolean('cover')->default(false);
8687
$table->timestamps();
88+
$table->softDeletes();
8789
});
8890

8991
Schema::create('tags', function (Blueprint $table) {

0 commit comments

Comments
 (0)