Initial unit tests for Document Properties (#1932)

* Initial unit tests for Document Properties
* Typehinting in the document properties class

src/PhpSpreadsheet/Document/Properties.php

@@ -5,12 +5,20 @@ namespace PhpOffice\PhpSpreadsheet\Document;
 class Properties
 {
     /** constants */
-    const PROPERTY_TYPE_BOOLEAN = 'b';
+    public const PROPERTY_TYPE_BOOLEAN = 'b';
-    const PROPERTY_TYPE_INTEGER = 'i';
+    public const PROPERTY_TYPE_INTEGER = 'i';
-    const PROPERTY_TYPE_FLOAT = 'f';
+    public const PROPERTY_TYPE_FLOAT = 'f';
-    const PROPERTY_TYPE_DATE = 'd';
+    public const PROPERTY_TYPE_DATE = 'd';
-    const PROPERTY_TYPE_STRING = 's';
+    public const PROPERTY_TYPE_STRING = 's';
-    const PROPERTY_TYPE_UNKNOWN = 'u';
+    public const PROPERTY_TYPE_UNKNOWN = 'u';
+
+    private const VALID_PROPERTY_TYPE_LIST = [
+        self::PROPERTY_TYPE_BOOLEAN,
+        self::PROPERTY_TYPE_INTEGER,
+        self::PROPERTY_TYPE_FLOAT,
+        self::PROPERTY_TYPE_DATE,
+        self::PROPERTY_TYPE_STRING,
+    ];
 
     /**
      * Creator.
@@ -92,7 +100,7 @@ class Properties
     /**
      * Custom Properties.
      *
-     * @var string
+     * @var string[]
      */
     private $customProperties = [];
 
@@ -109,10 +117,8 @@ class Properties
 
     /**
      * Get Creator.
-     *
-     * @return string
      */
-    public function getCreator()
+    public function getCreator(): string
     {
         return $this->creator;
     }
@@ -120,11 +126,9 @@ class Properties
     /**
      * Set Creator.
      *
-     * @param string $creator
-     *
      * @return $this
      */
-    public function setCreator($creator)
+    public function setCreator(string $creator): self
     {
         $this->creator = $creator;
 
@@ -133,10 +137,8 @@ class Properties
 
     /**
      * Get Last Modified By.
-     *
-     * @return string
      */
-    public function getLastModifiedBy()
+    public function getLastModifiedBy(): string
     {
         return $this->lastModifiedBy;
     }
@@ -144,23 +146,19 @@ class Properties
     /**
      * Set Last Modified By.
      *
-     * @param string $pValue
-     *
      * @return $this
      */
-    public function setLastModifiedBy($pValue)
+    public function setLastModifiedBy(string $modifier): self
     {
-        $this->lastModifiedBy = $pValue;
+        $this->lastModifiedBy = $modifier;
 
         return $this;
     }
 
     /**
      * Get Created.
-     *
-     * @return int
      */
-    public function getCreated()
+    public function getCreated(): int
     {
         return $this->created;
     }
@@ -168,33 +166,31 @@ class Properties
     /**
      * Set Created.
      *
-     * @param int|string $time
+     * @param null|int|string $timestamp
      *
      * @return $this
      */
-    public function setCreated($time)
+    public function setCreated($timestamp): self
     {
-        if ($time === null) {
+        if ($timestamp === null) {
-            $time = time();
+            $timestamp = time();
-        } elseif (is_string($time)) {
+        } elseif (is_string($timestamp)) {
-            if (is_numeric($time)) {
+            if (is_numeric($timestamp)) {
-                $time = (int) $time;
+                $timestamp = (int) $timestamp;
             } else {
-                $time = strtotime($time);
+                $timestamp = strtotime($timestamp);
             }
         }
 
-        $this->created = $time;
+        $this->created = $timestamp;
 
         return $this;
     }
 
     /**
      * Get Modified.
-     *
-     * @return int
      */
-    public function getModified()
+    public function getModified(): int
     {
         return $this->modified;
     }
@@ -202,33 +198,31 @@ class Properties
     /**
      * Set Modified.
      *
-     * @param int|string $time
+     * @param null|int|string $timestamp
      *
      * @return $this
      */
-    public function setModified($time)
+    public function setModified($timestamp): self
     {
-        if ($time === null) {
+        if ($timestamp === null) {
-            $time = time();
+            $timestamp = time();
-        } elseif (is_string($time)) {
+        } elseif (is_string($timestamp)) {
-            if (is_numeric($time)) {
+            if (is_numeric($timestamp)) {
-                $time = (int) $time;
+                $timestamp = (int) $timestamp;
             } else {
-                $time = strtotime($time);
+                $timestamp = strtotime($timestamp);
             }
         }
 
-        $this->modified = $time;
+        $this->modified = $timestamp;
 
         return $this;
     }
 
     /**
      * Get Title.
-     *
-     * @return string
      */
-    public function getTitle()
+    public function getTitle(): string
     {
         return $this->title;
     }
@@ -236,11 +230,9 @@ class Properties
     /**
      * Set Title.
      *
-     * @param string $title
-     *
      * @return $this
      */
-    public function setTitle($title)
+    public function setTitle(string $title): self
     {
         $this->title = $title;
 
@@ -249,10 +241,8 @@ class Properties
 
     /**
      * Get Description.
-     *
-     * @return string
      */
-    public function getDescription()
+    public function getDescription(): string
     {
         return $this->description;
     }
@@ -260,11 +250,9 @@ class Properties
     /**
      * Set Description.
      *
-     * @param string $description
-     *
      * @return $this
      */
-    public function setDescription($description)
+    public function setDescription(string $description): self
     {
         $this->description = $description;
 
@@ -273,10 +261,8 @@ class Properties
 
     /**
      * Get Subject.
-     *
-     * @return string
      */
-    public function getSubject()
+    public function getSubject(): string
     {
         return $this->subject;
     }
@@ -284,11 +270,9 @@ class Properties
     /**
      * Set Subject.
      *
-     * @param string $subject
-     *
      * @return $this
      */
-    public function setSubject($subject)
+    public function setSubject(string $subject): self
     {
         $this->subject = $subject;
 
@@ -297,10 +281,8 @@ class Properties
 
     /**
      * Get Keywords.
-     *
-     * @return string
      */
-    public function getKeywords()
+    public function getKeywords(): string
     {
         return $this->keywords;
     }
@@ -308,11 +290,9 @@ class Properties
     /**
      * Set Keywords.
      *
-     * @param string $keywords
-     *
      * @return $this
      */
-    public function setKeywords($keywords)
+    public function setKeywords(string $keywords): self
     {
         $this->keywords = $keywords;
 
@@ -321,10 +301,8 @@ class Properties
 
     /**
      * Get Category.
-     *
-     * @return string
      */
-    public function getCategory()
+    public function getCategory(): string
     {
         return $this->category;
     }
@@ -332,11 +310,9 @@ class Properties
     /**
      * Set Category.
      *
-     * @param string $category
-     *
      * @return $this
      */
-    public function setCategory($category)
+    public function setCategory(string $category): self
     {
         $this->category = $category;
 
@@ -345,10 +321,8 @@ class Properties
 
     /**
      * Get Company.
-     *
-     * @return string
      */
-    public function getCompany()
+    public function getCompany(): string
     {
         return $this->company;
     }
@@ -356,11 +330,9 @@ class Properties
     /**
      * Set Company.
      *
-     * @param string $company
-     *
      * @return $this
      */
-    public function setCompany($company)
+    public function setCompany(string $company): self
     {
         $this->company = $company;
 
@@ -369,10 +341,8 @@ class Properties
 
     /**
      * Get Manager.
-     *
-     * @return string
      */
-    public function getManager()
+    public function getManager(): string
     {
         return $this->manager;
     }
@@ -380,11 +350,9 @@ class Properties
     /**
      * Set Manager.
      *
-     * @param string $manager
-     *
      * @return $this
      */
-    public function setManager($manager)
+    public function setManager(string $manager): self
     {
         $this->manager = $manager;
 
@@ -394,33 +362,27 @@ class Properties
     /**
      * Get a List of Custom Property Names.
      *
-     * @return array of string
+     * @return string[]
      */
-    public function getCustomProperties()
+    public function getCustomProperties(): array
     {
         return array_keys($this->customProperties);
     }
 
     /**
      * Check if a Custom Property is defined.
-     *
-     * @param string $propertyName
-     *
-     * @return bool
      */
-    public function isCustomPropertySet($propertyName)
+    public function isCustomPropertySet(string $propertyName): bool
     {
-        return isset($this->customProperties[$propertyName]);
+        return array_key_exists($propertyName, $this->customProperties);
     }
 
     /**
      * Get a Custom Property Value.
      *
-     * @param string $propertyName
-     *
      * @return mixed
      */
-    public function getCustomPropertyValue($propertyName)
+    public function getCustomPropertyValue(string $propertyName)
     {
         if (isset($this->customProperties[$propertyName])) {
             return $this->customProperties[$propertyName]['value'];
@@ -430,21 +392,33 @@ class Properties
     /**
      * Get a Custom Property Type.
      *
-     * @param string $propertyName
-     *
      * @return string
      */
-    public function getCustomPropertyType($propertyName)
+    public function getCustomPropertyType(string $propertyName)
     {
         if (isset($this->customProperties[$propertyName])) {
             return $this->customProperties[$propertyName]['type'];
         }
     }
 
+    private function identifyPropertyType($propertyValue)
+    {
+        if ($propertyValue === null) {
+            return self::PROPERTY_TYPE_STRING;
+        } elseif (is_float($propertyValue)) {
+            return self::PROPERTY_TYPE_FLOAT;
+        } elseif (is_int($propertyValue)) {
+            return self::PROPERTY_TYPE_INTEGER;
+        } elseif (is_bool($propertyValue)) {
+            return self::PROPERTY_TYPE_BOOLEAN;
+        }
+
+        return self::PROPERTY_TYPE_STRING;
+    }
+
     /**
      * Set a Custom Property.
      *
-     * @param string $propertyName
      * @param mixed $propertyValue
      * @param string $propertyType
      *   'i' : Integer
@@ -455,27 +429,10 @@ class Properties
      *
      * @return $this
      */
-    public function setCustomProperty($propertyName, $propertyValue = '', $propertyType = null)
+    public function setCustomProperty(string $propertyName, $propertyValue = '', $propertyType = null): self
     {
-        if (
+        if (($propertyType === null) || (!in_array($propertyType, self::VALID_PROPERTY_TYPE_LIST))) {
-            ($propertyType === null) || (!in_array($propertyType, [self::PROPERTY_TYPE_INTEGER,
+            $propertyType = $this->identifyPropertyType($propertyValue);
-                self::PROPERTY_TYPE_FLOAT,
-                self::PROPERTY_TYPE_STRING,
-                self::PROPERTY_TYPE_DATE,
-                self::PROPERTY_TYPE_BOOLEAN,
-            ]))
-        ) {
-            if ($propertyValue === null) {
-                $propertyType = self::PROPERTY_TYPE_STRING;
-            } elseif (is_float($propertyValue)) {
-                $propertyType = self::PROPERTY_TYPE_FLOAT;
-            } elseif (is_int($propertyValue)) {
-                $propertyType = self::PROPERTY_TYPE_INTEGER;
-            } elseif (is_bool($propertyValue)) {
-                $propertyType = self::PROPERTY_TYPE_BOOLEAN;
-            } else {
-                $propertyType = self::PROPERTY_TYPE_STRING;
-            }
         }
 
         $this->customProperties[$propertyName] = [
@@ -501,7 +458,7 @@ class Properties
         }
     }
 
-    public static function convertProperty($propertyValue, $propertyType)
+    public static function convertProperty($propertyValue, string $propertyType)
     {
         switch ($propertyType) {
             case 'empty':     //    Empty
@@ -552,7 +509,7 @@ class Properties
         return $propertyValue;
     }
 
-    public static function convertPropertyType($propertyType)
+    public static function convertPropertyType(string $propertyType): string
     {
         switch ($propertyType) {
             case 'i1':       //    1-Byte Signed Integer

tests/PhpSpreadsheetTests/Document/PropertiesTest.php

@@ -0,0 +1,183 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Document;
+
+use PhpOffice\PhpSpreadsheet\Document\Properties;
+use PHPUnit\Framework\TestCase;
+
+class PropertiesTest extends TestCase
+{
+    protected $properties;
+
+    protected function setup(): void
+    {
+        $this->properties = new Properties();
+    }
+
+    public function testNewInstance(): void
+    {
+        $createdTime = $modifiedTime = time();
+        self::assertSame('Unknown Creator', $this->properties->getCreator());
+        self::assertSame('Unknown Creator', $this->properties->getLastModifiedBy());
+        self::assertSame('Untitled Spreadsheet', $this->properties->getTitle());
+        self::assertSame('Microsoft Corporation', $this->properties->getCompany());
+        self::assertSame($createdTime, $this->properties->getCreated());
+        self::assertSame($modifiedTime, $this->properties->getModified());
+    }
+
+    public function testSetCreator(): void
+    {
+        $creator = 'Mark Baker';
+
+        $this->properties->setCreator($creator);
+        self::assertSame($creator, $this->properties->getCreator());
+    }
+
+    /**
+     * @dataProvider providerCreationTime
+     *
+     * @param mixed $expectedCreationTime
+     * @param mixed $created
+     */
+    public function testSetCreated($expectedCreationTime, $created): void
+    {
+        $expectedCreationTime = $expectedCreationTime ?? time();
+
+        $this->properties->setCreated($created);
+        self::assertSame($expectedCreationTime, $this->properties->getCreated());
+    }
+
+    public function providerCreationTime(): array
+    {
+        return [
+            [null, null],
+            [1615980600, 1615980600],
+            [1615980600, '1615980600'],
+            [1615980600, '2021-03-17 11:30:00Z'],
+        ];
+    }
+
+    public function testSetModifier(): void
+    {
+        $creator = 'Mark Baker';
+
+        $this->properties->setLastModifiedBy($creator);
+        self::assertSame($creator, $this->properties->getLastModifiedBy());
+    }
+
+    /**
+     * @dataProvider providerModifiedTime
+     *
+     * @param mixed $expectedModifiedTime
+     * @param mixed $modified
+     */
+    public function testSetModified($expectedModifiedTime, $modified): void
+    {
+        $expectedModifiedTime = $expectedModifiedTime ?? time();
+
+        $this->properties->setModified($modified);
+        self::assertSame($expectedModifiedTime, $this->properties->getModified());
+    }
+
+    public function providerModifiedTime(): array
+    {
+        return [
+            [null, null],
+            [1615980600, 1615980600],
+            [1615980600, '1615980600'],
+            [1615980600, '2021-03-17 11:30:00Z'],
+        ];
+    }
+
+    public function testSetTitle(): void
+    {
+        $title = 'My spreadsheet title test';
+
+        $this->properties->setTitle($title);
+        self::assertSame($title, $this->properties->getTitle());
+    }
+
+    public function testSetDescription(): void
+    {
+        $description = 'A test for spreadsheet description';
+
+        $this->properties->setDescription($description);
+        self::assertSame($description, $this->properties->getDescription());
+    }
+
+    public function testSetSubject(): void
+    {
+        $subject = 'Test spreadsheet';
+
+        $this->properties->setSubject($subject);
+        self::assertSame($subject, $this->properties->getSubject());
+    }
+
+    public function testSetKeywords(): void
+    {
+        $keywords = 'Test PHPSpreadsheet Spreadsheet Excel LibreOffice Gnumeric OpenSpreadsheetML OASIS';
+
+        $this->properties->setKeywords($keywords);
+        self::assertSame($keywords, $this->properties->getKeywords());
+    }
+
+    public function testSetCategory(): void
+    {
+        $category = 'Testing';
+
+        $this->properties->setCategory($category);
+        self::assertSame($category, $this->properties->getCategory());
+    }
+
+    public function testSetCompany(): void
+    {
+        $company = 'PHPOffice Suite';
+
+        $this->properties->setCompany($company);
+        self::assertSame($company, $this->properties->getCompany());
+    }
+
+    public function testSetManager(): void
+    {
+        $manager = 'Mark Baker';
+
+        $this->properties->setManager($manager);
+        self::assertSame($manager, $this->properties->getManager());
+    }
+
+    /**
+     * @dataProvider providerCustomProperties
+     *
+     * @param mixed $expectedType
+     * @param mixed $expectedValue
+     * @param mixed $propertyName
+     */
+    public function testSetCustomProperties($expectedType, $expectedValue, $propertyName, ...$args): void
+    {
+        $this->properties->setCustomProperty($propertyName, ...$args);
+        self::assertTrue($this->properties->isCustomPropertySet($propertyName));
+        self::assertSame($expectedValue, $this->properties->getCustomPropertyValue($propertyName));
+        self::assertSame($expectedType, $this->properties->getCustomPropertyType($propertyName));
+    }
+
+    public function providerCustomProperties(): array
+    {
+        return [
+            [Properties::PROPERTY_TYPE_STRING, null, 'Editor', null],
+            [Properties::PROPERTY_TYPE_STRING, 'Mark Baker', 'Editor', 'Mark Baker'],
+            [Properties::PROPERTY_TYPE_FLOAT, 1.17, 'Version', 1.17],
+            [Properties::PROPERTY_TYPE_INTEGER, 2, 'Revision', 2],
+            [Properties::PROPERTY_TYPE_BOOLEAN, true, 'Tested', true],
+            [Properties::PROPERTY_TYPE_DATE, '2021-03-17', 'Test Date', '2021-03-17', Properties::PROPERTY_TYPE_DATE],
+        ];
+    }
+
+    public function testGetUnknownCustomProperties(): void
+    {
+        $propertyName = 'I DONT EXIST';
+
+        self::assertFalse($this->properties->isCustomPropertySet($propertyName));
+        self::assertNull($this->properties->getCustomPropertyValue($propertyName));
+        self::assertNull($this->properties->getCustomPropertyType($propertyName));
+    }
+}