Skip to content

Commit 257c509

Browse files
greg0iregreg0ire
committed
Use not nullable columns when part of the primary key
Using a nullable column that references another table as part of a primary key makes no sense, and is ignored by DBAL. Let us ignore it at the ORM level.
1 parent 64444dc commit 257c509

8 files changed

+189
-8
lines changed

‎src/Mapping/JoinColumnProperties.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ public function __construct(
1212
public readonly string|null $referencedColumnName = null,
1313
public readonly bool $deferrable = false,
1414
public readonly bool $unique = false,
15-
public readonly bool $nullable = true,
15+
public readonly bool|null $nullable = null,
1616
public readonly mixed $onDelete = null,
1717
public readonly string|null $columnDefinition = null,
1818
public readonly string|null $fieldName = null,

‎src/Mapping/ManyToManyOwningSideMapping.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,8 @@ public static function fromMappingArrayAndNamingStrategy(array $mappingArray, Na
127127
$mapping->joinTableColumns = [];
128128

129129
foreach ($mapping->joinTable->joinColumns as $joinColumn) {
130+
$joinColumn->nullable = false;
131+
130132
if (empty($joinColumn->referencedColumnName)) {
131133
$joinColumn->referencedColumnName = $namingStrategy->referenceColumnName();
132134
}
@@ -150,6 +152,8 @@ public static function fromMappingArrayAndNamingStrategy(array $mappingArray, Na
150152
}
151153

152154
foreach ($mapping->joinTable->inverseJoinColumns as $inverseJoinColumn) {
155+
$inverseJoinColumn->nullable = false;
156+
153157
if (empty($inverseJoinColumn->referencedColumnName)) {
154158
$inverseJoinColumn->referencedColumnName = $namingStrategy->referenceColumnName();
155159
}

‎src/Mapping/ToOneOwningSideMapping.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,12 @@ public static function fromMappingArrayAndName(
130130
$uniqueConstraintColumns = [];
131131

132132
foreach ($mapping->joinColumns as $joinColumn) {
133+
if ($mapping->id) {
134+
$joinColumn->nullable = false;
135+
} elseif ($joinColumn->nullable === null) {
136+
$joinColumn->nullable = true;
137+
}
138+
133139
if ($mapping->isOneToOne() && ! $isInheritanceTypeSingleTable) {
134140
if (count($mapping->joinColumns) === 1) {
135141
if (empty($mapping->id)) {

‎tests/Tests/ORM/Mapping/AttributeDriverTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public function testManyToManyAssociationWithNestedJoinColumns(): void
6666
'name' => 'assoz_id',
6767
'referencedColumnName' => 'assoz_id',
6868
'unique' => false,
69-
'nullable' => true,
69+
'nullable' => null,
7070
'onDelete' => null,
7171
'columnDefinition' => null,
7272
]),
@@ -80,7 +80,7 @@ public function testManyToManyAssociationWithNestedJoinColumns(): void
8080
'name' => 'inverse_assoz_id',
8181
'referencedColumnName' => 'inverse_assoz_id',
8282
'unique' => false,
83-
'nullable' => true,
83+
'nullable' => null,
8484
'onDelete' => null,
8585
'columnDefinition' => null,
8686
]),

‎tests/Tests/ORM/Mapping/ClassMetadataBuilderTest.php‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ public function testCreateManyToOneWithIdentity(): void
364364
[
365365
'name' => 'group_id',
366366
'referencedColumnName' => 'id',
367-
'nullable' => true,
367+
'nullable' => false,
368368
'unique' => false,
369369
'onDelete' => 'CASCADE',
370370
'columnDefinition' => null,
@@ -469,7 +469,7 @@ public function testCreateOneToOneWithIdentity(): void
469469
[
470470
'name' => 'group_id',
471471
'referencedColumnName' => 'id',
472-
'nullable' => true,
472+
'nullable' => false,
473473
'unique' => false,
474474
'onDelete' => 'CASCADE',
475475
'columnDefinition' => null,
@@ -539,7 +539,7 @@ public function testCreateManyToMany(): void
539539
[
540540
'name' => 'group_id',
541541
'referencedColumnName' => 'id',
542-
'nullable' => true,
542+
'nullable' => false,
543543
'unique' => false,
544544
'onDelete' => 'CASCADE',
545545
'columnDefinition' => null,
@@ -551,7 +551,7 @@ public function testCreateManyToMany(): void
551551
[
552552
'name' => 'user_id',
553553
'referencedColumnName' => 'id',
554-
'nullable' => true,
554+
'nullable' => false,
555555
'unique' => false,
556556
'onDelete' => null,
557557
'columnDefinition' => null,
@@ -742,7 +742,7 @@ public function testOrphanRemovalOnManyToMany(): void
742742
0 => [
743743
'name' => 'group_id',
744744
'referencedColumnName' => 'id',
745-
'nullable' => true,
745+
'nullable' => false,
746746
'unique' => false,
747747
'onDelete' => 'CASCADE',
748748
'columnDefinition' => null,

‎tests/Tests/ORM/Mapping/ManyToManyOwningSideMappingTest.php‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@
44

55
namespace Doctrine\Tests\ORM\Mapping;
66

7+
use Doctrine\ORM\Mapping\DefaultNamingStrategy;
78
use Doctrine\ORM\Mapping\JoinTableMapping;
89
use Doctrine\ORM\Mapping\ManyToManyOwningSideMapping;
10+
use PHPUnit\Framework\Attributes\DataProvider;
911
use PHPUnit\Framework\TestCase;
1012

1113
use function assert;
@@ -35,4 +37,57 @@ public function testItSurvivesSerialization(): void
3537
self::assertSame(['foo' => 'bar'], $resurrectedMapping->relationToSourceKeyColumns);
3638
self::assertSame(['bar' => 'baz'], $resurrectedMapping->relationToTargetKeyColumns);
3739
}
40+
41+
#[DataProvider('mappingsProvider')]
42+
public function testNullableDefaults(bool $expectedValue, ManyToManyOwningSideMapping $mapping): void
43+
{
44+
foreach ($mapping->joinTable->joinColumns as $joinColumn) {
45+
self::assertSame($expectedValue, $joinColumn->nullable);
46+
}
47+
}
48+
49+
/** @return iterable<string, array{bool, ManyToManyOwningSideMapping}> */
50+
public static function mappingsProvider(): iterable
51+
{
52+
$namingStrategy = new DefaultNamingStrategy();
53+
54+
yield 'defaults to false' => [
55+
false,
56+
ManyToManyOwningSideMapping::fromMappingArrayAndNamingStrategy([
57+
'fieldName' => 'foo',
58+
'sourceEntity' => self::class,
59+
'targetEntity' => self::class,
60+
'isOwningSide' => true,
61+
'joinTable' => [
62+
'name' => 'bar',
63+
'joinColumns' => [
64+
['name' => 'bar_id', 'referencedColumnName' => 'id'],
65+
],
66+
'inverseJoinColumns' => [
67+
['name' => 'foo_id', 'referencedColumnName' => 'id'],
68+
],
69+
],
70+
], $namingStrategy),
71+
];
72+
73+
yield 'explicitly marked as nullable' => [
74+
false, // user's intent is ignored at the ORM level
75+
ManyToManyOwningSideMapping::fromMappingArrayAndNamingStrategy([
76+
'fieldName' => 'foo',
77+
'sourceEntity' => self::class,
78+
'targetEntity' => self::class,
79+
'isOwningSide' => true,
80+
'joinTable' => [
81+
'name' => 'bar',
82+
'joinColumns' => [
83+
['name' => 'bar_id', 'referencedColumnName' => 'id', 'nullable' => true],
84+
],
85+
'inverseJoinColumns' => [
86+
['name' => 'foo_id', 'referencedColumnName' => 'id', 'nullable' => true],
87+
],
88+
],
89+
'id' => true,
90+
], $namingStrategy),
91+
];
92+
}
3893
}

‎tests/Tests/ORM/Mapping/ManyToOneAssociationMappingTest.php‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@
44

55
namespace Doctrine\Tests\ORM\Mapping;
66

7+
use Doctrine\ORM\Mapping\DefaultNamingStrategy;
78
use Doctrine\ORM\Mapping\JoinColumnMapping;
89
use Doctrine\ORM\Mapping\ManyToOneAssociationMapping;
10+
use PHPUnit\Framework\Attributes\DataProvider;
911
use PHPUnit\Framework\TestCase;
1012

1113
use function assert;
@@ -35,4 +37,60 @@ public function testItSurvivesSerialization(): void
3537
self::assertSame(['foo' => 'bar'], $resurrectedMapping->sourceToTargetKeyColumns);
3638
self::assertSame(['bar' => 'foo'], $resurrectedMapping->targetToSourceKeyColumns);
3739
}
40+
41+
#[DataProvider('mappingsProvider')]
42+
public function testNullableDefaults(bool $expectedValue, ManyToOneAssociationMapping $mapping): void
43+
{
44+
foreach ($mapping->joinColumns as $joinColumn) {
45+
self::assertSame($expectedValue, $joinColumn->nullable);
46+
}
47+
}
48+
49+
/** @return iterable<string, array{bool, ManyToOneAssociationMapping}> */
50+
public static function mappingsProvider(): iterable
51+
{
52+
$namingStrategy = new DefaultNamingStrategy();
53+
54+
yield 'not part of the identifier' => [
55+
true,
56+
ManyToOneAssociationMapping::fromMappingArrayAndName([
57+
'fieldName' => 'foo',
58+
'sourceEntity' => self::class,
59+
'targetEntity' => self::class,
60+
'isOwningSide' => true,
61+
'joinColumns' => [
62+
['name' => 'foo_id', 'referencedColumnName' => 'id'],
63+
],
64+
'id' => false,
65+
], $namingStrategy, self::class, null, false),
66+
];
67+
68+
yield 'part of the identifier' => [
69+
false,
70+
ManyToOneAssociationMapping::fromMappingArrayAndName([
71+
'fieldName' => 'foo',
72+
'sourceEntity' => self::class,
73+
'targetEntity' => self::class,
74+
'isOwningSide' => true,
75+
'joinColumns' => [
76+
['name' => 'foo_id', 'referencedColumnName' => 'id'],
77+
],
78+
'id' => true,
79+
], $namingStrategy, self::class, null, false),
80+
];
81+
82+
yield 'part of the identifier, but explicitly marked as nullable' => [
83+
false, // user's intent is ignored at the ORM level
84+
ManyToOneAssociationMapping::fromMappingArrayAndName([
85+
'fieldName' => 'foo',
86+
'sourceEntity' => self::class,
87+
'targetEntity' => self::class,
88+
'isOwningSide' => true,
89+
'joinColumns' => [
90+
['name' => 'foo_id', 'referencedColumnName' => 'id', 'nullable' => true],
91+
],
92+
'id' => true,
93+
], $namingStrategy, self::class, null, false),
94+
];
95+
}
3896
}

‎tests/Tests/ORM/Mapping/OneToOneOwningSideMappingTest.php‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@
44

55
namespace Doctrine\Tests\ORM\Mapping;
66

7+
use Doctrine\ORM\Mapping\DefaultNamingStrategy;
78
use Doctrine\ORM\Mapping\JoinColumnMapping;
89
use Doctrine\ORM\Mapping\OneToOneOwningSideMapping;
10+
use PHPUnit\Framework\Attributes\DataProvider;
911
use PHPUnit\Framework\TestCase;
1012

1113
use function assert;
@@ -35,4 +37,60 @@ public function testItSurvivesSerialization(): void
3537
self::assertSame(['foo' => 'bar'], $resurrectedMapping->sourceToTargetKeyColumns);
3638
self::assertSame(['bar' => 'foo'], $resurrectedMapping->targetToSourceKeyColumns);
3739
}
40+
41+
#[DataProvider('mappingsProvider')]
42+
public function testNullableDefaults(bool $expectedValue, OneToOneOwningSideMapping $mapping): void
43+
{
44+
foreach ($mapping->joinColumns as $joinColumn) {
45+
self::assertSame($expectedValue, $joinColumn->nullable);
46+
}
47+
}
48+
49+
/** @return iterable<string, array{bool, OneToOneOwningSideMapping}> */
50+
public static function mappingsProvider(): iterable
51+
{
52+
$namingStrategy = new DefaultNamingStrategy();
53+
54+
yield 'not part of the identifier' => [
55+
true,
56+
OneToOneOwningSideMapping::fromMappingArrayAndName([
57+
'fieldName' => 'foo',
58+
'sourceEntity' => self::class,
59+
'targetEntity' => self::class,
60+
'isOwningSide' => true,
61+
'joinColumns' => [
62+
['name' => 'foo_id', 'referencedColumnName' => 'id'],
63+
],
64+
'id' => false,
65+
], $namingStrategy, self::class, null, false),
66+
];
67+
68+
yield 'part of the identifier' => [
69+
false,
70+
OneToOneOwningSideMapping::fromMappingArrayAndName([
71+
'fieldName' => 'foo',
72+
'sourceEntity' => self::class,
73+
'targetEntity' => self::class,
74+
'isOwningSide' => true,
75+
'joinColumns' => [
76+
['name' => 'foo_id', 'referencedColumnName' => 'id'],
77+
],
78+
'id' => true,
79+
], $namingStrategy, self::class, null, false),
80+
];
81+
82+
yield 'part of the identifier, but explicitly marked as nullable' => [
83+
false, // user's intent ignored at the ORM level
84+
OneToOneOwningSideMapping::fromMappingArrayAndName([
85+
'fieldName' => 'foo',
86+
'sourceEntity' => self::class,
87+
'targetEntity' => self::class,
88+
'isOwningSide' => true,
89+
'joinColumns' => [
90+
['name' => 'foo_id', 'referencedColumnName' => 'id', 'nullable' => true],
91+
],
92+
'id' => true,
93+
], $namingStrategy, self::class, null, false),
94+
];
95+
}
3896
}

0 commit comments

Comments
 (0)