Improve doctrine types: remove string casting, add type checking

Alex Rothuis · Alex Rothuis · commit a668ee9ab5a3 · 2016-06-17T15:26:13.000+02:00
diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Doctrine/Type/ConfigurationContactInformationType.php b/src/Surfnet/StepupMiddleware/ApiBundle/Doctrine/Type/ConfigurationContactInformationType.php
@@ -25,7 +25,7 @@
 use Surfnet\Stepup\Configuration\Value\ContactInformation;
 
 /**
- * Custom Type for the Location Value Object for the Configuration domain
+ * Custom Type for the ContactInformation Value Object for the Configuration domain
  */
 class ConfigurationContactInformationType extends Type
 {
@@ -39,10 +39,20 @@ public function getSQLDeclaration(array $fieldDeclaration, AbstractPlatform $pla
     public function convertToDatabaseValue($value, AbstractPlatform $platform)
     {
         if (is_null($value)) {
-            return $value;
+            return null;
+        }
+
+        if (!$value instanceof ContactInformation) {
+            throw new ConversionException(
+                sprintf(
+                    "Encountered illegal contact information of type %s '%s', expected a ContactInformation instance",
+                    is_object($value) ? get_class($value) : gettype($value),
+                    is_scalar($value) ? (string) $value : ''
+                )
+            );
         }
 
-        return (string) $value;
+        return $value->getContactInformation();
     }
 
     public function convertToPHPValue($value, AbstractPlatform $platform)
diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Doctrine/Type/ConfigurationInstitutionType.php b/src/Surfnet/StepupMiddleware/ApiBundle/Doctrine/Type/ConfigurationInstitutionType.php
@@ -42,7 +42,17 @@ public function convertToDatabaseValue($value, AbstractPlatform $platform)
             return $value;
         }
 
-        return (string) $value;
+        if (!$value instanceof Institution) {
+            throw new ConversionException(
+                sprintf(
+                    "Encountered illegal institution of type %s '%s', expected an Institution instance",
+                    is_object($value) ? get_class($value) : gettype($value),
+                    is_scalar($value) ? (string) $value : ''
+                )
+            );
+        }
+
+        return $value->getInstitution();
     }
 
     public function convertToPHPValue($value, AbstractPlatform $platform)
diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Doctrine/Type/ConfigurationLocationType.php b/src/Surfnet/StepupMiddleware/ApiBundle/Doctrine/Type/ConfigurationLocationType.php
@@ -42,7 +42,17 @@ public function convertToDatabaseValue($value, AbstractPlatform $platform)
             return $value;
         }
 
-        return (string) $value;
+        if (!$value instanceof Location) {
+            throw new ConversionException(
+                sprintf(
+                    "Encountered illegal location of type %s '%s', expected a Location instance",
+                    is_object($value) ? get_class($value) : gettype($value),
+                    is_scalar($value) ? (string) $value : ''
+                )
+            );
+        }
+
+        return $value->getLocation();
     }
 
     public function convertToPHPValue($value, AbstractPlatform $platform)
diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Doctrine/Type/DocumentNumberType.php b/src/Surfnet/StepupMiddleware/ApiBundle/Doctrine/Type/DocumentNumberType.php
@@ -42,17 +42,28 @@ public function getSQLDeclaration(array $fieldDeclaration, AbstractPlatform $pla
     }
 
     /**
-     * @param mixed            $value
+     * @param mixed $value
      * @param AbstractPlatform $platform
      * @return null|string
+     * @throws ConversionException
      */
     public function convertToDatabaseValue($value, AbstractPlatform $platform)
     {
-        if (!$value instanceof DocumentNumber) {
+        if (is_null($value)) {
             return null;
         }
 
-        return (string) $value;
+        if (!$value instanceof DocumentNumber) {
+            throw new ConversionException(
+                sprintf(
+                    "Encountered illegal document number of type %s '%s', expected a DocumentNumber instance",
+                    is_object($value) ? get_class($value) : gettype($value),
+                    is_scalar($value) ? (string) $value : ''
+                )
+            );
+        }
+
+        return $value->getDocumentNumber();
     }
 
     /**
diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Doctrine/Type/RaLocationNameType.php b/src/Surfnet/StepupMiddleware/ApiBundle/Doctrine/Type/RaLocationNameType.php
@@ -42,7 +42,17 @@ public function convertToDatabaseValue($value, AbstractPlatform $platform)
             return $value;
         }
 
-        return (string) $value;
+        if (!$value instanceof RaLocationName) {
+            throw new ConversionException(
+                sprintf(
+                    "Encountered illegal RA location name of type %s '%s', expected a RaLocationName instance",
+                    is_object($value) ? get_class($value) : gettype($value),
+                    is_scalar($value) ? (string) $value : ''
+                )
+            );
+        }
+
+        return $value->getRaLocationName();
     }
 
     public function convertToPHPValue($value, AbstractPlatform $platform)
diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Tests/Doctrine/Type/ConfigurationContactInformationTypeTest.php b/src/Surfnet/StepupMiddleware/ApiBundle/Tests/Doctrine/Type/ConfigurationContactInformationTypeTest.php
@@ -47,6 +47,21 @@ public function setUp()
         $this->platform = new MySqlPlatform();
     }
 
+    /**
+     * @test
+     * @group doctrine
+     *
+     * @dataProvider \Surfnet\StepupMiddleware\ApiBundle\Tests\TestDataProvider::notNull
+     * @param $incorrectValue
+     */
+    public function a_value_can_only_be_converted_to_sql_if_it_is_contact_information_or_null($incorrectValue)
+    {
+        $this->setExpectedException('Doctrine\DBAL\Types\ConversionException');
+
+        $configurationContactInformation = Type::getType(ConfigurationContactInformationType::NAME);
+        $configurationContactInformation->convertToDatabaseValue($incorrectValue, $this->platform);
+    }
+
     /**
      * @test
      * @group doctrine
diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Tests/Doctrine/Type/ConfigurationInstitutionTypeTest.php b/src/Surfnet/StepupMiddleware/ApiBundle/Tests/Doctrine/Type/ConfigurationInstitutionTypeTest.php
@@ -60,6 +60,21 @@ public function a_null_value_remains_null_in_to_sql_conversion()
         $this->assertNull($value);
     }
 
+    /**
+     * @test
+     * @group doctrine
+     *
+     * @dataProvider \Surfnet\StepupMiddleware\ApiBundle\Tests\TestDataProvider::notNull
+     * @param $incorrectValue
+     */
+    public function a_value_can_only_be_converted_to_sql_if_it_is_an_institution_or_null($incorrectValue)
+    {
+        $this->setExpectedException('Doctrine\DBAL\Types\ConversionException');
+
+        $configurationContactInformation = Type::getType(ConfigurationInstitutionType::NAME);
+        $configurationContactInformation->convertToDatabaseValue($incorrectValue, $this->platform);
+    }
+
     /**
      * @test
      * @group doctrine
diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Tests/Doctrine/Type/ConfigurationLocationTypeTest.php b/src/Surfnet/StepupMiddleware/ApiBundle/Tests/Doctrine/Type/ConfigurationLocationTypeTest.php
@@ -60,6 +60,21 @@ public function a_null_value_remains_null_in_to_sql_conversion()
         $this->assertNull($value);
     }
 
+    /**
+     * @test
+     * @group doctrine
+     *
+     * @dataProvider \Surfnet\StepupMiddleware\ApiBundle\Tests\TestDataProvider::notNull
+     * @param $incorrectValue
+     */
+    public function a_value_can_only_be_converted_to_sql_if_it_is_a_location_or_null($incorrectValue)
+    {
+        $this->setExpectedException('Doctrine\DBAL\Types\ConversionException');
+
+        $configurationContactInformation = Type::getType(ConfigurationLocationType::NAME);
+        $configurationContactInformation->convertToDatabaseValue($incorrectValue, $this->platform);
+    }
+
     /**
      * @test
      * @group doctrine
diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Tests/Doctrine/Type/DocumentNumberTypeTest.php b/src/Surfnet/StepupMiddleware/ApiBundle/Tests/Doctrine/Type/DocumentNumberTypeTest.php
@@ -85,6 +85,21 @@ public function a_null_value_remains_null_when_converting_from_db_to_php_value()
         $this->assertNull($value);
     }
 
+    /**
+     * @test
+     * @group doctrine
+     *
+     * @dataProvider \Surfnet\StepupMiddleware\ApiBundle\Tests\TestDataProvider::notNull
+     * @param $incorrectValue
+     */
+    public function a_value_can_only_be_converted_to_sql_if_it_is_a_document_number_or_null($incorrectValue)
+    {
+        $this->setExpectedException('Doctrine\DBAL\Types\ConversionException');
+
+        $configurationContactInformation = Type::getType(DocumentNumberType::NAME);
+        $configurationContactInformation->convertToDatabaseValue($incorrectValue, $this->platform);
+    }
+
     /**
      * @test
      * @group doctrine
diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Tests/Doctrine/Type/RaLocationNameTypeTest.php b/src/Surfnet/StepupMiddleware/ApiBundle/Tests/Doctrine/Type/RaLocationNameTypeTest.php
@@ -76,6 +76,21 @@ public function a_non_null_value_is_converted_to_the_correct_format()
         $this->assertEquals($expected, $output);
     }
 
+    /**
+     * @test
+     * @group doctrine
+     *
+     * @dataProvider \Surfnet\StepupMiddleware\ApiBundle\Tests\TestDataProvider::notNull
+     * @param $incorrectValue
+     */
+    public function a_value_can_only_be_converted_to_sql_if_it_is_an_ra_location_or_null($incorrectValue)
+    {
+        $this->setExpectedException('Doctrine\DBAL\Types\ConversionException');
+
+        $configurationContactInformation = Type::getType(RaLocationNameType::NAME);
+        $configurationContactInformation->convertToDatabaseValue($incorrectValue, $this->platform);
+    }
+
     /**
      * @test
      * @group doctrine
