Merge pull request #5610 from mrclay/5600

Fixes #5600: Entities are kept out of entity cache during save

5 files changed, 71 insertions, 8 deletions

diff --git a/engine/classes/ElggEntity.php b/engine/classes/ElggEntity.php
index 8b3ceb551..dd1c7c114 100644
--- a/engine/classes/ElggEntity.php
+++ b/engine/classes/ElggEntity.php
@@ -1270,15 +1270,23 @@ abstract class ElggEntity extends ElggData implements
 	public function save() {
 		$guid = $this->getGUID();
 		if ($guid > 0) {
-			_elgg_cache_entity($this);
 
-			return update_entity(
+			// See #5600. This ensures the lower level can_edit_entity() check will use a
+			// fresh entity from the DB so it sees the persisted owner_guid
+			_elgg_disable_caching_for_entity($guid);
+
+			$ret = update_entity(
 				$guid,
 				$this->get('owner_guid'),
 				$this->get('access_id'),
 				$this->get('container_guid'),
 				$this->get('time_created')
 			);
+
+			_elgg_enable_caching_for_entity($guid);
+			_elgg_cache_entity($this);
+
+			return $ret;
 		} else {
 			// Create a new entity (nb: using attribute array directly
 			// 'cos set function does something special!)
diff --git a/engine/classes/ElggGroup.php b/engine/classes/ElggGroup.php
index 61f9163d5..7e69b7a84 100644
--- a/engine/classes/ElggGroup.php
+++ b/engine/classes/ElggGroup.php
@@ -352,7 +352,12 @@ class ElggGroup extends ElggEntity
 		}
 
 		// Now save specific stuff
-		return create_group_entity($this->get('guid'), $this->get('name'), $this->get('description'));
+
+		_elgg_disable_caching_for_entity($this->guid);
+		$ret = create_group_entity($this->get('guid'), $this->get('name'), $this->get('description'));
+		_elgg_enable_caching_for_entity($this->guid);
+
+		return $ret;
 	}
 
 	// EXPORTABLE INTERFACE ////////////////////////////////////////////////////////////
diff --git a/engine/classes/ElggObject.php b/engine/classes/ElggObject.php
index d54752dca..aeaa3ba5c 100644
--- a/engine/classes/ElggObject.php
+++ b/engine/classes/ElggObject.php
@@ -126,8 +126,12 @@ class ElggObject extends ElggEntity {
 		}
 
 		// Save ElggObject-specific attributes
-		return create_object_entity($this->get('guid'), $this->get('title'),
-			$this->get('description'));
+
+		_elgg_disable_caching_for_entity($this->guid);
+		$ret = create_object_entity($this->get('guid'), $this->get('title'), $this->get('description'));
+		_elgg_enable_caching_for_entity($this->guid);
+
+		return $ret;
 	}
 
 	/**
diff --git a/engine/classes/ElggUser.php b/engine/classes/ElggUser.php
index b2cada8ef..6163f9b62 100644
--- a/engine/classes/ElggUser.php
+++ b/engine/classes/ElggUser.php
@@ -132,9 +132,13 @@ class ElggUser extends ElggEntity
 		}
 
 		// Now save specific stuff
-		return create_user_entity($this->get('guid'), $this->get('name'), $this->get('username'),
+		_elgg_disable_caching_for_entity($this->guid);
+		$ret = create_user_entity($this->get('guid'), $this->get('name'), $this->get('username'),
 			$this->get('password'), $this->get('salt'), $this->get('email'), $this->get('language'),
 			$this->get('code'));
+		_elgg_enable_caching_for_entity($this->guid);
+
+		return $ret;
 	}
 
 	/**
diff --git a/engine/lib/entities.php b/engine/lib/entities.php
index 5cfeca6f8..d0d97aa95 100644
--- a/engine/lib/entities.php
+++ b/engine/lib/entities.php
@@ -17,6 +17,15 @@ global $ENTITY_CACHE;
 $ENTITY_CACHE = array();
 
 /**
+ * GUIDs of entities banned from the entity cache (during this request)
+ *
+ * @global array $ENTITY_CACHE_DISABLED_GUIDS
+ * @access private
+ */
+global $ENTITY_CACHE_DISABLED_GUIDS;
+$ENTITY_CACHE_DISABLED_GUIDS = array();
+
+/**
  * Cache subtypes and related class names.
  *
  * @global array|null $SUBTYPE_CACHE array once populated from DB, initially null
@@ -26,6 +35,34 @@ global $SUBTYPE_CACHE;
 $SUBTYPE_CACHE = null;
 
 /**
+ * Remove this entity from the entity cache and make sure it is not re-added
+ *
+ * @param int $guid The entity guid
+ *
+ * @access private
+ * @todo this is a workaround until #5604 can be implemented
+ */
+function _elgg_disable_caching_for_entity($guid) {
+	global $ENTITY_CACHE_DISABLED_GUIDS;
+
+	_elgg_invalidate_cache_for_entity($guid);
+	$ENTITY_CACHE_DISABLED_GUIDS[$guid] = true;
+}
+
+/**
+ * Allow this entity to be stored in the entity cache
+ *
+ * @param int $guid The entity guid
+ *
+ * @access private
+ */
+function _elgg_enable_caching_for_entity($guid) {
+	global $ENTITY_CACHE_DISABLED_GUIDS;
+
+	unset($ENTITY_CACHE_DISABLED_GUIDS[$guid]);
+}
+
+/**
  * Invalidate this class's entry in the cache.
  *
  * @param int $guid The entity guid
@@ -57,7 +94,7 @@ function _elgg_invalidate_cache_for_entity($guid) {
  * @todo Use an ElggCache object
  */
 function _elgg_cache_entity(ElggEntity $entity) {
-	global $ENTITY_CACHE;
+	global $ENTITY_CACHE, $ENTITY_CACHE_DISABLED_GUIDS;
 
 	// Don't cache non-plugin entities while access control is off, otherwise they could be
 	// exposed to users who shouldn't see them when control is re-enabled.
@@ -65,6 +102,11 @@ function _elgg_cache_entity(ElggEntity $entity) {
 		return;
 	}
 
+	$guid = $entity->getGUID();
+	if (isset($ENTITY_CACHE_DISABLED_GUIDS[$guid])) {
+		return;
+	}
+
 	// Don't store too many or we'll have memory problems
 	// @todo Pick a less arbitrary limit
 	if (count($ENTITY_CACHE) > 256) {
@@ -79,7 +121,7 @@ function _elgg_cache_entity(ElggEntity $entity) {
 		elgg_get_metadata_cache()->clear($random_guid);
 	}
 
-	$ENTITY_CACHE[$entity->guid] = $entity;
+	$ENTITY_CACHE[$guid] = $entity;
 }
 
 /**