- Fixed non-namespaced calls to HiddenField.

- Added JSON check to setValue() when called with JSONPath expression. - Added tests for JSONText::isJSON()
phptek · Jun 28, 2016 · c1f2b60 · c1f2b60
1 parent 5cb32aa
commit c1f2b60
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 9 deletions.
diff --git a/code/models/fieldtypes/JSONText.php b/code/models/fieldtypes/JSONText.php
@@ -122,7 +122,7 @@ public function requireField()
      */
     public function scaffoldSearchField($title = null)
     {
-        return HiddenField::create($this->getName());
+        return \HiddenField::create($this->getName());
     }
 
     /**
@@ -131,7 +131,7 @@ public function scaffoldSearchField($title = null)
      */
     public function scaffoldFormField($title = null)
     {
-        return HiddenField::create($this->getName());
+        return \HiddenField::create($this->getName());
     }
 
     /**
@@ -192,12 +192,30 @@ public function getStoreAsArray()
 
     /**
      * Utility method to determine whether the data is really JSON or not.
+     * The Peekmo JSONStore lib won't accept otherwise valid JSON scalars like `true`, `false` & `null` so these need
+     * to be disallowed.
      * 
      * @param string $value
+     * @param array $allowed    An optional array of allowed "invalid" JSON values
      * @return boolean
      */
-    public function isJson($value)
+    public function isJson($value, array $allowed = [])
     {
+        if (!isset($value)) {
+            return false;
+        }
+
+        $value = trim($value);
+
+        if (!empty($allowed)) {
+            $invalid = array_diff(['true', 'false', ''], $allowed);
+            if (in_array($value, $invalid)) {
+                return false;
+            }
+
+            return true;
+        }
+
         return !is_null(json_decode($value, true));
     }
 
@@ -421,6 +439,11 @@ private function marshallQuery($args, $type = 1)
     public function setValue($value, $record = null, $expr = '')
     {
         if (empty($expr)) {
+            $allowed = ['']; // Ordinarily this would be invalid JSON, but SS allows it to clear a field
+            if (!$this->isJson($value, $allowed)) {
+                $msg = 'Invalid data passed to ' . __FUNCTION__;
+                throw new JSONTextException($msg);
+            }
             $this->value = $value;
         } else {
             if (!$this->isValidExpression($expr)) {
@@ -435,8 +458,7 @@ public function setValue($value, $record = null, $expr = '')
 
             $this->value = $this->jsonStore->toString();
         }
-
-        // Deal with standard SS behaviour
+
         parent::setValue($this->value, $record);
 
         return $this;
@@ -446,7 +468,7 @@ public function setValue($value, $record = null, $expr = '')
      * Determine the desired userland format to return all query API method results in.
      * 
      * @param mixed
-     * @return mixed
+     * @return mixed array|null
      * @throws \JSONText\Exceptions\JSONTextException
      */
     private function returnAsType($data)

diff --git a/tests/JSONTextSetValueTest.php b/tests/JSONTextSetValueTest.php
@@ -66,6 +66,20 @@ public function testSetValueOnSourceArray()
         // Invalid #1
         $this->setExpectedException('\JSONText\Exceptions\JSONTextException');
         $field->setValue(99.99, null, '$[6]'); // Invalid JSON path expression
+
+        // Reset expected exception
+        $this->setExpectedException(null);
+
+        // Invalid #2
+        $this->setExpectedException('\JSONText\Exceptions\JSONTextException');
+        $field->setValue('true'); // Invalid JSON passed to setValue()
+
+        // Reset expected exception
+        $this->setExpectedException(null);
+
+        // Ensure default SS behaviour is respected
+        $field->setValue('');
+        $this->assertEquals('', $field->getValue());
     }
 
     /**

diff --git a/tests/JSONTextTest.php b/tests/JSONTextTest.php
@@ -24,4 +24,25 @@ public function testIsExpressionValid()
         $this->assertFalse($field->isValidExpression('$'));
         $this->assertFalse($field->isValidExpression('$[2]'));
     }
+
+    /**
+     * Ordinarily we can just use !is_null(json_decode($json)) but SS allows empty strings passed to setValue() so we need
+     * to allow otherwise invalid JSON by means of an optional 2nd param
+     */
+    public function testIsJson()
+    {
+        $field = JSONText::create('MyJSON');
+
+        $this->assertFalse($field->isJson(''));
+        $this->assertTrue($field->isJson('true'));
+        $this->assertTrue($field->isJson('false'));
+        $this->assertFalse($field->isJson('null'));
+        $this->assertFalse($field->isJson("['one']"));
+        $this->assertFalse($field->isJson('["one]'));
+        $this->assertTrue($field->isJson('[]'));
+        $this->assertTrue($field->isJson('["one"]'));
+        $this->assertTrue($field->isJson('["one","two"]'));
+        $this->assertTrue($field->isJson('{"cars":{"american":["buick","oldsmobile"]}}'));
+        $this->assertTrue($field->isJson('', ['']));
+    }
 }
diff --git a/tests/fixtures/yml/MyAwesomeJSONModel.yml b/tests/fixtures/yml/MyAwesomeJSONModel.yml
@@ -2,6 +2,4 @@ MyAwesomeJSONModel:
   json-as-object:
     MyJSON: '{"cars":{"american":["buick","oldsmobile"]}}'
   json-as-array:
-    MyJSON: '["buick","oldsmobile"]'
-  json-as-boolean:
-    MyJSON: 'true'
+    MyJSON: '["buick","oldsmobile"]'