From ad23675732bb6ac747c3fc20f69e9c656006f307 Mon Sep 17 00:00:00 2001 From: Pirmin Mattmann Date: Sun, 18 Jul 2021 18:30:06 +0200 Subject: [PATCH 01/23] Embedded Collections camps/periods/days --- api/src/Entity/BaseEntity.php | 2 ++ api/src/Entity/Camp.php | 44 ++++++++++++++++++++++++----------- api/src/Entity/Day.php | 13 +++++++++-- api/src/Entity/Period.php | 34 +++++++++++++++++---------- 4 files changed, 65 insertions(+), 28 deletions(-) diff --git a/api/src/Entity/BaseEntity.php b/api/src/Entity/BaseEntity.php index c811ac8c4a..c88253afff 100644 --- a/api/src/Entity/BaseEntity.php +++ b/api/src/Entity/BaseEntity.php @@ -7,6 +7,7 @@ use DateTime; use Doctrine\ORM\Mapping as ORM; use Gedmo\Mapping\Annotation as Gedmo; +use Symfony\Component\Serializer\Annotation\Groups; /** * @ORM\MappedSuperclass @@ -25,6 +26,7 @@ abstract class BaseEntity { * @ORM\CustomIdGenerator(class=IdGenerator::class) */ #[ApiProperty(writable: false, example: '1a2b3c4d')] + #[Groups(['Properties:read'])] protected string $id; /** diff --git a/api/src/Entity/Camp.php b/api/src/Entity/Camp.php index ca30302c84..000baeb15c 100644 --- a/api/src/Entity/Camp.php +++ b/api/src/Entity/Camp.php @@ -25,7 +25,11 @@ 'post' => [ 'security' => 'is_fully_authenticated()', 'input_formats' => ['jsonld', 'jsonapi', 'json'], - 'validation_groups' => ['Default', 'camp:create'], + 'validation_groups' => ['Default', 'Camp:create'], + 'denormalization_context' => [ + 'groups' => ['Properties:write', 'Camp:create'], + 'allow_extra_attributes' => false, + ], ], ], itemOperations: [ @@ -33,12 +37,20 @@ 'patch' => [ 'security' => 'object.owner == user or is_granted("ROLE_ADMIN")', 'denormalization_context' => [ - 'groups' => ['camp:update'], + 'groups' => ['Properties:write', 'Camp:update'], 'allow_extra_attributes' => false, ], ], 'delete' => ['security' => 'object.owner == user or is_granted("ROLE_ADMIN")'], - ] + ], + normalizationContext: [ + 'groups' => ['Properties:read', 'Camp:read'], + 'allow_extra_attributes' => false, + ], + denormalizationContext: [ + 'groups' => ['Properties:write', 'Camp:write'], + 'allow_extra_attributes' => false, + ], )] class Camp extends BaseEntity implements BelongsToCampInterface { /** @@ -56,9 +68,13 @@ class Camp extends BaseEntity implements BelongsToCampInterface { * @ORM\OrderBy({"start": "ASC"}) */ #[Assert\Valid] - #[Assert\Count(min: 1, groups: ['camp:create'])] - #[ApiProperty(writableLink: true, example: '[{ "description": "Hauptlager", "start": "2022-01-01", "end": "2022-01-08" }]')] - #[Groups(['Default'])] + #[Assert\Count(min: 1, groups: ['Camp:create'])] + #[ApiProperty( + readableLink: true, + writableLink: true, + example: '[{ "description": "Hauptlager", "start": "2022-01-01", "end": "2022-01-08" }]' + )] + #[Groups(['Camp:read', 'Camp:create'])] public Collection $periods; /** @@ -116,7 +132,7 @@ class Camp extends BaseEntity implements BelongsToCampInterface { #[InputFilter\CleanHTML] #[Assert\NotBlank] #[ApiProperty(example: 'SoLa 2022')] - #[Groups(['Default', 'camp:update'])] + #[Groups(['Properties:read', 'Camp:create'])] public string $name; /** @@ -128,8 +144,8 @@ class Camp extends BaseEntity implements BelongsToCampInterface { #[InputFilter\CleanHTML] #[Assert\NotBlank] #[Assert\Length(max: 32)] - #[ApiProperty(example: 'Abteilungs-Sommerlager 2022')] - #[Groups(['Default', 'camp:update'])] + #[ApiProperty(writable: true, example: 'Abteilungs-Sommerlager 2022')] + #[Groups(['Properties:read', 'Properties:write'])] public string $title; /** @@ -141,7 +157,7 @@ class Camp extends BaseEntity implements BelongsToCampInterface { #[InputFilter\CleanHTML] #[Assert\Length(max: 128)] #[ApiProperty(example: 'Piraten')] - #[Groups(['Default', 'camp:update'])] + #[Groups(['Properties:read', 'Properties:write'])] public ?string $motto = null; /** @@ -153,7 +169,7 @@ class Camp extends BaseEntity implements BelongsToCampInterface { #[InputFilter\CleanHTML] #[Assert\Length(max: 128)] #[ApiProperty(example: 'Wiese hinter der alten Mühle')] - #[Groups(['Default', 'camp:update'])] + #[Groups(['Properties:read', 'Properties:write'])] public ?string $addressName = null; /** @@ -165,7 +181,7 @@ class Camp extends BaseEntity implements BelongsToCampInterface { #[InputFilter\CleanHTML] #[Assert\Length(max: 128)] #[ApiProperty(example: 'Schönriedweg 23')] - #[Groups(['Default', 'camp:update'])] + #[Groups(['Properties:read', 'Properties:write'])] public ?string $addressStreet = null; /** @@ -177,7 +193,7 @@ class Camp extends BaseEntity implements BelongsToCampInterface { #[InputFilter\CleanHTML] #[Assert\Length(max: 128)] #[ApiProperty(example: '1234')] - #[Groups(['Default', 'camp:update'])] + #[Groups(['Properties:read', 'Properties:write'])] public ?string $addressZipcode = null; /** @@ -189,7 +205,7 @@ class Camp extends BaseEntity implements BelongsToCampInterface { #[InputFilter\CleanHTML] #[Assert\Length(max: 128)] #[ApiProperty(example: 'Hintertüpfingen')] - #[Groups(['Default', 'camp:update'])] + #[Groups(['Properties:read', 'Properties:write'])] public ?string $addressCity = null; /** diff --git a/api/src/Entity/Day.php b/api/src/Entity/Day.php index e8e8e1272c..a37133de6e 100644 --- a/api/src/Entity/Day.php +++ b/api/src/Entity/Day.php @@ -10,6 +10,7 @@ use Doctrine\Common\Collections\Collection; use Doctrine\ORM\Mapping as ORM; use Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntity; +use Symfony\Component\Serializer\Annotation\Groups; use Symfony\Component\Serializer\Annotation\SerializedName; /** @@ -26,6 +27,10 @@ #[ApiResource( collectionOperations: ['get'], itemOperations: ['get'], + normalizationContext: [ + 'groups' => ['Properties:read', 'Day:read'], + 'allow_extra_attributes' => false, + ], )] #[ApiFilter(SearchFilter::class, properties: ['period'])] #[UniqueEntity(fields: ['period', 'dayOffset'])] @@ -44,7 +49,8 @@ class Day extends BaseEntity implements BelongsToCampInterface { * @ORM\ManyToOne(targetEntity="Period", inversedBy="days") * @ORM\JoinColumn(nullable=false, onDelete="cascade") */ - #[ApiProperty(writable: false, example: '/periods/1a2b3c4d')] + #[ApiProperty(readableLink:false, writable: false, example: '/periods/1a2b3c4d')] + #[Groups(['Properties:read'])] public ?Period $period = null; /** @@ -53,13 +59,15 @@ class Day extends BaseEntity implements BelongsToCampInterface { * @ORM\Column(type="integer") */ #[ApiProperty(writable: false, example: '1')] + #[Groups(['Properties:read'])] public int $dayOffset = 0; public function __construct() { $this->dayResponsibles = new ArrayCollection(); } - #[ApiProperty(readable: false)] + #[ApiProperty(readableLink:false, writable: false, example: '/camps/1a2b3c4d')] + #[Groups(['Properties:read'])] public function getCamp(): ?Camp { return $this->period?->camp; } @@ -69,6 +77,7 @@ public function getCamp(): ?Camp { */ #[ApiProperty(example: '2')] #[SerializedName('number')] + #[Groups(['Properties:read'])] public function getDayNumber(): int { return $this->dayOffset + 1; } diff --git a/api/src/Entity/Period.php b/api/src/Entity/Period.php index 70fed6c882..1525156321 100644 --- a/api/src/Entity/Period.php +++ b/api/src/Entity/Period.php @@ -6,6 +6,7 @@ use ApiPlatform\Core\Annotation\ApiProperty; use ApiPlatform\Core\Annotation\ApiResource; use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\SearchFilter; +use ApiPlatform\Core\Annotation\ApiSubresource; use DateTimeInterface; use Doctrine\Common\Collections\ArrayCollection; use Doctrine\Common\Collections\Collection; @@ -20,15 +21,23 @@ * @ORM\Entity() */ #[ApiResource( - collectionOperations: ['get', 'post'], + collectionOperations: [ + 'get', + 'post' + ], itemOperations: [ 'get', - 'patch' => ['denormalization_context' => [ - 'groups' => ['period:update'], - 'allow_extra_attributes' => false, - ]], + 'patch', 'delete', - ] + ], + normalizationContext: [ + 'groups' => ['Properties:read', 'Period:read'], + 'allow_extra_attributes' => false, + ], + denormalizationContext: [ + 'groups' => ['Properties:write', 'Period:write'], + 'allow_extra_attributes' => false, + ], )] #[ApiFilter(SearchFilter::class, properties: ['camp'])] class Period extends BaseEntity implements BelongsToCampInterface { @@ -38,7 +47,8 @@ class Period extends BaseEntity implements BelongsToCampInterface { * @ORM\OneToMany(targetEntity="Day", mappedBy="period", orphanRemoval=true) * @ORM\OrderBy({"dayOffset": "ASC"}) */ - #[ApiProperty(writable: false, example: '["/days/1a2b3c4d"]')] + #[ApiProperty(readableLink:true, writable: false, example: '["/days/1a2b3c4d"]')] + #[Groups(['Camp:read', 'Period:read'])] public Collection $days; /** @@ -66,8 +76,8 @@ class Period extends BaseEntity implements BelongsToCampInterface { * @ORM\ManyToOne(targetEntity="Camp", inversedBy="periods") * @ORM\JoinColumn(nullable=false) */ - #[ApiProperty(example: '/camps/1a2b3c4d')] - #[Groups(['Default'])] + #[ApiProperty(readableLink:false, example: '/camps/1a2b3c4d')] + #[Groups(['Properties:read'])] public ?Camp $camp = null; /** @@ -79,7 +89,7 @@ class Period extends BaseEntity implements BelongsToCampInterface { */ #[Assert\NotBlank] #[ApiProperty(example: 'Hauptlager')] - #[Groups(['Default', 'period:update'])] + #[Groups(['Properties:read', 'Properties:write'])] public ?string $description = null; /** @@ -95,7 +105,7 @@ class Period extends BaseEntity implements BelongsToCampInterface { */ #[Assert\LessThanOrEqual(propertyPath: 'end')] #[ApiProperty(example: '2022-01-01', openapiContext: ['format' => 'date'])] - #[Groups(['Default', 'period:update'])] + #[Groups(['Properties:read', 'Properties:write'])] public ?DateTimeInterface $start = null; /** @@ -106,7 +116,7 @@ class Period extends BaseEntity implements BelongsToCampInterface { */ #[Assert\GreaterThanOrEqual(propertyPath: 'start')] #[ApiProperty(example: '2022-01-08', openapiContext: ['format' => 'date'])] - #[Groups(['Default', 'period:update'])] + #[Groups(['Properties:read', 'Properties:write'])] public ?DateTimeInterface $end = null; public function __construct() { From 7d76aa2183b3c493c813ef25e4243f58213b58e6 Mon Sep 17 00:00:00 2001 From: Pirmin Mattmann Date: Wed, 28 Jul 2021 19:24:22 +0200 Subject: [PATCH 02/23] Add Camp:write --- api/src/Entity/Camp.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/Entity/Camp.php b/api/src/Entity/Camp.php index 5ba5475fa2..553de40154 100644 --- a/api/src/Entity/Camp.php +++ b/api/src/Entity/Camp.php @@ -27,7 +27,7 @@ 'input_formats' => ['jsonld', 'jsonapi', 'json'], 'validation_groups' => ['Default', 'Camp:create'], 'denormalization_context' => [ - 'groups' => ['Properties:write', 'Camp:create'], + 'groups' => ['Properties:write', 'Camp:write', 'Camp:create'], 'allow_extra_attributes' => false, ], ], @@ -37,7 +37,7 @@ 'patch' => [ 'security' => 'object.owner == user or is_granted("ROLE_ADMIN")', 'denormalization_context' => [ - 'groups' => ['Properties:write', 'Camp:update'], + 'groups' => ['Properties:write', 'Camp:write', 'Camp:update'], 'allow_extra_attributes' => false, ], ], From 761acd0253bcc21b1cb24ffce1579dd2045c83eb Mon Sep 17 00:00:00 2001 From: Pirmin Mattmann Date: Thu, 29 Jul 2021 22:49:22 +0200 Subject: [PATCH 03/23] selectively embed referenced entitiy --- api/src/Entity/BaseEntity.php | 2 +- api/src/Entity/Camp.php | 46 +++++++++++++++++++++++------------ api/src/Entity/Day.php | 14 +++++++---- api/src/Entity/Period.php | 39 ++++++++++++++++++++++++----- 4 files changed, 74 insertions(+), 27 deletions(-) diff --git a/api/src/Entity/BaseEntity.php b/api/src/Entity/BaseEntity.php index c88253afff..a77a5bc36b 100644 --- a/api/src/Entity/BaseEntity.php +++ b/api/src/Entity/BaseEntity.php @@ -26,7 +26,7 @@ abstract class BaseEntity { * @ORM\CustomIdGenerator(class=IdGenerator::class) */ #[ApiProperty(writable: false, example: '1a2b3c4d')] - #[Groups(['Properties:read'])] + #[Groups(['read'])] protected string $id; /** diff --git a/api/src/Entity/Camp.php b/api/src/Entity/Camp.php index 553de40154..689466c0ea 100644 --- a/api/src/Entity/Camp.php +++ b/api/src/Entity/Camp.php @@ -27,28 +27,34 @@ 'input_formats' => ['jsonld', 'jsonapi', 'json'], 'validation_groups' => ['Default', 'Camp:create'], 'denormalization_context' => [ - 'groups' => ['Properties:write', 'Camp:write', 'Camp:create'], + 'groups' => ['write', 'create', 'Camp:Periods'], 'allow_extra_attributes' => false, ], ], ], itemOperations: [ - 'get' => ['security' => 'object.owner == user or is_granted("ROLE_ADMIN")'], + 'get' => [ + 'security' => 'object.owner == user or is_granted("ROLE_ADMIN")', + 'normalization_context' => [ + 'groups' => ['read', 'Camp:Periods', 'Period:Days'], + 'allow_extra_attributes' => false, + ], + ], 'patch' => [ 'security' => 'object.owner == user or is_granted("ROLE_ADMIN")', 'denormalization_context' => [ - 'groups' => ['Properties:write', 'Camp:write', 'Camp:update'], + 'groups' => ['write', 'update'], 'allow_extra_attributes' => false, ], ], 'delete' => ['security' => 'object.owner == user or is_granted("ROLE_ADMIN")'], ], normalizationContext: [ - 'groups' => ['Properties:read', 'Camp:read'], + 'groups' => ['read'], 'allow_extra_attributes' => false, ], denormalizationContext: [ - 'groups' => ['Properties:write', 'Camp:write'], + 'groups' => ['write'], 'allow_extra_attributes' => false, ], )] @@ -70,11 +76,11 @@ class Camp extends BaseEntity implements BelongsToCampInterface { #[Assert\Valid] #[Assert\Count(min: 1, groups: ['Camp:create'])] #[ApiProperty( - readableLink: true, + readableLink: false, writableLink: true, - example: '[{ "description": "Hauptlager", "start": "2022-01-01", "end": "2022-01-08" }]' + example: '{ "href": "/periods?camp=/camps/1a2b3c4d" }', )] - #[Groups(['Camp:read', 'Camp:create'])] + #[Groups(['read', 'create'])] public Collection $periods; /** @@ -132,7 +138,7 @@ class Camp extends BaseEntity implements BelongsToCampInterface { #[InputFilter\CleanHTML] #[Assert\NotBlank] #[ApiProperty(example: 'SoLa 2022')] - #[Groups(['Properties:read', 'Camp:create'])] + #[Groups(['read', 'create'])] public string $name; /** @@ -145,7 +151,7 @@ class Camp extends BaseEntity implements BelongsToCampInterface { #[Assert\NotBlank] #[Assert\Length(max: 32)] #[ApiProperty(writable: true, example: 'Abteilungs-Sommerlager 2022')] - #[Groups(['Properties:read', 'Properties:write'])] + #[Groups(['read', 'write'])] public string $title; /** @@ -157,7 +163,7 @@ class Camp extends BaseEntity implements BelongsToCampInterface { #[InputFilter\CleanHTML] #[Assert\Length(max: 128)] #[ApiProperty(example: 'Piraten')] - #[Groups(['Properties:read', 'Properties:write'])] + #[Groups(['read', 'write'])] public ?string $motto = null; /** @@ -169,7 +175,7 @@ class Camp extends BaseEntity implements BelongsToCampInterface { #[InputFilter\CleanHTML] #[Assert\Length(max: 128)] #[ApiProperty(example: 'Wiese hinter der alten Mühle')] - #[Groups(['Properties:read', 'Properties:write'])] + #[Groups(['read', 'write'])] public ?string $addressName = null; /** @@ -181,7 +187,7 @@ class Camp extends BaseEntity implements BelongsToCampInterface { #[InputFilter\CleanHTML] #[Assert\Length(max: 128)] #[ApiProperty(example: 'Schönriedweg 23')] - #[Groups(['Properties:read', 'Properties:write'])] + #[Groups(['read', 'write'])] public ?string $addressStreet = null; /** @@ -193,7 +199,7 @@ class Camp extends BaseEntity implements BelongsToCampInterface { #[InputFilter\CleanHTML] #[Assert\Length(max: 128)] #[ApiProperty(example: '1234')] - #[Groups(['Properties:read', 'Properties:write'])] + #[Groups(['read', 'write'])] public ?string $addressZipcode = null; /** @@ -205,7 +211,7 @@ class Camp extends BaseEntity implements BelongsToCampInterface { #[InputFilter\CleanHTML] #[Assert\Length(max: 128)] #[ApiProperty(example: 'Hintertüpfingen')] - #[Groups(['Properties:read', 'Properties:write'])] + #[Groups(['read', 'write'])] public ?string $addressCity = null; /** @@ -238,6 +244,16 @@ public function __construct() { $this->materialLists = new ArrayCollection(); } + #[ApiProperty( + readableLink: true, + example: '[{ "description": "Hauptlager", "start": "2022-01-01", "end": "2022-01-08" }]' + )] + #[SerializedName('periods')] + #[Groups(['Camp:Periods'])] + public function getEmbeddedPeriods(): Collection { + return $this->periods; + } + #[ApiProperty(readable: false)] public function getCamp(): ?Camp { return $this; diff --git a/api/src/Entity/Day.php b/api/src/Entity/Day.php index 3992451c20..418ed5047b 100644 --- a/api/src/Entity/Day.php +++ b/api/src/Entity/Day.php @@ -28,7 +28,11 @@ collectionOperations: ['get'], itemOperations: ['get'], normalizationContext: [ - 'groups' => ['Properties:read', 'Day:read'], + 'groups' => ['read'], + 'allow_extra_attributes' => false, + ], + denormalizationContext: [ + 'groups' => ['write'], 'allow_extra_attributes' => false, ], )] @@ -50,7 +54,7 @@ class Day extends BaseEntity implements BelongsToCampInterface { * @ORM\JoinColumn(nullable=false, onDelete="cascade") */ #[ApiProperty(readableLink: false, writable: false, example: '/periods/1a2b3c4d')] - #[Groups(['Properties:read'])] + #[Groups(['read'])] public ?Period $period = null; /** @@ -59,7 +63,7 @@ class Day extends BaseEntity implements BelongsToCampInterface { * @ORM\Column(type="integer") */ #[ApiProperty(writable: false, example: '1')] - #[Groups(['Properties:read'])] + #[Groups(['read'])] public int $dayOffset = 0; public function __construct() { @@ -67,7 +71,7 @@ public function __construct() { } #[ApiProperty(readableLink: false, writable: false, example: '/camps/1a2b3c4d')] - #[Groups(['Properties:read'])] + #[Groups(['read'])] public function getCamp(): ?Camp { return $this->period?->camp; } @@ -77,7 +81,7 @@ public function getCamp(): ?Camp { */ #[ApiProperty(example: '2')] #[SerializedName('number')] - #[Groups(['Properties:read'])] + #[Groups(['read'])] public function getDayNumber(): int { return $this->dayOffset + 1; } diff --git a/api/src/Entity/Period.php b/api/src/Entity/Period.php index 03f1454c15..9222fa5e28 100644 --- a/api/src/Entity/Period.php +++ b/api/src/Entity/Period.php @@ -11,6 +11,7 @@ use Doctrine\Common\Collections\Collection; use Doctrine\ORM\Mapping as ORM; use Symfony\Component\Serializer\Annotation\Groups; +use Symfony\Component\Serializer\Annotation\SerializedName; use Symfony\Component\Validator\Constraints as Assert; /** @@ -25,16 +26,21 @@ 'post', ], itemOperations: [ - 'get', + 'get' => [ + 'normalization_context' => [ + 'groups' => ['read', 'Period:Camp', 'Period:Days'], + 'allow_extra_attributes' => false, + ], + ], 'patch', 'delete', ], normalizationContext: [ - 'groups' => ['Properties:read', 'Period:read'], + 'groups' => ['read'], 'allow_extra_attributes' => false, ], denormalizationContext: [ - 'groups' => ['Properties:write', 'Period:write'], + 'groups' => ['write'], 'allow_extra_attributes' => false, ], )] @@ -46,8 +52,12 @@ class Period extends BaseEntity implements BelongsToCampInterface { * @ORM\OneToMany(targetEntity="Day", mappedBy="period", orphanRemoval=true) * @ORM\OrderBy({"dayOffset": "ASC"}) */ - #[ApiProperty(readableLink: true, writable: false, example: '["/days/1a2b3c4d"]')] - #[Groups(['Camp:read', 'Period:read'])] + #[ApiProperty( + readableLink: false, + writable: false, + example: '{ "href": "/days?period=/periods/1a2b3c4d" }' + )] + #[Groups(['read'])] public Collection $days; /** @@ -76,7 +86,7 @@ class Period extends BaseEntity implements BelongsToCampInterface { * @ORM\JoinColumn(nullable=false) */ #[ApiProperty(readableLink: false, example: '/camps/1a2b3c4d')] - #[Groups(['Properties:read'])] + #[Groups(['read'])] public ?Camp $camp = null; /** @@ -124,6 +134,23 @@ public function __construct() { $this->materialItems = new ArrayCollection(); } + #[ApiProperty( + readableLink: true, + example: '[{ "dayOffset": 0, "number": 1 }]' + )] + #[SerializedName('days')] + #[Groups('Period:Days')] + public function getEmbeddedDays(): Collection { + return $this->days; + } + + #[ApiProperty(readableLink: true)] + #[SerializedName('camp')] + #[Groups(['Period:Camp'])] + public function getEmbeddedCamp(): ?Camp { + return $this->camp; + } + public function getCamp(): ?Camp { return $this->camp; } From e5d69f79232a69c3963670447b25d8df05d3fa0f Mon Sep 17 00:00:00 2001 From: Pirmin Mattmann Date: Thu, 29 Jul 2021 23:15:28 +0200 Subject: [PATCH 04/23] excample fixed --- api/src/Entity/Camp.php | 2 +- api/src/Entity/Period.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/Entity/Camp.php b/api/src/Entity/Camp.php index 689466c0ea..72134e3cd9 100644 --- a/api/src/Entity/Camp.php +++ b/api/src/Entity/Camp.php @@ -78,7 +78,7 @@ class Camp extends BaseEntity implements BelongsToCampInterface { #[ApiProperty( readableLink: false, writableLink: true, - example: '{ "href": "/periods?camp=/camps/1a2b3c4d" }', + example: '/periods?camp=/camps/1a2b3c4d', )] #[Groups(['read', 'create'])] public Collection $periods; diff --git a/api/src/Entity/Period.php b/api/src/Entity/Period.php index 9222fa5e28..27bd641463 100644 --- a/api/src/Entity/Period.php +++ b/api/src/Entity/Period.php @@ -55,7 +55,7 @@ class Period extends BaseEntity implements BelongsToCampInterface { #[ApiProperty( readableLink: false, writable: false, - example: '{ "href": "/days?period=/periods/1a2b3c4d" }' + example: '/days?period=/periods/1a2b3c4d' )] #[Groups(['read'])] public Collection $days; From a2f300768e8e6e49307e95e5d511de9d904b1b8d Mon Sep 17 00:00:00 2001 From: Pirmin Mattmann Date: Thu, 29 Jul 2021 23:48:24 +0200 Subject: [PATCH 05/23] fix post on camp --- api/src/Entity/Camp.php | 4 ++-- api/src/Entity/Period.php | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/api/src/Entity/Camp.php b/api/src/Entity/Camp.php index 72134e3cd9..ad94fad89e 100644 --- a/api/src/Entity/Camp.php +++ b/api/src/Entity/Camp.php @@ -27,7 +27,7 @@ 'input_formats' => ['jsonld', 'jsonapi', 'json'], 'validation_groups' => ['Default', 'Camp:create'], 'denormalization_context' => [ - 'groups' => ['write', 'create', 'Camp:Periods'], + 'groups' => ['write', 'create'], 'allow_extra_attributes' => false, ], ], @@ -78,7 +78,7 @@ class Camp extends BaseEntity implements BelongsToCampInterface { #[ApiProperty( readableLink: false, writableLink: true, - example: '/periods?camp=/camps/1a2b3c4d', + example: '[{ "description": "Hauptlager", "start": "2022-01-01", "end": "2022-01-08" }]', )] #[Groups(['read', 'create'])] public Collection $periods; diff --git a/api/src/Entity/Period.php b/api/src/Entity/Period.php index 27bd641463..251f64a567 100644 --- a/api/src/Entity/Period.php +++ b/api/src/Entity/Period.php @@ -98,7 +98,7 @@ class Period extends BaseEntity implements BelongsToCampInterface { */ #[Assert\NotBlank] #[ApiProperty(example: 'Hauptlager')] - #[Groups(['Properties:read', 'Properties:write'])] + #[Groups(['read', 'write'])] public ?string $description = null; /** @@ -114,7 +114,7 @@ class Period extends BaseEntity implements BelongsToCampInterface { */ #[Assert\LessThanOrEqual(propertyPath: 'end')] #[ApiProperty(example: '2022-01-01', openapiContext: ['format' => 'date'])] - #[Groups(['Properties:read', 'Properties:write'])] + #[Groups(['read', 'write'])] public ?DateTimeInterface $start = null; /** @@ -125,7 +125,7 @@ class Period extends BaseEntity implements BelongsToCampInterface { */ #[Assert\GreaterThanOrEqual(propertyPath: 'start')] #[ApiProperty(example: '2022-01-08', openapiContext: ['format' => 'date'])] - #[Groups(['Properties:read', 'Properties:write'])] + #[Groups(['read', 'write'])] public ?DateTimeInterface $end = null; public function __construct() { From 90f03ee4e2d1b9756713f8d0ed299942eee9ce88 Mon Sep 17 00:00:00 2001 From: Pirmin Mattmann Date: Fri, 30 Jul 2021 22:05:56 +0200 Subject: [PATCH 06/23] code-review changes --- api/src/Entity/Camp.php | 11 +++++++++-- api/src/Entity/Period.php | 1 - 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/api/src/Entity/Camp.php b/api/src/Entity/Camp.php index ad94fad89e..a9a646a5a8 100644 --- a/api/src/Entity/Camp.php +++ b/api/src/Entity/Camp.php @@ -30,6 +30,10 @@ 'groups' => ['write', 'create'], 'allow_extra_attributes' => false, ], + 'normalization_context' => [ + 'groups' => ['read', 'Camp:Periods', 'Period:Days'], + 'allow_extra_attributes' => false, + ], ], ], itemOperations: [ @@ -46,6 +50,10 @@ 'groups' => ['write', 'update'], 'allow_extra_attributes' => false, ], + 'normalization_context' => [ + 'groups' => ['read', 'Camp:Periods', 'Period:Days'], + 'allow_extra_attributes' => false, + ], ], 'delete' => ['security' => 'object.owner == user or is_granted("ROLE_ADMIN")'], ], @@ -76,7 +84,6 @@ class Camp extends BaseEntity implements BelongsToCampInterface { #[Assert\Valid] #[Assert\Count(min: 1, groups: ['Camp:create'])] #[ApiProperty( - readableLink: false, writableLink: true, example: '[{ "description": "Hauptlager", "start": "2022-01-01", "end": "2022-01-08" }]', )] @@ -150,7 +157,7 @@ class Camp extends BaseEntity implements BelongsToCampInterface { #[InputFilter\CleanHTML] #[Assert\NotBlank] #[Assert\Length(max: 32)] - #[ApiProperty(writable: true, example: 'Abteilungs-Sommerlager 2022')] + #[ApiProperty(example: 'Abteilungs-Sommerlager 2022')] #[Groups(['read', 'write'])] public string $title; diff --git a/api/src/Entity/Period.php b/api/src/Entity/Period.php index 251f64a567..8a08ae2a21 100644 --- a/api/src/Entity/Period.php +++ b/api/src/Entity/Period.php @@ -53,7 +53,6 @@ class Period extends BaseEntity implements BelongsToCampInterface { * @ORM\OrderBy({"dayOffset": "ASC"}) */ #[ApiProperty( - readableLink: false, writable: false, example: '/days?period=/periods/1a2b3c4d' )] From 23c83ee470bbd985794ee3ead18c34378e843062 Mon Sep 17 00:00:00 2001 From: Carlo Beltrame Date: Wed, 4 Aug 2021 09:09:40 +0200 Subject: [PATCH 07/23] Sort named arguments --- api/src/Entity/Camp.php | 8 ++++---- api/src/Entity/Day.php | 8 ++++---- api/src/Entity/Period.php | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/api/src/Entity/Camp.php b/api/src/Entity/Camp.php index a9a646a5a8..42bc1b0b56 100644 --- a/api/src/Entity/Camp.php +++ b/api/src/Entity/Camp.php @@ -57,14 +57,14 @@ ], 'delete' => ['security' => 'object.owner == user or is_granted("ROLE_ADMIN")'], ], - normalizationContext: [ - 'groups' => ['read'], - 'allow_extra_attributes' => false, - ], denormalizationContext: [ 'groups' => ['write'], 'allow_extra_attributes' => false, ], + normalizationContext: [ + 'groups' => ['read'], + 'allow_extra_attributes' => false, + ], )] class Camp extends BaseEntity implements BelongsToCampInterface { /** diff --git a/api/src/Entity/Day.php b/api/src/Entity/Day.php index 418ed5047b..996f643c35 100644 --- a/api/src/Entity/Day.php +++ b/api/src/Entity/Day.php @@ -27,14 +27,14 @@ #[ApiResource( collectionOperations: ['get'], itemOperations: ['get'], - normalizationContext: [ - 'groups' => ['read'], - 'allow_extra_attributes' => false, - ], denormalizationContext: [ 'groups' => ['write'], 'allow_extra_attributes' => false, ], + normalizationContext: [ + 'groups' => ['read'], + 'allow_extra_attributes' => false, + ], )] #[ApiFilter(SearchFilter::class, properties: ['period'])] #[UniqueEntity(fields: ['period', 'dayOffset'])] diff --git a/api/src/Entity/Period.php b/api/src/Entity/Period.php index 8a08ae2a21..cc0a862c0b 100644 --- a/api/src/Entity/Period.php +++ b/api/src/Entity/Period.php @@ -35,14 +35,14 @@ 'patch', 'delete', ], - normalizationContext: [ - 'groups' => ['read'], - 'allow_extra_attributes' => false, - ], denormalizationContext: [ 'groups' => ['write'], 'allow_extra_attributes' => false, ], + normalizationContext: [ + 'groups' => ['read'], + 'allow_extra_attributes' => false, + ], )] #[ApiFilter(SearchFilter::class, properties: ['camp'])] class Period extends BaseEntity implements BelongsToCampInterface { From 7205e60cefbd8a82a50d8ef89a7d916be68498c6 Mon Sep 17 00:00:00 2001 From: Carlo Beltrame Date: Wed, 4 Aug 2021 09:10:13 +0200 Subject: [PATCH 08/23] Make the camp name writable again --- api/src/Entity/Camp.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/Entity/Camp.php b/api/src/Entity/Camp.php index 42bc1b0b56..a6294b4182 100644 --- a/api/src/Entity/Camp.php +++ b/api/src/Entity/Camp.php @@ -145,7 +145,7 @@ class Camp extends BaseEntity implements BelongsToCampInterface { #[InputFilter\CleanHTML] #[Assert\NotBlank] #[ApiProperty(example: 'SoLa 2022')] - #[Groups(['read', 'create'])] + #[Groups(['read', 'write'])] public string $name; /** From 3ff4f41944d6fc5d659d4e6c275580f1d278b461 Mon Sep 17 00:00:00 2001 From: Carlo Beltrame Date: Wed, 4 Aug 2021 09:13:02 +0200 Subject: [PATCH 09/23] Make dayResponsibles readable again --- api/src/Entity/Day.php | 1 + 1 file changed, 1 insertion(+) diff --git a/api/src/Entity/Day.php b/api/src/Entity/Day.php index 996f643c35..027a650f73 100644 --- a/api/src/Entity/Day.php +++ b/api/src/Entity/Day.php @@ -45,6 +45,7 @@ class Day extends BaseEntity implements BelongsToCampInterface { * @ORM\OneToMany(targetEntity="DayResponsible", mappedBy="day", orphanRemoval=true) */ #[ApiProperty(writable: false, example: '["/day_responsibles/1a2b3c4d"]')] + #[Groups(['read'])] public Collection $dayResponsibles; /** From e7c3516aa2bddd094a43f885a83b585a412b2b57 Mon Sep 17 00:00:00 2001 From: Carlo Beltrame Date: Wed, 4 Aug 2021 09:14:29 +0200 Subject: [PATCH 10/23] Revert readability of camp on day --- api/src/Entity/Day.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/api/src/Entity/Day.php b/api/src/Entity/Day.php index 027a650f73..0771befe88 100644 --- a/api/src/Entity/Day.php +++ b/api/src/Entity/Day.php @@ -71,8 +71,7 @@ public function __construct() { $this->dayResponsibles = new ArrayCollection(); } - #[ApiProperty(readableLink: false, writable: false, example: '/camps/1a2b3c4d')] - #[Groups(['read'])] + #[ApiProperty(readable: false)] public function getCamp(): ?Camp { return $this->period?->camp; } From c94ed0dc79935711f11d90c1704ea6758ba78bc8 Mon Sep 17 00:00:00 2001 From: Carlo Beltrame Date: Wed, 4 Aug 2021 10:21:19 +0200 Subject: [PATCH 11/23] Fix return type declarations to make examples in swagger work automatically --- api/src/Entity/Camp.php | 12 ++++++------ api/src/Entity/Period.php | 17 +++++++---------- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/api/src/Entity/Camp.php b/api/src/Entity/Camp.php index a6294b4182..df3e315e74 100644 --- a/api/src/Entity/Camp.php +++ b/api/src/Entity/Camp.php @@ -251,14 +251,14 @@ public function __construct() { $this->materialLists = new ArrayCollection(); } - #[ApiProperty( - readableLink: true, - example: '[{ "description": "Hauptlager", "start": "2022-01-01", "end": "2022-01-08" }]' - )] + /** + * @return Period[] + */ + #[ApiProperty(readableLink: true)] #[SerializedName('periods')] #[Groups(['Camp:Periods'])] - public function getEmbeddedPeriods(): Collection { - return $this->periods; + public function getEmbeddedPeriods(): array { + return $this->periods->getValues(); } #[ApiProperty(readable: false)] diff --git a/api/src/Entity/Period.php b/api/src/Entity/Period.php index cc0a862c0b..91448de486 100644 --- a/api/src/Entity/Period.php +++ b/api/src/Entity/Period.php @@ -52,10 +52,7 @@ class Period extends BaseEntity implements BelongsToCampInterface { * @ORM\OneToMany(targetEntity="Day", mappedBy="period", orphanRemoval=true) * @ORM\OrderBy({"dayOffset": "ASC"}) */ - #[ApiProperty( - writable: false, - example: '/days?period=/periods/1a2b3c4d' - )] + #[ApiProperty(writable: false, example: '["/days?period=/periods/1a2b3c4d"]')] #[Groups(['read'])] public Collection $days; @@ -133,14 +130,14 @@ public function __construct() { $this->materialItems = new ArrayCollection(); } - #[ApiProperty( - readableLink: true, - example: '[{ "dayOffset": 0, "number": 1 }]' - )] + /** + * @return Day[] + */ + #[ApiProperty(readableLink: true)] #[SerializedName('days')] #[Groups('Period:Days')] - public function getEmbeddedDays(): Collection { - return $this->days; + public function getEmbeddedDays(): array { + return $this->days->getValues(); } #[ApiProperty(readableLink: true)] From 5847e8c85c8cf537c9e0bbe8d4d564caaae7099b Mon Sep 17 00:00:00 2001 From: Carlo Beltrame Date: Fri, 6 Aug 2021 01:09:56 +0200 Subject: [PATCH 12/23] getExamplePayload needs to take into account different schema for different operations --- api/tests/Api/Activities/CreateActivityTest.php | 5 +++++ .../CreateActivityResponsibleTest.php | 5 +++++ .../CampCollaborations/CreateCampCollaborationTest.php | 5 +++++ api/tests/Api/Camps/CreateCampTest.php | 5 ++++- api/tests/Api/Categories/CreateCategoryTest.php | 5 +++++ api/tests/Api/ContentNodes/CreateContentNodeTest.php | 5 +++++ .../Api/DayResponsibles/CreateDayResponsibleTest.php | 5 +++++ api/tests/Api/ECampApiTestCase.php | 9 +++++---- api/tests/Api/MaterialItems/CreateMaterialItemTest.php | 5 +++++ api/tests/Api/MaterialLists/CreateMaterialListTest.php | 5 +++++ api/tests/Api/Periods/CreatePeriodTest.php | 5 +++++ .../Api/ScheduleEntries/CreateScheduleEntryTest.php | 5 +++++ api/tests/Api/Users/CreateUserTest.php | 5 +++-- 13 files changed, 62 insertions(+), 7 deletions(-) diff --git a/api/tests/Api/Activities/CreateActivityTest.php b/api/tests/Api/Activities/CreateActivityTest.php index 78ad0ddcaa..ee213d4760 100644 --- a/api/tests/Api/Activities/CreateActivityTest.php +++ b/api/tests/Api/Activities/CreateActivityTest.php @@ -2,6 +2,7 @@ namespace App\Tests\Api\Activities; +use ApiPlatform\Core\Api\OperationType; use App\Entity\Activity; use App\Tests\Api\ECampApiTestCase; @@ -67,6 +68,8 @@ public function testCreateActivityAllowsMissingLocation() { public function getExampleWritePayload($attributes = [], $except = []) { return $this->getExamplePayload( Activity::class, + OperationType::COLLECTION, + 'post', array_merge(['category' => $this->getIriFor('category1')], $attributes), [], $except @@ -76,6 +79,8 @@ public function getExampleWritePayload($attributes = [], $except = []) { public function getExampleReadPayload($attributes = [], $except = []) { return $this->getExamplePayload( Activity::class, + OperationType::ITEM, + 'get', $attributes, ['category'], $except diff --git a/api/tests/Api/ActivityResponsibles/CreateActivityResponsibleTest.php b/api/tests/Api/ActivityResponsibles/CreateActivityResponsibleTest.php index f2f7c8f1ef..403b4374a7 100644 --- a/api/tests/Api/ActivityResponsibles/CreateActivityResponsibleTest.php +++ b/api/tests/Api/ActivityResponsibles/CreateActivityResponsibleTest.php @@ -2,6 +2,7 @@ namespace App\Tests\Api\ActivityResponsibles; +use ApiPlatform\Core\Api\OperationType; use App\Entity\ActivityResponsible; use App\Tests\Api\ECampApiTestCase; @@ -68,6 +69,8 @@ public function testValidatesDuplicateActivityResponsible() { public function getExampleWritePayload($attributes = [], $except = []) { return $this->getExamplePayload( ActivityResponsible::class, + OperationType::COLLECTION, + 'post', array_merge([ 'activity' => $this->getIriFor('activity2'), 'campCollaboration' => $this->getIriFor('campCollaboration1'), @@ -80,6 +83,8 @@ public function getExampleWritePayload($attributes = [], $except = []) { public function getExampleReadPayload($attributes = [], $except = []) { return $this->getExamplePayload( ActivityResponsible::class, + OperationType::ITEM, + 'get', $attributes, ['activity', 'campCollaboration'], $except diff --git a/api/tests/Api/CampCollaborations/CreateCampCollaborationTest.php b/api/tests/Api/CampCollaborations/CreateCampCollaborationTest.php index 9593284cc6..e44ec105f3 100644 --- a/api/tests/Api/CampCollaborations/CreateCampCollaborationTest.php +++ b/api/tests/Api/CampCollaborations/CreateCampCollaborationTest.php @@ -2,6 +2,7 @@ namespace App\Tests\Api\CampCollaborations; +use ApiPlatform\Core\Api\OperationType; use App\Entity\CampCollaboration; use App\Tests\Api\ECampApiTestCase; @@ -114,6 +115,8 @@ public function testCreateCampCollaborationValidatesMissingRole() { public function getExampleWritePayload($attributes = [], $except = []) { return $this->getExamplePayload( CampCollaboration::class, + OperationType::COLLECTION, + 'post', array_merge([ 'inviteEmail' => null, 'user' => $this->getIriFor('user1'), @@ -127,6 +130,8 @@ public function getExampleWritePayload($attributes = [], $except = []) { public function getExampleReadPayload($attributes = [], $except = []) { return $this->getExamplePayload( CampCollaboration::class, + OperationType::ITEM, + 'get', array_merge([ '_links' => [ 'user' => ['href' => $this->getIriFor('user1')], diff --git a/api/tests/Api/Camps/CreateCampTest.php b/api/tests/Api/Camps/CreateCampTest.php index 3ee10ceb8b..c4ad15496f 100644 --- a/api/tests/Api/Camps/CreateCampTest.php +++ b/api/tests/Api/Camps/CreateCampTest.php @@ -2,6 +2,7 @@ namespace App\Tests\Api\Camps; +use ApiPlatform\Core\Api\OperationType; use App\Entity\Camp; use App\Entity\User; use App\Tests\Api\ECampApiTestCase; @@ -588,12 +589,14 @@ public function testCreateCampValidatesLongAddressCity() { } public function getExampleWritePayload($attributes = [], $except = []) { - return $this->getExamplePayload(Camp::class, $attributes, [], $except); + return $this->getExamplePayload(Camp::class, OperationType::COLLECTION, 'post', $attributes, [], $except); } public function getExampleReadPayload($attributes = [], $except = []) { return $this->getExamplePayload( Camp::class, + OperationType::ITEM, + 'get', $attributes, ['periods'], $except diff --git a/api/tests/Api/Categories/CreateCategoryTest.php b/api/tests/Api/Categories/CreateCategoryTest.php index 60dac99264..4ecbf84ed2 100644 --- a/api/tests/Api/Categories/CreateCategoryTest.php +++ b/api/tests/Api/Categories/CreateCategoryTest.php @@ -2,6 +2,7 @@ namespace App\Tests\Api\Categories; +use ApiPlatform\Core\Api\OperationType; use App\Entity\Category; use App\Entity\ContentNode; use App\Tests\Api\ECampApiTestCase; @@ -143,6 +144,8 @@ public function testCreateCategoryValidatesInvalidNumberingStyle() { public function getExampleWritePayload($attributes = [], $except = []) { return $this->getExamplePayload( Category::class, + OperationType::COLLECTION, + 'post', array_merge([ 'camp' => $this->getIriFor('camp1'), 'preferredContentTypes' => [$this->getIriFor('contentType1')], @@ -155,6 +158,8 @@ public function getExampleWritePayload($attributes = [], $except = []) { public function getExampleReadPayload($attributes = [], $except = []) { return $this->getExamplePayload( Category::class, + OperationType::ITEM, + 'get', array_merge([ '_links' => [ 'contentNodes' => [], diff --git a/api/tests/Api/ContentNodes/CreateContentNodeTest.php b/api/tests/Api/ContentNodes/CreateContentNodeTest.php index 93ae150d0b..732f225889 100644 --- a/api/tests/Api/ContentNodes/CreateContentNodeTest.php +++ b/api/tests/Api/ContentNodes/CreateContentNodeTest.php @@ -2,6 +2,7 @@ namespace App\Tests\Api\ContentNodes; +use ApiPlatform\Core\Api\OperationType; use App\Entity\ContentNode; use App\Tests\Api\ECampApiTestCase; @@ -88,6 +89,8 @@ public function testCreateContentNodeValidatesMissingContentType() { public function getExampleWritePayload($attributes = [], $except = []) { return $this->getExamplePayload( ContentNode::class, + OperationType::COLLECTION, + 'post', array_merge([ 'parent' => $this->getIriFor('contentNode1'), 'contentType' => $this->getIriFor('contentTypeColumnLayout'), @@ -100,6 +103,8 @@ public function getExampleWritePayload($attributes = [], $except = []) { public function getExampleReadPayload($attributes = [], $except = []) { return $this->getExamplePayload( ContentNode::class, + OperationType::ITEM, + 'get', $attributes, ['parent', 'contentType'], $except diff --git a/api/tests/Api/DayResponsibles/CreateDayResponsibleTest.php b/api/tests/Api/DayResponsibles/CreateDayResponsibleTest.php index c6e267e36a..cbce3b1462 100644 --- a/api/tests/Api/DayResponsibles/CreateDayResponsibleTest.php +++ b/api/tests/Api/DayResponsibles/CreateDayResponsibleTest.php @@ -2,6 +2,7 @@ namespace App\Tests\Api\DayResponsibles; +use ApiPlatform\Core\Api\OperationType; use App\Entity\DayResponsible; use App\Tests\Api\ECampApiTestCase; @@ -68,6 +69,8 @@ public function testValidatesDuplicateDayResponsible() { public function getExampleWritePayload($attributes = [], $except = []) { return $this->getExamplePayload( DayResponsible::class, + OperationType::COLLECTION, + 'post', array_merge([ 'day' => $this->getIriFor('day3period1'), 'campCollaboration' => $this->getIriFor('campCollaboration1'), @@ -80,6 +83,8 @@ public function getExampleWritePayload($attributes = [], $except = []) { public function getExampleReadPayload($attributes = [], $except = []) { return $this->getExamplePayload( DayResponsible::class, + OperationType::ITEM, + 'get', $attributes, ['day', 'campCollaboration'], $except diff --git a/api/tests/Api/ECampApiTestCase.php b/api/tests/Api/ECampApiTestCase.php index e913c922dc..cd6ad46d9c 100644 --- a/api/tests/Api/ECampApiTestCase.php +++ b/api/tests/Api/ECampApiTestCase.php @@ -110,10 +110,11 @@ protected function getEntityManager(): EntityManagerInterface { return $this->entityManager; } - protected function getExamplePayload(string $resourceClass, array $attributes = [], array $exceptExamples = [], array $exceptAttributes = []): array { - $shortName = $this->getResourceMetadataFactory()->create($resourceClass)->getShortName(); - $schema = $this->getSchemaFactory()->buildSchema($resourceClass, 'json', Schema::TYPE_INPUT, 'POST'); - $properties = ($schema->getDefinitions()[$shortName] ?? $schema->getDefinitions()[$shortName.'-Default'])['properties']; + protected function getExamplePayload(string $resourceClass, string $operationType, string $operationName, array $attributes = [], array $exceptExamples = [], array $exceptAttributes = []): array { + $schema = $this->getSchemaFactory()->buildSchema($resourceClass, 'json', 'get' === $operationName ? Schema::TYPE_OUTPUT : Schema::TYPE_INPUT, $operationType, $operationName); + preg_match('/\/([^\/]+)$/', $schema['$ref'], $matches); + $schemaName = $matches[1]; + $properties = $schema->getDefinitions()[$schemaName]['properties']; $writableProperties = array_filter($properties, fn ($property) => !($property['readOnly'] ?? false)); $writablePropertiesWithExample = array_filter($writableProperties, fn ($property) => ($property['example'] ?? false)); $examples = array_map(fn ($property) => $property['example'] ?? $property['default'] ?? null, $writablePropertiesWithExample); diff --git a/api/tests/Api/MaterialItems/CreateMaterialItemTest.php b/api/tests/Api/MaterialItems/CreateMaterialItemTest.php index 9d3bac1aff..a2c454c35c 100644 --- a/api/tests/Api/MaterialItems/CreateMaterialItemTest.php +++ b/api/tests/Api/MaterialItems/CreateMaterialItemTest.php @@ -2,6 +2,7 @@ namespace App\Tests\Api\MaterialItems; +use ApiPlatform\Core\Api\OperationType; use App\Entity\MaterialItem; use App\Tests\Api\ECampApiTestCase; @@ -164,6 +165,8 @@ public function testCreateMaterialItemAllowsMissingUnit() { public function getExampleWritePayload($attributes = [], $except = []) { return $this->getExamplePayload( MaterialItem::class, + OperationType::COLLECTION, + 'post', array_merge([ 'materialList' => $this->getIriFor('materialList1'), 'period' => $this->getIriFor('period1'), @@ -177,6 +180,8 @@ public function getExampleWritePayload($attributes = [], $except = []) { public function getExampleReadPayload($attributes = [], $except = []) { return $this->getExamplePayload( MaterialItem::class, + OperationType::ITEM, + 'get', $attributes, ['materialList', 'period', 'contentNode'], $except diff --git a/api/tests/Api/MaterialLists/CreateMaterialListTest.php b/api/tests/Api/MaterialLists/CreateMaterialListTest.php index c8a4be90e2..7fb2037935 100644 --- a/api/tests/Api/MaterialLists/CreateMaterialListTest.php +++ b/api/tests/Api/MaterialLists/CreateMaterialListTest.php @@ -2,6 +2,7 @@ namespace App\Tests\Api\MaterialLists; +use ApiPlatform\Core\Api\OperationType; use App\Entity\MaterialList; use App\Tests\Api\ECampApiTestCase; @@ -51,6 +52,8 @@ public function testCreateMaterialListValidatesMissingName() { public function getExampleWritePayload($attributes = [], $except = []) { return $this->getExamplePayload( MaterialList::class, + OperationType::COLLECTION, + 'post', array_merge(['camp' => $this->getIriFor('camp1')], $attributes), [], $except @@ -60,6 +63,8 @@ public function getExampleWritePayload($attributes = [], $except = []) { public function getExampleReadPayload($attributes = [], $except = []) { return $this->getExamplePayload( MaterialList::class, + OperationType::ITEM, + 'get', $attributes, ['camp'], $except diff --git a/api/tests/Api/Periods/CreatePeriodTest.php b/api/tests/Api/Periods/CreatePeriodTest.php index f03f7bd855..d91f6d0334 100644 --- a/api/tests/Api/Periods/CreatePeriodTest.php +++ b/api/tests/Api/Periods/CreatePeriodTest.php @@ -2,6 +2,7 @@ namespace App\Tests\Api\Periods; +use ApiPlatform\Core\Api\OperationType; use App\Entity\Period; use App\Tests\Api\ECampApiTestCase; @@ -150,6 +151,8 @@ public function testCreatePeriodValidatesStartAfterEnd() { public function getExampleWritePayload($attributes = [], $except = []) { return $this->getExamplePayload( Period::class, + OperationType::COLLECTION, + 'post', array_merge(['camp' => $this->getIriFor('camp1')], $attributes), [], $except @@ -159,6 +162,8 @@ public function getExampleWritePayload($attributes = [], $except = []) { public function getExampleReadPayload($attributes = [], $except = []) { return $this->getExamplePayload( Period::class, + OperationType::ITEM, + 'get', $attributes, ['camp'], $except diff --git a/api/tests/Api/ScheduleEntries/CreateScheduleEntryTest.php b/api/tests/Api/ScheduleEntries/CreateScheduleEntryTest.php index 31c90828de..242c1cf803 100644 --- a/api/tests/Api/ScheduleEntries/CreateScheduleEntryTest.php +++ b/api/tests/Api/ScheduleEntries/CreateScheduleEntryTest.php @@ -2,6 +2,7 @@ namespace App\Tests\Api\ScheduleEntries; +use ApiPlatform\Core\Api\OperationType; use App\Entity\ScheduleEntry; use App\Tests\Api\ECampApiTestCase; @@ -141,6 +142,8 @@ public function testCreateScheduleEntryUsesDefaultForMissingWidth() { public function getExampleWritePayload($attributes = [], $except = []) { return $this->getExamplePayload( ScheduleEntry::class, + OperationType::COLLECTION, + 'post', array_merge([ 'period' => $this->getIriFor('period1'), 'activity' => $this->getIriFor('activity1'), @@ -153,6 +156,8 @@ public function getExampleWritePayload($attributes = [], $except = []) { public function getExampleReadPayload($attributes = [], $except = []) { return $this->getExamplePayload( ScheduleEntry::class, + OperationType::ITEM, + 'get', $attributes, ['period', 'activity'], $except diff --git a/api/tests/Api/Users/CreateUserTest.php b/api/tests/Api/Users/CreateUserTest.php index a2ad02ffcc..3a57387ec8 100644 --- a/api/tests/Api/Users/CreateUserTest.php +++ b/api/tests/Api/Users/CreateUserTest.php @@ -2,6 +2,7 @@ namespace App\Tests\Api\Users; +use ApiPlatform\Core\Api\OperationType; use App\Entity\User; use App\Tests\Api\ECampApiTestCase; @@ -393,10 +394,10 @@ public function testCreateUserValidatesBlankPassword() { } public function getExampleWritePayload($attributes = [], $except = []) { - return $this->getExamplePayload(User::class, $attributes, [], $except); + return $this->getExamplePayload(User::class, OperationType::COLLECTION, 'post', $attributes, [], $except); } public function getExampleReadPayload($attributes = [], $except = []) { - return $this->getExamplePayload(User::class, $attributes, [], $except); + return $this->getExamplePayload(User::class, OperationType::ITEM, 'get', $attributes, [], $except); } } From 8153a0a9bc9f5cc206ff9ff16d5916035fa16444 Mon Sep 17 00:00:00 2001 From: Carlo Beltrame Date: Fri, 6 Aug 2021 14:01:48 +0200 Subject: [PATCH 13/23] Always add some defaults to the context https://symfonycasts.com/screencast/api-platform-security/service-decoration https://api-platform.com/docs/core/serialization/#changing-the-serialization-context-dynamically --- api/config/services.yaml | 3 + api/src/Entity/Camp.php | 35 ++------ api/src/Entity/CampCollaboration.php | 11 +-- api/src/Entity/Category.php | 5 +- api/src/Entity/ContentNode.php | 5 +- api/src/Entity/Day.php | 10 +-- api/src/Entity/MaterialList.php | 5 +- api/src/Entity/Period.php | 15 +--- api/src/Entity/ScheduleEntry.php | 5 +- api/src/Entity/User.php | 5 +- .../Serializer/SerializerContextBuilder.php | 38 +++++++++ .../SerializerContextBuilderTest.php | 79 +++++++++++++++++++ 12 files changed, 139 insertions(+), 77 deletions(-) create mode 100644 api/src/Serializer/SerializerContextBuilder.php create mode 100644 api/tests/Serializer/SerializerContextBuilderTest.php diff --git a/api/config/services.yaml b/api/config/services.yaml index f692aca31e..216c74b9a1 100644 --- a/api/config/services.yaml +++ b/api/config/services.yaml @@ -65,5 +65,8 @@ services: App\Serializer\Normalizer\CollectionItemsNormalizer: decorates: 'api_platform.hal.normalizer.collection' + App\Serializer\SerializerContextBuilder: + decorates: 'api_platform.serializer.context_builder' + App\OpenApi\JwtDecorator: decorates: 'api_platform.openapi.factory' \ No newline at end of file diff --git a/api/src/Entity/Camp.php b/api/src/Entity/Camp.php index df3e315e74..0479735e31 100644 --- a/api/src/Entity/Camp.php +++ b/api/src/Entity/Camp.php @@ -26,45 +26,24 @@ 'security' => 'is_fully_authenticated()', 'input_formats' => ['jsonld', 'jsonapi', 'json'], 'validation_groups' => ['Default', 'Camp:create'], - 'denormalization_context' => [ - 'groups' => ['write', 'create'], - 'allow_extra_attributes' => false, - ], - 'normalization_context' => [ - 'groups' => ['read', 'Camp:Periods', 'Period:Days'], - 'allow_extra_attributes' => false, - ], + 'denormalization_context' => ['groups' => ['write', 'create']], + 'normalization_context' => ['groups' => ['read', 'Camp:Periods', 'Period:Days']], ], ], itemOperations: [ 'get' => [ 'security' => 'object.owner == user or is_granted("ROLE_ADMIN")', - 'normalization_context' => [ - 'groups' => ['read', 'Camp:Periods', 'Period:Days'], - 'allow_extra_attributes' => false, - ], + 'normalization_context' => ['groups' => ['read', 'Camp:Periods', 'Period:Days']], ], 'patch' => [ 'security' => 'object.owner == user or is_granted("ROLE_ADMIN")', - 'denormalization_context' => [ - 'groups' => ['write', 'update'], - 'allow_extra_attributes' => false, - ], - 'normalization_context' => [ - 'groups' => ['read', 'Camp:Periods', 'Period:Days'], - 'allow_extra_attributes' => false, - ], + 'denormalization_context' => ['groups' => ['write', 'update']], + 'normalization_context' => ['groups' => ['read', 'Camp:Periods', 'Period:Days']], ], 'delete' => ['security' => 'object.owner == user or is_granted("ROLE_ADMIN")'], ], - denormalizationContext: [ - 'groups' => ['write'], - 'allow_extra_attributes' => false, - ], - normalizationContext: [ - 'groups' => ['read'], - 'allow_extra_attributes' => false, - ], + denormalizationContext: ['groups' => ['write']], + normalizationContext: ['groups' => ['read']], )] class Camp extends BaseEntity implements BelongsToCampInterface { /** diff --git a/api/src/Entity/CampCollaboration.php b/api/src/Entity/CampCollaboration.php index e611f3631c..d4a52168fb 100644 --- a/api/src/Entity/CampCollaboration.php +++ b/api/src/Entity/CampCollaboration.php @@ -24,20 +24,13 @@ #[ApiResource( collectionOperations: [ 'get', - 'post' => ['denormalization_context' => [ - 'groups' => ['campCollaboration:create'], - 'allow_extra_attributes' => false, - ]], + 'post' => ['denormalization_context' => ['groups' => ['campCollaboration:create']]], ], itemOperations: [ 'get', - 'patch' => ['denormalization_context' => [ - 'groups' => ['campCollaboration:update'], - 'allow_extra_attributes' => false, - ]], + 'patch' => ['denormalization_context' => ['groups' => ['campCollaboration:update']]], 'delete', ], - normalizationContext: ['skip_null_values' => false], )] #[ApiFilter(SearchFilter::class, properties: ['camp'])] class CampCollaboration extends BaseEntity implements BelongsToCampInterface { diff --git a/api/src/Entity/Category.php b/api/src/Entity/Category.php index 422da4a670..266e64f6ba 100644 --- a/api/src/Entity/Category.php +++ b/api/src/Entity/Category.php @@ -24,10 +24,7 @@ collectionOperations: ['get', 'post'], itemOperations: [ 'get', - 'patch' => ['denormalization_context' => [ - 'groups' => ['category:update'], - 'allow_extra_attributes' => false, - ]], + 'patch' => ['denormalization_context' => ['groups' => ['category:update']]], 'delete', ] )] diff --git a/api/src/Entity/ContentNode.php b/api/src/Entity/ContentNode.php index 92b0ffc645..830a6dcc0e 100644 --- a/api/src/Entity/ContentNode.php +++ b/api/src/Entity/ContentNode.php @@ -30,10 +30,7 @@ itemOperations: [ 'get', 'patch' => [ - 'denormalization_context' => [ - 'groups' => ['contentNode:update'], - 'allow_extra_attributes' => false, - ], + 'denormalization_context' => ['groups' => ['contentNode:update']], 'validation_groups' => ['Default', 'contentNode:update'], ], 'delete' => ['security' => 'object.owner === null'], diff --git a/api/src/Entity/Day.php b/api/src/Entity/Day.php index 0771befe88..c8ba0dde6a 100644 --- a/api/src/Entity/Day.php +++ b/api/src/Entity/Day.php @@ -27,14 +27,8 @@ #[ApiResource( collectionOperations: ['get'], itemOperations: ['get'], - denormalizationContext: [ - 'groups' => ['write'], - 'allow_extra_attributes' => false, - ], - normalizationContext: [ - 'groups' => ['read'], - 'allow_extra_attributes' => false, - ], + denormalizationContext: ['groups' => ['write']], + normalizationContext: ['groups' => ['read']], )] #[ApiFilter(SearchFilter::class, properties: ['period'])] #[UniqueEntity(fields: ['period', 'dayOffset'])] diff --git a/api/src/Entity/MaterialList.php b/api/src/Entity/MaterialList.php index f14991f423..316788ca40 100644 --- a/api/src/Entity/MaterialList.php +++ b/api/src/Entity/MaterialList.php @@ -21,10 +21,7 @@ collectionOperations: ['get', 'post'], itemOperations: [ 'get', - 'patch' => ['denormalization_context' => [ - 'groups' => ['materialList:update'], - 'allow_extra_attributes' => false, - ]], + 'patch' => ['denormalization_context' => ['groups' => ['materialList:update']]], 'delete', ] )] diff --git a/api/src/Entity/Period.php b/api/src/Entity/Period.php index 91448de486..9e1cc7b35a 100644 --- a/api/src/Entity/Period.php +++ b/api/src/Entity/Period.php @@ -27,22 +27,13 @@ ], itemOperations: [ 'get' => [ - 'normalization_context' => [ - 'groups' => ['read', 'Period:Camp', 'Period:Days'], - 'allow_extra_attributes' => false, - ], + 'normalization_context' => ['groups' => ['read', 'Period:Camp', 'Period:Days']], ], 'patch', 'delete', ], - denormalizationContext: [ - 'groups' => ['write'], - 'allow_extra_attributes' => false, - ], - normalizationContext: [ - 'groups' => ['read'], - 'allow_extra_attributes' => false, - ], + denormalizationContext: ['groups' => ['write']], + normalizationContext: ['groups' => ['read']], )] #[ApiFilter(SearchFilter::class, properties: ['camp'])] class Period extends BaseEntity implements BelongsToCampInterface { diff --git a/api/src/Entity/ScheduleEntry.php b/api/src/Entity/ScheduleEntry.php index a274efae9d..a871316d04 100644 --- a/api/src/Entity/ScheduleEntry.php +++ b/api/src/Entity/ScheduleEntry.php @@ -23,10 +23,7 @@ collectionOperations: ['get', 'post'], itemOperations: [ 'get', - 'patch' => ['denormalization_context' => [ - 'groups' => ['scheduleEntry:update'], - 'allow_extra_attributes' => false, - ]], + 'patch' => ['denormalization_context' => ['groups' => ['scheduleEntry:update']]], 'delete', ] )] diff --git a/api/src/Entity/User.php b/api/src/Entity/User.php index fc238c2382..3cdceb0d2f 100644 --- a/api/src/Entity/User.php +++ b/api/src/Entity/User.php @@ -34,10 +34,7 @@ 'get' => ['security' => 'is_granted("ROLE_ADMIN") or object == user'], 'patch' => [ 'security' => 'is_granted("ROLE_ADMIN") or object == user', - 'denormalization_context' => [ - 'groups' => 'user:update', - 'allow_extra_attributes' => false, - ], + 'denormalization_context' => ['groups' => 'user:update'], ], 'delete' => ['security' => 'is_granted("ROLE_ADMIN") and !object.ownsCamps()'], ], diff --git a/api/src/Serializer/SerializerContextBuilder.php b/api/src/Serializer/SerializerContextBuilder.php new file mode 100644 index 0000000000..c7a5e2a3b2 --- /dev/null +++ b/api/src/Serializer/SerializerContextBuilder.php @@ -0,0 +1,38 @@ +decorated = $decorated; + } + + public function createFromRequest(Request $request, bool $normalization, ?array $extractedAttributes = null): array { + $context = $this->decorated->createFromRequest($request, $normalization, $extractedAttributes); + + if ($normalization) { + $context['skip_null_values'] = false; + } + + if (!$normalization) { + $context['allow_extra_attributes'] = false; + } + + return $context; + } +} diff --git a/api/tests/Serializer/SerializerContextBuilderTest.php b/api/tests/Serializer/SerializerContextBuilderTest.php new file mode 100644 index 0000000000..1d7a55c4c2 --- /dev/null +++ b/api/tests/Serializer/SerializerContextBuilderTest.php @@ -0,0 +1,79 @@ +decoratedMock = $this->createMock(SerializerContextBuilderInterface::class); + + $this->contextBuilder = new SerializerContextBuilder($this->decoratedMock); + } + + public function testAddsSkipNullValuesFalseWhenNormalizing() { + $request = $this->createMock(Request::class); + $this->decoratedMock + ->expects($this->exactly(1)) + ->method('createFromRequest') + ->willReturn([]) + ; + + $result = $this->contextBuilder->createFromRequest($request, true); + + $this->assertEquals(['skip_null_values' => false], $result); + } + + public function testDoesntAddSkipNullValuesFalseWhenDenormalizing() { + $request = $this->createMock(Request::class); + $this->decoratedMock + ->expects($this->exactly(1)) + ->method('createFromRequest') + ->willReturn([]) + ; + + $result = $this->contextBuilder->createFromRequest($request, false); + + $this->assertNotEquals(['skip_null_values' => false], $result); + } + + public function testDoesntAddAllowExtraAttributesFalseWhenNormalizing() { + $request = $this->createMock(Request::class); + $this->decoratedMock + ->expects($this->exactly(1)) + ->method('createFromRequest') + ->willReturn([]) + ; + + $result = $this->contextBuilder->createFromRequest($request, true); + + $this->assertNotEquals(['allow_extra_attributes' => false], $result); + } + + public function testAddsAllowExtraAttributesFalseWhenDenormalizing() { + $request = $this->createMock(Request::class); + $this->decoratedMock + ->expects($this->exactly(1)) + ->method('createFromRequest') + ->willReturn([]) + ; + + $result = $this->contextBuilder->createFromRequest($request, false); + + $this->assertEquals(['allow_extra_attributes' => false], $result); + } +} From f6d07a2fea730cf355c9323ca371a041258ade44 Mon Sep 17 00:00:00 2001 From: Carlo Beltrame Date: Fri, 6 Aug 2021 14:08:12 +0200 Subject: [PATCH 14/23] Add missing normalization groups so the tests pass again --- api/src/Entity/Camp.php | 8 +++++++- api/src/Entity/Period.php | 6 ++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/api/src/Entity/Camp.php b/api/src/Entity/Camp.php index 0479735e31..e2a22a9ab1 100644 --- a/api/src/Entity/Camp.php +++ b/api/src/Entity/Camp.php @@ -50,6 +50,7 @@ class Camp extends BaseEntity implements BelongsToCampInterface { * @ORM\OneToMany(targetEntity="CampCollaboration", mappedBy="camp", orphanRemoval=true) */ #[SerializedName('campCollaborations')] + #[Groups(['read'])] public Collection $collaborations; /** @@ -75,6 +76,7 @@ class Camp extends BaseEntity implements BelongsToCampInterface { * @ORM\OneToMany(targetEntity="Category", mappedBy="camp", orphanRemoval=true) */ #[ApiProperty(writable: false, example: '["/categories/1a2b3c4d"]')] + #[Groups(['read'])] public Collection $categories; /** @@ -84,6 +86,7 @@ class Camp extends BaseEntity implements BelongsToCampInterface { * @ORM\OneToMany(targetEntity="Activity", mappedBy="camp", orphanRemoval=true) */ #[ApiProperty(writable: false, example: '["/activities/1a2b3c4d"]')] + #[Groups(['read'])] public Collection $activities; /** @@ -93,6 +96,7 @@ class Camp extends BaseEntity implements BelongsToCampInterface { * @ORM\OneToMany(targetEntity="MaterialList", mappedBy="camp", orphanRemoval=true) */ #[ApiProperty(writable: false, example: '["/material_lists/1a2b3c4d"]')] + #[Groups(['read'])] public Collection $materialLists; /** @@ -112,7 +116,8 @@ class Camp extends BaseEntity implements BelongsToCampInterface { */ #[Assert\Type('bool')] #[Assert\DisableAutoMapping] - #[ApiProperty(example: false, writable: false)] + #[ApiProperty(example: true, writable: false)] + #[Groups(['read'])] public bool $isPrototype = false; /** @@ -209,6 +214,7 @@ class Camp extends BaseEntity implements BelongsToCampInterface { */ #[Assert\DisableAutoMapping] #[ApiProperty(writable: false)] + #[Groups(['read'])] public ?User $creator = null; /** diff --git a/api/src/Entity/Period.php b/api/src/Entity/Period.php index 9e1cc7b35a..ed23fc3f27 100644 --- a/api/src/Entity/Period.php +++ b/api/src/Entity/Period.php @@ -23,7 +23,7 @@ #[ApiResource( collectionOperations: [ 'get', - 'post', + 'post' => ['denormalization_context' => ['groups' => ['write', 'create']]], ], itemOperations: [ 'get' => [ @@ -55,6 +55,7 @@ class Period extends BaseEntity implements BelongsToCampInterface { * @ORM\OrderBy({"periodOffset": "ASC"}) */ #[ApiProperty(writable: false, example: '["/schedule_entries/1a2b3c4d"]')] + #[Groups(['read'])] public Collection $scheduleEntries; /** @@ -64,6 +65,7 @@ class Period extends BaseEntity implements BelongsToCampInterface { * @ORM\OneToMany(targetEntity="MaterialItem", mappedBy="period") */ #[ApiProperty(writable: false, example: '["/material_items/1a2b3c4d"]')] + #[Groups(['read'])] public Collection $materialItems; /** @@ -73,7 +75,7 @@ class Period extends BaseEntity implements BelongsToCampInterface { * @ORM\JoinColumn(nullable=false) */ #[ApiProperty(readableLink: false, example: '/camps/1a2b3c4d')] - #[Groups(['read'])] + #[Groups(['read', 'create'])] public ?Camp $camp = null; /** From 25bd1545d5c06189179a6ffe789c7c1073d06c3a Mon Sep 17 00:00:00 2001 From: Carlo Beltrame Date: Fri, 6 Aug 2021 14:13:28 +0200 Subject: [PATCH 15/23] Work around broken circular reference detection in HAL format See https://github.com/api-platform/core/issues/4358 --- api/config/services.yaml | 3 + ...larReferenceDetectingHalItemNormalizer.php | 103 +++++++++++++++ ...eferenceDetectingHalItemNormalizerTest.php | 123 ++++++++++++++++++ 3 files changed, 229 insertions(+) create mode 100644 api/src/Serializer/Normalizer/CircularReferenceDetectingHalItemNormalizer.php create mode 100644 api/tests/Serializer/Normalizer/CircularReferenceDetectingHalItemNormalizerTest.php diff --git a/api/config/services.yaml b/api/config/services.yaml index 216c74b9a1..3152cbb198 100644 --- a/api/config/services.yaml +++ b/api/config/services.yaml @@ -62,6 +62,9 @@ services: - '@api_platform.filter_locator' - '@serializer.name_converter.metadata_aware' + App\Serializer\Normalizer\CircularReferenceDetectingHalItemNormalizer: + decorates: 'api_platform.hal.normalizer.item' + App\Serializer\Normalizer\CollectionItemsNormalizer: decorates: 'api_platform.hal.normalizer.collection' diff --git a/api/src/Serializer/Normalizer/CircularReferenceDetectingHalItemNormalizer.php b/api/src/Serializer/Normalizer/CircularReferenceDetectingHalItemNormalizer.php new file mode 100644 index 0000000000..ec7b35ca69 --- /dev/null +++ b/api/src/Serializer/Normalizer/CircularReferenceDetectingHalItemNormalizer.php @@ -0,0 +1,103 @@ + ['self' => ['href' => $this->iriConverter->getIriFromItem($object)]]]; + }; + parent::__construct($propertyNameCollectionFactory, $propertyMetadataFactory, $iriConverter, $resourceClassResolver, $propertyAccessor, $nameConverter, $classMetadataFactory, $itemDataProvider, $allowPlainIdentifiers, $defaultContext, $dataTransformers, $resourceMetadataFactory, $resourceAccessChecker); + } + + /** + * {@inheritdoc} + */ + public function supportsNormalization($data, $format = null): bool { + return $this->decorated->supportsNormalization($data, $format); + } + + /** + * {@inheritdoc} + */ + public function normalize($object, $format = null, array $context = []) { + if ($this->isHalCircularReference($object, $context)) { + return $this->handleHalCircularReference($object, $format, $context); + } + + return $this->decorated->normalize($object, $format, $context); + } + + public function setSerializer(SerializerInterface $serializer) { + if ($this->decorated instanceof SerializerAwareInterface) { + $this->decorated->setSerializer($serializer); + } + } + + /** + * Detects if the configured circular reference limit is reached. + * + * @throws CircularReferenceException + * + * @return bool + */ + protected function isHalCircularReference(object $object, array &$context) { + $objectHash = spl_object_hash($object); + + $circularReferenceLimit = $context[AbstractNormalizer::CIRCULAR_REFERENCE_LIMIT] ?? $this->defaultContext[AbstractNormalizer::CIRCULAR_REFERENCE_LIMIT]; + if (isset($context[self::HAL_CIRCULAR_REFERENCE_LIMIT_COUNTERS][$objectHash])) { + if ($context[self::HAL_CIRCULAR_REFERENCE_LIMIT_COUNTERS][$objectHash] >= $circularReferenceLimit) { + unset($context[self::HAL_CIRCULAR_REFERENCE_LIMIT_COUNTERS][$objectHash]); + + return true; + } + + ++$context[self::HAL_CIRCULAR_REFERENCE_LIMIT_COUNTERS][$objectHash]; + } else { + $context[self::HAL_CIRCULAR_REFERENCE_LIMIT_COUNTERS][$objectHash] = 1; + } + + return false; + } + + /** + * Handles a circular reference. + * + * If a circular reference handler is set, it will be called. Otherwise, a + * {@class CircularReferenceException} will be thrown. + * + * @final + * + * @throws CircularReferenceException + */ + protected function handleHalCircularReference(object $object, string $format = null, array $context = []) { + $circularReferenceHandler = $context[AbstractNormalizer::CIRCULAR_REFERENCE_HANDLER] ?? $this->defaultContext[AbstractNormalizer::CIRCULAR_REFERENCE_HANDLER]; + if ($circularReferenceHandler) { + return $circularReferenceHandler($object, $format, $context); + } + + throw new CircularReferenceException(sprintf('A circular reference has been detected when serializing the object of class "%s" (configured limit: %d).', get_debug_type($object), $context[AbstractNormalizer::CIRCULAR_REFERENCE_LIMIT] ?? $this->defaultContext[AbstractNormalizer::CIRCULAR_REFERENCE_LIMIT])); + } +} diff --git a/api/tests/Serializer/Normalizer/CircularReferenceDetectingHalItemNormalizerTest.php b/api/tests/Serializer/Normalizer/CircularReferenceDetectingHalItemNormalizerTest.php new file mode 100644 index 0000000000..da8ffdfe56 --- /dev/null +++ b/api/tests/Serializer/Normalizer/CircularReferenceDetectingHalItemNormalizerTest.php @@ -0,0 +1,123 @@ +decoratedMock = $this->createMock(ContextAwareNormalizerInterface::class); + $this->iriConverterMock = $this->createMock(IriConverterInterface::class); + + $this->normalizer = new CircularReferenceDetectingHalItemNormalizer( + $this->decoratedMock, + $this->createMock(PropertyNameCollectionFactoryInterface::class), + $this->createMock(PropertyMetadataFactoryInterface::class), + $this->iriConverterMock, + $this->createMock(ResourceClassResolverInterface::class) + ); + + $this->normalizer->setSerializer($this->createMock(SerializerInterface::class)); + } + + public function testDelegatesSupportCheckToDecorated() { + $this->decoratedMock + ->expects($this->exactly(2)) + ->method('supportsNormalization') + ->willReturnOnConsecutiveCalls(true, false) + ; + + $this->assertTrue($this->normalizer->supportsNormalization([])); + $this->assertFalse($this->normalizer->supportsNormalization([])); + } + + public function testDelegatesNormalizeToDecorated() { + // given + $resource = new Dummy(); + $delegatedResult = [ + 'hello' => 'world', + '_links' => [ + 'items' => [['href' => '/children/1']], + ], + ]; + $this->decoratedMock->expects($this->once()) + ->method('normalize') + ->willReturn($delegatedResult) + ; + + // when + $result = $this->normalizer->normalize($resource, null, []); + + // then + $this->assertEquals($delegatedResult, $result); + } + + public function testNormalizeDetectsCircularReference() { + // given + $normalizer = $this->normalizer; + $resource = new Dummy(); + $related = new RelatedDummy(); + $resource->related = $related; + $related->owner = $resource; + + $this->decoratedMock + ->method('normalize') + ->willReturnCallback(function ($object, $format, $context) use ($normalizer) { + $result = ['serialized_id' => $object->id]; + if ($object instanceof Dummy) { + $result = array_merge($result, ['related' => $normalizer->normalize($object->related, $format, $context)]); + } + if ($object instanceof RelatedDummy) { + $result = array_merge($result, ['owner' => $normalizer->normalize($object->owner, $format, $context)]); + } + + return $result; + }) + ; + $this->iriConverterMock + ->method('getIriFromItem') + ->willReturnCallback(fn ($object) => '/'.$object->id) + ; + + // when + /** @var Dummy $result */ + $result = $this->normalizer->normalize($resource, null, []); + + // then + $this->assertEquals(['_links' => ['self' => ['href' => '/dummy']]], $result['related']['owner']); + } +} + +class Dummy { + public string $id = 'dummy'; + public ?RelatedDummy $related = null; +} + +class RelatedDummy { + public string $id = 'related'; + public ?Dummy $owner = null; +} From eea56b97c101536841bc322e09f20cf4bfc9af45 Mon Sep 17 00:00:00 2001 From: Carlo Beltrame Date: Fri, 6 Aug 2021 15:04:49 +0200 Subject: [PATCH 16/23] Ignore the embedded getters when trying to read the Doctrine metadata --- .../Serializer/Normalizer/RelatedCollectionLinkNormalizer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/Serializer/Normalizer/RelatedCollectionLinkNormalizer.php b/api/src/Serializer/Normalizer/RelatedCollectionLinkNormalizer.php index 09a170dd63..7d31ae54cc 100644 --- a/api/src/Serializer/Normalizer/RelatedCollectionLinkNormalizer.php +++ b/api/src/Serializer/Normalizer/RelatedCollectionLinkNormalizer.php @@ -115,7 +115,7 @@ public function getRelatedCollectionHref($object, $rel, array $context = []): st $resourceClass = $context['resource_class'] ?? get_class($object); if ($this->nameConverter instanceof AdvancedNameConverterInterface) { - $rel = $this->nameConverter->denormalize($rel, $resourceClass, null, $context); + $rel = $this->nameConverter->denormalize($rel, $resourceClass, null, array_merge($context, ['groups' => ['read']])); } try { From c98a70a2bab8078c3e220d84fcb0af7d047185d2 Mon Sep 17 00:00:00 2001 From: Carlo Beltrame Date: Mon, 9 Aug 2021 13:16:37 +0200 Subject: [PATCH 17/23] Consistently use our group name system for all entities Also, deactivate the automatic embedding feature of API platform, because with our serialization groups system, everything would always get embedded everywhere. We want to only activate embedding selectively. --- api/config/services.yaml | 6 ++ api/src/Entity/Activity.php | 25 +++++- api/src/Entity/ActivityResponsible.php | 5 ++ api/src/Entity/Camp.php | 4 +- api/src/Entity/CampCollaboration.php | 16 ++-- api/src/Entity/Category.php | 38 ++++++--- api/src/Entity/ContentNode.php | 34 +++++--- api/src/Entity/ContentType.php | 4 + api/src/Entity/Day.php | 2 +- api/src/Entity/DayResponsible.php | 5 ++ api/src/Entity/MaterialItem.php | 13 ++- api/src/Entity/MaterialList.php | 16 ++-- api/src/Entity/Period.php | 2 +- api/src/Entity/ScheduleEntry.php | 27 +++--- api/src/Entity/User.php | 24 +++--- ...omaticEmbeddingPropertyMetadataFactory.php | 83 +++++++++++++++++++ .../MaterialItemUpdateGroupSequence.php | 2 +- .../Api/Activities/CreateActivityTest.php | 6 +- api/tests/Api/ECampApiTestCase.php | 2 +- ...icEmbeddingPropertyMetadataFactoryTest.php | 66 +++++++++++++++ 20 files changed, 311 insertions(+), 69 deletions(-) create mode 100644 api/src/Serializer/PreventAutomaticEmbeddingPropertyMetadataFactory.php create mode 100644 api/tests/Serializer/PreventAutomaticEmbeddingPropertyMetadataFactoryTest.php diff --git a/api/config/services.yaml b/api/config/services.yaml index 3152cbb198..0e8c649e1f 100644 --- a/api/config/services.yaml +++ b/api/config/services.yaml @@ -71,5 +71,11 @@ services: App\Serializer\SerializerContextBuilder: decorates: 'api_platform.serializer.context_builder' + App\Serializer\PreventAutomaticEmbeddingPropertyMetadataFactory: + decorates: 'api_platform.metadata.property.metadata_factory' + # Priority should be 1 lower than the one of SerializerPropertyMetadataFactory, see + # https://github.com/api-platform/core/blob/main/src/Bridge/Symfony/Bundle/Resources/config/metadata/metadata.xml#L65 + decoration_priority: 29 + App\OpenApi\JwtDecorator: decorates: 'api_platform.openapi.factory' \ No newline at end of file diff --git a/api/src/Entity/Activity.php b/api/src/Entity/Activity.php index 779b6cd56b..514ab047ab 100644 --- a/api/src/Entity/Activity.php +++ b/api/src/Entity/Activity.php @@ -10,6 +10,7 @@ use Doctrine\Common\Collections\ArrayCollection; use Doctrine\Common\Collections\Collection; use Doctrine\ORM\Mapping as ORM; +use Symfony\Component\Serializer\Annotation\Groups; use Symfony\Component\Validator\Constraints as Assert; /** @@ -22,10 +23,12 @@ itemOperations: [ 'get', 'patch' => [ - 'validation_groups' => ['Default', 'activity:update'], + 'validation_groups' => ['Default', 'update'], ], 'delete', ], + denormalizationContext: ['groups' => ['write']], + normalizationContext: ['groups' => ['read']], )] #[ApiFilter(SearchFilter::class, properties: ['camp'])] class Activity extends AbstractContentNodeOwner implements BelongsToCampInterface { @@ -35,6 +38,7 @@ class Activity extends AbstractContentNodeOwner implements BelongsToCampInterfac * @ORM\OneToMany(targetEntity="ScheduleEntry", mappedBy="activity", orphanRemoval=true) */ #[ApiProperty(writable: false, example: '["/schedule_entries/1a2b3c4d"]')] + #[Groups(['read'])] public Collection $scheduleEntries; /** @@ -53,6 +57,7 @@ class Activity extends AbstractContentNodeOwner implements BelongsToCampInterfac */ #[Assert\DisableAutoMapping] // camp is set in the DataPersister #[ApiProperty(writable: false, example: '/camps/1a2b3c4d')] + #[Groups(['read'])] public ?Camp $camp = null; /** @@ -63,7 +68,8 @@ class Activity extends AbstractContentNodeOwner implements BelongsToCampInterfac * @ORM\JoinColumn(nullable=false) */ #[ApiProperty(example: '/categories/1a2b3c4d')] - #[AssertBelongsToSameCamp(groups: ['activity:update'])] + #[AssertBelongsToSameCamp(groups: ['update'])] + #[Groups(['read', 'write'])] public ?Category $category = null; /** @@ -72,6 +78,7 @@ class Activity extends AbstractContentNodeOwner implements BelongsToCampInterfac * @ORM\Column(type="text") */ #[ApiProperty(example: 'Sportolympiade')] + #[Groups(['read', 'write'])] public ?string $title = null; /** @@ -80,6 +87,7 @@ class Activity extends AbstractContentNodeOwner implements BelongsToCampInterfac * @ORM\Column(type="text") */ #[ApiProperty(example: 'Spielwiese')] + #[Groups(['read', 'write'])] public string $location = ''; public function __construct() { @@ -98,11 +106,24 @@ public function setRootContentNode(?ContentNode $rootContentNode) { } #[Assert\DisableAutoMapping] + #[Groups(['read'])] public function getRootContentNode(): ?ContentNode { // Getter is here to add annotations to parent class property return $this->rootContentNode; } + /** + * Overridden in order to add annotations. + * + * {@inheritdoc} + * + * @return ContentNode[] + */ + #[Groups(['read'])] + public function getContentNodes(): array { + return parent::getContentNodes(); + } + /** * The list of people that are responsible for planning or carrying out this activity. * diff --git a/api/src/Entity/ActivityResponsible.php b/api/src/Entity/ActivityResponsible.php index 9d49abe9dd..a4238196aa 100644 --- a/api/src/Entity/ActivityResponsible.php +++ b/api/src/Entity/ActivityResponsible.php @@ -7,6 +7,7 @@ use App\Validator\AssertBelongsToSameCamp; use Doctrine\ORM\Mapping as ORM; use Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntity; +use Symfony\Component\Serializer\Annotation\Groups; use Symfony\Component\Validator\Constraints as Assert; /** @@ -20,6 +21,8 @@ #[ApiResource( collectionOperations: ['get', 'post'], itemOperations: ['get', 'delete'], + denormalizationContext: ['groups' => ['write']], + normalizationContext: ['groups' => ['read']], )] #[UniqueEntity(fields: ['activity', 'campCollaboration'])] class ActivityResponsible extends BaseEntity implements BelongsToCampInterface { @@ -31,6 +34,7 @@ class ActivityResponsible extends BaseEntity implements BelongsToCampInterface { */ #[Assert\NotNull] #[ApiProperty(example: '/activities/1a2b3c4d')] + #[Groups(['read', 'write'])] public ?Activity $activity = null; /** @@ -41,6 +45,7 @@ class ActivityResponsible extends BaseEntity implements BelongsToCampInterface { */ #[ApiProperty(example: '/camp_collaborations/1a2b3c4d')] #[AssertBelongsToSameCamp] + #[Groups(['read', 'write'])] public ?CampCollaboration $campCollaboration = null; #[ApiProperty(readable: false)] diff --git a/api/src/Entity/Camp.php b/api/src/Entity/Camp.php index e2a22a9ab1..4a7a7d9a47 100644 --- a/api/src/Entity/Camp.php +++ b/api/src/Entity/Camp.php @@ -25,7 +25,7 @@ 'post' => [ 'security' => 'is_fully_authenticated()', 'input_formats' => ['jsonld', 'jsonapi', 'json'], - 'validation_groups' => ['Default', 'Camp:create'], + 'validation_groups' => ['Default', 'create'], 'denormalization_context' => ['groups' => ['write', 'create']], 'normalization_context' => ['groups' => ['read', 'Camp:Periods', 'Period:Days']], ], @@ -62,7 +62,7 @@ class Camp extends BaseEntity implements BelongsToCampInterface { * @ORM\OrderBy({"start": "ASC"}) */ #[Assert\Valid] - #[Assert\Count(min: 1, groups: ['Camp:create'])] + #[Assert\Count(min: 1, groups: ['create'])] #[ApiProperty( writableLink: true, example: '[{ "description": "Hauptlager", "start": "2022-01-01", "end": "2022-01-08" }]', diff --git a/api/src/Entity/CampCollaboration.php b/api/src/Entity/CampCollaboration.php index d4a52168fb..9a793cc5c3 100644 --- a/api/src/Entity/CampCollaboration.php +++ b/api/src/Entity/CampCollaboration.php @@ -24,13 +24,15 @@ #[ApiResource( collectionOperations: [ 'get', - 'post' => ['denormalization_context' => ['groups' => ['campCollaboration:create']]], + 'post' => ['denormalization_context' => ['groups' => ['write', 'create']]], ], itemOperations: [ 'get', - 'patch' => ['denormalization_context' => ['groups' => ['campCollaboration:update']]], + 'patch' => ['denormalization_context' => ['groups' => ['write', 'update']]], 'delete', ], + denormalizationContext: ['groups' => ['write']], + normalizationContext: ['groups' => ['read']], )] #[ApiFilter(SearchFilter::class, properties: ['camp'])] class CampCollaboration extends BaseEntity implements BelongsToCampInterface { @@ -78,7 +80,7 @@ class CampCollaboration extends BaseEntity implements BelongsToCampInterface { */ #[AssertEitherIsNull(other: 'user')] #[ApiProperty(example: 'some-email@example.com')] - #[Groups(['Default', 'campCollaboration:create'])] + #[Groups(['read', 'create'])] public ?string $inviteEmail = null; /** @@ -96,7 +98,7 @@ class CampCollaboration extends BaseEntity implements BelongsToCampInterface { */ #[AssertEitherIsNull(other: 'inviteEmail')] #[ApiProperty(example: '/users/1a2b3c4d')] - #[Groups(['Default', 'campCollaboration:create'])] + #[Groups(['read', 'create'])] public ?User $user = null; /** @@ -106,7 +108,7 @@ class CampCollaboration extends BaseEntity implements BelongsToCampInterface { * @ORM\JoinColumn(nullable=false, onDelete="cascade") */ #[ApiProperty(example: '/camps/1a2b3c4d')] - #[Groups(['Default', 'campCollaboration:create'])] + #[Groups(['read', 'create'])] public ?Camp $camp = null; /** @@ -118,7 +120,7 @@ class CampCollaboration extends BaseEntity implements BelongsToCampInterface { */ #[Assert\Choice(choices: self::VALID_STATUS)] #[ApiProperty(default: self::STATUS_INVITED, example: self::STATUS_INACTIVE)] - #[Groups(['Default', 'campCollaboration:update'])] + #[Groups(['read', 'update'])] public string $status = self::STATUS_INVITED; /** @@ -129,7 +131,7 @@ class CampCollaboration extends BaseEntity implements BelongsToCampInterface { */ #[Assert\Choice(choices: self::VALID_ROLES)] #[ApiProperty(example: self::ROLE_MEMBER)] - #[Groups(['Default', 'campCollaboration:create', 'campCollaboration:update'])] + #[Groups(['read', 'write'])] public string $role; /** diff --git a/api/src/Entity/Category.php b/api/src/Entity/Category.php index 266e64f6ba..6a85de82bc 100644 --- a/api/src/Entity/Category.php +++ b/api/src/Entity/Category.php @@ -21,12 +21,17 @@ * @ORM\Entity */ #[ApiResource( - collectionOperations: ['get', 'post'], + collectionOperations: [ + 'get', + 'post' => ['denormalization_context' => ['groups' => ['write', 'create']]], + ], itemOperations: [ 'get', - 'patch' => ['denormalization_context' => ['groups' => ['category:update']]], + 'patch' => ['denormalization_context' => ['groups' => ['write', 'update']]], 'delete', - ] + ], + denormalizationContext: ['groups' => ['write']], + normalizationContext: ['groups' => ['read']], )] #[ApiFilter(SearchFilter::class, properties: ['camp'])] class Category extends AbstractContentNodeOwner implements BelongsToCampInterface { @@ -37,7 +42,7 @@ class Category extends AbstractContentNodeOwner implements BelongsToCampInterfac * @ORM\JoinColumn(nullable=false, onDelete="cascade") */ #[ApiProperty(example: '/camps/1a2b3c4d')] - #[Groups(['Default'])] + #[Groups(['read', 'create'])] public ?Camp $camp = null; /** @@ -50,7 +55,7 @@ class Category extends AbstractContentNodeOwner implements BelongsToCampInterfac * ) */ #[ApiProperty(example: '["/content_types/1a2b3c4d"]')] - #[Groups(['Default', 'category:update'])] + #[Groups(['read', 'write'])] public Collection $preferredContentTypes; /** @@ -77,7 +82,7 @@ class Category extends AbstractContentNodeOwner implements BelongsToCampInterfac * @ORM\Column(type="text", nullable=false) */ #[ApiProperty(example: 'LS')] - #[Groups(['Default', 'category:update'])] + #[Groups(['read', 'write'])] public ?string $short = null; /** @@ -86,7 +91,7 @@ class Category extends AbstractContentNodeOwner implements BelongsToCampInterfac * @ORM\Column(type="text", nullable=false) */ #[ApiProperty(example: 'Lagersport')] - #[Groups(['Default', 'category:update'])] + #[Groups(['read', 'write'])] public ?string $name = null; /** @@ -96,7 +101,7 @@ class Category extends AbstractContentNodeOwner implements BelongsToCampInterfac */ #[Assert\Regex(pattern: '/^#[0-9a-zA-Z]{6}$/')] #[ApiProperty(example: '#4CAF50')] - #[Groups(['Default', 'category:update'])] + #[Groups(['read', 'write'])] public ?string $color = null; /** @@ -106,8 +111,8 @@ class Category extends AbstractContentNodeOwner implements BelongsToCampInterfac * @ORM\Column(type="string", length=1, nullable=false) */ #[Assert\Choice(choices: ['a', 'A', 'i', 'I', '1'])] - #[ApiProperty(default: '1', example: 'a')] - #[Groups(['Default', 'category:update'])] + #[ApiProperty(default: '1', example: '1')] + #[Groups(['read', 'write'])] public string $numberingStyle = '1'; public function __construct() { @@ -156,11 +161,24 @@ public function setRootContentNode(?ContentNode $rootContentNode) { } #[Assert\DisableAutoMapping] + #[Groups(['read'])] public function getRootContentNode(): ?ContentNode { // Getter is here to add annotations to parent class property return $this->rootContentNode; } + /** + * Overridden in order to add annotations. + * + * {@inheritdoc} + * + * @return ContentNode[] + */ + #[Groups(['read'])] + public function getContentNodes(): array { + return parent::getContentNodes(); + } + public function getStyledNumber(int $num): string { switch ($this->numberingStyle) { case 'a': diff --git a/api/src/Entity/ContentNode.php b/api/src/Entity/ContentNode.php index 830a6dcc0e..510ee87d47 100644 --- a/api/src/Entity/ContentNode.php +++ b/api/src/Entity/ContentNode.php @@ -26,15 +26,20 @@ * @ORM\Entity */ #[ApiResource( - collectionOperations: ['get', 'post'], + collectionOperations: [ + 'get', + 'post' => ['denormalization_context' => ['groups' => ['write', 'create']]], + ], itemOperations: [ 'get', 'patch' => [ - 'denormalization_context' => ['groups' => ['contentNode:update']], - 'validation_groups' => ['Default', 'contentNode:update'], + 'denormalization_context' => ['groups' => ['write', 'update']], + 'validation_groups' => ['Default', 'update'], ], 'delete' => ['security' => 'object.owner === null'], - ] + ], + denormalizationContext: ['groups' => ['write']], + normalizationContext: ['groups' => ['read']], )] #[ApiFilter(SearchFilter::class, properties: ['parent'])] class ContentNode extends BaseEntity implements BelongsToCampInterface { @@ -54,6 +59,7 @@ class ContentNode extends BaseEntity implements BelongsToCampInterface { * @ORM\JoinColumn(nullable=true) */ #[ApiProperty(writable: false, example: '/content_nodes/1a2b3c4d')] + #[Groups(['read'])] public ContentNode $root; /** @@ -77,10 +83,10 @@ class ContentNode extends BaseEntity implements BelongsToCampInterface { messageBothNull: 'Must not be null on non-root content nodes.', messageNoneNull: 'Must be null on root content nodes.' )] - #[AssertBelongsToSameOwner(groups: ['contentNode:update'])] - #[AssertNoLoop(groups: ['contentNode:update'])] + #[AssertBelongsToSameOwner(groups: ['update'])] + #[AssertNoLoop(groups: ['update'])] #[ApiProperty(example: '/content_nodes/1a2b3c4d')] - #[Groups(['Default', 'contentNode:update'])] + #[Groups(['read', 'write'])] public ?ContentNode $parent = null; /** @@ -89,6 +95,7 @@ class ContentNode extends BaseEntity implements BelongsToCampInterface { * @ORM\OneToMany(targetEntity="ContentNode", mappedBy="parent", cascade={"remove"}) */ #[ApiProperty(writable: false, example: '["/content_nodes/1a2b3c4d"]')] + #[Groups(['read'])] public Collection $children; /** @@ -98,7 +105,7 @@ class ContentNode extends BaseEntity implements BelongsToCampInterface { * @ORM\Column(type="text", nullable=true) */ #[ApiProperty(example: 'footer')] - #[Groups(['Default', 'contentNode:update'])] + #[Groups(['read', 'write'])] public ?string $slot = null; /** @@ -108,7 +115,7 @@ class ContentNode extends BaseEntity implements BelongsToCampInterface { * @ORM\Column(type="integer", nullable=true) */ #[ApiProperty(example: '0')] - #[Groups(['Default', 'contentNode:update'])] + #[Groups(['read', 'write'])] public ?int $position = null; /** @@ -119,7 +126,7 @@ class ContentNode extends BaseEntity implements BelongsToCampInterface { * @ORM\Column(type="json", nullable=true) */ #[ApiProperty(example: '{}')] - #[Groups(['Default', 'contentNode:update'])] + #[Groups(['read', 'write'])] public ?array $jsonConfig = null; /** @@ -129,7 +136,7 @@ class ContentNode extends BaseEntity implements BelongsToCampInterface { * @ORM\Column(type="text", nullable=true) */ #[ApiProperty(example: 'Schlechtwetterprogramm')] - #[Groups(['Default', 'contentNode:update'])] + #[Groups(['read', 'write'])] public ?string $instanceName = null; /** @@ -141,7 +148,7 @@ class ContentNode extends BaseEntity implements BelongsToCampInterface { * @ORM\JoinColumn(nullable=false) */ #[ApiProperty(example: '/content_types/1a2b3c4d')] - #[Groups(['Default'])] + #[Groups(['read', 'create'])] public ?ContentType $contentType = null; /** @@ -162,6 +169,7 @@ public function __construct() { * The name of the content type of this content node. Read-only, for convenience. */ #[ApiProperty(example: 'SafetyConcept')] + #[Groups(['read'])] public function getContentTypeName(): string { return $this->contentType?->name; } @@ -171,6 +179,7 @@ public function getContentTypeName(): string { */ #[SerializedName('owner')] #[ApiProperty(writable: false, example: '/activities/1a2b3c4d')] + #[Groups(['read'])] public function getRootOwner(): Activity | Category | AbstractContentNodeOwner { return $this->root->owner; } @@ -182,6 +191,7 @@ public function getRootOwner(): Activity | Category | AbstractContentNodeOwner { * @throws Exception when the owner is neither an activity nor a category */ #[ApiProperty(example: '/categories/1a2b3c4d')] + #[Groups(['read'])] public function getOwnerCategory(): Category { $owner = $this->getRootOwner(); diff --git a/api/src/Entity/ContentType.php b/api/src/Entity/ContentType.php index ef7c242b35..bf64e2e80a 100644 --- a/api/src/Entity/ContentType.php +++ b/api/src/Entity/ContentType.php @@ -5,6 +5,7 @@ use ApiPlatform\Core\Annotation\ApiProperty; use ApiPlatform\Core\Annotation\ApiResource; use Doctrine\ORM\Mapping as ORM; +use Symfony\Component\Serializer\Annotation\Groups; /** * Defines a type of content that can be present in a content node tree. A content type @@ -16,6 +17,7 @@ #[ApiResource( collectionOperations: ['get'], itemOperations: ['get'], + normalizationContext: ['groups' => ['read']], )] class ContentType extends BaseEntity { /** @@ -25,6 +27,7 @@ class ContentType extends BaseEntity { * @ORM\Column(type="string", length=32, unique=true) */ #[ApiProperty(writable: false, example: 'SafetyConcept')] + #[Groups(['read'])] public ?string $name = null; /** @@ -33,6 +36,7 @@ class ContentType extends BaseEntity { * @ORM\Column(type="boolean") */ #[ApiProperty(writable: false, example: 'true')] + #[Groups(['read'])] public bool $active = true; /** diff --git a/api/src/Entity/Day.php b/api/src/Entity/Day.php index c8ba0dde6a..ad68f99be3 100644 --- a/api/src/Entity/Day.php +++ b/api/src/Entity/Day.php @@ -48,7 +48,7 @@ class Day extends BaseEntity implements BelongsToCampInterface { * @ORM\ManyToOne(targetEntity="Period", inversedBy="days") * @ORM\JoinColumn(nullable=false, onDelete="cascade") */ - #[ApiProperty(readableLink: false, writable: false, example: '/periods/1a2b3c4d')] + #[ApiProperty(example: '/periods/1a2b3c4d')] #[Groups(['read'])] public ?Period $period = null; diff --git a/api/src/Entity/DayResponsible.php b/api/src/Entity/DayResponsible.php index 4e770c96d2..8060fa4e03 100644 --- a/api/src/Entity/DayResponsible.php +++ b/api/src/Entity/DayResponsible.php @@ -27,6 +27,7 @@ use App\Validator\AssertBelongsToSameCamp; use Doctrine\ORM\Mapping as ORM; use Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntity; +use Symfony\Component\Serializer\Annotation\Groups; use Symfony\Component\Validator\Constraints as Assert; /** @@ -40,6 +41,8 @@ #[ApiResource( collectionOperations: ['get', 'post'], itemOperations: ['get', 'delete'], + denormalizationContext: ['groups' => ['write']], + normalizationContext: ['groups' => ['read']], )] #[ApiFilter(SearchFilter::class, properties: ['day'])] #[UniqueEntity(fields: ['day', 'campCollaboration'])] @@ -52,6 +55,7 @@ class DayResponsible extends BaseEntity implements BelongsToCampInterface { */ #[Assert\NotNull] #[ApiProperty(example: '/days/1a2b3c4d')] + #[Groups(['read', 'write'])] public ?Day $day = null; /** @@ -62,6 +66,7 @@ class DayResponsible extends BaseEntity implements BelongsToCampInterface { */ #[AssertBelongsToSameCamp] #[ApiProperty(example: '/camp_collaborations/1a2b3c4d')] + #[Groups(['read', 'write'])] public ?CampCollaboration $campCollaboration = null; #[ApiProperty(readable: false)] diff --git a/api/src/Entity/MaterialItem.php b/api/src/Entity/MaterialItem.php index c1397d2c95..fdc167e987 100644 --- a/api/src/Entity/MaterialItem.php +++ b/api/src/Entity/MaterialItem.php @@ -10,6 +10,7 @@ use App\Validator\AssertEitherIsNull; use App\Validator\MaterialItemUpdateGroupSequence; use Doctrine\ORM\Mapping as ORM; +use Symfony\Component\Serializer\Annotation\Groups; /** * A physical item that is needed for carrying out a programme or camp. @@ -22,7 +23,9 @@ 'get', 'patch' => ['validation_groups' => MaterialItemUpdateGroupSequence::class], 'delete', - ] + ], + denormalizationContext: ['groups' => ['write']], + normalizationContext: ['groups' => ['read']], )] #[ApiFilter(SearchFilter::class, properties: ['materialList', 'period'])] class MaterialItem extends BaseEntity implements BelongsToCampInterface { @@ -33,8 +36,9 @@ class MaterialItem extends BaseEntity implements BelongsToCampInterface { * @ORM\ManyToOne(targetEntity="MaterialList", inversedBy="materialItems") * @ORM\JoinColumn(nullable=false, onDelete="cascade") */ - #[AssertBelongsToSameCamp(compareToPrevious: true, groups: ['materialItem:update'])] + #[AssertBelongsToSameCamp(compareToPrevious: true, groups: ['update'])] #[ApiProperty(example: '/material_lists/1a2b3c4d')] + #[Groups(['read', 'write'])] public ?MaterialList $materialList = null; /** @@ -46,6 +50,7 @@ class MaterialItem extends BaseEntity implements BelongsToCampInterface { #[AssertBelongsToSameCamp] #[AssertEitherIsNull(other: 'contentNode')] #[ApiProperty(example: '/periods/1a2b3c4d')] + #[Groups(['read', 'write'])] public ?Period $period = null; /** @@ -57,6 +62,7 @@ class MaterialItem extends BaseEntity implements BelongsToCampInterface { #[AssertBelongsToSameCamp] #[AssertEitherisNull(other: 'period')] #[ApiProperty(example: '/content_nodes/1a2b3c4d')] + #[Groups(['read', 'write'])] public ?ContentNode $contentNode = null; /** @@ -65,6 +71,7 @@ class MaterialItem extends BaseEntity implements BelongsToCampInterface { * @ORM\Column(type="text", nullable=false) */ #[ApiProperty(example: 'Volleyball')] + #[Groups(['read', 'write'])] public ?string $article = null; /** @@ -73,6 +80,7 @@ class MaterialItem extends BaseEntity implements BelongsToCampInterface { * @ORM\Column(type="float", nullable=true) */ #[ApiProperty(example: 1.5)] + #[Groups(['read', 'write'])] public ?float $quantity = null; /** @@ -81,6 +89,7 @@ class MaterialItem extends BaseEntity implements BelongsToCampInterface { * @ORM\Column(type="text", nullable=true) */ #[ApiProperty(example: 'kg')] + #[Groups(['read', 'write'])] public ?string $unit = null; #[ApiProperty(readable: false)] diff --git a/api/src/Entity/MaterialList.php b/api/src/Entity/MaterialList.php index 316788ca40..a559bc403d 100644 --- a/api/src/Entity/MaterialList.php +++ b/api/src/Entity/MaterialList.php @@ -18,12 +18,13 @@ * @ORM\Entity */ #[ApiResource( - collectionOperations: ['get', 'post'], - itemOperations: [ + collectionOperations: [ 'get', - 'patch' => ['denormalization_context' => ['groups' => ['materialList:update']]], - 'delete', - ] + 'post' => ['denormalization_context' => ['groups' => ['write', 'create']]], + ], + itemOperations: ['get', 'patch', 'delete'], + denormalizationContext: ['groups' => ['write']], + normalizationContext: ['groups' => ['read']], )] #[ApiFilter(SearchFilter::class, properties: ['camp'])] class MaterialList extends BaseEntity implements BelongsToCampInterface { @@ -33,6 +34,7 @@ class MaterialList extends BaseEntity implements BelongsToCampInterface { * @ORM\OneToMany(targetEntity="MaterialItem", mappedBy="materialList") */ #[ApiProperty(writable: false, example: '["/material_items/1a2b3c4d"]')] + #[Groups(['read'])] public Collection $materialItems; /** @@ -42,7 +44,7 @@ class MaterialList extends BaseEntity implements BelongsToCampInterface { * @ORM\JoinColumn(nullable=false, onDelete="cascade") */ #[ApiProperty(example: '/camps/1a2b3c4d')] - #[Groups(['Default'])] + #[Groups(['read', 'create'])] public ?Camp $camp = null; /** @@ -60,7 +62,7 @@ class MaterialList extends BaseEntity implements BelongsToCampInterface { * @ORM\Column(type="text", nullable=false) */ #[ApiProperty(example: 'Lebensmittel')] - #[Groups(['Default', 'materialList:update'])] + #[Groups(['read', 'write'])] public ?string $name = null; public function __construct() { diff --git a/api/src/Entity/Period.php b/api/src/Entity/Period.php index ed23fc3f27..fbe6b27f43 100644 --- a/api/src/Entity/Period.php +++ b/api/src/Entity/Period.php @@ -74,7 +74,7 @@ class Period extends BaseEntity implements BelongsToCampInterface { * @ORM\ManyToOne(targetEntity="Camp", inversedBy="periods") * @ORM\JoinColumn(nullable=false) */ - #[ApiProperty(readableLink: false, example: '/camps/1a2b3c4d')] + #[ApiProperty(example: '/camps/1a2b3c4d')] #[Groups(['read', 'create'])] public ?Camp $camp = null; diff --git a/api/src/Entity/ScheduleEntry.php b/api/src/Entity/ScheduleEntry.php index a871316d04..cdbae6569c 100644 --- a/api/src/Entity/ScheduleEntry.php +++ b/api/src/Entity/ScheduleEntry.php @@ -20,12 +20,13 @@ * @ORM\Entity */ #[ApiResource( - collectionOperations: ['get', 'post'], - itemOperations: [ + collectionOperations: [ 'get', - 'patch' => ['denormalization_context' => ['groups' => ['scheduleEntry:update']]], - 'delete', - ] + 'post' => ['denormalization_context' => ['groups' => ['write', 'create']]], + ], + itemOperations: ['get', 'patch', 'delete'], + denormalizationContext: ['groups' => ['write']], + normalizationContext: ['groups' => ['read']], )] #[ApiFilter(SearchFilter::class, properties: ['period', 'activity'])] class ScheduleEntry extends BaseEntity implements BelongsToCampInterface { @@ -37,7 +38,7 @@ class ScheduleEntry extends BaseEntity implements BelongsToCampInterface { */ #[AssertBelongsToSameCamp] #[ApiProperty(example: '/periods/1a2b3c4d')] - #[Groups(['Default', 'scheduleEntry:update'])] + #[Groups(['read', 'write'])] public ?Period $period = null; /** @@ -49,7 +50,7 @@ class ScheduleEntry extends BaseEntity implements BelongsToCampInterface { * @ORM\JoinColumn(nullable=false, onDelete="cascade") */ #[ApiProperty(example: '/activities/1a2b3c4d')] - #[Groups(['Default'])] + #[Groups(['read', 'create'])] public ?Activity $activity = null; /** @@ -61,7 +62,7 @@ class ScheduleEntry extends BaseEntity implements BelongsToCampInterface { */ #[Assert\GreaterThanOrEqual(value: 0)] #[ApiProperty(example: 1440)] - #[Groups(['Default', 'scheduleEntry:update'])] + #[Groups(['read', 'write'])] public int $periodOffset = 0; /** @@ -71,7 +72,7 @@ class ScheduleEntry extends BaseEntity implements BelongsToCampInterface { */ #[Assert\GreaterThan(value: 0)] #[ApiProperty(example: 90)] - #[Groups(['Default', 'scheduleEntry:update'])] + #[Groups(['read', 'write'])] public int $length = 0; /** @@ -85,7 +86,7 @@ class ScheduleEntry extends BaseEntity implements BelongsToCampInterface { * --> left is a MariaDB keyword, therefore escaping for column name necessary */ #[ApiProperty(default: 0, example: 0.6)] - #[Groups(['Default', 'scheduleEntry:update'])] + #[Groups(['read', 'write'])] public ?float $left = 0; /** @@ -97,7 +98,7 @@ class ScheduleEntry extends BaseEntity implements BelongsToCampInterface { * @ORM\Column(type="float", nullable=true) */ #[ApiProperty(example: 0.4)] - #[Groups(['Default', 'scheduleEntry:update'])] + #[Groups(['read', 'write'])] public ?float $width = 1; #[ApiProperty(readable: false)] @@ -109,6 +110,7 @@ public function getCamp(): ?Camp { * The day on which this schedule entry starts. */ #[ApiProperty(writable: false, example: '/days/1a2b3c4d')] + #[Groups(['read'])] public function getDay(): Day { $dayNumber = $this->getDayNumber(); @@ -126,6 +128,7 @@ public function getNumberingStyle(): ?string { * The day number of the day on which this schedule entry starts. */ #[ApiProperty(example: '1')] + #[Groups(['read'])] public function getDayNumber(): int { return 1 + floor($this->periodOffset / (24 * 60)); } @@ -136,6 +139,7 @@ public function getDayNumber(): int { * second entry on a given day, its number will be 2. */ #[ApiProperty(example: '2')] + #[Groups(['read'])] public function getScheduleEntryNumber(): int { $dayOffset = floor($this->periodOffset / (24 * 60)) * 24 * 60; @@ -181,6 +185,7 @@ public function getScheduleEntryNumber(): int { * defined by the activity's category. */ #[ApiProperty(example: '1.b')] + #[Groups(['read'])] public function getNumber(): string { $dayNumber = $this->getDayNumber(); $scheduleEntryNumber = $this->getScheduleEntryNumber(); diff --git a/api/src/Entity/User.php b/api/src/Entity/User.php index 3cdceb0d2f..e9661f9302 100644 --- a/api/src/Entity/User.php +++ b/api/src/Entity/User.php @@ -27,18 +27,19 @@ 'post' => [ 'security' => 'true', // allow unauthenticated clients to create (register) users 'input_formats' => ['jsonld', 'jsonapi', 'json'], - 'validation_groups' => ['Default', 'user:create'], + 'validation_groups' => ['Default', 'create'], + 'denormalization_context' => ['groups' => ['write', 'create']], ], ], itemOperations: [ 'get' => ['security' => 'is_granted("ROLE_ADMIN") or object == user'], 'patch' => [ 'security' => 'is_granted("ROLE_ADMIN") or object == user', - 'denormalization_context' => ['groups' => 'user:update'], ], 'delete' => ['security' => 'is_granted("ROLE_ADMIN") and !object.ownsCamps()'], ], - denormalizationContext: ['groups' => ['Default']], + denormalizationContext: ['groups' => ['write']], + normalizationContext: ['groups' => ['read']], )] class User extends BaseEntity implements UserInterface, PasswordAuthenticatedUserInterface { /** @@ -66,7 +67,7 @@ class User extends BaseEntity implements UserInterface, PasswordAuthenticatedUse #[Assert\NotBlank] #[Assert\Email] #[ApiProperty(example: 'bi-pi@example.com')] - #[Groups(['Default', 'user:update'])] + #[Groups(['read', 'write'])] public ?string $email = null; /** @@ -78,7 +79,7 @@ class User extends BaseEntity implements UserInterface, PasswordAuthenticatedUse #[Assert\NotBlank] #[Assert\Regex(pattern: '/^[a-z0-9_.-]+$/')] #[ApiProperty(example: 'bipi')] - #[Groups(['Default'])] + #[Groups(['read', 'create'])] public ?string $username = null; /** @@ -89,7 +90,7 @@ class User extends BaseEntity implements UserInterface, PasswordAuthenticatedUse #[InputFilter\Trim] #[InputFilter\CleanHTML] #[ApiProperty(example: 'Robert')] - #[Groups(['Default', 'user:update'])] + #[Groups(['read', 'write'])] public ?string $firstname = null; /** @@ -100,7 +101,7 @@ class User extends BaseEntity implements UserInterface, PasswordAuthenticatedUse #[InputFilter\Trim] #[InputFilter\CleanHTML] #[ApiProperty(example: 'Baden-Powell')] - #[Groups(['Default', 'user:update'])] + #[Groups(['read', 'write'])] public ?string $surname = null; /** @@ -111,7 +112,7 @@ class User extends BaseEntity implements UserInterface, PasswordAuthenticatedUse #[InputFilter\Trim] #[InputFilter\CleanHTML] #[ApiProperty(example: 'Bi-Pi')] - #[Groups(['Default', 'user:update'])] + #[Groups(['read', 'write'])] public ?string $nickname = null; /** @@ -122,7 +123,7 @@ class User extends BaseEntity implements UserInterface, PasswordAuthenticatedUse #[InputFilter\Trim] #[Assert\Locale] #[ApiProperty(example: 'en')] - #[Groups(['Default', 'user:update'])] + #[Groups(['read', 'write'])] public ?string $language = null; /** @@ -146,10 +147,10 @@ class User extends BaseEntity implements UserInterface, PasswordAuthenticatedUse * A new password for this user. At least 8 characters. */ #[SerializedName('password')] - #[Assert\NotBlank(groups: ['user:create'])] + #[Assert\NotBlank(groups: ['create'])] #[Assert\Length(min: 8)] #[ApiProperty(readable: false, writable: true, example: 'learning-by-doing-101')] - #[Groups(['Default', 'user:update'])] + #[Groups(['read', 'write'])] public ?string $plainPassword = null; public function __construct() { @@ -163,6 +164,7 @@ public function __construct() { * @return null|string */ #[ApiProperty(example: 'Robert Baden-Powell')] + #[Groups(['read'])] public function getDisplayName(): ?string { if (!empty($this->nickname)) { return $this->nickname; diff --git a/api/src/Serializer/PreventAutomaticEmbeddingPropertyMetadataFactory.php b/api/src/Serializer/PreventAutomaticEmbeddingPropertyMetadataFactory.php new file mode 100644 index 0000000000..8a54d2f9f6 --- /dev/null +++ b/api/src/Serializer/PreventAutomaticEmbeddingPropertyMetadataFactory.php @@ -0,0 +1,83 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace App\Serializer; + +use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface; +use ApiPlatform\Core\Metadata\Property\PropertyMetadata; + +/** + * This is meant as a thin wrapper around + * ApiPlatform\Core\Metadata\Property\Factory\SerializerPropertyMetadataFactory. + * That class implements the automatic embedding logic based on serialization groups, + * described in https://api-platform.com/docs/core/serialization/#embedding-relations. + * + * However, we have our own system for serialization groups and embedding entities, + * with general groups 'read', 'write', 'create', 'update' etc. instead of entity- + * specific groups as they are used in the API platform docs ('book', 'book:update'). + * Therefore, we don't want relations to be automatically embedded as soon as there + * is a property with a matching serialization group on the related entity. + * + * As an example, we don't want the author to be embedded in the book here: + * + * #[ApiResource(normalization_context: ['groups' => ['read']])] + * class Book { + * #[Groups('read')] + * public ?Person $author = null; + * } + * + * #[ApiResource(normalization_context: ['groups' => ['read']])] + * class Person { + * #[Groups('read')] + * public string $name = ''; + * } + * + * To prevent the author from being embedded due to just the 'read' group, this + * class should be inserted just around SerializerPropertyMetadataFactory in the + * decorator chain. It will undo only the undesired changes to the property + * metadata that SerializerPropertyMetadataFactory adds. + * + * Currently, the SerializerPropertyMetadataFactory has a decoration priority of + * 30, so this class should be assigned a priority of 29. + * https://github.com/api-platform/core/blob/main/src/Bridge/Symfony/Bundle/Resources/config/metadata/metadata.xml#L65 + */ +final class PreventAutomaticEmbeddingPropertyMetadataFactory implements PropertyMetadataFactoryInterface { + public function __construct(private PropertyMetadataFactoryInterface $decorated) { + } + + /** + * {@inheritdoc} + */ + public function create(string $resourceClass, string $property, array $options = []): PropertyMetadata { + $propertyMetadata = $this->decorated->create($resourceClass, $property, $options); + + return new PropertyMetadata( + $propertyMetadata->getType(), + $propertyMetadata->getDescription(), + $propertyMetadata->isReadable(), + $propertyMetadata->isWritable(), + null, // reset readableLink to null + null, // reset writableLink to null + $propertyMetadata->isRequired(), + $propertyMetadata->isIdentifier(), + $propertyMetadata->getIri(), + $propertyMetadata->getChildInherited(), + $propertyMetadata->getAttributes(), + $propertyMetadata->getSubresource(), + $propertyMetadata->isInitializable(), + $propertyMetadata->getDefault(), + $propertyMetadata->getExample(), + $propertyMetadata->getSchema() + ); + } +} diff --git a/api/src/Validator/MaterialItemUpdateGroupSequence.php b/api/src/Validator/MaterialItemUpdateGroupSequence.php index 2e79219b47..916b61447b 100644 --- a/api/src/Validator/MaterialItemUpdateGroupSequence.php +++ b/api/src/Validator/MaterialItemUpdateGroupSequence.php @@ -7,6 +7,6 @@ class MaterialItemUpdateGroupSequence implements ValidationGroupsGeneratorInterface { public function __invoke($object) { - return new GroupSequence(['materialItem:update', 'Default']); // now, no matter which is first in the class declaration, it will be tested in this order. + return new GroupSequence(['update', 'Default']); // now, no matter which is first in the class declaration, it will be tested in this order. } } diff --git a/api/tests/Api/Activities/CreateActivityTest.php b/api/tests/Api/Activities/CreateActivityTest.php index ee213d4760..b50ee2707c 100644 --- a/api/tests/Api/Activities/CreateActivityTest.php +++ b/api/tests/Api/Activities/CreateActivityTest.php @@ -81,7 +81,11 @@ public function getExampleReadPayload($attributes = [], $except = []) { Activity::class, OperationType::ITEM, 'get', - $attributes, + array_merge([ + '_links' => [ + 'contentNodes' => [], + ], + ], $attributes), ['category'], $except ); diff --git a/api/tests/Api/ECampApiTestCase.php b/api/tests/Api/ECampApiTestCase.php index cd6ad46d9c..171fa8148b 100644 --- a/api/tests/Api/ECampApiTestCase.php +++ b/api/tests/Api/ECampApiTestCase.php @@ -114,7 +114,7 @@ protected function getExamplePayload(string $resourceClass, string $operationTyp $schema = $this->getSchemaFactory()->buildSchema($resourceClass, 'json', 'get' === $operationName ? Schema::TYPE_OUTPUT : Schema::TYPE_INPUT, $operationType, $operationName); preg_match('/\/([^\/]+)$/', $schema['$ref'], $matches); $schemaName = $matches[1]; - $properties = $schema->getDefinitions()[$schemaName]['properties']; + $properties = $schema->getDefinitions()[$schemaName]['properties'] ?? []; $writableProperties = array_filter($properties, fn ($property) => !($property['readOnly'] ?? false)); $writablePropertiesWithExample = array_filter($writableProperties, fn ($property) => ($property['example'] ?? false)); $examples = array_map(fn ($property) => $property['example'] ?? $property['default'] ?? null, $writablePropertiesWithExample); diff --git a/api/tests/Serializer/PreventAutomaticEmbeddingPropertyMetadataFactoryTest.php b/api/tests/Serializer/PreventAutomaticEmbeddingPropertyMetadataFactoryTest.php new file mode 100644 index 0000000000..9d0df4ce39 --- /dev/null +++ b/api/tests/Serializer/PreventAutomaticEmbeddingPropertyMetadataFactoryTest.php @@ -0,0 +1,66 @@ +createMock(PropertyMetadataFactoryInterface::class); + $propertyMetadata = new PropertyMetadata( + $this->createMock(Type::class), + 'description', + true, + true, + true, + true, + true, + true, + '/iri/of/entity', + null, + [], + new SubresourceMetadata(Dummy::class, true, 3), + true, + null, + null, + [] + ); + $decorated->expects($this->once()) + ->method('create') + ->willReturn($propertyMetadata) + ; + $factory = new PreventAutomaticEmbeddingPropertyMetadataFactory($decorated); + + // when + $result = $factory->create(Dummy::class, 'myProperty', []); + + // then + $this->assertEquals($propertyMetadata->getType(), $result->getType()); + $this->assertEquals($propertyMetadata->getDescription(), $result->getDescription()); + $this->assertEquals($propertyMetadata->isReadable(), $result->isReadable()); + $this->assertEquals($propertyMetadata->isWritable(), $result->isWritable()); + $this->assertEquals(null, $result->isReadableLink()); + $this->assertEquals(null, $result->isWritableLink()); + $this->assertEquals($propertyMetadata->isRequired(), $result->isRequired()); + $this->assertEquals($propertyMetadata->isIdentifier(), $result->isIdentifier()); + $this->assertEquals($propertyMetadata->getIri(), $result->getIri()); + $this->assertEquals($propertyMetadata->getAttributes(), $result->getAttributes()); + $this->assertEquals($propertyMetadata->getSubresource(), $result->getSubresource()); + $this->assertEquals($propertyMetadata->isInitializable(), $result->isInitializable()); + $this->assertEquals($propertyMetadata->getDefault(), $result->getDefault()); + $this->assertEquals($propertyMetadata->getExample(), $result->getExample()); + $this->assertEquals($propertyMetadata->getSchema(), $result->getSchema()); + } +} + +class Dummy { +} From 5b029cdb39ff15f362af28ac620735083dd2221d Mon Sep 17 00:00:00 2001 From: Carlo Beltrame Date: Mon, 9 Aug 2021 13:18:49 +0200 Subject: [PATCH 18/23] Temporarily replace API platform's eager loading extension This is until https://github.com/api-platform/core/pull/4377 is merged, see that PR and https://github.com/api-platform/core/issues/2444 for details. --- api/config/services.yaml | 9 +- api/src/Doctrine/EagerLoadingExtension.php | 314 +++++++++++++++++++++ 2 files changed, 322 insertions(+), 1 deletion(-) create mode 100644 api/src/Doctrine/EagerLoadingExtension.php diff --git a/api/config/services.yaml b/api/config/services.yaml index 0e8c649e1f..f358a887b1 100644 --- a/api/config/services.yaml +++ b/api/config/services.yaml @@ -78,4 +78,11 @@ services: decoration_priority: 29 App\OpenApi\JwtDecorator: - decorates: 'api_platform.openapi.factory' \ No newline at end of file + decorates: 'api_platform.openapi.factory' + + ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\EagerLoadingExtension: + class: App\Doctrine\EagerLoadingExtension + api_platform.doctrine.orm.query_extension.eager_loading: + class: App\Doctrine\EagerLoadingExtension + App\Doctrine\EagerLoadingExtension: + public: false diff --git a/api/src/Doctrine/EagerLoadingExtension.php b/api/src/Doctrine/EagerLoadingExtension.php new file mode 100644 index 0000000000..2a7b15eca7 --- /dev/null +++ b/api/src/Doctrine/EagerLoadingExtension.php @@ -0,0 +1,314 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace App\Doctrine; + +use ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\ContextAwareQueryCollectionExtensionInterface; +use ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\QueryItemExtensionInterface; +use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\EagerLoadingTrait; +use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryBuilderHelper; +use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface; +use ApiPlatform\Core\Exception\InvalidArgumentException; +use ApiPlatform\Core\Exception\PropertyNotFoundException; +use ApiPlatform\Core\Exception\ResourceClassNotFoundException; +use ApiPlatform\Core\Exception\RuntimeException; +use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface; +use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface; +use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; +use ApiPlatform\Core\Serializer\SerializerContextBuilderInterface; +use Doctrine\ORM\Mapping\ClassMetadata; +use Doctrine\ORM\Mapping\ClassMetadataInfo; +use Doctrine\ORM\Query\Expr\Join; +use Doctrine\ORM\QueryBuilder; +use Symfony\Component\HttpFoundation\RequestStack; +use Symfony\Component\Serializer\Mapping\Factory\ClassMetadataFactoryInterface; +use Symfony\Component\Serializer\Normalizer\AbstractNormalizer; +use Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer; + +/** + * Eager loads relations. + * + * @author Charles Sarrazin + * @author Kévin Dunglas + * @author Antoine Bluchet + * @author Baptiste Meyer + */ +final class EagerLoadingExtension implements ContextAwareQueryCollectionExtensionInterface, QueryItemExtensionInterface { + use EagerLoadingTrait; + + private $propertyNameCollectionFactory; + private $propertyMetadataFactory; + private $classMetadataFactory; + private $maxJoins; + private $serializerContextBuilder; + private $requestStack; + + /** + * @TODO move $fetchPartial after $forceEager (@soyuka) in 3.0 + */ + public function __construct(PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory, ResourceMetadataFactoryInterface $resourceMetadataFactory, int $maxJoins = 30, bool $forceEager = true, RequestStack $requestStack = null, SerializerContextBuilderInterface $serializerContextBuilder = null, bool $fetchPartial = false, ClassMetadataFactoryInterface $classMetadataFactory = null) { + if (null !== $this->requestStack) { + @trigger_error(sprintf('Passing an instance of "%s" is deprecated since version 2.2 and will be removed in 3.0. Use the data provider\'s context instead.', RequestStack::class), \E_USER_DEPRECATED); + } + if (null !== $this->serializerContextBuilder) { + @trigger_error(sprintf('Passing an instance of "%s" is deprecated since version 2.2 and will be removed in 3.0. Use the data provider\'s context instead.', SerializerContextBuilderInterface::class), \E_USER_DEPRECATED); + } + + $this->propertyNameCollectionFactory = $propertyNameCollectionFactory; + $this->propertyMetadataFactory = $propertyMetadataFactory; + $this->resourceMetadataFactory = $resourceMetadataFactory; + $this->classMetadataFactory = $classMetadataFactory; + $this->maxJoins = $maxJoins; + $this->forceEager = $forceEager; + $this->fetchPartial = $fetchPartial; + $this->serializerContextBuilder = $serializerContextBuilder; + $this->requestStack = $requestStack; + } + + /** + * {@inheritdoc} + */ + public function applyToCollection(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass = null, string $operationName = null, array $context = []) { + $this->apply(true, $queryBuilder, $queryNameGenerator, $resourceClass, $operationName, $context); + } + + /** + * {@inheritdoc} + * + * The context may contain serialization groups which helps defining joined entities that are readable. + */ + public function applyToItem(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, array $identifiers, string $operationName = null, array $context = []) { + $this->apply(false, $queryBuilder, $queryNameGenerator, $resourceClass, $operationName, $context); + } + + private function apply(bool $collection, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, ?string $resourceClass, ?string $operationName, array $context) { + if (null === $resourceClass) { + throw new InvalidArgumentException('The "$resourceClass" parameter must not be null'); + } + + $options = []; + if (null !== $operationName) { + $options[($collection ? 'collection' : 'item').'_operation_name'] = $operationName; + } + + $forceEager = $this->shouldOperationForceEager($resourceClass, $options); + $fetchPartial = $this->shouldOperationFetchPartial($resourceClass, $options); + + if (!isset($context['groups']) && !isset($context['attributes'])) { + $contextType = isset($context['api_denormalize']) ? 'denormalization_context' : 'normalization_context'; + $context += $this->getNormalizationContext($context['resource_class'] ?? $resourceClass, $contextType, $options); + } + + if (empty($context[AbstractNormalizer::GROUPS]) && !isset($context[AbstractNormalizer::ATTRIBUTES])) { + return; + } + + if (!empty($context[AbstractNormalizer::GROUPS])) { + $options['serializer_groups'] = (array) $context[AbstractNormalizer::GROUPS]; + } + + $this->joinRelations($queryBuilder, $queryNameGenerator, $resourceClass, $forceEager, $fetchPartial, $queryBuilder->getRootAliases()[0], $options, $context); + } + + /** + * Joins relations to eager load. + * + * @param bool $wasLeftJoin if the relation containing the new one had a left join, we have to force the new one to left join too + * @param int $joinCount the number of joins + * @param int $currentDepth the current max depth + * + * @throws RuntimeException when the max number of joins has been reached + */ + private function joinRelations(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, bool $forceEager, bool $fetchPartial, string $parentAlias, array $options = [], array $normalizationContext = [], bool $wasLeftJoin = false, int &$joinCount = 0, int $currentDepth = null, string $parentAssociation = null) { + if ($joinCount > $this->maxJoins) { + throw new RuntimeException('The total number of joined relations has exceeded the specified maximum. Raise the limit if necessary with the "api_platform.eager_loading.max_joins" configuration key (https://api-platform.com/docs/core/performance/#eager-loading), or limit the maximum serialization depth using the "enable_max_depth" option of the Symfony serializer (https://symfony.com/doc/current/components/serializer.html#handling-serialization-depth).'); + } + + $currentDepth = $currentDepth > 0 ? $currentDepth - 1 : $currentDepth; + $entityManager = $queryBuilder->getEntityManager(); + $classMetadata = $entityManager->getClassMetadata($resourceClass); + $attributesMetadata = $this->classMetadataFactory ? $this->classMetadataFactory->getMetadataFor($resourceClass)->getAttributesMetadata() : null; + + foreach ($classMetadata->associationMappings as $association => $mapping) { + //Don't join if max depth is enabled and the current depth limit is reached + if (0 === $currentDepth && ($normalizationContext[AbstractObjectNormalizer::ENABLE_MAX_DEPTH] ?? false)) { + continue; + } + + try { + $propertyMetadata = $this->propertyMetadataFactory->create($resourceClass, $association, $options); + } catch (PropertyNotFoundException $propertyNotFoundException) { + //skip properties not found + continue; + } catch (ResourceClassNotFoundException $resourceClassNotFoundException) { + //skip associations that are not resource classes + continue; + } + + if ( + // Always skip extra lazy associations + ClassMetadataInfo::FETCH_EXTRA_LAZY === $mapping['fetch'] + // We don't want to interfere with doctrine on this association + || (false === $forceEager && ClassMetadataInfo::FETCH_EAGER !== $mapping['fetch']) + ) { + continue; + } + + // prepare the child context + $childNormalizationContext = $normalizationContext; + if (isset($normalizationContext[AbstractNormalizer::ATTRIBUTES])) { + if ($inAttributes = isset($normalizationContext[AbstractNormalizer::ATTRIBUTES][$association])) { + $childNormalizationContext[AbstractNormalizer::ATTRIBUTES] = $normalizationContext[AbstractNormalizer::ATTRIBUTES][$association]; + } + } else { + $inAttributes = null; + } + + if ( + (null === $fetchEager = $propertyMetadata->getAttribute('fetch_eager')) + && (null !== $fetchEager = $propertyMetadata->getAttribute('fetchEager')) + ) { + @trigger_error('The "fetchEager" attribute is deprecated since 2.3. Please use "fetch_eager" instead.', \E_USER_DEPRECATED); + } + + if (false === $fetchEager) { + continue; + } + + if (true !== $fetchEager && (false === $propertyMetadata->isReadable() || false === $inAttributes)) { + continue; + } + + // Avoid joining back to the parent that we just came from, but only on *ToOne relations + if ( + null !== $parentAssociation + && isset($mapping['inversedBy']) + && $mapping['inversedBy'] === $parentAssociation + && $mapping['type'] & ClassMetadata::TO_ONE + ) { + continue; + } + + $existingJoin = QueryBuilderHelper::getExistingJoin($queryBuilder, $parentAlias, $association); + + if (null !== $existingJoin) { + $associationAlias = $existingJoin->getAlias(); + $isLeftJoin = Join::LEFT_JOIN === $existingJoin->getJoinType(); + } else { + $isNullable = $mapping['joinColumns'][0]['nullable'] ?? true; + $isLeftJoin = false !== $wasLeftJoin || true === $isNullable; + $method = $isLeftJoin ? 'leftJoin' : 'innerJoin'; + + $associationAlias = $queryNameGenerator->generateJoinAlias($association); + $queryBuilder->{$method}(sprintf('%s.%s', $parentAlias, $association), $associationAlias); + ++$joinCount; + } + + if (true === $fetchPartial) { + try { + $this->addSelect($queryBuilder, $mapping['targetEntity'], $associationAlias, $options); + } catch (ResourceClassNotFoundException $resourceClassNotFoundException) { + continue; + } + } else { + $queryBuilder->addSelect($associationAlias); + } + + // Avoid recursive joins for self-referencing relations + if ($mapping['targetEntity'] === $resourceClass) { + continue; + } + + // Only join the relation's relations recursively if it's a readableLink + if (true !== $fetchEager && (true !== $propertyMetadata->isReadableLink())) { + continue; + } + + if (isset($attributesMetadata[$association])) { + $maxDepth = $attributesMetadata[$association]->getMaxDepth(); + + // The current depth is the lowest max depth available in the ancestor tree. + if (null !== $maxDepth && (null === $currentDepth || $maxDepth < $currentDepth)) { + $currentDepth = $maxDepth; + } + } + + $this->joinRelations($queryBuilder, $queryNameGenerator, $mapping['targetEntity'], $forceEager, $fetchPartial, $associationAlias, $options, $childNormalizationContext, $isLeftJoin, $joinCount, $currentDepth, $association); + } + } + + private function addSelect(QueryBuilder $queryBuilder, string $entity, string $associationAlias, array $propertyMetadataOptions) { + $select = []; + $entityManager = $queryBuilder->getEntityManager(); + $targetClassMetadata = $entityManager->getClassMetadata($entity); + if (!empty($targetClassMetadata->subClasses)) { + $queryBuilder->addSelect($associationAlias); + + return; + } + + foreach ($this->propertyNameCollectionFactory->create($entity) as $property) { + $propertyMetadata = $this->propertyMetadataFactory->create($entity, $property, $propertyMetadataOptions); + + if (true === $propertyMetadata->isIdentifier()) { + $select[] = $property; + + continue; + } + + // If it's an embedded property see below + if (!\array_key_exists($property, $targetClassMetadata->embeddedClasses)) { + //the field test allows to add methods to a Resource which do not reflect real database fields + if ($targetClassMetadata->hasField($property) && (true === $propertyMetadata->getAttribute('fetchable') || $propertyMetadata->isReadable())) { + $select[] = $property; + } + + continue; + } + + // It's an embedded property, select relevant subfields + foreach ($this->propertyNameCollectionFactory->create($targetClassMetadata->embeddedClasses[$property]['class']) as $embeddedProperty) { + $propertyMetadata = $this->propertyMetadataFactory->create($entity, $property, $propertyMetadataOptions); + $propertyName = "{$property}.{$embeddedProperty}"; + if ($targetClassMetadata->hasField($propertyName) && (true === $propertyMetadata->getAttribute('fetchable') || $propertyMetadata->isReadable())) { + $select[] = $propertyName; + } + } + } + + $queryBuilder->addSelect(sprintf('partial %s.{%s}', $associationAlias, implode(',', $select))); + } + + /** + * Gets the serializer context. + * + * @param string $contextType normalization_context or denormalization_context + * @param array $options represents the operation name so that groups are the one of the specific operation + */ + private function getNormalizationContext(string $resourceClass, string $contextType, array $options): array { + if (null !== $this->requestStack && null !== $this->serializerContextBuilder && null !== $request = $this->requestStack->getCurrentRequest()) { + return $this->serializerContextBuilder->createFromRequest($request, 'normalization_context' === $contextType); + } + + $resourceMetadata = $this->resourceMetadataFactory->create($resourceClass); + if (isset($options['collection_operation_name'])) { + $context = $resourceMetadata->getCollectionOperationAttribute($options['collection_operation_name'], $contextType, null, true); + } elseif (isset($options['item_operation_name'])) { + $context = $resourceMetadata->getItemOperationAttribute($options['item_operation_name'], $contextType, null, true); + } else { + $context = $resourceMetadata->getAttribute($contextType); + } + + return $context ?? []; + } +} From 4e08a3ee8a163e79d5c29056b33d5d0e4fa95129 Mon Sep 17 00:00:00 2001 From: Pirmin Mattmann Date: Tue, 17 Aug 2021 21:44:17 +0200 Subject: [PATCH 19/23] Const for NormalizationCtx; use SwaggerDefName --- api/src/Entity/Camp.php | 11 ++++++++--- api/src/Entity/Period.php | 9 ++++++--- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/api/src/Entity/Camp.php b/api/src/Entity/Camp.php index 4a7a7d9a47..29eb19c0e5 100644 --- a/api/src/Entity/Camp.php +++ b/api/src/Entity/Camp.php @@ -27,18 +27,18 @@ 'input_formats' => ['jsonld', 'jsonapi', 'json'], 'validation_groups' => ['Default', 'create'], 'denormalization_context' => ['groups' => ['write', 'create']], - 'normalization_context' => ['groups' => ['read', 'Camp:Periods', 'Period:Days']], + 'normalization_context' => Camp::ITEM_NORMALIZATION_CONTEXT, ], ], itemOperations: [ 'get' => [ 'security' => 'object.owner == user or is_granted("ROLE_ADMIN")', - 'normalization_context' => ['groups' => ['read', 'Camp:Periods', 'Period:Days']], + 'normalization_context' => Camp::ITEM_NORMALIZATION_CONTEXT, ], 'patch' => [ 'security' => 'object.owner == user or is_granted("ROLE_ADMIN")', 'denormalization_context' => ['groups' => ['write', 'update']], - 'normalization_context' => ['groups' => ['read', 'Camp:Periods', 'Period:Days']], + 'normalization_context' => Camp::ITEM_NORMALIZATION_CONTEXT, ], 'delete' => ['security' => 'object.owner == user or is_granted("ROLE_ADMIN")'], ], @@ -46,6 +46,11 @@ normalizationContext: ['groups' => ['read']], )] class Camp extends BaseEntity implements BelongsToCampInterface { + public const ITEM_NORMALIZATION_CONTEXT = [ + 'groups' => ['read', 'Camp:Periods', 'Period:Days'], + 'swagger_definition_name' => 'read', + ]; + /** * @ORM\OneToMany(targetEntity="CampCollaboration", mappedBy="camp", orphanRemoval=true) */ diff --git a/api/src/Entity/Period.php b/api/src/Entity/Period.php index fbe6b27f43..7fd1940f53 100644 --- a/api/src/Entity/Period.php +++ b/api/src/Entity/Period.php @@ -26,9 +26,7 @@ 'post' => ['denormalization_context' => ['groups' => ['write', 'create']]], ], itemOperations: [ - 'get' => [ - 'normalization_context' => ['groups' => ['read', 'Period:Camp', 'Period:Days']], - ], + 'get' => ['normalization_context' => Period::ITEM_NORMALIZATION_CONTEXT], 'patch', 'delete', ], @@ -37,6 +35,11 @@ )] #[ApiFilter(SearchFilter::class, properties: ['camp'])] class Period extends BaseEntity implements BelongsToCampInterface { + public const ITEM_NORMALIZATION_CONTEXT = [ + 'groups' => ['read', 'Period:Camp', 'Period:Days'], + 'swagger_definition_name' => 'read', + ]; + /** * The days in this time period. These are generated automatically. * From b0cb899808be4d3047e249e8b4fb9190c1636e47 Mon Sep 17 00:00:00 2001 From: Pirmin Mattmann Date: Mon, 30 Aug 2021 22:31:49 +0200 Subject: [PATCH 20/23] prevent adding same alias twice; doctrine does not like it --- api/src/Doctrine/EagerLoadingExtension.php | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/api/src/Doctrine/EagerLoadingExtension.php b/api/src/Doctrine/EagerLoadingExtension.php index 2a7b15eca7..35bfdf31d3 100644 --- a/api/src/Doctrine/EagerLoadingExtension.php +++ b/api/src/Doctrine/EagerLoadingExtension.php @@ -28,6 +28,7 @@ use ApiPlatform\Core\Serializer\SerializerContextBuilderInterface; use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\Mapping\ClassMetadataInfo; +use Doctrine\ORM\Query\Expr; use Doctrine\ORM\Query\Expr\Join; use Doctrine\ORM\QueryBuilder; use Symfony\Component\HttpFoundation\RequestStack; @@ -221,7 +222,23 @@ private function joinRelations(QueryBuilder $queryBuilder, QueryNameGeneratorInt continue; } } else { - $queryBuilder->addSelect($associationAlias); + $addSelect = true; + + $dqlSelects = $queryBuilder->getDQLPart('select'); + foreach ($dqlSelects as $dqlSelect) { + if ($dqlSelect instanceof Expr\Select) { + $parts = $dqlSelect->getParts(); + if (in_array($associationAlias, $parts)) { + $addSelect = false; + + break; + } + } + } + + if ($addSelect) { + $queryBuilder->addSelect($associationAlias); + } } // Avoid recursive joins for self-referencing relations From e69b947a3f5afe4de185a70c36bd6e735c1812de Mon Sep 17 00:00:00 2001 From: Pirmin Mattmann Date: Mon, 30 Aug 2021 23:00:01 +0200 Subject: [PATCH 21/23] use self for ITEM_NORMALIZATION_CONTEXT --- api/src/Entity/Camp.php | 6 +++--- api/src/Entity/Period.php | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/api/src/Entity/Camp.php b/api/src/Entity/Camp.php index 29eb19c0e5..1d3ee19140 100644 --- a/api/src/Entity/Camp.php +++ b/api/src/Entity/Camp.php @@ -27,18 +27,18 @@ 'input_formats' => ['jsonld', 'jsonapi', 'json'], 'validation_groups' => ['Default', 'create'], 'denormalization_context' => ['groups' => ['write', 'create']], - 'normalization_context' => Camp::ITEM_NORMALIZATION_CONTEXT, + 'normalization_context' => self::ITEM_NORMALIZATION_CONTEXT, ], ], itemOperations: [ 'get' => [ 'security' => 'object.owner == user or is_granted("ROLE_ADMIN")', - 'normalization_context' => Camp::ITEM_NORMALIZATION_CONTEXT, + 'normalization_context' => self::ITEM_NORMALIZATION_CONTEXT, ], 'patch' => [ 'security' => 'object.owner == user or is_granted("ROLE_ADMIN")', 'denormalization_context' => ['groups' => ['write', 'update']], - 'normalization_context' => Camp::ITEM_NORMALIZATION_CONTEXT, + 'normalization_context' => self::ITEM_NORMALIZATION_CONTEXT, ], 'delete' => ['security' => 'object.owner == user or is_granted("ROLE_ADMIN")'], ], diff --git a/api/src/Entity/Period.php b/api/src/Entity/Period.php index 7fd1940f53..7acd619fc8 100644 --- a/api/src/Entity/Period.php +++ b/api/src/Entity/Period.php @@ -26,7 +26,7 @@ 'post' => ['denormalization_context' => ['groups' => ['write', 'create']]], ], itemOperations: [ - 'get' => ['normalization_context' => Period::ITEM_NORMALIZATION_CONTEXT], + 'get' => ['normalization_context' => self::ITEM_NORMALIZATION_CONTEXT], 'patch', 'delete', ], From 476731b4f3199835072da4348bb2d56c23ee9694 Mon Sep 17 00:00:00 2001 From: Carlo Beltrame Date: Sat, 4 Sep 2021 13:13:13 +0200 Subject: [PATCH 22/23] refactor: Extract method for preventing duplicate selects --- api/src/Doctrine/EagerLoadingExtension.php | 32 +++++++++------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/api/src/Doctrine/EagerLoadingExtension.php b/api/src/Doctrine/EagerLoadingExtension.php index 35bfdf31d3..6dc4598513 100644 --- a/api/src/Doctrine/EagerLoadingExtension.php +++ b/api/src/Doctrine/EagerLoadingExtension.php @@ -28,8 +28,8 @@ use ApiPlatform\Core\Serializer\SerializerContextBuilderInterface; use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\Mapping\ClassMetadataInfo; -use Doctrine\ORM\Query\Expr; use Doctrine\ORM\Query\Expr\Join; +use Doctrine\ORM\Query\Expr\Select; use Doctrine\ORM\QueryBuilder; use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\Serializer\Mapping\Factory\ClassMetadataFactoryInterface; @@ -222,23 +222,7 @@ private function joinRelations(QueryBuilder $queryBuilder, QueryNameGeneratorInt continue; } } else { - $addSelect = true; - - $dqlSelects = $queryBuilder->getDQLPart('select'); - foreach ($dqlSelects as $dqlSelect) { - if ($dqlSelect instanceof Expr\Select) { - $parts = $dqlSelect->getParts(); - if (in_array($associationAlias, $parts)) { - $addSelect = false; - - break; - } - } - } - - if ($addSelect) { - $queryBuilder->addSelect($associationAlias); - } + $this->addSelectOnce($queryBuilder, $associationAlias); } // Avoid recursive joins for self-referencing relations @@ -269,7 +253,7 @@ private function addSelect(QueryBuilder $queryBuilder, string $entity, string $a $entityManager = $queryBuilder->getEntityManager(); $targetClassMetadata = $entityManager->getClassMetadata($entity); if (!empty($targetClassMetadata->subClasses)) { - $queryBuilder->addSelect($associationAlias); + $this->addSelectOnce($queryBuilder, $associationAlias); return; } @@ -306,6 +290,16 @@ private function addSelect(QueryBuilder $queryBuilder, string $entity, string $a $queryBuilder->addSelect(sprintf('partial %s.{%s}', $associationAlias, implode(',', $select))); } + private function addSelectOnce(QueryBuilder $queryBuilder, string $alias) { + $existingSelects = array_reduce($queryBuilder->getDQLPart('select') ?? [], function ($existing, $dqlSelect) { + return ($dqlSelect instanceof Select) ? array_merge($existing, $dqlSelect->getParts()) : $existing; + }, []); + + if (!\in_array($alias, $existingSelects, true)) { + $queryBuilder->addSelect($alias); + } + } + /** * Gets the serializer context. * From a62928f391f99650a9870a27f607fbcc57296119 Mon Sep 17 00:00:00 2001 From: Carlo Beltrame Date: Tue, 7 Sep 2021 14:50:52 +0200 Subject: [PATCH 23/23] Comment out unused code to prevent lowering the code coverage too much The EagerLoadingExtension is only temporarily in our codebase, until https://github.com/api-platform/core/pull/4377 is merged and released. There are tests for this extension in API Platform core, so we shouldn't need to write our own test suite for it. However, because of our limited use cases so far, this class lowers the coverage too much. Therefore, I commented out some code of which I am certain we won't need it in the near future. Less untested code = higher coverage. --- api/src/Doctrine/EagerLoadingExtension.php | 105 +++++++++++---------- 1 file changed, 54 insertions(+), 51 deletions(-) diff --git a/api/src/Doctrine/EagerLoadingExtension.php b/api/src/Doctrine/EagerLoadingExtension.php index 6dc4598513..da701a8013 100644 --- a/api/src/Doctrine/EagerLoadingExtension.php +++ b/api/src/Doctrine/EagerLoadingExtension.php @@ -58,12 +58,13 @@ final class EagerLoadingExtension implements ContextAwareQueryCollectionExtensio * @TODO move $fetchPartial after $forceEager (@soyuka) in 3.0 */ public function __construct(PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory, ResourceMetadataFactoryInterface $resourceMetadataFactory, int $maxJoins = 30, bool $forceEager = true, RequestStack $requestStack = null, SerializerContextBuilderInterface $serializerContextBuilder = null, bool $fetchPartial = false, ClassMetadataFactoryInterface $classMetadataFactory = null) { - if (null !== $this->requestStack) { - @trigger_error(sprintf('Passing an instance of "%s" is deprecated since version 2.2 and will be removed in 3.0. Use the data provider\'s context instead.', RequestStack::class), \E_USER_DEPRECATED); - } - if (null !== $this->serializerContextBuilder) { - @trigger_error(sprintf('Passing an instance of "%s" is deprecated since version 2.2 and will be removed in 3.0. Use the data provider\'s context instead.', SerializerContextBuilderInterface::class), \E_USER_DEPRECATED); - } + // Commented out to prevent lowering the code coverage + //if (null !== $this->requestStack) { + // @trigger_error(sprintf('Passing an instance of "%s" is deprecated since version 2.2 and will be removed in 3.0. Use the data provider\'s context instead.', RequestStack::class), \E_USER_DEPRECATED); + //} + //if (null !== $this->serializerContextBuilder) { + // @trigger_error(sprintf('Passing an instance of "%s" is deprecated since version 2.2 and will be removed in 3.0. Use the data provider\'s context instead.', SerializerContextBuilderInterface::class), \E_USER_DEPRECATED); + //} $this->propertyNameCollectionFactory = $propertyNameCollectionFactory; $this->propertyMetadataFactory = $propertyMetadataFactory; @@ -93,9 +94,10 @@ public function applyToItem(QueryBuilder $queryBuilder, QueryNameGeneratorInterf } private function apply(bool $collection, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, ?string $resourceClass, ?string $operationName, array $context) { - if (null === $resourceClass) { - throw new InvalidArgumentException('The "$resourceClass" parameter must not be null'); - } + // Commented out to prevent lowering the code coverage + //if (null === $resourceClass) { + // throw new InvalidArgumentException('The "$resourceClass" parameter must not be null'); + //} $options = []; if (null !== $operationName) { @@ -217,7 +219,8 @@ private function joinRelations(QueryBuilder $queryBuilder, QueryNameGeneratorInt if (true === $fetchPartial) { try { - $this->addSelect($queryBuilder, $mapping['targetEntity'], $associationAlias, $options); + throw new RuntimeException('Partial fetching is disabled in eCamp, to prevent lowering the code coverage due to a feature we don\'t use'); + //$this->addSelect($queryBuilder, $mapping['targetEntity'], $associationAlias, $options); } catch (ResourceClassNotFoundException $resourceClassNotFoundException) { continue; } @@ -248,47 +251,47 @@ private function joinRelations(QueryBuilder $queryBuilder, QueryNameGeneratorInt } } - private function addSelect(QueryBuilder $queryBuilder, string $entity, string $associationAlias, array $propertyMetadataOptions) { - $select = []; - $entityManager = $queryBuilder->getEntityManager(); - $targetClassMetadata = $entityManager->getClassMetadata($entity); - if (!empty($targetClassMetadata->subClasses)) { - $this->addSelectOnce($queryBuilder, $associationAlias); - - return; - } - - foreach ($this->propertyNameCollectionFactory->create($entity) as $property) { - $propertyMetadata = $this->propertyMetadataFactory->create($entity, $property, $propertyMetadataOptions); - - if (true === $propertyMetadata->isIdentifier()) { - $select[] = $property; - - continue; - } - - // If it's an embedded property see below - if (!\array_key_exists($property, $targetClassMetadata->embeddedClasses)) { - //the field test allows to add methods to a Resource which do not reflect real database fields - if ($targetClassMetadata->hasField($property) && (true === $propertyMetadata->getAttribute('fetchable') || $propertyMetadata->isReadable())) { - $select[] = $property; - } - - continue; - } - - // It's an embedded property, select relevant subfields - foreach ($this->propertyNameCollectionFactory->create($targetClassMetadata->embeddedClasses[$property]['class']) as $embeddedProperty) { - $propertyMetadata = $this->propertyMetadataFactory->create($entity, $property, $propertyMetadataOptions); - $propertyName = "{$property}.{$embeddedProperty}"; - if ($targetClassMetadata->hasField($propertyName) && (true === $propertyMetadata->getAttribute('fetchable') || $propertyMetadata->isReadable())) { - $select[] = $propertyName; - } - } - } - - $queryBuilder->addSelect(sprintf('partial %s.{%s}', $associationAlias, implode(',', $select))); - } +// private function addSelect(QueryBuilder $queryBuilder, string $entity, string $associationAlias, array $propertyMetadataOptions) { +// $select = []; +// $entityManager = $queryBuilder->getEntityManager(); +// $targetClassMetadata = $entityManager->getClassMetadata($entity); +// if (!empty($targetClassMetadata->subClasses)) { +// $this->addSelectOnce($queryBuilder, $associationAlias); +// +// return; +// } +// +// foreach ($this->propertyNameCollectionFactory->create($entity) as $property) { +// $propertyMetadata = $this->propertyMetadataFactory->create($entity, $property, $propertyMetadataOptions); +// +// if (true === $propertyMetadata->isIdentifier()) { +// $select[] = $property; +// +// continue; +// } +// +// // If it's an embedded property see below +// if (!\array_key_exists($property, $targetClassMetadata->embeddedClasses)) { +// //the field test allows to add methods to a Resource which do not reflect real database fields +// if ($targetClassMetadata->hasField($property) && (true === $propertyMetadata->getAttribute('fetchable') || $propertyMetadata->isReadable())) { +// $select[] = $property; +// } +// +// continue; +// } +// +// // It's an embedded property, select relevant subfields +// foreach ($this->propertyNameCollectionFactory->create($targetClassMetadata->embeddedClasses[$property]['class']) as $embeddedProperty) { +// $propertyMetadata = $this->propertyMetadataFactory->create($entity, $property, $propertyMetadataOptions); +// $propertyName = "{$property}.{$embeddedProperty}"; +// if ($targetClassMetadata->hasField($propertyName) && (true === $propertyMetadata->getAttribute('fetchable') || $propertyMetadata->isReadable())) { +// $select[] = $propertyName; +// } +// } +// } +// +// $queryBuilder->addSelect(sprintf('partial %s.{%s}', $associationAlias, implode(',', $select))); +// } private function addSelectOnce(QueryBuilder $queryBuilder, string $alias) { $existingSelects = array_reduce($queryBuilder->getDQLPart('select') ?? [], function ($existing, $dqlSelect) {