90% - Prevent null values being added to ExtendedGraph

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;
     }
 
     /**
@@ -173,22 +179,49 @@ public function add_literal_triple($s, $p, $o, $lang = null, $dt = null) {
     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 );
+            $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.
+     *
+     * @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 isValidResourceValue($value){
+        if(!is_string($value)){
+            return false;
+        }
+        return true;
+    }
+
     /**
      * @deprecated this is deprecated
      */

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,65 @@ 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->isValidLiteralValue($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->isValidResourceValue($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->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);
                 }
             }
         }
+        // If we don't have any values, then respond with false
+        if(empty($simpleGraphValueObject)){
+            return false;
+        }
+        // Otherwise we have found valid values
         return $simpleGraphValueObject;
     }
 

test/unit/ExtendedGraphTest.php

@@ -23,6 +23,84 @@ 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(){})
+        );
+    }
+
+    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(){})
+        );
+    }
+
     public function testRemoveProperties()
     {
         $graph = new ExtendedGraph();