RotherOSS / otobo

OTOBO is one of the most flexible web-based ticketing systems used for Customer Service, Help Desk, IT Service Management. https://otobo.io/
GNU General Public License v3.0
251 stars 72 forks source link

Error in dynamic field when changing the case of company ID #1872

Open niccord opened 2 years ago

niccord commented 2 years ago

Hi everyone! If you create a customer company with some dynamic field value, then you update only the case of the company ID, you generate an error since the database returns a duplicate error message.

Step to reproduce:

  1. add a customer company dynamic field;
  2. add the right config in your Kernel/Config.pm in order to map your dynamic field properly;
  3. create a customer with ID and name "mycompany" and give a value to your dynamic field;
  4. update the customer renaming it to "MYCOMPANY"

It will be also not possible to create a new value for the dynamic field with a new update since the row is already in the database.

image

Message in the error log:

[Fri Aug 12 17:08:32 2022] plackup: DBD::mysql::db do failed: Duplicate entry 'MYCOMPANY-CustomerCompany' for key 'dynamic_field_object_name' at /opt/otobo/bin/psgi-bin/../../Kernel/System/DB.pm line 556.
ERROR: OTOBO-CGI-10 Perl: 5.32.0 OS: linux Time: Fri Aug 12 17:08:32 2022

 Message: Duplicate entry 'MYCOMPANY-CustomerCompany' for key 'dynamic_field_object_name', SQL: '
            INSERT INTO dynamic_field_obj_id_name
                (object_name, object_type)
            VALUES
                (?, ?)'

 RemoteAddress: 172.18.0.1
 RequestURI: /otobo/index.pl

 Traceback (11): 
   Module: Kernel::System::DynamicField::ObjectMappingCreate Line: 1385
   Module: Kernel::System::DynamicField::Backend::ValueSet Line: 490
   Module: Kernel::Modules::AdminCustomerCompany::Run Line: 224
   Module: Kernel::System::Web::InterfaceAgent::Content Line: 1239
   Module: Kernel::System::Web::InterfaceAgent::Response Line: 1321
   Module: Kernel::System::Web::App::call Line: 86
   Module: Plack::Component::__ANON__ Line: 50
   Module: Plack::App::URLMap::call Line: 71
   Module: Plack::Component::__ANON__ Line: 50
   Module: Plack::Sandbox::_2fopt_2fotobo_2fbin_2fpsgi_2dbin_2fotobo_2epsgi::__ANON__ Line: 202
   Module: Plack::Sandbox::_2fopt_2fotobo_2fbin_2fpsgi_2dbin_2fotobo_2epsgi::__ANON__ Line: 172
   Module: Plack::Sandbox::_2fopt_2fotobo_2fbin_2fpsgi_2dbin_2fotobo_2epsgi::__ANON__ Line: 158
   Module: Plack::Middleware::HTTPExceptions::try {...}  Line: 20

ERROR: OTOBO-CGI-10 Perl: 5.32.0 OS: linux Time: Fri Aug 12 17:08:32 2022

 Message: Unable to create object mapping for object name MYCOMPANY and type CustomerCompany!

 RemoteAddress: 172.18.0.1
 RequestURI: /otobo/index.pl

 Traceback (11): 
   Module: Kernel::System::DynamicField::Backend::ValueSet Line: 493
   Module: Kernel::Modules::AdminCustomerCompany::Run Line: 224
   Module: Kernel::System::Web::InterfaceAgent::Content Line: 1239
   Module: Kernel::System::Web::InterfaceAgent::Response Line: 1321
   Module: Kernel::System::Web::App::call Line: 86
   Module: Plack::Component::__ANON__ Line: 50
   Module: Plack::App::URLMap::call Line: 71
   Module: Plack::Component::__ANON__ Line: 50
   Module: Plack::Sandbox::_2fopt_2fotobo_2fbin_2fpsgi_2dbin_2fotobo_2epsgi::__ANON__ Line: 202
   Module: Plack::Sandbox::_2fopt_2fotobo_2fbin_2fpsgi_2dbin_2fotobo_2epsgi::__ANON__ Line: 172
   Module: Plack::Sandbox::_2fopt_2fotobo_2fbin_2fpsgi_2dbin_2fotobo_2epsgi::__ANON__ Line: 158
   Module: Plack::Middleware::HTTPExceptions::try {...}  Line: 20
stefanhaerter commented 2 years ago

Hi niccord,

I think this is due to this line in the event updating the object name:

https://github.com/RotherOSS/otobo/blob/8e5fe78e8f4f17ea183068f9fa85b4df72bfe0cf/Kernel/System/CustomerCompany/Event/DynamicFieldObjectNameUpdate.pm#L62

Could you test if the problem persists if you change the name to a value which isn't equivalent in lowercase, e.g. MYCOMPANY2? Nonetheless, I think this needs to be fixed.

bschmalhofer commented 2 years ago

Hi Nicola,

this seems to be a database design problem in OTOBO. I tested this with a fresh installation in Docker,

MariaDB [otobo]> select * from dynamic_field_obj_id_name -> ; Empty set (0.001 sec)

MariaDB [otobo]> insert into dynamic_field_obj_id_name (object_name, object_type) VALUE ( 'NAME1', 'TYPE1'); Query OK, 1 row affected (0.004 sec)

MariaDB [otobo]> insert into dynamic_field_obj_id_name (object_name, object_type) VALUE ( 'name1', 'TYPE1'); ERROR 1062 (23000): Duplicate entry 'name1-TYPE1' for key 'dynamic_field_object_name' MariaDB [otobo]>

The collation utf8mb4_unicode_ci of the table _dynamic_field_obj_idname is case insensitive:

dynamic_field_obj_id_name | CREATE TABLE dynamic_field_obj_id_name ( object_id int(11) NOT NULL AUTO_INCREMENT, object_name varchar(191) COLLATE utf8mb4_unicode_ci NOT NULL, object_type varchar(100) COLLATE utf8mb4_unicode_ci NOT NULL, PRIMARY KEY (object_id), UNIQUE KEY dynamic_field_object_name (object_name,object_type) ) ENGINE=InnoDB AUTO_INCREMENT=34 DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci

Thus string searches are case insensitive: https://dev.mysql.com/doc/refman/8.0/en/case-sensitivity.html . This includes the unique index.

I also wondered why, if searching is case insensitive, ObjectMappingGet() did not find the previous entry. Apparently this is because of line 1013 in Kernel/System/DynamicField/Backend.pm which does a case sensitive check.

    if ( !IsHashRefWithData($ObjectIDs) || !$ObjectIDs->{ $Param{ObjectName} } ) {
        $Kernel::OM->Get('Kernel::System::Log')->Log(
            Priority => 'debug',
            Message  =>
                "Unable to fetch object mapping for object name $Param{ObjectName} and type $Param{DynamicFieldConfig}->{ObjectType}!"
        );

        return;
    }

We need to discuss internally how to deal with this effect.

niccord commented 2 years ago

Applying the change @stefanhaerter made in his branch, I can confirm the error is fixed. I don't know if it's the path you want to follow to resolve the issue, though.

bschmalhofer commented 2 years ago

Hi Nicola,

I discussed this with @stefanhaerter and we ended up with the least invasive solution, that does not really change the current approach. We are aware that the current approach is more confusing than really necessary. Setting ChangeSensitive = 1 for the customer_company in Kernel/Config.pm does not indicate that searches should be case sensitive, it only says that the current collation of the database table should be accepted. In the MySQL case the default collation is case insensitive.

Best regards, Bernhard

niccord commented 2 years ago

I was referring to removing the lc parts in the DynamicFieldObjectNameUpdate class. I kept the case-sensitive part as it is because it sounds a little bit scary since I don't really know which side effects can cause.

svenoe commented 2 years ago

We will not instantly incorporate this. To my understanding there are configurations where we need the lc for this to skip things as intended. It needs a bit more discussion.