SQL and performance improvements (#2017)

This commit is contained in:
Kevin Papst
2020-10-23 00:31:53 +02:00
committed by GitHub
parent 6ed7ee76e7
commit 4533c994a3
14 changed files with 216 additions and 206 deletions

View File

@@ -78,6 +78,7 @@ final class PermissionController extends AbstractController
$role->setName($roleName); $role->setName($roleName);
$this->roleRepository->saveRole($role); $this->roleRepository->saveRole($role);
$existing[] = $roleName; $existing[] = $roleName;
$all[] = $role;
} }
} }
@@ -145,7 +146,7 @@ final class PermissionController extends AbstractController
'ROLE_TEAMLEAD' => null, 'ROLE_TEAMLEAD' => null,
'ROLE_USER' => null, 'ROLE_USER' => null,
]; ];
foreach ($this->roleRepository->findAll() as $role) { foreach ($all as $role) {
$roles[$role->getName()] = $role; $roles[$role->getName()] = $role;
} }

View File

@@ -105,8 +105,11 @@ trait FormTrait
$builder $builder
->add('activity', ActivityType::class, [ ->add('activity', ActivityType::class, [
'placeholder' => '', 'placeholder' => '',
'query_builder' => function (ActivityRepository $repo) use ($activity, $project) { 'query_builder' => function (ActivityRepository $repo) use ($builder, $activity, $project) {
return $repo->getQueryBuilderForFormType(new ActivityFormTypeQuery($activity, $project)); $query = new ActivityFormTypeQuery($activity, $project);
$query->setUser($builder->getOption('user'));
return $repo->getQueryBuilderForFormType($query);
}, },
]) ])
; ;
@@ -114,7 +117,7 @@ trait FormTrait
// replaces the activity select after submission, to make sure only activities for the selected project are displayed // replaces the activity select after submission, to make sure only activities for the selected project are displayed
$builder->addEventListener( $builder->addEventListener(
FormEvents::PRE_SUBMIT, FormEvents::PRE_SUBMIT,
function (FormEvent $event) use ($activity) { function (FormEvent $event) use ($builder, $activity) {
$data = $event->getData(); $data = $event->getData();
if (!isset($data['project']) || empty($data['project'])) { if (!isset($data['project']) || empty($data['project'])) {
return; return;
@@ -122,8 +125,11 @@ trait FormTrait
$event->getForm()->add('activity', ActivityType::class, [ $event->getForm()->add('activity', ActivityType::class, [
'placeholder' => '', 'placeholder' => '',
'query_builder' => function (ActivityRepository $repo) use ($data, $activity) { 'query_builder' => function (ActivityRepository $repo) use ($builder, $data, $activity) {
return $repo->getQueryBuilderForFormType(new ActivityFormTypeQuery($activity, $data['project'])); $query = new ActivityFormTypeQuery($activity, $data['project']);
$query->setUser($builder->getOption('user'));
return $repo->getQueryBuilderForFormType($query);
}, },
]); ]);
} }

View File

@@ -11,6 +11,7 @@ namespace App\Repository;
use App\Entity\Activity; use App\Entity\Activity;
use App\Entity\Project; use App\Entity\Project;
use App\Entity\Team;
use App\Entity\Timesheet; use App\Entity\Timesheet;
use App\Entity\User; use App\Entity\User;
use App\Model\ActivityStatistic; use App\Model\ActivityStatistic;
@@ -117,7 +118,7 @@ class ActivityRepository extends EntityRepository
return $stats; return $stats;
} }
private function addPermissionCriteria(QueryBuilder $qb, ?User $user = null, array $teams = []) private function addPermissionCriteria(QueryBuilder $qb, ?User $user = null, array $teams = [], bool $globalsOnly = false)
{ {
// make sure that all queries without a user see all projects // make sure that all queries without a user see all projects
if (null === $user && empty($teams)) { if (null === $user && empty($teams)) {
@@ -133,37 +134,41 @@ class ActivityRepository extends EntityRepository
$teams = array_merge($teams, $user->getTeams()->toArray()); $teams = array_merge($teams, $user->getTeams()->toArray());
} }
$qb->leftJoin('a.teams', 'teams')
->leftJoin('p.teams', 'p_teams')
->leftJoin('c.teams', 'c_teams');
if (empty($teams)) { if (empty($teams)) {
$qb->andWhere($qb->expr()->isNull('teams')); $qb->andWhere('SIZE(a.teams) = 0');
$qb->andWhere($qb->expr()->isNull('p_teams')); if (!$globalsOnly) {
$qb->andWhere($qb->expr()->isNull('c_teams')); $qb->andWhere('SIZE(p.teams) = 0');
$qb->andWhere('SIZE(c.teams) = 0');
}
return; return;
} }
$orActivity = $qb->expr()->orX( $orActivity = $qb->expr()->orX(
$qb->expr()->isNull('teams'), 'SIZE(a.teams) = 0',
$qb->expr()->isMemberOf(':teams', 'a.teams') $qb->expr()->isMemberOf(':teams', 'a.teams')
); );
$qb->andWhere($orActivity); $qb->andWhere($orActivity);
if (!$globalsOnly) {
$orProject = $qb->expr()->orX( $orProject = $qb->expr()->orX(
$qb->expr()->isNull('p_teams'), 'SIZE(p.teams) = 0',
$qb->expr()->isMemberOf(':teams', 'p.teams') $qb->expr()->isMemberOf(':teams', 'p.teams')
); );
$qb->andWhere($orProject); $qb->andWhere($orProject);
$orCustomer = $qb->expr()->orX( $orCustomer = $qb->expr()->orX(
$qb->expr()->isNull('c_teams'), 'SIZE(c.teams) = 0',
$qb->expr()->isMemberOf(':teams', 'c.teams') $qb->expr()->isMemberOf(':teams', 'c.teams')
); );
$qb->andWhere($orCustomer); $qb->andWhere($orCustomer);
}
$qb->setParameter('teams', $teams); $ids = array_values(array_unique(array_map(function (Team $team) {
return $team->getId();
}, $teams)));
$qb->setParameter('teams', $ids);
} }
/** /**
@@ -196,7 +201,7 @@ class ActivityRepository extends EntityRepository
$where = $qb->expr()->andX(); $where = $qb->expr()->andX();
$where->add('a.visible = :visible'); $where->add($qb->expr()->eq('a.visible', ':visible'));
$qb->setParameter('visible', true, \PDO::PARAM_BOOL); $qb->setParameter('visible', true, \PDO::PARAM_BOOL);
if (!$query->isGlobalsOnly()) { if (!$query->isGlobalsOnly()) {
@@ -208,19 +213,15 @@ class ActivityRepository extends EntityRepository
$where->add( $where->add(
$qb->expr()->orX( $qb->expr()->orX(
$qb->expr()->eq('c.visible', ':customer_visible'), $qb->expr()->isNull('a.project'),
$qb->expr()->isNull('c.visible') $qb->expr()->andX(
$qb->expr()->eq('p.visible', ':is_visible'),
$qb->expr()->eq('c.visible', ':is_visible')
) )
);
$where->add(
$qb->expr()->orX(
$qb->expr()->eq('p.visible', ':project_visible'),
$qb->expr()->isNull('p.visible')
) )
); );
$qb->setParameter('project_visible', true, \PDO::PARAM_BOOL); $qb->setParameter('is_visible', true, \PDO::PARAM_BOOL);
$qb->setParameter('customer_visible', true, \PDO::PARAM_BOOL);
} }
if ($query->isGlobalsOnly()) { if ($query->isGlobalsOnly()) {
@@ -228,8 +229,8 @@ class ActivityRepository extends EntityRepository
} elseif ($query->hasProjects()) { } elseif ($query->hasProjects()) {
$where->add( $where->add(
$qb->expr()->orX( $qb->expr()->orX(
$qb->expr()->in('a.project', ':project'), $qb->expr()->isNull('a.project'),
$qb->expr()->isNull('a.project') $qb->expr()->in('a.project', ':project')
) )
); );
$qb->setParameter('project', $query->getProjects()); $qb->setParameter('project', $query->getProjects());
@@ -240,6 +241,8 @@ class ActivityRepository extends EntityRepository
$qb->setParameter('ignored', $query->getActivityToIgnore()); $qb->setParameter('ignored', $query->getActivityToIgnore());
} }
$this->addPermissionCriteria($qb, $query->getUser(), $query->getTeams(), $query->isGlobalsOnly());
$or = $qb->expr()->orX(); $or = $qb->expr()->orX();
// this must always be the last part before the or // this must always be the last part before the or
@@ -287,21 +290,21 @@ class ActivityRepository extends EntityRepository
$where = $qb->expr()->andX(); $where = $qb->expr()->andX();
if (!$query->isShowBoth()) { if (!$query->isShowBoth()) {
$where->add($qb->expr()->eq('a.visible', ':visible'));
if (!$query->isGlobalsOnly()) { if (!$query->isGlobalsOnly()) {
$where->add( $where->add(
$qb->expr()->orX( $qb->expr()->orX(
$qb->expr()->isNull('a.project'), $qb->expr()->isNull('a.project'),
$qb->expr()->andX( $qb->expr()->andX(
$qb->expr()->eq('c.visible', ':is_visible'), $qb->expr()->eq('p.visible', ':is_visible'),
$qb->expr()->eq('p.visible', ':is_visible') $qb->expr()->eq('c.visible', ':is_visible')
) )
) )
); );
$qb->setParameter('is_visible', true, \PDO::PARAM_BOOL); $qb->setParameter('is_visible', true, \PDO::PARAM_BOOL);
} }
$where->add($qb->expr()->eq('a.visible', ':visible'));
if ($query->isShowVisible()) { if ($query->isShowVisible()) {
$qb->setParameter('visible', true, \PDO::PARAM_BOOL); $qb->setParameter('visible', true, \PDO::PARAM_BOOL);
} elseif ($query->isShowHidden()) { } elseif ($query->isShowHidden()) {
@@ -322,16 +325,16 @@ class ActivityRepository extends EntityRepository
$where->add($orX); $where->add($orX);
$qb->setParameter('project', $query->getProjects()); $qb->setParameter('project', $query->getProjects());
} elseif (null !== $query->getCustomer()) { } elseif ($query->hasCustomers()) {
$where->add('p.customer = :customer'); $where->add($qb->expr()->in('p.customer', ':customer'));
$qb->setParameter('customer', $query->getCustomer()); $qb->setParameter('customer', $query->getCustomers());
} }
if ($where->count() > 0) { if ($where->count() > 0) {
$qb->andWhere($where); $qb->andWhere($where);
} }
$this->addPermissionCriteria($qb, $query->getCurrentUser(), $query->getTeams()); $this->addPermissionCriteria($qb, $query->getCurrentUser(), $query->getTeams(), $query->isGlobalsOnly());
if ($query->hasSearchTerm()) { if ($query->hasSearchTerm()) {
$searchAnd = $qb->expr()->andX(); $searchAnd = $qb->expr()->andX();
@@ -364,13 +367,6 @@ class ActivityRepository extends EntityRepository
} }
} }
// this will make sure, that we do not accidentally create results with multiple rows,
// which would result in a wrong LIMIT with paginated results
$qb->addGroupBy('a');
// the second group by is needed to satisfy SQL standard (ONLY_FULL_GROUP_BY)
$qb->addGroupBy($orderBy);
return $qb; return $qb;
} }

View File

@@ -13,6 +13,7 @@ use App\Entity\Activity;
use App\Entity\Customer; use App\Entity\Customer;
use App\Entity\CustomerComment; use App\Entity\CustomerComment;
use App\Entity\Project; use App\Entity\Project;
use App\Entity\Team;
use App\Entity\Timesheet; use App\Entity\Timesheet;
use App\Entity\User; use App\Entity\User;
use App\Model\CustomerStatistic; use App\Model\CustomerStatistic;
@@ -150,21 +151,23 @@ class CustomerRepository extends EntityRepository
$teams = array_merge($teams, $user->getTeams()->toArray()); $teams = array_merge($teams, $user->getTeams()->toArray());
} }
$qb->leftJoin('c.teams', 'teams');
if (empty($teams)) { if (empty($teams)) {
$qb->andWhere($qb->expr()->isNull('teams')); $qb->andWhere('SIZE(c.teams) = 0');
return; return;
} }
$or = $qb->expr()->orX( $or = $qb->expr()->orX(
$qb->expr()->isNull('teams'), 'SIZE(c.teams) = 0',
$qb->expr()->isMemberOf(':teams', 'c.teams') $qb->expr()->isMemberOf(':teams', 'c.teams')
); );
$qb->andWhere($or); $qb->andWhere($or);
$qb->setParameter('teams', $teams); $ids = array_values(array_unique(array_map(function (Team $team) {
return $team->getId();
}, $teams)));
$qb->setParameter('teams', $ids);
} }
/** /**
@@ -270,10 +273,6 @@ class CustomerRepository extends EntityRepository
} }
} }
// this will make sure, that we do not accidentally create results with multiple rows,
// which would result in a wrong LIMIT with paginated results
$qb->addGroupBy('c');
return $qb; return $qb;
} }

View File

@@ -11,6 +11,7 @@ namespace App\Repository;
use App\Entity\Customer; use App\Entity\Customer;
use App\Entity\Invoice; use App\Entity\Invoice;
use App\Entity\Team;
use App\Entity\User; use App\Entity\User;
use App\Repository\Loader\InvoiceLoader; use App\Repository\Loader\InvoiceLoader;
use App\Repository\Paginator\LoaderPaginator; use App\Repository\Paginator\LoaderPaginator;
@@ -115,23 +116,25 @@ class InvoiceRepository extends EntityRepository
$teams = array_merge($teams, $user->getTeams()->toArray()); $teams = array_merge($teams, $user->getTeams()->toArray());
} }
$qb $qb->leftJoin('i.customer', 'c');
->leftJoin('i.customer', 'c')
->leftJoin('c.teams', 'c_teams');
if (empty($teams)) { if (empty($teams)) {
$qb->andWhere($qb->expr()->isNull('c_teams')); $qb->andWhere('SIZE(c.teams) = 0');
return; return;
} }
$orCustomer = $qb->expr()->orX( $orCustomer = $qb->expr()->orX(
$qb->expr()->isNull('c_teams'), 'SIZE(c.teams) = 0',
$qb->expr()->isMemberOf(':teams', 'c.teams') $qb->expr()->isMemberOf(':teams', 'c.teams')
); );
$qb->andWhere($orCustomer); $qb->andWhere($orCustomer);
$qb->setParameter('teams', $teams); $ids = array_values(array_unique(array_map(function (Team $team) {
return $team->getId();
}, $teams)));
$qb->setParameter('teams', $ids);
} }
private function getQueryBuilderForQuery(InvoiceQuery $query): QueryBuilder private function getQueryBuilderForQuery(InvoiceQuery $query): QueryBuilder
@@ -154,13 +157,6 @@ class InvoiceRepository extends EntityRepository
$this->addPermissionCriteria($qb, $query->getCurrentUser()); $this->addPermissionCriteria($qb, $query->getCurrentUser());
// this will make sure, that we do not accidentally create results with multiple rows,
// which would result in a wrong LIMIT with paginated results
$qb->addGroupBy('i');
// the second group by is needed to satisfy SQL standard (ONLY_FULL_GROUP_BY)
$qb->addGroupBy($orderBy);
return $qb; return $qb;
} }

View File

@@ -56,6 +56,7 @@ final class ActivityIdLoader implements LoaderInterface
->getQuery() ->getQuery()
->execute(); ->execute();
// global activities don't have projects
if (!empty($activities)) { if (!empty($activities)) {
$projectIds = array_map(function (Activity $activity) { $projectIds = array_map(function (Activity $activity) {
if (null === $activity->getProject()) { if (null === $activity->getProject()) {
@@ -93,6 +94,7 @@ final class ActivityIdLoader implements LoaderInterface
->andWhere($qb->expr()->in('a.id', $ids)) ->andWhere($qb->expr()->in('a.id', $ids))
->getQuery() ->getQuery()
->execute(); ->execute();
}
$qb = $em->createQueryBuilder(); $qb = $em->createQueryBuilder();
$qb->select('PARTIAL a.{id}', 'teams', 'teamlead') $qb->select('PARTIAL a.{id}', 'teams', 'teamlead')
@@ -104,4 +106,3 @@ final class ActivityIdLoader implements LoaderInterface
->execute(); ->execute();
} }
} }
}

View File

@@ -12,6 +12,7 @@ namespace App\Repository;
use App\Entity\Activity; use App\Entity\Activity;
use App\Entity\Project; use App\Entity\Project;
use App\Entity\ProjectComment; use App\Entity\ProjectComment;
use App\Entity\Team;
use App\Entity\Timesheet; use App\Entity\Timesheet;
use App\Entity\User; use App\Entity\User;
use App\Model\ProjectStatistic; use App\Model\ProjectStatistic;
@@ -142,29 +143,30 @@ class ProjectRepository extends EntityRepository
$teams = array_merge($teams, $user->getTeams()->toArray()); $teams = array_merge($teams, $user->getTeams()->toArray());
} }
$qb->leftJoin('p.teams', 'teams')
->leftJoin('c.teams', 'c_teams');
if (empty($teams)) { if (empty($teams)) {
$qb->andWhere($qb->expr()->isNull('c_teams')); $qb->andWhere('SIZE(c.teams) = 0');
$qb->andWhere($qb->expr()->isNull('teams')); $qb->andWhere('SIZE(p.teams) = 0');
return; return;
} }
$orProject = $qb->expr()->orX( $orProject = $qb->expr()->orX(
$qb->expr()->isNull('teams'), 'SIZE(p.teams) = 0',
$qb->expr()->isMemberOf(':teams', 'p.teams') $qb->expr()->isMemberOf(':teams', 'p.teams')
); );
$qb->andWhere($orProject); $qb->andWhere($orProject);
$orCustomer = $qb->expr()->orX( $orCustomer = $qb->expr()->orX(
$qb->expr()->isNull('c_teams'), 'SIZE(c.teams) = 0',
$qb->expr()->isMemberOf(':teams', 'c.teams') $qb->expr()->isMemberOf(':teams', 'c.teams')
); );
$qb->andWhere($orCustomer); $qb->andWhere($orCustomer);
$qb->setParameter('teams', $teams); $ids = array_values(array_unique(array_map(function (Team $team) {
return $team->getId();
}, $teams)));
$qb->setParameter('teams', $ids);
} }
/** /**
@@ -369,13 +371,6 @@ class ProjectRepository extends EntityRepository
} }
} }
// this will make sure, that we do not accidentally create results with multiple rows,
// which would result in a wrong LIMIT with paginated results
$qb->addGroupBy('p');
// the second group by is needed to satisfy SQL standard (ONLY_FULL_GROUP_BY)
$qb->addGroupBy($orderBy);
return $qb; return $qb;
} }

View File

@@ -10,12 +10,12 @@
namespace App\Repository; namespace App\Repository;
use App\Entity\Tag; use App\Entity\Tag;
use App\Repository\Paginator\QueryBuilderPaginator;
use App\Repository\Query\TagFormTypeQuery; use App\Repository\Query\TagFormTypeQuery;
use App\Repository\Query\TagQuery; use App\Repository\Query\TagQuery;
use Doctrine\ORM\EntityRepository; use Doctrine\ORM\EntityRepository;
use Doctrine\ORM\ORMException; use Doctrine\ORM\ORMException;
use Doctrine\ORM\QueryBuilder; use Doctrine\ORM\QueryBuilder;
use Pagerfanta\Adapter\DoctrineORMAdapter;
use Pagerfanta\Pagerfanta; use Pagerfanta\Pagerfanta;
/** /**
@@ -107,15 +107,32 @@ class TagRepository extends EntityRepository
* @return Pagerfanta * @return Pagerfanta
*/ */
public function getTagCount(TagQuery $query) public function getTagCount(TagQuery $query)
{
$qb = $this->getQueryBuilderForQuery($query);
$qb
->resetDQLPart('select')
->resetDQLPart('orderBy')
->select($qb->expr()->count('tag.name'))
;
$counter = (int) $qb->getQuery()->getSingleScalarResult();
$qb = $this->getQueryBuilderForQuery($query);
$paginator = new QueryBuilderPaginator($qb, $counter);
$pagerfanta = new Pagerfanta($paginator);
$pagerfanta->setMaxPerPage($query->getPageSize());
$pagerfanta->setCurrentPage($query->getPage());
return $pagerfanta;
}
private function getQueryBuilderForQuery(TagQuery $query): QueryBuilder
{ {
$qb = $this->createQueryBuilder('tag'); $qb = $this->createQueryBuilder('tag');
$qb $qb
->select('tag.id, tag.name, tag.color, count(timesheets.id) as amount') ->select('tag.id, tag.name, tag.color, SIZE(tag.timesheets) as amount')
->leftJoin('tag.timesheets', 'timesheets')
->addGroupBy('tag.id')
->addGroupBy('tag.name')
->addGroupBy('tag.color')
; ;
$orderBy = $query->getOrderBy(); $orderBy = $query->getOrderBy();
@@ -148,11 +165,7 @@ class TagRepository extends EntityRepository
} }
} }
$paginator = new Pagerfanta(new DoctrineORMAdapter($qb->getQuery(), false)); return $qb;
$paginator->setMaxPerPage($query->getPageSize());
$paginator->setCurrentPage($query->getPage());
return $paginator;
} }
public function getQueryBuilderForFormType(TagFormTypeQuery $query): QueryBuilder public function getQueryBuilderForFormType(TagFormTypeQuery $query): QueryBuilder

View File

@@ -13,6 +13,7 @@ use App\Entity\ActivityRate;
use App\Entity\CustomerRate; use App\Entity\CustomerRate;
use App\Entity\ProjectRate; use App\Entity\ProjectRate;
use App\Entity\RateInterface; use App\Entity\RateInterface;
use App\Entity\Team;
use App\Entity\Timesheet; use App\Entity\Timesheet;
use App\Entity\User; use App\Entity\User;
use App\Model\Statistic\Day; use App\Model\Statistic\Day;
@@ -253,13 +254,13 @@ class TimesheetRepository extends EntityRepository
if (!empty($begin)) { if (!empty($begin)) {
$qb $qb
->andWhere($qb->expr()->gte($this->getDatetimeFieldSql('t.begin'), ':from')) ->andWhere($qb->expr()->gte('t.begin', ':from'))
->setParameter('from', $begin); ->setParameter('from', $begin);
} }
if (!empty($end)) { if (!empty($end)) {
$qb $qb
->andWhere($qb->expr()->lte($this->getDatetimeFieldSql('t.end'), ':to')) ->andWhere($qb->expr()->lte('t.end', ':to'))
->setParameter('to', $end); ->setParameter('to', $end);
} }
@@ -320,14 +321,14 @@ class TimesheetRepository extends EntityRepository
; ;
if (!empty($begin)) { if (!empty($begin)) {
$qb->andWhere($qb->expr()->gte($this->getDatetimeFieldSql('t.begin'), ':from')) $qb->andWhere($qb->expr()->gte('t.begin', ':from'))
->setParameter('from', $begin); ->setParameter('from', $begin);
} else { } else {
$qb->andWhere($qb->expr()->isNotNull('t.begin')); $qb->andWhere($qb->expr()->isNotNull('t.begin'));
} }
if (!empty($end)) { if (!empty($end)) {
$qb->andWhere($qb->expr()->lte($this->getDatetimeFieldSql('t.end'), ':to')) $qb->andWhere($qb->expr()->lte('t.end', ':to'))
->setParameter('to', $end); ->setParameter('to', $end);
} else { } else {
$qb->andWhere($qb->expr()->isNotNull('t.end')); $qb->andWhere($qb->expr()->isNotNull('t.end'));
@@ -595,46 +596,59 @@ class TimesheetRepository extends EntityRepository
return $counter; return $counter;
} }
private function addPermissionCriteria(QueryBuilder $qb, ?User $user = null, array $teams = []) /**
* This method causes me some headaches ...
*
* Activity permissions are currently not checked (which would be easy to add)
*
* Especially the following question is still un-answered!
*
* Should a teamlead:
* 1 . see all records of his team-members, even if they recorded times for projects invisible to him
* 2. only see records for projects which can be accessed by hom (current situation)
*/
private function addPermissionCriteria(QueryBuilder $qb, ?User $user = null, array $teams = []): bool
{ {
// make sure that all queries without a user see all projects // make sure that all queries without a user see all projects
if (null === $user && empty($teams)) { if (null === $user && empty($teams)) {
return; return false;
} }
// make sure that admins see all timesheet records // make sure that admins see all timesheet records
if (null !== $user && $user->canSeeAllData()) { if (null !== $user && $user->canSeeAllData()) {
return; return false;
} }
if (null !== $user) { if (null !== $user) {
$teams = array_merge($teams, $user->getTeams()->toArray()); $teams = array_merge($teams, $user->getTeams()->toArray());
} }
$qb
->leftJoin('p.teams', 'teams')
->leftJoin('c.teams', 'c_teams');
if (empty($teams)) { if (empty($teams)) {
$qb->andWhere($qb->expr()->isNull('c_teams')); $qb->andWhere('SIZE(c.teams) = 0');
$qb->andWhere($qb->expr()->isNull('teams')); $qb->andWhere('SIZE(p.teams) = 0');
return; return true;
} }
$orProject = $qb->expr()->orX( $orProject = $qb->expr()->orX(
$qb->expr()->isNull('teams'), 'SIZE(p.teams) = 0',
$qb->expr()->isMemberOf(':teams', 'p.teams') $qb->expr()->isMemberOf(':teams', 'p.teams')
); );
$qb->andWhere($orProject); $qb->andWhere($orProject);
$orCustomer = $qb->expr()->orX( $orCustomer = $qb->expr()->orX(
$qb->expr()->isNull('c_teams'), 'SIZE(c.teams) = 0',
$qb->expr()->isMemberOf(':teams', 'c.teams') $qb->expr()->isMemberOf(':teams', 'c.teams')
); );
$qb->andWhere($orCustomer); $qb->andWhere($orCustomer);
$qb->setParameter('teams', $teams); $ids = array_values(array_unique(array_map(function (Team $team) {
return $team->getId();
}, $teams)));
$qb->setParameter('teams', $ids);
return true;
} }
public function getPagerfantaForQuery(TimesheetQuery $query): Pagerfanta public function getPagerfantaForQuery(TimesheetQuery $query): Pagerfanta
@@ -652,7 +666,8 @@ class TimesheetRepository extends EntityRepository
$qb $qb
->resetDQLPart('select') ->resetDQLPart('select')
->resetDQLPart('orderBy') ->resetDQLPart('orderBy')
->select($qb->expr()->countDistinct('t.id')) // faster then using "distinct id", as the user field is a separate (and smaller) index
->select($qb->expr()->count('t.user'))
; ;
$counter = (int) $qb->getQuery()->getSingleScalarResult(); $counter = (int) $qb->getQuery()->getSingleScalarResult();
@@ -695,23 +710,27 @@ class TimesheetRepository extends EntityRepository
{ {
$qb = $this->getEntityManager()->createQueryBuilder(); $qb = $this->getEntityManager()->createQueryBuilder();
$requiresProject = false;
$requiresCustomer = false;
$requiresActivity = false;
$qb $qb
->select('t') ->select('t')
->from(Timesheet::class, 't') ->from(Timesheet::class, 't')
->leftJoin('t.project', 'p')
->leftJoin('p.customer', 'c')
; ;
$orderBy = $query->getOrderBy(); $orderBy = $query->getOrderBy();
switch ($orderBy) { switch ($orderBy) {
case 'project': case 'project':
$orderBy = 'p.name'; $orderBy = 'p.name';
$requiresProject = true;
break; break;
case 'customer': case 'customer':
$requiresCustomer = true;
$orderBy = 'c.name'; $orderBy = 'c.name';
break; break;
case 'activity': case 'activity':
$qb->leftJoin('t.activity', 'a'); $requiresActivity = true;
$orderBy = 'a.name'; $orderBy = 'a.name';
break; break;
default: default:
@@ -728,10 +747,7 @@ class TimesheetRepository extends EntityRepository
$user = array_merge($user, $query->getUsers()); $user = array_merge($user, $query->getUsers());
if (empty($user) && null !== $query->getCurrentUser()) { if (empty($user) && null !== ($currentUser = $query->getCurrentUser()) && !$currentUser->canSeeAllData()) {
$currentUser = $query->getCurrentUser();
if (!$currentUser->canSeeAllData()) {
// make sure that the user himself is in the list of users, if he is part of a team // make sure that the user himself is in the list of users, if he is part of a team
// if teams are used and the user is not a teamlead, the list of users would be empty and then leading to NOT limit the select by user IDs // if teams are used and the user is not a teamlead, the list of users would be empty and then leading to NOT limit the select by user IDs
$user[] = $currentUser; $user[] = $currentUser;
@@ -742,7 +758,6 @@ class TimesheetRepository extends EntityRepository
} }
} }
} }
}
if (!empty($query->getTeams())) { if (!empty($query->getTeams())) {
foreach ($query->getTeams() as $team) { foreach ($query->getTeams() as $team) {
@@ -766,7 +781,7 @@ class TimesheetRepository extends EntityRepository
} }
if (null !== $query->getBegin()) { if (null !== $query->getBegin()) {
$qb->andWhere($qb->expr()->gte($this->getDatetimeFieldSql('t.begin'), ':begin')) $qb->andWhere($qb->expr()->gte('t.begin', ':begin'))
->setParameter('begin', $query->getBegin()); ->setParameter('begin', $query->getBegin());
} }
@@ -777,7 +792,7 @@ class TimesheetRepository extends EntityRepository
} }
if (null !== $query->getEnd()) { if (null !== $query->getEnd()) {
$qb->andWhere($qb->expr()->lte($this->getDatetimeFieldSql('t.begin'), ':end')) $qb->andWhere($qb->expr()->lte('t.begin', ':end'))
->setParameter('end', $query->getEnd()); ->setParameter('end', $query->getEnd());
} }
@@ -794,7 +809,7 @@ class TimesheetRepository extends EntityRepository
} }
if (null !== $query->getModifiedAfter()) { if (null !== $query->getModifiedAfter()) {
$qb->andWhere($qb->expr()->gte($this->getDatetimeFieldSql('t.modifiedAt'), ':modified_at')) $qb->andWhere($qb->expr()->gte('t.modifiedAt', ':modified_at'))
->setParameter('modified_at', $query->getModifiedAfter()); ->setParameter('modified_at', $query->getModifiedAfter());
} }
@@ -807,6 +822,7 @@ class TimesheetRepository extends EntityRepository
$qb->andWhere($qb->expr()->in('t.project', ':project')) $qb->andWhere($qb->expr()->in('t.project', ':project'))
->setParameter('project', $query->getProjects()); ->setParameter('project', $query->getProjects());
} elseif ($query->hasCustomers()) { } elseif ($query->hasCustomers()) {
$requiresCustomer = true;
$qb->andWhere($qb->expr()->in('p.customer', ':customer')) $qb->andWhere($qb->expr()->in('p.customer', ':customer'))
->setParameter('customer', $query->getCustomers()); ->setParameter('customer', $query->getCustomers());
} }
@@ -817,7 +833,7 @@ class TimesheetRepository extends EntityRepository
->setParameter('tags', $query->getTags()); ->setParameter('tags', $query->getTags());
} }
$this->addPermissionCriteria($qb, $query->getCurrentUser(), $query->getTeams()); $requiresTeams = $this->addPermissionCriteria($qb, $query->getCurrentUser(), $query->getTeams());
if ($query->hasSearchTerm()) { if ($query->hasSearchTerm()) {
$searchAnd = $qb->expr()->andX(); $searchAnd = $qb->expr()->andX();
@@ -847,6 +863,18 @@ class TimesheetRepository extends EntityRepository
} }
} }
if ($requiresCustomer || $requiresProject || $requiresTeams) {
$qb->leftJoin('t.project', 'p');
}
if ($requiresCustomer || $requiresTeams) {
$qb->leftJoin('p.customer', 'c');
}
if ($requiresActivity) {
$qb->leftJoin('t.activity', 'a');
}
return $qb; return $qb;
} }
@@ -883,7 +911,7 @@ class TimesheetRepository extends EntityRepository
} }
if (null !== $startFrom) { if (null !== $startFrom) {
$qb->andWhere($qb->expr()->gte($this->getDatetimeFieldSql('t.begin'), ':begin')) $qb->andWhere($qb->expr()->gte('t.begin', ':begin'))
->setParameter('begin', $startFrom); ->setParameter('begin', $startFrom);
} }
@@ -926,16 +954,6 @@ class TimesheetRepository extends EntityRepository
$em->commit(); $em->commit();
} }
private function getDatetimeFieldSql(string $field): string
{
// this would change the selected data for queries that join across multiple timezones
// but due to tax laws, this is disabled - exports/invoices should *always* include the data from
// the own timezone, not from the original users timezone
// return sprintf('CONVERT_TZ(%s, \'UTC\', t.timezone)', $field);
return $field;
}
/** /**
* @param Timesheet $timesheet * @param Timesheet $timesheet
* @return RateInterface[] * @return RateInterface[]

View File

@@ -11,6 +11,7 @@ namespace App\Repository;
use App\Entity\Role; use App\Entity\Role;
use App\Entity\User; use App\Entity\User;
use App\Repository\Loader\UserIdLoader;
use App\Repository\Loader\UserLoader; use App\Repository\Loader\UserLoader;
use App\Repository\Paginator\LoaderPaginator; use App\Repository\Paginator\LoaderPaginator;
use App\Repository\Paginator\PaginatorInterface; use App\Repository\Paginator\PaginatorInterface;
@@ -56,16 +57,15 @@ class UserRepository extends EntityRepository implements UserLoaderInterface
*/ */
public function getUserById($id): ?User public function getUserById($id): ?User
{ {
return $this->createQueryBuilder('u') /** @var User|null $user */
->select('u', 'p', 't', 'tu', 'tl') $user = $this->findOneBy(['id' => $id]);
->leftJoin('u.preferences', 'p')
->leftJoin('u.teams', 't') if ($user !== null) {
->leftJoin('t.users', 'tu') $loader = new UserIdLoader($this->getEntityManager());
->leftJoin('t.teamlead', 'tl') $loader->loadResults([$user->getId()]);
->where('u.id = :id') }
->setParameter('id', $id)
->getQuery() return $user;
->getOneOrNullResult();
} }
/** /**
@@ -127,17 +127,21 @@ class UserRepository extends EntityRepository implements UserLoaderInterface
*/ */
public function loadUserByUsername($username) public function loadUserByUsername($username)
{ {
return $this->createQueryBuilder('u') /** @var User|null $user */
->select('u', 'p', 't', 'tu', 'tl') $user = $this->createQueryBuilder('u')
->leftJoin('u.preferences', 'p') ->select('u')
->leftJoin('u.teams', 't')
->leftJoin('t.users', 'tu')
->leftJoin('t.teamlead', 'tl')
->where('u.username = :username') ->where('u.username = :username')
->orWhere('u.email = :username') ->orWhere('u.email = :username')
->setParameter('username', $username) ->setParameter('username', $username)
->getQuery() ->getQuery()
->getOneOrNullResult(); ->getOneOrNullResult();
if ($user !== null) {
$loader = new UserIdLoader($this->getEntityManager());
$loader->loadResults([$user->getId()]);
}
return $user;
} }
public function getQueryBuilderForFormType(UserFormTypeQuery $query): QueryBuilder public function getQueryBuilderForFormType(UserFormTypeQuery $query): QueryBuilder

View File

@@ -10,48 +10,37 @@
namespace App\Security; namespace App\Security;
use App\Entity\User; use App\Entity\User;
use App\Repository\UserRepository;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
/**
* @deprecated will be removed with 2.0
*/
final class CurrentUser final class CurrentUser
{ {
/** /**
* @var TokenStorageInterface * @var TokenStorageInterface
*/ */
private $storage; private $storage;
/**
* @var UserRepository
*/
private $repository;
/** /**
* @var User|null * @var User|null
*/ */
private $user; private $user;
/** public function __construct(TokenStorageInterface $storage)
* @param TokenStorageInterface $storage
* @param UserRepository $repository
*/
public function __construct(TokenStorageInterface $storage, UserRepository $repository)
{ {
$this->storage = $storage; $this->storage = $storage;
$this->repository = $repository;
} }
/** public function getUser(): ?User
* @return User|null
*/
public function getUser()
{ {
if (null === $this->storage->getToken()) {
return null;
}
// some inline caching to prevent multiple DB lookups
if (null !== $this->user) { if (null !== $this->user) {
return $this->user; return $this->user;
} }
if (null === $this->storage->getToken()) {
return null;
}
/** @var User $user */ /** @var User $user */
$user = $this->storage->getToken()->getUser(); $user = $this->storage->getToken()->getUser();
@@ -59,7 +48,7 @@ final class CurrentUser
return null; return null;
} }
$this->user = $this->repository->getUserById($user->getId()); $this->user = $user;
return $this->user; return $this->user;
} }

View File

@@ -14,7 +14,7 @@ use App\Security\CurrentUser;
use DateTimeZone; use DateTimeZone;
/** /**
* @internal use DateTimeFactory instead: this one relies on the global context and will be deprecated in the future * @deprecated will be removed with 2.0
*/ */
class UserDateTimeFactory extends DateTimeFactory class UserDateTimeFactory extends DateTimeFactory
{ {

View File

@@ -11,10 +11,8 @@ namespace App\Tests\Mocks\Security;
use App\Entity\User; use App\Entity\User;
use App\Entity\UserPreference; use App\Entity\UserPreference;
use App\Repository\UserRepository;
use App\Security\CurrentUser; use App\Security\CurrentUser;
use App\Tests\Mocks\AbstractMockFactory; use App\Tests\Mocks\AbstractMockFactory;
use PHPUnit\Framework\TestCase;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
@@ -34,11 +32,6 @@ class CurrentUserFactory extends AbstractMockFactory
$user->addPreference($pref); $user->addPreference($pref);
} }
$mock = $this->getMockBuilder(UserRepository::class)->onlyMethods(['getUserById'])->disableOriginalConstructor()->getMock();
$mock->expects(TestCase::atMost(1))->method('getUserById')->willReturn($user);
/** @var UserRepository $repository */
$repository = $mock;
$mock = $this->getMockBuilder(UsernamePasswordToken::class)->onlyMethods(['getUser'])->disableOriginalConstructor()->getMock(); $mock = $this->getMockBuilder(UsernamePasswordToken::class)->onlyMethods(['getUser'])->disableOriginalConstructor()->getMock();
$mock->method('getUser')->willReturn($user); $mock->method('getUser')->willReturn($user);
/** @var UsernamePasswordToken $token */ /** @var UsernamePasswordToken $token */
@@ -47,6 +40,6 @@ class CurrentUserFactory extends AbstractMockFactory
$tokenStorage = new TokenStorage(); $tokenStorage = new TokenStorage();
$tokenStorage->setToken($token); $tokenStorage->setToken($token);
return new CurrentUser($tokenStorage, $repository); return new CurrentUser($tokenStorage);
} }
} }

View File

@@ -20,8 +20,7 @@ class ActivityLoaderTest extends AbstractLoaderTest
{ {
public function testLoadResults() public function testLoadResults()
{ {
// mock needs improvements, because it should be 5 $em = $this->getEntityManagerMock(3);
$em = $this->getEntityManagerMock(2);
$sut = new ActivityLoader($em); $sut = new ActivityLoader($em);