search

doctrine-extensions / DoctrineExtensions

Doctrine2 behavioral extensions, Translatable, Sluggable, Tree-NestedSet, Timestampable, Loggable, Sortable
MIT License
4.03k stars 1.27k forks source link

ReferenceIntegrity extension broken? #2319

Open mbabker opened 2 years ago

mbabker commented 2 years ago

ReferenceIntegrityMappingTest is currently disabled because it relies on the removed YAML driver from the MongoDB ODM. OK, I made an attempt at restoring the test using the annotation driver instead (see https://github.com/doctrine-extensions/DoctrineExtensions/compare/main...mbabker:restore-odm-test for the diff, pretty simple in and of itself).

Except, this exposes a problem. The annotation and YAML drivers built the mapping configuration inconsistently.

// Annotation driver config structure
$config = [
    'referenceIntegrity' => [
        'referencedDocuments' => 'nullify',
    ],
];

// YAML driver config structure
$config = [
    'referenceIntegrity' => [
        'referencedDocuments' => [
            'referencer' => 'nullify',
        ],
    ],
];

Essentially the Annotation driver doesn't do an array for the reference integrity configuration for each field, whereas the YAML driver did. This test used the YAML driver's configuration structure.

Changing the annotation driver to use the same structure as the YAML driver breaks ReferenceIntegrityDocumentTest because ReferenceIntegrityListener relies on the current config structure of the annotation driver, meaning the extension has never worked with the YAML driver.

I don't know what right's supposed to look like and I don't use the ODM to use this feature (I just happen to have gone through the effort to get MongoDB running locally for another OSS package and can run the MongoDB tests here as a result), but I can work out that with the inconsistent configuration it's impossible for me to tell what the correct configuration is and how to get to a green build.

franmomu commented 2 years ago

I haven't used this extension either. Since the YAML driver does not exist anymore, I guess I would change ReferenceIntegrityMappingTest to use annotations.

github-actions[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.