From 1b8edf8a19e5833deff17b78b32b64da0b5c30ec Mon Sep 17 00:00:00 2001 From: Richard Tattersall Date: Tue, 21 Jul 2015 11:45:09 +0100 Subject: [PATCH 01/14] Prevent null values being added to ExtendedGraph --- src/classes/ExtendedGraph.class.php | 40 +++++++++++++++++++-------- src/mongo/MongoGraph.class.php | 43 +++++++++++++++++++++-------- 2 files changed, 60 insertions(+), 23 deletions(-) diff --git a/src/classes/ExtendedGraph.class.php b/src/classes/ExtendedGraph.class.php index f7579ff9..70816e9b 100644 --- a/src/classes/ExtendedGraph.class.php +++ b/src/classes/ExtendedGraph.class.php @@ -171,24 +171,40 @@ public function add_literal_triple($s, $p, $o, $lang = null, $dt = null) { * @return bool */ private function _add_triple($s, $p, Array $o_info) { - if (!isset($this->_index[$s])) { - $this->_index[$s] = array(); - $this->_index[$s][$p] = array( $o_info ); - return true; - } - elseif (!isset($this->_index[$s][$p])) { - $this->_index[$s][$p] = array( $o_info); - return true; - } - else { - if ( ! in_array( $o_info, $this->_index[$s][$p] ) ) { - $this->_index[$s][$p][] = $o_info; + if($this->isValueValid($o_info['value'])) { + if (!isset($this->_index[$s])) { + $this->_index[$s] = array(); + $this->_index[$s][$p] = array($o_info); + return true; + } elseif (!isset($this->_index[$s][$p])) { + $this->_index[$s][$p] = array($o_info); return true; + } else { + if (!in_array($o_info, $this->_index[$s][$p])) { + $this->_index[$s][$p][] = $o_info; + return true; + } } } return false; } + + /** + * Check if a triple value is valid. + * + * Currently null values are not valid, + * although this may expand to include any non-strings + * + * @return bool + */ + protected function isValueValid($value){ + if($value === null){ + return false; + } + return true; + } + /** * @deprecated this is deprecated */ diff --git a/src/mongo/MongoGraph.class.php b/src/mongo/MongoGraph.class.php index 554db802..a8958b56 100644 --- a/src/mongo/MongoGraph.class.php +++ b/src/mongo/MongoGraph.class.php @@ -129,7 +129,11 @@ private function add_tarray_to_index($tarray) if($key[0] != '_') { $predicate = $this->qname_to_uri($key); - $predObjects[$predicate] = $this->toGraphValueObject($value); + $graphValueObject = $this->toGraphValueObject($value); + // Only add if valid values have been found + if($graphValueObject !== false) { + $predObjects[$predicate] = $graphValueObject; + } } } $_i[$this->_labeller->qname_to_alias($tarray["_id"][_ID_RESOURCE])] = $predObjects; @@ -138,39 +142,56 @@ private function add_tarray_to_index($tarray) /** * Convert from Tripod value object format (comapct) to ExtendedGraph format (verbose) + * * @param array $mongoValueObject - * @return array + * @return array| false an array of values or false if the value is not valid */ private function toGraphValueObject($mongoValueObject) { - $simpleGraphValueObject = null; + $simpleGraphValueObject = array(); if (array_key_exists(VALUE_LITERAL,$mongoValueObject)) { - // single value literal - $simpleGraphValueObject[] = array( - 'type'=>'literal', - 'value'=>$mongoValueObject[VALUE_LITERAL]); + // only allow valid values + if($this->isValueValid($mongoValueObject[VALUE_LITERAL])){ + // single value literal + $simpleGraphValueObject[] = array( + 'type'=>'literal', + 'value'=>$mongoValueObject[VALUE_LITERAL]); + } } else if (array_key_exists(VALUE_URI,$mongoValueObject)) { - // single value literal - $simpleGraphValueObject[] = array( - 'type'=>'uri', - 'value'=>$this->_labeller->qname_to_alias($mongoValueObject[VALUE_URI])); + // only allow valid values + if($this->isValueValid($mongoValueObject[VALUE_URI])) { + // single value uri + $simpleGraphValueObject[] = array( + 'type' => 'uri', + 'value' => $this->_labeller->qname_to_alias($mongoValueObject[VALUE_URI])); + } } else { + // If we have an array of values foreach ($mongoValueObject as $kvp) { foreach ($kvp as $type=>$value) { + // Only add valid values + if(!$this->isValueValid($value)){ + continue; + } $simpleGraphValueObject[] = array( 'type'=>($type==VALUE_LITERAL) ? 'literal' : 'uri', 'value'=>($type==VALUE_URI) ? $this->_labeller->qname_to_alias($value) : $value); } } } + // If we don't have any values, then respond with false + if(empty($simpleGraphValueObject)){ + return false; + } + // Otherwise we have found valid values return $simpleGraphValueObject; } From ec3fb44f672cc5cc37a23b16c267267c02febcc2 Mon Sep 17 00:00:00 2001 From: Richard Tattersall Date: Tue, 21 Jul 2015 12:48:02 +0100 Subject: [PATCH 02/14] Added tests --- test/unit/ExtendedGraphTest.php | 31 ++++++++++++++++++++++++++++++ test/unit/mongo/MongoGraphTest.php | 29 ++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/test/unit/ExtendedGraphTest.php b/test/unit/ExtendedGraphTest.php index 37347d4e..c46c61c4 100644 --- a/test/unit/ExtendedGraphTest.php +++ b/test/unit/ExtendedGraphTest.php @@ -23,6 +23,37 @@ protected function setUp() echo "\nTest: {$className}->{$testName}\n"; } + public function testAddNullToLiteralResultsInNoTriple() + { + $graph = new ExtendedGraph(); + $graph->add_literal_triple('http://some/subject/1', 'http://some/predicate', null); + + $result = $graph->subject_has_property('http://some/subject/1', 'http://some/predicate'); + $this->assertFalse($result); + } + + public function testAddNullToResourceResultsInNoTriple() + { + $graph = new ExtendedGraph(); + $graph->add_resource_triple('http://some/subject/1', 'http://some/predicate', null); + + $result = $graph->subject_has_property('http://some/subject/1', 'http://some/predicate'); + $this->assertFalse($result); + } + + public function testAddGraphWithNullValueDoesNotAddNullTriple() + { + $graph = new ExtendedGraph(); + $graph->add_resource_triple('http://some/subject/1', 'http://some/predicate', null); + $graph->add_resource_triple('http://some/subject/1', 'http://some/other/predicate', 'triple'); + + $nullTriple = $graph->subject_has_property('http://some/subject/1', 'http://some/predicate'); + $stringTriple = $graph->subject_has_property('http://some/subject/1', 'http://some/other/predicate'); + + $this->assertFalse($nullTriple); + $this->assertTrue($stringTriple); + } + public function testRemoveProperties() { $graph = new ExtendedGraph(); diff --git a/test/unit/mongo/MongoGraphTest.php b/test/unit/mongo/MongoGraphTest.php index eba3556c..032a446f 100644 --- a/test/unit/mongo/MongoGraphTest.php +++ b/test/unit/mongo/MongoGraphTest.php @@ -102,6 +102,35 @@ public function testAddTripodArraySingleDoc() $this->assertEquals($expected, $g); } + public function testAddTripodArrayContainingNullValues() + { + $doc = array( + "_id"=>array("r"=>"http://talisaspire.com/works/4d101f63c10a6-2", "c"=>"http://talisaspire.com/works/4d101f63c10a6-2"), + "_version"=>0, + "dct:subject"=>array("u"=>"http://talisaspire.com/disciplines/physics"), + "dct:publisher"=>array("u"=>null), + "rdf:type"=>array( + array("u"=>null), + array("l"=>null), + array("l"=>"a Value"), + array("u"=>"http://talisaspire.com/schema#Work") + ), + "bibo:isbn13"=>array("l"=>"9211234567890"), + "bibo:isbn10"=>array("l"=>null) + ); + + $expected = new \Tripod\Mongo\MongoGraph(); + $expected->add_literal_triple("http://talisaspire.com/works/4d101f63c10a6-2", $expected->qname_to_uri("bibo:isbn13"),"9211234567890"); + $expected->add_resource_triple("http://talisaspire.com/works/4d101f63c10a6-2", $expected->qname_to_uri("dct:subject"),"http://talisaspire.com/disciplines/physics"); + $expected->add_literal_triple("http://talisaspire.com/works/4d101f63c10a6-2", $expected->qname_to_uri("rdf:type"),"a Value"); + $expected->add_resource_triple("http://talisaspire.com/works/4d101f63c10a6-2", $expected->qname_to_uri("rdf:type"),"http://talisaspire.com/schema#Work"); + + $g = new \Tripod\Mongo\MongoGraph(); + $g->add_tripod_array($doc); + + $this->assertEquals($expected, $g); + } + public function testAddTripodArrayWhenAddingViews() { // view contains 4 subgraphs From 5c898790157448913913b1a4734072a51279de09 Mon Sep 17 00:00:00 2001 From: Richard Tattersall Date: Tue, 21 Jul 2015 14:34:06 +0100 Subject: [PATCH 03/14] Make valid values stricter as per PR comments --- src/classes/ExtendedGraph.class.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/classes/ExtendedGraph.class.php b/src/classes/ExtendedGraph.class.php index 70816e9b..9a0d2024 100644 --- a/src/classes/ExtendedGraph.class.php +++ b/src/classes/ExtendedGraph.class.php @@ -199,7 +199,7 @@ private function _add_triple($s, $p, Array $o_info) { * @return bool */ protected function isValueValid($value){ - if($value === null){ + if(!is_string($value) && !is_int($value) && !is_array($value) && !is_float($value) && !is_bool($value)){ return false; } return true; From ac2071e0cb2f283c660a6c05f7a6a90ed20ac31b Mon Sep 17 00:00:00 2001 From: Richard Tattersall Date: Tue, 21 Jul 2015 14:36:45 +0100 Subject: [PATCH 04/14] renamed method as per PR --- src/classes/ExtendedGraph.class.php | 4 ++-- src/mongo/MongoGraph.class.php | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/classes/ExtendedGraph.class.php b/src/classes/ExtendedGraph.class.php index 9a0d2024..57328929 100644 --- a/src/classes/ExtendedGraph.class.php +++ b/src/classes/ExtendedGraph.class.php @@ -171,7 +171,7 @@ public function add_literal_triple($s, $p, $o, $lang = null, $dt = null) { * @return bool */ private function _add_triple($s, $p, Array $o_info) { - if($this->isValueValid($o_info['value'])) { + if($this->isValidTripleValue($o_info['value'])) { if (!isset($this->_index[$s])) { $this->_index[$s] = array(); $this->_index[$s][$p] = array($o_info); @@ -198,7 +198,7 @@ private function _add_triple($s, $p, Array $o_info) { * * @return bool */ - protected function isValueValid($value){ + protected function isValidTripleValue($value){ if(!is_string($value) && !is_int($value) && !is_array($value) && !is_float($value) && !is_bool($value)){ return false; } diff --git a/src/mongo/MongoGraph.class.php b/src/mongo/MongoGraph.class.php index a8958b56..2703005d 100644 --- a/src/mongo/MongoGraph.class.php +++ b/src/mongo/MongoGraph.class.php @@ -153,7 +153,7 @@ private function toGraphValueObject($mongoValueObject) if (array_key_exists(VALUE_LITERAL,$mongoValueObject)) { // only allow valid values - if($this->isValueValid($mongoValueObject[VALUE_LITERAL])){ + if($this->isValidTripleValue($mongoValueObject[VALUE_LITERAL])){ // single value literal $simpleGraphValueObject[] = array( 'type'=>'literal', @@ -163,7 +163,7 @@ private function toGraphValueObject($mongoValueObject) else if (array_key_exists(VALUE_URI,$mongoValueObject)) { // only allow valid values - if($this->isValueValid($mongoValueObject[VALUE_URI])) { + if($this->isValidTripleValue($mongoValueObject[VALUE_URI])) { // single value uri $simpleGraphValueObject[] = array( 'type' => 'uri', @@ -178,7 +178,7 @@ private function toGraphValueObject($mongoValueObject) foreach ($kvp as $type=>$value) { // Only add valid values - if(!$this->isValueValid($value)){ + if(!$this->isValidTripleValue($value)){ continue; } $simpleGraphValueObject[] = array( From 54fbb4f755cc814a5ceef93d69a4e3ec32bf452e Mon Sep 17 00:00:00 2001 From: Richard Tattersall Date: Tue, 21 Jul 2015 14:44:18 +0100 Subject: [PATCH 05/14] simplified conditional from PR --- src/classes/ExtendedGraph.class.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/classes/ExtendedGraph.class.php b/src/classes/ExtendedGraph.class.php index 57328929..371d2984 100644 --- a/src/classes/ExtendedGraph.class.php +++ b/src/classes/ExtendedGraph.class.php @@ -193,13 +193,10 @@ private function _add_triple($s, $p, Array $o_info) { /** * Check if a triple value is valid. * - * Currently null values are not valid, - * although this may expand to include any non-strings - * * @return bool */ protected function isValidTripleValue($value){ - if(!is_string($value) && !is_int($value) && !is_array($value) && !is_float($value) && !is_bool($value)){ + if(!is_scalar($value) && !is_array($value)){ return false; } return true; From cdb99bec81b1606ecca1c9b23acc365f6bc7f225 Mon Sep 17 00:00:00 2001 From: Richard Tattersall Date: Wed, 22 Jul 2015 12:30:17 +0100 Subject: [PATCH 06/14] Fixed up from PR comments --- src/classes/ExtendedGraph.class.php | 64 +++++++++++++++++++---------- src/mongo/MongoGraph.class.php | 21 +++++++--- test/unit/ExtendedGraphTest.php | 42 ++++++++++++++++--- 3 files changed, 93 insertions(+), 34 deletions(-) diff --git a/src/classes/ExtendedGraph.class.php b/src/classes/ExtendedGraph.class.php index 371d2984..22779539 100644 --- a/src/classes/ExtendedGraph.class.php +++ b/src/classes/ExtendedGraph.class.php @@ -141,7 +141,10 @@ public function make_resource_array($resource) { * @return boolean true if the triple was new, false if it already existed in the graph */ public function add_resource_triple($s, $p, $o) { - return $this->_add_triple($s, $p, array('type' => strpos($o, '_:' ) === 0 ? 'bnode' : 'uri', 'value' => $o)); + if($this->isValidResourceValue($o)) { + return $this->_add_triple($s, $p, array('type' => strpos($o, '_:') === 0 ? 'bnode' : 'uri', 'value' => $o)); + } + return false; } /** @@ -154,14 +157,17 @@ public function add_resource_triple($s, $p, $o) { * @return boolean true if the triple was new, false if it already existed in the graph */ public function add_literal_triple($s, $p, $o, $lang = null, $dt = null) { - $o_info = array('type' => 'literal', 'value' => $o); - if ( $lang != null ) { - $o_info['lang'] = $lang; - } - if ( $dt != null ) { - $o_info['datatype'] = $dt; + if($this->isValidLiteralValue($o)) { + $o_info = array('type' => 'literal', 'value' => $o); + if ($lang != null) { + $o_info['lang'] = $lang; + } + if ($dt != null) { + $o_info['datatype'] = $dt; + } + return $this->_add_triple($s, $p, $o_info); } - return $this->_add_triple($s, $p, $o_info); + return false; } /** @@ -171,32 +177,46 @@ public function add_literal_triple($s, $p, $o, $lang = null, $dt = null) { * @return bool */ private function _add_triple($s, $p, Array $o_info) { - if($this->isValidTripleValue($o_info['value'])) { - if (!isset($this->_index[$s])) { - $this->_index[$s] = array(); - $this->_index[$s][$p] = array($o_info); - return true; - } elseif (!isset($this->_index[$s][$p])) { - $this->_index[$s][$p] = array($o_info); + if (!isset($this->_index[$s])) { + $this->_index[$s] = array(); + $this->_index[$s][$p] = array($o_info); + return true; + } elseif (!isset($this->_index[$s][$p])) { + $this->_index[$s][$p] = array($o_info); + return true; + } else { + if (!in_array($o_info, $this->_index[$s][$p])) { + $this->_index[$s][$p][] = $o_info; return true; - } else { - if (!in_array($o_info, $this->_index[$s][$p])) { - $this->_index[$s][$p][] = $o_info; - return true; - } } } return false; } + /** + * Check if a triple value is valid. + * + * Ideally a valid literal value should be a string + * but accepting scalars so we can handle legacy data + * which was not type-checked. + * + * @return bool + */ + protected function isValidLiteralValue($value){ + if(!is_scalar($value)){ + return false; + } + return true; + } + /** * Check if a triple value is valid. * * @return bool */ - protected function isValidTripleValue($value){ - if(!is_scalar($value) && !is_array($value)){ + protected function isValidResourceValue($value){ + if(!is_string($value)){ return false; } return true; diff --git a/src/mongo/MongoGraph.class.php b/src/mongo/MongoGraph.class.php index 2703005d..7b2ead41 100644 --- a/src/mongo/MongoGraph.class.php +++ b/src/mongo/MongoGraph.class.php @@ -153,7 +153,7 @@ private function toGraphValueObject($mongoValueObject) if (array_key_exists(VALUE_LITERAL,$mongoValueObject)) { // only allow valid values - if($this->isValidTripleValue($mongoValueObject[VALUE_LITERAL])){ + if($this->isValidLiteralValue($mongoValueObject[VALUE_LITERAL])){ // single value literal $simpleGraphValueObject[] = array( 'type'=>'literal', @@ -163,7 +163,7 @@ private function toGraphValueObject($mongoValueObject) else if (array_key_exists(VALUE_URI,$mongoValueObject)) { // only allow valid values - if($this->isValidTripleValue($mongoValueObject[VALUE_URI])) { + if($this->isValidResourceValue($mongoValueObject[VALUE_URI])) { // single value uri $simpleGraphValueObject[] = array( 'type' => 'uri', @@ -177,12 +177,21 @@ private function toGraphValueObject($mongoValueObject) { foreach ($kvp as $type=>$value) { - // Only add valid values - if(!$this->isValidTripleValue($value)){ - continue; + // Make sure the value is valid + if($type==VALUE_LITERAL){ + if(!$this->isValidLiteralValue($value)){ + continue; + } + $valueTypeLabel = 'literal'; + } + else{ + if(!$this->isValidResourceValue($value)){ + continue; + } + $valueTypeLabel = 'uri'; } $simpleGraphValueObject[] = array( - 'type'=>($type==VALUE_LITERAL) ? 'literal' : 'uri', + 'type'=>$valueTypeLabel, 'value'=>($type==VALUE_URI) ? $this->_labeller->qname_to_alias($value) : $value); } } diff --git a/test/unit/ExtendedGraphTest.php b/test/unit/ExtendedGraphTest.php index c46c61c4..0610a0c3 100644 --- a/test/unit/ExtendedGraphTest.php +++ b/test/unit/ExtendedGraphTest.php @@ -23,28 +23,51 @@ protected function setUp() echo "\nTest: {$className}->{$testName}\n"; } - public function testAddNullToLiteralResultsInNoTriple() + /** + * @dataProvider addInvalidValueToLiteralResultsInNoTriple_Provider + */ + public function testAddInvalidValueToLiteralResultsInNoTriple($value) { $graph = new ExtendedGraph(); - $graph->add_literal_triple('http://some/subject/1', 'http://some/predicate', null); + $graph->add_literal_triple('http://some/subject/1', 'http://some/predicate', $value); $result = $graph->subject_has_property('http://some/subject/1', 'http://some/predicate'); $this->assertFalse($result); } + public function addInvalidValueToLiteralResultsInNoTriple_Provider(){ + return array( + array(null), + array(new stdClass()), + array(function(){}) + ); + } - public function testAddNullToResourceResultsInNoTriple() + /** + * @dataProvider addInvalidValueToResourceResultsInNoTriple_Provider + */ + public function testAddInvalidValueToResourceResultsInNoTriple($value) { $graph = new ExtendedGraph(); - $graph->add_resource_triple('http://some/subject/1', 'http://some/predicate', null); + $graph->add_resource_triple('http://some/subject/1', 'http://some/predicate', $value); $result = $graph->subject_has_property('http://some/subject/1', 'http://some/predicate'); $this->assertFalse($result); } + public function addInvalidValueToResourceResultsInNoTriple_Provider(){ + return array( + array(null), + array(new stdClass()), + array(function(){}) + ); + } - public function testAddGraphWithNullValueDoesNotAddNullTriple() + /** + * @dataProvider addGraphWithInvalidValueDoesNotAddNullTriple_Provider + */ + public function testAddGraphWithInvalidValueDoesNotAddNullTriple($value) { $graph = new ExtendedGraph(); - $graph->add_resource_triple('http://some/subject/1', 'http://some/predicate', null); + $graph->add_resource_triple('http://some/subject/1', 'http://some/predicate', $value); $graph->add_resource_triple('http://some/subject/1', 'http://some/other/predicate', 'triple'); $nullTriple = $graph->subject_has_property('http://some/subject/1', 'http://some/predicate'); @@ -53,6 +76,13 @@ public function testAddGraphWithNullValueDoesNotAddNullTriple() $this->assertFalse($nullTriple); $this->assertTrue($stringTriple); } + public function addGraphWithInvalidValueDoesNotAddNullTriple_Provider(){ + return array( + array(null), + array(new stdClass()), + array(function(){}) + ); + } public function testRemoveProperties() { From 3a963e405f5c7661cc6342d380ff53014ed03043 Mon Sep 17 00:00:00 2001 From: Richard Tattersall Date: Wed, 22 Jul 2015 12:47:58 +0100 Subject: [PATCH 07/14] Improve testing --- test/unit/ExtendedGraphTest.php | 42 +++++++--------------- test/unit/mongo/MongoGraphTest.php | 57 +++++++++++++++++++++++++----- 2 files changed, 62 insertions(+), 37 deletions(-) diff --git a/test/unit/ExtendedGraphTest.php b/test/unit/ExtendedGraphTest.php index 0610a0c3..12df67a9 100644 --- a/test/unit/ExtendedGraphTest.php +++ b/test/unit/ExtendedGraphTest.php @@ -29,10 +29,11 @@ protected function setUp() public function testAddInvalidValueToLiteralResultsInNoTriple($value) { $graph = new ExtendedGraph(); - $graph->add_literal_triple('http://some/subject/1', 'http://some/predicate', $value); + $addResult = $graph->add_literal_triple('http://some/subject/1', 'http://some/predicate', $value); + $this->assertFalse($addResult, 'The triple should not have been added for this value'); - $result = $graph->subject_has_property('http://some/subject/1', 'http://some/predicate'); - $this->assertFalse($result); + $hasPropertyResult = $graph->subject_has_property('http://some/subject/1', 'http://some/predicate'); + $this->assertFalse($hasPropertyResult, 'The triple should not have been added for this value'); } public function addInvalidValueToLiteralResultsInNoTriple_Provider(){ return array( @@ -48,36 +49,19 @@ public function addInvalidValueToLiteralResultsInNoTriple_Provider(){ public function testAddInvalidValueToResourceResultsInNoTriple($value) { $graph = new ExtendedGraph(); - $graph->add_resource_triple('http://some/subject/1', 'http://some/predicate', $value); - $result = $graph->subject_has_property('http://some/subject/1', 'http://some/predicate'); - $this->assertFalse($result); - } - public function addInvalidValueToResourceResultsInNoTriple_Provider(){ - return array( - array(null), - array(new stdClass()), - array(function(){}) - ); - } + $addResult = $graph->add_resource_triple('http://some/subject/1', 'http://some/predicate', $value); + $this->assertFalse($addResult, 'The triple should not have been added for this value'); - /** - * @dataProvider addGraphWithInvalidValueDoesNotAddNullTriple_Provider - */ - public function testAddGraphWithInvalidValueDoesNotAddNullTriple($value) - { - $graph = new ExtendedGraph(); - $graph->add_resource_triple('http://some/subject/1', 'http://some/predicate', $value); - $graph->add_resource_triple('http://some/subject/1', 'http://some/other/predicate', 'triple'); - - $nullTriple = $graph->subject_has_property('http://some/subject/1', 'http://some/predicate'); - $stringTriple = $graph->subject_has_property('http://some/subject/1', 'http://some/other/predicate'); - - $this->assertFalse($nullTriple); - $this->assertTrue($stringTriple); + $hasPropertyResult = $graph->subject_has_property('http://some/subject/1', 'http://some/predicate'); + $this->assertFalse($hasPropertyResult, 'The triple should not have been added for this value'); } - public function addGraphWithInvalidValueDoesNotAddNullTriple_Provider(){ + public function addInvalidValueToResourceResultsInNoTriple_Provider(){ return array( + array(1), + array(1.2), + array(true), + array(array()), array(null), array(new stdClass()), array(function(){}) diff --git a/test/unit/mongo/MongoGraphTest.php b/test/unit/mongo/MongoGraphTest.php index 032a446f..4ce4c99f 100644 --- a/test/unit/mongo/MongoGraphTest.php +++ b/test/unit/mongo/MongoGraphTest.php @@ -102,27 +102,57 @@ public function testAddTripodArraySingleDoc() $this->assertEquals($expected, $g); } - public function testAddTripodArrayContainingNullValues() + /** + * @dataProvider addTripodArrayContainingInvalidLiteralValues_Provider + */ + public function testAddTripodArrayContainingInvalidLiteralValues($value) { $doc = array( "_id"=>array("r"=>"http://talisaspire.com/works/4d101f63c10a6-2", "c"=>"http://talisaspire.com/works/4d101f63c10a6-2"), "_version"=>0, - "dct:subject"=>array("u"=>"http://talisaspire.com/disciplines/physics"), - "dct:publisher"=>array("u"=>null), "rdf:type"=>array( - array("u"=>null), - array("l"=>null), + array("l"=>$value), array("l"=>"a Value"), - array("u"=>"http://talisaspire.com/schema#Work") ), "bibo:isbn13"=>array("l"=>"9211234567890"), - "bibo:isbn10"=>array("l"=>null) + "bibo:isbn10"=>array("l"=>$value) ); $expected = new \Tripod\Mongo\MongoGraph(); $expected->add_literal_triple("http://talisaspire.com/works/4d101f63c10a6-2", $expected->qname_to_uri("bibo:isbn13"),"9211234567890"); - $expected->add_resource_triple("http://talisaspire.com/works/4d101f63c10a6-2", $expected->qname_to_uri("dct:subject"),"http://talisaspire.com/disciplines/physics"); $expected->add_literal_triple("http://talisaspire.com/works/4d101f63c10a6-2", $expected->qname_to_uri("rdf:type"),"a Value"); + + $g = new \Tripod\Mongo\MongoGraph(); + $g->add_tripod_array($doc); + + $this->assertEquals($expected, $g); + } + public function addTripodArrayContainingInvalidLiteralValues_Provider(){ + return array( + array(null), + array(new stdClass()), + array(function(){}) + ); + } + + /** + * @dataProvider addTripodArrayContainingInvalidResourceValues_Provider + */ + public function testAddTripodArrayContainingInvalidResourceValues($value) + { + $doc = array( + "_id"=>array("r"=>"http://talisaspire.com/works/4d101f63c10a6-2", "c"=>"http://talisaspire.com/works/4d101f63c10a6-2"), + "_version"=>0, + "dct:subject"=>array("u"=>"http://talisaspire.com/disciplines/physics"), + "dct:publisher"=>array("u"=>$value), + "rdf:type"=>array( + array("u"=>$value), + array("u"=>"http://talisaspire.com/schema#Work") + ), + ); + + $expected = new \Tripod\Mongo\MongoGraph(); + $expected->add_resource_triple("http://talisaspire.com/works/4d101f63c10a6-2", $expected->qname_to_uri("dct:subject"),"http://talisaspire.com/disciplines/physics"); $expected->add_resource_triple("http://talisaspire.com/works/4d101f63c10a6-2", $expected->qname_to_uri("rdf:type"),"http://talisaspire.com/schema#Work"); $g = new \Tripod\Mongo\MongoGraph(); @@ -130,6 +160,17 @@ public function testAddTripodArrayContainingNullValues() $this->assertEquals($expected, $g); } + public function addTripodArrayContainingInvalidResourceValues_Provider(){ + return array( + array(1), + array(1.2), + array(true), + array(array()), + array(null), + array(new stdClass()), + array(function(){}) + ); + } public function testAddTripodArrayWhenAddingViews() { From a8cfc267ee75e9768ed8b51457923d527c5ce940 Mon Sep 17 00:00:00 2001 From: Richard Tattersall Date: Wed, 22 Jul 2015 12:59:43 +0100 Subject: [PATCH 08/14] Better testing --- test/unit/ExtendedGraphTest.php | 33 ++++++++++++++++ test/unit/mongo/MongoGraphTest.php | 63 ++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+) diff --git a/test/unit/ExtendedGraphTest.php b/test/unit/ExtendedGraphTest.php index 12df67a9..500d9e83 100644 --- a/test/unit/ExtendedGraphTest.php +++ b/test/unit/ExtendedGraphTest.php @@ -23,6 +23,27 @@ protected function setUp() echo "\nTest: {$className}->{$testName}\n"; } + /** + * @dataProvider addValidValueToLiteralResultsInTriple_Provider + */ + public function testAddValidValueToLiteralResultsInTriple($value) + { + $graph = new ExtendedGraph(); + $addResult = $graph->add_literal_triple('http://some/subject/1', 'http://some/predicate', $value); + $this->assertTrue($addResult, 'The triple should have been added for this value'); + + $hasPropertyResult = $graph->subject_has_property('http://some/subject/1', 'http://some/predicate'); + $this->assertTrue($hasPropertyResult, 'The triple should have been added for this value'); + } + public function addValidValueToLiteralResultsInTriple_Provider(){ + return array( + array('String'), + array(1), + array(1.2), + array(true) + ); + } + /** * @dataProvider addInvalidValueToLiteralResultsInNoTriple_Provider */ @@ -43,6 +64,18 @@ public function addInvalidValueToLiteralResultsInNoTriple_Provider(){ ); } + public function testAddValidValueToResourceResultsInTriple() + { + $value = 'A String'; + $graph = new ExtendedGraph(); + $addResult = $graph->add_resource_triple('http://some/subject/1', 'http://some/predicate', $value); + $this->assertTrue($addResult, 'The triple should have been added for this value'); + + $hasPropertyResult = $graph->subject_has_property('http://some/subject/1', 'http://some/predicate'); + $this->assertTrue($hasPropertyResult, 'The triple should have been added for this value'); + } + + /** * @dataProvider addInvalidValueToResourceResultsInNoTriple_Provider */ diff --git a/test/unit/mongo/MongoGraphTest.php b/test/unit/mongo/MongoGraphTest.php index 4ce4c99f..7534baa0 100644 --- a/test/unit/mongo/MongoGraphTest.php +++ b/test/unit/mongo/MongoGraphTest.php @@ -102,6 +102,42 @@ public function testAddTripodArraySingleDoc() $this->assertEquals($expected, $g); } + /** + * @dataProvider addTripodArrayContainingValidLiteralValues_Provider + */ + public function testAddTripodArrayContainingValidLiteralValues($value) + { + $doc = array( + "_id"=>array("r"=>"http://talisaspire.com/works/4d101f63c10a6-2", "c"=>"http://talisaspire.com/works/4d101f63c10a6-2"), + "_version"=>0, + "rdf:type"=>array( + array("l"=>$value), + array("l"=>"a Value"), + ), + "bibo:isbn13"=>array("l"=>"9211234567890"), + "bibo:isbn10"=>array("l"=>$value) + ); + + $expected = new \Tripod\Mongo\MongoGraph(); + $expected->add_literal_triple("http://talisaspire.com/works/4d101f63c10a6-2", $expected->qname_to_uri("bibo:isbn13"),"9211234567890"); + $expected->add_literal_triple("http://talisaspire.com/works/4d101f63c10a6-2", $expected->qname_to_uri("bibo:isbn10"),$value); + $expected->add_literal_triple("http://talisaspire.com/works/4d101f63c10a6-2", $expected->qname_to_uri("rdf:type"),$value); + $expected->add_literal_triple("http://talisaspire.com/works/4d101f63c10a6-2", $expected->qname_to_uri("rdf:type"),"a Value"); + + $g = new \Tripod\Mongo\MongoGraph(); + $g->add_tripod_array($doc); + + $this->assertEquals($expected, $g); + } + public function addTripodArrayContainingValidLiteralValues_Provider(){ + return array( + array('A String'), + array(1), + array(1.2), + array(true) + ); + } + /** * @dataProvider addTripodArrayContainingInvalidLiteralValues_Provider */ @@ -135,6 +171,33 @@ public function addTripodArrayContainingInvalidLiteralValues_Provider(){ ); } + + public function testAddTripodArrayContainingValidResourceValues() + { + $value = 'A String'; + $doc = array( + "_id"=>array("r"=>"http://talisaspire.com/works/4d101f63c10a6-2", "c"=>"http://talisaspire.com/works/4d101f63c10a6-2"), + "_version"=>0, + "dct:subject"=>array("u"=>"http://talisaspire.com/disciplines/physics"), + "dct:publisher"=>array("u"=>$value), + "rdf:type"=>array( + array("u"=>$value), + array("u"=>"http://talisaspire.com/schema#Work") + ), + ); + + $expected = new \Tripod\Mongo\MongoGraph(); + $expected->add_resource_triple("http://talisaspire.com/works/4d101f63c10a6-2", $expected->qname_to_uri("dct:subject"),"http://talisaspire.com/disciplines/physics"); + $expected->add_resource_triple("http://talisaspire.com/works/4d101f63c10a6-2", $expected->qname_to_uri("dct:publisher"),$value); + $expected->add_resource_triple("http://talisaspire.com/works/4d101f63c10a6-2", $expected->qname_to_uri("rdf:type"),$value); + $expected->add_resource_triple("http://talisaspire.com/works/4d101f63c10a6-2", $expected->qname_to_uri("rdf:type"),"http://talisaspire.com/schema#Work"); + + $g = new \Tripod\Mongo\MongoGraph(); + $g->add_tripod_array($doc); + + $this->assertEquals($expected, $g); + } + /** * @dataProvider addTripodArrayContainingInvalidResourceValues_Provider */ From 6a96dcbc073bf04cd8ae763cd0f453f8397f769c Mon Sep 17 00:00:00 2001 From: Richard Tattersall Date: Wed, 22 Jul 2015 13:27:30 +0100 Subject: [PATCH 09/14] Fixed up docblock --- src/mongo/MongoGraph.class.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mongo/MongoGraph.class.php b/src/mongo/MongoGraph.class.php index 7b2ead41..1a5c319e 100644 --- a/src/mongo/MongoGraph.class.php +++ b/src/mongo/MongoGraph.class.php @@ -144,7 +144,7 @@ private function add_tarray_to_index($tarray) * Convert from Tripod value object format (comapct) to ExtendedGraph format (verbose) * * @param array $mongoValueObject - * @return array| false an array of values or false if the value is not valid + * @return array|bool an array of values or false if the value is not valid */ private function toGraphValueObject($mongoValueObject) { From 460f258a0b70a702efb6fb7352897e6857c1d33f Mon Sep 17 00:00:00 2001 From: Richard Tattersall Date: Wed, 22 Jul 2015 16:08:28 +0100 Subject: [PATCH 10/14] Enforce subjects and predicates are also strings --- src/classes/ExtendedGraph.class.php | 31 +++++---- src/mongo/MongoGraph.class.php | 23 +++++-- test/unit/ExtendedGraphTest.php | 100 ++++++++++++++++++++++++++++ test/unit/mongo/MongoGraphTest.php | 58 ++++++++++++++++ 4 files changed, 192 insertions(+), 20 deletions(-) diff --git a/src/classes/ExtendedGraph.class.php b/src/classes/ExtendedGraph.class.php index 22779539..46a967da 100644 --- a/src/classes/ExtendedGraph.class.php +++ b/src/classes/ExtendedGraph.class.php @@ -141,7 +141,7 @@ public function make_resource_array($resource) { * @return boolean true if the triple was new, false if it already existed in the graph */ public function add_resource_triple($s, $p, $o) { - if($this->isValidResourceValue($o)) { + if($this->isValidTripleValue($s) && $this->isValidTripleValue($p) && $this->isValidTripleValue($o)) { return $this->_add_triple($s, $p, array('type' => strpos($o, '_:') === 0 ? 'bnode' : 'uri', 'value' => $o)); } return false; @@ -157,7 +157,7 @@ public function add_resource_triple($s, $p, $o) { * @return boolean true if the triple was new, false if it already existed in the graph */ public function add_literal_triple($s, $p, $o, $lang = null, $dt = null) { - if($this->isValidLiteralValue($o)) { + if($this->isValidTripleValue($s) && $this->isValidTripleValue($p) && $this->isValidLiteralValue($o)) { $o_info = array('type' => 'literal', 'value' => $o); if ($lang != null) { $o_info['lang'] = $lang; @@ -177,17 +177,22 @@ public function add_literal_triple($s, $p, $o, $lang = null, $dt = null) { * @return bool */ private function _add_triple($s, $p, Array $o_info) { - if (!isset($this->_index[$s])) { - $this->_index[$s] = array(); - $this->_index[$s][$p] = array($o_info); - return true; - } elseif (!isset($this->_index[$s][$p])) { - $this->_index[$s][$p] = array($o_info); - return true; - } else { - if (!in_array($o_info, $this->_index[$s][$p])) { - $this->_index[$s][$p][] = $o_info; + // The value $o should already have been validated by this point + // It's validation differs depending on whether it is a literal or resource + // So just check the subject and predicate here... + if($this->isValidTripleValue($s) && $this->isValidTripleValue($p)) { + if (!isset($this->_index[$s])) { + $this->_index[$s] = array(); + $this->_index[$s][$p] = array($o_info); + return true; + } elseif (!isset($this->_index[$s][$p])) { + $this->_index[$s][$p] = array($o_info); return true; + } else { + if (!in_array($o_info, $this->_index[$s][$p])) { + $this->_index[$s][$p][] = $o_info; + return true; + } } } return false; @@ -215,7 +220,7 @@ protected function isValidLiteralValue($value){ * * @return bool */ - protected function isValidResourceValue($value){ + protected function isValidTripleValue($value){ if(!is_string($value)){ return false; } diff --git a/src/mongo/MongoGraph.class.php b/src/mongo/MongoGraph.class.php index 1a5c319e..ead47088 100644 --- a/src/mongo/MongoGraph.class.php +++ b/src/mongo/MongoGraph.class.php @@ -128,11 +128,20 @@ private function add_tarray_to_index($tarray) { if($key[0] != '_') { - $predicate = $this->qname_to_uri($key); - $graphValueObject = $this->toGraphValueObject($value); - // Only add if valid values have been found - if($graphValueObject !== false) { - $predObjects[$predicate] = $graphValueObject; + // Make sure the predicate is valid + if($this->isValidTripleValue($key)){ + $predicate = $this->qname_to_uri($key); + $graphValueObject = $this->toGraphValueObject($value); + // Only add if valid values have been found + if ($graphValueObject !== false) { + $predObjects[$predicate] = $graphValueObject; + } + } + } + else if($key == "_id"){ + // If the subject is not valid then return + if(!isset($value['r']) || !$this->isValidTripleValue($value['r'])){ + return; } } } @@ -163,7 +172,7 @@ private function toGraphValueObject($mongoValueObject) else if (array_key_exists(VALUE_URI,$mongoValueObject)) { // only allow valid values - if($this->isValidResourceValue($mongoValueObject[VALUE_URI])) { + if($this->isValidTripleValue($mongoValueObject[VALUE_URI])) { // single value uri $simpleGraphValueObject[] = array( 'type' => 'uri', @@ -185,7 +194,7 @@ private function toGraphValueObject($mongoValueObject) $valueTypeLabel = 'literal'; } else{ - if(!$this->isValidResourceValue($value)){ + if(!$this->isValidTripleValue($value)){ continue; } $valueTypeLabel = 'uri'; diff --git a/test/unit/ExtendedGraphTest.php b/test/unit/ExtendedGraphTest.php index 500d9e83..41134376 100644 --- a/test/unit/ExtendedGraphTest.php +++ b/test/unit/ExtendedGraphTest.php @@ -64,6 +64,56 @@ public function addInvalidValueToLiteralResultsInNoTriple_Provider(){ ); } + /** + * @dataProvider addInvalidSubjectToLiteralResultsInNoTriple_Provider + */ + public function testAddInvalidSubjectToLiteralResultsInNoTriple($value) + { + $graph = new ExtendedGraph(); + + $addResult = $graph->add_resource_triple($value, 'http://some/predicate', 'http://someplace.com'); + $this->assertFalse($addResult, 'The triple should not have been added for this value'); + + $graph->get_triple_count(); + $this->assertEquals(0, $graph->get_triple_count(), 'The triple should not have been added for this value'); + } + public function addInvalidSubjectToLiteralResultsInNoTriple_Provider(){ + return array( + array(1), + array(1.2), + array(true), + array(array()), + array(null), + array(new stdClass()), + array(function(){}) + ); + } + + /** + * @dataProvider addInvalidSubjectToLiteralResultsInNoTriple_Provider + */ + public function testAddInvalidPredicateToLiteralResultsInNoTriple($value) + { + $graph = new ExtendedGraph(); + + $addResult = $graph->add_resource_triple('http://some/subject/1', $value, 'http://someplace.com'); + $this->assertFalse($addResult, 'The triple should not have been added for this value'); + + $graph->get_triple_count(); + $this->assertEquals(0, $graph->get_triple_count(), 'The triple should not have been added for this value'); + } + public function addInvalidPredicateToLiteralResultsInNoTriple_Provider(){ + return array( + array(1), + array(1.2), + array(true), + array(array()), + array(null), + array(new stdClass()), + array(function(){}) + ); + } + public function testAddValidValueToResourceResultsInTriple() { $value = 'A String'; @@ -101,6 +151,56 @@ public function addInvalidValueToResourceResultsInNoTriple_Provider(){ ); } + /** + * @dataProvider addInvalidSubjectToResourceResultsInNoTriple_Provider + */ + public function testAddInvalidSubjectToResourceResultsInNoTriple($value) + { + $graph = new ExtendedGraph(); + + $addResult = $graph->add_resource_triple($value, 'http://some/predicate', 'http://someplace.com'); + $this->assertFalse($addResult, 'The triple should not have been added for this value'); + + $graph->get_triple_count(); + $this->assertEquals(0, $graph->get_triple_count(), 'The triple should not have been added for this value'); + } + public function addInvalidSubjectToResourceResultsInNoTriple_Provider(){ + return array( + array(1), + array(1.2), + array(true), + array(array()), + array(null), + array(new stdClass()), + array(function(){}) + ); + } + + /** + * @dataProvider addInvalidSubjectToLiteralResultsInNoTriple_Provider + */ + public function testAddInvalidPredicateToResourceResultsInNoTriple($value) + { + $graph = new ExtendedGraph(); + + $addResult = $graph->add_resource_triple('http://some/subject/1', $value, 'http://someplace.com'); + $this->assertFalse($addResult, 'The triple should not have been added for this value'); + + $graph->get_triple_count(); + $this->assertEquals(0, $graph->get_triple_count(), 'The triple should not have been added for this value'); + } + public function addInvalidPredicateToResourceResultsInNoTriple_Provider(){ + return array( + array(1), + array(1.2), + array(true), + array(array()), + array(null), + array(new stdClass()), + array(function(){}) + ); + } + public function testRemoveProperties() { $graph = new ExtendedGraph(); diff --git a/test/unit/mongo/MongoGraphTest.php b/test/unit/mongo/MongoGraphTest.php index 7534baa0..304027b7 100644 --- a/test/unit/mongo/MongoGraphTest.php +++ b/test/unit/mongo/MongoGraphTest.php @@ -171,6 +171,64 @@ public function addTripodArrayContainingInvalidLiteralValues_Provider(){ ); } + /** + * @dataProvider addTripodArrayContainingInvalidPredicates_Provider + */ + public function testAddTripodArrayContainingInvalidPredicates($value) + { + $doc = array( + "_id"=>array("r"=>"http://talisaspire.com/works/4d101f63c10a6-2", "c"=>"http://talisaspire.com/works/4d101f63c10a6-2"), + "_version"=>0, + "rdf:type"=>array( + array("l"=>"a Value"), + ), + "bibo:isbn13"=>array("l"=>"9211234567890"), + $value=>array("l"=>"9211234567890") + ); + + $expected = new \Tripod\Mongo\MongoGraph(); + $expected->add_literal_triple("http://talisaspire.com/works/4d101f63c10a6-2", $expected->qname_to_uri("bibo:isbn13"),"9211234567890"); + $expected->add_literal_triple("http://talisaspire.com/works/4d101f63c10a6-2", $expected->qname_to_uri("rdf:type"),"a Value"); + + $g = new \Tripod\Mongo\MongoGraph(); + $g->add_tripod_array($doc); + + $this->assertEquals($expected, $g); + } + public function addTripodArrayContainingInvalidPredicates_Provider(){ + return array( + array(1), + array(1.2), + array(true), + ); + } + + /** + * @dataProvider addTripodArrayContainingInvalidSubject_Provider + */ + public function testAddTripodArrayContainingInvalidSubject($value) + { + $doc = array( + "_id"=>array("r"=>$value, "c"=>"http://talisaspire.com/works/4d101f63c10a6-2"), + "_version"=>0, + "rdf:type"=>array( + array("l"=>"a Value"), + ), + "bibo:isbn13"=>array("l"=>"9211234567890"), + ); + + $g = new \Tripod\Mongo\MongoGraph(); + $g->add_tripod_array($doc); + $this->assertEquals(0, $g->get_triple_count()); + } + public function addTripodArrayContainingInvalidSubject_Provider(){ + return array( + array(1), + array(1.2), + array(true), + ); + } + public function testAddTripodArrayContainingValidResourceValues() { From 5396f609d2b2cb81f0b75b71e086bc859070fbce Mon Sep 17 00:00:00 2001 From: Richard Tattersall Date: Wed, 22 Jul 2015 19:57:10 +0100 Subject: [PATCH 11/14] Ensure exception is thrown if a blank subject is added to ExtendedGraph --- src/classes/ExtendedGraph.class.php | 4 ++++ src/mongo/MongoGraph.class.php | 3 +++ test/unit/ExtendedGraphTest.php | 14 ++++++++++++++ test/unit/mongo/MongoGraphTest.php | 16 ++++++++++++++++ 4 files changed, 37 insertions(+) diff --git a/src/classes/ExtendedGraph.class.php b/src/classes/ExtendedGraph.class.php index 46a967da..e16aaa0b 100644 --- a/src/classes/ExtendedGraph.class.php +++ b/src/classes/ExtendedGraph.class.php @@ -177,6 +177,10 @@ public function add_literal_triple($s, $p, $o, $lang = null, $dt = null) { * @return bool */ private function _add_triple($s, $p, Array $o_info) { + // Make sure the subject is not an empty string + if($s === ""){ + throw new \Tripod\Exceptions\Exception("The subject cannot be an empty string"); + } // The value $o should already have been validated by this point // It's validation differs depending on whether it is a literal or resource // So just check the subject and predicate here... diff --git a/src/mongo/MongoGraph.class.php b/src/mongo/MongoGraph.class.php index ead47088..3cded4bc 100644 --- a/src/mongo/MongoGraph.class.php +++ b/src/mongo/MongoGraph.class.php @@ -143,6 +143,9 @@ private function add_tarray_to_index($tarray) if(!isset($value['r']) || !$this->isValidTripleValue($value['r'])){ return; } + if($value['r'] === ""){ + throw new \Tripod\Exceptions\Exception("The subject cannot be an empty string"); + } } } $_i[$this->_labeller->qname_to_alias($tarray["_id"][_ID_RESOURCE])] = $predObjects; diff --git a/test/unit/ExtendedGraphTest.php b/test/unit/ExtendedGraphTest.php index 41134376..a97f9e96 100644 --- a/test/unit/ExtendedGraphTest.php +++ b/test/unit/ExtendedGraphTest.php @@ -89,6 +89,13 @@ public function addInvalidSubjectToLiteralResultsInNoTriple_Provider(){ ); } + public function testAddEmptySubjectToLiteralThrowsException() + { + $this->setExpectedException('\Tripod\Exceptions\Exception'); + $graph = new ExtendedGraph(); + $graph->add_literal_triple("", 'http://some/predicate', 'http://someplace.com'); + } + /** * @dataProvider addInvalidSubjectToLiteralResultsInNoTriple_Provider */ @@ -176,6 +183,13 @@ public function addInvalidSubjectToResourceResultsInNoTriple_Provider(){ ); } + public function testAddEmptySubjectToResourceThrowsException() + { + $this->setExpectedException('\Tripod\Exceptions\Exception'); + $graph = new ExtendedGraph(); + $graph->add_resource_triple("", 'http://some/predicate', 'http://someplace.com'); + } + /** * @dataProvider addInvalidSubjectToLiteralResultsInNoTriple_Provider */ diff --git a/test/unit/mongo/MongoGraphTest.php b/test/unit/mongo/MongoGraphTest.php index 304027b7..512cac19 100644 --- a/test/unit/mongo/MongoGraphTest.php +++ b/test/unit/mongo/MongoGraphTest.php @@ -229,6 +229,22 @@ public function addTripodArrayContainingInvalidSubject_Provider(){ ); } + public function testAddTripodArrayContainingEmptySubject() + { + $this->setExpectedException('\Tripod\Exceptions\Exception'); + $doc = array( + "_id"=>array("r"=>"", "c"=>"http://talisaspire.com/works/4d101f63c10a6-2"), + "_version"=>0, + "rdf:type"=>array( + array("l"=>"a Value"), + ), + "bibo:isbn13"=>array("l"=>"9211234567890"), + ); + + $g = new \Tripod\Mongo\MongoGraph(); + $g->add_tripod_array($doc); + } + public function testAddTripodArrayContainingValidResourceValues() { From 60d0e3c1b0b4731c6bb22dc3bfa736546e861bab Mon Sep 17 00:00:00 2001 From: Richard Tattersall Date: Wed, 22 Jul 2015 22:48:07 +0100 Subject: [PATCH 12/14] Fixed up from PR comments --- src/classes/ExtendedGraph.class.php | 22 +++++----- src/mongo/MongoGraph.class.php | 31 +++++---------- test/unit/ExtendedGraphTest.php | 62 ++++++++++------------------- test/unit/mongo/MongoGraphTest.php | 42 +++++++++++-------- 4 files changed, 69 insertions(+), 88 deletions(-) diff --git a/src/classes/ExtendedGraph.class.php b/src/classes/ExtendedGraph.class.php index e16aaa0b..c71cbcf0 100644 --- a/src/classes/ExtendedGraph.class.php +++ b/src/classes/ExtendedGraph.class.php @@ -141,7 +141,7 @@ public function make_resource_array($resource) { * @return boolean true if the triple was new, false if it already existed in the graph */ public function add_resource_triple($s, $p, $o) { - if($this->isValidTripleValue($s) && $this->isValidTripleValue($p) && $this->isValidTripleValue($o)) { + if($this->isValidResourceValue($o)) { return $this->_add_triple($s, $p, array('type' => strpos($o, '_:') === 0 ? 'bnode' : 'uri', 'value' => $o)); } return false; @@ -157,7 +157,7 @@ public function add_resource_triple($s, $p, $o) { * @return boolean true if the triple was new, false if it already existed in the graph */ public function add_literal_triple($s, $p, $o, $lang = null, $dt = null) { - if($this->isValidTripleValue($s) && $this->isValidTripleValue($p) && $this->isValidLiteralValue($o)) { + if($this->isValidLiteralValue($o)) { $o_info = array('type' => 'literal', 'value' => $o); if ($lang != null) { $o_info['lang'] = $lang; @@ -177,14 +177,15 @@ public function add_literal_triple($s, $p, $o, $lang = null, $dt = null) { * @return bool */ private function _add_triple($s, $p, Array $o_info) { - // Make sure the subject is not an empty string - if($s === ""){ - throw new \Tripod\Exceptions\Exception("The subject cannot be an empty string"); - } // The value $o should already have been validated by this point // It's validation differs depending on whether it is a literal or resource // So just check the subject and predicate here... - if($this->isValidTripleValue($s) && $this->isValidTripleValue($p)) { + if(!$this->isValidResourceValue($s)){ + throw new \Tripod\Exceptions\Exception("The subject is invalid"); + } + if(!$this->isValidResourceValue($p)){ + throw new \Tripod\Exceptions\Exception("The predicate is invalid"); + } if (!isset($this->_index[$s])) { $this->_index[$s] = array(); $this->_index[$s][$p] = array($o_info); @@ -198,7 +199,6 @@ private function _add_triple($s, $p, Array $o_info) { return true; } } - } return false; } @@ -210,6 +210,7 @@ private function _add_triple($s, $p, Array $o_info) { * but accepting scalars so we can handle legacy data * which was not type-checked. * + * @param string $value * @return bool */ protected function isValidLiteralValue($value){ @@ -222,10 +223,11 @@ protected function isValidLiteralValue($value){ /** * Check if a triple value is valid. * + * @param string $value * @return bool */ - protected function isValidTripleValue($value){ - if(!is_string($value)){ + protected function isValidResourceValue($value){ + if(!is_string($value) || empty($value)){ return false; } return true; diff --git a/src/mongo/MongoGraph.class.php b/src/mongo/MongoGraph.class.php index 3cded4bc..adbd1943 100644 --- a/src/mongo/MongoGraph.class.php +++ b/src/mongo/MongoGraph.class.php @@ -119,6 +119,7 @@ function to_tripod_view_array($docId,$context) /** * @param array $tarray + * @throws \Tripod\Exceptions\Exception */ private function add_tarray_to_index($tarray) { @@ -128,22 +129,16 @@ private function add_tarray_to_index($tarray) { if($key[0] != '_') { - // Make sure the predicate is valid - if($this->isValidTripleValue($key)){ - $predicate = $this->qname_to_uri($key); - $graphValueObject = $this->toGraphValueObject($value); - // Only add if valid values have been found - if ($graphValueObject !== false) { - $predObjects[$predicate] = $graphValueObject; - } + $predicate = $this->qname_to_uri($key); + $graphValueObject = $this->toGraphValueObject($value); + // Only add if valid values have been found + if (!empty($graphValueObject)) { + $predObjects[$predicate] = $graphValueObject; } } else if($key == "_id"){ - // If the subject is not valid then return - if(!isset($value['r']) || !$this->isValidTripleValue($value['r'])){ - return; - } - if($value['r'] === ""){ + // If the subject is invalid then throw an exception + if(!isset($value['r']) || !$this->isValidResourceValue($value['r'])){ throw new \Tripod\Exceptions\Exception("The subject cannot be an empty string"); } } @@ -156,7 +151,7 @@ private function add_tarray_to_index($tarray) * Convert from Tripod value object format (comapct) to ExtendedGraph format (verbose) * * @param array $mongoValueObject - * @return array|bool an array of values or false if the value is not valid + * @return array */ private function toGraphValueObject($mongoValueObject) { @@ -175,7 +170,7 @@ private function toGraphValueObject($mongoValueObject) else if (array_key_exists(VALUE_URI,$mongoValueObject)) { // only allow valid values - if($this->isValidTripleValue($mongoValueObject[VALUE_URI])) { + if($this->isValidResourceValue($mongoValueObject[VALUE_URI])) { // single value uri $simpleGraphValueObject[] = array( 'type' => 'uri', @@ -197,7 +192,7 @@ private function toGraphValueObject($mongoValueObject) $valueTypeLabel = 'literal'; } else{ - if(!$this->isValidTripleValue($value)){ + if(!$this->isValidResourceValue($value)){ continue; } $valueTypeLabel = 'uri'; @@ -208,10 +203,6 @@ private function toGraphValueObject($mongoValueObject) } } } - // If we don't have any values, then respond with false - if(empty($simpleGraphValueObject)){ - return false; - } // Otherwise we have found valid values return $simpleGraphValueObject; } diff --git a/test/unit/ExtendedGraphTest.php b/test/unit/ExtendedGraphTest.php index a97f9e96..f85986ed 100644 --- a/test/unit/ExtendedGraphTest.php +++ b/test/unit/ExtendedGraphTest.php @@ -67,18 +67,16 @@ public function addInvalidValueToLiteralResultsInNoTriple_Provider(){ /** * @dataProvider addInvalidSubjectToLiteralResultsInNoTriple_Provider */ - public function testAddInvalidSubjectToLiteralResultsInNoTriple($value) + public function testAddInvalidSubjectToLiteralThrowsException($value) { - $graph = new ExtendedGraph(); - - $addResult = $graph->add_resource_triple($value, 'http://some/predicate', 'http://someplace.com'); - $this->assertFalse($addResult, 'The triple should not have been added for this value'); + $this->setExpectedException('\Tripod\Exceptions\Exception'); - $graph->get_triple_count(); - $this->assertEquals(0, $graph->get_triple_count(), 'The triple should not have been added for this value'); + $graph = new ExtendedGraph(); + $graph->add_resource_triple($value, 'http://some/predicate', 'http://someplace.com'); } public function addInvalidSubjectToLiteralResultsInNoTriple_Provider(){ return array( + array(""), array(1), array(1.2), array(true), @@ -89,28 +87,19 @@ public function addInvalidSubjectToLiteralResultsInNoTriple_Provider(){ ); } - public function testAddEmptySubjectToLiteralThrowsException() - { - $this->setExpectedException('\Tripod\Exceptions\Exception'); - $graph = new ExtendedGraph(); - $graph->add_literal_triple("", 'http://some/predicate', 'http://someplace.com'); - } - /** * @dataProvider addInvalidSubjectToLiteralResultsInNoTriple_Provider */ - public function testAddInvalidPredicateToLiteralResultsInNoTriple($value) + public function testAddInvalidPredicateToLiteralThrowsException($value) { - $graph = new ExtendedGraph(); - - $addResult = $graph->add_resource_triple('http://some/subject/1', $value, 'http://someplace.com'); - $this->assertFalse($addResult, 'The triple should not have been added for this value'); + $this->setExpectedException('\Tripod\Exceptions\Exception'); - $graph->get_triple_count(); - $this->assertEquals(0, $graph->get_triple_count(), 'The triple should not have been added for this value'); + $graph = new ExtendedGraph(); + $graph->add_resource_triple('http://some/subject/1', $value, 'http://someplace.com'); } public function addInvalidPredicateToLiteralResultsInNoTriple_Provider(){ return array( + array(""), array(1), array(1.2), array(true), @@ -161,18 +150,16 @@ public function addInvalidValueToResourceResultsInNoTriple_Provider(){ /** * @dataProvider addInvalidSubjectToResourceResultsInNoTriple_Provider */ - public function testAddInvalidSubjectToResourceResultsInNoTriple($value) + public function testAddInvalidSubjectToResourceThrowsException($value) { - $graph = new ExtendedGraph(); - - $addResult = $graph->add_resource_triple($value, 'http://some/predicate', 'http://someplace.com'); - $this->assertFalse($addResult, 'The triple should not have been added for this value'); + $this->setExpectedException('\Tripod\Exceptions\Exception'); - $graph->get_triple_count(); - $this->assertEquals(0, $graph->get_triple_count(), 'The triple should not have been added for this value'); + $graph = new ExtendedGraph(); + $graph->add_resource_triple($value, 'http://some/predicate', 'http://someplace.com'); } public function addInvalidSubjectToResourceResultsInNoTriple_Provider(){ return array( + array(""), array(1), array(1.2), array(true), @@ -183,28 +170,19 @@ public function addInvalidSubjectToResourceResultsInNoTriple_Provider(){ ); } - public function testAddEmptySubjectToResourceThrowsException() - { - $this->setExpectedException('\Tripod\Exceptions\Exception'); - $graph = new ExtendedGraph(); - $graph->add_resource_triple("", 'http://some/predicate', 'http://someplace.com'); - } - /** * @dataProvider addInvalidSubjectToLiteralResultsInNoTriple_Provider */ - public function testAddInvalidPredicateToResourceResultsInNoTriple($value) + public function testAddInvalidPredicateToResourceThrowsException($value) { - $graph = new ExtendedGraph(); - - $addResult = $graph->add_resource_triple('http://some/subject/1', $value, 'http://someplace.com'); - $this->assertFalse($addResult, 'The triple should not have been added for this value'); + $this->setExpectedException('\Tripod\Exceptions\Exception'); - $graph->get_triple_count(); - $this->assertEquals(0, $graph->get_triple_count(), 'The triple should not have been added for this value'); + $graph = new ExtendedGraph(); + $graph->add_resource_triple('http://some/subject/1', $value, 'http://someplace.com'); } public function addInvalidPredicateToResourceResultsInNoTriple_Provider(){ return array( + array(""), array(1), array(1.2), array(true), diff --git a/test/unit/mongo/MongoGraphTest.php b/test/unit/mongo/MongoGraphTest.php index 512cac19..98243b98 100644 --- a/test/unit/mongo/MongoGraphTest.php +++ b/test/unit/mongo/MongoGraphTest.php @@ -176,6 +176,7 @@ public function addTripodArrayContainingInvalidLiteralValues_Provider(){ */ public function testAddTripodArrayContainingInvalidPredicates($value) { + $this->setExpectedException('\Tripod\Exceptions\LabellerException'); $doc = array( "_id"=>array("r"=>"http://talisaspire.com/works/4d101f63c10a6-2", "c"=>"http://talisaspire.com/works/4d101f63c10a6-2"), "_version"=>0, @@ -192,8 +193,6 @@ public function testAddTripodArrayContainingInvalidPredicates($value) $g = new \Tripod\Mongo\MongoGraph(); $g->add_tripod_array($doc); - - $this->assertEquals($expected, $g); } public function addTripodArrayContainingInvalidPredicates_Provider(){ return array( @@ -204,36 +203,40 @@ public function addTripodArrayContainingInvalidPredicates_Provider(){ } /** - * @dataProvider addTripodArrayContainingInvalidSubject_Provider + * + * We are expecting the labeller + * */ - public function testAddTripodArrayContainingInvalidSubject($value) + public function testAddTripodArrayContainingEmptyPredicate() { + // An Uninitialized string offset should occur if an empty predicate is passed. + $this->setExpectedException("PHPUnit_Framework_Error"); $doc = array( - "_id"=>array("r"=>$value, "c"=>"http://talisaspire.com/works/4d101f63c10a6-2"), + "_id"=>array("r"=>"http://talisaspire.com/works/4d101f63c10a6-2", "c"=>"http://talisaspire.com/works/4d101f63c10a6-2"), "_version"=>0, "rdf:type"=>array( array("l"=>"a Value"), ), "bibo:isbn13"=>array("l"=>"9211234567890"), + ""=>array("l"=>"9211234567890") ); + $expected = new \Tripod\Mongo\MongoGraph(); + $expected->add_literal_triple("http://talisaspire.com/works/4d101f63c10a6-2", $expected->qname_to_uri("bibo:isbn13"),"9211234567890"); + $expected->add_literal_triple("http://talisaspire.com/works/4d101f63c10a6-2", $expected->qname_to_uri("rdf:type"),"a Value"); + $g = new \Tripod\Mongo\MongoGraph(); $g->add_tripod_array($doc); - $this->assertEquals(0, $g->get_triple_count()); - } - public function addTripodArrayContainingInvalidSubject_Provider(){ - return array( - array(1), - array(1.2), - array(true), - ); } - public function testAddTripodArrayContainingEmptySubject() + /** + * @dataProvider addTripodArrayContainingInvalidSubject_Provider + */ + public function testAddTripodArrayContainingInvalidSubject($value) { $this->setExpectedException('\Tripod\Exceptions\Exception'); $doc = array( - "_id"=>array("r"=>"", "c"=>"http://talisaspire.com/works/4d101f63c10a6-2"), + "_id"=>array("r"=>$value, "c"=>"http://talisaspire.com/works/4d101f63c10a6-2"), "_version"=>0, "rdf:type"=>array( array("l"=>"a Value"), @@ -244,7 +247,14 @@ public function testAddTripodArrayContainingEmptySubject() $g = new \Tripod\Mongo\MongoGraph(); $g->add_tripod_array($doc); } - + public function addTripodArrayContainingInvalidSubject_Provider(){ + return array( + array(""), + array(1), + array(1.2), + array(true), + ); + } public function testAddTripodArrayContainingValidResourceValues() { From a895200282acdb89f22ddee8854f067e8c5dd7b9 Mon Sep 17 00:00:00 2001 From: Richard Tattersall Date: Wed, 22 Jul 2015 22:50:14 +0100 Subject: [PATCH 13/14] Fixed up formatting --- src/classes/ExtendedGraph.class.php | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/classes/ExtendedGraph.class.php b/src/classes/ExtendedGraph.class.php index c71cbcf0..63b1339a 100644 --- a/src/classes/ExtendedGraph.class.php +++ b/src/classes/ExtendedGraph.class.php @@ -174,6 +174,7 @@ public function add_literal_triple($s, $p, $o, $lang = null, $dt = null) { * @param string $s * @param string $p * @param array $o_info + * @throws Exceptions\Exception * @return bool */ private function _add_triple($s, $p, Array $o_info) { @@ -186,19 +187,19 @@ private function _add_triple($s, $p, Array $o_info) { if(!$this->isValidResourceValue($p)){ throw new \Tripod\Exceptions\Exception("The predicate is invalid"); } - if (!isset($this->_index[$s])) { - $this->_index[$s] = array(); - $this->_index[$s][$p] = array($o_info); - return true; - } elseif (!isset($this->_index[$s][$p])) { - $this->_index[$s][$p] = array($o_info); + if (!isset($this->_index[$s])) { + $this->_index[$s] = array(); + $this->_index[$s][$p] = array($o_info); + return true; + } elseif (!isset($this->_index[$s][$p])) { + $this->_index[$s][$p] = array($o_info); + return true; + } else { + if (!in_array($o_info, $this->_index[$s][$p])) { + $this->_index[$s][$p][] = $o_info; return true; - } else { - if (!in_array($o_info, $this->_index[$s][$p])) { - $this->_index[$s][$p][] = $o_info; - return true; - } } + } return false; } From c8ef3f686654b76e3a3841f76e6fabeeefd76a73 Mon Sep 17 00:00:00 2001 From: Richard Tattersall Date: Thu, 23 Jul 2015 09:26:38 +0100 Subject: [PATCH 14/14] Renamed Methods as per PR --- src/classes/ExtendedGraph.class.php | 12 ++++++------ src/mongo/MongoGraph.class.php | 10 +++++----- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/classes/ExtendedGraph.class.php b/src/classes/ExtendedGraph.class.php index 63b1339a..80aaa835 100644 --- a/src/classes/ExtendedGraph.class.php +++ b/src/classes/ExtendedGraph.class.php @@ -141,7 +141,7 @@ public function make_resource_array($resource) { * @return boolean true if the triple was new, false if it already existed in the graph */ public function add_resource_triple($s, $p, $o) { - if($this->isValidResourceValue($o)) { + if($this->isValidResource($o)) { return $this->_add_triple($s, $p, array('type' => strpos($o, '_:') === 0 ? 'bnode' : 'uri', 'value' => $o)); } return false; @@ -157,7 +157,7 @@ public function add_resource_triple($s, $p, $o) { * @return boolean true if the triple was new, false if it already existed in the graph */ public function add_literal_triple($s, $p, $o, $lang = null, $dt = null) { - if($this->isValidLiteralValue($o)) { + if($this->isValidLiteral($o)) { $o_info = array('type' => 'literal', 'value' => $o); if ($lang != null) { $o_info['lang'] = $lang; @@ -181,10 +181,10 @@ private function _add_triple($s, $p, Array $o_info) { // The value $o should already have been validated by this point // It's validation differs depending on whether it is a literal or resource // So just check the subject and predicate here... - if(!$this->isValidResourceValue($s)){ + if(!$this->isValidResource($s)){ throw new \Tripod\Exceptions\Exception("The subject is invalid"); } - if(!$this->isValidResourceValue($p)){ + if(!$this->isValidResource($p)){ throw new \Tripod\Exceptions\Exception("The predicate is invalid"); } if (!isset($this->_index[$s])) { @@ -214,7 +214,7 @@ private function _add_triple($s, $p, Array $o_info) { * @param string $value * @return bool */ - protected function isValidLiteralValue($value){ + protected function isValidLiteral($value){ if(!is_scalar($value)){ return false; } @@ -227,7 +227,7 @@ protected function isValidLiteralValue($value){ * @param string $value * @return bool */ - protected function isValidResourceValue($value){ + protected function isValidResource($value){ if(!is_string($value) || empty($value)){ return false; } diff --git a/src/mongo/MongoGraph.class.php b/src/mongo/MongoGraph.class.php index adbd1943..436131c9 100644 --- a/src/mongo/MongoGraph.class.php +++ b/src/mongo/MongoGraph.class.php @@ -138,7 +138,7 @@ private function add_tarray_to_index($tarray) } else if($key == "_id"){ // If the subject is invalid then throw an exception - if(!isset($value['r']) || !$this->isValidResourceValue($value['r'])){ + if(!isset($value['r']) || !$this->isValidResource($value['r'])){ throw new \Tripod\Exceptions\Exception("The subject cannot be an empty string"); } } @@ -160,7 +160,7 @@ private function toGraphValueObject($mongoValueObject) if (array_key_exists(VALUE_LITERAL,$mongoValueObject)) { // only allow valid values - if($this->isValidLiteralValue($mongoValueObject[VALUE_LITERAL])){ + if($this->isValidLiteral($mongoValueObject[VALUE_LITERAL])){ // single value literal $simpleGraphValueObject[] = array( 'type'=>'literal', @@ -170,7 +170,7 @@ private function toGraphValueObject($mongoValueObject) else if (array_key_exists(VALUE_URI,$mongoValueObject)) { // only allow valid values - if($this->isValidResourceValue($mongoValueObject[VALUE_URI])) { + if($this->isValidResource($mongoValueObject[VALUE_URI])) { // single value uri $simpleGraphValueObject[] = array( 'type' => 'uri', @@ -186,13 +186,13 @@ private function toGraphValueObject($mongoValueObject) { // Make sure the value is valid if($type==VALUE_LITERAL){ - if(!$this->isValidLiteralValue($value)){ + if(!$this->isValidLiteral($value)){ continue; } $valueTypeLabel = 'literal'; } else{ - if(!$this->isValidResourceValue($value)){ + if(!$this->isValidResource($value)){ continue; } $valueTypeLabel = 'uri';