From d2982e40d122b08cf348506acc0ab90bb186e5f8 Mon Sep 17 00:00:00 2001
From: Coleman Watts <coleman@civicrm.org>
Date: Fri, 25 Jun 2021 21:19:06 -0400
Subject: [PATCH] APIv4 - Fix same-table joins and remove unused code

This fixes implicit joins between a table and itself, e.g. the Contact.employer_id which links to the Contact table
Also removes alpha code no longer used by APIv4.
---
 Civi/Api4/Service/Schema/SchemaMap.php        | 40 +++----------------
 tests/phpunit/api/v4/Action/FkJoinTest.php    | 28 +++++++++++++
 .../v4/Service/Schema/SchemaMapperTest.php    | 28 -------------
 3 files changed, 33 insertions(+), 63 deletions(-)

diff --git a/Civi/Api4/Service/Schema/SchemaMap.php b/Civi/Api4/Service/Schema/SchemaMap.php
index f063afda2e..ff360f12bd 100644
--- a/Civi/Api4/Service/Schema/SchemaMap.php
+++ b/Civi/Api4/Service/Schema/SchemaMap.php
@@ -36,7 +36,7 @@ class SchemaMap {
       return $path;
     }
 
-    $this->findPaths($table, $targetTableAlias, 1, $path);
+    $this->findPaths($table, $targetTableAlias, $path);
 
     return $path;
   }
@@ -88,49 +88,19 @@ class SchemaMap {
   }
 
   /**
-   * 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;
-    $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;
       }
     }
   }
diff --git a/tests/phpunit/api/v4/Action/FkJoinTest.php b/tests/phpunit/api/v4/Action/FkJoinTest.php
index efadad0475..ead580dcfe 100644
--- a/tests/phpunit/api/v4/Action/FkJoinTest.php
+++ b/tests/phpunit/api/v4/Action/FkJoinTest.php
@@ -308,7 +308,35 @@ class FkJoinTest extends UnitTestCase {
     $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']);
   }
 
 }
diff --git a/tests/phpunit/api/v4/Service/Schema/SchemaMapperTest.php b/tests/phpunit/api/v4/Service/Schema/SchemaMapperTest.php
index d45c6f4fc6..1dc6d13e55 100644
--- a/tests/phpunit/api/v4/Service/Schema/SchemaMapperTest.php
+++ b/tests/phpunit/api/v4/Service/Schema/SchemaMapperTest.php
@@ -46,34 +46,6 @@ class SchemaMapperTest extends UnitTestCase {
     $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');
-- 
2.25.1