Hotfix for nullable types (#625)

* Handle nullable type errors Make sure no null values are set in the array Make setter accept nullable string Add test for privacy statements to be filled or ignored * Make certificate nullable * Make certificate nullable
SURFnet · Mar 19, 2024 · f4ce492 · f4ce492
1 parent 9509f18
commit f4ce492
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 20 deletions.
diff --git a/ci/qa/phpstan-baseline.php b/ci/qa/phpstan-baseline.php
@@ -2631,11 +2631,6 @@
 	'count' => 1,
 	'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Domain/Entity/ManageEntity.php',
 ];
-$ignoreErrors[] = [
-	'message' => '#^Method Surfnet\\\\ServiceProviderDashboard\\\\Domain\\\\Entity\\\\PrivacyQuestions\\:\\:privacyStatementUrls\\(\\) return type has no value type specified in iterable type array\\.$#',
-	'count' => 1,
-	'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Domain/Entity/PrivacyQuestions.php',
-];
 $ignoreErrors[] = [
 	'message' => '#^Parameter \\$targetEntity of attribute class Doctrine\\\\ORM\\\\Mapping\\\\OneToOne constructor expects class\\-string\\|null, string given\\.$#',
 	'count' => 1,
@@ -4641,11 +4636,6 @@
 	'count' => 1,
 	'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Infrastructure/Manage/Factory/SaveCommandFactory.php',
 ];
-$ignoreErrors[] = [
-	'message' => '#^Parameter \\#1 \\$certificate of method Surfnet\\\\ServiceProviderDashboard\\\\Application\\\\Command\\\\Entity\\\\SaveSamlEntityCommand\\:\\:setCertificate\\(\\) expects string, string\\|null given\\.$#',
-	'count' => 1,
-	'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Infrastructure/Manage/Factory/SaveCommandFactory.php',
-];
 $ignoreErrors[] = [
 	'message' => '#^Parameter \\#1 \\$descriptionEn of method Surfnet\\\\ServiceProviderDashboard\\\\Application\\\\Command\\\\Entity\\\\SaveOauthClientCredentialClientCommand\\:\\:setDescriptionEn\\(\\) expects string, string\\|null given\\.$#',
 	'count' => 1,
@@ -4741,11 +4731,6 @@
 	'count' => 1,
 	'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Infrastructure/Manage/Factory/SaveCommandFactory.php',
 ];
-$ignoreErrors[] = [
-	'message' => '#^Parameter \\#1 \\$metadataUrl of method Surfnet\\\\ServiceProviderDashboard\\\\Application\\\\Command\\\\Entity\\\\SaveSamlEntityCommand\\:\\:setMetadataUrl\\(\\) expects string, string\\|null given\\.$#',
-	'count' => 1,
-	'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Infrastructure/Manage/Factory/SaveCommandFactory.php',
-];
 $ignoreErrors[] = [
 	'message' => '#^Parameter \\#1 \\$nameEn of method Surfnet\\\\ServiceProviderDashboard\\\\Application\\\\Command\\\\Entity\\\\SaveOauthClientCredentialClientCommand\\:\\:setNameEn\\(\\) expects string, string\\|null given\\.$#',
 	'count' => 1,

diff --git a/src/Surfnet/ServiceProviderDashboard/Application/Command/Entity/SaveSamlEntityCommand.php b/src/Surfnet/ServiceProviderDashboard/Application/Command/Entity/SaveSamlEntityCommand.php
@@ -82,7 +82,7 @@ class SaveSamlEntityCommand implements SaveEntityCommandInterface
     private string $entityId;
 
     #[SpDashboardAssert\ValidSSLCertificate()]
-    private string $certificate;
+    private ?string $certificate = null;
 
     #[SpDashboardAssert\ValidLogo()]
     #[Assert\Url]
@@ -243,7 +243,7 @@ public function getMetadataUrl(): ?string
         return $this->metadataUrl;
     }
 
-    public function setMetadataUrl(string $metadataUrl): void
+    public function setMetadataUrl(?string $metadataUrl): void
     {
         $this->metadataUrl = $metadataUrl;
     }
@@ -283,7 +283,7 @@ public function getCertificate(): ?string
         return $this->certificate;
     }
 
-    public function setCertificate(string $certificate): void
+    public function setCertificate(?string $certificate): void
     {
         $this->certificate = $certificate;
     }

diff --git a/src/Surfnet/ServiceProviderDashboard/Domain/Entity/PrivacyQuestions.php b/src/Surfnet/ServiceProviderDashboard/Domain/Entity/PrivacyQuestions.php
@@ -153,13 +153,16 @@ public function setPrivacyStatementUrlEn(?string $privacyStatementUrlEn): void
         $this->privacyStatementUrlEn = $privacyStatementUrlEn;
     }
 
+    /**
+     * @return array<string, string>
+     */
     public function privacyStatementUrls(): array
     {
         $out = [];
-        if ($this->privacyStatementUrlEn !== '' && $this->privacyStatementUrlEn !== '0') {
+        if ($this->privacyStatementUrlEn !== '0' && $this->privacyStatementUrlEn !== null) {
             $out['mdui:PrivacyStatementURL:en'] = $this->privacyStatementUrlEn;
         }
-        if ($this->privacyStatementUrlNl !== '' && $this->privacyStatementUrlNl !== '0') {
+        if ($this->privacyStatementUrlNl !== '0' && $this->privacyStatementUrlNl !== null) {
             $out['mdui:PrivacyStatementURL:nl'] = $this->privacyStatementUrlNl;
         }
         return $out;

diff --git a/tests/unit/Application/Metadata/JsonGenerator/PrivacyQuestionsMetadataGeneratorTest.php b/tests/unit/Application/Metadata/JsonGenerator/PrivacyQuestionsMetadataGeneratorTest.php
@@ -71,6 +71,44 @@ public function test_it_can_build_privacy_question_metadata()
         $this->assertEquals('dpa_in_surf_agreement', $metadata['coin:privacy:dpa_type']); // DpaType::DPA_TYPE_IN_SURF_AGREEMENT
     }
 
+    public function test_nullable_privacy_statements_are_ignored()
+    {
+        $entity = m::mock(ManageEntity::class)->makePartial();
+        $service = m::mock(Service::class)->makePartial();
+        $privacyQuestions = new PrivacyQuestions();
+
+        $service->setPrivacyQuestionsEnabled(true);
+
+        $privacyQuestions->setWhatData('What data');
+        $privacyQuestions->setOtherInfo('Other information');
+        $privacyQuestions->setCountry('Country');
+        $privacyQuestions->setAccessData('Access data');
+        $privacyQuestions->setSecurityMeasures('Measures');
+        $privacyQuestions->setPrivacyStatementUrlEn(null);
+        $privacyQuestions->setPrivacyStatementUrlNl(null);
+        $privacyQuestions->setDpaType('dpa_in_surf_agreement'); // DpaType::DPA_TYPE_IN_SURF_AGREEMENT
+
+        $service->setPrivacyQuestions($privacyQuestions);
+        $entity->setService($service);
+
+        $factory = new PrivacyQuestionsMetadataGenerator(
+            new AttributesMetadataRepository(__DIR__ . '/../../../../../assets/Resources')
+        );
+
+        $metadata = $factory->build($entity);
+
+        $this->assertCount(6, $metadata);
+
+        $this->assertEquals('What data', $metadata['coin:privacy:what_data']);
+        $this->assertEquals('Access data', $metadata['coin:privacy:access_data']);
+        $this->assertEquals('Country', $metadata['coin:privacy:country']);
+        $this->assertEquals('Measures', $metadata['coin:privacy:security_measures']);
+        $this->assertEquals('Other information', $metadata['coin:privacy:other_info']);
+        $this->assertFalse(isset($metadata['mdui:PrivacyStatementURL:en']));
+        $this->assertFalse(isset($metadata['mdui:PrivacyStatementURL:nl']));
+        $this->assertEquals('dpa_in_surf_agreement', $metadata['coin:privacy:dpa_type']); // DpaType::DPA_TYPE_IN_SURF_AGREEMENT
+    }
+
     public function test_it_retuns_empty_array_when_disabled()
     {
         $entity = m::mock(ManageEntity::class)->makePartial();