Skip to content

Commit

Permalink
Merge pull request #82 from talis/do-not-allow-null-values-to-be-adde…
Browse files Browse the repository at this point in the history 
…d-to-extendedgraph

90% - Prevent null values being added to ExtendedGraph
  • Loading branch information
@lordtatty
lordtatty committed Jul 23, 2015
2 parents 14d5761 + c8ef3f6 commit 89044e8
Show file tree
Hide file tree
Showing 4 changed files with 491 additions and 26 deletions.
75 changes: 60 additions & 15 deletions src/classes/ExtendedGraph.class.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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->isValidResource($o)) {
return $this->_add_triple($s, $p, array('type' => strpos($o, '_:') === 0 ? 'bnode' : 'uri', 'value' => $o));
}
return false;
}

/**
Expand All @@ -154,41 +157,83 @@ 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->isValidLiteral($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;
}

/**
* @param string $s
* @param string $p
* @param array $o_info
* @throws Exceptions\Exception
* @return bool
*/
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->isValidResource($s)){
throw new \Tripod\Exceptions\Exception("The subject is invalid");
}
if(!$this->isValidResource($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 );
$this->_index[$s][$p] = array($o_info);
return true;
}
elseif (!isset($this->_index[$s][$p])) {
$this->_index[$s][$p] = array( $o_info);
} 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] ) ) {
} 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.
*
* @param string $value
* @return bool
*/
protected function isValidLiteral($value){
if(!is_scalar($value)){
return false;
}
return true;
}

/**
* Check if a triple value is valid.
*
* @param string $value
* @return bool
*/
protected function isValidResource($value){
if(!is_string($value) || empty($value)){
return false;
}
return true;
}

/**
* @deprecated this is deprecated
*/
Expand Down
55 changes: 44 additions & 11 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 @@ -129,7 +130,17 @@ 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 (!empty($graphValueObject)) {
$predObjects[$predicate] = $graphValueObject;
}
}
else if($key == "_id"){
// If the subject is invalid then throw an exception
if(!isset($value['r']) || !$this->isValidResource($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;
Expand All @@ -138,39 +149,61 @@ private function add_tarray_to_index($tarray)

/**
* Convert from Tripod value object format (comapct) to ExtendedGraph format (verbose)
*
* @param array $mongoValueObject
* @return array
*/
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->isValidLiteral($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->isValidResource($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)
{
// Make sure the value is valid
if($type==VALUE_LITERAL){
if(!$this->isValidLiteral($value)){
continue;
}
$valueTypeLabel = 'literal';
}
else{
if(!$this->isValidResource($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);
}
}
}
// Otherwise we have found valid values
return $simpleGraphValueObject;
}

Expand Down
170 changes: 170 additions & 0 deletions test/unit/ExtendedGraphTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,176 @@ 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
*/
public function testAddInvalidValueToLiteralResultsInNoTriple($value)
{
$graph = new ExtendedGraph();
$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');

$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(
array(null),
array(new stdClass()),
array(function(){})
);
}

/**
* @dataProvider addInvalidSubjectToLiteralResultsInNoTriple_Provider
*/
public function testAddInvalidSubjectToLiteralThrowsException($value)
{
$this->setExpectedException('\Tripod\Exceptions\Exception');

$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),
array(array()),
array(null),
array(new stdClass()),
array(function(){})
);
}

/**
* @dataProvider addInvalidSubjectToLiteralResultsInNoTriple_Provider
*/
public function testAddInvalidPredicateToLiteralThrowsException($value)
{
$this->setExpectedException('\Tripod\Exceptions\Exception');

$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),
array(array()),
array(null),
array(new stdClass()),
array(function(){})
);
}

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
*/
public function testAddInvalidValueToResourceResultsInNoTriple($value)
{
$graph = new ExtendedGraph();

$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');

$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 addInvalidValueToResourceResultsInNoTriple_Provider(){
return array(
array(1),
array(1.2),
array(true),
array(array()),
array(null),
array(new stdClass()),
array(function(){})
);
}

/**
* @dataProvider addInvalidSubjectToResourceResultsInNoTriple_Provider
*/
public function testAddInvalidSubjectToResourceThrowsException($value)
{
$this->setExpectedException('\Tripod\Exceptions\Exception');

$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),
array(array()),
array(null),
array(new stdClass()),
array(function(){})
);
}

/**
* @dataProvider addInvalidSubjectToLiteralResultsInNoTriple_Provider
*/
public function testAddInvalidPredicateToResourceThrowsException($value)
{
$this->setExpectedException('\Tripod\Exceptions\Exception');

$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),
array(array()),
array(null),
array(new stdClass()),
array(function(){})
);
}

public function testRemoveProperties()
{
$graph = new ExtendedGraph();
Expand Down
Loading

0 comments on commit 89044e8

Please sign in to comment.