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.

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

APIv4 - Fix same-table joins and remove unused code #20715

Merged
merged 1 commit into from
Jun 30, 2021
+33 −63
Merged

APIv4 - Fix same-table joins and remove unused code #20715

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 5 additions & 35 deletions Civi/Api4/Service/Schema/SchemaMap.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public function getPath($baseTableName, $targetTableAlias) {
return $path;
}

$this->findPaths($table, $targetTableAlias, 1, $path);
$this->findPaths($table, $targetTableAlias, $path);

return $path;
}
Expand Down Expand Up @@ -88,49 +88,19 @@ public function addTables(array $tables) {
}

/**
* Recursive function to traverse the schema looking for a path
* Traverse the schema looking for a path
*
* @param Table $table
* The current table to base fromm
* @param string $target
* The target joinable table alias
* @param int $depth
* The current level of recursion which reflects the number of joins needed
* @param \Civi\Api4\Service\Schema\Joinable\Joinable[] $path
* (By-reference) The possible paths to the target table
* @param \Civi\Api4\Service\Schema\Joinable\Joinable[] $currentPath
* For internal use only to track the path to reach the target table
*/
private function findPaths(Table $table, $target, $depth, &$path, $currentPath = []
) {
static $visited = [];

// reset if new call
if ($depth === 1) {
$visited = [];
}

$canBeShorter = empty($path) || count($currentPath) + 1 < count($path);
$tooFar = $depth > self::MAX_JOIN_DEPTH;
Copy link
Contributor

Choose a reason for hiding this comment

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

@colemanw this seems to be the only thing I wonder about with this, This seems to be something about trying to contort with something in MySQL. Is that replicated somewhere else?

Copy link
Member Author

Choose a reason for hiding this comment

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

The long complicated code here was all made to handle a type of join which no longer exists in APIv4.
During alpha development, there was work done to support a pseudo-join where the api would run the main query, collect the ids and do a second query behind-the-scenes to gather e.g. contact.activities. This functionality has been dropped in favor of explicit joins.

$beenHere = in_array($table->getName(), $visited);

if ($tooFar || $beenHere || !$canBeShorter) {
return;
}

// prevent circular reference
$visited[] = $table->getName();

foreach ($table->getExternalLinks() as $link) {
private function findPaths(Table $table, $target, &$path) {
foreach ($table->getTableLinks() as $link) {
if ($link->getAlias() === $target) {
$path = array_merge($currentPath, [$link]);
}
else {
$linkTable = $this->getTableByName($link->getTargetTable());
if ($linkTable) {
$nextStep = array_merge($currentPath, [$link]);
$this->findPaths($linkTable, $target, $depth + 1, $path, $nextStep);
}
$path[] = $link;
}
}
}
Expand Down
28 changes: 28 additions & 0 deletions tests/phpunit/api/v4/Action/FkJoinTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,35 @@ public function testBridgeJoinRelationshipContactActivity() {
$this->assertNull($result[3]['rel.id']);
$this->assertEquals($cid3, $result[4]['contact.id']);
$this->assertNull($result[3]['rel.id']);
}

public function testJoinToEmployerId() {
$employer = Contact::create(FALSE)
->addValue('contact_type', 'Organization')
->addValue('organization_name', 'TesterCo')
->execute()->first()['id'];
$employee = Contact::create(FALSE)
->addValue('employer_id', $employer)
->addValue('first_name', 'TesterMan')
->execute()->first()['id'];
$email = Email::create(FALSE)
->addValue('email', 'tester@test.com')
->addValue('contact_id', $employee)
->execute()->first()['id'];

$contactGet = Contact::get(FALSE)
->addWhere('id', '=', $employee)
->addSelect('employer_id', 'employer_id.display_name')
->execute()->first();
$this->assertEquals($employer, $contactGet['employer_id']);
$this->assertEquals('TesterCo', $contactGet['employer_id.display_name']);

$emailGet = Email::get(FALSE)
->addWhere('id', '=', $email)
->addSelect('contact_id.employer_id', 'contact_id.employer_id.display_name')
->execute()->first();
$this->assertEquals($employer, $emailGet['contact_id.employer_id']);
$this->assertEquals('TesterCo', $emailGet['contact_id.employer_id.display_name']);
}

}
28 changes: 0 additions & 28 deletions tests/phpunit/api/v4/Service/Schema/SchemaMapperTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,34 +46,6 @@ public function testWillHavePathWithSingleJump() {
$this->assertNotEmpty($map->getPath('civicrm_phone', 'location'));
}

public function testWillHavePathWithDoubleJump() {
$activity = new Table('activity');
$activityContact = new Table('activity_contact');
$middleLink = new Joinable('activity_contact', 'activity_id');
$contactLink = new Joinable('contact', 'id');
$activity->addTableLink('id', $middleLink);
$activityContact->addTableLink('contact_id', $contactLink);

$map = new SchemaMap();
$map->addTables([$activity, $activityContact]);

$this->assertNotEmpty($map->getPath('activity', 'contact'));
}

public function testPathWithTripleJoin() {
$first = new Table('first');
$second = new Table('second');
$third = new Table('third');
$first->addTableLink('id', new Joinable('second', 'id'));
$second->addTableLink('id', new Joinable('third', 'id'));
$third->addTableLink('id', new Joinable('fourth', 'id'));

$map = new SchemaMap();
$map->addTables([$first, $second, $third]);

$this->assertNotEmpty($map->getPath('first', 'fourth'));
}

public function testCircularReferenceWillNotBreakIt() {
$contactTable = new Table('contact');
$carTable = new Table('car');
Expand Down