fix(core): prevent issues with unsaved data and system log

Elgg · Nov 4, 2022 · 4048eee · 4048eee
1 parent 8967c46
commit 4048eee
Show file tree

Hide file tree

Showing 13 changed files with 33 additions and 19 deletions.
diff --git a/engine/classes/ElggAccessCollection.php b/engine/classes/ElggAccessCollection.php
@@ -234,7 +234,7 @@ public function toObject(array $params = []) {
 	 * {@inheritdoc}
 	 */
 	public function getSystemLogID(): int {
-		return $this->id;
+		return (int) $this->id;
 	}
 
 	/**

diff --git a/engine/classes/ElggEntity.php b/engine/classes/ElggEntity.php
@@ -1746,13 +1746,10 @@ public function getLongitude(): float {
 	 */
 
 	/**
-	 * Return an identification for the object for storage in the system log.
-	 * This id must be an integer.
-	 *
-	 * @return int
+	 * {@inheritdoc}
 	 */
 	public function getSystemLogID(): int {
-		return $this->getGUID();
+		return (int) $this->getGUID();
 	}
 
 	/**

diff --git a/engine/classes/ElggExtender.php b/engine/classes/ElggExtender.php
@@ -180,13 +180,10 @@ public function toObject(array $params = []) {
 	 */
 
 	/**
-	 * Return an identification for the object for storage in the system log.
-	 * This id must be an integer.
-	 *
-	 * @return int
+	 * {@inheritdoc}
 	 */
 	public function getSystemLogID(): int {
-		return $this->id;
+		return (int) $this->id;
 	}
 
 	/**

diff --git a/engine/classes/ElggRelationship.php b/engine/classes/ElggRelationship.php
@@ -197,13 +197,10 @@ public function toObject(array $params = []) {
 	// SYSTEM LOG INTERFACE ////////////////////////////////////////////////////////////
 
 	/**
-	 * Return an identification for the object for storage in the system log.
-	 * This id must be an integer.
-	 *
-	 * @return int
+	 * {@inheritdoc}
 	 */
 	public function getSystemLogID(): int {
-		return $this->id;
+		return (int) $this->id;
 	}
 
 	/**

diff --git a/engine/classes/Loggable.php b/engine/classes/Loggable.php
@@ -12,7 +12,7 @@
 interface Loggable {
 	/**
 	 * Return an identification for the object for storage in the system log.
-	 * This id must be an integer.
+	 * This id must be an integer. Unsaved implementations should return 0.
 	 *
 	 * @return int
 	 */

diff --git a/engine/tests/phpunit/unit/ElggAccessCollectionUnitTest.php b/engine/tests/phpunit/unit/ElggAccessCollectionUnitTest.php
@@ -21,4 +21,9 @@ public function invalidValuesProvider() {
 			['owner_guid', -1],
 		];
 	}
+
+	public function testIsLoggable() {
+		$unsaved = new \ElggAccessCollection();
+		$this->assertEmpty($unsaved->getSystemLogID());
+	}
 }
diff --git a/engine/tests/phpunit/unit/ElggGroupUnitTest.php b/engine/tests/phpunit/unit/ElggGroupUnitTest.php
@@ -47,6 +47,9 @@ public function testCanArrayAccessAttributes() {
 	}
 
 	public function testIsLoggable() {
+		$unsaved = new \ElggGroup();
+		$this->assertEmpty($unsaved->getSystemLogID());
+
 		$group = $this->createGroup();
 
 		$this->assertEquals($group->guid, $group->getSystemLogID());

diff --git a/engine/tests/phpunit/unit/ElggMetadataUnitTest.php b/engine/tests/phpunit/unit/ElggMetadataUnitTest.php
@@ -209,6 +209,9 @@ public function testCanArrayAccessAttributes() {
 	}
 
 	public function testIsLoggable() {
+		$unsaved = new \ElggMetadata();
+		$this->assertEmpty($unsaved->getSystemLogID());
+
 		$object = $this->createObject();
 		$metadata = new ElggMetadata();
 		$metadata->entity_guid = $object->guid;

diff --git a/engine/tests/phpunit/unit/ElggObjectUnitTest.php b/engine/tests/phpunit/unit/ElggObjectUnitTest.php
@@ -232,6 +232,9 @@ public function testCanArrayAccessAttributes() {
 	}
 
 	public function testIsLoggable() {
+		$unsaved = new \ElggObject();
+		$this->assertEmpty($unsaved->getSystemLogID());
+
 		$object = $this->createObject();
 
 		$this->assertEquals($object->guid, $object->getSystemLogID());

diff --git a/engine/tests/phpunit/unit/ElggRelationshipUnitTest.php b/engine/tests/phpunit/unit/ElggRelationshipUnitTest.php
@@ -93,6 +93,9 @@ public function testCanArrayAccessAttributes() {
 	}
 
 	public function testIsLoggable() {
+		$unsaved = new \ElggRelationship(new \stdClass());
+		$this->assertEmpty($unsaved->getSystemLogID());
+
 		$relationship = $this->createRelationship();
 
 		$this->assertEquals($relationship->id, $relationship->getSystemLogID());

diff --git a/engine/tests/phpunit/unit/ElggSiteUnitTest.php b/engine/tests/phpunit/unit/ElggSiteUnitTest.php
@@ -93,6 +93,9 @@ public function testCanArrayAccessAttributes() {
 	}
 
 	public function testIsLoggable() {
+		$unsaved = new \ElggSite();
+		$this->assertEmpty($unsaved->getSystemLogID());
+
 		$site = elgg_get_site_entity();
 
 		$this->assertEquals($site->guid, $site->getSystemLogID());

diff --git a/engine/tests/phpunit/unit/ElggUserUnitTest.php b/engine/tests/phpunit/unit/ElggUserUnitTest.php
@@ -96,6 +96,9 @@ public function testCanArrayAccessAttributes() {
 	}
 
 	public function testIsLoggable() {
+		$unsaved = new \ElggUser();
+		$this->assertEmpty($unsaved->getSystemLogID());
+
 		$user = $this->createUser();
 
 		$this->assertEquals($user->guid, $user->getSystemLogID());

diff --git a/mod/system_log/classes/Elgg/SystemLog/SystemLog.php b/mod/system_log/classes/Elgg/SystemLog/SystemLog.php
@@ -110,7 +110,7 @@ public function rowToSystemLogEntry(\stdClass $row): SystemLogEntry {
 	protected function prepareObjectForInsert(\Loggable $object): \stdClass {
 		$insert = new \stdClass();
 
-		$insert->object_id = (int) $object->getSystemLogID();
+		$insert->object_id = $object->getSystemLogID();
 		$insert->object_class = get_class($object);
 		$insert->object_type = $object->getType();
 		$insert->object_subtype = $object->getSubtype();
@@ -152,7 +152,7 @@ public function insert($object, $event): void {
 			return;
 		}
 
-		if (!$object instanceof \Loggable) {
+		if (!$object instanceof \Loggable || empty($object->getSystemLogID())) {
 			return;
 		}