fix(review): #4580 union-path facet skip, round 3

lib/Db/MagicMapper/MagicFacetHandler.php

@@ -750,7 +750,7 @@ private function getTermsFacetUnion(
 			$labelMap = [];
 			$firstConfig = reset($tableConfigs);
 			if ($isMetadata === false
-				&& $this->holdsObjectReferences(schema: $firstConfig['schema'], column: $field) === false
+				&& $this->anyHoldsObjectReferences(tableConfigs: $tableConfigs, column: $field) === false
 			) {
 				$uuidsToResolve = [];
 			}
@@ -1490,7 +1490,7 @@ private function getTermsFacet(
 	 *
 	 * @return bool False when the property declares a referenceType.
 	 *
-	 * @spec exclude performance guard on facet label resolution; labels for object references are unchanged
+	 * @spec exclude facet label performance guard; object-reference labels unchanged, a UUID-shaped integration id is labelled in full
 	 */
 	private function holdsObjectReferences(Schema $schema, string $column): bool {
 		foreach ($schema->getProperties() ?? [] as $name => $property) {
@@ -1505,6 +1505,30 @@ private function holdsObjectReferences(Schema $schema, string $column): bool {
 		return true;
 	}//end holdsObjectReferences()
 
+	/**
+	 * Whether any table of a union facet can hold object references in a column.
+	 *
+	 * Two schemas in one union can share a column name with a different meaning,
+	 * so the name lookup is skipped only when every schema declares a
+	 * referenceType for it; one schema holding object references keeps it.
+	 *
+	 * @param array<int, array{schema: Schema}> $tableConfigs The union's table configurations.
+	 * @param string                            $column       The column name as the facet queries it.
+	 *
+	 * @return bool False only when every schema declares a referenceType for the column.
+	 *
+	 * @spec exclude performance guard on facet label resolution, the multi-schema form of holdsObjectReferences()
+	 */
+	private function anyHoldsObjectReferences(array $tableConfigs, string $column): bool {
+		foreach ($tableConfigs as $config) {
+			if ($this->holdsObjectReferences(schema: $config['schema'], column: $column) === true) {
+				return true;
+			}
+		}
+
+		return false;
+	}//end anyHoldsObjectReferences()
+
 	/**
 	 * Pick the label for one terms bucket.
 	 *

tests/Unit/Db/MagicFacetHandlerBucketLabelTest.php

@@ -189,4 +189,113 @@ public function testIntegrationReferencesAreNotLookedUpAsObjects(): void {
 			array_column($facet['buckets'], 'label', 'key')
 		);
 	}//end testIntegrationReferencesAreNotLookedUpAsObjects()
+
+	/**
+	 * Run getTermsFacetUnion() over the given rows, one table per property set.
+	 *
+	 * @param array<int, array<string, mixed>>     $rows         The union rows the database returns.
+	 * @param CacheHandler                         $cacheHandler The name resolver.
+	 * @param string                               $column       The faceted column.
+	 * @param array<int, array<string, array>>     $propertySets One schema's property definitions per table.
+	 *
+	 * @return array<string, mixed> The facet result.
+	 */
+	private function unionFacetOver(
+		array $rows,
+		CacheHandler $cacheHandler,
+		string $column,
+		array $propertySets,
+	): array {
+		$db = $this->createMock(IDBConnection::class);
+		$db->method('getDatabasePlatform')->willReturn($this->createMock(PostgreSQLPlatform::class));
+
+		$rows[] = false;
+		$db->method('prepare')->willReturnCallback(
+			function (string $sql) use ($column, $rows): IPreparedStatement {
+				$statement = $this->createMock(IPreparedStatement::class);
+				$statement->method('execute')->willReturn($this->createMock(IResult::class));
+				if (str_contains($sql, 'information_schema') === true) {
+					$statement->method('fetch')->willReturnOnConsecutiveCalls(['col' => $column], false);
+					return $statement;
+				}
+
+				$statement->method('fetch')->willReturnOnConsecutiveCalls(...$rows);
+				return $statement;
+			}
+		);
+
+		$handler = new MagicFacetHandler(
+			db: $db,
+			logger: $this->createMock(LoggerInterface::class),
+			cacheHandler: $cacheHandler
+		);
+
+		$tableConfigs = [];
+		foreach ($propertySets as $index => $properties) {
+			$schema = $this->createMock(Schema::class);
+			$schema->method('getProperties')->willReturn($properties);
+			$tableConfigs[] = [
+				'tableName' => 'openregister_table_1_'.($index + 2),
+				'schema' => $schema,
+				'register' => $this->createMock(Register::class),
+			];
+		}
+
+		$method = new ReflectionMethod(MagicFacetHandler::class, 'getTermsFacetUnion');
+		$method->setAccessible(true);
+
+		return $method->invoke($handler, $tableConfigs, $column, [], $tableConfigs[0]['schema'], false);
+	}//end unionFacetOver()
+
+	/**
+	 * On the multi-schema union path a referenceType property is not looked up
+	 * as object names either.
+	 *
+	 * @return void
+	 */
+	public function testUnionIntegrationReferencesAreNotLookedUpAsObjects(): void {
+		$cacheHandler = $this->createMock(CacheHandler::class);
+		$cacheHandler->expects($this->never())->method('getMultipleObjectNames');
+
+		$facet = $this->unionFacetOver(
+			[
+				['facet_value' => self::DANGLING_A, 'doc_count' => 2],
+				['facet_value' => 'behandelaars', 'doc_count' => 4],
+			],
+			$cacheHandler,
+			'assigned_group',
+			[['assignedGroup' => ['type' => 'string', 'referenceType' => 'nextcloud-group']]]
+		);
+
+		$this->assertEquals(
+			[self::DANGLING_A => self::DANGLING_A, 'behandelaars' => 'behandelaars'],
+			array_column($facet['buckets'], 'label', 'key')
+		);
+	}//end testUnionIntegrationReferencesAreNotLookedUpAsObjects()
+
+	/**
+	 * When one schema of the union holds object references in the column, the
+	 * name lookup still runs, even if the first schema declares a referenceType.
+	 *
+	 * @return void
+	 */
+	public function testUnionKeepsTheLookupWhenAnySchemaHoldsObjectReferences(): void {
+		$cacheHandler = $this->createMock(CacheHandler::class);
+		$cacheHandler->expects($this->once())
+			->method('getMultipleObjectNames')
+			->with([self::KNOWN])
+			->willReturn([self::KNOWN => 'Ontvangen']);
+
+		$facet = $this->unionFacetOver(
+			[['facet_value' => self::KNOWN, 'doc_count' => 3]],
+			$cacheHandler,
+			'assigned_group',
+			[
+				['assignedGroup' => ['type' => 'string', 'referenceType' => 'nextcloud-group']],
+				['assignedGroup' => ['type' => 'string', 'format' => 'uuid', '$ref' => 'group']],
+			]
+		);
+
+		$this->assertSame('Ontvangen', array_column($facet['buckets'], 'label', 'key')[self::KNOWN]);
+	}//end testUnionKeepsTheLookupWhenAnySchemaHoldsObjectReferences()
 }//end class