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

Enhanced location handling #87

Closed
wants to merge 22 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions doc/research/better_location_handling.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# Research: better location handling

Change how GraphQL and the query field load items so that they're always loaded _with_ a location. The goal is to better serve use-cases where the location is important, like getting a URL alias, or getting the results of a query field.

## GraphQL

What location is loaded depends on how the items are queried:

- Single item
- If a contentId or remoteId is used, loads the main location
- If a locationid or locationRemoteId is used, the requested location is used
- Collection
- By default, items are returned with their main location

## Open questions

### Tree root

If the siteaccess uses a custom tree root, the locations should always be within that subtree.

### Multiple locations

Probably the biggest challenge. What do we do if a Query Field, resolved with `findLocations()`, returns several locations for the same item ? Do we want the same item several times ? Does it make sense ?

Is it likely ? Multiple locations in the same tree would most likely be used to model something. Can we just switch to searching for locations without further changes ?

## Tech stuff

- Do we introduce a custom value object used when loading domain items, a composition of content + location ?
- Or is the location dynamically determined based on... some context ?
- That custom value object could just be `Location`, since it always refer to a content item.

## Backward compatibility
The main BC change is with collections: the items returned by the same GraphQL query may differ after this change.
The easiest way to cover it would be to make the new resolver's tags optional, as an opt-in mechanism for the 2.x
release of ezplatform-graphql.

What could the new resolver be like ?
- `LocationCollectionResolver`
But does it only handle collections ? It also handles fields values.
26 changes: 26 additions & 0 deletions features/location_handling/MultiSite.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
Feature: Handling of multiple locations in a multi-site environment

#Background: Given a frontend siteaccess "site_a"
# And a folder "/site_a"
# And the tree_root for the siteaccess "site_a" is set to "/site_a"
# And a frontend siteaccess "site_b"
# And the tree_root for the siteaccess "site_b" is set to "/site_b"
# And a folder "/global"

Background: Given I add a siteaccess "site_a" to "site_group" with settings
| key | value |
| content.tree_root.location_id | /site_a |
And I add a siteaccess "site_b" to "site_group" with settings
| key | value |
| content.tree_root.location_id | /site_b |

And the tree_root for the siteaccess "site_a" is set to "/site_a"
And a frontend siteaccess "site_b"
And the tree_root for the siteaccess "site_b" is set to "/site_b"
And a folder "/global"

Scenario: Queries return the location from the active siteaccess tree root
Given a content item "foo" with with its main location in "/global" and alternate locations in "/site_a" and "/site_b"
And a GraphQL consumer using the siteaccess "site_a"
When the content item "foo" is queried
Then it is returned with the location "/site_a/foo"
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@

namespace spec\EzSystems\EzPlatformGraphQL\Schema\Domain\Content\Worker\ContentType;

use EzSystems\EzPlatformGraphQL\Schema\Domain\Content\Worker\ContentType\AddDomainContentToDomainGroup;
use EzSystems\EzPlatformGraphQL\Schema\Domain\Content\Worker\ContentType\AddItemToDomainGroup;

class AddDomainContentToDomainGroupSpec extends ContentTypeWorkerBehavior
{
function it_is_initializable()
{
$this->shouldHaveType(AddDomainContentToDomainGroup::class);
$this->shouldHaveType(AddItemToDomainGroup::class);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

use EzSystems\EzPlatformGraphQL\Schema\Builder\SchemaBuilder;
use EzSystems\EzPlatformGraphQL\Schema\Domain\Content\NameHelper;
use EzSystems\EzPlatformGraphQL\Schema\Domain\Content\Worker\ContentType\DefineDomainContent;
use EzSystems\EzPlatformGraphQL\Schema\Domain\Content\Worker\ContentType\DefineItem;
use spec\EzSystems\EzPlatformGraphQL\Tools\ContentTypeArgument;
use spec\EzSystems\EzPlatformGraphQL\Tools\TypeArgument;
use Prophecy\Argument;
Expand All @@ -24,7 +24,7 @@ function let(NameHelper $nameHelper)

function it_is_initializable()
{
$this->shouldHaveType(DefineDomainContent::class);
$this->shouldHaveType(DefineItem::class);
}

function it_can_not_work_if_args_do_not_include_a_ContentTypeGroup(SchemaBuilder $schema)
Expand Down
54 changes: 54 additions & 0 deletions src/Exception/MultipleValidLocationsException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<?php
/**
* @copyright Copyright (C) eZ Systems AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/
declare(strict_types=1);

namespace EzSystems\EzPlatformGraphQL\Exception;

use eZ\Publish\API\Repository\Values\Content\Content;
use eZ\Publish\API\Repository\Values\Content\Location;

class MultipleValidLocationsException extends \Exception
{
/**
* @var \eZ\Publish\API\Repository\Values\Content\Location[]
*/
private $locations = [];

/**
* @var \eZ\Publish\API\Repository\Values\Content\Content
*/
private $content;

/**
* @param \eZ\Publish\API\Repository\Values\Content\Content $content
* @param \eZ\Publish\API\Repository\Values\Content\Location[] $locations
*/
public function __construct(Content $content, array $locations)
{
$this->locations = $locations;
parent::__construct(
sprintf(
'Could not determine which location to return for content with id %s. Possible candidates: %s)',
$content->id,
implode(',', array_map(function (Location $location) { return $location->pathString; }, $locations))
)
);
$this->content = $content;
}

/**
* @return Location[]
*/
public function getLocations(): array
{
return $this->locations;
}

public function getContent(): Content
{
return $this->content;
}
}
35 changes: 35 additions & 0 deletions src/Exception/NoValidLocationsException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php
/**
* @copyright Copyright (C) eZ Systems AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/
declare(strict_types=1);

namespace EzSystems\EzPlatformGraphQL\Exception;

use Exception;
use eZ\Publish\API\Repository\Values\Content\Content;

class NoValidLocationsException extends Exception
{
/**
* @var \eZ\Publish\API\Repository\Values\Content\Content|\eZ\Publish\API\Repository\Values\Content\Content[]
*/
private $content;

/**
* NoValidLocationsException constructor.
*
* @param \eZ\Publish\API\Repository\Values\Content\Content $content
*/
public function __construct(Content $content)
{
parent::__construct("No valid location could be determined for content #{$content->id}");
$this->content = $content;
}

public function getContent(): Content
{
return $this->content;
}
}
2 changes: 1 addition & 1 deletion src/GraphQL/DataLoader/CachedContentLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public function findSingle(Criterion $filter): Content
*
* @return int
*/
public function count(Query $query)
public function count(Query $query): int
{
return $this->innerLoader->count($query);
}
Expand Down
4 changes: 2 additions & 2 deletions src/GraphQL/DataLoader/ContentLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ interface ContentLoader
*
* @return \eZ\Publish\API\Repository\Values\Content\Content[]
*/
public function find(Query $query): array;
public function find(Query $query): iterable;

/**
* Loads a single content item given a Query Criterion.
Expand All @@ -35,5 +35,5 @@ public function findSingle(Criterion $criterion): Content;
*
* @return int
*/
public function count(Query $query);
public function count(Query $query): int;
}
4 changes: 2 additions & 2 deletions src/GraphQL/DataLoader/LocationLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ public function find(LocationQuery $query): array;
public function findById($id): Location;

/**
* Loads a single location by remote id.
* Loads a single content item given a Query Criterion.
*
* @param string $remoteId A location remote id
*/
public function findByRemoteId($remoteId): Location;

/**
* Loads a single location by url alias.
* @param string $remoteId A location remote id
*/
public function findByUrlAlias(string $urlAlias): Location;

Expand Down
2 changes: 1 addition & 1 deletion src/GraphQL/DataLoader/SearchContentLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public function findSingle(Criterion $filter): Content
*
* @throws ArgumentsException
*/
public function count(Query $query)
public function count(Query $query): int
{
$countQuery = clone $query;
$countQuery->limit = 0;
Expand Down
36 changes: 32 additions & 4 deletions src/GraphQL/DataLoader/SearchLocationLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@
use eZ\Publish\API\Repository\URLAliasService;
use eZ\Publish\API\Repository\Values\Content\Location;
use eZ\Publish\API\Repository\Values\Content\LocationQuery;
use eZ\Publish\API\Repository\Values\Content\Query;
use eZ\Publish\API\Repository\Values\Content\Search\SearchHit;
use eZ\Publish\API\Repository\Values\Content\URLAlias;
use eZ\Publish\Core\MVC\ConfigResolverInterface;
use eZ\Publish\Core\MVC\Symfony\Routing\Generator\UrlAliasGenerator;
use EzSystems\EzPlatformGraphQL\GraphQL\DataLoader\Exception\ArgumentsException;

/**
Expand All @@ -36,12 +37,22 @@ class SearchLocationLoader implements LocationLoader
* @var \eZ\Publish\API\Repository\URLAliasService
*/
private $urlAliasService;
/**
* @var \eZ\Publish\Core\MVC\ConfigResolverInterface
*/
private $configResolver;
/**
* @var \eZ\Publish\Core\MVC\Symfony\Routing\Generator\UrlAliasGenerator
*/
private $urlAliasGenerator;

public function __construct(SearchService $searchService, LocationService $locationService, URLAliasService $urlAliasService)
public function __construct(SearchService $searchService, LocationService $locationService, URLAliasService $urlAliasService, ConfigResolverInterface $configResolver, UrlAliasGenerator $urlAliasGenerator)
{
$this->searchService = $searchService;
$this->locationService = $locationService;
$this->urlAliasService = $urlAliasService;
$this->configResolver = $configResolver;
$this->urlAliasGenerator = $urlAliasGenerator;
}

public function find(LocationQuery $query): array
Expand Down Expand Up @@ -76,7 +87,7 @@ public function findByRemoteId($id): Location

public function findByUrlAlias(string $urlAlias): Location
{
$alias = $this->urlAliasService->lookup($urlAlias);
$alias = $this->getUrlAlias($urlAlias);

return ($alias->type == URLAlias::LOCATION)
? $this->locationService->loadLocation($alias->destination)
Expand Down Expand Up @@ -104,4 +115,21 @@ public function count(LocationQuery $query)
throw new ArgumentsException($e->getMessage(), $e->getCode(), $e);
}
}
}

protected function getUrlAlias($pathinfo): URLAlias
{
$rootLocationId = $this->configResolver->getParameter('content.tree_root.location_id');
$pathPrefix = $this->urlAliasGenerator->getPathPrefixByRootLocationId($rootLocationId);

if (
$rootLocationId !== null &&
!$this->urlAliasGenerator->isUriPrefixExcluded($pathinfo) &&
$pathPrefix !== '/'
) {
$urlAlias = $pathPrefix . $pathinfo;
} else {
$urlAlias = $pathinfo;
}

return $this->urlAliasService->lookup($urlAlias);
}}
59 changes: 59 additions & 0 deletions src/GraphQL/InputMapper/ContentCollectionFilterBuilder.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
<?php

/**
* @copyright Copyright (C) eZ Systems AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/
declare(strict_types=1);

namespace EzSystems\EzPlatformGraphQL\GraphQL\InputMapper;

use eZ\Publish\API\Repository\Repository;
use eZ\Publish\API\Repository\Values\Content\Query\Criterion;
use eZ\Publish\API\Repository\Values\Content\Query\Criterion\Subtree;
use eZ\Publish\API\Repository\Values\Content\URLAlias;
use eZ\Publish\Core\MVC\ConfigResolverInterface;

/**
* Builds the base query used to retrieve locations collections.
*
* @internal
*/
class ContentCollectionFilterBuilder
{
/**
* @var \eZ\Publish\Core\MVC\ConfigResolverInterface
*/
private $configResolver;

/**
* @var \eZ\Publish\API\Repository\Repository
*/
private $repository;

public function __construct(ConfigResolverInterface $configResolver, Repository $repository)
{
$this->configResolver = $configResolver;
$this->repository = $repository;
}

/**
* Returns a criterion to be added as a global 'and' to a query's filters.
*/
public function buildFilter(): Criterion
{
$treeRootLocationId = $this->configResolver->getParameter('content.tree_root.location_id');
$rootLocation = $this->repository->getLocationService()->loadLocation($treeRootLocationId);

$includedSubtrees = [$rootLocation->pathString ?? '/'];

foreach ($this->configResolver->getParameter('content.tree_root.excluded_uri_prefixes') as $uriPrefix) {
$urlAlias = $this->repository->getURLAliasService()->lookup($uriPrefix);
if ($urlAlias->type === URLAlias::LOCATION) {
$includedSubtrees[] = $this->repository->getLocationService()->loadLocation($urlAlias->destination)->pathString;
}
}

return new SubTree($includedSubtrees);
}
}
Loading