From 60d0e3c1b0b4731c6bb22dc3bfa736546e861bab Mon Sep 17 00:00:00 2001 From: Richard Tattersall Date: Wed, 22 Jul 2015 22:48:07 +0100 Subject: [PATCH] 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() {