From b28e24ab5458a0481f7bf40c010bea31d2a09a95 Mon Sep 17 00:00:00 2001 From: Jesse Leite Date: Wed, 23 Mar 2016 22:19:51 -0400 Subject: [PATCH 1/8] Adding failing test to drive out nested array binding. --- tests/BindingTest.php | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/tests/BindingTest.php b/tests/BindingTest.php index f90d9fc..cdc48d5 100644 --- a/tests/BindingTest.php +++ b/tests/BindingTest.php @@ -143,14 +143,37 @@ public function testBindMagicProperty() public function testBindArray() { - $model = ['first_name' => 'John']; - $this->form->bind($model); + $array = ['first_name' => 'John']; + $this->form->bind($array); $expected = ''; $result = (string) $this->form->text('first_name'); $this->assertEquals($expected, $result); } + public function testBindNestedArray() + { + $array = [ + 'address' => [ + 'city' => 'Roswell', + 'tree' => [ + 'has' => [ + 'nested' => 'Bird' + ] + ], + ], + ]; + $this->form->bind($array); + + $expected = ''; + $result = (string) $this->form->text('address[city]'); + $this->assertEquals($expected, $result); + + $expected = ''; + $result = (string) $this->form->text('address[city][has][nested]'); + $this->assertEquals($expected, $result); + } + public function testCloseUnbindsModel() { $object = $this->getStubObject(); From 85ae7163a0afb2d6f2e6c9e9b6b497137dd9346d Mon Sep 17 00:00:00 2001 From: Jesse Leite Date: Fri, 25 Mar 2016 00:59:40 -0400 Subject: [PATCH 2/8] Support for binding nested array/object/mixed via new BoundData object. --- src/AdamWathan/Form/Binding/BoundData.php | 51 +++++++++++++++++++++++ src/AdamWathan/Form/FormBuilder.php | 22 ++++------ tests/BindingTest.php | 48 ++++++++++++++++++++- 3 files changed, 105 insertions(+), 16 deletions(-) create mode 100644 src/AdamWathan/Form/Binding/BoundData.php diff --git a/src/AdamWathan/Form/Binding/BoundData.php b/src/AdamWathan/Form/Binding/BoundData.php new file mode 100644 index 0000000..35681b9 --- /dev/null +++ b/src/AdamWathan/Form/Binding/BoundData.php @@ -0,0 +1,51 @@ +data = $data; + } + + public function has($name) + { + return $this->dotGet($this->transformKey($name)) !== false; + } + + public function get($name) + { + return $this->dotGet($this->transformKey($name)); + } + + protected function dotGet($dotKey) + { + $keyParts = array_filter(explode('.', $dotKey)); + + return $this->dataGet($this->data, $keyParts); + } + + protected function dataGet($target, $keyParts) + { + if (count($keyParts) == 0) { + return $target; + } + + $key = $keyParts[0]; + $remainingKeys = array_slice($keyParts, 1); + + if (is_array($target)) { + return isset($target[$key]) ? $this->dataGet($target[$key], $remainingKeys) : false; + } + + return isset($target->{$key}) ? $this->dataGet($target->{$key}, $remainingKeys) : false; + } + + protected function transformKey($key) + { + return str_replace(['[]', '[', ']'], ['', '.', ''], $key); + } +} diff --git a/src/AdamWathan/Form/FormBuilder.php b/src/AdamWathan/Form/FormBuilder.php index c59620c..3ac3549 100644 --- a/src/AdamWathan/Form/FormBuilder.php +++ b/src/AdamWathan/Form/FormBuilder.php @@ -2,6 +2,7 @@ namespace AdamWathan\Form; +use AdamWathan\Form\Binding\BoundData; use AdamWathan\Form\Elements\Button; use AdamWathan\Form\Elements\Checkbox; use AdamWathan\Form\Elements\Date; @@ -26,7 +27,7 @@ class FormBuilder private $csrfToken; - private $model; + public $model; public function setOldInputProvider(OldInputInterface $oldInputProvider) { @@ -218,9 +219,9 @@ public function getError($name, $format = null) return $message; } - public function bind($model) + public function bind($data) { - $this->model = is_array($model) ? (object) $model : $model; + $this->model = new BoundData($data); } public function getValueFor($name) @@ -256,21 +257,17 @@ protected function hasModelValue($name) return false; } - $name = $this->transformKey($name); - - return isset($this->model->{$name}) || method_exists($this->model, '__get'); + return $this->model->has($name); } protected function getModelValue($name) { - $name = $this->transformKey($name); - - return $this->escape($this->model->{$name}); + return $this->escape($this->model->get($name)); } protected function escape($value) { - if (!is_string($value)) { + if (! is_string($value)) { return $value; } @@ -301,9 +298,4 @@ public function selectMonth($name) return $this->select($name, $options); } - - protected function transformKey($key) - { - return str_replace('[]', '', $key); - } } diff --git a/tests/BindingTest.php b/tests/BindingTest.php index cdc48d5..62ad828 100644 --- a/tests/BindingTest.php +++ b/tests/BindingTest.php @@ -170,7 +170,53 @@ public function testBindNestedArray() $this->assertEquals($expected, $result); $expected = ''; - $result = (string) $this->form->text('address[city][has][nested]'); + $result = (string) $this->form->text('address[tree][has][nested]'); + $this->assertEquals($expected, $result); + } + + public function testBindNestedObject() + { + $object = json_decode(json_encode([ + 'address' => [ + 'city' => 'Roswell', + 'tree' => [ + 'has' => [ + 'nested' => 'Bird' + ] + ], + ], + ])); + $this->form->bind($object); + + $expected = ''; + $result = (string) $this->form->text('address[city]'); + $this->assertEquals($expected, $result); + + $expected = ''; + $result = (string) $this->form->text('address[tree][has][nested]'); + $this->assertEquals($expected, $result); + } + + public function testBindNestedMixed() + { + $object = [ + 'address' => [ + 'city' => 'Roswell', + 'tree' => json_decode(json_encode([ + 'has' => [ + 'nested' => 'Bird' + ] + ])), + ], + ]; + $this->form->bind($object); + + $expected = ''; + $result = (string) $this->form->text('address[city]'); + $this->assertEquals($expected, $result); + + $expected = ''; + $result = (string) $this->form->text('address[tree][has][nested]'); $this->assertEquals($expected, $result); } From 1dbb9b2f6ad9f2b98ba37b11746abf301a129532 Mon Sep 17 00:00:00 2001 From: Jesse Leite Date: Fri, 25 Mar 2016 01:19:54 -0400 Subject: [PATCH 3/8] Changing terminology a bit. --- src/AdamWathan/Form/FormBuilder.php | 24 ++++++++++++------------ tests/BindingTest.php | 4 ++-- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/AdamWathan/Form/FormBuilder.php b/src/AdamWathan/Form/FormBuilder.php index df7cfb1..ace64a0 100644 --- a/src/AdamWathan/Form/FormBuilder.php +++ b/src/AdamWathan/Form/FormBuilder.php @@ -27,7 +27,7 @@ class FormBuilder protected $csrfToken; - protected $model; + protected $boundData; public function setOldInputProvider(OldInputInterface $oldInputProvider) { @@ -62,7 +62,7 @@ protected function hasToken() public function close() { - $this->unbindModel(); + $this->unbindData(); return ''; } @@ -221,7 +221,7 @@ public function getError($name, $format = null) public function bind($data) { - $this->model = new BoundData($data); + $this->boundData = new BoundData($data); } public function getValueFor($name) @@ -230,8 +230,8 @@ public function getValueFor($name) return $this->getOldInput($name); } - if ($this->hasModelValue($name)) { - return $this->getModelValue($name); + if ($this->hasBoundValue($name)) { + return $this->getBoundValue($name); } return null; @@ -251,18 +251,18 @@ protected function getOldInput($name) return $this->escape($this->oldInput->getOldInput($name)); } - protected function hasModelValue($name) + protected function hasBoundValue($name) { - if (! isset($this->model)) { + if (! isset($this->boundData)) { return false; } - return $this->model->has($name); + return $this->boundData->has($name); } - protected function getModelValue($name) + protected function getBoundValue($name) { - return $this->escape($this->model->get($name)); + return $this->escape($this->boundData->get($name)); } protected function escape($value) @@ -274,9 +274,9 @@ protected function escape($value) return htmlentities($value, ENT_QUOTES, 'UTF-8'); } - protected function unbindModel() + protected function unbindData() { - $this->model = null; + $this->boundData = null; } public function selectMonth($name) diff --git a/tests/BindingTest.php b/tests/BindingTest.php index 62ad828..69e6884 100644 --- a/tests/BindingTest.php +++ b/tests/BindingTest.php @@ -220,7 +220,7 @@ public function testBindNestedMixed() $this->assertEquals($expected, $result); } - public function testCloseUnbindsModel() + public function testCloseUnbindsData() { $object = $this->getStubObject(); $this->form->bind($object); @@ -231,7 +231,7 @@ public function testCloseUnbindsModel() $this->assertEquals($expected, $result); } - public function testAgainstXSSAttacksInBoundModels() + public function testAgainstXSSAttacksInBoundData() { $object = $this->getStubObject(); $object->first_name = '" onmouseover="alert(\'xss\')'; From fa4e3d3fc485732b23e7e8efdf32e7544bd70ac3 Mon Sep 17 00:00:00 2001 From: Jesse Leite Date: Fri, 25 Mar 2016 02:02:23 -0400 Subject: [PATCH 4/8] Fixing magic getter regression. --- src/AdamWathan/Form/Binding/BoundData.php | 14 +++++++++++++- tests/BindingTest.php | 12 +++++++++--- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/AdamWathan/Form/Binding/BoundData.php b/src/AdamWathan/Form/Binding/BoundData.php index 35681b9..cee44cb 100644 --- a/src/AdamWathan/Form/Binding/BoundData.php +++ b/src/AdamWathan/Form/Binding/BoundData.php @@ -41,7 +41,19 @@ protected function dataGet($target, $keyParts) return isset($target[$key]) ? $this->dataGet($target[$key], $remainingKeys) : false; } - return isset($target->{$key}) ? $this->dataGet($target->{$key}, $remainingKeys) : false; + try { + if (property_exists($target, $key)) { + $this->dataGet($target->{$key}, $remainingKeys); + } elseif (method_exists($target, '__get')) { + $this->dataGet($target->__get($key), $remainingKeys); + } else { + return false; + } + } catch (Exception $exception) { + return false; + } + + return $this->dataGet($target->{$key}, $remainingKeys); } protected function transformKey($key) diff --git a/tests/BindingTest.php b/tests/BindingTest.php index 69e6884..9875bf6 100644 --- a/tests/BindingTest.php +++ b/tests/BindingTest.php @@ -136,8 +136,12 @@ public function testBindMagicProperty() $object = new MagicGetter; $this->form->bind($object); - $expected = ''; - $result = (string) $this->form->text('not_set'); + $expected = ''; + $result = (string) $this->form->text('not_magic'); + $this->assertEquals($expected, $result); + + $expected = ''; + $result = (string) $this->form->text('magic'); $this->assertEquals($expected, $result); } @@ -376,8 +380,10 @@ private function getStubObject() class MagicGetter { + public $not_magic = 'foo'; + public function __get($key) { - return 'foo'; + return 'bar'; } } From bbd1d92af18d9a38b7e42aaf2eed12a26165ec26 Mon Sep 17 00:00:00 2001 From: Jesse Leite Date: Fri, 25 Mar 2016 02:14:06 -0400 Subject: [PATCH 5/8] Updating terminology in readme. --- readme.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/readme.md b/readme.md index 8a74e88..f4fe086 100644 --- a/readme.md +++ b/readme.md @@ -11,7 +11,7 @@ Boring name for a boring package. Builds form HTML with a fluent-ish, hopefully - [Remembering Old Input](#remembering-old-input) - [Error Messages](#error-messages) - [CSRF Protection](#csrf-protection) -- [Model Binding](#model-binding) +- [Data Binding](#data-binding) ## Installation @@ -24,7 +24,7 @@ composer require adamwathan/form ### Laravel -> This package works great as a replacement Form Builder that was removed in Laravel 5. The API is different but all of the features are there. +> This package works great as a replacement Form Builder that was removed in Laravel 5. The API is different but all of the features are there. If you are using Laravel 4 or 5, you can register the FormServiceProvider to automatically gain access to the Old Input and Error Message functionality. @@ -353,10 +353,10 @@ Assuming you set a CSRF token when instantiating the Formbuilder (or you are usi token(); ?> ``` - -## Model Binding + +## Data Binding -Sometimes you might have a form where all of the fields match properties on some sort of object in your system, and you want the user to be able to edit those properties. Model binding makes this really easy by allowing you to bind a model to your form that will be used to automatically provide all of the default values for your fields. +Sometimes you might have a form where all of the fields match properties on some sort of object or array in your system, and you want the user to be able to edit that data. Data binding makes this really easy by allowing you to bind an object or array to your form that will be used to automatically provide all of the default values for your fields. ```php $model->first_name = "John"; @@ -375,6 +375,6 @@ $model->date_of_birth = new DateTime('1985-05-06'); > This will work out of the box with Laravel's Eloquent models. -When using model binding, old input will still take priority over any values on your model, so you can still easily redirect the user back to the form with any validation errors without losing any of the data they entered. +When using data binding, old input will still take priority over any of your bound values, so you can still easily redirect the user back to the form with any validation errors without losing any of the data they entered. > Note: Be sure to `bind` before creating any other form elements. From 8f4966c3d59c99ab71fec35142b8339bae28cbdf Mon Sep 17 00:00:00 2001 From: Jesse Leite Date: Fri, 25 Mar 2016 02:21:17 -0400 Subject: [PATCH 6/8] Cleanup. --- src/AdamWathan/Form/Binding/BoundData.php | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/src/AdamWathan/Form/Binding/BoundData.php b/src/AdamWathan/Form/Binding/BoundData.php index cee44cb..d741358 100644 --- a/src/AdamWathan/Form/Binding/BoundData.php +++ b/src/AdamWathan/Form/Binding/BoundData.php @@ -37,23 +37,15 @@ protected function dataGet($target, $keyParts) $key = $keyParts[0]; $remainingKeys = array_slice($keyParts, 1); - if (is_array($target)) { - return isset($target[$key]) ? $this->dataGet($target[$key], $remainingKeys) : false; + if (is_array($target) && isset($target[$key])) { + return $this->dataGet($target[$key], $remainingKeys); } - try { - if (property_exists($target, $key)) { - $this->dataGet($target->{$key}, $remainingKeys); - } elseif (method_exists($target, '__get')) { - $this->dataGet($target->__get($key), $remainingKeys); - } else { - return false; - } - } catch (Exception $exception) { - return false; + if (property_exists($target, $key) || method_exists($target, '__get')) { + return $this->dataGet($target->{$key}, $remainingKeys); } - return $this->dataGet($target->{$key}, $remainingKeys); + return false; } protected function transformKey($key) From 7816a6bf9f3425fba1cb027dffd317c77c3d277b Mon Sep 17 00:00:00 2001 From: Adam Wathan Date: Fri, 25 Mar 2016 08:58:18 -0400 Subject: [PATCH 7/8] Add failing test case to demonstrate issue with using false as default value --- tests/BindingTest.php | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/tests/BindingTest.php b/tests/BindingTest.php index 9875bf6..96aefae 100644 --- a/tests/BindingTest.php +++ b/tests/BindingTest.php @@ -258,13 +258,32 @@ public function testValueTakesPrecedenceOverBinding() public function testBindingOnCheckboxTakesPrecedenceOverDefaultToChecked() { - $object = $this->getStubObject(); + $object = (object) ['published' => 1]; $this->form->bind($object); $expected = ''; - $expected .= ''; $result = (string) $this->form->checkbox('published[]', 1); - $result .= (string) $this->form->checkbox('published[]', 0)->defaultToChecked(); + $this->assertEquals($expected, $result); + + $object = (object) ['published' => 0]; + $this->form->bind($object); + + $expected = ''; + $result = (string) $this->form->checkbox('published[]', 1)->defaultToChecked(); + $this->assertEquals($expected, $result); + + $object = (object) ['published' => true]; + $this->form->bind($object); + + $expected = ''; + $result = (string) $this->form->checkbox('published[]', 1); + $this->assertEquals($expected, $result); + + $object = (object) ['published' => false]; + $this->form->bind($object); + + $expected = ''; + $result = (string) $this->form->checkbox('published[]', 1)->defaultToChecked(); $this->assertEquals($expected, $result); } @@ -373,6 +392,7 @@ private function getStubObject() $obj->number = '0'; $obj->favourite_foods = ['fish', 'chips']; $obj->published = '1'; + $obj->private = false; return $obj; } From 8a7693d841a4cffc63abe5db267344869fbd7092 Mon Sep 17 00:00:00 2001 From: Adam Wathan Date: Fri, 25 Mar 2016 13:43:21 -0400 Subject: [PATCH 8/8] Remove has, use default value instead --- src/AdamWathan/Form/Binding/BoundData.php | 24 +++++++++-------------- src/AdamWathan/Form/FormBuilder.php | 16 ++++++--------- 2 files changed, 15 insertions(+), 25 deletions(-) diff --git a/src/AdamWathan/Form/Binding/BoundData.php b/src/AdamWathan/Form/Binding/BoundData.php index d741358..c4c4f5e 100644 --- a/src/AdamWathan/Form/Binding/BoundData.php +++ b/src/AdamWathan/Form/Binding/BoundData.php @@ -11,41 +11,35 @@ public function __construct($data) $this->data = $data; } - public function has($name) + public function get($name, $default = null) { - return $this->dotGet($this->transformKey($name)) !== false; + return $this->dotGet($this->transformKey($name), $default); } - public function get($name) - { - return $this->dotGet($this->transformKey($name)); - } - - protected function dotGet($dotKey) + protected function dotGet($dotKey, $default) { $keyParts = array_filter(explode('.', $dotKey)); - return $this->dataGet($this->data, $keyParts); + return $this->dataGet($this->data, $keyParts, $default); } - protected function dataGet($target, $keyParts) + protected function dataGet($target, $keyParts, $default) { if (count($keyParts) == 0) { return $target; } - $key = $keyParts[0]; - $remainingKeys = array_slice($keyParts, 1); + $key = array_shift($keyParts); if (is_array($target) && isset($target[$key])) { - return $this->dataGet($target[$key], $remainingKeys); + return $this->dataGet($target[$key], $keyParts, $default); } if (property_exists($target, $key) || method_exists($target, '__get')) { - return $this->dataGet($target->{$key}, $remainingKeys); + return $this->dataGet($target->{$key}, $keyParts, $default); } - return false; + return $default; } protected function transformKey($key) diff --git a/src/AdamWathan/Form/FormBuilder.php b/src/AdamWathan/Form/FormBuilder.php index ace64a0..ab97569 100644 --- a/src/AdamWathan/Form/FormBuilder.php +++ b/src/AdamWathan/Form/FormBuilder.php @@ -230,8 +230,8 @@ public function getValueFor($name) return $this->getOldInput($name); } - if ($this->hasBoundValue($name)) { - return $this->getBoundValue($name); + if ($this->hasBoundData()) { + return $this->getBoundValue($name, null); } return null; @@ -251,18 +251,14 @@ protected function getOldInput($name) return $this->escape($this->oldInput->getOldInput($name)); } - protected function hasBoundValue($name) + protected function hasBoundData() { - if (! isset($this->boundData)) { - return false; - } - - return $this->boundData->has($name); + return isset($this->boundData); } - protected function getBoundValue($name) + protected function getBoundValue($name, $default) { - return $this->escape($this->boundData->get($name)); + return $this->escape($this->boundData->get($name, $default)); } protected function escape($value)