Add filtering for site entities, faster filtering

diff --git a/engine/classes/ElggAttributeLoader.php b/engine/classes/ElggAttributeLoader.php
index b91e4b88a5549557f1a2e7abc6e866859d2c9553..0b770da757a2d5efdc295acce230c7b92ecd2737 100644 (file)
--- a/engine/classes/ElggAttributeLoader.php
+++ b/engine/classes/ElggAttributeLoader.php
@@ -24,7 +24,7 @@ class ElggAttributeLoader {
                'time_created',
                'time_updated',
                'last_action',
-               'enabled'
+               'enabled',
        );
 
        /**
@@ -200,6 +200,8 @@ class ElggAttributeLoader {
                                                // saved, these are stored w/ type "site", but with no sites_entity row. These
                                                // are probably only created in the unit tests.
                                                // @todo Don't save vanilla ElggEntities with type "site"
+
+                                               $row = $this->filterAddedColumns($row);
                                                $row['guid'] = (int) $row['guid'];
                                                return $row;
                                        }
@@ -209,6 +211,8 @@ class ElggAttributeLoader {
                        }
                }
 
+               $row = $this->filterAddedColumns($row);
+
                // Note: If there are still missing attributes, we're running on a 1.7 or earlier schema. We let
                // this pass so the upgrades can run.
 
@@ -217,4 +221,28 @@ class ElggAttributeLoader {
 
                return $row;
        }
+
+       /**
+        * Filter out keys returned by the query which should not appear in the entity's attributes
+        *
+        * @param array $row All columns from the query
+        * @return array Columns acceptable for the entity's attributes
+        */
+       protected function filterAddedColumns($row) {
+               // make an array with keys as acceptable attribute names
+               $acceptable_attrs = self::$primary_attr_names;
+               array_splice($acceptable_attrs, count($acceptable_attrs), 0, $this->secondary_attr_names);
+               $acceptable_attrs = array_combine($acceptable_attrs, $acceptable_attrs);
+
+               // @todo remove these when #4584 is in place
+               $acceptable_attrs['tables_split'] = true;
+               $acceptable_attrs['tables_loaded'] = true;
+
+               foreach ($row as $key => $val) {
+                       if (!isset($acceptable_attrs[$key])) {
+                               unset($row[$key]);
+                       }
+               }
+               return $row;
+       }
 }

diff --git a/engine/tests/regression/trac_bugs.php b/engine/tests/regression/trac_bugs.php
index 180fb5112c2cf8433436eadae6619c436a830311..d7bb20f3b0ae8dfd8009119724f569dcf1f13b2e 100644 (file)
--- a/engine/tests/regression/trac_bugs.php
+++ b/engine/tests/regression/trac_bugs.php
@@ -297,31 +297,34 @@ class ElggCoreRegressionBugsTest extends ElggCoreUnitTest {
                }
        }
        
-       /**\r
-        * Checks if additional select columns does not leak to entity attributes.
-        * \r
-        * https://github.com/Elgg/Elgg/issues/5538\r
-        */\r
-       public function test_sql_selects_leak_to_attributes() {
+       /**
+        * Ensure additional select columns do not end up in entity attributes.
+        * 
+        * https://github.com/Elgg/Elgg/issues/5538
+        */
+       public function test_extra_columns_dont_appear_in_attributes() {
                global $ENTITY_CACHE;
-               //may not have groups in DB - let's create one  
+
+               // may not have groups in DB - let's create one
                $group = new ElggGroup();
                $group->name = 'test_group';
-               $group->access_id = ACCESS_PUBLIC;\r
-               $this->assertTrue($group->save() !== false);\r
+               $group->access_id = ACCESS_PUBLIC;
+               $this->assertTrue($group->save() !== false);
                
-               //entity cache interferes with our test
+               // entity cache interferes with our test
                $ENTITY_CACHE = array();
                
                foreach (array('site', 'user', 'group', 'object') as $type) {
                        $entities = elgg_get_entities(array(
-                               'type' => $type,\r
-                               'selects' => array('42 as added_col'),\r
-                               'limit' => 1,\r
+                               'type' => $type,
+                               'selects' => array('1 as _nonexistent_test_column'),
+                               'limit' => 1,
                        ));
-                       $entity = array_shift($entities);
-                       $this->assertTrue($entity instanceof ElggEntity);
-                       $this->assertEqual($entity->added_col, null, "Additional select columns are leaking to attributes for " . get_class($entity));
+                       if (!$this->assertTrue($entities, "Query for '$type' did not return an entity.")) {
+                               continue;
+                       }
+                       $entity = $entities[0];
+                       $this->assertNull($entity->_nonexistent_test_column, "Additional select columns are leaking to attributes for '$type'");
                }
                
                $group->delete();