Skip to content

Fix relationship select #632

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.

Sign up for GitHub

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

Merged
abnegate merged 4 commits into from
Jul 22, 2025
Merged

Fix relationship select #632

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
26f86bd
Fix relationship selection not accounting for side
abnegate Jul 22, 2025
35e4208
Add tests
abnegate Jul 22, 2025
7e8a0c1
Fix stan
abnegate Jul 22, 2025
f72e855
Improve query processing
abnegate Jul 22, 2025
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
152 changes: 68 additions & 84 deletions src/Database/Database.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3221,43 +3221,7 @@ public function getDocument(string $collection, string $id, array $queries = [],

$selects = Query::groupByType($queries)['selections'];
$selections = $this->validateSelections($collection, $selects);
$nestedSelections = [];

foreach ($queries as $query) {
if ($query->getMethod() == Query::TYPE_SELECT) {
$values = $query->getValues();
foreach ($values as $valueIndex => $value) {
if (\str_contains($value, '.')) {
// Shift the top level off the dot-path to pass the selection down the chain
// 'foo.bar.baz' becomes 'bar.baz'
$nestedSelections[] = Query::select([
\implode('.', \array_slice(\explode('.', $value), 1))
]);

$key = \explode('.', $value)[0];

foreach ($relationships as $relationship) {
if ($relationship->getAttribute('key') === $key) {
switch ($relationship->getAttribute('options')['relationType']) {
case Database::RELATION_MANY_TO_MANY:
case Database::RELATION_ONE_TO_MANY:
unset($values[$valueIndex]);
break;

case Database::RELATION_MANY_TO_ONE:
case Database::RELATION_ONE_TO_ONE:
$values[$valueIndex] = $key;
break;
}
}
}
}
}
$query->setValues(\array_values($values));
}
}

$queries = \array_values($queries);
$nestedSelections = $this->processRelationshipQueries($relationships, $queries);

$validator = new Authorization(self::PERMISSION_READ);
$documentSecurity = $collection->getAttribute('documentSecurity', false);
Expand Down Expand Up @@ -6061,50 +6025,7 @@ public function find(string $collection, array $queries = [], string $forPermiss
);

$selections = $this->validateSelections($collection, $selects);
$nestedSelections = [];

foreach ($queries as $index => &$query) {
switch ($query->getMethod()) {
case Query::TYPE_SELECT:
$values = $query->getValues();
foreach ($values as $valueIndex => $value) {
if (\str_contains($value, '.')) {
// Shift the top level off the dot-path to pass the selection down the chain
// 'foo.bar.baz' becomes 'bar.baz'
$nestedSelections[] = Query::select([
\implode('.', \array_slice(\explode('.', $value), 1))
]);

$key = \explode('.', $value)[0];

foreach ($relationships as $relationship) {
if ($relationship->getAttribute('key') === $key) {
switch ($relationship->getAttribute('options')['relationType']) {
case Database::RELATION_MANY_TO_MANY:
case Database::RELATION_ONE_TO_MANY:
unset($values[$valueIndex]);
break;

case Database::RELATION_MANY_TO_ONE:
case Database::RELATION_ONE_TO_ONE:
$values[$valueIndex] = $key;
break;
}
}
}
}
}
$query->setValues(\array_values($values));
break;
default:
if (\str_contains($query->getAttribute(), '.')) {
unset($queries[$index]);
}
break;
}
}

$queries = \array_values($queries);
$nestedSelections = $this->processRelationshipQueries($relationships, $queries);

$getResults = fn () => $this->adapter->find(
$collection->getId(),
Expand Down Expand Up @@ -6133,8 +6054,6 @@ public function find(string $collection, array $queries = [], string $forPermiss
}
}

unset($query);

$this->trigger(self::EVENT_DOCUMENT_FIND, $results);

return $results;
Expand Down Expand Up @@ -6786,7 +6705,7 @@ public function getCacheKeys(string $collectionId, ?string $documentId = null, a
* @return void
* @throws QueryException
*/
public function checkQueriesType(array $queries)
private function checkQueriesType(array $queries): void
{
foreach ($queries as $query) {
if (!$query instanceof Query) {
Expand All @@ -6798,4 +6717,69 @@ public function checkQueriesType(array $queries)
}
}
}

/**
* Process relationship queries, extracting nested selections.
*
* @param array<Document> $relationships
* @param array<Query> $queries
* @return array<Query>
*/
private function processRelationshipQueries(
array $relationships,
array $queries,
): array {
$nestedSelections = [];

foreach ($queries as $query) {
if ($query->getMethod() == Query::TYPE_SELECT) {
$values = $query->getValues();
foreach ($values as $valueIndex => $value) {
if (\str_contains($value, '.')) {
Copy link
Copy Markdown
Contributor

@fogelito fogelito Jul 22, 2025

Choose a reason for hiding this comment

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

This can be a legit attribute name with DOT.
Can you move this check inside foreach ($relationships as $relationship) ?
So we know it must be a relationship attribute?

// Shift the top level off the dot-path to pass the selection down the chain
// 'foo.bar.baz' becomes 'bar.baz'
$nestedSelections[] = Query::select([
\implode('.', \array_slice(\explode('.', $value), 1))
]);

$selectedKey = \explode('.', $value)[0];

foreach ($relationships as $relationship) {
$key = $relationship->getAttribute('key');
$type = $relationship->getAttribute('options')['relationType'];
$side = $relationship->getAttribute('options')['side'];

if ($key === $selectedKey) {
switch ($type) {
case Database::RELATION_MANY_TO_MANY:
unset($values[$valueIndex]);
break;
case Database::RELATION_ONE_TO_MANY:
if ($side === Database::RELATION_SIDE_PARENT) {
unset($values[$valueIndex]);
} else {
$values[$valueIndex] = $selectedKey;
}
break;
case Database::RELATION_MANY_TO_ONE:
if ($side === Database::RELATION_SIDE_PARENT) {
$values[$valueIndex] = $selectedKey;
} else {
unset($values[$valueIndex]);
}
break;
case Database::RELATION_ONE_TO_ONE:
$values[$valueIndex] = $selectedKey;
break;
}
}
}
}
}
$query->setValues(\array_values($values));
}
}

return $nestedSelections;
}
}
67 changes: 65 additions & 2 deletions tests/e2e/Adapter/Scopes/RelationshipTests.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -931,14 +931,17 @@ public function testSelectRelationshipAttributes(): void
$database->createCollection('model');

$database->createAttribute('make', 'name', Database::VAR_STRING, 255, true);
$database->createAttribute('make', 'origin', Database::VAR_STRING, 255, true);
$database->createAttribute('model', 'name', Database::VAR_STRING, 255, true);
$database->createAttribute('model', 'year', Database::VAR_INTEGER, 0, true);

$database->createRelationship(
collection: 'make',
relatedCollection: 'model',
type: Database::RELATION_ONE_TO_MANY,
id: 'models'
twoWay: true,
id: 'models',
twoWayKey: 'make',
);

$database->createDocument('make', new Document([
Expand All @@ -947,6 +950,7 @@ public function testSelectRelationshipAttributes(): void
Permission::read(Role::any()),
],
'name' => 'Ford',
'origin' => 'USA',
'models' => [
[
'$id' => 'fiesta',
Expand Down Expand Up @@ -1143,9 +1147,68 @@ public function testSelectRelationshipAttributes(): void
if ($make->isEmpty()) {
throw new Exception('Make not found');
}

$this->assertEquals('Ford', $make['name']);
$this->assertArrayNotHasKey('models', $make);

// Select some parent attributes, all child attributes
$make = $database->findOne('make', [
Query::select(['name', 'models.*']),
]);

$this->assertEquals('Ford', $make['name']);
$this->assertEquals(2, \count($make['models']));

/*
* FROM CHILD TO PARENT
*/

// Select some parent attributes, some child attributes
$model = $database->findOne('model', [
Query::select(['name', 'make.name']),
]);

$this->assertEquals('Fiesta', $model['name']);
$this->assertEquals('Ford', $model['make']['name']);
$this->assertArrayNotHasKey('origin', $model['make']);
$this->assertArrayNotHasKey('year', $model);
$this->assertArrayHasKey('name', $model);

// Select all parent attributes, some child attributes
$model = $database->findOne('model', [
Query::select(['*', 'make.name']),
]);

$this->assertEquals('Fiesta', $model['name']);
$this->assertEquals('Ford', $model['make']['name']);
$this->assertArrayHasKey('year', $model);

// Select all parent attributes, all child attributes
$model = $database->findOne('model', [
Query::select(['*', 'make.*']),
]);

$this->assertEquals('Fiesta', $model['name']);
$this->assertEquals('Ford', $model['make']['name']);
$this->assertArrayHasKey('year', $model);
$this->assertArrayHasKey('name', $model['make']);

// Select all parent attributes, no child attributes
$model = $database->findOne('model', [
Query::select(['*']),
]);

$this->assertEquals('Fiesta', $model['name']);
$this->assertArrayHasKey('make', $model);
$this->assertArrayHasKey('year', $model);

// Select some parent attributes, all child attributes
$model = $database->findOne('model', [
Query::select(['name', 'make.*']),
]);

$this->assertEquals('Fiesta', $model['name']);
$this->assertEquals('Ford', $model['make']['name']);
$this->assertEquals('USA', $model['make']['origin']);
}

public function testInheritRelationshipPermissions(): void
Expand Down