Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IBX-7833: [PAPI] Implemented loading paginated relation list #343

Merged
merged 9 commits into from
Apr 22, 2024
3 changes: 1 addition & 2 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -16167,7 +16167,7 @@ parameters:

-
message: "#^Cannot call method fetchAll\\(\\) on Doctrine\\\\DBAL\\\\ForwardCompatibility\\\\Result\\|int\\|string\\.$#"
count: 17
count: 16
path: src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php

-
Expand Down Expand Up @@ -63259,4 +63259,3 @@ parameters:
message: "#^Method Ibexa\\\\Tests\\\\Core\\\\Specification\\\\Content\\\\ContentTypeSpecificationTest\\:\\:providerForIsSatisfiedBy\\(\\) return type has no value type specified in iterable type array\\.$#"
count: 1
path: tests/lib/Specification/Content/ContentTypeSpecificationTest.php

22 changes: 22 additions & 0 deletions src/contracts/Persistence/Content/Handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,8 @@ public function removeRelation($relationId, $type, ?int $destinationContentId =
/**
* Loads relations from $sourceContentId. Optionally, loads only those with $type and $sourceContentVersionNo.
*
* @deprecated 4.5.7 The "ContentService::loadRelations()" method is deprecated, will be removed in 5.0.
*
* @param mixed $sourceContentId Source Content ID
* @param mixed|null $sourceContentVersionNo Source Content Version, null if not specified
* @param int|null $type {@see \Ibexa\Contracts\Core\Repository\Values\Content\Relation::COMMON,
Expand All @@ -294,6 +296,26 @@ public function removeRelation($relationId, $type, ?int $destinationContentId =
*/
public function loadRelations($sourceContentId, $sourceContentVersionNo = null, $type = null);

/**
* Counts all outgoing relations for the given version.
*/
public function countRelations(
int $sourceContentId,
?int $sourceContentVersionNo = null,
?int $type = null
): int;

/**
* @return \Ibexa\Contracts\Core\Persistence\Content\Relation[]
*/
public function loadRelationList(
int $sourceContentId,
int $limit,
int $offset = 0,
?int $sourceContentVersionNo = null,
?int $type = null
): array;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably go for iterable unless it causes issues. WDYT @Steveb-p?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It depends. iterable can be tricky to work with, if it's for example a generator (since it can be exhausted, and therefore only iterated over once).

If there is a chance that we could load data using a cursor (i.e. return data rows one by one), then your point is valid 😅


/**
* Counts relations from $destinationContentId only against published versions. Optionally, count only those with $type.
*
Expand Down
28 changes: 27 additions & 1 deletion src/contracts/Repository/ContentService.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
*/
interface ContentService
{
public const DEFAULT_PAGE_SIZE = 25;

/**
* Loads a content info object.
*
Expand Down Expand Up @@ -398,12 +400,36 @@ public function copyContent(ContentInfo $contentInfo, LocationCreateStruct $dest
*
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\UnauthorizedException if the user is not allowed to read this version
*
* @param \Ibexa\Contracts\Core\Repository\Values\Content\VersionInfo $versionInfo
* @deprecated 4.5.7 The "ContentService::loadRelations()" method is deprecated, will be removed in 5.0.
*
* @return \Ibexa\Contracts\Core\Repository\Values\Content\Relation[]
*/
public function loadRelations(VersionInfo $versionInfo): iterable;

/**
* Loads all outgoing relations for the given version.
*
* If the user is not allowed to read specific version then a returned `RelationList` will contain `UnauthorizedRelationListItem`
*
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\BadStateException
*
* @see \Ibexa\Contracts\Core\Repository\Values\Content\RelationList\Item\UnauthorizedRelationListItem
*/
public function loadRelationList(
VersionInfo $versionInfo,
int $offset = 0,
int $limit = self::DEFAULT_PAGE_SIZE
): RelationList;

/**
* Counts all outgoing relations for the given version.
*
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\BadStateException
*/
public function countRelations(VersionInfo $versionInfo): int;

/**
* Counts all incoming relations for the given content object.
*
Expand Down
10 changes: 10 additions & 0 deletions src/contracts/Repository/Decorator/ContentServiceDecorator.php
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,16 @@ public function loadRelations(VersionInfo $versionInfo): iterable
return $this->innerService->loadRelations($versionInfo);
}

public function countRelations(VersionInfo $versionInfo): int
{
return $this->innerService->countRelations($versionInfo);
}

public function loadRelationList(VersionInfo $versionInfo, int $offset = 0, int $limit = self::DEFAULT_PAGE_SIZE): RelationList
{
return $this->innerService->loadRelationList($versionInfo, $offset, $limit);
}

public function countReverseRelations(ContentInfo $contentInfo): int
{
return $this->innerService->countReverseRelations($contentInfo);
Expand Down
95 changes: 95 additions & 0 deletions src/lib/Persistence/Cache/ContentHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ class ContentHandler extends AbstractInMemoryPersistenceHandler implements Conte
private const CONTENT_VERSION_LIST_IDENTIFIER = 'content_version_list';
private const CONTENT_VERSION_INFO_IDENTIFIER = 'content_version_info';
private const CONTENT_VERSION_IDENTIFIER = 'content_version';
private const CONTENT_RELATIONS_COUNT_WITH_VERSION_TYPE_IDENTIFIER = 'content_relations_count_with_by_version_type_suffix';
private const CONTENT_RELATION_IDENTIFIER = 'content_relation';
private const CONTENT_RELATIONS_LIST_IDENTIFIER = 'content_relations_list';
private const CONTENT_RELATIONS_LIST_WITH_VERSION_TYPE_IDENTIFIER = 'content_relations_list_with_by_version_type_suffix';
private const CONTENT_REVERSE_RELATIONS_COUNT_IDENTIFIER = 'content_reverse_relations_count';
private const RELATION_IDENTIFIER = 'relation';

Expand Down Expand Up @@ -517,6 +521,97 @@ public function loadRelations($sourceContentId, $sourceContentVersionNo = null,
return $this->persistenceHandler->contentHandler()->loadRelations($sourceContentId, $sourceContentVersionNo, $type);
}

public function countRelations(int $sourceContentId, ?int $sourceContentVersionNo = null, ?int $type = null): int
{
$cacheItem = $this->cache->getItem(
$this->cacheIdentifierGenerator->generateKey(
self::CONTENT_RELATIONS_COUNT_WITH_VERSION_TYPE_IDENTIFIER,
[$sourceContentId, $sourceContentVersionNo, $type],
true
)
);

if ($cacheItem->isHit()) {
$this->logger->logCacheHit(['content' => $sourceContentId, 'version' => $sourceContentVersionNo, 'type' => $type]);

return $cacheItem->get();
}

$this->logger->logCacheMiss(['content' => $sourceContentId, 'version' => $sourceContentVersionNo, 'type' => $type]);
$relationsCount = $this->persistenceHandler->contentHandler()->countRelations(
$sourceContentId,
$sourceContentVersionNo,
$type
);
$cacheItem->set($relationsCount);
$tags = [
$this->cacheIdentifierGenerator->generateTag(
self::CONTENT_IDENTIFIER,
[$sourceContentId]
),
];

$cacheItem->tag($tags);
$this->cache->save($cacheItem);

return $relationsCount;
}

public function loadRelationList(
int $sourceContentId,
int $limit,
int $offset = 0,
?int $sourceContentVersionNo = null,
?int $type = null
): array {
return $this->getListCacheValue(
$this->cacheIdentifierGenerator->generateKey(
self::CONTENT_RELATIONS_LIST_WITH_VERSION_TYPE_IDENTIFIER,
[$sourceContentId, $limit, $offset, $sourceContentVersionNo, $type],
true
),
function () use ($sourceContentId, $limit, $offset, $sourceContentVersionNo, $type): array {
return $this->persistenceHandler->contentHandler()->loadRelationList(
$sourceContentId,
$limit,
$offset,
$sourceContentVersionNo,
$type
);
},
function (Relation $relation): array {
return [
$this->cacheIdentifierGenerator->generateTag(
self::CONTENT_RELATION_IDENTIFIER,
[$relation->destinationContentId]
),
$this->cacheIdentifierGenerator->generateTag(
self::CONTENT_IDENTIFIER,
[$relation->destinationContentId]
),
];
},
function (Relation $relation): array {
return [
$this->cacheIdentifierGenerator->generateKey(self::CONTENT_IDENTIFIER, [$relation->destinationContentId], true),
];
},
function () use ($sourceContentId): array {
return [
$this->cacheIdentifierGenerator->generateTag(
self::CONTENT_RELATIONS_LIST_IDENTIFIER,
[$sourceContentId]
),
$this->cacheIdentifierGenerator->generateTag(
self::CONTENT_IDENTIFIER,
[$sourceContentId]
),
];
},
[$sourceContentId, $limit, $offset, $sourceContentVersionNo, $type]
);
}

/**
* {@inheritdoc}
*/
Expand Down
24 changes: 23 additions & 1 deletion src/lib/Persistence/Legacy/Content/Gateway.php
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,29 @@ abstract public function loadRelations(
): array;

/**
* Count number of related to/from $contentId.
* Counts number of related to/from $contentId.
*/
abstract public function countRelations(
int $contentId,
?int $contentVersionNo = null,
?int $relationType = null
): int;

/**
* Loads paginated data of related to/from $contentId.
*
* @return array<array<string, mixed>>
*/
abstract public function listRelations(
int $contentId,
int $limit,
int $offset = 0,
?int $contentVersionNo = null,
?int $relationType = null
): array;

/**
* Counts number of related to/from $contentId.
*/
abstract public function countReverseRelations(int $contentId, ?int $relationType = null): int;

Expand Down
65 changes: 54 additions & 11 deletions src/lib/Persistence/Legacy/Content/Gateway/DoctrineDatabase.php
Original file line number Diff line number Diff line change
Expand Up @@ -1387,18 +1387,61 @@ public function loadRelations(
?int $relationType = null
): array {
$query = $this->queryBuilder->createRelationFindQueryBuilder();
$query = $this->prepareRelationQuery($query, $contentId, $contentVersionNo, $relationType);

return $query->execute()->fetchAllAssociative();
}

public function countRelations(
int $contentId,
?int $contentVersionNo = null,
?int $relationType = null
): int {
$query = $this->connection->createQueryBuilder();
$query->select($this->databasePlatform->getCountExpression('l.id'))
->from(self::CONTENT_RELATION_TABLE, 'l');

$query = $this->prepareRelationQuery($query, $contentId, $contentVersionNo, $relationType);

return (int)$query->execute()->fetchOne();
}

public function listRelations(
int $contentId,
int $limit,
int $offset = 0,
?int $contentVersionNo = null,
?int $relationType = null
): array {
$query = $this->queryBuilder->createRelationFindQueryBuilder();
$query = $this->prepareRelationQuery($query, $contentId, $contentVersionNo, $relationType);

$query->setFirstResult($offset)
->setMaxResults($limit);

$query->orderBy('l.id', 'DESC');

return $query->execute()->fetchAllAssociative();
}

private function prepareRelationQuery(
DoctrineQueryBuilder $query,
int $contentId,
?int $contentVersionNo = null,
?int $relationType = null
): DoctrineQueryBuilder {
$expr = $query->expr();
$query
->innerJoin(
'l',
'ezcontentobject',
'ezcontentobject_to',
$expr->andX(
'l.to_contentobject_id = ezcontentobject_to.id',
'ezcontentobject_to.status = :status'
self::CONTENT_ITEM_TABLE,
'c_to',
$expr->and(
'l.to_contentobject_id = c_to.id',
'c_to.status = :status'
)
)
->where(
->andWhere(
'l.from_contentobject_id = :content_id'
)
->setParameter(
Expand All @@ -1409,18 +1452,18 @@ public function loadRelations(
->setParameter('content_id', $contentId, ParameterType::INTEGER);

// source version number
if (null !== $contentVersionNo) {
if ($contentVersionNo !== null) {
$query
->andWhere('l.from_contentobject_version = :version_no')
->setParameter('version_no', $contentVersionNo, ParameterType::INTEGER);
} else {
// from published version only
$query
->innerJoin(
'ezcontentobject_to',
'ezcontentobject',
'c_to',
self::CONTENT_ITEM_TABLE,
'c',
$expr->andX(
$expr->and(
'c.id = l.from_contentobject_id',
'c.current_version = l.from_contentobject_version'
)
Expand All @@ -1442,7 +1485,7 @@ public function loadRelations(
->setParameter('relation_type', $relationType, ParameterType::INTEGER);
}

return $query->execute()->fetchAll(FetchMode::ASSOCIATIVE);
return $query;
}

public function countReverseRelations(int $toContentId, ?int $relationType = null): int
Expand Down
32 changes: 32 additions & 0 deletions src/lib/Persistence/Legacy/Content/Gateway/ExceptionConversion.php
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,38 @@ public function loadRelations(
}
}

public function countRelations(
int $contentId,
?int $contentVersionNo = null,
?int $relationType = null
): int {
try {
return $this->innerGateway->countRelations($contentId, $contentVersionNo, $relationType);
} catch (DBALException | PDOException $e) {
throw DatabaseException::wrap($e);
}
}

public function listRelations(
int $contentId,
int $limit,
int $offset = 0,
?int $contentVersionNo = null,
?int $relationType = null
): array {
try {
return $this->innerGateway->listRelations(
$contentId,
$limit,
$offset,
$contentVersionNo,
$relationType
);
} catch (DBALException | PDOException $e) {
throw DatabaseException::wrap($e);
}
}

public function countReverseRelations(int $contentId, ?int $relationType = null): int
{
try {
Expand Down
Loading
Loading