Merge pull request #4452 from nextcloud/enh/perf

This commit is contained in:
Julius Härtl
2023-02-20 16:46:02 +01:00
committed by GitHub
31 changed files with 424 additions and 288 deletions

View File

@@ -74,11 +74,12 @@ jobs:
uses: shivammathur/setup-php@2.24.0 uses: shivammathur/setup-php@2.24.0
with: with:
php-version: ${{ matrix.php-versions }} php-version: ${{ matrix.php-versions }}
tools: phpunit extensions: mbstring, iconv, fileinfo, intl, sqlite, pdo_sqlite, mysql, pdo_mysql, pgsql, pdo_pgsql, apcu
extensions: mbstring, iconv, fileinfo, intl, sqlite, pdo_sqlite, mysql, pdo_mysql, pgsql, pdo_pgsql, ini-values:
apc.enable_cli=on
coverage: none coverage: none
- name: Set up PHPUnit - name: Set up dependencies
working-directory: apps/${{ env.APP_NAME }} working-directory: apps/${{ env.APP_NAME }}
run: composer i --no-dev run: composer i --no-dev
@@ -91,11 +92,63 @@ jobs:
fi fi
mkdir data mkdir data
./occ maintenance:install --verbose --database=${{ matrix.databases }} --database-name=nextcloud --database-host=127.0.0.1 --database-port=$DB_PORT --database-user=root --database-pass=rootpassword --admin-user admin --admin-pass admin ./occ maintenance:install --verbose --database=${{ matrix.databases }} --database-name=nextcloud --database-host=127.0.0.1 --database-port=$DB_PORT --database-user=root --database-pass=rootpassword --admin-user admin --admin-pass admin
./occ config:system:set hashing_default_password --value=true --type=boolean
./occ config:system:set memcache.local --value="\\OC\\Memcache\\APCu"
./occ config:system:set memcache.distributed --value="\\OC\\Memcache\\APCu"
cat config/config.php cat config/config.php
./occ user:list ./occ user:list
./occ app:enable --force ${{ env.APP_NAME }} ./occ app:enable --force ${{ env.APP_NAME }}
./occ config:system:set query_log_file --value '/home/runner/work/${{ env.APP_NAME }}/${{ env.APP_NAME }}/query.log'
php -S localhost:8080 & php -S localhost:8080 &
- name: Run behat - name: Run behat
working-directory: apps/${{ env.APP_NAME }}/tests/integration working-directory: apps/${{ env.APP_NAME }}/tests/integration
run: ./run.sh run: ./run.sh
- name: Query count
if: ${{ matrix.databases == 'mysql' }}
uses: actions/github-script@v5
with:
github-token: ${{secrets.GITHUB_TOKEN}}
script: |
let myOutput = ''
let myError = ''
const options = {}
options.listeners = {
stdout: (data) => {
myOutput += data.toString()
},
stderr: (data) => {
myError += data.toString()
}
}
await exec.exec(`/bin/bash -c "cat /home/runner/work/${{ env.APP_NAME }}/${{ env.APP_NAME }}/query.log | wc -l"`, [], options)
msg = myOutput
const queryCount = parseInt(myOutput, 10)
myOutput = ''
await exec.exec('cat', ['/home/runner/work/${{ env.APP_NAME }}/${{ env.APP_NAME }}/apps/${{ env.APP_NAME }}/tests/integration/base-query-count.txt'], options)
const baseCount = parseInt(myOutput, 10)
const absoluteIncrease = queryCount - baseCount
const relativeIncrease = baseCount <= 0 ? 100 : (parseInt((absoluteIncrease / baseCount * 10000), 10) / 100)
if (absoluteIncrease >= 100 || relativeIncrease > 5) {
const comment = `🐢 Performance warning.\nIt looks like the query count of the integration tests increased with this PR.\nDatabase query count is now ` + queryCount + ' was ' + baseCount + ' (+' + relativeIncrease + '%)\nPlease check your code again. If you added a new test this can be expected and the base value in tests/integration/base-query-count.txt can be increased.'
github.rest.issues.createComment({
issue_number: context.issue.number,
owner: context.repo.owner,
repo: context.repo.repo,
body: comment
})
}
if (queryCount < 100) {
const comment = `🐈 Performance messuring seems broken. Failed to get query count.`
github.rest.issues.createComment({
issue_number: context.issue.number,
owner: context.repo.owner,
repo: context.repo.repo,
body: comment
})
}

View File

@@ -65,7 +65,7 @@ class BoardApiController extends ApiController {
public function index($details = null) { public function index($details = null) {
$modified = $this->request->getHeader('If-Modified-Since'); $modified = $this->request->getHeader('If-Modified-Since');
if ($modified === null || $modified === '') { if ($modified === null || $modified === '') {
$boards = $this->boardService->findAll(0, $details); $boards = $this->boardService->findAll(0, $details === true);
} else { } else {
$date = Util::parseHTTPDate($modified); $date = Util::parseHTTPDate($modified);
if (!$date) { if (!$date) {

View File

@@ -59,7 +59,7 @@ class DeckCalendarBackend {
} }
public function getBoards(): array { public function getBoards(): array {
return $this->boardService->findAll(-1, null, false); return $this->boardService->findAll(-1, false, false);
} }
public function getBoard(int $id): Board { public function getBoard(int $id): Board {

View File

@@ -52,6 +52,20 @@ class AclMapper extends DeckMapper implements IPermissionMapper {
return $this->findEntities($qb); return $this->findEntities($qb);
} }
public function findIn(array $boardIds, ?int $limit = null, ?int $offset = null): array {
$qb = $this->db->getQueryBuilder();
$qb->select('id', 'board_id', 'type', 'participant', 'permission_edit', 'permission_share', 'permission_manage')
->from('deck_board_acl')
->where($qb->expr()->in('board_id', $qb->createParameter('boardIds')))
->setMaxResults($limit)
->setFirstResult($offset);
return iterator_to_array($this->chunkQuery($boardIds, function (array $ids) use ($qb) {
$qb->setParameter('boardIds', $ids, IQueryBuilder::PARAM_INT_ARRAY);
return $this->findEntities($qb);
}));
}
/** /**
* @param numeric $userId * @param numeric $userId
* @param numeric $id * @param numeric $id

View File

@@ -28,15 +28,13 @@ namespace OCA\Deck\Db;
use OCA\Deck\NotFoundException; use OCA\Deck\NotFoundException;
use OCA\Deck\Service\CirclesService; use OCA\Deck\Service\CirclesService;
use OCP\AppFramework\Db\Entity; use OCP\AppFramework\Db\Entity;
use OCP\AppFramework\Db\QBMapper;
use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IDBConnection; use OCP\IDBConnection;
use OCP\IGroupManager; use OCP\IGroupManager;
use OCP\IUserManager; use OCP\IUserManager;
use PDO;
/** @template-extends QBMapper<Assignment> */ /** @template-extends DeckMapper<Assignment> */
class AssignmentMapper extends QBMapper implements IPermissionMapper { class AssignmentMapper extends DeckMapper implements IPermissionMapper {
/** @var CardMapper */ /** @var CardMapper */
private $cardMapper; private $cardMapper;
@@ -60,7 +58,7 @@ class AssignmentMapper extends QBMapper implements IPermissionMapper {
$qb = $this->db->getQueryBuilder(); $qb = $this->db->getQueryBuilder();
$qb->select('*') $qb->select('*')
->from('deck_assigned_users') ->from('deck_assigned_users')
->where($qb->expr()->eq('card_id', $qb->createNamedParameter($cardId, PDO::PARAM_INT))); ->where($qb->expr()->eq('card_id', $qb->createNamedParameter($cardId, IQueryBuilder::PARAM_INT)));
$users = $this->findEntities($qb); $users = $this->findEntities($qb);
foreach ($users as $user) { foreach ($users as $user) {
$this->mapParticipant($user); $this->mapParticipant($user);
@@ -68,12 +66,29 @@ class AssignmentMapper extends QBMapper implements IPermissionMapper {
return $users; return $users;
} }
public function findIn(array $cardIds): array {
$qb = $this->db->getQueryBuilder();
$qb->select('*')
->from('deck_assigned_users')
->where($qb->expr()->in('card_id', $qb->createParameter('cardIds')));
$users = iterator_to_array($this->chunkQuery($cardIds, function (array $ids) use ($qb) {
$qb->setParameter('cardIds', $ids, IQueryBuilder::PARAM_INT_ARRAY);
return $this->findEntities($qb);
}));
foreach ($users as $user) {
$this->mapParticipant($user);
}
return $users;
}
public function findByParticipant(string $participant, $type = Assignment::TYPE_USER): array { public function findByParticipant(string $participant, $type = Assignment::TYPE_USER): array {
$qb = $this->db->getQueryBuilder(); $qb = $this->db->getQueryBuilder();
$qb->select('*') $qb->select('*')
->from('deck_assigned_users') ->from('deck_assigned_users')
->where($qb->expr()->eq('participant', $qb->createNamedParameter($participant, PDO::PARAM_STR))) ->where($qb->expr()->eq('participant', $qb->createNamedParameter($participant, IQueryBuilder::PARAM_STR)))
->andWhere($qb->expr()->eq('type', $qb->createNamedParameter($type, PDO::PARAM_INT))); ->andWhere($qb->expr()->eq('type', $qb->createNamedParameter($type, IQueryBuilder::PARAM_INT)));
return $this->findEntities($qb); return $this->findEntities($qb);
} }
@@ -132,8 +147,8 @@ class AssignmentMapper extends QBMapper implements IPermissionMapper {
private function getOrigin(Assignment $assignment) { private function getOrigin(Assignment $assignment) {
if ($assignment->getType() === Assignment::TYPE_USER) { if ($assignment->getType() === Assignment::TYPE_USER) {
$origin = $this->userManager->get($assignment->getParticipant()); $origin = $this->userManager->userExists($assignment->getParticipant());
return $origin ? new User($origin) : null; return $origin ? new User($assignment->getParticipant(), $this->userManager) : null;
} }
if ($assignment->getType() === Assignment::TYPE_GROUP) { if ($assignment->getType() === Assignment::TYPE_GROUP) {
$origin = $this->groupManager->get($assignment->getParticipant()); $origin = $this->groupManager->get($assignment->getParticipant());

View File

@@ -90,9 +90,6 @@ class BoardMapper extends QBMapper implements IPermissionMapper {
$this->boardCache[$id] = $this->findEntity($qb); $this->boardCache[$id] = $this->findEntity($qb);
} }
// FIXME is this necessary? it was NOT done with the old mapper
// $this->mapOwner($board);
// Add labels // Add labels
if ($withLabels && $this->boardCache[$id]->getLabels() === null) { if ($withLabels && $this->boardCache[$id]->getLabels() === null) {
$labels = $this->labelMapper->findAll($id); $labels = $this->labelMapper->findAll($id);
@@ -160,6 +157,20 @@ class BoardMapper extends QBMapper implements IPermissionMapper {
$groupBoards = $this->findAllByGroups($userId, $groups, null, null, $since, $includeArchived, $before, $term); $groupBoards = $this->findAllByGroups($userId, $groups, null, null, $since, $includeArchived, $before, $term);
$circleBoards = $this->findAllByCircles($userId, null, null, $since, $includeArchived, $before, $term); $circleBoards = $this->findAllByCircles($userId, null, null, $since, $includeArchived, $before, $term);
$allBoards = array_unique(array_merge($userBoards, $groupBoards, $circleBoards)); $allBoards = array_unique(array_merge($userBoards, $groupBoards, $circleBoards));
// Could be moved outside
$acls = $this->aclMapper->findIn(array_map(function ($board) {
return $board->getId();
}, $allBoards));
/* @var Board $entry */
foreach ($allBoards as $entry) {
$boardAcls = array_values(array_filter($acls, function ($acl) use ($entry) {
return $acl->getBoardId() === $entry->getId();
}));
$entry->setAcl($boardAcls);
}
foreach ($allBoards as $board) { foreach ($allBoards as $board) {
$this->boardCache[$board->getId()] = $board; $this->boardCache[$board->getId()] = $board;
} }
@@ -259,11 +270,7 @@ class BoardMapper extends QBMapper implements IPermissionMapper {
$entry->setShared(1); $entry->setShared(1);
} }
$entries = array_merge($entries, $sharedEntries); $entries = array_merge($entries, $sharedEntries);
/* @var Board $entry */
foreach ($entries as $entry) {
$acl = $this->aclMapper->findAll($entry->id);
$entry->setAcl($acl);
}
return $entries; return $entries;
} }
@@ -336,11 +343,6 @@ class BoardMapper extends QBMapper implements IPermissionMapper {
foreach ($entries as $entry) { foreach ($entries as $entry) {
$entry->setShared(2); $entry->setShared(2);
} }
/* @var Board $entry */
foreach ($entries as $entry) {
$acl = $this->aclMapper->findAll($entry->id);
$entry->setAcl($acl);
}
return $entries; return $entries;
} }
@@ -397,11 +399,6 @@ class BoardMapper extends QBMapper implements IPermissionMapper {
foreach ($entries as $entry) { foreach ($entries as $entry) {
$entry->setShared(2); $entry->setShared(2);
} }
/* @var Board $entry */
foreach ($entries as $entry) {
$acl = $this->aclMapper->findAll($entry->id);
$entry->setAcl($acl);
}
return $entries; return $entries;
} }
@@ -455,13 +452,11 @@ class BoardMapper extends QBMapper implements IPermissionMapper {
} }
public function mapAcl(Acl &$acl) { public function mapAcl(Acl &$acl) {
$userManager = $this->userManager;
$groupManager = $this->groupManager; $groupManager = $this->groupManager;
$acl->resolveRelation('participant', function ($participant) use (&$acl, &$userManager, &$groupManager) { $acl->resolveRelation('participant', function ($participant) use (&$acl, &$userManager, &$groupManager) {
if ($acl->getType() === Acl::PERMISSION_TYPE_USER) { if ($acl->getType() === Acl::PERMISSION_TYPE_USER) {
$user = $userManager->get($participant); if ($this->userManager->userExists($acl->getParticipant())) {
if ($user !== null) { return new User($acl->getParticipant(), $this->userManager);
return new User($user);
} }
$this->logger->debug('User ' . $acl->getId() . ' not found when mapping acl ' . $acl->getParticipant()); $this->logger->debug('User ' . $acl->getId() . ' not found when mapping acl ' . $acl->getParticipant());
return null; return null;
@@ -499,9 +494,8 @@ class BoardMapper extends QBMapper implements IPermissionMapper {
public function mapOwner(Board &$board) { public function mapOwner(Board &$board) {
$userManager = $this->userManager; $userManager = $this->userManager;
$board->resolveRelation('owner', function ($owner) use (&$userManager) { $board->resolveRelation('owner', function ($owner) use (&$userManager) {
$user = $userManager->get($owner); if ($this->userManager->userExists($owner)) {
if ($user !== null) { return new User($owner, $userManager);
return new User($user);
} }
return null; return null;
}); });

View File

@@ -254,13 +254,13 @@ class CardMapper extends QBMapper implements IPermissionMapper {
return $this->findEntities($qb); return $this->findEntities($qb);
} }
public function findAllWithDue($boardId) { public function findAllWithDue(array $boardIds) {
$qb = $this->db->getQueryBuilder(); $qb = $this->db->getQueryBuilder();
$qb->select('c.*') $qb->select('c.*')
->from('deck_cards', 'c') ->from('deck_cards', 'c')
->innerJoin('c', 'deck_stacks', 's', 's.id = c.stack_id') ->innerJoin('c', 'deck_stacks', 's', 's.id = c.stack_id')
->innerJoin('s', 'deck_boards', 'b', 'b.id = s.board_id') ->innerJoin('s', 'deck_boards', 'b', 'b.id = s.board_id')
->where($qb->expr()->eq('s.board_id', $qb->createNamedParameter($boardId, IQueryBuilder::PARAM_INT))) ->where($qb->expr()->in('s.board_id', $qb->createNamedParameter($boardIds, IQueryBuilder::PARAM_INT_ARRAY)))
->andWhere($qb->expr()->isNotNull('c.duedate')) ->andWhere($qb->expr()->isNotNull('c.duedate'))
->andWhere($qb->expr()->eq('c.archived', $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL))) ->andWhere($qb->expr()->eq('c.archived', $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL)))
->andWhere($qb->expr()->eq('c.deleted_at', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))) ->andWhere($qb->expr()->eq('c.deleted_at', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)))
@@ -270,14 +270,14 @@ class CardMapper extends QBMapper implements IPermissionMapper {
return $this->findEntities($qb); return $this->findEntities($qb);
} }
public function findToMeOrNotAssignedCards($boardId, $username) { public function findToMeOrNotAssignedCards(array $boardIds, string $username) {
$qb = $this->db->getQueryBuilder(); $qb = $this->db->getQueryBuilder();
$qb->select('c.*') $qb->select('c.*')
->from('deck_cards', 'c') ->from('deck_cards', 'c')
->innerJoin('c', 'deck_stacks', 's', 's.id = c.stack_id') ->innerJoin('c', 'deck_stacks', 's', 's.id = c.stack_id')
->innerJoin('s', 'deck_boards', 'b', 'b.id = s.board_id') ->innerJoin('s', 'deck_boards', 'b', 'b.id = s.board_id')
->leftJoin('c', 'deck_assigned_users', 'u', 'c.id = u.card_id') ->leftJoin('c', 'deck_assigned_users', 'u', 'c.id = u.card_id')
->where($qb->expr()->eq('s.board_id', $qb->createNamedParameter($boardId, IQueryBuilder::PARAM_INT))) ->where($qb->expr()->in('s.board_id', $qb->createNamedParameter($boardIds, IQueryBuilder::PARAM_INT_ARRAY)))
->andWhere($qb->expr()->orX( ->andWhere($qb->expr()->orX(
$qb->expr()->eq('u.participant', $qb->createNamedParameter($username, IQueryBuilder::PARAM_STR)), $qb->expr()->eq('u.participant', $qb->createNamedParameter($username, IQueryBuilder::PARAM_STR)),
$qb->expr()->isNull('u.participant')) $qb->expr()->isNull('u.participant'))
@@ -607,9 +607,8 @@ class CardMapper extends QBMapper implements IPermissionMapper {
public function mapOwner(Card &$card) { public function mapOwner(Card &$card) {
$userManager = $this->userManager; $userManager = $this->userManager;
$card->resolveRelation('owner', function ($owner) use (&$userManager) { $card->resolveRelation('owner', function ($owner) use (&$userManager) {
$user = $userManager->get($owner); if ($userManager->userExists($owner)) {
if ($user !== null) { return new User($owner, $this->userManager);
return new User($user);
} }
return null; return null;
}); });

View File

@@ -23,6 +23,7 @@
namespace OCA\Deck\Db; namespace OCA\Deck\Db;
use Generator;
use OCP\AppFramework\Db\Entity; use OCP\AppFramework\Db\Entity;
use OCP\AppFramework\Db\QBMapper; use OCP\AppFramework\Db\QBMapper;
use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\DB\QueryBuilder\IQueryBuilder;
@@ -35,7 +36,7 @@ abstract class DeckMapper extends QBMapper {
/** /**
* @param $id * @param $id
* @return \OCP\AppFramework\Db\Entity if not found * @return T
* @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
* @throws \OCP\AppFramework\Db\DoesNotExistException * @throws \OCP\AppFramework\Db\DoesNotExistException
*/ */
@@ -47,4 +48,21 @@ abstract class DeckMapper extends QBMapper {
return $this->findEntity($qb); return $this->findEntity($qb);
} }
/**
* Helper function to split passed array into chunks of 1000 elements and
* call a given callback for fetching query results
*
* Can be useful to limit to 1000 results per query for oracle compatiblity
* but still iterate over all results
*/
public function chunkQuery(array $ids, callable $callback): Generator {
$limit = 1000;
while (!empty($ids)) {
$slice = array_splice($ids, 0, $limit);
foreach ($callback($slice) as $item) {
yield $item;
}
}
}
} }

View File

@@ -79,6 +79,19 @@ class LabelMapper extends DeckMapper implements IPermissionMapper {
return $this->findEntities($qb); return $this->findEntities($qb);
} }
public function findAssignedLabelsForCards($cardIds, $limit = null, $offset = null): array {
$qb = $this->db->getQueryBuilder();
$qb->select('l.*', 'card_id')
->from($this->getTableName(), 'l')
->innerJoin('l', 'deck_assigned_labels', 'al', 'l.id = al.label_id')
->where($qb->expr()->in('card_id', $qb->createNamedParameter($cardIds, IQueryBuilder::PARAM_INT_ARRAY)))
->orderBy('l.id')
->setMaxResults($limit)
->setFirstResult($offset);
return $this->findEntities($qb);
}
/** /**
* @param numeric $boardId * @param numeric $boardId
* @param int|null $limit * @param int|null $limit

View File

@@ -33,7 +33,7 @@ class RelationalObject implements JsonSerializable {
* RelationalObject constructor. * RelationalObject constructor.
* *
* @param $primaryKey string * @param $primaryKey string
* @param $object * @param callable|mixed $object
*/ */
public function __construct($primaryKey, $object) { public function __construct($primaryKey, $object) {
$this->primaryKey = $primaryKey; $this->primaryKey = $primaryKey;
@@ -47,16 +47,24 @@ class RelationalObject implements JsonSerializable {
); );
} }
public function getObject() {
if (is_callable($this->object)) {
$this->object = call_user_func($this->object, $this);
}
return $this->object;
}
/** /**
* This method should be overwritten if object doesn't implement \JsonSerializable * This method should be overwritten if object doesn't implement \JsonSerializable
* *
* @throws \Exception * @throws \Exception
*/ */
public function getObjectSerialization() { public function getObjectSerialization() {
if ($this->object instanceof JsonSerializable) { if ($this->getObject() instanceof JsonSerializable) {
return $this->object->jsonSerialize(); return $this->getObject()->jsonSerialize();
} else { } else {
throw new \Exception('jsonSerialize is not implemented on ' . get_class($this)); throw new \Exception('jsonSerialize is not implemented on ' . get_class($this->getObject()));
} }
} }

View File

@@ -26,16 +26,19 @@ namespace OCA\Deck\Db;
use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\DoesNotExistException;
use OCP\AppFramework\Db\Entity; use OCP\AppFramework\Db\Entity;
use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\AppFramework\Db\MultipleObjectsReturnedException;
use OCP\Cache\CappedMemoryCache;
use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IDBConnection; use OCP\IDBConnection;
/** @template-extends DeckMapper<Stack> */ /** @template-extends DeckMapper<Stack> */
class StackMapper extends DeckMapper implements IPermissionMapper { class StackMapper extends DeckMapper implements IPermissionMapper {
private $cardMapper; private CappedMemoryCache $stackCache;
private CardMapper $cardMapper;
public function __construct(IDBConnection $db, CardMapper $cardMapper) { public function __construct(IDBConnection $db, CardMapper $cardMapper) {
parent::__construct($db, 'deck_stacks', Stack::class); parent::__construct($db, 'deck_stacks', Stack::class);
$this->cardMapper = $cardMapper; $this->cardMapper = $cardMapper;
$this->stackCache = new CappedMemoryCache();
} }
@@ -47,12 +50,17 @@ class StackMapper extends DeckMapper implements IPermissionMapper {
* @throws \OCP\DB\Exception * @throws \OCP\DB\Exception
*/ */
public function find($id): Stack { public function find($id): Stack {
if (isset($this->stackCache[(string)$id])) {
return $this->stackCache[(string)$id];
}
$qb = $this->db->getQueryBuilder(); $qb = $this->db->getQueryBuilder();
$qb->select('*') $qb->select('*')
->from($this->getTableName()) ->from($this->getTableName())
->where($qb->expr()->eq('id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT))); ->where($qb->expr()->eq('id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT)));
return $this->findEntity($qb); $this->stackCache[(string)$id] = $this->findEntity($qb);
return $this->stackCache[(string)$id];
} }
/** /**
@@ -113,9 +121,16 @@ class StackMapper extends DeckMapper implements IPermissionMapper {
return $this->findEntities($qb); return $this->findEntities($qb);
} }
public function update(Entity $entity): Entity {
$result = parent::update($entity);
$this->stackCache[(string)$entity->getId()] = $result;
return $result;
}
public function delete(Entity $entity): Entity { public function delete(Entity $entity): Entity {
// delete cards on stack // delete cards on stack
$this->cardMapper->deleteByStack($entity->getId()); $this->cardMapper->deleteByStack($entity->getId());
unset($this->stackCache[(string)$entity->getId()]);
return parent::delete($entity); return parent::delete($entity);
} }

View File

@@ -23,27 +23,30 @@
namespace OCA\Deck\Db; namespace OCA\Deck\Db;
use OCP\IUser; use OCP\IUserManager;
class User extends RelationalObject { class User extends RelationalObject {
public function __construct(IUser $user) { private IUserManager $userManager;
$primaryKey = $user->getUID(); public function __construct($uid, IUserManager $userManager) {
parent::__construct($primaryKey, $user); $this->userManager = $userManager;
parent::__construct($uid, function ($object) {
return $this->userManager->get($object->getPrimaryKey());
});
} }
public function getObjectSerialization() { public function getObjectSerialization() {
return [ return [
'uid' => $this->object->getUID(), 'uid' => $this->getObject()->getUID(),
'displayname' => $this->object->getDisplayName(), 'displayname' => $this->getObject()->getDisplayName(),
'type' => 0 'type' => Acl::PERMISSION_TYPE_USER
]; ];
} }
public function getUID() { public function getUID() {
return $this->object->getUID(); return $this->getPrimaryKey();
} }
public function getDisplayName() { public function getDisplayName() {
return $this->object->getDisplayName(); return $this->userManager->getDisplayName($this->getPrimaryKey());
} }
} }

View File

@@ -41,4 +41,8 @@ abstract class AAclEvent extends Event {
public function getAcl(): Acl { public function getAcl(): Acl {
return $this->acl; return $this->acl;
} }
public function getBoardId(): int {
return $this->acl->getBoardId();
}
} }

View File

@@ -79,7 +79,8 @@ class BoardService {
private IEventDispatcher $eventDispatcher; private IEventDispatcher $eventDispatcher;
private ChangeHelper $changeHelper; private ChangeHelper $changeHelper;
private CardMapper $cardMapper; private CardMapper $cardMapper;
private ?array $boardsCache = null; private ?array $boardsCacheFull = null;
private ?array $boardsCachePartial = null;
private IURLGenerator $urlGenerator; private IURLGenerator $urlGenerator;
private IDBConnection $connection; private IDBConnection $connection;
private BoardServiceValidator $boardServiceValidator; private BoardServiceValidator $boardServiceValidator;
@@ -147,96 +148,45 @@ class BoardService {
} }
/** /**
* @return array * @return Board[]
*/ */
public function findAll($since = -1, $details = null, $includeArchived = true) { public function findAll(int $since = -1, bool $fullDetails = false, bool $includeArchived = true): array {
if ($this->boardsCache) { if ($this->boardsCacheFull && $fullDetails) {
return $this->boardsCache; return $this->boardsCacheFull;
} }
if ($this->boardsCachePartial && !$fullDetails) {
return $this->boardsCachePartial;
}
$complete = $this->getUserBoards($since, $includeArchived); $complete = $this->getUserBoards($since, $includeArchived);
$result = []; return $this->enrichBoards($complete, $fullDetails);
/** @var Board $item */
foreach ($complete as &$item) {
$this->boardMapper->mapOwner($item);
if ($item->getAcl() !== null) {
foreach ($item->getAcl() as &$acl) {
$this->boardMapper->mapAcl($acl);
}
}
if ($details !== null) {
$this->enrichWithStacks($item);
$this->enrichWithLabels($item);
$this->enrichWithUsers($item);
}
$permissions = $this->permissionService->matchPermissions($item);
$item->setPermissions([
'PERMISSION_READ' => $permissions[Acl::PERMISSION_READ] ?? false,
'PERMISSION_EDIT' => $permissions[Acl::PERMISSION_EDIT] ?? false,
'PERMISSION_MANAGE' => $permissions[Acl::PERMISSION_MANAGE] ?? false,
'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE] ?? false
]);
$this->enrichWithBoardSettings($item);
$result[$item->getId()] = $item;
}
$this->boardsCache = $result;
return array_values($result);
} }
/** /**
* @param $boardId
* @return Board
* @throws DoesNotExistException * @throws DoesNotExistException
* @throws \OCA\Deck\NoPermissionException * @throws \OCA\Deck\NoPermissionException
* @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
* @throws BadRequestException * @throws BadRequestException
*/ */
public function find($boardId) { public function find(int $boardId, bool $fullDetails = true): Board {
$this->boardServiceValidator->check(compact('boardId')); $this->boardServiceValidator->check(compact('boardId'));
if ($this->boardsCache && isset($this->boardsCache[$boardId])) {
return $this->boardsCache[$boardId]; if (isset($this->boardsCacheFull[$boardId]) && $fullDetails) {
return $this->boardsCacheFull[$boardId];
} }
if (is_numeric($boardId) === false) {
throw new BadRequestException('board id must be a number'); if (isset($this->boardsCachePartial[$boardId]) && !$fullDetails) {
return $this->boardsCachePartial[$boardId];
} }
$this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_READ); $this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_READ);
/** @var Board $board */ /** @var Board $board */
$board = $this->boardMapper->find($boardId, true, true); $board = $this->boardMapper->find($boardId, true, true);
$this->boardMapper->mapOwner($board); [$board] = $this->enrichBoards([$board], $fullDetails);
if ($board->getAcl() !== null) {
foreach ($board->getAcl() as $acl) {
if ($acl !== null) {
$this->boardMapper->mapAcl($acl);
}
}
}
$permissions = $this->permissionService->matchPermissions($board);
$board->setPermissions([
'PERMISSION_READ' => $permissions[Acl::PERMISSION_READ] ?? false,
'PERMISSION_EDIT' => $permissions[Acl::PERMISSION_EDIT] ?? false,
'PERMISSION_MANAGE' => $permissions[Acl::PERMISSION_MANAGE] ?? false,
'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE] ?? false
]);
$this->enrichWithUsers($board);
$this->enrichWithBoardSettings($board);
$this->enrichWithActiveSessions($board);
$this->boardsCache[$board->getId()] = $board;
return $board; return $board;
} }
/**
* @return array
*/
private function getBoardPrerequisites() {
$groups = $this->groupManager->getUserGroupIds(
$this->userManager->get($this->userId)
);
return [
'user' => $this->userId,
'groups' => $groups
];
}
/** /**
* @param $mapper * @param $mapper
* @param $id * @param $id
@@ -691,6 +641,45 @@ class BoardService {
return $board; return $board;
} }
/** @param Board[] $boards */
private function enrichBoards(array $boards, bool $fullDetails = true): array {
$result = [];
foreach ($boards as $board) {
// FIXME The enrichment in here could make use of combined queries
$this->boardMapper->mapOwner($board);
if ($board->getAcl() !== null) {
foreach ($board->getAcl() as &$acl) {
$this->boardMapper->mapAcl($acl);
}
}
$permissions = $this->permissionService->matchPermissions($board);
$board->setPermissions([
'PERMISSION_READ' => $permissions[Acl::PERMISSION_READ] ?? false,
'PERMISSION_EDIT' => $permissions[Acl::PERMISSION_EDIT] ?? false,
'PERMISSION_MANAGE' => $permissions[Acl::PERMISSION_MANAGE] ?? false,
'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE] ?? false
]);
if ($fullDetails) {
$this->enrichWithStacks($board);
$this->enrichWithLabels($board);
$this->enrichWithUsers($board);
$this->enrichWithBoardSettings($board);
$this->enrichWithActiveSessions($board);
}
// Cache for further usage
if ($fullDetails) {
$this->boardsCacheFull[$board->getId()] = $board;
} else {
$this->boardsCachePartial[$board->getId()] = $board;
}
}
return $boards;
}
private function enrichWithStacks($board, $since = -1) { private function enrichWithStacks($board, $since = -1) {
$stacks = $this->stackMapper->findAll($board->getId(), null, null, $since); $stacks = $this->stackMapper->findAll($board->getId(), null, null, $since);
@@ -713,7 +702,7 @@ class BoardService {
private function enrichWithUsers($board, $since = -1) { private function enrichWithUsers($board, $since = -1) {
$boardUsers = $this->permissionService->findUsers($board->getId()); $boardUsers = $this->permissionService->findUsers($board->getId());
if (\count($boardUsers) === 0) { if ($boardUsers === null || \count($boardUsers) === 0) {
return; return;
} }
$board->setUsers(array_values($boardUsers)); $board->setUsers(array_values($boardUsers));
@@ -723,10 +712,6 @@ class BoardService {
return $this->urlGenerator->linkToRouteAbsolute('deck.page.index') . '#' . $endpoint; return $this->urlGenerator->linkToRouteAbsolute('deck.page.index') . '#' . $endpoint;
} }
private function clearBoardsCache() {
$this->boardsCache = null;
}
/** /**
* Clean a given board data from the Cache * Clean a given board data from the Cache
*/ */
@@ -735,7 +720,8 @@ class BoardService {
$boardOwnerId = $board->getOwner(); $boardOwnerId = $board->getOwner();
$this->boardMapper->flushCache($boardId, $boardOwnerId); $this->boardMapper->flushCache($boardId, $boardOwnerId);
unset($this->boardsCache[$boardId]); unset($this->boardsCacheFull[$boardId]);
unset($this->boardsCachePartial[$boardId]);
} }
private function enrichWithCards($board) { private function enrichWithCards($board) {

View File

@@ -28,11 +28,13 @@ namespace OCA\Deck\Service;
use OCA\Deck\Activity\ActivityManager; use OCA\Deck\Activity\ActivityManager;
use OCA\Deck\Activity\ChangeSet; use OCA\Deck\Activity\ChangeSet;
use OCA\Deck\Db\Assignment;
use OCA\Deck\Db\AssignmentMapper; use OCA\Deck\Db\AssignmentMapper;
use OCA\Deck\Db\Card; use OCA\Deck\Db\Card;
use OCA\Deck\Db\CardMapper; use OCA\Deck\Db\CardMapper;
use OCA\Deck\Db\Acl; use OCA\Deck\Db\Acl;
use OCA\Deck\Db\ChangeHelper; use OCA\Deck\Db\ChangeHelper;
use OCA\Deck\Db\Label;
use OCA\Deck\Db\StackMapper; use OCA\Deck\Db\StackMapper;
use OCA\Deck\Event\CardCreatedEvent; use OCA\Deck\Event\CardCreatedEvent;
use OCA\Deck\Event\CardDeletedEvent; use OCA\Deck\Event\CardDeletedEvent;
@@ -114,32 +116,52 @@ class CardService {
$this->cardServiceValidator = $cardServiceValidator; $this->cardServiceValidator = $cardServiceValidator;
} }
public function enrich($card) { public function enrichCards($cards) {
$user = $this->userManager->get($this->currentUser);
$cardIds = array_map(function (Card $card) use ($user) {
// Everything done in here might be heavy as it is executed for every card
$cardId = $card->getId(); $cardId = $card->getId();
$this->cardMapper->mapOwner($card); $this->cardMapper->mapOwner($card);
$card->setAssignedUsers($this->assignedUsersMapper->findAll($cardId));
$card->setLabels($this->labelMapper->findAssignedLabelsForCard($cardId));
$card->setAttachmentCount($this->attachmentService->count($cardId)); $card->setAttachmentCount($this->attachmentService->count($cardId));
$user = $this->userManager->get($this->currentUser);
$lastRead = $this->commentsManager->getReadMark('deckCard', (string)$card->getId(), $user); // TODO We should find a better way just to get the comment count so we can save 1-3 queries per card here
$countUnreadComments = $this->commentsManager->getNumberOfCommentsForObject('deckCard', (string)$card->getId(), $lastRead);
$countComments = $this->commentsManager->getNumberOfCommentsForObject('deckCard', (string)$card->getId()); $countComments = $this->commentsManager->getNumberOfCommentsForObject('deckCard', (string)$card->getId());
$lastRead = $countComments > 0 ? $this->commentsManager->getReadMark('deckCard', (string)$card->getId(), $user) : null;
$countUnreadComments = $lastRead ? $this->commentsManager->getNumberOfCommentsForObject('deckCard', (string)$card->getId(), $lastRead) : 0;
$card->setCommentsUnread($countUnreadComments); $card->setCommentsUnread($countUnreadComments);
$card->setCommentsCount($countComments); $card->setCommentsCount($countComments);
$stack = $this->stackMapper->find($card->getStackId()); $stack = $this->stackMapper->find($card->getStackId());
$board = $this->boardService->find($stack->getBoardId()); $board = $this->boardService->find($stack->getBoardId(), false);
$card->setRelatedStack($stack); $card->setRelatedStack($stack);
$card->setRelatedBoard($board); $card->setRelatedBoard($board);
return $card->getId();
}, $cards);
$assignedLabels = $this->labelMapper->findAssignedLabelsForCards($cardIds);
$assignedUsers = $this->assignedUsersMapper->findIn($cardIds);
foreach ($cards as $card) {
$cardLabels = array_values(array_filter($assignedLabels, function (Label $label) use ($card) {
return $label->getCardId() === $card->getId();
}));
$cardAssignedUsers = array_values(array_filter($assignedUsers, function (Assignment $assignment) use ($card) {
return $assignment->getCardId() === $card->getId();
}));
$card->setLabels($cardLabels);
$card->setAssignedUsers($cardAssignedUsers);
} }
return $cards;
}
public function fetchDeleted($boardId) { public function fetchDeleted($boardId) {
$this->cardServiceValidator->check(compact('boardId')); $this->cardServiceValidator->check(compact('boardId'));
$this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_READ); $this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_READ);
$cards = $this->cardMapper->findDeleted($boardId); $cards = $this->cardMapper->findDeleted($boardId);
foreach ($cards as $card) { $this->enrichCards($cards);
$this->enrich($card);
}
return $cards; return $cards;
} }
@@ -153,16 +175,17 @@ class CardService {
public function find(int $cardId) { public function find(int $cardId) {
$this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_READ); $this->permissionService->checkPermission($this->cardMapper, $cardId, Acl::PERMISSION_READ);
$card = $this->cardMapper->find($cardId); $card = $this->cardMapper->find($cardId);
$assignedUsers = $this->assignedUsersMapper->findAll($card->getId()); [$card] = $this->enrichCards([$card]);
// Attachments are only enriched on individual card fetching
$attachments = $this->attachmentService->findAll($cardId, true); $attachments = $this->attachmentService->findAll($cardId, true);
if ($this->request->getParam('apiVersion') === '1.0') { if ($this->request->getParam('apiVersion') === '1.0') {
$attachments = array_filter($attachments, function ($attachment) { $attachments = array_filter($attachments, function ($attachment) {
return $attachment->getType() === 'deck_file'; return $attachment->getType() === 'deck_file';
}); });
} }
$card->setAssignedUsers($assignedUsers);
$card->setAttachments($attachments); $card->setAttachments($attachments);
$this->enrich($card);
return $card; return $card;
} }
@@ -174,9 +197,7 @@ class CardService {
return []; return [];
} }
$cards = $this->cardMapper->findCalendarEntries($boardId); $cards = $this->cardMapper->findCalendarEntries($boardId);
foreach ($cards as $card) { $this->enrichCards($cards);
$this->enrich($card);
}
return $cards; return $cards;
} }

View File

@@ -62,9 +62,8 @@ class DefaultBoardService {
*/ */
public function checkFirstRun($userId): bool { public function checkFirstRun($userId): bool {
$firstRun = $this->config->getUserValue($userId, Application::APP_ID, 'firstRun', 'yes'); $firstRun = $this->config->getUserValue($userId, Application::APP_ID, 'firstRun', 'yes');
$userBoards = $this->boardMapper->findAllByUser($userId);
if ($firstRun === 'yes' && count($userBoards) === 0) { if ($firstRun === 'yes') {
try { try {
$this->config->setUserValue($userId, Application::APP_ID, 'firstRun', 'no'); $this->config->setUserValue($userId, Application::APP_ID, 'firstRun', 'no');
} catch (PreConditionNotMetException $e) { } catch (PreConditionNotMetException $e) {

View File

@@ -28,7 +28,7 @@ declare(strict_types=1);
namespace OCA\Deck\Service; namespace OCA\Deck\Service;
use OCA\Deck\Db\AssignmentMapper; use OCA\Deck\Db\AssignmentMapper;
use OCA\Deck\Db\Card; use OCA\Deck\Db\Board;
use OCA\Deck\Db\CardMapper; use OCA\Deck\Db\CardMapper;
use OCA\Deck\Model\CardDetails; use OCA\Deck\Model\CardDetails;
use OCP\Comments\ICommentsManager; use OCP\Comments\ICommentsManager;
@@ -37,6 +37,7 @@ use OCA\Deck\Db\LabelMapper;
use OCP\IUserManager; use OCP\IUserManager;
class OverviewService { class OverviewService {
private CardService $cardService;
private BoardMapper $boardMapper; private BoardMapper $boardMapper;
private LabelMapper $labelMapper; private LabelMapper $labelMapper;
private CardMapper $cardMapper; private CardMapper $cardMapper;
@@ -46,6 +47,7 @@ class OverviewService {
private AttachmentService $attachmentService; private AttachmentService $attachmentService;
public function __construct( public function __construct(
CardService $cardService,
BoardMapper $boardMapper, BoardMapper $boardMapper,
LabelMapper $labelMapper, LabelMapper $labelMapper,
CardMapper $cardMapper, CardMapper $cardMapper,
@@ -54,6 +56,7 @@ class OverviewService {
ICommentsManager $commentsManager, ICommentsManager $commentsManager,
AttachmentService $attachmentService AttachmentService $attachmentService
) { ) {
$this->cardService = $cardService;
$this->boardMapper = $boardMapper; $this->boardMapper = $boardMapper;
$this->labelMapper = $labelMapper; $this->labelMapper = $labelMapper;
$this->cardMapper = $cardMapper; $this->cardMapper = $cardMapper;
@@ -63,48 +66,26 @@ class OverviewService {
$this->attachmentService = $attachmentService; $this->attachmentService = $attachmentService;
} }
public function enrich(Card $card, string $userId): void {
$cardId = $card->getId();
$this->cardMapper->mapOwner($card);
$card->setAssignedUsers($this->assignedUsersMapper->findAll($cardId));
$card->setLabels($this->labelMapper->findAssignedLabelsForCard($cardId));
$card->setAttachmentCount($this->attachmentService->count($cardId));
$user = $this->userManager->get($userId);
if ($user !== null) {
$lastRead = $this->commentsManager->getReadMark('deckCard', (string)$card->getId(), $user);
$count = $this->commentsManager->getNumberOfCommentsForObject('deckCard', (string)$card->getId(), $lastRead);
$card->setCommentsUnread($count);
}
}
public function findAllWithDue(string $userId): array {
$userBoards = $this->boardMapper->findAllForUser($userId);
$allDueCards = [];
foreach ($userBoards as $userBoard) {
$allDueCards[] = array_map(function ($card) use ($userBoard, $userId) {
$this->enrich($card, $userId);
return (new CardDetails($card, $userBoard))->jsonSerialize();
}, $this->cardMapper->findAllWithDue($userBoard->getId()));
}
return array_merge(...$allDueCards);
}
public function findUpcomingCards(string $userId): array { public function findUpcomingCards(string $userId): array {
$userBoards = $this->boardMapper->findAllForUser($userId); $userBoards = $this->boardMapper->findAllForUser($userId);
$overview = [];
foreach ($userBoards as $userBoard) {
if (count($userBoard->getAcl()) === 0) {
// private board: get cards with due date
$cards = $this->cardMapper->findAllWithDue($userBoard->getId());
} else {
// shared board: get all my assigned or unassigned cards
$cards = $this->cardMapper->findToMeOrNotAssignedCards($userBoard->getId(), $userId);
}
foreach ($cards as $card) { $boardOwnerIds = array_filter(array_map(function (Board $board) {
$this->enrich($card, $userId); return count($board->getAcl()) === 0 ? $board->getId() : null;
}, $userBoards));
$boardSharedIds = array_filter(array_map(function (Board $board) {
return count($board->getAcl()) > 0 ? $board->getId() : null;
}, $userBoards));
$foundCards = array_merge(
// private board: get cards with due date
$this->cardMapper->findAllWithDue($boardOwnerIds),
// shared board: get all my assigned or unassigned cards
$this->cardMapper->findToMeOrNotAssignedCards($boardSharedIds, $userId)
);
$this->cardService->enrichCards($foundCards);
$overview = [];
foreach ($foundCards as $card) {
$diffDays = $card->getDaysUntilDue(); $diffDays = $card->getDaysUntilDue();
$key = 'later'; $key = 'later';
@@ -120,10 +101,9 @@ class OverviewService {
$key = 'nextSevenDays'; $key = 'nextSevenDays';
} }
$card = (new CardDetails($card, $userBoard)); $card = (new CardDetails($card, $card->getRelatedBoard()));
$overview[$key][] = $card->jsonSerialize(); $overview[$key][] = $card->jsonSerialize();
} }
}
return $overview; return $overview;
} }
} }

View File

@@ -143,7 +143,7 @@ class PermissionService {
* @return bool * @return bool
* @throws NoPermissionException * @throws NoPermissionException
*/ */
public function checkPermission($mapper, $id, $permission, $userId = null) { public function checkPermission($mapper, $id, $permission, $userId = null): bool {
$boardId = $id; $boardId = $id;
if ($mapper instanceof IPermissionMapper && !($mapper instanceof BoardMapper)) { if ($mapper instanceof IPermissionMapper && !($mapper instanceof BoardMapper)) {
$boardId = $mapper->findBoardId($id); $boardId = $mapper->findBoardId($id);
@@ -153,23 +153,11 @@ class PermissionService {
throw new NoPermissionException('Permission denied'); throw new NoPermissionException('Permission denied');
} }
if ($permission === Acl::PERMISSION_SHARE && $this->shareManager->sharingDisabledForUser($this->userId)) { $permissions = $this->getPermissions($boardId);
throw new NoPermissionException('Permission denied'); if ($permissions[$permission] === true) {
}
if ($this->userIsBoardOwner($boardId, $userId)) {
return true; return true;
} }
try {
$acls = $this->getBoard($boardId)->getAcl() ?? [];
$result = $this->userCan($acls, $permission, $userId);
if ($result) {
return true;
}
} catch (DoesNotExistException | MultipleObjectsReturnedException $e) {
}
// Throw NoPermission to not leak information about existing entries // Throw NoPermission to not leak information about existing entries
throw new NoPermissionException('Permission denied'); throw new NoPermissionException('Permission denied');
} }
@@ -260,22 +248,20 @@ class PermissionService {
} }
$users = []; $users = [];
$owner = $this->userManager->get($board->getOwner()); if (!$this->userManager->userExists($board->getOwner())) {
if ($owner === null) {
$this->logger->info('No owner found for board ' . $board->getId()); $this->logger->info('No owner found for board ' . $board->getId());
} else { } else {
$users[$owner->getUID()] = new User($owner); $users[$board->getOwner()] = new User($board->getOwner(), $this->userManager);
} }
$acls = $this->aclMapper->findAll($boardId); $acls = $this->aclMapper->findAll($boardId);
/** @var Acl $acl */ /** @var Acl $acl */
foreach ($acls as $acl) { foreach ($acls as $acl) {
if ($acl->getType() === Acl::PERMISSION_TYPE_USER) { if ($acl->getType() === Acl::PERMISSION_TYPE_USER) {
$user = $this->userManager->get($acl->getParticipant()); if (!$this->userManager->userExists($acl->getParticipant())) {
if ($user === null) {
$this->logger->info('No user found for acl rule ' . $acl->getId()); $this->logger->info('No user found for acl rule ' . $acl->getId());
continue; continue;
} }
$users[$user->getUID()] = new User($user); $users[$acl->getParticipant()] = new User($acl->getParticipant(), $this->userManager);
} }
if ($acl->getType() === Acl::PERMISSION_TYPE_GROUP) { if ($acl->getType() === Acl::PERMISSION_TYPE_GROUP) {
$group = $this->groupManager->get($acl->getParticipant()); $group = $this->groupManager->get($acl->getParticipant());
@@ -284,7 +270,7 @@ class PermissionService {
continue; continue;
} }
foreach ($group->getUsers() as $user) { foreach ($group->getUsers() as $user) {
$users[$user->getUID()] = new User($user); $users[$user->getUID()] = new User($user->getUID(), $this->userManager);
} }
} }
@@ -305,7 +291,7 @@ class PermissionService {
if ($user === null) { if ($user === null) {
$this->logger->info('No user found for circle member ' . $member->getUserId()); $this->logger->info('No user found for circle member ' . $member->getUserId());
} else { } else {
$users[$member->getUserId()] = new User($user); $users[$member->getUserId()] = new User($member->getUserId(), $this->userManager);
} }
} }
} catch (\Exception $e) { } catch (\Exception $e) {

View File

@@ -82,11 +82,7 @@ class SearchService {
}, $boards); }, $boards);
$matchedCards = $this->cardMapper->search($boardIds, $this->filterStringParser->parse($term), $limit, $cursor); $matchedCards = $this->cardMapper->search($boardIds, $this->filterStringParser->parse($term), $limit, $cursor);
$self = $this; return $this->cardService->enrichCards($matchedCards);
return array_map(function (Card $card) use ($self) {
$self->cardService->enrich($card);
return $card;
}, $matchedCards);
} }
public function searchBoards(string $term, ?int $limit, ?int $cursor): array { public function searchBoards(string $term, ?int $limit, ?int $cursor): array {
@@ -117,7 +113,8 @@ class SearchService {
$comment = $this->commentsManager->get($cardRow['comment_id']); $comment = $this->commentsManager->get($cardRow['comment_id']);
unset($cardRow['comment_id']); unset($cardRow['comment_id']);
$card = Card::fromRow($cardRow); $card = Card::fromRow($cardRow);
$self->cardService->enrich($card); // TODO: Only perform one enrich call here
$self->cardService->enrichCards([$card]);
$user = $this->userManager->get($comment->getActorId()); $user = $this->userManager->get($comment->getActorId());
$displayName = $user ? $user->getDisplayName() : ''; $displayName = $user ? $user->getDisplayName() : '';
return new CommentSearchResultEntry($comment->getId(), $comment->getMessage(), $displayName, $card, $this->urlGenerator, $this->l10n); return new CommentSearchResultEntry($comment->getId(), $comment->getMessage(), $displayName, $card, $this->urlGenerator, $this->l10n);

View File

@@ -94,9 +94,9 @@ class StackService {
return; return;
} }
$this->cardService->enrichCards($cards);
$cards = array_map( $cards = array_map(
function (Card $card): CardDetails { function (Card $card): CardDetails {
$this->cardService->enrich($card);
return new CardDetails($card); return new CardDetails($card);
}, },
$cards $cards

View File

@@ -0,0 +1 @@
61324

View File

@@ -40,7 +40,6 @@ class ServerContext implements Context {
} }
public function getCookieJar(): CookieJar { public function getCookieJar(): CookieJar {
echo $this->currentUser;
return $this->cookieJar; return $this->cookieJar;
} }

View File

@@ -27,16 +27,16 @@ if [ -z "$EXECUTOR_NUMBER" ]; then
fi fi
PORT=$((9090 + $EXECUTOR_NUMBER)) PORT=$((9090 + $EXECUTOR_NUMBER))
echo $PORT echo $PORT
php -S localhost:$PORT -t $OC_PATH & php -q -S localhost:$PORT -t $OC_PATH &
PHPPID=$! PHPPID=$!
echo $PHPPID echo $PHPPID
export TEST_SERVER_URL="http://localhost:$PORT/ocs/" export TEST_SERVER_URL="http://localhost:$PORT/ocs/"
vendor/bin/behat $SCENARIO_TO_RUN vendor/bin/behat --colors $SCENARIO_TO_RUN
RESULT=$? RESULT=$?
kill $PHPPID kill -9 $PHPPID
echo "runsh: Exit code: $RESULT" echo "runsh: Exit code: $RESULT"
exit $RESULT exit $RESULT

View File

@@ -24,6 +24,7 @@
namespace OCA\Deck\Db; namespace OCA\Deck\Db;
use OCP\IUser; use OCP\IUser;
use OCP\IUserManager;
class UserTest extends \Test\TestCase { class UserTest extends \Test\TestCase {
public function testGroupObjectSerialize() { public function testGroupObjectSerialize() {
@@ -35,7 +36,11 @@ class UserTest extends \Test\TestCase {
$user->expects($this->any()) $user->expects($this->any())
->method('getDisplayName') ->method('getDisplayName')
->willReturn('myuser displayname'); ->willReturn('myuser displayname');
$userRelationalObject = new User($user); $userManager = $this->createMock(IUserManager::class);
$userManager->expects($this->any())
->method('get')
->willReturn($user);
$userRelationalObject = new User('myuser', $userManager);
$expected = [ $expected = [
'uid' => 'myuser', 'uid' => 'myuser',
'displayname' => 'myuser displayname', 'displayname' => 'myuser displayname',
@@ -53,7 +58,11 @@ class UserTest extends \Test\TestCase {
$user->expects($this->any()) $user->expects($this->any())
->method('getDisplayName') ->method('getDisplayName')
->willReturn('myuser displayname'); ->willReturn('myuser displayname');
$userRelationalObject = new User($user); $userManager = $this->createMock(IUserManager::class);
$userManager->expects($this->any())
->method('get')
->willReturn($user);
$userRelationalObject = new User('myuser', $userManager);
$expected = [ $expected = [
'uid' => 'myuser', 'uid' => 'myuser',
'displayname' => 'myuser displayname', 'displayname' => 'myuser displayname',

View File

@@ -37,6 +37,7 @@ use OCP\IConfig;
use OCP\IGroup; use OCP\IGroup;
use OCP\IGroupManager; use OCP\IGroupManager;
use OCP\IUser; use OCP\IUser;
use OCP\IUserManager;
use OCP\Notification\IManager; use OCP\Notification\IManager;
use OCP\Notification\INotification; use OCP\Notification\INotification;
use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\MockObject\MockObject;
@@ -219,8 +220,9 @@ class NotificationHelperTest extends \Test\TestCase {
'title' => 'MyCardTitle', 'title' => 'MyCardTitle',
'duedate' => '2020-12-24' 'duedate' => '2020-12-24'
]); ]);
$userManager = $this->createMock(IUserManager::class);
$card->setAssignedUsers([ $card->setAssignedUsers([
new User($users[0]) new User($users[0]->getUID(), $userManager)
]); ]);
$this->cardMapper->expects($this->once()) $this->cardMapper->expects($this->once())
->method('findBoardId') ->method('findBoardId')
@@ -308,8 +310,9 @@ class NotificationHelperTest extends \Test\TestCase {
'title' => 'MyCardTitle', 'title' => 'MyCardTitle',
'duedate' => '2020-12-24' 'duedate' => '2020-12-24'
]); ]);
$userManager = $this->createMock(IUserManager::class);
$card->setAssignedUsers([ $card->setAssignedUsers([
new User($users[0]) new User($users[0]->getUID(), $userManager)
]); ]);
$this->cardMapper->expects($this->once()) $this->cardMapper->expects($this->once())
->method('findBoardId') ->method('findBoardId')

View File

@@ -24,6 +24,7 @@
namespace OCA\Deck\Service; namespace OCA\Deck\Service;
use OCA\Deck\Activity\ActivityManager; use OCA\Deck\Activity\ActivityManager;
use OCA\Deck\Db\Assignment;
use OCA\Deck\Db\AssignmentMapper; use OCA\Deck\Db\AssignmentMapper;
use OCA\Deck\Db\Board; use OCA\Deck\Db\Board;
use OCA\Deck\Db\Card; use OCA\Deck\Db\Card;
@@ -155,7 +156,8 @@ class CardServiceTest extends TestCase {
->method('getNumberOfCommentsForObject') ->method('getNumberOfCommentsForObject')
->willReturn(0); ->willReturn(0);
$boardMock = $this->createMock(Board::class); $boardMock = $this->createMock(Board::class);
$stackMock = $this->createMock(Stack::class); $stackMock = new Stack();
$stackMock->setBoardId(1234);
$this->stackMapper->expects($this->any()) $this->stackMapper->expects($this->any())
->method('find') ->method('find')
->willReturn($stackMock); ->willReturn($stackMock);
@@ -168,13 +170,21 @@ class CardServiceTest extends TestCase {
->method('find') ->method('find')
->with(123) ->with(123)
->willReturn($card); ->willReturn($card);
$a1 = new Assignment();
$a1->setCardId(1337);
$a1->setType(0);
$a1->setParticipant('user1');
$a2 = new Assignment();
$a2->setCardId(1337);
$a2->setType(0);
$a2->setParticipant('user2');
$this->assignedUsersMapper->expects($this->any()) $this->assignedUsersMapper->expects($this->any())
->method('findAll') ->method('findIn')
->with(1337) ->with([1337])
->willReturn(['user1', 'user2']); ->willReturn([$a1, $a2]);
$cardExpected = new Card(); $cardExpected = new Card();
$cardExpected->setId(1337); $cardExpected->setId(1337);
$cardExpected->setAssignedUsers(['user1', 'user2']); $cardExpected->setAssignedUsers([$a1, $a2]);
$cardExpected->setRelatedBoard($boardMock); $cardExpected->setRelatedBoard($boardMock);
$cardExpected->setRelatedStack($stackMock); $cardExpected->setRelatedStack($stackMock);
$cardExpected->setLabels([]); $cardExpected->setLabels([]);

View File

@@ -83,10 +83,6 @@ class DefaultBoardServiceTest extends TestCase {
->method('getUserValue') ->method('getUserValue')
->willReturn('yes'); ->willReturn('yes');
$this->boardMapper->expects($this->once())
->method('findAllByUser')
->willReturn($userBoards);
$this->config->expects($this->once()) $this->config->expects($this->once())
->method('setUserValue'); ->method('setUserValue');
@@ -107,10 +103,6 @@ class DefaultBoardServiceTest extends TestCase {
->method('getUserValue') ->method('getUserValue')
->willReturn('no'); ->willReturn('no');
$this->boardMapper->expects($this->once())
->method('findAllByUser')
->willReturn($userBoards);
$result = $this->service->checkFirstRun($this->userId); $result = $this->service->checkFirstRun($this->userId);
$this->assertEquals($result, false); $this->assertEquals($result, false);
} }

View File

@@ -30,6 +30,9 @@ use OCP\IUserManager;
use OCP\Server; use OCP\Server;
use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\MockObject\MockObject;
/**
* @group DB
*/
class TrelloJsonServiceTest extends \Test\TestCase { class TrelloJsonServiceTest extends \Test\TestCase {
private TrelloJsonService $service; private TrelloJsonService $service;
/** @var IURLGenerator|MockObject */ /** @var IURLGenerator|MockObject */
@@ -57,7 +60,7 @@ class TrelloJsonServiceTest extends \Test\TestCase {
} }
public function testValidateUsersWithInvalidUser() { public function testValidateUsersWithInvalidUser() {
$this->expectErrorMessage('Trello user trello_user not found in property "members" of json data'); $this->expectExceptionMessage('Trello user trello_user not found in property "members" of json data');
$importService = $this->createMock(BoardImportService::class); $importService = $this->createMock(BoardImportService::class);
$importService $importService
->method('getConfig') ->method('getConfig')

View File

@@ -236,6 +236,11 @@ class PermissionServiceTest extends \Test\TestCase {
$board->setAcl($this->getAcls($boardId)); $board->setAcl($this->getAcls($boardId));
$this->boardMapper->expects($this->any())->method('find')->willReturn($board); $this->boardMapper->expects($this->any())->method('find')->willReturn($board);
$this->aclMapper->expects($this->any())
->method('findAll')
->with($boardId)
->willReturn($this->getAcls($boardId));
$this->shareManager->expects($this->any()) $this->shareManager->expects($this->any())
->method('sharingDisabledForUser') ->method('sharingDisabledForUser')
->willReturn(false); ->willReturn(false);
@@ -262,12 +267,17 @@ class PermissionServiceTest extends \Test\TestCase {
$this->boardMapper->expects($this->any())->method('find')->willReturn($board); $this->boardMapper->expects($this->any())->method('find')->willReturn($board);
} }
$this->aclMapper->expects($this->any())
->method('findAll')
->with($boardId)
->willReturn($this->getAcls($boardId));
if ($result) { if ($result) {
$actual = $this->service->checkPermission($mapper, 1234, $permission); $actual = $this->service->checkPermission($mapper, $boardId, $permission);
$this->assertTrue($actual); $this->assertTrue($actual);
} else { } else {
$this->expectException(NoPermissionException::class); $this->expectException(NoPermissionException::class);
$this->service->checkPermission($mapper, 1234, $permission); $this->service->checkPermission($mapper, $boardId, $permission);
} }
} }
@@ -340,7 +350,7 @@ class PermissionServiceTest extends \Test\TestCase {
$aclGroup->setParticipant('group1'); $aclGroup->setParticipant('group1');
$board = $this->createMock(Board::class); $board = $this->createMock(Board::class);
$board->expects($this->once()) $board->expects($this->any())
->method('__call') ->method('__call')
->with('getOwner', []) ->with('getOwner', [])
->willReturn('user1'); ->willReturn('user1');
@@ -352,8 +362,8 @@ class PermissionServiceTest extends \Test\TestCase {
->method('find') ->method('find')
->with(123) ->with(123)
->willReturn($board); ->willReturn($board);
$this->userManager->expects($this->exactly(2)) $this->userManager->expects($this->any())
->method('get') ->method('userExists')
->withConsecutive(['user1'], ['user2']) ->withConsecutive(['user1'], ['user2'])
->willReturnOnConsecutiveCalls($user1, $user2); ->willReturnOnConsecutiveCalls($user1, $user2);
@@ -367,9 +377,9 @@ class PermissionServiceTest extends \Test\TestCase {
->willReturn($group); ->willReturn($group);
$users = $this->service->findUsers(123); $users = $this->service->findUsers(123);
$this->assertEquals([ $this->assertEquals([
'user1' => new User($user1), 'user1' => new User($user1->getUID(), $this->userManager),
'user2' => new User($user2), 'user2' => new User($user2->getUID(), $this->userManager),
'user3' => new User($user3), 'user3' => new User($user3->getUID(), $this->userManager),
], $users); ], $users);
} }
} }

View File

@@ -110,11 +110,13 @@ class StackServiceTest extends TestCase {
public function testFindAll() { public function testFindAll() {
$this->permissionService->expects($this->once())->method('checkPermission'); $this->permissionService->expects($this->once())->method('checkPermission');
$this->stackMapper->expects($this->once())->method('findAll')->willReturn($this->getStacks()); $this->stackMapper->expects($this->once())->method('findAll')->willReturn($this->getStacks());
$this->cardService->expects($this->atLeastOnce())->method('enrich')->will( $this->cardService->expects($this->atLeastOnce())->method('enrichCards')->will(
$this->returnCallback( $this->returnCallback(
function ($card) { function ($cards) {
foreach ($cards as $card) {
$card->setLabels($this->getLabels()[$card->getId()]); $card->setLabels($this->getLabels()[$card->getId()]);
} }
}
) )
); );
$this->cardMapper->expects($this->any())->method('findAll')->willReturn($this->getCards(222)); $this->cardMapper->expects($this->any())->method('findAll')->willReturn($this->getCards(222));

View File

@@ -24,6 +24,7 @@
namespace OCA\Deck\Controller; namespace OCA\Deck\Controller;
use OCA\Deck\Db\Acl; use OCA\Deck\Db\Acl;
use OCA\Deck\Db\Board;
use OCP\IUser; use OCP\IUser;
class BoardControllerTest extends \Test\TestCase { class BoardControllerTest extends \Test\TestCase {
@@ -88,11 +89,12 @@ class BoardControllerTest extends \Test\TestCase {
} }
public function testRead() { public function testRead() {
$board = new Board();
$this->boardService->expects($this->once()) $this->boardService->expects($this->once())
->method('find') ->method('find')
->with(123) ->with(123)
->willReturn(1); ->willReturn($board);
$this->assertEquals(1, $this->controller->read(123)); $this->assertEquals($board, $this->controller->read(123));
} }
public function testCreate() { public function testCreate() {