Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Schema:create fails with single collection inheritance #2392

Merged
merged 3 commits into from
Dec 27, 2021

Conversation

JJarrie
Copy link
Contributor

@JJarrie JJarrie commented Nov 23, 2021

Q A
Type improvement
BC Break no
Fixed issues #2182

Summary

Maintain in memory already processed collection in case of single collection inheritance and skip next procession of these collections.

@malarzm malarzm added the Bug label Nov 23, 2021
@malarzm malarzm added this to the 2.2.3 milestone Nov 23, 2021
Copy link
Member

@malarzm malarzm left a comment

Choose a reason for hiding this comment

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

Thanks @JJarrie for the PR! 🎉 The solution you've provided is simple, we're requiring PRs to have tests not only to prove the solution is working, but to prevent regressions in the future. Could you please write one? Don't hesitate to ask should you need any help :)

@franmomu franmomu modified the milestones: 2.2.3, 2.3.1 Nov 30, 2021
@JJarrie
Copy link
Contributor Author

JJarrie commented Nov 30, 2021

Thanks @JJarrie for the PR! tada The solution you've provided is simple, we're requiring PRs to have tests not only to prove the solution is working, but to prevent regressions in the future. Could you please write one? Don't hesitate to ask should you need any help :)

Hello, I need some help. I tried to add the same structure as the one who raised the error in the issue ticket.

<?php

declare(strict_types=1);

namespace Documents\Functional;

use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM;

/**
 * @ODM\MappedSuperclass
 * @ODM\InheritanceType("SINGLE_COLLECTION")
 * @ODM\DiscriminatorField("@type")
 * @ODM\DiscriminatorMap(SingleInheritanceCollectionSuperClass::TYPES)
 */
class SingleInheritanceCollectionSuperClass
{
    public const TYPES = [
        'subType1' => SingleInheritanceCollectionSubType1::class,
        'subType2' => SingleInheritanceCollectionSubType2::class,
    ];

    /** @ODM\Id */
    private $id;

    /** @ODM\Field(type="string") */
    private $name;
}
<?php

declare(strict_types=1);

namespace Documents\Functional;

use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM;

/**
 * @ODM\Document
 */
class SingleInheritanceCollectionSubType1 extends SingleInheritanceCollectionSuperClass
{
    /** @ODM\Field(type="string") */
    private $someFieldA;
}
<?php

declare(strict_types=1);

namespace Documents\Functional;

use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM;

/**
 * @ODM\Document
 */
class SingleInheritanceCollectionSubType2 extends SingleInheritanceCollectionSuperClass
{
    /** @ODM\Field(type="string") */
    private $someFieldB;
}

And as the database is mocked in SchemaManagerTest::testCreateCollections, the original error (coming from database) is not raised. And I have some difficulties to understand how to handle my case into the functionnal suite. If someone have an idea or an explanation, I take it.

:)

@IonBazan
Copy link
Member

IonBazan commented Dec 2, 2021

@JJarrie I've added a test which makes sure one of our existing documents using single collection inheritance trigger the collection creation only once.

@IonBazan IonBazan requested a review from malarzm December 6, 2021 14:34
@JJarrie
Copy link
Contributor Author

JJarrie commented Dec 6, 2021

@JJarrie I've added a test which makes sure one of our existing documents using single collection inheritance trigger the collection creation only once.

@IonBazan thank you for your help.

@@ -470,6 +475,7 @@ public function testCreateCollections(array $expectedWriteOptions, ?int $maxTime
}

$this->schemaManager->createCollections($maxTimeMs, $writeConcern);
self::assertSame(1, array_count_values($createdCollections)['Tournament']);
Copy link
Member

Choose a reason for hiding this comment

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

To prevent this test from becoming obsolete there should be an assertion in the beginning that Tournament has inheritance and is using single collection. Otherwise the change in existing entities can render this test useless

@JJarrie JJarrie requested a review from malarzm December 9, 2021 15:38
@malarzm malarzm changed the base branch from 2.2.x to 2.3.x December 27, 2021 23:04
@malarzm malarzm merged commit a935640 into doctrine:2.3.x Dec 27, 2021
@malarzm
Copy link
Member

malarzm commented Dec 27, 2021

Thanks @JJarrie!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants