From 7c6f0ec1128018ae52918ee87d092d0dd6b7e4b7 Mon Sep 17 00:00:00 2001 From: Chad Sikorra Date: Sun, 18 Oct 2026 14:56:03 -0400 Subject: [PATCH 1/5] Match linked values as DNs when removing them in memory. --- .../Storage/Adapter/InMemoryStorage.php | 24 ++++- .../Adapter/Pdo/Writer/EntryLinkWriter.php | 19 ++-- .../Concern/LinkedAttributeTestsTrait.php | 18 ++++ .../Storage/Pdo/EntryLinkWriterTest.php | 91 +++++++++++++++++++ .../Storage/Adapter/InMemoryStorageTest.php | 20 ++++ 5 files changed, 163 insertions(+), 9 deletions(-) create mode 100644 tests/integration/Storage/Pdo/EntryLinkWriterTest.php diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/InMemoryStorage.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/InMemoryStorage.php index 5625d91f..75d9c262 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/InMemoryStorage.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/InMemoryStorage.php @@ -438,6 +438,28 @@ private function linkedValuesOf( ))); } + /** + * @param array $held + * @param list $removed + * + * @return list + */ + private static function withoutLinkValues( + array $held, + array $removed, + ): array { + $gone = []; + + foreach ($removed as $value) { + $gone[Dn::normalizedOrNull($value) ?? $value] = true; + } + + return array_values(array_filter( + $held, + static fn(string $value): bool => !isset($gone[Dn::normalizedOrNull($value) ?? $value]), + )); + } + /** * The entry carrying the linked values the one it replaces held. */ @@ -479,7 +501,7 @@ private function withLinks( foreach ($links->names() as $name) { $values = array_merge( - array_diff( + self::withoutLinkValues( $previous?->get($name, true)?->getValues() ?? [], $links->removed($name), ), diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Pdo/Writer/EntryLinkWriter.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Pdo/Writer/EntryLinkWriter.php index 105f9e34..e8e731d1 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Pdo/Writer/EntryLinkWriter.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Pdo/Writer/EntryLinkWriter.php @@ -172,14 +172,17 @@ public function heldLinkValues( return []; } $held = []; - $params = [ - $owner->normalizedString(), - Attribute::normalizeName($attribute), - ...array_keys($asked), - ]; - - foreach ($this->rowsOf($this->dialect->queryHeldLinkValues(count($asked)), $params) as $row) { - $held[] = $asked[$this->stringColumn($row['lc_dn'] ?? null)] ?? null; + + foreach (array_chunk(array_keys($asked), self::DNS_PER_STATEMENT) as $chunk) { + $params = [ + $owner->normalizedString(), + Attribute::normalizeName($attribute), + ...$chunk, + ]; + + foreach ($this->rowsOf($this->dialect->queryHeldLinkValues(count($chunk)), $params) as $row) { + $held[] = $asked[$this->stringColumn($row['lc_dn'] ?? null)] ?? null; + } } return array_values(array_filter($held)); diff --git a/tests/integration/Storage/Concern/LinkedAttributeTestsTrait.php b/tests/integration/Storage/Concern/LinkedAttributeTestsTrait.php index 5cdda50f..5a30e893 100644 --- a/tests/integration/Storage/Concern/LinkedAttributeTestsTrait.php +++ b/tests/integration/Storage/Concern/LinkedAttributeTestsTrait.php @@ -67,6 +67,24 @@ public function testDeletingAMemberLeavesTheRestLinked(): void ); } + public function testDeletingAMemberInAnotherSpellingRemovesIt(): void + { + $group = $this->seedGroup( + 'link-delete-spelling', + self::LINKED_ADMIN, + ); + + $this->ldapClient()->send(Operations::modify( + $group, + Change::delete('member', 'CN=User, DC=foo, DC=bar'), + )); + + self::assertSame( + [self::LINKED_ADMIN], + $this->membersOf($group), + ); + } + public function testReplacingTheMembersWritesThemWhole(): void { $group = $this->seedGroup('link-replace'); diff --git a/tests/integration/Storage/Pdo/EntryLinkWriterTest.php b/tests/integration/Storage/Pdo/EntryLinkWriterTest.php new file mode 100644 index 00000000..ec8bc1b3 --- /dev/null +++ b/tests/integration/Storage/Pdo/EntryLinkWriterTest.php @@ -0,0 +1,91 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Tests\Integration\FreeDSx\Ldap\Storage\Pdo; + +use FreeDSx\Ldap\Container; +use FreeDSx\Ldap\Entry\Attribute; +use FreeDSx\Ldap\Entry\Dn; +use FreeDSx\Ldap\Entry\Entry; +use FreeDSx\Ldap\Server\Backend\Storage\Adapter\Pdo\Writer\EntryLinkWriter; +use FreeDSx\Ldap\Server\Backend\Storage\Import\LdapImporter; +use PHPUnit\Framework\TestCase; +use Tests\Support\FreeDSx\Ldap\Server\Configuration\TestServerOptions; + +final class EntryLinkWriterTest extends TestCase +{ + private const BASE = 'dc=example,dc=com'; + + private const ADMINS = 'cn=Admins,dc=example,dc=com'; + + private const BOB = 'cn=Bob,dc=example,dc=com'; + + private EntryLinkWriter $subject; + + protected function setUp(): void + { + $container = Container::forServer(TestServerOptions::sqlite()); + $this->subject = $container->get(EntryLinkWriter::class); + + $container->get(LdapImporter::class)->importEntries([ + new Entry( + new Dn(self::BASE), + new Attribute('dc', 'example'), + ), + new Entry( + new Dn(self::BOB), + new Attribute('objectClass', 'person'), + new Attribute('cn', 'Bob'), + new Attribute('sn', 'Bob'), + ), + new Entry( + new Dn(self::ADMINS), + new Attribute('objectClass', 'groupOfNames'), + new Attribute('cn', 'Admins'), + new Attribute('member', self::BOB), + ), + ]); + } + + public function test_a_held_value_is_found_past_the_first_statement_of_values(): void + { + $asked = []; + + for ($i = 0; $i < 1200; $i++) { + $asked[] = $i === 1100 + ? 'CN=Bob, DC=example, DC=com' + : "cn=nobody{$i}," . self::BASE; + } + + self::assertSame( + ['CN=Bob, DC=example, DC=com'], + $this->subject->heldLinkValues( + new Dn(self::ADMINS), + 'member', + $asked, + ), + ); + } + + public function test_a_value_the_owner_does_not_link_is_not_held(): void + { + self::assertSame( + [], + $this->subject->heldLinkValues( + new Dn(self::ADMINS), + 'member', + ['cn=Carol,' . self::BASE], + ), + ); + } +} diff --git a/tests/unit/Server/Backend/Storage/Adapter/InMemoryStorageTest.php b/tests/unit/Server/Backend/Storage/Adapter/InMemoryStorageTest.php index 651265d4..b5e9235d 100644 --- a/tests/unit/Server/Backend/Storage/Adapter/InMemoryStorageTest.php +++ b/tests/unit/Server/Backend/Storage/Adapter/InMemoryStorageTest.php @@ -144,6 +144,26 @@ public function test_a_delta_applies_against_the_members_already_stored(): void ); } + public function test_a_delta_removes_a_member_named_in_another_spelling(): void + { + $this->subject->store(EntryFixture::withUuid(Entry::fromArray( + 'cn=admins,dc=ex,dc=com', + ['cn' => ['admins'], 'member' => ['cn=bob,dc=ex,dc=com', 'cn=carol,dc=ex,dc=com']], + ))); + + $this->subject->store( + EntryFixture::withUuid(Entry::fromArray('cn=admins,dc=ex,dc=com', ['cn' => ['admins']])), + links: new LinkDelta(removed: ['member' => ['CN=Bob, DC=ex, DC=com']]), + ); + + self::assertSame( + ['cn=carol,dc=ex,dc=com'], + $this->subject->find(new Dn('cn=admins,dc=ex,dc=com')) + ?->get('member') + ?->getValues(), + ); + } + public function test_a_delta_emptying_an_attribute_drops_it(): void { $this->subject->store(EntryFixture::withUuid(Entry::fromArray( From 367a77a20c0f92622a33f587a4a6357263f2e55d Mon Sep 17 00:00:00 2001 From: Chad Sikorra Date: Sun, 18 Oct 2026 16:11:56 -0400 Subject: [PATCH 2/5] Apply linked attribute changes in the order they are requested. --- .../DirectoryServerContainerProvider.php | 2 + .../Provider/StorageContainerProvider.php | 10 +- .../Write/Operation/LinkedChangeReplay.php | 224 ++++++++++++++++++ .../Backend/Write/Operation/LinkedChanges.php | 135 ++--------- .../Backend/Write/Operation/ModifiedEntry.php | 32 +++ .../Operation/TransactionalEntryWrite.php | 27 ++- .../Write/Operation/UpdateEntryHandler.php | 33 ++- .../Concern/LinkedAttributeTestsTrait.php | 51 ++++ .../Storage/Pdo/UpdateEntryHandlerTest.php | 13 + .../Operation/LinkedChangeReplayTest.php | 200 ++++++++++++++++ .../Operation/UpdateEntryHandlerTest.php | 30 +++ 11 files changed, 613 insertions(+), 144 deletions(-) create mode 100644 src/FreeDSx/Ldap/Server/Backend/Write/Operation/LinkedChangeReplay.php create mode 100644 src/FreeDSx/Ldap/Server/Backend/Write/Operation/ModifiedEntry.php create mode 100644 tests/unit/Server/Backend/Write/Operation/LinkedChangeReplayTest.php diff --git a/src/FreeDSx/Ldap/Container/Provider/DirectoryServerContainerProvider.php b/src/FreeDSx/Ldap/Container/Provider/DirectoryServerContainerProvider.php index 38003b47..1e09c14d 100644 --- a/src/FreeDSx/Ldap/Container/Provider/DirectoryServerContainerProvider.php +++ b/src/FreeDSx/Ldap/Container/Provider/DirectoryServerContainerProvider.php @@ -89,6 +89,7 @@ use FreeDSx\Ldap\Server\Backend\Write\Operation\EntryMutation; use FreeDSx\Ldap\Server\Backend\Write\Operation\EntryPlacementGuard; use FreeDSx\Ldap\Server\Backend\Write\Operation\MoveEntryHandler; +use FreeDSx\Ldap\Server\Backend\Write\Operation\LinkedChangeReplay; use FreeDSx\Ldap\Server\Backend\Write\Operation\LinkedChanges; use FreeDSx\Ldap\Server\Backend\Write\Operation\UpdateEntryHandler; use FreeDSx\Ldap\Server\Backend\Write\Schema\SchemaViolationGate; @@ -439,6 +440,7 @@ private function makeUpdateEntryHandler(Container $container): UpdateEntryHandle mutation: $container->get(EntryMutation::class), placement: $container->get(EntryPlacementGuard::class), linkedChanges: $container->get(LinkedChanges::class), + replay: $container->get(LinkedChangeReplay::class), ); } diff --git a/src/FreeDSx/Ldap/Container/Provider/StorageContainerProvider.php b/src/FreeDSx/Ldap/Container/Provider/StorageContainerProvider.php index d6db1612..5d426aff 100644 --- a/src/FreeDSx/Ldap/Container/Provider/StorageContainerProvider.php +++ b/src/FreeDSx/Ldap/Container/Provider/StorageContainerProvider.php @@ -33,6 +33,7 @@ use FreeDSx\Ldap\Server\Backend\Storage\Capability\RowLockableInterface; use FreeDSx\Ldap\Server\Backend\Storage\Capability\UnlockedRows; use FreeDSx\Ldap\Server\Backend\Storage\Filter\LinkedLeafWitness; +use FreeDSx\Ldap\Server\Backend\Write\Operation\LinkedChangeReplay; use FreeDSx\Ldap\Server\Backend\Write\Operation\LinkedChanges; use FreeDSx\Ldap\Server\Backend\Storage\Contract\ListEntryInterface; use FreeDSx\Ldap\Server\Backend\Storage\Contract\ReadEntryInterface; @@ -76,6 +77,7 @@ public function factories(): array LinkedValueLookupInterface::class => $this->makeLinkedValueLookup(...), LinkedLeafWitness::class => $this->makeLinkedLeafWitness(...), LinkedChanges::class => $this->makeLinkedChanges(...), + LinkedChangeReplay::class => $this->makeLinkedChangeReplay(...), EntryIndexReindexer::class => $this->makeEntryIndexReindexer(...), ChangeJournalInterface::class => $this->makeChangeJournal(...), ]; @@ -144,8 +146,12 @@ private function makeLinkedValueLookup(Container $container): LinkedValueLookupI private function makeLinkedChanges(Container $container): LinkedChanges { - return new LinkedChanges( - $container->get(LinkedAttributes::class), + return new LinkedChanges($container->get(LinkedAttributes::class)); + } + + private function makeLinkedChangeReplay(Container $container): LinkedChangeReplay + { + return new LinkedChangeReplay( $this->isPdo($container) ? $container->get(EntryLinkWriter::class) : $container->get(InMemoryStorage::class), diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/LinkedChangeReplay.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/LinkedChangeReplay.php new file mode 100644 index 00000000..5e85fd72 --- /dev/null +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/LinkedChangeReplay.php @@ -0,0 +1,224 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Ldap\Server\Backend\Write\Operation; + +use FreeDSx\Ldap\Entry\Attribute; +use FreeDSx\Ldap\Entry\Change; +use FreeDSx\Ldap\Entry\Dn; +use FreeDSx\Ldap\Exception\OperationException; +use FreeDSx\Ldap\Operation\ResultCode; +use FreeDSx\Ldap\Schema\Validation\SchemaValidator; +use FreeDSx\Ldap\Server\Backend\Storage\Capability\LinkedValueLookupInterface; +use FreeDSx\Ldap\Server\Backend\Storage\Link\LinkDelta; + +use function array_diff_key; +use function array_values; +use function sprintf; + +/** + * Applies a modify's linked changes in request order to what the owner already links. + * + * @internal + * + * @author Chad Sikorra + */ +readonly class LinkedChangeReplay +{ + public function __construct( + private LinkedValueLookupInterface $lookup, + private SchemaValidator $schema, + ) {} + + /** + * RFC 4511 4.6: each change applies to the state the ones before it left. + * + * @param list $changes + * + * @throws OperationException when a value is already linked, is not linked, or is not a DN + */ + public function settle( + array $changes, + Dn $owner, + ): LinkDelta { + if ($changes === []) { + return new LinkDelta(); + } + $before = $this->heldBefore( + $changes, + $owner, + ); + $state = $before; + + foreach ($changes as $change) { + $name = Attribute::normalizeName($change->getAttribute()->getName()); + $values = array_values($change->getAttribute()->getValues()); + + $state[$name] = $change->isAdd() + ? $this->withAdded($name, $state[$name] ?? [], $values) + : $this->withRemoved($name, $state[$name] ?? [], $values); + } + + return $this->netOf( + $before, + $state, + ); + } + + /** + * What the owner links of every value the changes name. + * + * @param list $changes + * + * @return array> + */ + private function heldBefore( + array $changes, + Dn $owner, + ): array { + $named = []; + + foreach ($changes as $change) { + $name = Attribute::normalizeName($change->getAttribute()->getName()); + + foreach ($change->getAttribute()->getValues() as $value) { + $named[$name][self::keyOf($value)] = $value; + } + } + $held = []; + + foreach ($named as $name => $values) { + foreach ($this->lookup->heldLinkValues($owner, $name, array_values($values)) as $value) { + $held[$name][self::keyOf($value)] = $value; + } + } + + return $held; + } + + /** + * @param array $current + * @param list $values + * + * @return array + * + * @throws OperationException + */ + private function withAdded( + string $name, + array $current, + array $values, + ): array { + $adding = []; + + foreach ($values as $value) { + $key = self::keyOf($value); + + if (isset($adding[$key])) { + throw new OperationException( + sprintf('The attribute "%s" was given the same value twice.', $name), + ResultCode::ATTRIBUTE_OR_VALUE_EXISTS, + ); + } + if (isset($current[$key])) { + throw new OperationException( + sprintf('The entry already holds a value given for the attribute "%s".', $name), + ResultCode::ATTRIBUTE_OR_VALUE_EXISTS, + ); + } + + $adding[$key] = $value; + } + + return $current + $adding; + } + + /** + * Every value is checked against the state before the change. + * + * @param array $current + * @param list $values + * + * @return array + * + * @throws OperationException + */ + private function withRemoved( + string $name, + array $current, + array $values, + ): array { + $removing = []; + + foreach ($values as $value) { + $key = self::keyOf($value); + + if (!isset($current[$key])) { + throw new OperationException( + sprintf('The entry does not hold a value given for the attribute "%s".', $name), + ResultCode::NO_SUCH_ATTRIBUTE, + ); + } + + $removing[$key] = true; + } + + return array_diff_key( + $current, + $removing, + ); + } + + /** + * Values the replay left changed, held to the syntax the whole entry would have been for the ones it adds. + * + * @param array> $before + * @param array> $after + * + * @throws OperationException + */ + private function netOf( + array $before, + array $after, + ): LinkDelta { + $added = []; + $removed = []; + + foreach ($after as $name => $values) { + $held = $before[$name] ?? []; + $linking = array_values(array_diff_key($values, $held)); + $unlinking = array_values(array_diff_key($held, $values)); + + if ($linking !== []) { + $this->schema->validateValues(new Attribute($name, ...$linking)); + $added[$name] = $linking; + } + if ($unlinking !== []) { + $removed[$name] = $unlinking; + } + } + + // Changes that cancel out leave the stored links as they are. + return $added === [] && $removed === [] + ? LinkDelta::untouched() + : new LinkDelta( + $added, + $removed, + ); + } + + private static function keyOf(string $value): string + { + return Dn::normalizedOrNull($value) ?? $value; + } +} diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/LinkedChanges.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/LinkedChanges.php index 80d76a73..212cefb4 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/LinkedChanges.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/LinkedChanges.php @@ -13,22 +13,14 @@ namespace FreeDSx\Ldap\Server\Backend\Write\Operation; -use FreeDSx\Ldap\Entry\Attribute; use FreeDSx\Ldap\Entry\Change; use FreeDSx\Ldap\Entry\Dn; -use FreeDSx\Ldap\Exception\OperationException; -use FreeDSx\Ldap\Operation\ResultCode; -use FreeDSx\Ldap\Schema\Validation\SchemaValidator; -use FreeDSx\Ldap\Server\Backend\Storage\Capability\LinkedValueLookupInterface; -use FreeDSx\Ldap\Server\Backend\Storage\Link\LinkDelta; use FreeDSx\Ldap\Server\Backend\Storage\Schema\LinkedAttributes; use FreeDSx\Ldap\Server\Backend\Write\Command\UpdateCommand; use FreeDSx\Ldap\Server\Backend\Write\WriteContext; -use function array_diff; use function array_filter; use function array_values; -use function sprintf; /** * Separates the linked values a modify changes from the changes its entry is still stored whole for. @@ -37,59 +29,22 @@ */ final readonly class LinkedChanges { - public function __construct( - private LinkedAttributes $linked, - private LinkedValueLookupInterface $lookup, - private SchemaValidator $schema, - ) {} + public function __construct(private LinkedAttributes $linked) {} /** - * Holds the delta to what the owner links, which the entry is no longer carrying for the validator to see. + * The linked changes applied apart from the entry, or none when the modify has to be applied to the whole entry. * - * @throws OperationException when a value is already linked, is not linked, or is not a DN + * @return list */ - public function assertApplicable( - LinkDelta $delta, - Dn $owner, - ): void { - foreach ($delta->names() as $name) { - $added = $delta->added($name); - $removed = $delta->removed($name); - - $this->assertDistinctDns($name, $added); - $this->assertDistinctDns($name, $removed); - - $this->refuse( - $name, - $this->lookup->heldLinkValues($owner, $name, $added), - 'already holds', - ResultCode::ATTRIBUTE_OR_VALUE_EXISTS, - ); - $this->refuse( - $name, - array_values(array_diff( - $removed, - $this->lookup->heldLinkValues($owner, $name, $removed), - )), - 'does not hold', - ResultCode::NO_SUCH_ATTRIBUTE, - ); - } - } - - /** - * The values the modify links and unlinks or nothing when it has to be applied to the whole entry. - */ - public function delta( + public function changesApart( UpdateCommand $command, WriteContext $context, - ): LinkDelta { + ): array { // A control reading the entry needs every value it holds, which only the whole-entry path materializes. if ($this->linked->isEmpty() || $context->controlEvaluator()?->inspectsEntry() === true) { - return new LinkDelta(); + return []; } - $added = []; - $removed = []; + $apart = []; foreach ($command->changes as $change) { if (!$this->linked->links($change->getAttribute())) { @@ -98,31 +53,25 @@ public function delta( // One change the attribute has to be written whole for settles it for every value of that attribute. if (!$this->isApplicable($change, $command->dn)) { - return new LinkDelta(); - } - $name = Attribute::normalizeName($change->getAttribute()->getName()); - $values = array_values($change->getAttribute()->getValues()); - - if ($change->isAdd()) { - $added[$name] = [...$added[$name] ?? [], ...$values]; - - continue; + return []; } - $removed[$name] = [...$removed[$name] ?? [], ...$values]; + $apart[] = $change; } - return new LinkDelta($added, $removed); + return $apart; } /** - * The command without the changes the delta carries, which is what the entry is still stored for. + * The command without the changes applied apart, which is what the entry is still stored for. + * + * @param list $apart */ public function remaining( UpdateCommand $command, - LinkDelta $delta, + array $apart, ): UpdateCommand { - if ($delta->isEmpty()) { + if ($apart === []) { return $command; } @@ -136,60 +85,6 @@ public function remaining( ); } - /** - * Every value has to be a DN the attribute does not already name twice in the one request. - * - * @param list $values - * - * @throws OperationException - */ - private function assertDistinctDns( - string $name, - array $values, - ): void { - if ($values === []) { - return; - } - - // Held to the same syntax the whole entry would have been, which no longer carries these values. - $this->schema->validateValues(new Attribute($name, ...$values)); - $seen = []; - - foreach ($values as $value) { - $normalized = Dn::normalizedOrNull($value) ?? $value; - - if (isset($seen[$normalized])) { - throw new OperationException( - sprintf('The attribute "%s" was given the same value twice.', $name), - ResultCode::ATTRIBUTE_OR_VALUE_EXISTS, - ); - } - - $seen[$normalized] = true; - } - } - - /** - * @param list $offending - * - * @throws OperationException - */ - private function refuse( - string $name, - array $offending, - string $reason, - int $code, - ): void { - if ($offending === []) { - return; - } - - throw new OperationException( - sprintf('The entry %s a value given for the attribute "%s".', $reason, $name), - $code, - ); - } - /** * Whether the change names the values it changes, rather than restating what the attribute holds. */ diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/ModifiedEntry.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/ModifiedEntry.php new file mode 100644 index 00000000..9992fd2b --- /dev/null +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/ModifiedEntry.php @@ -0,0 +1,32 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Ldap\Server\Backend\Write\Operation; + +use FreeDSx\Ldap\Entry\Entry; +use FreeDSx\Ldap\Server\Backend\Storage\Link\LinkDelta; + +/** + * The entry a modify leaves behind and the linked values stored apart from it. + * + * @internal + * + * @author Chad Sikorra + */ +readonly class ModifiedEntry +{ + public function __construct( + public Entry $entry, + public LinkDelta $links = new LinkDelta(), + ) {} +} diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/TransactionalEntryWrite.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/TransactionalEntryWrite.php index e9d5af23..13dde840 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/TransactionalEntryWrite.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/TransactionalEntryWrite.php @@ -70,8 +70,8 @@ public function add( /** * Stores the entry a modify leaves behind, with the target locked and loaded for the producer to work from. * - * @param LinkDelta $links Linked values the modify changes, which its entry is then left without. - * @param Closure(Entry): ?Entry $produce Answers null when the change turns out to be nothing to write. + * @param EntryProjection $projection What of the target the producer needs loaded. + * @param Closure(Entry): ?ModifiedEntry $produce Answers null when the change turns out to be nothing to write. * * @return ?Entry what was stored, or null when the producer had nothing to write * @@ -79,22 +79,25 @@ public function add( */ public function update( Dn $dn, - LinkDelta $links, + EntryProjection $projection, WriteContext $context, Closure $produce, ): ?Entry { return $this->locked->withEntry( $dn, $context, - fn(Entry $current): ?Entry => $this->stored( - $produce($current), - $links, - $context, - ), - // Only a write the entry itself is the whole truth for has to read what it links. - $links->isEmpty() && !$links->isUntouched() - ? EntryProjection::unbounded() - : new EntryProjection(linkCap: 0), + function (Entry $current) use ($produce, $context): ?Entry { + $modified = $produce($current); + + return $modified === null + ? null + : $this->stored( + $modified->entry, + $modified->links, + $context, + ); + }, + $projection, ); } diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/UpdateEntryHandler.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/UpdateEntryHandler.php index d5eebc08..ec513471 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/UpdateEntryHandler.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/UpdateEntryHandler.php @@ -15,6 +15,7 @@ use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Exception\OperationException; +use FreeDSx\Ldap\Server\Backend\Storage\Search\EntryProjection; use FreeDSx\Ldap\Server\Backend\Write\Command\UpdateCommand; use FreeDSx\Ldap\Server\Backend\Write\WriteContext; @@ -32,6 +33,7 @@ public function __construct( private EntryMutation $mutation, private EntryPlacementGuard $placement, private LinkedChanges $linkedChanges, + private LinkedChangeReplay $replay, ) {} /** @@ -41,23 +43,34 @@ public function handle( UpdateCommand $command, WriteContext $context, ): void { - $links = $this->linkedChanges->delta($command, $context); - $remaining = $this->linkedChanges->remaining($command, $links); + $apart = $this->linkedChanges->changesApart( + $command, + $context, + ); + $remaining = $this->linkedChanges->remaining( + $command, + $apart, + ); $this->writes->update( $command->dn->normalize(), - $links, + $apart === [] + ? EntryProjection::unbounded() + : new EntryProjection(linkCap: 0), $context, - function (Entry $current) use ($remaining, $links, $context): Entry { - $this->linkedChanges->assertApplicable( - $links, + function (Entry $current) use ($apart, $remaining, $context): ModifiedEntry { + $links = $this->replay->settle( + $apart, $current->getDn(), ); - return $this->updated( - $remaining, - $context, - $current, + return new ModifiedEntry( + $this->updated( + $remaining, + $context, + $current, + ), + $links, ); }, ); diff --git a/tests/integration/Storage/Concern/LinkedAttributeTestsTrait.php b/tests/integration/Storage/Concern/LinkedAttributeTestsTrait.php index 5a30e893..23b799e8 100644 --- a/tests/integration/Storage/Concern/LinkedAttributeTestsTrait.php +++ b/tests/integration/Storage/Concern/LinkedAttributeTestsTrait.php @@ -85,6 +85,57 @@ public function testDeletingAMemberInAnotherSpellingRemovesIt(): void ); } + public function testAddingAndDeletingAMemberInOneModifyKeepsTheOthers(): void + { + $group = $this->seedGroup( + 'link-add-delete', + self::LINKED_ADMIN, + ); + + $this->ldapClient()->send(Operations::modify( + $group, + Change::add('member', self::LINKED_ALICE), + Change::delete('member', self::LINKED_ALICE), + )); + + self::assertEqualsCanonicalizing( + [self::LINKED_USER, self::LINKED_ADMIN], + $this->membersOf($group), + ); + } + + public function testDeletingAndAddingAMemberInOneModifyKeepsIt(): void + { + $group = $this->seedGroup( + 'link-delete-add', + self::LINKED_ADMIN, + ); + + $this->ldapClient()->send(Operations::modify( + $group, + Change::delete('member', self::LINKED_USER), + Change::add('member', self::LINKED_USER), + )); + + self::assertEqualsCanonicalizing( + [self::LINKED_USER, self::LINKED_ADMIN], + $this->membersOf($group), + ); + } + + public function testDeletingAMemberTwiceInOneModifyIsRefused(): void + { + $group = $this->seedGroup('link-delete-twice'); + + $this->expectOperationCode(ResultCode::NO_SUCH_ATTRIBUTE); + + $this->ldapClient()->send(Operations::modify( + $group, + Change::delete('member', self::LINKED_USER), + Change::delete('member', self::LINKED_USER), + )); + } + public function testReplacingTheMembersWritesThemWhole(): void { $group = $this->seedGroup('link-replace'); diff --git a/tests/integration/Storage/Pdo/UpdateEntryHandlerTest.php b/tests/integration/Storage/Pdo/UpdateEntryHandlerTest.php index 91078748..b0d423b5 100644 --- a/tests/integration/Storage/Pdo/UpdateEntryHandlerTest.php +++ b/tests/integration/Storage/Pdo/UpdateEntryHandlerTest.php @@ -81,6 +81,19 @@ public function test_deleting_a_member_leaves_the_rest_linked(): void ); } + public function test_adding_and_deleting_a_member_in_one_modify_keeps_the_others(): void + { + $this->modify( + new Change(Change::TYPE_ADD, 'member', $this->dnOf('Carol')), + new Change(Change::TYPE_DELETE, 'member', $this->dnOf('Carol')), + ); + + self::assertSame( + [$this->dnOf('Bob')], + $this->members(), + ); + } + public function test_replacing_the_members_writes_the_attribute_whole(): void { $this->modify(new Change(Change::TYPE_REPLACE, 'member', $this->dnOf('Carol'))); diff --git a/tests/unit/Server/Backend/Write/Operation/LinkedChangeReplayTest.php b/tests/unit/Server/Backend/Write/Operation/LinkedChangeReplayTest.php new file mode 100644 index 00000000..6a3caaaa --- /dev/null +++ b/tests/unit/Server/Backend/Write/Operation/LinkedChangeReplayTest.php @@ -0,0 +1,200 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Tests\Unit\FreeDSx\Ldap\Server\Backend\Write\Operation; + +use FreeDSx\Ldap\Entry\Change; +use FreeDSx\Ldap\Entry\Dn; +use FreeDSx\Ldap\Entry\Entry; +use FreeDSx\Ldap\Exception\OperationException; +use FreeDSx\Ldap\Operation\ResultCode; +use FreeDSx\Ldap\Server\Backend\Storage\Link\LinkDelta; +use FreeDSx\Ldap\Server\Backend\Write\Operation\LinkedChangeReplay; +use PHPUnit\Framework\TestCase; +use Tests\Support\FreeDSx\Ldap\ServerContainerTrait; +use Tests\Support\FreeDSx\Ldap\Storage\EntryFixture; + +final class LinkedChangeReplayTest extends TestCase +{ + use ServerContainerTrait; + + private const ADMINS = 'cn=Admins,dc=example,dc=com'; + + private const BOB = 'cn=Bob,dc=example,dc=com'; + + private const CAROL = 'cn=Carol,dc=example,dc=com'; + + private LinkedChangeReplay $subject; + + protected function setUp(): void + { + $storage = EntryFixture::inMemoryStorage(Entry::fromArray( + self::ADMINS, + [ + 'objectClass' => ['groupOfNames'], + 'cn' => ['Admins'], + 'member' => [self::BOB], + ], + )); + $this->subject = $this->containerFor($storage)->get(LinkedChangeReplay::class); + } + + public function test_no_changes_leave_the_entry_the_whole_truth_for_its_links(): void + { + $delta = $this->subject->settle( + [], + new Dn(self::ADMINS), + ); + + self::assertTrue($delta->isEmpty()); + self::assertFalse($delta->isUntouched()); + } + + public function test_a_delete_and_an_add_net_to_both(): void + { + $delta = $this->settle( + Change::delete('member', self::BOB), + Change::add('member', self::CAROL), + ); + + self::assertSame( + [self::CAROL], + $delta->added('member'), + ); + self::assertSame( + [self::BOB], + $delta->removed('member'), + ); + } + + public function test_adding_then_deleting_a_value_leaves_the_links_untouched(): void + { + $delta = $this->settle( + Change::add('member', self::CAROL), + Change::delete('member', self::CAROL), + ); + + self::assertTrue($delta->isUntouched()); + } + + public function test_deleting_then_adding_a_held_value_leaves_the_links_untouched(): void + { + $delta = $this->settle( + Change::delete('member', self::BOB), + Change::add('member', self::BOB), + ); + + self::assertTrue($delta->isUntouched()); + } + + public function test_adding_a_value_again_after_deleting_it_links_it(): void + { + $delta = $this->settle( + Change::add('member', self::CAROL), + Change::delete('member', self::CAROL), + Change::add('member', self::CAROL), + ); + + self::assertSame( + [self::CAROL], + $delta->added('member'), + ); + self::assertSame( + [], + $delta->removed('member'), + ); + } + + public function test_adding_a_value_an_earlier_change_added_is_refused(): void + { + $this->expectOperationCode(ResultCode::ATTRIBUTE_OR_VALUE_EXISTS); + + $this->settle( + Change::add('member', self::CAROL), + Change::add('member', self::CAROL), + ); + } + + public function test_the_same_value_twice_in_one_add_is_refused(): void + { + $this->expectOperationCode(ResultCode::ATTRIBUTE_OR_VALUE_EXISTS); + + $this->settle(Change::add( + 'member', + self::CAROL, + 'CN=Carol, DC=example, DC=com', + )); + } + + public function test_deleting_a_value_an_earlier_change_deleted_is_refused(): void + { + $this->expectOperationCode(ResultCode::NO_SUCH_ATTRIBUTE); + + $this->settle( + Change::delete('member', self::BOB), + Change::delete('member', self::BOB), + ); + } + + public function test_a_value_named_twice_in_one_delete_is_removed_once(): void + { + $delta = $this->settle(Change::delete( + 'member', + self::BOB, + 'CN=Bob, DC=example, DC=com', + )); + + self::assertCount( + 1, + $delta->removed('member'), + ); + } + + public function test_deleting_a_value_that_is_not_a_dn_is_answered_as_not_held(): void + { + $this->expectOperationCode(ResultCode::NO_SUCH_ATTRIBUTE); + + $this->settle(Change::delete('member', 'not a dn')); + } + + public function test_adding_a_value_that_is_not_a_dn_is_refused_for_its_syntax(): void + { + $this->expectOperationCode(ResultCode::INVALID_ATTRIBUTE_SYNTAX); + + $this->settle(Change::add('member', 'not a dn')); + } + + public function test_a_value_that_is_not_a_dn_added_then_deleted_is_never_held_to_its_syntax(): void + { + $delta = $this->settle( + Change::add('member', 'not a dn'), + Change::delete('member', 'not a dn'), + ); + + self::assertTrue($delta->isUntouched()); + } + + private function settle(Change ...$changes): LinkDelta + { + return $this->subject->settle( + array_values($changes), + new Dn(self::ADMINS), + ); + } + + private function expectOperationCode(int $code): void + { + $this->expectException(OperationException::class); + $this->expectExceptionCode($code); + } +} diff --git a/tests/unit/Server/Backend/Write/Operation/UpdateEntryHandlerTest.php b/tests/unit/Server/Backend/Write/Operation/UpdateEntryHandlerTest.php index c40c72e0..60a102c6 100644 --- a/tests/unit/Server/Backend/Write/Operation/UpdateEntryHandlerTest.php +++ b/tests/unit/Server/Backend/Write/Operation/UpdateEntryHandlerTest.php @@ -90,6 +90,36 @@ public function test_it_removes_only_the_linked_value_it_names(): void ); } + public function test_adding_then_deleting_a_member_in_one_modify_keeps_the_others(): void + { + $this->seedAdmins(); + + $this->modifyAdmins( + new Change(Change::TYPE_ADD, 'member', self::ALICE), + new Change(Change::TYPE_DELETE, 'member', self::ALICE), + ); + + self::assertSame( + ['cn=Bob,ou=People,dc=example,dc=com'], + array_values($this->find(self::ADMINS)?->get('member')?->getValues() ?? []), + ); + } + + public function test_deleting_then_adding_a_member_in_one_modify_keeps_it(): void + { + $this->seedAdmins(self::ALICE); + + $this->modifyAdmins( + new Change(Change::TYPE_DELETE, 'member', self::ALICE), + new Change(Change::TYPE_ADD, 'member', self::ALICE), + ); + + self::assertEqualsCanonicalizing( + ['cn=Bob,ou=People,dc=example,dc=com', self::ALICE], + $this->find(self::ADMINS)?->get('member')?->getValues(), + ); + } + public function test_it_replaces_a_linked_attribute_through_the_whole_entry(): void { $this->seedAdmins(self::ALICE); From 094915bc1a7ce05925d49f27181ee62edd0edca6 Mon Sep 17 00:00:00 2001 From: Chad Sikorra Date: Sun, 18 Oct 2026 17:09:33 -0400 Subject: [PATCH 3/5] Reorganize operation handler collaborators. --- .../Container/Provider/DirectoryServerContainerProvider.php | 6 +++--- .../Ldap/Container/Provider/StorageContainerProvider.php | 4 ++-- .../Server/Backend/Write/Operation/DeleteSubtreeHandler.php | 3 +++ .../Write/Operation/{ => Linked}/LinkedChangeReplay.php | 2 +- .../Backend/Write/Operation/{ => Linked}/LinkedChanges.php | 2 +- .../Write/Operation/{ => SubtreeDelete}/ChunkOutcome.php | 2 +- .../Write/Operation/{ => SubtreeDelete}/DeletionQueue.php | 2 +- .../Operation/{ => SubtreeDelete}/SubtreeChunkRemover.php | 3 ++- .../Operation/{ => SubtreeDelete}/SubtreeDeleteProgress.php | 2 +- .../Server/Backend/Write/Operation/UpdateEntryHandler.php | 2 ++ .../Backend/Write/Operation/DeleteSubtreeHandlerTest.php | 2 +- .../Write/Operation/{ => Linked}/LinkedChangeReplayTest.php | 4 ++-- .../Operation/{ => SubtreeDelete}/DeletionQueueTest.php | 4 ++-- 13 files changed, 22 insertions(+), 16 deletions(-) rename src/FreeDSx/Ldap/Server/Backend/Write/Operation/{ => Linked}/LinkedChangeReplay.php (99%) rename src/FreeDSx/Ldap/Server/Backend/Write/Operation/{ => Linked}/LinkedChanges.php (98%) rename src/FreeDSx/Ldap/Server/Backend/Write/Operation/{ => SubtreeDelete}/ChunkOutcome.php (92%) rename src/FreeDSx/Ldap/Server/Backend/Write/Operation/{ => SubtreeDelete}/DeletionQueue.php (97%) rename src/FreeDSx/Ldap/Server/Backend/Write/Operation/{ => SubtreeDelete}/SubtreeChunkRemover.php (97%) rename src/FreeDSx/Ldap/Server/Backend/Write/Operation/{ => SubtreeDelete}/SubtreeDeleteProgress.php (95%) rename tests/unit/Server/Backend/Write/Operation/{ => Linked}/LinkedChangeReplayTest.php (98%) rename tests/unit/Server/Backend/Write/Operation/{ => SubtreeDelete}/DeletionQueueTest.php (97%) diff --git a/src/FreeDSx/Ldap/Container/Provider/DirectoryServerContainerProvider.php b/src/FreeDSx/Ldap/Container/Provider/DirectoryServerContainerProvider.php index 1e09c14d..f0bb448c 100644 --- a/src/FreeDSx/Ldap/Container/Provider/DirectoryServerContainerProvider.php +++ b/src/FreeDSx/Ldap/Container/Provider/DirectoryServerContainerProvider.php @@ -85,12 +85,12 @@ use FreeDSx\Ldap\Server\Backend\Write\Operation\ComputeUpdateHandler; use FreeDSx\Ldap\Server\Backend\Write\Operation\DeleteEntryHandler; use FreeDSx\Ldap\Server\Backend\Write\Operation\DeleteSubtreeHandler; -use FreeDSx\Ldap\Server\Backend\Write\Operation\SubtreeChunkRemover; +use FreeDSx\Ldap\Server\Backend\Write\Operation\SubtreeDelete\SubtreeChunkRemover; use FreeDSx\Ldap\Server\Backend\Write\Operation\EntryMutation; use FreeDSx\Ldap\Server\Backend\Write\Operation\EntryPlacementGuard; use FreeDSx\Ldap\Server\Backend\Write\Operation\MoveEntryHandler; -use FreeDSx\Ldap\Server\Backend\Write\Operation\LinkedChangeReplay; -use FreeDSx\Ldap\Server\Backend\Write\Operation\LinkedChanges; +use FreeDSx\Ldap\Server\Backend\Write\Operation\Linked\LinkedChangeReplay; +use FreeDSx\Ldap\Server\Backend\Write\Operation\Linked\LinkedChanges; use FreeDSx\Ldap\Server\Backend\Write\Operation\UpdateEntryHandler; use FreeDSx\Ldap\Server\Backend\Write\Schema\SchemaViolationGate; use FreeDSx\Ldap\Server\Subentry\SubentryPlacementGuard; diff --git a/src/FreeDSx/Ldap/Container/Provider/StorageContainerProvider.php b/src/FreeDSx/Ldap/Container/Provider/StorageContainerProvider.php index 5d426aff..f13cce9b 100644 --- a/src/FreeDSx/Ldap/Container/Provider/StorageContainerProvider.php +++ b/src/FreeDSx/Ldap/Container/Provider/StorageContainerProvider.php @@ -33,8 +33,8 @@ use FreeDSx\Ldap\Server\Backend\Storage\Capability\RowLockableInterface; use FreeDSx\Ldap\Server\Backend\Storage\Capability\UnlockedRows; use FreeDSx\Ldap\Server\Backend\Storage\Filter\LinkedLeafWitness; -use FreeDSx\Ldap\Server\Backend\Write\Operation\LinkedChangeReplay; -use FreeDSx\Ldap\Server\Backend\Write\Operation\LinkedChanges; +use FreeDSx\Ldap\Server\Backend\Write\Operation\Linked\LinkedChangeReplay; +use FreeDSx\Ldap\Server\Backend\Write\Operation\Linked\LinkedChanges; use FreeDSx\Ldap\Server\Backend\Storage\Contract\ListEntryInterface; use FreeDSx\Ldap\Server\Backend\Storage\Contract\ReadEntryInterface; use FreeDSx\Ldap\Server\Backend\Storage\Contract\TransactionalWriteInterface; diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/DeleteSubtreeHandler.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/DeleteSubtreeHandler.php index 61ed036d..1b3a926f 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/DeleteSubtreeHandler.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/DeleteSubtreeHandler.php @@ -18,6 +18,9 @@ use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Operation\ResultCode; use FreeDSx\Ldap\Server\Backend\Write\Command\DeleteSubtreeCommand; +use FreeDSx\Ldap\Server\Backend\Write\Operation\SubtreeDelete\DeletionQueue; +use FreeDSx\Ldap\Server\Backend\Write\Operation\SubtreeDelete\SubtreeChunkRemover; +use FreeDSx\Ldap\Server\Backend\Write\Operation\SubtreeDelete\SubtreeDeleteProgress; use FreeDSx\Ldap\Server\Backend\Write\WriteContext; use FreeDSx\Ldap\Server\Clock\Sleeper\BlockingSleeper; use FreeDSx\Ldap\Server\Clock\Sleeper\SleeperInterface; diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/LinkedChangeReplay.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/LinkedChangeReplay.php similarity index 99% rename from src/FreeDSx/Ldap/Server/Backend/Write/Operation/LinkedChangeReplay.php rename to src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/LinkedChangeReplay.php index 5e85fd72..3e00c58b 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/LinkedChangeReplay.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/LinkedChangeReplay.php @@ -11,7 +11,7 @@ * file that was distributed with this source code. */ -namespace FreeDSx\Ldap\Server\Backend\Write\Operation; +namespace FreeDSx\Ldap\Server\Backend\Write\Operation\Linked; use FreeDSx\Ldap\Entry\Attribute; use FreeDSx\Ldap\Entry\Change; diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/LinkedChanges.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/LinkedChanges.php similarity index 98% rename from src/FreeDSx/Ldap/Server/Backend/Write/Operation/LinkedChanges.php rename to src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/LinkedChanges.php index 212cefb4..2f307992 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/LinkedChanges.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/LinkedChanges.php @@ -11,7 +11,7 @@ * file that was distributed with this source code. */ -namespace FreeDSx\Ldap\Server\Backend\Write\Operation; +namespace FreeDSx\Ldap\Server\Backend\Write\Operation\Linked; use FreeDSx\Ldap\Entry\Change; use FreeDSx\Ldap\Entry\Dn; diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/ChunkOutcome.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/SubtreeDelete/ChunkOutcome.php similarity index 92% rename from src/FreeDSx/Ldap/Server/Backend/Write/Operation/ChunkOutcome.php rename to src/FreeDSx/Ldap/Server/Backend/Write/Operation/SubtreeDelete/ChunkOutcome.php index 63a5b688..03c48b44 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/ChunkOutcome.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/SubtreeDelete/ChunkOutcome.php @@ -11,7 +11,7 @@ * file that was distributed with this source code. */ -namespace FreeDSx\Ldap\Server\Backend\Write\Operation; +namespace FreeDSx\Ldap\Server\Backend\Write\Operation\SubtreeDelete; /** * What one transaction of a subtree delete removed, and what it left for a later one. diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/DeletionQueue.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/SubtreeDelete/DeletionQueue.php similarity index 97% rename from src/FreeDSx/Ldap/Server/Backend/Write/Operation/DeletionQueue.php rename to src/FreeDSx/Ldap/Server/Backend/Write/Operation/SubtreeDelete/DeletionQueue.php index 916beccd..68c622c4 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/DeletionQueue.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/SubtreeDelete/DeletionQueue.php @@ -11,7 +11,7 @@ * file that was distributed with this source code. */ -namespace FreeDSx\Ldap\Server\Backend\Write\Operation; +namespace FreeDSx\Ldap\Server\Backend\Write\Operation\SubtreeDelete; use ArrayIterator; use FreeDSx\Ldap\Entry\Dn; diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/SubtreeChunkRemover.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/SubtreeDelete/SubtreeChunkRemover.php similarity index 97% rename from src/FreeDSx/Ldap/Server/Backend/Write/Operation/SubtreeChunkRemover.php rename to src/FreeDSx/Ldap/Server/Backend/Write/Operation/SubtreeDelete/SubtreeChunkRemover.php index a9322040..f3e6a420 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/SubtreeChunkRemover.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/SubtreeDelete/SubtreeChunkRemover.php @@ -11,7 +11,7 @@ * file that was distributed with this source code. */ -namespace FreeDSx\Ldap\Server\Backend\Write\Operation; +namespace FreeDSx\Ldap\Server\Backend\Write\Operation\SubtreeDelete; use FreeDSx\Ldap\Entry\Dn; use FreeDSx\Ldap\Entry\Entry; @@ -21,6 +21,7 @@ use FreeDSx\Ldap\Server\Backend\Storage\Journal\Capture\ChangeRecorderInterface; use FreeDSx\Ldap\Server\Backend\Storage\Journal\Capture\UnrecordedChanges; use FreeDSx\Ldap\Server\Backend\Storage\Search\EntryProjection; +use FreeDSx\Ldap\Server\Backend\Write\Operation\LockedEntryAccess; use FreeDSx\Ldap\Server\Backend\Write\WriteContext; use function array_filter; diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/SubtreeDeleteProgress.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/SubtreeDelete/SubtreeDeleteProgress.php similarity index 95% rename from src/FreeDSx/Ldap/Server/Backend/Write/Operation/SubtreeDeleteProgress.php rename to src/FreeDSx/Ldap/Server/Backend/Write/Operation/SubtreeDelete/SubtreeDeleteProgress.php index ef39db6f..4dcc6f61 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/SubtreeDeleteProgress.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/SubtreeDelete/SubtreeDeleteProgress.php @@ -11,7 +11,7 @@ * file that was distributed with this source code. */ -namespace FreeDSx\Ldap\Server\Backend\Write\Operation; +namespace FreeDSx\Ldap\Server\Backend\Write\Operation\SubtreeDelete; use FreeDSx\Ldap\Control\SubtreeDeleteProgressControl; diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/UpdateEntryHandler.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/UpdateEntryHandler.php index ec513471..7d312741 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/UpdateEntryHandler.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/UpdateEntryHandler.php @@ -17,6 +17,8 @@ use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Server\Backend\Storage\Search\EntryProjection; use FreeDSx\Ldap\Server\Backend\Write\Command\UpdateCommand; +use FreeDSx\Ldap\Server\Backend\Write\Operation\Linked\LinkedChangeReplay; +use FreeDSx\Ldap\Server\Backend\Write\Operation\Linked\LinkedChanges; use FreeDSx\Ldap\Server\Backend\Write\WriteContext; /** diff --git a/tests/unit/Server/Backend/Write/Operation/DeleteSubtreeHandlerTest.php b/tests/unit/Server/Backend/Write/Operation/DeleteSubtreeHandlerTest.php index 55b8a9c8..8c27e2e8 100644 --- a/tests/unit/Server/Backend/Write/Operation/DeleteSubtreeHandlerTest.php +++ b/tests/unit/Server/Backend/Write/Operation/DeleteSubtreeHandlerTest.php @@ -41,7 +41,7 @@ use FreeDSx\Ldap\Server\Backend\Write\Operation\DeleteSubtreeHandler; use FreeDSx\Ldap\Server\Backend\Write\Operation\EntryPlacementGuard; use FreeDSx\Ldap\Server\Backend\Write\Operation\LockedEntryAccess; -use FreeDSx\Ldap\Server\Backend\Write\Operation\SubtreeChunkRemover; +use FreeDSx\Ldap\Server\Backend\Write\Operation\SubtreeDelete\SubtreeChunkRemover; use FreeDSx\Ldap\Server\Backend\Write\WriteContext; use FreeDSx\Ldap\Server\Clock\Sleeper\BlockingSleeper; use FreeDSx\Ldap\Server\Clock\Sleeper\SleeperInterface; diff --git a/tests/unit/Server/Backend/Write/Operation/LinkedChangeReplayTest.php b/tests/unit/Server/Backend/Write/Operation/Linked/LinkedChangeReplayTest.php similarity index 98% rename from tests/unit/Server/Backend/Write/Operation/LinkedChangeReplayTest.php rename to tests/unit/Server/Backend/Write/Operation/Linked/LinkedChangeReplayTest.php index 6a3caaaa..3bf82257 100644 --- a/tests/unit/Server/Backend/Write/Operation/LinkedChangeReplayTest.php +++ b/tests/unit/Server/Backend/Write/Operation/Linked/LinkedChangeReplayTest.php @@ -11,7 +11,7 @@ * file that was distributed with this source code. */ -namespace Tests\Unit\FreeDSx\Ldap\Server\Backend\Write\Operation; +namespace Tests\Unit\FreeDSx\Ldap\Server\Backend\Write\Operation\Linked; use FreeDSx\Ldap\Entry\Change; use FreeDSx\Ldap\Entry\Dn; @@ -19,7 +19,7 @@ use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Operation\ResultCode; use FreeDSx\Ldap\Server\Backend\Storage\Link\LinkDelta; -use FreeDSx\Ldap\Server\Backend\Write\Operation\LinkedChangeReplay; +use FreeDSx\Ldap\Server\Backend\Write\Operation\Linked\LinkedChangeReplay; use PHPUnit\Framework\TestCase; use Tests\Support\FreeDSx\Ldap\ServerContainerTrait; use Tests\Support\FreeDSx\Ldap\Storage\EntryFixture; diff --git a/tests/unit/Server/Backend/Write/Operation/DeletionQueueTest.php b/tests/unit/Server/Backend/Write/Operation/SubtreeDelete/DeletionQueueTest.php similarity index 97% rename from tests/unit/Server/Backend/Write/Operation/DeletionQueueTest.php rename to tests/unit/Server/Backend/Write/Operation/SubtreeDelete/DeletionQueueTest.php index 3e952caa..742fd7b4 100644 --- a/tests/unit/Server/Backend/Write/Operation/DeletionQueueTest.php +++ b/tests/unit/Server/Backend/Write/Operation/SubtreeDelete/DeletionQueueTest.php @@ -11,12 +11,12 @@ * file that was distributed with this source code. */ -namespace Tests\Unit\FreeDSx\Ldap\Server\Backend\Write\Operation; +namespace Tests\Unit\FreeDSx\Ldap\Server\Backend\Write\Operation\SubtreeDelete; use FreeDSx\Ldap\Entry\Dn; use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Server\Backend\Storage\Directory\SubtreeEnumerator; -use FreeDSx\Ldap\Server\Backend\Write\Operation\DeletionQueue; +use FreeDSx\Ldap\Server\Backend\Write\Operation\SubtreeDelete\DeletionQueue; use PHPUnit\Framework\TestCase; use Tests\Support\FreeDSx\Ldap\Storage\EntryFixture; From 90ff7db92a0f3bdb2bfd4df62c94a7db51adff64 Mon Sep 17 00:00:00 2001 From: Chad Sikorra Date: Sun, 18 Oct 2026 17:42:13 -0400 Subject: [PATCH 4/5] Hold linked attribute changes to the schema. --- .../Check/AllowedAttributesCheck.php | 10 +- .../Check/EquivalentValuesCheck.php | 2 +- .../Validation/Check/SingleValueCheck.php | 11 +- .../Ldap/Schema/Validation/SchemaWrite.php | 35 ++++ .../Contract/PdoLinkReadDialectInterface.php | 7 + .../Dialect/Sql/PdoLinkReadDialectTrait.php | 14 ++ .../Storage/Adapter/InMemoryStorage.php | 15 +- .../Adapter/Pdo/Writer/EntryLinkWriter.php | 49 ++++- .../Capability/LinkedValueCountInterface.php | 33 ++++ .../Write/Operation/AppliesEntryUpdate.php | 3 + .../Backend/Write/Operation/EntryMutation.php | 6 + .../Operation/Linked/LinkedChangeReplay.php | 127 +++++++++++-- .../Write/Operation/Linked/LinkedChanges.php | 13 ++ .../Write/Operation/Linked/SettledLinks.php | 38 ++++ .../Write/Operation/UpdateEntryHandler.php | 3 +- .../Write/Schema/SchemaViolationGate.php | 9 +- .../Concern/LinkedAttributeTestsTrait.php | 178 ++++++++++++++++-- .../Storage/Pdo/EntryLinkWriterTest.php | 79 ++++++-- .../Storage/Pdo/UpdateEntryHandlerTest.php | 10 +- .../Configuration/TestServerOptions.php | 15 +- .../Check/AllowedAttributesCheckTest.php | 57 ++++++ .../Check/EquivalentValuesCheckTest.php | 27 +++ .../Validation/Check/SingleValueCheckTest.php | 60 +++++- .../Linked/LinkedChangeReplayTest.php | 145 +++++++++++--- .../Operation/UpdateEntryHandlerTest.php | 116 +++++++++++- 25 files changed, 953 insertions(+), 109 deletions(-) create mode 100644 src/FreeDSx/Ldap/Server/Backend/Storage/Capability/LinkedValueCountInterface.php create mode 100644 src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/SettledLinks.php diff --git a/src/FreeDSx/Ldap/Schema/Validation/Check/AllowedAttributesCheck.php b/src/FreeDSx/Ldap/Schema/Validation/Check/AllowedAttributesCheck.php index 860fa114..617740d0 100644 --- a/src/FreeDSx/Ldap/Schema/Validation/Check/AllowedAttributesCheck.php +++ b/src/FreeDSx/Ldap/Schema/Validation/Check/AllowedAttributesCheck.php @@ -55,13 +55,13 @@ public function check( $classes->may, )); - foreach ($write->entry->getAttributes() as $attribute) { - $attrType = $this->schema->getAttributeType($attribute->getName()); + foreach ($write->valueCounts() as $name => $count) { + $attrType = $this->schema->getAttributeType($name); - if ($attrType === null || $attrType->usage !== AttributeUsage::UserApplications) { + if ($count === 0 || $attrType === null || $attrType->usage !== AttributeUsage::UserApplications) { continue; } - if (isset($allowed[strtolower($attrType->names[0] ?? $attribute->getName())])) { + if (isset($allowed[strtolower($attrType->names[0] ?? $name)])) { continue; } @@ -71,7 +71,7 @@ public function check( ResultCode::OBJECT_CLASS_VIOLATION, sprintf( 'Attribute "%s" is not permitted by any object class.', - $attribute->getName(), + $attrType->primaryName(), ), ), ]; diff --git a/src/FreeDSx/Ldap/Schema/Validation/Check/EquivalentValuesCheck.php b/src/FreeDSx/Ldap/Schema/Validation/Check/EquivalentValuesCheck.php index 14a8355a..ec39d37f 100644 --- a/src/FreeDSx/Ldap/Schema/Validation/Check/EquivalentValuesCheck.php +++ b/src/FreeDSx/Ldap/Schema/Validation/Check/EquivalentValuesCheck.php @@ -50,7 +50,7 @@ public function check( if ($only !== null && !isset($only[Attribute::normalizeName($attribute->getDescription())])) { continue; } - if (!$this->hasEquivalentValues($attribute)) { + if ($write->hasLinkedCount($attribute) || !$this->hasEquivalentValues($attribute)) { continue; } diff --git a/src/FreeDSx/Ldap/Schema/Validation/Check/SingleValueCheck.php b/src/FreeDSx/Ldap/Schema/Validation/Check/SingleValueCheck.php index 8b35e39c..c61fb337 100644 --- a/src/FreeDSx/Ldap/Schema/Validation/Check/SingleValueCheck.php +++ b/src/FreeDSx/Ldap/Schema/Validation/Check/SingleValueCheck.php @@ -20,7 +20,6 @@ use FreeDSx\Ldap\Schema\Validation\SchemaRule; use FreeDSx\Ldap\Schema\Validation\SchemaWrite; -use function count; use function sprintf; /** @@ -38,9 +37,8 @@ public function check( SchemaWrite $write, ObjectClassChain $classes, ): array { - foreach ($write->entry->getAttributes() as $attribute) { - $attrType = $this->schema->getAttributeType($attribute->getName()); - $count = count($attribute->getValues()); + foreach ($write->valueCounts() as $name => $count) { + $attrType = $this->schema->getAttributeType($name); if ($attrType === null || !$attrType->singleValue || $count <= 1) { continue; @@ -51,9 +49,8 @@ public function check( SchemaRule::SingleValue, ResultCode::CONSTRAINT_VIOLATION, sprintf( - 'Attribute "%s" is single-valued but has %d values.', - $attribute->getName(), - $count, + 'Attribute "%s" is single-valued but has more than one value.', + $attrType->primaryName(), ), ), ]; diff --git a/src/FreeDSx/Ldap/Schema/Validation/SchemaWrite.php b/src/FreeDSx/Ldap/Schema/Validation/SchemaWrite.php index 29e066e1..60a96374 100644 --- a/src/FreeDSx/Ldap/Schema/Validation/SchemaWrite.php +++ b/src/FreeDSx/Ldap/Schema/Validation/SchemaWrite.php @@ -13,10 +13,13 @@ namespace FreeDSx\Ldap\Schema\Validation; +use FreeDSx\Ldap\Entry\Attribute; use FreeDSx\Ldap\Entry\Change; use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Operation\OperationType; +use function count; + /** * The entry a write leaves behind, and what the client supplied to get there. * @@ -27,12 +30,14 @@ /** * @param Change[] $changes * @param bool $isSystem Whether the server initiated the write. + * @param array $linkedCounts Values of linked attributes the entry does not carry, by lowercased name. */ private function __construct( public OperationType $operation, public Entry $entry, public array $changes, public bool $isSystem, + public array $linkedCounts = [], ) {} public static function add( @@ -49,20 +54,50 @@ public static function add( /** * @param Change[] $changes + * @param array $linkedCounts */ public static function modify( Entry $result, array $changes, bool $isSystem = false, + array $linkedCounts = [], ): self { return new self( OperationType::Modify, $result, $changes, $isSystem, + $linkedCounts, ); } + /** + * Whether a linked count, rather than the entry, says what the attribute holds. + */ + public function hasLinkedCount(Attribute $attribute): bool + { + return !$attribute->hasOptions() + && isset($this->linkedCounts[Attribute::normalizeName($attribute->getName())]); + } + + /** + * How many values each attribute holds, a linked count standing in for what the entry carries. + * + * @return iterable + */ + public function valueCounts(): iterable + { + foreach ($this->entry->getAttributes() as $attribute) { + if ($this->hasLinkedCount($attribute)) { + continue; + } + + yield $attribute->getName() => count($attribute->getValues()); + } + + yield from $this->linkedCounts; + } + /** * The new RDN is read from the entry's DN, which the rename has already given it. */ diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/Contract/PdoLinkReadDialectInterface.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/Contract/PdoLinkReadDialectInterface.php index 0645f2e7..514e6233 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/Contract/PdoLinkReadDialectInterface.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/Contract/PdoLinkReadDialectInterface.php @@ -70,4 +70,11 @@ public function queryHeldLinkValues(int $count): string; * Parameters: [owner lc_dn, attr_name_lower] */ public function queryAnyLinkValue(): string; + + /** + * How many DNs an owner links under an attribute, counted no further than the bound. + * + * Parameters: [owner lc_dn, attr_name_lower, bound] + */ + public function queryLinkCount(): string; } diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/Sql/PdoLinkReadDialectTrait.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/Sql/PdoLinkReadDialectTrait.php index 474fad92..ff832ebe 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/Sql/PdoLinkReadDialectTrait.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/Sql/PdoLinkReadDialectTrait.php @@ -91,6 +91,20 @@ public function queryAnyLinkValue(): string SQL; } + public function queryLinkCount(): string + { + return <<linkedValuesOf($owner, $attribute)), + $upTo, + ); + } + public function anyLinkValue( Dn $owner, string $attribute, diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Pdo/Writer/EntryLinkWriter.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Pdo/Writer/EntryLinkWriter.php index e8e731d1..e2635323 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Pdo/Writer/EntryLinkWriter.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Pdo/Writer/EntryLinkWriter.php @@ -20,6 +20,7 @@ use FreeDSx\Ldap\Server\Backend\Storage\Adapter\Dialect\Contract\PdoLinkReadDialectInterface; use FreeDSx\Ldap\Server\Backend\Storage\Adapter\Pdo\Connection\PdoConnection; use FreeDSx\Ldap\Server\Backend\Storage\Adapter\Pdo\Statement\PdoColumnCastTrait; +use FreeDSx\Ldap\Server\Backend\Storage\Capability\LinkedValueCountInterface; use FreeDSx\Ldap\Server\Backend\Storage\Capability\LinkedValueLookupInterface; use FreeDSx\Ldap\Server\Backend\Storage\Link\LinkDelta; use FreeDSx\Ldap\Server\Backend\Storage\Schema\LinkedAttributes; @@ -35,7 +36,7 @@ * * @author Chad Sikorra */ -final readonly class EntryLinkWriter implements LinkedValueLookupInterface +final readonly class EntryLinkWriter implements LinkedValueLookupInterface, LinkedValueCountInterface { use PdoColumnCastTrait; @@ -173,14 +174,22 @@ public function heldLinkValues( } $held = []; - foreach (array_chunk(array_keys($asked), self::DNS_PER_STATEMENT) as $chunk) { - $params = [ - $owner->normalizedString(), - Attribute::normalizeName($attribute), - ...$chunk, - ]; + $chunks = array_chunk( + array_keys($asked), + self::DNS_PER_STATEMENT, + ); + + foreach ($chunks as $chunk) { + $rows = $this->rowsOf( + $this->dialect->queryHeldLinkValues(count($chunk)), + [ + $owner->normalizedString(), + Attribute::normalizeName($attribute), + ...$chunk, + ], + ); - foreach ($this->rowsOf($this->dialect->queryHeldLinkValues(count($chunk)), $params) as $row) { + foreach ($rows as $row) { $held[] = $asked[$this->stringColumn($row['lc_dn'] ?? null)] ?? null; } } @@ -188,6 +197,30 @@ public function heldLinkValues( return array_values(array_filter($held)); } + /** + * How many values the owner links, counted on the connection its own write runs on. + */ + public function linkCount( + Dn $owner, + string $attribute, + int $upTo, + ): int { + $row = $this->connection + ->execute( + $this->dialect->queryLinkCount(), + [ + $owner->normalizedString(), + Attribute::normalizeName($attribute), + $upTo, + ], + ) + ->fetch(); + + return is_array($row) + ? $this->intColumn($row['link_count'] ?? null) + : 0; + } + /** * One value the owner links, which is all that answering whether it holds the attribute takes. */ diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Capability/LinkedValueCountInterface.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Capability/LinkedValueCountInterface.php new file mode 100644 index 00000000..2122f545 --- /dev/null +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/Capability/LinkedValueCountInterface.php @@ -0,0 +1,33 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Ldap\Server\Backend\Storage\Capability; + +use FreeDSx\Ldap\Entry\Dn; + +/** + * Counts what an entry links without reading the values it links. + * + * @author Chad Sikorra + */ +interface LinkedValueCountInterface +{ + /** + * How many values the entry links for the attribute, counted no further than the bound. + */ + public function linkCount( + Dn $owner, + string $attribute, + int $upTo, + ): int; +} diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/AppliesEntryUpdate.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/AppliesEntryUpdate.php index 2a3ad954..b6c624b3 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/AppliesEntryUpdate.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/AppliesEntryUpdate.php @@ -16,6 +16,7 @@ use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Server\Backend\Write\Command\UpdateCommand; +use FreeDSx\Ldap\Server\Backend\Write\Operation\Linked\SettledLinks; use FreeDSx\Ldap\Server\Backend\Write\WriteContext; /** @@ -38,11 +39,13 @@ private function updated( UpdateCommand $command, WriteContext $context, Entry $current, + SettledLinks $links = new SettledLinks(), ): Entry { $updated = $this->mutation->forUpdate( $current, $command, $context, + $links, ); $this->placement->assertUpdatePlacement( $updated, diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/EntryMutation.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/EntryMutation.php index 04dd4398..2918e197 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/EntryMutation.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/EntryMutation.php @@ -19,6 +19,7 @@ use FreeDSx\Ldap\Server\Backend\Write\OperationalAttributeGenerator; use FreeDSx\Ldap\Server\Backend\Write\Command\MoveCommand; use FreeDSx\Ldap\Server\Backend\Write\Command\UpdateCommand; +use FreeDSx\Ldap\Server\Backend\Write\Operation\Linked\SettledLinks; use FreeDSx\Ldap\Server\Backend\Write\Schema\SchemaViolationGate; use FreeDSx\Ldap\Server\Backend\Write\WriteContext; @@ -36,18 +37,23 @@ public function __construct( ) {} /** + * @param SettledLinks $links Linked changes the command no longer carries, held to the schema with it. + * * @throws OperationException */ public function forUpdate( Entry $entry, UpdateCommand $command, WriteContext $context, + SettledLinks $links = new SettledLinks(), ): Entry { $updated = $this->entryHandler->apply($entry, $command); $this->schemaGate->assertModifyAllowed( $command, $updated, $context, + $links->changes, + $links->counts, ); $this->operationalAttrs->applyForModify( $updated, diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/LinkedChangeReplay.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/LinkedChangeReplay.php index 3e00c58b..a6b428c7 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/LinkedChangeReplay.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/LinkedChangeReplay.php @@ -19,11 +19,15 @@ use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Operation\ResultCode; use FreeDSx\Ldap\Schema\Validation\SchemaValidator; +use FreeDSx\Ldap\Server\Backend\Storage\Capability\LinkedValueCountInterface; use FreeDSx\Ldap\Server\Backend\Storage\Capability\LinkedValueLookupInterface; use FreeDSx\Ldap\Server\Backend\Storage\Link\LinkDelta; use function array_diff_key; +use function array_keys; use function array_values; +use function count; +use function min; use function sprintf; /** @@ -35,8 +39,13 @@ */ readonly class LinkedChangeReplay { + /** + * A count only has to tell none, one, and more than one value apart. + */ + private const COUNTED_UP_TO = 2; + public function __construct( - private LinkedValueLookupInterface $lookup, + private LinkedValueLookupInterface&LinkedValueCountInterface $lookup, private SchemaValidator $schema, ) {} @@ -50,9 +59,9 @@ public function __construct( public function settle( array $changes, Dn $owner, - ): LinkDelta { + ): SettledLinks { if ($changes === []) { - return new LinkDelta(); + return new SettledLinks(); } $before = $this->heldBefore( $changes, @@ -65,14 +74,82 @@ public function settle( $values = array_values($change->getAttribute()->getValues()); $state[$name] = $change->isAdd() - ? $this->withAdded($name, $state[$name] ?? [], $values) - : $this->withRemoved($name, $state[$name] ?? [], $values); + ? $this->withAdded( + $name, + $state[$name] ?? [], + $values, + ) + : $this->withRemoved( + $name, + $state[$name] ?? [], + $values, + ); } - return $this->netOf( + $delta = $this->netOf( $before, $state, ); + + return new SettledLinks( + $delta, + $changes, + $this->countsAfter( + array_keys($state), + $delta, + $owner, + ), + ); + } + + /** + * What each attribute holds once the delta applies. + * + * @param list $names + * + * @return array + */ + private function countsAfter( + array $names, + LinkDelta $delta, + Dn $owner, + ): array { + $counts = []; + + foreach ($names as $name) { + $counts[$name] = $this->countAfter( + $owner, + $name, + count($delta->added($name)), + count($delta->removed($name)), + ); + } + + return $counts; + } + + /** + * Stored values are counted only as far as the ones removed plus the bound, whatever the attribute holds. + */ + private function countAfter( + Dn $owner, + string $name, + int $adding, + int $removing, + ): int { + if ($adding >= self::COUNTED_UP_TO) { + return self::COUNTED_UP_TO; + } + $stored = $this->lookup->linkCount( + $owner, + $name, + $removing + self::COUNTED_UP_TO, + ); + + return min( + self::COUNTED_UP_TO, + $stored - $removing + $adding, + ); } /** @@ -98,7 +175,13 @@ private function heldBefore( $held = []; foreach ($named as $name => $values) { - foreach ($this->lookup->heldLinkValues($owner, $name, array_values($values)) as $value) { + $heldValues = $this->lookup->heldLinkValues( + $owner, + $name, + array_values($values), + ); + + foreach ($heldValues as $value) { $held[$name][self::keyOf($value)] = $value; } } @@ -126,13 +209,19 @@ private function withAdded( if (isset($adding[$key])) { throw new OperationException( - sprintf('The attribute "%s" was given the same value twice.', $name), + sprintf( + 'The attribute "%s" was given the same value twice.', + $name, + ), ResultCode::ATTRIBUTE_OR_VALUE_EXISTS, ); } if (isset($current[$key])) { throw new OperationException( - sprintf('The entry already holds a value given for the attribute "%s".', $name), + sprintf( + 'The entry already holds a value given for the attribute "%s".', + $name, + ), ResultCode::ATTRIBUTE_OR_VALUE_EXISTS, ); } @@ -165,7 +254,10 @@ private function withRemoved( if (!isset($current[$key])) { throw new OperationException( - sprintf('The entry does not hold a value given for the attribute "%s".', $name), + sprintf( + 'The entry does not hold a value given for the attribute "%s".', + $name, + ), ResultCode::NO_SUCH_ATTRIBUTE, ); } @@ -196,11 +288,20 @@ private function netOf( foreach ($after as $name => $values) { $held = $before[$name] ?? []; - $linking = array_values(array_diff_key($values, $held)); - $unlinking = array_values(array_diff_key($held, $values)); + $linking = array_values(array_diff_key( + $values, + $held, + )); + $unlinking = array_values(array_diff_key( + $held, + $values, + )); if ($linking !== []) { - $this->schema->validateValues(new Attribute($name, ...$linking)); + $this->schema->validateValues(new Attribute( + $name, + ...$linking, + )); $added[$name] = $linking; } if ($unlinking !== []) { diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/LinkedChanges.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/LinkedChanges.php index 2f307992..d745f369 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/LinkedChanges.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/LinkedChanges.php @@ -13,14 +13,17 @@ namespace FreeDSx\Ldap\Server\Backend\Write\Operation\Linked; +use FreeDSx\Ldap\Entry\Attribute; use FreeDSx\Ldap\Entry\Change; use FreeDSx\Ldap\Entry\Dn; +use FreeDSx\Ldap\Schema\Definition\AttributeTypeOid; use FreeDSx\Ldap\Server\Backend\Storage\Schema\LinkedAttributes; use FreeDSx\Ldap\Server\Backend\Write\Command\UpdateCommand; use FreeDSx\Ldap\Server\Backend\Write\WriteContext; use function array_filter; use function array_values; +use function strtolower; /** * Separates the linked values a modify changes from the changes its entry is still stored whole for. @@ -47,6 +50,10 @@ public function changesApart( $apart = []; foreach ($command->changes as $change) { + // Changed classes are held to every linked value the entry keeps, which only the whole entry carries. + if (self::changesObjectClass($change)) { + return []; + } if (!$this->linked->links($change->getAttribute())) { continue; } @@ -85,6 +92,12 @@ public function remaining( ); } + private static function changesObjectClass(Change $change): bool + { + return Attribute::normalizeName($change->getAttribute()->getName()) + === strtolower(AttributeTypeOid::NAME_OBJECT_CLASS); + } + /** * Whether the change names the values it changes, rather than restating what the attribute holds. */ diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/SettledLinks.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/SettledLinks.php new file mode 100644 index 00000000..40138ed2 --- /dev/null +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/SettledLinks.php @@ -0,0 +1,38 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FreeDSx\Ldap\Server\Backend\Write\Operation\Linked; + +use FreeDSx\Ldap\Entry\Change; +use FreeDSx\Ldap\Server\Backend\Storage\Link\LinkDelta; + +/** + * What a modify's linked changes amount to once replayed against what the owner links. + * + * @internal + * + * @author Chad Sikorra + */ +readonly class SettledLinks +{ + /** + * @param LinkDelta $delta The values to store, net of changes that cancel out. + * @param list $changes The linked changes as the client supplied them. + * @param array $counts Values each attribute holds afterwards, by lowercased name, counted up to two. + */ + public function __construct( + public LinkDelta $delta = new LinkDelta(), + public array $changes = [], + public array $counts = [], + ) {} +} diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/UpdateEntryHandler.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/UpdateEntryHandler.php index 7d312741..1d203f2a 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/UpdateEntryHandler.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/UpdateEntryHandler.php @@ -71,8 +71,9 @@ function (Entry $current) use ($apart, $remaining, $context): ModifiedEntry { $remaining, $context, $current, + $links, ), - $links, + $links->delta, ); }, ); diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Schema/SchemaViolationGate.php b/src/FreeDSx/Ldap/Server/Backend/Write/Schema/SchemaViolationGate.php index b6d6ef10..d7ed0be9 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Schema/SchemaViolationGate.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Schema/SchemaViolationGate.php @@ -15,6 +15,7 @@ use FreeDSx\Ldap\Control\Control; use FreeDSx\Ldap\Entry\Attribute; +use FreeDSx\Ldap\Entry\Change; use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Exception\SchemaRuleException; @@ -53,18 +54,24 @@ public function assertAddAllowed( } /** + * @param list $linkedChanges Changes to linked values, which the command no longer carries. + * @param array $linkedCounts What those linked attributes come to, by lowercased name. + * * @throws OperationException */ public function assertModifyAllowed( UpdateCommand $command, Entry $updated, WriteContext $context, + array $linkedChanges = [], + array $linkedCounts = [], ): void { $this->assertAllowed( SchemaWrite::modify( $updated, - $command->changes, + [...$command->changes, ...$linkedChanges], $context->isSystem(), + $linkedCounts, ), $context, ); diff --git a/tests/integration/Storage/Concern/LinkedAttributeTestsTrait.php b/tests/integration/Storage/Concern/LinkedAttributeTestsTrait.php index 23b799e8..65ca8281 100644 --- a/tests/integration/Storage/Concern/LinkedAttributeTestsTrait.php +++ b/tests/integration/Storage/Concern/LinkedAttributeTestsTrait.php @@ -13,6 +13,7 @@ namespace Tests\Integration\FreeDSx\Ldap\Storage\Concern; +use FreeDSx\Ldap\Controls; use FreeDSx\Ldap\Entry\Change; use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Exception\OperationException; @@ -39,7 +40,10 @@ public function testAddingAMemberKeepsThoseAlreadyLinked(): void $this->ldapClient()->send(Operations::modify( $group, - Change::add('member', self::LINKED_ADMIN), + Change::add( + 'member', + self::LINKED_ADMIN, + ), )); self::assertEqualsCanonicalizing( @@ -54,11 +58,17 @@ public function testDeletingAMemberLeavesTheRestLinked(): void $this->ldapClient()->send(Operations::modify( $group, - Change::add('member', self::LINKED_ADMIN), + Change::add( + 'member', + self::LINKED_ADMIN, + ), )); $this->ldapClient()->send(Operations::modify( $group, - Change::delete('member', self::LINKED_USER), + Change::delete( + 'member', + self::LINKED_USER, + ), )); self::assertSame( @@ -76,7 +86,10 @@ public function testDeletingAMemberInAnotherSpellingRemovesIt(): void $this->ldapClient()->send(Operations::modify( $group, - Change::delete('member', 'CN=User, DC=foo, DC=bar'), + Change::delete( + 'member', + 'CN=User, DC=foo, DC=bar', + ), )); self::assertSame( @@ -94,8 +107,14 @@ public function testAddingAndDeletingAMemberInOneModifyKeepsTheOthers(): void $this->ldapClient()->send(Operations::modify( $group, - Change::add('member', self::LINKED_ALICE), - Change::delete('member', self::LINKED_ALICE), + Change::add( + 'member', + self::LINKED_ALICE, + ), + Change::delete( + 'member', + self::LINKED_ALICE, + ), )); self::assertEqualsCanonicalizing( @@ -113,8 +132,14 @@ public function testDeletingAndAddingAMemberInOneModifyKeepsIt(): void $this->ldapClient()->send(Operations::modify( $group, - Change::delete('member', self::LINKED_USER), - Change::add('member', self::LINKED_USER), + Change::delete( + 'member', + self::LINKED_USER, + ), + Change::add( + 'member', + self::LINKED_USER, + ), )); self::assertEqualsCanonicalizing( @@ -131,8 +156,85 @@ public function testDeletingAMemberTwiceInOneModifyIsRefused(): void $this->ldapClient()->send(Operations::modify( $group, - Change::delete('member', self::LINKED_USER), - Change::delete('member', self::LINKED_USER), + Change::delete( + 'member', + self::LINKED_USER, + ), + Change::delete( + 'member', + self::LINKED_USER, + ), + )); + } + + public function testALinkedValueTheObjectClassesDoNotPermitIsRefused(): void + { + $person = $this->seedPerson('link-person'); + + $this->expectOperationCode(ResultCode::OBJECT_CLASS_VIOLATION); + + $this->ldapClient()->send(Operations::modify( + $person, + Change::add( + 'member', + self::LINKED_ADMIN, + ), + )); + } + + public function testALinkedValueTheObjectClassesDoNotPermitIsRefusedWhenAControlReadsTheEntry(): void + { + $person = $this->seedPerson('link-person-read'); + + $this->expectOperationCode(ResultCode::OBJECT_CLASS_VIOLATION); + + $this->ldapClient()->send( + Operations::modify( + $person, + Change::add( + 'member', + self::LINKED_ADMIN, + ), + ), + Controls::preRead('cn'), + ); + } + + public function testAUserAddingAPasswordPolicyPointerIsRefused(): void + { + $person = $this->seedPerson('link-policy'); + + $this->expectOperationCode(ResultCode::CONSTRAINT_VIOLATION); + + $this->ldapClient()->send(Operations::modify( + $person, + Change::add( + 'pwdPolicySubentry', + self::LINKED_ADMIN, + ), + )); + } + + public function testDroppingTheClassThatPermitsAKeptLinkedValueIsRefused(): void + { + $person = $this->seedPerson( + 'link-classes', + ['extensibleObject'], + ['member' => self::LINKED_USER], + ); + + $this->expectOperationCode(ResultCode::OBJECT_CLASS_VIOLATION); + + $this->ldapClient()->send(Operations::modify( + $person, + Change::delete( + 'objectClass', + 'extensibleObject', + ), + Change::add( + 'seeAlso', + self::LINKED_ADMIN, + ), )); } @@ -142,7 +244,10 @@ public function testReplacingTheMembersWritesThemWhole(): void $this->ldapClient()->send(Operations::modify( $group, - Change::replace('member', self::LINKED_ADMIN), + Change::replace( + 'member', + self::LINKED_ADMIN, + ), )); self::assertSame( @@ -159,7 +264,10 @@ public function testAddingAMemberAlreadyLinkedIsRefused(): void $this->ldapClient()->send(Operations::modify( $group, - Change::add('member', self::LINKED_USER), + Change::add( + 'member', + self::LINKED_USER, + ), )); } @@ -171,7 +279,10 @@ public function testDeletingAMemberThatIsNotLinkedIsRefused(): void $this->ldapClient()->send(Operations::modify( $group, - Change::delete('member', self::LINKED_ADMIN), + Change::delete( + 'member', + self::LINKED_ADMIN, + ), )); } @@ -183,7 +294,11 @@ public function testTheSameMemberGivenTwiceIsRefused(): void $this->ldapClient()->send(Operations::modify( $group, - Change::add('member', self::LINKED_ADMIN, self::LINKED_ADMIN), + Change::add( + 'member', + self::LINKED_ADMIN, + self::LINKED_ADMIN, + ), )); } @@ -195,7 +310,10 @@ public function testAMemberThatIsNotADistinguishedNameIsRefused(): void $this->ldapClient()->send(Operations::modify( $group, - Change::add('member', 'not a dn'), + Change::add( + 'member', + 'not a dn', + ), )); } @@ -206,7 +324,10 @@ public function testARefusedMemberChangeLeavesTheGroupAlone(): void try { $this->ldapClient()->send(Operations::modify( $group, - Change::add('member', self::LINKED_USER), + Change::add( + 'member', + self::LINKED_USER, + ), )); } catch (OperationException) { } @@ -358,6 +479,31 @@ private function seedGroup( return $dn; } + /** + * @param list $auxiliaries + * @param array $attributes + */ + private function seedPerson( + string $cn, + array $auxiliaries = [], + array $attributes = [], + ): string { + $this->authenticateAdmin(); + $dn = "cn={$cn},dc=foo,dc=bar"; + + $this->ldapClient()->create(Entry::fromArray( + $dn, + [ + 'cn' => $cn, + 'sn' => $cn, + 'objectClass' => ['person', ...$auxiliaries], + ...$attributes, + ], + )); + + return $dn; + } + /** * @return list */ diff --git a/tests/integration/Storage/Pdo/EntryLinkWriterTest.php b/tests/integration/Storage/Pdo/EntryLinkWriterTest.php index ec8bc1b3..102128d1 100644 --- a/tests/integration/Storage/Pdo/EntryLinkWriterTest.php +++ b/tests/integration/Storage/Pdo/EntryLinkWriterTest.php @@ -14,7 +14,6 @@ namespace Tests\Integration\FreeDSx\Ldap\Storage\Pdo; use FreeDSx\Ldap\Container; -use FreeDSx\Ldap\Entry\Attribute; use FreeDSx\Ldap\Entry\Dn; use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Server\Backend\Storage\Adapter\Pdo\Writer\EntryLinkWriter; @@ -30,6 +29,8 @@ final class EntryLinkWriterTest extends TestCase private const BOB = 'cn=Bob,dc=example,dc=com'; + private const CAROL = 'cn=Carol,dc=example,dc=com'; + private EntryLinkWriter $subject; protected function setUp(): void @@ -38,21 +39,33 @@ protected function setUp(): void $this->subject = $container->get(EntryLinkWriter::class); $container->get(LdapImporter::class)->importEntries([ - new Entry( - new Dn(self::BASE), - new Attribute('dc', 'example'), + Entry::fromArray( + self::BASE, + ['dc' => ['example']], ), - new Entry( - new Dn(self::BOB), - new Attribute('objectClass', 'person'), - new Attribute('cn', 'Bob'), - new Attribute('sn', 'Bob'), + Entry::fromArray( + self::BOB, + [ + 'objectClass' => ['person'], + 'cn' => ['Bob'], + 'sn' => ['Bob'], + ], ), - new Entry( - new Dn(self::ADMINS), - new Attribute('objectClass', 'groupOfNames'), - new Attribute('cn', 'Admins'), - new Attribute('member', self::BOB), + Entry::fromArray( + self::CAROL, + [ + 'objectClass' => ['person'], + 'cn' => ['Carol'], + 'sn' => ['Carol'], + ], + ), + Entry::fromArray( + self::ADMINS, + [ + 'objectClass' => ['groupOfNames'], + 'cn' => ['Admins'], + 'member' => [self::BOB, self::CAROL], + ], ), ]); } @@ -84,7 +97,43 @@ public function test_a_value_the_owner_does_not_link_is_not_held(): void $this->subject->heldLinkValues( new Dn(self::ADMINS), 'member', - ['cn=Carol,' . self::BASE], + ['cn=Dan,' . self::BASE], + ), + ); + } + + public function test_a_count_under_its_bound_is_every_value_linked(): void + { + self::assertSame( + 2, + $this->subject->linkCount( + new Dn(self::ADMINS), + 'member', + 5, + ), + ); + } + + public function test_a_count_stops_at_its_bound(): void + { + self::assertSame( + 1, + $this->subject->linkCount( + new Dn(self::ADMINS), + 'member', + 1, + ), + ); + } + + public function test_an_attribute_linking_nothing_counts_none(): void + { + self::assertSame( + 0, + $this->subject->linkCount( + new Dn(self::ADMINS), + 'seeAlso', + 5, ), ); } diff --git a/tests/integration/Storage/Pdo/UpdateEntryHandlerTest.php b/tests/integration/Storage/Pdo/UpdateEntryHandlerTest.php index b0d423b5..ea659166 100644 --- a/tests/integration/Storage/Pdo/UpdateEntryHandlerTest.php +++ b/tests/integration/Storage/Pdo/UpdateEntryHandlerTest.php @@ -84,8 +84,14 @@ public function test_deleting_a_member_leaves_the_rest_linked(): void public function test_adding_and_deleting_a_member_in_one_modify_keeps_the_others(): void { $this->modify( - new Change(Change::TYPE_ADD, 'member', $this->dnOf('Carol')), - new Change(Change::TYPE_DELETE, 'member', $this->dnOf('Carol')), + Change::add( + 'member', + $this->dnOf('Carol'), + ), + Change::delete( + 'member', + $this->dnOf('Carol'), + ), ); self::assertSame( diff --git a/tests/support/Server/Configuration/TestServerOptions.php b/tests/support/Server/Configuration/TestServerOptions.php index 5c8e53d4..3d5ce460 100644 --- a/tests/support/Server/Configuration/TestServerOptions.php +++ b/tests/support/Server/Configuration/TestServerOptions.php @@ -43,10 +43,23 @@ public static function unvalidatedCore(): ServerOptions */ public static function validatedCore(SchemaValidationMode $mode): ServerOptions { + return self::validated( + $mode, + SchemaResource::Core, + ); + } + + /** + * The given definitions enforced at the given mode, merged in order. + */ + public static function validated( + SchemaValidationMode $mode, + SchemaSourceInterface ...$sources, + ): ServerOptions { return new ServerOptions( self::transientStorage(), schemaConfig: (new SchemaConfig()) - ->setSources(SchemaResource::Core) + ->setSources(...$sources) ->setValidationMode($mode), ); } diff --git a/tests/unit/Schema/Validation/Check/AllowedAttributesCheckTest.php b/tests/unit/Schema/Validation/Check/AllowedAttributesCheckTest.php index d79f266e..b509997d 100644 --- a/tests/unit/Schema/Validation/Check/AllowedAttributesCheckTest.php +++ b/tests/unit/Schema/Validation/Check/AllowedAttributesCheckTest.php @@ -78,6 +78,63 @@ public function test_an_operational_attribute_needs_no_object_class_to_permit_it ); } + public function test_a_linked_count_names_an_attribute_the_entry_does_not_carry(): void + { + self::assertEquals( + [ + new RuleViolation( + SchemaRule::AllowedAttributes, + ResultCode::OBJECT_CLASS_VIOLATION, + 'Attribute "member" is not permitted by any object class.', + ), + ], + $this->checkWithLinkedCounts( + $this->personOf(['top', 'person']), + ['member' => 1], + ), + ); + } + + public function test_a_linked_count_of_none_stands_in_for_values_the_entry_carries(): void + { + self::assertSame( + [], + $this->checkWithLinkedCounts( + $this->personOf( + ['top', 'person'], + new Attribute( + 'member', + 'cn=Bob,dc=example,dc=com', + ), + ), + ['member' => 0], + ), + ); + } + + /** + * @param array $linkedCounts + * + * @return list + */ + private function checkWithLinkedCounts( + Entry $entry, + array $linkedCounts, + ): array { + return $this->subject->check( + SchemaWrite::modify( + $entry, + [], + false, + $linkedCounts, + ), + ObjectClassChain::forEntry( + $this->schema, + $entry, + ), + ); + } + /** * @return list */ diff --git a/tests/unit/Schema/Validation/Check/EquivalentValuesCheckTest.php b/tests/unit/Schema/Validation/Check/EquivalentValuesCheckTest.php index 6eceec72..dba160d9 100644 --- a/tests/unit/Schema/Validation/Check/EquivalentValuesCheckTest.php +++ b/tests/unit/Schema/Validation/Check/EquivalentValuesCheckTest.php @@ -149,6 +149,33 @@ public function test_modify_leaves_an_attribute_the_change_does_not_touch_unchec ); } + public function test_modify_leaves_an_attribute_with_a_linked_count_unchecked(): void + { + $write = SchemaWrite::modify( + $this->widgetEntry(new Attribute( + 'widgetLabel', + 'same', + 'SAME', + )), + [ + Change::add(new Attribute( + 'widgetLabel', + 'other', + )), + ], + false, + ['widgetlabel' => 2], + ); + + self::assertSame( + [], + $this->checkWidget( + self::never(), + $write, + ), + ); + } + /** * @return list */ diff --git a/tests/unit/Schema/Validation/Check/SingleValueCheckTest.php b/tests/unit/Schema/Validation/Check/SingleValueCheckTest.php index 600c068c..7f281d94 100644 --- a/tests/unit/Schema/Validation/Check/SingleValueCheckTest.php +++ b/tests/unit/Schema/Validation/Check/SingleValueCheckTest.php @@ -45,7 +45,7 @@ public function test_add_single_valued_violation_reports_a_constraint_violation( new RuleViolation( SchemaRule::SingleValue, ResultCode::CONSTRAINT_VIOLATION, - 'Attribute "employeeNumber" is single-valued but has 2 values.', + 'Attribute "employeeNumber" is single-valued but has more than one value.', ), ], $this->check($this->inetOrgPersonWith(new Attribute('employeeNumber', '001', '002'))), @@ -60,6 +60,64 @@ public function test_a_single_valued_attribute_holding_one_value_passes(): void ); } + public function test_a_linked_count_of_more_than_one_breaks_a_single_valued_attribute(): void + { + self::assertEquals( + [ + new RuleViolation( + SchemaRule::SingleValue, + ResultCode::CONSTRAINT_VIOLATION, + 'Attribute "employeeNumber" is single-valued but has more than one value.', + ), + ], + $this->checkWithLinkedCounts( + $this->inetOrgPersonWith(new Attribute( + 'description', + 'linked elsewhere', + )), + ['employeenumber' => 2], + ), + ); + } + + public function test_a_linked_count_stands_in_for_the_values_the_entry_carries(): void + { + self::assertSame( + [], + $this->checkWithLinkedCounts( + $this->inetOrgPersonWith(new Attribute( + 'employeeNumber', + '001', + '002', + )), + ['employeenumber' => 1], + ), + ); + } + + /** + * @param array $linkedCounts + * + * @return list + */ + private function checkWithLinkedCounts( + Entry $entry, + array $linkedCounts, + ): array { + return $this->subject->check( + SchemaWrite::modify( + $entry, + [], + false, + $linkedCounts, + ), + ObjectClassChain::forEntry( + $this->schema, + $entry, + ), + ); + } + /** * @return list */ diff --git a/tests/unit/Server/Backend/Write/Operation/Linked/LinkedChangeReplayTest.php b/tests/unit/Server/Backend/Write/Operation/Linked/LinkedChangeReplayTest.php index 3bf82257..6f952171 100644 --- a/tests/unit/Server/Backend/Write/Operation/Linked/LinkedChangeReplayTest.php +++ b/tests/unit/Server/Backend/Write/Operation/Linked/LinkedChangeReplayTest.php @@ -20,6 +20,7 @@ use FreeDSx\Ldap\Operation\ResultCode; use FreeDSx\Ldap\Server\Backend\Storage\Link\LinkDelta; use FreeDSx\Ldap\Server\Backend\Write\Operation\Linked\LinkedChangeReplay; +use FreeDSx\Ldap\Server\Backend\Write\Operation\Linked\SettledLinks; use PHPUnit\Framework\TestCase; use Tests\Support\FreeDSx\Ldap\ServerContainerTrait; use Tests\Support\FreeDSx\Ldap\Storage\EntryFixture; @@ -34,18 +35,32 @@ final class LinkedChangeReplayTest extends TestCase private const CAROL = 'cn=Carol,dc=example,dc=com'; + private const DAN = 'cn=Dan,dc=example,dc=com'; + + private const STAFF = 'cn=Staff,dc=example,dc=com'; + private LinkedChangeReplay $subject; protected function setUp(): void { - $storage = EntryFixture::inMemoryStorage(Entry::fromArray( - self::ADMINS, - [ - 'objectClass' => ['groupOfNames'], - 'cn' => ['Admins'], - 'member' => [self::BOB], - ], - )); + $storage = EntryFixture::inMemoryStorage( + Entry::fromArray( + self::ADMINS, + [ + 'objectClass' => ['groupOfNames'], + 'cn' => ['Admins'], + 'member' => [self::BOB], + ], + ), + Entry::fromArray( + self::STAFF, + [ + 'objectClass' => ['groupOfNames'], + 'cn' => ['Staff'], + 'member' => [self::BOB, self::CAROL, self::DAN], + ], + ), + ); $this->subject = $this->containerFor($storage)->get(LinkedChangeReplay::class); } @@ -54,17 +69,68 @@ public function test_no_changes_leave_the_entry_the_whole_truth_for_its_links(): $delta = $this->subject->settle( [], new Dn(self::ADMINS), - ); + )->delta; self::assertTrue($delta->isEmpty()); self::assertFalse($delta->isUntouched()); } + public function test_the_changes_are_kept_as_supplied(): void + { + $changes = [ + self::adding(self::CAROL), + self::deleting(self::CAROL), + ]; + + self::assertSame( + $changes, + $this->settled(...$changes)->changes, + ); + } + + public function test_adding_to_a_held_value_counts_more_than_one(): void + { + self::assertSame( + ['member' => 2], + $this->settled(self::adding(self::CAROL))->counts, + ); + } + + public function test_deleting_the_only_held_value_counts_none(): void + { + self::assertSame( + ['member' => 0], + $this->settled(self::deleting(self::BOB))->counts, + ); + } + + public function test_replacing_the_only_held_value_counts_one(): void + { + self::assertSame( + ['member' => 1], + $this->settled( + self::deleting(self::BOB), + self::adding(self::CAROL), + )->counts, + ); + } + + public function test_a_count_stops_once_it_is_more_than_one(): void + { + self::assertSame( + ['member' => 2], + $this->subject->settle( + [self::deleting(self::BOB)], + new Dn(self::STAFF), + )->counts, + ); + } + public function test_a_delete_and_an_add_net_to_both(): void { $delta = $this->settle( - Change::delete('member', self::BOB), - Change::add('member', self::CAROL), + self::deleting(self::BOB), + self::adding(self::CAROL), ); self::assertSame( @@ -80,8 +146,8 @@ public function test_a_delete_and_an_add_net_to_both(): void public function test_adding_then_deleting_a_value_leaves_the_links_untouched(): void { $delta = $this->settle( - Change::add('member', self::CAROL), - Change::delete('member', self::CAROL), + self::adding(self::CAROL), + self::deleting(self::CAROL), ); self::assertTrue($delta->isUntouched()); @@ -90,8 +156,8 @@ public function test_adding_then_deleting_a_value_leaves_the_links_untouched(): public function test_deleting_then_adding_a_held_value_leaves_the_links_untouched(): void { $delta = $this->settle( - Change::delete('member', self::BOB), - Change::add('member', self::BOB), + self::deleting(self::BOB), + self::adding(self::BOB), ); self::assertTrue($delta->isUntouched()); @@ -100,9 +166,9 @@ public function test_deleting_then_adding_a_held_value_leaves_the_links_untouche public function test_adding_a_value_again_after_deleting_it_links_it(): void { $delta = $this->settle( - Change::add('member', self::CAROL), - Change::delete('member', self::CAROL), - Change::add('member', self::CAROL), + self::adding(self::CAROL), + self::deleting(self::CAROL), + self::adding(self::CAROL), ); self::assertSame( @@ -120,8 +186,8 @@ public function test_adding_a_value_an_earlier_change_added_is_refused(): void $this->expectOperationCode(ResultCode::ATTRIBUTE_OR_VALUE_EXISTS); $this->settle( - Change::add('member', self::CAROL), - Change::add('member', self::CAROL), + self::adding(self::CAROL), + self::adding(self::CAROL), ); } @@ -129,8 +195,7 @@ public function test_the_same_value_twice_in_one_add_is_refused(): void { $this->expectOperationCode(ResultCode::ATTRIBUTE_OR_VALUE_EXISTS); - $this->settle(Change::add( - 'member', + $this->settle(self::adding( self::CAROL, 'CN=Carol, DC=example, DC=com', )); @@ -141,15 +206,14 @@ public function test_deleting_a_value_an_earlier_change_deleted_is_refused(): vo $this->expectOperationCode(ResultCode::NO_SUCH_ATTRIBUTE); $this->settle( - Change::delete('member', self::BOB), - Change::delete('member', self::BOB), + self::deleting(self::BOB), + self::deleting(self::BOB), ); } public function test_a_value_named_twice_in_one_delete_is_removed_once(): void { - $delta = $this->settle(Change::delete( - 'member', + $delta = $this->settle(self::deleting( self::BOB, 'CN=Bob, DC=example, DC=com', )); @@ -164,27 +228,48 @@ public function test_deleting_a_value_that_is_not_a_dn_is_answered_as_not_held() { $this->expectOperationCode(ResultCode::NO_SUCH_ATTRIBUTE); - $this->settle(Change::delete('member', 'not a dn')); + $this->settle(self::deleting('not a dn')); } public function test_adding_a_value_that_is_not_a_dn_is_refused_for_its_syntax(): void { $this->expectOperationCode(ResultCode::INVALID_ATTRIBUTE_SYNTAX); - $this->settle(Change::add('member', 'not a dn')); + $this->settle(self::adding('not a dn')); } public function test_a_value_that_is_not_a_dn_added_then_deleted_is_never_held_to_its_syntax(): void { $delta = $this->settle( - Change::add('member', 'not a dn'), - Change::delete('member', 'not a dn'), + self::adding('not a dn'), + self::deleting('not a dn'), ); self::assertTrue($delta->isUntouched()); } + private static function adding(string ...$members): Change + { + return Change::add( + 'member', + ...$members, + ); + } + + private static function deleting(string ...$members): Change + { + return Change::delete( + 'member', + ...$members, + ); + } + private function settle(Change ...$changes): LinkDelta + { + return $this->settled(...$changes)->delta; + } + + private function settled(Change ...$changes): SettledLinks { return $this->subject->settle( array_values($changes), diff --git a/tests/unit/Server/Backend/Write/Operation/UpdateEntryHandlerTest.php b/tests/unit/Server/Backend/Write/Operation/UpdateEntryHandlerTest.php index 60a102c6..d8291b9d 100644 --- a/tests/unit/Server/Backend/Write/Operation/UpdateEntryHandlerTest.php +++ b/tests/unit/Server/Backend/Write/Operation/UpdateEntryHandlerTest.php @@ -23,6 +23,7 @@ use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Operation\ResultCode; +use FreeDSx\Ldap\Schema\SchemaResource; use FreeDSx\Ldap\Schema\SchemaValidationMode; use FreeDSx\Ldap\Search\Filters; use FreeDSx\Ldap\Server\Backend\Storage\Contract\TransactionalWriteInterface; @@ -95,8 +96,14 @@ public function test_adding_then_deleting_a_member_in_one_modify_keeps_the_other $this->seedAdmins(); $this->modifyAdmins( - new Change(Change::TYPE_ADD, 'member', self::ALICE), - new Change(Change::TYPE_DELETE, 'member', self::ALICE), + Change::add( + 'member', + self::ALICE, + ), + Change::delete( + 'member', + self::ALICE, + ), ); self::assertSame( @@ -110,8 +117,14 @@ public function test_deleting_then_adding_a_member_in_one_modify_keeps_it(): voi $this->seedAdmins(self::ALICE); $this->modifyAdmins( - new Change(Change::TYPE_DELETE, 'member', self::ALICE), - new Change(Change::TYPE_ADD, 'member', self::ALICE), + Change::delete( + 'member', + self::ALICE, + ), + Change::add( + 'member', + self::ALICE, + ), ); self::assertEqualsCanonicalizing( @@ -391,6 +404,89 @@ public function test_a_system_write_still_enforces_a_single_valued_attribute(): ); } + public function test_a_linked_value_the_object_classes_do_not_permit_is_refused(): void + { + $this->validatedGraph(SchemaValidationMode::Strict); + + self::expectException(OperationException::class); + self::expectExceptionCode(ResultCode::OBJECT_CLASS_VIOLATION); + + $this->modify(Change::add( + 'member', + self::ALICE, + )); + } + + public function test_a_linked_value_the_object_classes_do_not_permit_is_refused_when_a_control_reads_the_entry(): void + { + $this->validatedGraph(SchemaValidationMode::Strict); + + self::expectException(OperationException::class); + self::expectExceptionCode(ResultCode::OBJECT_CLASS_VIOLATION); + + $this->updates()->handle( + new UpdateCommand( + new Dn(self::ALICE), + [ + Change::add( + 'member', + self::ALICE, + ), + ], + ), + $this->controlledContext(Controls::preRead('cn')), + ); + } + + public function test_a_user_adding_a_password_policy_pointer_is_refused(): void + { + $this->validatedGraph( + SchemaValidationMode::Strict, + SchemaResource::PasswordPolicy, + ); + + self::expectException(OperationException::class); + self::expectExceptionCode(ResultCode::CONSTRAINT_VIOLATION); + + $this->modify(Change::add( + 'pwdPolicySubentry', + 'dc=example,dc=com', + )); + } + + public function test_a_system_write_adding_a_second_password_policy_pointer_is_refused(): void + { + $this->validatedGraph( + SchemaValidationMode::Strict, + SchemaResource::PasswordPolicy, + ); + $this->storage->store(EntryFixture::withUuid(Entry::fromArray( + self::ALICE, + [ + 'objectClass' => ['top', 'person'], + 'cn' => ['Alice'], + 'sn' => ['Smith'], + 'pwdPolicySubentry' => ['dc=example,dc=com'], + ], + ))); + + self::expectException(OperationException::class); + self::expectExceptionCode(ResultCode::CONSTRAINT_VIOLATION); + + $this->updates()->handle( + new UpdateCommand( + new Dn(self::ALICE), + [ + Change::add( + 'pwdPolicySubentry', + self::ALICE, + ), + ], + ), + $this->systemContext(), + ); + } + public function test_it_is_journaled(): void { $journal = $this->journaledGraph(); @@ -466,8 +562,10 @@ private function modify(Change ...$changes): void /** * A validated fixture needs an entry the core schema actually accepts. */ - private function validatedGraph(SchemaValidationMode $mode): void - { + private function validatedGraph( + SchemaValidationMode $mode, + SchemaResource ...$extra, + ): void { $this->writeGraph( EntryFixture::inMemoryStorage( new Entry( @@ -482,7 +580,11 @@ private function validatedGraph(SchemaValidationMode $mode): void new Attribute('sn', 'Smith'), ), ), - TestServerOptions::validatedCore($mode), + TestServerOptions::validated( + $mode, + SchemaResource::Core, + ...$extra, + ), ); } From c90505081da1d2d3e64c33fe8dbef41ebe321449 Mon Sep 17 00:00:00 2001 From: Chad Sikorra Date: Sun, 18 Oct 2026 18:43:06 -0400 Subject: [PATCH 5/5] Simplify how linked attribute changes are held to the schema. --- .../Provider/StorageContainerProvider.php | 7 +- .../Check/AllowedAttributesCheck.php | 10 +- .../Check/EquivalentValuesCheck.php | 2 +- .../Validation/Check/SingleValueCheck.php | 11 +- .../Ldap/Schema/Validation/SchemaWrite.php | 35 ----- .../Contract/PdoLinkReadDialectInterface.php | 7 - .../Dialect/Sql/PdoLinkReadDialectTrait.php | 14 -- .../Storage/Adapter/InMemoryStorage.php | 15 +-- .../Adapter/Pdo/Writer/EntryLinkWriter.php | 27 +--- .../Capability/LinkedValueCountInterface.php | 33 ----- .../Write/Operation/AppliesEntryUpdate.php | 4 +- .../Backend/Write/Operation/EntryMutation.php | 39 +++++- .../Operation/Linked/LinkedChangeReplay.php | 125 ++++++------------ .../Write/Operation/Linked/LinkedChanges.php | 17 ++- .../Write/Operation/Linked/SettledLinks.php | 38 ------ .../Backend/Write/Operation/ModifiedEntry.php | 5 +- .../Write/Operation/UpdateEntryHandler.php | 7 +- .../Write/Schema/SchemaViolationGate.php | 9 +- .../Storage/Pdo/EntryLinkWriterTest.php | 50 +------ .../Pdo/ValidatedUpdateEntryHandlerTest.php | 98 ++++++++++++++ .../Configuration/TestServerOptions.php | 34 ++++- .../Check/AllowedAttributesCheckTest.php | 57 -------- .../Check/EquivalentValuesCheckTest.php | 27 ---- .../Validation/Check/SingleValueCheckTest.php | 60 +-------- .../Storage/Adapter/InMemoryStorageTest.php | 10 +- .../Linked/LinkedChangeReplayTest.php | 112 +++------------- .../Operation/UpdateEntryHandlerTest.php | 21 +++ 27 files changed, 292 insertions(+), 582 deletions(-) delete mode 100644 src/FreeDSx/Ldap/Server/Backend/Storage/Capability/LinkedValueCountInterface.php delete mode 100644 src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/SettledLinks.php create mode 100644 tests/integration/Storage/Pdo/ValidatedUpdateEntryHandlerTest.php diff --git a/src/FreeDSx/Ldap/Container/Provider/StorageContainerProvider.php b/src/FreeDSx/Ldap/Container/Provider/StorageContainerProvider.php index f13cce9b..5d421b38 100644 --- a/src/FreeDSx/Ldap/Container/Provider/StorageContainerProvider.php +++ b/src/FreeDSx/Ldap/Container/Provider/StorageContainerProvider.php @@ -18,7 +18,6 @@ use FreeDSx\Ldap\Server\Backend\Storage\Adapter\EntryIndexReindexer; use FreeDSx\Ldap\Server\Backend\Storage\Adapter\InMemoryStorage; use FreeDSx\Ldap\Schema\Matching\EqualityComparatorResolver; -use FreeDSx\Ldap\Schema\Validation\SchemaValidator; use FreeDSx\Ldap\Schema\Validation\Syntax\AttributeSyntaxResolver; use FreeDSx\Ldap\Server\Backend\Storage\Adapter\Pdo\Query\EntryLister; use FreeDSx\Ldap\Server\Backend\Storage\Adapter\Pdo\Query\EntryReader; @@ -146,7 +145,10 @@ private function makeLinkedValueLookup(Container $container): LinkedValueLookupI private function makeLinkedChanges(Container $container): LinkedChanges { - return new LinkedChanges($container->get(LinkedAttributes::class)); + return new LinkedChanges( + $container->get(LinkedAttributes::class), + $container->get(ServerOptions::class)->getSchema(), + ); } private function makeLinkedChangeReplay(Container $container): LinkedChangeReplay @@ -155,7 +157,6 @@ private function makeLinkedChangeReplay(Container $container): LinkedChangeRepla $this->isPdo($container) ? $container->get(EntryLinkWriter::class) : $container->get(InMemoryStorage::class), - $container->get(SchemaValidator::class), ); } diff --git a/src/FreeDSx/Ldap/Schema/Validation/Check/AllowedAttributesCheck.php b/src/FreeDSx/Ldap/Schema/Validation/Check/AllowedAttributesCheck.php index 617740d0..860fa114 100644 --- a/src/FreeDSx/Ldap/Schema/Validation/Check/AllowedAttributesCheck.php +++ b/src/FreeDSx/Ldap/Schema/Validation/Check/AllowedAttributesCheck.php @@ -55,13 +55,13 @@ public function check( $classes->may, )); - foreach ($write->valueCounts() as $name => $count) { - $attrType = $this->schema->getAttributeType($name); + foreach ($write->entry->getAttributes() as $attribute) { + $attrType = $this->schema->getAttributeType($attribute->getName()); - if ($count === 0 || $attrType === null || $attrType->usage !== AttributeUsage::UserApplications) { + if ($attrType === null || $attrType->usage !== AttributeUsage::UserApplications) { continue; } - if (isset($allowed[strtolower($attrType->names[0] ?? $name)])) { + if (isset($allowed[strtolower($attrType->names[0] ?? $attribute->getName())])) { continue; } @@ -71,7 +71,7 @@ public function check( ResultCode::OBJECT_CLASS_VIOLATION, sprintf( 'Attribute "%s" is not permitted by any object class.', - $attrType->primaryName(), + $attribute->getName(), ), ), ]; diff --git a/src/FreeDSx/Ldap/Schema/Validation/Check/EquivalentValuesCheck.php b/src/FreeDSx/Ldap/Schema/Validation/Check/EquivalentValuesCheck.php index ec39d37f..14a8355a 100644 --- a/src/FreeDSx/Ldap/Schema/Validation/Check/EquivalentValuesCheck.php +++ b/src/FreeDSx/Ldap/Schema/Validation/Check/EquivalentValuesCheck.php @@ -50,7 +50,7 @@ public function check( if ($only !== null && !isset($only[Attribute::normalizeName($attribute->getDescription())])) { continue; } - if ($write->hasLinkedCount($attribute) || !$this->hasEquivalentValues($attribute)) { + if (!$this->hasEquivalentValues($attribute)) { continue; } diff --git a/src/FreeDSx/Ldap/Schema/Validation/Check/SingleValueCheck.php b/src/FreeDSx/Ldap/Schema/Validation/Check/SingleValueCheck.php index c61fb337..8b35e39c 100644 --- a/src/FreeDSx/Ldap/Schema/Validation/Check/SingleValueCheck.php +++ b/src/FreeDSx/Ldap/Schema/Validation/Check/SingleValueCheck.php @@ -20,6 +20,7 @@ use FreeDSx\Ldap\Schema\Validation\SchemaRule; use FreeDSx\Ldap\Schema\Validation\SchemaWrite; +use function count; use function sprintf; /** @@ -37,8 +38,9 @@ public function check( SchemaWrite $write, ObjectClassChain $classes, ): array { - foreach ($write->valueCounts() as $name => $count) { - $attrType = $this->schema->getAttributeType($name); + foreach ($write->entry->getAttributes() as $attribute) { + $attrType = $this->schema->getAttributeType($attribute->getName()); + $count = count($attribute->getValues()); if ($attrType === null || !$attrType->singleValue || $count <= 1) { continue; @@ -49,8 +51,9 @@ public function check( SchemaRule::SingleValue, ResultCode::CONSTRAINT_VIOLATION, sprintf( - 'Attribute "%s" is single-valued but has more than one value.', - $attrType->primaryName(), + 'Attribute "%s" is single-valued but has %d values.', + $attribute->getName(), + $count, ), ), ]; diff --git a/src/FreeDSx/Ldap/Schema/Validation/SchemaWrite.php b/src/FreeDSx/Ldap/Schema/Validation/SchemaWrite.php index 60a96374..29e066e1 100644 --- a/src/FreeDSx/Ldap/Schema/Validation/SchemaWrite.php +++ b/src/FreeDSx/Ldap/Schema/Validation/SchemaWrite.php @@ -13,13 +13,10 @@ namespace FreeDSx\Ldap\Schema\Validation; -use FreeDSx\Ldap\Entry\Attribute; use FreeDSx\Ldap\Entry\Change; use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Operation\OperationType; -use function count; - /** * The entry a write leaves behind, and what the client supplied to get there. * @@ -30,14 +27,12 @@ /** * @param Change[] $changes * @param bool $isSystem Whether the server initiated the write. - * @param array $linkedCounts Values of linked attributes the entry does not carry, by lowercased name. */ private function __construct( public OperationType $operation, public Entry $entry, public array $changes, public bool $isSystem, - public array $linkedCounts = [], ) {} public static function add( @@ -54,50 +49,20 @@ public static function add( /** * @param Change[] $changes - * @param array $linkedCounts */ public static function modify( Entry $result, array $changes, bool $isSystem = false, - array $linkedCounts = [], ): self { return new self( OperationType::Modify, $result, $changes, $isSystem, - $linkedCounts, ); } - /** - * Whether a linked count, rather than the entry, says what the attribute holds. - */ - public function hasLinkedCount(Attribute $attribute): bool - { - return !$attribute->hasOptions() - && isset($this->linkedCounts[Attribute::normalizeName($attribute->getName())]); - } - - /** - * How many values each attribute holds, a linked count standing in for what the entry carries. - * - * @return iterable - */ - public function valueCounts(): iterable - { - foreach ($this->entry->getAttributes() as $attribute) { - if ($this->hasLinkedCount($attribute)) { - continue; - } - - yield $attribute->getName() => count($attribute->getValues()); - } - - yield from $this->linkedCounts; - } - /** * The new RDN is read from the entry's DN, which the rename has already given it. */ diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/Contract/PdoLinkReadDialectInterface.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/Contract/PdoLinkReadDialectInterface.php index 514e6233..0645f2e7 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/Contract/PdoLinkReadDialectInterface.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/Contract/PdoLinkReadDialectInterface.php @@ -70,11 +70,4 @@ public function queryHeldLinkValues(int $count): string; * Parameters: [owner lc_dn, attr_name_lower] */ public function queryAnyLinkValue(): string; - - /** - * How many DNs an owner links under an attribute, counted no further than the bound. - * - * Parameters: [owner lc_dn, attr_name_lower, bound] - */ - public function queryLinkCount(): string; } diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/Sql/PdoLinkReadDialectTrait.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/Sql/PdoLinkReadDialectTrait.php index ff832ebe..474fad92 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/Sql/PdoLinkReadDialectTrait.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Dialect/Sql/PdoLinkReadDialectTrait.php @@ -91,20 +91,6 @@ public function queryAnyLinkValue(): string SQL; } - public function queryLinkCount(): string - { - return <<linkedValuesOf($owner, $attribute)), - $upTo, - ); - } - public function anyLinkValue( Dn $owner, string $attribute, diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Pdo/Writer/EntryLinkWriter.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Pdo/Writer/EntryLinkWriter.php index e2635323..2bf89730 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Pdo/Writer/EntryLinkWriter.php +++ b/src/FreeDSx/Ldap/Server/Backend/Storage/Adapter/Pdo/Writer/EntryLinkWriter.php @@ -20,7 +20,6 @@ use FreeDSx\Ldap\Server\Backend\Storage\Adapter\Dialect\Contract\PdoLinkReadDialectInterface; use FreeDSx\Ldap\Server\Backend\Storage\Adapter\Pdo\Connection\PdoConnection; use FreeDSx\Ldap\Server\Backend\Storage\Adapter\Pdo\Statement\PdoColumnCastTrait; -use FreeDSx\Ldap\Server\Backend\Storage\Capability\LinkedValueCountInterface; use FreeDSx\Ldap\Server\Backend\Storage\Capability\LinkedValueLookupInterface; use FreeDSx\Ldap\Server\Backend\Storage\Link\LinkDelta; use FreeDSx\Ldap\Server\Backend\Storage\Schema\LinkedAttributes; @@ -36,7 +35,7 @@ * * @author Chad Sikorra */ -final readonly class EntryLinkWriter implements LinkedValueLookupInterface, LinkedValueCountInterface +final readonly class EntryLinkWriter implements LinkedValueLookupInterface { use PdoColumnCastTrait; @@ -197,30 +196,6 @@ public function heldLinkValues( return array_values(array_filter($held)); } - /** - * How many values the owner links, counted on the connection its own write runs on. - */ - public function linkCount( - Dn $owner, - string $attribute, - int $upTo, - ): int { - $row = $this->connection - ->execute( - $this->dialect->queryLinkCount(), - [ - $owner->normalizedString(), - Attribute::normalizeName($attribute), - $upTo, - ], - ) - ->fetch(); - - return is_array($row) - ? $this->intColumn($row['link_count'] ?? null) - : 0; - } - /** * One value the owner links, which is all that answering whether it holds the attribute takes. */ diff --git a/src/FreeDSx/Ldap/Server/Backend/Storage/Capability/LinkedValueCountInterface.php b/src/FreeDSx/Ldap/Server/Backend/Storage/Capability/LinkedValueCountInterface.php deleted file mode 100644 index 2122f545..00000000 --- a/src/FreeDSx/Ldap/Server/Backend/Storage/Capability/LinkedValueCountInterface.php +++ /dev/null @@ -1,33 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace FreeDSx\Ldap\Server\Backend\Storage\Capability; - -use FreeDSx\Ldap\Entry\Dn; - -/** - * Counts what an entry links without reading the values it links. - * - * @author Chad Sikorra - */ -interface LinkedValueCountInterface -{ - /** - * How many values the entry links for the attribute, counted no further than the bound. - */ - public function linkCount( - Dn $owner, - string $attribute, - int $upTo, - ): int; -} diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/AppliesEntryUpdate.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/AppliesEntryUpdate.php index b6c624b3..b056a359 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/AppliesEntryUpdate.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/AppliesEntryUpdate.php @@ -15,8 +15,8 @@ use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Exception\OperationException; +use FreeDSx\Ldap\Server\Backend\Storage\Link\LinkDelta; use FreeDSx\Ldap\Server\Backend\Write\Command\UpdateCommand; -use FreeDSx\Ldap\Server\Backend\Write\Operation\Linked\SettledLinks; use FreeDSx\Ldap\Server\Backend\Write\WriteContext; /** @@ -39,7 +39,7 @@ private function updated( UpdateCommand $command, WriteContext $context, Entry $current, - SettledLinks $links = new SettledLinks(), + LinkDelta $links = new LinkDelta(), ): Entry { $updated = $this->mutation->forUpdate( $current, diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/EntryMutation.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/EntryMutation.php index 2918e197..7dbf0ab3 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/EntryMutation.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/EntryMutation.php @@ -13,13 +13,14 @@ namespace FreeDSx\Ldap\Server\Backend\Write\Operation; +use FreeDSx\Ldap\Entry\Attribute; use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Server\Backend\Storage\Adapter\Operation\WriteEntryOperationHandler; +use FreeDSx\Ldap\Server\Backend\Storage\Link\LinkDelta; use FreeDSx\Ldap\Server\Backend\Write\OperationalAttributeGenerator; use FreeDSx\Ldap\Server\Backend\Write\Command\MoveCommand; use FreeDSx\Ldap\Server\Backend\Write\Command\UpdateCommand; -use FreeDSx\Ldap\Server\Backend\Write\Operation\Linked\SettledLinks; use FreeDSx\Ldap\Server\Backend\Write\Schema\SchemaViolationGate; use FreeDSx\Ldap\Server\Backend\Write\WriteContext; @@ -37,7 +38,7 @@ public function __construct( ) {} /** - * @param SettledLinks $links Linked changes the command no longer carries, held to the schema with it. + * @param LinkDelta $links Linked values applied apart from the entry, whose additions are held to the schema with it. * * @throws OperationException */ @@ -45,15 +46,16 @@ public function forUpdate( Entry $entry, UpdateCommand $command, WriteContext $context, - SettledLinks $links = new SettledLinks(), + LinkDelta $links = new LinkDelta(), ): Entry { $updated = $this->entryHandler->apply($entry, $command); $this->schemaGate->assertModifyAllowed( $command, - $updated, + self::withLinkedAdditions( + $updated, + $links, + ), $context, - $links->changes, - $links->counts, ); $this->operationalAttrs->applyForModify( $updated, @@ -84,4 +86,29 @@ public function forMove( return $moved; } + + private static function withLinkedAdditions( + Entry $entry, + LinkDelta $links, + ): Entry { + if ($links->isEmpty()) { + return $entry; + } + $copy = $entry->makeCopy(); + + foreach ($links->names() as $name) { + $added = $links->added($name); + + if ($added === []) { + continue; + } + + $copy->add(new Attribute( + $name, + ...$added, + )); + } + + return $copy; + } } diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/LinkedChangeReplay.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/LinkedChangeReplay.php index a6b428c7..8c7f25dc 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/LinkedChangeReplay.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/LinkedChangeReplay.php @@ -18,16 +18,12 @@ use FreeDSx\Ldap\Entry\Dn; use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Operation\ResultCode; -use FreeDSx\Ldap\Schema\Validation\SchemaValidator; -use FreeDSx\Ldap\Server\Backend\Storage\Capability\LinkedValueCountInterface; use FreeDSx\Ldap\Server\Backend\Storage\Capability\LinkedValueLookupInterface; use FreeDSx\Ldap\Server\Backend\Storage\Link\LinkDelta; +use FreeDSx\Ldap\Server\Backend\Storage\Search\EntryProjection; use function array_diff_key; -use function array_keys; use function array_values; -use function count; -use function min; use function sprintf; /** @@ -39,36 +35,60 @@ */ readonly class LinkedChangeReplay { + public function __construct(private LinkedValueLookupInterface $lookup) {} + /** - * A count only has to tell none, one, and more than one value apart. + * What of the target the modify needs loaded: none of its links when changes are applied apart, all otherwise. + * + * @param list $changes */ - private const COUNTED_UP_TO = 2; - - public function __construct( - private LinkedValueLookupInterface&LinkedValueCountInterface $lookup, - private SchemaValidator $schema, - ) {} + public function projectionFor(array $changes): EntryProjection + { + return $changes === [] + ? EntryProjection::unbounded() + : new EntryProjection(linkCap: 0); + } /** * RFC 4511 4.6: each change applies to the state the ones before it left. * * @param list $changes * - * @throws OperationException when a value is already linked, is not linked, or is not a DN + * @throws OperationException when a value is already linked or is not linked */ public function settle( array $changes, Dn $owner, - ): SettledLinks { + ): LinkDelta { if ($changes === []) { - return new SettledLinks(); + return new LinkDelta(); } $before = $this->heldBefore( $changes, $owner, ); - $state = $before; + return $this->netOf( + $before, + $this->replayed( + $before, + $changes, + ), + ); + } + + /** + * @param array> $state + * @param list $changes + * + * @return array> + * + * @throws OperationException + */ + private function replayed( + array $state, + array $changes, + ): array { foreach ($changes as $change) { $name = Attribute::normalizeName($change->getAttribute()->getName()); $values = array_values($change->getAttribute()->getValues()); @@ -86,70 +106,7 @@ public function settle( ); } - $delta = $this->netOf( - $before, - $state, - ); - - return new SettledLinks( - $delta, - $changes, - $this->countsAfter( - array_keys($state), - $delta, - $owner, - ), - ); - } - - /** - * What each attribute holds once the delta applies. - * - * @param list $names - * - * @return array - */ - private function countsAfter( - array $names, - LinkDelta $delta, - Dn $owner, - ): array { - $counts = []; - - foreach ($names as $name) { - $counts[$name] = $this->countAfter( - $owner, - $name, - count($delta->added($name)), - count($delta->removed($name)), - ); - } - - return $counts; - } - - /** - * Stored values are counted only as far as the ones removed plus the bound, whatever the attribute holds. - */ - private function countAfter( - Dn $owner, - string $name, - int $adding, - int $removing, - ): int { - if ($adding >= self::COUNTED_UP_TO) { - return self::COUNTED_UP_TO; - } - $stored = $this->lookup->linkCount( - $owner, - $name, - $removing + self::COUNTED_UP_TO, - ); - - return min( - self::COUNTED_UP_TO, - $stored - $removing + $adding, - ); + return $state; } /** @@ -272,12 +229,10 @@ private function withRemoved( } /** - * Values the replay left changed, held to the syntax the whole entry would have been for the ones it adds. + * The values the replay left changed. * * @param array> $before * @param array> $after - * - * @throws OperationException */ private function netOf( array $before, @@ -298,10 +253,6 @@ private function netOf( )); if ($linking !== []) { - $this->schema->validateValues(new Attribute( - $name, - ...$linking, - )); $added[$name] = $linking; } if ($unlinking !== []) { diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/LinkedChanges.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/LinkedChanges.php index d745f369..cb7bb1c0 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/LinkedChanges.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/LinkedChanges.php @@ -17,6 +17,7 @@ use FreeDSx\Ldap\Entry\Change; use FreeDSx\Ldap\Entry\Dn; use FreeDSx\Ldap\Schema\Definition\AttributeTypeOid; +use FreeDSx\Ldap\Schema\Schema; use FreeDSx\Ldap\Server\Backend\Storage\Schema\LinkedAttributes; use FreeDSx\Ldap\Server\Backend\Write\Command\UpdateCommand; use FreeDSx\Ldap\Server\Backend\Write\WriteContext; @@ -32,7 +33,10 @@ */ final readonly class LinkedChanges { - public function __construct(private LinkedAttributes $linked) {} + public function __construct( + private LinkedAttributes $linked, + private Schema $schema, + ) {} /** * The linked changes applied apart from the entry, or none when the modify has to be applied to the whole entry. @@ -109,6 +113,9 @@ private function isApplicable( if ($dn->getRdn()->has($change->getAttribute()->getName())) { return false; } + if ($this->isHeldToEveryValue($change)) { + return false; + } if ($change->isAdd()) { return $change->getAttribute()->getValues() !== []; @@ -117,4 +124,12 @@ private function isApplicable( // A delete naming no value empties the attribute, which takes knowing everything it holds. return $change->isDelete() && !$change->isReset(); } + + private function isHeldToEveryValue(Change $change): bool + { + $type = $this->schema->getAttributeType($change->getAttribute()->getName()); + + return $type !== null + && ($type->singleValue || $type->noUserModification); + } } diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/SettledLinks.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/SettledLinks.php deleted file mode 100644 index 40138ed2..00000000 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/Linked/SettledLinks.php +++ /dev/null @@ -1,38 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace FreeDSx\Ldap\Server\Backend\Write\Operation\Linked; - -use FreeDSx\Ldap\Entry\Change; -use FreeDSx\Ldap\Server\Backend\Storage\Link\LinkDelta; - -/** - * What a modify's linked changes amount to once replayed against what the owner links. - * - * @internal - * - * @author Chad Sikorra - */ -readonly class SettledLinks -{ - /** - * @param LinkDelta $delta The values to store, net of changes that cancel out. - * @param list $changes The linked changes as the client supplied them. - * @param array $counts Values each attribute holds afterwards, by lowercased name, counted up to two. - */ - public function __construct( - public LinkDelta $delta = new LinkDelta(), - public array $changes = [], - public array $counts = [], - ) {} -} diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/ModifiedEntry.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/ModifiedEntry.php index 9992fd2b..53495e39 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/ModifiedEntry.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/ModifiedEntry.php @@ -25,8 +25,11 @@ */ readonly class ModifiedEntry { + /** + * @param LinkDelta $links Stated by every producer, since a plain delta rewrites the links from the entry itself. + */ public function __construct( public Entry $entry, - public LinkDelta $links = new LinkDelta(), + public LinkDelta $links, ) {} } diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/UpdateEntryHandler.php b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/UpdateEntryHandler.php index 1d203f2a..c06fcb73 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Operation/UpdateEntryHandler.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Operation/UpdateEntryHandler.php @@ -15,7 +15,6 @@ use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Exception\OperationException; -use FreeDSx\Ldap\Server\Backend\Storage\Search\EntryProjection; use FreeDSx\Ldap\Server\Backend\Write\Command\UpdateCommand; use FreeDSx\Ldap\Server\Backend\Write\Operation\Linked\LinkedChangeReplay; use FreeDSx\Ldap\Server\Backend\Write\Operation\Linked\LinkedChanges; @@ -56,9 +55,7 @@ public function handle( $this->writes->update( $command->dn->normalize(), - $apart === [] - ? EntryProjection::unbounded() - : new EntryProjection(linkCap: 0), + $this->replay->projectionFor($apart), $context, function (Entry $current) use ($apart, $remaining, $context): ModifiedEntry { $links = $this->replay->settle( @@ -73,7 +70,7 @@ function (Entry $current) use ($apart, $remaining, $context): ModifiedEntry { $current, $links, ), - $links->delta, + $links, ); }, ); diff --git a/src/FreeDSx/Ldap/Server/Backend/Write/Schema/SchemaViolationGate.php b/src/FreeDSx/Ldap/Server/Backend/Write/Schema/SchemaViolationGate.php index d7ed0be9..b6d6ef10 100644 --- a/src/FreeDSx/Ldap/Server/Backend/Write/Schema/SchemaViolationGate.php +++ b/src/FreeDSx/Ldap/Server/Backend/Write/Schema/SchemaViolationGate.php @@ -15,7 +15,6 @@ use FreeDSx\Ldap\Control\Control; use FreeDSx\Ldap\Entry\Attribute; -use FreeDSx\Ldap\Entry\Change; use FreeDSx\Ldap\Entry\Entry; use FreeDSx\Ldap\Exception\OperationException; use FreeDSx\Ldap\Exception\SchemaRuleException; @@ -54,24 +53,18 @@ public function assertAddAllowed( } /** - * @param list $linkedChanges Changes to linked values, which the command no longer carries. - * @param array $linkedCounts What those linked attributes come to, by lowercased name. - * * @throws OperationException */ public function assertModifyAllowed( UpdateCommand $command, Entry $updated, WriteContext $context, - array $linkedChanges = [], - array $linkedCounts = [], ): void { $this->assertAllowed( SchemaWrite::modify( $updated, - [...$command->changes, ...$linkedChanges], + $command->changes, $context->isSystem(), - $linkedCounts, ), $context, ); diff --git a/tests/integration/Storage/Pdo/EntryLinkWriterTest.php b/tests/integration/Storage/Pdo/EntryLinkWriterTest.php index 102128d1..10a67596 100644 --- a/tests/integration/Storage/Pdo/EntryLinkWriterTest.php +++ b/tests/integration/Storage/Pdo/EntryLinkWriterTest.php @@ -29,8 +29,6 @@ final class EntryLinkWriterTest extends TestCase private const BOB = 'cn=Bob,dc=example,dc=com'; - private const CAROL = 'cn=Carol,dc=example,dc=com'; - private EntryLinkWriter $subject; protected function setUp(): void @@ -51,20 +49,12 @@ protected function setUp(): void 'sn' => ['Bob'], ], ), - Entry::fromArray( - self::CAROL, - [ - 'objectClass' => ['person'], - 'cn' => ['Carol'], - 'sn' => ['Carol'], - ], - ), Entry::fromArray( self::ADMINS, [ 'objectClass' => ['groupOfNames'], 'cn' => ['Admins'], - 'member' => [self::BOB, self::CAROL], + 'member' => [self::BOB], ], ), ]); @@ -97,43 +87,7 @@ public function test_a_value_the_owner_does_not_link_is_not_held(): void $this->subject->heldLinkValues( new Dn(self::ADMINS), 'member', - ['cn=Dan,' . self::BASE], - ), - ); - } - - public function test_a_count_under_its_bound_is_every_value_linked(): void - { - self::assertSame( - 2, - $this->subject->linkCount( - new Dn(self::ADMINS), - 'member', - 5, - ), - ); - } - - public function test_a_count_stops_at_its_bound(): void - { - self::assertSame( - 1, - $this->subject->linkCount( - new Dn(self::ADMINS), - 'member', - 1, - ), - ); - } - - public function test_an_attribute_linking_nothing_counts_none(): void - { - self::assertSame( - 0, - $this->subject->linkCount( - new Dn(self::ADMINS), - 'seeAlso', - 5, + ['cn=Carol,' . self::BASE], ), ); } diff --git a/tests/integration/Storage/Pdo/ValidatedUpdateEntryHandlerTest.php b/tests/integration/Storage/Pdo/ValidatedUpdateEntryHandlerTest.php new file mode 100644 index 00000000..87ffcfbe --- /dev/null +++ b/tests/integration/Storage/Pdo/ValidatedUpdateEntryHandlerTest.php @@ -0,0 +1,98 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Tests\Integration\FreeDSx\Ldap\Storage\Pdo; + +use FreeDSx\Ldap\Container; +use FreeDSx\Ldap\Control\ControlBag; +use FreeDSx\Ldap\Entry\Change; +use FreeDSx\Ldap\Entry\Dn; +use FreeDSx\Ldap\Entry\Entry; +use FreeDSx\Ldap\Exception\OperationException; +use FreeDSx\Ldap\Operation\ResultCode; +use FreeDSx\Ldap\Schema\SchemaResource; +use FreeDSx\Ldap\Schema\SchemaValidationMode; +use FreeDSx\Ldap\Server\Backend\Storage\Import\LdapImporter; +use FreeDSx\Ldap\Server\Backend\Write\Command\UpdateCommand; +use FreeDSx\Ldap\Server\Backend\Write\Operation\UpdateEntryHandler; +use FreeDSx\Ldap\Server\Backend\Write\WriteContext; +use FreeDSx\Ldap\Server\Token\AnonToken; +use PHPUnit\Framework\TestCase; +use Tests\Support\FreeDSx\Ldap\Server\Configuration\TestServerOptions; + +final class ValidatedUpdateEntryHandlerTest extends TestCase +{ + private const BASE = 'o=example'; + + private const BOB = 'cn=Bob,o=example'; + + private UpdateEntryHandler $subject; + + protected function setUp(): void + { + $container = Container::forServer(TestServerOptions::validatedSqlite( + SchemaValidationMode::Strict, + SchemaResource::Core, + SchemaResource::PasswordPolicy, + )); + $this->subject = $container->get(UpdateEntryHandler::class); + + $container->get(LdapImporter::class)->importEntries([ + Entry::fromArray( + self::BASE, + [ + 'objectClass' => ['organization'], + 'o' => ['example'], + ], + ), + Entry::fromArray( + self::BOB, + [ + 'objectClass' => ['person'], + 'cn' => ['Bob'], + 'sn' => ['Bob'], + ], + ), + ]); + } + + public function test_a_system_write_adding_a_second_password_policy_pointer_is_refused(): void + { + $this->systemModify(Change::add( + 'pwdPolicySubentry', + self::BASE, + )); + + $this->expectException(OperationException::class); + $this->expectExceptionCode(ResultCode::CONSTRAINT_VIOLATION); + + $this->systemModify(Change::add( + 'pwdPolicySubentry', + self::BOB, + )); + } + + private function systemModify(Change ...$changes): void + { + $this->subject->handle( + new UpdateCommand( + new Dn(self::BOB), + $changes, + ), + WriteContext::system( + new AnonToken(), + new ControlBag(), + ), + ); + } +} diff --git a/tests/support/Server/Configuration/TestServerOptions.php b/tests/support/Server/Configuration/TestServerOptions.php index 3d5ce460..8b609ec4 100644 --- a/tests/support/Server/Configuration/TestServerOptions.php +++ b/tests/support/Server/Configuration/TestServerOptions.php @@ -56,11 +56,24 @@ public static function validated( SchemaValidationMode $mode, SchemaSourceInterface ...$sources, ): ServerOptions { - return new ServerOptions( + return self::validatedOn( self::transientStorage(), - schemaConfig: (new SchemaConfig()) - ->setSources(...$sources) - ->setValidationMode($mode), + $mode, + ...$sources, + ); + } + + /** + * A transient database whose writes are held to the given definitions at the given mode. + */ + public static function validatedSqlite( + SchemaValidationMode $mode, + SchemaSourceInterface ...$sources, + ): ServerOptions { + return self::validatedOn( + PdoConfig::forSqlite(':memory:'), + $mode, + ...$sources, ); } @@ -123,6 +136,19 @@ public static function cheaplyHashed(): ServerOptions ); } + private static function validatedOn( + StorageConfigInterface $storageConfig, + SchemaValidationMode $mode, + SchemaSourceInterface ...$sources, + ): ServerOptions { + return new ServerOptions( + $storageConfig, + schemaConfig: (new SchemaConfig()) + ->setSources(...$sources) + ->setValidationMode($mode), + ); + } + private static function unvalidatedCoreSchema(): SchemaConfig { return (new SchemaConfig()) diff --git a/tests/unit/Schema/Validation/Check/AllowedAttributesCheckTest.php b/tests/unit/Schema/Validation/Check/AllowedAttributesCheckTest.php index b509997d..d79f266e 100644 --- a/tests/unit/Schema/Validation/Check/AllowedAttributesCheckTest.php +++ b/tests/unit/Schema/Validation/Check/AllowedAttributesCheckTest.php @@ -78,63 +78,6 @@ public function test_an_operational_attribute_needs_no_object_class_to_permit_it ); } - public function test_a_linked_count_names_an_attribute_the_entry_does_not_carry(): void - { - self::assertEquals( - [ - new RuleViolation( - SchemaRule::AllowedAttributes, - ResultCode::OBJECT_CLASS_VIOLATION, - 'Attribute "member" is not permitted by any object class.', - ), - ], - $this->checkWithLinkedCounts( - $this->personOf(['top', 'person']), - ['member' => 1], - ), - ); - } - - public function test_a_linked_count_of_none_stands_in_for_values_the_entry_carries(): void - { - self::assertSame( - [], - $this->checkWithLinkedCounts( - $this->personOf( - ['top', 'person'], - new Attribute( - 'member', - 'cn=Bob,dc=example,dc=com', - ), - ), - ['member' => 0], - ), - ); - } - - /** - * @param array $linkedCounts - * - * @return list - */ - private function checkWithLinkedCounts( - Entry $entry, - array $linkedCounts, - ): array { - return $this->subject->check( - SchemaWrite::modify( - $entry, - [], - false, - $linkedCounts, - ), - ObjectClassChain::forEntry( - $this->schema, - $entry, - ), - ); - } - /** * @return list */ diff --git a/tests/unit/Schema/Validation/Check/EquivalentValuesCheckTest.php b/tests/unit/Schema/Validation/Check/EquivalentValuesCheckTest.php index dba160d9..6eceec72 100644 --- a/tests/unit/Schema/Validation/Check/EquivalentValuesCheckTest.php +++ b/tests/unit/Schema/Validation/Check/EquivalentValuesCheckTest.php @@ -149,33 +149,6 @@ public function test_modify_leaves_an_attribute_the_change_does_not_touch_unchec ); } - public function test_modify_leaves_an_attribute_with_a_linked_count_unchecked(): void - { - $write = SchemaWrite::modify( - $this->widgetEntry(new Attribute( - 'widgetLabel', - 'same', - 'SAME', - )), - [ - Change::add(new Attribute( - 'widgetLabel', - 'other', - )), - ], - false, - ['widgetlabel' => 2], - ); - - self::assertSame( - [], - $this->checkWidget( - self::never(), - $write, - ), - ); - } - /** * @return list */ diff --git a/tests/unit/Schema/Validation/Check/SingleValueCheckTest.php b/tests/unit/Schema/Validation/Check/SingleValueCheckTest.php index 7f281d94..600c068c 100644 --- a/tests/unit/Schema/Validation/Check/SingleValueCheckTest.php +++ b/tests/unit/Schema/Validation/Check/SingleValueCheckTest.php @@ -45,7 +45,7 @@ public function test_add_single_valued_violation_reports_a_constraint_violation( new RuleViolation( SchemaRule::SingleValue, ResultCode::CONSTRAINT_VIOLATION, - 'Attribute "employeeNumber" is single-valued but has more than one value.', + 'Attribute "employeeNumber" is single-valued but has 2 values.', ), ], $this->check($this->inetOrgPersonWith(new Attribute('employeeNumber', '001', '002'))), @@ -60,64 +60,6 @@ public function test_a_single_valued_attribute_holding_one_value_passes(): void ); } - public function test_a_linked_count_of_more_than_one_breaks_a_single_valued_attribute(): void - { - self::assertEquals( - [ - new RuleViolation( - SchemaRule::SingleValue, - ResultCode::CONSTRAINT_VIOLATION, - 'Attribute "employeeNumber" is single-valued but has more than one value.', - ), - ], - $this->checkWithLinkedCounts( - $this->inetOrgPersonWith(new Attribute( - 'description', - 'linked elsewhere', - )), - ['employeenumber' => 2], - ), - ); - } - - public function test_a_linked_count_stands_in_for_the_values_the_entry_carries(): void - { - self::assertSame( - [], - $this->checkWithLinkedCounts( - $this->inetOrgPersonWith(new Attribute( - 'employeeNumber', - '001', - '002', - )), - ['employeenumber' => 1], - ), - ); - } - - /** - * @param array $linkedCounts - * - * @return list - */ - private function checkWithLinkedCounts( - Entry $entry, - array $linkedCounts, - ): array { - return $this->subject->check( - SchemaWrite::modify( - $entry, - [], - false, - $linkedCounts, - ), - ObjectClassChain::forEntry( - $this->schema, - $entry, - ), - ); - } - /** * @return list */ diff --git a/tests/unit/Server/Backend/Storage/Adapter/InMemoryStorageTest.php b/tests/unit/Server/Backend/Storage/Adapter/InMemoryStorageTest.php index b5e9235d..5f2f0429 100644 --- a/tests/unit/Server/Backend/Storage/Adapter/InMemoryStorageTest.php +++ b/tests/unit/Server/Backend/Storage/Adapter/InMemoryStorageTest.php @@ -148,11 +148,17 @@ public function test_a_delta_removes_a_member_named_in_another_spelling(): void { $this->subject->store(EntryFixture::withUuid(Entry::fromArray( 'cn=admins,dc=ex,dc=com', - ['cn' => ['admins'], 'member' => ['cn=bob,dc=ex,dc=com', 'cn=carol,dc=ex,dc=com']], + [ + 'cn' => ['admins'], + 'member' => ['cn=bob,dc=ex,dc=com', 'cn=carol,dc=ex,dc=com'], + ], ))); $this->subject->store( - EntryFixture::withUuid(Entry::fromArray('cn=admins,dc=ex,dc=com', ['cn' => ['admins']])), + EntryFixture::withUuid(Entry::fromArray( + 'cn=admins,dc=ex,dc=com', + ['cn' => ['admins']], + )), links: new LinkDelta(removed: ['member' => ['CN=Bob, DC=ex, DC=com']]), ); diff --git a/tests/unit/Server/Backend/Write/Operation/Linked/LinkedChangeReplayTest.php b/tests/unit/Server/Backend/Write/Operation/Linked/LinkedChangeReplayTest.php index 6f952171..6405ad24 100644 --- a/tests/unit/Server/Backend/Write/Operation/Linked/LinkedChangeReplayTest.php +++ b/tests/unit/Server/Backend/Write/Operation/Linked/LinkedChangeReplayTest.php @@ -20,7 +20,6 @@ use FreeDSx\Ldap\Operation\ResultCode; use FreeDSx\Ldap\Server\Backend\Storage\Link\LinkDelta; use FreeDSx\Ldap\Server\Backend\Write\Operation\Linked\LinkedChangeReplay; -use FreeDSx\Ldap\Server\Backend\Write\Operation\Linked\SettledLinks; use PHPUnit\Framework\TestCase; use Tests\Support\FreeDSx\Ldap\ServerContainerTrait; use Tests\Support\FreeDSx\Ldap\Storage\EntryFixture; @@ -35,95 +34,40 @@ final class LinkedChangeReplayTest extends TestCase private const CAROL = 'cn=Carol,dc=example,dc=com'; - private const DAN = 'cn=Dan,dc=example,dc=com'; - - private const STAFF = 'cn=Staff,dc=example,dc=com'; - private LinkedChangeReplay $subject; protected function setUp(): void { - $storage = EntryFixture::inMemoryStorage( - Entry::fromArray( - self::ADMINS, - [ - 'objectClass' => ['groupOfNames'], - 'cn' => ['Admins'], - 'member' => [self::BOB], - ], - ), - Entry::fromArray( - self::STAFF, - [ - 'objectClass' => ['groupOfNames'], - 'cn' => ['Staff'], - 'member' => [self::BOB, self::CAROL, self::DAN], - ], - ), - ); + $storage = EntryFixture::inMemoryStorage(Entry::fromArray( + self::ADMINS, + [ + 'objectClass' => ['groupOfNames'], + 'cn' => ['Admins'], + 'member' => [self::BOB], + ], + )); $this->subject = $this->containerFor($storage)->get(LinkedChangeReplay::class); } - public function test_no_changes_leave_the_entry_the_whole_truth_for_its_links(): void + public function test_changes_applied_apart_load_none_of_the_links(): void { - $delta = $this->subject->settle( - [], - new Dn(self::ADMINS), - )->delta; - - self::assertTrue($delta->isEmpty()); - self::assertFalse($delta->isUntouched()); - } - - public function test_the_changes_are_kept_as_supplied(): void - { - $changes = [ - self::adding(self::CAROL), - self::deleting(self::CAROL), - ]; - self::assertSame( - $changes, - $this->settled(...$changes)->changes, + 0, + $this->subject->projectionFor([self::adding(self::CAROL)])->linkCap, ); } - public function test_adding_to_a_held_value_counts_more_than_one(): void + public function test_no_changes_applied_apart_load_every_link(): void { - self::assertSame( - ['member' => 2], - $this->settled(self::adding(self::CAROL))->counts, - ); - } - - public function test_deleting_the_only_held_value_counts_none(): void - { - self::assertSame( - ['member' => 0], - $this->settled(self::deleting(self::BOB))->counts, - ); + self::assertNull($this->subject->projectionFor([])->linkCap); } - public function test_replacing_the_only_held_value_counts_one(): void + public function test_no_changes_leave_the_entry_the_whole_truth_for_its_links(): void { - self::assertSame( - ['member' => 1], - $this->settled( - self::deleting(self::BOB), - self::adding(self::CAROL), - )->counts, - ); - } + $delta = $this->settle(); - public function test_a_count_stops_once_it_is_more_than_one(): void - { - self::assertSame( - ['member' => 2], - $this->subject->settle( - [self::deleting(self::BOB)], - new Dn(self::STAFF), - )->counts, - ); + self::assertTrue($delta->isEmpty()); + self::assertFalse($delta->isUntouched()); } public function test_a_delete_and_an_add_net_to_both(): void @@ -231,23 +175,6 @@ public function test_deleting_a_value_that_is_not_a_dn_is_answered_as_not_held() $this->settle(self::deleting('not a dn')); } - public function test_adding_a_value_that_is_not_a_dn_is_refused_for_its_syntax(): void - { - $this->expectOperationCode(ResultCode::INVALID_ATTRIBUTE_SYNTAX); - - $this->settle(self::adding('not a dn')); - } - - public function test_a_value_that_is_not_a_dn_added_then_deleted_is_never_held_to_its_syntax(): void - { - $delta = $this->settle( - self::adding('not a dn'), - self::deleting('not a dn'), - ); - - self::assertTrue($delta->isUntouched()); - } - private static function adding(string ...$members): Change { return Change::add( @@ -265,11 +192,6 @@ private static function deleting(string ...$members): Change } private function settle(Change ...$changes): LinkDelta - { - return $this->settled(...$changes)->delta; - } - - private function settled(Change ...$changes): SettledLinks { return $this->subject->settle( array_values($changes), diff --git a/tests/unit/Server/Backend/Write/Operation/UpdateEntryHandlerTest.php b/tests/unit/Server/Backend/Write/Operation/UpdateEntryHandlerTest.php index d8291b9d..f1e904d8 100644 --- a/tests/unit/Server/Backend/Write/Operation/UpdateEntryHandlerTest.php +++ b/tests/unit/Server/Backend/Write/Operation/UpdateEntryHandlerTest.php @@ -417,6 +417,27 @@ public function test_a_linked_value_the_object_classes_do_not_permit_is_refused( )); } + public function test_a_linked_value_that_is_not_a_dn_is_refused_for_its_syntax(): void + { + $this->validatedGraph(SchemaValidationMode::Strict); + $this->storage->store(EntryFixture::withUuid(Entry::fromArray( + self::ADMINS, + [ + 'objectClass' => ['groupOfNames'], + 'cn' => ['Admins'], + 'member' => [self::ALICE], + ], + ))); + + self::expectException(OperationException::class); + self::expectExceptionCode(ResultCode::INVALID_ATTRIBUTE_SYNTAX); + + $this->modifyAdmins(Change::add( + 'member', + 'not a dn', + )); + } + public function test_a_linked_value_the_object_classes_do_not_permit_is_refused_when_a_control_reads_the_entry(): void { $this->validatedGraph(SchemaValidationMode::Strict);