Skip to content

Commit ce844d9

Browse files
greg0iregreg0ire
authored
Merge pull request #12126 from greg0ire/depr-nullable-prim-keys
Deprecate specifying nullable on primary key columns
2 parents c1f7a60 + d1d13d5 commit ce844d9

21 files changed

+386
-48
lines changed

‎UPGRADE.md‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,39 @@
11
# Upgrade to 3.6
22

3+
## Deprecate specifying `nullable` on columns that end up being used in a primary key
4+
5+
Specifying `nullable` on join columns that are part of a primary key is
6+
deprecated and will be an error in 4.0.
7+
8+
This can happen when using a join column mapping together with an id mapping,
9+
or when using a join column mapping or an inverse join column mapping on a
10+
many-to-many relationship.
11+
12+
```diff
13+
class User
14+
{
15+
#[ORM\Id]
16+
#[ORM\Column(type: 'integer')]
17+
private int $id;
18+
19+
#[ORM\Id]
20+
#[ORM\ManyToOne(targetEntity: Family::class, inversedBy: 'users')]
21+
- #[ORM\JoinColumn(name: 'family_id', referencedColumnName: 'id', nullable: true)]
22+
+ #[ORM\JoinColumn(name: 'family_id', referencedColumnName: 'id')]
23+
private ?Family $family;
24+
25+
#[ORM\ManyToMany(targetEntity: Group::class)]
26+
#[ORM\JoinTable(name: 'user_group')]
27+
- #[ORM\JoinColumn(name: 'user_id', referencedColumnName: 'id', nullable: true)]
28+
- #[ORM\InverseJoinColumn(name: 'group_id', referencedColumnName: 'id', nullable: true)]
29+
+ #[ORM\JoinColumn(name: 'user_id', referencedColumnName: 'id')]
30+
+ #[ORM\InverseJoinColumn(name: 'group_id', referencedColumnName: 'id')]
31+
private Collection $groups;
32+
}
33+
```
34+
35+
## Deprecate `Doctrine\ORM\QueryBuilder::add('join', ...)` with a list of join parts
36+
337
Using `Doctrine\ORM\QueryBuilder::add('join', ...)` with a list of join parts
438
is deprecated in favor of using an associative array of join parts with the
539
root alias as key.

‎src/Mapping/Builder/AssociationBuilder.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,10 @@ public function addJoinColumn(
113113
string|null $onDelete = null,
114114
string|null $columnDef = null,
115115
): static {
116+
if ($this->mapping['id'] ?? false) {
117+
$nullable = null;
118+
}
119+
116120
$this->joinColumns[] = [
117121
'name' => $columnName,
118122
'referencedColumnName' => $referencedColumnName,
@@ -133,6 +137,9 @@ public function addJoinColumn(
133137
public function makePrimaryKey(): static
134138
{
135139
$this->mapping['id'] = true;
140+
foreach ($this->joinColumns ?? [] as $i => $joinColumn) {
141+
$this->joinColumns[$i]['nullable'] = null;
142+
}
136143

137144
return $this;
138145
}

‎src/Mapping/Builder/ManyToManyAssociationBuilder.php‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,30 @@ public function setJoinTable(string $name): static
2424
return $this;
2525
}
2626

27+
/**
28+
* Add Join Columns.
29+
*
30+
* @return $this
31+
*/
32+
public function addJoinColumn(
33+
string $columnName,
34+
string $referencedColumnName,
35+
bool $nullable = true,
36+
bool $unique = false,
37+
string|null $onDelete = null,
38+
string|null $columnDef = null,
39+
): static {
40+
$this->joinColumns[] = [
41+
'name' => $columnName,
42+
'referencedColumnName' => $referencedColumnName,
43+
'unique' => $unique,
44+
'onDelete' => $onDelete,
45+
'columnDefinition' => $columnDef,
46+
];
47+
48+
return $this;
49+
}
50+
2751
/**
2852
* Adds Inverse Join Columns.
2953
*
@@ -40,7 +64,6 @@ public function addInverseJoinColumn(
4064
$this->inverseJoinColumns[] = [
4165
'name' => $columnName,
4266
'referencedColumnName' => $referencedColumnName,
43-
'nullable' => $nullable,
4467
'unique' => $unique,
4568
'onDelete' => $onDelete,
4669
'columnDefinition' => $columnDef,

‎src/Mapping/JoinTableMapping.php‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,14 @@ public function offsetSet(mixed $offset, mixed $value): void
8484
/** @return mixed[] */
8585
public function toArray(): array
8686
{
87-
$array = (array) $this;
87+
$array = (array) $this;
88+
$toArray = static function (JoinColumnMapping $column) {
89+
$array = (array) $column;
8890

89-
$toArray = static fn (JoinColumnMapping $column): array => (array) $column;
91+
unset($array['nullable']);
92+
93+
return $array;
94+
};
9095
$array['joinColumns'] = array_map($toArray, $array['joinColumns']);
9196
$array['inverseJoinColumns'] = array_map($toArray, $array['inverseJoinColumns']);
9297

‎src/Mapping/ManyToManyOwningSideMapping.php‎

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

55
namespace Doctrine\ORM\Mapping;
66

7+
use Doctrine\Deprecations\Deprecation;
8+
79
use function strtolower;
810
use function trim;
911

@@ -127,6 +129,20 @@ public static function fromMappingArrayAndNamingStrategy(array $mappingArray, Na
127129
$mapping->joinTableColumns = [];
128130

129131
foreach ($mapping->joinTable->joinColumns as $joinColumn) {
132+
if ($joinColumn->nullable !== null) {
133+
Deprecation::trigger(
134+
'doctrine/orm',
135+
'https://github/doctrine/orm/pull/12125',
136+
<<<'DEPRECATION'
137+
Specifying the "nullable" attribute for join columns in many-to-many associations (here, %s::$%s) is a no-op.
138+
The ORM will always set it to false.
139+
Doing so is deprecated and will be an error in 4.0.
140+
DEPRECATION,
141+
$mapping->sourceEntity,
142+
$mapping->fieldName,
143+
);
144+
}
145+
130146
$joinColumn->nullable = false;
131147

132148
if (empty($joinColumn->referencedColumnName)) {
@@ -152,6 +168,20 @@ public static function fromMappingArrayAndNamingStrategy(array $mappingArray, Na
152168
}
153169

154170
foreach ($mapping->joinTable->inverseJoinColumns as $inverseJoinColumn) {
171+
if ($inverseJoinColumn->nullable !== null) {
172+
Deprecation::trigger(
173+
'doctrine/orm',
174+
'https://github/doctrine/orm/pull/12125',
175+
<<<'DEPRECATION'
176+
Specifying the "nullable" attribute for join columns in many-to-many associations (here, %s::$%s) is a no-op.
177+
The ORM will always set it to false.
178+
Doing so is deprecated and will be an error in 4.0.
179+
DEPRECATION,
180+
$mapping->targetEntity,
181+
$mapping->fieldName,
182+
);
183+
}
184+
155185
$inverseJoinColumn->nullable = false;
156186

157187
if (empty($inverseJoinColumn->referencedColumnName)) {

‎src/Mapping/ToOneOwningSideMapping.php‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace Doctrine\ORM\Mapping;
66

7+
use Doctrine\Deprecations\Deprecation;
78
use RuntimeException;
89

910
use function array_flip;
@@ -131,6 +132,20 @@ public static function fromMappingArrayAndName(
131132

132133
foreach ($mapping->joinColumns as $joinColumn) {
133134
if ($mapping->id) {
135+
if ($joinColumn->nullable !== null) {
136+
Deprecation::trigger(
137+
'doctrine/orm',
138+
'https://github/doctrine/orm/pull/12125',
139+
<<<'DEPRECATION'
140+
Specifying the "nullable" attribute for join columns in to-one associations (here, %s::$%s) that are part of the identifier is a no-op.
141+
The ORM will always set it to false.
142+
Doing so is deprecated and will be an error in 4.0.
143+
DEPRECATION,
144+
$mapping->sourceEntity,
145+
$mapping->fieldName,
146+
);
147+
}
148+
134149
$joinColumn->nullable = false;
135150
} elseif ($joinColumn->nullable === null) {
136151
$joinColumn->nullable = true;
@@ -200,7 +215,12 @@ public function toArray(): array
200215

201216
$joinColumns = [];
202217
foreach ($array['joinColumns'] as $column) {
203-
$joinColumns[] = (array) $column;
218+
$columnArray = (array) $column;
219+
if ($this->id) {
220+
unset($columnArray['nullable']);
221+
}
222+
223+
$joinColumns[] = $columnArray;
204224
}
205225

206226
$array['joinColumns'] = $joinColumns;

‎tests/Tests/Models/DDC3579/DDC3579Admin.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,14 @@
66

77
use Doctrine\ORM\Mapping\AssociationOverride;
88
use Doctrine\ORM\Mapping\AssociationOverrides;
9+
use Doctrine\ORM\Mapping\ClassMetadata;
910
use Doctrine\ORM\Mapping\Entity;
1011

1112
#[Entity]
1213
#[AssociationOverrides([new AssociationOverride(name: 'groups', inversedBy: 'admins')])]
1314
class DDC3579Admin extends DDC3579User
1415
{
15-
public static function loadMetadata($metadata): void
16+
public static function loadMetadata(ClassMetadata $metadata): void
1617
{
1718
$metadata->setAssociationOverride('groups', ['inversedBy' => 'admins']);
1819
}

‎tests/Tests/Models/DDC3579/DDC3579User.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public function getGroups(): ArrayCollection
5858
return $this->groups;
5959
}
6060

61-
public static function loadMetadata($metadata): void
61+
public static function loadMetadata(ClassMetadata $metadata): void
6262
{
6363
$metadata->isMappedSuperclass = true;
6464

‎tests/Tests/Models/GH10334/GH10334Foo.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ class GH10334Foo
1616
{
1717
#[Id]
1818
#[ManyToOne(targetEntity: GH10334FooCollection::class, inversedBy: 'foos')]
19-
#[JoinColumn(name: 'foo_collection_id', referencedColumnName: 'id', nullable: false)]
19+
#[JoinColumn(name: 'foo_collection_id', referencedColumnName: 'id')]
2020
#[GeneratedValue]
2121
protected GH10334FooCollection $collection;
2222

‎tests/Tests/Models/OneToOneInverseSideWithAssociativeIdLoad/InverseSide.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ class InverseSide
1717
/** Associative id (owning identifier) */
1818
#[Id]
1919
#[OneToOne(targetEntity: InverseSideIdTarget::class, inversedBy: 'inverseSide')]
20-
#[JoinColumn(nullable: false, name: 'associativeId')]
20+
#[JoinColumn(name: 'associativeId')]
2121
public InverseSideIdTarget $associativeId;
2222

2323
#[OneToOne(targetEntity: OwningSide::class, mappedBy: 'inverse')]

0 commit comments

Comments
 (0)