Fixing PUBLIC usage, Fixing DB-View, Creating tests

database.sql

@@ -2587,3 +2587,23 @@ CREATE VIEW `workerqueue-view` AS SELECT
 	FROM `process`
 			INNER JOIN `workerqueue` ON `workerqueue`.`pid` = `process`.`pid`
 			WHERE NOT `workerqueue`.`done`;
+
+--
+-- VIEW profile_field-view
+--
+DROP VIEW IF EXISTS `profile_field-view`;
+CREATE VIEW `profile_field-view` AS SELECT
+	  `profile_field`.`id` AS `id`,
+	  `profile_field`.`uid` AS `uid`,
+	  `profile_field`.`label` AS `label`,
+	  `profile_field`.`value` AS `value`,
+	  `profile_field`.`order` AS `order`,
+	  `profile_field`.`psid` AS `psid`,
+	  `permissionset`.`allow_cid` AS `allow_cid`,
+	  `permissionset`.`allow_gid` AS `allow_gid`,
+	  `permissionset`.`deny_cid` AS `deny_cid`,
+	  `permissionset`.`deny_gid` AS `deny_gid`,
+	  `profile_field`.`created` AS `created`,
+	  `profile_field`.`edited` AS `edited`
+  FROM `profile_field`
+		   INNER JOIN `permissionset` ON `permissionset`.`id` = `profile_field`.`psid`;

src/BaseEntity.php

@@ -53,4 +53,18 @@ abstract class BaseEntity extends BaseDataTransferObject
 
 		return $this->$name;
 	}
+
+	/**
+	 * @param $name
+	 * @return bool
+	 * @throws HTTPException\InternalServerErrorException
+	 */
+	public function __isset($name)
+	{
+		if (!property_exists($this, $name)) {
+			throw new HTTPException\InternalServerErrorException('Unknown property ' . $name . ' in Entity ' . static::class);
+		}
+
+		return !empty($this->$name);
+	}
 }

src/Factory/Api/Mastodon/Account.php

@@ -27,8 +27,7 @@ use Friendica\Collection\Api\Mastodon\Fields;
 use Friendica\Model\APContact;
 use Friendica\Model\Contact;
 use Friendica\Network\HTTPException;
-use Friendica\Repository\ProfileField;
+use Friendica\Profile\ProfileField\Depository\ProfileField as ProfileFieldDepository;
-use Friendica\Security\PermissionSet\Depository\PermissionSet;
 use ImagickException;
 use Psr\Log\LoggerInterface;
 
@@ -36,17 +35,17 @@ class Account extends BaseFactory
 {
 	/** @var BaseURL */
 	private $baseUrl;
-	/** @var ProfileField */
+	/** @var ProfileFieldDepository */
-	private $profileFieldRepo;
+	private $profileFieldDepo;
 	/** @var Field */
 	private $mstdnFieldFactory;
 
-	public function __construct(LoggerInterface $logger, BaseURL $baseURL, ProfileField $profileFieldRepo, Field $mstdnFieldFactory)
+	public function __construct(LoggerInterface $logger, BaseURL $baseURL, ProfileFieldDepository $profileFieldDepo, Field $mstdnFieldFactory)
 	{
 		parent::__construct($logger);
 
 		$this->baseUrl           = $baseURL;
-		$this->profileFieldRepo  = $profileFieldRepo;
+		$this->profileFieldDepo  = $profileFieldDepo;
 		$this->mstdnFieldFactory = $mstdnFieldFactory;
 	}
 
@@ -77,7 +76,7 @@ class Account extends BaseFactory
 
 		$self_contact = Contact::selectFirst(['uid'], ['nurl' => $publicContact['nurl'], 'self' => true]);
 		if (!empty($self_contact['uid'])) {
-			$profileFields = $this->profileFieldRepo->select(['uid' => $self_contact['uid'], 'psid' => PermissionSet::PUBLIC]);
+			$profileFields = $this->profileFieldDepo->selectPublicFieldsByUserId($self_contact['uid']);
 			$fields        = $this->mstdnFieldFactory->createFromProfileFields($profileFields);
 		} else {
 			$fields = new Fields();
@@ -95,7 +94,7 @@ class Account extends BaseFactory
 	{
 		$publicContact = Contact::selectFirst([], ['uid' => $userId, 'self' => true]);
 
-		$profileFields = $this->profileFieldRepo->select(['uid' => $userId, 'psid' => PermissionSet::PUBLIC]);
+		$profileFields = $this->profileFieldDepo->selectPublicFieldsByUserId($userId);
 		$fields        = $this->mstdnFieldFactory->createFromProfileFields($profileFields);
 
 		$apContact = APContact::getByURL($publicContact['url'], false);

src/Factory/Api/Mastodon/Field.php

@@ -31,7 +31,7 @@ use Friendica\Network\HTTPException;
 class Field extends BaseFactory
 {
 	/**
-	 * @param \Friendica\Profile\ProfileField\Entity\ProfileField $profileField
+	 * @param ProfileField $profileField
 	 *
 	 * @return \Friendica\Object\Api\Mastodon\Field
 	 * @throws HTTPException\InternalServerErrorException

src/Module/ActivityPub/Objects.php

@@ -86,7 +86,7 @@ class Objects extends BaseModule
 					$permissionSets = DI::permissionSet()->selectByContactId($requester_id, $item['uid']);
 					if (!empty($permissionSets)) {
 						$psid = array_merge($permissionSets->column('id'),
-							[DI::permissionSet()->selectEmptyForUser($item['uid'])]);
+							[DI::permissionSet()->selectPublic($item['uid'])]);
 						$validated = in_array($item['psid'], $psid);
 					}
 				}

src/Module/Profile/Status.php

@@ -213,7 +213,7 @@ class Status extends BaseProfile
 				$permissionSets = DI::permissionSet()->selectByContactId($remote_user, $profile['uid']);
 				if (!empty($permissionSets)) {
 					$condition = ['psid' => array_merge($permissionSets->column('id'),
-							[DI::permissionSet()->selectEmptyForUser($profile['uid'])->id])];
+							[DI::permissionSet()->selectPublic($profile['uid'])->id])];
 				}
 			} elseif ($profile['uid'] == local_user()) {
 				$condition = [];

src/Module/Settings/Profile/Index.php

@@ -287,11 +287,11 @@ class Index extends BaseSettings
 				$profileFieldInputs['new']['value'],
 				$permissionSet
 			));
-
-			unset($profileFieldInputs['new']);
-			unset($profileFieldOrder['new']);
 		}
 
+		unset($profileFieldInputs['new']);
+		unset($profileFieldOrder['new']);
+
 		foreach ($profileFieldInputs as $id => $profileFieldInput) {
 			$permissionSet = DI::permissionSet()->selectOrCreate(DI::permissionSetFactory()->createFromString(
 				$uid,

src/Profile/ProfileField/Depository/ProfileField.php

@@ -125,9 +125,11 @@ class ProfileField extends BaseDepository
 	public function selectPublicFieldsByUserId(int $uid): Collection\ProfileFields
 	{
 		try {
+			$publicPermissionSet = $this->permissionSetDepository->selectPublic($uid);
+
 			return $this->select([
 				'uid'  => $uid,
-				'psid' => PermissionSetDepository::PUBLIC,
+				'psid' => $publicPermissionSet->id
 			]);
 		} catch (\Exception $exception) {
 			throw new ProfileFieldPersistenceException(sprintf('Cannot select public ProfileField for user "%d"', $uid), $exception);
@@ -166,7 +168,7 @@ class ProfileField extends BaseDepository
 		$permissionSetIds = $permissionSets->column('id');
 
 		// Includes public custom fields
-		$permissionSetIds[] = PermissionSetDepository::PUBLIC;
+		$permissionSetIds[] = $this->permissionSetDepository->selectPublic($uid)->id;
 
 		return $this->select(
 			['uid' => $uid, 'psid' => $permissionSetIds],
@@ -215,6 +217,10 @@ class ProfileField extends BaseDepository
 	 */
 	public function save(Entity\ProfileField $profileField): Entity\ProfileField
 	{
+		if ($profileField->permissionSet->id === null) {
+			throw new ProfileFieldPersistenceException('PermissionSet needs to be saved first.');
+		}
+
 		$fields = $this->convertToTableRow($profileField);
 
 		try {

src/Profile/ProfileField/Factory/ProfileField.php

@@ -49,7 +49,7 @@ class ProfileField extends BaseFactory implements ICanCreateFromTableRow
 	public function createFromTableRow(array $row, PermissionSet $permissionSet = null): Entity\ProfileField
 	{
 		if (empty($permissionSet) &&
-			(empty($row['psid']) || !array_key_exists('allow_cid', $row) || !array_key_exists('allow_gid', $row) || !array_key_exists('deny_cid', $row) || !array_key_exists('deny_gid', $row))
+			(!array_key_exists('psid', $row) || !array_key_exists('allow_cid', $row) || !array_key_exists('allow_gid', $row) || !array_key_exists('deny_cid', $row) || !array_key_exists('deny_gid', $row))
 		) {
 			throw new UnexpectedPermissionSetException('Either set the PermissionSet fields (join) or the PermissionSet itself');
 		}

src/Protocol/ActivityPub/Transmitter.php

@@ -229,7 +229,7 @@ class Transmitter
 				$permissionSets = DI::permissionSet()->selectByContactId($requester_id, $owner['uid']);
 				if (!empty($permissionSets)) {
 					$condition = ['psid' => array_merge($permissionSets->column('id'),
-							[DI::permissionSet()->selectEmptyForUser($owner['uid'])])];
+							[DI::permissionSet()->selectPublic($owner['uid'])])];
 				}
 			}
 		}

src/Security/PermissionSet/Depository/PermissionSet.php

@@ -53,6 +53,22 @@ class PermissionSet extends BaseDepository
 		$this->aclFormatter = $aclFormatter;
 	}
 
+	/**
+	 * replaces the PUBLIC id for the public permissionSet
+	 * (no need to create the default permission set over and over again)
+	 *
+	 * @param $condition
+	 */
+	private function checkPublicSelect(&$condition)
+	{
+		if (empty($condition['allow_cid']) &&
+			empty($condition['allow_gid']) &&
+			empty($condition['deny_cid']) &&
+			empty($condition['deny_gid'])) {
+			$condition['uid'] = self::PUBLIC;
+		}
+	}
+
 	/**
 	 * @param array $condition
 	 * @param array $params
@@ -70,6 +86,16 @@ class PermissionSet extends BaseDepository
 		return new Collection\PermissionSets(parent::_select($condition, $params)->getArrayCopy());
 	}
 
+	private function checkPublic(Entity\PermissionSet $permissionSet): bool
+	{
+		return (($permissionSet->id === self::PUBLIC) ||
+				(is_null($permissionSet->id) &&
+				 empty($permissionSet->allow_cid) &&
+				 empty($permissionSet->allow_gid) &&
+				 empty($permissionSet->deny_cid) &&
+				 empty($permissionSet->deny_gid)));
+	}
+
 	/**
 	 * Converts a given PermissionSet into a DB compatible row array
 	 *
@@ -89,22 +115,18 @@ class PermissionSet extends BaseDepository
 	}
 
 	/**
-	 * @param int      $id A permissionset table row id or self::PUBLIC
+	 * @param int $id  A PermissionSet table row id or self::PUBLIC
-	 * @param int|null $uid Should be provided when id can be self::PUBLIC
+	 * @param int $uid The owner of the PermissionSet
 	 * @return Entity\PermissionSet
 	 * @throws NotFoundException
 	 */
-	public function selectOneById(int $id, int $uid = null): Entity\PermissionSet
+	public function selectOneById(int $id, int $uid): Entity\PermissionSet
 	{
 		if ($id === self::PUBLIC) {
-			if (empty($uid)) {
-				throw new \InvalidArgumentException('Missing uid for Public permission set instantiation');
-			}
-
 			return $this->factory->createFromString($uid);
 		}
 
-		return $this->selectOne(['id' => $id]);
+		return $this->selectOne(['id' => $id, 'uid' => $uid]);
 	}
 
 	/**
@@ -174,15 +196,15 @@ class PermissionSet extends BaseDepository
 	}
 
 	/**
-	 * Fetch the empty PermissionSet for a given user, create it if it doesn't exist
+	 * Fetch the public PermissionSet
 	 *
 	 * @param int $uid
 	 *
 	 * @return Entity\PermissionSet
 	 */
-	public function selectEmptyForUser(int $uid): Entity\PermissionSet
+	public function selectPublic(int $uid): Entity\PermissionSet
 	{
-		return $this->selectOrCreate($this->factory->createFromString($uid));
+		return $this->factory->createFromString($uid, '', '', '', '', self::PUBLIC);
 	}
 
 	/**
@@ -198,6 +220,11 @@ class PermissionSet extends BaseDepository
 			return $permissionSet;
 		}
 
+		// Don't select/update Public permission sets
+		if ($this->checkPublic($permissionSet)) {
+			return $this->selectPublic($permissionSet->uid);
+		}
+
 		try {
 			return $this->selectOne($this->convertToTableRow($permissionSet));
 		} catch (NotFoundException $exception) {
@@ -205,8 +232,19 @@ class PermissionSet extends BaseDepository
 		}
 	}
 
+	/**
+	 * @param Entity\PermissionSet $permissionSet
+	 *
+	 * @return Entity\PermissionSet
+	 * @throws NotFoundException
+	 */
 	public function save(Entity\PermissionSet $permissionSet): Entity\PermissionSet
 	{
+		// Don't save/update the common public PermissionSet
+		if ($this->checkPublic($permissionSet)) {
+			return $this->selectPublic($permissionSet->uid);
+		}
+
 		$fields = $this->convertToTableRow($permissionSet);
 
 		if ($permissionSet->id) {
@@ -214,7 +252,7 @@ class PermissionSet extends BaseDepository
 		} else {
 			$this->db->insert(self::$table_name, $fields);
 
-			$permissionSet = $this->selectOneById($this->db->lastInsertId());
+			$permissionSet = $this->selectOneById($this->db->lastInsertId(), $permissionSet->uid);
 		}
 
 		return $permissionSet;

static/dbview.config.php

@@ -1058,7 +1058,9 @@
 			"allow_cid" => ["permissionset", "allow_cid"],
 			"allow_gid" => ["permissionset", "allow_gid"],
 			"deny_cid" => ["permissionset", "deny_cid"],
-			"deny_gid" => ["permissionset", "deny_gid"]
+			"deny_gid" => ["permissionset", "deny_gid"],
+			"created" => ["profile_field", "created"],
+			"updated" => ["profile_field", "updated"],
 		],
 		"query" => "FROM `profile_field`
 			INNER JOIN `permissionset` ON `permissionset`.`id` = `profile_field`.`psid`"

tests/src/Profile/ProfileField/Depository/ProfileFieldTest.php

@@ -0,0 +1,148 @@
+<?php
+
+namespace Friendica\Test\src\Profile\ProfileField\Depository;
+
+use Dice\Dice;
+use Friendica\Database\Database;
+use Friendica\DI;
+use Friendica\Profile\ProfileField\Depository\ProfileField as ProfileFieldDepository;
+use Friendica\Profile\ProfileField\Entity\ProfileField;
+use Friendica\Profile\ProfileField\Exception\ProfileFieldPersistenceException;
+use Friendica\Profile\ProfileField\Factory\ProfileField as ProfileFieldFactory;
+use Friendica\Security\PermissionSet\Depository\PermissionSet;
+use Friendica\Security\PermissionSet\Factory\PermissionSet as PermissionSetFactory;
+use Friendica\Security\PermissionSet\Depository\PermissionSet as PermissionSetDepository;
+use Friendica\Test\DatabaseTest;
+use Friendica\Test\Util\Database\StaticDatabase;
+
+class ProfileFieldTest extends DatabaseTest
+{
+	/** @var ProfileFieldDepository */
+	private $depository;
+	/** @var ProfileFieldFactory */
+	private $factory;
+	/** @var PermissionSetFactory */
+	private $permissionSetFactory;
+	/** @var PermissionSetDepository */
+	private $permissionSetDepository;
+	/** @var Database */
+	private $dba;
+
+	public function setUp(): void
+	{
+		parent::setUp();
+
+		$dice = (new Dice())
+			->addRules(include __DIR__ . '/../../../../../static/dependencies.config.php')
+			->addRule(Database::class, ['instanceOf' => StaticDatabase::class, 'shared' => true]);
+
+		$this->depository              = $dice->create(ProfileFieldDepository::class);
+		$this->factory                 = $dice->create(ProfileFieldFactory::class);
+		$this->permissionSetFactory    = $dice->create(PermissionSetFactory::class);
+		$this->permissionSetDepository = $dice->create(PermissionSetDepository::class);
+		$this->dba                     = $dice->create(Database::class);
+	}
+
+	/**
+	 * Test create ProfileField without a valid PermissionSet
+	 */
+	public function testSavingWithoutPermissionSet()
+	{
+		self::expectExceptionMessage('PermissionSet needs to be saved first.');
+		self::expectException(ProfileFieldPersistenceException::class);
+
+		$this->loadFixture(__DIR__ . '/../../../../datasets/api.fixture.php', DI::dba());
+
+		$profileField = $this->factory->createFromValues(42, 0, 'public', 'value', $this->permissionSetFactory->createFromString(42, '', '<~>'));
+
+		self::assertEquals($profileField->uid, $profileField->permissionSet->uid);
+
+		$this->depository->save($profileField);
+	}
+
+	/**
+	 * Test saving a new entity
+	 */
+	public function testSaveNew()
+	{
+		$this->loadFixture(__DIR__ . '/../../../../datasets/api.fixture.php', DI::dba());
+
+		$profileField = $this->factory->createFromValues(42, 0, 'public', 'value', $this->permissionSetDepository->save($this->permissionSetFactory->createFromString(42, '', '<~>')));
+
+		self::assertEquals($profileField->uid, $profileField->permissionSet->uid);
+
+		$savedProfileField = $this->depository->save($profileField);
+
+		self::assertNotNull($savedProfileField->id);
+		self::assertNull($profileField->id);
+
+		/** @var ProfileField $selectedProfileField */
+		$selectedProfileField = $this->depository->selectOneById($savedProfileField->id);
+
+		self::assertEquals($savedProfileField, $selectedProfileField);
+	}
+
+	/**
+	 * Test updating the order of a ProfileField
+	 */
+	public function testUpdateOrder()
+	{
+		$this->loadFixture(__DIR__ . '/../../../../datasets/api.fixture.php', DI::dba());
+
+		$profileField = $this->factory->createFromValues(42, 0, 'public', 'value', $this->permissionSetDepository->save($this->permissionSetFactory->createFromString(42, '', '<~>')));
+
+		self::assertEquals($profileField->uid, $profileField->permissionSet->uid);
+
+		$savedProfileField = $this->depository->save($profileField);
+
+		self::assertNotNull($savedProfileField->id);
+		self::assertNull($profileField->id);
+
+		/** @var ProfileField $selectedProfileField */
+		$selectedProfileField = $this->depository->selectOneById($savedProfileField->id);
+
+		self::assertEquals($savedProfileField, $selectedProfileField);
+
+		$selectedProfileField->setOrder(66);
+
+		$updatedOrderProfileField = $this->depository->save($selectedProfileField);
+
+		self::assertEquals($selectedProfileField->id, $updatedOrderProfileField->id);
+		self::assertEquals(66, $updatedOrderProfileField->order);
+
+		// Even using the ID of the old, saved ProfileField returns the right instance
+		$updatedFromOldProfileField = $this->depository->selectOneById($savedProfileField->id);
+		self::assertEquals(66, $updatedFromOldProfileField->order);
+	}
+
+	/**
+	 * Test updating a whole entity
+	 */
+	public function testUpdate()
+	{
+		$this->loadFixture(__DIR__ . '/../../../../datasets/api.fixture.php', DI::dba());
+
+		$profileField = $this->factory->createFromValues(42, 0, 'public', 'value', $this->permissionSetDepository->save($this->permissionSetFactory->createFromString(42, '', '<~>')));
+
+		self::assertEquals($profileField->uid, $profileField->permissionSet->uid);
+
+		$savedProfileField = $this->depository->save($profileField);
+
+		self::assertNotNull($savedProfileField->id);
+		self::assertNull($profileField->id);
+
+		/** @var ProfileField $selectedProfileField */
+		$selectedProfileField = $this->depository->selectOneById($savedProfileField->id);
+
+		self::assertEquals($savedProfileField, $selectedProfileField);
+
+		$savedProfileField->update('another', 5, $this->permissionSetDepository->selectPublic(42));
+		self::assertEquals(PermissionSet::PUBLIC, $savedProfileField->permissionSet->id);
+
+		$publicProfileField = $this->depository->save($savedProfileField);
+
+		self::assertEquals($this->permissionSetDepository->selectPublic(42), $publicProfileField->permissionSet);
+		self::assertEquals('another', $publicProfileField->value);
+		self::assertEquals(5, $publicProfileField->order);
+	}
+}

tests/src/Security/PermissionSet/Depository/PermissionSetTest.php

@@ -5,36 +5,68 @@ namespace Friendica\Test\src\Security\PermissionSet\Depository;
 use Dice\Dice;
 use Friendica\Database\Database;
 use Friendica\DI;
-use Friendica\Security\PermissionSet\Depository\PermissionSet;
+use Friendica\Security\PermissionSet\Depository\PermissionSet as PermissionSetDepository;
-use Friendica\Test\MockedTest;
+use Friendica\Security\PermissionSet\Factory\PermissionSet as PermissionSetFactory;
+use Friendica\Test\DatabaseTest;
 use Friendica\Test\Util\Database\StaticDatabase;
 
-class PermissionSetTest extends MockedTest
+class PermissionSetTest extends DatabaseTest
 {
-	/** @var PermissionSet */
+	/** @var PermissionSetDepository */
 	private $depository;
+	/** @var PermissionSetFactory */
+	private $factory;
 
 	public function setUp(): void
 	{
+		parent::setUp();
+
 		$dice = (new Dice())
 			->addRules(include __DIR__ . '/../../../../../static/dependencies.config.php')
 			->addRule(Database::class, ['instanceOf' => StaticDatabase::class, 'shared' => true]);
-		DI::init($dice);
 
-		$this->depository = DI::permissionSet();
+		$this->depository = $dice->create(PermissionSetDepository::class);
-	}
+		$this->factory    = $dice->create(PermissionSetFactory::class);
-
-	public function testSelectOneByIdPublicMissingUid()
-	{
-		$this->expectException(\InvalidArgumentException::class);
-
-		$this->depository->selectOneById(PermissionSet::PUBLIC);
 	}
 
 	public function testSelectOneByIdPublic()
 	{
-		$permissionSet = $this->depository->selectOneById(PermissionSet::PUBLIC, 1);
+		$permissionSet = $this->depository->selectPublic(1);
 
 		$this->assertInstanceOf(\Friendica\Security\PermissionSet\Entity\PermissionSet::class, $permissionSet);
+		self::assertEmpty($permissionSet->allow_cid);
+		self::assertEmpty($permissionSet->allow_gid);
+		self::assertEmpty($permissionSet->deny_cid);
+		self::assertEmpty($permissionSet->deny_gid);
+		self::assertEmpty(PermissionSetDepository::PUBLIC, $permissionSet->id);
+		self::assertEquals(1, $permissionSet->uid);
+	}
+
+	/**
+	 * Test create/update PermissionSets
+	 */
+	public function testSaving()
+	{
+		$this->loadFixture(__DIR__ . '/../../../../datasets/api.fixture.php', DI::dba());
+
+		$permissionSet = $this->factory->createFromString(42, '', '<~>');
+
+		$permissionSet = $this->depository->selectOrCreate($permissionSet);
+
+		self::assertNotNull($permissionSet->id);
+
+		$permissionSetSelected = $this->depository->selectOneById($permissionSet->id, 42);
+
+		self::assertEquals($permissionSet, $permissionSetSelected);
+
+		$newPermissionSet   = $permissionSet->withAllowedContacts(['1', '2']);
+		$savedPermissionSet = $this->depository->save($newPermissionSet);
+
+		self::assertNotNull($savedPermissionSet->id);
+		self::assertNull($newPermissionSet->id);
+
+		$permissionSetSavedSelected = $this->depository->selectOneById($savedPermissionSet->id, 42);
+
+		self::assertEquals($savedPermissionSet, $permissionSetSavedSelected);
 	}
 }