Merge pull request #1 from kingsquare/v1.1

Tests / strictness
kingsquare · Jun 19, 2019 · 62a0f82 · 62a0f82
2 parents 31c4eff + 0470ca3
commit 62a0f82
Show file tree

Hide file tree

Showing 10 changed files with 257 additions and 29 deletions.
diff --git a/.editorconfig b/.editorconfig
@@ -7,3 +7,6 @@ indent_size = 4
 indent_style = space
 insert_final_newline = true
 trim_trailing_whitespace = true
+
+[*.json]
+indent_size = 2
diff --git a/.travis.yml b/.travis.yml
@@ -0,0 +1,22 @@
+language: php
+
+sudo: false
+
+php:
+  - 7.1
+  - 7.2
+  - 7.3
+
+matrix:
+  fast_finish: true
+
+cache:
+    directories:
+        - $HOME/.composer/cache/files
+
+install:
+    - travis_retry composer install --no-interaction --prefer-dist
+
+script:
+    - vendor/bin/phpstan.phar analyse -l max --no-interaction --no-progress src/ tests/
+    - vendor/bin/phpunit
diff --git a/README.md b/README.md
@@ -1,5 +1,8 @@
 # Communibase DataBag
 
+[![Build Status](https://travis-ci.org/kingsquare/communibase-databag.svg?branch=master)](https://travis-ci.org/kingsquare/communibase-databag)
+[![Scrutinizer Code Quality](https://scrutinizer-ci.com/g/kingsquare/communibase-databag/badges/quality-score.png?b=master)](https://scrutinizer-ci.com/g/kingsquare/communibase-databag/?branch=master)
+
 It's a bag, for CB data. If we need to create a CB object from CB data (array) we can use this dataBag object as
 a private entity class property. The dataBag can contain one or more entities. For each entity we can get/set
 properties by path. If we need to persist the entity back into CB use getState to fetch the (updated) data array.

diff --git a/composer.json b/composer.json
@@ -11,6 +11,10 @@
     {
       "name": "Misha van Tol",
       "email": "[email protected]"
+    },
+    {
+      "name": "Robin Speekenbrink",
+      "email": "[email protected]"
     }
   ],
   "require": {
@@ -23,10 +27,11 @@
   },
   "autoload-dev": {
     "psr-4": {
-      "Communibase\\Tests\\": "test/"
+      "Communibase\\Tests\\": "tests/"
     }
   },
   "require-dev": {
-    "phpunit/phpunit": "^5"
+    "phpunit/phpunit": "^5",
+    "phpstan/phpstan-shim": "^0.11.8"
   }
 }
diff --git a/phpunit.xml.dist b/phpunit.xml.dist
@@ -5,4 +5,9 @@
 			<directory>./tests</directory>
 		</testsuite>
 	</testsuites>
+    <filter>
+        <whitelist>
+            <directory suffix=".php">src/</directory>
+        </whitelist>
+    </filter>
 </phpunit>
diff --git a/src/DataBag.php b/src/DataBag.php
@@ -98,11 +98,6 @@ public function addEntityData($entityType, array $data)
      */
     private function getByPath($path, $default = null)
     {
-        // Invalid path
-        if (strpos($path, '.') === false) {
-            throw new InvalidDataBagPathException('Invalid get path: ' . $path);
-        }
-
         list($entityType, $path) = explode('.', $path, 2);
 
         // Direct property
@@ -119,7 +114,7 @@ private function getByPath($path, $default = null)
 
         // Indexed with 'type' property
         $field = null;
-        if (substr_count($index, '.') > 0) {
+        if (strpos($index, '.') > 0) {
             list($index, $field) = explode('.', $index, 2);
         }
 
@@ -162,6 +157,8 @@ private function getByPath($path, $default = null)
      */
     public function get($path, $default = null)
     {
+        $this->guardAgainstInvalidPath($path);
+
         if (!array_key_exists($path, $this->cache)) {
             $this->cache[$path] = $this->getByPath($path, $default);
         }
@@ -178,13 +175,9 @@ public function get($path, $default = null)
      */
     public function set($path, $value)
     {
-        unset($this->cache[$path]);
-
-        // Invalid path, can't store
-        if (substr_count($path, '.') === 0) {
-            throw new InvalidDataBagPathException('Invalid set path: ' . $path);
-        }
+        $this->guardAgainstInvalidPath($path);
 
+        unset($this->cache[$path]);
         if ($value === null) {
             $this->remove($path);
             return;
@@ -193,7 +186,7 @@ public function set($path, $value)
         list($entityType, $path) = explode('.', $path, 2);
 
         // Direct property
-        if (substr_count($path, '.') === 0) {
+        if (strpos($path, '.') === false) {
             $this->data[$entityType][$path] = $value;
             return;
         }
@@ -202,7 +195,7 @@ public function set($path, $value)
         list($path, $index) = explode('.', $path, 2);
 
         $field = null;
-        if (substr_count($index, '.') > 0) {
+        if (strpos($index, '.') > 0) {
             list($index, $field) = explode('.', $index, 2);
         }
 
@@ -268,15 +261,12 @@ public function hasEntityData($entityType)
      */
     public function remove($path, $removeAll = true)
     {
-        // Invalid path, can't remove
-        if (substr_count($path, '.') === 0) {
-            throw new InvalidDataBagPathException('Invalid remove path: ' . $path);
-        }
+        $this->guardAgainstInvalidPath($path);
 
         list($entityType, $path) = explode('.', $path, 2);
 
         // Direct property
-        if (substr_count($path, '.') === 0) {
+        if (strpos($path, '.') === false) {
             if (!isset($this->data[$entityType][$path])) {
                 return;
             }
@@ -365,4 +355,23 @@ public function getState($entityType = null)
         }
         return isset($this->data[$entityType]) ? $this->data[$entityType] : [];
     }
+
+    /**
+     * @param string $path
+     */
+    private function guardAgainstInvalidPath($path)
+    {
+        if (!is_string($path)) {
+            throw new InvalidDataBagPathException('Invalid path provided: path must be a string ' . gettype($path).' given');
+        }
+
+        if ($path === '' // empty
+            || strpos($path, '..') !== false // has .. somewhere
+            || substr($path, -1) === '.' // ends with .
+            || in_array(strpos($path, '.'), [false, 0], true) // starts with or doesnt have any .
+        ) {
+            throw new InvalidDataBagPathException('Invalid path provided: ' . $path);
+        }
+    }
+
 }
diff --git a/tests/GetFromDataBagTest.php b/tests/GetFromDataBagTest.php
@@ -17,9 +17,6 @@ final class GetFromDataBagTest extends TestCase
      */
     private $dataBag;
 
-    /**
-     *
-     */
     protected function setUp()
     {
         $personData = [
@@ -39,20 +36,40 @@ protected function setUp()
         $this->dataBag->addEntityData('person', $personData);
     }
 
-    /**
-     *
-     */
     protected function tearDown()
     {
         unset($this->dataBag);
     }
 
     /**
+     * @return array
+     */
+    public function invalidPathProvider()
+    {
+        return [
+            'empty' => [''],
+            'non-existant' => ['invalidPath'],
+            'ends with .' => ['person.'],
+            'ends with . on subpath' => ['person.firstName.'],
+            'not a string (int)' => [1],
+            'not a string (array)' => [[]],
+            'not a string (null)' => [null],
+            'not a string (bool)' => [true],
+            'not a string (float)' => [1.1001],
+            'not a string (object)' => [new \stdClass],
+            'starting with .' => ['.person.firstName'],
+            'has ..' => ['person..firstName'],
+            'has .. and .' => ['person..firstName.'],
+        ];
+    }
+
+    /**
+     * @dataProvider invalidPathProvider
      * @expectedException \Communibase\InvalidDataBagPathException
      */
-    public function testInvalidPath()
+    public function testInvalidPath($path)
     {
-        $this->dataBag->get('invalidPath');
+        $this->dataBag->get($path);
     }
 
     /**
@@ -66,6 +83,8 @@ public function provider()
             ['person.emailAddresses.0', ['emailAddress' => '[email protected]', 'type' => 'private']],
             ['person.emailAddresses.0.emailAddress', '[email protected]'],
             ['person.emailAddresses.privateGsm.emailAddress', '[email protected]'],
+            ['person.addresses.test', 'default'],
+            ['person.emailAddresses.test', 'default'],
         ];
     }
 

diff --git a/tests/HasEntityDataTest.php b/tests/HasEntityDataTest.php
@@ -0,0 +1,36 @@
+<?php
+
+namespace Communibase\Tests;
+
+use Communibase\DataBag;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Class RemoveFromBag
+ * @author Kingsquare ([email protected])
+ * @copyright Copyright (c) Kingsquare BV (http://www.kingsquare.nl)
+ */
+class HasEntityDataTest extends TestCase
+{
+    public function testHasEntityData()
+    {
+        $personData = [
+            'firstName' => 'John',
+            'emailAddresses' => [
+                [
+                    'emailAddress' => '[email protected]',
+                    'type' => 'private',
+                ],
+                [
+                    'emailAddress' => '[email protected]',
+                    'type' => 'privateGsm',
+                ]
+            ],
+        ];
+        $dataBag = DataBag::create();
+        $dataBag->addEntityData('person', $personData);
+
+        $this->assertTrue($dataBag->hasEntityData('person'));
+        $this->assertFalse($dataBag->hasEntityData('company'));
+    }
+}
diff --git a/tests/IsDirtyTest.php b/tests/IsDirtyTest.php
@@ -0,0 +1,117 @@
+<?php
+
+namespace Communibase\Tests;
+
+use Communibase\DataBag;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Class RemoveFromBag
+ * @author Kingsquare ([email protected])
+ * @copyright Copyright (c) Kingsquare BV (http://www.kingsquare.nl)
+ */
+class IsDirtyTest extends TestCase
+{
+    public function testIsDirtyWhenFieldIsChanged()
+    {
+        $personData = [
+            'firstName' => 'John',
+            'emailAddresses' => [
+                [
+                    'emailAddress' => '[email protected]',
+                    'type' => 'private',
+                ],
+                [
+                    'emailAddress' => '[email protected]',
+                    'type' => 'privateGsm',
+                ]
+            ],
+        ];
+        $dataBag = DataBag::create();
+        $dataBag->addEntityData('person', $personData);
+        $dataBag->set('person.firstName', 'Darko');
+        $this->assertSame(true, $dataBag->isDirty('person'));
+    }
+
+    public function testIsDirtyWhenFieldIsUnchanged()
+    {
+        $personData = [
+            'firstName' => 'John',
+            'emailAddresses' => [
+                [
+                    'emailAddress' => '[email protected]',
+                    'type' => 'private',
+                ],
+                [
+                    'emailAddress' => '[email protected]',
+                    'type' => 'privateGsm',
+                ]
+            ],
+        ];
+        $dataBag = DataBag::create();
+        $dataBag->addEntityData('person', $personData);
+        $dataBag->set('person.firstName', 'John');
+        $this->assertSame(false, $dataBag->isDirty('person'));
+    }
+
+    public function testIsDirtyWhenRemovedFromBag()
+    {
+        $personData = [
+            'firstName' => 'John',
+            'emailAddresses' => [
+                [
+                    'emailAddress' => '[email protected]',
+                    'type' => 'private',
+                ],
+                [
+                    'emailAddress' => '[email protected]',
+                    'type' => 'privateGsm',
+                ]
+            ],
+        ];
+        $dataBag = DataBag::create();
+        $dataBag->addEntityData('person', $personData);
+        $dataBag->remove('person.firstName', true);
+        $this->assertSame(true, $dataBag->isDirty('person'));
+    }
+
+    public function testIsDirtyWithUnknownPath()
+    {
+        $personData = [
+            'firstName' => 'John',
+            'emailAddresses' => [
+                [
+                    'emailAddress' => '[email protected]',
+                    'type' => 'private',
+                ],
+                [
+                    'emailAddress' => '[email protected]',
+                    'type' => 'privateGsm',
+                ]
+            ],
+        ];
+        $dataBag = DataBag::create();
+        $dataBag->addEntityData('person', $personData);
+        $this->assertNull($dataBag->isDirty('company'));
+    }
+    public function testIsDirtyWitNewPath()
+    {
+        $personData = [
+            'firstName' => 'John',
+            'emailAddresses' => [
+                [
+                    'emailAddress' => '[email protected]',
+                    'type' => 'private',
+                ],
+                [
+                    'emailAddress' => '[email protected]',
+                    'type' => 'privateGsm',
+                ]
+            ],
+        ];
+        $dataBag = DataBag::create();
+        $dataBag->addEntityData('person', $personData);
+        $dataBag->set('company.title', 'Kingsquare BV');
+        $this->assertSame(true, $dataBag->isDirty('company'));
+    }
+}