Skip to content

Commit

Permalink
Modified behaviour of setValue(). Mostly refuses to accept invalid JSON
Browse files Browse the repository at this point in the history 
in the first place apart from zero-length strings which SS needs to
clear DBField types with.

Updated docs.
Updated tests.
  • Loading branch information
@phptek
phptek committed Jun 29, 2016
1 parent 2afc862 commit 6ca8f1b
Show file tree
Hide file tree
Showing 8 changed files with 112 additions and 93 deletions.
32 changes: 27 additions & 5 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,42 @@ JSON storage, querying and modification.

## Features

* Query custom JSON data via simple accessors, Postgres-like operators or [JSONPath](http://goessner.net/articles/JsonPath/) expressions.
* Selectively return queries as JSON, Array or cast to SilverStripe `Varchar`, `Int`, `Float` or `Boolean` objects.
* Store JSON in a dedicated JSON-specific `DBField` subclass
* Query JSON data via simple accessors, Postgres-like operators or [JSONPath](http://goessner.net/articles/JsonPath/) expressions.
* Selectively return results of queries as JSON, Array or cast to SilverStripe `Varchar`, `Int`, `Float` or `Boolean` objects.
* Selectively update portions of your source JSON, using [JSONPath](http://goessner.net/articles/JsonPath/) expressions.

## Introduction

The module exposes a fully featured JSON query and update API allowing developers to use XPath-like queries via [JSONPath](http://goessner.net/articles/JsonPath/)
or [Postgres' JSON operators](https://www.postgresql.org/docs/9.5/static/functions-json.html) (with some differences, see below) to query and update JSON data.

### Why?

I've been wanting to write this module for over two years. Prior-to and during that time I have encountered
scenarios in more than one project where storing several (10s) of terse configuration parameters in separate database columns
just seemed crazy.

There's also the time all you wanted was a simple key -> value store but didn't want to muck
about with the overhead of two or more datastores like MySQL or Postgres together with MongoDB for example.

On one project I even went as far as an entire tab within `getCMSFields()` that used multiple standard CMS form fields
taking their data from, and updating to, a single field comprising JSON data for one spect of the system's
configuration.

And if you needed further convincing that a JSON store in an RDBMS is not such a crazy idea, well Postgres, MySQL, Oracle and MSSQL 2016
all have, or at time of writing, are planning to have such a field...

### Postgres

In Postgres both the `->` and `->>` operators act as string and integer key matchers on a JSON array or object respectively. The module
however treats both source types the same - they are after all *both JSON* so `->` is used as an **Integer Matcher** and `->>` as a **String Matcher**
*regardless* of the "type" of source JSON stored. The `#>` **Path Matcher** operator can act as an object or a text matcher, but the module wishes to simplify things and as such
the `#>` operator is *just a simple path matcher*.

Regardless of the type of query in-use you can set what type you'd like the data returned in via the `setReturnType()` method, on a query by query basis.
### Return types

Regardless of the type of query you can set what type you'd like the data returned in via the `setReturnType()` method on a query by query basis.

Legitimate types are:

Expand All @@ -45,9 +66,10 @@ If using `SilverStripe`, the module will automatically cast the result(s) to one

If there are multiple results from a query, the output will be an indexed array containing a single-value array for each result found.

The module also allows developers to selectively *update* all, or just parts of the source JSON, via JSONPath expressions.
The module also allows developers to selectively *update* all, or just parts of the source JSON, via JSONPath expressions passed
to an overloaded `setValue()` method.

See [the usage docs](docs/en/usage.md) for examples of JSONPath and Postgres queries and updating.
See [the usage docs](docs/en/usage.md) for examples of JSONPath and Postgres querying and updating.

Note: This module's query API is based on a relatively simple JSON to array conversion principle.
It does *not* use Postgres' or MySQL's native JSON operators at or below the level of the ORM. The aim however
Expand Down
2 changes: 1 addition & 1 deletion code/backends/JSONBackend.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ abstract public function matchOnPath();
public function matchOnExpr()
{
if (!is_string($this->operand)) {
$msg = 'Non-string operand passed to: ' . __FUNCTION__;
$msg = 'Non-string operand passed to: ' . __FUNCTION__ . '()';
throw new JSONTextException($msg);
}

Expand Down
6 changes: 3 additions & 3 deletions code/backends/PostgresJSONBackend.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class PostgresJSONBackend extends JSONBackend
public function matchOnInt()
{
if (!is_int($this->operand)) {
$msg = 'Non-integer passed to: ' . __FUNCTION__;
$msg = 'Non-integer passed to: ' . __FUNCTION__ . '()';
throw new JSONTextException($msg);
}

Expand All @@ -44,7 +44,7 @@ public function matchOnInt()
public function matchOnStr()
{
if (!is_string($this->operand)) {
$msg = 'Non-string passed to: ' . __FUNCTION__;
$msg = 'Non-string passed to: ' . __FUNCTION__ . '()';
throw new JSONTextException($msg);
}

Expand All @@ -64,7 +64,7 @@ public function matchOnStr()
*/
public function matchOnPath()
{
if (!is_string($this->operand) || !$this->jsonText->isJson($this->operand)) {
if (!is_string($this->operand) || !$this->jsonText->isValidJson($this->operand)) {
$msg = 'Invalid JSON passed as operand on RHS.';
throw new JSONTextException($msg);
}
Expand Down
79 changes: 44 additions & 35 deletions code/models/fieldtypes/JSONText.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ public function scaffoldFormField($title = null)
public function setReturnType($type)
{
if (!in_array($type, $this->config()->return_types)) {
$msg = 'Bad type: ' . $type . ' passed to ' . __FUNCTION__;
$msg = 'Bad type: ' . $type . ' passed to ' . __FUNCTION__ . '()';
throw new JSONTextException($msg);
}

Expand All @@ -165,7 +165,7 @@ public function getJSONStore()
return new JsonStore('[]');
}

if (!$this->isJson($value)) {
if (!$this->isValidJson($value)) {
$msg = 'DB data is munged.';
throw new JSONTextException($msg);
}
Expand All @@ -190,32 +190,6 @@ public function getStoreAsArray()
return $store;
}

/**
* 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, array $allowed = [])
{
if (!isset($value)) {
return false;
}

$value = trim($value);

if (!empty($allowed)) {
// Valid JSON, but invalid from the perspective of {@link JSONPath}.
$invalid = array_diff(['true', 'false', ''], $allowed);
return !in_array($value, $invalid);
}

return !is_null(json_decode($value, true));
}

/**
* Convert an array to JSON via json_encode().
*
Expand Down Expand Up @@ -328,7 +302,7 @@ public function nth($n)
}

if (!is_int($n)) {
$msg = 'Argument passed to ' . __FUNCTION__ . ' must be an integer.';
$msg = 'Argument passed to ' . __FUNCTION__ . '() must be an integer.';
throw new JSONTextException($msg);
}

Expand Down Expand Up @@ -430,20 +404,20 @@ 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__;
if (!$this->isValidDBValue($value)) {
$msg = 'Invalid data passed to ' . __FUNCTION__ . '()';
throw new JSONTextException($msg);
}

$this->value = $value;
} else {
if (!$this->isValidExpression($expr)) {
$msg = 'Invalid JSONPath expression: ' . $expr . ' passed to ' . __FUNCTION__;
$msg = 'Invalid JSONPath expression: ' . $expr . ' passed to ' . __FUNCTION__ . '()';
throw new JSONTextException($msg);
}

if (!$this->getJSONStore()->set($expr, $value)) {
$msg = 'Failed to properly set custom data to the JSONStore in ' . __FUNCTION__;
$msg = 'Failed to properly set custom data to the JSONStore in ' . __FUNCTION__ . '()';
throw new JSONTextException($msg);
}

Expand Down Expand Up @@ -490,7 +464,7 @@ private function returnAsType($data)
return $this->toSSTypes($data);
}

$msg = 'Bad argument passed to ' . __FUNCTION__;
$msg = 'Bad argument passed to ' . __FUNCTION__ . '()';
throw new JSONTextException($msg);
}

Expand Down Expand Up @@ -518,6 +492,41 @@ protected function createBackendInst($operand)
]);
}

/**
* Utility method to determine whether a value is really valid JSON or not.
* The Peekmo JSONStore lib won't accept otherwise valid JSON values like `true`, `false` & `""` so these need
* to be disallowed.
*
* @param string $value
* @return boolean
*/
public function isValidJson($value)
{
if (!isset($value)) {
return false;
}

$value = trim($value);
return !is_null(json_decode($value, true));
}

/**
* @return boolean
*/
public function isValidDBValue($value) {
$value = trim($value);

if (in_array($value, ['true', 'false'])) {
return false;
}

if (is_string($value) && strlen($value) === 0) {
return true;
}

return $this->isValidJson($value);
}

/**
* Is the passed JSON operator valid?
*
Expand Down
22 changes: 0 additions & 22 deletions tests/JSONTextBasicTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,6 @@ public function testGetValueAsJSONStore()

$field->setValue('');
$this->assertEquals([], $field->getStoreAsArray());

$field->setValue('{');
$this->setExpectedException('\JSONText\Exceptions\JSONTextException');
$field->getJSONStore();
}

public function testFirst()
Expand Down Expand Up @@ -91,12 +87,6 @@ public function testFirst()
$field->setValue('');
$this->assertInternalType('array', $field->first());
$this->assertCount(0, $field->first());

// Test: Invalid
$field->setReturnType('array');
$field->setValue('{');
$this->setExpectedException('\JSONText\Exceptions\JSONTextException');
$field->first();
}

public function testLast()
Expand Down Expand Up @@ -131,12 +121,6 @@ public function testLast()
$field->setValue('');
$this->assertInternalType('array', $field->last());
$this->assertCount(0, $field->last());

// Test: Invalid
$field->setReturnType('array');
$field->setValue('{');
$this->setExpectedException('\JSONText\Exceptions\JSONTextException');
$field->last();
}

public function testNth()
Expand Down Expand Up @@ -171,12 +155,6 @@ public function testNth()
$field->setValue('');
$this->assertInternalType('array', $field->nth(1));
$this->assertCount(0, $field->nth(1));

// Test: Invalid
$field->setReturnType('array');
$field->setValue('{');
$this->setExpectedException('\JSONText\Exceptions\JSONTextException');
$field->nth(1);
}

/**
Expand Down
12 changes: 0 additions & 12 deletions tests/JSONTextQueryTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,6 @@ public function testQueryWithMatchOnInt()
$field->setReturnType('array');
$field->setValue('["ass"]');
$this->assertEquals(['ass'], $field->query('->', 0));

// Test: Invalid #3
$field->setReturnType('array');
$field->setValue('{');
$this->setExpectedException('\JSONText\Exceptions\JSONTextException');
$field->query('->', 3);
}

/**
Expand Down Expand Up @@ -233,12 +227,6 @@ public function testQueryWithMatchOnStr()
$field->setReturnType('array');
$field->setValue('["trabant"]');
$this->assertEquals([], $field->query('->', 1));

// Test: Invalid #2
$field->setReturnType('array');
$field->setValue('{');
$this->setExpectedException('\JSONText\Exceptions\JSONTextException');
$field->query('->', 3);
}

/**
Expand Down
6 changes: 5 additions & 1 deletion tests/JSONTextSetValueTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,11 @@ public function testSetValueOnSourceArray()
// Reset expected exception
$this->setExpectedException(null);

// Ensure default SS behaviour is respected
// Invalid #3
$this->setExpectedException('\JSONText\Exceptions\JSONTextException');
$field->setValue('{'); // Invalid JSON. Period.

// Ensure default SS behaviour is respected with empty strings, evenm though it's invalid JSON
$field->setValue('');
$this->assertEquals('', $field->getValue());
}
Expand Down
46 changes: 32 additions & 14 deletions tests/JSONTextTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class JSONTextTest extends SapphireTest
* @todo There are a ton more permutations of a JSONPath regex
* See the walk() method in JSONStore
*/
public function testIsExpressionValid()
public function testIsValidExpression()
{
$field = JSONText::create('MyJSON');

Expand All @@ -25,24 +25,42 @@ public function testIsExpressionValid()
$this->assertFalse($field->isValidExpression('$[2]'));
}

/**
* @return void
*/
public function testIsValidJson()
{
$field = JSONText::create('MyJSON');

$this->assertFalse($field->isValidJson(''));
$this->assertTrue($field->isValidJson('true'));
$this->assertTrue($field->isValidJson('false'));
$this->assertFalse($field->isValidJson('null'));
$this->assertFalse($field->isValidJson("['one']"));
$this->assertFalse($field->isValidJson('["one]'));
$this->assertFalse($field->isValidJson('{{{'));
$this->assertTrue($field->isValidJson('[]'));
$this->assertTrue($field->isValidJson('["one"]'));
$this->assertTrue($field->isValidJson('["one","two"]'));
$this->assertTrue($field->isValidJson('{"cars":{"american":["buick","oldsmobile"]}}'));
}


/**
* 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
*
* @return void
*/
public function testIsJson()
public function testIsValidDBValue()
{
$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('', ['']));

$this->assertFalse($field->isValidDBValue('true'));
$this->assertFalse($field->isValidDBValue('false'));
$this->assertFalse($field->isValidDBValue('null'));
$this->assertTrue($field->isValidDBValue(''));
$this->assertTrue($field->isValidJson('["one","two"]'));
$this->assertTrue($field->isValidJson('{"cars":{"american":["buick","oldsmobile"]}}'));
}
}

0 comments on commit 6ca8f1b

Please sign in to comment.