Audit field type registry bug

[andriusvo]

Summary
In class AuditFieldTypeRegistry method validateAuditType works incorrectly.

Method validateAuditType checks whether provided audit type exists and if it's exist -> exception is raised:

        if (!empty(static::$auditTypes[$auditType])) {
            throw new \LogicException(sprintf('Unknown audit type %s.', $auditType));
        }

Should be:

        if (empty(static::$auditTypes[$auditType])) {
            throw new \LogicException(sprintf('Unknown audit type %s.', $auditType));
        }

Steps to reproduce
Use method addType to add new type to auditable fields.

Actual Result
Exception Unknown audit type is raised

Expected Result
New audit type should be added

Details about your environment

• OroPlatform version: 4.2.6
• PHP version: 8.0

[anyt]

Hi @andriusvo,
You can extend the Oro\Bundle\DataAuditBundle\Model\AuditFieldTypeRegistry to override the maps as they are stored in protected properties

[andriusvo]

@anyt yes, but this is a bug and in any case should be fixed ;)

[anyt]

The data audit feature in the oro/platform package supports the limited list of field types. If you add an unsupported type - the validation triggers an error, that is, as I understand, expected behavior.

[andriusvo]

@anyt Please check if statement one more time. In Oro code this if statement says that: If provided $auditType is not empty (== is found) - thrown an exception) And there should be exactly the opposite - `If provided $auditType is empty (== found) - thrown an exception.

[anyt]

I see there is a unit test for this method
\Oro\Bundle\DataAuditBundle\Tests\Unit\Model\AuditFieldTypeRegistryTest::testAuditFieldTypeRegistry.
Could you, please, change it to reproduce the bug?

[anyt]

It seems the validation message is missliding. According to the test, it throws an exception if you add the type that is already in the map.

[andriusvo]

Ok, let me give a real example for you. In AuditFieldTypeRegistry we have $typesMap where doctrine type is mapped to audit type. And now, f.e. I want to add integer_array (Doctrine type) to as simplearray (audit type). How I will achieve this? By writing this code:

        if (false === AuditFieldTypeRegistry::hasType(IntegerArrayType::TYPE)) {
            AuditFieldTypeRegistry::addType(IntegerArrayType::TYPE, 'simplearray');
        }

Now, method validateAuditType will be called and will be checked if audit type simplearray exists, and if exists -> the exception is thrown <---- Here is a bug! Should be the opposite, if the audit type is not found, only then the exception should be thrown. We are not checking duplicates of mapping here, we are checking only if this audit type exists. So in line 209 the exclamation mark should be removed, because it acts wrongly.