Skip to content

Commit

Permalink
Fixed up from PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@lordtatty
lordtatty committed Jul 22, 2015
1 parent 5396f60 commit 60d0e3c
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 88 deletions.
22 changes: 12 additions & 10 deletions src/classes/ExtendedGraph.class.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -198,7 +199,6 @@ private function _add_triple($s, $p, Array $o_info) {
return true;
}
}
}
return false;
}

Expand All @@ -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){
Expand All @@ -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;
Expand Down
31 changes: 11 additions & 20 deletions src/mongo/MongoGraph.class.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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");
}
}
Expand All @@ -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)
{
Expand All @@ -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',
Expand All @@ -197,7 +192,7 @@ private function toGraphValueObject($mongoValueObject)
$valueTypeLabel = 'literal';
}
else{
if(!$this->isValidTripleValue($value)){
if(!$this->isValidResourceValue($value)){
continue;
}
$valueTypeLabel = 'uri';
Expand All @@ -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;
}
Expand Down
62 changes: 20 additions & 42 deletions test/unit/ExtendedGraphTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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),
Expand Down Expand Up @@ -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),
Expand All @@ -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),
Expand Down
42 changes: 26 additions & 16 deletions test/unit/mongo/MongoGraphTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(
Expand All @@ -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"),
Expand All @@ -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()
{
Expand Down

0 comments on commit 60d0e3c

Please sign in to comment.