AttributeDriver ignores index and unique constraint definitions in Table attribute

[DaDeather]

Bug Report

Q	A
BC Break	no
Version	>=2.9.x

Summary

Since the implementation of the AttributeDriver there's a bug that the properties of the Table attribute aren't counted in for the indices and unique constraints.

Current behavior

Currently setting the indices and unique constraints through the Table attribute results in them being ignored which therefore disallows the usage of nested attributes.

This is being handled here:

orm/src/Mapping/Driver/AttributeDriver.php

Lines 165 to 173 in a809a71

	if (isset($classAttributes[Mapping\Table::class])) {
	$tableAnnot = $classAttributes[Mapping\Table::class];
	$primaryTable['name'] = $tableAnnot->name;
	$primaryTable['schema'] = $tableAnnot->schema;
	if ($tableAnnot->options) {
	$primaryTable['options'] = $tableAnnot->options;
	}
	}

Before with annotations, this was possible and the code for the correct interpretation of these was implemented in the AnnotationDriver:

orm/src/Mapping/Driver/AnnotationDriver.php

Lines 137 to 224 in a809a71

	if (isset($classAnnotations[Mapping\Table::class])) {
	$tableAnnot = $classAnnotations[Mapping\Table::class];
	assert($tableAnnot instanceof Mapping\Table);
	$primaryTable = [
	'name' => $tableAnnot->name,
	'schema' => $tableAnnot->schema,
	];
	foreach ($tableAnnot->indexes ?? [] as $indexAnnot) {
	$index = [];
	if (! empty($indexAnnot->columns)) {
	$index['columns'] = $indexAnnot->columns;
	}
	if (! empty($indexAnnot->fields)) {
	$index['fields'] = $indexAnnot->fields;
	}
	if (
	isset($index['columns'], $index['fields'])
	|| (
	! isset($index['columns'])
	&& ! isset($index['fields'])
	)
	) {
	throw MappingException::invalidIndexConfiguration(
	$className,
	(string) ($indexAnnot->name ?? count($primaryTable['indexes']))
	);
	}
	if (! empty($indexAnnot->flags)) {
	$index['flags'] = $indexAnnot->flags;
	}
	if (! empty($indexAnnot->options)) {
	$index['options'] = $indexAnnot->options;
	}
	if (! empty($indexAnnot->name)) {
	$primaryTable['indexes'][$indexAnnot->name] = $index;
	} else {
	$primaryTable['indexes'][] = $index;
	}
	}
	foreach ($tableAnnot->uniqueConstraints ?? [] as $uniqueConstraintAnnot) {
	$uniqueConstraint = [];
	if (! empty($uniqueConstraintAnnot->columns)) {
	$uniqueConstraint['columns'] = $uniqueConstraintAnnot->columns;
	}
	if (! empty($uniqueConstraintAnnot->fields)) {
	$uniqueConstraint['fields'] = $uniqueConstraintAnnot->fields;
	}
	if (
	isset($uniqueConstraint['columns'], $uniqueConstraint['fields'])
	|| (
	! isset($uniqueConstraint['columns'])
	&& ! isset($uniqueConstraint['fields'])
	)
	) {
	throw MappingException::invalidUniqueConstraintConfiguration(
	$className,
	(string) ($uniqueConstraintAnnot->name ?? count($primaryTable['uniqueConstraints']))
	);
	}
	if (! empty($uniqueConstraintAnnot->options)) {
	$uniqueConstraint['options'] = $uniqueConstraintAnnot->options;
	}
	if (! empty($uniqueConstraintAnnot->name)) {
	$primaryTable['uniqueConstraints'][$uniqueConstraintAnnot->name] = $uniqueConstraint;
	} else {
	$primaryTable['uniqueConstraints'][] = $uniqueConstraint;
	}
	}
	if ($tableAnnot->options) {
	$primaryTable['options'] = $tableAnnot->options;
	}
	$metadata->setPrimaryTable($primaryTable);
	}

How to reproduce

Creating an entity with the following attributes:

#[Orm\Entity()]
#[Orm\Table(
    'some_entities',
    indexes: [
        new Orm\Index(['some_column_1'], name: 'some_name_1'),
        new Orm\Index(['some_column_2'], name: 'some_name_2'),
    ],
    uniqueConstraints: [
        new Orm\UniqueConstraint('some_unique_constraint', ['some_column_3']),
    ],
)]
class SomeEntity
{
    // [...]
}

This wouldn't be handled by the AttributeDriver.

But doing it this way works:

#[Orm\Entity()]
#[Orm\Table('some_entities')]
#[Orm\Index(['some_column_1'], name: 'some_name_1')]
#[Orm\Index(['some_column_2'], name: 'some_name_2')]
#[Orm\UniqueConstraint('some_unique_constraint', ['some_column_3'])]
class SomeEntity
{
    // [...]
}

Expected behavior

The nested attribute variant should also work as expected if indices and / or unique constrains are defined within the Table attribute.

OR if this is unwanted behavior it should be removed to reduce confusions in terms of defining the table definitions.

[DaDeather]

Relates to:
PR - #9241
Issue - #9240

[derrabus]

We should remove those properties indexes and uniqueConstraints indeed as they don't have a function anymore. We should not provide two ways of achieving the same thing. Do you want to work on a PR?

[DaDeather]

We should remove those properties indexes and uniqueConstraints indeed as they don't have a function anymore. We should not provide two ways of achieving the same thing. Do you want to work on a PR?

As we both agree on it being unnecessary to have 2 different approaches to the same, I've created a PR to remove these properties from the Table attribute instead to reduce unnecessary complexity and confusion 👍.